Merge pull request #10741 from codablock/nlb-subnets

Allow to control which subnets and IPs get used for the API loadbalancer

cloudmock/aws/mockelbv2/loadbalancers.go

@@ -92,6 +92,16 @@ func (m *MockELBV2) CreateLoadBalancer(request *elbv2.CreateLoadBalancerInput) (
 			SubnetId: subnet,
 		})
 	}
+	for _, subnetMapping := range request.SubnetMappings {
+		var lbAddrs []*elbv2.LoadBalancerAddress
+		if subnetMapping.PrivateIPv4Address != nil {
+			lbAddrs = append(lbAddrs, &elbv2.LoadBalancerAddress{PrivateIPv4Address: subnetMapping.PrivateIPv4Address})
+		}
+		zones = append(zones, &elbv2.AvailabilityZone{
+			SubnetId:              subnetMapping.SubnetId,
+			LoadBalancerAddresses: lbAddrs,
+		})
+	}
 	lb.AvailabilityZones = zones
 
 	// This is hardcoded because AWS derives it from the subnets above

docs/cluster_spec.md

@@ -101,6 +101,45 @@ spec:
       type: Public
 ```
 
+### Load Balancer Subnet configuration 
+
+**AWS only**
+
+By default, kops will try to choose one suitable subnet per availability zone and use these for the API load balancer.
+Depending on the `type`, kops will choose from either `Private` or `Public` subnets. If this default logic is not
+suitable for you (e.g. because you have a more granular separation between subnets), you can explicitly configure
+the to-be-use subnets:
+
+```yaml
+spec:
+  api:
+    loadBalancer:
+      type: Public
+      subnets:
+        - name: subnet-a
+        - name: subnet-b
+        - name: subnet-c
+````
+
+It is only allowed to add more subnets and forbidden to remove existing ones. This is due to limitations on AWS
+ELBs and NLBs.
+
+If the `type` is `Internal` and the `class` is `Network`, you can also specify a static private IPv4 address per subnet:
+```yaml
+spec:
+  api:
+    loadBalancer:
+      type: Internal
+      subnets:
+        - name: subnet-a
+          privateIPv4Address: 172.16.1.10
+```
+
+The specified IPv4 addresses must be part of the subnets CIDR. They can not be changed after initial deployment.
+
+If you made a mistake or need to change subnets for any other reason, you're currently forced to manually delete the
+underlying ELB/NLB and re-run `kops update`.
+
 ## etcdClusters
 
 ### The default etcd configuration

k8s/crds/kops.k8s.io_clusters.yaml

@@ -112,6 +112,23 @@ spec:
                         description: SSLPolicy allows you to overwrite the LB listener's
                           Security Policy
                         type: string
+                      subnets:
+                        description: Subnets allows you to specify the subnets that
+                          must be used for the load balancer
+                        items:
+                          description: LoadBalancerSubnetSpec provides configuration
+                            for subnets used for a load balancer
+                          properties:
+                            name:
+                              description: Name specifies the name of the cluster
+                                subnet
+                              type: string
+                            privateIPv4Address:
+                              description: PrivateIPv4Address specifies the private
+                                IPv4 address to use for a NLB
+                              type: string
+                          type: object
+                        type: array
                       type:
                         description: Type of load balancer to create may Public or
                           Internal.

pkg/apis/kops/cluster.go

@@ -377,6 +377,14 @@ var SupportedLoadBalancerClasses = []string{
 	string(LoadBalancerClassNetwork),
 }
 
