Merge pull request #3336 from justinsb/nodeportaccess

Automatic merge from submit-queue

nodePortAccess, experimental spec override flag

cmd/kops/create_cluster.go

@@ -79,6 +79,9 @@ type CreateClusterOptions struct {
 	MasterSecurityGroups []string
 	AssociatePublicIP    *bool
 
+	// Overrides allows settings values direct in the spec
+	Overrides []string
+
 	// Channel is the location of the api.Channel to use for our defaults
 	Channel string
 
@@ -284,6 +287,10 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
 
 	cmd.Flags().StringVar(&options.APILoadBalancerType, "api-loadbalancer-type", options.APILoadBalancerType, "Sets the API loadbalancer type to either 'public' or 'internal'")
 
+	if featureflag.SpecOverrideFlag.Enabled() {
+		cmd.Flags().StringSliceVar(&options.Overrides, "override", options.Overrides, "Directly configure values in the spec")
+	}
+
 	if featureflag.VSphereCloudProvider.Enabled() {
 		// vSphere flags
 		cmd.Flags().StringVar(&options.VSphereServer, "vsphere-server", options.VSphereServer, "vsphere-server is required for vSphere. Set vCenter URL Ex: 10.192.10.30 or myvcenter.io (without https://)")
@@ -860,6 +867,10 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
 		cluster.Spec.SSHAccess = c.SSHAccess
 	}
 
+	if err := setOverrides(c.Overrides, cluster, instanceGroups); err != nil {
+		return err
+	}
+
 	err = cloudup.PerformAssignments(cluster)
 	if err != nil {
 		return fmt.Errorf("error populating configuration: %v", err)
@@ -1039,3 +1050,22 @@ func parseCloudLabels(s string) (map[string]string, error) {
 	}
 	return m, nil
 }
+
+// setOverrides sets override values in the spec
+func setOverrides(overrides []string, cluster *api.Cluster, instanceGroups []*api.InstanceGroup) error {
+	for _, override := range overrides {
+		kv := strings.SplitN(override, "=", 2)
+		if len(kv) != 2 {
+			return fmt.Errorf("unhandled override: %q", override)
+		}
+
+		// For now we have hard-code the values we want to support; we'll get test coverage and then do this properly...
+		switch kv[0] {
+		case "cluster.spec.nodePortAccess":
+			cluster.Spec.NodePortAccess = append(cluster.Spec.NodePortAccess, kv[1])
+		default:
+			return fmt.Errorf("unhandled override: %q", override)
+		}
+	}
+	return nil
+}

cmd/kops/create_cluster_integration_test.go

@@ -41,6 +41,11 @@ func TestCreateClusterMinimal(t *testing.T) {
 	runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/minimal", "v1alpha2")
 }
 
+// TestCreateClusterOverride tests the override flag
+func TestCreateClusterOverride(t *testing.T) {
+	runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/overrides", "v1alpha2")
+}
+
 // TestCreateClusterComplex runs kops create cluster, with a grab-bag of edge cases
 func TestCreateClusterComplex(t *testing.T) {
 	runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/complex", "v1alpha2")

pkg/apis/kops/cluster.go

@@ -92,6 +92,8 @@ type ClusterSpec struct {
 	NonMasqueradeCIDR string `json:"nonMasqueradeCIDR,omitempty"`
 	// SSHAccess is a list of the CIDRs that can access SSH.
 	SSHAccess []string `json:"sshAccess,omitempty"`
+	// NodePortAccess is a list of the CIDRs that can access the node ports range (30000-32767).
+	NodePortAccess []string `json:"nodePortAccess,omitempty"`
 	// HTTPProxy defines connection information to support use of a private cluster behind an forward HTTP Proxy
 	EgressProxy *EgressProxySpec `json:"egressProxy,omitempty"`
 

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

@@ -761,6 +761,7 @@ func autoConvert_kops_ClusterSpec_To_v1alpha1_ClusterSpec(in *kops.ClusterSpec,
 	out.ServiceClusterIPRange = in.ServiceClusterIPRange
 	out.NonMasqueradeCIDR = in.NonMasqueradeCIDR
 	// WARNING: in.SSHAccess requires manual conversion: does not exist in peer-type
+	// WARNING: in.NodePortAccess requires manual conversion: does not exist in peer-type
 	if in.EgressProxy != nil {
 		in, out := &in.EgressProxy, &out.EgressProxy
 		*out = new(EgressProxySpec)
@@ -1169,6 +1170,7 @@ func autoConvert_v1alpha1_ExternalDNSConfig_To_kops_ExternalDNSConfig(in *Extern
 	return nil
 }
 
+// Convert_v1alpha1_ExternalDNSConfig_To_kops_ExternalDNSConfig is an autogenerated conversion function.
 func Convert_v1alpha1_ExternalDNSConfig_To_kops_ExternalDNSConfig(in *ExternalDNSConfig, out *kops.ExternalDNSConfig, s conversion.Scope) error {
 	return autoConvert_v1alpha1_ExternalDNSConfig_To_kops_ExternalDNSConfig(in, out, s)
 }
@@ -1178,6 +1180,7 @@ func autoConvert_kops_ExternalDNSConfig_To_v1alpha1_ExternalDNSConfig(in *kops.E
 	return nil
 }
 
+// Convert_kops_ExternalDNSConfig_To_v1alpha1_ExternalDNSConfig is an autogenerated conversion function.
 func Convert_kops_ExternalDNSConfig_To_v1alpha1_ExternalDNSConfig(in *kops.ExternalDNSConfig, out *ExternalDNSConfig, s conversion.Scope) error {
 	return autoConvert_kops_ExternalDNSConfig_To_v1alpha1_ExternalDNSConfig(in, out, s)
 }

pkg/apis/kops/v1alpha2/cluster.go

@@ -109,9 +109,11 @@ type ClusterSpec struct {
 	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 []string `json:"sshAccess,omitempty"`
 
+	// NodePortAccess is a list of the CIDRs that can access the node ports range (30000-32767).
+	NodePortAccess []string `json:"nodePortAccess,omitempty"`
+
 	// HTTPProxy defines connection information to support use of a private cluster behind an forward HTTP Proxy
 	EgressProxy *EgressProxySpec `json:"egressProxy,omitempty"`
 

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

@@ -584,6 +584,7 @@ func autoConvert_v1alpha2_ClusterSpec_To_kops_ClusterSpec(in *ClusterSpec, out *
 	out.ServiceClusterIPRange = in.ServiceClusterIPRange
 	out.NonMasqueradeCIDR = in.NonMasqueradeCIDR
 	out.SSHAccess = in.SSHAccess
+	out.NodePortAccess = in.NodePortAccess
 	if in.EgressProxy != nil {
 		in, out := &in.EgressProxy, &out.EgressProxy
 		*out = new(kops.EgressProxySpec)
@@ -822,6 +823,7 @@ func autoConvert_kops_ClusterSpec_To_v1alpha2_ClusterSpec(in *kops.ClusterSpec,
 	out.ServiceClusterIPRange = in.ServiceClusterIPRange
 	out.NonMasqueradeCIDR = in.NonMasqueradeCIDR
 	out.SSHAccess = in.SSHAccess
+	out.NodePortAccess = in.NodePortAccess
 	if in.EgressProxy != nil {
 		in, out := &in.EgressProxy, &out.EgressProxy
 		*out = new(EgressProxySpec)
@@ -1275,6 +1277,7 @@ func autoConvert_v1alpha2_ExternalDNSConfig_To_kops_ExternalDNSConfig(in *Extern
 	return nil
 }
 
+// Convert_v1alpha2_ExternalDNSConfig_To_kops_ExternalDNSConfig is an autogenerated conversion function.
 func Convert_v1alpha2_ExternalDNSConfig_To_kops_ExternalDNSConfig(in *ExternalDNSConfig, out *kops.ExternalDNSConfig, s conversion.Scope) error {
 	return autoConvert_v1alpha2_ExternalDNSConfig_To_kops_ExternalDNSConfig(in, out, s)
 }
@@ -1284,6 +1287,7 @@ func autoConvert_kops_ExternalDNSConfig_To_v1alpha2_ExternalDNSConfig(in *kops.E
 	return nil
 }
 
+// Convert_kops_ExternalDNSConfig_To_v1alpha2_ExternalDNSConfig is an autogenerated conversion function.
 func Convert_kops_ExternalDNSConfig_To_v1alpha2_ExternalDNSConfig(in *kops.ExternalDNSConfig, out *ExternalDNSConfig, s conversion.Scope) error {
 	return autoConvert_kops_ExternalDNSConfig_To_v1alpha2_ExternalDNSConfig(in, out, s)
 }

pkg/apis/kops/validation/validation.go

@@ -51,11 +51,16 @@ func validateClusterSpec(spec *kops.ClusterSpec, fieldPath *field.Path) field.Er
 		allErrs = append(allErrs, validateCIDR(cidr, fieldPath.Child("sshAccess").Index(i))...)
 	}
 
-	// AdminAccess
+	// KubernetesAPIAccess
 	for i, cidr := range spec.KubernetesAPIAccess {
 		allErrs = append(allErrs, validateCIDR(cidr, fieldPath.Child("kubernetesAPIAccess").Index(i))...)
 	}
 
+	// NodePortAccess
+	for i, cidr := range spec.NodePortAccess {
+		allErrs = append(allErrs, validateCIDR(cidr, fieldPath.Child("nodePortAccess").Index(i))...)
+	}
+
 	for i := range spec.Hooks {
 		allErrs = append(allErrs, validateHookSpec(&spec.Hooks[i], fieldPath.Child("hooks").Index(i))...)
 	}

pkg/featureflag/featureflag.go

@@ -52,6 +52,9 @@ var VSphereCloudProvider = New("VSphereCloudProvider", Bool(false))
 
 var EnableExternalDNS = New("EnableExternalDNS", Bool(false))
 
+// SpecOverrideFlag allows setting spec values on create
+var SpecOverrideFlag = New("SpecOverrideFlag", Bool(false))
+
 var flags = make(map[string]*FeatureFlag)
 var flagsMutex sync.Mutex
 

pkg/model/external_access.go

@@ -71,6 +71,27 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.ModelBuilderContext) error {
 		}
 	}
 
+	for _, nodePortAccess := range b.Cluster.Spec.NodePortAccess {
+		c.AddTask(&awstasks.SecurityGroupRule{
+			Name:          s("nodeport-tcp-external-to-node-" + nodePortAccess),
+			Lifecycle:     b.Lifecycle,
+			SecurityGroup: b.LinkToSecurityGroup(kops.InstanceGroupRoleNode),
+			Protocol:      s("tcp"),
+			FromPort:      i64(30000),
+			ToPort:        i64(32767),
+			CIDR:          s(nodePortAccess),
+		})
+		c.AddTask(&awstasks.SecurityGroupRule{
+			Name:          s("nodeport-udp-external-to-node-" + nodePortAccess),
+			Lifecycle:     b.Lifecycle,
+			SecurityGroup: b.LinkToSecurityGroup(kops.InstanceGroupRoleNode),
+			Protocol:      s("udp"),
+			FromPort:      i64(30000),
+			ToPort:        i64(32767),
+			CIDR:          s(nodePortAccess),
+		})
+	}
+
 	if !b.UseLoadBalancerForAPI() {
 		// Configuration for the master, when not using a Loadbalancer (ELB)
 		// We expect that either the IP address is published, or DNS is set up to point to the IPs

pkg/model/gcemodel/external_access.go

@@ -68,6 +68,15 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.ModelBuilderContext) error {
 		})
 	}
 
+	c.AddTask(&gcetasks.FirewallRule{
+		Name:         s(b.SafeObjectName("nodeport-external-to-node")),
+		Lifecycle:    b.Lifecycle,
+		TargetTags:   []string{b.GCETagForRole(kops.InstanceGroupRoleNode)},
+		Allowed:      []string{"tcp:30000-32767,udp:30000-32767"},
+		SourceRanges: b.Cluster.Spec.NodePortAccess,
+		Network:      b.LinkToNetwork(),
+	})
+
 	if !b.UseLoadBalancerForAPI() {
 		// Configuration for the master, when not using a Loadbalancer (ELB)
 		// We expect that either the IP address is published, or DNS is set up to point to the IPs

tests/integration/complex/in-v1alpha2.yaml

@@ -27,9 +27,12 @@ spec:
   networkCIDR: 172.20.0.0/16
   networking:
     kubenet: {}
+  nodePortAccess:
+  - 1.2.3.4/32
+  - 10.20.30.0/24
   nonMasqueradeCIDR: 100.64.0.0/10
   sshAccess:
-    - 0.0.0.0/0
+  - 0.0.0.0/0
   topology:
     masters: public
     nodes: public

tests/integration/complex/kubernetes.tf

@@ -362,6 +362,42 @@ resource "aws_security_group_rule" "node-to-master-udp-1-65535" {
   protocol                 = "udp"
 }
 
+resource "aws_security_group_rule" "nodeport-tcp-external-to-node-1-2-3-4--32" {
+  type              = "ingress"
+  security_group_id = "${aws_security_group.nodes-complex-example-com.id}"
+  from_port         = 30000
+  to_port           = 32767
+  protocol          = "tcp"
+  cidr_blocks       = ["1.2.3.4/32"]
+}
+
+resource "aws_security_group_rule" "nodeport-tcp-external-to-node-10-20-30-0--24" {
+  type              = "ingress"
+  security_group_id = "${aws_security_group.nodes-complex-example-com.id}"
+  from_port         = 30000
+  to_port           = 32767
+  protocol          = "tcp"
+  cidr_blocks       = ["10.20.30.0/24"]
+}
+
+resource "aws_security_group_rule" "nodeport-udp-external-to-node-1-2-3-4--32" {
+  type              = "ingress"
+  security_group_id = "${aws_security_group.nodes-complex-example-com.id}"
+  from_port         = 30000
+  to_port           = 32767
+  protocol          = "udp"
+  cidr_blocks       = ["1.2.3.4/32"]
+}
+
+resource "aws_security_group_rule" "nodeport-udp-external-to-node-10-20-30-0--24" {
+  type              = "ingress"
+  security_group_id = "${aws_security_group.nodes-complex-example-com.id}"
+  from_port         = 30000
+  to_port           = 32767
+  protocol          = "udp"
+  cidr_blocks       = ["10.20.30.0/24"]
+}
+
 resource "aws_security_group_rule" "ssh-external-to-master-0-0-0-0--0" {
   type              = "ingress"
   security_group_id = "${aws_security_group.masters-complex-example-com.id}"

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

@@ -0,0 +1,83 @@
+apiVersion: kops/v1alpha2
+kind: Cluster
+metadata:
+  creationTimestamp: 2017-01-01T00:00:00Z
+  name: overrides.example.com
+spec:
+  api:
+    dns: {}
+  authorization:
+    alwaysAllow: {}
+  channel: stable
+  cloudProvider: aws
+  configBase: memfs://tests/overrides.example.com
+  etcdClusters:
+  - etcdMembers:
+    - instanceGroup: master-us-test-1a
+      name: a
+    name: main
+  - etcdMembers:
+    - instanceGroup: master-us-test-1a
+      name: a
+    name: events
+  iam:
+    legacy: false
+  kubernetesApiAccess:
+  - 0.0.0.0/0
+  kubernetesVersion: v1.7.5
+  masterPublicName: api.overrides.example.com
+  networkCIDR: 172.20.0.0/16
+  networking:
+    kubenet: {}
+  nodePortAccess:
+  - 1.2.3.4/32
+  - 10.20.30.0/24
+  nonMasqueradeCIDR: 100.64.0.0/10
+  sshAccess:
+  - 0.0.0.0/0
+  subnets:
+  - cidr: 172.20.32.0/19
+    name: us-test-1a
+    type: Public
+    zone: us-test-1a
+  topology:
+    dns:
+      type: Public
+    masters: public
+    nodes: public
+
+---
+
+apiVersion: kops/v1alpha2
+kind: InstanceGroup
+metadata:
+  creationTimestamp: 2017-01-01T00:00:00Z
+  labels:
+    kops.k8s.io/cluster: overrides.example.com
+  name: master-us-test-1a
+spec:
+  image: kope.io/k8s-1.7-debian-jessie-amd64-hvm-ebs-2017-07-28
+  machineType: m3.medium
+  maxSize: 1
+  minSize: 1
+  role: Master
+  subnets:
+  - us-test-1a
+
+---
+
+apiVersion: kops/v1alpha2
+kind: InstanceGroup
+metadata:
+  creationTimestamp: 2017-01-01T00:00:00Z
+  labels:
+    kops.k8s.io/cluster: overrides.example.com
+  name: nodes
+spec:
+  image: kope.io/k8s-1.7-debian-jessie-amd64-hvm-ebs-2017-07-28
+  machineType: t2.medium
+  maxSize: 2
+  minSize: 2
+  role: Node
+  subnets:
+  - us-test-1a

tests/integration/create_cluster/overrides/options.yaml

@@ -0,0 +1,8 @@
+ClusterName: overrides.example.com
+Zones:
+- us-test-1a
+Cloud: aws
+KubernetesVersion: v1.7.5
+Overrides:
+- cluster.spec.nodePortAccess=1.2.3.4/32
+- cluster.spec.nodePortAccess=10.20.30.0/24

upup/pkg/fi/dryrun_target.go

@@ -372,7 +372,7 @@ func getTaskName(t Task) string {
 	return s
 }
 
-// asString returns a human-readable string representation of the passed value
+// ValueAsString returns a human-readable string representation of the passed value
 func ValueAsString(value reflect.Value) string {
 	b := &bytes.Buffer{}