diff --git a/api/v1/installation_types.go b/api/v1/installation_types.go index 10ffae27a9..4f58c33332 100644 --- a/api/v1/installation_types.go +++ b/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 diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 6118828039..c8b6eb9e30 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -1194,16 +1194,10 @@ func (in *CalicoNetworkSpec) DeepCopyInto(out *CalicoNetworkSpec) { *out = new(ContainerIPForwardingType) **out = **in } - if in.SysctlTuning != nil { - in, out := &in.SysctlTuning, &out.SysctlTuning - *out = new(map[SysctlTuningKey]string) - if **in != nil { - in, out := *in, *out - *out = make(map[SysctlTuningKey]string, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } + if in.Sysctl != nil { + in, out := &in.Sysctl, &out.Sysctl + *out = make([]Sysctl, len(*in)) + copy(*out, *in) } } @@ -3779,6 +3773,21 @@ func (in *SplunkStoreSpec) DeepCopy() *SplunkStoreSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Sysctl) DeepCopyInto(out *Sysctl) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Sysctl. +func (in *Sysctl) DeepCopy() *Sysctl { + if in == nil { + return nil + } + out := new(Sysctl) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SyslogStoreSpec) DeepCopyInto(out *SyslogStoreSpec) { *out = *in diff --git a/pkg/controller/installation/validation.go b/pkg/controller/installation/validation.go index 1dddf39ae0..4fa7f89085 100644 --- a/pkg/controller/installation/validation.go +++ b/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 } } diff --git a/pkg/controller/installation/validation_test.go b/pkg/controller/installation/validation_test.go index b5845e9c85..bcf368825d 100644 --- a/pkg/controller/installation/validation_test.go +++ b/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()) diff --git a/pkg/controller/migration/convert/network.go b/pkg/controller/migration/convert/network.go index 6f6a9c894c..e98e498c69 100644 --- a/pkg/controller/migration/convert/network.go +++ b/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,25 @@ 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 := []operatorv1.Sysctl{} + for _, setting := range tuningSpecData.Sysctl { + sysctl := operatorv1.Sysctl{ + Key: setting.Key, + Value: setting.Value, + } + sysctlTuning = append(sysctlTuning, sysctl) } - sysctlTuning := make(map[operatorv1.SysctlTuningKey]string) - for key, value := range *tuningSpecData.Sysctl { - if _, allowed := allowedKeys[key]; allowed { - sysctlTuning[key] = value + + if err = utils.VerifySysctl(sysctlTuning); err != nil { + return ErrIncompatibleCluster{ + err: err.Error(), + component: ComponentCNIConfig, + fix: "remove unsupported Tuning parameter from CNI config", } } if len(sysctlTuning) > 0 { - install.Spec.CalicoNetwork.SysctlTuning = &sysctlTuning + install.Spec.CalicoNetwork.Sysctl = sysctlTuning } } diff --git a/pkg/controller/migration/convert/network_test.go b/pkg/controller/migration/convert/network_test.go index f91fb88119..a7cfba5507 100644 --- a/pkg/controller/migration/convert/network_test.go +++ b/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,23 @@ 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(cfg).ToNot(BeNil()) - Expect(cfg.Spec.CalicoNetwork.SysctlTuning).To(BeNil()) + Expect(err).To(HaveOccurred()) + Expect(cfg).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 +893,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()) }) }) diff --git a/pkg/controller/utils/merge.go b/pkg/controller/utils/merge.go index 4b00be6291..80b0a8c1be 100644 --- a/pkg/controller/utils/merge.go +++ b/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 } diff --git a/pkg/controller/utils/merge_test.go b/pkg/controller/utils/merge_test.go index 4a708d64d0..5faa86a64d 100644 --- a/pkg/controller/utils/merge_test.go +++ b/pkg/controller/utils/merge_test.go @@ -479,33 +479,41 @@ 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), - Entry("Main only set", &_sysctlTuningA, nil, &_sysctlTuningA), - Entry("Second only set", nil, &_sysctlTuningB, &_sysctlTuningB), - Entry("Both set equal", &_sysctlTuningA, &_sysctlTuningA, &_sysctlTuningA), - Entry("Both set not matching", &_sysctlTuningA, &_sysctlTuningB, &_sysctlTuningB), + Entry("Main only set", _sysctlTuningA, nil, _sysctlTuningA), + Entry("Second only set", nil, _sysctlTuningB, _sysctlTuningB), + Entry("Both set equal", _sysctlTuningA, _sysctlTuningA, _sysctlTuningA), + Entry("Both set not matching", _sysctlTuningA, _sysctlTuningB, _sysctlTuningB), ) }) diff --git a/pkg/controller/utils/utils.go b/pkg/controller/utils/utils.go index 4c42b865b9..0ed12b6d9c 100644 --- a/pkg/controller/utils/utils.go +++ b/pkg/controller/utils/utils.go @@ -73,6 +73,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. @@ -844,3 +851,11 @@ func IsDexDisabled(authentication *operatorv1.Authentication) bool { } return disableDex } +func VerifySysctl(pluginData []operatorv1.Sysctl) error { + for _, setting := range pluginData { + if _, ok := AllowedSysctlKeys[setting.Key]; !ok { + return fmt.Errorf("key %s is not allowed in spec.calicoNetwork.sysctl", setting.Key) + } + } + return nil +} diff --git a/pkg/crds/operator/operator.tigera.io_installations.yaml b/pkg/crds/operator/operator.tigera.io_installations.yaml index 3848b7cf1e..c38cd21a09 100644 --- a/pkg/crds/operator/operator.tigera.io_installations.yaml +++ b/pkg/crds/operator/operator.tigera.io_installations.yaml @@ -1483,12 +1483,23 @@ spec: on interfaces that do not match the given regex. type: string type: object - sysctlTuning: - additionalProperties: - type: string - description: SysctlTuning configures sysctl parameters for tuning - plugin - type: object + sysctl: + description: Sysctl configures sysctl parameters for tuning plugin + items: + properties: + key: + enum: + - net.ipv4.tcp_keepalive_intvl + - net.ipv4.tcp_keepalive_probes + - net.ipv4.tcp_keepalive_time + type: string + value: + type: string + required: + - key + - value + type: object + type: array windowsDataplane: description: 'WindowsDataplane is used to select the dataplane used for Windows nodes. In particular, it causes the operator @@ -10591,12 +10602,24 @@ spec: on interfaces that do not match the given regex. type: string type: object - sysctlTuning: - additionalProperties: - type: string - description: SysctlTuning configures sysctl parameters for - tuning plugin - type: object + sysctl: + description: Sysctl configures sysctl parameters for tuning + plugin + items: + properties: + key: + enum: + - net.ipv4.tcp_keepalive_intvl + - net.ipv4.tcp_keepalive_probes + - net.ipv4.tcp_keepalive_time + type: string + value: + type: string + required: + - key + - value + type: object + type: array windowsDataplane: description: 'WindowsDataplane is used to select the dataplane used for Windows nodes. In particular, it causes the operator diff --git a/pkg/render/node.go b/pkg/render/node.go index b00b2cd05a..2537cc371a 100644 --- a/pkg/render/node.go +++ b/pkg/render/node.go @@ -716,9 +716,9 @@ func (c *nodeComponent) createTuningPlugin() map[string]interface{} { // tuning plugin (sysctl) tuningPlugin := map[string]interface{}{ "type": "tuning", - "sysctl": map[string]string{}, + "sysctl": []operatorv1.Sysctl{}, } - tuningPlugin["sysctl"] = *c.cfg.Installation.CalicoNetwork.SysctlTuning + tuningPlugin["sysctl"] = c.cfg.Installation.CalicoNetwork.Sysctl return tuningPlugin } @@ -742,7 +742,7 @@ func (c *nodeComponent) nodeCNIConfigMap() *corev1.ConfigMap { } // optional tuning plugin - if c.cfg.Installation.CalicoNetwork.SysctlTuning != nil { + if c.cfg.Installation.CalicoNetwork.Sysctl != nil { plugins = append(plugins, c.createTuningPlugin()) } diff --git a/pkg/render/node_test.go b/pkg/render/node_test.go index 5e5877c8a6..3296c0ffcb 100644 --- a/pkg/render/node_test.go +++ b/pkg/render/node_test.go @@ -2682,12 +2682,20 @@ var _ = Describe("Node rendering tests", func() { }) It("should render cni config with sysctl parameters", func() { - sysctl := map[operatorv1.SysctlTuningKey]string{ - "net.ipv4.tcp_keepalive_intvl": "15", - "net.ipv4.tcp_keepalive_probes": "6", - "net.ipv4.tcp_keepalive_time": "40", + sysctl := []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", + }, } - defaultInstance.CalicoNetwork.SysctlTuning = &sysctl + defaultInstance.CalicoNetwork.Sysctl = sysctl component := render.Node(&cfg) Expect(component.ResolveImages(nil)).To(BeNil()) resources, _ := component.Objects() @@ -2740,11 +2748,20 @@ var _ = Describe("Node rendering tests", func() { "type": "portmap" }, { - "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" + } + ], "type": "tuning" } ]