Merge pull request #312 from weaveworks/stack-template-desc

Improve stack template descriptions

pkg/cfn/builder/api.go

@@ -9,13 +9,9 @@ import (
 )
 
 const (
-	clusterTemplateDescription                = "EKS cluster"
-	clusterTemplateDescriptionDefaultFeatures = " (with dedicated VPC & IAM role) "
-
-	nodeGroupTemplateDescription                = "EKS nodes"
-	nodeGroupTemplateDescriptionDefaultFeatures = " (Amazon Linux 2 with SSH) "
-
-	templateDescriptionSuffix = " [created and managed by eksctl]"
+	clusterTemplateDescription   = "EKS cluster"
+	nodeGroupTemplateDescription = "EKS nodes"
+	templateDescriptionSuffix    = "[created and managed by eksctl]"
 )
 
 type awsCloudFormationResource struct {

pkg/cfn/builder/api_test.go

@@ -35,7 +35,8 @@ const (
 )
 
 type Template struct {
-	Resources map[string]struct {
+	Description string
+	Resources   map[string]struct {
 		Properties struct {
 			Tags []struct {
 				Key   interface{}
@@ -319,7 +320,6 @@ var _ = Describe("CloudFormation template builder API", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 			Expect(templateBody).ShouldNot(BeEmpty())
 		})
-
 		obj := Template{}
 		It("should parse JSON withon errors", func() {
 			err := json.Unmarshal(templateBody, &obj)
@@ -412,7 +412,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 		})
 	})
 
-	Describe("NodeGroup{PrivateNetworking=true}", func() {
+	Describe("NodeGroup{PrivateNetworking=true AllowSSH=true}", func() {
 		cfg := api.NewClusterConfig()
 		ng := cfg.NewNodeGroup()
 
@@ -422,6 +422,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 		ng.AllowSSH = true
 		ng.InstanceType = "t2.medium"
 		ng.PrivateNetworking = true
+		ng.AMIFamily = "AmazonLinux2"
 
 		rs := NewNodeGroupResourceSet(cfg, "eksctl-test-private-ng", 0)
 		rs.AddAllResources()
@@ -436,6 +437,12 @@ var _ = Describe("CloudFormation template builder API", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 		})
 
+		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"))
+		})
+
 		It("should have correct resources and attributes", func() {
 			Expect(len(obj.Resources)).ToNot(Equal(0))
 
@@ -473,6 +480,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 		ng.AllowSSH = true
 		ng.InstanceType = "t2.medium"
 		ng.PrivateNetworking = false
+		ng.AMIFamily = "AmazonLinux2"
 
 		rs := NewNodeGroupResourceSet(cfg, "eksctl-test-public-ng", 0)
 		rs.AddAllResources()
@@ -487,6 +495,12 @@ var _ = Describe("CloudFormation template builder API", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 		})
 
+		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"))
+		})
+
 		It("should have correct resources and attributes", func() {
 			Expect(len(obj.Resources)).ToNot(Equal(0))
 
@@ -557,6 +571,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 		ng.AllowSSH = false
 		ng.InstanceType = "t2.medium"
 		ng.PrivateNetworking = false
+		ng.AMIFamily = "AmazonLinux2"
 
 		It("should have 1 AZ for the nodegroup", func() {
 			Expect(ng.AvailabilityZones).To(Equal([]string{"us-west-2a"}))
@@ -575,6 +590,12 @@ var _ = Describe("CloudFormation template builder API", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 		})
 
+		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"))
+		})
+
 		It("should have correct resources and attributes", func() {
 			Expect(len(obj.Resources)).ToNot(Equal(0))
 
@@ -646,9 +667,8 @@ var _ = Describe("CloudFormation template builder API", func() {
 		It("should serialise JSON without errors", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 		})
-
+		obj := Template{}
 		It("should parse JSON withon errors and extract valid cloud-config using our implementation", func() {
-			obj := Template{}
 			err = json.Unmarshal(template, &obj)
 			Expect(err).ShouldNot(HaveOccurred())
 			Expect(len(obj.Resources)).ToNot(Equal(0))
@@ -713,9 +733,8 @@ var _ = Describe("CloudFormation template builder API", func() {
 		It("should serialise JSON without errors", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 		})
-
+		obj := Template{}
 		It("should parse JSON withon errors and extract valid cloud-config using our implementation", func() {
-			obj := Template{}
 			err = json.Unmarshal(template, &obj)
 			Expect(err).ShouldNot(HaveOccurred())
 			Expect(len(obj.Resources)).ToNot(Equal(0))
@@ -727,6 +746,12 @@ var _ = Describe("CloudFormation template builder API", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 		})
 
+		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"))
+		})
+
 		It("should have packages, scripts and commands in cloud-config", func() {
 			Expect(c).ToNot(BeNil())
 

pkg/cfn/builder/cluster.go

@@ -1,6 +1,8 @@
 package builder
 
 import (
+	"fmt"
+
 	cfn "github.com/aws/aws-sdk-go/service/cloudformation"
 	gfn "github.com/awslabs/goformation/cloudformation"
 
@@ -31,18 +33,22 @@ func NewClusterResourceSet(provider api.ClusterProvider, spec *api.ClusterConfig
 
 // AddAllResources adds all the information about the cluster to the resource set
 func (c *ClusterResourceSet) AddAllResources() error {
+	dedicatedVPC := c.spec.VPC.ID == ""
 
-	templateDescriptionFeatures := clusterTemplateDescriptionDefaultFeatures
+	c.rs.template.Description = fmt.Sprintf(
+		"%s (dedicated VPC: %v, dedicatd IAM: %v) %s",
+		clusterTemplateDescription,
+		dedicatedVPC, true,
+		templateDescriptionSuffix)
 
 	if err := c.spec.HasSufficientSubnets(); err != nil {
 		return err
 	}
 
-	if c.spec.VPC.ID != "" {
-		c.importResourcesForVPC()
-		templateDescriptionFeatures = " (with shared VPC and dedicated IAM role) "
-	} else {
+	if dedicatedVPC {
 		c.addResourcesForVPC()
+	} else {
+		c.importResourcesForVPC()
 	}
 	c.addOutputsForVPC()
 
@@ -52,10 +58,6 @@ func (c *ClusterResourceSet) AddAllResources() error {
 
 	c.rs.newOutput(cfnOutputClusterStackName, gfn.RefStackName, false)
 
-	c.rs.template.Description = clusterTemplateDescription
-	c.rs.template.Description += templateDescriptionFeatures
-	c.rs.template.Description += templateDescriptionSuffix
-
 	return nil
 }
 

pkg/cfn/builder/nodegroup.go

@@ -41,9 +41,11 @@ func NewNodeGroupResourceSet(spec *api.ClusterConfig, clusterStackName string, i
 
 // AddAllResources adds all the information about the node group to the resource set
 func (n *NodeGroupResourceSet) AddAllResources() error {
-	n.rs.template.Description = nodeGroupTemplateDescription
-	n.rs.template.Description += nodeGroupTemplateDescriptionDefaultFeatures
-	n.rs.template.Description += templateDescriptionSuffix
+	n.rs.template.Description = fmt.Sprintf(
+		"%s (AMI family: %s, SSH access: %v, subnet topology: %s) %s",
+		nodeGroupTemplateDescription,
+		n.spec.AMIFamily, n.spec.AllowSSH, n.spec.SubnetTopology(),
+		templateDescriptionSuffix)
 
 	n.vpc = makeImportValue(n.clusterStackName, cfnOutputClusterVPC)
 

pkg/nodebootstrap/userdata.go

@@ -86,7 +86,7 @@ func makeClientConfigData(spec *api.ClusterConfig, nodeGroupID int) ([]byte, err
 func clusterDNS(spec *api.ClusterConfig) string {
 	// Default service network is 10.100.0.0, but it gets set 172.20.0.0 automatically when pod network
 	// is anywhere within 10.0.0.0/8
-	if spec.VPC.CIDR.IP[0] == 10 {
+	if spec.VPC.CIDR != nil && spec.VPC.CIDR.IP[0] == 10 {
 		return "172.20.0.10"
 	}
 	return "10.100.0.10"