Skip to content

OCPBUGS-77827 fix(api): add missing has() check for servingCerts in validation #7901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
eponafyrefly wants to merge 1 commit into from
Closed

OCPBUGS-77827 fix(api): add missing has() check for servingCerts in validation #7901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/hypershift/v1beta1/hostedcluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ type Capabilities struct {
// +kubebuilder:validation:XValidation:rule=`self.platform.type == "Azure" ? self.services.exists(s, s.service == "Konnectivity" && s.servicePublishingStrategy.type == "Route") : true`,message="Azure platform requires Konnectivity to use Route service publishing strategy"
// +kubebuilder:validation:XValidation:rule=`self.platform.type == "Azure" ? self.services.exists(s, s.service == "Ignition" && s.servicePublishingStrategy.type == "Route") : true`,message="Azure platform requires Ignition to use Route service publishing strategy"
// +kubebuilder:validation:XValidation:rule=`has(self.issuerURL) || !has(self.serviceAccountSigningKey)`,message="If serviceAccountSigningKey is set, issuerURL must be set"
// +kubebuilder:validation:XValidation:rule=`!self.services.exists(s, s.service == 'APIServer' && has(s.servicePublishingStrategy.loadBalancer) && s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration) && has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert, cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))`, message="APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]"
// +kubebuilder:validation:XValidation:rule=`!self.services.exists(s, s.service == 'APIServer' && has(s.servicePublishingStrategy.loadBalancer) && s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration) && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert, cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))`, message="APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]"
// +kubebuilder:validation:XValidation:rule="!has(self.operatorConfiguration) || !has(self.operatorConfiguration.clusterNetworkOperator) || !has(self.operatorConfiguration.clusterNetworkOperator.disableMultiNetwork) || !self.operatorConfiguration.clusterNetworkOperator.disableMultiNetwork || self.networking.networkType == 'Other'",message="disableMultiNetwork can only be set to true when networkType is 'Other'"
// +kubebuilder:validation:XValidation:rule="self.networking.networkType == 'OVNKubernetes' || !has(self.operatorConfiguration) || !has(self.operatorConfiguration.clusterNetworkOperator) || !has(self.operatorConfiguration.clusterNetworkOperator.ovnKubernetesConfig)", message="ovnKubernetesConfig is forbidden when networkType is not OVNKubernetes"
type HostedClusterSpec struct {
Expand Down
3 changes: 2 additions & 1 deletion ...erated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6001,7 +6001,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
Comment on lines +6004 to 6006

@coderabbitai coderabbitai Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's check if the file exists and read the relevant section
head -c 500000 api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/AAA_ungated.yaml | tail -c 50000 | tail -n 100

Repository: openshift/hypershift

Length of output: 5603

🏁 Script executed:

# Get the exact lines mentioned in the review
sed -n '5990,6020p' api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/AAA_ungated.yaml

Repository: openshift/hypershift

Length of output: 2471

🏁 Script executed:

# Search for the schema definition of servingCerts to see if namedCertificates is required
sed -n '1,6000p' api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/AAA_ungated.yaml | grep -A 50 "servingCerts" | head -100

Repository: openshift/hypershift

Length of output: 3530

🌐 Web query:

Kubernetes CEL optional fields has() guard namedCertificates exists() semantics

💡 Result:

has() (presence guard) in Kubernetes CEL

  • has(e.f) is a CEL macro used to test presence / availability of a field before you rely on it. Kubernetes explicitly calls out has() as the way to check whether a field of a dynamically typed value is accessible (to avoid errors when the field might not exist in the schema for the object you’re evaluating). (kubernetes.io)
  • Precise CEL-spec semantics depend on the underlying type of e (github.com):

Implication for namedCertificates (a list field):
has(self.namedCertificates) is true only when the list is non-empty; it’s false for both “missing” and “present but empty”. (github.com)

exists() semantics (including error/short-circuit behavior)

  • list.exists(x, p) returns true if any element satisfies predicate p; for an empty list it returns false. (github.com)
  • exists() “behaves like logical OR (||)” regarding short-circuiting and error absorption: if some element makes the result true, errors from other elements/predicates can be ignored (consistent with CEL’s OR behavior). (github.com)

So for namedCertificates:

  • self.namedCertificates.exists(c, /*match*/) is false when the list is empty (and typically also when it’s absent but treated as empty by the underlying object model). (github.com)
  • If you need “only enforce when configured”, a common pattern is:
    • !has(self.namedCertificates) || self.namedCertificates.exists(c, /*match*/) (skip rule unless non-empty), because has() on a list means non-empty. (github.com)

Citations:

Guard namedCertificates and cert.names before dereferencing.

Lines 6005–6006 access two optional fields without has() guards: servingCerts.namedCertificates and cert.names. Both are optional in the schema—namedCertificates states "If no named certificates are provided..." and names is marked "optional list". Per Kubernetes CEL best practices, optional fields must be guarded with has() before dereferencing to prevent errors when fields are absent or empty.

Suggested fix 
-                && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
-                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
-                cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
+                && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
+                && has(self.configuration.apiServer.servingCerts.namedCertificates)
+                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
+                has(cert.names) && cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& has(self.configuration.apiServer.servingCerts.namedCertificates)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
has(cert.names) && cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/AAA_ungated.yaml`
around lines 6004 - 6006, The CEL expression dereferences optional fields
without guards—specifically
self.configuration.apiServer.servingCerts.namedCertificates and cert.names—so
update the predicate to wrap those accesses with has() checks (e.g.,
has(self.configuration.apiServer.servingCerts.namedCertificates) and
has(cert.names)) before calling .exists; adjust the inner exists() lambdas (the
one iterating namedCertificates and the cert.names.exists(...) call) to only run
when those has() checks pass so you don't access missing fields in the
namedCertificates and cert.names references.

- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ....featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/AutoNodeKarpenter.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6028,7 +6028,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...egated-crd-manifests/hostedclusters.hypershift.openshift.io/ClusterUpdateAcceptRisks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5984,7 +5984,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...manifests/hostedclusters.hypershift.openshift.io/ClusterVersionOperatorConfiguration.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6004,7 +6004,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...rated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDC.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6352,7 +6352,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
Comment on lines +6355 to +6356

@coderabbitai coderabbitai Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Kubernetes CRD CEL validation rules: when should optional object/list fields be guarded with has(self.field), and are null or absent fields treated as absent during evaluation?

💡 Result:

When to guard optional object/list fields with has(self.field)

Guard any optional field when your rule would otherwise dereference it (access subfields) or call methods/macros on it (for lists: .size(), .all(), .exists(), indexing, etc.). The canonical pattern is to use short-circuiting:

  • Optional list: !has(self.items) || self.items.all(x, /* ... */)
  • Optional object: !has(self.cfg) || self.cfg.mode in ['A','B']

Kubernetes’ own examples use this style for optional fields (including optional lists) in CRD validation/transition rules. (kubernetes.io)

Are null and “absent” treated the same?

No.

  • Absent field: has(self.field) is false (field is unset). (kubernetes.io)
  • Field present with value null (only possible if the schema allows null): CEL sees a real null_type value. (kubernetes.io)
  • In CEL, null counts as “present” rather than “absent” (so has(self.field) can be true even if the value is null). (github.com)

So, if your schema permits null and you want “null behaves like unset”, explicitly guard both:

!has(self.field) || self.field == null || /* safe use of self.field */

(or equivalently: has(self.field) && self.field != null && ... when it must be provided).

Citations:

🏁 Script executed:

# First, check if the file exists and examine the context around lines 6355-6356
head -n 6360 api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDC.yaml | tail -n 20

Repository: openshift/hypershift

Length of output: 1690

🏁 Script executed:

# Find the schema definition for namedCertificates to confirm it's optional
rg "namedCertificates" api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDC.yaml -B 5 -A 5

Repository: openshift/hypershift

Length of output: 2361

🏁 Script executed:

# Check if namedCertificates is in any required array
rg "required:" api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDC.yaml -A 20 | grep -i "namedCertificates\|servingCerts"

Repository: openshift/hypershift

Length of output: 46

🏁 Script executed:

# Find the schema definition for servingCerts and check its required array
rg "servingCerts:" api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDC.yaml -A 50 | head -n 70

Repository: openshift/hypershift

Length of output: 3530

🏁 Script executed:

# Search more broadly for the required array that contains servingCerts properties
rg -A 100 "namedCertificates:" api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDC.yaml | grep -A 30 "type:"

Repository: openshift/hypershift

Length of output: 5122

Guard namedCertificates with has() before calling .exists().

Line 6356 calls .exists() on self.configuration.apiServer.servingCerts.namedCertificates without first checking that the field exists. namedCertificates is an optional list field (not required), so a resource with servingCerts: {} will still hit the same validation failure this patch intended to fix. Kubernetes CEL validation rules require optional fields to be guarded with has(self.field) before dereferencing or calling methods on them.

Suggested fix 
                && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
+               && has(self.configuration.apiServer.servingCerts.namedCertificates)
                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& has(self.configuration.apiServer.servingCerts.namedCertificates)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDC.yaml`
around lines 6355 - 6356, The CEL expression calls .exists() on
self.configuration.apiServer.servingCerts.namedCertificates without first
checking the field exists; update the boolean guard by adding
has(self.configuration.apiServer.servingCerts.namedCertificates) before calling
.exists() so the expression becomes ... && has(self.configuration.apiServer) &&
has(self.configuration.apiServer.servingCerts) &&
has(self.configuration.apiServer.servingCerts.namedCertificates) &&
self.configuration.apiServer.servingCerts.namedCertificates.exists(...),
ensuring namedCertificates is present before dereferencing in the validation
rule.

cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...ests/hostedclusters.hypershift.openshift.io/ExternalOIDCWithUIDAndExtraClaimMappings.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6506,7 +6506,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDCWithUpstreamParity.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6447,7 +6447,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...erated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/GCPPlatform.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6364,7 +6364,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
Comment on lines 6365 to 6369

@coderabbitai coderabbitai Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard the other optional fields in this CEL rule too.

This only fixes the servingCerts dereference. Line 6366 still reads the optional loadBalancer.hostname, and Lines 6368-6369 still read optional namedCertificates and cert.names. Valid specs such as servingCerts: {} or named certificates that rely on implicit names will still fail validation here instead of evaluating to false.

Suggested fix 
-              rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
-                && s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
-                && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
-                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
-                cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
+              rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
+                && has(s.servicePublishingStrategy.loadBalancer.hostname)
+                && s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
+                && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
+                && has(self.configuration.apiServer.servingCerts.namedCertificates)
+                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
+                has(cert.names) && cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& has(s.servicePublishingStrategy.loadBalancer.hostname)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& has(self.configuration.apiServer.servingCerts.namedCertificates)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
has(cert.names) && cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/GCPPlatform.yaml`
around lines 6365 - 6369, The CEL validation rule currently dereferences
optional fields without guarding them; update the rule string so each optional
access is protected: add a
has(s.servicePublishingStrategy.loadBalancer.hostname) (or check hostname != ""
only after confirming has(loadBalancer) and has(hostname)), add
has(self.configuration.apiServer.servingCerts.namedCertificates) before calling
namedCertificates.exists(...), and guard cert.names with has(cert.names) (or
ensure cert.names exists/non-empty) before calling cert.names.exists; keep the
existing checks (e.g., has(self.configuration) and
has(self.configuration.apiServer) and
has(self.configuration.apiServer.servingCerts)) and apply these additional
has(...) checks around s.servicePublishingStrategy.loadBalancer.hostname,
self.configuration.apiServer.servingCerts.namedCertificates, and cert.names in
the CEL rule.

- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...fests/hostedclusters.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6006,7 +6006,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
Comment on lines +6009 to +6010

@coderabbitai coderabbitai Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's check the file exists and read the context around lines 6009-6010
file_path="api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml"

# Check file size first
wc -l "$file_path"

Repository: openshift/hypershift

Length of output: 215

🏁 Script executed:

# Read the context around lines 6009-6010 (lines 6000-6020 for context)
file_path="api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml"
sed -n '6000,6020p' "$file_path"

Repository: openshift/hypershift

Length of output: 1977

🏁 Script executed:

# Let's also look at the broader context to find this validation rule
file_path="api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml"
# Look at more context - the rule definition should be visible
sed -n '5995,6025p' "$file_path"

Repository: openshift/hypershift

Length of output: 2471

🏁 Script executed:

# Search for the schema definition of namedCertificates to confirm if it's optional
# First, let's find where the CRD schema is defined
rg -A 5 -B 5 "namedCertificates" api/hypershift/v1beta1/ --type yaml | head -100

Repository: openshift/hypershift

Length of output: 19415

🏁 Script executed:

# Let's also look for the servingCerts definition to understand the structure
rg -A 10 "servingCerts:" api/hypershift/v1beta1/ --type yaml | head -80

Repository: openshift/hypershift

Length of output: 15269

🌐 Web query:

Kubernetes CEL optional fields has() macro .exists() method documentation

💡 Result:

  • Kubernetes CEL (how to use has() for optional / dynamically-typed fields, and example using exists): Kubernetes “Common Expression Language in Kubernetes” reference. (kubernetes.io)

    • Shows has(self.expired) for checking presence before access, and an exists example like self.widgets.exists(w, ...). (kubernetes.io)

  • CEL language spec (what has() and exists() actually are): CEL spec “Language Definition → Macros”. (github.com)

Citations:

🌐 Web query:

CEL validation rule optional field dereference Kubernetes CRD schema

💡 Result:

In Kubernetes CRD CEL rules (x-kubernetes-validations), you must guard any dereference of an optional / possibly-absent field using the has() macro (and rely on && / || short-circuiting), otherwise the expression can fail when that field is unset.

Pattern (recommended):

!has(self.spec.optionalField) || self.spec.optionalField.subField == "x"

or

has(self.spec.optionalField) && self.spec.optionalField.subField == "x"

Kubernetes’ CEL docs explicitly show this style (for example: has(self.expired) && ...) and list has as a standard macro available for CRD CEL. [1] The Kubernetes blog post on CRD immutability also demonstrates guarding optional fields with has() (for example: !has(oldSelf.value) || has(self.value)). [2]

If you’re on Kubernetes v1.29+, Kubernetes also supports CEL optional types, but has()-guarding remains the portable approach across versions. [1]

Sources
[1] Kubernetes docs: “Common Expression Language (CEL) in Kubernetes” https://kubernetes.io/docs/reference/using-api/cel/
[2] Kubernetes blog: “Enforce CRD Immutability with CEL Transition Rules” https://kubernetes.io/blog/2022/09/29/enforce-immutability-using-cel/

Guard namedCertificates before calling .exists().

servingCerts is now protected, but namedCertificates is optional in the schema. If servingCerts is present while namedCertificates is omitted, the rule will attempt to dereference an absent field. Kubernetes CEL requires guarding optional field access with has() before dereferencing. Add has(self.configuration.apiServer.servingCerts.namedCertificates) before calling .exists().

Suggested fix 
-                && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
-                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
+                && has(self.configuration.apiServer)
+                && has(self.configuration.apiServer.servingCerts)
+                && has(self.configuration.apiServer.servingCerts.namedCertificates)
+                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer)
&& has(self.configuration.apiServer.servingCerts)
&& has(self.configuration.apiServer.servingCerts.namedCertificates)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml`
around lines 6009 - 6010, The CEL expression dereferences an optional field:
before calling
self.configuration.apiServer.servingCerts.namedCertificates.exists(cert, ...)
you must guard namedCertificates with
has(self.configuration.apiServer.servingCerts.namedCertificates); update the
condition that currently checks has(self.configuration.apiServer) &&
has(self.configuration.apiServer.servingCerts) to also include
has(self.configuration.apiServer.servingCerts.namedCertificates) so the call to
.exists() on namedCertificates is only evaluated when that field is present.

cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...turegated-crd-manifests/hostedclusters.hypershift.openshift.io/ImageStreamImportMode.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6002,7 +6002,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...turegated-crd-manifests/hostedclusters.hypershift.openshift.io/KMSEncryptionProvider.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6060,7 +6060,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...egated-crd-manifests/hostedclusters.hypershift.openshift.io/NetworkDiagnosticsConfig.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6136,7 +6136,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...enerated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/OpenStack.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6535,7 +6535,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
Comment on lines +6538 to 6540

@coderabbitai coderabbitai Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check if the file exists and get basic info
file_path="api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/OpenStack.yaml"
if [ -f "$file_path" ]; then
  echo "File found."
  wc -l "$file_path"
else
  echo "File not found. Searching for similar paths..."
  find . -name "OpenStack.yaml" -type f 2>/dev/null | head -20
fi

Repository: openshift/hypershift

Length of output: 197

🏁 Script executed:

# Read the specific lines mentioned in the review
file_path="api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/OpenStack.yaml"
if [ -f "$file_path" ]; then
  echo "=== Lines 6530-6550 context ==="
  sed -n '6530,6550p' "$file_path"
fi

Repository: openshift/hypershift

Length of output: 1939

🏁 Script executed:

# Search for the CEL rule pattern to understand the validation rule
rg -A 10 -B 5 "servicePublishingStrategy.loadBalancer.hostname" . --type yaml 2>/dev/null | head -50

Repository: openshift/hypershift

Length of output: 10165

🏁 Script executed:

# Look for schema definitions related to servingCerts and namedCertificates
rg -A 5 "servingCerts" . --type yaml 2>/dev/null | grep -A 5 "namedCertificates" | head -40

Repository: openshift/hypershift

Length of output: 6385

🏁 Script executed:

# Search for the source schema or struct definition for servingCerts
find . -name "*.go" -type f -exec grep -l "namedCertificates" {} \; 2>/dev/null | head -20

Repository: openshift/hypershift

Length of output: 150

🏁 Script executed:

# Look for CRD source or schema files that define servingCerts structure
find . -name "*.go" -type f -exec grep -l "servingCerts" {} \; 2>/dev/null | head -20

Repository: openshift/hypershift

Length of output: 150

🏁 Script executed:

# Search for apiserver servingcerts structure definition
rg "type.*ServingCert" . --type go 2>/dev/null | head -20

Repository: openshift/hypershift

Length of output: 1660

🏁 Script executed:

# Read the APIServerServingCerts struct definition
cat -n ./api/vendor/github.com/openshift/api/config/v1/types_apiserver.go | grep -A 30 "type APIServerServingCerts struct"

Repository: openshift/hypershift

Length of output: 2216

🏁 Script executed:

# Also check the APIServerNamedServingCert struct
cat -n ./api/vendor/github.com/openshift/api/config/v1/types_apiserver.go | grep -A 15 "type APIServerNamedServingCert struct"

Repository: openshift/hypershift

Length of output: 1137

🌐 Web query:

Kubernetes CEL optional field access validation rules kubernetes.io

💡 Result:

To safely write Kubernetes CEL validation rules that touch optional (possibly absent) fields, use presence checks and/or CEL Optional types so the API server never evaluates a field access that isn’t valid.

1) Guard optional field access with has() + short-circuiting

