From dd8f8c61e7057c2d262a384e81ec5f1711460887 Mon Sep 17 00:00:00 2001 From: Alex Williams Date: Fri, 19 Feb 2021 03:26:25 +0000 Subject: [PATCH] Add support for specifying PrivateIpv4Address via annotation (#1762) * Add support for specifying PrivateIpv4Address via annotation Signed-off-by: Alex Williams * Find IP my matching CIDR. Change annoation. * Rename ipForSubnet and change if structure * Catch ip parsing errors. Add test. Fmt changes --- docs/guide/service/annotations.md | 53 ++-- pkg/annotations/constants.go | 1 + pkg/networking/subnet_resolver.go | 2 +- pkg/networking/subnet_resolver_test.go | 7 +- pkg/service/model_build_load_balancer.go | 61 ++++- pkg/service/model_build_load_balancer_test.go | 244 +++++++++++++++++- 6 files changed, 325 insertions(+), 43 deletions(-) diff --git a/docs/guide/service/annotations.md b/docs/guide/service/annotations.md index e511b6e27..e1799d9d6 100644 --- a/docs/guide/service/annotations.md +++ b/docs/guide/service/annotations.md @@ -9,32 +9,33 @@ - json: `"{ \"key\": \"value\" }"` ## Annotations -| Name | Type | Default | Notes | -|--------------------------------------------------------------------------------|------------|---------------------------|------------------------| -| service.beta.kubernetes.io/aws-load-balancer-type | string | | | -| service.beta.kubernetes.io/aws-load-balancer-internal | boolean | false | | -| [service.beta.kubernetes.io/aws-load-balancer-proxy-protocol](#proxy-protocol-v2) | string | | Set to `"*"` to enable | -| service.beta.kubernetes.io/aws-load-balancer-ip-address-type | string | ipv4 | ipv4 \| dualstack | -| service.beta.kubernetes.io/aws-load-balancer-access-log-enabled | boolean | false | | -| service.beta.kubernetes.io/aws-load-balancer-access-log-s3-bucket-name | string | | | -| service.beta.kubernetes.io/aws-load-balancer-access-log-s3-bucket-prefix | string | | | -| service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled | boolean | false | | -| service.beta.kubernetes.io/aws-load-balancer-ssl-cert | stringList | | | -| service.beta.kubernetes.io/aws-load-balancer-ssl-ports | stringList | | | -| service.beta.kubernetes.io/aws-load-balancer-ssl-negotiation-policy | string | ELBSecurityPolicy-2016-08 | | -| service.beta.kubernetes.io/aws-load-balancer-backend-protocol | string | | | -| service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags | stringMap | | | -| service.beta.kubernetes.io/aws-load-balancer-healthcheck-healthy-threshold | integer | 3 | | -| service.beta.kubernetes.io/aws-load-balancer-healthcheck-unhealthy-threshold | integer | 3 | | -| service.beta.kubernetes.io/aws-load-balancer-healthcheck-timeout | integer | 10 | | -| service.beta.kubernetes.io/aws-load-balancer-healthcheck-interval | integer | 10 | | -| service.beta.kubernetes.io/aws-load-balancer-healthcheck-protocol | string | TCP | | -| service.beta.kubernetes.io/aws-load-balancer-healthcheck-port | integer \| traffic-port | traffic-port | | -| service.beta.kubernetes.io/aws-load-balancer-healthcheck-path | string | "/" for HTTP(S) protocols | | -| service.beta.kubernetes.io/aws-load-balancer-eip-allocations | stringList | | | -| [service.beta.kubernetes.io/aws-load-balancer-target-group-attributes](#target-group-attributes) | stringMap | | | -| [service.beta.kubernetes.io/aws-load-balancer-subnets](#subnets) | stringList | | | -| [service.beta.kubernetes.io/aws-load-balancer-alpn-policy](#alpn-policy) | stringList | | | +| Name | Type | Default | Notes | +|--------------------------------------------------------------------------------------------------|-------------------------|---------------------------|--------------------------------------------------------| +| service.beta.kubernetes.io/aws-load-balancer-type | string | | | +| service.beta.kubernetes.io/aws-load-balancer-internal | boolean | false | | +| [service.beta.kubernetes.io/aws-load-balancer-proxy-protocol](#proxy-protocol-v2) | string | | Set to `"*"` to enable | +| service.beta.kubernetes.io/aws-load-balancer-ip-address-type | string | ipv4 | ipv4 \| dualstack | +| service.beta.kubernetes.io/aws-load-balancer-access-log-enabled | boolean | false | | +| service.beta.kubernetes.io/aws-load-balancer-access-log-s3-bucket-name | string | | | +| service.beta.kubernetes.io/aws-load-balancer-access-log-s3-bucket-prefix | string | | | +| service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled | boolean | false | | +| service.beta.kubernetes.io/aws-load-balancer-ssl-cert | stringList | | | +| service.beta.kubernetes.io/aws-load-balancer-ssl-ports | stringList | | | +| service.beta.kubernetes.io/aws-load-balancer-ssl-negotiation-policy | string | ELBSecurityPolicy-2016-08 | | +| service.beta.kubernetes.io/aws-load-balancer-backend-protocol | string | | | +| service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags | stringMap | | | +| service.beta.kubernetes.io/aws-load-balancer-healthcheck-healthy-threshold | integer | 3 | | +| service.beta.kubernetes.io/aws-load-balancer-healthcheck-unhealthy-threshold | integer | 3 | | +| service.beta.kubernetes.io/aws-load-balancer-healthcheck-timeout | integer | 10 | | +| service.beta.kubernetes.io/aws-load-balancer-healthcheck-interval | integer | 10 | | +| service.beta.kubernetes.io/aws-load-balancer-healthcheck-protocol | string | TCP | | +| service.beta.kubernetes.io/aws-load-balancer-healthcheck-port | integer \| traffic-port | traffic-port | | +| service.beta.kubernetes.io/aws-load-balancer-healthcheck-path | string | "/" for HTTP(S) protocols | | +| service.beta.kubernetes.io/aws-load-balancer-eip-allocations | stringList | | Public Facing lb only. Length/order must match subnets | +| service.beta.kubernetes.io/aws-load-balancer-private-ipv4-addresses | stringList | | Internal lb only. Length/order must match subnets | +| [service.beta.kubernetes.io/aws-load-balancer-target-group-attributes](#target-group-attributes) | stringMap | | | +| [service.beta.kubernetes.io/aws-load-balancer-subnets](#subnets) | stringList | | | +| [service.beta.kubernetes.io/aws-load-balancer-alpn-policy](#alpn-policy) | stringList | | | ## Traffic Routing diff --git a/pkg/annotations/constants.go b/pkg/annotations/constants.go index cbe7248aa..48896bbf4 100644 --- a/pkg/annotations/constants.go +++ b/pkg/annotations/constants.go @@ -66,6 +66,7 @@ const ( SvcLBSuffixHCPort = "aws-load-balancer-healthcheck-port" SvcLBSuffixHCPath = "aws-load-balancer-healthcheck-path" SvcLBSuffixEIPAllocations = "aws-load-balancer-eip-allocations" + SvcLBSuffixPrivateIpv4Addresses = "aws-load-balancer-private-ipv4-addresses" SvcLBSuffixTargetGroupAttributes = "aws-load-balancer-target-group-attributes" SvcLBSuffixSubnets = "aws-load-balancer-subnets" SvcLBSuffixALPNPolicy = "aws-load-balancer-alpn-policy" diff --git a/pkg/networking/subnet_resolver.go b/pkg/networking/subnet_resolver.go index 93952f2a2..0d64614f8 100644 --- a/pkg/networking/subnet_resolver.go +++ b/pkg/networking/subnet_resolver.go @@ -341,4 +341,4 @@ func (r *defaultSubnetsResolver) checkSubnetIsNotTaggedForOtherClusters(subnet * return false } return true -} \ No newline at end of file +} diff --git a/pkg/networking/subnet_resolver_test.go b/pkg/networking/subnet_resolver_test.go index c3725b8ae..380561649 100644 --- a/pkg/networking/subnet_resolver_test.go +++ b/pkg/networking/subnet_resolver_test.go @@ -393,7 +393,7 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { { name: "subnet with cluster tag gets precedence", fields: fields{ - vpcID: "vpc-1", + vpcID: "vpc-1", clusterName: "kube-cluster", describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { @@ -453,7 +453,7 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { { name: "subnets tagged for some other clusters get ignored", fields: fields{ - vpcID: "vpc-1", + vpcID: "vpc-1", clusterName: "kube-cluster", describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { @@ -551,7 +551,7 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { { name: "subnets with multiple cluster tags", fields: fields{ - vpcID: "vpc-1", + vpcID: "vpc-1", clusterName: "kube-cluster", describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { @@ -610,7 +610,6 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, }, }, - }, } diff --git a/pkg/service/model_build_load_balancer.go b/pkg/service/model_build_load_balancer.go index b373ce506..cf1577568 100644 --- a/pkg/service/model_build_load_balancer.go +++ b/pkg/service/model_build_load_balancer.go @@ -5,14 +5,16 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "net" + "regexp" + "strconv" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/pkg/errors" - "regexp" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" "sigs.k8s.io/aws-load-balancer-controller/pkg/networking" - "strconv" ) const ( @@ -46,7 +48,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerSpec(ctx context.Context, schem if err != nil { return elbv2model.LoadBalancerSpec{}, err } - subnetMappings, err := t.buildLoadBalancerSubnetMappings(ctx, t.ec2Subnets) + subnetMappings, err := t.buildLoadBalancerSubnetMappings(ctx, scheme, t.ec2Subnets) if err != nil { return elbv2model.LoadBalancerSpec{}, err } @@ -109,25 +111,70 @@ func (t *defaultModelBuildTask) buildLoadBalancerTags(ctx context.Context) (map[ return t.buildAdditionalResourceTags(ctx) } -func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(_ context.Context, ec2Subnets []*ec2.Subnet) ([]elbv2model.SubnetMapping, error) { +func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Context, scheme elbv2model.LoadBalancerScheme, ec2Subnets []*ec2.Subnet) ([]elbv2model.SubnetMapping, error) { var eipAllocation []string eipConfigured := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixEIPAllocations, &eipAllocation, t.service.Annotations) - if eipConfigured && len(eipAllocation) != len(ec2Subnets) { - return []elbv2model.SubnetMapping{}, errors.Errorf("number of EIP allocations (%d) and subnets (%d) must match", len(eipAllocation), len(ec2Subnets)) + var privateIpv4Addresses []string + ipv4Configured := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixPrivateIpv4Addresses, &privateIpv4Addresses, t.service.Annotations) + + // Validation + if eipConfigured && ipv4Configured { + return []elbv2model.SubnetMapping{}, errors.Errorf("only one of EIP allocations or PrivateIpv4Addresses can be set") + } + if eipConfigured { + if scheme == elbv2model.LoadBalancerSchemeInternal { + return []elbv2model.SubnetMapping{}, errors.Errorf("EIP allocations can only be set for internet facing load balancers") + } else if len(eipAllocation) != len(ec2Subnets) { + return []elbv2model.SubnetMapping{}, errors.Errorf("number of EIP allocations (%d) and subnets (%d) must match", len(eipAllocation), len(ec2Subnets)) + } + } + if ipv4Configured { + if scheme == elbv2model.LoadBalancerSchemeInternetFacing { + return []elbv2model.SubnetMapping{}, errors.Errorf("PrivateIpv4Addresses can only be set for internal balancers") + } else if len(privateIpv4Addresses) != len(ec2Subnets) { + return []elbv2model.SubnetMapping{}, errors.Errorf("number of PrivateIpv4Addresses (%d) and subnets (%d) must match", len(privateIpv4Addresses), len(ec2Subnets)) + } } + subnetMappings := make([]elbv2model.SubnetMapping, 0, len(ec2Subnets)) for idx, subnet := range ec2Subnets { mapping := elbv2model.SubnetMapping{ SubnetID: aws.StringValue(subnet.SubnetId), } - if idx < len(eipAllocation) { + if eipConfigured { mapping.AllocationID = aws.String(eipAllocation[idx]) } + if ipv4Configured { + ip, err := t.getMatchingIPforSubnet(ctx, subnet, privateIpv4Addresses) + if err != nil { + return []elbv2model.SubnetMapping{}, err + } + mapping.PrivateIPv4Address = aws.String(ip) + } subnetMappings = append(subnetMappings, mapping) } return subnetMappings, nil } +// Return the ip address which is in the subnet. Error if not match +// Can be extended for ipv6 if required +func (t *defaultModelBuildTask) getMatchingIPforSubnet(_ context.Context, subnet *ec2.Subnet, privateIpv4Addresses []string) (string, error) { + _, ipv4Net, err := net.ParseCIDR(*subnet.CidrBlock) + if err != nil { + return "", errors.Wrap(err, "subnet CIDR block could not be parsed") + } + for _, ipString := range privateIpv4Addresses { + ip := net.ParseIP(ipString) + if ip == nil { + return "", errors.Errorf("cannot parse ip %s", ipString) + } + if ipv4Net.Contains(ip) { + return ipString, nil + } + } + return "", errors.Errorf("no matching ip for subnet %s", *subnet.SubnetId) +} + func (t *defaultModelBuildTask) resolveLoadBalancerSubnets(ctx context.Context, scheme elbv2model.LoadBalancerScheme) ([]*ec2.Subnet, error) { var rawSubnetNameOrIDs []string if exists := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSubnets, &rawSubnetNameOrIDs, t.service.Annotations); exists { diff --git a/pkg/service/model_build_load_balancer_test.go b/pkg/service/model_build_load_balancer_test.go index ce46b905c..3b567a1ca 100644 --- a/pkg/service/model_build_load_balancer_test.go +++ b/pkg/service/model_build_load_balancer_test.go @@ -3,6 +3,8 @@ package service import ( "context" "errors" + "testing" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/golang/mock/gomock" @@ -12,7 +14,6 @@ import ( mock_networking "sigs.k8s.io/aws-load-balancer-controller/mocks/networking" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" - "testing" ) func Test_defaultModelBuilderTask_buildLBAttributes(t *testing.T) { @@ -131,13 +132,15 @@ func Test_defaultModelBuilderTask_buildLBAttributes(t *testing.T) { func Test_defaultModelBuilderTask_buildSubnetMappings(t *testing.T) { tests := []struct { name string + scheme elbv2.LoadBalancerScheme subnets []*ec2.Subnet want []elbv2.SubnetMapping svc *corev1.Service wantErr error }{ { - name: "Multiple subnets", + name: "Multiple subnets", + scheme: elbv2.LoadBalancerSchemeInternetFacing, subnets: []*ec2.Subnet{ { SubnetId: aws.String("subnet-1"), @@ -161,7 +164,8 @@ func Test_defaultModelBuilderTask_buildSubnetMappings(t *testing.T) { }, }, { - name: "When EIP allocation is configured", + name: "When EIP allocation is configured", + scheme: elbv2.LoadBalancerSchemeInternetFacing, subnets: []*ec2.Subnet{ { SubnetId: aws.String("subnet-1"), @@ -193,7 +197,8 @@ func Test_defaultModelBuilderTask_buildSubnetMappings(t *testing.T) { }, }, { - name: "When EIP allocation and subnet mismatch", + name: "When EIP allocation and subnet mismatch", + scheme: elbv2.LoadBalancerSchemeInternetFacing, subnets: []*ec2.Subnet{ { SubnetId: aws.String("subnet-1"), @@ -215,6 +220,164 @@ func Test_defaultModelBuilderTask_buildSubnetMappings(t *testing.T) { }, wantErr: errors.New("number of EIP allocations (1) and subnets (2) must match"), }, + { + name: "When PrivateIpv4Addresses is configured", + scheme: elbv2.LoadBalancerSchemeInternal, + subnets: []*ec2.Subnet{ + { + SubnetId: aws.String("subnet-1"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + CidrBlock: aws.String("172.17.0.0/16"), + }, + { + SubnetId: aws.String("subnet-2"), + AvailabilityZone: aws.String("us-west-2b"), + VpcId: aws.String("vpc-1"), + CidrBlock: aws.String("172.16.0.0/16"), // not in the same order as annoation + }, + }, + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-private-ipv4-addresses": "172.16.1.1, 172.17.1.1", + }, + }, + }, + want: []elbv2.SubnetMapping{ + { + SubnetID: "subnet-1", + PrivateIPv4Address: aws.String("172.17.1.1"), + }, + { + SubnetID: "subnet-2", + PrivateIPv4Address: aws.String("172.16.1.1"), + }, + }, + }, + { + name: "When PrivateIPv4Address outside of CIDR", + scheme: elbv2.LoadBalancerSchemeInternal, + subnets: []*ec2.Subnet{ + { + SubnetId: aws.String("subnet-1"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + CidrBlock: aws.String("172.17.0.0/16"), + }, + { + SubnetId: aws.String("subnet-2"), + AvailabilityZone: aws.String("us-west-2b"), + VpcId: aws.String("vpc-1"), + CidrBlock: aws.String("172.16.0.0/16"), // not in the same order as annoation + }, + }, + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-private-ipv4-addresses": "172.100.1.1, 172.200.1.1", + }, + }, + }, + wantErr: errors.New("no matching ip for subnet subnet-1"), + }, + { + name: "When PrivateIpv4Addresses and subnet mismatch", + scheme: elbv2.LoadBalancerSchemeInternal, + subnets: []*ec2.Subnet{ + { + SubnetId: aws.String("subnet-1"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + }, + { + SubnetId: aws.String("subnet-2"), + AvailabilityZone: aws.String("us-west-2b"), + VpcId: aws.String("vpc-1"), + }, + }, + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-private-ipv4-addresses": "172.16.1.1", + }, + }, + }, + wantErr: errors.New("number of PrivateIpv4Addresses (1) and subnets (2) must match"), + }, + { + name: "When both EIP allocation and PrivateIpv4Addresses set", + scheme: elbv2.LoadBalancerSchemeInternal, + subnets: []*ec2.Subnet{ + { + SubnetId: aws.String("subnet-1"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + }, + { + SubnetId: aws.String("subnet-2"), + AvailabilityZone: aws.String("us-west-2b"), + VpcId: aws.String("vpc-1"), + }, + }, + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-private-ipv4-addresses": "172.16.1.1, 172.17.1.1", + "service.beta.kubernetes.io/aws-load-balancer-eip-allocations": "eip1, eip2", + }, + }, + }, + wantErr: errors.New("only one of EIP allocations or PrivateIpv4Addresses can be set"), + }, + { + name: "When EIP allocation and LoadBalancerSchemeInternal set", + scheme: elbv2.LoadBalancerSchemeInternal, + subnets: []*ec2.Subnet{ + { + SubnetId: aws.String("subnet-1"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + }, + { + SubnetId: aws.String("subnet-2"), + AvailabilityZone: aws.String("us-west-2b"), + VpcId: aws.String("vpc-1"), + }, + }, + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-eip-allocations": "eip1, eip2", + }, + }, + }, + wantErr: errors.New("EIP allocations can only be set for internet facing load balancers"), + }, + { + name: "When PrivateIpv4Addresses and LoadBalancerSchemeInternetFacing set", + scheme: elbv2.LoadBalancerSchemeInternetFacing, + subnets: []*ec2.Subnet{ + { + SubnetId: aws.String("subnet-1"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + }, + { + SubnetId: aws.String("subnet-2"), + AvailabilityZone: aws.String("us-west-2b"), + VpcId: aws.String("vpc-1"), + }, + }, + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-private-ipv4-addresses": "172.16.1.1, 172.17.1.1", + }, + }, + }, + wantErr: errors.New("PrivateIpv4Addresses can only be set for internal balancers"), + }, } for _, tt := range tests { @@ -224,7 +387,78 @@ func Test_defaultModelBuilderTask_buildSubnetMappings(t *testing.T) { annotationParser := annotations.NewSuffixAnnotationParser("service.beta.kubernetes.io") builder := &defaultModelBuildTask{service: tt.svc, annotationParser: annotationParser} - got, err := builder.buildLoadBalancerSubnetMappings(context.Background(), tt.subnets) + got, err := builder.buildLoadBalancerSubnetMappings(context.Background(), tt.scheme, tt.subnets) + if tt.wantErr != nil { + assert.EqualError(t, err, tt.wantErr.Error()) + } else { + assert.Equal(t, tt.want, got) + } + }) + } +} + +func Test_defaultModelBuilderTask_getMatchingIPforSubnet(t *testing.T) { + tests := []struct { + name string + subnet *ec2.Subnet + privateIpv4Addresses []string + want string + wantErr error + }{ + { + name: "When ip is found for subnet", + subnet: &ec2.Subnet{ + SubnetId: aws.String("subnet-1"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + CidrBlock: aws.String("172.16.0.0/16"), + }, + privateIpv4Addresses: []string{"172.17.1.1", "172.16.1.1"}, + want: "172.16.1.1", + }, + { + name: "When CIDR cannot be parsed", + subnet: &ec2.Subnet{ + SubnetId: aws.String("subnet-1"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + CidrBlock: aws.String("172.16.0.0.0/16"), + }, + privateIpv4Addresses: []string{"172.17.1.1", "172.16.1.1"}, + wantErr: errors.New("subnet CIDR block could not be parsed: invalid CIDR address: 172.16.0.0.0/16"), + }, + { + name: "When IP cannot be parsed", + subnet: &ec2.Subnet{ + SubnetId: aws.String("subnet-1"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + CidrBlock: aws.String("172.16.0.0/16"), + }, + privateIpv4Addresses: []string{"172.17.1.1.1", "172.16.1.1"}, + wantErr: errors.New("cannot parse ip 172.17.1.1.1"), + }, + { + name: "When no valid ip in cidr range", + subnet: &ec2.Subnet{ + SubnetId: aws.String("subnet-1"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + CidrBlock: aws.String("172.16.0.0/16"), + }, + privateIpv4Addresses: []string{"172.100.1.1", "172.200.1.1"}, + wantErr: errors.New("no matching ip for subnet subnet-1"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + annotationParser := annotations.NewSuffixAnnotationParser("service.beta.kubernetes.io") + builder := &defaultModelBuildTask{service: nil, annotationParser: annotationParser} + got, err := builder.getMatchingIPforSubnet(context.Background(), tt.subnet, tt.privateIpv4Addresses) if tt.wantErr != nil { assert.EqualError(t, err, tt.wantErr.Error()) } else {