diff --git a/cmd/kubeadm/app/apis/kubeadm/argument.go b/cmd/kubeadm/app/apis/kubeadm/argument.go new file mode 100644 index 00000000000..91d373c8ee4 --- /dev/null +++ b/cmd/kubeadm/app/apis/kubeadm/argument.go @@ -0,0 +1,63 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kubeadm + +// GetArgValue traverses an argument slice backwards and returns the value +// of the given argument name and the index where it was found. +// If the argument does not exist an empty string and -1 are returned. +// startIdx defines where the iteration starts. If startIdx is a negative +// value or larger than the size of the argument slice the iteration +// will start from the last element. +func GetArgValue(args []Arg, name string, startIdx int) (string, int) { + if startIdx < 0 || startIdx > len(args)-1 { + startIdx = len(args) - 1 + } + for i := startIdx; i >= 0; i-- { + arg := args[i] + if arg.Name == name { + return arg.Value, i + } + } + return "", -1 +} + +// SetArgValues updates the value of one or more arguments or adds a new +// one if missing. The function works backwards in the argument list. +// nArgs holds how many existing arguments with this name should be set. +// If nArgs is less than 1, all of them will be updated. +func SetArgValues(args []Arg, name, value string, nArgs int) []Arg { + var count int + var found bool + for i := len(args) - 1; i >= 0; i-- { + if args[i].Name == name { + found = true + args[i].Value = value + if nArgs < 1 { + continue + } + count++ + if count >= nArgs { + return args + } + } + } + if found { + return args + } + args = append(args, Arg{Name: name, Value: value}) + return args +} diff --git a/cmd/kubeadm/app/apis/kubeadm/argument_test.go b/cmd/kubeadm/app/apis/kubeadm/argument_test.go new file mode 100644 index 00000000000..fcb523e4daa --- /dev/null +++ b/cmd/kubeadm/app/apis/kubeadm/argument_test.go @@ -0,0 +1,123 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kubeadm + +import ( + "reflect" + "testing" +) + +func TestGetArgValue(t *testing.T) { + var tests = []struct { + testName string + args []Arg + name string + expectedValue string + startIdx int + expectedIdx int + }{ + { + testName: "argument exists with non-empty value", + args: []Arg{{Name: "a", Value: "a1"}, {Name: "b", Value: "b1"}, {Name: "c", Value: "c1"}}, + name: "b", + expectedValue: "b1", + expectedIdx: 1, + startIdx: -1, + }, + { + testName: "argument exists with non-empty value (offset index)", + args: []Arg{{Name: "a", Value: "a1"}, {Name: "b", Value: "b1"}, {Name: "c", Value: "c1"}}, + name: "a", + expectedValue: "a1", + expectedIdx: 0, + startIdx: 0, + }, + { + testName: "argument exists with empty value", + args: []Arg{{Name: "foo1", Value: ""}, {Name: "foo2", Value: ""}}, + name: "foo2", + expectedValue: "", + expectedIdx: 1, + startIdx: -1, + }, + { + testName: "argument does not exists", + args: []Arg{{Name: "foo", Value: "bar"}}, + name: "z", + expectedValue: "", + expectedIdx: -1, + startIdx: -1, + }, + } + + for _, rt := range tests { + t.Run(rt.testName, func(t *testing.T) { + value, idx := GetArgValue(rt.args, rt.name, rt.startIdx) + if idx != rt.expectedIdx { + t.Errorf("expected index: %v, got: %v", rt.expectedIdx, idx) + } + if value != rt.expectedValue { + t.Errorf("expected value: %s, got: %s", rt.expectedValue, value) + } + }) + } +} + +func TestSetArgValues(t *testing.T) { + var tests = []struct { + testName string + args []Arg + name string + value string + nArgs int + expectedArgs []Arg + }{ + { + testName: "update 1 argument", + args: []Arg{{Name: "foo", Value: "bar1"}, {Name: "foo", Value: "bar2"}}, + name: "foo", + value: "zz", + nArgs: 1, + expectedArgs: []Arg{{Name: "foo", Value: "bar1"}, {Name: "foo", Value: "zz"}}, + }, + { + testName: "update all arguments", + args: []Arg{{Name: "foo", Value: "bar1"}, {Name: "foo", Value: "bar2"}}, + name: "foo", + value: "zz", + nArgs: -1, + expectedArgs: []Arg{{Name: "foo", Value: "zz"}, {Name: "foo", Value: "zz"}}, + }, + { + testName: "add new argument", + args: []Arg{{Name: "foo", Value: "bar1"}, {Name: "foo", Value: "bar2"}}, + name: "z", + value: "zz", + nArgs: -1, + expectedArgs: []Arg{{Name: "foo", Value: "bar1"}, {Name: "foo", Value: "bar2"}, {Name: "z", Value: "zz"}}, + }, + } + + for _, rt := range tests { + t.Run(rt.testName, func(t *testing.T) { + args := SetArgValues(rt.args, rt.name, rt.value, rt.nArgs) + if !reflect.DeepEqual(args, rt.expectedArgs) { + t.Errorf("expected args: %#v, got: %#v", rt.expectedArgs, args) + } + }) + } +} diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation.go index e1bf34f22d1..e764df511b8 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go +++ b/cmd/kubeadm/app/apis/kubeadm/validation/validation.go @@ -64,6 +64,8 @@ func ValidateClusterConfiguration(c *kubeadm.ClusterConfiguration) field.ErrorLi allErrs = append(allErrs, ValidateDNS(&c.DNS, field.NewPath("dns"))...) allErrs = append(allErrs, ValidateNetworking(c, field.NewPath("networking"))...) allErrs = append(allErrs, ValidateAPIServer(&c.APIServer, field.NewPath("apiServer"))...) + allErrs = append(allErrs, ValidateControllerManager(&c.ControllerManager, field.NewPath("controllerManager"))...) + allErrs = append(allErrs, ValidateScheduler(&c.Scheduler, field.NewPath("scheduler"))...) allErrs = append(allErrs, ValidateAbsolutePath(c.CertificatesDir, field.NewPath("certificatesDir"))...) allErrs = append(allErrs, ValidateFeatureGates(c.FeatureGates, field.NewPath("featureGates"))...) allErrs = append(allErrs, ValidateHostPort(c.ControlPlaneEndpoint, field.NewPath("controlPlaneEndpoint"))...) @@ -77,6 +79,21 @@ func ValidateClusterConfiguration(c *kubeadm.ClusterConfiguration) field.ErrorLi func ValidateAPIServer(a *kubeadm.APIServer, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} allErrs = append(allErrs, ValidateCertSANs(a.CertSANs, fldPath.Child("certSANs"))...) + allErrs = append(allErrs, ValidateExtraArgs(a.ExtraArgs, fldPath.Child("extraArgs"))...) + return allErrs +} + +// ValidateControllerManager validates the controller manager object and collects all encountered errors +func ValidateControllerManager(a *kubeadm.ControlPlaneComponent, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + allErrs = append(allErrs, ValidateExtraArgs(a.ExtraArgs, fldPath.Child("extraArgs"))...) + return allErrs +} + +// ValidateScheduler validates the scheduler object and collects all encountered errors +func ValidateScheduler(a *kubeadm.ControlPlaneComponent, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + allErrs = append(allErrs, ValidateExtraArgs(a.ExtraArgs, fldPath.Child("extraArgs"))...) return allErrs } @@ -115,6 +132,7 @@ func ValidateNodeRegistrationOptions(nro *kubeadm.NodeRegistrationOptions, fldPa } } allErrs = append(allErrs, ValidateSocketPath(nro.CRISocket, fldPath.Child("criSocket"))...) + allErrs = append(allErrs, ValidateExtraArgs(nro.KubeletExtraArgs, fldPath.Child("kubeletExtraArgs"))...) // TODO: Maybe validate .Taints as well in the future using something like validateNodeTaints() in pkg/apis/core/validation return allErrs } @@ -287,6 +305,7 @@ func ValidateEtcd(e *kubeadm.Etcd, fldPath *field.Path) field.ErrorList { if len(e.Local.ImageRepository) > 0 { allErrs = append(allErrs, ValidateImageRepository(e.Local.ImageRepository, localPath.Child("imageRepository"))...) } + allErrs = append(allErrs, ValidateExtraArgs(e.Local.ExtraArgs, localPath.Child("extraArgs"))...) } if e.External != nil { requireHTTPS := true @@ -478,9 +497,10 @@ func getClusterNodeMask(c *kubeadm.ClusterConfiguration, isIPv6 bool) (int, erro maskArg = "node-cidr-mask-size-ipv4" } - if v, ok := c.ControllerManager.ExtraArgs[maskArg]; ok && v != "" { + maskValue, _ := kubeadm.GetArgValue(c.ControllerManager.ExtraArgs, maskArg, -1) + if len(maskValue) != 0 { // assume it is an integer, if not it will fail later - maskSize, err = strconv.Atoi(v) + maskSize, err = strconv.Atoi(maskValue) if err != nil { return 0, errors.Wrapf(err, "could not parse the value of the kube-controller-manager flag %s as an integer", maskArg) } @@ -518,7 +538,8 @@ func ValidateNetworking(c *kubeadm.ClusterConfiguration, fldPath *field.Path) fi } if len(c.Networking.PodSubnet) != 0 { allErrs = append(allErrs, ValidateIPNetFromString(c.Networking.PodSubnet, constants.MinimumAddressesInPodSubnet, fldPath.Child("podSubnet"))...) - if c.ControllerManager.ExtraArgs["allocate-node-cidrs"] != "false" { + val, _ := kubeadm.GetArgValue(c.ControllerManager.ExtraArgs, "allocate-node-cidrs", -1) + if val != "false" { // Pod subnet was already validated, we need to validate now against the node-mask allErrs = append(allErrs, ValidatePodSubnetNodeMask(c.Networking.PodSubnet, c, fldPath.Child("podSubnet"))...) } @@ -656,3 +677,16 @@ func ValidateResetConfiguration(c *kubeadm.ResetConfiguration) field.ErrorList { allErrs = append(allErrs, ValidateAbsolutePath(c.CertificatesDir, field.NewPath("certificatesDir"))...) return allErrs } + +// ValidateExtraArgs validates a set of arguments and collects all encountered errors +func ValidateExtraArgs(args []kubeadm.Arg, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + for idx, arg := range args { + if len(arg.Name) == 0 { + allErrs = append(allErrs, field.Invalid(fldPath, fmt.Sprintf("index %d", idx), "argument has no name")) + } + } + + return allErrs +} diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go index c98fcdbef12..773a58cdec8 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go +++ b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go @@ -254,24 +254,24 @@ func TestValidatePodSubnetNodeMask(t *testing.T) { var tests = []struct { name string subnet string - cmExtraArgs map[string]string + cmExtraArgs []kubeadmapi.Arg expected bool }{ // dual-stack: {"dual IPv4 only, but mask too small. Default node-mask", "10.0.0.16/29", nil, false}, - {"dual IPv4 only, but mask too small. Configured node-mask", "10.0.0.16/24", map[string]string{"node-cidr-mask-size-ipv4": "23"}, false}, + {"dual IPv4 only, but mask too small. Configured node-mask", "10.0.0.16/24", []kubeadmapi.Arg{{Name: "node-cidr-mask-size-ipv4", Value: "23"}}, false}, {"dual IPv6 only, but mask too small. Default node-mask", "2001:db8::1/112", nil, false}, - {"dual IPv6 only, but mask too small. Configured node-mask", "2001:db8::1/64", map[string]string{"node-cidr-mask-size-ipv6": "24"}, false}, + {"dual IPv6 only, but mask too small. Configured node-mask", "2001:db8::1/64", []kubeadmapi.Arg{{Name: "node-cidr-mask-size-ipv6", Value: "24"}}, false}, {"dual IPv6 only, but mask difference greater than 16. Default node-mask", "2001:db8::1/12", nil, false}, - {"dual IPv6 only, but mask difference greater than 16. Configured node-mask", "2001:db8::1/64", map[string]string{"node-cidr-mask-size-ipv6": "120"}, false}, + {"dual IPv6 only, but mask difference greater than 16. Configured node-mask", "2001:db8::1/64", []kubeadmapi.Arg{{Name: "node-cidr-mask-size-ipv6", Value: "120"}}, false}, {"dual IPv4 only CIDR", "10.0.0.16/12", nil, true}, {"dual IPv6 only CIDR", "2001:db8::/48", nil, true}, {"dual, but IPv4 mask too small. Default node-mask", "10.0.0.16/29,2001:db8::/48", nil, false}, - {"dual, but IPv4 mask too small. Configured node-mask", "10.0.0.16/24,2001:db8::/48", map[string]string{"node-cidr-mask-size-ipv4": "23"}, false}, + {"dual, but IPv4 mask too small. Configured node-mask", "10.0.0.16/24,2001:db8::/48", []kubeadmapi.Arg{{Name: "node-cidr-mask-size-ipv4", Value: "23"}}, false}, {"dual, but IPv6 mask too small. Default node-mask", "2001:db8::1/112,10.0.0.16/16", nil, false}, - {"dual, but IPv6 mask too small. Configured node-mask", "10.0.0.16/16,2001:db8::1/64", map[string]string{"node-cidr-mask-size-ipv6": "24"}, false}, + {"dual, but IPv6 mask too small. Configured node-mask", "10.0.0.16/16,2001:db8::1/64", []kubeadmapi.Arg{{Name: "node-cidr-mask-size-ipv6", Value: "24"}}, false}, {"dual, but mask difference greater than 16. Default node-mask", "2001:db8::1/12,10.0.0.16/16", nil, false}, - {"dual, but mask difference greater than 16. Configured node-mask", "10.0.0.16/16,2001:db8::1/64", map[string]string{"node-cidr-mask-size-ipv6": "120"}, false}, + {"dual, but mask difference greater than 16. Configured node-mask", "10.0.0.16/16,2001:db8::1/64", []kubeadmapi.Arg{{Name: "node-cidr-mask-size-ipv6", Value: "120"}}, false}, {"dual IPv4 IPv6", "2001:db8::/48,10.0.0.16/12", nil, true}, {"dual IPv6 IPv4", "2001:db8::/48,10.0.0.16/12", nil, true}, } @@ -1169,7 +1169,10 @@ func TestGetClusterNodeMask(t *testing.T) { name: "dual ipv4 custom mask", cfg: &kubeadmapi.ClusterConfiguration{ ControllerManager: kubeadmapi.ControlPlaneComponent{ - ExtraArgs: map[string]string{"node-cidr-mask-size": "21", "node-cidr-mask-size-ipv4": "23"}, + ExtraArgs: []kubeadmapi.Arg{ + {Name: "node-cidr-mask-size", Value: "21"}, + {Name: "node-cidr-mask-size-ipv4", Value: "23"}, + }, }, }, isIPv6: false, @@ -1185,7 +1188,9 @@ func TestGetClusterNodeMask(t *testing.T) { name: "dual ipv6 custom mask", cfg: &kubeadmapi.ClusterConfiguration{ ControllerManager: kubeadmapi.ControlPlaneComponent{ - ExtraArgs: map[string]string{"node-cidr-mask-size-ipv6": "83"}, + ExtraArgs: []kubeadmapi.Arg{ + {Name: "node-cidr-mask-size-ipv6", Value: "83"}, + }, }, }, isIPv6: true, @@ -1195,7 +1200,9 @@ func TestGetClusterNodeMask(t *testing.T) { name: "dual ipv4 custom mask", cfg: &kubeadmapi.ClusterConfiguration{ ControllerManager: kubeadmapi.ControlPlaneComponent{ - ExtraArgs: map[string]string{"node-cidr-mask-size-ipv4": "23"}, + ExtraArgs: []kubeadmapi.Arg{ + {Name: "node-cidr-mask-size-ipv4", Value: "23"}, + }, }, }, isIPv6: false, @@ -1205,7 +1212,9 @@ func TestGetClusterNodeMask(t *testing.T) { name: "dual ipv4 wrong mask", cfg: &kubeadmapi.ClusterConfiguration{ ControllerManager: kubeadmapi.ControlPlaneComponent{ - ExtraArgs: map[string]string{"node-cidr-mask-size-ipv4": "aa"}, + ExtraArgs: []kubeadmapi.Arg{ + {Name: "node-cidr-mask-size-ipv4", Value: "aa"}, + }, }, }, isIPv6: false, @@ -1215,7 +1224,9 @@ func TestGetClusterNodeMask(t *testing.T) { name: "dual ipv6 default mask and legacy flag", cfg: &kubeadmapi.ClusterConfiguration{ ControllerManager: kubeadmapi.ControlPlaneComponent{ - ExtraArgs: map[string]string{"node-cidr-mask-size": "23"}, + ExtraArgs: []kubeadmapi.Arg{ + {Name: "node-cidr-mask-size", Value: "23"}, + }, }, }, isIPv6: true, @@ -1225,7 +1236,10 @@ func TestGetClusterNodeMask(t *testing.T) { name: "dual ipv6 custom mask and legacy flag", cfg: &kubeadmapi.ClusterConfiguration{ ControllerManager: kubeadmapi.ControlPlaneComponent{ - ExtraArgs: map[string]string{"node-cidr-mask-size": "23", "node-cidr-mask-size-ipv6": "83"}, + ExtraArgs: []kubeadmapi.Arg{ + {Name: "node-cidr-mask-size", Value: "23"}, + {Name: "node-cidr-mask-size-ipv6", Value: "83"}, + }, }, }, isIPv6: true, @@ -1235,7 +1249,10 @@ func TestGetClusterNodeMask(t *testing.T) { name: "dual ipv6 custom mask and wrong flag", cfg: &kubeadmapi.ClusterConfiguration{ ControllerManager: kubeadmapi.ControlPlaneComponent{ - ExtraArgs: map[string]string{"node-cidr-mask-size": "23", "node-cidr-mask-size-ipv6": "a83"}, + ExtraArgs: []kubeadmapi.Arg{ + {Name: "node-cidr-mask-size", Value: "23"}, + {Name: "node-cidr-mask-size-ipv6", Value: "a83"}, + }, }, }, isIPv6: true, @@ -1354,3 +1371,34 @@ func TestValidateAbsolutePath(t *testing.T) { } } } + +func TestValidateExtraArgs(t *testing.T) { + var tests = []struct { + name string + args []kubeadmapi.Arg + expectedErrors int + }{ + { + name: "valid argument", + args: []kubeadmapi.Arg{{Name: "foo", Value: "bar"}}, + expectedErrors: 0, + }, + { + name: "invalid one argument", + args: []kubeadmapi.Arg{{Name: "", Value: "bar"}}, + expectedErrors: 1, + }, + { + name: "invalid two arguments", + args: []kubeadmapi.Arg{{Name: "", Value: "foo"}, {Name: "", Value: "bar"}}, + expectedErrors: 2, + }, + } + + for _, tc := range tests { + actual := ValidateExtraArgs(tc.args, nil) + if len(actual) != tc.expectedErrors { + t.Errorf("case %q:\n\t expected errors: %v\n\t got: %v\n\t errors: %v", tc.name, tc.expectedErrors, len(actual), actual) + } + } +} diff --git a/cmd/kubeadm/app/util/arguments.go b/cmd/kubeadm/app/util/arguments.go index 72c79d9be2a..82ab5d48a50 100644 --- a/cmd/kubeadm/app/util/arguments.go +++ b/cmd/kubeadm/app/util/arguments.go @@ -24,41 +24,54 @@ import ( "github.com/pkg/errors" "k8s.io/klog/v2" + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" ) -// BuildArgumentListFromMap takes two string-string maps, one with the base arguments and one +// ArgumentsToCommand takes two Arg slices, one with the base arguments and one // with optional override arguments. In the return list override arguments will precede base -// arguments -func BuildArgumentListFromMap(baseArguments map[string]string, overrideArguments map[string]string) []string { +// arguments. If an argument is present in the overrides, it will cause +// all instances of the same argument in the base list to be discarded, leaving +// only the instances of this argument in the overrides to be applied. +func ArgumentsToCommand(base []kubeadmapi.Arg, overrides []kubeadmapi.Arg) []string { var command []string - var keys []string + // Copy the base arguments into a new slice. + args := make([]kubeadmapi.Arg, len(base)) + copy(args, base) - argsMap := make(map[string]string) - - for k, v := range baseArguments { - argsMap[k] = v + // Go trough the override arguments and delete all instances of arguments with the same name + // in the base list of arguments. + for i := 0; i < len(overrides); i++ { + repeat: + for j := 0; j < len(args); j++ { + if overrides[i].Name == args[j].Name { + // Remove this existing argument and search for another argument + // with the same name in base. + args = append(args[:j], args[j+1:]...) + goto repeat + } + } } - for k, v := range overrideArguments { - argsMap[k] = v - } + // Concatenate the overrides and the base arguments and then sort them. + args = append(args, overrides...) + sort.Slice(args, func(i, j int) bool { + if args[i].Name == args[j].Name { + return args[i].Value < args[j].Value + } + return args[i].Name < args[j].Name + }) - for k := range argsMap { - keys = append(keys, k) - } - - sort.Strings(keys) - for _, k := range keys { - command = append(command, fmt.Sprintf("--%s=%s", k, argsMap[k])) + for _, arg := range args { + command = append(command, fmt.Sprintf("--%s=%s", arg.Name, arg.Value)) } return command } -// ParseArgumentListToMap parses a CLI argument list in the form "--foo=bar" to a string-string map -func ParseArgumentListToMap(arguments []string) map[string]string { - resultingMap := map[string]string{} - for i, arg := range arguments { +// ArgumentsFromCommand parses a CLI command in the form "--foo=bar" to an Arg slice +func ArgumentsFromCommand(command []string) []kubeadmapi.Arg { + args := []kubeadmapi.Arg{} + for i, arg := range command { key, val, err := parseArgument(arg) // Ignore if the first argument doesn't satisfy the criteria, it's most often the binary name @@ -70,24 +83,16 @@ func ParseArgumentListToMap(arguments []string) map[string]string { continue } - resultingMap[key] = val + args = append(args, kubeadmapi.Arg{Name: key, Value: val}) } - return resultingMap -} -// ReplaceArgument gets a command list; converts it to a map for easier modification, runs the provided function that -// returns a new modified map, and then converts the map back to a command string slice -func ReplaceArgument(command []string, argMutateFunc func(map[string]string) map[string]string) []string { - argMap := ParseArgumentListToMap(command) - - // Save the first command (the executable) if we're sure it's not an argument (i.e. no --) - var newCommand []string - if len(command) > 0 && !strings.HasPrefix(command[0], "--") { - newCommand = append(newCommand, command[0]) - } - newArgMap := argMutateFunc(argMap) - newCommand = append(newCommand, BuildArgumentListFromMap(newArgMap, map[string]string{})...) - return newCommand + sort.Slice(args, func(i, j int) bool { + if args[i].Name == args[j].Name { + return args[i].Value < args[j].Value + } + return args[i].Name < args[j].Name + }) + return args } // parseArgument parses the argument "--foo=bar" to "foo" and "bar" diff --git a/cmd/kubeadm/app/util/arguments_test.go b/cmd/kubeadm/app/util/arguments_test.go index 31bc9e88a3a..a796293fb1d 100644 --- a/cmd/kubeadm/app/util/arguments_test.go +++ b/cmd/kubeadm/app/util/arguments_test.go @@ -20,23 +20,25 @@ import ( "reflect" "sort" "testing" + + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" ) -func TestBuildArgumentListFromMap(t *testing.T) { +func TestArgumentsToCommand(t *testing.T) { var tests = []struct { name string - base map[string]string - overrides map[string]string + base []kubeadmapi.Arg + overrides []kubeadmapi.Arg expected []string }{ { name: "override an argument from the base", - base: map[string]string{ - "admission-control": "NamespaceLifecycle", - "allow-privileged": "true", + base: []kubeadmapi.Arg{ + {Name: "admission-control", Value: "NamespaceLifecycle"}, + {Name: "allow-privileged", Value: "true"}, }, - overrides: map[string]string{ - "admission-control": "NamespaceLifecycle,LimitRanger", + overrides: []kubeadmapi.Arg{ + {Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"}, }, expected: []string{ "--admission-control=NamespaceLifecycle,LimitRanger", @@ -44,12 +46,43 @@ func TestBuildArgumentListFromMap(t *testing.T) { }, }, { - name: "add an argument that is not in base", - base: map[string]string{ - "allow-privileged": "true", + name: "override an argument from the base and add duplicate", + base: []kubeadmapi.Arg{ + {Name: "token-auth-file", Value: "/token"}, + {Name: "tls-sni-cert-key", Value: "/some/path/"}, }, - overrides: map[string]string{ - "admission-control": "NamespaceLifecycle,LimitRanger", + overrides: []kubeadmapi.Arg{ + {Name: "tls-sni-cert-key", Value: "/some/new/path"}, + {Name: "tls-sni-cert-key", Value: "/some/new/path/subpath"}, + }, + expected: []string{ + "--tls-sni-cert-key=/some/new/path", + "--tls-sni-cert-key=/some/new/path/subpath", + "--token-auth-file=/token", + }, + }, + { + name: "override all duplicate arguments from base", + base: []kubeadmapi.Arg{ + {Name: "token-auth-file", Value: "/token"}, + {Name: "tls-sni-cert-key", Value: "foo"}, + {Name: "tls-sni-cert-key", Value: "bar"}, + }, + overrides: []kubeadmapi.Arg{ + {Name: "tls-sni-cert-key", Value: "/some/new/path"}, + }, + expected: []string{ + "--tls-sni-cert-key=/some/new/path", + "--token-auth-file=/token", + }, + }, + { + name: "add an argument that is not in base", + base: []kubeadmapi.Arg{ + {Name: "allow-privileged", Value: "true"}, + }, + overrides: []kubeadmapi.Arg{ + {Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"}, }, expected: []string{ "--admission-control=NamespaceLifecycle,LimitRanger", @@ -58,12 +91,12 @@ func TestBuildArgumentListFromMap(t *testing.T) { }, { name: "allow empty strings in base", - base: map[string]string{ - "allow-privileged": "true", - "something-that-allows-empty-string": "", + base: []kubeadmapi.Arg{ + {Name: "allow-privileged", Value: "true"}, + {Name: "something-that-allows-empty-string", Value: ""}, }, - overrides: map[string]string{ - "admission-control": "NamespaceLifecycle,LimitRanger", + overrides: []kubeadmapi.Arg{ + {Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"}, }, expected: []string{ "--admission-control=NamespaceLifecycle,LimitRanger", @@ -73,13 +106,13 @@ func TestBuildArgumentListFromMap(t *testing.T) { }, { name: "allow empty strings in overrides", - base: map[string]string{ - "allow-privileged": "true", - "something-that-allows-empty-string": "foo", + base: []kubeadmapi.Arg{ + {Name: "allow-privileged", Value: "true"}, + {Name: "something-that-allows-empty-string", Value: "foo"}, }, - overrides: map[string]string{ - "admission-control": "NamespaceLifecycle,LimitRanger", - "something-that-allows-empty-string": "", + overrides: []kubeadmapi.Arg{ + {Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"}, + {Name: "something-that-allows-empty-string", Value: ""}, }, expected: []string{ "--admission-control=NamespaceLifecycle,LimitRanger", @@ -91,19 +124,19 @@ func TestBuildArgumentListFromMap(t *testing.T) { for _, rt := range tests { t.Run(rt.name, func(t *testing.T) { - actual := BuildArgumentListFromMap(rt.base, rt.overrides) + actual := ArgumentsToCommand(rt.base, rt.overrides) if !reflect.DeepEqual(actual, rt.expected) { - t.Errorf("failed BuildArgumentListFromMap:\nexpected:\n%v\nsaw:\n%v", rt.expected, actual) + t.Errorf("failed ArgumentsToCommand:\nexpected:\n%v\nsaw:\n%v", rt.expected, actual) } }) } } -func TestParseArgumentListToMap(t *testing.T) { +func TestArgumentsFromCommand(t *testing.T) { var tests = []struct { - name string - args []string - expectedMap map[string]string + name string + args []string + expected []kubeadmapi.Arg }{ { name: "normal case", @@ -111,9 +144,9 @@ func TestParseArgumentListToMap(t *testing.T) { "--admission-control=NamespaceLifecycle,LimitRanger", "--allow-privileged=true", }, - expectedMap: map[string]string{ - "admission-control": "NamespaceLifecycle,LimitRanger", - "allow-privileged": "true", + expected: []kubeadmapi.Arg{ + {Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"}, + {Name: "allow-privileged", Value: "true"}, }, }, { @@ -123,10 +156,10 @@ func TestParseArgumentListToMap(t *testing.T) { "--allow-privileged=true", "--feature-gates=EnableFoo=true,EnableBar=false", }, - expectedMap: map[string]string{ - "admission-control": "NamespaceLifecycle,LimitRanger", - "allow-privileged": "true", - "feature-gates": "EnableFoo=true,EnableBar=false", + expected: []kubeadmapi.Arg{ + {Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"}, + {Name: "allow-privileged", Value: "true"}, + {Name: "feature-gates", Value: "EnableFoo=true,EnableBar=false"}, }, }, { @@ -137,75 +170,32 @@ func TestParseArgumentListToMap(t *testing.T) { "--allow-privileged=true", "--feature-gates=EnableFoo=true,EnableBar=false", }, - expectedMap: map[string]string{ - "admission-control": "NamespaceLifecycle,LimitRanger", - "allow-privileged": "true", - "feature-gates": "EnableFoo=true,EnableBar=false", + expected: []kubeadmapi.Arg{ + {Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"}, + {Name: "allow-privileged", Value: "true"}, + {Name: "feature-gates", Value: "EnableFoo=true,EnableBar=false"}, + }, + }, + { + name: "allow duplicate args", + args: []string{ + "--admission-control=NamespaceLifecycle,LimitRanger", + "--tls-sni-cert-key=/some/path", + "--tls-sni-cert-key=/some/path/subpath", + }, + expected: []kubeadmapi.Arg{ + {Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"}, + {Name: "tls-sni-cert-key", Value: "/some/path"}, + {Name: "tls-sni-cert-key", Value: "/some/path/subpath"}, }, }, } for _, rt := range tests { t.Run(rt.name, func(t *testing.T) { - actualMap := ParseArgumentListToMap(rt.args) - if !reflect.DeepEqual(actualMap, rt.expectedMap) { - t.Errorf("failed ParseArgumentListToMap:\nexpected:\n%v\nsaw:\n%v", rt.expectedMap, actualMap) - } - }) - } -} - -func TestReplaceArgument(t *testing.T) { - var tests = []struct { - name string - args []string - mutateFunc func(map[string]string) map[string]string - expectedArgs []string - }{ - { - name: "normal case", - args: []string{ - "kube-apiserver", - "--admission-control=NamespaceLifecycle,LimitRanger", - "--allow-privileged=true", - }, - mutateFunc: func(argMap map[string]string) map[string]string { - argMap["admission-control"] = "NamespaceLifecycle,LimitRanger,ResourceQuota" - return argMap - }, - expectedArgs: []string{ - "kube-apiserver", - "--admission-control=NamespaceLifecycle,LimitRanger,ResourceQuota", - "--allow-privileged=true", - }, - }, - { - name: "another normal case", - args: []string{ - "kube-apiserver", - "--admission-control=NamespaceLifecycle,LimitRanger", - "--allow-privileged=true", - }, - mutateFunc: func(argMap map[string]string) map[string]string { - argMap["new-arg-here"] = "foo" - return argMap - }, - expectedArgs: []string{ - "kube-apiserver", - "--admission-control=NamespaceLifecycle,LimitRanger", - "--allow-privileged=true", - "--new-arg-here=foo", - }, - }, - } - - for _, rt := range tests { - t.Run(rt.name, func(t *testing.T) { - actualArgs := ReplaceArgument(rt.args, rt.mutateFunc) - sort.Strings(actualArgs) - sort.Strings(rt.expectedArgs) - if !reflect.DeepEqual(actualArgs, rt.expectedArgs) { - t.Errorf("failed ReplaceArgument:\nexpected:\n%v\nsaw:\n%v", rt.expectedArgs, actualArgs) + actual := ArgumentsFromCommand(rt.args) + if !reflect.DeepEqual(actual, rt.expected) { + t.Errorf("failed ArgumentsFromCommand:\nexpected:\n%v\nsaw:\n%v", rt.expected, actual) } }) } @@ -236,7 +226,7 @@ func TestRoundtrip(t *testing.T) { for _, rt := range tests { t.Run(rt.name, func(t *testing.T) { // These two methods should be each other's opposite functions, test that by chaining the methods and see if you get the same result back - actual := BuildArgumentListFromMap(ParseArgumentListToMap(rt.args), map[string]string{}) + actual := ArgumentsToCommand(ArgumentsFromCommand(rt.args), []kubeadmapi.Arg{}) sort.Strings(actual) sort.Strings(rt.args)