diff --git a/hack/.golint_failures b/hack/.golint_failures index 37019ea5b38..dfb01b7005c 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -151,6 +151,7 @@ pkg/kubectl/cmd/util pkg/kubectl/cmd/util/editor pkg/kubectl/cmd/util/jsonmerge pkg/kubectl/cmd/util/sanity +pkg/kubectl/genericclioptions pkg/kubectl/genericclioptions/resource pkg/kubectl/metricsutil pkg/kubectl/util diff --git a/pkg/kubectl/cmd/cmd.go b/pkg/kubectl/cmd/cmd.go index a1f7498ca5b..623b0e6e400 100644 --- a/pkg/kubectl/cmd/cmd.go +++ b/pkg/kubectl/cmd/cmd.go @@ -274,8 +274,15 @@ func NewKubectlCommand(in io.Reader, out, err io.Writer) *cobra.Command { BashCompletionFunction: bashCompletionFunc, } - kubeConfigFlags := cmdutil.NewConfigFlags() - kubeConfigFlags.AddFlags(cmds.PersistentFlags()) + flags := cmds.PersistentFlags() + flags.SetNormalizeFunc(utilflag.WarnWordSepNormalizeFunc) // Warn for "_" flags + + // Normalize all flags that are coming from other packages or pre-configurations + // a.k.a. change all "_" to "-". e.g. glog package + flags.SetNormalizeFunc(utilflag.WordSepNormalizeFunc) + + kubeConfigFlags := genericclioptions.NewConfigFlags() + kubeConfigFlags.AddFlags(flags) matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags) matchVersionKubeConfigFlags.AddFlags(cmds.PersistentFlags()) diff --git a/pkg/kubectl/cmd/config/config_test.go b/pkg/kubectl/cmd/config/config_test.go index ca8d26c222c..9678014878b 100644 --- a/pkg/kubectl/cmd/config/config_test.go +++ b/pkg/kubectl/cmd/config/config_test.go @@ -864,7 +864,7 @@ func testConfigCommand(args []string, startingConfig clientcmdapi.Config, t *tes argsToUse = append(argsToUse, args...) streams, _, buf, _ := genericclioptions.NewTestIOStreams() - cmd := NewCmdConfig(cmdutil.NewFactory(cmdutil.NewTestConfigFlags()), clientcmd.NewDefaultPathOptions(), streams) + cmd := NewCmdConfig(cmdutil.NewFactory(genericclioptions.NewTestConfigFlags()), clientcmd.NewDefaultPathOptions(), streams) cmd.SetArgs(argsToUse) cmd.Execute() diff --git a/pkg/kubectl/cmd/config/view_test.go b/pkg/kubectl/cmd/config/view_test.go index b3f05d48bf4..98f02651e86 100644 --- a/pkg/kubectl/cmd/config/view_test.go +++ b/pkg/kubectl/cmd/config/view_test.go @@ -142,7 +142,7 @@ func (test viewClusterTest) run(t *testing.T) { pathOptions.GlobalFile = fakeKubeFile.Name() pathOptions.EnvVar = "" streams, _, buf, _ := genericclioptions.NewTestIOStreams() - cmd := NewCmdConfigView(cmdutil.NewFactory(cmdutil.NewTestConfigFlags()), streams, pathOptions) + cmd := NewCmdConfigView(cmdutil.NewFactory(genericclioptions.NewTestConfigFlags()), streams, pathOptions) cmd.Flags().Parse(test.flags) if err := cmd.Execute(); err != nil { t.Fatalf("unexpected error executing command: %v,kubectl config view flags: %v", err, test.flags) diff --git a/pkg/kubectl/cmd/set/set_test.go b/pkg/kubectl/cmd/set/set_test.go index fa985fa65ea..70fd92f7e54 100644 --- a/pkg/kubectl/cmd/set/set_test.go +++ b/pkg/kubectl/cmd/set/set_test.go @@ -26,7 +26,7 @@ import ( ) func TestLocalAndDryRunFlags(t *testing.T) { - f := clientcmdutil.NewFactory(clientcmdutil.NewTestConfigFlags()) + f := clientcmdutil.NewFactory(genericclioptions.NewTestConfigFlags()) setCmd := NewCmdSet(f, genericclioptions.NewTestIOStreamsDiscard()) ensureLocalAndDryRunFlagsOnChildren(t, setCmd, "") } diff --git a/pkg/kubectl/cmd/testing/BUILD b/pkg/kubectl/cmd/testing/BUILD index 0e73194b3a5..2f11fe58e43 100644 --- a/pkg/kubectl/cmd/testing/BUILD +++ b/pkg/kubectl/cmd/testing/BUILD @@ -16,6 +16,7 @@ go_library( "//pkg/kubectl/cmd/util:go_default_library", "//pkg/kubectl/cmd/util/openapi:go_default_library", "//pkg/kubectl/cmd/util/openapi/testing:go_default_library", + "//pkg/kubectl/genericclioptions:go_default_library", "//pkg/kubectl/genericclioptions/resource:go_default_library", "//pkg/kubectl/validation:go_default_library", "//pkg/printers:go_default_library", diff --git a/pkg/kubectl/cmd/testing/fake.go b/pkg/kubectl/cmd/testing/fake.go index 813194fa814..e5851bda4b8 100644 --- a/pkg/kubectl/cmd/testing/fake.go +++ b/pkg/kubectl/cmd/testing/fake.go @@ -50,6 +50,7 @@ import ( cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi" openapitesting "k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi/testing" + "k8s.io/kubernetes/pkg/kubectl/genericclioptions" "k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource" "k8s.io/kubernetes/pkg/kubectl/validation" "k8s.io/kubernetes/pkg/printers" @@ -267,7 +268,7 @@ func NewTestFactory() *TestFactory { fallbackReader := bytes.NewBuffer([]byte{}) clientConfig := clientcmd.NewInteractiveDeferredLoadingClientConfig(loadingRules, overrides, fallbackReader) - configFlags := cmdutil.NewTestConfigFlags(). + configFlags := genericclioptions.NewTestConfigFlags(). WithClientConfig(clientConfig). WithRESTMapper(testRESTMapper()) diff --git a/pkg/kubectl/cmd/util/BUILD b/pkg/kubectl/cmd/util/BUILD index 55d011890d4..2de6e2d002d 100644 --- a/pkg/kubectl/cmd/util/BUILD +++ b/pkg/kubectl/cmd/util/BUILD @@ -3,7 +3,6 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", srcs = [ - "config_flags.go", "conversion.go", "factory.go", "factory_builder.go", @@ -57,15 +56,12 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/yaml:go_default_library", "//vendor/k8s.io/apimachinery/pkg/watch:go_default_library", - "//vendor/k8s.io/apiserver/pkg/util/flag:go_default_library", "//vendor/k8s.io/client-go/discovery:go_default_library", "//vendor/k8s.io/client-go/dynamic:go_default_library", "//vendor/k8s.io/client-go/kubernetes:go_default_library", "//vendor/k8s.io/client-go/rest:go_default_library", - "//vendor/k8s.io/client-go/restmapper:go_default_library", "//vendor/k8s.io/client-go/scale:go_default_library", "//vendor/k8s.io/client-go/tools/clientcmd:go_default_library", - "//vendor/k8s.io/client-go/util/homedir:go_default_library", "//vendor/k8s.io/utils/exec:go_default_library", ], ) @@ -89,6 +85,7 @@ go_test( "//pkg/client/clientset_generated/internalclientset/fake:go_default_library", "//pkg/controller:go_default_library", "//pkg/kubectl:go_default_library", + "//pkg/kubectl/genericclioptions:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", diff --git a/pkg/kubectl/cmd/util/factory_client_access.go b/pkg/kubectl/cmd/util/factory_client_access.go index ad3c4a8f7c0..f54d58d4d23 100644 --- a/pkg/kubectl/cmd/util/factory_client_access.go +++ b/pkg/kubectl/cmd/util/factory_client_access.go @@ -24,7 +24,6 @@ import ( "io" "os" "path/filepath" - "regexp" "strings" "k8s.io/api/core/v1" @@ -578,19 +577,6 @@ func (f *ring0Factory) CanBeAutoscaled(kind schema.GroupKind) error { return nil } -// overlyCautiousIllegalFileCharacters matches characters that *might* not be supported. Windows is really restrictive, so this is really restrictive -var overlyCautiousIllegalFileCharacters = regexp.MustCompile(`[^(\w/\.)]`) - -// computeDiscoverCacheDir takes the parentDir and the host and comes up with a "usually non-colliding" name. -func computeDiscoverCacheDir(parentDir, host string) string { - // strip the optional scheme from host if its there: - schemelessHost := strings.Replace(strings.Replace(host, "https://", "", 1), "http://", "", 1) - // now do a simple collapse of non-AZ09 characters. Collisions are possible but unlikely. Even if we do collide the problem is short lived - safeHost := overlyCautiousIllegalFileCharacters.ReplaceAllString(schemelessHost, "_") - - return filepath.Join(parentDir, safeHost) -} - // this method exists to help us find the points still relying on internal types. func InternalVersionDecoder() runtime.Decoder { return legacyscheme.Codecs.UniversalDecoder() diff --git a/pkg/kubectl/cmd/util/factory_test.go b/pkg/kubectl/cmd/util/factory_test.go index 2cc4eddb49a..7303540473a 100644 --- a/pkg/kubectl/cmd/util/factory_test.go +++ b/pkg/kubectl/cmd/util/factory_test.go @@ -37,10 +37,11 @@ import ( "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/kubectl" + "k8s.io/kubernetes/pkg/kubectl/genericclioptions" ) func TestPortsForObject(t *testing.T) { - f := NewFactory(NewTestConfigFlags()) + f := NewFactory(genericclioptions.NewTestConfigFlags()) pod := &api.Pod{ ObjectMeta: metav1.ObjectMeta{Name: "baz", Namespace: "test", ResourceVersion: "12"}, @@ -71,7 +72,7 @@ func TestPortsForObject(t *testing.T) { } func TestProtocolsForObject(t *testing.T) { - f := NewFactory(NewTestConfigFlags()) + f := NewFactory(genericclioptions.NewTestConfigFlags()) pod := &api.Pod{ ObjectMeta: metav1.ObjectMeta{Name: "baz", Namespace: "test", ResourceVersion: "12"}, @@ -109,7 +110,7 @@ func TestProtocolsForObject(t *testing.T) { } func TestLabelsForObject(t *testing.T) { - f := NewFactory(NewTestConfigFlags()) + f := NewFactory(genericclioptions.NewTestConfigFlags()) tests := []struct { name string @@ -160,7 +161,7 @@ func TestLabelsForObject(t *testing.T) { } func TestCanBeExposed(t *testing.T) { - factory := NewFactory(NewTestConfigFlags()) + factory := NewFactory(genericclioptions.NewTestConfigFlags()) tests := []struct { kind schema.GroupKind expectErr bool diff --git a/pkg/kubectl/genericclioptions/BUILD b/pkg/kubectl/genericclioptions/BUILD index 5d1b8e69398..652f846874d 100644 --- a/pkg/kubectl/genericclioptions/BUILD +++ b/pkg/kubectl/genericclioptions/BUILD @@ -3,6 +3,8 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", srcs = [ + "config_flags.go", + "config_flags_fake.go", "doc.go", "io_options.go", "record_flags.go", @@ -12,9 +14,15 @@ go_library( deps = [ "//vendor/github.com/evanphx/json-patch:go_default_library", "//vendor/github.com/spf13/cobra:go_default_library", + "//vendor/github.com/spf13/pflag:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library", + "//vendor/k8s.io/client-go/discovery:go_default_library", + "//vendor/k8s.io/client-go/rest:go_default_library", + "//vendor/k8s.io/client-go/restmapper:go_default_library", + "//vendor/k8s.io/client-go/tools/clientcmd:go_default_library", + "//vendor/k8s.io/client-go/util/homedir:go_default_library", ], ) diff --git a/pkg/kubectl/cmd/util/config_flags.go b/pkg/kubectl/genericclioptions/config_flags.go similarity index 82% rename from pkg/kubectl/cmd/util/config_flags.go rename to pkg/kubectl/genericclioptions/config_flags.go index 8f3a4ba2f3b..0e0e7a276b0 100644 --- a/pkg/kubectl/cmd/util/config_flags.go +++ b/pkg/kubectl/genericclioptions/config_flags.go @@ -14,18 +14,18 @@ See the License for the specific language governing permissions and limitations under the License. */ -package util +package genericclioptions import ( - "fmt" "os" "path/filepath" + "regexp" + "strings" "time" "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/api/meta" - utilflag "k8s.io/apiserver/pkg/util/flag" "k8s.io/client-go/discovery" "k8s.io/client-go/rest" "k8s.io/client-go/restmapper" @@ -54,8 +54,6 @@ const ( var defaultCacheDir = filepath.Join(homedir.HomeDir(), ".kube", "http-cache") -// TODO(juanvallejo): move to pkg/kubectl/genericclioptions once -// the dependency on cmdutil is broken here. // ConfigFlags composes the set of values necessary // for obtaining a REST client config type ConfigFlags struct { @@ -88,6 +86,9 @@ func (f *ConfigFlags) ToRESTConfig() (*rest.Config, error) { return f.ToRawKubeConfigLoader().ClientConfig() } +// ToRawKubeConfigLoader binds config flag values to config overrides +// Returns an interactive clientConfig if the password flag is enabled, +// or a non-interactive clientConfig otherwise. func (f *ConfigFlags) ToRawKubeConfigLoader() clientcmd.ClientConfig { loadingRules := clientcmd.NewDefaultClientConfigLoadingRules() // use the standard defaults for this client command @@ -189,7 +190,7 @@ func (f *ConfigFlags) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, e return discovery.NewCachedDiscoveryClientForConfig(config, discoveryCacheDir, httpCacheDir, time.Duration(10*time.Minute)) } -// RESTMapper returns a mapper. +// ToRESTMapper returns a mapper. func (f *ConfigFlags) ToRESTMapper() (meta.RESTMapper, error) { discoveryClient, err := f.ToDiscoveryClient() if err != nil { @@ -201,13 +202,8 @@ func (f *ConfigFlags) ToRESTMapper() (meta.RESTMapper, error) { return expander, nil } +// AddFlags binds client configuration flags to a given flagset func (f *ConfigFlags) AddFlags(flags *pflag.FlagSet) { - flags.SetNormalizeFunc(utilflag.WarnWordSepNormalizeFunc) // Warn for "_" flags - - // Normalize all flags that are coming from other packages or pre-configurations - // a.k.a. change all "_" to "-". e.g. glog package - flags.SetNormalizeFunc(utilflag.WordSepNormalizeFunc) - if f.KubeConfig != nil { flags.StringVar(f.KubeConfig, "kubeconfig", *f.KubeConfig, "Path to the kubeconfig file to use for CLI requests.") } @@ -265,12 +261,14 @@ func (f *ConfigFlags) AddFlags(flags *pflag.FlagSet) { } +// WithDeprecatedPasswordFlag enables the username and password config flags func (f *ConfigFlags) WithDeprecatedPasswordFlag() *ConfigFlags { f.Username = stringptr("") f.Password = stringptr("") return f } +// NewConfigFlags returns ConfigFlags with default values set func NewConfigFlags() *ConfigFlags { impersonateGroup := []string{} insecure := false @@ -299,56 +297,14 @@ func stringptr(val string) *string { return &val } -// TODO(juanvallejo): move to separate file when config_flags are moved -// to pkg/kubectl/genericclioptions -type TestConfigFlags struct { - clientConfig clientcmd.ClientConfig - discoveryClient discovery.CachedDiscoveryInterface - restMapper meta.RESTMapper -} +// overlyCautiousIllegalFileCharacters matches characters that *might* not be supported. Windows is really restrictive, so this is really restrictive +var overlyCautiousIllegalFileCharacters = regexp.MustCompile(`[^(\w/\.)]`) -func (f *TestConfigFlags) ToRawKubeConfigLoader() clientcmd.ClientConfig { - if f.clientConfig == nil { - panic("attempt to obtain a test RawKubeConfigLoader with no clientConfig specified") - } - return f.clientConfig -} - -func (f *TestConfigFlags) ToRESTConfig() (*rest.Config, error) { - return f.ToRawKubeConfigLoader().ClientConfig() -} - -func (f *TestConfigFlags) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { - return f.discoveryClient, nil -} - -func (f *TestConfigFlags) ToRESTMapper() (meta.RESTMapper, error) { - if f.restMapper != nil { - return f.restMapper, nil - } - if f.discoveryClient != nil { - mapper := restmapper.NewDeferredDiscoveryRESTMapper(f.discoveryClient) - expander := restmapper.NewShortcutExpander(mapper, f.discoveryClient) - return expander, nil - } - return nil, fmt.Errorf("no restmapper") -} - -func (f *TestConfigFlags) WithClientConfig(clientConfig clientcmd.ClientConfig) *TestConfigFlags { - f.clientConfig = clientConfig - return f -} - -func (f *TestConfigFlags) WithRESTMapper(mapper meta.RESTMapper) *TestConfigFlags { - f.restMapper = mapper - return f -} - -func (f *TestConfigFlags) WithDiscoveryClient(c discovery.CachedDiscoveryInterface) *TestConfigFlags { - f.discoveryClient = c - return f -} - -func NewTestConfigFlags() *TestConfigFlags { - return &TestConfigFlags{} +// computeDiscoverCacheDir takes the parentDir and the host and comes up with a "usually non-colliding" name. +func computeDiscoverCacheDir(parentDir, host string) string { + // strip the optional scheme from host if its there: + schemelessHost := strings.Replace(strings.Replace(host, "https://", "", 1), "http://", "", 1) + // now do a simple collapse of non-AZ09 characters. Collisions are possible but unlikely. Even if we do collide the problem is short lived + safeHost := overlyCautiousIllegalFileCharacters.ReplaceAllString(schemelessHost, "_") + return filepath.Join(parentDir, safeHost) } diff --git a/pkg/kubectl/genericclioptions/config_flags_fake.go b/pkg/kubectl/genericclioptions/config_flags_fake.go new file mode 100644 index 00000000000..d019b8c93a2 --- /dev/null +++ b/pkg/kubectl/genericclioptions/config_flags_fake.go @@ -0,0 +1,79 @@ +/* +Copyright 2018 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 genericclioptions + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/discovery" + "k8s.io/client-go/rest" + "k8s.io/client-go/restmapper" + "k8s.io/client-go/tools/clientcmd" +) + +type TestConfigFlags struct { + clientConfig clientcmd.ClientConfig + discoveryClient discovery.CachedDiscoveryInterface + restMapper meta.RESTMapper +} + +func (f *TestConfigFlags) ToRawKubeConfigLoader() clientcmd.ClientConfig { + if f.clientConfig == nil { + panic("attempt to obtain a test RawKubeConfigLoader with no clientConfig specified") + } + return f.clientConfig +} + +func (f *TestConfigFlags) ToRESTConfig() (*rest.Config, error) { + return f.ToRawKubeConfigLoader().ClientConfig() +} + +func (f *TestConfigFlags) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { + return f.discoveryClient, nil +} + +func (f *TestConfigFlags) ToRESTMapper() (meta.RESTMapper, error) { + if f.restMapper != nil { + return f.restMapper, nil + } + if f.discoveryClient != nil { + mapper := restmapper.NewDeferredDiscoveryRESTMapper(f.discoveryClient) + expander := restmapper.NewShortcutExpander(mapper, f.discoveryClient) + return expander, nil + } + return nil, fmt.Errorf("no restmapper") +} + +func (f *TestConfigFlags) WithClientConfig(clientConfig clientcmd.ClientConfig) *TestConfigFlags { + f.clientConfig = clientConfig + return f +} + +func (f *TestConfigFlags) WithRESTMapper(mapper meta.RESTMapper) *TestConfigFlags { + f.restMapper = mapper + return f +} + +func (f *TestConfigFlags) WithDiscoveryClient(c discovery.CachedDiscoveryInterface) *TestConfigFlags { + f.discoveryClient = c + return f +} + +func NewTestConfigFlags() *TestConfigFlags { + return &TestConfigFlags{} +} diff --git a/test/integration/apiserver/BUILD b/test/integration/apiserver/BUILD index 238349f9498..aad43852e8d 100644 --- a/test/integration/apiserver/BUILD +++ b/test/integration/apiserver/BUILD @@ -23,6 +23,7 @@ go_test( "//pkg/api/testapi:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/kubectl/cmd/util:go_default_library", + "//pkg/kubectl/genericclioptions:go_default_library", "//pkg/master:go_default_library", "//pkg/printers:go_default_library", "//pkg/printers/internalversion:go_default_library", diff --git a/test/integration/apiserver/print_test.go b/test/integration/apiserver/print_test.go index 16c7e0ff12c..542a53f5d2c 100644 --- a/test/integration/apiserver/print_test.go +++ b/test/integration/apiserver/print_test.go @@ -40,6 +40,7 @@ import ( "k8s.io/gengo/examples/set-gen/sets" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/kubectl/cmd/util" + "k8s.io/kubernetes/pkg/kubectl/genericclioptions" "k8s.io/kubernetes/pkg/printers" printersinternal "k8s.io/kubernetes/pkg/printers/internalversion" "k8s.io/kubernetes/test/integration/framework" @@ -149,7 +150,7 @@ func TestServerSidePrint(t *testing.T) { tableParam := fmt.Sprintf("application/json;as=Table;g=%s;v=%s, application/json", metav1beta1.GroupName, metav1beta1.SchemeGroupVersion.Version) printer := newFakePrinter(printersinternal.AddHandlers) - configFlags := util.NewTestConfigFlags(). + configFlags := genericclioptions.NewTestConfigFlags(). WithClientConfig(clientcmd.NewDefaultClientConfig(*createKubeConfig(s.URL), &clientcmd.ConfigOverrides{})) restConfig, err := configFlags.ToRESTConfig()