Use the CEL has() macro to check presence, then rely on || / && short-circuiting so the field is only accessed when present. This pattern is shown in Kubernetes docs for validating optional parameters:

!has(params.optionalNumber) || (params.optionalNumber >= 5 && params.optionalNumber <= 10)

[1]

The Kubernetes CEL reference also calls out has() as the way to check accessibility before reading a (possibly missing) field (especially on dynamically-typed variables like object in admission policies). [2]

2) For CRD transition rules, compare presence across updates

When validating transitions with oldSelf, you can validate optional field “set/unset” transitions using:

has(self.field) == has(oldSelf.field)

(as documented in the CRD validation rules blog). [3]

3) If you need oldSelf during create / missing-old cases: optionalOldSelf

In CRD x-kubernetes-validations, setting optionalOldSelf: true makes oldSelf a CEL Optional so you can safely check and unwrap it:

  • oldSelf.hasValue() to test presence
  • oldSelf.value() to unwrap
    Example from the CRD docs:
self.foo == "foo" || (oldSelf.hasValue() && oldSelf.value().foo != "foo")

[4]

4) Know the version gate for CEL Optional Values

Kubernetes documents that CEL Optional Values were introduced in Kubernetes 1.29, and only API servers 1.29+ accept new writes of expressions using those optional features. [2]

