WIP: CORS-4317: Add IPFamily to Ingress API to enable DualStack installs#2663
WIP: CORS-4317: Add IPFamily to Ingress API to enable DualStack installs#2663sadasu 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
|
@sadasu: This pull request references CORS-4317 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. |
|
Hello @sadasu! Some important instructions when contributing to openshift/api: |
📝 WalkthroughWalkthroughAdds an 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
sadasu
changed the title
openshift-ci
bot
added
the
size/XXL
|
@sadasu: This pull request references CORS-4317 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. |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||||
|
[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 |
config/v1/types_ingress.go
Outdated
| // +openshift:enable:FeatureGate=AWSDualStackInstall | ||
| // +openshift:enable:FeatureGate=AzureDualStackInstall | ||
| // +optional | ||
| IPFamily IPFamilyType `json:"ipFamily,omitempty"` |
There was a problem hiding this comment.
In the case of AWS, I think the IPFamily (or at least the 2 dual-stack variants) is only applicable for Network Load Balancer, right? Classic Load Balancers can only be IPV4.
Should we validate if spec.loadBalancer.platform.aws.type is "NLB" here or elsewhere?
There was a problem hiding this comment.
Should we validate if
spec.loadBalancer.platform.aws.typeis"NLB"here or elsewhere?
That validation will happen in the Installer so when the Ingress manifest is created on Day-), we have the right values. But, I am not sure if the LB type can be changed on Day-2. If that is allowed, then we should disallow changing the LBType to Classic when IPfamily is DualStackIPv6Primary.
What about DualStackIPv4Primary? Can the LBType be Classic in that case? @tthvo @alebedev87
There was a problem hiding this comment.
As the field description says:
This field determines whether the load balancer usesIPv4-only, or dual-stack networking with IPv4 or IPv6 as the primary protocol family.
I think it may make users incorrectly expect that classic load balancer will be somehow "dual-stack" in the below snippet:
spec:
ipFamily: DualStackIPv4Primary
loadBalancer:
platform:
aws:
type: Classic
type: AWSThe idea of "dual-stack" is not applicable to Classic load balancers at all on AWS (docs). Thus, I think we should keep the classic load balancer within only "IPv4" ipFamily scope.
There was a problem hiding this comment.
But, I am not sure if the LB type can be changed on Day-2. If that is allowed, then we should disallow changing the LBType to Classic when IPfamily is DualStackIPv6Primary.
I think it is possible to switch the LB type day-2 (reading from this enhancement) as it just holds the "default" LB type for ingress controllers (possible override at individual ingress controller CR).
So, we should block "Classic" for both dual-stack variants (see above reasoning)...
There was a problem hiding this comment.
In the case of AWS, I think the IPFamily (or at least the 2 dual-stack variants) is only applicable for Network Load Balancer, right? Classic Load Balancers can only be IPV4.
Right, the dual stack family can only be implemented on load balancers of type NLB.
I think it is possible to switch the LB type day-2 (reading from this enhancement) as it just holds the "default" LB type for ingress controllers (possible override at individual ingress controller CR).
Right, it's possible to switch the LB type from Classic to NLB and vice versa as a day2 operation.
So, we should block "Classic" for both dual-stack variants (see above reasoning)...
Currently the day2 IngressController API has separate fields for Classic and NLB parameters. I was thinking of adding the IP family (or IP address type as it's called in ALBC) field only under the NLB specific parameters. This way the dual stack can only be set on the type where it's supported: NLB.
Now talking about the Ingress config API, first I think that ipFamily makes sense only under spec.loadBalancer field as it defines a load balancer setting. However, it's not 100% evident where it should be placed exactly.
An idea can be to follow the approach similar to IngressController API and split Classic and NLB parameters, something like this:
spec:
loadBalancer:
platform:
type: AWS
aws:
type: NLB
networkLoadBalancer:
ipFamily: DualStackIPv4PrimaryIn this case, the validation of the installer config should disallow setting the IPFamily if the AWS LB type is Classic (still the default value as of now). Also, this approach would imply adding a new field into .spec.loadBalancer.platform for Azure.
|
/cc @alebedev87 @Miciah |
config/v1/types_ingress.go
Outdated
| // ingress load balancer. This field determines whether the load balancer uses | ||
| // IPv4-only, or dual-stack networking with IPv4 or IPv6 as the primary | ||
| // protocol family. | ||
| // Valid values are "IPv4", "DualStackIPv6Primary", and "DualStackIPv4Primary". |
There was a problem hiding this comment.
I don't think that 2 distinct DualStack flavors can be applied to the AWS load balancers. For AWS NLB, there is only dualstack type without a possibility to specify which family is primary. Azure may be different though as it relies on Kubernetes service fields like service.spec.ipFamilyPolicy and service.spec.ipFamilies. I'm wondering whether the order in the ipFamilies field mattes for Azure to decide which family is primary. cc @nrb for details in Azure.
There was a problem hiding this comment.
We can make the AWS implementation also use the service.spec.ipFamilyPolicy and service.spec.ipFamilies fields; I initially started it with annotations to match ALBC, but I'm wondering if perhaps we should instead focus on just configuring service.spec fields and not use annotations. ALBC uses annotations because it is not part of the core Kube API, only acting on Services with the correct LoadBalancerClass value, while Azure's CCM uses the service.spec fields directly.
Based on https://kubernetes.io/docs/concepts/services-networking/dual-stack/#services, I think the following combinations need to be supported to comply.
| service.spec.ipFamilyPolicy | service.spec.ipFamilies | OCP's ipFamily |
|---|---|---|
| SingleStack | ["IPv4"] | IPv4 |
| PreferDualStack | ["IPv4", "IPv6"] | DualStackIPv4Primary |
| RequireDualStack | ["IPv4", "IPv6"] | DualStackIPv4Primary |
| PreferDualStack | ["IPv6", "IPv4"] | DualStackIPv6Primary |
| RequireDualStack | ["IPv6", "IPv4"] | DualStackIPv6Primary |
Note: the difference between PreferDualStack and RequireDualStack is failure mode - PreferDualStack will try to allocate two IPs but fall back to IPv4 if it fails, while RequireDualStack will fail if two IPs cannot be allocated for whatever reason.
service.spec.ipFamilyPolicy and service.spec.ipFamilies can be set per Service according to the Kubernetes API. That means that OCP is defining the default behavior with its ipFamily field, but per Kubernetes, individual load balancer could vary.
I'm wondering how we want to define OCP's behavior in that case - is defining a ipFamily for an OCP cluster setting the defaults for each new service? Is it a "maximum" supported configuration - for example, if I configure OCP to use DualStackIPv6Primary, could I then create a Service with spec.SingleStack in order for it to be IPv4 only? Or are we going to say that OCP's ipFamily configuration is the only configuration that Services within a cluster can support?
There was a problem hiding this comment.
I'm wondering how we want to define OCP's behavior in that case - is defining a ipFamily for an OCP cluster setting the defaults for each new service? Is it a "maximum" supported configuration - for example, if I configure OCP to use DualStackIPv6Primary, could I then create a Service with spec.SingleStack in order for it to be IPv4 only? Or are we going to say that OCP's ipFamily configuration is the only configuration that Services within a cluster can support?
This is an interesting angle. I was thinking in the context of this PR. Which would answer your question as "no, the dual stack config is set in Ingress Config API so only the service in front of the router will be impacted". However using service.spec.ipFamilyPolicy and service.spec.ipFamilies for the AWS CCM load balancer provisioning kinda ties us to the core networking because SingleStack policy is assigned by default (Kubernetes doc):
When you create this Service, Kubernetes assigns a cluster IP for the Service from the first configured service-cluster-ip-range and sets the .spec.ipFamilyPolicy to SingleStack.
I'm wondering whether we really need a dedicated API configuration only for the OCP ingress.
sadasu
force-pushed
the
ingress-ipfamily
branch
from
9d63e2e to
b3d7f7c
Compare
openshift-ci
bot
added
size/L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@openapi/generated_openapi/zz_generated.openapi.go`:
- Around line 9156-9162: The ipFamily description in the generated schema
contains a grammar/repetition bug ("supported with only with"); update the
source doc string in the ingress API docs (e.g., the ipFamily comment in
config/v1/types_ingress.go or the corresponding entry in
config/v1/zz_generated.swagger_doc_generated.go) to read "supported only with
Network Load Balancer" for both "DualStackIPv4Primary" and
"DualStackIPv6Primary" entries, then regenerate the OpenAPI output so
zz_generated.openapi.go (the SchemaProps for ipFamily) is rebuilt with the
corrected sentence; ensure the rest of the description and quoting remain
unchanged.
|
@sadasu: This pull request references CORS-4317 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. |
|
/retitle WIP: CORS-4317: Add IPFamily to Ingress API to enable DualStack installs |
openshift-ci
bot
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
sadasu
force-pushed
the
ingress-ipfamily
branch
from
b3d7f7c to
3f9d0a8
Compare
openshift-ci
bot
added
size/XXL
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml`:
- Around line 165-192: Add a cross-field x-kubernetes-validations rule into the
CRD so the AWS Classic LB disallows DualStack: under the same object that
defines aws/type (the AWSIngressSpec section that maps to
spec.loadBalancer.platform.aws) add an x-kubernetes-validations entry with a
rule that mirrors the Go validation: rule="has(self.type) && self.type ==
'Classic' ? (!has(../../ipFamily) || ../../ipFamily == 'IPv4') : true" and
message="when type is Classic, ipFamily must be IPv4 or omitted (defaults to
IPv4)"; reference the fields ipFamily and aws.type (the existing type property)
so the CRD enforces the same constraint as config/v1/types_ingress.go.
payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml
Show resolved
Hide resolved
sadasu
force-pushed
the
ingress-ipfamily
branch
from
3f9d0a8 to
9dc4404
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@openapi/generated_openapi/zz_generated.openapi.go`:
- Around line 9156-9158: The ipFamily description doc string contains two
DualStack bullets on the same line; update the source comment that documents the
ipFamily field (the doc string in the ingress types, e.g., the comment for
IpFamily in config/v1/types_ingress.go or the related swagger doc entry) to
insert a newline before the `"DualStackIPv6Primary"` bullet so each bullet is on
its own line, then regenerate the OpenAPI output so zz_generated.openapi.go
reflects the corrected multi-line list.
In
`@payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml`:
- Around line 193-195: Update the CRD source marker comment that emits the CEL
validation for the LoadBalancer/AWS platform in the types_ingress.go declaration
so the generated rule references self.ipFamily (not ../../ipFamily); change the
text of the generator comment attached to the LoadBalancer/AWS platform field to
produce the expression has(self.type) && self.type == "Classic" ?
(!has(self.ipFamily) || self.ipFamily == "IPv4") : true so the CRD generator
outputs self.ipFamily in the x-kubernetes-validations rule.
payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml
Outdated
Show resolved
Hide resolved
sadasu
force-pushed
the
ingress-ipfamily
branch
from
9dc4404 to
edc8fdf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@payload-manifests/crds/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml`:
- Around line 133-152: The CRD places ipFamily at the top-level spec but the Go
type defines IPFamily on AWSIngressSpec, breaking the intended validation that
references self.type (Classic LB) — either move ipFamily into the AWS-specific
spec or update Go types; specifically either (A) modify the CRD to nest ipFamily
under spec.loadBalancer.platform.aws to match the Go struct AWSIngressSpec and
preserve the existing x-kubernetes-validations that compare self.type, or (B)
change the Go type definitions (IngressSpec) to add IPFamily at the cluster/spec
level and update the validation in config/v1/types_ingress.go so the Classic LB
check references the correct field; pick the intended design and keep ipFamily
and the Classic LB x-validation colocated (same struct) so the validation rule
that checks type/self.type remains valid.
payload-manifests/crds/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml
Show resolved
Hide resolved
Add the IPFamliy field to Ingress API's IngressSpec that enables Day 0 configuration of the default Ingress controller with DualStack configuration. This field defaults to IPv4 when not set. And when the LB Type is "Classic", IPFamily can only have the value "IPv4". Only LB Type of "NLB" is allowed for IPFamily values of "DualStackIPv4Primary" and "DualStackIPv6Primary".
sadasu
force-pushed
the
ingress-ipfamily
branch
from
edc8fdf to
2a3253c
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@config/v1/types_ingress.go`:
- Around line 156-171: Add a field-level immutability XValidation annotation to
the IPFamily field in AWSIngressSpec (IPFamily IPFamilyType
`json:"ipFamily,omitempty"`) so updates after creation are rejected, mirroring
the same XValidation used for ipFamily in config/v1/types_infrastructure.go;
locate the IPFamily declaration in types_ingress.go and add the same kubebuilder
XValidation expression/annotation that enforces "immutable after creation" for
this field.
In
`@payload-manifests/crds/0000_10_config-operator_01_ingresses-Default.crd.yaml`:
- Around line 145-173: Add the missing aws.ipFamily property to the CRD schema
under the aws object so the field spec.loadBalancer.platform.aws.ipFamily is
preserved; define ipFamily as a string with the same enum values used in the Go
type (e.g., "IPv4", "IPv6", "DualStack" or whatever exact enum names the API
uses) and add the same CEL validation rule that enforces the Classic-type
constraint (the existing Classic rule applied to aws.type) so the CRD validation
matches the API (reference aws.type and spec.loadBalancer.platform.aws.ipFamily
when adding the property and rule).
In
`@payload-manifests/crds/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml`:
- Around line 133-192: Add an x-kubernetes-validations block right after the aws
object definition (the object with required: [type] and type: object) that
enforces: if aws.type == "Classic" then ipFamily must be either omitted or
"IPv4". Target the aws/type property under loadBalancer.platform.aws and
implement the validation rule so it returns true for all other cases and only
fails when type is Classic and ipFamily is set to a dual-stack value.
In `@payload-manifests/crds/0000_10_config-operator_01_ingresses-OKD.crd.yaml`:
- Around line 145-172: The aws ingress schema only defines the "type" property
and is missing the new "ipFamily" field (so
spec.loadBalancer.platform.aws.ipFamily will be rejected); add an optional
"ipFamily" property under the aws properties with a descriptive "description",
type: string, and an enum of the allowed values (e.g., "IPv4", "IPv6",
"DualStack") to match the Go/OpenAPI types, do not add it to the required list,
and then regenerate the CRD manifests so the YAML aligns with the updated types.
In
`@payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml`:
- Around line 133-195: The ipFamily field is defined at the top spec level but
the CEL validation in the aws object expects self.ipFamily; move the ipFamily
definition into the aws properties so it becomes loadBalancer -> platform -> aws
-> properties -> ipFamily (preserve the description, enum values
["IPv4","DualStackIPv6Primary","DualStackIPv4Primary"], type: string, and
immutability x-kubernetes-validations if needed), and remove the duplicate
top-level ipFamily block; this aligns the CRD with the Go AWSIngressSpec and
activates the CEL rule that references self.ipFamily for the aws.type
validation.
🧹 Nitpick comments (1)
payload-manifests/crds/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml (1)
150-152: Immutability validation rule contains unreachable condition.The condition
oldSelf == ''will never be true because:
- The field has
default: IPv4, so it's never stored as an empty string- The enum does not include empty string as a valid value
The immutability enforcement still works via
self == oldSelf, but the empty string check is dead code and suggests a misunderstanding about how defaults work with transition rules.🔧 Suggested fix to simplify the rule
x-kubernetes-validations: - message: ipFamily is immutable once set - rule: oldSelf == '' || self == oldSelf + rule: self == oldSelf
| // ipFamily allows users to specify the IP protocol Families that the Load Balancer service supports. | ||
| // The default ingress controller uses this information to configure its Load Balancer. | ||
| // Valid values are: | ||
| // | ||
| // * "IPv4": Only value supported with a "Classic" Load Balancer. When type is "Classic", | ||
| // this field must be set to "IPv4" or omitted (which defaults to "IPv4"). | ||
| // | ||
| // * "DualStackIPv4Primary": supported only with Network Load Balancer. | ||
| // | ||
| // * "DualStackIPv6Primary": supported only with Network Load Balancer. | ||
| // | ||
| // +default="IPv4" | ||
| // +kubebuilder:validation:Enum="IPv4";"DualStackIPv4Primary";"DualStackIPv6Primary" | ||
| // +optional | ||
| // +unionMember,optional | ||
| IPFamily IPFamilyType `json:"ipFamily,omitempty"` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "types_ingress.go" 2>/dev/null | head -5Repository: openshift/api
Length of output: 39
🏁 Script executed:
find . -name "types_ingress.go" -type f | head -10Repository: openshift/api
Length of output: 116
🏁 Script executed:
cat -n config/v1/types_ingress.go | sed -n '150,175p'Repository: openshift/api
Length of output: 1218
🏁 Script executed:
# Search for PR objectives or comments about immutability in commit messages or comments
rg -i "immutable|immutability" config/v1/types_ingress.go -B 3 -A 3Repository: openshift/api
Length of output: 39
🏁 Script executed:
# Look for other XValidation rules in the same file to understand the pattern
rg "XValidation" config/v1/types_ingress.go -B 2 -A 2Repository: openshift/api
Length of output: 440
🏁 Script executed:
# Check if there are comments or documentation about immutability in the PR or nearby context
rg -i "immutable" config/v1/ -l | head -10Repository: openshift/api
Length of output: 1191
🏁 Script executed:
# Look for other field-level XValidation rules to understand the pattern
rg "XValidation.*rule.*oldSelf" config/ -B 2 -A 1 | head -40Repository: openshift/api
Length of output: 4407
🏁 Script executed:
# Search in operator directory for comparison
rg "XValidation.*oldSelf" operator/ -B 2 -A 1 | head -40Repository: openshift/api
Length of output: 4702
🏁 Script executed:
# Check the broader context around AWSIngressSpec to understand immutability intent
cat -n config/v1/types_ingress.go | sed -n '100,180p'Repository: openshift/api
Length of output: 4443
🏁 Script executed:
# Search for any documentation or design comments about immutability
rg -i "day.?2|immutable|update|modify" config/v1/types_ingress.go | head -20Repository: openshift/api
Length of output: 397
🏁 Script executed:
# Look for any issues or design docs referenced in comments
rg -i "design|issue|bug|rfc" config/v1/types_ingress.go | head -10Repository: openshift/api
Length of output: 307
🏁 Script executed:
# Check git history for context on this addition (recent commits)
git log --oneline config/v1/types_ingress.go | head -10Repository: openshift/api
Length of output: 94
🏁 Script executed:
# Search for related discussions about immutability in AWS ingress context
rg -i "ipfamily.*immut|immut.*ipfamily" . 2>/dev/null | head -10Repository: openshift/api
Length of output: 1705
🏁 Script executed:
# Check the actual generated CRD manifest for ingresses to see current validation
cat payload-manifests/crds/0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml | grep -A 20 "ipFamily" | head -40Repository: openshift/api
Length of output: 2388
🏁 Script executed:
# Verify the exact XValidation rule in the manifest
rg "ipFamily is immutable" payload-manifests/crds/0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml -B 5 -A 2Repository: openshift/api
Length of output: 371
🏁 Script executed:
# Check if there are other IPFamily fields in infrastructure types with immutability
rg "ipFamily.*immutable|immutable.*ipFamily" config/ -B 3 -A 3Repository: openshift/api
Length of output: 17670
🏁 Script executed:
# Search for IPFamilyType definition to understand the context
rg "type IPFamilyType" config/ -B 2 -A 5Repository: openshift/api
Length of output: 644
Add field-level immutability validation to ipFamily to enforce API consistency.
The ipFamily field in AWSIngressSpec lacks the XValidation rule enforced in similar fields elsewhere in the API. Infrastructure platform specs define identical ipFamily fields with immutability rules (see config/v1/types_infrastructure.go), and test cases expect this validation to prevent Day-2 updates. Add the rule to match the established pattern.
🛠️ Suggested fix
// +default="IPv4"
// +kubebuilder:validation:Enum="IPv4";"DualStackIPv4Primary";"DualStackIPv6Primary"
+// +kubebuilder:validation:XValidation:rule="oldSelf == '' || self == oldSelf",message="ipFamily is immutable once set"
// +optional
// +unionMember,optional
IPFamily IPFamilyType `json:"ipFamily,omitempty"`📝 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.
| // ipFamily allows users to specify the IP protocol Families that the Load Balancer service supports. | |
| // The default ingress controller uses this information to configure its Load Balancer. | |
| // Valid values are: | |
| // | |
| // * "IPv4": Only value supported with a "Classic" Load Balancer. When type is "Classic", | |
| // this field must be set to "IPv4" or omitted (which defaults to "IPv4"). | |
| // | |
| // * "DualStackIPv4Primary": supported only with Network Load Balancer. | |
| // | |
| // * "DualStackIPv6Primary": supported only with Network Load Balancer. | |
| // | |
| // +default="IPv4" | |
| // +kubebuilder:validation:Enum="IPv4";"DualStackIPv4Primary";"DualStackIPv6Primary" | |
| // +optional | |
| // +unionMember,optional | |
| IPFamily IPFamilyType `json:"ipFamily,omitempty"` | |
| // ipFamily allows users to specify the IP protocol Families that the Load Balancer service supports. | |
| // The default ingress controller uses this information to configure its Load Balancer. | |
| // Valid values are: | |
| // | |
| // * "IPv4": Only value supported with a "Classic" Load Balancer. When type is "Classic", | |
| // this field must be set to "IPv4" or omitted (which defaults to "IPv4"). | |
| // | |
| // * "DualStackIPv4Primary": supported only with Network Load Balancer. | |
| // | |
| // * "DualStackIPv6Primary": supported only with Network Load Balancer. | |
| // | |
| // +default="IPv4" | |
| // +kubebuilder:validation:Enum="IPv4";"DualStackIPv4Primary";"DualStackIPv6Primary" | |
| // +kubebuilder:validation:XValidation:rule="oldSelf == '' || self == oldSelf",message="ipFamily is immutable once set" | |
| // +optional | |
| // +unionMember,optional | |
| IPFamily IPFamilyType `json:"ipFamily,omitempty"` |
🤖 Prompt for AI Agents
In `@config/v1/types_ingress.go` around lines 156 - 171, Add a field-level
immutability XValidation annotation to the IPFamily field in AWSIngressSpec
(IPFamily IPFamilyType `json:"ipFamily,omitempty"`) so updates after creation
are rejected, mirroring the same XValidation used for ipFamily in
config/v1/types_infrastructure.go; locate the IPFamily declaration in
types_ingress.go and add the same kubebuilder XValidation expression/annotation
that enforces "immutable after creation" for this field.
| aws: | ||
| description: aws contains settings specific to the Amazon | ||
| Web Services infrastructure provider. | ||
| properties: | ||
| type: | ||
| description: |- | ||
| type allows user to set a load balancer type. | ||
| When this field is set the default ingresscontroller will get created using the specified LBType. | ||
| If this field is not set then the default ingress controller of LBType Classic will be created. | ||
| Valid values are: | ||
|
|
||
| * "Classic": A Classic Load Balancer that makes routing decisions at either | ||
| the transport layer (TCP/SSL) or the application layer (HTTP/HTTPS). See | ||
| the following for additional details: | ||
|
|
||
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#clb | ||
|
|
||
| * "NLB": A Network Load Balancer that makes routing decisions at the | ||
| transport layer (TCP/SSL). See the following for additional details: | ||
|
|
||
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb | ||
| enum: | ||
| - NLB | ||
| - Classic | ||
| type: string | ||
| required: | ||
| - type | ||
| type: object | ||
| type: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the actual CRD file around the specified lines
cat -n payload-manifests/crds/0000_10_config-operator_01_ingresses-Default.crd.yaml | sed -n '145,173p'Repository: openshift/api
Length of output: 1930
🏁 Script executed:
# Check if ipFamily is mentioned anywhere in this CRD file
rg "ipFamily" payload-manifests/crds/0000_10_config-operator_01_ingresses-Default.crd.yamlRepository: openshift/api
Length of output: 39
🏁 Script executed:
# Search for aws.ipFamily references in Go code
rg "ipFamily" --type goRepository: openshift/api
Length of output: 21372
🏁 Script executed:
# Look for go types related to loadbalancer platform aws
rg -A 10 "type.*AWS" --type go | grep -A 10 "ipFamily"Repository: openshift/api
Length of output: 39
🏁 Script executed:
# Check for tests that reference aws.ipFamily
rg "aws.*ipFamily\|ipFamily.*aws" --type go -B 2 -A 2Repository: openshift/api
Length of output: 39
🏁 Script executed:
# Look at the AWS ingress type structure in config/v1/types_ingress.go
rg -B 5 -A 15 "type.*AWS.*struct" config/v1/types_ingress.goRepository: openshift/api
Length of output: 1332
🏁 Script executed:
# Get the full context of the ipFamily field definition in types_ingress.go
sed -n '1,300p' config/v1/types_ingress.go | tail -200Repository: openshift/api
Length of output: 9970
🏁 Script executed:
# Search for the complete AWS LoadBalancer field definition
rg -B 10 -A 20 "IPFamily.*IPFamilyType" config/v1/types_ingress.goRepository: openshift/api
Length of output: 1293
🏁 Script executed:
# Search entire CRD file for ipFamily
grep -n "ipFamily" payload-manifests/crds/0000_10_config-operator_01_ingresses-Default.crd.yamlRepository: openshift/api
Length of output: 39
AWS schema is missing ipFamily, so user input will be pruned.
The CRD only defines aws.type; the ipFamily field defined in the Go type (spec.loadBalancer.platform.aws.ipFamily) is absent from the schema and will be dropped on API requests. Add the ipFamily property with the enum values and the Classic-type CEL validation rule to match the API definition.
🛠️ Suggested fix (add aws.ipFamily + Classic rule)
aws:
description: aws contains settings specific to the Amazon
Web Services infrastructure provider.
properties:
type:
description: |-
type allows user to set a load balancer type.
@@
enum:
- NLB
- Classic
type: string
+ ipFamily:
+ default: IPv4
+ description: |-
+ ipFamily allows users to specify the IP protocol Families that the Load Balancer service supports.
+ The default ingress controller uses this information to configure its Load Balancer.
+ enum:
+ - IPv4
+ - DualStackIPv4Primary
+ - DualStackIPv6Primary
+ type: string
required:
- type
type: object
+ x-kubernetes-validations:
+ - message: when type is Classic, ipFamily must be IPv4 or omitted (defaults to IPv4)
+ rule: 'has(self.type) && self.type == ''Classic'' ? (!has(self.ipFamily) || self.ipFamily == ''IPv4'') : true'🤖 Prompt for AI Agents
In `@payload-manifests/crds/0000_10_config-operator_01_ingresses-Default.crd.yaml`
around lines 145 - 173, Add the missing aws.ipFamily property to the CRD schema
under the aws object so the field spec.loadBalancer.platform.aws.ipFamily is
preserved; define ipFamily as a string with the same enum values used in the Go
type (e.g., "IPv4", "IPv6", "DualStack" or whatever exact enum names the API
uses) and add the same CEL validation rule that enforces the Classic-type
constraint (the existing Classic rule applied to aws.type) so the CRD validation
matches the API (reference aws.type and spec.loadBalancer.platform.aws.ipFamily
when adding the property and rule).
| ipFamily: | ||
| default: IPv4 | ||
| description: |- | ||
| ipFamily specifies the IP protocol family that should be used for the | ||
| ingress load balancer. This field determines whether the load balancer uses | ||
| IPv4-only, or dual-stack networking with IPv4 or IPv6 as the primary | ||
| protocol family. | ||
| Valid values are "IPv4", "DualStackIPv6Primary", and "DualStackIPv4Primary". | ||
| When set to IPv4, the load balancer will use IPv4 addressing only. | ||
| When set to DualStackIPv6Primary, the load balancer will use dual-stack networking with IPv6 as primary. | ||
| When set to DualStackIPv4Primary, the load balancer will use dual-stack networking with IPv4 as primary. | ||
| When omitted, the default value is IPv4. | ||
| enum: | ||
| - IPv4 | ||
| - DualStackIPv6Primary | ||
| - DualStackIPv4Primary | ||
| type: string | ||
| x-kubernetes-validations: | ||
| - message: ipFamily is immutable once set | ||
| rule: oldSelf == '' || self == oldSelf | ||
| loadBalancer: | ||
| description: |- | ||
| loadBalancer contains the load balancer details in general which are not only specific to the underlying infrastructure | ||
| provider of the current cluster and are required for Ingress Controller to work on OpenShift. | ||
| properties: | ||
| platform: | ||
| description: |- | ||
| platform holds configuration specific to the underlying | ||
| infrastructure provider for the ingress load balancers. | ||
| When omitted, this means the user has no opinion and the platform is left | ||
| to choose reasonable defaults. These defaults are subject to change over time. | ||
| properties: | ||
| aws: | ||
| description: aws contains settings specific to the Amazon | ||
| Web Services infrastructure provider. | ||
| properties: | ||
| type: | ||
| description: |- | ||
| type allows user to set a load balancer type. | ||
| When this field is set the default ingresscontroller will get created using the specified LBType. | ||
| If this field is not set then the default ingress controller of LBType Classic will be created. | ||
| Valid values are: | ||
|
|
||
| * "Classic": A Classic Load Balancer that makes routing decisions at either | ||
| the transport layer (TCP/SSL) or the application layer (HTTP/HTTPS). See | ||
| the following for additional details: | ||
|
|
||
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#clb | ||
|
|
||
| * "NLB": A Network Load Balancer that makes routing decisions at the | ||
| transport layer (TCP/SSL). See the following for additional details: | ||
|
|
||
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb | ||
| enum: | ||
| - NLB | ||
| - Classic | ||
| type: string | ||
| required: | ||
| - type | ||
| type: object |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's understand the file structure
echo "=== Current file location ==="
ls -la payload-manifests/crds/0000_10_config-operator_01_ingresses*.yaml 2>/dev/null | head -20
echo ""
echo "=== Searching for validation rules about Classic LB and ipFamily ==="
rg -n 'Classic.*ipFamily|ipFamily.*Classic' --type yaml --type go -i 2>/dev/null | head -30
echo ""
echo "=== Looking for x-kubernetes-validations patterns in ingresses CRDs ==="
rg -n 'x-kubernetes-validations' payload-manifests/crds/ --type yaml -A 3 2>/dev/null | grep -E 'ingresses|Classic|ipFamily' -B 2 -A 2 | head -50
echo ""
echo "=== Check if there are multiple variants of ingresses CRD ==="
find payload-manifests/crds/ -name '*ingresses*.yaml' -exec basename {} \;
echo ""
echo "=== Search for load balancer type validation in the codebase ==="
rg -n 'type.*Classic|Classic.*type' payload-manifests/crds/0000_10_config-operator_01_ingresses*.yaml -A 5 | head -60Repository: openshift/api
Length of output: 12022
🏁 Script executed:
#!/bin/bash
# Check if the validation exists in DevPreviewNoUpgrade variant
echo "=== Searching for Classic ipFamily validation in DevPreviewNoUpgrade ==="
grep -n "when type is Classic, ipFamily must be IPv4" payload-manifests/crds/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml
echo ""
echo "=== Checking lines around aws section in DevPreviewNoUpgrade ==="
sed -n '185,210p' payload-manifests/crds/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml
echo ""
echo "=== Comparing aws section between DevPreviewNoUpgrade and TechPreviewNoUpgrade ==="
echo "--- TechPreviewNoUpgrade (lines 185-210) ---"
sed -n '185,210p' payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml
echo ""
echo "--- DevPreviewNoUpgrade (lines 185-210) ---"
sed -n '185,210p' payload-manifests/crds/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml
echo ""
echo "=== Check file sizes and if they contain aws section ==="
grep -n 'aws:' payload-manifests/crds/0000_10_config-operator_01_ingresses*.yaml | grep -E 'DevPreviewNoUpgrade|TechPreviewNoUpgrade'Repository: openshift/api
Length of output: 4773
Add missing validation: Classic load balancer type must restrict ipFamily to IPv4.
The x-kubernetes-validations rule preventing incompatible configurations of Classic LB with dual-stack ipFamily values is present in the TechPreviewNoUpgrade variant and in the source code (config/v1/types_ingress.go), but is missing from this DevPreviewNoUpgrade variant.
Add the following validation block after the aws object definition (after type: object at line 191):
Validation to add
x-kubernetes-validations:
- message: when type is Classic, ipFamily must be IPv4 or omitted (defaults to IPv4)
rule: 'has(self.type) && self.type == ''Classic'' ? (!has(self.ipFamily) || self.ipFamily == ''IPv4'') : true'Without this validation, users can configure an invalid combination (Classic LB with DualStackIPv6Primary or DualStackIPv4Primary), which would fail at runtime since Classic Load Balancers do not support dual-stack networking.
🤖 Prompt for AI Agents
In
`@payload-manifests/crds/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml`
around lines 133 - 192, Add an x-kubernetes-validations block right after the
aws object definition (the object with required: [type] and type: object) that
enforces: if aws.type == "Classic" then ipFamily must be either omitted or
"IPv4". Target the aws/type property under loadBalancer.platform.aws and
implement the validation rule so it returns true for all other cases and only
fails when type is Classic and ipFamily is set to a dual-stack value.
| aws: | ||
| description: aws contains settings specific to the Amazon | ||
| Web Services infrastructure provider. | ||
| properties: | ||
| type: | ||
| description: |- | ||
| type allows user to set a load balancer type. | ||
| When this field is set the default ingresscontroller will get created using the specified LBType. | ||
| If this field is not set then the default ingress controller of LBType Classic will be created. | ||
| Valid values are: | ||
|
|
||
| * "Classic": A Classic Load Balancer that makes routing decisions at either | ||
| the transport layer (TCP/SSL) or the application layer (HTTP/HTTPS). See | ||
| the following for additional details: | ||
|
|
||
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#clb | ||
|
|
||
| * "NLB": A Network Load Balancer that makes routing decisions at the | ||
| transport layer (TCP/SSL). See the following for additional details: | ||
|
|
||
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb | ||
| enum: | ||
| - NLB | ||
| - Classic | ||
| type: string | ||
| required: | ||
| - type | ||
| type: object |
There was a problem hiding this comment.
Add ipFamily to the OKD CRD schema for AWS ingress.
The AWS ingress schema here only exposes type; the new ipFamily field documented in the API isn’t present, so spec.loadBalancer.platform.aws.ipFamily will be rejected/stripped in the OKD feature set. Please regenerate the CRDs (or add the field + validations) to keep this manifest aligned with the Go types/OpenAPI.
💡 Suggested schema addition
aws:
description: aws contains settings specific to the Amazon
Web Services infrastructure provider.
properties:
type:
description: |-
type allows user to set a load balancer type.
...
enum:
- NLB
- Classic
type: string
+ ipFamily:
+ description: |-
+ ipFamily allows users to specify the IP protocol Families that the Load Balancer service supports.
+ enum:
+ - IPv4
+ - DualStackIPv6Primary
+ - DualStackIPv4Primary
+ default: IPv4
+ type: string
required:
- type
type: object📝 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.
| aws: | |
| description: aws contains settings specific to the Amazon | |
| Web Services infrastructure provider. | |
| properties: | |
| type: | |
| description: |- | |
| type allows user to set a load balancer type. | |
| When this field is set the default ingresscontroller will get created using the specified LBType. | |
| If this field is not set then the default ingress controller of LBType Classic will be created. | |
| Valid values are: | |
| * "Classic": A Classic Load Balancer that makes routing decisions at either | |
| the transport layer (TCP/SSL) or the application layer (HTTP/HTTPS). See | |
| the following for additional details: | |
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#clb | |
| * "NLB": A Network Load Balancer that makes routing decisions at the | |
| transport layer (TCP/SSL). See the following for additional details: | |
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb | |
| enum: | |
| - NLB | |
| - Classic | |
| type: string | |
| required: | |
| - type | |
| type: object | |
| aws: | |
| description: aws contains settings specific to the Amazon | |
| Web Services infrastructure provider. | |
| properties: | |
| type: | |
| description: |- | |
| type allows user to set a load balancer type. | |
| When this field is set the default ingresscontroller will get created using the specified LBType. | |
| If this field is not set then the default ingress controller of LBType Classic will be created. | |
| Valid values are: | |
| * "Classic": A Classic Load Balancer that makes routing decisions at either | |
| the transport layer (TCP/SSL) or the application layer (HTTP/HTTPS). See | |
| the following for additional details: | |
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#clb | |
| * "NLB": A Network Load Balancer that makes routing decisions at the | |
| transport layer (TCP/SSL). See the following for additional details: | |
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb | |
| enum: | |
| - NLB | |
| - Classic | |
| type: string | |
| ipFamily: | |
| description: |- | |
| ipFamily allows users to specify the IP protocol Families that the Load Balancer service supports. | |
| enum: | |
| - IPv4 | |
| - DualStackIPv6Primary | |
| - DualStackIPv4Primary | |
| default: IPv4 | |
| type: string | |
| required: | |
| - type | |
| type: object |
🤖 Prompt for AI Agents
In `@payload-manifests/crds/0000_10_config-operator_01_ingresses-OKD.crd.yaml`
around lines 145 - 172, The aws ingress schema only defines the "type" property
and is missing the new "ipFamily" field (so
spec.loadBalancer.platform.aws.ipFamily will be rejected); add an optional
"ipFamily" property under the aws properties with a descriptive "description",
type: string, and an enum of the allowed values (e.g., "IPv4", "IPv6",
"DualStack") to match the Go/OpenAPI types, do not add it to the required list,
and then regenerate the CRD manifests so the YAML aligns with the updated types.
| ipFamily: | ||
| default: IPv4 | ||
| description: |- | ||
| ipFamily specifies the IP protocol family that should be used for the | ||
| ingress load balancer. This field determines whether the load balancer uses | ||
| IPv4-only, or dual-stack networking with IPv4 or IPv6 as the primary | ||
| protocol family. | ||
| Valid values are "IPv4", "DualStackIPv6Primary", and "DualStackIPv4Primary". | ||
| When set to IPv4, the load balancer will use IPv4 addressing only. | ||
| When set to DualStackIPv6Primary, the load balancer will use dual-stack networking with IPv6 as primary. | ||
| When set to DualStackIPv4Primary, the load balancer will use dual-stack networking with IPv4 as primary. | ||
| When omitted, the default value is IPv4. | ||
| enum: | ||
| - IPv4 | ||
| - DualStackIPv6Primary | ||
| - DualStackIPv4Primary | ||
| type: string | ||
| x-kubernetes-validations: | ||
| - message: ipFamily is immutable once set | ||
| rule: oldSelf == '' || self == oldSelf | ||
| loadBalancer: | ||
| description: |- | ||
| loadBalancer contains the load balancer details in general which are not only specific to the underlying infrastructure | ||
| provider of the current cluster and are required for Ingress Controller to work on OpenShift. | ||
| properties: | ||
| platform: | ||
| description: |- | ||
| platform holds configuration specific to the underlying | ||
| infrastructure provider for the ingress load balancers. | ||
| When omitted, this means the user has no opinion and the platform is left | ||
| to choose reasonable defaults. These defaults are subject to change over time. | ||
| properties: | ||
| aws: | ||
| description: aws contains settings specific to the Amazon | ||
| Web Services infrastructure provider. | ||
| properties: | ||
| type: | ||
| description: |- | ||
| type allows user to set a load balancer type. | ||
| When this field is set the default ingresscontroller will get created using the specified LBType. | ||
| If this field is not set then the default ingress controller of LBType Classic will be created. | ||
| Valid values are: | ||
|
|
||
| * "Classic": A Classic Load Balancer that makes routing decisions at either | ||
| the transport layer (TCP/SSL) or the application layer (HTTP/HTTPS). See | ||
| the following for additional details: | ||
|
|
||
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#clb | ||
|
|
||
| * "NLB": A Network Load Balancer that makes routing decisions at the | ||
| transport layer (TCP/SSL). See the following for additional details: | ||
|
|
||
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb | ||
| enum: | ||
| - NLB | ||
| - Classic | ||
| type: string | ||
| required: | ||
| - type | ||
| type: object | ||
| x-kubernetes-validations: | ||
| - message: when type is Classic, ipFamily must be IPv4 or omitted (defaults to IPv4) | ||
| rule: 'has(self.type) && self.type == ''Classic'' ? (!has(self.ipFamily) || self.ipFamily == ''IPv4'') : true' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the complete CRD file and understand its structure
find . -name "*ingresses-TechPreviewNoUpgrade.crd.yaml" -type fRepository: openshift/api
Length of output: 148
🏁 Script executed:
# Once we have the file, let's examine the full structure around lines 133-195
# and look for ipFamily and aws definitions
rg -n "ipFamily|aws:" "payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml" -B 2 -A 2 | head -100Repository: openshift/api
Length of output: 1538
🏁 Script executed:
# Search for Go type definitions related to Ingress configuration
find . -name "*.go" -type f | xargs grep -l "type.*Ingress\|ipFamily" | grep -E "(config|operator)" | head -10Repository: openshift/api
Length of output: 233
🏁 Script executed:
# Look for test files that might validate the ipFamily or loadBalancer structure
find . -name "*test.go" -type f | xargs grep -l "ipFamily\|loadBalancer" 2>/dev/null | head -5Repository: openshift/api
Length of output: 39
🏁 Script executed:
# Examine the Go type definitions for ingress in operator/v1
cat -n ./operator/v1/types_ingress.go | head -200Repository: openshift/api
Length of output: 11027
🏁 Script executed:
# Look for LoadBalancer and ipFamily definitions
grep -n "ipFamily\|LoadBalancer\|type AWS" ./operator/v1/types_ingress.go | head -30Repository: openshift/api
Length of output: 2365
🏁 Script executed:
# Get the complete LoadBalancer and AWS type definitions
sed -n '1,600p' ./operator/v1/types_ingress.go | cat -nRepository: openshift/api
Length of output: 32940
🏁 Script executed:
# Look for the CEL validation rule in the CRD and see if there are other AWS-specific fields
rg -n "x-kubernetes-validations" "payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml" -A 5 | head -50Repository: openshift/api
Length of output: 916
🏁 Script executed:
# Check the actual structure: find where aws properties end and what comes after type
sed -n '165,210p' "payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml" | cat -nRepository: openshift/api
Length of output: 3150
🏁 Script executed:
# Let's see the full AWS type definition in the Go code
grep -n "type AWS" ./operator/v1/types_ingress.go -A 50 | head -80Repository: openshift/api
Length of output: 3969
🏁 Script executed:
# Search more broadly for aws-related types
grep -n "type.*AWS\|type.*aws" ./operator/v1/types_ingress.go | head -20Repository: openshift/api
Length of output: 488
🏁 Script executed:
# Let's look at the ProviderLoadBalancerParameters struct more closely
sed -n '583,700p' ./operator/v1/types_ingress.go | cat -nRepository: openshift/api
Length of output: 5721
🏁 Script executed:
# Check if there are any other files that define AWS types or configuration
find . -name "*.go" -type f -path "*/config/v1/*" | xargs grep -l "ipFamily\|AWS" | head -5Repository: openshift/api
Length of output: 218
🏁 Script executed:
# Search the config/v1 ingress types for ipFamily
grep -n "ipFamily" ./config/v1/types_ingress.go -B 3 -A 3Repository: openshift/api
Length of output: 1203
🏁 Script executed:
# Let's check what this CRD actually defines - look at the kind and group
head -50 "payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml"Repository: openshift/api
Length of output: 2138
🏁 Script executed:
# Let's check the full Go type definition for AWSLoadBalancerParameters to confirm no ipFamily
sed -n '655,686p' ./operator/v1/types_ingress.go | cat -nRepository: openshift/api
Length of output: 1588
🏁 Script executed:
# Let's look at AWSClassicLoadBalancerParameters and AWSNetworkLoadBalancerParameters
sed -n '832,950p' ./operator/v1/types_ingress.go | cat -nRepository: openshift/api
Length of output: 7540
🏁 Script executed:
# Compare the config/v1 AWSIngressSpec with the operator/v1 AWSLoadBalancerParameters
grep -n "type AWSIngressSpec\|type AWSLoadBalancerParameters" ./config/v1/types_ingress.go ./operator/v1/types_ingress.go -A 30 | head -100Repository: openshift/api
Length of output: 4993
🏁 Script executed:
# Let's look at the actual YAML structure more carefully around the ipFamily and aws definitions
sed -n '125,210p' "payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml" | cat -nRepository: openshift/api
Length of output: 5841
🏁 Script executed:
# Let's also check if config/v1 AWSIngressSpec is used to generate the CRD for config.openshift.io/v1
grep -n "ipFamily" ./config/v1/types_ingress.go | head -20Repository: openshift/api
Length of output: 451
Move ipFamily from spec level into the aws object to align with Go types and activate the CEL validation rule.
The CRD currently places ipFamily at spec level, but the CEL rule within the aws object references self.ipFamily, which doesn't exist. The Go type definition (AWSIngressSpec) confirms that ipFamily should be a direct property of the AWS configuration, not at the top spec level. This mismatch prevents the CEL validation from working and causes a schema divergence.
Fix: Move ipFamily into aws
- ipFamily:
- default: IPv4
- description: |-
- ipFamily specifies the IP protocol family that should be used for the
- ingress load balancer. This field determines whether the load balancer uses
- IPv4-only, or dual-stack networking with IPv4 or IPv6 as the primary
- protocol family.
- Valid values are "IPv4", "DualStackIPv6Primary", and "DualStackIPv4Primary".
- When set to IPv4, the load balancer will use IPv4 addressing only.
- When set to DualStackIPv6Primary, the load balancer will use dual-stack networking with IPv6 as primary.
- When set to DualStackIPv4Primary, the load balancer will use dual-stack networking with IPv4 as primary.
- When omitted, the default value is IPv4.
- enum:
- - IPv4
- - DualStackIPv6Primary
- - DualStackIPv4Primary
- type: string
- x-kubernetes-validations:
- - message: ipFamily is immutable once set
- rule: oldSelf == '' || self == oldSelf
loadBalancer:
description: |-
loadBalancer contains the load balancer details in general which are not only specific to the underlying infrastructure
provider of the current cluster and are required for Ingress Controller to work on OpenShift.
properties:
platform:
properties:
aws:
description: aws contains settings specific to the Amazon
Web Services infrastructure provider.
properties:
type:
description: type allows user to set a load balancer type.
enum:
- NLB
- Classic
type: string
+ ipFamily:
+ default: IPv4
+ description: ipFamily specifies the IP protocol family for the AWS load balancer.
+ enum:
+ - IPv4
+ - DualStackIPv6Primary
+ - DualStackIPv4Primary
+ type: string
+ x-kubernetes-validations:
+ - message: ipFamily is immutable once set
+ rule: oldSelf == '' || self == oldSelf
required:
- type
type: object
x-kubernetes-validations:
- message: when type is Classic, ipFamily must be IPv4 or omitted (defaults to IPv4)
rule: 'has(self.type) && self.type == ''Classic'' ? (!has(self.ipFamily) || self.ipFamily == ''IPv4'') : true'🤖 Prompt for AI Agents
In
`@payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml`
around lines 133 - 195, The ipFamily field is defined at the top spec level but
the CEL validation in the aws object expects self.ipFamily; move the ipFamily
definition into the aws properties so it becomes loadBalancer -> platform -> aws
-> properties -> ipFamily (preserve the description, enum values
["IPv4","DualStackIPv6Primary","DualStackIPv4Primary"], type: string, and
immutability x-kubernetes-validations if needed), and remove the duplicate
top-level ipFamily block; this aligns the CRD with the Go AWSIngressSpec and
activates the CEL rule that references self.ipFamily for the aws.type
validation.
|
@sadasu: The following tests failed, say
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. |
6 participants
User description
Add IPFamily to Ingress API
Add the IPFamliy field to Ingress API's IngressSpec that enables
Day 0 configuration of the default Ingress controller with DualStack
configuration.
This field defaults to IPv4 when not set and is available only
when the AWSDualStackInstall and AzureDualStack install featuregates
are enabled.
This field cannot be modified once set so the IPFamily of the default
Ingress controller cannot be changed on Day-2 after set by the installer
on Day-0.
PR Type
Enhancement, Tests, Documentation
Description
Added
IPFamilyfield toIngressSpecstruct enabling Day 0 configuration of the default Ingress controller with DualStack supportField supports three values: "IPv4" (default), "DualStackIPv6Primary", and "DualStackIPv4Primary"
Implemented immutability constraint preventing changes after initial set via kubebuilder validation rules
Updated all CRD manifests across multiple feature sets (Default, CustomNoUpgrade, DevPreviewNoUpgrade, TechPreviewNoUpgrade, OKD) with
ipFamilyfield definition and validationUpdated Swagger and OpenAPI documentation with
ipFamilyfield specificationsDiagram Walkthrough
File Walkthrough
5 files
types_ingress.go
Add IPFamily field to IngressSpec with dual-stack supportconfig/v1/types_ingress.go
IPFamilyfield toIngressSpecstruct with typeIPFamilyType"DualStackIPv6Primary", "DualStackIPv4Primary"
preventing changes after initial set
AWSDualStackInstallandAzureDualStackInstallfeature gates
0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml
Add IPFamily field to Ingress CRD with immutability constraintconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml
CustomNoUpgradefeature setipFamilyfield in IngressSpec with default valueIPv4andthree valid enum values:
IPv4,DualStackIPv6Primary,DualStackIPv4PrimaryipFamilyfield usingx-kubernetes-validationsto prevent changes after initial setnetworking configuration options
0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml
Add IPFamily field to Ingress CRD with immutability constraintpayload-manifests/crds/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml
DevPreviewNoUpgradefeature setipFamilyfield in IngressSpec with default valueIPv4andthree valid enum values:
IPv4,DualStackIPv6Primary,DualStackIPv4PrimaryipFamilyfield usingx-kubernetes-validationsto prevent changes after initial setnetworking configuration options
0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml
Add IPFamily field to Ingress CRD with immutability constraintpayload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml
TechPreviewNoUpgradefeature setipFamilyfield in IngressSpec with default valueIPv4andthree valid enum values:
IPv4,DualStackIPv6Primary,DualStackIPv4PrimaryipFamilyfield usingx-kubernetes-validationsto prevent changes after initial setnetworking configuration options
0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml
Add IPFamily field to Ingress CRD with immutability constraintpayload-manifests/crds/0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml
CustomNoUpgradefeature setipFamilyfield in IngressSpec with default valueIPv4andthree valid enum values:
IPv4,DualStackIPv6Primary,DualStackIPv4PrimaryipFamilyfield usingx-kubernetes-validationsto prevent changes after initial setnetworking configuration options
3 files
zz_generated.swagger_doc_generated.go
Update swagger documentation for IPFamily fieldconfig/v1/zz_generated.swagger_doc_generated.go
ipFamilyfield inIngressSpecbehavior
zz_generated.openapi.go
Generate OpenAPI schema for IPFamily fieldopenapi/generated_openapi/zz_generated.openapi.go
ipFamilyfield inIngressSpecopenapi.json
Update OpenAPI specification with IPFamily fieldopenapi/openapi.json
ipFamilyfield definition to OpenAPI specification with defaultvalue "IPv4"
and "External" values
9 files
AWSDualStackInstall.yaml
Add feature-gated CRD manifest for AWS dual-stack supportconfig/v1/zz_generated.featuregated-crd-manifests/ingresses.config.openshift.io/AWSDualStackInstall.yaml
AWSDualStackInstallfeaturegate
IngressSpecschema withipFamilyfield definitionand validation rules
x-kubernetes-validations
AzureDualStackInstall.yaml
Add feature-gated CRD manifest for Azure dual-stack supportconfig/v1/zz_generated.featuregated-crd-manifests/ingresses.config.openshift.io/AzureDualStackInstall.yaml
AzureDualStackInstallfeaturegate
IngressSpecschema withipFamilyfield definitionand validation rules
x-kubernetes-validations
0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml
Add CRD manifest for DevPreviewNoUpgrade feature setconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml
DevPreviewNoUpgradefeature setipFamilyfield with enum validation and immutabilityconstraint
0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml
Add CRD manifest for TechPreviewNoUpgrade feature setconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml
TechPreviewNoUpgradefeature setipFamilyfield with enum validation and immutabilityconstraint
0000_10_config-operator_01_ingresses-OKD.crd.yaml
Add feature-set annotation to OKD CRD manifestconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-OKD.crd.yaml
release.openshift.io/feature-set: OKDannotation to CRD metadata0000_10_config-operator_01_ingresses-OKD.crd.yaml
Add feature-set annotation to OKD payload manifestpayload-manifests/crds/0000_10_config-operator_01_ingresses-OKD.crd.yaml
release.openshift.io/feature-set: OKDannotation to CRD metadatazz_generated.featuregated-crd-manifests.yaml
Register dual-stack feature gates for Ingress CRDconfig/v1/zz_generated.featuregated-crd-manifests.yaml
ingresses.config.openshift.iotoinclude
AWSDualStackInstallandAzureDualStackInstall0000_10_config-operator_01_ingresses-Default.crd.yaml
Add Ingress CRD manifest with cluster-wide ingress configurationconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-Default.crd.yaml
resource
ingresses.config.openshift.iowith v1API version
settings, component routes, and HSTS policies
default placement configuration
0000_10_config-operator_01_ingresses-Default.crd.yaml
Add Ingress CRD manifest with cluster-wide ingress configurationpayload-manifests/crds/0000_10_config-operator_01_ingresses-Default.crd.yaml
resource
ingresses.config.openshift.iowith v1API version
settings, component routes, and HSTS policies
default placement configuration
3 files
AWSDualStackInstall.yaml
Add validation tests for IPFamily field with AWS dual-stackconfig/v1/tests/ingresses.config.openshift.io/AWSDualStackInstall.yaml
ipFamilyfield underAWSDualStackInstallfeature gatevalues, and immutability constraints
AzureDualStackInstall.yaml
Add validation tests for IPFamily field with Azure dual-stackconfig/v1/tests/ingresses.config.openshift.io/AzureDualStackInstall.yaml
ipFamilyfield underAzureDualStackInstallfeature gatevalues, and immutability constraints
AAA_ungated.yaml
Add feature gate configuration to Ingress API testsconfig/v1/tests/ingresses.config.openshift.io/AAA_ungated.yaml
featureGatessection to test configuration with two disabledfeature gates
-AWSDualStackInstalland-AzureDualStackInstallto ensuretests run without these feature gates enabled