From a504aed54d028dbc8ea2508142c94d309f5f1ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 11 Oct 2023 18:04:11 +0300 Subject: [PATCH] Add shortname ambiguity warning in shortcut expander (#117668) * Add warning handler callback function in shortcut expander Currently, errors in client-go are propagated back to the callers via function returns. However, there is no elegant way for just warning users. For example, when user wants to get a resource with it's short name format and if there are multiple resources belonging to this short name, we need to warn user about this ambugity which one is picked and which ones are discarded. Not only to overcome this particular case mentioned above, but also propose a way for the possible warnings in the future, this commit adds a warningHandler callback function in shortcutExpander. * Add warningPrinter functionality in ConfigFlags ConfigFlags has neither warning user in a standardized format functionality nor passing warning callback functions to other upper level libraries such as client-go. This commit adds an ability that user can set warningPrinters according to their IOStreams and this warningPrinters will be used to raise possible warnings happening not only in cli-runtime but also in client-go. * Pass warning callback function in ConfigFlags to shortcutExpander This commit passes warning callback function to print possible warnings happened in shortcut expander to warn user in a standardized format. * Add integration test for CRDs having ambiguous short names This commit adds integration test to assure that warning message related to this ambiguity is printed when resources are being retrieved via their short name representations in cases where multiple resources have same short names. This integration test also ensures that the logic behind which resource will be selected hasn't been changed which may cause disperancies in clusters. * Remove defaultConfigFlag global variable * Move default config flags initialization into function * Skip warning for versions of same group/resource * Run update-vendor * Warn only once when there are multiple versions registered for ambiguous resource * Apply gocritic review * Add multi-resource multi-version ambiguity unit test --- staging/src/k8s.io/cli-runtime/go.mod | 2 + staging/src/k8s.io/cli-runtime/go.sum | 7 + .../pkg/genericclioptions/config_flags.go | 17 +- .../genericclioptions/config_flags_fake.go | 2 +- .../cli-runtime/pkg/printers/terminal.go | 36 +++ .../k8s.io/client-go/restmapper/shortcut.go | 34 ++- .../client-go/restmapper/shortcut_test.go | 205 +++++++++++++++++- staging/src/k8s.io/kubectl/pkg/cmd/cmd.go | 11 +- .../k8s.io/kubectl/pkg/cmd/testing/fake.go | 2 +- .../src/k8s.io/kubectl/pkg/util/term/term.go | 40 +--- staging/src/k8s.io/sample-cli-plugin/go.mod | 2 + staging/src/k8s.io/sample-cli-plugin/go.sum | 7 + test/cmd/discovery.sh | 104 +++++++++ test/cmd/legacy-script.sh | 8 + 14 files changed, 431 insertions(+), 46 deletions(-) diff --git a/staging/src/k8s.io/cli-runtime/go.mod b/staging/src/k8s.io/cli-runtime/go.mod index d1992eee419..129c804fa2f 100644 --- a/staging/src/k8s.io/cli-runtime/go.mod +++ b/staging/src/k8s.io/cli-runtime/go.mod @@ -10,6 +10,7 @@ require ( github.com/google/go-cmp v0.5.9 github.com/google/uuid v1.3.0 github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de + github.com/moby/term v0.0.0-20221205130635-1aeaba878587 github.com/spf13/cobra v1.7.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.3 @@ -28,6 +29,7 @@ require ( ) require ( + github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect github.com/go-errors/errors v1.4.2 // indirect diff --git a/staging/src/k8s.io/cli-runtime/go.sum b/staging/src/k8s.io/cli-runtime/go.sum index c354b73f941..d24df766cbe 100644 --- a/staging/src/k8s.io/cli-runtime/go.sum +++ b/staging/src/k8s.io/cli-runtime/go.sum @@ -1,5 +1,7 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go/compute/metadata v0.2.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k= +github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOElx5B5HZ4hJQsoJ/PvUvKRhJHDQXO8P8= +github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= @@ -11,6 +13,8 @@ github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMn github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= +github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= +github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -97,6 +101,8 @@ github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de/go.mod h1:zAbeS9 github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/moby/spdystream v0.2.0/go.mod h1:f7i0iNDQJ059oMTcWxx8MA/zKFIuD/lY+0GqbN2Wy8c= +github.com/moby/term v0.0.0-20221205130635-1aeaba878587 h1:HfkjXDfhgVaN5rmueG8cL8KKeFNecRCXFhaJ2qZ5SKA= +github.com/moby/term v0.0.0-20221205130635-1aeaba878587/go.mod h1:8FzsFHVUBGZdbDsJw/ot+X+d5HLUbvklYLJ9uGfcI3Y= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -181,6 +187,7 @@ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go b/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go index 03f4b5332bb..9c168350960 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go +++ b/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go @@ -27,6 +27,8 @@ import ( "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/cli-runtime/pkg/genericiooptions" + "k8s.io/cli-runtime/pkg/printers" "k8s.io/client-go/discovery" diskcached "k8s.io/client-go/discovery/cached/disk" "k8s.io/client-go/rest" @@ -122,6 +124,9 @@ type ConfigFlags struct { // Allows increasing qps used for discovery, this is useful // in clusters with many registered resources discoveryQPS float32 + // Allows all possible warnings are printed in a standardized + // format. + warningPrinter *printers.WarningPrinter } // ToRESTConfig implements RESTClientGetter. @@ -332,7 +337,11 @@ func (f *ConfigFlags) toRESTMapper() (meta.RESTMapper, error) { } mapper := restmapper.NewDeferredDiscoveryRESTMapper(discoveryClient) - expander := restmapper.NewShortcutExpander(mapper, discoveryClient) + expander := restmapper.NewShortcutExpander(mapper, discoveryClient, func(a string) { + if f.warningPrinter != nil { + f.warningPrinter.Print(a) + } + }) return expander, nil } @@ -428,6 +437,12 @@ func (f *ConfigFlags) WithWrapConfigFn(wrapConfigFn func(*rest.Config) *rest.Con return f } +// WithWarningPrinter initializes WarningPrinter with the given IOStreams +func (f *ConfigFlags) WithWarningPrinter(ioStreams genericiooptions.IOStreams) *ConfigFlags { + f.warningPrinter = printers.NewWarningPrinter(ioStreams.ErrOut, printers.WarningPrinterOptions{Color: printers.AllowsColorOutput(ioStreams.ErrOut)}) + return f +} + // NewConfigFlags returns ConfigFlags with default values set func NewConfigFlags(usePersistentConfig bool) *ConfigFlags { impersonateGroup := []string{} diff --git a/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags_fake.go b/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags_fake.go index 7a96481552d..9bb5e28c94e 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags_fake.go +++ b/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags_fake.go @@ -66,7 +66,7 @@ func (f *TestConfigFlags) ToRESTMapper() (meta.RESTMapper, error) { } if f.discoveryClient != nil { mapper := restmapper.NewDeferredDiscoveryRESTMapper(f.discoveryClient) - expander := restmapper.NewShortcutExpander(mapper, f.discoveryClient) + expander := restmapper.NewShortcutExpander(mapper, f.discoveryClient, nil) return expander, nil } return nil, fmt.Errorf("no restmapper") diff --git a/staging/src/k8s.io/cli-runtime/pkg/printers/terminal.go b/staging/src/k8s.io/cli-runtime/pkg/printers/terminal.go index 5a59491e492..9dc904e59c4 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/printers/terminal.go +++ b/staging/src/k8s.io/cli-runtime/pkg/printers/terminal.go @@ -18,7 +18,11 @@ package printers import ( "io" + "os" + "runtime" "strings" + + "github.com/moby/term" ) // terminalEscaper replaces ANSI escape sequences and other terminal special @@ -37,3 +41,35 @@ func WriteEscaped(writer io.Writer, output string) error { func EscapeTerminal(in string) string { return terminalEscaper.Replace(in) } + +// IsTerminal returns whether the passed object is a terminal or not +func IsTerminal(i interface{}) bool { + _, terminal := term.GetFdInfo(i) + return terminal +} + +// AllowsColorOutput returns true if the specified writer is a terminal and +// the process environment indicates color output is supported and desired. +func AllowsColorOutput(w io.Writer) bool { + if !IsTerminal(w) { + return false + } + + // https://en.wikipedia.org/wiki/Computer_terminal#Dumb_terminals + if os.Getenv("TERM") == "dumb" { + return false + } + + // https://no-color.org/ + if _, nocolor := os.LookupEnv("NO_COLOR"); nocolor { + return false + } + + // On Windows WT_SESSION is set by the modern terminal component. + // Older terminals have poor support for UTF-8, VT escape codes, etc. + if runtime.GOOS == "windows" && os.Getenv("WT_SESSION") == "" { + return false + } + + return true +} diff --git a/staging/src/k8s.io/client-go/restmapper/shortcut.go b/staging/src/k8s.io/client-go/restmapper/shortcut.go index 7ab3cd46fe3..ca517a01d4d 100644 --- a/staging/src/k8s.io/client-go/restmapper/shortcut.go +++ b/staging/src/k8s.io/client-go/restmapper/shortcut.go @@ -17,6 +17,7 @@ limitations under the License. package restmapper import ( + "fmt" "strings" "k8s.io/klog/v2" @@ -32,13 +33,15 @@ type shortcutExpander struct { RESTMapper meta.RESTMapper discoveryClient discovery.DiscoveryInterface + + warningHandler func(string) } var _ meta.ResettableRESTMapper = shortcutExpander{} // NewShortcutExpander wraps a restmapper in a layer that expands shortcuts found via discovery -func NewShortcutExpander(delegate meta.RESTMapper, client discovery.DiscoveryInterface) meta.RESTMapper { - return shortcutExpander{RESTMapper: delegate, discoveryClient: client} +func NewShortcutExpander(delegate meta.RESTMapper, client discovery.DiscoveryInterface, warningHandler func(string)) meta.RESTMapper { + return shortcutExpander{RESTMapper: delegate, discoveryClient: client, warningHandler: warningHandler} } // KindFor fulfills meta.RESTMapper @@ -145,16 +148,37 @@ func (e shortcutExpander) expandResourceShortcut(resource schema.GroupVersionRes } } + found := false + var rsc schema.GroupVersionResource + warnedAmbiguousShortcut := make(map[schema.GroupResource]bool) for _, item := range shortcutResources { if len(resource.Group) != 0 && resource.Group != item.ShortForm.Group { continue } if resource.Resource == item.ShortForm.Resource { - resource.Resource = item.LongForm.Resource - resource.Group = item.LongForm.Group - return resource + if found { + if item.LongForm.Group == rsc.Group && item.LongForm.Resource == rsc.Resource { + // It is common and acceptable that group/resource has multiple + // versions registered in cluster. This does not introduce ambiguity + // in terms of shortname usage. + continue + } + if !warnedAmbiguousShortcut[item.LongForm] { + if e.warningHandler != nil { + e.warningHandler(fmt.Sprintf("short name %q could also match lower priority resource %s", resource.Resource, item.LongForm.String())) + } + warnedAmbiguousShortcut[item.LongForm] = true + } + continue + } + rsc.Resource = item.LongForm.Resource + rsc.Group = item.LongForm.Group + found = true } } + if found { + return rsc + } // we didn't find exact match so match on group prefixing. This allows autoscal to match autoscaling if len(resource.Group) == 0 { diff --git a/staging/src/k8s.io/client-go/restmapper/shortcut_test.go b/staging/src/k8s.io/client-go/restmapper/shortcut_test.go index fa2355d5f1d..e1f87ae26c0 100644 --- a/staging/src/k8s.io/client-go/restmapper/shortcut_test.go +++ b/staging/src/k8s.io/client-go/restmapper/shortcut_test.go @@ -133,7 +133,7 @@ func TestReplaceAliases(t *testing.T) { ds.serverResourcesHandler = func() ([]*metav1.APIResourceList, error) { return test.srvRes, nil } - mapper := NewShortcutExpander(&fakeRESTMapper{}, ds).(shortcutExpander) + mapper := NewShortcutExpander(&fakeRESTMapper{}, ds, nil).(shortcutExpander) actual := mapper.expandResourceShortcut(schema.GroupVersionResource{Resource: test.arg}) if actual != test.expected { @@ -187,7 +187,9 @@ func TestKindFor(t *testing.T) { } delegate := &fakeRESTMapper{} - mapper := NewShortcutExpander(delegate, ds) + mapper := NewShortcutExpander(delegate, ds, func(a string) { + t.Fatalf("unexpected warning message %s", a) + }) mapper.KindFor(test.in) if delegate.kindForInput != test.expected { @@ -242,7 +244,9 @@ func TestKindForWithNewCRDs(t *testing.T) { // will answer the initial request, only failure to match will trigger // the cache invalidation and live discovery call delegate := NewDeferredDiscoveryRESTMapper(fakeCachedDiscovery) - mapper := NewShortcutExpander(delegate, fakeCachedDiscovery) + mapper := NewShortcutExpander(delegate, fakeCachedDiscovery, func(a string) { + t.Fatalf("unexpected warning message %s", a) + }) gvk, err := mapper.KindFor(test.in) if err != nil { @@ -255,6 +259,201 @@ func TestKindForWithNewCRDs(t *testing.T) { } } +func TestWarnAmbigious(t *testing.T) { + tests := []struct { + name string + arg string + expected schema.GroupVersionResource + expectedWarningLogs []string + srvRes []*metav1.APIResourceList + }{ + { + name: "warn ambiguity", + arg: "hpa", + expected: schema.GroupVersionResource{Resource: "superhorizontalpodautoscalers", Group: "autoscaling"}, + expectedWarningLogs: []string{`short name "hpa" could also match lower priority resource horizontalpodautoscalers.autoscaling`}, + srvRes: []*metav1.APIResourceList{ + { + GroupVersion: "autoscaling/v1", + APIResources: []metav1.APIResource{ + { + Name: "superhorizontalpodautoscalers", + ShortNames: []string{"hpa"}, + }, + }, + }, + { + GroupVersion: "autoscaling/v1", + APIResources: []metav1.APIResource{ + { + Name: "horizontalpodautoscalers", + ShortNames: []string{"hpa"}, + }, + }, + }, + }, + }, + { + name: "warn-builtin-shortname-ambugity", + arg: "po", + expected: schema.GroupVersionResource{Resource: "pods", Group: ""}, + expectedWarningLogs: []string{`short name "po" could also match lower priority resource poddlers.acme.com`}, + srvRes: []*metav1.APIResourceList{ + { + GroupVersion: "v1", + APIResources: []metav1.APIResource{{Name: "pods", SingularName: "pod", ShortNames: []string{"po"}}}, + }, + { + GroupVersion: "acme.com/v1", + APIResources: []metav1.APIResource{{Name: "poddlers", ShortNames: []string{"po"}}}, + }, + }, + }, + { + name: "warn-builtin-shortname-ambugity-multi-version", + arg: "po", + expected: schema.GroupVersionResource{Resource: "pods", Group: ""}, + expectedWarningLogs: []string{`short name "po" could also match lower priority resource poddlers.acme.com`}, + srvRes: []*metav1.APIResourceList{ + { + GroupVersion: "v1", + APIResources: []metav1.APIResource{{Name: "pods", SingularName: "pod", ShortNames: []string{"po"}}}, + }, + { + GroupVersion: "acme.com/v1", + APIResources: []metav1.APIResource{{Name: "poddlers", ShortNames: []string{"po"}}}, + }, + { + GroupVersion: "acme.com/v1beta1", + APIResources: []metav1.APIResource{{Name: "poddlers", ShortNames: []string{"po"}}}, + }, + }, + }, + { + name: "resource-match-singular-preferred", + arg: "pod", + expected: schema.GroupVersionResource{Resource: "pod", Group: ""}, + srvRes: []*metav1.APIResourceList{ + { + GroupVersion: "v1", + APIResources: []metav1.APIResource{{Name: "pods", SingularName: "pod"}}, + }, + { + GroupVersion: "acme.com/v1", + APIResources: []metav1.APIResource{{Name: "poddlers", ShortNames: []string{"pods", "pod"}}}, + }, + }, + }, + { + name: "resource-multiple-versions-shortform", + arg: "hpa", + expected: schema.GroupVersionResource{Resource: "horizontalpodautoscalers", Group: "autoscaling"}, + expectedWarningLogs: []string{}, + srvRes: []*metav1.APIResourceList{ + { + GroupVersion: "autoscaling/v1alphav1", + APIResources: []metav1.APIResource{ + { + Name: "horizontalpodautoscalers", + ShortNames: []string{"hpa"}, + }, + }, + }, + { + GroupVersion: "autoscaling/v1", + APIResources: []metav1.APIResource{ + { + Name: "horizontalpodautoscalers", + ShortNames: []string{"hpa"}, + }, + }, + }, + }, + }, + { + name: "multi-resource-multiple-versions-shortform", + arg: "hpa", + expected: schema.GroupVersionResource{Resource: "horizontalpodautoscalers", Group: "autoscaling"}, + expectedWarningLogs: []string{ + `short name "hpa" could also match lower priority resource foo.foo`, + `short name "hpa" could also match lower priority resource bar.bar`, + }, + srvRes: []*metav1.APIResourceList{ + { + GroupVersion: "autoscaling/v1alphav1", + APIResources: []metav1.APIResource{ + { + Name: "horizontalpodautoscalers", + ShortNames: []string{"hpa"}, + }, + }, + }, + { + GroupVersion: "autoscaling/v1", + APIResources: []metav1.APIResource{ + { + Name: "horizontalpodautoscalers", + ShortNames: []string{"hpa"}, + }, + }, + }, + { + GroupVersion: "foo/v1", + APIResources: []metav1.APIResource{ + { + Name: "foo", + ShortNames: []string{"hpa"}, + }, + }, + }, + { + GroupVersion: "foo/v1beta1", + APIResources: []metav1.APIResource{ + { + Name: "foo", + ShortNames: []string{"hpa"}, + }, + }, + }, + { + GroupVersion: "bar/v1", + APIResources: []metav1.APIResource{ + { + Name: "bar", + ShortNames: []string{"hpa"}, + }, + }, + }, + }, + }, + } + + for _, test := range tests { + ds := &fakeDiscoveryClient{} + ds.serverResourcesHandler = func() ([]*metav1.APIResourceList, error) { + return test.srvRes, nil + } + + var actualWarnings []string + mapper := NewShortcutExpander(&fakeRESTMapper{}, ds, func(a string) { + actualWarnings = append(actualWarnings, a) + }).(shortcutExpander) + + actual := mapper.expandResourceShortcut(schema.GroupVersionResource{Resource: test.arg}) + if actual != test.expected { + t.Errorf("%s: unexpected argument: expected %s, got %s", test.name, test.expected, actual) + } + + if len(actualWarnings) == 0 && len(test.expectedWarningLogs) == 0 { + continue + } + + if !cmp.Equal(test.expectedWarningLogs, actualWarnings) { + t.Fatalf("expected warning message %s but got %s", test.expectedWarningLogs, actualWarnings) + } + } +} + type fakeRESTMapper struct { kindForInput schema.GroupVersionResource } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go index 6b3a84d50fb..cc554dea1a6 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go @@ -92,15 +92,18 @@ type KubectlOptions struct { genericiooptions.IOStreams } -var defaultConfigFlags = genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag().WithDiscoveryBurst(300).WithDiscoveryQPS(50.0) +func defaultConfigFlags() *genericclioptions.ConfigFlags { + return genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag().WithDiscoveryBurst(300).WithDiscoveryQPS(50.0) +} // NewDefaultKubectlCommand creates the `kubectl` command with default arguments func NewDefaultKubectlCommand() *cobra.Command { + ioStreams := genericiooptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr} return NewDefaultKubectlCommandWithArgs(KubectlOptions{ PluginHandler: NewDefaultPluginHandler(plugin.ValidPluginFilenamePrefixes), Arguments: os.Args, - ConfigFlags: defaultConfigFlags, - IOStreams: genericiooptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}, + ConfigFlags: defaultConfigFlags().WithWarningPrinter(ioStreams), + IOStreams: ioStreams, }) } @@ -364,7 +367,7 @@ func NewKubectlCommand(o KubectlOptions) *cobra.Command { kubeConfigFlags := o.ConfigFlags if kubeConfigFlags == nil { - kubeConfigFlags = defaultConfigFlags + kubeConfigFlags = defaultConfigFlags().WithWarningPrinter(o.IOStreams) } kubeConfigFlags.AddFlags(flags) matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/testing/fake.go b/staging/src/k8s.io/kubectl/pkg/cmd/testing/fake.go index b10d1739371..d5e78a72e14 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/testing/fake.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/testing/fake.go @@ -641,7 +641,7 @@ func testRESTMapper() meta.RESTMapper { } fakeDs := NewFakeCachedDiscoveryClient() - expander := restmapper.NewShortcutExpander(mapper, fakeDs) + expander := restmapper.NewShortcutExpander(mapper, fakeDs, nil) return expander } diff --git a/staging/src/k8s.io/kubectl/pkg/util/term/term.go b/staging/src/k8s.io/kubectl/pkg/util/term/term.go index 6bcda59d73d..93a992fe31e 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/term/term.go +++ b/staging/src/k8s.io/kubectl/pkg/util/term/term.go @@ -19,7 +19,8 @@ package term import ( "io" "os" - "runtime" + + "k8s.io/cli-runtime/pkg/printers" "github.com/moby/term" @@ -56,46 +57,23 @@ type TTY struct { // IsTerminalIn returns true if t.In is a terminal. Does not check /dev/tty // even if TryDev is set. func (t TTY) IsTerminalIn() bool { - return IsTerminal(t.In) + return printers.IsTerminal(t.In) } // IsTerminalOut returns true if t.Out is a terminal. Does not check /dev/tty // even if TryDev is set. func (t TTY) IsTerminalOut() bool { - return IsTerminal(t.Out) + return printers.IsTerminal(t.Out) } -// IsTerminal returns whether the passed object is a terminal or not -func IsTerminal(i interface{}) bool { - _, terminal := term.GetFdInfo(i) - return terminal -} +// IsTerminal returns whether the passed object is a terminal or not. +// Deprecated: use printers.IsTerminal instead. +var IsTerminal = printers.IsTerminal // AllowsColorOutput returns true if the specified writer is a terminal and // the process environment indicates color output is supported and desired. -func AllowsColorOutput(w io.Writer) bool { - if !IsTerminal(w) { - return false - } - - // https://en.wikipedia.org/wiki/Computer_terminal#Dumb_terminals - if os.Getenv("TERM") == "dumb" { - return false - } - - // https://no-color.org/ - if _, nocolor := os.LookupEnv("NO_COLOR"); nocolor { - return false - } - - // On Windows WT_SESSION is set by the modern terminal component. - // Older terminals have poor support for UTF-8, VT escape codes, etc. - if runtime.GOOS == "windows" && os.Getenv("WT_SESSION") == "" { - return false - } - - return true -} +// Deprecated: use printers.AllowsColorOutput instead. +var AllowsColorOutput = printers.AllowsColorOutput // Safe invokes the provided function and will attempt to ensure that when the // function returns (or a termination signal is sent) that the terminal state diff --git a/staging/src/k8s.io/sample-cli-plugin/go.mod b/staging/src/k8s.io/sample-cli-plugin/go.mod index 92faccfd0b6..b6758679a2b 100644 --- a/staging/src/k8s.io/sample-cli-plugin/go.mod +++ b/staging/src/k8s.io/sample-cli-plugin/go.mod @@ -12,6 +12,7 @@ require ( ) require ( + github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect github.com/evanphx/json-patch v4.12.0+incompatible // indirect @@ -35,6 +36,7 @@ require ( github.com/json-iterator/go v1.1.12 // indirect github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de // indirect github.com/mailru/easyjson v0.7.7 // indirect + github.com/moby/term v0.0.0-20221205130635-1aeaba878587 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect diff --git a/staging/src/k8s.io/sample-cli-plugin/go.sum b/staging/src/k8s.io/sample-cli-plugin/go.sum index c354b73f941..d24df766cbe 100644 --- a/staging/src/k8s.io/sample-cli-plugin/go.sum +++ b/staging/src/k8s.io/sample-cli-plugin/go.sum @@ -1,5 +1,7 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go/compute/metadata v0.2.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k= +github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOElx5B5HZ4hJQsoJ/PvUvKRhJHDQXO8P8= +github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= @@ -11,6 +13,8 @@ github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMn github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= +github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= +github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -97,6 +101,8 @@ github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de/go.mod h1:zAbeS9 github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/moby/spdystream v0.2.0/go.mod h1:f7i0iNDQJ059oMTcWxx8MA/zKFIuD/lY+0GqbN2Wy8c= +github.com/moby/term v0.0.0-20221205130635-1aeaba878587 h1:HfkjXDfhgVaN5rmueG8cL8KKeFNecRCXFhaJ2qZ5SKA= +github.com/moby/term v0.0.0-20221205130635-1aeaba878587/go.mod h1:8FzsFHVUBGZdbDsJw/ot+X+d5HLUbvklYLJ9uGfcI3Y= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -181,6 +187,7 @@ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/test/cmd/discovery.sh b/test/cmd/discovery.sh index 4979f4356d1..b8f473e139f 100755 --- a/test/cmd/discovery.sh +++ b/test/cmd/discovery.sh @@ -303,3 +303,107 @@ run_swagger_tests() { set +o nounset set +o errexit } + +run_ambiguous_shortname_tests() { + set -o nounset + set -o errexit + + create_and_use_new_namespace + kube::log::status "Testing ambiguous short name" + + kubectl create -f - << __EOF__ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: foos.bar.com +spec: + group: bar.com + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + test: + type: string + names: + plural: foos + singular: foo + shortNames: + - exmp + kind: Foo + categories: + - all +__EOF__ + + # Test that we can list this new custom resource + kube::test::wait_object_assert customresourcedefinitions "{{range.items}}{{if eq ${id_field:?} \"foos.bar.com\"}}{{$id_field}}:{{end}}{{end}}" 'foos.bar.com:' + + kubectl create -f - << __EOF__ +apiVersion: bar.com/v1 +kind: Foo +metadata: + name: test-crd-foo +spec: + test: test +__EOF__ + + # Test that we can list this new custom resource + kube::test::wait_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" 'test-crd-foo:' + + output_message=$(kubectl get exmp) + kube::test::if_has_string "${output_message}" "test-crd-foo" + + kubectl create -f - << __EOF__ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.test.com +spec: + group: test.com + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + test: + type: string + names: + plural: examples + singular: example + shortNames: + - exmp + kind: Example +__EOF__ + + # Test that we can list this new custom resource + kube::test::wait_object_assert customresourcedefinitions "{{range.items}}{{if eq ${id_field:?} \"examples.test.com\"}}{{$id_field}}:{{end}}{{end}}" 'examples.test.com:' + + output_message=$(kubectl get examples 2>&1 "${kube_flags[@]}") + kube::test::if_has_string "${output_message}" 'No resources found' + + output_message=$(kubectl get exmp 2>&1) + kube::test::if_has_string "${output_message}" "test-crd-foo" + kube::test::if_has_string "${output_message}" "short name \"exmp\" could also match lower priority resource examples.test.com" + + # Cleanup + kubectl delete foos/test-crd-foo + kubectl delete customresourcedefinition foos.bar.com + kubectl delete customresourcedefinition examples.test.com + + set +o nounset + set +o errexit +} diff --git a/test/cmd/legacy-script.sh b/test/cmd/legacy-script.sh index d3e0220be25..7b809b197e9 100755 --- a/test/cmd/legacy-script.sh +++ b/test/cmd/legacy-script.sh @@ -518,6 +518,14 @@ runTests() { record_command run_assert_singular_name_tests fi + ######################### + # Ambiguous short name # + ######################### + + if kube::test::if_supports_resource "${customresourcedefinitions}" ; then + record_command run_ambiguous_shortname_tests + fi + ######################### # Assert categories # #########################