Merge pull request #29236 from juanvallejo/jvallejo_add-configuration-missing-error-type

Automatic merge from submit-queue

Update client config invalid option errors to be more specific

This patch adds better error handling for cases where a global option (such as --context or --cluster) causes an invalid config to be returned.

```release-note
release-note-none
```

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 #
   ####################

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()
+	_, err = config.getContext()
-	configClusterInfo := config.getCluster()
+	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())...)
+	authInfoName, _ := config.getAuthInfoName()
-	validationErrors = append(validationErrors, validateClusterInfo(config.getClusterName(), config.getCluster())...)
+	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.