Sources

[1] Kubernetes docs – Validating Admission Policy (optional parameters + has() + short-circuit)
[2] Kubernetes docs – Common Expression Language (CEL) in Kubernetes (has() guidance; Optional Values availability)
[3] Kubernetes blog – CRD Validation Rules (transition rule example has(self.field) == has(oldSelf.field))
[4] Kubernetes docs – Extend the Kubernetes API with CRDs (optionalOldSelf, oldSelf.hasValue(), oldSelf.value() examples)

Guard the remaining optional fields in this CEL rule.

The validation still dereferences two optional fields without presence checks: servingCerts.namedCertificates and cert.names. Both are defined as +optional in the schema, so the rule can fail on valid specs like servingCerts: {} or named certificate entries without explicit names. Kubernetes CEL guidance requires guarding optional field access with has() before dereferencing. (kubernetes.io)

Suggested fix 
-                && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
-                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
-                cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
+                && has(self.configuration.apiServer)
+                && has(self.configuration.apiServer.servingCerts)
+                && has(self.configuration.apiServer.servingCerts.namedCertificates)
+                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
+                has(cert.names) && cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
&& has(self.configuration.apiServer)
&& has(self.configuration.apiServer.servingCerts)
&& has(self.configuration.apiServer.servingCerts.namedCertificates)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
has(cert.names) && cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/OpenStack.yaml`
around lines 6538 - 6540, The CEL rule dereferences two optional fields without
presence checks: self.configuration.apiServer.servingCerts.namedCertificates and
cert.names; update the expression used in the rule to guard those accesses with
has() checks (e.g., add
has(self.configuration.apiServer.servingCerts.namedCertificates) before
iterating namedCertificates and add has(cert.names) before calling
cert.names.exists) so the overall condition only evaluates those sub-expressions
when those optional fields are present.

- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...ft-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-CustomNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7861,7 +7861,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6695,7 +6695,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
Comment on lines +6698 to 6700

@coderabbitai coderabbitai Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Schema/source definitions for servingCerts and namedCertificates =="
rg -n -C3 'servingCerts:|namedCertificates:|json:".*namedCertificates,omitempty"|json:".*servingCerts,omitempty"' \
  api/hypershift/v1beta1 \
  cmd/install/assets/hypershift-operator/zz_generated.crd-manifests

echo
echo "== All copies of the APIServer loadBalancer hostname validation =="
rg -n -C2 "APIServer loadBalancer hostname cannot be in ClusterConfiguration\\.apiserver\\.servingCerts\\.namedCertificates\\[\\]|namedCertificates\\.exists\\(cert" \
  api \
  cmd/install

Repository: openshift/hypershift

Length of output: 50377

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check if namedCertificates is optional in the schema (check required fields) =="
# Find servingCerts definitions and check if namedCertificates is in required list
rg -A 10 "servingCerts:" cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml | head -30

echo ""
echo "== Find the exact CEL validation rule for loadBalancer.hostname =="
rg -n "namedCertificates\.exists\(cert" cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml

echo ""
echo "== Count total occurrences of this pattern in all generated manifests =="
rg "namedCertificates\.exists\(cert" cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/ | wc -l

echo ""
echo "== List files with this pattern =="
rg -l "namedCertificates\.exists\(cert" cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/

Repository: openshift/hypershift

Length of output: 1572

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Full context around the CEL rule (lines 6695-6705) =="
sed -n '6695,6705p' cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml

echo ""
echo "== Check for 'required' list in servingCerts section =="
# Look for the required fields list near servingCerts definition
rg -B 5 -A 30 "servingCerts:" cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml | grep -A 20 "servingCerts:" | grep -E "(required:|namedCertificates)" | head -20

Repository: openshift/hypershift

Length of output: 1424

🏁 Script executed:

#!/bin/bash
set -euo pipefannat

echo "== Check all 3 files for the same pattern ==  "
for file in \
  cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-CustomNoUpgrade.crd.yaml \
  cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml \
  cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml; do
  echo "=== File: $(basename $file) ==="
  rg -B 3 "namedCertificates\.exists\(cert" "$file"
  echo ""
done

echo "== Verify namedCertificates is NOT in required list =="
# Search the actual schema for required fields under servingCerts
rg -A 50 '"servingCerts"' cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml | grep -A 40 'properties:' | head -50

Repository: openshift/hypershift

Length of output: 119

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check all 3 files for the same pattern ==  "
for file in \
  cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-CustomNoUpgrade.crd.yaml \
  cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml \
  cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml; do
  echo "=== File: $(basename $file) ==="
  rg -B 3 "namedCertificates\.exists\(cert" "$file"
  echo ""
done

echo "== Verify namedCertificates is NOT in required list =="
# Search the actual schema for required fields under servingCerts
rg -A 50 '"servingCerts"' cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml | grep -A 40 'properties:' | head -50

Repository: openshift/hypershift

Length of output: 1617

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the section defining servingCerts and check for required fields
echo "== Context: servingCerts schema definition with required list check =="
rg -n "servingCerts:" cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml | head -1

# Get line number and read 100 lines from that point
LINE=$(rg -n "servingCerts:" cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml | head -1 | cut -d: -f1)
START=$((LINE - 2))
END=$((LINE + 50))
echo "Lines $START to $END:"
sed -n "${START},${END}p" cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml

echo ""
echo "== Check if namedCertificates field has minItems or is in required list =="
sed -n "${START},${END}p" cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml | grep -E "required:|minItems:|namedCertificates"

Repository: openshift/hypershift

Length of output: 4048

Guard namedCertificates before calling .exists().

Line 6699 still dereferences an optional field. servingCerts is now checked, but namedCertificates is also optional in the schema (not in any required list), so a spec like configuration.apiServer.servingCerts: {} remains valid and can still fail CEL evaluation.

Proposed fix 
-                && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
-                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
+                && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
+                && has(self.configuration.apiServer.servingCerts.namedCertificates)
+                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,

This same pattern appears in 3 manifest files and must be fixed in all of them:

  • hostedclusters-Hypershift-Default.crd.yaml
  • hostedclusters-Hypershift-CustomNoUpgrade.crd.yaml
  • hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml`
