From 00e4a599f6f3510644549b7e3a3c0ee741ce264a Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 28 Sep 2021 20:59:15 +0200 Subject: [PATCH] command lines: always show flags with hyphens All Kubernetes commands should show flags with hyphens in their help text even when the flag originally was defined with underscore. Converting a command to this style is not breaking its command line API because the old-style parameter with underscore is accepted as alias. The easiest solution to achieve this is to set normalization shortly before running the command in the new central cli.Run or the few places where that function isn't used yet. There may be some texts which depends on normalization at flag definition time, like the --logging-format usage warning. Those get generated assuming that hyphens will be used. --- cmd/cloud-controller-manager/main.go | 4 ---- cmd/kube-apiserver/apiserver.go | 5 ----- cmd/kube-controller-manager/controller-manager.go | 5 ----- cmd/kube-proxy/app/server.go | 1 - cmd/kube-proxy/proxy.go | 5 ----- cmd/kube-scheduler/app/server.go | 2 -- cmd/kube-scheduler/scheduler.go | 5 ----- cmd/kubectl-convert/kubectl-convert.go | 3 --- cmd/kubectl/kubectl.go | 5 ----- cmd/kubelet/app/options/options.go | 1 - cmd/kubemark/hollow-node.go | 3 --- pkg/kubectl/cmd/convert/convert.go | 2 -- .../src/k8s.io/cloud-provider/app/controllermanager.go | 2 -- .../src/k8s.io/cloud-provider/app/testing/testserver.go | 3 --- staging/src/k8s.io/cloud-provider/sample/basic_main.go | 3 --- staging/src/k8s.io/component-base/cli/run.go | 7 ++++++- staging/src/k8s.io/component-base/logs/config.go | 6 +++++- .../src/k8s.io/component-base/logs/example/cmd/logger.go | 9 --------- staging/src/k8s.io/component-base/logs/options_test.go | 2 +- staging/src/k8s.io/component-base/logs/validate.go | 2 +- .../pod-security-admission/cmd/webhook/server/server.go | 2 -- .../k8s.io/pod-security-admission/cmd/webhook/webhook.go | 3 --- staging/src/k8s.io/sample-apiserver/go.mod | 1 - staging/src/k8s.io/sample-apiserver/main.go | 5 ----- 24 files changed, 13 insertions(+), 73 deletions(-) diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index 5eb38d73605..048ef9ad486 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -27,8 +27,6 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/apimachinery/pkg/util/wait" cloudprovider "k8s.io/cloud-provider" "k8s.io/cloud-provider/app" @@ -44,8 +42,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - ccmOptions, err := options.NewCloudControllerManagerOptions() if err != nil { klog.Fatalf("unable to initialize command options: %v", err) diff --git a/cmd/kube-apiserver/apiserver.go b/cmd/kube-apiserver/apiserver.go index fd61264a737..42956a9bf6e 100644 --- a/cmd/kube-apiserver/apiserver.go +++ b/cmd/kube-apiserver/apiserver.go @@ -21,10 +21,7 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" _ "k8s.io/component-base/logs/json/register" // for JSON log format registration _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration @@ -32,8 +29,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := app.NewAPIServerCommand() code := cli.Run(command) os.Exit(code) diff --git a/cmd/kube-controller-manager/controller-manager.go b/cmd/kube-controller-manager/controller-manager.go index df6a2c7fa09..2a34cfcdf3e 100644 --- a/cmd/kube-controller-manager/controller-manager.go +++ b/cmd/kube-controller-manager/controller-manager.go @@ -23,10 +23,7 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" _ "k8s.io/component-base/logs/json/register" // for JSON log format registration _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugin _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration @@ -34,8 +31,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := app.NewControllerManagerCommand() code := cli.Run(command) os.Exit(code) diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index d01dc81a28a..a5c90f0ad03 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -510,7 +510,6 @@ with the apiserver API to configure the proxy.`, return nil }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) var err error opts.config, err = opts.ApplyDefaults(opts.config) diff --git a/cmd/kube-proxy/proxy.go b/cmd/kube-proxy/proxy.go index d668c8da42d..86ef9dff68e 100644 --- a/cmd/kube-proxy/proxy.go +++ b/cmd/kube-proxy/proxy.go @@ -19,18 +19,13 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" _ "k8s.io/component-base/metrics/prometheus/restclient" // for client metric registration _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration "k8s.io/kubernetes/cmd/kube-proxy/app" ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := app.NewProxyCommand() code := cli.Run(command) os.Exit(code) diff --git a/cmd/kube-scheduler/app/server.go b/cmd/kube-scheduler/app/server.go index 0be5f9c5146..1bda9d0e579 100644 --- a/cmd/kube-scheduler/app/server.go +++ b/cmd/kube-scheduler/app/server.go @@ -25,7 +25,6 @@ import ( goruntime "runtime" "github.com/spf13/cobra" - "github.com/spf13/pflag" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apiserver/pkg/authentication/authenticator" @@ -100,7 +99,6 @@ for more information about scheduling and the kube-scheduler component.`, return nil }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) fs := cmd.Flags() verflag.AddFlags(namedFlagSets.FlagSet("global")) diff --git a/cmd/kube-scheduler/scheduler.go b/cmd/kube-scheduler/scheduler.go index 912e484bc73..71739808dd2 100644 --- a/cmd/kube-scheduler/scheduler.go +++ b/cmd/kube-scheduler/scheduler.go @@ -19,10 +19,7 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" _ "k8s.io/component-base/logs/json/register" // for JSON log format registration _ "k8s.io/component-base/metrics/prometheus/clientgo" _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration @@ -30,8 +27,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := app.NewSchedulerCommand() code := cli.Run(command) os.Exit(code) diff --git a/cmd/kubectl-convert/kubectl-convert.go b/cmd/kubectl-convert/kubectl-convert.go index 7b7610f4be4..d47b120515b 100644 --- a/cmd/kubectl-convert/kubectl-convert.go +++ b/cmd/kubectl-convert/kubectl-convert.go @@ -23,15 +23,12 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubernetes/pkg/kubectl/cmd/convert" ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) flags := pflag.NewFlagSet("kubectl-convert", pflag.ExitOnError) - flags.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) pflag.CommandLine = flags kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag() diff --git a/cmd/kubectl/kubectl.go b/cmd/kubectl/kubectl.go index 67576646e5f..7c7924a5f5f 100644 --- a/cmd/kubectl/kubectl.go +++ b/cmd/kubectl/kubectl.go @@ -19,10 +19,7 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" "k8s.io/kubectl/pkg/cmd" // Import to initialize client auth plugins. @@ -30,8 +27,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := cmd.NewDefaultKubectlCommand() code := cli.Run(command) os.Exit(code) diff --git a/cmd/kubelet/app/options/options.go b/cmd/kubelet/app/options/options.go index 0343cd6d820..527057d35d4 100644 --- a/cmd/kubelet/app/options/options.go +++ b/cmd/kubelet/app/options/options.go @@ -383,7 +383,6 @@ func (f *KubeletFlags) AddFlags(mainfs *pflag.FlagSet) { // AddKubeletConfigFlags adds flags for a specific kubeletconfig.KubeletConfiguration to the specified FlagSet func AddKubeletConfigFlags(mainfs *pflag.FlagSet, c *kubeletconfig.KubeletConfiguration) { fs := pflag.NewFlagSet("", pflag.ExitOnError) - fs.SetNormalizeFunc(mainfs.GetNormalizeFunc()) defer func() { // All KubeletConfiguration flags are now deprecated, and any new flags that point to // KubeletConfiguration fields are deprecated-on-creation. When removing flags at the end diff --git a/cmd/kubemark/hollow-node.go b/cmd/kubemark/hollow-node.go index df302d8ed3a..de254acb964 100644 --- a/cmd/kubemark/hollow-node.go +++ b/cmd/kubemark/hollow-node.go @@ -130,8 +130,6 @@ func (c *hollowNodeConfig) createHollowKubeletOptions() *kubemark.HollowKubletOp } func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := newHollowNodeCommand() code := cli.Run(command) os.Exit(code) @@ -160,7 +158,6 @@ func newHollowNodeCommand() *cobra.Command { return nil }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) fs := cmd.Flags() fs.AddGoFlagSet(goflag.CommandLine) // for flags like --docker-only diff --git a/pkg/kubectl/cmd/convert/convert.go b/pkg/kubectl/cmd/convert/convert.go index 3557704e27b..74b16ccbbe4 100644 --- a/pkg/kubectl/cmd/convert/convert.go +++ b/pkg/kubectl/cmd/convert/convert.go @@ -20,7 +20,6 @@ import ( "fmt" "github.com/spf13/cobra" - "github.com/spf13/pflag" "k8s.io/klog/v2" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -101,7 +100,6 @@ func NewCmdConvert(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *co cmdutil.CheckErr(o.RunConvert()) }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) cmd.Flags().BoolVar(&o.local, "local", o.local, "If true, convert will NOT try to contact api-server but run locally.") cmd.Flags().StringVar(&o.OutputVersion, "output-version", o.OutputVersion, i18n.T("Output the formatted object with the given group version (for ex: 'extensions/v1beta1').")) diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index 628e3a97cef..a458c4e0236 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -25,7 +25,6 @@ import ( "time" "github.com/spf13/cobra" - "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -103,7 +102,6 @@ the cloud specific control loops shipped with Kubernetes.`, return nil }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) fs := cmd.Flags() namedFlagSets := s.Flags(ControllerNames(controllerInitFuncConstructors), ControllersDisabledByDefault.List()) diff --git a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go index 9e334bf5524..16181464ddb 100644 --- a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go +++ b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go @@ -25,7 +25,6 @@ import ( "strings" "time" - "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" @@ -33,7 +32,6 @@ import ( "k8s.io/cloud-provider/app" "k8s.io/cloud-provider/app/config" "k8s.io/cloud-provider/options" - cliflag "k8s.io/component-base/cli/flag" ) // TearDownFunc is to be called to tear down a test server. @@ -104,7 +102,6 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err } fss := cliflag.NamedFlagSets{} command := app.NewCloudControllerManagerCommand(s, cloudInitializer, app.DefaultInitFuncConstructors, fss, stopCh) - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) commandArgs := []string{} listeners := []net.Listener{} diff --git a/staging/src/k8s.io/cloud-provider/sample/basic_main.go b/staging/src/k8s.io/cloud-provider/sample/basic_main.go index 4d6f820fefb..10af37ada12 100644 --- a/staging/src/k8s.io/cloud-provider/sample/basic_main.go +++ b/staging/src/k8s.io/cloud-provider/sample/basic_main.go @@ -23,7 +23,6 @@ package main import ( "os" - "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/util/wait" cloudprovider "k8s.io/cloud-provider" "k8s.io/cloud-provider/app" @@ -39,8 +38,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - ccmOptions, err := options.NewCloudControllerManagerOptions() if err != nil { klog.Fatalf("unable to initialize command options: %v", err) diff --git a/staging/src/k8s.io/component-base/cli/run.go b/staging/src/k8s.io/component-base/cli/run.go index 0d430e04ddc..8456b178a68 100644 --- a/staging/src/k8s.io/component-base/cli/run.go +++ b/staging/src/k8s.io/component-base/cli/run.go @@ -25,15 +25,20 @@ import ( "github.com/spf13/cobra" "k8s.io/component-base/logs" + cliflag "k8s.io/component-base/cli/flag" ) // Run provides the common boilerplate code around executing a cobra command. // For example, it ensures that logging is set up properly. Logging -// flags get added to the command line if not added already. +// flags get added to the command line if not added already. Flags get normalized +// so that help texts show them with hyphens. Underscores are accepted +// as alternative for the command parameters. func Run(cmd *cobra.Command) int { rand.Seed(time.Now().UnixNano()) defer logs.FlushLogs() + cmd.SetGlobalNormalizationFunc(cliflag.WordSepNormalizeFunc) + // This is idempotent. logs.AddFlags(cmd.PersistentFlags()) diff --git a/staging/src/k8s.io/component-base/logs/config.go b/staging/src/k8s.io/component-base/logs/config.go index 296cfb9ce6b..3bec829a238 100644 --- a/staging/src/k8s.io/component-base/logs/config.go +++ b/staging/src/k8s.io/component-base/logs/config.go @@ -24,6 +24,7 @@ import ( "github.com/spf13/pflag" + cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/config" "k8s.io/klog/v2" ) @@ -58,7 +59,10 @@ var supportedLogsFlags = map[string]struct{}{ // BindLoggingFlags binds the Options struct fields to a flagset func BindLoggingFlags(c *config.LoggingConfiguration, fs *pflag.FlagSet) { - unsupportedFlags := strings.Join(unsupportedLoggingFlagNames(fs.GetNormalizeFunc()), ", ") + // The help text is generated assuming that flags will eventually use + // hyphens, even if currently no normalization function is set for the + // flag set yet. + unsupportedFlags := strings.Join(unsupportedLoggingFlagNames(cliflag.WordSepNormalizeFunc), ", ") formats := fmt.Sprintf(`"%s"`, strings.Join(LogRegistry.List(), `", "`)) fs.StringVar(&c.Format, "logging-format", c.Format, fmt.Sprintf("Sets the log format. Permitted formats: %s.\nNon-default formats don't honor these flags: %s.\nNon-default choices are currently alpha and subject to change without warning.", formats, unsupportedFlags)) // No new log formats should be added after generation is of flag options diff --git a/staging/src/k8s.io/component-base/logs/example/cmd/logger.go b/staging/src/k8s.io/component-base/logs/example/cmd/logger.go index 0e63e565b74..c3ee30f1082 100644 --- a/staging/src/k8s.io/component-base/logs/example/cmd/logger.go +++ b/staging/src/k8s.io/component-base/logs/example/cmd/logger.go @@ -22,10 +22,8 @@ import ( "os" "github.com/spf13/cobra" - "github.com/spf13/pflag" "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/logs" "k8s.io/klog/v2" @@ -33,11 +31,6 @@ import ( ) func main() { - // NamedFlagSets use the global pflag.CommandLine. We don't use those - // in this command (yet), but set it globally anyway for consistency - // with other commands. - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := NewLoggerCommand() code := cli.Run(command) os.Exit(code) @@ -56,8 +49,6 @@ func NewLoggerCommand() *cobra.Command { runLogger() }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) - o.AddFlags(cmd.Flags()) return cmd } diff --git a/staging/src/k8s.io/component-base/logs/options_test.go b/staging/src/k8s.io/component-base/logs/options_test.go index 3472f949d89..f33d060f2d4 100644 --- a/staging/src/k8s.io/component-base/logs/options_test.go +++ b/staging/src/k8s.io/component-base/logs/options_test.go @@ -37,7 +37,7 @@ func TestFlags(t *testing.T) { want := ` --experimental-logging-sanitization [Experimental] When enabled prevents logging of fields tagged as sensitive (passwords, keys, tokens). Runtime log sanitization may introduce significant computation overhead and therefore should not be enabled in production. --logging-format string Sets the log format. Permitted formats: "text". - Non-default formats don't honor these flags: --add_dir_header, --alsologtostderr, --log_backtrace_at, --log_dir, --log_file, --log_file_max_size, --logtostderr, --one_output, --skip_headers, --skip_log_headers, --stderrthreshold, --vmodule. + Non-default formats don't honor these flags: --add-dir-header, --alsologtostderr, --log-backtrace-at, --log-dir, --log-file, --log-file-max-size, --logtostderr, --one-output, --skip-headers, --skip-log-headers, --stderrthreshold, --vmodule. Non-default choices are currently alpha and subject to change without warning. (default "text") ` if !assert.Equal(t, want, output.String()) { diff --git a/staging/src/k8s.io/component-base/logs/validate.go b/staging/src/k8s.io/component-base/logs/validate.go index c6858af49ae..f0ed8cde1ce 100644 --- a/staging/src/k8s.io/component-base/logs/validate.go +++ b/staging/src/k8s.io/component-base/logs/validate.go @@ -27,7 +27,7 @@ import ( func ValidateLoggingConfiguration(c *config.LoggingConfiguration, fldPath *field.Path) field.ErrorList { errs := field.ErrorList{} if c.Format != DefaultLogFormat { - // WordSepNormalizeFunc is just a guess. Most commands use it, + // WordSepNormalizeFunc is just a guess. Commands should use it, // but we cannot know for sure. allFlags := UnsupportedLoggingFlags(cliflag.WordSepNormalizeFunc) for _, f := range allFlags { diff --git a/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go b/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go index e8ea13022ba..d922529e03a 100644 --- a/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go +++ b/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go @@ -27,7 +27,6 @@ import ( "time" "github.com/spf13/cobra" - "github.com/spf13/pflag" admissionv1 "k8s.io/api/admission/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -59,7 +58,6 @@ Security Standards.`, }, Args: cobra.NoArgs, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) opts.AddFlags(cmd.Flags()) return cmd diff --git a/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go b/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go index 874184d56a6..0297288b36a 100644 --- a/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go +++ b/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go @@ -25,9 +25,6 @@ import ( ) func main() { - // Due to historic reasons, this command does *not* normalize command - // line flags. - command := server.NewServerCommand() code := cli.Run(command) os.Exit(code) diff --git a/staging/src/k8s.io/sample-apiserver/go.mod b/staging/src/k8s.io/sample-apiserver/go.mod index b0801662214..2a1dbd2803f 100644 --- a/staging/src/k8s.io/sample-apiserver/go.mod +++ b/staging/src/k8s.io/sample-apiserver/go.mod @@ -7,7 +7,6 @@ go 1.16 require ( github.com/google/gofuzz v1.1.0 github.com/spf13/cobra v1.2.1 - github.com/spf13/pflag v1.0.5 k8s.io/apimachinery v0.0.0 k8s.io/apiserver v0.0.0 k8s.io/client-go v0.0.0 diff --git a/staging/src/k8s.io/sample-apiserver/main.go b/staging/src/k8s.io/sample-apiserver/main.go index a926f0cee4b..708a2a43125 100644 --- a/staging/src/k8s.io/sample-apiserver/main.go +++ b/staging/src/k8s.io/sample-apiserver/main.go @@ -19,17 +19,12 @@ package main import ( "os" - "github.com/spf13/pflag" - genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" "k8s.io/sample-apiserver/pkg/cmd/server" ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - stopCh := genericapiserver.SetupSignalHandler() options := server.NewWardleServerOptions(os.Stdout, os.Stderr) cmd := server.NewCommandStartWardleServer(options, stopCh)