Don't disable AWS src/dst checks in Calico IPv6

2022-11-25 19:54:03 -08:00 · 2022-11-25 19:54:03 -08:00 · 0424c474a3
parent c6349285d4
commit 0424c474a3
12 changed files with 24 additions and 18 deletions
--- a/k8s/crds/kops.k8s.io_clusters.yaml
+++ b/k8s/crds/kops.k8s.io_clusters.yaml
@ -4430,8 +4430,8 @@ spec:
                        type: boolean
                      awsSrcDstCheck:
                        description: 'AWSSrcDstCheck enables/disables ENI source/destination
-                          checks (AWS only) Options: Disable (default), Enable, or
-                          DoNothing'
+                          checks (AWS IPv4 only) Options: Disable (default for IPv4),
+                          Enable, or DoNothing'
                        type: string
                      bpfEnabled:
                        description: BPFEnabled enables the eBPF dataplane mode.
--- a/pkg/apis/kops/networking.go
+++ b/pkg/apis/kops/networking.go
@ -105,8 +105,8 @@ type CalicoNetworkingSpec struct {
 	// AllowIPForwarding enable ip_forwarding setting within the container namespace.
 	// (default: false)
 	AllowIPForwarding bool `json:"allowIPForwarding,omitempty"`
-	// AWSSrcDstCheck enables/disables ENI source/destination checks (AWS only)
-	// Options: Disable (default), Enable, or DoNothing
+	// AWSSrcDstCheck enables/disables ENI source/destination checks (AWS IPv4 only)
+	// Options: Disable (default for IPv4), Enable, or DoNothing
 	AWSSrcDstCheck string `json:"awsSrcDstCheck,omitempty"`
 	// BPFEnabled enables the eBPF dataplane mode.
 	BPFEnabled bool `json:"bpfEnabled,omitempty"`
--- a/pkg/apis/kops/v1alpha2/networking.go
+++ b/pkg/apis/kops/v1alpha2/networking.go
@ -108,8 +108,8 @@ type CalicoNetworkingSpec struct {
 	// AllowIPForwarding enable ip_forwarding setting within the container namespace.
 	// (default: false)
 	AllowIPForwarding bool `json:"allowIPForwarding,omitempty"`
-	// AWSSrcDstCheck enables/disables ENI source/destination checks (AWS only)
-	// Options: Disable (default), Enable, or DoNothing
+	// AWSSrcDstCheck enables/disables ENI source/destination checks (AWS IPv4 only)
+	// Options: Disable (default for IPv4), Enable, or DoNothing
 	AWSSrcDstCheck string `json:"awsSrcDstCheck,omitempty"`
 	// BPFEnabled enables the eBPF dataplane mode.
 	BPFEnabled bool `json:"bpfEnabled,omitempty"`
--- a/pkg/apis/kops/v1alpha3/networking.go
+++ b/pkg/apis/kops/v1alpha3/networking.go
@ -105,8 +105,8 @@ type CalicoNetworkingSpec struct {
 	// AllowIPForwarding enable ip_forwarding setting within the container namespace.
 	// (default: false)
 	AllowIPForwarding bool `json:"allowIPForwarding,omitempty"`
-	// AWSSrcDstCheck enables/disables ENI source/destination checks (AWS only)
-	// Options: Disable (default), Enable, or DoNothing
+	// AWSSrcDstCheck enables/disables ENI source/destination checks (AWS IPv4 only)
+	// Options: Disable (default for IPv4), Enable, or DoNothing
 	AWSSrcDstCheck string `json:"awsSrcDstCheck,omitempty"`
 	// BPFEnabled enables the eBPF dataplane mode.
 	BPFEnabled bool `json:"bpfEnabled,omitempty"`
--- a/pkg/apis/kops/validation/validation.go
+++ b/pkg/apis/kops/validation/validation.go
@ -1223,8 +1223,12 @@ func validateNetworkingCalico(c *kops.ClusterSpec, v *kops.CalicoNetworkingSpec,
 	allErrs := field.ErrorList{}

 	if v.AWSSrcDstCheck != "" {
-		valid := []string{"Enable", "Disable", "DoNothing"}
-		allErrs = append(allErrs, IsValidValue(fldPath.Child("awsSrcDstCheck"), &v.AWSSrcDstCheck, valid)...)
+		if c.IsIPv6Only() && v.AWSSrcDstCheck != "DoNothing" {
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("awsSrcDstCheck"), "awsSrcDstCheck may only be \"DoNothing\" for IPv6 clusters"))
+		} else {
+			valid := []string{"Enable", "Disable", "DoNothing"}
+			allErrs = append(allErrs, IsValidValue(fldPath.Child("awsSrcDstCheck"), &v.AWSSrcDstCheck, valid)...)
+		}
 	}

 	if v.CrossSubnet != nil {
--- a/pkg/apis/kops/validation/validation_test.go
+++ b/pkg/apis/kops/validation/validation_test.go
@ -602,6 +602,7 @@ func Test_Validate_Calico(t *testing.T) {
 		{
 			Description: "AWS source/destination checks off",
 			Input: caliInput{
+				Cluster: &kops.ClusterSpec{},
 				Calico: &kops.CalicoNetworkingSpec{
 					AWSSrcDstCheck: "off",
 				},
@ -611,6 +612,7 @@ func Test_Validate_Calico(t *testing.T) {
 		{
 			Description: "AWS source/destination checks enabled",
 			Input: caliInput{
+				Cluster: &kops.ClusterSpec{},
 				Calico: &kops.CalicoNetworkingSpec{
 					AWSSrcDstCheck: "Enable",
 				},
@ -619,6 +621,7 @@ func Test_Validate_Calico(t *testing.T) {
 		{
 			Description: "AWS source/destination checks disabled",
 			Input: caliInput{
+				Cluster: &kops.ClusterSpec{},
 				Calico: &kops.CalicoNetworkingSpec{
 					AWSSrcDstCheck: "Disable",
 				},
@ -627,6 +630,7 @@ func Test_Validate_Calico(t *testing.T) {
 		{
 			Description: "AWS source/destination checks left as is",
 			Input: caliInput{
+				Cluster: &kops.ClusterSpec{},
 				Calico: &kops.CalicoNetworkingSpec{
 					AWSSrcDstCheck: "DoNothing",
 				},
--- a/pkg/model/iam/iam_builder.go
+++ b/pkg/model/iam/iam_builder.go
@ -373,7 +373,7 @@ func (r *NodeRoleAPIServer) BuildAWSPolicy(b *PolicyBuilder) (*Policy, error) {
 		addCiliumEniPermissions(p)
 	}

-	if b.Cluster.Spec.Networking != nil && b.Cluster.Spec.Networking.Calico != nil && b.Cluster.Spec.Networking.Calico.AWSSrcDstCheck != "DoNothing" {
+	if b.Cluster.Spec.Networking != nil && b.Cluster.Spec.Networking.Calico != nil && b.Cluster.Spec.Networking.Calico.AWSSrcDstCheck != "DoNothing" && !b.Cluster.Spec.IsIPv6Only() {
 		addCalicoSrcDstCheckPermissions(p)
 	}

@ -452,7 +452,7 @@ func (r *NodeRoleMaster) BuildAWSPolicy(b *PolicyBuilder) (*Policy, error) {
 		addCiliumEniPermissions(p)
 	}

-	if b.Cluster.Spec.Networking != nil && b.Cluster.Spec.Networking.Calico != nil && b.Cluster.Spec.Networking.Calico.AWSSrcDstCheck != "DoNothing" {
+	if b.Cluster.Spec.Networking != nil && b.Cluster.Spec.Networking.Calico != nil && b.Cluster.Spec.Networking.Calico.AWSSrcDstCheck != "DoNothing" && !b.Cluster.Spec.IsIPv6Only() {
 		addCalicoSrcDstCheckPermissions(p)
 	}

@ -478,7 +478,7 @@ func (r *NodeRoleNode) BuildAWSPolicy(b *PolicyBuilder) (*Policy, error) {
 		addAmazonVPCCNIPermissions(p)
 	}

-	if b.Cluster.Spec.Networking != nil && b.Cluster.Spec.Networking.Calico != nil && b.Cluster.Spec.Networking.Calico.AWSSrcDstCheck != "DoNothing" {
+	if b.Cluster.Spec.Networking != nil && b.Cluster.Spec.Networking.Calico != nil && b.Cluster.Spec.Networking.Calico.AWSSrcDstCheck != "DoNothing" && !b.Cluster.Spec.IsIPv6Only() {
 		addCalicoSrcDstCheckPermissions(p)
 	}

--- a/tests/integration/update_cluster/minimal-ipv6-calico/data/aws_iam_role_policy_masters.minimal-ipv6.example.com_policy
+++ b/tests/integration/update_cluster/minimal-ipv6-calico/data/aws_iam_role_policy_masters.minimal-ipv6.example.com_policy
@ -185,7 +185,6 @@
        "ec2:DescribeVolumes",
        "ec2:DescribeVolumesModifications",
        "ec2:DescribeVpcs",
-        "ec2:ModifyNetworkInterfaceAttribute",
        "elasticloadbalancing:DescribeListeners",
        "elasticloadbalancing:DescribeLoadBalancerAttributes",
        "elasticloadbalancing:DescribeLoadBalancerPolicies",
--- a/tests/integration/update_cluster/minimal-ipv6-calico/data/aws_iam_role_policy_nodes.minimal-ipv6.example.com_policy
+++ b/tests/integration/update_cluster/minimal-ipv6-calico/data/aws_iam_role_policy_nodes.minimal-ipv6.example.com_policy
@ -19,7 +19,6 @@
        "ec2:DescribeInstanceTypes",
        "ec2:DescribeInstances",
        "ec2:DescribeRegions",
-        "ec2:ModifyNetworkInterfaceAttribute",
        "iam:GetServerCertificate",
        "iam:ListServerCertificates",
        "kms:GenerateRandom"
--- a/tests/integration/update_cluster/minimal-ipv6-calico/data/aws_s3_object_minimal-ipv6.example.com-addons-bootstrap_content
+++ b/tests/integration/update_cluster/minimal-ipv6-calico/data/aws_s3_object_minimal-ipv6.example.com-addons-bootstrap_content
@ -55,7 +55,7 @@ spec:
    version: 9.99.0
  - id: k8s-1.25
    manifest: networking.projectcalico.org/k8s-1.25.yaml
-    manifestHash: decc34a7f133bfc9bed78456047eccd399a67a4e17c49259e06f487a4b74092e
+    manifestHash: 87046f7e66df6437ddf0a74ce7871dad02e0961a4a47c36a0fd6fff6434aac41
    name: networking.projectcalico.org
    prune:
      kinds:
--- a/tests/integration/update_cluster/minimal-ipv6-calico/data/aws_s3_object_minimal-ipv6.example.com-addons-networking.projectcalico.org-k8s-1.25_content
+++ b/tests/integration/update_cluster/minimal-ipv6-calico/data/aws_s3_object_minimal-ipv6.example.com-addons-networking.projectcalico.org-k8s-1.25_content
@ -4547,7 +4547,7 @@ spec:
        - name: FELIX_HEALTHENABLED
          value: "true"
        - name: FELIX_AWSSRCDSTCHECK
-          value: Disable
+          value: DoNothing
        - name: FELIX_BPFENABLED
          value: "false"
        - name: FELIX_BPFEXTERNALSERVICEMODE
--- a/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.25.yaml.template
+++ b/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.25.yaml.template
@ -4661,7 +4661,7 @@ spec:
            # kops additions
            # Enable source/destination checks for AWS
            - name: FELIX_AWSSRCDSTCHECK
-              value: "{{- if eq GetCloudProvider "aws" -}}{{- or .Networking.Calico.AWSSrcDstCheck "Disable" -}}{{- else -}}DoNothing{{- end -}}"
+              value: "{{- if and (eq GetCloudProvider "aws") (not IsIPv6Only) -}}{{- or .Networking.Calico.AWSSrcDstCheck "Disable" -}}{{- else -}}DoNothing{{- end -}}"
            # Enable eBPF dataplane mode
            - name: FELIX_BPFENABLED
              value: "{{ .Networking.Calico.BPFEnabled }}"