around lines 6698 - 6700, The CEL expression dereferences an optional field:
guard self.configuration.apiServer.servingCerts.namedCertificates before calling
.exists(); update the predicate that currently uses
self.configuration.apiServer.servingCerts.namedCertificates.exists(cert, ...) to
first check has(self.configuration.apiServer.servingCerts.namedCertificates) &&
then call .exists(...). Apply the same change to the identical expressions in
hostedclusters-Hypershift-Default.crd.yaml,
hostedclusters-Hypershift-CustomNoUpgrade.crd.yaml, and
hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml so all occurrences check
namedCertificates presence before .exists().

- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
3 changes: 2 additions & 1 deletion ...erator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7672,7 +7672,8 @@ spec:
- message: APIServer loadBalancer hostname cannot be in ClusterConfiguration.apiserver.servingCerts.namedCertificates[]
rule: '!self.services.exists(s, s.service == ''APIServer'' && has(s.servicePublishingStrategy.loadBalancer)
&& s.servicePublishingStrategy.loadBalancer.hostname != "" && has(self.configuration)
&& has(self.configuration.apiServer) && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
Comment on lines +7675 to 7677

@coderabbitai coderabbitai Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and get the exact content at lines 7675-7677
wc -l cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml

Repository: openshift/hypershift

