stop setting bad defaults that are indistinguishable from real values for clients

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 {
+	// TODO chase down uses and tolerate nil
-		g, err := api.Registry.Group(api.GroupName)
+	if config.GroupVersion == nil {
-		if err != nil {
+		config.GroupVersion = &schema.GroupVersion{}
-			return err
-		}
-		copyGroupVersion := g.GroupVersion
-		config.GroupVersion = &copyGroupVersion
 	}
 	if config.NegotiatedSerializer == nil {
 		config.NegotiatedSerializer = api.Codecs

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

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
 }

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,12 +97,6 @@ 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
-	}
-
 	// only lookup in the cache if the requiredVersion is set
 	if requiredVersion != nil {
 		if config, ok := c.configs[*requiredVersion]; ok {
@@ -113,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)

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
+// ServerSupportsVersion returns an error if the server doesn't have the required version
-// a version that both client and server support.
+func ServerSupportsVersion(client DiscoveryInterface, requiredGV schema.GroupVersion) error {
-// - 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())
-	}
 	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 serverVersions.Has(requiredGV.String()) {
-	// If server does not support warn, but try to negotiate a lower version.
+		return nil
-	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)
 	}
 
-	for _, clientGV := range clientRegisteredGVs {
+	// If the server supports no versions, then we should pretend it has the version because of old servers.
-		if serverVersions.Has(clientGV.String()) {
+	// This can happen because discovery fails due to 403 Forbidden errors
-			// Version was not explicitly requested in command config (--api-version).
+	if len(serverVersions) == 0 {
-			// Ok to fall back to a supported version with a warning.
+		return nil
-			// 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 we have no server versions and we have no required version, choose the first clientRegisteredVersion
+	return fmt.Errorf("server does not support API version %q", requiredGV)
-	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
 }
 
 // GroupVersionResources converts APIResourceLists to the GroupVersionResources.

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
+		requiredVersion schema.GroupVersion
-		expectedVersion *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"},
+			requiredVersion: schema.GroupVersion{Version: "version1"},
-			clientVersions:  []schema.GroupVersion{{Version: "version1"}, v1.SchemeGroupVersion},
-			expectedVersion: &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)
-		}
 	}
 }
 

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