Merge pull request #63514 from juanvallejo/jvallejo/move-config-flags-genericclioptions

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

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())
 

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()
 

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)

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, "")
 }

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",

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())
 

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",

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()

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

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",
     ],
 )
 

pkg/kubectl/cmd/util/config_flags.go → 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
-}
-
-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
-}
+// overlyCautiousIllegalFileCharacters matches characters that *might* not be supported.  Windows is really restrictive, so this is really restrictive
+var overlyCautiousIllegalFileCharacters = regexp.MustCompile(`[^(\w/\.)]`)
 
-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)
 }

pkg/kubectl/genericclioptions/config_flags_fake.go

+/*
+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{}
+}

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",

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()