Skip to content

Commit

Permalink
Subnets availability validation should use AZs resolved by EC2::Descr…
Browse files Browse the repository at this point in the history
…ibeSubnets call
  • Loading branch information
TiberiuGC committed Jun 4, 2024
1 parent 7331dc1 commit e7f79cd
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 4 deletions.
20 changes: 16 additions & 4 deletions pkg/actions/nodegroup/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
}
23 changes: 23 additions & 0 deletions pkg/actions/nodegroup/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
)

type ngEntry struct {
runThisTest bool
version string
opts nodegroup.CreateOpts
mockCalls func(mockCalls)
Expand Down Expand Up @@ -94,6 +95,9 @@ func (s *stackManagerDelegate) ClusterHasDedicatedVPC(_ context.Context) (bool,
}

var _ = DescribeTable("Create", func(t ngEntry) {
if !t.runThisTest {
return
}
cfg := newClusterConfig()
cfg.Metadata.Version = t.version
if t.updateClusterConfig != nil {
Expand Down Expand Up @@ -269,6 +273,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{
Expand All @@ -277,6 +282,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
Expand All @@ -290,9 +296,26 @@ 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{
runThisTest: true,
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{
Expand Down

0 comments on commit e7f79cd

Please sign in to comment.