Use dedicated fields for each of subnet topologies

- switch from map to fields, so that we can control JSON field names
- update docs

examples/04-existing-vpc.yaml

@@ -11,8 +11,8 @@ vpc:
   id: "vpc-0dd338ecf29863c55"  # (optional, must match VPC ID used for each subnet below)
   cidr: "192.168.0.0/16"       # (optional, must match CIDR used by the given VPC)
   subnets:
-    # must provide 'Private' and/or 'Public' subnets by availibility zone as shown
-    Private:
+    # must provide 'private' and/or 'public' subnets by availibility zone as shown
+    private:
       us-west-2a:
         id: "subnet-0b2512f8c6ae9bf30"
         cidr: "192.168.128.0/19" # (optional, must match CIDR used by the given subnet)

pkg/apis/eksctl.io/v1alpha4/types.go

@@ -357,15 +357,6 @@ type NodeGroup struct {
 	IAM *NodeGroupIAM `json:"iam"`
 }
 
-// SubnetTopology check which topology is used for the subnet of
-// the given nodegroup
-func (n *NodeGroup) SubnetTopology() SubnetTopology {
-	if n.PrivateNetworking {
-		return SubnetTopologyPrivate
-	}
-	return SubnetTopologyPublic
-}
-
 // ListOptions returns metav1.ListOptions with label selector for the nodegroup
 func (n *NodeGroup) ListOptions() metav1.ListOptions {
 	return metav1.ListOptions{

pkg/apis/eksctl.io/v1alpha4/vpc.go

@@ -17,14 +17,19 @@ type (
 		// subnets are either public or private for use with separate nodegroups
 		// these are keyed by AZ for convenience
 		// +optional
-		Subnets map[SubnetTopology]map[string]Network `json:"subnets,omitempty"`
+		Subnets *ClusterSubnets `json:"subnets,omitempty"`
 		// for additional CIDR associations, e.g. to use with separate CIDR for
 		// private subnets or any ad-hoc subnets
 		// +optional
 		ExtraCIDRs []*ipnet.IPNet `json:"extraCIDRs,omitempty"`
 		// for pre-defined shared node SG
 		SharedNodeSecurityGroup string `json:"sharedNodeSecurityGroup,omitempty"`
 	}
+	// ClusterSubnets holds private and public subnets
+	ClusterSubnets struct {
+		Private map[string]Network `json:"private,omitempty"`
+		Public  map[string]Network `json:"public,omitempty"`
+	}
 	// SubnetTopology can be SubnetTopologyPrivate or SubnetTopologyPublic
 	SubnetTopology string
 	// Network holds ID and CIDR
@@ -65,29 +70,47 @@ func DefaultCIDR() ipnet.IPNet {
 	}
 }
 
-// SubnetIDs returns list of subnets
-func (c *ClusterConfig) SubnetIDs(topology SubnetTopology) []string {
+// PrivateSubnetIDs returns list of subnets
+func (c *ClusterConfig) PrivateSubnetIDs() []string {
 	subnets := []string{}
-	if t, ok := c.VPC.Subnets[topology]; ok {
-		for _, s := range t {
+	if c.VPC.Subnets != nil {
+		for _, s := range c.VPC.Subnets.Private {
 			subnets = append(subnets, s.ID)
 		}
 	}
 	return subnets
 }
 
+// PublicSubnetIDs returns list of subnets
+func (c *ClusterConfig) PublicSubnetIDs() []string {
+	subnets := []string{}
+	for _, s := range c.VPC.Subnets.Public {
+		subnets = append(subnets, s.ID)
+	}
+	return subnets
+}
+
 // ImportSubnet loads a given subnet into cluster config
 func (c *ClusterConfig) ImportSubnet(topology SubnetTopology, az, subnetID, cidr string) error {
 	subnetCIDR, _ := ipnet.ParseCIDR(cidr)
 
 	if c.VPC.Subnets == nil {
-		c.VPC.Subnets = make(map[SubnetTopology]map[string]Network)
+		c.VPC.Subnets = &ClusterSubnets{}
+	}
+
+	var subnets map[string]Network
+	switch topology {
+	case SubnetTopologyPrivate:
+		subnets = c.VPC.Subnets.Private
+	case SubnetTopologyPublic:
+		subnets = c.VPC.Subnets.Public
 	}
-	if _, ok := c.VPC.Subnets[topology]; !ok {
-		c.VPC.Subnets[topology] = map[string]Network{}
+	if subnets == nil {
+		subnets = make(map[string]Network)
 	}
-	if network, ok := c.VPC.Subnets[topology][az]; !ok {
-		c.VPC.Subnets[topology][az] = Network{ID: subnetID, CIDR: subnetCIDR}
+
+	if network, ok := subnets[az]; !ok {
+		subnets[az] = Network{ID: subnetID, CIDR: subnetCIDR}
 	} else {
 		if network.ID == "" {
 			network.ID = subnetID
@@ -99,21 +122,21 @@ func (c *ClusterConfig) ImportSubnet(topology SubnetTopology, az, subnetID, cidr
 		} else if network.CIDR.String() != subnetCIDR.String() {
 			return fmt.Errorf("subnet CIDR %q is not the same as %q", network.CIDR.String(), subnetCIDR.String())
 		}
-		c.VPC.Subnets[topology][az] = network
+		subnets[az] = network
 	}
 	return nil
 }
 
-// HasSufficientPublicSubnets validates if there is a sufficient
-// number of public subnets available to create a cluster
-func (c *ClusterConfig) HasSufficientPublicSubnets() bool {
-	return len(c.SubnetIDs(SubnetTopologyPublic)) >= MinRequiredSubnets
-}
-
 // HasSufficientPrivateSubnets validates if there is a sufficient
 // number of private subnets available to create a cluster
 func (c *ClusterConfig) HasSufficientPrivateSubnets() bool {
-	return len(c.SubnetIDs(SubnetTopologyPrivate)) >= MinRequiredSubnets
+	return len(c.PrivateSubnetIDs()) >= MinRequiredSubnets
+}
+
+// HasSufficientPublicSubnets validates if there is a sufficient
+// number of public subnets available to create a cluster
+func (c *ClusterConfig) HasSufficientPublicSubnets() bool {
+	return len(c.PublicSubnetIDs()) >= MinRequiredSubnets
 }
 
 var errInsufficientSubnets = fmt.Errorf(
@@ -126,12 +149,12 @@ var errInsufficientSubnets = fmt.Errorf(
 // less then MinRequiredSubnets of each, but allowing to have
 // public-only or private-only
 func (c *ClusterConfig) HasSufficientSubnets() error {
-	numPublic := len(c.SubnetIDs(SubnetTopologyPublic))
+	numPublic := len(c.PublicSubnetIDs())
 	if numPublic > 0 && numPublic < MinRequiredSubnets {
 		return errInsufficientSubnets
 	}
 
-	numPrivate := len(c.SubnetIDs(SubnetTopologyPrivate))
+	numPrivate := len(c.PrivateSubnetIDs())
 	if numPrivate > 0 && numPrivate < MinRequiredSubnets {
 		return errInsufficientSubnets
 	}

pkg/cfn/builder/api_test.go

@@ -109,8 +109,8 @@ func testVPC() *api.ClusterVPC {
 		},
 		SecurityGroup:           "sg-0b44c48bcba5b7362",
 		SharedNodeSecurityGroup: "sg-shared",
-		Subnets: map[api.SubnetTopology]map[string]api.Network{
-			"Public": map[string]api.Network{
+		Subnets: &api.ClusterSubnets{
+			Public: map[string]api.Network{
 				"us-west-2b": {
 					ID: "subnet-0f98135715dfcf55f",
 					CIDR: &ipnet.IPNet{
@@ -139,7 +139,7 @@ func testVPC() *api.ClusterVPC {
 					},
 				},
 			},
-			"Private": map[string]api.Network{
+			Private: map[string]api.Network{
 				"us-west-2b": {
 					ID: "subnet-0f98135715dfcf55a",
 					CIDR: &ipnet.IPNet{
@@ -253,7 +253,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 		}, nil)
 
 		for t := range subnetLists {
-			func(list string, subnetsByAz map[string]api.Network) {
+			fn := func(list string, subnetsByAz map[string]api.Network) {
 				subnets := strings.Split(list, ",")
 
 				output := &ec2.DescribeSubnetsOutput{
@@ -277,7 +277,14 @@ var _ = Describe("CloudFormation template builder API", func() {
 					fmt.Fprintf(GinkgoWriter, "%s subnets = %#v\n", t, output)
 					return joinCompare(input, list)
 				})).Return(output, nil)
-			}(subnetLists[t], testVPC.Subnets[t])
+			}
+			switch t {
+			case "Private":
+				fn(subnetLists[t], testVPC.Subnets.Private)
+			case "Public":
+				fn(subnetLists[t], testVPC.Subnets.Public)
+			}
+
 		}
 	}
 
@@ -333,11 +340,8 @@ var _ = Describe("CloudFormation template builder API", func() {
 		})
 
 		It("should have public and private subnets", func() {
-			Expect(len(cfg.VPC.Subnets)).To(Equal(2))
-			for _, k := range []api.SubnetTopology{"Public", "Private"} {
-				Expect(cfg.VPC.Subnets).To(HaveKey(k))
-				Expect(len(cfg.VPC.Subnets[k])).To(Equal(3))
-			}
+			Expect(len(cfg.VPC.Subnets.Private)).To(Equal(3))
+			Expect(len(cfg.VPC.Subnets.Public)).To(Equal(3))
 		})
 
 		rs := NewClusterResourceSet(p, cfg)
@@ -546,7 +550,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 		It("should have correct description", func() {
 			Expect(obj.Description).To(ContainSubstring("AMI family: AmazonLinux2"))
 			Expect(obj.Description).To(ContainSubstring("SSH access: true"))
-			Expect(obj.Description).To(ContainSubstring("subnet topology: Private"))
+			Expect(obj.Description).To(ContainSubstring("private networking: true"))
 		})
 
 		It("should have correct resources and attributes", func() {
@@ -604,7 +608,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 		It("should have correct description", func() {
 			Expect(obj.Description).To(ContainSubstring("AMI family: AmazonLinux2"))
 			Expect(obj.Description).To(ContainSubstring("SSH access: true"))
-			Expect(obj.Description).To(ContainSubstring("subnet topology: Public"))
+			Expect(obj.Description).To(ContainSubstring("private networking: false"))
 		})
 
 		It("should have correct resources and attributes", func() {
@@ -644,8 +648,8 @@ var _ = Describe("CloudFormation template builder API", func() {
 				ID: vpcID,
 			},
 			SecurityGroup: "sg-0b44c48bcba5b7362",
-			Subnets: map[api.SubnetTopology]map[string]api.Network{
-				"Public": map[string]api.Network{
+			Subnets: &api.ClusterSubnets{
+				Public: map[string]api.Network{
 					"us-west-2b": {
 						ID: "subnet-0f98135715dfcf55f",
 					},
@@ -656,7 +660,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 						ID: "subnet-0e2e63ff1712bf6ef",
 					},
 				},
-				"Private": map[string]api.Network{
+				Private: map[string]api.Network{
 					"us-west-2b": {
 						ID: "subnet-0f98135715dfcf55a",
 					},
@@ -699,7 +703,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 		It("should have correct description", func() {
 			Expect(obj.Description).To(ContainSubstring("AMI family: AmazonLinux2"))
 			Expect(obj.Description).To(ContainSubstring("SSH access: false"))
-			Expect(obj.Description).To(ContainSubstring("subnet topology: Public"))
+			Expect(obj.Description).To(ContainSubstring("private networking: false"))
 		})
 
 		It("should have correct resources and attributes", func() {
@@ -709,7 +713,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 			x, ok := obj.Resources["NodeGroup"].Properties.VPCZoneIdentifier.([]interface{})
 			Expect(ok).To(BeTrue())
 			refSubnets := []interface{}{
-				cfg.VPC.Subnets["Public"]["us-west-2a"].ID,
+				cfg.VPC.Subnets.Public["us-west-2a"].ID,
 			}
 			Expect(x).To((Equal(refSubnets)))
 
@@ -852,7 +856,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 		It("should have correct description", func() {
 			Expect(obj.Description).To(ContainSubstring("AMI family: Ubuntu1804"))
 			Expect(obj.Description).To(ContainSubstring("SSH access: false"))
-			Expect(obj.Description).To(ContainSubstring("subnet topology: Public"))
+			Expect(obj.Description).To(ContainSubstring("private networking: false"))
 		})
 
 		It("should have packages, scripts and commands in cloud-config", func() {

pkg/cfn/builder/cluster.go

@@ -85,7 +85,7 @@ func (c *ClusterResourceSet) addResourcesForControlPlane() {
 	clusterVPC := &gfn.AWSEKSCluster_ResourcesVpcConfig{
 		SecurityGroupIds: c.securityGroups,
 	}
-	for topology := range c.spec.VPC.Subnets {
+	for topology := range c.subnets {
 		clusterVPC.SubnetIds = append(clusterVPC.SubnetIds, c.subnets[topology]...)
 	}
 

pkg/cfn/builder/nodegroup.go

@@ -42,9 +42,9 @@ func NewNodeGroupResourceSet(provider api.ClusterProvider, spec *api.ClusterConf
 // AddAllResources adds all the information about the node group to the resource set
 func (n *NodeGroupResourceSet) AddAllResources() error {
 	n.rs.template.Description = fmt.Sprintf(
-		"%s (AMI family: %s, SSH access: %v, subnet topology: %s) %s",
+		"%s (AMI family: %s, SSH access: %v, private networking: %v) %s",
 		nodeGroupTemplateDescription,
-		n.spec.AMIFamily, n.spec.AllowSSH, n.spec.SubnetTopology(),
+		n.spec.AMIFamily, n.spec.AllowSSH, n.spec.PrivateNetworking,
 		templateDescriptionSuffix)
 
 	n.rs.defineOutputWithoutCollector(outputs.NodeGroupFeaturePrivateNetworking, n.spec.PrivateNetworking, false)
@@ -138,7 +138,10 @@ func (n *NodeGroupResourceSet) addResourcesForNodeGroup() error {
 	// and tags don't have `PropagateAtLaunch` field, so we have a custom method here until this gets resolved
 	var vpcZoneIdentifier interface{}
 	if numNodeGroupsAZs := len(n.spec.AvailabilityZones); numNodeGroupsAZs > 0 {
-		subnets := n.clusterSpec.VPC.Subnets[n.spec.SubnetTopology()]
+		subnets := n.clusterSpec.VPC.Subnets.Private
+		if !n.spec.PrivateNetworking {
+			subnets = n.clusterSpec.VPC.Subnets.Public
+		}
 		errorDesc := fmt.Sprintf("(subnets=%#v AZs=%#v)", subnets, n.spec.AvailabilityZones)
 		if len(subnets) < numNodeGroupsAZs {
 			return fmt.Errorf("VPC doesn't have enough subnets for nodegroup AZs %s", errorDesc)
@@ -152,11 +155,12 @@ func (n *NodeGroupResourceSet) addResourcesForNodeGroup() error {
 			vpcZoneIdentifier.([]interface{})[i] = subnet.ID
 		}
 	} else {
+		subnets := makeImportValue(n.clusterStackName, outputs.ClusterSubnetsPrivate)
+		if !n.spec.PrivateNetworking {
+			subnets = makeImportValue(n.clusterStackName, outputs.ClusterSubnetsPublic)
+		}
 		vpcZoneIdentifier = map[string][]interface{}{
-			gfn.FnSplit: []interface{}{
-				",",
-				makeImportValue(n.clusterStackName, outputs.ClusterSubnets+string(n.spec.SubnetTopology())),
-			},
+			gfn.FnSplit: []interface{}{",", subnets},
 		}
 	}
 	tags := []map[string]interface{}{