Revert "Erroring out if the subnet is not public"

pkg/vpc/vpc.go

@@ -414,7 +414,7 @@ func ValidateLegacySubnetsForNodeGroups(spec *api.ClusterConfig, provider api.Cl
 		}
 	}
 
-	if err := ValidateExistingPublicSubnets(provider, spec, subnetsToValidate.List()); err != nil {
+	if err := ValidateExistingPublicSubnets(provider, spec.VPC.ID, subnetsToValidate.List()); err != nil {
 		// If the cluster endpoint is reachable from the VPC nodes might still be able to join
 		if spec.HasPrivateEndpointAccess() {
 			logger.Warning("public subnets for one or more nodegroups have %q disabled. This means that nodes won't "+
@@ -424,21 +424,23 @@ func ValidateLegacySubnetsForNodeGroups(spec *api.ClusterConfig, provider api.Cl
 		}
 
 		logger.Critical(err.Error())
-		return fmt.Errorf("subnets for one or more new nodegroups don't meet requirements: %w", err)
+		return errors.Errorf("subnets for one or more new nodegroups don't meet requirements. "+
+			"To fix this, please run `eksctl utils update-legacy-subnet-settings --cluster %s`",
+			spec.Metadata.Name)
 	}
 	return nil
 }
 
 // ValidateExistingPublicSubnets makes sure that subnets have the property MapPublicIpOnLaunch enabled
-func ValidateExistingPublicSubnets(provider api.ClusterProvider, cfg *api.ClusterConfig, subnetIDs []string) error {
+func ValidateExistingPublicSubnets(provider api.ClusterProvider, vpcID string, subnetIDs []string) error {
 	if len(subnetIDs) == 0 {
 		return nil
 	}
-	subnets, err := describeSubnets(provider.EC2(), cfg.VPC.ID, subnetIDs, []string{}, []string{})
+	subnets, err := describeSubnets(provider.EC2(), vpcID, subnetIDs, []string{}, []string{})
 	if err != nil {
 		return err
 	}
-	return validatePublicSubnet(cfg, subnets)
+	return validatePublicSubnet(subnets)
 }
 
 // EnsureMapPublicIPOnLaunchEnabled will enable MapPublicIpOnLaunch in EC2 for all given subnet IDs
@@ -522,28 +524,16 @@ func cleanupSubnets(spec *api.ClusterConfig) {
 	cleanup(&spec.VPC.Subnets.Public)
 }
 
-func validatePublicSubnet(cfg *api.ClusterConfig, subnets []*ec2.Subnet) error {
-	containsSubnet := func(id string) bool {
-		for _, f := range cfg.VPC.Subnets.Public.WithIDs() {
-			if f == id {
-				return true
-			}
-		}
-		return false
-	}
-	var legacySubnets []string
+func validatePublicSubnet(subnets []*ec2.Subnet) error {
+	legacySubnets := make([]string, 0)
 	for _, sn := range subnets {
 		if sn.MapPublicIpOnLaunch == nil || !*sn.MapPublicIpOnLaunch {
 			legacySubnets = append(legacySubnets, *sn.SubnetId)
 		}
-		if !containsSubnet(*sn.SubnetId) {
-			return fmt.Errorf("subnet %q could not be found in CF template or is not a public subnet", *sn.SubnetId)
-		}
 	}
 	if len(legacySubnets) > 0 {
 		return fmt.Errorf("found mis-configured subnets %q. Expected public subnets with property "+
-			"\"MapPublicIpOnLaunch\" enabled. Without it new nodes won't get an IP assigned. "+
-			"To fix this, please run `eksctl utils update-legacy-subnet-settings --cluster %s`", legacySubnets, cfg.Metadata.Name)
+			"\"MapPublicIpOnLaunch\" enabled. Without it new nodes won't get an IP assigned", legacySubnets)
 	}
 
 	return nil

pkg/vpc/vpc_test.go

@@ -1031,87 +1031,4 @@ var _ = Describe("VPC", func() {
 			})
 		})
 	})
-
-	Context("ValidateExistingPublicSubnets", func() {
-		var (
-			subnetIDPublic  string
-			subnetIDPrivate string
-			provider        *mockprovider.MockProvider
-			vpcID           string
-			spec            *api.ClusterConfig
-		)
-		BeforeEach(func() {
-			subnetIDPublic = "id-public"
-			subnetIDPrivate = "id-private"
-			vpcID = "vpc-id"
-			provider = mockprovider.NewMockProvider()
-			spec = api.NewClusterConfig()
-			spec.VPC.Subnets = &api.ClusterSubnets{
-				Private: api.AZSubnetMapping{
-					"a": api.AZSubnetSpec{
-						ID: subnetIDPrivate,
-					},
-				},
-				Public: api.AZSubnetMapping{
-					"a": api.AZSubnetSpec{
-						ID: subnetIDPublic,
-					},
-				},
-			}
-		})
-		When("validation is called with correct values", func() {
-			BeforeEach(func() {
-				provider.MockEC2().On("DescribeSubnets", &ec2.DescribeSubnetsInput{
-					SubnetIds: aws.StringSlice([]string{subnetIDPublic}),
-				}).Return(&ec2.DescribeSubnetsOutput{
-					Subnets: []*ec2.Subnet{
-						{
-							SubnetId:            &subnetIDPublic,
-							VpcId:               &vpcID,
-							MapPublicIpOnLaunch: api.Enabled(),
-						},
-					},
-				}, nil)
-			})
-			It("returns no errors", func() {
-				Expect(ValidateExistingPublicSubnets(provider, spec, []string{subnetIDPublic})).To(Succeed())
-			})
-		})
-		When("validation is called with MapPublicIpOnLaunch disabled for a public subnet", func() {
-			BeforeEach(func() {
-				provider.MockEC2().On("DescribeSubnets", &ec2.DescribeSubnetsInput{
-					SubnetIds: aws.StringSlice([]string{subnetIDPublic}),
-				}).Return(&ec2.DescribeSubnetsOutput{
-					Subnets: []*ec2.Subnet{
-						{
-							SubnetId: &subnetIDPublic,
-							VpcId:    &vpcID,
-						},
-					},
-				}, nil)
-			})
-			It("errors", func() {
-				err := ValidateExistingPublicSubnets(provider, spec, []string{subnetIDPublic})
-				Expect(err).To(MatchError(ContainSubstring("\"MapPublicIpOnLaunch\" enabled. Without it new nodes won't get an IP assigned.")))
-			})
-		})
-		When("validation is called with a private subnet for a public subnet", func() {
-			BeforeEach(func() {
-				provider.MockEC2().On("DescribeSubnets", &ec2.DescribeSubnetsInput{
-					SubnetIds: aws.StringSlice([]string{subnetIDPrivate}),
-				}).Return(&ec2.DescribeSubnetsOutput{
-					Subnets: []*ec2.Subnet{
-						{
-							SubnetId: &subnetIDPrivate,
-							VpcId:    &vpcID,
-						},
-					},
-				}, nil)
-			})
-			It("errors", func() {
-				err := ValidateExistingPublicSubnets(provider, spec, []string{subnetIDPrivate})
-				Expect(err).To(MatchError(ContainSubstring("subnet \"id-private\" could not be found in CF template or is not a public subnet")))
-			})
-		})
-	})
 })