kubeadm: continue supporting extraArgs flags for v1beta4

Flags for kubeadm init such as --apiserver-extra-args prior
to v1beta4 used a map[string]string for pflag.Value storage. This no
longer works since v1beta4 extra args are a slice of Arg.

Add a new flag type argSlice and implement a solution for
parsing these flags.

At the same time deprecate these flags and show a warning
that users should use config.

cmd/kubeadm/app/cmd/options/argslice.go

@@ -0,0 +1,73 @@
+/*
+Copyright 2024 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 options
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/pkg/errors"
+
+	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4"
+)
+
+type argSlice struct {
+	args *[]kubeadmapiv1.Arg
+}
+
+func newArgSlice(args *[]kubeadmapiv1.Arg) *argSlice {
+	return &argSlice{args: args}
+}
+
+// String implements github.com/spf13/pflag.Value
+func (s *argSlice) String() string {
+	if s == nil || s.args == nil || len(*s.args) == 0 {
+		return ""
+	}
+
+	pairs := make([]string, 0, len(*s.args))
+	for _, a := range *s.args {
+		pairs = append(pairs, fmt.Sprintf("%s=%s", a.Name, a.Value))
+	}
+
+	return strings.Join(pairs, ",")
+}
+
+// Set implements github.com/spf13/pflag.Value
+func (s *argSlice) Set(value string) error {
+	if s.args == nil {
+		s.args = &[]kubeadmapiv1.Arg{}
+	}
+
+	pairs := strings.Split(value, ",")
+
+	for _, p := range pairs {
+		m := strings.Split(p, "=")
+		if len(m) != 2 {
+			return errors.Errorf("malformed key=value pair in flag value: %s", value)
+		}
+		arg := kubeadmapiv1.Arg{Name: m[0], Value: m[1]}
+		*s.args = append(*s.args, arg)
+	}
+
+	return nil
+}
+
+// Type implements github.com/spf13/pflag.Value
+func (s *argSlice) Type() string {
+	return "[]kubeadmapiv1.Arg"
+}

cmd/kubeadm/app/cmd/options/argslice_test.go

@@ -0,0 +1,76 @@
+/*
+Copyright 2024 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 options
+
+import (
+	"reflect"
+	"testing"
+
+	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4"
+)
+
+func TestArgSliceStringSet(t *testing.T) {
+	tests := []struct {
+		name   string
+		input  *[]kubeadmapiv1.Arg
+		output string
+	}{
+		// a test case with nil is input is not needed because ExtraArgs are never nil in ClusterConfiguration
+		{
+			name:   "no args",
+			input:  &[]kubeadmapiv1.Arg{},
+			output: "",
+		},
+		{
+			name: "one arg",
+			input: &[]kubeadmapiv1.Arg{
+				{Name: "foo", Value: "bar"},
+			},
+			output: "foo=bar",
+		},
+		{
+			name: "two args",
+			input: &[]kubeadmapiv1.Arg{
+				{Name: "foo", Value: "bar"},
+				{Name: "baz", Value: "qux"},
+			},
+			output: "foo=bar,baz=qux",
+		},
+		{
+			name: "three args with a duplicate",
+			input: &[]kubeadmapiv1.Arg{
+				{Name: "foo", Value: "bar"},
+				{Name: "foo", Value: "qux"},
+				{Name: "baz", Value: "qux"},
+			},
+			output: "foo=bar,foo=qux,baz=qux",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s := newArgSlice(tt.input)
+			gotString := s.String()
+			if ok := reflect.DeepEqual(gotString, tt.output); !ok {
+				t.Errorf("String()\ngot: %v\noutput: %v", gotString, tt.output)
+			}
+			_ = s.Set(gotString)
+			if ok := reflect.DeepEqual(s.args, tt.input); !ok {
+				t.Errorf("Set()\ngot: %+v\noutput: %+v", s.args, tt.input)
+			}
+		})
+	}
+}

cmd/kubeadm/app/cmd/options/generic.go

@@ -17,11 +17,11 @@ limitations under the License.
 package options
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/spf13/pflag"
 
-	cliflag "k8s.io/component-base/cli/flag"
 	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4"
 
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
@@ -55,13 +55,18 @@ func AddIgnorePreflightErrorsFlag(fs *pflag.FlagSet, ignorePreflightErrors *[]st
 
 // AddControlPlanExtraArgsFlags adds the ExtraArgs flags for control plane components
 func AddControlPlanExtraArgsFlags(fs *pflag.FlagSet, apiServerExtraArgs, controllerManagerExtraArgs, schedulerExtraArgs *[]kubeadmapiv1.Arg) {
-	// TODO: https://github.com/kubernetes/kubeadm/issues/1601
-	// Either deprecate these flags or handle duplicate keys.
-	// Currently the map[string]string returned by NewMapStringString() doesn't allow this.
-	stub := &map[string]string{} // TODO
-	fs.Var(cliflag.NewMapStringString(stub), APIServerExtraArgs, "A set of extra flags to pass to the API Server or override default ones in form of <flagname>=<value>")
-	fs.Var(cliflag.NewMapStringString(stub), ControllerManagerExtraArgs, "A set of extra flags to pass to the Controller Manager or override default ones in form of <flagname>=<value>")
-	fs.Var(cliflag.NewMapStringString(stub), SchedulerExtraArgs, "A set of extra flags to pass to the Scheduler or override default ones in form of <flagname>=<value>")
+	// TODO: these flags are deprecated, remove them and related logic:
+	// - AddControlPlanExtraArgsFlag()
+	// - files app/cmd/options/argslice*.go
+	// - options.*ExtraArgs
+	// - usages in app/cmd/init.go and app/cmd/phases/init/controlplane.go
+	fs.Var(newArgSlice(apiServerExtraArgs), APIServerExtraArgs, "A set of extra flags to pass to the API Server or override default ones in form of <flagname>=<value>")
+	fs.Var(newArgSlice(controllerManagerExtraArgs), ControllerManagerExtraArgs, "A set of extra flags to pass to the Controller Manager or override default ones in form of <flagname>=<value>")
+	fs.Var(newArgSlice(schedulerExtraArgs), SchedulerExtraArgs, "A set of extra flags to pass to the Scheduler or override default ones in form of <flagname>=<value>")
+	const future = "This flag will be removed in a future version"
+	_ = fs.MarkDeprecated(APIServerExtraArgs, fmt.Sprintf("use 'ClusterConfiguration.apiServer.extraArgs' instead. %s", future))
+	_ = fs.MarkDeprecated(ControllerManagerExtraArgs, fmt.Sprintf("use 'ClusterConfiguration.controllerManager.extraArgs' instead. %s", future))
+	_ = fs.MarkDeprecated(SchedulerExtraArgs, fmt.Sprintf("use 'ClusterConfiguration.scheduler.extraArgs' instead. %s", future))
 }
 
 // AddImageMetaFlags adds the --image-repository flag to the given flagset