Skip to content

feat[66]: Add advanced CEL validation#210

Merged
AndresNico merged 4 commits intomainfrom
feat/add-advanced-cel-validation
Mar 16, 2026
Merged

feat[66]: Add advanced CEL validation#210
AndresNico merged 4 commits intomainfrom
feat/add-advanced-cel-validation

Conversation

@AndresNico
Copy link
Copy Markdown
Collaborator

@AndresNico AndresNico commented Feb 24, 2026

Closes: #74, Closes: #75, Closes: #76, Closes: #77, Closes: #78, Closes: #79
Part Of: #66

Advanced CEL validation

  • adds advanced CEL validation logic to the CRDs mentioned in the issued while keeping existing rules in
  • also added the logic of an empty spec.forProvider if managementPolicy is observe only to the OrgMember, SpaceMember and ServiceInstance for consistency purposes
  • issues with some rules as described below

Reference Fields Issue

  • affects OrgReference, SpaceReference, AppReference, ServiceInstanceReference
  • all have three fields to set e.g. org, orgRef and orgSelector
    • When setting orgSelector, the controller populates orgRef and org
    • When setting orgRef, the controller populates org
    • When setting org, none of the others are populated
  • hence we can not enforce that exactly one of the fields is set but only that at least one of them is set

Resources modified:

Domain #79

  • ✅ If spec.managementPolicies is Observe only, then spec.forProvider may be empty, regardless of whether the fields are required.
  • ❌ Either forProvider.name is provided or both forProvider.domain and forProvider.sub_domain is set.

When forProvider.Domain and forProvider.subDomain is set, the controller populates forProvider.name. It is not possible to enforce that either one or the others are set only that at least one or the others are set.

Space #78

  • ✅ If spec.managementPolicies is Observe only, then spec.forProvider may be empty, regardless of whether the fields are required.
  • ❌ OrgReference is required and exact one of forProvider.orgName, forProvider.orgRef, or forProvider.orgSelector can be set.

See Reference Fields

ServiceCredentialBinding #77

  • ✅ If spec.managementPolicies is Observe only, then spec.forProvider may be empty, regardless of whether the fields are required.
  • ❌ Exact one of forProvider.serviceInstance, forProvider.serviceInstanceRef, or forProvider.serviceInstanceSelector is set.

See Reference Fields

  • ✅ If forProvider.type is key, forProvider.name must be set
  • ❌ if forProvider.type is app, one of forProvider.app, forProvider.appRef, or forProvider.appSelector must be set

See Reference Fields

  • ✅ Either forProvider.parameters or forProvider.parametersSecretRef may be set, but not both.

OrgRole & SpaceRole #76

  • ✅ If spec.managementPolicies is Observe only, then spec.forProvider may be empty, regardless of whether the fields are required.
  • ❌ Exact one of forProvider.orgName, forProvider.orgRef, or forProvider.orgSelector can be set. If one is set, the others must be unset.

See Reference Fields

  • ❌ Exact one of forProvider.spaceName, forProvider.spaceRef, or forProvider.spaceSelector can be set, if one is set, the others must be unset

See Reference Fields

OrgMembers & SpaceMembers #75

  • ✅ spec.forProviders.members must not be a empty list
  • ❌ Only one of forProvider.orgName, forProvider.orgRef, or forProvider.orgSelector can be set. If one is set, the others must be unset.

See Reference Fields

  • ❌ Only of forProvider.spaceName, forProvider.spaceRef, or forProvider.spaceSelector can be set, if one is set, the others must be unset

See Reference Fields

ServiceInstance #74

  • ✅ If type is managed, then servicePlan is required.
  • ❌ In servicePlan, either both offering and plan are set or id is set

Basically the same as domain. Controller populates id if offering and plan is set. Can only make sure that at least one or the other is set

  • ✅ If type is user-provided, service credential must be provided using one of the fields credentials, jsonCredentials, or credentialSecretRef.
  • ✅ At most one of parameters, jsonParams, or paramSecretRef may be set.
  • ✅ At most one of credentials, jsonCredentials, or credentialSecretRef may be set.

Testing

Tested the changes locally to ensure the expected evaluation behavior

@AndresNico AndresNico force-pushed the feat/add-advanced-cel-validation branch from 78cf6ed to b4a9518 Compare March 16, 2026 07:57
@AndresNico AndresNico temporarily deployed to pr-e2e-no-approval March 16, 2026 07:57 — with GitHub Actions Inactive
@AndresNico AndresNico requested a review from SatabdiG March 16, 2026 09:21
Copy link
Copy Markdown
Collaborator

@SatabdiG SatabdiG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work. Some comments from my side.

Comment thread apis/resources/v1alpha1/domain_types.go Outdated

// (String) The username of the Cloud Foundry user to assign the role to.
// +kubebuilder:validation:Required
Username string `json:"username,omitempty" tf:"username,omitempty"`
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: Both username and type have omit empty. If someone enters an empty Type, CEL would fire with Type required, the field itself gives no indication that it's required.

Maybe we can add +kubebuilder:validation:Optional, just to make the intent clear?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added that to both username and type

Comment thread apis/resources/v1alpha1/serviceinstance_types.go
Copy link
Copy Markdown
Collaborator

@SatabdiG SatabdiG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@AndresNico AndresNico temporarily deployed to pr-e2e-no-approval March 16, 2026 13:16 — with GitHub Actions Inactive
@AndresNico AndresNico temporarily deployed to pr-e2e-no-approval March 16, 2026 13:31 — with GitHub Actions Inactive
@AndresNico AndresNico temporarily deployed to pr-e2e-no-approval March 16, 2026 13:49 — with GitHub Actions Inactive
@AndresNico AndresNico enabled auto-merge (squash) March 16, 2026 13:49
@AndresNico AndresNico merged commit 806db90 into main Mar 16, 2026
12 checks passed
@AndresNico AndresNico deleted the feat/add-advanced-cel-validation branch March 16, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment