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.
kubernetes · Aug 30, 2016 · 06cbb29 · 06cbb29
1 parent 3ccb99d
commit 06cbb29
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 15 deletions.
diff --git a/pkg/client/unversioned/clientcmd/client_config.go b/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)

diff --git a/pkg/client/unversioned/clientcmd/client_config_test.go b/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,30 +323,57 @@ 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)
 	matchBoolArg(config.Clusters["clean"].InsecureSkipTLSVerify, clientConfig.Insecure, 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}
 
 	if !reflect.DeepEqual(expectedConfig, clientConfig) {
 		t.Errorf("Expected %#v, got %#v", expectedConfig, clientConfig)
 	}
-
 }
 
 func matchBoolArg(expected, got bool, t *testing.T) {

diff --git a/pkg/client/unversioned/clientcmd/overrides.go b/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

diff --git a/pkg/kubectl/cmd/util/factory.go b/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"

diff --git a/plugin/pkg/admission/imagepolicy/admission_test.go b/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",

diff --git a/plugin/pkg/auth/authorizer/webhook/webhook_test.go b/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",