diff --git a/cloudmock/aws/mockelbv2/loadbalancers.go b/cloudmock/aws/mockelbv2/loadbalancers.go index 52daada518..ae0d54b39a 100644 --- a/cloudmock/aws/mockelbv2/loadbalancers.go +++ b/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 diff --git a/docs/cluster_spec.md b/docs/cluster_spec.md index 55720b5cfe..b4d01e1f58 100644 --- a/docs/cluster_spec.md +++ b/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 diff --git a/k8s/crds/kops.k8s.io_clusters.yaml b/k8s/crds/kops.k8s.io_clusters.yaml index f457e225fd..be73b563c1 100644 --- a/k8s/crds/kops.k8s.io_clusters.yaml +++ b/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. diff --git a/pkg/apis/kops/cluster.go b/pkg/apis/kops/cluster.go index 0fa38a1869..5adf26d6f1 100644 --- a/pkg/apis/kops/cluster.go +++ b/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 diff --git a/pkg/apis/kops/v1alpha2/cluster.go b/pkg/apis/kops/v1alpha2/cluster.go index 8fe5f20e8a..e91611ab72 100644 --- a/pkg/apis/kops/v1alpha2/cluster.go +++ b/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 diff --git a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go index bbfdc90843..300a9783ae 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go +++ b/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 diff --git a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go index 1a8a7c5a4d..a543d8f23a 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go +++ b/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 diff --git a/pkg/apis/kops/validation/aws.go b/pkg/apis/kops/validation/aws.go index 67352409d4..14472c7da8 100644 --- a/pkg/apis/kops/validation/aws.go +++ b/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 +} diff --git a/pkg/apis/kops/validation/aws_test.go b/pkg/apis/kops/validation/aws_test.go index 4b777acbb3..bccf33d784 100644 --- a/pkg/apis/kops/validation/aws_test.go +++ b/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) + } +} diff --git a/pkg/apis/kops/zz_generated.deepcopy.go b/pkg/apis/kops/zz_generated.deepcopy.go index 5baf5668f6..37dec81945 100644 --- a/pkg/apis/kops/zz_generated.deepcopy.go +++ b/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 diff --git a/pkg/model/awsmodel/api_loadbalancer.go b/pkg/model/awsmodel/api_loadbalancer.go index 7e771c97fa..ad95969f6b 100644 --- a/pkg/model/awsmodel/api_loadbalancer.go +++ b/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), diff --git a/tests/integration/update_cluster/complex/cloudformation.json b/tests/integration/update_cluster/complex/cloudformation.json index 0c3c4783a9..a615e17961 100644 --- a/tests/integration/update_cluster/complex/cloudformation.json +++ b/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", diff --git a/tests/integration/update_cluster/complex/kubernetes.tf b/tests/integration/update_cluster/complex/kubernetes.tf index 0fa7a0d7ad..9ca30c1915 100644 --- a/tests/integration/update_cluster/complex/kubernetes.tf +++ b/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" diff --git a/upup/pkg/fi/cloudup/awstasks/BUILD.bazel b/upup/pkg/fi/cloudup/awstasks/BUILD.bazel index ffb6fd859c..cdd8ff64e1 100644 --- a/upup/pkg/fi/cloudup/awstasks/BUILD.bazel +++ b/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", diff --git a/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go b/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go index 8566b0d22c..12a8aab19a 100644 --- a/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go +++ b/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 diff --git a/upup/pkg/fi/cloudup/awstasks/subnet_mapping.go b/upup/pkg/fi/cloudup/awstasks/subnet_mapping.go new file mode 100644 index 0000000000..5854bab7df --- /dev/null +++ b/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 +}