Merge pull request #10019 from monicagangwar/master

[calico] awsSrcDstCheck to disable src/dest checks in AWS

docs/networking/calico.md

@@ -55,11 +55,18 @@ To enable this mode in a cluster, add the following to the cluster spec:
     calico:
       crossSubnet: true
 ```
-
 In the case of AWS, EC2 instances have source/destination checks enabled by default.
-When you enable cross-subnet mode in kops, an addon controller ([k8s-ec2-srcdst](https://github.com/ottoyiu/k8s-ec2-srcdst))
+When you enable cross-subnet mode in kops 1.19+, it is equivalent to: 
+```yaml
+  networking:
+    calico:
+      awsSrcDstCheck: Disable
+      IPIPMode: CrossSubnet
+```
+An IAM policy will be added to all nodes to allow Calico to execute `ec2:DescribeInstances` and `ec2:ModifyNetworkInterfaceAttribute`, as required when [awsSrcDstCheck](https://docs.projectcalico.org/reference/resources/felixconfig#spec) is set.
+For older versions of kops, an addon controller ([k8s-ec2-srcdst](https://github.com/ottoyiu/k8s-ec2-srcdst))
 will be deployed as a Pod (which will be scheduled on one of the masters) to facilitate the disabling of said source/destination address checks.
-Only the masters have the IAM policy (`ec2:*`) to allow k8s-ec2-srcdst to execute `ec2:ModifyInstanceAttribute`.
+Only the control plane nodes have an IAM policy to allow k8s-ec2-srcdst to execute `ec2:ModifyInstanceAttribute`.
 
 ### Configuring Calico MTU
 

k8s/crds/kops.k8s.io_clusters.yaml

@@ -2122,6 +2122,9 @@ spec:
                   calico:
                     description: CalicoNetworkingSpec declares that we want Calico networking
                     properties:
+                      awsSrcDstCheck:
+                        description: 'AwsSrcDstCheck enables/disables source/destination checks (AWS only) Options: "DoNothing" (default) , "Enable" or "Disable"'
+                        type: string
                       chainInsertMode:
                         description: 'ChainInsertMode controls whether Felix inserts rules to the top of iptables chains, or appends to the bottom. Leaving the default option is safest to prevent accidentally breaking connectivity. Default: ''insert'' (other options: ''append'')'
                         type: string

pkg/apis/kops/networking.go

@@ -108,6 +108,9 @@ type CalicoNetworkingSpec struct {
 	CPURequest *resource.Quantity `json:"cpuRequest,omitempty"`
 	// CrossSubnet enables Calico's cross-subnet mode when set to true
 	CrossSubnet bool `json:"crossSubnet,omitempty"`
+	// AwsSrcDstCheck enables/disables source/destination checks (AWS only)
+	// Options: "DoNothing" (default) , "Enable" or "Disable"
+	AwsSrcDstCheck string `json:"awsSrcDstCheck,omitempty"`
 	// LogSeverityScreen lets us set the desired log level. (Default: info)
 	LogSeverityScreen string `json:"logSeverityScreen,omitempty"`
 	// MTU to be set in the cni-network-config for calico.

pkg/apis/kops/v1alpha2/networking.go

@@ -108,6 +108,9 @@ type CalicoNetworkingSpec struct {
 	CPURequest *resource.Quantity `json:"cpuRequest,omitempty"`
 	// CrossSubnet enables Calico's cross-subnet mode when set to true
 	CrossSubnet bool `json:"crossSubnet,omitempty"`
+	// AwsSrcDstCheck enables/disables source/destination checks (AWS only)
+	// Options: "DoNothing" (default) , "Enable" or "Disable"
+	AwsSrcDstCheck string `json:"awsSrcDstCheck,omitempty"`
 	// LogSeverityScreen lets us set the desired log level. (Default: info)
 	LogSeverityScreen string `json:"logSeverityScreen,omitempty"`
 	// MTU to be set in the cni-network-config for calico.

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

@@ -1318,6 +1318,7 @@ func autoConvert_v1alpha2_CalicoNetworkingSpec_To_kops_CalicoNetworkingSpec(in *
 	out.ChainInsertMode = in.ChainInsertMode
 	out.CPURequest = in.CPURequest
 	out.CrossSubnet = in.CrossSubnet
+	out.AwsSrcDstCheck = in.AwsSrcDstCheck
 	out.LogSeverityScreen = in.LogSeverityScreen
 	out.MTU = in.MTU
 	out.PrometheusMetricsEnabled = in.PrometheusMetricsEnabled
@@ -1344,6 +1345,7 @@ func autoConvert_kops_CalicoNetworkingSpec_To_v1alpha2_CalicoNetworkingSpec(in *
 	out.ChainInsertMode = in.ChainInsertMode
 	out.CPURequest = in.CPURequest
 	out.CrossSubnet = in.CrossSubnet
+	out.AwsSrcDstCheck = in.AwsSrcDstCheck
 	out.LogSeverityScreen = in.LogSeverityScreen
 	out.MTU = in.MTU
 	out.PrometheusMetricsEnabled = in.PrometheusMetricsEnabled

pkg/apis/kops/validation/validation.go

@@ -966,6 +966,11 @@ func validateNetworkingCalico(v *kops.CalicoNetworkingSpec, e kops.EtcdClusterSp
 		allErrs = append(allErrs, IsValidValue(fldPath.Child("chainInsertMode"), &v.ChainInsertMode, valid)...)
 	}
 
+	if v.AwsSrcDstCheck != "" {
+		valid := []string{"Enable", "Disable", "DoNothing"}
+		allErrs = append(allErrs, IsValidValue(fldPath.Child("awsSrcDstCheck"), &v.AwsSrcDstCheck, valid)...)
+	}
+
 	if v.IptablesBackend != "" {
 		valid := []string{"Auto", "Legacy", "NFT"}
 		allErrs = append(allErrs, IsValidValue(fldPath.Child("iptablesBackend"), &v.IptablesBackend, valid)...)

pkg/apis/kops/validation/validation_test.go

@@ -516,6 +516,39 @@ func Test_Validate_Calico(t *testing.T) {
 			},
 			ExpectedErrors: []string{"Invalid value::calico.ipv4AutoDetectionMethod"},
 		},
+		{
+			Input: caliInput{
+				Calico: &kops.CalicoNetworkingSpec{
+					AwsSrcDstCheck: "off",
+				},
+				Etcd: kops.EtcdClusterSpec{},
+			},
+			ExpectedErrors: []string{"Unsupported value::calico.awsSrcDstCheck"},
+		},
+		{
+			Input: caliInput{
+				Calico: &kops.CalicoNetworkingSpec{
+					AwsSrcDstCheck: "Enable",
+				},
+				Etcd: kops.EtcdClusterSpec{},
+			},
+		},
+		{
+			Input: caliInput{
+				Calico: &kops.CalicoNetworkingSpec{
+					AwsSrcDstCheck: "Disable",
+				},
+				Etcd: kops.EtcdClusterSpec{},
+			},
+		},
+		{
+			Input: caliInput{
+				Calico: &kops.CalicoNetworkingSpec{
+					AwsSrcDstCheck: "DoNothing",
+				},
+				Etcd: kops.EtcdClusterSpec{},
+			},
+		},
 	}
 	for _, g := range grid {
 		errs := validateNetworkingCalico(g.Input.Calico, g.Input.Etcd, field.NewPath("calico"))

pkg/model/iam/iam_builder.go

@@ -275,6 +275,10 @@ func (r *NodeRoleMaster) BuildAWSPolicy(b *PolicyBuilder) (*Policy, error) {
 		addCiliumEniPermissions(p, resource, b.Cluster.Spec.IAM.Legacy)
 	}
 
+	if b.Cluster.Spec.Networking != nil && b.Cluster.Spec.Networking.Calico != nil && b.Cluster.Spec.Networking.Calico.AwsSrcDstCheck != "" {
+		addCalicoSrcDstCheckPermissions(p)
+	}
+
 	return p, nil
 }
 
@@ -310,6 +314,10 @@ func (r *NodeRoleNode) BuildAWSPolicy(b *PolicyBuilder) (*Policy, error) {
 		addLyftVPCPermissions(p, resource, b.Cluster.Spec.IAM.Legacy, b.Cluster.GetName())
 	}
 
+	if b.Cluster.Spec.Networking != nil && b.Cluster.Spec.Networking.Calico != nil && b.Cluster.Spec.Networking.Calico.AwsSrcDstCheck != "" {
+		addCalicoSrcDstCheckPermissions(p)
+	}
+
 	return p, nil
 }
 
@@ -667,6 +675,17 @@ func addECRPermissions(p *Policy) {
 	})
 }
 
+func addCalicoSrcDstCheckPermissions(p *Policy) {
+	p.Statement = append(p.Statement, &Statement{
+		Effect: StatementEffectAllow,
+		Action: stringorslice.Of(
+			"ec2:DescribeInstances",
+			"ec2:ModifyNetworkInterfaceAttribute",
+		),
+		Resource: stringorslice.Slice([]string{"*"}),
+	})
+}
+
 // addLegacyDNSControllerPermissions adds legacy IAM permissions used by the node roles.
 func addLegacyDNSControllerPermissions(b *PolicyBuilder, p *Policy) {
 	// Legacy IAM permissions for node roles

upup/models/bindata.go

@@ -12873,6 +12873,9 @@ spec:
             # Enable Prometheus process metrics collection
             - name: FELIX_PROMETHEUSPROCESSMETRICSENABLED
               value: "{{- or .Networking.Calico.PrometheusProcessMetricsEnabled "true" }}"
+            # Enable / Disable source/destination checks in AWS
+            - name: FELIX_AWSSRCDSTCHECK
+              value: "{{- if and (eq .CloudProvider "aws") (.Networking.Calico.CrossSubnet) -}}Disable{{- else -}} {{- or .Networking.Calico.AwsSrcDstCheck "DoNothing" -}} {{- end -}}"
           securityContext:
             privileged: true
           resources:
@@ -13042,6 +13045,7 @@ metadata:
 # pod) may not match the receiving machine's address.
 #
 # This only applies for AWS environments.
+# This is a deprecated setting, use awsSrcDstCheck instead
 ---
 
 kind: ClusterRole
@@ -13099,7 +13103,7 @@ metadata:
     k8s-app: k8s-ec2-srcdst
     role.kubernetes.io/networking: "1"
 spec:
-  replicas: 1
+  replicas: 0
   selector:
     matchLabels:
       k8s-app: k8s-ec2-srcdst

upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.16.yaml.template

@@ -3893,6 +3893,9 @@ spec:
             # Enable Prometheus process metrics collection
             - name: FELIX_PROMETHEUSPROCESSMETRICSENABLED
               value: "{{- or .Networking.Calico.PrometheusProcessMetricsEnabled "true" }}"
+            # Enable / Disable source/destination checks in AWS
+            - name: FELIX_AWSSRCDSTCHECK
+              value: "{{- if and (eq .CloudProvider "aws") (.Networking.Calico.CrossSubnet) -}}Disable{{- else -}} {{- or .Networking.Calico.AwsSrcDstCheck "DoNothing" -}} {{- end -}}"
           securityContext:
             privileged: true
           resources:
@@ -4062,6 +4065,7 @@ metadata:
 # pod) may not match the receiving machine's address.
 #
 # This only applies for AWS environments.
+# This is a deprecated setting, use awsSrcDstCheck instead
 ---
 
 kind: ClusterRole
@@ -4119,7 +4123,7 @@ metadata:
     k8s-app: k8s-ec2-srcdst
     role.kubernetes.io/networking: "1"
 spec:
-  replicas: 1
+  replicas: 0
   selector:
     matchLabels:
       k8s-app: k8s-ec2-srcdst

upup/pkg/fi/cloudup/bootstrapchannelbuilder.go

@@ -837,7 +837,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.ModelBuilderContext) (*chann
 			"k8s-1.7":    "2.6.12-kops.1",
 			"k8s-1.7-v3": "3.8.0-kops.2",
 			"k8s-1.12":   "3.9.6-kops.1",
-			"k8s-1.16":   "3.16.1-kops.2",
+			"k8s-1.16":   "3.16.1-kops.3",
 		}
 
 		{