Length of output: 190

🏁 Script executed:

# Read the target lines with context
sed -n '7670,7680p' cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml

Repository: openshift/hypershift

Length of output: 1094

🏁 Script executed:

# Find the schema definitions for servingCerts, namedCertificates, and names fields
# Search for where these are defined in the CRD
rg -n "servingCerts|namedCertificates" cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml | head -30

Repository: openshift/hypershift

Length of output: 644

🏁 Script executed:

# Find the apiServer schema definition to check if servingCerts is optional
rg -n "apiServer:" cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml | head -20

Repository: openshift/hypershift

Length of output: 130

🏁 Script executed:

# Get context around servingCerts definition to check required/optional status
sed -n '555,590p' cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml

Repository: openshift/hypershift

Length of output: 2574

🏁 Script executed:

# Search for namedCertificates item schema and names field definition
sed -n '560,620p' cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml

Repository: openshift/hypershift

Length of output: 4091

🏁 Script executed:

# Check the required fields at each level to confirm optionality
# Looking at servingCerts definition
sed -n '555,610p' cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml | grep -A 5 "required:"

Repository: openshift/hypershift

Length of output: 319

🏁 Script executed:

# Check if there's a required array for servingCerts or namedCertificates
sed -n '555,625p' cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml | cat -n

