kubeadm: do not sort extraArgs alpha-numerically

If the user has provided extraArgs with an order that has
significance (e.g. --service-account-issuer for kube-apiserver),
kubeadm will correctly override any base args, but will end up
sorting the entire resulting list, which is not desired.

Instead, only sort the base arguments and preserve the order
of overrides provided by the user.

cmd/kubeadm/app/apis/kubeadm/types.go

@@ -165,6 +165,7 @@ type ControlPlaneComponent struct {
 	// An argument name in this list is the flag name as it appears on the
 	// command line except without leading dash(es). Extra arguments will override existing
 	// default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	ExtraArgs []Arg
 
 	// ExtraVolumes is an extra set of host volumes, mounted to the control plane component.
@@ -247,6 +248,7 @@ type NodeRegistrationOptions struct {
 	// Flags have higher priority when parsing. These values are local and specific to the node kubeadm is executing on.
 	// An argument name in this list is the flag name as it appears on the command line except without leading dash(es).
 	// Extra arguments will override existing default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	KubeletExtraArgs []Arg
 
 	// IgnorePreflightErrors provides a slice of pre-flight errors to be ignored when the current node is registered, e.g. 'IsPrivilegedUser,Swap'.
@@ -298,6 +300,7 @@ type LocalEtcd struct {
 	// An argument name in this list is the flag name as it appears on the
 	// command line except without leading dash(es). Extra arguments will override existing
 	// default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	ExtraArgs []Arg
 
 	// ExtraEnvs is an extra set of environment variables to pass to the control plane component.

cmd/kubeadm/app/apis/kubeadm/v1beta4/types.go

@@ -170,6 +170,7 @@ type ControlPlaneComponent struct {
 	// An argument name in this list is the flag name as it appears on the
 	// command line except without leading dash(es). Extra arguments will override existing
 	// default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	// +optional
 	ExtraArgs []Arg `json:"extraArgs,omitempty"`
 
@@ -260,6 +261,7 @@ type NodeRegistrationOptions struct {
 	// Flags have higher priority when parsing. These values are local and specific to the node kubeadm is executing on.
 	// An argument name in this list is the flag name as it appears on the command line except without leading dash(es).
 	// Extra arguments will override existing default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	// +optional
 	KubeletExtraArgs []Arg `json:"kubeletExtraArgs,omitempty"`
 
@@ -321,6 +323,7 @@ type LocalEtcd struct {
 	// An argument name in this list is the flag name as it appears on the
 	// command line except without leading dash(es). Extra arguments will override existing
 	// default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	// +optional
 	ExtraArgs []Arg `json:"extraArgs,omitempty"`
 

cmd/kubeadm/app/util/arguments.go

@@ -30,37 +30,34 @@ import (
 )
 
 // 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. If an argument is present in the overrides, it will cause
+// with optional override arguments. In the return list, base arguments will precede
+// override 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
-	// Copy the overrides arguments into a new slice.
-	args := make([]kubeadmapi.Arg, len(overrides))
-	copy(args, overrides)
+func ArgumentsToCommand(base, overrides []kubeadmapi.Arg) []string {
+	// Sort only the base.
+	sortArgsSlice(&base)
 
-	// overrideArgs is a set of args which will replace the args defined in the base
+	// Collect all overrides in a set.
 	overrideArgs := sets.New[string]()
 	for _, arg := range overrides {
 		overrideArgs.Insert(arg.Name)
 	}
 
+	// Append only the base args that do not have overrides.
+	args := make([]kubeadmapi.Arg, 0, len(base)+len(overrides))
 	for _, arg := range base {
 		if !overrideArgs.Has(arg.Name) {
 			args = append(args, arg)
 		}
 	}
 
-	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
-	})
+	// Append the overrides.
+	args = append(args, overrides...)
 
-	for _, arg := range args {
-		command = append(command, fmt.Sprintf("--%s=%s", arg.Name, arg.Value))
+	command := make([]string, len(args))
+	for i, arg := range args {
+		command[i] = fmt.Sprintf("--%s=%s", arg.Name, arg.Value)
 	}
 
 	return command
@@ -86,12 +83,8 @@ func ArgumentsFromCommand(command []string) []kubeadmapi.Arg {
 		args = append(args, kubeadmapi.Arg{Name: key, Value: val})
 	}
 
-	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
-	})
+	sortArgsSlice(&args)
+
 	return args
 }
 
@@ -118,3 +111,14 @@ func parseArgument(arg string) (string, string, error) {
 
 	return keyvalSlice[0], keyvalSlice[1], nil
 }
+
+// sortArgsSlice sorts a slice of Args alpha-numerically.
+func sortArgsSlice(argsPtr *[]kubeadmapi.Arg) {
+	args := *argsPtr
+	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
+	})
+}

cmd/kubeadm/app/util/arguments_test.go

@@ -41,8 +41,8 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"},
 			},
 			expected: []string{
-				"--admission-control=NamespaceLifecycle,LimitRanger",
 				"--allow-privileged=true",
+				"--admission-control=NamespaceLifecycle,LimitRanger",
 			},
 		},
 		{
@@ -56,9 +56,9 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "tls-sni-cert-key", Value: "/some/new/path/subpath"},
 			},
 			expected: []string{
+				"--token-auth-file=/token",
 				"--tls-sni-cert-key=/some/new/path",
 				"--tls-sni-cert-key=/some/new/path/subpath",
-				"--token-auth-file=/token",
 			},
 		},
 		{
@@ -72,8 +72,8 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "tls-sni-cert-key", Value: "/some/new/path"},
 			},
 			expected: []string{
-				"--tls-sni-cert-key=/some/new/path",
 				"--token-auth-file=/token",
+				"--tls-sni-cert-key=/some/new/path",
 			},
 		},
 		{
@@ -85,8 +85,8 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"},
 			},
 			expected: []string{
-				"--admission-control=NamespaceLifecycle,LimitRanger",
 				"--allow-privileged=true",
+				"--admission-control=NamespaceLifecycle,LimitRanger",
 			},
 		},
 		{
@@ -99,9 +99,9 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"},
 			},
 			expected: []string{
-				"--admission-control=NamespaceLifecycle,LimitRanger",
 				"--allow-privileged=true",
 				"--something-that-allows-empty-string=",
+				"--admission-control=NamespaceLifecycle,LimitRanger",
 			},
 		},
 		{
@@ -115,11 +115,31 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "something-that-allows-empty-string", Value: ""},
 			},
 			expected: []string{
-				"--admission-control=NamespaceLifecycle,LimitRanger",
 				"--allow-privileged=true",
+				"--admission-control=NamespaceLifecycle,LimitRanger",
 				"--something-that-allows-empty-string=",
 			},
 		},
