Merge pull request #11753 from johngmyers/cilium-masq

Cilium: disable masquerade by default when in ENI IPAM mode
kubernetes · Jun 14, 2021 · 9a5259c · 9a5259c
2 parents 8b285d8 + 0b7f6e3
commit 9a5259c
Show file tree

Hide file tree

Showing 12 changed files with 39 additions and 25 deletions.
diff --git a/docs/networking/cilium.md b/docs/networking/cilium.md
@@ -103,11 +103,19 @@ kops rolling-update cluster --yes
 
 ### Enabling Cilium ENI IPAM
 
-This feature is in beta state as of kOps 1.18.
+{{ kops_feature_table(kops_added_default='1.18') }}
+
+This feature is in beta state.
 
-As of kOps 1.18, you can have Cilium provision AWS managed addresses and attach them directly to Pods much like Lyft VPC and AWS VPC. See [the Cilium docs for more information](https://docs.cilium.io/en/v1.6/concepts/ipam/eni/)
+You can have Cilium provision AWS managed addresses and attach them directly to Pods much like Lyft VPC and AWS VPC. See [the Cilium docs for more information](https://docs.cilium.io/en/v1.6/concepts/ipam/eni/)
+
+```yaml
+  networking:
+    cilium:
+      ipam: eni
+```
 
-When using ENI IPAM you need to disable masquerading in Cilium as well.
+In kOps versions before 1.22, when using ENI IPAM you need to explicitly disable masquerading in Cilium as well.
 
 ```yaml
   networking:

diff --git a/pkg/apis/kops/networking.go b/pkg/apis/kops/networking.go
@@ -396,7 +396,7 @@ type CiliumNetworkingSpec struct {
 	// Setting this has no effect.
 	LogstashProbeTimer uint32 `json:"logstashProbeTimer,omitempty"`
 	// DisableMasquerade disables masquerading traffic to external destinations behind the node IP.
-	DisableMasquerade bool `json:"disableMasquerade,omitempty"`
+	DisableMasquerade *bool `json:"disableMasquerade,omitempty"`
 	// Nat6Range is not implemented and may be removed in the future.
 	// Setting this has no effect.
 	Nat46Range string `json:"nat46Range,omitempty"`

diff --git a/pkg/apis/kops/v1alpha2/networking.go b/pkg/apis/kops/v1alpha2/networking.go
@@ -394,7 +394,7 @@ type CiliumNetworkingSpec struct {
 	// Setting this has no effect.
 	LogstashProbeTimer uint32 `json:"logstashProbeTimer,omitempty"`
 	// DisableMasquerade disables masquerading traffic to external destinations behind the node IP.
-	DisableMasquerade bool `json:"disableMasquerade,omitempty"`
+	DisableMasquerade *bool `json:"disableMasquerade,omitempty"`
 	// Nat6Range is not implemented and may be removed in the future.
 	// Setting this has no effect.
 	Nat46Range string `json:"nat46Range,omitempty"`

diff --git a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go
diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go
@@ -855,7 +855,7 @@ func validateNetworkingCilium(cluster *kops.Cluster, v *kops.CiliumNetworkingSpe
 			if c.CloudProvider != string(kops.CloudProviderAWS) {
 				allErrs = append(allErrs, field.Forbidden(fldPath.Child("ipam"), "Cilum ENI IPAM is supported only in AWS"))
 			}
-			if !v.DisableMasquerade {
+			if v.DisableMasquerade != nil && !*v.DisableMasquerade {
 				allErrs = append(allErrs, field.Forbidden(fldPath.Child("disableMasquerade"), "Masquerade must be disabled when ENI IPAM is used"))
 			}
 		}

diff --git a/pkg/apis/kops/validation/validation_test.go b/pkg/apis/kops/validation/validation_test.go
@@ -790,16 +790,15 @@ func Test_Validate_Cilium(t *testing.T) {
 		},
 		{
 			Cilium: kops.CiliumNetworkingSpec{
-				DisableMasquerade: true,
-				Ipam:              "eni",
+				Ipam: "eni",
 			},
 			Spec: kops.ClusterSpec{
 				CloudProvider: "aws",
 			},
 		},
 		{
 			Cilium: kops.CiliumNetworkingSpec{
-				DisableMasquerade: true,
+				DisableMasquerade: fi.Bool(true),
 				Ipam:              "eni",
 			},
 			Spec: kops.ClusterSpec{
@@ -814,7 +813,8 @@ func Test_Validate_Cilium(t *testing.T) {
 		},
 		{
 			Cilium: kops.CiliumNetworkingSpec{
-				Ipam: "eni",
+				DisableMasquerade: fi.Bool(false),
+				Ipam:              "eni",
 			},
 			Spec: kops.ClusterSpec{
 				CloudProvider: "aws",
@@ -823,8 +823,7 @@ func Test_Validate_Cilium(t *testing.T) {
 		},
 		{
 			Cilium: kops.CiliumNetworkingSpec{
-				DisableMasquerade: true,
-				Ipam:              "eni",
+				Ipam: "eni",
 			},
 			Spec: kops.ClusterSpec{
 				CloudProvider: "gce",

diff --git a/pkg/apis/kops/zz_generated.deepcopy.go b/pkg/apis/kops/zz_generated.deepcopy.go
diff --git a/pkg/commands/set_cluster_test.go b/pkg/commands/set_cluster_test.go
@@ -288,7 +288,7 @@ func TestSetClusterFields(t *testing.T) {
 				Spec: kops.ClusterSpec{
 					Networking: &kops.NetworkingSpec{
 						Cilium: &kops.CiliumNetworkingSpec{
-							DisableMasquerade: true,
+							DisableMasquerade: fi.Bool(true),
 						},
 					},
 				},
@@ -367,7 +367,7 @@ func TestSetCiliumFields(t *testing.T) {
 						Cilium: &kops.CiliumNetworkingSpec{
 							Ipam:              "eni",
 							EnableNodePort:    true,
-							DisableMasquerade: true,
+							DisableMasquerade: fi.Bool(true),
 						},
 					},
 				},

diff --git a/pkg/model/components/cilium.go b/pkg/model/components/cilium.go
@@ -17,7 +17,6 @@ limitations under the License.
 package components
 
 import (
-	"github.com/blang/semver/v4"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/wellknownports"
@@ -43,8 +42,6 @@ func (b *CiliumOptionsBuilder) BuildOptions(o interface{}) error {
 		c.Version = "v1.10.0"
 	}
 
-	version, _ := semver.ParseTolerant(c.Version)
-
 	if c.BPFCTGlobalAnyMax == 0 {
 		c.BPFCTGlobalAnyMax = 262144
 
@@ -78,11 +75,11 @@ func (b *CiliumOptionsBuilder) BuildOptions(o interface{}) error {
 	}
 
 	if c.Ipam == "" {
-		if version.Minor >= 8 {
-			c.Ipam = "kubernetes"
-		} else {
-			c.Ipam = "hostscope"
-		}
+		c.Ipam = "kubernetes"
+	}
+
+	if c.DisableMasquerade == nil {
+		c.DisableMasquerade = fi.Bool(c.Ipam == "eni")
 	}
 
 	if c.Tunnel == "" {

diff --git a/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.8.yaml.template b/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.8.yaml.template
@@ -149,7 +149,7 @@ data:
   # - auto (automatically detect the container runtime)
   #
   container-runtime: "{{ .ContainerRuntimeLabels }}"
-  masquerade: "{{- if .DisableMasquerade -}}false{{- else -}}true{{- end -}}"
+  masquerade: "{{- if WithDefaultBool .DisableMasquerade false -}}false{{- else -}}true{{- end -}}"
   install-iptables-rules: "{{- if .IPTablesRulesNoinstall -}}false{{- else -}}true{{- end -}}"
   auto-direct-node-routes: "{{ .AutoDirectNodeRoutes }}"
   {{ if .EnableHostReachableServices }}

diff --git a/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.9.yaml.template b/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.9.yaml.template
@@ -170,7 +170,7 @@ data:
   # - auto (automatically detect the container runtime)
   #
   container-runtime: "{{ .ContainerRuntimeLabels }}"
-  masquerade: "{{- if .DisableMasquerade -}}false{{- else -}}true{{- end -}}"
+  masquerade: "{{- if WithDefaultBool .DisableMasquerade false -}}false{{- else -}}true{{- end -}}"
   install-iptables-rules: "{{- if .IPTablesRulesNoinstall -}}false{{- else -}}true{{- end -}}"
   auto-direct-node-routes: "{{ .AutoDirectNodeRoutes }}"
   {{ if .EnableHostReachableServices }}

diff --git a/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.16-v1.10.yaml.template b/upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.16-v1.10.yaml.template
@@ -170,7 +170,7 @@ data:
   # - auto (automatically detect the container runtime)
   #
   container-runtime: "{{ .ContainerRuntimeLabels }}"
-  masquerade: "{{- if .DisableMasquerade -}}false{{- else -}}true{{- end -}}"
+  masquerade: "{{- if WithDefaultBool .DisableMasquerade false -}}false{{- else -}}true{{- end -}}"
   install-iptables-rules: "{{- if .IPTablesRulesNoinstall -}}false{{- else -}}true{{- end -}}"
   auto-direct-node-routes: "{{ .AutoDirectNodeRoutes }}"
   {{ if .EnableHostReachableServices }}