Merge pull request #32393 from smarterclayton/incluster_namespace

Automatic merge from submit-queue

Allow namespace to be loaded from in-cluster config

This is a follow on from the previous commit that fixed ClientConfig.
Namespace can also be defaulted from ICC, and this correctly handles
that logic. Also add two debugging lines to ensure that it is easier in
the future to uncover problems here.

Fixes #32352

@deads2k

pkg/client/unversioned/clientcmd/merged_client_builder.go

@@ -21,6 +21,8 @@ import (
 	"reflect"
 	"sync"
 
+	"github.com/golang/glog"
+
 	"k8s.io/kubernetes/pkg/client/restclient"
 	clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api"
 )
@@ -120,6 +122,7 @@ func (config *DeferredLoadingClientConfig) ClientConfig() (*restclient.Config, e
 
 	// check for in-cluster configuration and use it
 	if config.icc.Possible() {
+		glog.V(4).Infof("Using in-cluster configuration")
 		return config.icc.ClientConfig()
 	}
 
@@ -134,7 +137,18 @@ func (config *DeferredLoadingClientConfig) Namespace() (string, bool, error) {
 		return "", false, err
 	}
 
-	return mergedKubeConfig.Namespace()
+	ns, ok, err := mergedKubeConfig.Namespace()
+	// if we get an error and it is not empty config, or if the merged config defined an explicit namespace, or
+	// if in-cluster config is not possible, return immediately
+	if (err != nil && !IsEmptyConfig(err)) || ok || !config.icc.Possible() {
+		// return on any error except empty config
+		return ns, ok, err
+	}
+
+	glog.V(4).Infof("Using in-cluster namespace")
+
+	// allow the namespace from the service account token directory to be used.
+	return config.icc.Namespace()
 }
 
 // ConfigAccess implements ClientConfig

pkg/client/unversioned/clientcmd/merged_client_builder_test.go

@@ -38,8 +38,10 @@ func (l *testLoader) Load() (*clientcmdapi.Config, error) {
 }
 
 type testClientConfig struct {
-	config *restclient.Config
-	err    error
+	config             *restclient.Config
+	namespace          string
+	namespaceSpecified bool
+	err                error
 }
 
 func (c *testClientConfig) RawConfig() (clientcmdapi.Config, error) {
@@ -49,7 +51,7 @@ func (c *testClientConfig) ClientConfig() (*restclient.Config, error) {
 	return c.config, c.err
 }
 func (c *testClientConfig) Namespace() (string, bool, error) {
-	return "", false, fmt.Errorf("unexpected call")
+	return c.namespace, c.namespaceSpecified, c.err
 }
 func (c *testClientConfig) ConfigAccess() ConfigAccess {
 	return nil
@@ -192,3 +194,112 @@ func TestInClusterConfig(t *testing.T) {
 		}
 	}
 }
+
+func TestInClusterConfigNamespace(t *testing.T) {
+	err1 := fmt.Errorf("unique error")
+
+	testCases := map[string]struct {
+		clientConfig *testClientConfig
+		icc          *testICC
+
+		checkedICC bool
+		result     string
+		ok         bool
+		err        error
+	}{
+		"in-cluster checked on empty error": {
+			clientConfig: &testClientConfig{err: ErrEmptyConfig},
+			icc:          &testICC{},
+
+			checkedICC: true,
+			err:        ErrEmptyConfig,
+		},
+
+		"in-cluster not checked on non-empty error": {
+			clientConfig: &testClientConfig{err: ErrEmptyCluster},
+			icc:          &testICC{},
+
+			err: ErrEmptyCluster,
+		},
+
+		"in-cluster checked when config is default": {
+			clientConfig: &testClientConfig{},
+			icc:          &testICC{},
+
+			checkedICC: true,
+		},
+
+		"in-cluster not checked when config is not equal to default": {
+			clientConfig: &testClientConfig{namespace: "test", namespaceSpecified: true},
+			icc:          &testICC{},
+
+			result: "test",
+			ok:     true,
+		},
+
+		"in-cluster checked when namespace is not specified, but is defaulted": {
+			clientConfig: &testClientConfig{namespace: "test", namespaceSpecified: false},
+			icc:          &testICC{},
+
+			checkedICC: true,
+			result:     "test",
+			ok:         false,
+		},
+
+		"in-cluster error returned when config is empty": {
+			clientConfig: &testClientConfig{err: ErrEmptyConfig},
+			icc: &testICC{
+				possible: true,
+				testClientConfig: testClientConfig{
+					err: err1,
+				},
+			},
+
+			checkedICC: true,
+			err:        err1,
+		},
+
+		"in-cluster config returned when config is empty": {
+			clientConfig: &testClientConfig{err: ErrEmptyConfig},
+			icc: &testICC{
+				possible: true,
+				testClientConfig: testClientConfig{
+					namespace:          "test",
+					namespaceSpecified: true,
+				},
+			},
+
+			checkedICC: true,
+			result:     "test",
+			ok:         true,
+		},
+
+		"in-cluster config returned when config is empty and namespace is defaulted but not explicitly set": {
+			clientConfig: &testClientConfig{err: ErrEmptyConfig},
+			icc: &testICC{
+				possible: true,
+				testClientConfig: testClientConfig{
+					namespace:          "test",
+					namespaceSpecified: false,
+				},
+			},
+
+			checkedICC: true,
+			result:     "test",
+			ok:         false,
+		},
+	}
+
+	for name, test := range testCases {
+		c := &DeferredLoadingClientConfig{icc: test.icc}
+		c.clientConfig = test.clientConfig
+
+		ns, ok, err := c.Namespace()
+		if test.icc.called != test.checkedICC {
+			t.Errorf("%s: unexpected in-cluster-config call %t", name, test.icc.called)
+		}
+		if err != test.err || ns != test.result || ok != test.ok {
+			t.Errorf("%s: unexpected result: %v %s %t", name, err, ns, ok)
+		}
+	}
+}