Merge pull request #10872 from timothyclarke/feature/NLB-EIP

Adding Elastic IP Allocations to NLB API

cloudmock/aws/mockelbv2/loadbalancers.go

@@ -97,6 +97,9 @@ func (m *MockELBV2) CreateLoadBalancer(request *elbv2.CreateLoadBalancerInput) (
 		if subnetMapping.PrivateIPv4Address != nil {
 			lbAddrs = append(lbAddrs, &elbv2.LoadBalancerAddress{PrivateIPv4Address: subnetMapping.PrivateIPv4Address})
 		}
+		if subnetMapping.AllocationId != nil {
+			lbAddrs = append(lbAddrs, &elbv2.LoadBalancerAddress{AllocationId: subnetMapping.AllocationId})
+		}
 		zones = append(zones, &elbv2.AvailabilityZone{
 			SubnetId:              subnetMapping.SubnetId,
 			LoadBalancerAddresses: lbAddrs,

docs/cluster_spec.md

@@ -137,6 +137,19 @@ spec:
 
 The specified IPv4 addresses must be part of the subnets CIDR. They can not be changed after initial deployment.
 
+If the `type` is `Public` and the `class` is `Network`, you can also specify an Elastic IP allocationID to bind a fixed public IP address per subnet. Pleae note only IPv4 addresses have been tested:
+```yaml
+spec:
+  api:
+    loadBalancer:
+      type: Public
+      subnets:
+        - name: utility-subnet-a
+          allocationID: eipalloc-222ghi789
+```
+
+The specified Allocation ID's must already be created manually or external infrastructure as code, eg Terraform. You will need to place the loadBalanacer in the utility subnets for external connectivity.
+
 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`.
 

k8s/crds/kops.k8s.io_clusters.yaml

@@ -119,6 +119,10 @@ spec:
                           description: LoadBalancerSubnetSpec provides configuration
                             for subnets used for a load balancer
                           properties:
+                            allocationId:
+                              description: AllocationID specifies the Elastic IP Allocation
+                                ID for use by a NLB
+                              type: string
                             name:
                               description: Name specifies the name of the cluster
                                 subnet

pkg/apis/kops/cluster.go

@@ -383,6 +383,8 @@ type LoadBalancerSubnetSpec struct {
 	Name string `json:"name,omitempty"`
 	// PrivateIPv4Address specifies the private IPv4 address to use for a NLB
 	PrivateIPv4Address *string `json:"privateIPv4Address,omitempty"`
+	// AllocationID specifies the Elastic IP Allocation ID for use by a NLB
+	AllocationID *string `json:"allocationId,omitempty"`
 }
 
 // LoadBalancerAccessSpec provides configuration details related to API LoadBalancer and its access

pkg/apis/kops/v1alpha2/cluster.go

@@ -385,6 +385,8 @@ type LoadBalancerSubnetSpec struct {
 	Name string `json:"name,omitempty"`
 	// PrivateIPv4Address specifies the private IPv4 address to use for a NLB
 	PrivateIPv4Address *string `json:"privateIPv4Address,omitempty"`
+	// AllocationID specifies the Elastic IP Allocation ID for use by a NLB
+	AllocationID *string `json:"allocationId,omitempty"`
 }
 
 // LoadBalancerAccessSpec provides configuration details related to API LoadBalancer and its access

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

@@ -5183,6 +5183,7 @@ func Convert_kops_LoadBalancerAccessSpec_To_v1alpha2_LoadBalancerAccessSpec(in *
 func autoConvert_v1alpha2_LoadBalancerSubnetSpec_To_kops_LoadBalancerSubnetSpec(in *LoadBalancerSubnetSpec, out *kops.LoadBalancerSubnetSpec, s conversion.Scope) error {
 	out.Name = in.Name
 	out.PrivateIPv4Address = in.PrivateIPv4Address
+	out.AllocationID = in.AllocationID
 	return nil
 }
 
@@ -5194,6 +5195,7 @@ func Convert_v1alpha2_LoadBalancerSubnetSpec_To_kops_LoadBalancerSubnetSpec(in *
 func autoConvert_kops_LoadBalancerSubnetSpec_To_v1alpha2_LoadBalancerSubnetSpec(in *kops.LoadBalancerSubnetSpec, out *LoadBalancerSubnetSpec, s conversion.Scope) error {
 	out.Name = in.Name
 	out.PrivateIPv4Address = in.PrivateIPv4Address
+	out.AllocationID = in.AllocationID
 	return nil
 }
 

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

@@ -3503,6 +3503,11 @@ func (in *LoadBalancerSubnetSpec) DeepCopyInto(out *LoadBalancerSubnetSpec) {
 		*out = new(string)
 		**out = **in
 	}
+	if in.AllocationID != nil {
+		in, out := &in.AllocationID, &out.AllocationID
+		*out = new(string)
+		**out = **in
+	}
 	return
 }
 

pkg/apis/kops/validation/aws.go

@@ -263,6 +263,16 @@ func awsValidateLoadBalancerSubnets(fieldPath *field.Path, spec kops.ClusterSpec
 				allErrs = append(allErrs, field.Forbidden(fieldPath.Index(i).Child("privateIPv4Address"), "privateIPv4Address only allowed for internal NLBs"))
 			}
 		}
+
+		if subnet.AllocationID != nil {
+			if *subnet.AllocationID == "" {
+				allErrs = append(allErrs, field.Required(fieldPath.Index(i).Child("allocationID"), "allocationID can't be empty"))
+			}
+
+			if lbSpec.Class != kops.LoadBalancerClassNetwork || lbSpec.Type == kops.LoadBalancerTypeInternal {
+				allErrs = append(allErrs, field.Forbidden(fieldPath.Index(i).Child("allocationID"), "allocationID only allowed for Public NLBs"))
+			}
+		}
 	}
 
 	return allErrs

pkg/apis/kops/validation/aws_test.go

@@ -352,16 +352,18 @@ func TestLoadBalancerSubnets(t *testing.T) {
 		lbSubnets      []kops.LoadBalancerSubnetSpec
 		expected       []string
 	}{
-		{ // valid (no privateIPv4Address)
+		{ // valid (no privateIPv4Address, no allocationID)
 			clusterSubnets: []string{"a", "b", "c"},
 			lbSubnets: []kops.LoadBalancerSubnetSpec{
 				{
 					Name:               "a",
 					PrivateIPv4Address: nil,
+					AllocationID:       nil,
 				},
 				{
 					Name:               "b",
 					PrivateIPv4Address: nil,
+					AllocationID:       nil,
 				},
 			},
 		},
@@ -371,10 +373,12 @@ func TestLoadBalancerSubnets(t *testing.T) {
 				{
 					Name:               "a",
 					PrivateIPv4Address: fi.String("10.0.0.10"),
+					AllocationID:       nil,
 				},
 				{
 					Name:               "b",
 					PrivateIPv4Address: nil,
+					AllocationID:       nil,
 				},
 			},
 		},
@@ -384,6 +388,7 @@ func TestLoadBalancerSubnets(t *testing.T) {
 				{
 					Name:               "",
 					PrivateIPv4Address: nil,
+					AllocationID:       nil,
 				},
 			},
 			expected: []string{"Required value::spec.api.loadBalancer.subnets[0].name"},
@@ -394,62 +399,103 @@ func TestLoadBalancerSubnets(t *testing.T) {
 				{
 					Name:               "d",
 					PrivateIPv4Address: nil,
+					AllocationID:       nil,
 				},
 			},
 			expected: []string{"Not found::spec.api.loadBalancer.subnets[0].name"},
 		},
-		{ // empty privateIPv4Address
+		{ // empty privateIPv4Address, no allocationID
 			clusterSubnets: []string{"a", "b", "c"},
 			lbSubnets: []kops.LoadBalancerSubnetSpec{
 				{
 					Name:               "a",
 					PrivateIPv4Address: fi.String(""),
+					AllocationID:       nil,
 				},
 			},
 			expected: []string{"Required value::spec.api.loadBalancer.subnets[0].privateIPv4Address"},
 		},
-		{ // invalid privateIPv4Address
+		{ // empty no privateIPv4Address, with allocationID
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "a",
+					PrivateIPv4Address: nil,
+					AllocationID:       fi.String(""),
+				},
+			},
+			expected: []string{"Required value::spec.api.loadBalancer.subnets[0].allocationID"},
+		},
+		{ // invalid privateIPv4Address, no allocationID
 			clusterSubnets: []string{"a", "b", "c"},
 			lbSubnets: []kops.LoadBalancerSubnetSpec{
 				{
 					Name:               "a",
 					PrivateIPv4Address: fi.String("invalidip"),
+					AllocationID:       nil,
 				},
 			},
 			expected: []string{"Invalid value::spec.api.loadBalancer.subnets[0].privateIPv4Address"},
 		},
-		{ // privateIPv4Address not matching subnet cidr
+		{ // privateIPv4Address not matching subnet cidr, no allocationID
 			clusterSubnets: []string{"a", "b", "c"},
 			lbSubnets: []kops.LoadBalancerSubnetSpec{
 				{
 					Name:               "a",
 					PrivateIPv4Address: fi.String("11.0.0.10"),
+					AllocationID:       nil,
 				},
 			},
 			expected: []string{"Invalid value::spec.api.loadBalancer.subnets[0].privateIPv4Address"},
 		},
-		{ // invalid class
+		{ // invalid class - with privateIPv4Address, no allocationID
 			class:          fi.String(string(kops.LoadBalancerClassClassic)),
 			clusterSubnets: []string{"a", "b", "c"},
 			lbSubnets: []kops.LoadBalancerSubnetSpec{
 				{
 					Name:               "a",
 					PrivateIPv4Address: fi.String("10.0.0.10"),
+					AllocationID:       nil,
 				},
 			},
 			expected: []string{"Forbidden::spec.api.loadBalancer.subnets[0].privateIPv4Address"},
 		},
-		{ // invalid type
+		{ // invalid class - no privateIPv4Address, with allocationID
+			class:          fi.String(string(kops.LoadBalancerClassClassic)),
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "a",
+					PrivateIPv4Address: nil,
+					AllocationID:       fi.String("eipalloc-222ghi789"),
+				},
+			},
+			expected: []string{"Forbidden::spec.api.loadBalancer.subnets[0].allocationID"},
+		},
+		{ // invalid type external for private IP
 			lbType:         fi.String(string(kops.LoadBalancerTypePublic)),
 			clusterSubnets: []string{"a", "b", "c"},
 			lbSubnets: []kops.LoadBalancerSubnetSpec{
 				{
 					Name:               "a",
 					PrivateIPv4Address: fi.String("10.0.0.10"),
+					AllocationID:       nil,
 				},
 			},
 			expected: []string{"Forbidden::spec.api.loadBalancer.subnets[0].privateIPv4Address"},
 		},
+		{ // invalid type Internal for public IP
+			lbType:         fi.String(string(kops.LoadBalancerTypeInternal)),
+			clusterSubnets: []string{"a", "b", "c"},
+			lbSubnets: []kops.LoadBalancerSubnetSpec{
+				{
+					Name:               "a",
+					PrivateIPv4Address: nil,
+					AllocationID:       fi.String("eipalloc-222ghi789"),
+				},
+			},
+			expected: []string{"Forbidden::spec.api.loadBalancer.subnets[0].allocationID"},
+		},
 	}
 
 	for _, test := range tests {

pkg/apis/kops/zz_generated.deepcopy.go

@@ -3701,6 +3701,11 @@ func (in *LoadBalancerSubnetSpec) DeepCopyInto(out *LoadBalancerSubnetSpec) {
 		*out = new(string)
 		**out = **in
 	}
+	if in.AllocationID != nil {
+		in, out := &in.AllocationID, &out.AllocationID
+		*out = new(string)
+		**out = **in
+	}
 	return
 }
 

pkg/model/awsmodel/api_loadbalancer.go

@@ -79,6 +79,9 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 					if subnet.PrivateIPv4Address != nil {
 						nlbSubnetMapping.PrivateIPv4Address = subnet.PrivateIPv4Address
 					}
+					if subnet.AllocationID != nil {
+						nlbSubnetMapping.AllocationID = subnet.AllocationID
+					}
 					nlbSubnetMappings = append(nlbSubnetMappings, nlbSubnetMapping)
 					break
 				}

tests/integration/update_cluster/complex/cloudformation.json

@@ -1272,7 +1272,8 @@
           {
             "SubnetId": {
               "Ref": "AWSEC2Subnetustest1acomplexexamplecom"
-            }
+            },
+            "AllocationId": "eipalloc-012345a678b9cdefa"
           }
         ],
         "Type": "network",

tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml

@@ -14,6 +14,9 @@ spec:
       class: Network
       sslCertificate: arn:aws:acm:us-test-1:000000000000:certificate/123456789012-1234-1234-1234-12345678
       sslPolicy: ELBSecurityPolicy-2016-08
+      subnets:
+        - name: us-test-1a
+          allocationId: eipalloc-012345a678b9cdefa
   kubernetesApiAccess:
   - 1.1.1.0/24
   - 2001:0:8500::/40

tests/integration/update_cluster/complex/in-v1alpha2.yaml

@@ -13,7 +13,10 @@ spec:
       crossZoneLoadBalancing: true
       class: Network
       sslCertificate: arn:aws:acm:us-test-1:000000000000:certificate/123456789012-1234-1234-1234-12345678
-      sslPolicy: ELBSecurityPolicy-2016-08 
+      sslPolicy: ELBSecurityPolicy-2016-08
+      subnets:
+        - name: us-test-1a
+          allocationId: eipalloc-012345a678b9cdefa
   kubernetesApiAccess:
   - 1.1.1.0/24
   - 2001:0:8500::/40

tests/integration/update_cluster/complex/kubernetes.tf

@@ -541,7 +541,8 @@ resource "aws_lb" "api-complex-example-com" {
   load_balancer_type               = "network"
   name                             = "api-complex-example-com-vd3t5n"
   subnet_mapping {
-    subnet_id = aws_subnet.us-test-1a-complex-example-com.id
+    allocation_id = "eipalloc-012345a678b9cdefa"
+    subnet_id     = aws_subnet.us-test-1a-complex-example-com.id
   }
   tags = {
     "KubernetesCluster"                         = "complex.example.com"

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

@@ -362,6 +362,12 @@ func (e *NetworkLoadBalancer) Find(c *fi.Context) (*NetworkLoadBalancer, error)
 				}
 				sm.PrivateIPv4Address = a.PrivateIPv4Address
 			}
+			if a.AllocationId != nil {
+				if sm.AllocationID != nil {
+					return nil, fmt.Errorf("NLB has more then one AllocationID per subnet, which is unexpected. This is a bug in kOps, please open a GitHub issue.")
+				}
+				sm.AllocationID = a.AllocationId
+			}
 		}
 		actual.SubnetMappings = append(actual.SubnetMappings, sm)
 	}
@@ -528,7 +534,13 @@ func (s *NetworkLoadBalancer) CheckChanges(a, e, changes *NetworkLoadBalancer) e
 		if len(changes.SubnetMappings) > 0 {
 			expectedSubnets := make(map[string]*string)
 			for _, s := range e.SubnetMappings {
-				expectedSubnets[*s.Subnet.ID] = s.PrivateIPv4Address
+				//expectedSubnets[*s.Subnet.ID] = s
+				if s.AllocationID != nil {
+					expectedSubnets[*s.Subnet.ID] = s.AllocationID
+				}
+				if s.PrivateIPv4Address != nil {
+					expectedSubnets[*s.Subnet.ID] = s.PrivateIPv4Address
+				}
 			}
 
 			for _, s := range a.SubnetMappings {
@@ -536,7 +548,7 @@ func (s *NetworkLoadBalancer) CheckChanges(a, e, changes *NetworkLoadBalancer) e
 				if !ok {
 					return fmt.Errorf("network load balancers do not support detaching subnets")
 				}
-				if fi.StringValue(eIP) != fi.StringValue(s.PrivateIPv4Address) {
+				if fi.StringValue(eIP) != fi.StringValue(s.PrivateIPv4Address) || fi.StringValue(eIP) != fi.StringValue(s.AllocationID) {
 					return fmt.Errorf("network load balancers do not support modifying address settings")
 				}
 			}
@@ -573,6 +585,7 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne
 		for _, subnetMapping := range e.SubnetMappings {
 			request.SubnetMappings = append(request.SubnetMappings, &elbv2.SubnetMapping{
 				SubnetId:           subnetMapping.Subnet.ID,
+				AllocationId:       subnetMapping.AllocationID,
 				PrivateIPv4Address: subnetMapping.PrivateIPv4Address,
 			})
 		}
@@ -623,18 +636,25 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne
 		if changes.SubnetMappings != nil {
 			actualSubnets := make(map[string]*string)
 			for _, s := range a.SubnetMappings {
-				actualSubnets[*s.Subnet.ID] = s.PrivateIPv4Address
+				//actualSubnets[*s.Subnet.ID] = s
+				if s.AllocationID != nil {
+					actualSubnets[*s.Subnet.ID] = s.AllocationID
+				}
+				if s.PrivateIPv4Address != nil {
+					actualSubnets[*s.Subnet.ID] = s.PrivateIPv4Address
+				}
 			}
 
 			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) {
+				if !ok || (fi.StringValue(s.PrivateIPv4Address) != fi.StringValue(aIP) && fi.StringValue(s.AllocationID) != fi.StringValue(aIP)) {
 					hasChanges = true
 				}
 				awsSubnetMappings = append(awsSubnetMappings, &elbv2.SubnetMapping{
 					SubnetId:           s.Subnet.ID,
+					AllocationId:       s.AllocationID,
 					PrivateIPv4Address: s.PrivateIPv4Address,
 				})
 			}
@@ -747,6 +767,7 @@ func (_ *NetworkLoadBalancer) RenderTerraform(t *terraform.TerraformTarget, a, e
 	for _, subnetMapping := range e.SubnetMappings {
 		nlbTF.SubnetMappings = append(nlbTF.SubnetMappings, terraformNetworkLoadBalancerSubnetMapping{
 			Subnet:             subnetMapping.Subnet.TerraformLink(),
+			AllocationID:       subnetMapping.AllocationID,
 			PrivateIPv4Address: subnetMapping.PrivateIPv4Address,
 		})
 	}
@@ -844,6 +865,7 @@ func (_ *NetworkLoadBalancer) RenderCloudformation(t *cloudformation.Cloudformat
 	for _, subnetMapping := range e.SubnetMappings {
 		nlbCF.SubnetMappings = append(nlbCF.SubnetMappings, &cloudformationSubnetMapping{
 			Subnet:             subnetMapping.Subnet.CloudformationLink(),
+			AllocationId:       subnetMapping.AllocationID,
 			PrivateIPv4Address: subnetMapping.PrivateIPv4Address,
 		})
 	}

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

@@ -26,6 +26,8 @@ type SubnetMapping struct {
 
 	// PrivateIPv4Address only valid for NLBs
 	PrivateIPv4Address *string
+	// AllocationID only valid for NLBs
+	AllocationID *string
 }
 
 // OrderSubnetsById implements sort.Interface for []Subnet, based on ID
@@ -37,7 +39,12 @@ 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)
+		if a[i].PrivateIPv4Address != nil && a[j].PrivateIPv4Address != nil {
+			return fi.StringValue(a[i].PrivateIPv4Address) < fi.StringValue(a[j].PrivateIPv4Address)
+		}
+		if a[i].AllocationID != nil && a[j].AllocationID != nil {
+			return fi.StringValue(a[i].AllocationID) < fi.StringValue(a[j].AllocationID)
+		}
 	}
 	return v1 < v2
 }
@@ -67,6 +74,9 @@ func subnetMappingSlicesEqualIgnoreOrder(l, r []*SubnetMapping) bool {
 		if fi.StringValue(s.PrivateIPv4Address) != fi.StringValue(s2.PrivateIPv4Address) {
 			return false
 		}
+		if fi.StringValue(s.AllocationID) != fi.StringValue(s2.AllocationID) {
+			return false
+		}
 	}
 	return true
 }