From 2f6ce5cfc51a28257cf2710873c437257d1ef605 Mon Sep 17 00:00:00 2001 From: Yang Yang Date: Tue, 25 Mar 2025 17:26:33 -0700 Subject: [PATCH 1/2] refactor subnet resolver and add discovery by subnet's reachability(public/private) --- docs/deploy/subnet_discovery.md | 91 +- docs/install/iam_policy.json | 1 + docs/install/iam_policy_cn.json | 1 + docs/install/iam_policy_iso.json | 1 + docs/install/iam_policy_isob.json | 1 + docs/install/iam_policy_isoe.json | 1 + docs/install/iam_policy_isof.json | 1 + docs/install/iam_policy_us-gov.json | 1 + main.go | 9 +- pkg/aws/services/ec2.go | 21 + pkg/aws/services/ec2_mocks.go | 15 + pkg/config/feature_gates.go | 50 +- pkg/ingress/model_build_load_balancer.go | 18 +- pkg/ingress/model_build_load_balancer_test.go | 87 +- pkg/networking/subnet_resolver.go | 584 ++-- pkg/networking/subnet_resolver_mocks.go | 2 +- pkg/networking/subnet_resolver_test.go | 2414 +++++++++++------ pkg/service/model_build_load_balancer.go | 4 - 18 files changed, 2136 insertions(+), 1166 deletions(-) diff --git a/docs/deploy/subnet_discovery.md b/docs/deploy/subnet_discovery.md index fc4c318848..8fe4f7470a 100644 --- a/docs/deploy/subnet_discovery.md +++ b/docs/deploy/subnet_discovery.md @@ -1,38 +1,71 @@ -# Subnet auto-discovery -By default, the AWS Load Balancer Controller (LBC) auto-discovers network subnets that it can create AWS Network Load Balancers (NLB) and AWS Application Load Balancers (ALB) in. ALBs require at least two subnets across Availability Zones by default, -set [Feature Gate ALBSingleSubnet](https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/deploy/configurations/#feature-gates) to "true" allows using only 1 subnet for provisioning ALB. NLBs require one subnet. -The subnets must be tagged appropriately for auto-discovery to work. The controller chooses one subnet from each Availability Zone. During auto-discovery, the controller -considers subnets with at least eight available IP addresses. In the case of multiple qualified tagged subnets in an Availability Zone, the controller chooses the first one in lexicographical -order by the subnet IDs. -For more information about the subnets for the LBC, see [Application Load Balancers](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/application-load-balancers.html) -and [Network Load Balancers](https://docs.aws.amazon.com/elasticloadbalancing/latest/network/network-load-balancers.html). -If you used `eksctl` or an Amazon EKS AWS CloudFormation template to create your VPC after March 26, 2020, then the subnets are tagged appropriately when they're created. For -more information about the Amazon EKS AWS CloudFormation VPC templates, see [Creating a VPC for your Amazon EKS cluster](https://docs.aws.amazon.com/eks/latest/userguide/create-public-private-vpc.html). +# Subnet Auto-Discovery +The AWS Load Balancer Controller (LBC) automatically discovers subnets for creating AWS Network Load Balancers (NLB) and AWS Application Load Balancers (ALB). This auto-discovery process follows three main steps: -## Public subnets -Public subnets are used for internet-facing load balancers. These subnets must have the following tags: +1. **Candidate Subnet Determination**: The controller identifies potential candidate subnets +2. **Subnet Filtering**: The controller filters these candidates based on eligibility criteria +3. **Final Selection**: The controller selects one subnet per availability zone -| Key | Value | -| --------------------------------------- | --------------------- | -| `kubernetes.io/role/elb` | `1` or `` | +## Candidate Subnet Determination +The controller determines candidate subnets using the following process: -## Private subnets -Private subnets are used for internal load balancers. These subnets must have the following tags: +1. **If tag filters are specified**: Only subnets matching these filters become candidates + + !!!tip + You can only specify subnet tag filters for Ingress via IngressClassParams -| Key | Value | -| --------------------------------------- | --------------------- | -| `kubernetes.io/role/internal-elb` | `1` or `` | +2. **If no tag filters are specified**: + * If subnets with matching [role tag](#subnet-role-tag) exists: Only these become candidates + * [**For LBC version >= 2.12.1**] If no subnets have role tags: Candidates are subnets whose [reachability](#subnet-reachability) (public/private) matches the LoadBalancer's schema -## Common tag -In version v2.1.1 and older of the LBC, both the public and private subnets must be tagged with the cluster name as follows: +### Subnet Role Tag +Subnets can be tagged appropriately for auto-discovery selection: -| Key | Value | -| --------------------------------------- | --------------------- | -| `kubernetes.io/cluster/${cluster-name}` | `owned` or `shared` | +* **For internet-facing load balancers**, the controller looks for public subnets with following tags: - `${cluster-name}` is the name of the Kubernetes cluster. - -The cluster tag is not required in versions v2.1.2 to v2.4.1, unless a cluster tag for another cluster is present. + | Key | Value | + | --------------------------------------- | --------------------- | + | `kubernetes.io/role/elb` | `1` or `` | -With versions v2.4.2 and later, you can disable the cluster tag check completely by specifying the feature gate `SubnetsClusterTagCheck=false` +* **For internal load balancers**, the controller looks for private subnets with following tags: + + | Key | Value | + | --------------------------------------- | --------------------- | + | `kubernetes.io/role/internal-elb` | `1` or `` | + +### Subnet reachability +The controller automatically discovers all subnets in your VPC and determines whether each is a public or private subnet based on its associated route table configuration. +A subnet is classified as public if its route table contains a route to an Internet Gateway. + +!!!tip + You can disable this behavior via SubnetDiscoveryByReachability feature flag. + +## Subnet Filtering + +1. **Cluster Tag Check**: The controller checks for cluster tags on subnets. Subnets with ineligible cluster tags will be filtered out. + + | Key | Value | + | --------------------------------------- | --------------------- | + | `kubernetes.io/cluster/${cluster-name}` | `owned` or `shared` | + + * If such cluster tag exists but no `` matches the current cluster, those subnets will be filtered out. + * [**For LBC version < 2.1.1**] subnets without a cluster tag matching cluster name will be filtered out. + + !!! tip + You can disable this behavior via the `SubnetsClusterTagCheck` feature flag. When disabled, no cluster tag check will be performed against subnets. + +2. **IP Address Availability**: Subnets with insufficient available IP addresses(**<8**) are filtered out. + +## Final Selection + +The controller selects one subnet per availability zone. When multiple subnets exist per Availability Zone, the following priority order applies: + +1. Subnets with cluster tag for the current cluster (`kubernetes.io/cluster/`) are prioritized +2. Subnets with lower lexicographical order of subnet ID are prioritized + +## Minimum Subnet Requirements + +* **ALBs**: Require at least two subnets across different Availability Zones by default + + !!! tip + For customers whitelished by the AWS Elastic Load Balancing team, you can enable the [ALBSingleSubnet feature gate](https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/deploy/configurations/#feature-gates). This allows provisioning an ALB with just one subnet instead of the standard requirement of two subnets. \ No newline at end of file diff --git a/docs/install/iam_policy.json b/docs/install/iam_policy.json index fe19761702..761d0e733c 100644 --- a/docs/install/iam_policy.json +++ b/docs/install/iam_policy.json @@ -31,6 +31,7 @@ "ec2:DescribeCoipPools", "ec2:GetSecurityGroupsForVpc", "ec2:DescribeIpamPools", + "ec2:DescribeRouteTables", "elasticloadbalancing:DescribeLoadBalancers", "elasticloadbalancing:DescribeLoadBalancerAttributes", "elasticloadbalancing:DescribeListeners", diff --git a/docs/install/iam_policy_cn.json b/docs/install/iam_policy_cn.json index bd2b8b8c98..f073f07e5d 100644 --- a/docs/install/iam_policy_cn.json +++ b/docs/install/iam_policy_cn.json @@ -31,6 +31,7 @@ "ec2:DescribeCoipPools", "ec2:GetSecurityGroupsForVpc", "ec2:DescribeIpamPools", + "ec2:DescribeRouteTables", "elasticloadbalancing:DescribeLoadBalancers", "elasticloadbalancing:DescribeLoadBalancerAttributes", "elasticloadbalancing:DescribeListeners", diff --git a/docs/install/iam_policy_iso.json b/docs/install/iam_policy_iso.json index 9d032e395f..64d2a22f1f 100644 --- a/docs/install/iam_policy_iso.json +++ b/docs/install/iam_policy_iso.json @@ -31,6 +31,7 @@ "ec2:DescribeCoipPools", "ec2:GetSecurityGroupsForVpc", "ec2:DescribeIpamPools", + "ec2:DescribeRouteTables", "elasticloadbalancing:DescribeLoadBalancers", "elasticloadbalancing:DescribeLoadBalancerAttributes", "elasticloadbalancing:DescribeListeners", diff --git a/docs/install/iam_policy_isob.json b/docs/install/iam_policy_isob.json index 6288c9c62c..146dff5077 100644 --- a/docs/install/iam_policy_isob.json +++ b/docs/install/iam_policy_isob.json @@ -31,6 +31,7 @@ "ec2:DescribeCoipPools", "ec2:GetSecurityGroupsForVpc", "ec2:DescribeIpamPools", + "ec2:DescribeRouteTables", "elasticloadbalancing:DescribeLoadBalancers", "elasticloadbalancing:DescribeLoadBalancerAttributes", "elasticloadbalancing:DescribeListeners", diff --git a/docs/install/iam_policy_isoe.json b/docs/install/iam_policy_isoe.json index 499afd947b..efce77b1b3 100644 --- a/docs/install/iam_policy_isoe.json +++ b/docs/install/iam_policy_isoe.json @@ -31,6 +31,7 @@ "ec2:DescribeCoipPools", "ec2:GetSecurityGroupsForVpc", "ec2:DescribeIpamPools", + "ec2:DescribeRouteTables", "elasticloadbalancing:DescribeLoadBalancers", "elasticloadbalancing:DescribeLoadBalancerAttributes", "elasticloadbalancing:DescribeListeners", diff --git a/docs/install/iam_policy_isof.json b/docs/install/iam_policy_isof.json index 08c1d1390a..4406e21c5f 100644 --- a/docs/install/iam_policy_isof.json +++ b/docs/install/iam_policy_isof.json @@ -31,6 +31,7 @@ "ec2:DescribeCoipPools", "ec2:GetSecurityGroupsForVpc", "ec2:DescribeIpamPools", + "ec2:DescribeRouteTables", "elasticloadbalancing:DescribeLoadBalancers", "elasticloadbalancing:DescribeLoadBalancerAttributes", "elasticloadbalancing:DescribeListeners", diff --git a/docs/install/iam_policy_us-gov.json b/docs/install/iam_policy_us-gov.json index e08541c3ad..9557eac0d1 100644 --- a/docs/install/iam_policy_us-gov.json +++ b/docs/install/iam_policy_us-gov.json @@ -31,6 +31,7 @@ "ec2:DescribeCoipPools", "ec2:GetSecurityGroupsForVpc", "ec2:DescribeIpamPools", + "ec2:DescribeRouteTables", "elasticloadbalancing:DescribeLoadBalancers", "elasticloadbalancing:DescribeLoadBalancerAttributes", "elasticloadbalancing:DescribeListeners", diff --git a/main.go b/main.go index c4641fb109..ca07522fc8 100644 --- a/main.go +++ b/main.go @@ -17,9 +17,10 @@ limitations under the License. package main import ( - "k8s.io/client-go/util/workqueue" "os" + "k8s.io/client-go/util/workqueue" + elbv2deploy "sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/elbv2" "github.com/go-logr/logr" @@ -119,7 +120,11 @@ func main() { sgReconciler := networking.NewDefaultSecurityGroupReconciler(sgManager, ctrl.Log) azInfoProvider := networking.NewDefaultAZInfoProvider(cloud.EC2(), ctrl.Log.WithName("az-info-provider")) vpcInfoProvider := networking.NewDefaultVPCInfoProvider(cloud.EC2(), ctrl.Log.WithName("vpc-info-provider")) - subnetResolver := networking.NewDefaultSubnetsResolver(azInfoProvider, cloud.EC2(), cloud.VpcID(), controllerCFG.ClusterName, ctrl.Log.WithName("subnets-resolver")) + subnetResolver := networking.NewDefaultSubnetsResolver(azInfoProvider, cloud.EC2(), cloud.VpcID(), controllerCFG.ClusterName, + controllerCFG.FeatureGates.Enabled(config.SubnetsClusterTagCheck), + controllerCFG.FeatureGates.Enabled(config.ALBSingleSubnet), + controllerCFG.FeatureGates.Enabled(config.SubnetDiscoveryByReachability), + ctrl.Log.WithName("subnets-resolver")) multiClusterManager := targetgroupbinding.NewMultiClusterManager(mgr.GetClient(), mgr.GetAPIReader(), ctrl.Log) tgbResManager := targetgroupbinding.NewDefaultResourceManager(mgr.GetClient(), cloud.ELBV2(), cloud.EC2(), podInfoRepo, sgManager, sgReconciler, vpcInfoProvider, multiClusterManager, lbcMetricsCollector, diff --git a/pkg/aws/services/ec2.go b/pkg/aws/services/ec2.go index 3acd3564da..7084bc1459 100644 --- a/pkg/aws/services/ec2.go +++ b/pkg/aws/services/ec2.go @@ -2,6 +2,7 @@ package services import ( "context" + "github.com/aws/aws-sdk-go-v2/service/ec2" "github.com/aws/aws-sdk-go-v2/service/ec2/types" "sigs.k8s.io/aws-load-balancer-controller/pkg/aws/provider" @@ -24,6 +25,9 @@ type EC2 interface { // DescribeVPCsAsList wraps the DescribeVpcsPagesWithContext API, which aggregates paged results into list. DescribeVPCsAsList(ctx context.Context, input *ec2.DescribeVpcsInput) ([]types.Vpc, error) + // DescribeRouteTablesAsList wraps the DescribeRouteTablesWithContext API, which aggregates paged results into list. + DescribeRouteTablesAsList(ctx context.Context, input *ec2.DescribeRouteTablesInput) ([]types.RouteTable, error) + CreateTagsWithContext(ctx context.Context, input *ec2.CreateTagsInput) (*ec2.CreateTagsOutput, error) DeleteTagsWithContext(ctx context.Context, input *ec2.DeleteTagsInput) (*ec2.DeleteTagsOutput, error) CreateSecurityGroupWithContext(ctx context.Context, input *ec2.CreateSecurityGroupInput) (*ec2.CreateSecurityGroupOutput, error) @@ -141,6 +145,23 @@ func (c *ec2Client) DescribeVPCsAsList(ctx context.Context, input *ec2.DescribeV return result, nil } +func (c *ec2Client) DescribeRouteTablesAsList(ctx context.Context, input *ec2.DescribeRouteTablesInput) ([]types.RouteTable, error) { + var result []types.RouteTable + client, err := c.awsClientsProvider.GetEC2Client(ctx, "DescribeRouteTables") + if err != nil { + return nil, err + } + paginator := ec2.NewDescribeRouteTablesPaginator(client, input) + for paginator.HasMorePages() { + output, err := paginator.NextPage(ctx) + if err != nil { + return nil, err + } + result = append(result, output.RouteTables...) + } + return result, nil +} + func (c *ec2Client) CreateTagsWithContext(ctx context.Context, input *ec2.CreateTagsInput) (*ec2.CreateTagsOutput, error) { client, err := c.awsClientsProvider.GetEC2Client(ctx, "CreateTags") if err != nil { diff --git a/pkg/aws/services/ec2_mocks.go b/pkg/aws/services/ec2_mocks.go index 00a6dc886d..7de2d04272 100644 --- a/pkg/aws/services/ec2_mocks.go +++ b/pkg/aws/services/ec2_mocks.go @@ -171,6 +171,21 @@ func (mr *MockEC2MockRecorder) DescribeNetworkInterfacesAsList(arg0, arg1 interf return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeNetworkInterfacesAsList", reflect.TypeOf((*MockEC2)(nil).DescribeNetworkInterfacesAsList), arg0, arg1) } +// DescribeRouteTablesAsList mocks base method. +func (m *MockEC2) DescribeRouteTablesAsList(arg0 context.Context, arg1 *ec2.DescribeRouteTablesInput) ([]types.RouteTable, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DescribeRouteTablesAsList", arg0, arg1) + ret0, _ := ret[0].([]types.RouteTable) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DescribeRouteTablesAsList indicates an expected call of DescribeRouteTablesAsList. +func (mr *MockEC2MockRecorder) DescribeRouteTablesAsList(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeRouteTablesAsList", reflect.TypeOf((*MockEC2)(nil).DescribeRouteTablesAsList), arg0, arg1) +} + // DescribeSecurityGroupsAsList mocks base method. func (m *MockEC2) DescribeSecurityGroupsAsList(arg0 context.Context, arg1 *ec2.DescribeSecurityGroupsInput) ([]types.SecurityGroup, error) { m.ctrl.T.Helper() diff --git a/pkg/config/feature_gates.go b/pkg/config/feature_gates.go index e220faff7a..09576ae418 100644 --- a/pkg/config/feature_gates.go +++ b/pkg/config/feature_gates.go @@ -11,18 +11,19 @@ import ( type Feature string const ( - ListenerRulesTagging Feature = "ListenerRulesTagging" - WeightedTargetGroups Feature = "WeightedTargetGroups" - ServiceTypeLoadBalancerOnly Feature = "ServiceTypeLoadBalancerOnly" - EndpointsFailOpen Feature = "EndpointsFailOpen" - EnableServiceController Feature = "EnableServiceController" - EnableIPTargetType Feature = "EnableIPTargetType" - EnableRGTAPI Feature = "EnableRGTAPI" - SubnetsClusterTagCheck Feature = "SubnetsClusterTagCheck" - NLBHealthCheckAdvancedConfig Feature = "NLBHealthCheckAdvancedConfig" - NLBSecurityGroup Feature = "NLBSecurityGroup" - ALBSingleSubnet Feature = "ALBSingleSubnet" - LBCapacityReservation Feature = "LBCapacityReservation" + ListenerRulesTagging Feature = "ListenerRulesTagging" + WeightedTargetGroups Feature = "WeightedTargetGroups" + ServiceTypeLoadBalancerOnly Feature = "ServiceTypeLoadBalancerOnly" + EndpointsFailOpen Feature = "EndpointsFailOpen" + EnableServiceController Feature = "EnableServiceController" + EnableIPTargetType Feature = "EnableIPTargetType" + EnableRGTAPI Feature = "EnableRGTAPI" + SubnetsClusterTagCheck Feature = "SubnetsClusterTagCheck" + NLBHealthCheckAdvancedConfig Feature = "NLBHealthCheckAdvancedConfig" + NLBSecurityGroup Feature = "NLBSecurityGroup" + ALBSingleSubnet Feature = "ALBSingleSubnet" + SubnetDiscoveryByReachability Feature = "SubnetDiscoveryByReachability" + LBCapacityReservation Feature = "LBCapacityReservation" ) type FeatureGates interface { @@ -50,18 +51,19 @@ type defaultFeatureGates struct { func NewFeatureGates() FeatureGates { return &defaultFeatureGates{ featureState: map[Feature]bool{ - ListenerRulesTagging: true, - WeightedTargetGroups: true, - ServiceTypeLoadBalancerOnly: false, - EndpointsFailOpen: true, - EnableServiceController: true, - EnableIPTargetType: true, - EnableRGTAPI: false, - SubnetsClusterTagCheck: true, - NLBHealthCheckAdvancedConfig: true, - NLBSecurityGroup: true, - ALBSingleSubnet: false, - LBCapacityReservation: true, + ListenerRulesTagging: true, + WeightedTargetGroups: true, + ServiceTypeLoadBalancerOnly: false, + EndpointsFailOpen: true, + EnableServiceController: true, + EnableIPTargetType: true, + EnableRGTAPI: false, + SubnetsClusterTagCheck: true, + NLBHealthCheckAdvancedConfig: true, + NLBSecurityGroup: true, + ALBSingleSubnet: false, + SubnetDiscoveryByReachability: true, + LBCapacityReservation: true, }, } } diff --git a/pkg/ingress/model_build_load_balancer.go b/pkg/ingress/model_build_load_balancer.go index 1db3b8a2fb..e83419fa8f 100644 --- a/pkg/ingress/model_build_load_balancer.go +++ b/pkg/ingress/model_build_load_balancer.go @@ -5,16 +5,16 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "regexp" + awssdk "github.com/aws/aws-sdk-go-v2/aws" ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/sets" - "regexp" "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1" "sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" - "sigs.k8s.io/aws-load-balancer-controller/pkg/config" "sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/tracking" "sigs.k8s.io/aws-load-balancer-controller/pkg/equality" "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" @@ -24,8 +24,7 @@ import ( ) const ( - resourceIDLoadBalancer = "LoadBalancer" - minimalAvailableIPAddressCount = int32(8) + resourceIDLoadBalancer = "LoadBalancer" ) func (t *defaultModelBuildTask) buildLoadBalancer(ctx context.Context, listenPortConfigByPort map[int32]listenPortConfig) (*elbv2model.LoadBalancer, error) { @@ -199,11 +198,11 @@ func (t *defaultModelBuildTask) buildLoadBalancerIPAddressType(_ context.Context } func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Context, scheme elbv2model.LoadBalancerScheme) ([]elbv2model.SubnetMapping, error) { - var explicitSubnetSelectorList []*v1beta1.SubnetSelector + var explicitSubnetSelectorList []v1beta1.SubnetSelector var explicitSubnetNameOrIDsList [][]string for _, member := range t.ingGroup.Members { if member.IngClassConfig.IngClassParams != nil && member.IngClassConfig.IngClassParams.Spec.Subnets != nil { - explicitSubnetSelectorList = append(explicitSubnetSelectorList, member.IngClassConfig.IngClassParams.Spec.Subnets) + explicitSubnetSelectorList = append(explicitSubnetSelectorList, *member.IngClassConfig.IngClassParams.Spec.Subnets) continue } var rawSubnetNameOrIDs []string @@ -219,15 +218,13 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Cont } chosenSubnetSelector := explicitSubnetSelectorList[0] for _, subnetSelector := range explicitSubnetSelectorList[1:] { - if !cmp.Equal(*chosenSubnetSelector, *subnetSelector) { + if !cmp.Equal(chosenSubnetSelector, subnetSelector) { return nil, errors.Errorf("conflicting IngressClassParams subnet specifications") } } chosenSubnets, err := t.subnetsResolver.ResolveViaSelector(ctx, chosenSubnetSelector, networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), networking.WithSubnetsResolveLBScheme(scheme), - networking.WithSubnetsClusterTagCheck(t.featureGates.Enabled(config.SubnetsClusterTagCheck)), - networking.WithALBSingleSubnet(t.featureGates.Enabled(config.ALBSingleSubnet)), ) if err != nil { return nil, err @@ -246,7 +243,6 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Cont chosenSubnets, err := t.subnetsResolver.ResolveViaNameOrIDSlice(ctx, chosenSubnetNameOrIDs, networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), networking.WithSubnetsResolveLBScheme(scheme), - networking.WithALBSingleSubnet(t.featureGates.Enabled(config.ALBSingleSubnet)), ) if err != nil { return nil, err @@ -264,8 +260,6 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Cont chosenSubnets, err := t.subnetsResolver.ResolveViaDiscovery(ctx, networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), networking.WithSubnetsResolveLBScheme(scheme), - networking.WithSubnetsResolveAvailableIPAddressCount(minimalAvailableIPAddressCount), - networking.WithSubnetsClusterTagCheck(t.featureGates.Enabled(config.SubnetsClusterTagCheck)), ) if err != nil { return nil, errors.Wrap(err, "couldn't auto-discover subnets") diff --git a/pkg/ingress/model_build_load_balancer_test.go b/pkg/ingress/model_build_load_balancer_test.go index 8b1c43a2dc..9ebecafdb5 100644 --- a/pkg/ingress/model_build_load_balancer_test.go +++ b/pkg/ingress/model_build_load_balancer_test.go @@ -4,10 +4,11 @@ import ( "context" "errors" "fmt" - ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" "strings" "testing" + ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" + awssdk "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/ec2" "github.com/go-logr/logr" @@ -685,13 +686,15 @@ func Test_defaultModelBuildTask_buildLoadBalancerName(t *testing.T) { var ( subnet1 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-1"), } subnet2 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("az2"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("az2"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("Name"), @@ -701,13 +704,15 @@ var ( VpcId: awssdk.String("vpc-1"), } subnet3 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-3"), - AvailabilityZone: awssdk.String("az3"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("az3"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-1"), } subnet4 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-4"), - AvailabilityZone: awssdk.String("az4"), + SubnetId: awssdk.String("subnet-4"), + AvailabilityZone: awssdk.String("az4"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("Name"), @@ -717,8 +722,9 @@ var ( VpcId: awssdk.String("vpc-1"), } subnet5 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-5"), - AvailabilityZone: awssdk.String("az5"), + SubnetId: awssdk.String("subnet-5"), + AvailabilityZone: awssdk.String("az5"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("Name"), @@ -728,8 +734,9 @@ var ( VpcId: awssdk.String("vpc-2"), } subnet6 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-6"), - AvailabilityZone: awssdk.String("az6"), + SubnetId: awssdk.String("subnet-6"), + AvailabilityZone: awssdk.String("az6"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("Name"), @@ -739,8 +746,9 @@ var ( VpcId: awssdk.String("vpc-1"), } subnet7 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-7"), - AvailabilityZone: awssdk.String("az7"), + SubnetId: awssdk.String("subnet-7"), + AvailabilityZone: awssdk.String("az7"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("Name"), @@ -753,8 +761,9 @@ var ( VpcId: awssdk.String("vpc-1"), } subnet8 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-8"), - AvailabilityZone: awssdk.String("az8"), + SubnetId: awssdk.String("subnet-8"), + AvailabilityZone: awssdk.String("az8"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("alb"), @@ -764,8 +773,9 @@ var ( VpcId: awssdk.String("vpc-1"), } subnet9 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-9"), - AvailabilityZone: awssdk.String("az9"), + SubnetId: awssdk.String("subnet-9"), + AvailabilityZone: awssdk.String("az9"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("Name"), @@ -782,8 +792,9 @@ var ( VpcId: awssdk.String("vpc-1"), } subnet10 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-10"), - AvailabilityZone: awssdk.String("az10"), + SubnetId: awssdk.String("subnet-10"), + AvailabilityZone: awssdk.String("az10"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("alb"), @@ -797,8 +808,9 @@ var ( VpcId: awssdk.String("vpc-1"), } subnet11 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-11"), - AvailabilityZone: awssdk.String("az11"), + SubnetId: awssdk.String("subnet-11"), + AvailabilityZone: awssdk.String("az11"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("tagtest"), @@ -812,8 +824,9 @@ var ( VpcId: awssdk.String("vpc-1"), } subnet12 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-12"), - AvailabilityZone: awssdk.String("az12"), + SubnetId: awssdk.String("subnet-12"), + AvailabilityZone: awssdk.String("az12"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("tagtest"), @@ -823,8 +836,9 @@ var ( VpcId: awssdk.String("vpc-1"), } subnet13 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-13"), - AvailabilityZone: awssdk.String("az13"), + SubnetId: awssdk.String("subnet-13"), + AvailabilityZone: awssdk.String("az13"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("tagtest"), @@ -838,8 +852,9 @@ var ( VpcId: awssdk.String("vpc-1"), } subnet14 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-14"), - AvailabilityZone: awssdk.String("az14"), + SubnetId: awssdk.String("subnet-14"), + AvailabilityZone: awssdk.String("az14"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("tagtest2"), @@ -849,8 +864,9 @@ var ( VpcId: awssdk.String("vpc-1"), } subnet15 = ec2types.Subnet{ - SubnetId: awssdk.String("subnet-15"), - AvailabilityZone: awssdk.String("az14"), + SubnetId: awssdk.String("subnet-15"), + AvailabilityZone: awssdk.String("az14"), + AvailableIpAddressCount: awssdk.Int32(8), Tags: []ec2types.Tag{ { Key: awssdk.String("tagtest2"), @@ -1177,7 +1193,7 @@ func Test_defaultModelBuildTask_buildLoadBalancerSubnets(t *testing.T) { }, }, }, - wantErr: "couldn't find all subnets, IDs: [subnet-1234 subnet-1], found: 1", + wantErr: "failed to list subnets by IDs: couldn't find all subnets, want: [subnet-1234 subnet-1], found: [subnet-1]", }, { name: "classparams ignore tagged other cluster", @@ -1271,6 +1287,9 @@ func Test_defaultModelBuildTask_buildLoadBalancerSubnets(t *testing.T) { mockEC2, "vpc-1", "test-cluster", + true, + true, + true, logr.New(&log.NullLogSink{}), ) diff --git a/pkg/networking/subnet_resolver.go b/pkg/networking/subnet_resolver.go index f00e8a51d1..a3086d0a43 100644 --- a/pkg/networking/subnet_resolver.go +++ b/pkg/networking/subnet_resolver.go @@ -2,6 +2,7 @@ package networking import ( "context" + "errors" "fmt" "sort" "strings" @@ -11,7 +12,6 @@ import ( awssdk "github.com/aws/aws-sdk-go-v2/aws" ec2sdk "github.com/aws/aws-sdk-go-v2/service/ec2" "github.com/go-logr/logr" - "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/sets" elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1" "sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services" @@ -38,6 +38,11 @@ const ( zoneTypeWavelengthZone string = "wavelength-zone" ) +const ( + // both ALB & NLB requires minimal 8 ip address count for it's subnet + defaultMinimalAvailableIPAddressCount = 8 +) + // options for resolve subnets. type SubnetsResolveOptions struct { // The Load Balancer Type. @@ -46,12 +51,6 @@ type SubnetsResolveOptions struct { // The Load Balancer Scheme. // By default, it's internet-facing. LBScheme elbv2model.LoadBalancerScheme - // count of available ip addresses - AvailableIPAddressCount int32 - // whether to check the cluster tag - SubnetsClusterTagCheck bool - // whether to allow using only 1 subnet for provisioning ALB, default to false - ALBSingleSubnet bool } // ApplyOptions applies slice of SubnetsResolveOption. @@ -85,187 +84,139 @@ func WithSubnetsResolveLBScheme(lbScheme elbv2model.LoadBalancerScheme) SubnetsR } } -// WithSubnetsResolveAvailableIPAddressCount generates an option that configures AvailableIPAddressCount. -func WithSubnetsResolveAvailableIPAddressCount(AvailableIPAddressCount int32) SubnetsResolveOption { - return func(opts *SubnetsResolveOptions) { - opts.AvailableIPAddressCount = AvailableIPAddressCount - } -} - -// WithSubnetsClusterTagCheck generates an option that configures SubnetsClusterTagCheck. -func WithSubnetsClusterTagCheck(SubnetsClusterTagCheck bool) SubnetsResolveOption { - return func(opts *SubnetsResolveOptions) { - opts.SubnetsClusterTagCheck = SubnetsClusterTagCheck - } -} - -// WithALBSingleSubnet generate an option that configures ALBSingleSubnet -func WithALBSingleSubnet(ALBSingleSubnet bool) SubnetsResolveOption { - return func(opts *SubnetsResolveOptions) { - opts.ALBSingleSubnet = ALBSingleSubnet - } -} - // SubnetsResolver is responsible for resolve EC2 Subnets for Load Balancers. type SubnetsResolver interface { // ResolveViaDiscovery resolve subnets by auto discover matching subnets. - // Discovery candidate includes all subnets within the clusterVPC. Additionally, - // * for internet-facing Load Balancer, "kubernetes.io/role/elb" tag must be present. - // * for internal Load Balancer, "kubernetes.io/role/internal-elb" tag must be present. - // * if SubnetsClusterTagCheck is enabled, subnets within the clusterVPC must contain no cluster tag at all - // or contain the "kubernetes.io/cluster/" tag for the current cluster - // If multiple subnets are found for specific AZ, one subnet is chosen based on the lexical order of subnetID. ResolveViaDiscovery(ctx context.Context, opts ...SubnetsResolveOption) ([]ec2types.Subnet, error) // ResolveViaSelector resolves subnets using a SubnetSelector. - ResolveViaSelector(ctx context.Context, selector *elbv2api.SubnetSelector, opts ...SubnetsResolveOption) ([]ec2types.Subnet, error) + ResolveViaSelector(ctx context.Context, selector elbv2api.SubnetSelector, opts ...SubnetsResolveOption) ([]ec2types.Subnet, error) // ResolveViaNameOrIDSlice resolve subnets using subnet name or ID. ResolveViaNameOrIDSlice(ctx context.Context, subnetNameOrIDs []string, opts ...SubnetsResolveOption) ([]ec2types.Subnet, error) } // NewDefaultSubnetsResolver constructs new defaultSubnetsResolver. -func NewDefaultSubnetsResolver(azInfoProvider AZInfoProvider, ec2Client services.EC2, vpcID string, clusterName string, logger logr.Logger) *defaultSubnetsResolver { +func NewDefaultSubnetsResolver( + azInfoProvider AZInfoProvider, + ec2Client services.EC2, + vpcID string, + clusterName string, + clusterTagCheckEnabled bool, + albSingleSubnetEnabled bool, + discoveryByReachabilityEnabled bool, + logger logr.Logger) *defaultSubnetsResolver { return &defaultSubnetsResolver{ - azInfoProvider: azInfoProvider, - ec2Client: ec2Client, - vpcID: vpcID, - clusterName: clusterName, - logger: logger, + azInfoProvider: azInfoProvider, + ec2Client: ec2Client, + vpcID: vpcID, + clusterName: clusterName, + clusterTagCheckEnabled: clusterTagCheckEnabled, + albSingleSubnetEnabled: albSingleSubnetEnabled, + discoverByReachabilityEnabled: discoveryByReachabilityEnabled, + minimalAvailableIPAddressCount: defaultMinimalAvailableIPAddressCount, + logger: logger, } } var _ SubnetsResolver = &defaultSubnetsResolver{} // default implementation for SubnetsResolver. +// 1. If a specific subnet name/id is provided, those subnets will be selected directly. +// 2. Otherwise, the controller selects one subnet per Availability Zone using the subnet selection algorithm: +// a. Candidate subnet determination: +// - If tag filters are specified: Only subnets matching these filters become candidates +// - If no tag filters are specified: +// * If subnets with role tags exists: Only these become candidates +// * If no subnets have role tags: Candidates are subnets whose reachability(public/private) matches the ELB's schema +// b. Subnet filtering: +// - Subnets tagged for other clusters (not the current cluster) are filtered out +// - Subnets with insufficient available IP addresses are filtered out +// c. Final selection: +// - One subnet per AZ is selected based on: +// * Priority given to subnets with cluster tag for current cluster +// * When priority is equal, selection is based on lexicographical ordering of subnet IDs type defaultSubnetsResolver struct { azInfoProvider AZInfoProvider ec2Client services.EC2 vpcID string clusterName string - logger logr.Logger + // whether enable the cluster tag check on subnets + // when enabled, only below subnets are eligible to be used as loadbalancer subnet + // - The subnet has no Kubernetes cluster tags at all + // - The subnet has a Kubernetes cluster tag matching the current cluster + clusterTagCheckEnabled bool + // whether to enable a single subnet as ALB subnet + // by default ALB requires two subent, only whitelisted users can use a single subnet + albSingleSubnetEnabled bool + // whether to enable discovery subnet by reachability(public/private) + discoverByReachabilityEnabled bool + // the minimal available IP address required for ELB subnets + minimalAvailableIPAddressCount int32 + logger logr.Logger } func (r *defaultSubnetsResolver) ResolveViaDiscovery(ctx context.Context, opts ...SubnetsResolveOption) ([]ec2types.Subnet, error) { resolveOpts := defaultSubnetsResolveOptions() resolveOpts.ApplyOptions(opts) - subnetRoleTagKey := "" + var subnetRoleTagKey string + var needsPublicSubnet bool switch resolveOpts.LBScheme { case elbv2model.LoadBalancerSchemeInternal: subnetRoleTagKey = TagKeySubnetInternalELB + needsPublicSubnet = false case elbv2model.LoadBalancerSchemeInternetFacing: subnetRoleTagKey = TagKeySubnetPublicELB + needsPublicSubnet = true + default: + return nil, fmt.Errorf("unknown lbScheme: %v", resolveOpts.LBScheme) } - - return r.ResolveViaSelector(ctx, &elbv2api.SubnetSelector{ - Tags: map[string][]string{ - subnetRoleTagKey: {"", "1"}, - }, - }, opts...) + tagFilters := map[string][]string{subnetRoleTagKey: {"", "1"}} + subnets, err := r.listSubnetsByTagFilters(ctx, tagFilters) + if err != nil { + return nil, fmt.Errorf("failed to list subnets by role tag: %w", err) + } + // when there are no subnets with matching role tag, we fallback to discovery by subnet reachability. + if len(subnets) == 0 && r.discoverByReachabilityEnabled { + subnets, err = r.listSubnetsByReachability(ctx, needsPublicSubnet) + if err != nil { + return nil, fmt.Errorf("failed to list subnets by reachability: %w", err) + } + } + chosenSubnets, err := r.chooseAndValidateSubnetsPerAZ(ctx, subnets, resolveOpts) + if err != nil { + return nil, err + } + return chosenSubnets, nil } -func (r *defaultSubnetsResolver) ResolveViaSelector(ctx context.Context, selector *elbv2api.SubnetSelector, opts ...SubnetsResolveOption) ([]ec2types.Subnet, error) { +func (r *defaultSubnetsResolver) ResolveViaSelector(ctx context.Context, selector elbv2api.SubnetSelector, opts ...SubnetsResolveOption) ([]ec2types.Subnet, error) { resolveOpts := defaultSubnetsResolveOptions() resolveOpts.ApplyOptions(opts) - var chosenSubnets []ec2types.Subnet - var err error - var explanation string - if selector.IDs != nil { - req := &ec2sdk.DescribeSubnetsInput{ - SubnetIds: make([]string, 0, len(selector.IDs)), - } + if len(selector.IDs) > 0 { + subnetIDs := make([]string, 0, len(selector.IDs)) for _, subnetID := range selector.IDs { - id := string(subnetID) - req.SubnetIds = append(req.SubnetIds, id) + subnetIDs = append(subnetIDs, string(subnetID)) } - chosenSubnets, err = r.ec2Client.DescribeSubnetsAsList(ctx, req) + subnets, err := r.listSubnetsByIDs(ctx, subnetIDs) if err != nil { - return nil, err - } - if len(chosenSubnets) != len(selector.IDs) { - return nil, errors.Errorf("couldn't find all subnets, IDs: %v, found: %v", selector.IDs, len(chosenSubnets)) + return nil, fmt.Errorf("failed to list subnets by IDs: %w", err) } - if err := r.validateSubnetsAZExclusivity(chosenSubnets); err != nil { - return nil, err - } - // todo validate here? - } else { - req := &ec2sdk.DescribeSubnetsInput{ - Filters: []ec2types.Filter{ - { - Name: awssdk.String("vpc-id"), - Values: []string{r.vpcID}, - }, - }, - } - - targetTagKeys := []string{} - for key, values := range selector.Tags { - targetTagKeys = append(targetTagKeys, key) - req.Filters = append(req.Filters, ec2types.Filter{ - Name: awssdk.String("tag:" + key), - Values: values, - }) - } - - allSubnets, err := r.ec2Client.DescribeSubnetsAsList(ctx, req) - if err != nil { + if err := r.validateSpecifiedSubnets(ctx, subnets, resolveOpts); err != nil { return nil, err } - explanation = fmt.Sprintf("%d match VPC and tags: %s", len(allSubnets), targetTagKeys) - - var subnets []ec2types.Subnet - taggedOtherCluster := 0 - for _, subnet := range allSubnets { - if r.checkSubnetIsNotTaggedForOtherClusters(subnet, resolveOpts.SubnetsClusterTagCheck) { - subnets = append(subnets, subnet) - } else { - taggedOtherCluster += 1 - } - } - if taggedOtherCluster > 0 { - explanation += fmt.Sprintf(", %d tagged for other cluster", taggedOtherCluster) - } - filteredSubnets, insufficientIPs := r.filterSubnetsByAvailableIPAddress(subnets, resolveOpts.AvailableIPAddressCount) - if insufficientIPs > 0 { - explanation += fmt.Sprintf(", %d have fewer than %d free IPs", insufficientIPs, resolveOpts.AvailableIPAddressCount) - } - subnetsByAZ := mapSDKSubnetsByAZ(filteredSubnets) - chosenSubnets = make([]ec2types.Subnet, 0, len(subnetsByAZ)) - for az, subnets := range subnetsByAZ { - if len(subnets) == 1 { - chosenSubnets = append(chosenSubnets, subnets[0]) - } else if len(subnets) > 1 { - sort.Slice(subnets, func(i, j int) bool { - clusterTagI := r.checkSubnetHasClusterTag(subnets[i]) - clusterTagJ := r.checkSubnetHasClusterTag(subnets[j]) - if clusterTagI != clusterTagJ { - if clusterTagI { - return true - } - return false - } - return awssdk.ToString(subnets[i].SubnetId) < awssdk.ToString(subnets[j].SubnetId) - }) - r.logger.Info("multiple subnet in the same AvailabilityZone", "AvailabilityZone", az, - "chosen", subnets[0].SubnetId, "ignored", subnets[1:]) - chosenSubnets = append(chosenSubnets, subnets[0]) - } - } + return subnets, nil } - if len(chosenSubnets) == 0 { - return nil, fmt.Errorf("unable to resolve at least one subnet (%s)", explanation) - } - subnetLocale, err := r.validateSubnetsLocaleUniformity(ctx, chosenSubnets) + subnets, err := r.listSubnetsByTagFilters(ctx, selector.Tags) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to list subnets by tag filters: %w", err) } - if err := r.validateSubnetsMinimalCount(chosenSubnets, subnetLocale, resolveOpts); err != nil { + chosenSubnets, err := r.chooseAndValidateSubnetsPerAZ(ctx, subnets, resolveOpts) + if err != nil { return nil, err } - sortSubnetsByID(chosenSubnets) return chosenSubnets, nil } @@ -273,6 +224,18 @@ func (r *defaultSubnetsResolver) ResolveViaNameOrIDSlice(ctx context.Context, su resolveOpts := defaultSubnetsResolveOptions() resolveOpts.ApplyOptions(opts) + subnets, err := r.listSubnetsByNameOrIDs(ctx, subnetNameOrIDs) + if err != nil { + return nil, fmt.Errorf("failed to list subnets by names or IDs: %w", err) + } + if err := r.validateSpecifiedSubnets(ctx, subnets, resolveOpts); err != nil { + return nil, err + } + return subnets, nil +} + +// listSubnetsByNameOrIDs lists subnets within vpc matching given ID or name. +func (r *defaultSubnetsResolver) listSubnetsByNameOrIDs(ctx context.Context, subnetNameOrIDs []string) ([]ec2types.Subnet, error) { var subnetIDs []string var subnetNames []string for _, nameOrID := range subnetNameOrIDs { @@ -284,52 +247,279 @@ func (r *defaultSubnetsResolver) ResolveViaNameOrIDSlice(ctx context.Context, su } var resolvedSubnets []ec2types.Subnet if len(subnetIDs) > 0 { - req := &ec2sdk.DescribeSubnetsInput{ - SubnetIds: subnetIDs, - } - subnets, err := r.ec2Client.DescribeSubnetsAsList(ctx, req) + subnets, err := r.listSubnetsByIDs(ctx, subnetIDs) if err != nil { return nil, err } resolvedSubnets = append(resolvedSubnets, subnets...) } - if len(subnetNames) > 0 { - req := &ec2sdk.DescribeSubnetsInput{ - Filters: []ec2types.Filter{ - { - Name: awssdk.String("tag:Name"), - Values: subnetNames, - }, - { - Name: awssdk.String("vpc-id"), - Values: []string{r.vpcID}, - }, - }, - } - subnets, err := r.ec2Client.DescribeSubnetsAsList(ctx, req) + subnets, err := r.listSubnetsByNames(ctx, subnetNames) if err != nil { return nil, err } resolvedSubnets = append(resolvedSubnets, subnets...) } - if len(resolvedSubnets) != len(subnetNameOrIDs) { - return nil, errors.Errorf("couldn't find all subnets, nameOrIDs: %v, found: %v", subnetNameOrIDs, len(resolvedSubnets)) + return resolvedSubnets, nil +} + +func (r *defaultSubnetsResolver) listSubnetsByIDs(ctx context.Context, subnetIDs []string) ([]ec2types.Subnet, error) { + req := &ec2sdk.DescribeSubnetsInput{ + SubnetIds: subnetIDs, + } + subnets, err := r.ec2Client.DescribeSubnetsAsList(ctx, req) + if err != nil { + return nil, err } - if len(resolvedSubnets) == 0 { - return nil, errors.New("unable to resolve at least one subnet") + if len(subnets) != len(subnetIDs) { + return nil, fmt.Errorf("couldn't find all subnets, want: %v, found: %v", subnetIDs, extractSubnetIDs(subnets)) } - if err := r.validateSubnetsAZExclusivity(resolvedSubnets); err != nil { + return subnets, nil +} + +func (r *defaultSubnetsResolver) listSubnetsByNames(ctx context.Context, subnetNames []string) ([]ec2types.Subnet, error) { + req := &ec2sdk.DescribeSubnetsInput{ + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{r.vpcID}, + }, + { + Name: awssdk.String("tag:Name"), + Values: subnetNames, + }, + }, + } + subnets, err := r.ec2Client.DescribeSubnetsAsList(ctx, req) + if err != nil { return nil, err } - subnetLocale, err := r.validateSubnetsLocaleUniformity(ctx, resolvedSubnets) + if len(subnets) != len(subnetNames) { + return nil, fmt.Errorf("couldn't find all subnets, want: %v, found: %v", subnetNames, extractSubnetIDs(subnets)) + } + return subnets, nil +} + +// listSubnetsByTagFilters list subnets in vpc matches specified tag filter +func (r *defaultSubnetsResolver) listSubnetsByTagFilters(ctx context.Context, tagFilters map[string][]string) ([]ec2types.Subnet, error) { + req := &ec2sdk.DescribeSubnetsInput{ + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{r.vpcID}, + }, + }, + } + for key, values := range tagFilters { + req.Filters = append(req.Filters, ec2types.Filter{ + Name: awssdk.String("tag:" + key), + Values: values, + }) + } + return r.ec2Client.DescribeSubnetsAsList(ctx, req) +} + +// listSubnetsByReachability list subnets in vpc that matches given public/private subnet +func (r *defaultSubnetsResolver) listSubnetsByReachability(ctx context.Context, needsPublicSubnet bool) ([]ec2types.Subnet, error) { + subnetsReq := &ec2sdk.DescribeSubnetsInput{ + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{r.vpcID}, + }, + }, + } + subnets, err := r.ec2Client.DescribeSubnetsAsList(ctx, subnetsReq) if err != nil { return nil, err } - if err := r.validateSubnetsMinimalCount(resolvedSubnets, subnetLocale, resolveOpts); err != nil { + routeTablesReq := &ec2sdk.DescribeRouteTablesInput{ + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{r.vpcID}, + }, + }, + } + routeTables, err := r.ec2Client.DescribeRouteTablesAsList(ctx, routeTablesReq) + if err != nil { return nil, err } - return resolvedSubnets, nil + var subnetsMatchingReachability []ec2types.Subnet + for _, subnet := range subnets { + isPublicSubnet, err := r.isSubnetContainsRouteToIGW(awssdk.ToString(subnet.SubnetId), routeTables) + if err != nil { + return nil, err + } + if isPublicSubnet == needsPublicSubnet { + subnetsMatchingReachability = append(subnetsMatchingReachability, subnet) + } + } + return subnetsMatchingReachability, nil +} + +// isSubnetContainsRouteToIGW tests whether a subnet contains a route to internetIGW, i.e. have route to IGW. +func (r *defaultSubnetsResolver) isSubnetContainsRouteToIGW(subnetID string, routeTables []ec2types.RouteTable) (bool, error) { + var mainRT *ec2types.RouteTable + var subnetRT *ec2types.RouteTable + for i, rt := range routeTables { + for _, rtAssoc := range rt.Associations { + if awssdk.ToBool(rtAssoc.Main) { + mainRT = &routeTables[i] + } + if awssdk.ToString(rtAssoc.SubnetId) == subnetID { + subnetRT = &routeTables[i] + break + } + } + } + // If there is no explicit association, the subnet will be implicitly associated with the VPC's main routing table. + if subnetRT == nil { + if mainRT == nil { + // every VPC shall have a main route table. let's make this an error so we know if this precondition breaks. + return false, fmt.Errorf("[this shall never happen] could not identify main route table for vpc: %v", r.vpcID) + } + subnetRT = mainRT + } + for _, route := range subnetRT.Routes { + if strings.HasPrefix(awssdk.ToString(route.GatewayId), "igw") { + return true, nil + } + } + return false, nil +} + +// validateSpecifiedSubnets will validate explicitly supplied subnets from given id or name. +func (r *defaultSubnetsResolver) validateSpecifiedSubnets(ctx context.Context, subnets []ec2types.Subnet, resolveOpts SubnetsResolveOptions) error { + if len(subnets) == 0 { + return errors.New("unable to resolve at least one subnet") + } + if err := r.validateSubnetsAZExclusivity(subnets); err != nil { + return err + } + subnetLocale, err := r.validateSubnetsLocaleUniformity(ctx, subnets) + if err != nil { + return err + } + if err := r.validateSubnetsMinimalCount(subnets, subnetLocale, resolveOpts); err != nil { + return err + } + return nil +} + +// chooseAndValidateSubnetsPerAZ will choose one subnet per AZ from eligible subnets and then validate against chosen subnets. +func (r *defaultSubnetsResolver) chooseAndValidateSubnetsPerAZ(ctx context.Context, subnets []ec2types.Subnet, resolveOpts SubnetsResolveOptions) ([]ec2types.Subnet, error) { + categorizedSubnets := r.categorizeSubnetsByEligibility(subnets) + chosenSubnets := r.chooseSubnetsPerAZ(categorizedSubnets.eligible) + if len(chosenSubnets) == 0 { + return nil, fmt.Errorf("unable to resolve at least one subnet. Evaluated %d subnets: %d are tagged for other clusters, and %d have insufficient available IP addresses", + len(subnets), len(categorizedSubnets.ineligibleClusterTag), len(categorizedSubnets.insufficientIPs)) + } + subnetLocale, err := r.validateSubnetsLocaleUniformity(ctx, chosenSubnets) + if err != nil { + return nil, err + } + if err := r.validateSubnetsMinimalCount(chosenSubnets, subnetLocale, resolveOpts); err != nil { + return nil, err + } + return chosenSubnets, nil +} + +type categorizeSubnetsByEligibilityResult struct { + eligible []ec2types.Subnet + ineligibleClusterTag []ec2types.Subnet + insufficientIPs []ec2types.Subnet +} + +// categorizeSubnetsByEligibility will categorize subnets based it's eligibility of ELB subnet +func (r *defaultSubnetsResolver) categorizeSubnetsByEligibility(subnets []ec2types.Subnet) categorizeSubnetsByEligibilityResult { + var ret categorizeSubnetsByEligibilityResult + for _, subnet := range subnets { + if !r.isSubnetContainsEligibleClusterTag(subnet) { + ret.ineligibleClusterTag = append(ret.ineligibleClusterTag, subnet) + continue + } + if !r.isSubnetContainsSufficientIPAddresses(subnet) { + ret.insufficientIPs = append(ret.insufficientIPs, subnet) + continue + } + ret.eligible = append(ret.eligible, subnet) + } + return ret +} + +// chooseSubnetsPerAZ will choose one subnet per AZ. +// * subnets with current cluster tag will be prioritized. +func (r *defaultSubnetsResolver) chooseSubnetsPerAZ(subnets []ec2types.Subnet) []ec2types.Subnet { + subnetsByAZ := mapSDKSubnetsByAZ(subnets) + chosenSubnets := make([]ec2types.Subnet, 0, len(subnetsByAZ)) + for az, azSubnets := range subnetsByAZ { + if len(azSubnets) == 1 { + chosenSubnets = append(chosenSubnets, azSubnets[0]) + } else if len(subnets) > 1 { + sort.Slice(subnets, func(i, j int) bool { + subnetIHasCurrentClusterTag := r.isSubnetContainsCurrentClusterTag(subnets[i]) + subnetJHasCurrentClusterTag := r.isSubnetContainsCurrentClusterTag(subnets[j]) + if subnetIHasCurrentClusterTag && (!subnetJHasCurrentClusterTag) { + return true + } else if (!subnetIHasCurrentClusterTag) && subnetJHasCurrentClusterTag { + return false + } + return awssdk.ToString(subnets[i].SubnetId) < awssdk.ToString(subnets[j].SubnetId) + }) + r.logger.V(1).Info("multiple subnets in the same AvailabilityZone", "AvailabilityZone", az, + "chosen", subnets[0].SubnetId, "ignored", extractSubnetIDs(subnets[1:])) + chosenSubnets = append(chosenSubnets, subnets[0]) + } + } + sortSubnetsByID(chosenSubnets) + return chosenSubnets +} + +// isSubnetContainsCurrentClusterTag checks whether a subnet is tagged with current Kubernetes cluster tag. +func (r *defaultSubnetsResolver) isSubnetContainsCurrentClusterTag(subnet ec2types.Subnet) bool { + clusterResourceTagKey := fmt.Sprintf("kubernetes.io/cluster/%s", r.clusterName) + for _, tag := range subnet.Tags { + tagKey := awssdk.ToString(tag.Key) + if tagKey == clusterResourceTagKey { + return true + } + } + return false +} + +// isSubnetContainsEligibleClusterTag checks whether a subnet is eligible as load balancer subnet based on Kubernetes cluster tag existence +// Returns true if either: +// - The subnet has no Kubernetes cluster tags at all +// - The subnet has a Kubernetes cluster tag matching the current cluster +// - The clusterTagCheck feature is disabled +// +// Returns false if the subnet only contains tags for other clusters and subnetsClusterTagCheck is enabled. +// This prevents load balancers from using subnets tagged exclusively for other clusters. +func (r *defaultSubnetsResolver) isSubnetContainsEligibleClusterTag(subnet ec2types.Subnet) bool { + if !r.clusterTagCheckEnabled { + return true + } + clusterResourceTagPrefix := "kubernetes.io/cluster" + clusterResourceTagKey := fmt.Sprintf("kubernetes.io/cluster/%s", r.clusterName) + hasClusterResourceTagPrefix := false + for _, tag := range subnet.Tags { + tagKey := awssdk.ToString(tag.Key) + if tagKey == clusterResourceTagKey { + return true + } + if strings.HasPrefix(tagKey, clusterResourceTagPrefix) { + // If the cluster tag is for a different cluster, keep track of it and exclude + // the subnet if no matching tag found for the current cluster. + hasClusterResourceTagPrefix = true + } + } + return !hasClusterResourceTagPrefix +} + +// isSubnetContainsSufficientIPAddresses checks whether subnet has minimal AvailableIPAddressAcount needed. +func (r *defaultSubnetsResolver) isSubnetContainsSufficientIPAddresses(subnet ec2types.Subnet) bool { + return awssdk.ToInt32(subnet.AvailableIpAddressCount) >= r.minimalAvailableIPAddressCount } // validateSDKSubnetsAZExclusivity validates subnets belong to different AZs. @@ -338,11 +528,7 @@ func (r *defaultSubnetsResolver) validateSubnetsAZExclusivity(subnets []ec2types subnetsByAZ := mapSDKSubnetsByAZ(subnets) for az, subnets := range subnetsByAZ { if len(subnets) > 1 { - subnetIDs := make([]string, 0, len(subnets)) - for _, subnet := range subnets { - subnetIDs = append(subnetIDs, awssdk.ToString(subnet.SubnetId)) - } - return errors.Errorf("multiple subnets in same Availability Zone %v: %v", az, subnetIDs) + return fmt.Errorf("multiple subnets in same Availability Zone %v: %v", az, extractSubnetIDs(subnets)) } } return nil @@ -360,7 +546,7 @@ func (r *defaultSubnetsResolver) validateSubnetsLocaleUniformity(ctx context.Con subnetLocales.Insert(string(subnetLocale)) } if len(subnetLocales) > 1 { - return "", errors.Errorf("subnets in multiple locales: %v", subnetLocales.List()) + return "", fmt.Errorf("subnets in multiple locales: %v", subnetLocales.List()) } subnetLocale, _ := subnetLocales.PopAny() return subnetLocaleType(subnetLocale), nil @@ -370,7 +556,7 @@ func (r *defaultSubnetsResolver) validateSubnetsLocaleUniformity(ctx context.Con func (r *defaultSubnetsResolver) validateSubnetsMinimalCount(subnets []ec2types.Subnet, subnetLocale subnetLocaleType, resolveOpts SubnetsResolveOptions) error { minimalCount := r.computeSubnetsMinimalCount(subnetLocale, resolveOpts) if len(subnets) < minimalCount { - return errors.Errorf("subnets count less than minimal required count: %v < %v", len(subnets), minimalCount) + return fmt.Errorf("subnets count less than minimal required count: %v < %v", len(subnets), minimalCount) } return nil } @@ -378,7 +564,7 @@ func (r *defaultSubnetsResolver) validateSubnetsMinimalCount(subnets []ec2types. // computeSubnetsMinimalCount returns the minimal count requirement for subnets. func (r *defaultSubnetsResolver) computeSubnetsMinimalCount(subnetLocale subnetLocaleType, resolveOpts SubnetsResolveOptions) int { minimalCount := 1 - if resolveOpts.LBType == elbv2model.LoadBalancerTypeApplication && subnetLocale == subnetLocaleTypeAvailabilityZone && !resolveOpts.ALBSingleSubnet { + if resolveOpts.LBType == elbv2model.LoadBalancerTypeApplication && subnetLocale == subnetLocaleTypeAvailabilityZone && !r.albSingleSubnetEnabled { minimalCount = 2 } return minimalCount @@ -404,49 +590,10 @@ func (r *defaultSubnetsResolver) buildSDKSubnetLocaleType(ctx context.Context, s case zoneTypeWavelengthZone: return subnetLocaleTypeWavelengthZone, nil default: - return "", errors.Errorf("unknown zone type for subnet %v: %v", awssdk.ToString(subnet.SubnetId), subnetZoneType) + return "", fmt.Errorf("unknown zone type for subnet %v: %v", awssdk.ToString(subnet.SubnetId), subnetZoneType) } } -// checkSubnetHasClusterTag checks if the subnet is tagged for the current cluster -func (r *defaultSubnetsResolver) checkSubnetHasClusterTag(subnet ec2types.Subnet) bool { - clusterResourceTagKey := fmt.Sprintf("kubernetes.io/cluster/%s", r.clusterName) - for _, tag := range subnet.Tags { - if clusterResourceTagKey == awssdk.ToString(tag.Key) { - return true - } - } - return false -} - -// checkSubnetIsNotTaggedForOtherClusters checks whether the subnet is tagged for the current cluster -// or it doesn't contain the cluster tag at all. If the subnet contains a tag for other clusters, then -// this check returns false so that the subnet does not used for the load balancer. -// it returns true if the subnetsClusterTagCheck is disabled -func (r *defaultSubnetsResolver) checkSubnetIsNotTaggedForOtherClusters(subnet ec2types.Subnet, subnetsClusterTagCheck bool) bool { - if !subnetsClusterTagCheck { - return true - } - clusterResourceTagPrefix := "kubernetes.io/cluster" - clusterResourceTagKey := fmt.Sprintf("kubernetes.io/cluster/%s", r.clusterName) - hasClusterResourceTagPrefix := false - for _, tag := range subnet.Tags { - tagKey := awssdk.ToString(tag.Key) - if tagKey == clusterResourceTagKey { - return true - } - if strings.HasPrefix(tagKey, clusterResourceTagPrefix) { - // If the cluster tag is for a different cluster, keep track of it and exclude - // the subnet if no matching tag found for the current cluster. - hasClusterResourceTagPrefix = true - } - } - if hasClusterResourceTagPrefix { - return false - } - return true -} - // mapSDKSubnetsByAZ builds the subnets slice by AZ mapping. func mapSDKSubnetsByAZ(subnets []ec2types.Subnet) map[string][]ec2types.Subnet { subnetsByAZ := make(map[string][]ec2types.Subnet) @@ -464,18 +611,11 @@ func sortSubnetsByID(subnets []ec2types.Subnet) { }) } -func (r *defaultSubnetsResolver) filterSubnetsByAvailableIPAddress(subnets []ec2types.Subnet, availableIPAddressCount int32) ([]ec2types.Subnet, int) { - filteredSubnets := make([]ec2types.Subnet, 0, len(subnets)) - - insufficientIPs := 0 +// extractSubnetIDs for given subnets. +func extractSubnetIDs(subnets []ec2types.Subnet) []string { + subnetIDs := make([]string, 0, len(subnets)) for _, subnet := range subnets { - if awssdk.ToInt32(subnet.AvailableIpAddressCount) >= availableIPAddressCount { - filteredSubnets = append(filteredSubnets, subnet) - } else { - insufficientIPs += 1 - r.logger.Info("ELB requires at least 8 free IP addresses in each subnet", - "not enough IP addresses found in ", awssdk.ToString(subnet.SubnetId)) - } + subnetIDs = append(subnetIDs, awssdk.ToString(subnet.SubnetId)) } - return filteredSubnets, insufficientIPs + return subnetIDs } diff --git a/pkg/networking/subnet_resolver_mocks.go b/pkg/networking/subnet_resolver_mocks.go index 83f7fb1d4f..94dabc2233 100644 --- a/pkg/networking/subnet_resolver_mocks.go +++ b/pkg/networking/subnet_resolver_mocks.go @@ -77,7 +77,7 @@ func (mr *MockSubnetsResolverMockRecorder) ResolveViaNameOrIDSlice(arg0, arg1 in } // ResolveViaSelector mocks base method. -func (m *MockSubnetsResolver) ResolveViaSelector(arg0 context.Context, arg1 *v1beta1.SubnetSelector, arg2 ...SubnetsResolveOption) ([]types.Subnet, error) { +func (m *MockSubnetsResolver) ResolveViaSelector(arg0 context.Context, arg1 v1beta1.SubnetSelector, arg2 ...SubnetsResolveOption) ([]types.Subnet, error) { m.ctrl.T.Helper() varargs := []interface{}{arg0, arg1} for _, a := range arg2 { diff --git a/pkg/networking/subnet_resolver_test.go b/pkg/networking/subnet_resolver_test.go index 38bf50e7e1..1f30ab5b4d 100644 --- a/pkg/networking/subnet_resolver_test.go +++ b/pkg/networking/subnet_resolver_test.go @@ -3,9 +3,10 @@ package networking import ( "context" "errors" - ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" "testing" + ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" + awssdk "github.com/aws/aws-sdk-go-v2/aws" ec2sdk "github.com/aws/aws-sdk-go-v2/service/ec2" "github.com/go-logr/logr" @@ -13,6 +14,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" + elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1" "sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services" elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" "sigs.k8s.io/controller-runtime/pkg/log" @@ -24,24 +26,27 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { output []ec2types.Subnet err error } + type describeRouteTablesAsListCall struct { + input *ec2sdk.DescribeRouteTablesInput + output []ec2types.RouteTable + err error + } type fetchAZInfosCall struct { availabilityZoneIDs []string azInfoByAZID map[string]ec2types.AvailabilityZone err error } type fields struct { - vpcID string - clusterName string - describeSubnetsAsListCalls []describeSubnetsAsListCall - fetchAZInfosCalls []fetchAZInfosCall + clusterTagCheckEnabled bool + albSingleSubnetEnabled bool + discoveryByReachabilityEnabled bool + describeSubnetsAsListCalls []describeSubnetsAsListCall + describeRouteTablesAsListCalls []describeRouteTablesAsListCall + fetchAZInfosCalls []fetchAZInfosCall } type args struct { opts []SubnetsResolveOption } - const ( - minimalAvailableIPAddressCount = int32(8) - defaultSubnetsClusterTagCheck = true - ) tests := []struct { name string fields fields @@ -50,17 +55,18 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { wantErr error }{ { - name: "ALB internet facing", + name: "alb/internet-facing, discovered via role tag", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Values: []string{"vpc-dummy"}, }, { Name: awssdk.String("tag:kubernetes.io/role/elb"), @@ -70,16 +76,30 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, }, }, }, @@ -113,31 +133,46 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, want: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, }, }, }, { - name: "ALB internal", + name: "alb/internal, discovered via role tag", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Values: []string{"vpc-dummy"}, }, { Name: awssdk.String("tag:kubernetes.io/role/internal-elb"), @@ -147,16 +182,30 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/internal-elb"), + Value: awssdk.String("1"), + }, + }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/internal-elb"), + Value: awssdk.String("1"), + }, + }, }, }, }, @@ -190,106 +239,148 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, want: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/internal-elb"), + Value: awssdk.String("1"), + }, + }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/internal-elb"), + Value: awssdk.String("1"), + }, + }, }, }, }, { - name: "ALB with no matching subnets (internal)", + name: "alb/internet-facing, discovered via fallback to reachability", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Values: []string{"vpc-dummy"}, }, { - Name: awssdk.String("tag:kubernetes.io/role/internal-elb"), + Name: awssdk.String("tag:kubernetes.io/role/elb"), Values: []string{"", "1"}, }, }, }, output: nil, }, - }, - }, - args: args{ - opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), - }, - }, - wantErr: errors.New("unable to resolve at least one subnet (0 match VPC and tags: [kubernetes.io/role/internal-elb])"), - }, - { - name: "ALB with no matching subnets (internet-facing)", - fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", - describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, - }, - { - Name: awssdk.String("tag:kubernetes.io/role/elb"), - Values: []string{"", "1"}, + Values: []string{"vpc-dummy"}, }, }, }, - output: nil, + output: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-4"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + }, }, }, - }, - args: args{ - opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), - }, - }, - wantErr: errors.New("unable to resolve at least one subnet (0 match VPC and tags: [kubernetes.io/role/elb])"), - }, - { - name: "NLB with one matching subnet", - fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", - describeSubnetsAsListCalls: []describeSubnetsAsListCall{ + describeRouteTablesAsListCalls: []describeRouteTablesAsListCall{ { - input: &ec2sdk.DescribeSubnetsInput{ + input: &ec2sdk.DescribeRouteTablesInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, - }, - { - Name: awssdk.String("tag:kubernetes.io/role/internal-elb"), - Values: []string{"", "1"}, + Values: []string{"vpc-dummy"}, }, }, }, - output: []ec2types.Subnet{ + output: []ec2types.RouteTable{ + { + RouteTableId: awssdk.String("rtb-main"), + Associations: []ec2types.RouteTableAssociation{ + { + Main: awssdk.Bool(true), + }, + }, + }, + { + RouteTableId: awssdk.String("rtb-public"), + Associations: []ec2types.RouteTableAssociation{ + { + Main: awssdk.Bool(false), + SubnetId: awssdk.String("subnet-1"), + }, + { + Main: awssdk.Bool(false), + SubnetId: awssdk.String("subnet-2"), + }, + }, + Routes: []ec2types.Route{ + { + GatewayId: awssdk.String("igw-xxx"), + }, + }, + }, { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + RouteTableId: awssdk.String("rtb-private"), + Associations: []ec2types.RouteTableAssociation{ + { + Main: awssdk.Bool(false), + }, + { + Main: awssdk.Bool(false), + SubnetId: awssdk.String("subnet-3"), + }, + { + Main: awssdk.Bool(false), + SubnetId: awssdk.String("subnet-4"), + }, + }, }, }, }, @@ -304,35 +395,53 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, }, }, + { + availabilityZoneIDs: []string{"usw2-az2"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az2": { + ZoneId: awssdk.String("usw2-az2"), + ZoneType: awssdk.String("availability-zone"), + }, + }, + }, }, }, args: args{ opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), }, }, want: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, { - name: "ALB with one matching availability-zone subnet", + name: "alb/internal, discovered via fallback to reachability", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Values: []string{"vpc-dummy"}, }, { Name: awssdk.String("tag:kubernetes.io/role/internal-elb"), @@ -340,182 +449,101 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, }, }, - output: []ec2types.Subnet{ - { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - }, - }, - }, - }, - fetchAZInfosCalls: []fetchAZInfosCall{ - { - availabilityZoneIDs: []string{"usw2-az1"}, - azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-az1": { - ZoneId: awssdk.String("usw2-az1"), - ZoneType: awssdk.String("availability-zone"), - }, - }, + output: nil, }, - }, - }, - args: args{ - opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), - }, - }, - wantErr: errors.New("subnets count less than minimal required count: 1 < 2"), - }, - { - name: "ALB with one matching local-zone subnet", - fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", - describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, - }, - { - Name: awssdk.String("tag:kubernetes.io/role/internal-elb"), - Values: []string{"", "1"}, + Values: []string{"vpc-dummy"}, }, }, }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2-lax-1a"), - AvailabilityZoneId: awssdk.String("usw2-lax1-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-4"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, }, - fetchAZInfosCalls: []fetchAZInfosCall{ + describeRouteTablesAsListCalls: []describeRouteTablesAsListCall{ { - availabilityZoneIDs: []string{"usw2-lax1-az1"}, - azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-lax1-az1": { - ZoneId: awssdk.String("usw2-lax1-az1"), - ZoneType: awssdk.String("local-zone"), + input: &ec2sdk.DescribeRouteTablesInput{ + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, + }, }, }, - }, - }, - }, - args: args{ - opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), - }, - }, - want: []ec2types.Subnet{ - { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2-lax-1a"), - AvailabilityZoneId: awssdk.String("usw2-lax1-az1"), - VpcId: awssdk.String("vpc-1"), - }, - }, - }, - { - name: "ALB with one matching outpost subnet", - fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", - describeSubnetsAsListCalls: []describeSubnetsAsListCall{ - { - input: &ec2sdk.DescribeSubnetsInput{ - Filters: []ec2types.Filter{ - { - Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, - }, - { - Name: awssdk.String("tag:kubernetes.io/role/internal-elb"), - Values: []string{"", "1"}, + output: []ec2types.RouteTable{ + { + RouteTableId: awssdk.String("rtb-main"), + Associations: []ec2types.RouteTableAssociation{ + { + Main: awssdk.Bool(true), + }, }, }, - }, - output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - OutpostArn: awssdk.String("outpost-xxx"), - VpcId: awssdk.String("vpc-1"), - }, - }, - }, - }, - }, - args: args{ - opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), - }, - }, - want: []ec2types.Subnet{ - { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - OutpostArn: awssdk.String("outpost-xxx"), - VpcId: awssdk.String("vpc-1"), - }, - }, - }, - { - name: "multiple subnets per az", - fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", - describeSubnetsAsListCalls: []describeSubnetsAsListCall{ - { - input: &ec2sdk.DescribeSubnetsInput{ - Filters: []ec2types.Filter{ - { - Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + RouteTableId: awssdk.String("rtb-public"), + Associations: []ec2types.RouteTableAssociation{ + { + Main: awssdk.Bool(false), + SubnetId: awssdk.String("subnet-1"), + }, + { + Main: awssdk.Bool(false), + SubnetId: awssdk.String("subnet-2"), + }, }, - { - Name: awssdk.String("tag:kubernetes.io/role/internal-elb"), - Values: []string{"", "1"}, + Routes: []ec2types.Route{ + { + GatewayId: awssdk.String("igw-xxx"), + }, }, }, - }, - output: []ec2types.Subnet{ - { - SubnetId: awssdk.String("subnet-3"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - }, - { - SubnetId: awssdk.String("subnet-4"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), - }, - { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + RouteTableId: awssdk.String("rtb-private"), + Associations: []ec2types.RouteTableAssociation{ + { + Main: awssdk.Bool(false), + }, + { + Main: awssdk.Bool(false), + SubnetId: awssdk.String("subnet-3"), + }, + { + Main: awssdk.Bool(false), + SubnetId: awssdk.String("subnet-4"), + }, + }, }, }, }, @@ -549,51 +577,84 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, want: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-4"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, { - name: "multiple subnet locales", + name: "subnets tagged for other clusters shall be filtered out when clusterTagCheckEnabled enabled", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Values: []string{"vpc-dummy"}, }, { - Name: awssdk.String("tag:kubernetes.io/role/internal-elb"), + Name: awssdk.String("tag:kubernetes.io/role/elb"), Values: []string{"", "1"}, }, }, }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + { + Key: awssdk.String("kubernetes.io/cluster/other-cluster"), + Value: awssdk.String("owned"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), - OutpostArn: awssdk.String("outpost-xxx"), + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, }, }, }, @@ -608,59 +669,65 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, }, }, + { + availabilityZoneIDs: []string{"usw2-az2"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az2": { + ZoneId: awssdk.String("usw2-az2"), + ZoneType: awssdk.String("availability-zone"), + }, + }, + }, }, }, args: args{ opts: []SubnetsResolveOption{ WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), }, }, - wantErr: errors.New("subnets in multiple locales: [availability-zone outpost]"), - }, - { - name: "describeSubnetsAsList returns error", - fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", - describeSubnetsAsListCalls: []describeSubnetsAsListCall{ - { - input: &ec2sdk.DescribeSubnetsInput{ - Filters: []ec2types.Filter{ - { - Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, - }, - { - Name: awssdk.String("tag:kubernetes.io/role/internal-elb"), - Values: []string{"", "1"}, - }, - }, + want: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), }, - err: errors.New("some error"), }, }, - }, - args: args{ - opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, }, }, - wantErr: errors.New("some error"), }, { - name: "subnet with cluster tag gets precedence", + name: "subnets tagged for other clusters shall not be filtered out when clusterTagCheckEnabled disabled", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: false, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Values: []string{"vpc-dummy"}, }, { Name: awssdk.String("tag:kubernetes.io/role/elb"), @@ -670,20 +737,45 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + { + Key: awssdk.String("kubernetes.io/cluster/other-cluster"), + Value: awssdk.String("owned"), + }, + }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), Tags: []ec2types.Tag{ { - Key: awssdk.String("kubernetes.io/cluster/kube-cluster"), - Value: awssdk.String("owned"), + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), }, }, }, @@ -691,6 +783,15 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, }, fetchAZInfosCalls: []fetchAZInfosCall{ + { + availabilityZoneIDs: []string{"usw2-az1"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az1": { + ZoneId: awssdk.String("usw2-az1"), + ZoneType: awssdk.String("availability-zone"), + }, + }, + }, { availabilityZoneIDs: []string{"usw2-az2"}, azInfoByAZID: map[string]ec2types.AvailabilityZone{ @@ -704,37 +805,56 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, args: args{ opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork), + WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), }, }, want: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), Tags: []ec2types.Tag{ { - Key: awssdk.String("kubernetes.io/cluster/kube-cluster"), + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + { + Key: awssdk.String("kubernetes.io/cluster/other-cluster"), Value: awssdk.String("owned"), }, }, }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, + }, }, }, { - name: "subnets tagged for some other clusters get ignored, with SubnetsClusterTagCheck enabled", + name: "subnets with insufficient available ip addresses shall be filtered out", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Values: []string{"vpc-dummy"}, }, { Name: awssdk.String("tag:kubernetes.io/role/elb"), @@ -744,59 +864,44 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - Tags: []ec2types.Tag{ - { - Key: awssdk.String("kubernetes.io/cluster/some-other-cluster"), - Value: awssdk.String("owned"), - }, - }, - }, - { - SubnetId: awssdk.String("subnet-3"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(2), + VpcId: awssdk.String("vpc-dummy"), Tags: []ec2types.Tag{ { - Key: awssdk.String("kubernetes.io/cluster/kube-cluster"), - Value: awssdk.String("owned"), + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), }, }, }, { - SubnetId: awssdk.String("subnet-4"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), Tags: []ec2types.Tag{ { - Key: awssdk.String("kubernetes.io/cluster/no-cluster"), - Value: awssdk.String("owned"), + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), }, }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), Tags: []ec2types.Tag{ { - Key: awssdk.String("kubernetes.io/cluster/kube-cluster"), - Value: awssdk.String("owned"), + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), }, }, }, - { - SubnetId: awssdk.String("subnet-5"), - AvailabilityZone: awssdk.String("us-west-2c"), - AvailabilityZoneId: awssdk.String("usw2-az3"), - VpcId: awssdk.String("vpc-1"), - }, }, }, }, @@ -811,10 +916,10 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, }, { - availabilityZoneIDs: []string{"usw2-az3"}, + availabilityZoneIDs: []string{"usw2-az2"}, azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-az3": { - ZoneId: awssdk.String("usw2-az3"), + "usw2-az2": { + ZoneId: awssdk.String("usw2-az2"), ZoneType: awssdk.String("availability-zone"), }, }, @@ -823,44 +928,52 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, args: args{ opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork), + WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), - WithSubnetsClusterTagCheck(defaultSubnetsClusterTagCheck), }, }, want: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), Tags: []ec2types.Tag{ { - Key: awssdk.String("kubernetes.io/cluster/kube-cluster"), - Value: awssdk.String("owned"), + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), }, }, }, { - SubnetId: awssdk.String("subnet-5"), - AvailabilityZone: awssdk.String("us-west-2c"), - AvailabilityZoneId: awssdk.String("usw2-az3"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, }, }, }, { - name: "subnets tagged for some other clusters doesn't get ignored, with SubnetsClusterTagCheck disabled", + name: "subnets are either tagged for other clusters or with insufficient ip address", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Values: []string{"vpc-dummy"}, }, { Name: awssdk.String("tag:kubernetes.io/role/elb"), @@ -870,111 +983,60 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-3"), - AvailabilityZone: awssdk.String("us-west-2c"), - AvailabilityZoneId: awssdk.String("usw2-az3"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), Tags: []ec2types.Tag{ { - Key: awssdk.String("kubernetes.io/cluster/some-other-cluster"), - Value: awssdk.String("owned"), + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), }, - }, - }, - { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - Tags: []ec2types.Tag{ { - Key: awssdk.String("kubernetes.io/cluster/some-other-cluster"), + Key: awssdk.String("kubernetes.io/cluster/other-cluster"), Value: awssdk.String("owned"), }, }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2c"), - AvailabilityZoneId: awssdk.String("usw2-az3"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(2), + VpcId: awssdk.String("vpc-dummy"), Tags: []ec2types.Tag{ { - Key: awssdk.String("kubernetes.io/cluster/kube-cluster"), - Value: awssdk.String("owned"), + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), }, }, }, }, }, }, - fetchAZInfosCalls: []fetchAZInfosCall{ - { - availabilityZoneIDs: []string{"usw2-az1"}, - azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-az1": { - ZoneId: awssdk.String("usw2-az1"), - ZoneType: awssdk.String("availability-zone"), - }, - }, - }, - { - availabilityZoneIDs: []string{"usw2-az3"}, - azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-az3": { - ZoneId: awssdk.String("usw2-az3"), - ZoneType: awssdk.String("availability-zone"), - }, - }, - }, - }, }, args: args{ opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork), + WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), - WithSubnetsClusterTagCheck(false), - }, - }, - want: []ec2types.Subnet{ - { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - Tags: []ec2types.Tag{ - { - Key: awssdk.String("kubernetes.io/cluster/some-other-cluster"), - Value: awssdk.String("owned"), - }, - }, - }, - { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2c"), - AvailabilityZoneId: awssdk.String("usw2-az3"), - VpcId: awssdk.String("vpc-1"), - Tags: []ec2types.Tag{ - { - Key: awssdk.String("kubernetes.io/cluster/kube-cluster"), - Value: awssdk.String("owned"), - }, - }, }, }, + wantErr: errors.New("unable to resolve at least one subnet. Evaluated 2 subnets: 1 are tagged for other clusters, and 1 have insufficient available IP addresses"), }, { - name: "subnets with multiple cluster tags", + name: "fallback to reachability were disabled", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: false, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Values: []string{"vpc-dummy"}, }, { Name: awssdk.String("tag:kubernetes.io/role/elb"), @@ -982,75 +1044,63 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, }, }, - output: []ec2types.Subnet{ - { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - Tags: []ec2types.Tag{ - { - Key: awssdk.String("kubernetes.io/cluster/some-other-cluster"), - Value: awssdk.String("owned"), - }, - { - Key: awssdk.String("kubernetes.io/cluster/kube-cluster"), - Value: awssdk.String("shared"), - }, - }, - }, - }, + output: nil, }, }, - fetchAZInfosCalls: []fetchAZInfosCall{ + }, + args: args{ + opts: []SubnetsResolveOption{ + WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), + }, + }, + wantErr: errors.New("unable to resolve at least one subnet. Evaluated 0 subnets: 0 are tagged for other clusters, and 0 have insufficient available IP addresses"), + }, + { + name: "failed to list subnets by role tag", + fields: fields{ + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, + describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { - availabilityZoneIDs: []string{"usw2-az1"}, - azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-az1": { - ZoneId: awssdk.String("usw2-az1"), - ZoneType: awssdk.String("availability-zone"), + input: &ec2sdk.DescribeSubnetsInput{ + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, + }, + { + Name: awssdk.String("tag:kubernetes.io/role/elb"), + Values: []string{"", "1"}, + }, }, }, + err: errors.New("some auth error"), }, }, }, args: args{ opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork), + WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), }, }, - want: []ec2types.Subnet{ - { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - Tags: []ec2types.Tag{ - { - Key: awssdk.String("kubernetes.io/cluster/some-other-cluster"), - Value: awssdk.String("owned"), - }, - { - Key: awssdk.String("kubernetes.io/cluster/kube-cluster"), - Value: awssdk.String("shared"), - }, - }, - }, - }, + wantErr: errors.New("failed to list subnets by role tag: some auth error"), }, { - name: "subnets with insufficient available ip addresses get ignored", + name: "failed to fallback to reachability", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Values: []string{"vpc-dummy"}, }, { Name: awssdk.String("tag:kubernetes.io/role/elb"), @@ -1058,89 +1108,70 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, }, }, + output: nil, + }, + { + input: &ec2sdk.DescribeSubnetsInput{ + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, + }, + }, + }, output: []ec2types.Subnet{ { SubnetId: awssdk.String("subnet-1"), AvailabilityZone: awssdk.String("us-west-2a"), AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - AvailableIpAddressCount: awssdk.Int32(0), - }, - { - SubnetId: awssdk.String("subnet-3"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, { - SubnetId: awssdk.String("subnet-4"), + SubnetId: awssdk.String("subnet-2"), AvailabilityZone: awssdk.String("us-west-2b"), AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), - AvailableIpAddressCount: awssdk.Int32(25), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, { - SubnetId: awssdk.String("subnet-2"), + SubnetId: awssdk.String("subnet-3"), AvailabilityZone: awssdk.String("us-west-2a"), AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - AvailableIpAddressCount: awssdk.Int32(2), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, { - SubnetId: awssdk.String("subnet-5"), + SubnetId: awssdk.String("subnet-4"), AvailabilityZone: awssdk.String("us-west-2b"), AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), - AvailableIpAddressCount: awssdk.Int32(10), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, }, - fetchAZInfosCalls: []fetchAZInfosCall{ - { - availabilityZoneIDs: []string{"usw2-az1"}, - azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-az1": { - ZoneId: awssdk.String("usw2-az1"), - ZoneType: awssdk.String("availability-zone"), - }, - }, - }, + describeRouteTablesAsListCalls: []describeRouteTablesAsListCall{ { - availabilityZoneIDs: []string{"usw2-az2"}, - azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-az2": { - ZoneId: awssdk.String("usw2-az2"), - ZoneType: awssdk.String("availability-zone"), + input: &ec2sdk.DescribeRouteTablesInput{ + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, + }, }, }, + err: errors.New("some error"), }, }, }, args: args{ opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork), + WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), - WithSubnetsResolveAvailableIPAddressCount(minimalAvailableIPAddressCount), - }, - }, - want: []ec2types.Subnet{ - { - SubnetId: awssdk.String("subnet-3"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - AvailableIpAddressCount: awssdk.Int32(8), - }, - { - SubnetId: awssdk.String("subnet-4"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), - AvailableIpAddressCount: awssdk.Int32(25), }, }, + wantErr: errors.New("failed to list subnets by reachability: some error"), }, } @@ -1153,29 +1184,22 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { for _, call := range tt.fields.describeSubnetsAsListCalls { ec2Client.EXPECT().DescribeSubnetsAsList(gomock.Any(), call.input).Return(call.output, call.err) } - + for _, call := range tt.fields.describeRouteTablesAsListCalls { + ec2Client.EXPECT().DescribeRouteTablesAsList(gomock.Any(), call.input).Return(call.output, call.err) + } azInfoProvider := NewMockAZInfoProvider(ctrl) for _, call := range tt.fields.fetchAZInfosCalls { azInfoProvider.EXPECT().FetchAZInfos(gomock.Any(), call.availabilityZoneIDs).Return(call.azInfoByAZID, call.err) } - - r := &defaultSubnetsResolver{ - azInfoProvider: azInfoProvider, - ec2Client: ec2Client, - vpcID: tt.fields.vpcID, - clusterName: tt.fields.clusterName, - logger: logr.New(&log.NullLogSink{}), - } - + r := NewDefaultSubnetsResolver(azInfoProvider, ec2Client, "vpc-dummy", "cluster-dummy", + tt.fields.clusterTagCheckEnabled, tt.fields.albSingleSubnetEnabled, tt.fields.discoveryByReachabilityEnabled, + logr.New(&log.NullLogSink{})) got, err := r.ResolveViaDiscovery(context.Background(), tt.args.opts...) if tt.wantErr != nil { assert.EqualError(t, err, tt.wantErr.Error()) } else { assert.NoError(t, err) opts := cmp.Options{ - cmpopts.SortSlices(func(lhs *ec2types.Subnet, rhs *ec2types.Subnet) bool { - return awssdk.ToString(lhs.SubnetId) < awssdk.ToString(rhs.SubnetId) - }), cmpopts.IgnoreUnexported(ec2types.Subnet{}), cmpopts.IgnoreUnexported(ec2types.Tag{}), } @@ -1185,7 +1209,7 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { } } -func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { +func Test_defaultSubnetsResolver_ResolveViaSelector(t *testing.T) { type describeSubnetsAsListCall struct { input *ec2sdk.DescribeSubnetsInput output []ec2types.Subnet @@ -1197,14 +1221,15 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { err error } type fields struct { - vpcID string - clusterName string - describeSubnetsAsListCalls []describeSubnetsAsListCall - fetchAZInfosCalls []fetchAZInfosCall + clusterTagCheckEnabled bool + albSingleSubnetEnabled bool + discoveryByReachabilityEnabled bool + describeSubnetsAsListCalls []describeSubnetsAsListCall + fetchAZInfosCalls []fetchAZInfosCall } type args struct { - subnetNameOrIDs []string - opts []SubnetsResolveOption + selector elbv2api.SubnetSelector + opts []SubnetsResolveOption } tests := []struct { name string @@ -1214,10 +1239,11 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { wantErr error }{ { - name: "ALB with subnetID only", + name: "resolved via subnetIDs", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ @@ -1225,16 +1251,18 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, @@ -1261,143 +1289,144 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { }, }, args: args{ - subnetNameOrIDs: []string{"subnet-1", "subnet-2"}, + selector: elbv2api.SubnetSelector{ + IDs: []elbv2api.SubnetID{"subnet-1", "subnet-2"}, + }, opts: []SubnetsResolveOption{ WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), }, }, want: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, { - name: "ALB with subnet Name only", + name: "subnets specified via ID are in same AZ", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ - Filters: []ec2types.Filter{ - { - Name: awssdk.String("tag:Name"), - Values: []string{"my-name-1", "my-name-2"}, - }, - { - Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, - }, - }, + SubnetIds: []string{"subnet-1", "subnet-2"}, }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), - }, - }, - }, - }, - fetchAZInfosCalls: []fetchAZInfosCall{ - { - availabilityZoneIDs: []string{"usw2-az1"}, - azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-az1": { - ZoneId: awssdk.String("usw2-az1"), - ZoneType: awssdk.String("availability-zone"), - }, - }, - }, - { - availabilityZoneIDs: []string{"usw2-az2"}, - azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-az2": { - ZoneId: awssdk.String("usw2-az2"), - ZoneType: awssdk.String("availability-zone"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, }, }, args: args{ - subnetNameOrIDs: []string{"my-name-1", "my-name-2"}, + selector: elbv2api.SubnetSelector{ + IDs: []elbv2api.SubnetID{"subnet-1", "subnet-2"}, + }, opts: []SubnetsResolveOption{ WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), - }, - }, - want: []ec2types.Subnet{ - { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - }, - { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), }, }, + wantErr: errors.New("multiple subnets in same Availability Zone us-west-2a: [subnet-1 subnet-2]"), }, { - name: "ALB with subnetID and subnet Name", + name: "listSubnetsByIDs failed", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ - SubnetIds: []string{"subnet-1"}, - }, - output: []ec2types.Subnet{ - { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - }, + SubnetIds: []string{"subnet-1", "subnet-2"}, }, + err: errors.New("some error"), }, + }, + }, + args: args{ + selector: elbv2api.SubnetSelector{ + IDs: []elbv2api.SubnetID{"subnet-1", "subnet-2"}, + }, + opts: []SubnetsResolveOption{ + WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), + }, + }, + wantErr: errors.New("failed to list subnets by IDs: some error"), + }, + { + name: "resolved via tag selector", + fields: fields{ + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, + describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { - Name: awssdk.String("tag:Name"), - Values: []string{"my-name-2"}, + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, }, { - Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Name: awssdk.String("tag:tagA"), + Values: []string{"tagAVal1", "tagAVal2"}, }, }, }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, }, }, }, @@ -1424,148 +1453,364 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { }, }, args: args{ - subnetNameOrIDs: []string{"subnet-1", "my-name-2"}, + selector: elbv2api.SubnetSelector{ + Tags: map[string][]string{ + "tagA": {"tagAVal1", "tagAVal2"}, + }, + }, opts: []SubnetsResolveOption{ WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), }, }, want: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, }, }, }, { - name: "cannot resolve all subnet names", + name: "subnets tagged for other clusters shall be filtered out when clusterTagCheckEnabled enabled", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { - Name: awssdk.String("tag:Name"), - Values: []string{"my-name-1", "my-name-2", "my-name-3"}, + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, }, { - Name: awssdk.String("vpc-id"), - Values: []string{"vpc-1"}, + Name: awssdk.String("tag:tagA"), + Values: []string{"tagAVal1", "tagAVal2"}, }, }, }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + { + Key: awssdk.String("kubernetes.io/cluster/other-cluster"), + Value: awssdk.String("owned"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, + }, + }, + }, + }, + fetchAZInfosCalls: []fetchAZInfosCall{ + { + availabilityZoneIDs: []string{"usw2-az1"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az1": { + ZoneId: awssdk.String("usw2-az1"), + ZoneType: awssdk.String("availability-zone"), + }, + }, + }, + { + availabilityZoneIDs: []string{"usw2-az2"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az2": { + ZoneId: awssdk.String("usw2-az2"), + ZoneType: awssdk.String("availability-zone"), }, }, }, }, }, args: args{ - subnetNameOrIDs: []string{"my-name-1", "my-name-2", "my-name-3"}, + selector: elbv2api.SubnetSelector{ + Tags: map[string][]string{ + "tagA": {"tagAVal1", "tagAVal2"}, + }, + }, opts: []SubnetsResolveOption{ WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), }, }, - wantErr: errors.New("couldn't find all subnets, nameOrIDs: [my-name-1 my-name-2 my-name-3], found: 2"), - }, - { - name: "empty subnet name or IDs", - fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", - describeSubnetsAsListCalls: nil, - }, - args: args{ - subnetNameOrIDs: nil, - opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + want: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, }, }, - wantErr: errors.New("unable to resolve at least one subnet"), }, { - name: "multiple subnet in same AZ", + name: "subnets tagged for other clusters shall not be filtered out when clusterTagCheckEnabled disabled", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: false, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ - SubnetIds: []string{"subnet-1", "subnet-2", "subnet-3"}, + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, + }, + { + Name: awssdk.String("tag:tagA"), + Values: []string{"tagAVal1", "tagAVal2"}, + }, + }, }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + { + Key: awssdk.String("kubernetes.io/cluster/other-cluster"), + Value: awssdk.String("owned"), + }, + }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, }, { - SubnetId: awssdk.String("subnet-3"), - AvailabilityZone: awssdk.String("us-west-2a"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, + }, + }, + }, + }, + fetchAZInfosCalls: []fetchAZInfosCall{ + { + availabilityZoneIDs: []string{"usw2-az1"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az1": { + ZoneId: awssdk.String("usw2-az1"), + ZoneType: awssdk.String("availability-zone"), + }, + }, + }, + { + availabilityZoneIDs: []string{"usw2-az2"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az2": { + ZoneId: awssdk.String("usw2-az2"), + ZoneType: awssdk.String("availability-zone"), }, }, }, }, }, args: args{ - subnetNameOrIDs: []string{"subnet-1", "subnet-2", "subnet-3"}, + selector: elbv2api.SubnetSelector{ + Tags: map[string][]string{ + "tagA": {"tagAVal1", "tagAVal2"}, + }, + }, opts: []SubnetsResolveOption{ WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), + }, + }, + want: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + { + Key: awssdk.String("kubernetes.io/cluster/other-cluster"), + Value: awssdk.String("owned"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, }, }, - wantErr: errors.New("multiple subnets in same Availability Zone us-west-2a: [subnet-1 subnet-3]"), }, { - name: "multiple subnet locales", + name: "subnets with insufficient available ip addresses shall be filtered out", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ - SubnetIds: []string{"subnet-1", "subnet-2"}, + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, + }, + { + Name: awssdk.String("tag:tagA"), + Values: []string{"tagAVal1", "tagAVal2"}, + }, + }, }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(2), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, }, { - SubnetId: awssdk.String("subnet-2"), - AvailabilityZone: awssdk.String("us-west-2b"), - AvailabilityZoneId: awssdk.String("usw2-az2"), - VpcId: awssdk.String("vpc-1"), - OutpostArn: awssdk.String("outpost-xxx"), + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, }, }, }, @@ -1580,33 +1825,211 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { }, }, }, + { + availabilityZoneIDs: []string{"usw2-az2"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az2": { + ZoneId: awssdk.String("usw2-az2"), + ZoneType: awssdk.String("availability-zone"), + }, + }, + }, }, }, args: args{ - subnetNameOrIDs: []string{"subnet-1", "subnet-2"}, + selector: elbv2api.SubnetSelector{ + Tags: map[string][]string{ + "tagA": {"tagAVal1", "tagAVal2"}, + }, + }, opts: []SubnetsResolveOption{ WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), + }, + }, + want: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, }, }, - wantErr: errors.New("subnets in multiple locales: [availability-zone outpost]"), }, { - name: "ALB with one availability-zone subnet", + name: "subnets are either tagged for other clusters or with insufficient ip address", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ - SubnetIds: []string{"subnet-1"}, + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, + }, + { + Name: awssdk.String("tag:tagA"), + Values: []string{"tagAVal1", "tagAVal2"}, + }, + }, + }, + output: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + { + Key: awssdk.String("kubernetes.io/cluster/other-cluster"), + Value: awssdk.String("owned"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(2), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("tagA"), + Value: awssdk.String("tagAVal1"), + }, + }, + }, + }, + }, + }, + }, + args: args{ + selector: elbv2api.SubnetSelector{ + Tags: map[string][]string{ + "tagA": {"tagAVal1", "tagAVal2"}, + }, + }, + opts: []SubnetsResolveOption{ + WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), + }, + }, + wantErr: errors.New("unable to resolve at least one subnet. Evaluated 2 subnets: 1 are tagged for other clusters, and 1 have insufficient available IP addresses"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + ec2Client := services.NewMockEC2(ctrl) + for _, call := range tt.fields.describeSubnetsAsListCalls { + ec2Client.EXPECT().DescribeSubnetsAsList(gomock.Any(), call.input).Return(call.output, call.err) + } + azInfoProvider := NewMockAZInfoProvider(ctrl) + for _, call := range tt.fields.fetchAZInfosCalls { + azInfoProvider.EXPECT().FetchAZInfos(gomock.Any(), call.availabilityZoneIDs).Return(call.azInfoByAZID, call.err) + } + r := NewDefaultSubnetsResolver(azInfoProvider, ec2Client, "vpc-dummy", "cluster-dummy", + tt.fields.clusterTagCheckEnabled, tt.fields.albSingleSubnetEnabled, tt.fields.discoveryByReachabilityEnabled, + logr.New(&log.NullLogSink{})) + got, err := r.ResolveViaSelector(context.Background(), tt.args.selector, tt.args.opts...) + if tt.wantErr != nil { + assert.EqualError(t, err, tt.wantErr.Error()) + } else { + assert.NoError(t, err) + opts := cmp.Options{ + cmpopts.IgnoreUnexported(ec2types.Subnet{}), + cmpopts.IgnoreUnexported(ec2types.Tag{}), + } + assert.True(t, cmp.Equal(tt.want, got, opts), "diff", cmp.Diff(tt.want, got, opts)) + } + }) + } +} + +func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { + type describeSubnetsAsListCall struct { + input *ec2sdk.DescribeSubnetsInput + output []ec2types.Subnet + err error + } + type fetchAZInfosCall struct { + availabilityZoneIDs []string + azInfoByAZID map[string]ec2types.AvailabilityZone + err error + } + type fields struct { + clusterTagCheckEnabled bool + albSingleSubnetEnabled bool + discoveryByReachabilityEnabled bool + describeSubnetsAsListCalls []describeSubnetsAsListCall + fetchAZInfosCalls []fetchAZInfosCall + } + type args struct { + nameOrIDs []string + opts []SubnetsResolveOption + } + tests := []struct { + name string + fields fields + args args + want []ec2types.Subnet + wantErr error + }{ + { + name: "resolved via subnetIDs", + fields: fields{ + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, + describeSubnetsAsListCalls: []describeSubnetsAsListCall{ + { + input: &ec2sdk.DescribeSubnetsInput{ + SubnetIds: []string{"subnet-1", "subnet-2"}, }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, @@ -1621,70 +2044,130 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { }, }, }, + { + availabilityZoneIDs: []string{"usw2-az2"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az2": { + ZoneId: awssdk.String("usw2-az2"), + ZoneType: awssdk.String("availability-zone"), + }, + }, + }, }, }, args: args{ - subnetNameOrIDs: []string{"subnet-1"}, + nameOrIDs: []string{"subnet-1", "subnet-2"}, opts: []SubnetsResolveOption{ WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), + }, + }, + want: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, - wantErr: errors.New("subnets count less than minimal required count: 1 < 2"), }, { - name: "ALB with one local-zone subnet", + name: "resolved via subnetNames", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ - SubnetIds: []string{"subnet-1"}, + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, + }, + { + Name: awssdk.String("tag:Name"), + Values: []string{"name-1", "name-2"}, + }, + }, }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2-lax-1a"), - AvailabilityZoneId: awssdk.String("usw2-lax1-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, }, fetchAZInfosCalls: []fetchAZInfosCall{ { - availabilityZoneIDs: []string{"usw2-lax1-az1"}, + availabilityZoneIDs: []string{"usw2-az1"}, azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-lax1-az1": { - ZoneId: awssdk.String("usw2-lax1-az1"), - ZoneType: awssdk.String("local-zone"), + "usw2-az1": { + ZoneId: awssdk.String("usw2-az1"), + ZoneType: awssdk.String("availability-zone"), + }, + }, + }, + { + availabilityZoneIDs: []string{"usw2-az2"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az2": { + ZoneId: awssdk.String("usw2-az2"), + ZoneType: awssdk.String("availability-zone"), }, }, }, }, }, args: args{ - subnetNameOrIDs: []string{"subnet-1"}, + nameOrIDs: []string{"name-1", "name-2"}, opts: []SubnetsResolveOption{ WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), }, }, want: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2-lax-1a"), - AvailabilityZoneId: awssdk.String("usw2-lax1-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, { - name: "ALB with one outpost subnet", + name: "resolved via both id and name", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ @@ -1692,83 +2175,123 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - OutpostArn: awssdk.String("outpost-xxx"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + }, + }, + { + input: &ec2sdk.DescribeSubnetsInput{ + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, + }, + { + Name: awssdk.String("tag:Name"), + Values: []string{"name-2"}, + }, + }, + }, + output: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + }, + }, + }, + fetchAZInfosCalls: []fetchAZInfosCall{ + { + availabilityZoneIDs: []string{"usw2-az1"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az1": { + ZoneId: awssdk.String("usw2-az1"), + ZoneType: awssdk.String("availability-zone"), + }, + }, + }, + { + availabilityZoneIDs: []string{"usw2-az2"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az2": { + ZoneId: awssdk.String("usw2-az2"), + ZoneType: awssdk.String("availability-zone"), }, }, }, }, }, args: args{ - subnetNameOrIDs: []string{"subnet-1"}, + nameOrIDs: []string{"subnet-1", "name-2"}, opts: []SubnetsResolveOption{ WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), }, }, want: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), - OutpostArn: awssdk.String("outpost-xxx"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, { - name: "NLB with one availabilityZone subnet", + name: "subnets specified are in same AZ", fields: fields{ - vpcID: "vpc-1", - clusterName: "kube-cluster", + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, describeSubnetsAsListCalls: []describeSubnetsAsListCall{ { input: &ec2sdk.DescribeSubnetsInput{ - SubnetIds: []string{"subnet-1"}, + SubnetIds: []string{"subnet-1", "subnet-2"}, }, output: []ec2types.Subnet{ { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, - }, - }, - }, - fetchAZInfosCalls: []fetchAZInfosCall{ - { - availabilityZoneIDs: []string{"usw2-az1"}, - azInfoByAZID: map[string]ec2types.AvailabilityZone{ - "usw2-az1": { - ZoneId: awssdk.String("usw2-az1"), - ZoneType: awssdk.String("availability-zone"), + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), }, }, }, }, }, args: args{ - subnetNameOrIDs: []string{"subnet-1"}, + nameOrIDs: []string{"subnet-1", "subnet-2"}, opts: []SubnetsResolveOption{ - WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork), - WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal), - }, - }, - want: []ec2types.Subnet{ - { - SubnetId: awssdk.String("subnet-1"), - AvailabilityZone: awssdk.String("us-west-2a"), - AvailabilityZoneId: awssdk.String("usw2-az1"), - VpcId: awssdk.String("vpc-1"), + WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), }, }, + wantErr: errors.New("multiple subnets in same Availability Zone us-west-2a: [subnet-1 subnet-2]"), }, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctrl := gomock.NewController(t) @@ -1782,23 +2305,15 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { for _, call := range tt.fields.fetchAZInfosCalls { azInfoProvider.EXPECT().FetchAZInfos(gomock.Any(), call.availabilityZoneIDs).Return(call.azInfoByAZID, call.err) } - - r := &defaultSubnetsResolver{ - azInfoProvider: azInfoProvider, - ec2Client: ec2Client, - vpcID: tt.fields.vpcID, - clusterName: tt.fields.clusterName, - logger: logr.New(&log.NullLogSink{}), - } - got, err := r.ResolveViaNameOrIDSlice(context.Background(), tt.args.subnetNameOrIDs, tt.args.opts...) + r := NewDefaultSubnetsResolver(azInfoProvider, ec2Client, "vpc-dummy", "cluster-dummy", + tt.fields.clusterTagCheckEnabled, tt.fields.albSingleSubnetEnabled, tt.fields.discoveryByReachabilityEnabled, + logr.New(&log.NullLogSink{})) + got, err := r.ResolveViaNameOrIDSlice(context.Background(), tt.args.nameOrIDs, tt.args.opts...) if tt.wantErr != nil { assert.EqualError(t, err, tt.wantErr.Error()) } else { assert.NoError(t, err) opts := cmp.Options{ - cmpopts.SortSlices(func(lhs *ec2types.Subnet, rhs *ec2types.Subnet) bool { - return awssdk.ToString(lhs.SubnetId) < awssdk.ToString(rhs.SubnetId) - }), cmpopts.IgnoreUnexported(ec2types.Subnet{}), cmpopts.IgnoreUnexported(ec2types.Tag{}), } @@ -1808,6 +2323,196 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) { } } +func Test_defaultSubnetsResolver_chooseSubnetsPerAZ(t *testing.T) { + tests := []struct { + name string // description of this test case + subnets []ec2types.Subnet + want []ec2types.Subnet + }{ + { + name: "sort by lexlexicographical order of subnet-id by default", + subnets: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + }, + want: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + }, + }, + { + name: "subnets with current cluster tag gets priority", + subnets: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/cluster/cluster-dummy"), + Value: awssdk.String("owned"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + }, + want: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/cluster/cluster-dummy"), + Value: awssdk.String("owned"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := NewDefaultSubnetsResolver(nil, nil, "vpc-dummy", "cluster-dummy", true, false, true, + logr.New(&log.NullLogSink{})) + got := r.chooseSubnetsPerAZ(tt.subnets) + assert.Equal(t, tt.want, got) + }) + } +} + +func Test_defaultSubnetsResolver_computeSubnetsMinimalCount(t *testing.T) { + type fields struct { + albSingleSubnetEnabled bool + } + type args struct { + subnetLocale subnetLocaleType + resolveOpts SubnetsResolveOptions + } + tests := []struct { + name string + fields fields + args args + want int + }{ + { + name: "ALB needs 2 subnet by default", + fields: fields{ + albSingleSubnetEnabled: false, + }, + args: args{ + subnetLocale: subnetLocaleTypeAvailabilityZone, + resolveOpts: SubnetsResolveOptions{ + LBType: elbv2model.LoadBalancerTypeApplication, + }, + }, + want: 2, + }, + { + name: "ALB needs 1 subnet in localZone", + fields: fields{ + albSingleSubnetEnabled: false, + }, + args: args{ + subnetLocale: subnetLocaleTypeLocalZone, + resolveOpts: SubnetsResolveOptions{ + LBType: elbv2model.LoadBalancerTypeApplication, + }, + }, + want: 1, + }, + { + name: "ALB needs 1 subnet when albSingleSubnet enabled", + fields: fields{ + albSingleSubnetEnabled: true, + }, + args: args{ + subnetLocale: subnetLocaleTypeAvailabilityZone, + resolveOpts: SubnetsResolveOptions{ + LBType: elbv2model.LoadBalancerTypeApplication, + }, + }, + want: 1, + }, + { + name: "NLB needs 1 subnet by default", + fields: fields{ + albSingleSubnetEnabled: false, + }, + args: args{ + subnetLocale: subnetLocaleTypeAvailabilityZone, + resolveOpts: SubnetsResolveOptions{ + LBType: elbv2model.LoadBalancerTypeNetwork, + }, + }, + want: 1, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := NewDefaultSubnetsResolver(nil, nil, "vpc-dummy", "cluster-dummy", true, tt.fields.albSingleSubnetEnabled, false, + logr.New(&log.NullLogSink{})) + got := r.computeSubnetsMinimalCount(tt.args.subnetLocale, tt.args.resolveOpts) + assert.Equal(t, tt.want, got) + }) + } +} + func Test_defaultSubnetsResolver_buildSDKSubnetLocaleType(t *testing.T) { type fetchAZInfosCall struct { availabilityZoneIDs []string @@ -2058,3 +2763,36 @@ func Test_sortSubnetsByID(t *testing.T) { }) } } + +func Test_extractSubnetIDs(t *testing.T) { + tests := []struct { + name string // description of this test case + // Named input parameters for target function. + subnets []ec2types.Subnet + want []string + }{ + { + name: "0 subnets", + subnets: nil, + want: []string{}, + }, + { + name: "multiple subnets", + subnets: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-1"), + }, + { + SubnetId: awssdk.String("subnet-2"), + }, + }, + want: []string{"subnet-1", "subnet-2"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := extractSubnetIDs(tt.subnets) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/service/model_build_load_balancer.go b/pkg/service/model_build_load_balancer.go index 8d67a72c63..c97a9a8d09 100644 --- a/pkg/service/model_build_load_balancer.go +++ b/pkg/service/model_build_load_balancer.go @@ -35,7 +35,6 @@ const ( partialAvailabilityZoneAffinity = "partial_availability_zone_affinity" anyAvailabilityZone = "any_availability_zone" resourceIDLoadBalancer = "LoadBalancer" - minimalAvailableIPAddressCount = int32(8) ) func (t *defaultModelBuildTask) buildLoadBalancer(ctx context.Context, scheme elbv2model.LoadBalancerScheme) error { @@ -465,14 +464,11 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnets(ctx context.Context, sc return t.subnetsResolver.ResolveViaDiscovery(ctx, networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork), networking.WithSubnetsResolveLBScheme(scheme), - networking.WithSubnetsResolveAvailableIPAddressCount(minimalAvailableIPAddressCount), - networking.WithSubnetsClusterTagCheck(t.featureGates.Enabled(config.SubnetsClusterTagCheck)), ) } return t.subnetsResolver.ResolveViaDiscovery(ctx, networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork), networking.WithSubnetsResolveLBScheme(scheme), - networking.WithSubnetsClusterTagCheck(t.featureGates.Enabled(config.SubnetsClusterTagCheck)), ) } From c183219b9370316d02116a8795106d2ff884750b Mon Sep 17 00:00:00 2001 From: Yang Yang Date: Fri, 28 Mar 2025 13:28:00 -0700 Subject: [PATCH 2/2] address comments --- docs/deploy/subnet_discovery.md | 2 +- pkg/networking/subnet_resolver.go | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/deploy/subnet_discovery.md b/docs/deploy/subnet_discovery.md index 8fe4f7470a..9a1646224c 100644 --- a/docs/deploy/subnet_discovery.md +++ b/docs/deploy/subnet_discovery.md @@ -68,4 +68,4 @@ The controller selects one subnet per availability zone. When multiple subnets e * **ALBs**: Require at least two subnets across different Availability Zones by default !!! tip - For customers whitelished by the AWS Elastic Load Balancing team, you can enable the [ALBSingleSubnet feature gate](https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/deploy/configurations/#feature-gates). This allows provisioning an ALB with just one subnet instead of the standard requirement of two subnets. \ No newline at end of file + For customers allowlisted by the AWS Elastic Load Balancing team, you can enable the [ALBSingleSubnet feature gate](https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/deploy/configurations/#feature-gates). This allows provisioning an ALB with just one subnet instead of the standard requirement of two subnets. \ No newline at end of file diff --git a/pkg/networking/subnet_resolver.go b/pkg/networking/subnet_resolver.go index a3086d0a43..3ea34846f0 100644 --- a/pkg/networking/subnet_resolver.go +++ b/pkg/networking/subnet_resolver.go @@ -41,6 +41,8 @@ const ( const ( // both ALB & NLB requires minimal 8 ip address count for it's subnet defaultMinimalAvailableIPAddressCount = 8 + // the ec2's vpcID filter + ec2FilterNameVpcID = "vpc-id" ) // options for resolve subnets. @@ -147,7 +149,7 @@ type defaultSubnetsResolver struct { // - The subnet has a Kubernetes cluster tag matching the current cluster clusterTagCheckEnabled bool // whether to enable a single subnet as ALB subnet - // by default ALB requires two subent, only whitelisted users can use a single subnet + // by default ALB requires two subent, only allowlisted users can use a single subnet albSingleSubnetEnabled bool // whether to enable discovery subnet by reachability(public/private) discoverByReachabilityEnabled bool @@ -281,7 +283,7 @@ func (r *defaultSubnetsResolver) listSubnetsByNames(ctx context.Context, subnetN req := &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { - Name: awssdk.String("vpc-id"), + Name: awssdk.String(ec2FilterNameVpcID), Values: []string{r.vpcID}, }, { @@ -305,7 +307,7 @@ func (r *defaultSubnetsResolver) listSubnetsByTagFilters(ctx context.Context, ta req := &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { - Name: awssdk.String("vpc-id"), + Name: awssdk.String(ec2FilterNameVpcID), Values: []string{r.vpcID}, }, }, @@ -324,7 +326,7 @@ func (r *defaultSubnetsResolver) listSubnetsByReachability(ctx context.Context, subnetsReq := &ec2sdk.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { - Name: awssdk.String("vpc-id"), + Name: awssdk.String(ec2FilterNameVpcID), Values: []string{r.vpcID}, }, }, @@ -336,7 +338,7 @@ func (r *defaultSubnetsResolver) listSubnetsByReachability(ctx context.Context, routeTablesReq := &ec2sdk.DescribeRouteTablesInput{ Filters: []ec2types.Filter{ { - Name: awssdk.String("vpc-id"), + Name: awssdk.String(ec2FilterNameVpcID), Values: []string{r.vpcID}, }, },