refactor

api/v1/installation_types.go

@@ -403,8 +403,11 @@ const (
 	WindowsDataplaneHNS      WindowsDataplaneOption = "HNS"
 )
 
-// +kubebuilder:validation:Enum=net.ipv4.tcp_keepalive_intvl;net.ipv4.tcp_keepalive_probes;net.ipv4.tcp_keepalive_time
-type SysctlTuningKey string
+type Sysctl struct {
+	// +kubebuilder:validation:Enum=net.ipv4.tcp_keepalive_intvl;net.ipv4.tcp_keepalive_probes;net.ipv4.tcp_keepalive_time
+	Key   string `json:"key"`
+	Value string `json:"value"`
+}
 
 // CalicoNetworkSpec specifies configuration options for Calico provided pod networking.
 type CalicoNetworkSpec struct {
@@ -467,9 +470,9 @@ type CalicoNetworkSpec struct {
 	// +kubebuilder:validation:Enum=Enabled;Disabled
 	ContainerIPForwarding *ContainerIPForwardingType `json:"containerIPForwarding,omitempty"`
 
-	// SysctlTuning configures sysctl parameters for tuning plugin
+	// Sysctl configures sysctl parameters for tuning plugin
 	// +optional
-	SysctlTuning *map[SysctlTuningKey]string `json:"sysctlTuning,omitempty"`
+	Sysctl []Sysctl `json:"sysctl,omitempty"`
 }
 
 // NodeAddressAutodetection provides configuration options for auto-detecting node addresses. At most one option

pkg/controller/installation/validation.go

@@ -28,6 +28,7 @@ import (
 	kubecontrollers "github.com/tigera/operator/pkg/common/validation/kube-controllers"
 	typha "github.com/tigera/operator/pkg/common/validation/typha"
 	"github.com/tigera/operator/pkg/controller/k8sapi"
+	"github.com/tigera/operator/pkg/controller/utils"
 	"github.com/tigera/operator/pkg/render"
 	appsv1 "k8s.io/api/apps/v1"
 
@@ -337,26 +338,12 @@ func validateCustomResource(instance *operatorv1.Installation) error {
 			}
 		}
 
-		type TuningSpec struct {
-			Sysctl *map[operatorv1.SysctlTuningKey]string `json:"sysctl,omitempty"`
-			Type   string                                 `json:"type"`
-		}
-
-		if instance.Spec.CalicoNetwork.SysctlTuning != nil {
+		if instance.Spec.CalicoNetwork.Sysctl != nil {
 			// CNI tuning plugin
-			pluginData := instance.Spec.CalicoNetwork.SysctlTuning
-
-			// sysctl settings
-			allowedKeys := map[operatorv1.SysctlTuningKey]struct{}{
-				"net.ipv4.tcp_keepalive_intvl":  {},
-				"net.ipv4.tcp_keepalive_probes": {},
-				"net.ipv4.tcp_keepalive_time":   {},
-			}
+			pluginData := instance.Spec.CalicoNetwork.Sysctl
 
-			for key, value := range *pluginData {
-				if _, allowed := allowedKeys[key]; !allowed {
-					return fmt.Errorf("value %s not allowed in spec.calicoNetwork.sysctlTuning", value)
-				}
+			if err := utils.VerifySysctl(pluginData); err != nil {
+				return err
 			}
 
 		}

pkg/controller/installation/validation_test.go

@@ -411,11 +411,30 @@ var _ = Describe("Installation validation tests", func() {
 		Expect(err).To(HaveOccurred())
 	})
 
+	It("should pass on allowed CNI sysctl tuning plugin config", func() {
+		instance.Spec.CalicoNetwork.Sysctl = []operator.Sysctl{
+			{
+				Key:   "net.ipv4.tcp_keepalive_intvl",
+				Value: "15",
+			}, {
+				Key:   "net.ipv4.tcp_keepalive_probes",
+				Value: "6",
+			},
+			{
+				Key:   "net.ipv4.tcp_keepalive_time",
+				Value: "40",
+			},
+		}
+		err := validateCustomResource(instance)
+		Expect(err).ShouldNot(HaveOccurred())
+	})
+
 	It("should error on not-allowed CNI sysctl tuning plugin config", func() {
-		instance.Spec.CalicoNetwork.SysctlTuning = &map[operator.SysctlTuningKey]string{
-			"net.ipv4.not_allowed_setting":  "15",
-			"net.ipv4.tcp_keepalive_probes": "6",
-			"net.ipv4.tcp_keepalive_time":   "40",
+		instance.Spec.CalicoNetwork.Sysctl = []operator.Sysctl{
+			{
+				Key:   "net.ipv4.not_allowed_parameter",
+				Value: "1",
+			},
 		}
 		err := validateCustomResource(instance)
 		Expect(err).To(HaveOccurred())

pkg/controller/migration/convert/network.go

@@ -24,6 +24,7 @@ import (
 	operatorv1 "github.com/tigera/operator/api/v1"
 	v1 "github.com/tigera/operator/api/v1"
 	"github.com/tigera/operator/pkg/controller/migration/cni"
+	"github.com/tigera/operator/pkg/controller/utils"
 )
 
 const (
@@ -200,8 +201,8 @@ func handleCalicoCNI(c *components, install *operatorv1.Installation) error {
 	}
 
 	type TuningSpec struct {
-		Sysctl *map[operatorv1.SysctlTuningKey]string `json:"sysctl,omitempty"`
-		Type   string                                 `json:"type"`
+		Sysctl []operatorv1.Sysctl `json:"sysctl,omitempty"`
+		Type   string              `json:"type"`
 	}
 
 	// CNI tuning plugin
@@ -217,21 +218,19 @@ func handleCalicoCNI(c *components, install *operatorv1.Installation) error {
 			}
 		}
 
-		// sysctl settings
-		allowedKeys := map[operatorv1.SysctlTuningKey]struct{}{
-			"net.ipv4.tcp_keepalive_intvl":  {},
-			"net.ipv4.tcp_keepalive_probes": {},
-			"net.ipv4.tcp_keepalive_time":   {},
-		}
-		sysctlTuning := make(map[operatorv1.SysctlTuningKey]string)
-		for key, value := range *tuningSpecData.Sysctl {
-			if _, allowed := allowedKeys[key]; allowed {
-				sysctlTuning[key] = value
+		sysctlTuning := []operatorv1.Sysctl{}
+		for _, setting := range tuningSpecData.Sysctl {
+			if _, allowed := utils.AllowedSysctlKeys[setting.Key]; allowed {
+				sysctl := operatorv1.Sysctl{
+					Key:   setting.Key,
+					Value: setting.Value,
+				}
+				sysctlTuning = append(sysctlTuning, sysctl)
 			}
 		}
 
 		if len(sysctlTuning) > 0 {
-			install.Spec.CalicoNetwork.SysctlTuning = &sysctlTuning
+			install.Spec.CalicoNetwork.Sysctl = sysctlTuning
 		}
 	}
 

pkg/controller/migration/convert/network_test.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2022 Tigera, Inc. All rights reserved.
+// Copyright (c) 2023 Tigera, Inc. All rights reserved.
 
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -782,11 +782,20 @@ var _ = Describe("Convert network tests", func() {
   },
   {
 	"type": "tuning",
-	"sysctl": {
-		"net.ipv4.tcp_keepalive_intvl": "15",
-		"net.ipv4.tcp_keepalive_probes": "6",
-		"net.ipv4.tcp_keepalive_time": "40"
-	}
+	"sysctl": [
+		  {
+			"key": "net.ipv4.tcp_keepalive_intvl",
+			"value": "15"
+		  },
+		  {
+			"key": "net.ipv4.tcp_keepalive_probes",
+			"value": "6"
+		  },
+		  {
+			"key": "net.ipv4.tcp_keepalive_time",
+			"value": "40"
+		  }
+		]
   }
   ]
 }`,
@@ -795,15 +804,23 @@ var _ = Describe("Convert network tests", func() {
 					cfg, err := Convert(ctx, c)
 					Expect(err).ToNot(HaveOccurred())
 					Expect(cfg).ToNot(BeNil())
-					Expect(cfg.Spec.CalicoNetwork.SysctlTuning).ToNot(BeNil())
-					Expect(*cfg.Spec.CalicoNetwork.SysctlTuning).To(Equal(map[string]string{
-						"net.ipv4.tcp_keepalive_time":   "40",
-						"net.ipv4.tcp_keepalive_intvl":  "15",
-						"net.ipv4.tcp_keepalive_probes": "6",
+					Expect(cfg.Spec.CalicoNetwork.Sysctl).ToNot(BeNil())
+					Expect(cfg.Spec.CalicoNetwork.Sysctl).To(Equal([]operatorv1.Sysctl{
+						{
+							Key:   "net.ipv4.tcp_keepalive_intvl",
+							Value: "15",
+						}, {
+							Key:   "net.ipv4.tcp_keepalive_probes",
+							Value: "6",
+						},
+						{
+							Key:   "net.ipv4.tcp_keepalive_time",
+							Value: "40",
+						},
 					}))
 				})
 
-				It("no sysctl tuning in config, cfg must be nil", func() {
+				It("not allowed sysctl tuning in config", func() {
 					ds := emptyNodeSpec()
 					ds.Spec.Template.Spec.InitContainers[0].Env = []corev1.EnvVar{{
 						Name: "CNI_NETWORK_CONFIG",
@@ -829,21 +846,24 @@ var _ = Describe("Convert network tests", func() {
 },
 {
 	"type": "tuning",
-	"sysctl": {
-		"net.ipv4.not_allowed": "40"
-	}
+	"sysctl": [
+		{
+		  "key": "net.ipv4.not_allowed",
+		  "value": "40"
+		}
+	]
   }
 ]
 }`,
 					}}
 					c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ds, emptyKubeControllerSpec(), v4pool, emptyFelixConfig()).Build()
 					cfg, err := Convert(ctx, c)
-					Expect(err).ToNot(HaveOccurred())
+					Expect(err).To(HaveOccurred())
 					Expect(cfg).ToNot(BeNil())
-					Expect(cfg.Spec.CalicoNetwork.SysctlTuning).To(BeNil())
+					Expect(cfg.Spec.CalicoNetwork.Sysctl).To(BeNil())
 				})
 
-				It("not allowed sysctl tuning in config", func() {
+				It("no sysctl tuning in config, cfg must be nil", func() {
 					ds := emptyNodeSpec()
 					ds.Spec.Template.Spec.InitContainers[0].Env = []corev1.EnvVar{{
 						Name: "CNI_NETWORK_CONFIG",
@@ -874,7 +894,7 @@ var _ = Describe("Convert network tests", func() {
 					cfg, err := Convert(ctx, c)
 					Expect(err).ToNot(HaveOccurred())
 					Expect(cfg).ToNot(BeNil())
-					Expect(cfg.Spec.CalicoNetwork.SysctlTuning).To(BeNil())
+					Expect(cfg.Spec.CalicoNetwork.Sysctl).To(BeNil())
 				})
 			})
 

pkg/controller/utils/merge.go

@@ -303,9 +303,9 @@ func mergeCalicoNetwork(cfg, override *operatorv1.CalicoNetworkSpec) *operatorv1
 		out.ContainerIPForwarding = override.ContainerIPForwarding
 	}
 
-	switch compareFields(out.SysctlTuning, override.SysctlTuning) {
+	switch compareFields(out.Sysctl, override.Sysctl) {
 	case BOnlySet, Different:
-		out.SysctlTuning = override.SysctlTuning
+		out.Sysctl = override.Sysctl
 	}
 	return out
 }

pkg/controller/utils/merge_test.go

@@ -479,26 +479,34 @@ var _ = Describe("Installation merge tests", func() {
 				}),
 		)
 
-		_sysctlTuningA := map[opv1.SysctlTuningKey]string{
-			"net.ipv4.tcp_keepalive_time":   "40",
-			"net.ipv4.tcp_keepalive_intvl":  "15",
-			"net.ipv4.tcp_keepalive_probes": "6",
+		_sysctlTuningA := []opv1.Sysctl{
+			{
+				Key:   "net.ipv4.tcp_keepalive_intvl",
+				Value: "15",
+			}, {
+				Key:   "net.ipv4.tcp_keepalive_probes",
+				Value: "6",
+			},
+			{
+				Key:   "net.ipv4.tcp_keepalive_time",
+				Value: "40",
+			},
 		}
-		_sysctlTuningB := map[opv1.SysctlTuningKey]string{}
-		DescribeTable("merge CNI Tuning", func(main, second, expect *map[opv1.SysctlTuningKey]string) {
+		_sysctlTuningB := []opv1.Sysctl{}
+		DescribeTable("merge CNI Tuning", func(main, second, expect []opv1.Sysctl) {
 			m := opv1.InstallationSpec{}
 			s := opv1.InstallationSpec{}
 			if main != nil {
-				m.CalicoNetwork = &opv1.CalicoNetworkSpec{SysctlTuning: main}
+				m.CalicoNetwork = &opv1.CalicoNetworkSpec{Sysctl: main}
 			}
 			if second != nil {
-				s.CalicoNetwork = &opv1.CalicoNetworkSpec{SysctlTuning: second}
+				s.CalicoNetwork = &opv1.CalicoNetworkSpec{Sysctl: second}
 			}
 			inst := OverrideInstallationSpec(m, s)
 			if expect == nil {
 				Expect(inst.CalicoNetwork).To(BeNil())
 			} else {
-				Expect(*inst.CalicoNetwork.SysctlTuning).To(Equal(*expect))
+				Expect(inst.CalicoNetwork.Sysctl).To(Equal(expect))
 			}
 		},
 			Entry("Both unset", nil, nil, nil),

pkg/controller/utils/utils.go

@@ -69,6 +69,13 @@ var (
 	// most scenarios. Retries should be used sparingly, and only in extraordinary
 	// circumstances. Use this as a default when retries are needed.
 	StandardRetry = 30 * time.Second
+
+	// AllowedSysctlKeys controls the allowed Sysctl keys can be set in Tuning plugin
+	AllowedSysctlKeys = map[string]bool{
+		"net.ipv4.tcp_keepalive_intvl":  true,
+		"net.ipv4.tcp_keepalive_probes": true,
+		"net.ipv4.tcp_keepalive_time":   true,
+	}
 )
 
 // ContextLoggerForResource provides a logger instance with context set for the provided object.
@@ -779,3 +786,12 @@ func GetDNSServiceName(provider operatorv1.Provider) types.NamespacedName {
 	}
 	return kubeDNSServiceName
 }
+
+func VerifySysctl(pluginData []operatorv1.Sysctl) error {
+	for _, setting := range pluginData {
+		if !AllowedSysctlKeys[setting.Key] {
+			return fmt.Errorf("key %s is not allowed in spec.calicoNetwork.sysctl", setting.Key)
+		}
+	}
+	return nil
+}