Skip to content

Commit 168d987

Browse files
apoorvajagtapapoorvajagtap
authored
Remove keyValueArgs dependency and preserve user-defined kube-apiserver-arg (#1096) (#1123)
1 parent 91e1b73 commit 168d987

File tree

3 files changed

+73
-99
lines changed

3 files changed

+73
-99
lines changed

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

Lines changed: 24 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -60,48 +60,10 @@ var gvr = schema.GroupVersionResource{
6060
Resource: "clusters",
6161
}
6262

63-
// keyValueArg represents a key-value pair configuration argument.
64-
type keyValueArg struct {
65-
key string
66-
value string
67-
}
68-
69-
type keyValueArgs []keyValueArg
70-
71-
// parseFromRawArgs converts an interface representing a slice of "key=value" strings & returns a slice of keyValueArg.
72-
func parseFromRawArgs(input interface{}) (keyValueArgs, error) {
73-
parsed := convert.ToInterfaceSlice(input)
74-
if parsed == nil {
75-
return nil, fmt.Errorf("failed to convert input into slice: invalid type: %T, expected interface{}", input)
76-
}
77-
args := keyValueArgs{}
78-
for _, arg := range parsed {
79-
key, val, found := strings.Cut(convert.ToString(arg), "=")
80-
if !found {
81-
logrus.Warnf("skipping argument [%s] which does not have right format", arg)
82-
continue
83-
}
84-
args.update(key, val)
85-
}
86-
return args, nil
87-
}
88-
89-
// update updates the value for the given key if it exists in the slice; otherwise it appends a new key-value pair.
90-
func (kv *keyValueArgs) update(key, val string) {
91-
idx := slices.IndexFunc(*kv, func(arg keyValueArg) bool {
92-
return arg.key == key
93-
})
94-
if idx != -1 {
95-
(*kv)[idx].value = val
96-
} else {
97-
*kv = append(*kv, keyValueArg{key: key, value: val})
98-
}
99-
}
100-
101-
// keyHasValue returns true if the given key-value pair exists in the slice of keyValueArg.
102-
func (kv *keyValueArgs) keyHasValue(key, val string) bool {
103-
for _, arg := range *kv {
104-
if arg.key == key && arg.value == val {
63+
// keyHasValue returns true if args list contains the exact "key=value" pair.
64+
func keyHasValue(args []string, key, value string) bool {
65+
for _, pair := range args {
66+
if pair == fmt.Sprintf("%s=%s", key, value) {
10567
return true
10668
}
10769
}
@@ -238,6 +200,7 @@ func (m *ProvisioningClusterMutator) handlePSACT(request *admission.Request, clu
238200

239201
secretName := fmt.Sprintf(secretName, cluster.Name)
240202
mountPath := fmt.Sprintf(mountPath, getRuntime(cluster.Spec.KubernetesVersion))
203+
admissionConfigArg := fmt.Sprintf("%s=%s", kubeAPIAdmissionConfigOption, mountPath)
241204
templateName := cluster.Spec.DefaultPodSecurityAdmissionConfigurationTemplateName
242205

243206
switch request.Operation {
@@ -261,10 +224,12 @@ func (m *ProvisioningClusterMutator) handlePSACT(request *admission.Request, clu
261224
if err != nil {
262225
return nil, fmt.Errorf("[provisioning cluster mutator] failed to get the kube-apiserver arguments: %w", err)
263226
}
264-
newArgs := slices.DeleteFunc(args, func(arg keyValueArg) bool {
265-
return arg.key == kubeAPIAdmissionConfigOption && arg.value == mountPath
266-
})
267-
setKubeAPIServerArgs(newArgs, cluster)
227+
if slices.Contains(args, admissionConfigArg) {
228+
args = slices.DeleteFunc(args, func(arg string) bool {
229+
return arg == admissionConfigArg
230+
})
231+
}
232+
setKubeAPIServerArgs(args, cluster)
268233
} else {
269234
// Now, handle the case of PSACT being set when creating or updating the cluster
270235
template, err := m.psact.Get(templateName)
@@ -296,7 +261,9 @@ func (m *ProvisioningClusterMutator) handlePSACT(request *admission.Request, clu
296261
if err != nil {
297262
return nil, fmt.Errorf("[provisioning cluster mutator] failed to get the kube-apiserver arguments: %w", err)
298263
}
299-
args.update(kubeAPIAdmissionConfigOption, mountPath)
264+
if !slices.Contains(args, admissionConfigArg) {
265+
args = append([]string{admissionConfigArg}, args...)
266+
}
300267
setKubeAPIServerArgs(args, cluster)
301268
}
302269
}
@@ -335,36 +302,31 @@ func (m *ProvisioningClusterMutator) ensureSecret(namespace, name string, data m
335302
return nil
336303
}
337304

338-
// getKubeAPIServerArgs returns a slice of keyValueArg representing the parsed value of
339-
// "kube-apiserver-arg" from the cluster's MachineGlobalConfig.
305+
// getKubeAPIServerArgs returns []string representing the parsed value of "kube-apiserver-arg" from the cluster's MachineGlobalConfig.
340306
// An empty slice is returned if "kube-apiserver-arg" is not set or an error is encountered during parsing.
341-
func getKubeAPIServerArgs(cluster *v1.Cluster) (keyValueArgs, error) {
307+
func getKubeAPIServerArgs(cluster *v1.Cluster) ([]string, error) {
342308
rawArgs, exists := cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"]
343309
if !exists {
344-
return keyValueArgs{}, nil
310+
return []string{}, nil
345311
}
346-
args, err := parseFromRawArgs(rawArgs)
347-
if err != nil {
348-
return keyValueArgs{}, err
312+
args := convert.ToStringSlice(rawArgs)
313+
if args == nil {
314+
return []string{}, fmt.Errorf("failed to convert input into slice: invalid type: %T, expected interface{}", rawArgs)
349315
}
350316
return args, nil
351317
}
352318

353319
// setKubeAPIServerArgs uses the provided arg to overwrite the value of kube-apiserver-arg under the cluster's MachineGlobalConfig.
354-
// If the provided arg is an empty map, setKubeAPIServerArg removes the existing kube-apiserver-arg from the cluster's MachineGlobalConfig.
355-
func setKubeAPIServerArgs(args keyValueArgs, cluster *v1.Cluster) {
320+
// If the provided arg is an empty slice, setKubeAPIServerArg removes the existing kube-apiserver-arg from the cluster's MachineGlobalConfig.
321+
func setKubeAPIServerArgs(args []string, cluster *v1.Cluster) {
356322
if len(args) == 0 {
357323
delete(cluster.Spec.RKEConfig.MachineGlobalConfig.Data, "kube-apiserver-arg")
358324
return
359325
}
360-
parsed := make([]any, len(args))
361-
for i, arg := range args {
362-
parsed[i] = arg.key + "=" + arg.value
363-
}
364326
if cluster.Spec.RKEConfig.MachineGlobalConfig.Data == nil {
365-
cluster.Spec.RKEConfig.MachineGlobalConfig.Data = make(map[string]interface{})
327+
cluster.Spec.RKEConfig.MachineGlobalConfig.Data = make(map[string]any)
366328
}
367-
cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"] = parsed
329+
cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"] = args
368330
}
369331

370332
// machineSelectorFileForPSA generates an RKEProvisioningFiles that mounts the secret which contains

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

Lines changed: 47 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package cluster
22

33
import (
44
"encoding/json"
5-
"errors"
5+
"fmt"
66
"reflect"
77
"testing"
88

@@ -18,17 +18,16 @@ import (
1818
)
1919

2020
func Test_GetKubeAPIServerArgs(t *testing.T) {
21-
errInvalidType := errors.New("failed to convert input into slice: invalid type: []string, expected interface{}")
2221
tests := []struct {
2322
name string
2423
cluster *v1.Cluster
25-
expected keyValueArgs
24+
expected []string
2625
expectedErr error
2726
}{
2827
{
2928
name: "cluster without kube-apiserver-arg",
3029
cluster: clusterWithoutKubeAPIServerArg(),
31-
expected: keyValueArgs{},
30+
expected: []string{},
3231
expectedErr: nil,
3332
},
3433
{
@@ -38,42 +37,49 @@ func Test_GetKubeAPIServerArgs(t *testing.T) {
3837
RKEConfig: &v1.RKEConfig{},
3938
},
4039
},
41-
expected: keyValueArgs{},
40+
expected: []string{},
4241
expectedErr: nil,
4342
},
4443
{
4544
name: "cluster with kube-apiserver-arg",
4645
cluster: clusterWithKubeAPIServerArg(),
47-
expected: keyValueArgs{
48-
{key: "foo", value: "bar"},
49-
{key: "foo2", value: "bar2"},
46+
expected: []string{
47+
"foo=bar",
48+
"foo2=bar2",
5049
},
5150
expectedErr: nil,
5251
},
5352
{
5453
name: "cluster with kube-apiserver-arg-2",
5554
cluster: clusterWithKubeAPIServerArg2(),
56-
expected: keyValueArgs{
57-
{key: "foo", value: "bar"},
58-
{key: "foo2", value: "bar2"},
59-
{key: "foo3", value: "bar3=baz3"},
55+
expected: []string{
56+
"foo=bar",
57+
"foo2=bar2",
58+
"foo3=bar3=baz3",
6059
},
6160
expectedErr: nil,
6261
},
6362
{
6463
name: "cluster with duplicate keys in kube-apiserver-arg",
6564
cluster: clusterWithKubeAPIServerArg3(),
66-
expected: keyValueArgs{
67-
{key: "foo", value: "bar"},
68-
{key: "foo2", value: "bar2=baz2"},
65+
expected: []string{
66+
"foo=bar",
67+
"foo2=bar2",
68+
"foo2=bar2=baz2",
6969
},
7070
expectedErr: nil,
7171
},
7272
{
73-
name: "cluster with invalid data in kube-apiserver-arg",
74-
cluster: clusterWithInvalidKubeAPIServerArg(),
75-
expected: keyValueArgs{},
76-
expectedErr: errInvalidType,
73+
name: "cluster with bool flag data in kube-apiserver-arg",
74+
cluster: clusterWithBoolFlagKubeAPIServerArg(),
75+
expected: []string{"foo", "foo2=bar2"},
76+
expectedErr: nil,
77+
},
78+
{
79+
name: "cluster with invalid data type in kube-apiserver-arg",
80+
cluster: clusterWithInvalidKubeAPIServerArgType(),
81+
expected: []string{},
82+
expectedErr: fmt.Errorf("failed to convert input into slice: invalid type: int, expected interface{}"),
7783
},
7884
}
7985
for _, tt := range tests {
@@ -85,7 +91,7 @@ func Test_GetKubeAPIServerArgs(t *testing.T) {
8591
}
8692
}
8793
if !reflect.DeepEqual(tt.expected, got) {
88-
t.Errorf("got: %v, expected: %v", got, tt.expected)
94+
t.Errorf("expected: %v, got: %v", tt.expected, got)
8995
}
9096
})
9197
}
@@ -94,15 +100,15 @@ func Test_GetKubeAPIServerArgs(t *testing.T) {
94100
func Test_SetKubeAPIServerArgs(t *testing.T) {
95101
tests := []struct {
96102
name string
97-
arg keyValueArgs
103+
args []string
98104
cluster *v1.Cluster
99105
expected *v1.Cluster
100106
}{
101107
{
102108
name: "cluster that already has kube-apiserver-arg",
103-
arg: keyValueArgs{
104-
{key: "foo", value: "bar"},
105-
{key: "foo2", value: "bar2"},
109+
args: []string{
110+
"foo=bar",
111+
"foo2=bar2",
106112
},
107113
cluster: &v1.Cluster{
108114
Spec: v1.ClusterSpec{
@@ -123,9 +129,9 @@ func Test_SetKubeAPIServerArgs(t *testing.T) {
123129
},
124130
{
125131
name: "cluster that does not have MachineGlobalConfig",
126-
arg: keyValueArgs{
127-
{key: "foo", value: "bar"},
128-
{key: "foo2", value: "bar2"},
132+
args: []string{
133+
"foo=bar",
134+
"foo2=bar2",
129135
},
130136
cluster: &v1.Cluster{
131137
Spec: v1.ClusterSpec{
@@ -136,9 +142,9 @@ func Test_SetKubeAPIServerArgs(t *testing.T) {
136142
},
137143
{
138144
name: "cluster does not have kube-apiserver-arg but other args",
139-
arg: keyValueArgs{
140-
{key: "foo", value: "bar"},
141-
{key: "foo2", value: "bar2"},
145+
args: []string{
146+
"foo=bar",
147+
"foo2=bar2",
142148
},
143149
cluster: &v1.Cluster{
144150
Spec: v1.ClusterSpec{
@@ -162,9 +168,9 @@ func Test_SetKubeAPIServerArgs(t *testing.T) {
162168
}
163169
for _, tt := range tests {
164170
t.Run(tt.name, func(t *testing.T) {
165-
setKubeAPIServerArgs(tt.arg, tt.cluster)
166-
got, _ := parseFromRawArgs(tt.cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"])
167-
expected, _ := parseFromRawArgs(tt.expected.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"])
171+
setKubeAPIServerArgs(tt.args, tt.cluster)
172+
got, _ := getKubeAPIServerArgs(tt.cluster)
173+
expected, _ := getKubeAPIServerArgs(tt.expected)
168174
if !reflect.DeepEqual(got, expected) {
169175
t.Errorf("got: %v, expected: %v", got, expected)
170176
}
@@ -485,15 +491,21 @@ func clusterWithKubeAPIServerArg3() *v1.Cluster {
485491
return cluster
486492
}
487493

488-
func clusterWithInvalidKubeAPIServerArg() *v1.Cluster {
494+
func clusterWithBoolFlagKubeAPIServerArg() *v1.Cluster {
489495
cluster := clusterWithoutKubeAPIServerArg()
490496
var arg []string
491-
arg = append(arg, "foo=bar")
497+
arg = append(arg, "foo")
492498
arg = append(arg, "foo2=bar2")
493499
cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"] = arg
494500
return cluster
495501
}
496502

503+
func clusterWithInvalidKubeAPIServerArgType() *v1.Cluster {
504+
cluster := clusterWithoutKubeAPIServerArg()
505+
cluster.Spec.RKEConfig.MachineGlobalConfig.Data["kube-apiserver-arg"] = 123
506+
return cluster
507+
}
508+
497509
func machineSelectorFile3() rkev1.RKEProvisioningFiles {
498510
file := machineSelectorFile1()
499511
file.FileSources[0].Secret.Items[0].Hash = "expected-value-for-file-3"

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ func (p *provisioningAdmitter) validatePSACT(request *admission.Request, respons
480480
if err != nil {
481481
return fmt.Errorf("[provisioning cluster validator] failed to get the kube-apiserver arguments: %w", err)
482482
}
483-
if args.keyHasValue(kubeAPIAdmissionConfigOption, mountPath) {
483+
if keyHasValue(args, kubeAPIAdmissionConfigOption, mountPath) {
484484
return fmt.Errorf("[provisioning cluster validator] admission-control-config-file under kube-apiserver-arg should not be set to %s", mountPath)
485485
}
486486
} else {
@@ -526,7 +526,7 @@ func (p *provisioningAdmitter) validatePSACT(request *admission.Request, respons
526526
if err != nil {
527527
return fmt.Errorf("[provisioning cluster validator] failed to get the kube-apiserver arguments: %w", err)
528528
}
529-
if !args.keyHasValue(kubeAPIAdmissionConfigOption, mountPath) {
529+
if !keyHasValue(args, kubeAPIAdmissionConfigOption, mountPath) {
530530
return fmt.Errorf("[provisioning cluster validator] admission-control-config-file under kube-apiserver-arg should be set to %s", mountPath)
531531
}
532532
}

0 commit comments

Comments
 (0)