+// LoadBalancerSubnetSpec provides configuration for subnets used for a load balancer
+type LoadBalancerSubnetSpec struct {
+	// Name specifies the name of the cluster subnet
+	Name string `json:"name,omitempty"`
+	// PrivateIPv4Address specifies the private IPv4 address to use for a NLB
+	PrivateIPv4Address *string `json:"privateIPv4Address,omitempty"`
+}
+
 // LoadBalancerAccessSpec provides configuration details related to API LoadBalancer and its access
 type LoadBalancerAccessSpec struct {
 	// LoadBalancerClass specifies the class of load balancer to create: Classic, Network.
@@ -397,6 +405,8 @@ type LoadBalancerAccessSpec struct {
 	SSLPolicy *string `json:"sslPolicy,omitempty"`
 	// CrossZoneLoadBalancing allows you to enable the cross zone load balancing
 	CrossZoneLoadBalancing *bool `json:"crossZoneLoadBalancing,omitempty"`
+	// Subnets allows you to specify the subnets that must be used for the load balancer
+	Subnets []LoadBalancerSubnetSpec `json:"subnets,omitempty"`
 }
 
 // KubeDNSConfig defines the kube dns configuration

pkg/apis/kops/v1alpha2/cluster.go

@@ -379,6 +379,14 @@ var SupportedLoadBalancerClasses = []string{
 	string(LoadBalancerClassNetwork),
 }
 
+// LoadBalancerSubnetSpec provides configuration for subnets used for a load balancer
+type LoadBalancerSubnetSpec struct {
+	// Name specifies the name of the cluster subnet
+	Name string `json:"name,omitempty"`
+	// PrivateIPv4Address specifies the private IPv4 address to use for a NLB
+	PrivateIPv4Address *string `json:"privateIPv4Address,omitempty"`
+}
+
 // LoadBalancerAccessSpec provides configuration details related to API LoadBalancer and its access
 type LoadBalancerAccessSpec struct {
 	// LoadBalancerClass specifies the class of load balancer to create: Classic, Network
@@ -399,6 +407,8 @@ type LoadBalancerAccessSpec struct {
 	SSLPolicy *string `json:"sslPolicy,omitempty"`
 	// CrossZoneLoadBalancing allows you to enable the cross zone load balancing
 	CrossZoneLoadBalancing *bool `json:"crossZoneLoadBalancing,omitempty"`
+	// Subnets allows you to specify the subnets that must be used for the load balancer
+	Subnets []LoadBalancerSubnetSpec `json:"subnets,omitempty"`
 }
 
 // KubeDNSConfig defines the kube dns configuration

pkg/apis/kops/v1alpha2/zz_generated.conversion.go

@@ -753,6 +753,16 @@ func RegisterConversions(s *runtime.Scheme) error {
 	}); err != nil {
 		return err
 	}
+	if err := s.AddGeneratedConversionFunc((*LoadBalancerSubnetSpec)(nil), (*kops.LoadBalancerSubnetSpec)(nil), func(a, b interface{}, scope conversion.Scope) error {
+		return Convert_v1alpha2_LoadBalancerSubnetSpec_To_kops_LoadBalancerSubnetSpec(a.(*LoadBalancerSubnetSpec), b.(*kops.LoadBalancerSubnetSpec), scope)
+	}); err != nil {
+		return err
+	}
+	if err := s.AddGeneratedConversionFunc((*kops.LoadBalancerSubnetSpec)(nil), (*LoadBalancerSubnetSpec)(nil), func(a, b interface{}, scope conversion.Scope) error {
+		return Convert_kops_LoadBalancerSubnetSpec_To_v1alpha2_LoadBalancerSubnetSpec(a.(*kops.LoadBalancerSubnetSpec), b.(*LoadBalancerSubnetSpec), scope)
+	}); err != nil {
+		return err
+	}
 	if err := s.AddGeneratedConversionFunc((*LyftVPCNetworkingSpec)(nil), (*kops.LyftVPCNetworkingSpec)(nil), func(a, b interface{}, scope conversion.Scope) error {
 		return Convert_v1alpha2_LyftVPCNetworkingSpec_To_kops_LyftVPCNetworkingSpec(a.(*LyftVPCNetworkingSpec), b.(*kops.LyftVPCNetworkingSpec), scope)
 	}); err != nil {
@@ -5120,6 +5130,17 @@ func autoConvert_v1alpha2_LoadBalancerAccessSpec_To_kops_LoadBalancerAccessSpec(
 	out.SSLCertificate = in.SSLCertificate
 	out.SSLPolicy = in.SSLPolicy
 	out.CrossZoneLoadBalancing = in.CrossZoneLoadBalancing
+	if in.Subnets != nil {
+		in, out := &in.Subnets, &out.Subnets
+		*out = make([]kops.LoadBalancerSubnetSpec, len(*in))
+		for i := range *in {
+			if err := Convert_v1alpha2_LoadBalancerSubnetSpec_To_kops_LoadBalancerSubnetSpec(&(*in)[i], &(*out)[i], s); err != nil {
+				return err
+			}
+		}
+	} else {
+		out.Subnets = nil
+	}
 	return nil
 }
 
@@ -5138,6 +5159,17 @@ func autoConvert_kops_LoadBalancerAccessSpec_To_v1alpha2_LoadBalancerAccessSpec(
 	out.SSLCertificate = in.SSLCertificate
 	out.SSLPolicy = in.SSLPolicy
 	out.CrossZoneLoadBalancing = in.CrossZoneLoadBalancing
+	if in.Subnets != nil {
+		in, out := &in.Subnets, &out.Subnets
+		*out = make([]LoadBalancerSubnetSpec, len(*in))
+		for i := range *in {
+			if err := Convert_kops_LoadBalancerSubnetSpec_To_v1alpha2_LoadBalancerSubnetSpec(&(*in)[i], &(*out)[i], s); err != nil {
+				return err
+			}
+		}
+	} else {
+		out.Subnets = nil
+	}
 	return nil
 }
 
@@ -5146,6 +5178,28 @@ func Convert_kops_LoadBalancerAccessSpec_To_v1alpha2_LoadBalancerAccessSpec(in *
 	return autoConvert_kops_LoadBalancerAccessSpec_To_v1alpha2_LoadBalancerAccessSpec(in, out, s)
 }
 
+func autoConvert_v1alpha2_LoadBalancerSubnetSpec_To_kops_LoadBalancerSubnetSpec(in *LoadBalancerSubnetSpec, out *kops.LoadBalancerSubnetSpec, s conversion.Scope) error {
+	out.Name = in.Name
+	out.PrivateIPv4Address = in.PrivateIPv4Address
+	return nil
+}
+
+// Convert_v1alpha2_LoadBalancerSubnetSpec_To_kops_LoadBalancerSubnetSpec is an autogenerated conversion function.
+func Convert_v1alpha2_LoadBalancerSubnetSpec_To_kops_LoadBalancerSubnetSpec(in *LoadBalancerSubnetSpec, out *kops.LoadBalancerSubnetSpec, s conversion.Scope) error {
+	return autoConvert_v1alpha2_LoadBalancerSubnetSpec_To_kops_LoadBalancerSubnetSpec(in, out, s)
+}
+
+func autoConvert_kops_LoadBalancerSubnetSpec_To_v1alpha2_LoadBalancerSubnetSpec(in *kops.LoadBalancerSubnetSpec, out *LoadBalancerSubnetSpec, s conversion.Scope) error {
+	out.Name = in.Name
+	out.PrivateIPv4Address = in.PrivateIPv4Address
+	return nil
+}
+
+// Convert_kops_LoadBalancerSubnetSpec_To_v1alpha2_LoadBalancerSubnetSpec is an autogenerated conversion function.
+func Convert_kops_LoadBalancerSubnetSpec_To_v1alpha2_LoadBalancerSubnetSpec(in *kops.LoadBalancerSubnetSpec, out *LoadBalancerSubnetSpec, s conversion.Scope) error {
+	return autoConvert_kops_LoadBalancerSubnetSpec_To_v1alpha2_LoadBalancerSubnetSpec(in, out, s)
+}
+
 func autoConvert_v1alpha2_LyftVPCNetworkingSpec_To_kops_LyftVPCNetworkingSpec(in *LyftVPCNetworkingSpec, out *kops.LyftVPCNetworkingSpec, s conversion.Scope) error {
 	out.SubnetTags = in.SubnetTags
 	return nil

pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go

@@ -3470,6 +3470,13 @@ func (in *LoadBalancerAccessSpec) DeepCopyInto(out *LoadBalancerAccessSpec) {
 		*out = new(bool)
 		**out = **in
 	}
+	if in.Subnets != nil {
+		in, out := &in.Subnets, &out.Subnets
+		*out = make([]LoadBalancerSubnetSpec, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
+	}
 	return
 }
 
@@ -3483,6 +3490,27 @@ func (in *LoadBalancerAccessSpec) DeepCopy() *LoadBalancerAccessSpec {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *LoadBalancerSubnetSpec) DeepCopyInto(out *LoadBalancerSubnetSpec) {
+	*out = *in
+	if in.PrivateIPv4Address != nil {
+		in, out := &in.PrivateIPv4Address, &out.PrivateIPv4Address
+		*out = new(string)
+		**out = **in
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancerSubnetSpec.
+func (in *LoadBalancerSubnetSpec) DeepCopy() *LoadBalancerSubnetSpec {
+	if in == nil {
+		return nil
+	}
+	out := new(LoadBalancerSubnetSpec)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *LyftVPCNetworkingSpec) DeepCopyInto(out *LyftVPCNetworkingSpec) {
 	*out = *in

pkg/apis/kops/validation/aws.go

@@ -17,6 +17,8 @@ limitations under the License.
 package validation
 
 import (
+	"fmt"
+	"net"
 	"strconv"
 	"strings"
 
@@ -35,6 +37,7 @@ func awsValidateCluster(c *kops.Cluster) field.ErrorList {
 		if c.Spec.API.LoadBalancer != nil {
 			allErrs = append(allErrs, awsValidateAdditionalSecurityGroups(field.NewPath("spec", "api", "loadBalancer", "additionalSecurityGroups"), c.Spec.API.LoadBalancer.AdditionalSecurityGroups)...)
 			allErrs = append(allErrs, awsValidateSSLPolicy(field.NewPath("spec", "api", "loadBalancer", "sslPolicy"), c.Spec.API.LoadBalancer)...)
+			allErrs = append(allErrs, awsValidateLoadBalancerSubnets(field.NewPath("spec", "api", "loadBalancer", "subnets"), c.Spec)...)
 		}
 	}
 
@@ -196,3 +199,49 @@ func awsValidateSSLPolicy(fieldPath *field.Path, spec *kops.LoadBalancerAccessSp
 
 	return allErrs
 }
+
+func awsValidateLoadBalancerSubnets(fieldPath *field.Path, spec kops.ClusterSpec) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	lbSpec := spec.API.LoadBalancer
+
+	for i, subnet := range lbSpec.Subnets {
+		var clusterSubnet *kops.ClusterSubnetSpec
+		if subnet.Name == "" {
+			allErrs = append(allErrs, field.Required(fieldPath.Index(i).Child("name"), "subnet name can't be empty"))
+		} else {
+			for _, cs := range spec.Subnets {
+				if subnet.Name == cs.Name {
+					clusterSubnet = &cs
+					break
+				}
+			}
+			if clusterSubnet == nil {
+				allErrs = append(allErrs, field.NotFound(fieldPath.Index(i).Child("name"), fmt.Sprintf("subnet %q not found in cluster subnets", subnet.Name)))
+			}
+		}
+
+		if subnet.PrivateIPv4Address != nil {
+			if *subnet.PrivateIPv4Address == "" {
+				allErrs = append(allErrs, field.Required(fieldPath.Index(i).Child("privateIPv4Address"), "privateIPv4Address can't be empty"))
+			}
+			ip := net.ParseIP(*subnet.PrivateIPv4Address)
+			if ip == nil || ip.To4() == nil {
+				allErrs = append(allErrs, field.Invalid(fieldPath.Index(i).Child("privateIPv4Address"), subnet, "privateIPv4Address is not a valid IPv4 address"))
+			} else if clusterSubnet != nil {
+				_, ipNet, err := net.ParseCIDR(clusterSubnet.CIDR)
+				if err == nil { // we assume that the cidr is actually valid
+					if !ipNet.Contains(ip) {
+						allErrs = append(allErrs, field.Invalid(fieldPath.Index(i).Child("privateIPv4Address"), subnet, "privateIPv4Address is not part of the subnet CIDR"))
+					}
+				}
+
+			}
+			if lbSpec.Class != kops.LoadBalancerClassNetwork || lbSpec.Type != kops.LoadBalancerTypeInternal {
+				allErrs = append(allErrs, field.Forbidden(fieldPath.Index(i).Child("privateIPv4Address"), "privateIPv4Address only allowed for internal NLBs"))
+			}
+		}
+	}
+
+	return allErrs
+}

pkg/apis/kops/validation/aws_test.go

@@ -239,3 +239,141 @@ func TestInstanceMetadataOptions(t *testing.T) {
 		testErrors(t, test.ig.ObjectMeta.Name, errs, test.expected)
 	}
 }
+
+func TestLoadBalancerSubnets(t *testing.T) {
+	cidr := "10.0.0.0/24"
+	tests := []struct {
+		lbType         *string
+		class          *string
+		clusterSubnets []string
+		lbSubnets      []kops.LoadBalancerSubnetSpec
+		expected       []string
+	}{
+		{ // valid (no privateIPv4Address)
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "a",
+					PrivateIPv4Address: nil,
+				},
+				{
+					Name:               "b",
+					PrivateIPv4Address: nil,
+				},
+			},
+		},
+		{ // valid (with privateIPv4Address)
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "a",
+					PrivateIPv4Address: fi.String("10.0.0.10"),
+				},
+				{
+					Name:               "b",
+					PrivateIPv4Address: nil,
+				},
+			},
+		},
+		{ // empty subnet name
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "",
+					PrivateIPv4Address: nil,
+				},
+			},
+			expected: []string{"Required value::spec.api.loadBalancer.subnets[0].name"},
+		},
+		{ // subnet not found
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "d",
+					PrivateIPv4Address: nil,
+				},
+			},
+			expected: []string{"Not found::spec.api.loadBalancer.subnets[0].name"},
+		},
+		{ // empty privateIPv4Address
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "a",
+					PrivateIPv4Address: fi.String(""),
+				},
+			},
+			expected: []string{"Required value::spec.api.loadBalancer.subnets[0].privateIPv4Address"},
+		},
+		{ // invalid privateIPv4Address
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "a",
+					PrivateIPv4Address: fi.String("invalidip"),
+				},
+			},
+			expected: []string{"Invalid value::spec.api.loadBalancer.subnets[0].privateIPv4Address"},
+		},
+		{ // privateIPv4Address not matching subnet cidr
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "a",
+					PrivateIPv4Address: fi.String("11.0.0.10"),
+				},
+			},
+			expected: []string{"Invalid value::spec.api.loadBalancer.subnets[0].privateIPv4Address"},
+		},
+		{ // invalid class
+			class:          fi.String(string(kops.LoadBalancerClassClassic)),
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "a",
+					PrivateIPv4Address: fi.String("10.0.0.10"),
+				},
+			},
+			expected: []string{"Forbidden::spec.api.loadBalancer.subnets[0].privateIPv4Address"},
+		},
+		{ // invalid type
+			lbType:         fi.String(string(kops.LoadBalancerTypePublic)),
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "a",
+					PrivateIPv4Address: fi.String("10.0.0.10"),
+				},
+			},
+			expected: []string{"Forbidden::spec.api.loadBalancer.subnets[0].privateIPv4Address"},
+		},
+	}
+
+	for _, test := range tests {
+		cluster := kops.Cluster{
+			Spec: kops.ClusterSpec{
+				API: &kops.AccessSpec{
+					LoadBalancer: &kops.LoadBalancerAccessSpec{
+						Class: kops.LoadBalancerClassNetwork,
+						Type:  kops.LoadBalancerTypeInternal,
+					},
+				},
+			},
+		}
+		if test.class != nil {
+			cluster.Spec.API.LoadBalancer.Class = kops.LoadBalancerClass(*test.class)
+		}
+		if test.lbType != nil {
+			cluster.Spec.API.LoadBalancer.Type = kops.LoadBalancerType(*test.lbType)
+		}
+		for _, s := range test.clusterSubnets {
+			cluster.Spec.Subnets = append(cluster.Spec.Subnets, kops.ClusterSubnetSpec{
+				Name: s,
+				CIDR: cidr,
+			})
+		}
+		cluster.Spec.API.LoadBalancer.Subnets = test.lbSubnets
+		errs := awsValidateCluster(&cluster)
+		testErrors(t, test, errs, test.expected)
+	}
+}

