Merge pull request #30808 from smarterclayton/no_defaults

Automatic merge from submit-queue

ClientConfig should not default to http://localhost:8080

This changes clientcmd to skip the default cluster, but preserves the
behavior in kubectl. This prevents the possibility of an administrator
misconfiguration in kubelet or other server component from allowing a
third party who can bind to 8080 on that host from potentially
impersonating an API server and gaining root access.

@mikedanese @deads2k this removes the defaulting of http://localhost:8080 for server from everything except kubectl.



```release-note
Kubernetes server components using `kubeconfig` files no longer default to `http://localhost:8080`.  Administrators must specify a server value in their kubeconfig files.
```

pkg/client/unversioned/clientcmd/client_config.go

@@ -329,7 +329,7 @@ func (config *DirectClientConfig) getCluster() clientcmdapi.Cluster {
 	clusterInfoName := config.getClusterName()
 
 	var mergedClusterInfo clientcmdapi.Cluster
-	mergo.Merge(&mergedClusterInfo, DefaultCluster)
+	mergo.Merge(&mergedClusterInfo, config.overrides.ClusterDefaults)
 	mergo.Merge(&mergedClusterInfo, EnvVarCluster)
 	if configClusterInfo, exists := clusterInfos[clusterInfoName]; exists {
 		mergo.Merge(&mergedClusterInfo, configClusterInfo)

pkg/client/unversioned/clientcmd/client_config_test.go

@@ -304,11 +304,13 @@ func TestCreateCleanWithPrefix(t *testing.T) {
 		cleanConfig.Server = tc.server
 		config.Clusters["clean"] = cleanConfig
 
-		clientBuilder := NewNonInteractiveClientConfig(*config, "clean", &ConfigOverrides{}, nil)
+		clientBuilder := NewNonInteractiveClientConfig(*config, "clean", &ConfigOverrides{
+			ClusterDefaults: DefaultCluster,
+		}, nil)
 
 		clientConfig, err := clientBuilder.ClientConfig()
 		if err != nil {
-			t.Errorf("Unexpected error: %v", err)
+			t.Fatalf("Unexpected error: %v", err)
 		}
 
 		matchStringArg(tc.host, clientConfig.Host, t)
@@ -321,7 +323,7 @@ func TestCreateCleanDefault(t *testing.T) {
 
 	clientConfig, err := clientBuilder.ClientConfig()
 	if err != nil {
-		t.Errorf("Unexpected error: %v", err)
+		t.Fatalf("Unexpected error: %v", err)
 	}
 
 	matchStringArg(config.Clusters["clean"].Server, clientConfig.Host, t)
@@ -329,14 +331,42 @@ func TestCreateCleanDefault(t *testing.T) {
 	matchStringArg(config.AuthInfos["clean"].Token, clientConfig.BearerToken, t)
 }
 
-func TestCreateMissingContext(t *testing.T) {
+func TestCreateCleanDefaultCluster(t *testing.T) {
+	config := createValidTestConfig()
+	clientBuilder := NewDefaultClientConfig(*config, &ConfigOverrides{
+		ClusterDefaults: DefaultCluster,
+	})
+
+	clientConfig, err := clientBuilder.ClientConfig()
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	matchStringArg(config.Clusters["clean"].Server, clientConfig.Host, t)
+	matchBoolArg(config.Clusters["clean"].InsecureSkipTLSVerify, clientConfig.Insecure, t)
+	matchStringArg(config.AuthInfos["clean"].Token, clientConfig.BearerToken, t)
+}
+
+func TestCreateMissingContextNoDefault(t *testing.T) {
 	const expectedErrorContains = "Context was not found for specified context"
 	config := createValidTestConfig()
 	clientBuilder := NewNonInteractiveClientConfig(*config, "not-present", &ConfigOverrides{}, nil)
 
+	_, err := clientBuilder.ClientConfig()
+	if err == nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}
+func TestCreateMissingContext(t *testing.T) {
+	const expectedErrorContains = "Context was not found for specified context"
+	config := createValidTestConfig()
+	clientBuilder := NewNonInteractiveClientConfig(*config, "not-present", &ConfigOverrides{
+		ClusterDefaults: DefaultCluster,
+	}, nil)
+
 	clientConfig, err := clientBuilder.ClientConfig()
 	if err != nil {
-		t.Errorf("Unexpected error: %v", err)
+		t.Fatalf("Unexpected error: %v", err)
 	}
 
 	expectedConfig := &restclient.Config{Host: clientConfig.Host}
@@ -344,7 +374,6 @@ func TestCreateMissingContext(t *testing.T) {
 	if !reflect.DeepEqual(expectedConfig, clientConfig) {
 		t.Errorf("Expected %#v, got %#v", expectedConfig, clientConfig)
 	}
-
 }
 
 func matchBoolArg(expected, got bool, t *testing.T) {

pkg/client/unversioned/clientcmd/overrides.go

@@ -27,10 +27,12 @@ import (
 // ConfigOverrides holds values that should override whatever information is pulled from the actual Config object.  You can't
 // simply use an actual Config object, because Configs hold maps, but overrides are restricted to "at most one"
 type ConfigOverrides struct {
-	AuthInfo       clientcmdapi.AuthInfo
-	ClusterInfo    clientcmdapi.Cluster
-	Context        clientcmdapi.Context
-	CurrentContext string
+	AuthInfo clientcmdapi.AuthInfo
+	// ClusterDefaults are applied before the configured cluster info is loaded.
+	ClusterDefaults clientcmdapi.Cluster
+	ClusterInfo     clientcmdapi.Cluster
+	Context         clientcmdapi.Context
+	CurrentContext  string
 }
 
 // ConfigOverrideFlags holds the flag names to be used for binding command line flags.  Notice that this structure tightly

pkg/kubectl/cmd/util/factory.go

@@ -33,6 +33,7 @@ import (
 	"time"
 
 	"github.com/emicklei/go-restful/swagger"
+	"github.com/imdario/mergo"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
@@ -1219,6 +1220,9 @@ func DefaultClientConfig(flags *pflag.FlagSet) clientcmd.ClientConfig {
 	flags.StringVar(&loadingRules.ExplicitPath, "kubeconfig", "", "Path to the kubeconfig file to use for CLI requests.")
 
 	overrides := &clientcmd.ConfigOverrides{}
+	// use the standard defaults for this client config
+	mergo.Merge(&overrides.ClusterDefaults, clientcmd.DefaultCluster)
+
 	flagNames := clientcmd.RecommendedConfigOverrideFlags("")
 	// short flagnames are disabled by default.  These are here for compatibility with existing scripts
 	flagNames.ClusterOverrideFlags.APIServer.ShortName = "s"

plugin/pkg/admission/imagepolicy/admission_test.go

@@ -115,7 +115,7 @@ users:
     client-certificate: {{ .Cert }}
     client-key: {{ .Key }}
 `,
-			wantErr: false,
+			wantErr: true,
 		},
 		{
 			msg: "multiple clusters with no context",
@@ -135,7 +135,7 @@ users:
     client-certificate: {{ .Cert }}
     client-key: {{ .Key }}
 `,
-			wantErr: false,
+			wantErr: true,
 		},
 		{
 			msg: "multiple clusters with a context",

plugin/pkg/auth/authorizer/webhook/webhook_test.go

@@ -89,7 +89,7 @@ users:
     client-certificate: {{ .Cert }}
     client-key: {{ .Key }}
 `,
-			wantErr: false,
+			wantErr: true,
 		},
 		{
 			msg: "multiple clusters with no context",
@@ -109,7 +109,7 @@ users:
     client-certificate: {{ .Cert }}
     client-key: {{ .Key }}
 `,
-			wantErr: false,
+			wantErr: true,
 		},
 		{
 			msg: "multiple clusters with a context",