From d1d20e5340d427b7a6a4c022c37f4cecb36ce52b Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Fri, 9 Sep 2016 11:58:07 -0400 Subject: [PATCH] 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. --- .../clientcmd/merged_client_builder.go | 16 ++- .../clientcmd/merged_client_builder_test.go | 117 +++++++++++++++++- 2 files changed, 129 insertions(+), 4 deletions(-) diff --git a/pkg/client/unversioned/clientcmd/merged_client_builder.go b/pkg/client/unversioned/clientcmd/merged_client_builder.go index 8ab93cdd944..f64903335a3 100644 --- a/pkg/client/unversioned/clientcmd/merged_client_builder.go +++ b/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 diff --git a/pkg/client/unversioned/clientcmd/merged_client_builder_test.go b/pkg/client/unversioned/clientcmd/merged_client_builder_test.go index 2bb7c465f9d..304c4ec7460 100644 --- a/pkg/client/unversioned/clientcmd/merged_client_builder_test.go +++ b/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) + } + } +}