CORS-4341: Allow DNSType in Infra CR's Platform Status to be updated Day-2 for Azure#2729
Open
sadasu wants to merge 1 commit intoopenshift:masterfrom
Open
CORS-4341: Allow DNSType in Infra CR's Platform Status to be updated Day-2 for Azure#2729sadasu wants to merge 1 commit intoopenshift:masterfrom
sadasu wants to merge 1 commit intoopenshift:masterfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
Hello @sadasu! Some important instructions when contributing to openshift/api: |
|
@sadasu: This pull request references CORS-4341 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
the
size/XL
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/jira refresh |
|
@sadasu: This pull request references CORS-4341 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting ⛔ Files ignored due to path filters (18)
📒 Files selected for processing (17)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughValidation for cloudLoadBalancerConfig.dnsType was moved from a global top-level rule to platform-scoped rules. AWS and GCP now enforce dnsType immutability once set; Azure allows dnsType updates after being set. Tests were adjusted (error messages and one Azure test now expects a successful update). Go types received per-platform XValidation annotations and documentation, generated Swagger docs were updated, and multiple CRD variants had descriptions and x-kubernetes-validations changed to the new has(oldSelf.dnsType)-based rule. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
Contributor
Author
|
/jira refresh |
|
@sadasu: This pull request references CORS-4341 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
payload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml (1)
1544-1559: AzurednsTypedescription duplicates AWS/GCP copy — consider platform-specific wordingThe
dnsTypedescription understatus.platformStatus.azure.cloudLoadBalancerConfig(lines 1550-1551) is identical to the AWS/GCP copy: it says the value is immutable for AWS and GCP but mutable for Azure. Placing AWS/GCP-specific immutability language inside an Azure-only field description is mildly confusing to readers introspecting the Azure status. A tighter description stating only that this field can be updated after initial set (without cross-referencing other platforms) would improve clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml` around lines 1544 - 1559, Update the description for status.platformStatus.azure.cloudLoadBalancerConfig.dnsType to remove the AWS/GCP-specific immutability wording and instead state only Azure-relevant behavior: explain that dnsType indicates the cluster DNS solution, list the enum values (PlatformDefault, ClusterHosted), and note that for Azure this field can be updated after initial set (without referencing other platforms or immutability rules for AWS/GCP); modify the description text in the dnsType property under status.platformStatus.azure.cloudLoadBalancerConfig accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/v1/types_infrastructure.go`:
- Line 562: The XValidation CEL rule attached to the cloudLoadBalancerConfig
field incorrectly references self.cloudLoadBalancerConfig.dnsType and
oldSelf.cloudLoadBalancerConfig.dnsType even though at field-level scope self is
already the CloudLoadBalancerConfig object; update the
+kubebuilder:validation:XValidation rule(s) for cloudLoadBalancerConfig (the
annotation using the immutability check for dnsType) to use self.dnsType and
oldSelf.dnsType (remove the cloudLoadBalancerConfig. prefix) for both
occurrences so the immutability check works at field scope.
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml`:
- Around line 1355-1361: The CEL immutability rules under
x-kubernetes-validations wrongly reference self.cloudLoadBalancerConfig (which
inside the cloudLoadBalancerConfig block makes self already equal to
cloudLoadBalancerConfig), causing has(self.cloudLoadBalancerConfig) to always be
false; fix the rule expressions in the cloudLoadBalancerConfig
x-kubernetes-validations by removing the redundant prefix and referencing
dnsType directly (use has(self.dnsType) and has(oldSelf.dnsType) and compare
oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType), and apply the
identical change to the GCP copy of the rule (the rule blocks around the
cloudLoadBalancerConfig for AWS and GCP).
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-Default.crd.yaml`:
- Around line 1599-1605: The CEL immutability rule currently referenced nested
paths that are out-of-scope (it uses self.cloudLoadBalancerConfig.dnsType and
oldSelf.cloudLoadBalancerConfig.dnsType while the rule is evaluated in the
cloudLoadBalancerConfig scope); update the rule to reference the local fields
instead (use self.dnsType and oldSelf.dnsType) so the CEL expression can
evaluate correctly, and apply the same change to the two other occurrences in
the TechPreviewNoUpgrade variant as well so all three immutability checks in
both Default and TechPreviewNoUpgrade use the in-scope field names.
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yaml`:
- Around line 1355-1361: The CEL immutability rule inside the
cloudLoadBalancerConfig schema is using fully-qualified paths
(self.cloudLoadBalancerConfig.dnsType and
oldSelf.cloudLoadBalancerConfig.dnsType) but must use the current object scope;
update the rule expressions for the cloudLoadBalancerConfig.dnsType immutability
checks (the rule entries under the cloudLoadBalancerConfig schema at the two
occurrences) to reference self.dnsType and oldSelf.dnsType instead, mirroring
how clusterHosted validation uses the local scope.
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-OKD.crd.yaml`:
- Around line 1599-1605: The CEL immutability rule is using the wrong scope
(referencing self.cloudLoadBalancerConfig.dnsType) inside the
cloudLoadBalancerConfig validation block; update the predicate to use
self.dnsType (and oldSelf.dnsType) instead. Edit the immutability rule
definition in the AWSPlatformStatus and GCPPlatformStatus types (the
cloudLoadBalancerConfig validation for dnsType in the types that generate the
CRDs) to replace occurrences of self.cloudLoadBalancerConfig.dnsType and
oldSelf.cloudLoadBalancerConfig.dnsType with self.dnsType and oldSelf.dnsType so
the rule matches the local object scope and regenerates the CRDs.
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml`:
- Around line 1355-1361: The CEL immutability rule is scoped to the
cloudLoadBalancerConfig object so references to self.cloudLoadBalancerConfig and
oldSelf.cloudLoadBalancerConfig are incorrect and make the check always skip;
update the rule under the cloudLoadBalancerConfig x-kubernetes-validations (the
rule string currently referencing cloudLoadBalancerConfig.dnsType) to instead
reference self.dnsType and oldSelf.dnsType (and the corresponding has(...)
checks) so the expression becomes conditional on has(self.dnsType) &&
has(oldSelf.dnsType) and enforces oldSelf.dnsType == '' || self.dnsType ==
oldSelf.dnsType, applying the same fix for both the AWS and GCP rules (and the
Default.crd.yaml occurrence).
In
`@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yaml`:
- Around line 1641-1648: The CEL immutability rule inside the
cloudLoadBalancerConfig validation block is using the wrong scope (it references
self.cloudLoadBalancerConfig.dnsType and
oldSelf.cloudLoadBalancerConfig.dnsType), so the
has(self.cloudLoadBalancerConfig) check is always false and the rule is skipped;
update the rule to reference the local object fields directly (use self.dnsType
and oldSelf.dnsType) and adjust the has() predicates accordingly (e.g.,
has(self.dnsType) && has(oldSelf.dnsType)) so the immutability condition
actually compares oldSelf.dnsType == self.dnsType, and apply the same fix to
both the AWS and GCP cloudLoadBalancerConfig validation blocks referenced in the
CRD.
In
`@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yaml`:
- Line 1762: Update the confusing cross-platform immutability wording in the
description for cloudLoadBalancerConfig.dnsType so the Azure-specific behavior
is clear; specifically, edit the dnsType description under the Azure
cloudLoadBalancerConfig block to either state only the Azure behavior (e.g.,
"This value can be updated after it has been set.") or at minimum place the
Azure sentence first and then mention AWS/GCP immutability, and apply the same
change to the other dnsType description occurrence (both
cloudLoadBalancerConfig.dnsType entries referenced in the CRD).
- Around line 1636-1648: The immutability CEL rule under
x-kubernetes-validations for cloudLoadBalancerConfig is using
has(self.cloudLoadBalancerConfig) which is always false because the validation
is scoped to cloudLoadBalancerConfig; change the predicate to reference the
fields on the current scope directly (use has(self.dnsType) &&
has(oldSelf.dnsType) and compare oldSelf.dnsType to self.dnsType) so the ternary
checks the actual dnsType presence and enforces immutability for dnsType within
cloudLoadBalancerConfig.
- Around line 2228-2240: The immutability CEL under x-kubernetes-validations for
the GCP cloudLoadBalancerConfig block is scoped to the cloudLoadBalancerConfig
object but references
self.cloudLoadBalancerConfig/oldSelf.cloudLoadBalancerConfig (which don’t exist
in that scope), so update the rule to use the local field names (compare
self.dnsType to oldSelf.dnsType) or move the entire validation up to the parent
scope where cloudLoadBalancerConfig/oldSelf.cloudLoadBalancerConfig exist;
adjust the rule expression accordingly so it actually guards immutability of
cloudLoadBalancerConfig.dnsType.
In
`@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yaml`:
- Around line 1885-1892: The validation rule inside the cloudLoadBalancerConfig
x-kubernetes-validations is checking has(self.cloudLoadBalancerConfig) and
has(oldSelf.cloudLoadBalancerConfig) which is wrong because self is already
cloudLoadBalancerConfig; change the condition to check the dnsType fields
directly (e.g., has(self.dnsType) && has(oldSelf.dnsType)) and make the ternary
compare oldSelf.dnsType and self.dnsType (or simply reference self.dnsType and
oldSelf.dnsType), updating the rule string accordingly for the
cloudLoadBalancerConfig block and then search other CRD variants for the same
broken pattern (has(self.cloudLoadBalancerConfig) /
has(oldSelf.cloudLoadBalancerConfig)) to apply the same fix where present.
In
`@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yaml`:
- Around line 2233-2240: The validation rule for cloudLoadBalancerConfig.dnsType
uses the wrong self path (it references self.cloudLoadBalancerConfig) so the
immutability check never runs; update the rule expression to check
has(self.dnsType) and has(oldSelf.dnsType) and compare oldSelf.dnsType and
self.dnsType (mirroring the AWS fix) so it becomes: has(self) && has(oldSelf) &&
has(self.dnsType) && has(oldSelf.dnsType) ? (oldSelf.dnsType == '' ||
self.dnsType == oldSelf.dnsType) : true, ensuring the immutability of
cloudLoadBalancerConfig.dnsType is enforced.
- Around line 1641-1648: The CEL immutability rule incorrectly references a
nested field (self.cloudLoadBalancerConfig.dnsType) causing the check to always
short-circuit; update the rule to reference the fields on the object itself (use
self.dnsType and oldSelf.dnsType) and replace has(self.cloudLoadBalancerConfig)
/ has(oldSelf.cloudLoadBalancerConfig) with has(self.dnsType) /
has(oldSelf.dnsType) (or just has(self) checks appropriate to the schema) so the
ternary expression actually compares oldSelf.dnsType == '' || self.dnsType ==
oldSelf.dnsType and enforces immutability for dnsType on
cloudLoadBalancerConfig.
---
Nitpick comments:
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml`:
- Around line 1544-1559: Update the description for
status.platformStatus.azure.cloudLoadBalancerConfig.dnsType to remove the
AWS/GCP-specific immutability wording and instead state only Azure-relevant
behavior: explain that dnsType indicates the cluster DNS solution, list the enum
values (PlatformDefault, ClusterHosted), and note that for Azure this field can
be updated after initial set (without referencing other platforms or
immutability rules for AWS/GCP); modify the description text in the dnsType
property under status.platformStatus.azure.cloudLoadBalancerConfig accordingly.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (17)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/AWSClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/AzureClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/GCPClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AWSClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AzureClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/GCPClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**
📒 Files selected for processing (17)
config/v1/tests/infrastructures.config.openshift.io/AWSClusterHostedDNS.yamlconfig/v1/tests/infrastructures.config.openshift.io/AWSClusterHostedDNSInstall.yamlconfig/v1/tests/infrastructures.config.openshift.io/AzureClusterHostedDNSInstall.yamlconfig/v1/tests/infrastructures.config.openshift.io/GCPClusterHostedDNS.yamlconfig/v1/tests/infrastructures.config.openshift.io/GCPClusterHostedDNSInstall.yamlconfig/v1/types_infrastructure.goconfig/v1/zz_generated.swagger_doc_generated.gopayload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-Default.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-OKD.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yaml
payload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml
Outdated
Show resolved
Hide resolved
payload-manifests/crds/0000_10_config-operator_01_infrastructures-Default.crd.yaml
Outdated
Show resolved
Hide resolved
payload-manifests/crds/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yaml
Outdated
Show resolved
Hide resolved
payload-manifests/crds/0000_10_config-operator_01_infrastructures-OKD.crd.yaml
Outdated
Show resolved
Hide resolved
payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yaml
Show resolved
Hide resolved
payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yaml
Outdated
Show resolved
Hide resolved
payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yaml
Outdated
Show resolved
Hide resolved
...oad-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yaml
Outdated
Show resolved
Hide resolved
...oad-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yaml
Outdated
Show resolved
Hide resolved
sadasu
force-pushed
the
dnstype-updates
branch
from
26e1385 to
45ce17c
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (10)
payload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml (1)
1355-1358:⚠️ Potential issue | 🔴 CriticalFix CEL scope in DNSType immutability rules (currently no-op).
At Line [1357] and Line [1940], the rule references
self.cloudLoadBalancerConfig.*/oldSelf.cloudLoadBalancerConfig.*inside thecloudLoadBalancerConfigobject scope. That path is out of scope, so the immutability guard does not evaluate as intended for AWS/GCP.🐛 Proposed fix
- - message: cloudLoadBalancerConfig.dnsType is immutable once - set - rule: oldSelf.cloudLoadBalancerConfig.dnsType == '' || self.cloudLoadBalancerConfig.dnsType - == oldSelf.cloudLoadBalancerConfig.dnsType + - message: cloudLoadBalancerConfig.dnsType is immutable once + set + rule: '!has(oldSelf.dnsType) || oldSelf.dnsType == '' || + self.dnsType == oldSelf.dnsType' ... - - message: cloudLoadBalancerConfig.dnsType is immutable once - set - rule: oldSelf.cloudLoadBalancerConfig.dnsType == '' || self.cloudLoadBalancerConfig.dnsType - == oldSelf.cloudLoadBalancerConfig.dnsType + - message: cloudLoadBalancerConfig.dnsType is immutable once + set + rule: '!has(oldSelf.dnsType) || oldSelf.dnsType == '' || + self.dnsType == oldSelf.dnsType'#!/bin/bash set -euo pipefail file="payload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml" echo "Checking for out-of-scope CEL references:" rg -n "self\.cloudLoadBalancerConfig\.dnsType|oldSelf\.cloudLoadBalancerConfig\.dnsType" "$file" echo echo "Showing immutability rule blocks:" rg -n "cloudLoadBalancerConfig\.dnsType is immutable once set" -A3 "$file"Also applies to: 1938-1941
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml` around lines 1355 - 1358, The CEL immutability rule inside the cloudLoadBalancerConfig object is referencing out-of-scope paths (self.cloudLoadBalancerConfig.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType) so it never evaluates; update the CEL expression in the cloudLoadBalancerConfig validation rule to refer to the local field names (use self.dnsType and oldSelf.dnsType) so the rule becomes e.g. oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType (and apply the same change for the duplicate rule elsewhere), ensuring the message about dnsType immutability remains unchanged.payload-manifests/crds/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml (2)
1355-1358:⚠️ Potential issue | 🔴 CriticalAWS immutability CEL rule references a non-existent path and is completely ineffective.
This
x-kubernetes-validationsblock is scoped to thecloudLoadBalancerConfigobject (line 1349type: object), soselfis thecloudLoadBalancerConfiginstance. Referencingself.cloudLoadBalancerConfig.dnsTypetries to access a nestedcloudLoadBalancerConfigproperty that doesn't exist withincloudLoadBalancerConfig. The rule will either error out or be silently bypassed, meaning dnsType is not actually immutable on AWS.Use
self.dnsType/oldSelf.dnsTypeinstead:🐛 Proposed fix
- message: cloudLoadBalancerConfig.dnsType is immutable once set - rule: oldSelf.cloudLoadBalancerConfig.dnsType == '' || self.cloudLoadBalancerConfig.dnsType - == oldSelf.cloudLoadBalancerConfig.dnsType + rule: oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml` around lines 1355 - 1358, The CEL immutability rule inside the x-kubernetes-validations for the cloudLoadBalancerConfig object incorrectly references self.cloudLoadBalancerConfig.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType (which don't exist because self is already the cloudLoadBalancerConfig), so the dnsType immutability check is ineffective; update the rule to use self.dnsType and oldSelf.dnsType (i.e., replace both occurrences of self.cloudLoadBalancerConfig.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType with self.dnsType and oldSelf.dnsType) so the dnsType immutability is actually enforced for cloudLoadBalancerConfig.
1938-1941:⚠️ Potential issue | 🔴 CriticalGCP immutability CEL rule has the same incorrect scoping — dnsType is not actually immutable on GCP.
Same issue as the AWS rule. At this scope,
selfis thecloudLoadBalancerConfigobject;self.cloudLoadBalancerConfigdoesn't exist.🐛 Proposed fix
- message: cloudLoadBalancerConfig.dnsType is immutable once set - rule: oldSelf.cloudLoadBalancerConfig.dnsType == '' || self.cloudLoadBalancerConfig.dnsType - == oldSelf.cloudLoadBalancerConfig.dnsType + rule: oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml` around lines 1938 - 1941, The CEL immutability rule is using incorrect scoping (it references self.cloudLoadBalancerConfig which doesn't exist at this scope); update the rule for cloudLoadBalancerConfig.dnsType to compare dnsType directly on self and oldSelf, e.g. replace "oldSelf.cloudLoadBalancerConfig.dnsType == '' || self.cloudLoadBalancerConfig.dnsType == oldSelf.cloudLoadBalancerConfig.dnsType" with "oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType" so the rule uses self.dnsType and oldSelf.dnsType.payload-manifests/crds/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yaml (1)
1355-1358:⚠️ Potential issue | 🔴 CriticalFix CEL field scope in AWS and GCP
cloudLoadBalancerConfigimmutability rules.The
x-kubernetes-validationsblock at Lines 1350–1358 (AWS) and 1933–1941 (GCP) are attached to thecloudLoadBalancerConfigobject schema, soself/oldSelfalready refers tocloudLoadBalancerConfigitself. The new immutability rule incorrectly usesself.cloudLoadBalancerConfig.dnsType/oldSelf.cloudLoadBalancerConfig.dnsType, which tries to traverse a non-existent sub-field and will never match. Compare to the adjacent sibling rule on the same object (Line 1353 / 1936) which correctly usesself.dnsType/has(self.dnsType).🔧 Proposed fix — AWS (Lines 1355–1358)
- - message: cloudLoadBalancerConfig.dnsType is immutable once - set - rule: oldSelf.cloudLoadBalancerConfig.dnsType == '' || self.cloudLoadBalancerConfig.dnsType - == oldSelf.cloudLoadBalancerConfig.dnsType + - message: cloudLoadBalancerConfig.dnsType is immutable once + set + rule: oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType🔧 Proposed fix — GCP (Lines 1938–1941)
- - message: cloudLoadBalancerConfig.dnsType is immutable once - set - rule: oldSelf.cloudLoadBalancerConfig.dnsType == '' || self.cloudLoadBalancerConfig.dnsType - == oldSelf.cloudLoadBalancerConfig.dnsType + - message: cloudLoadBalancerConfig.dnsType is immutable once + set + rule: oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsTypeAlso applies to: 1938-1941
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yaml` around lines 1355 - 1358, The CEL immutability rule attached to the cloudLoadBalancerConfig object incorrectly references self.cloudLoadBalancerConfig.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType (which do not exist at that scope); update the rule expressions in the x-kubernetes-validations for both AWS and GCP to use self.dnsType and oldSelf.dnsType respectively so the immutability check runs against the dnsType field of cloudLoadBalancerConfig itself (look for the validation entries with message "cloudLoadBalancerConfig.dnsType is immutable once set" and the associated rule).payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yaml (1)
1641-1645:⚠️ Potential issue | 🔴 CriticalFix unresolved CEL path bug in
dnsTypeimmutability rules (AWS/GCP).This is still using
self.cloudLoadBalancerConfig.dnsTypeat thecloudLoadBalancerConfigobject scope, so the immutability check does not evaluate the intended field path.🐛 Proposed fix
- - message: cloudLoadBalancerConfig.dnsType is immutable - once set - rule: oldSelf.cloudLoadBalancerConfig.dnsType == - '' || self.cloudLoadBalancerConfig.dnsType == - oldSelf.cloudLoadBalancerConfig.dnsType + - message: cloudLoadBalancerConfig.dnsType is immutable + once set + rule: 'has(self.dnsType) && has(oldSelf.dnsType) + ? (oldSelf.dnsType == '''' || self.dnsType == oldSelf.dnsType) + : true'#!/bin/bash # Verify unresolved invalid CEL field paths in this CRD rg -n -C2 "self\.cloudLoadBalancerConfig\.dnsType|oldSelf\.cloudLoadBalancerConfig\.dnsType" \ payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yaml # Optional: confirm all similar CRDs are clean after regeneration/fix rg -n -C2 "self\.cloudLoadBalancerConfig\.dnsType|oldSelf\.cloudLoadBalancerConfig\.dnsType" \ payload-manifests/crds/*.crd.yamlAlso applies to: 2230-2234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yaml` around lines 1641 - 1645, The CEL immutability rule is referencing self.cloudLoadBalancerConfig.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType from within the cloudLoadBalancerConfig object scope, which makes the path invalid; update the expressions to use the correct relative field paths (e.g. replace self.cloudLoadBalancerConfig.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType with self.dnsType and oldSelf.dnsType) so the rule on cloudLoadBalancerConfig.dnsType evaluates the intended field (also apply the same change for the duplicated occurrences around lines 2230-2234).payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yaml (2)
1641-1645:⚠️ Potential issue | 🔴 CriticalFix CEL scope: immutability rule references a non-existent nested object.
At Line 1643 and Line 2232,
selfis alreadycloudLoadBalancerConfig, soself.cloudLoadBalancerConfig.*/oldSelf.cloudLoadBalancerConfig.*are out of scope and the rule won’t enforcednsTypeimmutability correctly.🐛 Proposed fix (apply in both AWS and GCP blocks)
- - message: cloudLoadBalancerConfig.dnsType is immutable - once set - rule: oldSelf.cloudLoadBalancerConfig.dnsType == - '' || self.cloudLoadBalancerConfig.dnsType == - oldSelf.cloudLoadBalancerConfig.dnsType + - message: dnsType is immutable once set + rule: '!has(oldSelf.dnsType) || oldSelf.dnsType == '''' || self.dnsType == oldSelf.dnsType'#!/bin/bash set -euo pipefail file="payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yaml" echo "Current problematic references (expect matches before fix, none after fix):" rg -n -C3 "oldSelf\.cloudLoadBalancerConfig\.dnsType|self\.cloudLoadBalancerConfig\.dnsType" "$file" echo echo "Post-fix sanity pattern (expect immutable rules to use oldSelf.dnsType/self.dnsType):" rg -n -C2 "dnsType is immutable once set|oldSelf\.dnsType|self\.dnsType" "$file"Also applies to: 2230-2234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yaml` around lines 1641 - 1645, The CEL immutability rule inside the cloudLoadBalancerConfig block incorrectly references nested fields (self.cloudLoadBalancerConfig.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType) which are out of scope; update the rule in both AWS and GCP cloudLoadBalancerConfig blocks so it compares dnsType directly (use self.dnsType and oldSelf.dnsType) and keep the rest of the expression (e.g., oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType) intact to enforce immutability once set.
1835-1836:⚠️ Potential issue | 🟡 MinorKeep
dnsTypedescriptions platform-local to avoid cross-platform confusion.At Line 1835 and Line 2214, the Azure/GCP section text still embeds other platform behavior. This makes each platform block harder to scan.
✍️ Suggested wording adjustment
# Azure dnsType description block - The value is immutable after it has been set at install time for AWS and GCP platforms. - For Azure platform, this value can be updated after it has been set. + For Azure platform, this value can be updated after it has been set. # GCP dnsType description block - The value is immutable after it has been set at install time for AWS and GCP platforms. - For Azure platform, this value can be updated after it has been set. + For GCP platform, this value is immutable after it has been set at install time.Also applies to: 2214-2215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yaml` around lines 1835 - 1836, The dnsType description currently mixes platform behaviors across blocks; update the dnsType description entries so each platform block only states that platform's behavior: in the Azure dnsType description remove the sentence about AWS/GCP immutability and state only that Azure values can be updated after install, and in the GCP (and AWS) dnsType descriptions remove any mention of Azure updateability and state only that the value is immutable after install for those platforms; locate the descriptions by searching for the dnsType CRD description text and the Azure/GCP platform headings to make the edits.payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yaml (1)
1885-1889:⚠️ Potential issue | 🔴 CriticalFix CEL scope: immutability rule dereferences a non-existent nested object.
In this validation scope,
self/oldSelfalready arecloudLoadBalancerConfig; referencingself.cloudLoadBalancerConfig.*breaks the check and can fail to enforce immutability.🐛 Proposed fix
- - message: cloudLoadBalancerConfig.dnsType is immutable - once set - rule: oldSelf.cloudLoadBalancerConfig.dnsType == - '' || self.cloudLoadBalancerConfig.dnsType == - oldSelf.cloudLoadBalancerConfig.dnsType + - message: cloudLoadBalancerConfig.dnsType is immutable + once set + rule: 'has(self.dnsType) && has(oldSelf.dnsType) + ? (oldSelf.dnsType == '''' || self.dnsType == oldSelf.dnsType) + : true'#!/bin/bash # Verify broken CEL field paths in CRD manifests rg -n "self\\.cloudLoadBalancerConfig\\.dnsType|oldSelf\\.cloudLoadBalancerConfig\\.dnsType" --type yaml # Inspect this exact validation block with context rg -n -A8 -B4 "cloudLoadBalancerConfig\\.dnsType is immutable" payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yaml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yaml` around lines 1885 - 1889, The CEL immutability rule incorrectly dereferences cloudLoadBalancerConfig inside a scope where self/oldSelf already refer to cloudLoadBalancerConfig; update the rule that currently uses self.cloudLoadBalancerConfig.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType to reference dnsType directly (e.g., rule: oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType) so the check correctly enforces immutability of dnsType within the cloudLoadBalancerConfig validation block.payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yaml (1)
1641-1645:⚠️ Potential issue | 🔴 CriticalCEL immutability rules use wrong
selfscope — check is silently skipped.Both the AWS (lines 1641–1645) and GCP (lines 2230–2234)
x-kubernetes-validationsblocks are placed on thecloudLoadBalancerConfigobject itself, meaningselfalready is thecloudLoadBalancerConfigobject. The rules referenceself.cloudLoadBalancerConfig.dnsType/oldSelf.cloudLoadBalancerConfig.dnsType, butcloudLoadBalancerConfighas no child field namedcloudLoadBalancerConfig. As a result,has(self.cloudLoadBalancerConfig)is alwaysfalse, the ternary short-circuits totrue, and the immutability constraint is never enforced.The first rule in each block (
clusterHostedguard) correctly usesself.dnsType— the new immutability rules must follow the same pattern.🔧 Fix (apply to both AWS block lines 1641-1645 and GCP block lines 2230-2234)
- - message: cloudLoadBalancerConfig.dnsType is immutable - once set - rule: oldSelf.cloudLoadBalancerConfig.dnsType == - '' || self.cloudLoadBalancerConfig.dnsType == - oldSelf.cloudLoadBalancerConfig.dnsType + - message: cloudLoadBalancerConfig.dnsType is immutable + once set + rule: 'has(self.dnsType) && has(oldSelf.dnsType) + ? (oldSelf.dnsType == '''' || self.dnsType == + oldSelf.dnsType) : true'Also applies to: 2230-2234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yaml` around lines 1641 - 1645, The CEL immutability rule in the cloudLoadBalancerConfig x-kubernetes-validations block is referencing nonexistent nested fields (self.cloudLoadBalancerConfig.dnsType / oldSelf.cloudLoadBalancerConfig.dnsType) so the guard always short-circuits; update the rule to use the correct scope by replacing self.cloudLoadBalancerConfig.dnsType with self.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType with oldSelf.dnsType (mirror the existing clusterHosted guard pattern) in both the AWS and GCP cloudLoadBalancerConfig validation blocks so the immutability check actually enforces dnsType.config/v1/types_infrastructure.go (1)
562-562:⚠️ Potential issue | 🔴 CriticalFix CEL scope in
cloudLoadBalancerConfigimmutability rule.At Line 562 and Line 792, this is a field-scoped
XValidation;self/oldSelfalready refer toCloudLoadBalancerConfig. Usingself.cloudLoadBalancerConfig.*is out-of-scope and breaks AWS/GCP immutability enforcement.🔧 Proposed fix
- // +kubebuilder:validation:XValidation:rule=oldSelf.cloudLoadBalancerConfig.dnsType == '' || self.cloudLoadBalancerConfig.dnsType == oldSelf.cloudLoadBalancerConfig.dnsType,message="cloudLoadBalancerConfig.dnsType is immutable once set" + // +kubebuilder:validation:XValidation:rule="oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType",message="cloudLoadBalancerConfig.dnsType is immutable once set" ... - // +kubebuilder:validation:XValidation:rule="oldSelf.cloudLoadBalancerConfig.dnsType == '' || self.cloudLoadBalancerConfig.dnsType == oldSelf.cloudLoadBalancerConfig.dnsType",message="cloudLoadBalancerConfig.dnsType is immutable once set" + // +kubebuilder:validation:XValidation:rule="oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType",message="cloudLoadBalancerConfig.dnsType is immutable once set"Run this read-only check after patching/regenerating:
#!/bin/bash set -euo pipefail echo "Checking for out-of-scope CEL references (should return no matches):" rg -n 'self\.cloudLoadBalancerConfig\.dnsType|oldSelf\.cloudLoadBalancerConfig\.dnsType' \ config/v1/types_infrastructure.go \ payload-manifests/crds/0000_10_config-operator_01_infrastructures-Default.crd.yaml echo echo "Checking for in-scope CEL references (should match AWS/GCP source + generated CRD):" rg -n 'oldSelf\.dnsType == .*\|\| self\.dnsType == oldSelf\.dnsType' \ config/v1/types_infrastructure.go \ payload-manifests/crds/0000_10_config-operator_01_infrastructures-Default.crd.yamlAlso applies to: 792-792
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/v1/types_infrastructure.go` at line 562, The CEL XValidation for cloudLoadBalancerConfig is using out-of-scope qualifiers (self.cloudLoadBalancerConfig.* / oldSelf.cloudLoadBalancerConfig.*) which breaks immutability checks; update the XValidation annotation on the CloudLoadBalancerConfig field (the +kubebuilder:validation:XValidation rule for dnsType) to reference the field-scoped names (oldSelf.dnsType and self.dnsType) instead of self.cloudLoadBalancerConfig.dnsType/oldSelf.cloudLoadBalancerConfig.dnsType so the rule reads the in-scope comparison (oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType). Ensure you apply the same change to the duplicate rule at the other occurrence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@config/v1/types_infrastructure.go`:
- Line 562: The CEL XValidation for cloudLoadBalancerConfig is using
out-of-scope qualifiers (self.cloudLoadBalancerConfig.* /
oldSelf.cloudLoadBalancerConfig.*) which breaks immutability checks; update the
XValidation annotation on the CloudLoadBalancerConfig field (the
+kubebuilder:validation:XValidation rule for dnsType) to reference the
field-scoped names (oldSelf.dnsType and self.dnsType) instead of
self.cloudLoadBalancerConfig.dnsType/oldSelf.cloudLoadBalancerConfig.dnsType so
the rule reads the in-scope comparison (oldSelf.dnsType == '' || self.dnsType ==
oldSelf.dnsType). Ensure you apply the same change to the duplicate rule at the
other occurrence.
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml`:
- Around line 1355-1358: The CEL immutability rule inside the
cloudLoadBalancerConfig object is referencing out-of-scope paths
(self.cloudLoadBalancerConfig.dnsType and
oldSelf.cloudLoadBalancerConfig.dnsType) so it never evaluates; update the CEL
expression in the cloudLoadBalancerConfig validation rule to refer to the local
field names (use self.dnsType and oldSelf.dnsType) so the rule becomes e.g.
oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType (and apply the same
change for the duplicate rule elsewhere), ensuring the message about dnsType
immutability remains unchanged.
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yaml`:
- Around line 1355-1358: The CEL immutability rule attached to the
cloudLoadBalancerConfig object incorrectly references
self.cloudLoadBalancerConfig.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType
(which do not exist at that scope); update the rule expressions in the
x-kubernetes-validations for both AWS and GCP to use self.dnsType and
oldSelf.dnsType respectively so the immutability check runs against the dnsType
field of cloudLoadBalancerConfig itself (look for the validation entries with
message "cloudLoadBalancerConfig.dnsType is immutable once set" and the
associated rule).
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml`:
- Around line 1355-1358: The CEL immutability rule inside the
x-kubernetes-validations for the cloudLoadBalancerConfig object incorrectly
references self.cloudLoadBalancerConfig.dnsType and
oldSelf.cloudLoadBalancerConfig.dnsType (which don't exist because self is
already the cloudLoadBalancerConfig), so the dnsType immutability check is
ineffective; update the rule to use self.dnsType and oldSelf.dnsType (i.e.,
replace both occurrences of self.cloudLoadBalancerConfig.dnsType and
oldSelf.cloudLoadBalancerConfig.dnsType with self.dnsType and oldSelf.dnsType)
so the dnsType immutability is actually enforced for cloudLoadBalancerConfig.
- Around line 1938-1941: The CEL immutability rule is using incorrect scoping
(it references self.cloudLoadBalancerConfig which doesn't exist at this scope);
update the rule for cloudLoadBalancerConfig.dnsType to compare dnsType directly
on self and oldSelf, e.g. replace "oldSelf.cloudLoadBalancerConfig.dnsType == ''
|| self.cloudLoadBalancerConfig.dnsType ==
oldSelf.cloudLoadBalancerConfig.dnsType" with "oldSelf.dnsType == '' ||
self.dnsType == oldSelf.dnsType" so the rule uses self.dnsType and
oldSelf.dnsType.
In
`@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yaml`:
- Around line 1641-1645: The CEL immutability rule in the
cloudLoadBalancerConfig x-kubernetes-validations block is referencing
nonexistent nested fields (self.cloudLoadBalancerConfig.dnsType /
oldSelf.cloudLoadBalancerConfig.dnsType) so the guard always short-circuits;
update the rule to use the correct scope by replacing
self.cloudLoadBalancerConfig.dnsType with self.dnsType and
oldSelf.cloudLoadBalancerConfig.dnsType with oldSelf.dnsType (mirror the
existing clusterHosted guard pattern) in both the AWS and GCP
cloudLoadBalancerConfig validation blocks so the immutability check actually
enforces dnsType.
In
`@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yaml`:
- Around line 1641-1645: The CEL immutability rule inside the
cloudLoadBalancerConfig block incorrectly references nested fields
(self.cloudLoadBalancerConfig.dnsType and
oldSelf.cloudLoadBalancerConfig.dnsType) which are out of scope; update the rule
in both AWS and GCP cloudLoadBalancerConfig blocks so it compares dnsType
directly (use self.dnsType and oldSelf.dnsType) and keep the rest of the
expression (e.g., oldSelf.dnsType == '' || self.dnsType == oldSelf.dnsType)
intact to enforce immutability once set.
- Around line 1835-1836: The dnsType description currently mixes platform
behaviors across blocks; update the dnsType description entries so each platform
block only states that platform's behavior: in the Azure dnsType description
remove the sentence about AWS/GCP immutability and state only that Azure values
can be updated after install, and in the GCP (and AWS) dnsType descriptions
remove any mention of Azure updateability and state only that the value is
immutable after install for those platforms; locate the descriptions by
searching for the dnsType CRD description text and the Azure/GCP platform
headings to make the edits.
In
`@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yaml`:
- Around line 1885-1889: The CEL immutability rule incorrectly dereferences
cloudLoadBalancerConfig inside a scope where self/oldSelf already refer to
cloudLoadBalancerConfig; update the rule that currently uses
self.cloudLoadBalancerConfig.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType
to reference dnsType directly (e.g., rule: oldSelf.dnsType == '' || self.dnsType
== oldSelf.dnsType) so the check correctly enforces immutability of dnsType
within the cloudLoadBalancerConfig validation block.
In
`@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yaml`:
- Around line 1641-1645: The CEL immutability rule is referencing
self.cloudLoadBalancerConfig.dnsType and oldSelf.cloudLoadBalancerConfig.dnsType
from within the cloudLoadBalancerConfig object scope, which makes the path
invalid; update the expressions to use the correct relative field paths (e.g.
replace self.cloudLoadBalancerConfig.dnsType and
oldSelf.cloudLoadBalancerConfig.dnsType with self.dnsType and oldSelf.dnsType)
so the rule on cloudLoadBalancerConfig.dnsType evaluates the intended field
(also apply the same change for the duplicated occurrences around lines
2230-2234).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (18)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/AWSClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/AzureClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/GCPClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AWSClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AzureClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/GCPClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**openapi/openapi.jsonis excluded by!openapi/**
📒 Files selected for processing (17)
config/v1/tests/infrastructures.config.openshift.io/AWSClusterHostedDNS.yamlconfig/v1/tests/infrastructures.config.openshift.io/AWSClusterHostedDNSInstall.yamlconfig/v1/tests/infrastructures.config.openshift.io/AzureClusterHostedDNSInstall.yamlconfig/v1/tests/infrastructures.config.openshift.io/GCPClusterHostedDNS.yamlconfig/v1/tests/infrastructures.config.openshift.io/GCPClusterHostedDNSInstall.yamlconfig/v1/types_infrastructure.goconfig/v1/zz_generated.swagger_doc_generated.gopayload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-Default.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-OKD.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (7)
- config/v1/tests/infrastructures.config.openshift.io/AWSClusterHostedDNS.yaml
- config/v1/tests/infrastructures.config.openshift.io/GCPClusterHostedDNS.yaml
- config/v1/tests/infrastructures.config.openshift.io/AzureClusterHostedDNSInstall.yaml
- config/v1/zz_generated.swagger_doc_generated.go
- payload-manifests/crds/0000_10_config-operator_01_infrastructures-OKD.crd.yaml
- config/v1/tests/infrastructures.config.openshift.io/GCPClusterHostedDNSInstall.yaml
- payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yaml
JoelSpeed
reviewed
Feb 26, 2026
config/v1/types_infrastructure.go
Outdated
| // | ||
| // +default={"dnsType": "PlatformDefault"} | ||
| // +kubebuilder:default={"dnsType": "PlatformDefault"} | ||
| // +kubebuilder:validation:XValidation:rule="oldSelf.cloudLoadBalancerConfig.dnsType == '' || self.cloudLoadBalancerConfig.dnsType == oldSelf.cloudLoadBalancerConfig.dnsType",message="cloudLoadBalancerConfig.dnsType is immutable once set" |
Contributor
There was a problem hiding this comment.
Why not put this directly on the CloudLoadBalancerConfig struct?
Contributor
Author
There was a problem hiding this comment.
@JoelSpeed It was in the CloudLoadBalancerConfig when the behavior was common for AWS, Azure and GCP. https://github.com/openshift/api/blob/master/config/v1/types_infrastructure.go#L888
Now, we want to support allowing DNSType to be changed on Day-2 for Azure. So, this validation had to be moved from the common location to a platform specific location for AWS and GCP.
sadasu
force-pushed
the
dnstype-updates
branch
from
45ce17c to
3a8beaf
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
payload-manifests/crds/0000_10_config-operator_01_infrastructures-OKD.crd.yaml (1)
1171-1253:⚠️ Potential issue | 🔴 CriticalAWS platformStatus is missing cloudLoadBalancerConfig in OKD (and Default) CRD variant.
The Go type
AWSPlatformStatusincludes aCloudLoadBalancerConfigfield with thednsTypeimmutability validation rule, and this field is already present in the TechPreviewNoUpgrade variant of the CRD. However, the OKD variant (lines 1171-1253) only definesregion,resourceTags, andserviceEndpointsproperties. ThecloudLoadBalancerConfigfield is missing and should be added to match the Go type definition and maintain consistency with other CRD variants. GCP platformStatus already includes this field in the OKD variant with the same immutability validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_10_config-operator_01_infrastructures-OKD.crd.yaml` around lines 1171 - 1253, The AWS OKD CRD variant is missing the cloudLoadBalancerConfig property on AWSPlatformStatus; add a cloudLoadBalancerConfig object property (name: cloudLoadBalancerConfig) under the aws.properties block to match the Go type AWSPlatformStatus and the TechPreviewNoUpgrade CRD variant, include a dnsType string property with the same immutability validation (x-kubernetes-validations rule enforcing immutability) and allowed pattern/value constraints used in the other CRD variants, and ensure the new property is typed as an object and placed alongside region/resourceTags/serviceEndpoints so validation and consistency with AWSPlatformStatus and the TechPreviewNoUpgrade variant are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yaml`:
- Around line 1869-1870: The description for cloudLoadBalancerConfig in the
ControllerConfig CRD incorrectly references platform-specific immutability for
AWS and Azure; update the ControllerConfig CRD description for the field
cloudLoadBalancerConfig to either explicitly state it is GCP-specific (e.g.,
"cloudLoadBalancerConfig (GCP only) and is immutable after install") or remove
the AWS/Azure immutability/update language entirely so the description only
reflects supported platforms for ControllerConfig; locate the
cloudLoadBalancerConfig description string in the ControllerConfig CRD and
adjust wording to match actual CRD fields and platform support.
In
`@payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yaml`:
- Around line 1641-1644: The current validation uses "has(self.dnsType) &&
has(oldSelf.dnsType) ? ..." which allows bypass by clearing
cloudLoadBalancerConfig; change the predicate to check the old value instead:
replace the rule string in both cloudLoadBalancerConfig validation blocks (the
ones referencing dnsType) with "has(oldSelf.dnsType) ? (oldSelf.dnsType == '' ||
self.dnsType == oldSelf.dnsType) : true" so that if an existing dnsType was set
it cannot be changed (even via a null transition) for both AWS and GCP blocks.
---
Outside diff comments:
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-OKD.crd.yaml`:
- Around line 1171-1253: The AWS OKD CRD variant is missing the
cloudLoadBalancerConfig property on AWSPlatformStatus; add a
cloudLoadBalancerConfig object property (name: cloudLoadBalancerConfig) under
the aws.properties block to match the Go type AWSPlatformStatus and the
TechPreviewNoUpgrade CRD variant, include a dnsType string property with the
same immutability validation (x-kubernetes-validations rule enforcing
immutability) and allowed pattern/value constraints used in the other CRD
variants, and ensure the new property is typed as an object and placed alongside
region/resourceTags/serviceEndpoints so validation and consistency with
AWSPlatformStatus and the TechPreviewNoUpgrade variant are preserved.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (18)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/AWSClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/AzureClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/GCPClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AWSClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AzureClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/GCPClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**openapi/openapi.jsonis excluded by!openapi/**
📒 Files selected for processing (17)
config/v1/tests/infrastructures.config.openshift.io/AWSClusterHostedDNS.yamlconfig/v1/tests/infrastructures.config.openshift.io/AWSClusterHostedDNSInstall.yamlconfig/v1/tests/infrastructures.config.openshift.io/AzureClusterHostedDNSInstall.yamlconfig/v1/tests/infrastructures.config.openshift.io/GCPClusterHostedDNS.yamlconfig/v1/tests/infrastructures.config.openshift.io/GCPClusterHostedDNSInstall.yamlconfig/v1/types_infrastructure.goconfig/v1/zz_generated.swagger_doc_generated.gopayload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-Default.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-OKD.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- config/v1/tests/infrastructures.config.openshift.io/GCPClusterHostedDNS.yaml
- config/v1/tests/infrastructures.config.openshift.io/AWSClusterHostedDNS.yaml
- config/v1/tests/infrastructures.config.openshift.io/AWSClusterHostedDNSInstall.yaml
- payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-OKD.crd.yaml
payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yaml
Outdated
Show resolved
Hide resolved
payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yaml
Outdated
Show resolved
Hide resolved
sadasu
changed the title
Currently, the DNSType is immutable once set. ARO needs to enable this capability of starting in-cluster DNS on Day-2. That would require changing the value of DNSType on Day-2 to `ClusterHosted`. Similarly, they would also like the option to disable this feature on Day-2 as well.
sadasu
force-pushed
the
dnstype-updates
branch
from
3a8beaf to
24f92fd
Compare
Contributor
|
@sadasu: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
ⓘ Your monthly quota for Qodo has expired. Upgrade your plan ⓘ Paying users. Check that your Qodo account is linked with this Git user account |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, the DNSType is immutable once set. ARO needs to enable this capability of starting in-cluster DNS on Day-2. That would require changing the value of DNSType on Day-2 to
ClusterHosted. Similarly, they would also like the option to disable this feature on Day-2 as well.Currently, limiting this capability to Azure. DNSType is still left as immutable for AWS and GCP.