Skip to content

CORS-4317: Add IPFamily to Ingress API to enable DualStack installs #2663

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
sadasu wants to merge 1 commit into
base: master
Choose a base branch
from
+5,872 −2
Open

CORS-4317: Add IPFamily to Ingress API to enable DualStack installs #2663

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions config/v1/tests/ingresses.config.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
name: "Ingress"
crdName: ingresses.config.openshift.io
featureGates:
- -AWSDualStackInstall
- -AzureDualStackInstall
tests:
onCreate:
- name: Should be able to create a minimal Ingress
Expand Down
131 changes: 131 additions & 0 deletions config/v1/tests/ingresses.config.openshift.io/AWSDualStackInstall.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
name: "Ingress"
crdName: ingresses.config.openshift.io
featureGates:
- AWSDualStackInstall
tests:
onCreate:
- name: Should be able to create an Ingress with default ipFamily
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: IPv4
- name: Should be able to create an Ingress with ipFamily set to IPv4
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: IPv4
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: IPv4
- name: Should be able to create an Ingress with ipFamily set to DualStackIPv6Primary
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
- name: Should be able to create an Ingress with ipFamily set to DualStackIPv4Primary
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv4Primary
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv4Primary
- name: Should not be able to create an Ingress with invalid ipFamily value
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: InvalidValue
expectedError: "spec.ipFamily: Unsupported value: \"InvalidValue\": supported values: \"IPv4\", \"DualStackIPv6Primary\", \"DualStackIPv4Primary\""
onUpdate:
- name: Should default ipFamily to IPv4 when not specified
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
updated: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: IPv4
- name: Should not allow changing ipFamily once set
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: IPv4
updated: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
expectedError: "spec.ipFamily: Invalid value: \"string\": ipFamily is immutable once set"
- name: Should allow setting the same ipFamily value
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
updated: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
- name: Should not allow unsetting ipFamily once it has been set
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv4Primary
updated: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
expectedError: "spec.ipFamily: Invalid value: \"string\": ipFamily is immutable once set"

131 changes: 131 additions & 0 deletions config/v1/tests/ingresses.config.openshift.io/AzureDualStackInstall.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
name: "Ingress"
crdName: ingresses.config.openshift.io
featureGates:
- AzureDualStackInstall
tests:
onCreate:
- name: Should be able to create an Ingress with default ipFamily
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: IPv4
- name: Should be able to create an Ingress with ipFamily set to IPv4
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: IPv4
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: IPv4
- name: Should be able to create an Ingress with ipFamily set to DualStackIPv6Primary
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
- name: Should be able to create an Ingress with ipFamily set to DualStackIPv4Primary
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv4Primary
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv4Primary
- name: Should not be able to create an Ingress with invalid ipFamily value
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: InvalidValue
expectedError: "spec.ipFamily: Unsupported value: \"InvalidValue\": supported values: \"IPv4\", \"DualStackIPv6Primary\", \"DualStackIPv4Primary\""
onUpdate:
- name: Should default ipFamily to IPv4 when not specified
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
updated: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: IPv4
- name: Should not allow changing ipFamily once set
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: IPv4
updated: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
expectedError: "spec.ipFamily: Invalid value: \"string\": ipFamily is immutable once set"
- name: Should allow setting the same ipFamily value
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
updated: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
expected: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv6Primary
- name: Should not allow unsetting ipFamily once it has been set
initial: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
ipFamily: DualStackIPv4Primary
updated: |
apiVersion: config.openshift.io/v1
kind: Ingress
spec:
domain: apps.example.com
expectedError: "spec.ipFamily: Invalid value: \"string\": ipFamily is immutable once set"

17 changes: 17 additions & 0 deletions config/v1/types_ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,23 @@ type IngressSpec struct {
// provider of the current cluster and are required for Ingress Controller to work on OpenShift.
// +optional
LoadBalancer LoadBalancer `json:"loadBalancer,omitempty"`

// ipFamily specifies the IP protocol family that should be used for the
// ingress load balancer. This field determines whether the load balancer uses
Copy link
Contributor

@patrickdillon patrickdillon Jan 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This field determines whether the load balancer uses IPv4-only, or dual-stack networking with IPv4 or IPv6 as the primary protocol family.

I don’t think this is accurate. My understanding is this field could be used by the ingress operator to set annotations on the service. The aws cloud provider config is what would determine the details for the LB itself.

Perhaps we want to omit this detail entirely.

// IPv4-only, or dual-stack networking with IPv4 or IPv6 as the primary
// protocol family.
// Valid values are "IPv4", "DualStackIPv6Primary", and "DualStackIPv4Primary".
Copy link
Contributor

@alebedev87 alebedev87 Jan 23, 2026

Choose a reason for hiding this comment

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

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.

// 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.
//
// +default="IPv4"
// +kubebuilder:validation:XValidation:rule="oldSelf == '' || self == oldSelf",message="ipFamily is immutable once set"
// +openshift:enable:FeatureGate=AWSDualStackInstall
Copy link
Contributor

@patrickdillon patrickdillon Jan 22, 2026

Choose a reason for hiding this comment

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

I’m not familiar with these feature gate validations, but it seems a little strange when there is more than one…

Based on the tests, it seems like there is a requirement that either featuregate—not both—is enabled. That is good, for example, that AWS is not blocked by Azure; on the other hand, Azure would be enabled by AWS (it seems).

That question relates to another one: what happens if this field is set for platforms that are not AWS or Azure?

This field is not platform-specific so it makes sense to not have it nested in a platform, but we may want platform validation in this case.

The featuregate would work better if this field were nested in respective platforms.

Copy link
Contributor

@patrickdillon patrickdillon Jan 23, 2026

Choose a reason for hiding this comment

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

Oh I just learned of // +openshift:validation:FeatureGateAwareXValidation that seems like it could resolve concerns like these

// +openshift:enable:FeatureGate=AzureDualStackInstall
// +optional
IPFamily IPFamilyType `json:"ipFamily,omitempty"`
Copy link
Member

@tthvo tthvo Jan 22, 2026

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@sadasu sadasu Jan 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Should we validate if spec.loadBalancer.platform.aws.type is "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

Copy link
Member

@tthvo tthvo Jan 22, 2026

Choose a reason for hiding this comment

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

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: AWS

The 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.

Copy link
Member

@tthvo tthvo Jan 22, 2026

Choose a reason for hiding this comment

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

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)...

Copy link
Contributor

@alebedev87 alebedev87 Jan 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

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: DualStackIPv4Primary

In 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.

}

// IngressPlatformSpec holds the desired state of Ingress specific to the underlying infrastructure provider
Expand Down
Loading