pkg/apis/kops/zz_generated.deepcopy.go

@@ -3668,6 +3668,13 @@ func (in *LoadBalancerAccessSpec) DeepCopyInto(out *LoadBalancerAccessSpec) {
 		*out = new(bool)
 		**out = **in
 	}
+	if in.Subnets != nil {
+		in, out := &in.Subnets, &out.Subnets
+		*out = make([]LoadBalancerSubnetSpec, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
+	}
 	return
 }
 
@@ -3681,6 +3688,27 @@ func (in *LoadBalancerAccessSpec) DeepCopy() *LoadBalancerAccessSpec {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *LoadBalancerSubnetSpec) DeepCopyInto(out *LoadBalancerSubnetSpec) {
+	*out = *in
+	if in.PrivateIPv4Address != nil {
+		in, out := &in.PrivateIPv4Address, &out.PrivateIPv4Address
+		*out = new(string)
+		**out = **in
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancerSubnetSpec.
+func (in *LoadBalancerSubnetSpec) DeepCopy() *LoadBalancerSubnetSpec {
+	if in == nil {
+		return nil
+	}
+	out := new(LoadBalancerSubnetSpec)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *LyftVPCNetworkingSpec) DeepCopyInto(out *LyftVPCNetworkingSpec) {
 	*out = *in

pkg/model/awsmodel/api_loadbalancer.go

@@ -63,9 +63,29 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 		return fmt.Errorf("unhandled LoadBalancer type %q", lbSpec.Type)
 	}
 
-	// Compute the subnets - only one per zone, and then break ties based on chooseBestSubnetForELB
 	var elbSubnets []*awstasks.Subnet
-	{
+	var nlbSubnetMappings []*awstasks.SubnetMapping
+	if len(lbSpec.Subnets) != 0 {
+		// Subnets have been explicitly set
+		for _, subnet := range lbSpec.Subnets {
+			for _, clusterSubnet := range b.Cluster.Spec.Subnets {
+				if subnet.Name == clusterSubnet.Name {
+					elbSubnet := b.LinkToSubnet(&clusterSubnet)
+					elbSubnets = append(elbSubnets, elbSubnet)
+
+					nlbSubnetMapping := &awstasks.SubnetMapping{
+						Subnet: elbSubnet,
+					}
+					if subnet.PrivateIPv4Address != nil {
+						nlbSubnetMapping.PrivateIPv4Address = subnet.PrivateIPv4Address
+					}
+					nlbSubnetMappings = append(nlbSubnetMappings, nlbSubnetMapping)
+					break
+				}
+			}
+		}
+	} else {
+		// Compute the subnets - only one per zone, and then break ties based on chooseBestSubnetForELB
 		subnetsByZone := make(map[string][]*kops.ClusterSubnetSpec)
 		for i := range b.Cluster.Spec.Subnets {
 			subnet := &b.Cluster.Spec.Subnets[i]
@@ -91,7 +111,9 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 		for zone, subnets := range subnetsByZone {
 			subnet := b.chooseBestSubnetForELB(zone, subnets)
 
-			elbSubnets = append(elbSubnets, b.LinkToSubnet(subnet))
+			elbSubnet := b.LinkToSubnet(subnet)
+			elbSubnets = append(elbSubnets, elbSubnet)
+			nlbSubnetMappings = append(nlbSubnetMappings, &awstasks.SubnetMapping{Subnet: elbSubnet})
 		}
 	}
 
@@ -148,7 +170,7 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 			Lifecycle: b.Lifecycle,
 
 			LoadBalancerName: fi.String(loadBalancerName),
-			Subnets:          elbSubnets,
+			SubnetMappings:   nlbSubnetMappings,
 			Listeners:        nlbListeners,
 			TargetGroups:     make([]*awstasks.TargetGroup, 0),
 

tests/integration/update_cluster/complex/cloudformation.json

@@ -1268,9 +1268,11 @@
       "Properties": {
         "Name": "api-complex-example-com-vd3t5n",
         "Scheme": "internet-facing",
-        "Subnets": [
+        "SubnetMappings": [
           {
-            "Ref": "AWSEC2Subnetustest1acomplexexamplecom"
+            "SubnetId": {
+              "Ref": "AWSEC2Subnetustest1acomplexexamplecom"
+            }
           }
         ],
         "Type": "network",

tests/integration/update_cluster/complex/kubernetes.tf

@@ -540,7 +540,9 @@ resource "aws_lb" "api-complex-example-com" {
   internal                         = false
   load_balancer_type               = "network"
   name                             = "api-complex-example-com-vd3t5n"
-  subnets                          = [aws_subnet.us-test-1a-complex-example-com.id]
+  subnet_mapping {
+    subnet_id = aws_subnet.us-test-1a-complex-example-com.id
+  }
   tags = {
     "KubernetesCluster"                         = "complex.example.com"
     "Name"                                      = "api.complex.example.com"

upup/pkg/fi/cloudup/awstasks/BUILD.bazel

@@ -65,6 +65,7 @@ go_library(
         "sshkey_fitask.go",
         "subnet.go",
         "subnet_fitask.go",
+        "subnet_mapping.go",
         "tags.go",
         "targetgroup.go",
         "targetgroup_fitask.go",

upup/pkg/fi/cloudup/awstasks/network_load_balancer.go

@@ -26,13 +26,11 @@ import (
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/elbv2"
 	"github.com/aws/aws-sdk-go/service/route53"
-	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/klog/v2"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
 	"k8s.io/kops/upup/pkg/fi/cloudup/cloudformation"
 	"k8s.io/kops/upup/pkg/fi/cloudup/terraform"
-	"k8s.io/kops/util/pkg/slice"
 )
 
 // NetworkLoadBalancer manages an NLB.  We find the existing NLB using the Name tag.
@@ -53,7 +51,7 @@ type NetworkLoadBalancer struct {
 	DNSName      *string
 	HostedZoneId *string
 
-	Subnets []*Subnet
+	SubnetMappings []*SubnetMapping
 
 	Listeners []*NetworkLoadBalancerListener
 
@@ -354,7 +352,18 @@ func (e *NetworkLoadBalancer) Find(c *fi.Context) (*NetworkLoadBalancer, error)
 	}
 
 	for _, az := range lb.AvailabilityZones {
-		actual.Subnets = append(actual.Subnets, &Subnet{ID: az.SubnetId})
+		sm := &SubnetMapping{
+			Subnet: &Subnet{ID: az.SubnetId},
+		}
+		for _, a := range az.LoadBalancerAddresses {
+			if a.PrivateIPv4Address != nil {
+				if sm.PrivateIPv4Address != nil {
+					return nil, fmt.Errorf("NLB has more then one PrivateIPv4Address, which is unexpected. This is a bug in kOps, please open a GitHub issue.")
+				}
+				sm.PrivateIPv4Address = a.PrivateIPv4Address
+			}
+		}
+		actual.SubnetMappings = append(actual.SubnetMappings, sm)
 	}
 
 	{
@@ -433,8 +442,8 @@ func (e *NetworkLoadBalancer) Find(c *fi.Context) (*NetworkLoadBalancer, error)
 	}
 
 	// Avoid spurious mismatches
-	if subnetSlicesEqualIgnoreOrder(actual.Subnets, e.Subnets) {
-		actual.Subnets = e.Subnets
+	if subnetMappingSlicesEqualIgnoreOrder(actual.SubnetMappings, e.SubnetMappings) {
+		actual.SubnetMappings = e.SubnetMappings
 	}
 	if e.DNSName == nil {
 		e.DNSName = actual.DNSName
@@ -496,7 +505,7 @@ func (e *NetworkLoadBalancer) Run(c *fi.Context) error {
 
 func (e *NetworkLoadBalancer) Normalize() {
 	// We need to sort our arrays consistently, so we don't get spurious changes
-	sort.Stable(OrderSubnetsById(e.Subnets))
+	sort.Stable(OrderSubnetMappingsByID(e.SubnetMappings))
 	sort.Stable(OrderListenersByPort(e.Listeners))
 	sort.Stable(OrderTargetGroupsByName(e.TargetGroups))
 }
@@ -506,8 +515,8 @@ func (s *NetworkLoadBalancer) CheckChanges(a, e, changes *NetworkLoadBalancer) e
 		if fi.StringValue(e.Name) == "" {
 			return fi.RequiredField("Name")
 		}
-		if len(e.Subnets) == 0 {
-			return fi.RequiredField("Subnets")
+		if len(e.SubnetMappings) == 0 {
+			return fi.RequiredField("SubnetMappings")
 		}
 
 		if e.CrossZoneLoadBalancing != nil {
@@ -516,8 +525,21 @@ func (s *NetworkLoadBalancer) CheckChanges(a, e, changes *NetworkLoadBalancer) e
 			}
 		}
 	} else {
-		if len(changes.Subnets) > 0 {
-			return fi.FieldIsImmutable(e.Subnets, a.Subnets, field.NewPath("Subnets"))
+		if len(changes.SubnetMappings) > 0 {
+			expectedSubnets := make(map[string]*string)
+			for _, s := range e.SubnetMappings {
+				expectedSubnets[*s.Subnet.ID] = s.PrivateIPv4Address
+			}
+
+			for _, s := range a.SubnetMappings {
+				eIP, ok := expectedSubnets[*s.Subnet.ID]
+				if !ok {
+					return fmt.Errorf("network load balancers do not support detaching subnets")
+				}
+				if fi.StringValue(eIP) != fi.StringValue(s.PrivateIPv4Address) {
+					return fmt.Errorf("network load balancers do not support modifying address settings")
+				}
+			}
 		}
 	}
 	return nil
@@ -548,8 +570,11 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne
 		request.Scheme = e.Scheme
 		request.Type = e.Type
 
-		for _, subnet := range e.Subnets {
-			request.Subnets = append(request.Subnets, subnet.ID)
+		for _, subnetMapping := range e.SubnetMappings {
+			request.SubnetMappings = append(request.SubnetMappings, &elbv2.SubnetMapping{
+				SubnetId:           subnetMapping.Subnet.ID,
+				PrivateIPv4Address: subnetMapping.PrivateIPv4Address,
+			})
 		}
 
 		{
@@ -595,28 +620,29 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne
 
 		loadBalancerArn = fi.StringValue(lb.LoadBalancerArn)
 
-		if changes.Subnets != nil {
-			var expectedSubnets []string
-			for _, s := range e.Subnets {
-				expectedSubnets = append(expectedSubnets, fi.StringValue(s.ID))
+		if changes.SubnetMappings != nil {
+			actualSubnets := make(map[string]*string)
+			for _, s := range a.SubnetMappings {
+				actualSubnets[*s.Subnet.ID] = s.PrivateIPv4Address
 			}
 
-			var actualSubnets []string
-			for _, s := range a.Subnets {
-				actualSubnets = append(actualSubnets, fi.StringValue(s.ID))
+			var awsSubnetMappings []*elbv2.SubnetMapping
+			hasChanges := false
+			for _, s := range e.SubnetMappings {
+				aIP, ok := actualSubnets[*s.Subnet.ID]
+				if !ok || fi.StringValue(s.PrivateIPv4Address) != fi.StringValue(aIP) {
+					hasChanges = true
+				}
+				awsSubnetMappings = append(awsSubnetMappings, &elbv2.SubnetMapping{
+					SubnetId:           s.Subnet.ID,
+					PrivateIPv4Address: s.PrivateIPv4Address,
+				})
 			}
 
-			oldSubnetIDs := slice.GetUniqueStrings(expectedSubnets, actualSubnets)
-			if len(oldSubnetIDs) > 0 {
-				return fmt.Errorf("network load balancers do not support detaching subnets")
-			}
-
-			newSubnetIDs := slice.GetUniqueStrings(actualSubnets, expectedSubnets)
-			if len(newSubnetIDs) > 0 {
-
+			if hasChanges {
 				request := &elbv2.SetSubnetsInput{}
 				request.SetLoadBalancerArn(loadBalancerArn)
-				request.SetSubnets(aws.StringSlice(append(actualSubnets, newSubnetIDs...)))
+				request.SetSubnetMappings(awsSubnetMappings)
 
 				klog.V(2).Infof("Attaching Load Balancer to new subnets")
 				if _, err := t.Cloud.ELBV2().SetSubnets(request); err != nil {
@@ -680,15 +706,21 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne
 }
 
 type terraformNetworkLoadBalancer struct {
-	Name                   string               `json:"name" cty:"name"`
-	Internal               bool                 `json:"internal" cty:"internal"`
-	Type                   string               `json:"load_balancer_type" cty:"load_balancer_type"`
-	Subnets                []*terraform.Literal `json:"subnets" cty:"subnets"`
-	CrossZoneLoadBalancing bool                 `json:"enable_cross_zone_load_balancing" cty:"enable_cross_zone_load_balancing"`
+	Name                   string                                      `json:"name" cty:"name"`
+	Internal               bool                                        `json:"internal" cty:"internal"`
+	Type                   string                                      `json:"load_balancer_type" cty:"load_balancer_type"`
+	SubnetMappings         []terraformNetworkLoadBalancerSubnetMapping `json:"subnet_mapping" cty:"subnet_mapping"`
+	CrossZoneLoadBalancing bool                                        `json:"enable_cross_zone_load_balancing" cty:"enable_cross_zone_load_balancing"`
 
 	Tags map[string]string `json:"tags" cty:"tags"`
 }
 
+type terraformNetworkLoadBalancerSubnetMapping struct {
+	Subnet             *terraform.Literal `json:"subnet_id" cty:"subnet_id"`
+	AllocationID       *string            `json:"allocation_id,omitempty" cty:"allocation_id"`
+	PrivateIPv4Address *string            `json:"private_ipv4_address,omitempty" cty:"private_ipv4_address"`
+}
+
 type terraformNetworkLoadBalancerListener struct {
 	LoadBalancer   *terraform.Literal                           `json:"load_balancer_arn" cty:"load_balancer_arn"`
 	Port           int64                                        `json:"port" cty:"port"`
@@ -709,12 +741,14 @@ func (_ *NetworkLoadBalancer) RenderTerraform(t *terraform.TerraformTarget, a, e
 		Internal:               fi.StringValue(e.Scheme) == elbv2.LoadBalancerSchemeEnumInternal,
 		Type:                   elbv2.LoadBalancerTypeEnumNetwork,
 		Tags:                   e.Tags,
-		Subnets:                make([]*terraform.Literal, 0),
 		CrossZoneLoadBalancing: fi.BoolValue(e.CrossZoneLoadBalancing),
 	}
 
-	for _, subnet := range e.Subnets {
-		nlbTF.Subnets = append(nlbTF.Subnets, subnet.TerraformLink())
+	for _, subnetMapping := range e.SubnetMappings {
+		nlbTF.SubnetMappings = append(nlbTF.SubnetMappings, terraformNetworkLoadBalancerSubnetMapping{
+			Subnet:             subnetMapping.Subnet.TerraformLink(),
+			PrivateIPv4Address: subnetMapping.PrivateIPv4Address,
+		})
 	}
 
 	err := t.RenderResource("aws_lb", *e.Name, nlbTF)
@@ -770,11 +804,17 @@ func (e *NetworkLoadBalancer) TerraformLink(params ...string) *terraform.Literal
 }
 
 type cloudformationNetworkLoadBalancer struct {
-	Name    string                    `json:"Name"`
-	Scheme  string                    `json:"Scheme"`
-	Subnets []*cloudformation.Literal `json:"Subnets"`
-	Type    string                    `json:"Type"`
-	Tags    []cloudformationTag       `json:"Tags"`
+	Name           string                         `json:"Name"`
+	Scheme         string                         `json:"Scheme"`
+	SubnetMappings []*cloudformationSubnetMapping `json:"SubnetMappings"`
+	Type           string                         `json:"Type"`
+	Tags           []cloudformationTag            `json:"Tags"`
+}
+
+type cloudformationSubnetMapping struct {
+	Subnet             *cloudformation.Literal `json:"SubnetId"`
+	AllocationId       *string                 `json:"AllocationId,omitempty"`
+	PrivateIPv4Address *string                 `json:"PrivateIPv4Address,omitempty"`
 }
 
 type cloudformationNetworkLoadBalancerListener struct {
@@ -797,13 +837,15 @@ type cloudformationNetworkLoadBalancerListenerAction struct {
 
 func (_ *NetworkLoadBalancer) RenderCloudformation(t *cloudformation.CloudformationTarget, a, e, changes *NetworkLoadBalancer) error {
 	nlbCF := &cloudformationNetworkLoadBalancer{
-		Name:    *e.LoadBalancerName,
-		Subnets: make([]*cloudformation.Literal, 0),
-		Type:    elbv2.LoadBalancerTypeEnumNetwork,
-		Tags:    buildCloudformationTags(e.Tags),
+		Name: *e.LoadBalancerName,
+		Type: elbv2.LoadBalancerTypeEnumNetwork,
+		Tags: buildCloudformationTags(e.Tags),
 	}
-	for _, subnet := range e.Subnets {
-		nlbCF.Subnets = append(nlbCF.Subnets, subnet.CloudformationLink())
+	for _, subnetMapping := range e.SubnetMappings {
+		nlbCF.SubnetMappings = append(nlbCF.SubnetMappings, &cloudformationSubnetMapping{
+			Subnet:             subnetMapping.Subnet.CloudformationLink(),
+			PrivateIPv4Address: subnetMapping.PrivateIPv4Address,
+		})
 	}
 	if e.Scheme != nil {
 		nlbCF.Scheme = *e.Scheme

upup/pkg/fi/cloudup/awstasks/subnet_mapping.go

@@ -0,0 +1,82 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package awstasks
+
+import (
+	"k8s.io/klog/v2"
+	"k8s.io/kops/upup/pkg/fi"
+)
+
+type SubnetMapping struct {
+	Subnet *Subnet
+
+	// PrivateIPv4Address only valid for NLBs
+	PrivateIPv4Address *string
+}
+
+// OrderSubnetsById implements sort.Interface for []Subnet, based on ID
+type OrderSubnetMappingsByID []*SubnetMapping
+
+func (a OrderSubnetMappingsByID) Len() int      { return len(a) }
+func (a OrderSubnetMappingsByID) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
+func (a OrderSubnetMappingsByID) Less(i, j int) bool {
+	v1 := fi.StringValue(a[i].Subnet.ID)
+	v2 := fi.StringValue(a[j].Subnet.ID)
+	if v1 == v2 {
+		return fi.StringValue(a[i].PrivateIPv4Address) < fi.StringValue(a[j].PrivateIPv4Address)
+	}
+	return v1 < v2
+}
+
+func subnetMappingSlicesEqualIgnoreOrder(l, r []*SubnetMapping) bool {
+	lBySubnet := make(map[string]*SubnetMapping)
+	for _, s := range l {
+		lBySubnet[*s.Subnet.ID] = s
+	}
+	rBySubnet := make(map[string]*SubnetMapping)
+	for _, s := range r {
+		if s.Subnet == nil || s.Subnet.ID == nil {
+			klog.V(4).Infof("Subnet ID not set; returning not-equal: %v", s)
+			return false
+		}
+		rBySubnet[*s.Subnet.ID] = s
+	}
+	if len(lBySubnet) != len(rBySubnet) {
+		return false
+	}
+
+	for n, s := range lBySubnet {
+		s2, ok := rBySubnet[n]
+		if !ok {
+			return false
+		}
+		if fi.StringValue(s.PrivateIPv4Address) != fi.StringValue(s2.PrivateIPv4Address) {
+			return false
+		}
+	}
+	return true
+}
+
+func (e *SubnetMapping) GetDependencies(tasks map[string]fi.Task) []fi.Task {
+	var deps []fi.Task
+	for _, task := range tasks {
+		if _, ok := task.(*Subnet); ok {
+			deps = append(deps, task)
+		}
+	}
+	return deps
+}