Skip to content

OCPBUGS-32275: Add ingress.spec.domain immutability validation#2695

Open
grzpiotrowski wants to merge 1 commit intoopenshift:masterfrom
grzpiotrowski:OCPBUGS-32275
Open

OCPBUGS-32275: Add ingress.spec.domain immutability validation#2695
grzpiotrowski wants to merge 1 commit intoopenshift:masterfrom
grzpiotrowski:OCPBUGS-32275

Conversation

@grzpiotrowski
Copy link
Contributor

This PR fixes 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.

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.
@qodo-code-review
Copy link

Review Summary by Qodo

Add immutability validation for Ingress spec.domain field

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• 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
Diagram
flowchart 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"]
Loading

Grey Divider

File Changes

1. config/v1/types_ingress.go ✨ Enhancement +2/-0

Add domain immutability validation rule

• Add kubebuilder:validation:XValidation rule to enforce domain immutability
• Rule allows empty initial value or unchanged domain value
• Mark domain field as optional with +optional annotation
• Validation message: "domain is immutable once set"

config/v1/types_ingress.go


2. openapi/generated_openapi/zz_generated.openapi.go ⚙️ Configuration changes +0/-1

Remove domain from required fields

• Remove domain from required fields list in IngressSpec schema
• Makes domain field optional in OpenAPI specification

openapi/generated_openapi/zz_generated.openapi.go


3. config/v1/tests/ingresses.config.openshift.io/AAA_ungated.yaml 🧪 Tests +57/-0

Add comprehensive domain immutability tests

• Add test for creating Ingress with domain set
• Add test for setting domain initially when empty
• Add test verifying domain cannot be changed once set with expected error
• Add test for updating other fields without changing domain

config/v1/tests/ingresses.config.openshift.io/AAA_ungated.yaml


View more (3)
4. config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses.crd.yaml ⚙️ Configuration changes +3/-0

Add domain validation to CRD manifest

• Add x-kubernetes-validations section to domain field
• Include validation rule: oldSelf == '' || self == oldSelf
• Set validation message: "domain is immutable once set"

config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses.crd.yaml


5. config/v1/zz_generated.featuregated-crd-manifests/ingresses.config.openshift.io/AAA_ungated.yaml ⚙️ Configuration changes +3/-0

Add domain validation to featuregated CRD

• Add x-kubernetes-validations section to domain field
• Include validation rule: oldSelf == '' || self == oldSelf
• Set validation message: "domain is immutable once set"

config/v1/zz_generated.featuregated-crd-manifests/ingresses.config.openshift.io/AAA_ungated.yaml


6. payload-manifests/crds/0000_10_config-operator_01_ingresses.crd.yaml ⚙️ Configuration changes +3/-0

Add domain validation to payload CRD manifest

• Add x-kubernetes-validations section to domain field
• Include validation rule: oldSelf == '' || self == oldSelf
• Set validation message: "domain is immutable once set"

payload-manifests/crds/0000_10_config-operator_01_ingresses.crd.yaml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Domain markers undocumented 📘 Rule violation ✓ Correctness
Description
• The Domain field now has +optional and an XValidation immutability rule, but the field
  comment does not explicitly document optional/omitted behavior or clearly describe the validation
  behavior in human-readable terms.
• This creates an undocumented API constraint for consumers (especially around whether omission vs
  empty string is allowed and when updates are permitted), which can lead to misunderstanding and
  incorrect client usage.
Code

config/v1/types_ingress.go[R45-48]

	// Once set, changing domain is not currently supported.
+	// +kubebuilder:validation:XValidation:rule="oldSelf == '' || self == oldSelf",message="domain is immutable once set"
+	// +optional
	Domain string `json:"domain"`
Evidence
Compliance rule 7 requires that any kubebuilder validation markers and +optional behavior be
documented in the field’s Go comment, including what happens when the field is omitted. The Domain
field includes +optional and an XValidation rule, but its comment only states immutability at a
high level and does not explain omitted/empty behavior or the precise update allowance implied by
the CEL expression.

AGENTS.md
config/v1/types_ingress.go[37-48]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `Domain` field has kubebuilder markers (`+optional` and `+kubebuilder:validation:XValidation`) but the Go field comment does not fully document what those markers mean, including behavior when the field is omitted.

## Issue Context
Compliance requires that every kubebuilder validation marker applied to a field be documented in the field comment, including optional vs required behavior (and what happens when omitted), and the meaning/behavior of the validation rule.

## Fix Focus Areas
- config/v1/types_ingress.go[37-48]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant