Remove API fields for nodegroup SGs

eksctl-io · Jan 24, 2019 · d22a84d · d22a84d
1 parent 7fbc627
commit d22a84d
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 33 deletions.
diff --git a/pkg/apis/eksctl.io/v1alpha4/types.go b/pkg/apis/eksctl.io/v1alpha4/types.go
@@ -265,12 +265,16 @@ func (c *ClusterConfig) AppendAvailabilityZone(newAZ string) {
 // it returns pointer to the nodegroup for convenience
 func (c *ClusterConfig) NewNodeGroup() *NodeGroup {
 	ng := &NodeGroup{
-		PrivateNetworking:   false,
-		SharedSecurityGroup: true,
-		DesiredCapacity:     DefaultNodeCount,
-		InstanceType:        DefaultNodeType,
-		VolumeSize:          0,
-		VolumeType:          DefaultNodeVolumeType,
+		PrivateNetworking: false,
+		SecurityGroups: NodeGroupSGs{
+			WithShared: true,
+			WithLocal:  true,
+			AttachIDs:  []string{},
+		},
+		DesiredCapacity: DefaultNodeCount,
+		InstanceType:    DefaultNodeType,
+		VolumeSize:      0,
+		VolumeType:      DefaultNodeVolumeType,
 	}
 
 	c.NodeGroups = append(c.NodeGroups, ng)
@@ -291,14 +295,13 @@ type NodeGroup struct {
 	// +optional
 	AvailabilityZones []string `json:"availabilityZones,omitempty"`
 	// +optional
-	SharedSecurityGroup bool `json:"sharedSecurityGroup,omitempty"`
-	// +optional
-	SecurityGroups []string `json:"securityGroups,omitempty"`
-	// +optional
 	Tags map[string]string `json:"tags,omitempty"`
 	// +optional
 	PrivateNetworking bool `json:"privateNetworking"`
 
+	// +optional
+	SecurityGroups NodeGroupSGs `json:"securityGroups,omitempty"`
+
 	// +optional
 	DesiredCapacity int `json:"desiredCapacity"`
 	// +optional
@@ -347,6 +350,15 @@ func (n *NodeGroup) ListOptions() metav1.ListOptions {
 }
 
 type (
+	// NodeGroupSGs holds all SG attributes of a NodeGroup
+	NodeGroupSGs struct {
+		// +optional
+		AttachIDs []string
+		// +optional
+		WithShared bool
+		// +optional
+		WithLocal bool
+	}
 	// NodeGroupIAM holds all IAM attributes of a NodeGroup
 	NodeGroupIAM struct {
 		// +optional

diff --git a/pkg/cfn/builder/api_test.go b/pkg/cfn/builder/api_test.go
@@ -298,15 +298,19 @@ var _ = Describe("CloudFormation template builder API", func() {
 			VPC:               testVPC(),
 			NodeGroups: []*api.NodeGroup{
 				{
-					AMI:                 "",
-					AMIFamily:           "AmazonLinux2",
-					InstanceType:        "t2.medium",
-					Name:                "ng-abcd1234",
-					PrivateNetworking:   false,
-					SharedSecurityGroup: true,
-					DesiredCapacity:     2,
-					VolumeSize:          2,
-					VolumeType:          api.NodeVolumeTypeIO1,
+					AMI:               "",
+					AMIFamily:         "AmazonLinux2",
+					InstanceType:      "t2.medium",
+					Name:              "ng-abcd1234",
+					PrivateNetworking: false,
+					SecurityGroups: api.NodeGroupSGs{
+						WithLocal:  true,
+						WithShared: true,
+						AttachIDs:  []string{},
+					},
+					DesiredCapacity: 2,
+					VolumeSize:      2,
+					VolumeType:      api.NodeVolumeTypeIO1,
 				},
 			},
 		}

diff --git a/pkg/cfn/builder/nodegroup.go b/pkg/cfn/builder/nodegroup.go
@@ -46,7 +46,8 @@ func (n *NodeGroupResourceSet) AddAllResources() error {
 		templateDescriptionSuffix)
 
 	n.rs.defineOutputWithoutCollector(outputs.NodeGroupFeaturePrivateNetworking, n.spec.PrivateNetworking, false)
-	n.rs.defineOutputWithoutCollector(outputs.NodeGroupFeatureSharedSecurityGroup, n.spec.SharedSecurityGroup, false)
+	n.rs.defineOutputWithoutCollector(outputs.NodeGroupFeatureSharedSecurityGroup, n.spec.SecurityGroups.WithShared, false)
+	n.rs.defineOutputWithoutCollector(outputs.NodeGroupFeatureLocalSecurityGroup, n.spec.SecurityGroups.WithLocal, false)
 
 	n.vpc = makeImportValue(n.clusterStackName, outputs.ClusterVPC)
 

diff --git a/pkg/cfn/builder/vpc.go b/pkg/cfn/builder/vpc.go
@@ -179,6 +179,19 @@ func (c *ClusterResourceSet) addResourcesForSecurityGroups() {
 }
 
 func (n *NodeGroupResourceSet) addResourcesForSecurityGroups() {
+	for _, id := range n.spec.SecurityGroups.AttachIDs {
+		n.securityGroups = append(n.securityGroups, gfn.NewString(id))
+	}
+
+	if n.spec.SecurityGroups.WithShared {
+		refClusterSharedNodeSG := makeImportValue(n.clusterStackName, outputs.ClusterSharedNodeSecurityGroup)
+		n.securityGroups = append(n.securityGroups, refClusterSharedNodeSG)
+	}
+
+	if !n.spec.SecurityGroups.WithLocal {
+		return
+	}
+
 	desc := "worker nodes in group " + n.nodeGroupName
 
 	allInternalIPv4 := gfn.NewString(n.clusterSpec.VPC.CIDR.String())
@@ -194,16 +207,7 @@ func (n *NodeGroupResourceSet) addResourcesForSecurityGroups() {
 		}},
 	})
 
-	n.securityGroups = []*gfn.Value{refNodeGroupLocalSG}
-
-	if n.spec.SharedSecurityGroup {
-		refClusterSharedNodeSG := makeImportValue(n.clusterStackName, outputs.ClusterSharedNodeSecurityGroup)
-		n.securityGroups = append(n.securityGroups, refClusterSharedNodeSG)
-	}
-
-	for _, id := range n.spec.SecurityGroups {
-		n.securityGroups = append(n.securityGroups, gfn.NewString(id))
-	}
+	n.securityGroups = append(n.securityGroups, refNodeGroupLocalSG)
 
 	n.newResource("IngressInterCluster", &gfn.AWSEC2SecurityGroupIngress{
 		GroupId:               refNodeGroupLocalSG,

diff --git a/pkg/cfn/outputs/api.go b/pkg/cfn/outputs/api.go
@@ -31,6 +31,7 @@ const (
 	// and integrity, e.g. networking
 	NodeGroupFeaturePrivateNetworking   = "FeaturePrivateNetworking"
 	NodeGroupFeatureSharedSecurityGroup = "FeatureSharedSecurityGroup"
+	NodeGroupFeatureLocalSecurityGroup  = "FeatureLocalSecurityGroup"
 )
 
 // MustCollect will use each of the keys and attempt to find an output in the given

diff --git a/pkg/ctl/cmdutils/nodegroup.go b/pkg/ctl/cmdutils/nodegroup.go
@@ -37,16 +37,16 @@ func AddCommonCreateNodeGroupFlags(fs *pflag.FlagSet, p *api.ProviderConfig, cfg
 
 	fs.BoolVarP(&ng.PrivateNetworking, "node-private-networking", "P", false, "whether to make nodegroup networking private")
 
-	fs.StringSliceVar(&ng.SecurityGroups, "node-security-groups", []string{}, "Attach additional security groups to nodes, so that it can be used to allow extra ingress/egress access from/to pods")
+	fs.StringSliceVar(&ng.SecurityGroups.AttachIDs, "node-security-groups", []string{}, "Attach additional security groups to nodes, so that it can be used to allow extra ingress/egress access from/to pods")
 
 	fs.Var(&ng.Labels, "node-labels", `Extra labels to add when registering the nodes in the nodegroup, e.g. "partition=backend,nodeclass=hugememory"`)
 	fs.StringSliceVar(&ng.AvailabilityZones, "node-zones", nil, "(inherited from the cluster if unspecified)")
 }
 
 // AddCommonCreateNodeGroupIAMAddonsFlags adds flags to set ng.IAM.WithAddonPolicies
 func AddCommonCreateNodeGroupIAMAddonsFlags(fs *pflag.FlagSet, ng *api.NodeGroup) {
-	fs.StringSliceVar(&ng.IAM.AttachPolicyARNs, "temp-node-role-policies", []string{}, "Advanced use cases only. " +
-		"All the IAM policies to be associated to the node's instance role. " +
+	fs.StringSliceVar(&ng.IAM.AttachPolicyARNs, "temp-node-role-policies", []string{}, "Advanced use cases only. "+
+		"All the IAM policies to be associated to the node's instance role. "+
 		"Beware that you MUST include the policies for EKS and CNI related AWS API Access, like `arn:aws:iam::aws:policy/AmazonEKSWorkerNodePolicy` and `arn:aws:iam::aws:policy/AmazonEKS_CNI_Policy` that are used by default when this flag is omitted.")
 	fs.MarkHidden("temp-node-role-policies")
 	fs.StringVar(&ng.IAM.InstanceRoleName, "temp-node-role-name", "", "Advanced use cases only. Specify the exact name of the node's instance role for easier integration with K8S-IAM integrations like kube2iam. See https://github.com/weaveworks/eksctl/issues/398 for more information.")