From 45cf1697ba0080fd9443245323b4ce2e455d7ae2 Mon Sep 17 00:00:00 2001
From: brianpursley <bpursley@cinlogic.com>
Date: Thu, 22 Jul 2021 12:38:18 -0400
Subject: [PATCH] Changed flag name underscore warning to avoid recommending
 potentially invalid flag name

---
 .../k8s.io/component-base/cli/flag/flags.go   |   7 +-
 .../component-base/cli/flag/flags_test.go     | 107 ++++++++++++++++++
 2 files changed, 113 insertions(+), 1 deletion(-)
 create mode 100644 staging/src/k8s.io/component-base/cli/flag/flags_test.go

diff --git a/staging/src/k8s.io/component-base/cli/flag/flags.go b/staging/src/k8s.io/component-base/cli/flag/flags.go
index 70146f33591..2388340d5ca 100644
--- a/staging/src/k8s.io/component-base/cli/flag/flags.go
+++ b/staging/src/k8s.io/component-base/cli/flag/flags.go
@@ -24,6 +24,8 @@ import (
 	"k8s.io/klog/v2"
 )
 
+var underscoreWarnings = make(map[string]bool)
+
 // WordSepNormalizeFunc changes all flags that contain "_" separators
 func WordSepNormalizeFunc(f *pflag.FlagSet, name string) pflag.NormalizedName {
 	if strings.Contains(name, "_") {
@@ -36,7 +38,10 @@ func WordSepNormalizeFunc(f *pflag.FlagSet, name string) pflag.NormalizedName {
 func WarnWordSepNormalizeFunc(f *pflag.FlagSet, name string) pflag.NormalizedName {
 	if strings.Contains(name, "_") {
 		nname := strings.Replace(name, "_", "-", -1)
-		klog.Warningf("%s is DEPRECATED and will be removed in a future version. Use %s instead.", name, nname)
+		if _, alreadyWarned := underscoreWarnings[name]; !alreadyWarned {
+			klog.Warningf("using an underscore in a flag name is not supported. %s has been converted to %s.", name, nname)
+			underscoreWarnings[name] = true
+		}
 
 		return pflag.NormalizedName(nname)
 	}
diff --git a/staging/src/k8s.io/component-base/cli/flag/flags_test.go b/staging/src/k8s.io/component-base/cli/flag/flags_test.go
new file mode 100644
index 00000000000..1c532518060
--- /dev/null
+++ b/staging/src/k8s.io/component-base/cli/flag/flags_test.go
@@ -0,0 +1,107 @@
+/*
+Copyright 2021 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 flag
+
+import (
+	"bytes"
+	"github.com/go-logr/logr"
+	"github.com/spf13/pflag"
+	"github.com/stretchr/testify/assert"
+	"k8s.io/klog/v2"
+	"testing"
+)
+
+type FakeLogger struct {
+	logr.Logger
+	infoBuffer  bytes.Buffer
+	errorBuffer bytes.Buffer
+}
+
+func (logger *FakeLogger) Enabled() bool { return true }
+func (logger *FakeLogger) Info(msg string, keysAndValues ...interface{}) {
+	logger.infoBuffer.WriteString(msg)
+}
+func (logger *FakeLogger) Error(err error, msg string, keysAndValues ...interface{}) {
+	logger.errorBuffer.WriteString(msg)
+}
+func (logger *FakeLogger) V(level int) logr.Logger                             { return logger }
+func (logger *FakeLogger) WithValues(keysAndValues ...interface{}) logr.Logger { return logger }
+func (logger *FakeLogger) WithName(name string) logr.Logger                    { return logger }
+
+func TestWordSepNormalizeFunc(t *testing.T) {
+	cases := []struct {
+		flagName         string
+		expectedFlagName string
+	}{
+		{
+			flagName:         "foo",
+			expectedFlagName: "foo",
+		},
+		{
+			flagName:         "foo-bar",
+			expectedFlagName: "foo-bar",
+		},
+		{
+			flagName:         "foo_bar",
+			expectedFlagName: "foo-bar",
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.flagName, func(t *testing.T) {
+			fakeLogger := &FakeLogger{}
+			klog.SetLogger(fakeLogger)
+			result := WordSepNormalizeFunc(nil, tc.flagName)
+			assert.Equal(t, pflag.NormalizedName(tc.expectedFlagName), result)
+			assert.Equal(t, "", fakeLogger.infoBuffer.String())
+			assert.Equal(t, "", fakeLogger.errorBuffer.String())
+		})
+	}
+}
+
+func TestWarnWordSepNormalizeFunc(t *testing.T) {
+	cases := []struct {
+		flagName         string
+		expectedFlagName string
+		expectedWarning  string
+	}{
+		{
+			flagName:         "foo",
+			expectedFlagName: "foo",
+			expectedWarning:  "",
+		},
+		{
+			flagName:         "foo-bar",
+			expectedFlagName: "foo-bar",
+			expectedWarning:  "",
+		},
+		{
+			flagName:         "foo_bar",
+			expectedFlagName: "foo-bar",
+			expectedWarning:  "using an underscore in a flag name is not supported. foo_bar has been converted to foo-bar.\n",
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.flagName, func(t *testing.T) {
+			fakeLogger := &FakeLogger{}
+			klog.SetLogger(fakeLogger)
+			result := WarnWordSepNormalizeFunc(nil, tc.flagName)
+			assert.Equal(t, pflag.NormalizedName(tc.expectedFlagName), result)
+			assert.Equal(t, tc.expectedWarning, fakeLogger.infoBuffer.String())
+			assert.Equal(t, "", fakeLogger.errorBuffer.String())
+		})
+	}
+}