From 3248794ad325f9237956eed09b8e72a8c2942cc1 Mon Sep 17 00:00:00 2001
From: Tibi <110664232+TiberiuGC@users.noreply.github.com>
Date: Wed, 5 Jun 2024 12:36:25 +0300
Subject: [PATCH] Subnets availability validation should use AZs resolved by
 EC2::DescribeSubnets call

---
 pkg/actions/nodegroup/create.go      | 20 ++++++++++++++++----
 pkg/actions/nodegroup/create_test.go | 18 ++++++++++++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/pkg/actions/nodegroup/create.go b/pkg/actions/nodegroup/create.go
index b11f3bab8e..d9e0a455ce 100644
--- a/pkg/actions/nodegroup/create.go
+++ b/pkg/actions/nodegroup/create.go
@@ -410,6 +410,16 @@ func validateSecurityGroup(ctx context.Context, ec2API awsapi.EC2, securityGroup
 }
 
 func validateSubnetsAvailability(spec *api.ClusterConfig) error {
+	getAZs := func(subnetMapping api.AZSubnetMapping) map[string]struct{} {
+		azs := make(map[string]struct{})
+		for _, subnet := range subnetMapping {
+			azs[subnet.AZ] = struct{}{}
+		}
+		return azs
+	}
+	privateAZs := getAZs(spec.VPC.Subnets.Private)
+	publicAZs := getAZs(spec.VPC.Subnets.Public)
+
 	validateSubnetsAvailabilityForNg := func(np api.NodePool) error {
 		ng := np.BaseNodeGroup()
 		subnetTypeForPrivateNetworking := map[bool]string{
@@ -433,27 +443,29 @@ func validateSubnetsAvailability(spec *api.ClusterConfig) error {
 		shouldCheckAcrossAllAZs := true
 		for _, az := range ng.AvailabilityZones {
 			shouldCheckAcrossAllAZs = false
-			if _, ok := spec.VPC.Subnets.Private[az]; !ok && ng.PrivateNetworking {
+			if _, ok := privateAZs[az]; !ok && ng.PrivateNetworking {
 				return unavailableSubnetsErr(az)
 			}
-			if _, ok := spec.VPC.Subnets.Public[az]; !ok && !ng.PrivateNetworking {
+			if _, ok := publicAZs[az]; !ok && !ng.PrivateNetworking {
 				return unavailableSubnetsErr(az)
 			}
 		}
 		if shouldCheckAcrossAllAZs {
-			if ng.PrivateNetworking && len(spec.VPC.Subnets.Private) == 0 {
+			if ng.PrivateNetworking && len(privateAZs) == 0 {
 				return unavailableSubnetsErr(spec.VPC.ID)
 			}
-			if !ng.PrivateNetworking && len(spec.VPC.Subnets.Public) == 0 {
+			if !ng.PrivateNetworking && len(publicAZs) == 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 f734b0738a..0d511089d7 100644
--- a/pkg/actions/nodegroup/create_test.go
+++ b/pkg/actions/nodegroup/create_test.go
@@ -269,6 +269,7 @@ var _ = DescribeTable("Create", func(t ngEntry) {
 		},
 		expectedErr: fmt.Errorf("all private subnets from vpc-1, that the cluster was originally created on, have been deleted; to create private nodegroups within vpc-1 please manually set valid private subnets via nodeGroup.SubnetIDs"),
 	}),
+
 	Entry("fails when nodegroup uses privateNetworking:false and there's no public subnet within vpc", ngEntry{
 		mockCalls: func(m mockCalls) {
 			mockProviderWithVPCSubnets(m.mockProvider, &vpcSubnets{
@@ -277,6 +278,7 @@ var _ = DescribeTable("Create", func(t ngEntry) {
 		},
 		expectedErr: fmt.Errorf("all public subnets from vpc-1, that the cluster was originally created on, have been deleted; to create public nodegroups within vpc-1 please manually set valid public subnets via nodeGroup.SubnetIDs"),
 	}),
+
 	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
@@ -290,9 +292,25 @@ var _ = DescribeTable("Create", func(t ngEntry) {
 		},
 		expectedErr: fmt.Errorf("all private subnets from us-west-2b, that the cluster was originally created on, have been deleted; to create private nodegroups within us-west-2b please manually set valid private subnets via nodeGroup.SubnetIDs"),
 	}),
+
 	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"}
+			c.VPC.Subnets = &api.ClusterSubnets{
+				Private: api.AZSubnetMapping{
+					"private-1": api.AZSubnetSpec{
+						ID: "subnet-private-1",
+					},
+					"private-2": api.AZSubnetSpec{
+						ID: "subnet-private-2",
+					},
+				},
+				Public: api.AZSubnetMapping{
+					"public-1": api.AZSubnetSpec{
+						ID: "subnet-public-2",
+					},
+				},
+			}
 		},
 		mockCalls: func(m mockCalls) {
 			mockProviderWithVPCSubnets(m.mockProvider, &vpcSubnets{