[backport v2.11] Preserves kube-apiserver-arg, webhook prepends PSACT mount path if needed

pkg/resources/provisioning.cattle.io/v1/cluster/mutator.go

@@ -60,48 +60,10 @@ var gvr = schema.GroupVersionResource{
 	Resource: "clusters",
 }
 
-// keyValueArg represents a key-value pair configuration argument.
-type keyValueArg struct {
-	key   string
-	value string
-}
-
-type keyValueArgs []keyValueArg
-
-// parseFromRawArgs converts an interface representing a slice of "key=value" strings & returns a slice of keyValueArg.
-func parseFromRawArgs(input interface{}) (keyValueArgs, error) {
-	parsed := convert.ToInterfaceSlice(input)
-	if parsed == nil {
-		return nil, fmt.Errorf("failed to convert input into slice: invalid type: %T, expected interface{}", input)
-	}
-	args := keyValueArgs{}
-	for _, arg := range parsed {
-		key, val, found := strings.Cut(convert.ToString(arg), "=")
-		if !found {
-			logrus.Warnf("skipping argument [%s] which does not have right format", arg)
-			continue
-		}
-		args.update(key, val)
-	}
-	return args, nil
-}
-
-// update updates the value for the given key if it exists in the slice; otherwise it appends a new key-value pair.
-func (kv *keyValueArgs) update(key, val string) {
-	idx := slices.IndexFunc(*kv, func(arg keyValueArg) bool {
-		return arg.key == key
-	})
-	if idx != -1 {
-		(*kv)[idx].value = val
-	} else {
-		*kv = append(*kv, keyValueArg{key: key, value: val})
-	}
-}
-
-// keyHasValue returns true if the given key-value pair exists in the slice of keyValueArg.
-func (kv *keyValueArgs) keyHasValue(key, val string) bool {
-	for _, arg := range *kv {
-		if arg.key == key && arg.value == val {
+// keyHasValue returns true if args list contains the exact "key=value" pair.
+func keyHasValue(args []string, key, value string) bool {
+	for _, pair := range args {
+		if pair == fmt.Sprintf("%s=%s", key, value) {
 			return true
 		}
 	}
@@ -238,6 +200,7 @@ func (m *ProvisioningClusterMutator) handlePSACT(request *admission.Request, clu
 
 	secretName := fmt.Sprintf(secretName, cluster.Name)
 	mountPath := fmt.Sprintf(mountPath, getRuntime(cluster.Spec.KubernetesVersion))
+	admissionConfigArg := fmt.Sprintf("%s=%s", kubeAPIAdmissionConfigOption, mountPath)
 	templateName := cluster.Spec.DefaultPodSecurityAdmissionConfigurationTemplateName
 
 	switch request.Operation {
@@ -261,10 +224,12 @@ func (m *ProvisioningClusterMutator) handlePSACT(request *admission.Request, clu
 			if err != nil {
 				return nil, fmt.Errorf("[provisioning cluster mutator] failed to get the kube-apiserver arguments: %w", err)
 			}
-			newArgs := slices.DeleteFunc(args, func(arg keyValueArg) bool {
-				return arg.key == kubeAPIAdmissionConfigOption && arg.value == mountPath
-			})
-			setKubeAPIServerArgs(newArgs, cluster)
+			if slices.Contains(args, admissionConfigArg) {
+				args = slices.DeleteFunc(args, func(arg string) bool {
+					return arg == admissionConfigArg
+				})
+			}
+			setKubeAPIServerArgs(args, cluster)
 		} else {
 			// Now, handle the case of PSACT being set when creating or updating the cluster
 			template, err := m.psact.Get(templateName)
@@ -296,7 +261,9 @@ func (m *ProvisioningClusterMutator) handlePSACT(request *admission.Request, clu
 			if err != nil {
 				return nil, fmt.Errorf("[provisioning cluster mutator] failed to get the kube-apiserver arguments: %w", err)
 			}
-			args.update(kubeAPIAdmissionConfigOption, mountPath)
+			if !slices.Contains(args, admissionConfigArg) {
+				args = append([]string{admissionConfigArg}, args...)
+			}
 			setKubeAPIServerArgs(args, cluster)
 		}
 	}
@@ -335,36 +302,31 @@ func (m *ProvisioningClusterMutator) ensureSecret(namespace, name string, data m
 	return nil
 }
 
-// getKubeAPIServerArgs returns a slice of keyValueArg representing the parsed value of
-// "kube-apiserver-arg" from the cluster's MachineGlobalConfig.
+// getKubeAPIServerArgs returns []string representing the parsed value of "kube-apiserver-arg" from the cluster's MachineGlobalConfig.
 // An empty slice is returned if "kube-apiserver-arg" is not set or an error is encountered during parsing.
-func getKubeAPIServerArgs(cluster *v1.Cluster) (keyValueArgs, error) {
+func getKubeAPIServerArgs(cluster *v1.Cluster) ([]string, error) {
 	rawArgs, exists := cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"]
 	if !exists {
-		return keyValueArgs{}, nil
+		return []string{}, nil
 	}
-	args, err := parseFromRawArgs(rawArgs)
-	if err != nil {
-		return keyValueArgs{}, err
+	args := convert.ToStringSlice(rawArgs)
+	if args == nil {
+		return []string{}, fmt.Errorf("failed to convert input into slice: invalid type: %T, expected interface{}", rawArgs)
 	}
 	return args, nil
 }
 
 // setKubeAPIServerArgs uses the provided arg to overwrite the value of kube-apiserver-arg under the cluster's MachineGlobalConfig.
-// If the provided arg is an empty map, setKubeAPIServerArg removes the existing kube-apiserver-arg from the cluster's MachineGlobalConfig.
-func setKubeAPIServerArgs(args keyValueArgs, cluster *v1.Cluster) {
+// If the provided arg is an empty slice, setKubeAPIServerArg removes the existing kube-apiserver-arg from the cluster's MachineGlobalConfig.
+func setKubeAPIServerArgs(args []string, cluster *v1.Cluster) {
 	if len(args) == 0 {
 		delete(cluster.Spec.RKEConfig.MachineGlobalConfig.Data, "kube-apiserver-arg")
 		return
 	}
-	parsed := make([]any, len(args))
-	for i, arg := range args {
-		parsed[i] = arg.key + "=" + arg.value
-	}
 	if cluster.Spec.RKEConfig.MachineGlobalConfig.Data == nil {
-		cluster.Spec.RKEConfig.MachineGlobalConfig.Data = make(map[string]interface{})
+		cluster.Spec.RKEConfig.MachineGlobalConfig.Data = make(map[string]any)
 	}
-	cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"] = parsed
+	cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"] = args
 }
 
 // machineSelectorFileForPSA generates an RKEProvisioningFiles that mounts the secret which contains

pkg/resources/provisioning.cattle.io/v1/cluster/mutator_test.go

@@ -2,7 +2,7 @@ package cluster
 
 import (
 	"encoding/json"
-	"errors"
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -18,17 +18,16 @@ import (
 )
 
 func Test_GetKubeAPIServerArgs(t *testing.T) {
-	errInvalidType := errors.New("failed to convert input into slice: invalid type: []string, expected interface{}")
 	tests := []struct {
 		name        string
 		cluster     *v1.Cluster
-		expected    keyValueArgs
+		expected    []string
 		expectedErr error
 	}{
 		{
 			name:        "cluster without kube-apiserver-arg",
 			cluster:     clusterWithoutKubeAPIServerArg(),
-			expected:    keyValueArgs{},
+			expected:    []string{},
 			expectedErr: nil,
 		},
 		{
@@ -38,42 +37,49 @@ func Test_GetKubeAPIServerArgs(t *testing.T) {
 					RKEConfig: &v1.RKEConfig{},
 				},
 			},
-			expected:    keyValueArgs{},
+			expected:    []string{},
 			expectedErr: nil,
 		},
 		{
 			name:    "cluster with kube-apiserver-arg",
 			cluster: clusterWithKubeAPIServerArg(),
-			expected: keyValueArgs{
-				{key: "foo", value: "bar"},
-				{key: "foo2", value: "bar2"},
+			expected: []string{
+				"foo=bar",
+				"foo2=bar2",
 			},
 			expectedErr: nil,
 		},
 		{
 			name:    "cluster with kube-apiserver-arg-2",
 			cluster: clusterWithKubeAPIServerArg2(),
-			expected: keyValueArgs{
-				{key: "foo", value: "bar"},
-				{key: "foo2", value: "bar2"},
-				{key: "foo3", value: "bar3=baz3"},
+			expected: []string{
+				"foo=bar",
+				"foo2=bar2",
+				"foo3=bar3=baz3",
 			},
 			expectedErr: nil,
 		},
 		{
 			name:    "cluster with duplicate keys in kube-apiserver-arg",
 			cluster: clusterWithKubeAPIServerArg3(),
-			expected: keyValueArgs{
-				{key: "foo", value: "bar"},
-				{key: "foo2", value: "bar2=baz2"},
+			expected: []string{
+				"foo=bar",
+				"foo2=bar2",
+				"foo2=bar2=baz2",
 			},
 			expectedErr: nil,
 		},
 		{
-			name:        "cluster with invalid data in kube-apiserver-arg",
-			cluster:     clusterWithInvalidKubeAPIServerArg(),
-			expected:    keyValueArgs{},
-			expectedErr: errInvalidType,
+			name:        "cluster with bool flag data in kube-apiserver-arg",
+			cluster:     clusterWithBoolFlagKubeAPIServerArg(),
+			expected:    []string{"foo", "foo2=bar2"},
+			expectedErr: nil,
+		},
+		{
+			name:        "cluster with invalid data type in kube-apiserver-arg",
+			cluster:     clusterWithInvalidKubeAPIServerArgType(),
+			expected:    []string{},
+			expectedErr: fmt.Errorf("failed to convert input into slice: invalid type: int, expected interface{}"),
 		},
 	}
 	for _, tt := range tests {
@@ -85,7 +91,7 @@ func Test_GetKubeAPIServerArgs(t *testing.T) {
 				}
 			}
 			if !reflect.DeepEqual(tt.expected, got) {
-				t.Errorf("got: %v, expected: %v", got, tt.expected)
+				t.Errorf("expected: %v, got: %v", tt.expected, got)
 			}
 		})
 	}
@@ -94,15 +100,15 @@ func Test_GetKubeAPIServerArgs(t *testing.T) {
 func Test_SetKubeAPIServerArgs(t *testing.T) {
 	tests := []struct {
 		name     string
-		arg      keyValueArgs
+		args     []string
 		cluster  *v1.Cluster
 		expected *v1.Cluster
 	}{
 		{
 			name: "cluster that already has kube-apiserver-arg",
-			arg: keyValueArgs{
-				{key: "foo", value: "bar"},
-				{key: "foo2", value: "bar2"},
+			args: []string{
+				"foo=bar",
+				"foo2=bar2",
 			},
 			cluster: &v1.Cluster{
 				Spec: v1.ClusterSpec{
@@ -123,9 +129,9 @@ func Test_SetKubeAPIServerArgs(t *testing.T) {
 		},
 		{
 			name: "cluster that does not have MachineGlobalConfig",
-			arg: keyValueArgs{
-				{key: "foo", value: "bar"},
-				{key: "foo2", value: "bar2"},
+			args: []string{
+				"foo=bar",
+				"foo2=bar2",
 			},
 			cluster: &v1.Cluster{
 				Spec: v1.ClusterSpec{
@@ -136,9 +142,9 @@ func Test_SetKubeAPIServerArgs(t *testing.T) {
 		},
 		{
 			name: "cluster does not have kube-apiserver-arg but other args",
-			arg: keyValueArgs{
-				{key: "foo", value: "bar"},
-				{key: "foo2", value: "bar2"},
+			args: []string{
+				"foo=bar",
+				"foo2=bar2",
 			},
 			cluster: &v1.Cluster{
 				Spec: v1.ClusterSpec{
@@ -162,9 +168,9 @@ func Test_SetKubeAPIServerArgs(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			setKubeAPIServerArgs(tt.arg, tt.cluster)
-			got, _ := parseFromRawArgs(tt.cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"])
-			expected, _ := parseFromRawArgs(tt.expected.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"])
+			setKubeAPIServerArgs(tt.args, tt.cluster)
+			got, _ := getKubeAPIServerArgs(tt.cluster)
+			expected, _ := getKubeAPIServerArgs(tt.expected)
 			if !reflect.DeepEqual(got, expected) {
 				t.Errorf("got: %v, expected: %v", got, expected)
 			}
@@ -485,15 +491,21 @@ func clusterWithKubeAPIServerArg3() *v1.Cluster {
 	return cluster
 }
 
-func clusterWithInvalidKubeAPIServerArg() *v1.Cluster {
+func clusterWithBoolFlagKubeAPIServerArg() *v1.Cluster {
 	cluster := clusterWithoutKubeAPIServerArg()
 	var arg []string
-	arg = append(arg, "foo=bar")
+	arg = append(arg, "foo")
 	arg = append(arg, "foo2=bar2")
 	cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"] = arg
 	return cluster
 }
 
+func clusterWithInvalidKubeAPIServerArgType() *v1.Cluster {
+	cluster := clusterWithoutKubeAPIServerArg()
+	cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"] = 123
+	return cluster
+}
+
 func machineSelectorFile3() rkev1.RKEProvisioningFiles {
 	file := machineSelectorFile1()
 	file.FileSources[0].Secret.Items[0].Hash = "expected-value-for-file-3"

pkg/resources/provisioning.cattle.io/v1/cluster/validator.go

@@ -480,7 +480,7 @@ func (p *provisioningAdmitter) validatePSACT(request *admission.Request, respons
 			if err != nil {
 				return fmt.Errorf("[provisioning cluster validator] failed to get the kube-apiserver arguments: %w", err)
 			}
-			if args.keyHasValue(kubeAPIAdmissionConfigOption, mountPath) {
+			if keyHasValue(args, kubeAPIAdmissionConfigOption, mountPath) {
 				return fmt.Errorf("[provisioning cluster validator] admission-control-config-file under kube-apiserver-arg should not be set to %s", mountPath)
 			}
 		} else {
@@ -526,7 +526,7 @@ func (p *provisioningAdmitter) validatePSACT(request *admission.Request, respons
 			if err != nil {
 				return fmt.Errorf("[provisioning cluster validator] failed to get the kube-apiserver arguments: %w", err)
 			}
-			if !args.keyHasValue(kubeAPIAdmissionConfigOption, mountPath) {
+			if !keyHasValue(args, kubeAPIAdmissionConfigOption, mountPath) {
 				return fmt.Errorf("[provisioning cluster validator] admission-control-config-file under kube-apiserver-arg should be set to %s", mountPath)
 			}
 		}