Merge pull request #1444 from tsupertramp/allow-adding-existing-security-groups

Allow adding existing security groups

cmd/kops/create_cluster.go

@@ -42,8 +42,8 @@ type CreateClusterOptions struct {
 	Target               string
 	Models               string
 	Cloud                string
-	Zones             string
-	MasterZones       string
+	Zones                []string
+	MasterZones          []string
 	NodeSize             string
 	MasterSize           string
 	NodeCount            int32
@@ -57,6 +57,8 @@ type CreateClusterOptions struct {
 	DNSZone              string
 	AdminAccess          []string
 	Networking           string
+	NodeSecurityGroups   []string
+	MasterSecurityGroups []string
 	AssociatePublicIP    *bool
 
 	// Channel is the location of the api.Channel to use for our defaults
@@ -126,8 +128,8 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
 
 	cmd.Flags().StringVar(&options.Cloud, "cloud", options.Cloud, "Cloud provider to use - gce, aws")
 
-	cmd.Flags().StringVar(&options.Zones, "zones", options.Zones, "Zones in which to run the cluster")
-	cmd.Flags().StringVar(&options.MasterZones, "master-zones", options.MasterZones, "Zones in which to run masters (must be an odd number)")
+	cmd.Flags().StringSliceVar(&options.Zones, "zones", options.Zones, "Zones in which to run the cluster")
+	cmd.Flags().StringSliceVar(&options.MasterZones, "master-zones", options.MasterZones, "Zones in which to run masters (must be an odd number)")
 
 	cmd.Flags().StringVar(&options.Project, "project", options.Project, "Project to use (must be set on GCE)")
 	cmd.Flags().StringVar(&options.KubernetesVersion, "kubernetes-version", options.KubernetesVersion, "Version of kubernetes to run (defaults to version in channel)")
@@ -154,6 +156,9 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
 	// TODO: Can we deprecate this flag - it is awkward?
 	cmd.Flags().BoolVar(&associatePublicIP, "associate-public-ip", false, "Specify --associate-public-ip=[true|false] to enable/disable association of public IP for master ASG and nodes. Default is 'true'.")
 
+	cmd.Flags().StringSliceVar(&options.NodeSecurityGroups, "node-security-groups", options.NodeSecurityGroups, "Add precreated additional security groups to nodes.")
+	cmd.Flags().StringSliceVar(&options.MasterSecurityGroups, "master-security-groups", options.MasterSecurityGroups, "Add precreated additional security groups to masters.")
+
 	cmd.Flags().StringVar(&options.Channel, "channel", options.Channel, "Channel for default versions and configuration to use")
 
 	// Network topology
@@ -251,13 +256,13 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
 
 	glog.V(4).Infof("networking mode=%s => %s", c.Networking, fi.DebugAsJsonString(cluster.Spec.Networking))
 
-	if c.Zones != "" {
+	if len(c.Zones) != 0 {
 		existingSubnets := make(map[string]*api.ClusterSubnetSpec)
 		for i := range cluster.Spec.Subnets {
 			subnet := &cluster.Spec.Subnets[i]
 			existingSubnets[subnet.Name] = subnet
 		}
-		for _, zoneName := range parseZoneList(c.Zones) {
+		for _, zoneName := range c.Zones {
 			// We create default subnets named the same as the zones
 			subnetName := zoneName
 			if existingSubnets[subnetName] == nil {
@@ -278,7 +283,7 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
 	var nodes []*api.InstanceGroup
 	var instanceGroups []*api.InstanceGroup
 
-	if c.MasterZones == "" {
+	if len(c.MasterZones) == 0 {
 		if len(masters) == 0 {
 			// We default to single-master (not HA), unless the user explicitly specifies it
 			// HA master is a little slower, not as well tested yet, and requires more resources
@@ -300,7 +305,7 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
 	} else {
 		if len(masters) == 0 {
 			// Use the specified master zones (this is how the user gets HA master)
-			for _, subnetName := range parseZoneList(c.MasterZones) {
+			for _, subnetName := range c.MasterZones {
 				g := &api.InstanceGroup{}
 				g.Spec.Role = api.InstanceGroupRoleMaster
 				g.Spec.Subnets = []string{subnetName}
@@ -370,6 +375,7 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
 	if len(nodes) == 0 {
 		g := &api.InstanceGroup{}
 		g.Spec.Role = api.InstanceGroupRoleNode
+
 		g.ObjectMeta.Name = "nodes"
 		instanceGroups = append(instanceGroups, g)
 		nodes = append(nodes, g)
@@ -400,6 +406,18 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
 		}
 	}
 
+	if len(c.NodeSecurityGroups) > 0 {
+		for _, group := range nodes {
+			group.Spec.AdditionalSecurityGroups = c.NodeSecurityGroups
+		}
+	}
+
+	if len(c.MasterSecurityGroups) > 0 {
+		for _, group := range masters {
+			group.Spec.AdditionalSecurityGroups = c.MasterSecurityGroups
+		}
+	}
+
 	if c.MasterSize != "" {
 		for _, group := range masters {
 			group.Spec.MachineType = c.MasterSize
@@ -684,19 +702,6 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
 	return nil
 }
 
-func parseZoneList(s string) []string {
-	var filtered []string
-	for _, v := range strings.Split(s, ",") {
-		v = strings.TrimSpace(v)
-		if v == "" {
-			continue
-		}
-		v = strings.ToLower(v)
-		filtered = append(filtered, v)
-	}
-	return filtered
-}
-
 func supportsPrivateTopology(n *api.NetworkingSpec) bool {
 
 	if n.CNI != nil || n.Kopeio != nil || n.Weave != nil || n.Calico != nil {

docs/advanced_create.md

@@ -10,6 +10,8 @@ kops create cluster \
     --dns-zone kubernetes.com \
     --node-size t2.medium \
     --master-size t2.medium \
+    --node-security-groups i-123456 \
+    --master-security-groups i-123456,i-123457 \
     --topology private \
     --networking weave \
     --image 293135079892/k8s-1.4-debian-jessie-amd64-hvm-ebs-2016-11-16 \

docs/cli/kops_create_cluster.md

@@ -22,12 +22,14 @@ kops create cluster
       --dns-zone string                 DNS hosted zone to use (defaults to longest matching zone)
       --image string                    Image to use
       --kubernetes-version string       Version of kubernetes to run (defaults to version in channel)
+      --master-security-groups string   Add precreated additional security groups to masters.
       --master-size string              Set instance size for masters
       --master-zones string             Zones in which to run masters (must be an odd number)
       --model string                    Models to apply (separate multiple models with commas) (default "config,proto,cloudup")
       --network-cidr string             Set to override the default network CIDR
       --networking string               Networking mode to use.  kubenet (default), classic, external, cni, kopeio-vxlan, weave, calico. (default "kubenet")
       --node-count int                  Set the number of nodes
+      --node-security-groups string     Add precreated additional security groups to nodes.
       --node-size string                Set instance size for nodes
       --out string                      Path to write any local output
       --project string                  Project to use (must be set on GCE)

pkg/apis/kops/instancegroup.go

@@ -81,6 +81,9 @@ type InstanceGroupSpec struct {
 	// AssociatePublicIP is true if we want instances to have a public IP
 	AssociatePublicIP *bool `json:"associatePublicIp,omitempty"`
 
+	// AdditionalSecurityGroups attaches additional security groups (e.g. i-123456)
+	AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
+
 	// CloudLabels indicates the labels for instances in this group, at the AWS level
 	CloudLabels map[string]string `json:"cloudLabels,omitempty"`
 

pkg/apis/kops/v1alpha1/instancegroup.go

@@ -69,6 +69,9 @@ type InstanceGroupSpec struct {
 	// AssociatePublicIP is true if we want instances to have a public IP
 	AssociatePublicIP *bool `json:"associatePublicIp,omitempty"`
 
+	// AdditionalSecurityGroups attaches additional security groups (e.g. i-123456)
+	AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
+
 	// CloudLabels indicates the labels for instances in this group, at the AWS level
 	CloudLabels map[string]string `json:"cloudLabels,omitempty"`
 

pkg/apis/kops/v1alpha1/zz_generated.conversion.go

@@ -899,6 +899,7 @@ func autoConvert_v1alpha1_InstanceGroupSpec_To_kops_InstanceGroupSpec(in *Instan
 	// WARNING: in.Zones requires manual conversion: does not exist in peer-type
 	out.MaxPrice = in.MaxPrice
 	out.AssociatePublicIP = in.AssociatePublicIP
+	out.AdditionalSecurityGroups = in.AdditionalSecurityGroups
 	out.CloudLabels = in.CloudLabels
 	out.NodeLabels = in.NodeLabels
 	return nil
@@ -915,6 +916,7 @@ func autoConvert_kops_InstanceGroupSpec_To_v1alpha1_InstanceGroupSpec(in *kops.I
 	// WARNING: in.Subnets requires manual conversion: does not exist in peer-type
 	out.MaxPrice = in.MaxPrice
 	out.AssociatePublicIP = in.AssociatePublicIP
+	out.AdditionalSecurityGroups = in.AdditionalSecurityGroups
 	out.CloudLabels = in.CloudLabels
 	out.NodeLabels = in.NodeLabels
 	return nil

pkg/apis/kops/v1alpha2/instancegroup.go

@@ -76,6 +76,9 @@ type InstanceGroupSpec struct {
 	// AssociatePublicIP is true if we want instances to have a public IP
 	AssociatePublicIP *bool `json:"associatePublicIp,omitempty"`
 
+	// AdditionalSecurityGroups attaches additional security groups (e.g. i-123456)
+	AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
+
 	// CloudLabels indicates the labels for instances in this group, at the AWS level
 	CloudLabels map[string]string `json:"cloudLabels,omitempty"`
 

pkg/model/autoscalinggroup.go

@@ -66,6 +66,7 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error {
 				SecurityGroups: []*awstasks.SecurityGroup{
 					b.LinkToSecurityGroup(ig.Spec.Role),
 				},
+				AdditionalSecurityGroupIDs: ig.Spec.AdditionalSecurityGroups,
 				IAMInstanceProfile:         b.LinkToIAMInstanceProfile(ig),
 				ImageID:                    s(ig.Spec.Image),
 				InstanceType:               s(ig.Spec.MachineType),

tests/integration/create_cluster/private/expected-v1alpha1.yaml

@@ -68,6 +68,9 @@ metadata:
     kops.k8s.io/cluster: private.example.com
   name: master-us-test-1a
 spec:
+  additionalSecurityGroups:
+  - i-exampleid3
+  - i-exampleid4
   image: kope.io/k8s-1.4-debian-jessie-amd64-hvm-ebs-2016-10-21
   machineType: m3.medium
   maxSize: 1
@@ -86,6 +89,9 @@ metadata:
     kops.k8s.io/cluster: private.example.com
   name: nodes
 spec:
+  additionalSecurityGroups:
+  - i-exampleid
+  - i-exampleid2
   image: kope.io/k8s-1.4-debian-jessie-amd64-hvm-ebs-2016-10-21
   machineType: t2.medium
   maxSize: 2

tests/integration/create_cluster/private/expected-v1alpha2.yaml

@@ -74,6 +74,9 @@ metadata:
     kops.k8s.io/cluster: private.example.com
   name: master-us-test-1a
 spec:
+  additionalSecurityGroups:
+  - i-exampleid3
+  - i-exampleid4
   image: kope.io/k8s-1.4-debian-jessie-amd64-hvm-ebs-2016-10-21
   machineType: m3.medium
   maxSize: 1
@@ -92,6 +95,9 @@ metadata:
     kops.k8s.io/cluster: private.example.com
   name: nodes
 spec:
+  additionalSecurityGroups:
+  - i-exampleid
+  - i-exampleid2
   image: kope.io/k8s-1.4-debian-jessie-amd64-hvm-ebs-2016-10-21
   machineType: t2.medium
   maxSize: 2

tests/integration/create_cluster/private/options.yaml

@@ -4,3 +4,5 @@ Cloud: aws
 Topology: private
 Networking: kopeio-vxlan
 Bastion: true
+NodeSecurityGroupIDs: i-exampleid, i-exampleid2
+MasterSecurityGroupIDs: i-exampleid3, i-exampleid4

upup/pkg/fi/cloudup/awstasks/launchconfiguration.go

@@ -40,6 +40,7 @@ type LaunchConfiguration struct {
 	InstanceType               *string
 	SSHKey                     *SSHKey
 	SecurityGroups             []*SecurityGroup
+	AdditionalSecurityGroupIDs []string
 	AssociatePublicIP          *bool
 	IAMInstanceProfile         *IAMInstanceProfile
 
@@ -227,13 +228,20 @@ func (_ *LaunchConfiguration) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *La
 	request.LaunchConfigurationName = &launchConfigurationName
 	request.ImageId = image.ImageId
 	request.InstanceType = e.InstanceType
+
 	if e.SSHKey != nil {
 		request.KeyName = e.SSHKey.Name
 	}
+
 	securityGroupIDs := []*string{}
 	for _, sg := range e.SecurityGroups {
 		securityGroupIDs = append(securityGroupIDs, sg.ID)
 	}
+
+	for i := range e.AdditionalSecurityGroupIDs {
+		securityGroupIDs = append(securityGroupIDs, &e.AdditionalSecurityGroupIDs[i])
+	}
+
 	request.SecurityGroups = securityGroupIDs
 	request.AssociatePublicIpAddress = e.AssociatePublicIP
 	if e.SpotPrice != "" {