diff --git a/pkg/apis/kops/validation/aws.go b/pkg/apis/kops/validation/aws.go index 67352409d4..65f189d850 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.Invalid(fieldPath.Index(i).Child("name"), subnet, "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.Invalid(fieldPath.Index(i).Child("name"), subnet, fmt.Sprintf("subnet %q not found in cluster subnets", subnet.Name))) + } + } + + if subnet.PrivateIPv4Address != nil { + if *subnet.PrivateIPv4Address == "" { + allErrs = append(allErrs, field.Invalid(fieldPath.Index(i).Child("privateIPv4Address"), subnet, "privateIPv4Address can't be empty")) + } + ip := net.ParseIP(*subnet.PrivateIPv4Address) + if ip == 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.Invalid(fieldPath.Index(i).Child("privateIPv4Address"), subnet, "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..32f64635f1 100644 --- a/pkg/apis/kops/validation/aws_test.go +++ b/pkg/apis/kops/validation/aws_test.go @@ -239,3 +239,143 @@ 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{"Invalid value::spec.api.loadBalancer.subnets[0].name"}, + }, + { // subnet not found + clusterSubnets: []string{"a", "b", "c"}, + lbSubnets: []kops.LoadBalancerSubnetSpec{ + { + Name: "d", + PrivateIPv4Address: nil, + }, + }, + expected: []string{"Invalid value::spec.api.loadBalancer.subnets[0].name"}, + }, + { // empty privateIPv4Address + clusterSubnets: []string{"a", "b", "c"}, + lbSubnets: []kops.LoadBalancerSubnetSpec{ + { + Name: "a", + PrivateIPv4Address: fi.String(""), + }, + }, + expected: []string{"Invalid 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{"Invalid value::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{"Invalid value::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, + }) + } + for _, s := range test.lbSubnets { + cluster.Spec.API.LoadBalancer.Subnets = append(cluster.Spec.API.LoadBalancer.Subnets, s) + } + errs := awsValidateCluster(&cluster) + testErrors(t, test, errs, test.expected) + } +} diff --git a/pkg/model/awsmodel/api_loadbalancer.go b/pkg/model/awsmodel/api_loadbalancer.go index 7d86fd30c3..f199eb948b 100644 --- a/pkg/model/awsmodel/api_loadbalancer.go +++ b/pkg/model/awsmodel/api_loadbalancer.go @@ -68,31 +68,19 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error { if len(lbSpec.Subnets) != 0 { // Subnets have been explicitly set for _, subnet := range lbSpec.Subnets { - found := false for _, clusterSubnet := range b.Cluster.Spec.Subnets { if subnet.Name == clusterSubnet.Name { - found = true elbSubnet := b.LinkToSubnet(&clusterSubnet) elbSubnets = append(elbSubnets, elbSubnet) nlbSubnetMappings = append(nlbSubnetMappings, &awstasks.SubnetMapping{ Subnet: elbSubnet, }) if subnet.PrivateIPv4Address != nil { - if *subnet.PrivateIPv4Address == "" { - return fmt.Errorf("privateIPv4Address can't be empty") - } - if lbSpec.Class != kops.LoadBalancerClassNetwork || lbSpec.Type != kops.LoadBalancerTypeInternal { - return fmt.Errorf("privateIPv4Address only allowed for internal NLBs") - } - nlbSubnetMappings[len(nlbSubnetMappings)-1].PrivateIPv4Address = subnet.PrivateIPv4Address } break } } - if !found { - return fmt.Errorf("subnet %q not found in cluster subnets", subnet.Name) - } } } else { // Compute the subnets - only one per zone, and then break ties based on chooseBestSubnetForELB