diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index b177486f06c..4bbeec0352e 100755 --- a/hack/make-rules/test-cmd.sh +++ b/hack/make-rules/test-cmd.sh @@ -1652,6 +1652,51 @@ __EOF__ kubectl delete configmap test-configmap --namespace=test-configmaps kubectl delete namespace test-configmaps + #################### + # Client Config # + #################### + + # Command + # Pre-condition: kubeconfig "missing" is not a file or directory + output_message=$(! kubectl get pod --context="" --kubeconfig=missing 2>&1) + kube::test::if_has_string "${output_message}" "missing: no such file or directory" + + # Pre-condition: kubeconfig "missing" is not a file or directory + # Command + output_message=$(! kubectl get pod --user="" --kubeconfig=missing 2>&1) + # Post-condition: --user contains a valid / empty value, missing config file returns error + kube::test::if_has_string "${output_message}" "missing: no such file or directory" + # Command + output_message=$(! kubectl get pod --cluster="" --kubeconfig=missing 2>&1) + # Post-condition: --cluster contains a "valid" value, missing config file returns error + kube::test::if_has_string "${output_message}" "missing: no such file or directory" + + # Pre-condition: context "missing-context" does not exist + # Command + output_message=$(! kubectl get pod --context="missing-context" 2>&1) + kube::test::if_has_string "${output_message}" 'context "missing-context" does not exist' + # Post-condition: invalid or missing context returns error + + # Pre-condition: cluster "missing-cluster" does not exist + # Command + output_message=$(! kubectl get pod --cluster="missing-cluster" 2>&1) + kube::test::if_has_string "${output_message}" 'cluster "missing-cluster" does not exist' + # Post-condition: invalid or missing cluster returns error + + # Pre-condition: user "missing-user" does not exist + # Command + output_message=$(! kubectl get pod --user="missing-user" 2>&1) + kube::test::if_has_string "${output_message}" 'auth info "missing-user" does not exist' + # Post-condition: invalid or missing user returns error + + # test invalid config + kubectl config view | sed -E "s/apiVersion: .*/apiVersion: v-1/g" > "${TMPDIR:-/tmp}"/newconfig.yaml + output_message=$(! "${KUBE_OUTPUT_HOSTBIN}/kubectl" get pods --context="" --user="" --kubeconfig=/tmp/newconfig.yaml 2>&1) + kube::test::if_has_string "${output_message}" "Error loading config file" + + output_message=$(! kubectl get pod --kubeconfig=missing-config 2>&1) + kube::test::if_has_string "${output_message}" 'no such file or directory' + #################### # Service Accounts # #################### diff --git a/pkg/client/unversioned/clientcmd/client_config.go b/pkg/client/unversioned/clientcmd/client_config.go index 8223bad94ef..5eebc02b18d 100644 --- a/pkg/client/unversioned/clientcmd/client_config.go +++ b/pkg/client/unversioned/clientcmd/client_config.go @@ -99,12 +99,28 @@ func (config *DirectClientConfig) RawConfig() (clientcmdapi.Config, error) { // ClientConfig implements ClientConfig func (config *DirectClientConfig) ClientConfig() (*restclient.Config, error) { - if err := config.ConfirmUsable(); err != nil { + // check that getAuthInfo, getContext, and getCluster do not return an error. + // Do this before checking if the curent config is usable in the event that an + // AuthInfo, Context, or Cluster config with user-defined names are not found. + // This provides a user with the immediate cause for error if one is found + configAuthInfo, err := config.getAuthInfo() + if err != nil { return nil, err } - configAuthInfo := config.getAuthInfo() - configClusterInfo := config.getCluster() + _, err = config.getContext() + if err != nil { + return nil, err + } + + configClusterInfo, err := config.getCluster() + if err != nil { + return nil, err + } + + if err := config.ConfirmUsable(); err != nil { + return nil, err + } clientConfig := &restclient.Config{} clientConfig.Host = configClusterInfo.Server @@ -126,7 +142,8 @@ func (config *DirectClientConfig) ClientConfig() (*restclient.Config, error) { // Our mergo.Merge version is older than this change. var persister restclient.AuthProviderConfigPersister if config.configAccess != nil { - persister = PersisterForUser(config.configAccess, config.getAuthInfoName()) + authInfoName, _ := config.getAuthInfoName() + persister = PersisterForUser(config.configAccess, authInfoName) } userAuthPartialConfig, err := getUserIdentificationPartialConfig(configAuthInfo, config.fallbackReader, persister) if err != nil { @@ -252,7 +269,10 @@ func (config *DirectClientConfig) Namespace() (string, bool, error) { return "", false, err } - configContext := config.getContext() + configContext, err := config.getContext() + if err != nil { + return "", false, err + } if len(configContext.Namespace) == 0 { return api.NamespaceDefault, false, nil @@ -289,8 +309,12 @@ func (config *DirectClientConfig) ConfirmUsable() error { } } - validationErrors = append(validationErrors, validateAuthInfo(config.getAuthInfoName(), config.getAuthInfo())...) - validationErrors = append(validationErrors, validateClusterInfo(config.getClusterName(), config.getCluster())...) + authInfoName, _ := config.getAuthInfoName() + authInfo, _ := config.getAuthInfo() + validationErrors = append(validationErrors, validateAuthInfo(authInfoName, authInfo)...) + clusterName, _ := config.getClusterName() + cluster, _ := config.getCluster() + validationErrors = append(validationErrors, validateClusterInfo(clusterName, cluster)...) // when direct client config is specified, and our only error is that no server is defined, we should // return a standard "no config" error if len(validationErrors) == 1 && validationErrors[0] == ErrEmptyCluster { @@ -299,65 +323,84 @@ func (config *DirectClientConfig) ConfirmUsable() error { return newErrConfigurationInvalid(validationErrors) } -func (config *DirectClientConfig) getContextName() string { +// getContextName returns the default, or user-set context name, and a boolean that indicates +// whether the default context name has been overwritten by a user-set flag, or left as its default value +func (config *DirectClientConfig) getContextName() (string, bool) { if len(config.overrides.CurrentContext) != 0 { - return config.overrides.CurrentContext + return config.overrides.CurrentContext, true } if len(config.contextName) != 0 { - return config.contextName + return config.contextName, false } - return config.config.CurrentContext + return config.config.CurrentContext, false } -func (config *DirectClientConfig) getAuthInfoName() string { +// getAuthInfoName returns a string containing the current authinfo name for the current context, +// and a boolean indicating whether the default authInfo name is overwritten by a user-set flag, or +// left as its default value +func (config *DirectClientConfig) getAuthInfoName() (string, bool) { if len(config.overrides.Context.AuthInfo) != 0 { - return config.overrides.Context.AuthInfo + return config.overrides.Context.AuthInfo, true } - return config.getContext().AuthInfo + context, _ := config.getContext() + return context.AuthInfo, false } -func (config *DirectClientConfig) getClusterName() string { +// getClusterName returns a string containing the default, or user-set cluster name, and a boolean +// indicating whether the default clusterName has been overwritten by a user-set flag, or left as +// its default value +func (config *DirectClientConfig) getClusterName() (string, bool) { if len(config.overrides.Context.Cluster) != 0 { - return config.overrides.Context.Cluster + return config.overrides.Context.Cluster, true } - return config.getContext().Cluster + context, _ := config.getContext() + return context.Cluster, false } -func (config *DirectClientConfig) getContext() clientcmdapi.Context { +// getContext returns the clientcmdapi.Context, or an error if a required context is not found. +func (config *DirectClientConfig) getContext() (clientcmdapi.Context, error) { contexts := config.config.Contexts - contextName := config.getContextName() + contextName, required := config.getContextName() var mergedContext clientcmdapi.Context if configContext, exists := contexts[contextName]; exists { mergo.Merge(&mergedContext, configContext) + } else if required { + return clientcmdapi.Context{}, fmt.Errorf("context %q does not exist", contextName) } mergo.Merge(&mergedContext, config.overrides.Context) - return mergedContext + return mergedContext, nil } -func (config *DirectClientConfig) getAuthInfo() clientcmdapi.AuthInfo { +// getAuthInfo returns the clientcmdapi.AuthInfo, or an error if a required auth info is not found. +func (config *DirectClientConfig) getAuthInfo() (clientcmdapi.AuthInfo, error) { authInfos := config.config.AuthInfos - authInfoName := config.getAuthInfoName() + authInfoName, required := config.getAuthInfoName() var mergedAuthInfo clientcmdapi.AuthInfo if configAuthInfo, exists := authInfos[authInfoName]; exists { mergo.Merge(&mergedAuthInfo, configAuthInfo) + } else if required { + return clientcmdapi.AuthInfo{}, fmt.Errorf("auth info %q does not exist", authInfoName) } mergo.Merge(&mergedAuthInfo, config.overrides.AuthInfo) - return mergedAuthInfo + return mergedAuthInfo, nil } -func (config *DirectClientConfig) getCluster() clientcmdapi.Cluster { +// getCluster returns the clientcmdapi.Cluster, or an error if a required cluster is not found. +func (config *DirectClientConfig) getCluster() (clientcmdapi.Cluster, error) { clusterInfos := config.config.Clusters - clusterInfoName := config.getClusterName() + clusterInfoName, required := config.getClusterName() var mergedClusterInfo clientcmdapi.Cluster mergo.Merge(&mergedClusterInfo, config.overrides.ClusterDefaults) if configClusterInfo, exists := clusterInfos[clusterInfoName]; exists { mergo.Merge(&mergedClusterInfo, configClusterInfo) + } else if required { + return clientcmdapi.Cluster{}, fmt.Errorf("cluster %q does not exist", clusterInfoName) } mergo.Merge(&mergedClusterInfo, config.overrides.ClusterInfo) // An override of --insecure-skip-tls-verify=true and no accompanying CA/CA data should clear already-set CA/CA data @@ -369,7 +412,7 @@ func (config *DirectClientConfig) getCluster() clientcmdapi.Cluster { mergedClusterInfo.CertificateAuthorityData = nil } - return mergedClusterInfo + return mergedClusterInfo, nil } // inClusterClientConfig makes a config that will work from within a kubernetes cluster container environment.