+		{
+			name: "base are sorted and overrides are not",
+			base: []kubeadmapi.Arg{
+				{Name: "b", Value: "true"},
+				{Name: "c", Value: "true"},
+				{Name: "a", Value: "true"},
+			},
+			overrides: []kubeadmapi.Arg{
+				{Name: "e", Value: "true"},
+				{Name: "b", Value: "true"},
+				{Name: "d", Value: "true"},
+			},
+			expected: []string{
+				"--a=true",
+				"--c=true",
+				"--e=true",
+				"--b=true",
+				"--d=true",
+			},
+		},
 	}
 
 	for _, rt := range tests {
@@ -189,6 +209,21 @@ func TestArgumentsFromCommand(t *testing.T) {
 				{Name: "tls-sni-cert-key", Value: "/some/path/subpath"},
 			},
 		},
+		{
+			name: "args are sorted",
+			args: []string{
+				"--c=foo",
+				"--a=foo",
+				"--b=foo",
+				"--b=bar",
+			},
+			expected: []kubeadmapi.Arg{
+				{Name: "a", Value: "foo"},
+				{Name: "b", Value: "bar"},
+				{Name: "b", Value: "foo"},
+				{Name: "c", Value: "foo"},
+			},
+		},
 	}
 
 	for _, rt := range tests {