OCPBUGS-32275: Add ingress.spec.domain immutability validation #2695
Conversation
Review Summary by QodoAdd immutability validation for Ingress spec.domain field
WalkthroughsDescription• Add immutability validation for spec.domain field in Ingress CRD • Prevent domain changes after initial set to avoid cluster operator degradation • Make domain field optional in validation schema • Add comprehensive test cases for domain immutability behavior Diagramflowchart LR
A["Ingress spec.domain"] -->|Add XValidation rule| B["Immutability constraint"]
B -->|oldSelf == '' OR self == oldSelf| C["Allow initial set or no change"]
C -->|Reject domain changes| D["Prevent operator degradation"]
A -->|Make optional| E["Updated CRD schema"]
F["Test cases"] -->|Validate behavior| G["Domain immutability verified"]
File Changes1. config/v1/types_ingress.go
|
Code Review by Qodo
1. Domain markers undocumented
|
This commit fixes OCPBUGS-32275. https://issues.redhat.com/browse/OCPBUGS-32275 Adds `spec.domain` field in the Ingress config CRD validation to make it immutable and match the documentation. Prior to this commit the domain value could be changed and cause degraded state of some cluster operators.
grzpiotrowski
force-pushed
the
OCPBUGS-32275
branch
from
1057ced to
a58ab73
Compare
📝 WalkthroughWalkthroughThis pull request implements immutability validation for the Ingress domain field. The changes include adding a kubebuilder validation annotation to the Domain field in the IngressSpec type definition, updating the CRD schema validation to include x-kubernetes-validations enforcing that the domain field cannot be changed after initial creation, and adding test cases that verify domain immutability behavior and validate the error message when attempting to update the domain. 🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
bot
added
the
size/M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@config/v1/tests/ingresses.config.openshift.io/AAA_ungated.yaml`:
- Around line 26-38: Add a new onUpdate test case named "Should not be able to
set domain if initially empty" that uses an initial Ingress manifest with spec:
{} and an updated manifest that sets spec.domain to apps.example.com, and assert
expectedError: "domain is immutable once set"; place this case alongside the
existing "Should not be able to change domain once set" entry so the suite
verifies the transition from empty→set and ensures the validation rule for
immutability-once-set is covered.
In `@payload-manifests/crds/0000_10_config-operator_01_ingresses.crd.yaml`:
- Around line 132-134: The current validation rule "self == oldSelf" (under
x-kubernetes-validations) prevents any change including setting domain from
empty; update the rule to allow initial population by changing it to something
like "oldSelf.domain == null || oldSelf.domain == '' || self == oldSelf" so the
domain can be set when previously empty but remains immutable afterwards;
alternatively, if domain is always set at creation, simply reword the message to
"domain is immutable" instead of implying an initial set is allowed.
|
[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 |
|
/test verify |
|
PR-Agent: could not fine a component named |
|
@grzpiotrowski: 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. |
|
/jira refresh |
openshift-ci-robot
added
jira/severity-important
|
@grzpiotrowski: This pull request references Jira Issue OCPBUGS-32275, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
/jira refresh |
openshift-ci-robot
added
the
jira/valid-reference
|
@grzpiotrowski: This pull request references Jira Issue OCPBUGS-32275, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
|
/jira refresh |
|
@grzpiotrowski: An error was encountered querying GitHub for users with public email (hongli@redhat.com) for bug OCPBUGS-32275 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details. Full error message.
non-200 OK status code: 429 Too Many Requests body: "{\"message\":\"This endpoint is temporarily being throttled. Please try again later. For more on scraping GitHub and how it may affect your rights, please review our Terms of Service (https://docs.github.com/en/site-policy/github-terms/github-terms-of-service)\",\"documentation_url\":\"https://docs.github.com/graphql/using-the-rest-api/rate-limits-for-the-rest-api\",\"status\":\"429\"}"
Please contact an administrator to resolve this issue, then request a bug refresh with 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. |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@lihongan: This pull request references Jira Issue OCPBUGS-32275, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
|
/retest-required |
|
Not sure now why the e2e ci jobs don't run. Do they need to be triggered explicitly? |
|
Pre-merge tested and got below Looks the |
Thank you. I agree it reads a bit confusing, but I don't think this is something configurable and this is a standard output for the Example: Enforce CRD Immutability with CEL Transition Rules: Immutability upon object creation |
3 participants
This PR fixes OCPBUGS-32275.
Adds
spec.domainfield validation in the Ingress config CRD to make it immutable and match the documentation. Prior to this commit the domain value could be changed and cause degraded state of some cluster operators.