diff --git a/pkg/kubectl/cmd/cmd.go b/pkg/kubectl/cmd/cmd.go index eda25fc5902..a1f7498ca5b 100644 --- a/pkg/kubectl/cmd/cmd.go +++ b/pkg/kubectl/cmd/cmd.go @@ -276,11 +276,12 @@ func NewKubectlCommand(in io.Reader, out, err io.Writer) *cobra.Command { kubeConfigFlags := cmdutil.NewConfigFlags() kubeConfigFlags.AddFlags(cmds.PersistentFlags()) + matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags) + matchVersionKubeConfigFlags.AddFlags(cmds.PersistentFlags()) cmds.PersistentFlags().AddGoFlagSet(flag.CommandLine) - f := cmdutil.NewFactory(kubeConfigFlags) - f.BindFlags(cmds.PersistentFlags()) + f := cmdutil.NewFactory(matchVersionKubeConfigFlags) // Sending in 'nil' for the getLanguageFn() results in using // the LANG environment variable. diff --git a/pkg/kubectl/cmd/util/BUILD b/pkg/kubectl/cmd/util/BUILD index 3ac8cfa5524..659846272cc 100644 --- a/pkg/kubectl/cmd/util/BUILD +++ b/pkg/kubectl/cmd/util/BUILD @@ -11,6 +11,7 @@ go_library( "factory_client_access.go", "factory_object_mapping.go", "helpers.go", + "kubectl_match_version.go", "printing.go", ], importpath = "k8s.io/kubernetes/pkg/kubectl/cmd/util", diff --git a/pkg/kubectl/cmd/util/config_flags.go b/pkg/kubectl/cmd/util/config_flags.go index 59f89697d5a..db6d1a4655f 100644 --- a/pkg/kubectl/cmd/util/config_flags.go +++ b/pkg/kubectl/cmd/util/config_flags.go @@ -53,6 +53,7 @@ const ( flagUsername = "username" flagPassword = "password" flagTimeout = "request-timeout" + flagHTTPCacheDir = "cache-dir" ) // TODO(juanvallejo): move to pkg/kubectl/genericclioptions once @@ -225,7 +226,7 @@ func (f *ConfigFlags) AddFlags(flags *pflag.FlagSet) { flags.StringVar(f.KubeConfig, "kubeconfig", *f.KubeConfig, "Path to the kubeconfig file to use for CLI requests.") } if f.CacheDir != nil { - flags.StringVar(f.CacheDir, FlagHTTPCacheDir, *f.CacheDir, "Default HTTP cache directory") + flags.StringVar(f.CacheDir, flagHTTPCacheDir, *f.CacheDir, "Default HTTP cache directory") } // add config options diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 2b4e2fc5c0a..eaf06c0aa52 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -50,14 +50,6 @@ import ( "k8s.io/kubernetes/pkg/printers" ) -const ( - FlagMatchBinaryVersion = "match-server-version" -) - -var ( - FlagHTTPCacheDir = "cache-dir" -) - // Factory provides abstractions that allow the Kubectl command to be extended across multiple types // of resources and different API sets. // The rings are here for a reason. In order for composers to be able to provide alternative factory implementations @@ -109,6 +101,10 @@ type ClientAccessFactory interface { // just directions to the server. People use this to build RESTMappers on top of BareClientConfig() (*restclient.Config, error) + // NewBuilder returns an object that assists in loading objects from both disk and the server + // and which implements the common patterns for CLI interactions with generic resources. + NewBuilder() *resource.Builder + // UpdatePodSpecForObject will call the provided function on the pod spec this object supports, // return false if no pod spec is supported, or return an error. UpdatePodSpecForObject(obj runtime.Object, fn func(*v1.PodSpec) error) (bool, error) @@ -128,9 +124,6 @@ type ClientAccessFactory interface { // using the factory. Command(cmd *cobra.Command, showSecrets bool) string - // BindFlags adds any flags that are common to all kubectl sub commands. - BindFlags(flags *pflag.FlagSet) - // SuggestedPodTemplateResources returns a list of resource types that declare a pod template SuggestedPodTemplateResources() []schema.GroupResource @@ -204,9 +197,6 @@ type ObjectMappingFactory interface { // BuilderFactory holds the third level of factory methods. These functions depend upon ObjectMappingFactory and ClientAccessFactory methods. // Generally they depend upon client mapper functions type BuilderFactory interface { - // NewBuilder returns an object that assists in loading objects from both disk and the server - // and which implements the common patterns for CLI interactions with generic resources. - NewBuilder() *resource.Builder // PluginLoader provides the implementation to be used to load cli plugins. PluginLoader() plugins.PluginLoader // PluginRunner provides the implementation to be used to run cli plugins. diff --git a/pkg/kubectl/cmd/util/factory_builder.go b/pkg/kubectl/cmd/util/factory_builder.go index 571e9dc19fb..1d9cbead9e1 100644 --- a/pkg/kubectl/cmd/util/factory_builder.go +++ b/pkg/kubectl/cmd/util/factory_builder.go @@ -26,7 +26,6 @@ import ( scaleclient "k8s.io/client-go/scale" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/plugins" - "k8s.io/kubernetes/pkg/kubectl/resource" ) type ring2Factory struct { @@ -43,20 +42,6 @@ func NewBuilderFactory(clientAccessFactory ClientAccessFactory, objectMappingFac return f } -// NewBuilder returns a new resource builder for structured api objects. -func (f *ring2Factory) NewBuilder() *resource.Builder { - mapper, mapperErr := f.clientAccessFactory.RESTMapper() - categoryExpander, categoryExpanderError := f.objectMappingFactory.CategoryExpander() - - return resource.NewBuilder( - f.clientAccessFactory.ClientConfig, - mapper, - categoryExpander, - ). - AddError(mapperErr). - AddError(categoryExpanderError) -} - // PluginLoader loads plugins from a path set by the KUBECTL_PLUGINS_PATH env var. // If this env var is not set, it defaults to // "~/.kube/plugins", plus diff --git a/pkg/kubectl/cmd/util/factory_client_access.go b/pkg/kubectl/cmd/util/factory_client_access.go index e2c2118caa8..5529bf49d5c 100644 --- a/pkg/kubectl/cmd/util/factory_client_access.go +++ b/pkg/kubectl/cmd/util/factory_client_access.go @@ -26,7 +26,6 @@ import ( "path/filepath" "regexp" "strings" - "sync" "k8s.io/api/core/v1" @@ -58,7 +57,6 @@ import ( "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/resource" - "k8s.io/kubernetes/pkg/version" ) type RESTClientGetter interface { @@ -70,10 +68,6 @@ type RESTClientGetter interface { type ring0Factory struct { clientGetter RESTClientGetter - - requireMatchedServerVersion bool - checkServerVersion sync.Once - matchesServerVersionErr error } func NewClientAccessFactory(clientGetter RESTClientGetter) ClientAccessFactory { @@ -88,6 +82,18 @@ func NewClientAccessFactory(clientGetter RESTClientGetter) ClientAccessFactory { return f } +func (f *ring0Factory) ClientConfig() (*restclient.Config, error) { + return f.clientGetter.ToRESTConfig() +} + +func (f *ring0Factory) RESTMapper() (meta.RESTMapper, error) { + return f.clientGetter.ToRESTMapper() +} + +func (f *ring0Factory) BareClientConfig() (*restclient.Config, error) { + return f.clientGetter.ToRESTConfig() +} + func (f *ring0Factory) DiscoveryClient() (discovery.CachedDiscoveryInterface, error) { return f.clientGetter.ToDiscoveryClient() } @@ -115,40 +121,10 @@ func (f *ring0Factory) DynamicClient() (dynamic.DynamicInterface, error) { } return dynamic.NewForConfig(clientConfig) } -func (f *ring0Factory) checkMatchingServerVersion() error { - f.checkServerVersion.Do(func() { - if !f.requireMatchedServerVersion { - return - } - discoveryClient, err := f.DiscoveryClient() - if err != nil { - f.matchesServerVersionErr = err - return - } - f.matchesServerVersionErr = discovery.MatchesServerVersion(version.Get(), discoveryClient) - }) - return f.matchesServerVersionErr -} - -func (f *ring0Factory) ClientConfig() (*restclient.Config, error) { - if err := f.checkMatchingServerVersion(); err != nil { - return nil, err - } - clientConfig, err := f.clientGetter.ToRESTConfig() - if err != nil { - return nil, err - } - setKubernetesDefaults(clientConfig) - return clientConfig, nil -} - -func (f *ring0Factory) RESTMapper() (meta.RESTMapper, error) { - return f.clientGetter.ToRESTMapper() -} - -func (f *ring0Factory) BareClientConfig() (*restclient.Config, error) { - return f.clientGetter.ToRESTConfig() +// NewBuilder returns a new resource builder for structured api objects. +func (f *ring0Factory) NewBuilder() *resource.Builder { + return resource.NewBuilder(f.clientGetter) } func (f *ring0Factory) RESTClient() (*restclient.RESTClient, error) { @@ -156,6 +132,7 @@ func (f *ring0Factory) RESTClient() (*restclient.RESTClient, error) { if err != nil { return nil, err } + setKubernetesDefaults(clientConfig) return restclient.RESTClientFor(clientConfig) } @@ -325,14 +302,6 @@ func (f *ring0Factory) Command(cmd *cobra.Command, showSecrets bool) string { return base + args + flags } -func (f *ring0Factory) BindFlags(flags *pflag.FlagSet) { - // Globally persistent flags across all subcommands. - // TODO Change flag names to consts to allow safer lookup from subcommands. - // TODO Add a verbose flag that turns on glog logging. Probably need a way - // to do that automatically for every subcommand. - flags.BoolVar(&f.requireMatchedServerVersion, FlagMatchBinaryVersion, false, "Require server version to match client version") -} - func (f *ring0Factory) SuggestedPodTemplateResources() []schema.GroupResource { return []schema.GroupResource{ {Resource: "replicationcontroller"}, @@ -635,19 +604,3 @@ func InternalVersionJSONEncoder() runtime.Encoder { encoder := legacyscheme.Codecs.LegacyCodec(legacyscheme.Scheme.PrioritizedVersionsAllGroups()...) return unstructured.JSONFallbackEncoder{Encoder: encoder} } - -// setKubernetesDefaults sets default values on the provided client config for accessing the -// Kubernetes API or returns an error if any of the defaults are impossible or invalid. -// TODO this isn't what we want. Each clientset should be setting defaults as it sees fit. -func setKubernetesDefaults(config *restclient.Config) error { - // TODO remove this hack. This is allowing the GetOptions to be serialized. - config.GroupVersion = &schema.GroupVersion{Group: "", Version: "v1"} - - if config.APIPath == "" { - config.APIPath = "/api" - } - if config.NegotiatedSerializer == nil { - config.NegotiatedSerializer = legacyscheme.Codecs - } - return restclient.SetKubernetesDefaults(config) -} diff --git a/pkg/kubectl/cmd/util/factory_object_mapping.go b/pkg/kubectl/cmd/util/factory_object_mapping.go index fd62e3afdef..78efe957f2b 100644 --- a/pkg/kubectl/cmd/util/factory_object_mapping.go +++ b/pkg/kubectl/cmd/util/factory_object_mapping.go @@ -81,7 +81,7 @@ func (f *ring1Factory) CategoryExpander() (restmapper.CategoryExpander, error) { return nil, err } - return restmapper.NewDiscoveryCategoryExpander(discoveryClient) + return restmapper.NewDiscoveryCategoryExpander(discoveryClient), nil } func (f *ring1Factory) ClientForMapping(mapping *meta.RESTMapping) (resource.RESTClient, error) { diff --git a/pkg/kubectl/cmd/util/kubectl_match_version.go b/pkg/kubectl/cmd/util/kubectl_match_version.go new file mode 100644 index 00000000000..954216dc4f9 --- /dev/null +++ b/pkg/kubectl/cmd/util/kubectl_match_version.go @@ -0,0 +1,125 @@ +/* +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 util + +import ( + "sync" + + "github.com/spf13/pflag" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/discovery" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + + "k8s.io/kubernetes/pkg/api/legacyscheme" + "k8s.io/kubernetes/pkg/version" +) + +const ( + flagMatchBinaryVersion = "match-server-version" +) + +// MatchVersionFlags is for setting the "match server version" function. +type MatchVersionFlags struct { + Delegate RESTClientGetter + + RequireMatchedServerVersion bool + checkServerVersion sync.Once + matchesServerVersionErr error +} + +var _ RESTClientGetter = &MatchVersionFlags{} + +func (f *MatchVersionFlags) checkMatchingServerVersion() error { + f.checkServerVersion.Do(func() { + if !f.RequireMatchedServerVersion { + return + } + discoveryClient, err := f.Delegate.ToDiscoveryClient() + if err != nil { + f.matchesServerVersionErr = err + return + } + f.matchesServerVersionErr = discovery.MatchesServerVersion(version.Get(), discoveryClient) + }) + + return f.matchesServerVersionErr +} + +// ToRESTConfig implements RESTClientGetter. +// Returns a REST client configuration based on a provided path +// to a .kubeconfig file, loading rules, and config flag overrides. +// Expects the AddFlags method to have been called. +func (f *MatchVersionFlags) ToRESTConfig() (*rest.Config, error) { + if err := f.checkMatchingServerVersion(); err != nil { + return nil, err + } + clientConfig, err := f.Delegate.ToRESTConfig() + if err != nil { + return nil, err + } + // TODO we should not have to do this. It smacks of something going wrong. + setKubernetesDefaults(clientConfig) + return clientConfig, nil +} + +func (f *MatchVersionFlags) ToRawKubeConfigLoader() clientcmd.ClientConfig { + return f.Delegate.ToRawKubeConfigLoader() +} + +func (f *MatchVersionFlags) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { + if err := f.checkMatchingServerVersion(); err != nil { + return nil, err + } + return f.Delegate.ToDiscoveryClient() +} + +// RESTMapper returns a mapper. +func (f *MatchVersionFlags) ToRESTMapper() (meta.RESTMapper, error) { + if err := f.checkMatchingServerVersion(); err != nil { + return nil, err + } + return f.Delegate.ToRESTMapper() +} + +func (f *MatchVersionFlags) AddFlags(flags *pflag.FlagSet) { + flags.BoolVar(&f.RequireMatchedServerVersion, flagMatchBinaryVersion, f.RequireMatchedServerVersion, "Require server version to match client version") +} + +func NewMatchVersionFlags(delegate RESTClientGetter) *MatchVersionFlags { + return &MatchVersionFlags{ + Delegate: delegate, + } +} + +// setKubernetesDefaults sets default values on the provided client config for accessing the +// Kubernetes API or returns an error if any of the defaults are impossible or invalid. +// TODO this isn't what we want. Each clientset should be setting defaults as it sees fit. +func setKubernetesDefaults(config *rest.Config) error { + // TODO remove this hack. This is allowing the GetOptions to be serialized. + config.GroupVersion = &schema.GroupVersion{Group: "", Version: "v1"} + + if config.APIPath == "" { + config.APIPath = "/api" + } + if config.NegotiatedSerializer == nil { + config.NegotiatedSerializer = legacyscheme.Codecs + } + return rest.SetKubernetesDefaults(config) +} diff --git a/pkg/kubectl/resource/BUILD b/pkg/kubectl/resource/BUILD index ed6223d03a6..a9cd926b5a7 100644 --- a/pkg/kubectl/resource/BUILD +++ b/pkg/kubectl/resource/BUILD @@ -42,6 +42,7 @@ 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/client-go/discovery:go_default_library", "//vendor/k8s.io/client-go/dynamic:go_default_library", "//vendor/k8s.io/client-go/rest:go_default_library", "//vendor/k8s.io/client-go/restmapper:go_default_library", diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index 0ab4f920c6b..71bb3253e5a 100644 --- a/pkg/kubectl/resource/builder.go +++ b/pkg/kubectl/resource/builder.go @@ -144,7 +144,7 @@ type resourceTuple struct { type FakeClientFunc func(version schema.GroupVersion) (RESTClient, error) func NewFakeBuilder(fakeClientFn FakeClientFunc, restMapper meta.RESTMapper, categoryExpander restmapper.CategoryExpander) *Builder { - ret := NewBuilder(nil, restMapper, categoryExpander) + ret := newBuilder(nil, restMapper, categoryExpander) ret.fakeClientFn = fakeClientFn return ret } @@ -153,7 +153,7 @@ func NewFakeBuilder(fakeClientFn FakeClientFunc, restMapper meta.RESTMapper, cat // internal or unstructured must be specified. // TODO: Add versioned client (although versioned is still lossy) // TODO remove internal and unstructured mapper and instead have them set the negotiated serializer for use in the client -func NewBuilder(clientConfigFn ClientConfigFunc, restMapper meta.RESTMapper, categoryExpander restmapper.CategoryExpander) *Builder { +func newBuilder(clientConfigFn ClientConfigFunc, restMapper meta.RESTMapper, categoryExpander restmapper.CategoryExpander) *Builder { return &Builder{ clientConfigFn: clientConfigFn, restMapper: restMapper, @@ -162,6 +162,21 @@ func NewBuilder(clientConfigFn ClientConfigFunc, restMapper meta.RESTMapper, cat } } +func NewBuilder(restClientGetter RESTClientGetter) *Builder { + restMapper, mapperErr := restClientGetter.ToRESTMapper() + discoveryClient, discoveryErr := restClientGetter.ToDiscoveryClient() + var categoryExpander restmapper.CategoryExpander + if discoveryErr == nil { + categoryExpander = restmapper.NewDiscoveryCategoryExpander(discoveryClient) + } + + return newBuilder( + restClientGetter.ToRESTConfig, + restMapper, + categoryExpander, + ).AddError(mapperErr).AddError(discoveryErr) +} + func (b *Builder) Schema(schema validation.Schema) *Builder { b.schema = schema return b diff --git a/pkg/kubectl/resource/interfaces.go b/pkg/kubectl/resource/interfaces.go index 359a014846c..c776eb6bfec 100644 --- a/pkg/kubectl/resource/interfaces.go +++ b/pkg/kubectl/resource/interfaces.go @@ -17,10 +17,18 @@ limitations under the License. package resource import ( + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/discovery" "k8s.io/client-go/rest" ) +type RESTClientGetter interface { + ToRESTConfig() (*rest.Config, error) + ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) + ToRESTMapper() (meta.RESTMapper, error) +} + type ClientConfigFunc func() (*rest.Config, error) // RESTClient is a client helper for dealing with RESTful resources diff --git a/staging/src/k8s.io/client-go/restmapper/category_expansion.go b/staging/src/k8s.io/client-go/restmapper/category_expansion.go index fe6f884bcea..1620bbcf810 100644 --- a/staging/src/k8s.io/client-go/restmapper/category_expansion.go +++ b/staging/src/k8s.io/client-go/restmapper/category_expansion.go @@ -48,11 +48,11 @@ type discoveryCategoryExpander struct { // NewDiscoveryCategoryExpander returns a category expander that makes use of the "categories" fields from // the API, found through the discovery client. In case of any error or no category found (which likely // means we're at a cluster prior to categories support, fallback to the expander provided. -func NewDiscoveryCategoryExpander(client discovery.DiscoveryInterface) (CategoryExpander, error) { +func NewDiscoveryCategoryExpander(client discovery.DiscoveryInterface) CategoryExpander { if client == nil { panic("Please provide discovery client to shortcut expander") } - return discoveryCategoryExpander{discoveryClient: client}, nil + return discoveryCategoryExpander{discoveryClient: client} } // Expand fulfills CategoryExpander diff --git a/staging/src/k8s.io/client-go/restmapper/category_expansion_test.go b/staging/src/k8s.io/client-go/restmapper/category_expansion_test.go index bda77a89c4f..8537a6b49b9 100644 --- a/staging/src/k8s.io/client-go/restmapper/category_expansion_test.go +++ b/staging/src/k8s.io/client-go/restmapper/category_expansion_test.go @@ -135,10 +135,7 @@ func TestDiscoveryCategoryExpander(t *testing.T) { dc.serverResourcesHandler = func() ([]*metav1.APIResourceList, error) { return test.serverResponse, nil } - expander, err := NewDiscoveryCategoryExpander(dc) - if err != nil { - t.Fatalf("unexpected error %v", err) - } + expander := NewDiscoveryCategoryExpander(dc) expanded, _ := expander.Expand(test.category) if !reflect.DeepEqual(expanded, test.expected) { t.Errorf("expected %v, got %v", test.expected, expanded)