Merge pull request #11753 from johngmyers/cilium-masq

Cilium: disable masquerade by default when in ENI IPAM mode

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') }}
 
-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/)
+This feature is in beta state.
 
-When using ENI IPAM you need to disable masquerading in Cilium as well.
+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
+```
+
+In kOps versions before 1.22, when using ENI IPAM you need to explicitly disable masquerading in Cilium as well.
 
 ```yaml
   networking:

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"`

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"`

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

@@ -531,6 +531,11 @@ func (in *CiliumNetworkingSpec) DeepCopyInto(out *CiliumNetworkingSpec) {
 			(*out)[key] = val
 		}
 	}
+	if in.DisableMasquerade != nil {
+		in, out := &in.DisableMasquerade, &out.DisableMasquerade
+		*out = new(bool)
+		**out = **in
+	}
 	if in.EnableRemoteNodeIdentity != nil {
 		in, out := &in.EnableRemoteNodeIdentity, &out.EnableRemoteNodeIdentity
 		*out = new(bool)

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"))
 			}
 		}

pkg/apis/kops/validation/validation_test.go

@@ -790,8 +790,7 @@ func Test_Validate_Cilium(t *testing.T) {
 		},
 		{
 			Cilium: kops.CiliumNetworkingSpec{
-				DisableMasquerade: true,
-				Ipam:              "eni",
+				Ipam: "eni",
 			},
 			Spec: kops.ClusterSpec{
 				CloudProvider: "aws",
@@ -799,7 +798,7 @@ func Test_Validate_Cilium(t *testing.T) {
 		},
 		{
 			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",

pkg/apis/kops/zz_generated.deepcopy.go

@@ -615,6 +615,11 @@ func (in *CiliumNetworkingSpec) DeepCopyInto(out *CiliumNetworkingSpec) {
 			(*out)[key] = val
 		}
 	}
+	if in.DisableMasquerade != nil {
+		in, out := &in.DisableMasquerade, &out.DisableMasquerade
+		*out = new(bool)
+		**out = **in
+	}
 	if in.EnableRemoteNodeIdentity != nil {
 		in, out := &in.EnableRemoteNodeIdentity, &out.EnableRemoteNodeIdentity
 		*out = new(bool)

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),
 						},
 					},
 				},

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 == "" {

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 }}

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 }}

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 }}