Repository: openshift/hypershift

Length of output: 5045

🌐 Web query:

Kubernetes CRD CEL validation .exists() on optional list fields

💡 Result:

In CRD x-kubernetes-validations CEL, an optional list field may be absent, and calling list macros like .exists() on an absent field will fail evaluation. Guard the call with has() (field presence check) or short-circuit around it. (kubernetes.io)

Common patterns:

  • If the list is optional (skip validation when missing):

    !has(self.spec.items) || self.spec.items.exists(x, x.name == "foo")

  • If the list must be present and satisfy a condition:

    has(self.spec.items) && self.spec.items.exists(x, x.name == "foo")

  • If you also need “non-empty”:

    has(self.spec.items) && size(self.spec.items) > 0 &&
self.spec.items.exists(x, x.name == "foo")

.exists() itself is the right macro for “any element matches” once you’ve ensured the list value is present. (kubernetes.io)

Citations:

🌐 Web query:

Kubernetes CEL validation has() guard for optional array fields before exists()

💡 Result:

In Kubernetes CEL, guard an optional list/array field with has() and boolean short-circuiting before calling .exists(...):

!has(self.spec.myList) || self.spec.myList.exists(x, /* predicate using x */)

For nested optional parents, guard each level:

!has(self.spec) || !has(self.spec.myList) || self.spec.myList.exists(x, /* predicate */)

