Merge pull request #1661 from justinsb/fix_1641

Allow specification of multiple CIDRs to create cluster

cmd/kops/create_cluster.go

@@ -55,7 +55,7 @@ type CreateClusterOptions struct {
 	VPCID             string
 	NetworkCIDR       string
 	DNSZone           string
-	AdminAccess       string
+	AdminAccess       []string
 	Networking        string
 	AssociatePublicIP *bool
 
@@ -85,6 +85,9 @@ func (o *CreateClusterOptions) InitDefaults() {
 	o.Topology = api.TopologyPublic
 	o.DNSType = string(api.DNSTypePublic)
 	o.Bastion = false
+
+	// Default to open API & SSH access
+	o.AdminAccess = []string{"0.0.0.0/0"}
 }
 
 func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
@@ -146,7 +149,7 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
 
 	cmd.Flags().StringVar(&options.DNSZone, "dns-zone", options.DNSZone, "DNS hosted zone to use (defaults to longest matching zone)")
 	cmd.Flags().StringVar(&options.OutDir, "out", options.OutDir, "Path to write any local output")
-	cmd.Flags().StringVar(&options.AdminAccess, "admin-access", options.AdminAccess, "Restrict access to admin endpoints (SSH, HTTPS) to this CIDR.  If not set, access will not be restricted by IP.")
+	cmd.Flags().StringSliceVar(&options.AdminAccess, "admin-access", options.AdminAccess, "Restrict access to admin endpoints (SSH, HTTPS) to this CIDR.  If not set, access will not be restricted by IP.")
 
 	// 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'.")
@@ -565,9 +568,9 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
 		glog.Infof("Using SSH public key: %v\n", c.SSHPublicKey)
 	}
 
