Make ng.SSH a pointer to SSHConfig

eksctl-io · Apr 8, 2019 · 92ce14d · 92ce14d
1 parent 93cbd19
commit 92ce14d
Show file tree

Hide file tree

Showing 12 changed files with 71 additions and 33 deletions.
diff --git a/pkg/apis/eksctl.io/v1alpha4/defaults.go b/pkg/apis/eksctl.io/v1alpha4/defaults.go
@@ -24,12 +24,18 @@ func SetNodeGroupDefaults(_ int, ng *NodeGroup) error {
 		ng.SecurityGroups.WithShared = NewBoolTrue()
 	}
 
+	if ng.SSH == nil {
+		ng.SSH = &SSHConfig{
+			Allow: NewBoolFalse(),
+		}
+	}
+
 	// Enable SSH when a key is provided
 	if ng.SSH.PublicKeyPath != nil {
-		ng.SSH.Allow = true
+		ng.SSH.Allow = NewBoolTrue()
 	}
 
-	if ng.SSH.Allow && ng.SSH.PublicKeyPath == nil {
+	if *ng.SSH.Allow && ng.SSH.PublicKeyPath == nil {
 		ng.SSH.PublicKeyPath = &DefaultNodeSSHPublicKeyPath
 	}
 

diff --git a/pkg/apis/eksctl.io/v1alpha4/defaults_test.go b/pkg/apis/eksctl.io/v1alpha4/defaults_test.go
@@ -14,21 +14,21 @@ var _ = Describe("Default settings", func() {
 
 		It("Providing an SSH key enables SSH", func() {
 			testNodeGroup := NodeGroup{
-				SSH: SSHConfig{
-					Allow:         false,
+				SSH: &SSHConfig{
+					Allow:         NewBoolFalse(),
 					PublicKeyPath: &testKeyPath,
 				},
 			}
 
 			SetNodeGroupDefaults(0, &testNodeGroup)
 
-			Expect(testNodeGroup.SSH.Allow).To(BeTrue())
+			Expect(*testNodeGroup.SSH.Allow).To(BeTrue())
 		})
 
 		It("Enabling SSH without a key uses default key", func() {
 			testNodeGroup := NodeGroup{
-				SSH: SSHConfig{
-					Allow: true,
+				SSH: &SSHConfig{
+					Allow: NewBoolTrue(),
 				},
 			}
 

diff --git a/pkg/apis/eksctl.io/v1alpha4/types.go b/pkg/apis/eksctl.io/v1alpha4/types.go
@@ -336,6 +336,10 @@ func (c *ClusterConfig) NewNodeGroup() *NodeGroup {
 				ALBIngress:   NewBoolFalse(),
 			},
 		},
+		SSH: &SSHConfig{
+			Allow:         NewBoolFalse(),
+			PublicKeyPath: &DefaultNodeSSHPublicKeyPath,
+		},
 	}
 
 	c.NodeGroups = append(c.NodeGroups, ng)
@@ -383,7 +387,7 @@ type NodeGroup struct {
 	// +optional
 	Taints map[string]string `json:"taints,omitempty"`
 
-	SSH SSHConfig `json:"ssh"`
+	SSH *SSHConfig `json:"ssh"`
 
 	// +optional
 	IAM *NodeGroupIAM `json:"iam"`
@@ -450,7 +454,7 @@ type (
 	// SSHConfig holds all the ssh access configuration to a NodeGroup
 	SSHConfig struct {
 		// +optional
-		Allow bool `json:"allow"`
+		Allow *bool `json:"allow"`
 		// +optional
 		PublicKeyPath *string `json:"publicKeyPath,omitempty"`
 		// +optional

diff --git a/pkg/apis/eksctl.io/v1alpha4/zz_generated.deepcopy.go b/pkg/apis/eksctl.io/v1alpha4/zz_generated.deepcopy.go
diff --git a/pkg/cfn/builder/api_test.go b/pkg/cfn/builder/api_test.go
@@ -304,6 +304,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 	}
 
 	Describe("GetAllOutputsFromClusterStack", func() {
+		defaultSSHKeyPath := "~/.ssh/id_rsa.pub"
 		expected := &api.ClusterConfig{
 			TypeMeta: api.ClusterConfigTypeMeta(),
 			Metadata: &api.ClusterMeta{
@@ -348,6 +349,10 @@ var _ = Describe("CloudFormation template builder API", func() {
 							ALBIngress:   api.NewBoolFalse(),
 						},
 					},
+					SSH: &api.SSHConfig{
+						Allow:         api.NewBoolFalse(),
+						PublicKeyPath: &defaultSSHKeyPath,
+					},
 				},
 			},
 		}
@@ -762,7 +767,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 	Context("NodeGroup{PrivateNetworking=true SSH.Allow=true}", func() {
 		cfg, ng := newClusterConfigAndNodegroup(true)
 
-		ng.SSH.Allow = true
+		ng.SSH.Allow = api.NewBoolTrue()
 		keyName := ""
 		ng.SSH.PublicKeyName = &keyName
 		ng.InstanceType = "t2.medium"
@@ -820,7 +825,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 	Context("NodeGroup{PrivateNetworking=false SSH.Allow=true}", func() {
 		cfg, ng := newClusterConfigAndNodegroup(true)
 
-		ng.SSH.Allow = true
+		ng.SSH.Allow = api.NewBoolTrue()
 		keyName := ""
 		ng.SSH.PublicKeyName = &keyName
 		ng.InstanceType = "t2.medium"
@@ -901,7 +906,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 		}
 
 		ng.AvailabilityZones = []string{testAZs[1]}
-		ng.SSH.Allow = false
+		ng.SSH.Allow = api.NewBoolFalse()
 		ng.InstanceType = "t2.medium"
 		ng.PrivateNetworking = false
 		ng.AMIFamily = "AmazonLinux2"

diff --git a/pkg/cfn/builder/nodegroup.go b/pkg/cfn/builder/nodegroup.go
@@ -44,7 +44,7 @@ func (n *NodeGroupResourceSet) AddAllResources() error {
 	n.rs.template.Description = fmt.Sprintf(
 		"%s (AMI family: %s, SSH access: %v, private networking: %v) %s",
 		nodeGroupTemplateDescription,
-		n.spec.AMIFamily, n.spec.SSH.Allow, n.spec.PrivateNetworking,
+		n.spec.AMIFamily, *n.spec.SSH.Allow, n.spec.PrivateNetworking,
 		templateDescriptionSuffix)
 
 	n.rs.defineOutputWithoutCollector(outputs.NodeGroupFeaturePrivateNetworking, n.spec.PrivateNetworking, false)
@@ -114,7 +114,7 @@ func (n *NodeGroupResourceSet) addResourcesForNodeGroup() error {
 		InstanceType:       gfn.NewString(n.spec.InstanceType),
 		UserData:           n.userData,
 	}
-	if n.spec.SSH.Allow && n.spec.SSH.PublicKeyName != nil {
+	if *n.spec.SSH.Allow && *n.spec.SSH.PublicKeyName != "" {
 		lc.KeyName = gfn.NewString(*n.spec.SSH.PublicKeyName)
 	}
 	if n.spec.PrivateNetworking {

diff --git a/pkg/cfn/builder/vpc.go b/pkg/cfn/builder/vpc.go
@@ -251,7 +251,7 @@ func (n *NodeGroupResourceSet) addResourcesForSecurityGroups() {
 		FromPort:              sgPortHTTPS,
 		ToPort:                sgPortHTTPS,
 	})
-	if n.spec.SSH.Allow {
+	if *n.spec.SSH.Allow {
 		if n.spec.PrivateNetworking {
 			n.newResource("SSHIPv4", &gfn.AWSEC2SecurityGroupIngress{
 				GroupId:     refNodeGroupLocalSG,

diff --git a/pkg/ctl/cmdutils/configfile.go b/pkg/ctl/cmdutils/configfile.go
@@ -180,12 +180,11 @@ func NewCreateClusterLoader(provider *api.ProviderConfig, spec *api.ClusterConfi
 		}
 
 		return ngFilter.CheckEachNodeGroup(l.spec.NodeGroups, func(i int, ng *api.NodeGroup) error {
-			if ng.SSH.Allow && ng.SSH.PublicKeyPath == nil {
-				return fmt.Errorf("--ssh-public-key must be non-empty string")
-			}
-
 			if cmd.Flag("ssh-public-key").Changed {
-				ng.SSH.Allow = true
+				if *ng.SSH.PublicKeyPath == "" {
+					return fmt.Errorf("--ssh-public-key must be non-empty string")
+				}
+				ng.SSH.Allow = api.NewBoolTrue()
 			}
 
 			// generate nodegroup name or use flag
@@ -243,12 +242,12 @@ func NewCreateNodeGroupLoader(provider *api.ProviderConfig, spec *api.ClusterCon
 		}
 
 		return ngFilter.CheckEachNodeGroup(spec.NodeGroups, func(i int, ng *api.NodeGroup) error {
-			if ng.SSH.Allow && ng.SSH.PublicKeyPath == nil {
-				return fmt.Errorf("--ssh-public-key must be non-empty string")
-			}
 
 			if cmd.Flag("ssh-public-key").Changed {
-				ng.SSH.Allow = true
+				if *ng.SSH.PublicKeyPath == "" {
+					return fmt.Errorf("--ssh-public-key must be non-empty string")
+				}
+				ng.SSH.Allow = api.NewBoolTrue()
 			}
 
 			// generate nodegroup name or use either flag or argument

diff --git a/pkg/ctl/cmdutils/nodegroup_filter_test.go b/pkg/ctl/cmdutils/nodegroup_filter_test.go
@@ -136,17 +136,20 @@ func addGroupA(cfg *api.ClusterConfig) {
 	ng.VolumeType = "io1"
 	ng.IAM.AttachPolicyARNs = []string{"foo"}
 	ng.Labels = map[string]string{"group": "a", "seq": "1"}
+	ng.SSH = nil
 
 	ng = cfg.NewNodeGroup()
 	ng.Name = "test-ng2a"
 	ng.IAM.AttachPolicyARNs = []string{"bar"}
 	ng.Labels = map[string]string{"group": "a", "seq": "2"}
+	ng.SSH = nil
 
 	ng = cfg.NewNodeGroup()
 	ng.Name = "test-ng3a"
 	ng.ClusterDNS = "1.2.3.4"
 	ng.InstanceType = "m3.large"
-	ng.SSH.Allow = true
+	ng.SSH.Allow = api.NewBoolTrue()
+	ng.SSH.PublicKeyPath = nil
 	ng.Labels = map[string]string{"group": "a", "seq": "3"}
 }
 
@@ -155,7 +158,8 @@ func addGroupB(cfg *api.ClusterConfig) {
 
 	ng = cfg.NewNodeGroup()
 	ng.Name = "test-ng1b"
-	ng.SSH.Allow = true
+	ng.SSH.Allow = api.NewBoolTrue()
+	ng.SSH.PublicKeyPath = nil
 	ng.Labels = map[string]string{"group": "b", "seq": "1"}
 
 	ng = cfg.NewNodeGroup()
@@ -165,13 +169,15 @@ func addGroupB(cfg *api.ClusterConfig) {
 	ng.SecurityGroups.AttachIDs = []string{"sg-1", "sg-2"}
 	ng.SecurityGroups.WithLocal = api.NewBoolFalse()
 	ng.Labels = map[string]string{"group": "b", "seq": "1"}
+	ng.SSH = nil
 
 	ng = cfg.NewNodeGroup()
 	ng.Name = "test-ng3b"
 	ng.VolumeSize = 192
 	ng.SecurityGroups.AttachIDs = []string{"sg-1", "sg-2"}
 	ng.SecurityGroups.WithLocal = api.NewBoolFalse()
 	ng.Labels = map[string]string{"group": "b", "seq": "1"}
+	ng.SSH = nil
 }
 
 const expected = `

diff --git a/pkg/ctl/cmdutils/nodegroup_flags.go b/pkg/ctl/cmdutils/nodegroup_flags.go
@@ -36,8 +36,8 @@ func AddCommonCreateNodeGroupFlags(cmd *cobra.Command, fs *pflag.FlagSet, p *api
 
 	fs.IntVar(&ng.MaxPodsPerNode, "max-pods-per-node", 0, "maximum number of pods per node (set automatically if unspecified)")
 
-	fs.BoolVar(&ng.SSH.Allow, "ssh-access", false, "control SSH access for nodes")
-	ng.SSH.PublicKeyPath = fs.String("ssh-public-key", api.DefaultNodeSSHPublicKeyPath, "SSH public key to use for nodes (import from local path, or use existing EC2 key pair)")
+	ng.SSH.Allow = fs.Bool("ssh-access", *ng.SSH.Allow, "control SSH access for nodes")
+	ng.SSH.PublicKeyPath = fs.String("ssh-public-key", *ng.SSH.PublicKeyPath, "SSH public key to use for nodes (import from local path, or use existing EC2 key pair)")
 
 	fs.StringVar(&ng.AMI, "node-ami", ami.ResolverStatic, "Advanced use cases only. If 'static' is supplied (default) then eksctl will use static AMIs; if 'auto' is supplied then eksctl will automatically set the AMI based on version/region/instance type; if any other value is supplied it will override the AMI to use for the nodes. Use with extreme care.")
 	fs.StringVar(&ng.AMIFamily, "node-ami-family", api.DefaultNodeImageFamily, "Advanced use cases only. If 'AmazonLinux2' is supplied (default), then eksctl will use the official AWS EKS AMIs (Amazon Linux 2); if 'Ubuntu1804' is supplied, then eksctl will use the official Canonical EKS AMIs (Ubuntu 18.04).")

diff --git a/pkg/eks/auth.go b/pkg/eks/auth.go
@@ -86,8 +86,7 @@ func (c *ClusterProvider) importSSHPublicKeyIfNeeded(clusterName string, ng *api
 
 // LoadSSHPublicKey loads the given SSH public key
 func (c *ClusterProvider) LoadSSHPublicKey(clusterName string, ng *api.NodeGroup) error {
-	if !ng.SSH.Allow {
-		// TODO: https://github.com/weaveworks/eksctl/issues/144
+	if !*ng.SSH.Allow {
 		return nil
 	}
 	keyPath := utils.ExpandPath(*ng.SSH.PublicKeyPath)

diff --git a/pkg/utils/kubeconfig/kubeconfig_test.go b/pkg/utils/kubeconfig/kubeconfig_test.go
@@ -145,8 +145,8 @@ var _ = Describe("Kubeconfig", func() {
 					InstanceType:      "m5.large",
 					AvailabilityZones: []string{"us-west-2b", "us-west-2a", "us-west-2c"},
 					PrivateNetworking: false,
-					SSH: eksctlapi.SSHConfig{
-						Allow:         false,
+					SSH: &eksctlapi.SSHConfig{
+						Allow:         eksctlapi.NewBoolFalse(),
 						PublicKeyPath: &exampleSSHKeyPath,
 						PublicKey:     []uint8(nil),
 						PublicKeyName: nil,