CORS-4142: ensure lbType is NLB when dual-stack is configured

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

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 <string>
+      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 <boolean>
       PreserveBootstrapIgnition is deprecated. Use bestEffortDeleteIgnition instead.

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

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)
+		})
+	}
+}

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"`
 

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 {

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