diff --git a/pkg/client/clientcmd/client_config.go b/pkg/client/clientcmd/client_config.go index 3b2990339c0..b921afbd607 100644 --- a/pkg/client/clientcmd/client_config.go +++ b/pkg/client/clientcmd/client_config.go @@ -26,7 +26,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/client" clientcmdapi "github.com/GoogleCloudPlatform/kubernetes/pkg/client/clientcmd/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/clientauth" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" ) var ( @@ -261,7 +260,7 @@ func (config DirectClientConfig) ConfirmUsable() error { validationErrors = append(validationErrors, validateAuthInfo(config.getAuthInfoName(), config.getAuthInfo())...) validationErrors = append(validationErrors, validateClusterInfo(config.getClusterName(), config.getCluster())...) - return errors.NewAggregate(validationErrors) + return newErrConfigurationInvalid(validationErrors) } func (config DirectClientConfig) getContextName() string { diff --git a/pkg/client/clientcmd/validation.go b/pkg/client/clientcmd/validation.go index f922513e332..b1f2eb6e325 100644 --- a/pkg/client/clientcmd/validation.go +++ b/pkg/client/clientcmd/validation.go @@ -43,10 +43,47 @@ func IsContextNotFound(err error) bool { if err == nil { return false } - + if _, ok := err.(*errContextNotFound); ok || err == ErrNoContext { + return true + } return strings.Contains(err.Error(), "context was not found for specified context") } +// errConfigurationInvalid is a set of errors indicating the configuration is invalid. +type errConfigurationInvalid []error + +// errConfigurationInvalid implements error and Aggregate +var _ error = errConfigurationInvalid{} +var _ utilerrors.Aggregate = errConfigurationInvalid{} + +func newErrConfigurationInvalid(errs []error) error { + switch len(errs) { + case 0: + return nil + default: + return errConfigurationInvalid(errs) + } +} + +// Error implements the error interface +func (e errConfigurationInvalid) Error() string { + return fmt.Sprintf("invalid configuration: %v", utilerrors.NewAggregate(e).Error()) +} + +// Errors implements the AggregateError interface +func (e errConfigurationInvalid) Errors() []error { + return e +} + +// IsConfigurationInvalid returns true if the provided error indicates the configuration is invalid. +func IsConfigurationInvalid(err error) bool { + switch err.(type) { + case *errContextNotFound, errConfigurationInvalid: + return true + } + return IsContextNotFound(err) +} + // Validate checks for errors in the Config. It does not return early so that it can find as many errors as possible. func Validate(config clientcmdapi.Config) error { validationErrors := make([]error, 0) @@ -69,7 +106,7 @@ func Validate(config clientcmdapi.Config) error { validationErrors = append(validationErrors, validateClusterInfo(clusterName, clusterInfo)...) } - return utilerrors.NewAggregate(validationErrors) + return newErrConfigurationInvalid(validationErrors) } // ConfirmUsable looks a particular context and determines if that particular part of the config is useable. There might still be errors in the config, @@ -99,7 +136,7 @@ func ConfirmUsable(config clientcmdapi.Config, passedContextName string) error { validationErrors = append(validationErrors, validateClusterInfo(context.Cluster, config.Clusters[context.Cluster])...) } - return utilerrors.NewAggregate(validationErrors) + return newErrConfigurationInvalid(validationErrors) } // validateClusterInfo looks for conflicts and errors in the cluster info @@ -107,7 +144,11 @@ func validateClusterInfo(clusterName string, clusterInfo clientcmdapi.Cluster) [ validationErrors := make([]error, 0) if len(clusterInfo.Server) == 0 { - validationErrors = append(validationErrors, fmt.Errorf("no server found for %v", clusterName)) + if len(clusterName) == 0 { + validationErrors = append(validationErrors, fmt.Errorf("default cluster has no server defined")) + } else { + validationErrors = append(validationErrors, fmt.Errorf("no server found for cluster %q", clusterName)) + } } // Make sure CA data and CA file aren't both specified if len(clusterInfo.CertificateAuthority) != 0 && len(clusterInfo.CertificateAuthorityData) != 0 { @@ -155,7 +196,7 @@ func validateAuthInfo(authInfoName string, authInfo clientcmdapi.AuthInfo) []err } // Make sure key data and file aren't both specified if len(authInfo.ClientKey) != 0 && len(authInfo.ClientKeyData) != 0 { - validationErrors = append(validationErrors, fmt.Errorf("client-key-data and client-key are both specified for %v. client-key-data will override.", authInfoName)) + validationErrors = append(validationErrors, fmt.Errorf("client-key-data and client-key are both specified for %v; client-key-data will override", authInfoName)) } // Make sure a key is specified if len(authInfo.ClientKey) == 0 && len(authInfo.ClientKeyData) == 0 { @@ -180,7 +221,7 @@ func validateAuthInfo(authInfoName string, authInfo clientcmdapi.AuthInfo) []err // authPath also provides information for the client to identify the server, so allow multiple auth methods in that case if (len(methods) > 1) && (!usingAuthPath) { - validationErrors = append(validationErrors, fmt.Errorf("more than one authentication method found for %v. Found %v, only one is allowed", authInfoName, methods)) + validationErrors = append(validationErrors, fmt.Errorf("more than one authentication method found for %v; found %v, only one is allowed", authInfoName, methods)) } return validationErrors @@ -191,19 +232,19 @@ func validateContext(contextName string, context clientcmdapi.Context, config cl validationErrors := make([]error, 0) if len(context.AuthInfo) == 0 { - validationErrors = append(validationErrors, fmt.Errorf("user was not specified for Context %v", contextName)) + validationErrors = append(validationErrors, fmt.Errorf("user was not specified for context %q", contextName)) } else if _, exists := config.AuthInfos[context.AuthInfo]; !exists { - validationErrors = append(validationErrors, fmt.Errorf("user, %v, was not found for Context %v", context.AuthInfo, contextName)) + validationErrors = append(validationErrors, fmt.Errorf("user %q was not found for context %q", context.AuthInfo, contextName)) } if len(context.Cluster) == 0 { - validationErrors = append(validationErrors, fmt.Errorf("cluster was not specified for Context %v", contextName)) + validationErrors = append(validationErrors, fmt.Errorf("cluster was not specified for context %q", contextName)) } else if _, exists := config.Clusters[context.Cluster]; !exists { - validationErrors = append(validationErrors, fmt.Errorf("cluster, %v, was not found for Context %v", context.Cluster, contextName)) + validationErrors = append(validationErrors, fmt.Errorf("cluster %q was not found for context %q", context.Cluster, contextName)) } if (len(context.Namespace) != 0) && !util.IsDNS952Label(context.Namespace) { - validationErrors = append(validationErrors, fmt.Errorf("namespace, %v, for context %v, does not conform to the kubernetes DNS952 rules", context.Namespace, contextName)) + validationErrors = append(validationErrors, fmt.Errorf("namespace %q for context %q does not conform to the kubernetes DNS952 rules", context.Namespace, contextName)) } return validationErrors diff --git a/pkg/client/clientcmd/validation_test.go b/pkg/client/clientcmd/validation_test.go index 0bb16cdb6e4..aee278aed01 100644 --- a/pkg/client/clientcmd/validation_test.go +++ b/pkg/client/clientcmd/validation_test.go @@ -127,14 +127,33 @@ func TestIsContextNotFound(t *testing.T) { if !IsContextNotFound(err) { t.Errorf("Expected context not found, but got %v", err) } + if !IsConfigurationInvalid(err) { + t.Errorf("Expected configuration invalid, but got %v", err) + } } + +func TestIsConfigurationInvalid(t *testing.T) { + if newErrConfigurationInvalid([]error{}) != nil { + t.Errorf("unexpected error") + } + if newErrConfigurationInvalid([]error{ErrNoContext}) == ErrNoContext { + t.Errorf("unexpected error") + } + if newErrConfigurationInvalid([]error{ErrNoContext, ErrNoContext}) == nil { + t.Errorf("unexpected error") + } + if !IsConfigurationInvalid(newErrConfigurationInvalid([]error{ErrNoContext, ErrNoContext})) { + t.Errorf("unexpected error") + } +} + func TestValidateMissingReferencesConfig(t *testing.T) { config := clientcmdapi.NewConfig() config.CurrentContext = "anything" config.Contexts["anything"] = clientcmdapi.Context{Cluster: "missing", AuthInfo: "missing"} test := configValidationTest{ config: config, - expectedErrorSubstring: []string{"user, missing, was not found for Context anything", "cluster, missing, was not found for Context anything"}, + expectedErrorSubstring: []string{"user \"missing\" was not found for context \"anything\"", "cluster \"missing\" was not found for context \"anything\""}, } test.testContext("anything", t) @@ -146,7 +165,7 @@ func TestValidateEmptyContext(t *testing.T) { config.Contexts["anything"] = clientcmdapi.Context{} test := configValidationTest{ config: config, - expectedErrorSubstring: []string{"user was not specified for Context anything", "cluster was not specified for Context anything"}, + expectedErrorSubstring: []string{"user was not specified for context \"anything\"", "cluster was not specified for context \"anything\""}, } test.testContext("anything", t) @@ -377,6 +396,9 @@ func (c configValidationTest) testConfig(t *testing.T) { t.Errorf("Expected error containing: %v, but got %v", c.expectedErrorSubstring, err) } } + if !IsConfigurationInvalid(err) { + t.Errorf("all errors should be configuration invalid: %v", err) + } } } else { if err != nil {