-	if c.AdminAccess != "" {
-		cluster.Spec.SSHAccess = []string{c.AdminAccess}
-		cluster.Spec.KubernetesAPIAccess = []string{c.AdminAccess}
+	if len(c.AdminAccess) != 0 {
+		cluster.Spec.SSHAccess = c.AdminAccess
+		cluster.Spec.KubernetesAPIAccess = c.AdminAccess
 	}
 
 	err = cloudup.PerformAssignments(cluster)

pkg/apis/kops/cluster.go

@@ -112,12 +112,10 @@ type ClusterSpec struct {
 	// It cannot overlap ServiceClusterIPRange
 	NonMasqueradeCIDR string `json:"nonMasqueradeCIDR,omitempty"`
 
-	// SSHAccess determines the permitted access to SSH
-	// Currently only a single CIDR is supported (though a richer grammar could be added in future)
+	// SSHAccess is a list of the CIDRs that can access SSH.
 	SSHAccess []string `json:"sshAccess,omitempty"`
 
-	// KubernetesAPIAccess determines the permitted access to the Kubernetes API endpoints (master HTTPS)
-	// Currently only a single CIDR is supported (though a richer grammar could be added in future)
+	// KubernetesAPIAccess is a list of the CIDRs that can access the Kubernetes API endpoint (master HTTPS)
 	KubernetesAPIAccess []string `json:"kubernetesApiAccess,omitempty"`
 
 	// IsolatesMasters determines whether we should lock down masters so that they are not on the pod network.
@@ -341,21 +339,12 @@ type ClusterSubnetSpec struct {
 // This is different from PerformAssignments, because these values are changeable, and thus we don't need to
 // store them (i.e. we don't need to 'lock them')
 func (c *Cluster) FillDefaults() error {
-	// TODO: Move elsewhere
-	if len(c.Spec.SSHAccess) == 0 {
-		c.Spec.SSHAccess = append(c.Spec.SSHAccess, "0.0.0.0/0")
-	}
-
 	// Topology support
 	if c.Spec.Topology == nil {
 		c.Spec.Topology = &TopologySpec{Masters: TopologyPublic, Nodes: TopologyPublic}
 		c.Spec.Topology.DNS = &DNSSpec{Type: DNSTypePublic}
 	}
 
-	if len(c.Spec.KubernetesAPIAccess) == 0 {
-		c.Spec.KubernetesAPIAccess = append(c.Spec.KubernetesAPIAccess, "0.0.0.0/0")
-	}
-
 	if c.Spec.Networking == nil {
 		c.Spec.Networking = &NetworkingSpec{}
 	}

pkg/apis/kops/validation/legacy.go

@@ -248,11 +248,13 @@ func ValidateCluster(c *kops.Cluster, strict bool) error {
 
 	// AdminAccess
 	if strict && len(c.Spec.SSHAccess) == 0 {
+		// TODO: We may want to allow this
 		return fmt.Errorf("SSHAccess not configured")
 	}
 
 	// AdminAccess
 	if strict && len(c.Spec.KubernetesAPIAccess) == 0 {
+		// TODO: We may want to allow this (maybe)
 		return fmt.Errorf("KubernetesAPIAccess not configured")
 	}
 

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

@@ -4,6 +4,8 @@ metadata:
   creationTimestamp: "2017-01-01T00:00:00Z"
   name: ha.example.com
 spec:
+  adminAccess:
+  - 0.0.0.0/0
   api:
     dns: {}
   channel: stable

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

@@ -26,12 +26,16 @@ spec:
     - instanceGroup: master-us-test-1c
       name: us-test-1c
     name: events
+  kubernetesApiAccess:
+  - 0.0.0.0/0
   kubernetesVersion: v1.4.7
   masterPublicName: api.ha.example.com
   networkCIDR: 172.20.0.0/16
   networking:
     kubenet: {}
   nonMasqueradeCIDR: 100.64.0.0/10
+  sshAccess:
+  - 0.0.0.0/0
   subnets:
   - cidr: 172.20.32.0/19
     name: us-test-1a

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

@@ -4,6 +4,8 @@ metadata:
   creationTimestamp: "2017-01-01T00:00:00Z"
   name: minimal.example.com
 spec:
+  adminAccess:
+  - 0.0.0.0/0
   api:
     dns: {}
   channel: stable

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

@@ -18,12 +18,16 @@ spec:
     - instanceGroup: master-us-test-1a
       name: us-test-1a
     name: events
+  kubernetesApiAccess:
+  - 0.0.0.0/0
   kubernetesVersion: v1.4.7
   masterPublicName: api.minimal.example.com
   networkCIDR: 172.20.0.0/16
   networking:
     kubenet: {}
   nonMasqueradeCIDR: 100.64.0.0/10
+  sshAccess:
+  - 0.0.0.0/0
   subnets:
   - cidr: 172.20.32.0/19
     name: us-test-1a

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

@@ -4,6 +4,8 @@ metadata:
   creationTimestamp: "2017-01-01T00:00:00Z"
   name: private.example.com
 spec:
+  adminAccess:
+  - 0.0.0.0/0
   api:
     loadBalancer:
       type: Public

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

@@ -19,12 +19,16 @@ spec:
     - instanceGroup: master-us-test-1a
       name: us-test-1a
     name: events
+  kubernetesApiAccess:
+  - 0.0.0.0/0
   kubernetesVersion: v1.4.7
   masterPublicName: api.private.example.com
   networkCIDR: 172.20.0.0/16
   networking:
     kopeio: {}
   nonMasqueradeCIDR: 100.64.0.0/10
+  sshAccess:
+  - 0.0.0.0/0
   subnets:
   - cidr: 172.20.32.0/19
     egress: nat-09123456

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

@@ -4,6 +4,8 @@ metadata:
   creationTimestamp: "2017-01-01T00:00:00Z"
   name: private.example.com
 spec:
+  adminAccess:
+  - 0.0.0.0/0
   api:
     loadBalancer:
       type: Public

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

@@ -19,12 +19,16 @@ spec:
     - instanceGroup: master-us-test-1a
       name: us-test-1a
     name: events
+  kubernetesApiAccess:
+  - 0.0.0.0/0
   kubernetesVersion: v1.4.7
   masterPublicName: api.private.example.com
   networkCIDR: 172.20.0.0/16
   networking:
     kopeio: {}
   nonMasqueradeCIDR: 100.64.0.0/10
+  sshAccess:
+  - 0.0.0.0/0
   subnets:
   - cidr: 172.20.32.0/19
     name: us-test-1a

upup/pkg/fi/cloudup/populatecluster_test.go

@@ -35,6 +35,10 @@ func buildMinimalCluster() *api.Cluster {
 		{Name: "subnet-us-mock-1b", Zone: "us-mock-1b", CIDR: "172.20.2.0/24"},
 		{Name: "subnet-us-mock-1c", Zone: "us-mock-1c", CIDR: "172.20.3.0/24"},
 	}
+
+	c.Spec.KubernetesAPIAccess = []string{"0.0.0.0/0"}
+	c.Spec.SSHAccess = []string{"0.0.0.0/0"}
+
 	// Default to public topology
 	c.Spec.Topology = &api.TopologySpec{
 		Masters: api.TopologyPublic,