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

Kubernetes-commit: edd2d973a3c777afd691d6abe7b1e6420ad43cba
This commit is contained in:
deads2k 2017-05-12 15:12:04 -04:00 committed by Kubernetes Publisher
parent cf4cea8c3f
commit cf4d5c32f3
3 changed files with 16 additions and 103 deletions

View File

@ -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.

View File

@ -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)
}
}
}

View File

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