This works because Kubernetes CEL supports has() for field presence checks, and || short-circuits (so .exists() won’t be evaluated when the field is absent). It also supports list macros like .exists(...) on list fields. [1][2]

Sources:
[1] https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/
[2] https://kubernetes.io/docs/reference/using-api/cel/

Guard optional list fields before calling .exists() to prevent CEL validation failures.

The rule at lines 7675-7677 guards servingCerts presence but omits guards for two optional list fields: namedCertificates and names. Since both are optional per the schema, a spec with an absent namedCertificates list or a namedCertificates item with an absent names field will cause CEL evaluation to fail, rejecting otherwise valid HostedCluster specs. Add has() checks before each .exists() call on these optional lists.

Suggested patch 
-                && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
-                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
-                cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
+                && has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
+                && has(self.configuration.apiServer.servingCerts.namedCertificates)
+                && self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
+                has(cert.names) && cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
&& has(self.configuration.apiServer) && has(self.configuration.apiServer.servingCerts)
&& has(self.configuration.apiServer.servingCerts.namedCertificates)
&& self.configuration.apiServer.servingCerts.namedCertificates.exists(cert,
has(cert.names) && cert.names.exists(n, n == s.servicePublishingStrategy.loadBalancer.hostname)))'
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml`
around lines 7675 - 7677, The CEL validation currently checks
self.configuration.apiServer.servingCerts but calls .exists() on two optional
lists without guarding them: servingCerts.namedCertificates.exists(...) and
cert.names.exists(...); update the rule to first check
has(self.configuration.apiServer.servingCerts.namedCertificates) before calling
namedCertificates.exists(...) and has(cert.names) before calling
cert.names.exists(...) so both optional lists are guarded and CEL evaluation
won't fail when those lists are absent.

- message: disableMultiNetwork can only be set to true when networkType
is 'Other'
Expand Down
2 changes: 1 addition & 1 deletion vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.