From a7db5b524b470272a2546988ba2fdda2b4d391c9 Mon Sep 17 00:00:00 2001 From: Thuan Vo Date: Mon, 26 Jan 2026 17:03:01 -0800 Subject: [PATCH 1/2] CORS-4142: ensure lbType is NLB when dual-stack is configured This commit adds validation to ensure platform.aws.lbType is NLB when dual-stack networking is configured on AWS. AWS does not support dual-stack for Classic load balancers. Additionally, if lbType is left empty and ipFamily is set to dual-stack values, the installer will default lbType to NLB. --- data/data/install.openshift.io_installconfigs.yaml | 8 ++++++-- pkg/types/aws/defaults/platform.go | 6 ++++++ pkg/types/aws/platform.go | 6 ++++-- pkg/types/aws/validation/platform.go | 14 ++++++++++++++ 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index b1c1f3a421..faa3c30603 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -5083,8 +5083,12 @@ spec: transport layer (TCP/SSL). See the following for additional details: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb - If this field is not set explicitly, it defaults to "Classic". This - default is subject to change over time. + If this field is not set explicitly, it defaults to "Classic". This + default is subject to change over time. If ipFamily field is set to + "DualStackIPv4Primary" or "DualStackIPv6Primary", the default is "NLB". + enum: + - Classic + - NLB type: string preserveBootstrapIgnition: description: PreserveBootstrapIgnition is deprecated. Use bestEffortDeleteIgnition diff --git a/pkg/types/aws/defaults/platform.go b/pkg/types/aws/defaults/platform.go index 6d3a161e9d..038d190cf6 100644 --- a/pkg/types/aws/defaults/platform.go +++ b/pkg/types/aws/defaults/platform.go @@ -40,6 +40,12 @@ func SetPlatformDefaults(p *aws.Platform) { p.IPFamily = network.IPv4 logrus.Infof("ipFamily is not specified in install-config; defaulting to %q", network.IPv4) } + + if p.LBType == "" { + if p.IPFamily.DualStackEnabled() { + p.LBType = configv1.NLB + } + } } // InstanceTypes returns a list of instance types, in decreasing priority order, which we should use for a given diff --git a/pkg/types/aws/platform.go b/pkg/types/aws/platform.go index 58761283ee..71d67e09dc 100644 --- a/pkg/types/aws/platform.go +++ b/pkg/types/aws/platform.go @@ -100,9 +100,11 @@ type Platform struct { // transport layer (TCP/SSL). See the following for additional details: // https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb // - // If this field is not set explicitly, it defaults to "Classic". This - // default is subject to change over time. + // If this field is not set explicitly, it defaults to "Classic". This + // default is subject to change over time. If ipFamily field is set to + // "DualStackIPv4Primary" or "DualStackIPv6Primary", the default is "NLB". // + // +kubebuilder:validation:Enum="Classic";"NLB" // +optional LBType configv1.AWSLBType `json:"lbType,omitempty"` diff --git a/pkg/types/aws/validation/platform.go b/pkg/types/aws/validation/platform.go index cfbf7d4e2e..b9acbda4f9 100644 --- a/pkg/types/aws/validation/platform.go +++ b/pkg/types/aws/validation/platform.go @@ -10,6 +10,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/aws" "github.com/openshift/installer/pkg/types/network" @@ -57,6 +58,7 @@ func ValidatePlatform(p *aws.Platform, publish types.PublishingStrategy, cm type allErrs = append(allErrs, validateServiceEndpoints(p.ServiceEndpoints, fldPath.Child("serviceEndpoints"))...) allErrs = append(allErrs, validateUserTags(p.UserTags, p.PropagateUserTag, fldPath.Child("userTags"))...) allErrs = append(allErrs, validateIPFamily(p.IPFamily, fldPath.Child("ipFamily"))...) + allErrs = append(allErrs, validateLBType(p.LBType, p.IPFamily, fldPath.Child("lbType"))...) if p.DefaultMachinePlatform != nil { allErrs = append(allErrs, ValidateMachinePool(p, p.DefaultMachinePlatform, fldPath.Child("defaultMachinePlatform"))...) @@ -84,6 +86,18 @@ func validateIPFamily(ipFamily network.IPFamily, fldPath *field.Path) field.Erro return allErrs } +func validateLBType(lbType configv1.AWSLBType, ipFamily network.IPFamily, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if ipFamily.DualStackEnabled() { + if lbType == configv1.Classic { + allErrs = append(allErrs, field.Invalid(fldPath, lbType, "Classic load balancers do not support dual-stack networking")) + } + } + + return allErrs +} + func validateUserTags(tags map[string]string, propagatingTags bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if len(tags) == 0 { From d1f15d1c3315968799dfcb291c2400d262773fb0 Mon Sep 17 00:00:00 2001 From: Thuan Vo Date: Mon, 26 Jan 2026 17:09:38 -0800 Subject: [PATCH 2/2] tests: add unit tests for lbType validations --- pkg/explain/printer_test.go | 6 +- pkg/types/aws/defaults/platform_test.go | 86 +++++++++++++++++++++++ pkg/types/aws/validation/platform_test.go | 58 +++++++++++++++ 3 files changed, 148 insertions(+), 2 deletions(-) diff --git a/pkg/explain/printer_test.go b/pkg/explain/printer_test.go index f47c3dd7ad..6e6759ec04 100644 --- a/pkg/explain/printer_test.go +++ b/pkg/explain/printer_test.go @@ -244,6 +244,7 @@ networking with IPv6 as the primary address family. When using dual-stack, the V and subnets must be configured with both IPv4 and IPv6 CIDR blocks. lbType + Valid Values: "Classic","NLB" LBType is an optional field to specify a load balancer type. When this field is specified, all ingresscontrollers (including the default ingresscontroller) will be created using the specified load-balancer @@ -260,8 +261,9 @@ https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types. transport layer (TCP/SSL). See the following for additional details: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb -If this field is not set explicitly, it defaults to "Classic". This -default is subject to change over time. +If this field is not set explicitly, it defaults to "Classic". This +default is subject to change over time. If ipFamily field is set to +"DualStackIPv4Primary" or "DualStackIPv6Primary", the default is "NLB". preserveBootstrapIgnition PreserveBootstrapIgnition is deprecated. Use bestEffortDeleteIgnition instead. diff --git a/pkg/types/aws/defaults/platform_test.go b/pkg/types/aws/defaults/platform_test.go index 39bf32750c..14154d8047 100644 --- a/pkg/types/aws/defaults/platform_test.go +++ b/pkg/types/aws/defaults/platform_test.go @@ -7,6 +7,8 @@ import ( configv1 "github.com/openshift/api/config/v1" "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/aws" + "github.com/openshift/installer/pkg/types/network" ) func TestInstanceTypes(t *testing.T) { @@ -65,3 +67,87 @@ func TestInstanceTypes(t *testing.T) { }) } } + +func TestSetPlatformDefaults(t *testing.T) { + cases := []struct { + name string + platform *aws.Platform + expected *aws.Platform + }{ + { + name: "empty platform should default IPFamily to IPv4", + platform: &aws.Platform{}, + expected: &aws.Platform{ + IPFamily: network.IPv4, + }, + }, + { + name: "IPFamily already set should not be overridden", + platform: &aws.Platform{ + IPFamily: network.DualStackIPv4Primary, + }, + expected: &aws.Platform{ + IPFamily: network.DualStackIPv4Primary, + LBType: configv1.NLB, // LBType gets set to NLB when IPFamily is dual-stack + }, + }, + { + name: "LBType should default to NLB for DualStackIPv4Primary", + platform: &aws.Platform{ + IPFamily: network.DualStackIPv4Primary, + }, + expected: &aws.Platform{ + IPFamily: network.DualStackIPv4Primary, + LBType: configv1.NLB, + }, + }, + { + name: "LBType should default to NLB for DualStackIPv6Primary", + platform: &aws.Platform{ + IPFamily: network.DualStackIPv6Primary, + }, + expected: &aws.Platform{ + IPFamily: network.DualStackIPv6Primary, + LBType: configv1.NLB, + }, + }, + { + name: "LBType should not be set for IPv4", + platform: &aws.Platform{ + IPFamily: network.IPv4, + }, + expected: &aws.Platform{ + IPFamily: network.IPv4, + }, + }, + { + name: "LBType already set should not be overridden", + platform: &aws.Platform{ + IPFamily: network.DualStackIPv4Primary, + LBType: configv1.Classic, + }, + expected: &aws.Platform{ + IPFamily: network.DualStackIPv4Primary, + LBType: configv1.Classic, + }, + }, + { + name: "empty IPFamily should default to IPv4 and LBType remains empty", + platform: &aws.Platform{ + LBType: "", + }, + expected: &aws.Platform{ + IPFamily: network.IPv4, + LBType: "", + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + SetPlatformDefaults(tc.platform) + assert.Equal(t, tc.expected.IPFamily, tc.platform.IPFamily) + assert.Equal(t, tc.expected.LBType, tc.platform.LBType) + }) + } +} diff --git a/pkg/types/aws/validation/platform_test.go b/pkg/types/aws/validation/platform_test.go index b4c838ae11..34e3f548e4 100644 --- a/pkg/types/aws/validation/platform_test.go +++ b/pkg/types/aws/validation/platform_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/validation/field" + configv1 "github.com/openshift/api/config/v1" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/aws" "github.com/openshift/installer/pkg/types/network" @@ -625,6 +626,63 @@ func TestValidatePlatform(t *testing.T) { }, expected: `^\Qtest-path.ipFamily: Unsupported value: "DualStack": supported values: "IPv4", "DualStackIPv4Primary", "DualStackIPv6Primary"\E$`, }, + { + name: "valid lbType Classic with IPv4", + platform: &aws.Platform{ + Region: "us-east-1", + IPFamily: network.IPv4, + LBType: configv1.Classic, + }, + }, + { + name: "valid lbType NLB with IPv4", + platform: &aws.Platform{ + Region: "us-east-1", + IPFamily: network.IPv4, + LBType: configv1.NLB, + }, + }, + { + name: "valid lbType NLB with DualStackIPv4Primary", + platform: &aws.Platform{ + Region: "us-east-1", + IPFamily: network.DualStackIPv4Primary, + LBType: configv1.NLB, + }, + }, + { + name: "valid lbType NLB with DualStackIPv6Primary", + platform: &aws.Platform{ + Region: "us-east-1", + IPFamily: network.DualStackIPv6Primary, + LBType: configv1.NLB, + }, + }, + { + name: "invalid lbType Classic with DualStackIPv4Primary", + platform: &aws.Platform{ + Region: "us-east-1", + IPFamily: network.DualStackIPv4Primary, + LBType: configv1.Classic, + }, + expected: `^\Qtest-path.lbType: Invalid value: "Classic": Classic load balancers do not support dual-stack networking\E$`, + }, + { + name: "invalid lbType Classic with DualStackIPv6Primary", + platform: &aws.Platform{ + Region: "us-east-1", + IPFamily: network.DualStackIPv6Primary, + LBType: configv1.Classic, + }, + expected: `^\Qtest-path.lbType: Invalid value: "Classic": Classic load balancers do not support dual-stack networking\E$`, + }, + { + name: "valid empty lbType", + platform: &aws.Platform{ + Region: "us-east-1", + LBType: "", + }, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) {