diff --git a/pkg/client/unversioned/helper.go b/pkg/client/unversioned/helper.go index c8d09ef41e7..74baf75d0d5 100644 --- a/pkg/client/unversioned/helper.go +++ b/pkg/client/unversioned/helper.go @@ -17,6 +17,7 @@ limitations under the License. package unversioned import ( + "k8s.io/apimachinery/pkg/runtime/schema" restclient "k8s.io/client-go/rest" "k8s.io/kubernetes/pkg/api" // Import solely to initialize client auth plugins. @@ -35,13 +36,9 @@ func SetKubernetesDefaults(config *restclient.Config) error { if config.APIPath == "" { config.APIPath = legacyAPIPath } - if config.GroupVersion == nil || config.GroupVersion.Group != api.GroupName { - g, err := api.Registry.Group(api.GroupName) - if err != nil { - return err - } - copyGroupVersion := g.GroupVersion - config.GroupVersion = ©GroupVersion + // TODO chase down uses and tolerate nil + if config.GroupVersion == nil { + config.GroupVersion = &schema.GroupVersion{} } if config.NegotiatedSerializer == nil { config.NegotiatedSerializer = api.Codecs diff --git a/pkg/client/unversioned/helper_test.go b/pkg/client/unversioned/helper_test.go index 3e884accaa6..52966101591 100644 --- a/pkg/client/unversioned/helper_test.go +++ b/pkg/client/unversioned/helper_test.go @@ -42,7 +42,7 @@ func TestSetKubernetesDefaults(t *testing.T) { restclient.Config{ APIPath: "/api", ContentConfig: restclient.ContentConfig{ - GroupVersion: &api.Registry.GroupOrDie(api.GroupName).GroupVersion, + GroupVersion: &schema.GroupVersion{}, NegotiatedSerializer: testapi.Default.NegotiatedSerializer(), }, }, diff --git a/pkg/kubectl/cmd/config/create_cluster.go b/pkg/kubectl/cmd/config/create_cluster.go index 703496f16bb..fd97a40b27d 100644 --- a/pkg/kubectl/cmd/config/create_cluster.go +++ b/pkg/kubectl/cmd/config/create_cluster.go @@ -37,7 +37,6 @@ type createClusterOptions struct { configAccess clientcmd.ConfigAccess name string server flag.StringFlag - apiVersion flag.StringFlag insecureSkipTLSVerify flag.Tristate certificateAuthority flag.StringFlag embedCAData flag.Tristate @@ -78,7 +77,6 @@ func NewCmdConfigSetCluster(out io.Writer, configAccess clientcmd.ConfigAccess) options.insecureSkipTLSVerify.Default(false) cmd.Flags().Var(&options.server, clientcmd.FlagAPIServer, clientcmd.FlagAPIServer+" for the cluster entry in kubeconfig") - cmd.Flags().Var(&options.apiVersion, clientcmd.FlagAPIVersion, clientcmd.FlagAPIVersion+" for the cluster entry in kubeconfig") f := cmd.Flags().VarPF(&options.insecureSkipTLSVerify, clientcmd.FlagInsecure, "", clientcmd.FlagInsecure+" for the cluster entry in kubeconfig") f.NoOptDefVal = "true" cmd.Flags().Var(&options.certificateAuthority, clientcmd.FlagCAFile, "path to "+clientcmd.FlagCAFile+" file for the cluster entry in kubeconfig") @@ -118,9 +116,6 @@ func (o createClusterOptions) run() error { func (o *createClusterOptions) modifyCluster(existingCluster clientcmdapi.Cluster) clientcmdapi.Cluster { modifiedCluster := existingCluster - if o.apiVersion.Provided() { - modifiedCluster.APIVersion = o.apiVersion.Value() - } if o.server.Provided() { modifiedCluster.Server = o.server.Value() } diff --git a/pkg/kubectl/cmd/explain.go b/pkg/kubectl/cmd/explain.go index 67de24d80a3..245dc99f4a1 100644 --- a/pkg/kubectl/cmd/explain.go +++ b/pkg/kubectl/cmd/explain.go @@ -57,6 +57,7 @@ func NewCmdExplain(f cmdutil.Factory, out, cmdErr io.Writer) *cobra.Command { }, } cmd.Flags().Bool("recursive", false, "Print the fields of fields (Currently only 1 level deep)") + cmd.Flags().String("api-version", "", "The API version to use when talking to the server") cmdutil.AddInclude3rdPartyFlags(cmd) return cmd } diff --git a/pkg/kubectl/cmd/util/clientcache.go b/pkg/kubectl/cmd/util/clientcache.go index 68dec41a406..06c574386d4 100644 --- a/pkg/kubectl/cmd/util/clientcache.go +++ b/pkg/kubectl/cmd/util/clientcache.go @@ -24,7 +24,6 @@ import ( restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" fedclientset "k8s.io/kubernetes/federation/client/clientset_generated/federation_internalclientset" - "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" oldclient "k8s.io/kubernetes/pkg/client/unversioned" "k8s.io/kubernetes/pkg/version" @@ -98,19 +97,7 @@ func (c *ClientCache) ClientConfigForVersion(requiredVersion *schema.GroupVersio // clientConfigForVersion returns the correct config for a server func (c *ClientCache) clientConfigForVersion(requiredVersion *schema.GroupVersion) (*restclient.Config, error) { - // TODO: have a better config copy method - config, discoveryClient, err := c.getDefaultConfig() - if err != nil { - return nil, err - } - if requiredVersion == nil && config.GroupVersion != nil { - // if someone has set the values via flags, our config will have the groupVersion set - // that means it is required. - requiredVersion = config.GroupVersion - } - - // required version may still be nil, since config.GroupVersion may have been nil. Do the check - // before looking up from the cache + // only lookup in the cache if the requiredVersion is set if requiredVersion != nil { if config, ok := c.configs[*requiredVersion]; ok { return copyConfig(config), nil @@ -119,11 +106,21 @@ func (c *ClientCache) clientConfigForVersion(requiredVersion *schema.GroupVersio return copyConfig(c.noVersionConfig), nil } - negotiatedVersion, err := discovery.NegotiateVersion(discoveryClient, requiredVersion, api.Registry.EnabledVersions()) + // this returns a shallow copy to work with + config, discoveryClient, err := c.getDefaultConfig() if err != nil { return nil, err } - config.GroupVersion = negotiatedVersion + + if requiredVersion != nil { + if err := discovery.ServerSupportsVersion(discoveryClient, *requiredVersion); err != nil { + return nil, err + } + config.GroupVersion = requiredVersion + } else { + // TODO remove this hack. This is allowing the GetOptions to be serialized. + config.GroupVersion = &schema.GroupVersion{Group: "", Version: "v1"} + } // TODO this isn't what we want. Each clientset should be setting defaults as it sees fit. oldclient.SetKubernetesDefaults(&config) diff --git a/staging/src/k8s.io/client-go/discovery/helper.go b/staging/src/k8s.io/client-go/discovery/helper.go index f184bc9295c..353d34b3c5a 100644 --- a/staging/src/k8s.io/client-go/discovery/helper.go +++ b/staging/src/k8s.io/client-go/discovery/helper.go @@ -41,25 +41,13 @@ func MatchesServerVersion(clientVersion apimachineryversion.Info, client Discove return nil } -// NegotiateVersion queries the server's supported api versions to find -// a version that both client and server support. -// - If no version is provided, try registered client versions in order of -// preference. -// - If version is provided and the server does not support it, -// return an error. -// TODO negotiation should be reserved for cases where we need a version for a given group. In those cases, it should return an ordered list of -// server preferences. From that list, a separate function can match from an ordered list of client versions. -// This is not what the function has ever done before, but it makes more logical sense. -func NegotiateVersion(client DiscoveryInterface, requiredGV *schema.GroupVersion, clientRegisteredGVs []schema.GroupVersion) (*schema.GroupVersion, error) { - clientVersions := sets.String{} - for _, gv := range clientRegisteredGVs { - clientVersions.Insert(gv.String()) - } +// ServerSupportsVersion returns an error if the server doesn't have the required version +func ServerSupportsVersion(client DiscoveryInterface, requiredGV schema.GroupVersion) error { groups, err := client.ServerGroups() if err != nil { // This is almost always a connection error, and higher level code should treat this as a generic error, // not a negotiation specific error. - return nil, err + return err } versions := metav1.ExtractGroupVersions(groups) serverVersions := sets.String{} @@ -67,46 +55,17 @@ func NegotiateVersion(client DiscoveryInterface, requiredGV *schema.GroupVersion serverVersions.Insert(v) } - // If version explicitly requested verify that both client and server support it. - // If server does not support warn, but try to negotiate a lower version. - if requiredGV != nil { - if !clientVersions.Has(requiredGV.String()) { - return nil, fmt.Errorf("client does not support API version %q; client supported API versions: %v", requiredGV, clientVersions) - - } - // If the server supports no versions, then we should just use the preferredGV - // This can happen because discovery fails due to 403 Forbidden errors - if len(serverVersions) == 0 { - return requiredGV, nil - } - if serverVersions.Has(requiredGV.String()) { - return requiredGV, nil - } - // If we are using an explicit config version the server does not support, fail. - return nil, fmt.Errorf("server does not support API version %q", requiredGV) + if serverVersions.Has(requiredGV.String()) { + return nil } - for _, clientGV := range clientRegisteredGVs { - if serverVersions.Has(clientGV.String()) { - // Version was not explicitly requested in command config (--api-version). - // Ok to fall back to a supported version with a warning. - // TODO: caesarxuchao: enable the warning message when we have - // proper fix. Please refer to issue #14895. - // if len(version) != 0 { - // glog.Warningf("Server does not support API version '%s'. Falling back to '%s'.", version, clientVersion) - // } - t := clientGV - return &t, nil - } + // If the server supports no versions, then we should pretend it has the version because of old servers. + // This can happen because discovery fails due to 403 Forbidden errors + if len(serverVersions) == 0 { + return nil } - // if we have no server versions and we have no required version, choose the first clientRegisteredVersion - if len(serverVersions) == 0 && len(clientRegisteredGVs) > 0 { - return &clientRegisteredGVs[0], nil - } - - // fall back to an empty GroupVersion. Most client commands no longer respect a GroupVersion anyway - return &schema.GroupVersion{}, nil + return fmt.Errorf("server does not support API version %q", requiredGV) } // GroupVersionResources converts APIResourceLists to the GroupVersionResources. diff --git a/staging/src/k8s.io/client-go/discovery/helper_blackbox_test.go b/staging/src/k8s.io/client-go/discovery/helper_blackbox_test.go index ac4df8a911c..7b7236bc44a 100644 --- a/staging/src/k8s.io/client-go/discovery/helper_blackbox_test.go +++ b/staging/src/k8s.io/client-go/discovery/helper_blackbox_test.go @@ -46,81 +46,40 @@ func objBody(object interface{}) io.ReadCloser { return ioutil.NopCloser(bytes.NewReader([]byte(output))) } -func TestNegotiateVersion(t *testing.T) { +func TestServerSupportsVersion(t *testing.T) { tests := []struct { name string - requiredVersion *schema.GroupVersion - expectedVersion *schema.GroupVersion + requiredVersion schema.GroupVersion serverVersions []string - clientVersions []schema.GroupVersion expectErr func(err error) bool sendErr error statusCode int }{ - { - name: "server supports client default", - serverVersions: []string{"version1", v1.SchemeGroupVersion.String()}, - clientVersions: []schema.GroupVersion{{Version: "version1"}, v1.SchemeGroupVersion}, - expectedVersion: &schema.GroupVersion{Version: "version1"}, - statusCode: http.StatusOK, - }, - { - name: "server falls back to client supported", - serverVersions: []string{"version1"}, - clientVersions: []schema.GroupVersion{{Version: "version1"}, v1.SchemeGroupVersion}, - expectedVersion: &schema.GroupVersion{Version: "version1"}, - statusCode: http.StatusOK, - }, { name: "explicit version supported", - requiredVersion: &schema.GroupVersion{Version: "v1"}, + requiredVersion: schema.GroupVersion{Version: "v1"}, serverVersions: []string{"/version1", v1.SchemeGroupVersion.String()}, - clientVersions: []schema.GroupVersion{{Version: "version1"}, v1.SchemeGroupVersion}, - expectedVersion: &schema.GroupVersion{Version: "v1"}, statusCode: http.StatusOK, }, { name: "explicit version not supported on server", - requiredVersion: &schema.GroupVersion{Version: "v1"}, + requiredVersion: schema.GroupVersion{Version: "v1"}, serverVersions: []string{"version1"}, - clientVersions: []schema.GroupVersion{{Version: "version1"}, v1.SchemeGroupVersion}, expectErr: func(err error) bool { return strings.Contains(err.Error(), `server does not support API version "v1"`) }, statusCode: http.StatusOK, }, - { - name: "explicit version not supported on client", - requiredVersion: &schema.GroupVersion{Version: "v1"}, - serverVersions: []string{"v1"}, - clientVersions: []schema.GroupVersion{{Version: "version1"}}, - expectErr: func(err error) bool { return strings.Contains(err.Error(), `client does not support API version "v1"`) }, - statusCode: http.StatusOK, - }, { name: "connection refused error", serverVersions: []string{"version1"}, - clientVersions: []schema.GroupVersion{{Version: "version1"}, v1.SchemeGroupVersion}, sendErr: errors.New("connection refused"), expectErr: func(err error) bool { return strings.Contains(err.Error(), "connection refused") }, statusCode: http.StatusOK, }, - { - name: "discovery fails due to 403 Forbidden errors and thus serverVersions is empty, use default GroupVersion", - clientVersions: []schema.GroupVersion{{Version: "version1"}, v1.SchemeGroupVersion}, - expectedVersion: &schema.GroupVersion{Version: "version1"}, - statusCode: http.StatusForbidden, - }, { name: "discovery fails due to 404 Not Found errors and thus serverVersions is empty, use requested GroupVersion", - requiredVersion: &schema.GroupVersion{Version: "version1"}, - clientVersions: []schema.GroupVersion{{Version: "version1"}, v1.SchemeGroupVersion}, - expectedVersion: &schema.GroupVersion{Version: "version1"}, + requiredVersion: schema.GroupVersion{Version: "version1"}, statusCode: http.StatusNotFound, }, - { - name: "discovery fails due to 403 Forbidden errors and thus serverVersions is empty, fallback to empty GroupVersion", - expectedVersion: &schema.GroupVersion{}, - statusCode: http.StatusForbidden, - }, } for _, test := range tests { @@ -141,7 +100,7 @@ func TestNegotiateVersion(t *testing.T) { } c := discovery.NewDiscoveryClientForConfigOrDie(&restclient.Config{}) c.RESTClient().(*restclient.RESTClient).Client = fakeClient.Client - response, err := discovery.NegotiateVersion(c, test.requiredVersion, test.clientVersions) + err := discovery.ServerSupportsVersion(c, test.requiredVersion) if err == nil && test.expectErr != nil { t.Errorf("expected error, got nil for [%s].", test.name) } @@ -151,9 +110,6 @@ func TestNegotiateVersion(t *testing.T) { } continue } - if *response != *test.expectedVersion { - t.Errorf("%s: expected version %s, got %s.", test.name, test.expectedVersion, response) - } } } diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go index 11a5e7a2774..76090c6f56d 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go @@ -30,11 +30,8 @@ type Config struct { // TODO(jlowdermilk): remove this after eliminating downstream dependencies. // +optional Kind string `json:"kind,omitempty"` - // DEPRECATED: APIVersion is the preferred api version for communicating with the kubernetes cluster (v1, v2, etc). - // Because a cluster can run multiple API groups and potentially multiple versions of each, it no longer makes sense to specify - // a single value for the cluster version. - // This field isn't really needed anyway, so we are deprecating it without replacement. - // It will be ignored if it is present. + // Legacy field from pkg/api/types.go TypeMeta. + // TODO(jlowdermilk): remove this after eliminating downstream dependencies. // +optional APIVersion string `json:"apiVersion,omitempty"` // Preferences holds general information to be use for cli interactions @@ -67,9 +64,6 @@ type Cluster struct { LocationOfOrigin string // Server is the address of the kubernetes cluster (https://hostname:port). Server string `json:"server"` - // APIVersion is the preferred api version for communicating with the kubernetes cluster (v1, v2, etc). - // +optional - APIVersion string `json:"api-version,omitempty"` // InsecureSkipTLSVerify skips the validity check for the server's certificate. This will make your HTTPS connections insecure. // +optional InsecureSkipTLSVerify bool `json:"insecure-skip-tls-verify,omitempty"` diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go index bad2f6ac2ec..21b94d25fdc 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go @@ -29,11 +29,8 @@ type Config struct { // TODO(jlowdermilk): remove this after eliminating downstream dependencies. // +optional Kind string `json:"kind,omitempty"` - // DEPRECATED: APIVersion is the preferred api version for communicating with the kubernetes cluster (v1, v2, etc). - // Because a cluster can run multiple API groups and potentially multiple versions of each, it no longer makes sense to specify - // a single value for the cluster version. - // This field isn't really needed anyway, so we are deprecating it without replacement. - // It will be ignored if it is present. + // Legacy field from pkg/api/types.go TypeMeta. + // TODO(jlowdermilk): remove this after eliminating downstream dependencies. // +optional APIVersion string `json:"apiVersion,omitempty"` // Preferences holds general information to be use for cli interactions @@ -63,9 +60,6 @@ type Preferences struct { type Cluster struct { // Server is the address of the kubernetes cluster (https://hostname:port). Server string `json:"server"` - // APIVersion is the preferred api version for communicating with the kubernetes cluster (v1, v2, etc). - // +optional - APIVersion string `json:"api-version,omitempty"` // InsecureSkipTLSVerify skips the validity check for the server's certificate. This will make your HTTPS connections insecure. // +optional InsecureSkipTLSVerify bool `json:"insecure-skip-tls-verify,omitempty"` diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/overrides.go b/staging/src/k8s.io/client-go/tools/clientcmd/overrides.go index bd817bbf5f3..963ac8fae9b 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/overrides.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/overrides.go @@ -135,7 +135,6 @@ const ( FlagContext = "context" FlagNamespace = "namespace" FlagAPIServer = "server" - FlagAPIVersion = "api-version" FlagInsecure = "insecure-skip-tls-verify" FlagCertFile = "client-certificate" FlagKeyFile = "client-key" @@ -178,7 +177,6 @@ func RecommendedAuthOverrideFlags(prefix string) AuthOverrideFlags { func RecommendedClusterOverrideFlags(prefix string) ClusterOverrideFlags { return ClusterOverrideFlags{ APIServer: FlagInfo{prefix + FlagAPIServer, "", "", "The address and port of the Kubernetes API server"}, - APIVersion: FlagInfo{prefix + FlagAPIVersion, "", "", "DEPRECATED: The API version to use when talking to the server"}, CertificateAuthority: FlagInfo{prefix + FlagCAFile, "", "", "Path to a cert file for the certificate authority"}, InsecureSkipTLSVerify: FlagInfo{prefix + FlagInsecure, "", "false", "If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure"}, } @@ -216,9 +214,6 @@ func BindAuthInfoFlags(authInfo *clientcmdapi.AuthInfo, flags *pflag.FlagSet, fl // BindClusterFlags is a convenience method to bind the specified flags to their associated variables func BindClusterFlags(clusterInfo *clientcmdapi.Cluster, flags *pflag.FlagSet, flagNames ClusterOverrideFlags) { flagNames.APIServer.BindStringFlag(flags, &clusterInfo.Server) - // TODO: remove --api-version flag in 1.3. - flagNames.APIVersion.BindStringFlag(flags, &clusterInfo.APIVersion) - flags.MarkDeprecated(FlagAPIVersion, "flag is no longer respected and will be deleted in the next release") flagNames.CertificateAuthority.BindStringFlag(flags, &clusterInfo.CertificateAuthority) flagNames.InsecureSkipTLSVerify.BindBoolFlag(flags, &clusterInfo.InsecureSkipTLSVerify) } diff --git a/test/integration/examples/apiserver_test.go b/test/integration/examples/apiserver_test.go index 2ab3f4e9be7..fd136b4947d 100644 --- a/test/integration/examples/apiserver_test.go +++ b/test/integration/examples/apiserver_test.go @@ -378,9 +378,6 @@ func createKubeConfig(clientCfg *rest.Config) *clientcmdapi.Config { cluster.CertificateAuthorityData = clientCfg.CAData } cluster.InsecureSkipTLSVerify = clientCfg.Insecure - if clientCfg.GroupVersion != nil { - cluster.APIVersion = clientCfg.GroupVersion.String() - } config.Clusters[clusterNick] = cluster context := clientcmdapi.NewContext()