diff --git a/integration/tests/managed/managed_nodegroup_test.go b/integration/tests/managed/managed_nodegroup_test.go index a65a6281941..00a498f3b56 100644 --- a/integration/tests/managed/managed_nodegroup_test.go +++ b/integration/tests/managed/managed_nodegroup_test.go @@ -11,9 +11,10 @@ import ( "time" "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/aws-sdk-go-v2/service/ec2" + awsec2 "github.com/aws/aws-sdk-go-v2/service/ec2" ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" awseks "github.com/aws/aws-sdk-go-v2/service/eks" + ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -76,6 +77,7 @@ var _ = Describe("(Integration) Create Managed Nodegroups", func() { ubuntuNodegroup = "ng-ubuntu" publicNodeGroup = "ng-public" privateNodeGroup = "ng-private" + removedSubnetNodeGroup = "ng-removed-subnet" ) var ( @@ -488,14 +490,16 @@ var _ = Describe("(Integration) Create Managed Nodegroups", func() { }) }) - Context("eksctl utils update-cluster-vpc-config", Serial, func() { - makeAWSProvider := func(ctx context.Context, clusterConfig *api.ClusterConfig) api.ClusterProvider { - clusterProvider, err := eks.New(ctx, &api.ProviderConfig{Region: params.Region}, clusterConfig) - Expect(err).NotTo(HaveOccurred()) - return clusterProvider.AWSProvider - } - getPrivateSubnetIDs := func(ctx context.Context, ec2API awsapi.EC2, vpcID string) []string { - out, err := ec2API.DescribeSubnets(ctx, &ec2.DescribeSubnetsInput{ + Context("eksctl utils update-cluster-vpc-config", Serial, Ordered, func() { + var ( + cluster *ekstypes.Cluster + clusterConfig *api.ClusterConfig + awsProvider api.ClusterProvider + publicSubnetIDs, privateSubnetIDs []string + ) + + getVPCSubnetIDs := func(ctx context.Context, ec2API awsapi.EC2, vpcID string) (publicIDs, privateIDs []string) { + out, err := ec2API.DescribeSubnets(ctx, &awsec2.DescribeSubnetsInput{ Filters: []ec2types.Filter{ { Name: aws.String("vpc-id"), @@ -504,33 +508,46 @@ var _ = Describe("(Integration) Create Managed Nodegroups", func() { }, }) Expect(err).NotTo(HaveOccurred()) - var subnetIDs []string for _, s := range out.Subnets { - if !*s.MapPublicIpOnLaunch { - subnetIDs = append(subnetIDs, *s.SubnetId) + if *s.MapPublicIpOnLaunch { + publicIDs = append(publicIDs, *s.SubnetId) + continue } + privateIDs = append(privateIDs, *s.SubnetId) } - return subnetIDs + return publicIDs, privateIDs } - It("should update the VPC config", func() { - clusterConfig := makeClusterConfig() + + BeforeAll(func() { ctx := context.Background() - awsProvider := makeAWSProvider(ctx, clusterConfig) - cluster, err := awsProvider.EKS().DescribeCluster(ctx, &awseks.DescribeClusterInput{ + clusterConfig = makeClusterConfig() + + // initialize AWS provider + clusterProvider, err := eks.New(ctx, &api.ProviderConfig{Region: params.Region}, clusterConfig) + Expect(err).NotTo(HaveOccurred()) + awsProvider = clusterProvider.AWSProvider + + // get cluster subnets + output, err := awsProvider.EKS().DescribeCluster(ctx, &awseks.DescribeClusterInput{ Name: aws.String(params.ClusterName), }) Expect(err).NotTo(HaveOccurred(), "error describing cluster") - clusterSubnetIDs := getPrivateSubnetIDs(ctx, awsProvider.EC2(), *cluster.Cluster.ResourcesVpcConfig.VpcId) - Expect(len(cluster.Cluster.ResourcesVpcConfig.SecurityGroupIds) > 0).To(BeTrue(), "at least one security group ID must be associated with the cluster") + cluster = output.Cluster + publicSubnetIDs, privateSubnetIDs = getVPCSubnetIDs(ctx, awsProvider.EC2(), *cluster.ResourcesVpcConfig.VpcId) + // enforce security group condition + Expect(len(cluster.ResourcesVpcConfig.SecurityGroupIds) > 0).To(BeTrue(), "at least one security group ID must be associated with the cluster") + }) + + It("should update the VPC config", func() { clusterVPC := &api.ClusterVPC{ ClusterEndpoints: &api.ClusterEndpoints{ PrivateAccess: api.Enabled(), PublicAccess: api.Enabled(), }, PublicAccessCIDRs: []string{"127.0.0.1/32"}, - ControlPlaneSubnetIDs: clusterSubnetIDs, - ControlPlaneSecurityGroupIDs: []string{cluster.Cluster.ResourcesVpcConfig.SecurityGroupIds[0]}, + ControlPlaneSubnetIDs: publicSubnetIDs, + ControlPlaneSecurityGroupIDs: []string{cluster.ResourcesVpcConfig.SecurityGroupIds[0]}, } By("accepting CLI options") cmd := params.EksctlUtilsCmd.WithArgs( @@ -587,6 +604,32 @@ var _ = Describe("(Integration) Create Managed Nodegroups", func() { ) Expect(cmd).To(RunSuccessfully()) }) + + It("should support creating nodegroups after deleting a removed control plane subnet", func() { + _, err := awsProvider.EC2().DeleteSubnet(context.Background(), &awsec2.DeleteSubnetInput{SubnetId: &privateSubnetIDs[0]}) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should fail early if all private subnets were deleted and privateNetworking is set to true", func() { + clusterConfig.ManagedNodeGroups = append(clusterConfig.ManagedNodeGroups, &api.ManagedNodeGroup{ + NodeGroupBase: &api.NodeGroupBase{ + Name: removedSubnetNodeGroup, + PrivateNetworking: true, + }, + }) + cmd := params.EksctlCreateCmd.WithArgs( + "nodegroup", + "--config-file", "-", + ). + WithoutArg("--region", params.Region). + WithStdin(clusterutils.Reader(clusterConfig)) + session := cmd.Run() + Expect(session.ExitCode()).To(Equal(1)) + Expect(strings.Split(string(session.Buffer().Contents()), "\n")).To(ContainElements( + ContainSubstring(fmt.Sprintf("%s was found on cluster cloudformation stack, but has been removed from %s outside of eksctl", privateSubnetIDs[0], *cluster.ResourcesVpcConfig.VpcId)), + ContainSubstring(fmt.Sprintf("cannot have privateNetworking:true for nodegroup %s, since no private subnets were found within %s", removedSubnetNodeGroup, *cluster.ResourcesVpcConfig.VpcId)), + )) + }) }) }) diff --git a/pkg/actions/nodegroup/create.go b/pkg/actions/nodegroup/create.go index 136c27f6281..7dd89ea71a5 100644 --- a/pkg/actions/nodegroup/create.go +++ b/pkg/actions/nodegroup/create.go @@ -135,6 +135,10 @@ func (m *Manager) Create(ctx context.Context, options CreateOpts, nodegroupFilte } } + if err := validateSubnetsAvailability(cfg); err != nil { + return err + } + if err := vpc.ValidateLegacySubnetsForNodeGroups(ctx, cfg, ctl.AWSProvider); err != nil { return err } @@ -404,3 +408,52 @@ func validateSecurityGroup(ctx context.Context, ec2API awsapi.EC2, securityGroup } return hasDefaultEgressRule, nil } + +func validateSubnetsAvailability(spec *api.ClusterConfig) error { + validateSubnetsAvailabilityForNg := func(np api.NodePool) error { + ng := np.BaseNodeGroup() + subnetTypeForPrivateNetworking := map[bool]string{ + true: "private", + false: "public", + } + unavailableSubnetsErr := func(subnetLocation string) error { + return fmt.Errorf("cannot have privateNetworking:%t for nodegroup %s, since no %s subnets were found within %s", + ng.PrivateNetworking, ng.Name, subnetTypeForPrivateNetworking[ng.PrivateNetworking], subnetLocation) + } + + // don't check private networking compatibility for: + // self-managed nodegroups on local zones + if nodeGroup, ok := np.(*api.NodeGroup); (ok && len(nodeGroup.LocalZones) > 0) || + // nodegroups on outposts + (ng.OutpostARN != "" || spec.IsControlPlaneOnOutposts()) || + // nodegroups on user specified subnets + len(ng.Subnets) > 0 { + return nil + } + shouldCheckAcrossAllAZs := true + for _, az := range ng.AvailabilityZones { + shouldCheckAcrossAllAZs = false + if _, ok := spec.VPC.Subnets.Private[az]; !ok && ng.PrivateNetworking { + return unavailableSubnetsErr(az) + } + if _, ok := spec.VPC.Subnets.Public[az]; !ok && !ng.PrivateNetworking { + return unavailableSubnetsErr(az) + } + } + if shouldCheckAcrossAllAZs { + if ng.PrivateNetworking && len(spec.VPC.Subnets.Private) == 0 { + return unavailableSubnetsErr(spec.VPC.ID) + } + if !ng.PrivateNetworking && len(spec.VPC.Subnets.Public) == 0 { + return unavailableSubnetsErr(spec.VPC.ID) + } + } + return nil + } + for _, np := range nodes.ToNodePools(spec) { + if err := validateSubnetsAvailabilityForNg(np); err != nil { + return err + } + } + return nil +} diff --git a/pkg/actions/nodegroup/create_test.go b/pkg/actions/nodegroup/create_test.go index 19765ae44cc..4454124a783 100644 --- a/pkg/actions/nodegroup/create_test.go +++ b/pkg/actions/nodegroup/create_test.go @@ -62,6 +62,11 @@ type expectedCalls struct { clientset *fake.Clientset } +type vpcSubnets struct { + publicIDs []string + privateIDs []string +} + //counterfeiter:generate -o fakes/fake_nodegroup_task_creator.go . nodeGroupTaskCreator type nodeGroupTaskCreator interface { NewUnmanagedNodeGroupTask(context.Context, []*api.NodeGroup, bool, bool, bool, vpc.Importer) *tasks.TaskTree @@ -253,6 +258,51 @@ var _ = DescribeTable("Create", func(t ngEntry) { }, expectedErr: errors.Wrap(errors.New("shared node security group missing, to fix this run 'eksctl update cluster --name=my-cluster --region='"), "cluster compatibility check failed")}), + Entry("fails when nodegroup uses privateNetworking:true and there's no private subnet within vpc", ngEntry{ + updateClusterConfig: func(c *api.ClusterConfig) { + c.NodeGroups[0].PrivateNetworking = true + }, + mockCalls: func(m mockCalls) { + mockProviderWithVPCSubnets(m.mockProvider, &vpcSubnets{ + publicIDs: []string{"subnet-public-1", "subnet-public-2"}, + }) + }, + expectedErr: fmt.Errorf("cannot have privateNetworking:true for nodegroup my-ng, since no private subnets were found within vpc-1"), + }), + Entry("fails when nodegroup uses privateNetworking:false and there's no public subnet within vpc", ngEntry{ + mockCalls: func(m mockCalls) { + mockProviderWithVPCSubnets(m.mockProvider, &vpcSubnets{ + publicIDs: []string{"subnet-private-1", "subnet-private-2"}, + }) + }, + expectedErr: fmt.Errorf("cannot have privateNetworking:false for nodegroup my-ng, since no public subnets were found within vpc-1"), + }), + Entry("fails when nodegroup uses privateNetworking:true and there's no private subnet within az", ngEntry{ + updateClusterConfig: func(c *api.ClusterConfig) { + c.NodeGroups[0].PrivateNetworking = true + c.NodeGroups[0].AvailabilityZones = []string{"us-west-2b"} + }, + mockCalls: func(m mockCalls) { + mockProviderWithVPCSubnets(m.mockProvider, &vpcSubnets{ + publicIDs: []string{"subnet-public-1", "subnet-public-2"}, + privateIDs: []string{"subnet-private-1"}, + }) + }, + expectedErr: fmt.Errorf("cannot have privateNetworking:true for nodegroup my-ng, since no private subnets were found within us-west-2b"), + }), + Entry("fails when nodegroup uses privateNetworking:false and there's no private subnet within az", ngEntry{ + updateClusterConfig: func(c *api.ClusterConfig) { + c.NodeGroups[0].AvailabilityZones = []string{"us-west-2a", "us-west-2b"} + }, + mockCalls: func(m mockCalls) { + mockProviderWithVPCSubnets(m.mockProvider, &vpcSubnets{ + publicIDs: []string{"subnet-public-2"}, + privateIDs: []string{"subnet-private-1", "subnet-private-2"}, + }) + }, + expectedErr: fmt.Errorf("cannot have privateNetworking:false for nodegroup my-ng, since no public subnets were found within us-west-2a"), + }), + Entry("fails when existing local ng stacks in config file is not listed", ngEntry{ mockCalls: func(m mockCalls) { m.nodeGroupFilter.SetOnlyLocalReturns(errors.New("err")) @@ -435,7 +485,7 @@ var _ = DescribeTable("Create", func(t ngEntry) { m.kubeProvider.NewRawClientReturns(nil, &kubernetes.APIServerUnreachableError{ Err: errors.New("timeout"), }) - mockProviderWithConfig(m.mockProvider, defaultOutput, &ekstypes.VpcConfigResponse{ + mockProviderWithConfig(m.mockProvider, defaultOutput, nil, &ekstypes.VpcConfigResponse{ ClusterSecurityGroupId: aws.String("csg-1234"), EndpointPublicAccess: false, EndpointPrivateAccess: true, @@ -499,7 +549,7 @@ var _ = DescribeTable("Create", func(t ngEntry) { UpdateAuthConfigMap: api.Disabled(), }, mockCalls: func(m mockCalls) { - mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{ + mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, nil, &ekstypes.AccessConfigResponse{ AuthenticationMode: ekstypes.AuthenticationModeApi, }) defaultProviderMocks(m.mockProvider, defaultOutput) @@ -519,7 +569,7 @@ var _ = DescribeTable("Create", func(t ngEntry) { UpdateAuthConfigMap: api.Enabled(), }, mockCalls: func(m mockCalls) { - mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{ + mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, nil, &ekstypes.AccessConfigResponse{ AuthenticationMode: ekstypes.AuthenticationModeApi, }) defaultProviderMocks(m.mockProvider, defaultOutput) @@ -536,7 +586,7 @@ var _ = DescribeTable("Create", func(t ngEntry) { Entry("creates nodegroup using access entries when authenticationMode is API_AND_CONFIG_MAP and updateAuthConfigMap is not supplied", ngEntry{ mockCalls: func(m mockCalls) { - mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{ + mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, nil, &ekstypes.AccessConfigResponse{ AuthenticationMode: ekstypes.AuthenticationModeApiAndConfigMap, }) defaultProviderMocks(m.mockProvider, defaultOutput) @@ -553,7 +603,7 @@ var _ = DescribeTable("Create", func(t ngEntry) { Entry("creates nodegroup using aws-auth ConfigMap when authenticationMode is CONFIG_MAP and updateAuthConfigMap is true", ngEntry{ mockCalls: func(m mockCalls) { - mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{ + mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, nil, &ekstypes.AccessConfigResponse{ AuthenticationMode: ekstypes.AuthenticationModeConfigMap, }) defaultProviderMocks(m.mockProvider, defaultOutput) @@ -567,7 +617,7 @@ var _ = DescribeTable("Create", func(t ngEntry) { Entry("creates nodegroup using aws-auth ConfigMap when authenticationMode is CONFIG_MAP and updateAuthConfigMap is not supplied", ngEntry{ mockCalls: func(m mockCalls) { - mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{ + mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, nil, &ekstypes.AccessConfigResponse{ AuthenticationMode: ekstypes.AuthenticationModeConfigMap, }) defaultProviderMocks(m.mockProvider, defaultOutput) @@ -581,7 +631,7 @@ var _ = DescribeTable("Create", func(t ngEntry) { Entry("creates nodegroup but does not use either aws-auth ConfigMap or access entries when authenticationMode is API_AND_CONFIG_MAP and updateAuthConfigMap is false", ngEntry{ mockCalls: func(m mockCalls) { - mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{ + mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, nil, &ekstypes.AccessConfigResponse{ AuthenticationMode: ekstypes.AuthenticationModeApiAndConfigMap, }) defaultProviderMocks(m.mockProvider, defaultOutput) @@ -602,7 +652,7 @@ var _ = DescribeTable("Create", func(t ngEntry) { Entry("creates nodegroup but does not use either aws-auth ConfigMap or access entries when authenticationMode is CONFIG_MAP and updateAuthConfigMap is false", ngEntry{ mockCalls: func(m mockCalls) { - mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{ + mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, nil, &ekstypes.AccessConfigResponse{ AuthenticationMode: ekstypes.AuthenticationModeConfigMap, }) defaultProviderMocks(m.mockProvider, defaultOutput) @@ -623,7 +673,7 @@ var _ = DescribeTable("Create", func(t ngEntry) { Entry("authorizes nodegroups using aws-auth ConfigMap when authenticationMode is API_AND_CONFIG_MAP and updateAuthConfigMap is true", ngEntry{ mockCalls: func(m mockCalls) { - mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{ + mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, nil, &ekstypes.AccessConfigResponse{ AuthenticationMode: ekstypes.AuthenticationModeApiAndConfigMap, }) defaultProviderMocks(m.mockProvider, defaultOutput) @@ -734,6 +784,14 @@ var defaultOutput = []cftypes.Output{ OutputKey: aws.String("SharedNodeSecurityGroup"), OutputValue: aws.String("sg-1"), }, + { + OutputKey: aws.String("SubnetsPublic"), + OutputValue: aws.String("subnet-public-1,subnet-public-2"), + }, + { + OutputKey: aws.String("SubnetsPrivate"), + OutputValue: aws.String("subnet-private-1,subnet-private-2"), + }, } func getIAMIdentities(clientset kubernetes.Interface) []iam.Identity { @@ -766,14 +824,18 @@ func expectedCallsForAWSAuth(e expectedCalls) { } func defaultProviderMocks(p *mockprovider.MockProvider, output []cftypes.Output) { - mockProviderWithConfig(p, output, nil, nil, nil) + mockProviderWithConfig(p, output, nil, nil, nil, nil) } func mockProviderWithOutpostConfig(p *mockprovider.MockProvider, describeStacksOutput []cftypes.Output, outpostConfig *ekstypes.OutpostConfigResponse) { - mockProviderWithConfig(p, describeStacksOutput, nil, outpostConfig, nil) + mockProviderWithConfig(p, describeStacksOutput, nil, nil, outpostConfig, nil) +} + +func mockProviderWithVPCSubnets(p *mockprovider.MockProvider, subnets *vpcSubnets) { + mockProviderWithConfig(p, defaultOutput, subnets, nil, nil, nil) } -func mockProviderWithConfig(p *mockprovider.MockProvider, describeStacksOutput []cftypes.Output, vpcConfigRes *ekstypes.VpcConfigResponse, outpostConfig *ekstypes.OutpostConfigResponse, accessConfig *ekstypes.AccessConfigResponse) { +func mockProviderWithConfig(p *mockprovider.MockProvider, describeStacksOutput []cftypes.Output, subnets *vpcSubnets, vpcConfigRes *ekstypes.VpcConfigResponse, outpostConfig *ekstypes.OutpostConfigResponse, accessConfig *ekstypes.AccessConfigResponse) { p.MockCloudFormation().On("ListStacks", mock.Anything, mock.Anything).Return(&cloudformation.ListStacksOutput{ StackSummaries: []cftypes.StackSummary{ { @@ -854,6 +916,88 @@ func mockProviderWithConfig(p *mockprovider.MockProvider, describeStacksOutput [ }, }, }, nil) + + if subnets == nil { + subnets = &vpcSubnets{ + publicIDs: []string{"subnet-public-1", "subnet-public-2"}, + privateIDs: []string{"subnet-private-1", "subnet-private-2"}, + } + } + + subnetPublic1 := ec2types.Subnet{ + SubnetId: aws.String("subnet-public-1"), + CidrBlock: aws.String("192.168.64.0/20"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + MapPublicIpOnLaunch: aws.Bool(true), + } + subnetPrivate1 := ec2types.Subnet{ + SubnetId: aws.String("subnet-private-1"), + CidrBlock: aws.String("192.168.128.0/20"), + AvailabilityZone: aws.String("us-west-2a"), + VpcId: aws.String("vpc-1"), + MapPublicIpOnLaunch: aws.Bool(false), + } + subnetPublic2 := ec2types.Subnet{ + SubnetId: aws.String("subnet-public-2"), + CidrBlock: aws.String("192.168.80.0/20"), + AvailabilityZone: aws.String("us-west-2b"), + VpcId: aws.String("vpc-1"), + MapPublicIpOnLaunch: aws.Bool(true), + } + subnetPrivate2 := ec2types.Subnet{ + SubnetId: aws.String("subnet-private-2"), + CidrBlock: aws.String("192.168.32.0/20"), + AvailabilityZone: aws.String("us-west-2b"), + VpcId: aws.String("vpc-1"), + MapPublicIpOnLaunch: aws.Bool(false), + } + + subnetsForID := map[string]ec2types.Subnet{ + "subnet-public-1": subnetPublic1, + "subnet-private-1": subnetPrivate1, + "subnet-public-2": subnetPublic2, + "subnet-private-2": subnetPrivate2, + } + + mockDescribeSubnets := func(mp *mockprovider.MockProvider, vpcID string, subnetIDs []string) { + var subnets []ec2types.Subnet + for _, id := range subnetIDs { + subnets = append(subnets, subnetsForID[id]) + } + if vpcID == "" { + mp.MockEC2().On("DescribeSubnets", mock.Anything, &ec2.DescribeSubnetsInput{ + SubnetIds: subnetIDs, + }).Return(&ec2.DescribeSubnetsOutput{Subnets: subnets}, nil) + return + } + mp.MockEC2().On("DescribeSubnets", mock.Anything, &ec2.DescribeSubnetsInput{ + Filters: []ec2types.Filter{ + { + Name: aws.String("vpc-id"), + Values: []string{vpcID}, + }, + }, + }).Return(&ec2.DescribeSubnetsOutput{Subnets: subnets}, nil) + } + + mockDescribeSubnets(p, "", subnets.publicIDs) + mockDescribeSubnets(p, "", subnets.privateIDs) + mockDescribeSubnets(p, "vpc-1", append(subnets.publicIDs, subnets.privateIDs...)) + + p.MockEC2().On("DescribeVpcs", mock.Anything, mock.Anything).Return(&ec2.DescribeVpcsOutput{ + Vpcs: []ec2types.Vpc{ + { + CidrBlock: aws.String("192.168.0.0/20"), + VpcId: aws.String("vpc-1"), + CidrBlockAssociationSet: []ec2types.VpcCidrBlockAssociation{ + { + CidrBlock: aws.String("192.168.0.0/20"), + }, + }, + }, + }, + }, nil) } func mockProviderForUnownedCluster(p *mockprovider.MockProvider, k *eksfakes.FakeKubeProvider, extraSGRules ...ec2types.SecurityGroupRule) { diff --git a/pkg/cfn/outputs/api.go b/pkg/cfn/outputs/api.go index d6fad41a008..d667427e81d 100644 --- a/pkg/cfn/outputs/api.go +++ b/pkg/cfn/outputs/api.go @@ -108,10 +108,10 @@ func Exists(stack types.Stack, key string) bool { // Collect the outputs of a stack using required and optional CollectorSets func Collect(stack types.Stack, required, optional map[string]Collector) error { - if err := NewCollectorSet(optional).doCollect(false, stack); err != nil { + if err := NewCollectorSet(required).doCollect(true, stack); err != nil { return err } - return NewCollectorSet(required).doCollect(true, stack) + return NewCollectorSet(optional).doCollect(false, stack) } // MustCollect will error if any of the outputs are missing diff --git a/pkg/vpc/vpc.go b/pkg/vpc/vpc.go index 5c015cac3b7..5a9784082d1 100644 --- a/pkg/vpc/vpc.go +++ b/pkg/vpc/vpc.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "fmt" "net" + "slices" "strings" "github.com/aws/aws-sdk-go-v2/aws" @@ -300,7 +301,42 @@ func UseFromClusterStack(ctx context.Context, provider api.ClusterProvider, stac return strings.Split(v, ",") } importSubnetsFromIDList := func(subnetMapping api.AZSubnetMapping, value string) error { - return ImportSubnetsFromIDList(ctx, provider.EC2(), spec, subnetMapping, splitOutputValue(value)) + var ( + vpcSubnets []string + stackSubnets []string + toBeImported []string + ) + // collect VPC subnets as returned by CFN stack outputs + stackSubnets = splitOutputValue(value) + + // collect VPC subnets as returned by EC2 API + ec2API := provider.EC2() + output, err := ec2API.DescribeSubnets(ctx, &ec2.DescribeSubnetsInput{ + Filters: []ec2types.Filter{ + { + Name: aws.String("vpc-id"), + Values: []string{spec.VPC.ID}, + }, + }, + }) + if err != nil { + return err + } + for _, o := range output.Subnets { + vpcSubnets = append(vpcSubnets, *o.SubnetId) + } + + // if a subnet is present on the stack outputs, but actually missing from VPC + // e.g. it was manually deleted by the user using AWS CLI/Console + // than log a warning and don't import it into cluster spec + for _, ssID := range stackSubnets { + if !slices.Contains(vpcSubnets, ssID) { + logger.Warning("%s was found on cluster cloudformation stack, but has been removed from %s outside of eksctl", ssID, spec.VPC.ID) + continue + } + toBeImported = append(toBeImported, ssID) + } + return ImportSubnetsFromIDList(ctx, provider.EC2(), spec, subnetMapping, toBeImported) } optionalCollectors := map[string]outputs.Collector{ diff --git a/pkg/vpc/vpc_test.go b/pkg/vpc/vpc_test.go index 63970f803b2..e4090852e41 100644 --- a/pkg/vpc/vpc_test.go +++ b/pkg/vpc/vpc_test.go @@ -500,6 +500,15 @@ var _ = Describe("VPC", func() { mockEC2: func(ec2Mock *mocksv2.EC2) { ec2Mock.On("DescribeSubnets", Anything, Anything).Return(func(_ context.Context, input *ec2.DescribeSubnetsInput, _ ...func(options *ec2.Options)) *ec2.DescribeSubnetsOutput { + if len(input.Filters) > 0 { + return &ec2.DescribeSubnetsOutput{ + Subnets: []ec2types.Subnet{ + { + SubnetId: aws.String("subnet-1"), + }, + }, + } + } return &ec2.DescribeSubnetsOutput{ Subnets: []ec2types.Subnet{ { @@ -582,6 +591,18 @@ var _ = Describe("VPC", func() { mockEC2: func(ec2Mock *mocksv2.EC2) { ec2Mock.On("DescribeSubnets", Anything, Anything).Return(func(_ context.Context, input *ec2.DescribeSubnetsInput, _ ...func(options *ec2.Options)) *ec2.DescribeSubnetsOutput { + if len(input.Filters) > 0 { + return &ec2.DescribeSubnetsOutput{ + Subnets: []ec2types.Subnet{ + { + SubnetId: aws.String("subnet-1"), + }, + { + SubnetId: aws.String("subnet-lz1"), + }, + }, + } + } subnet := ec2types.Subnet{ SubnetId: aws.String(input.SubnetIds[0]), VpcId: aws.String("vpc-123"), @@ -653,6 +674,18 @@ var _ = Describe("VPC", func() { }, mockEC2: func(ec2Mock *mocksv2.EC2) { ec2Mock.On("DescribeSubnets", Anything, Anything).Return(func(_ context.Context, input *ec2.DescribeSubnetsInput, _ ...func(options *ec2.Options)) *ec2.DescribeSubnetsOutput { + if len(input.Filters) > 0 { + return &ec2.DescribeSubnetsOutput{ + Subnets: []ec2types.Subnet{ + { + SubnetId: aws.String("subnet-public-1"), + }, + { + SubnetId: aws.String("subnet-private-1"), + }, + }, + } + } subnet := ec2types.Subnet{ SubnetId: aws.String(input.SubnetIds[0]), VpcId: aws.String("vpc-123"),