diff --git a/pkg/client/unversioned/clientcmd/client_config.go b/pkg/client/unversioned/clientcmd/client_config.go index 014c6a24069..339b5d84785 100644 --- a/pkg/client/unversioned/clientcmd/client_config.go +++ b/pkg/client/unversioned/clientcmd/client_config.go @@ -350,6 +350,8 @@ func (config *DirectClientConfig) getCluster() clientcmdapi.Cluster { // inClusterClientConfig makes a config that will work from within a kubernetes cluster container environment. type inClusterClientConfig struct{} +var _ ClientConfig = inClusterClientConfig{} + func (inClusterClientConfig) RawConfig() (clientcmdapi.Config, error) { return clientcmdapi.Config{}, fmt.Errorf("inCluster environment config doesn't support multiple clusters") } @@ -358,21 +360,21 @@ func (inClusterClientConfig) ClientConfig() (*restclient.Config, error) { return restclient.InClusterConfig() } -func (inClusterClientConfig) Namespace() (string, error) { +func (inClusterClientConfig) Namespace() (string, bool, error) { // This way assumes you've set the POD_NAMESPACE environment variable using the downward API. // This check has to be done first for backwards compatibility with the way InClusterConfig was originally set up if ns := os.Getenv("POD_NAMESPACE"); ns != "" { - return ns, nil + return ns, true, nil } // Fall back to the namespace associated with the service account token, if available if data, err := ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace"); err == nil { if ns := strings.TrimSpace(string(data)); len(ns) > 0 { - return ns, nil + return ns, true, nil } } - return "default", nil + return "default", false, nil } func (inClusterClientConfig) ConfigAccess() ConfigAccess { diff --git a/pkg/client/unversioned/clientcmd/merged_client_builder.go b/pkg/client/unversioned/clientcmd/merged_client_builder.go index 0180469127f..8ab93cdd944 100644 --- a/pkg/client/unversioned/clientcmd/merged_client_builder.go +++ b/pkg/client/unversioned/clientcmd/merged_client_builder.go @@ -21,8 +21,6 @@ import ( "reflect" "sync" - "github.com/golang/glog" - "k8s.io/kubernetes/pkg/client/restclient" clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api" ) @@ -39,16 +37,25 @@ type DeferredLoadingClientConfig struct { clientConfig ClientConfig loadingLock sync.Mutex + + // provided for testing + icc InClusterConfig +} + +// InClusterConfig abstracts details of whether the client is running in a cluster for testing. +type InClusterConfig interface { + ClientConfig + Possible() bool } // NewNonInteractiveDeferredLoadingClientConfig creates a ConfigClientClientConfig using the passed context name func NewNonInteractiveDeferredLoadingClientConfig(loader ClientConfigLoader, overrides *ConfigOverrides) ClientConfig { - return &DeferredLoadingClientConfig{loader: loader, overrides: overrides} + return &DeferredLoadingClientConfig{loader: loader, overrides: overrides, icc: inClusterClientConfig{}} } // NewInteractiveDeferredLoadingClientConfig creates a ConfigClientClientConfig using the passed context name and the fallback auth reader func NewInteractiveDeferredLoadingClientConfig(loader ClientConfigLoader, overrides *ConfigOverrides, fallbackReader io.Reader) ClientConfig { - return &DeferredLoadingClientConfig{loader: loader, overrides: overrides, fallbackReader: fallbackReader} + return &DeferredLoadingClientConfig{loader: loader, overrides: overrides, icc: inClusterClientConfig{}, fallbackReader: fallbackReader} } func (config *DeferredLoadingClientConfig) createClientConfig() (ClientConfig, error) { @@ -92,18 +99,32 @@ func (config *DeferredLoadingClientConfig) ClientConfig() (*restclient.Config, e return nil, err } + // load the configuration and return on non-empty errors and if the + // content differs from the default config mergedConfig, err := mergedClientConfig.ClientConfig() - if err != nil { + switch { + case err != nil && !IsEmptyConfig(err): + // return on any error except empty config return nil, err + case mergedConfig != nil: + // if the configuration has any settings at all, we cannot use ICC + // TODO: we need to discriminate better between "empty due to env" and + // "empty due to defaults" + // TODO: this shouldn't be a global - the client config rules should be + // handling this. + defaultConfig, err := DefaultClientConfig.ClientConfig() + if err == nil && !reflect.DeepEqual(mergedConfig, defaultConfig) { + return mergedConfig, nil + } } - // Are we running in a cluster and were no other configs found? If so, use the in-cluster-config. - icc := inClusterClientConfig{} - defaultConfig, err := DefaultClientConfig.ClientConfig() - if icc.Possible() && err == nil && reflect.DeepEqual(mergedConfig, defaultConfig) { - glog.V(2).Info("No kubeconfig could be created, falling back to service account.") - return icc.ClientConfig() + + // check for in-cluster configuration and use it + if config.icc.Possible() { + return config.icc.ClientConfig() } - return mergedConfig, nil + + // return the result of the merged client config + return mergedConfig, err } // Namespace implements KubeConfig diff --git a/pkg/client/unversioned/clientcmd/merged_client_builder_test.go b/pkg/client/unversioned/clientcmd/merged_client_builder_test.go new file mode 100644 index 00000000000..2bb7c465f9d --- /dev/null +++ b/pkg/client/unversioned/clientcmd/merged_client_builder_test.go @@ -0,0 +1,194 @@ +/* +Copyright 2014 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package clientcmd + +import ( + "fmt" + "testing" + + "k8s.io/kubernetes/pkg/client/restclient" + clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api" +) + +type testLoader struct { + ClientConfigLoader + + called bool + config *clientcmdapi.Config + err error +} + +func (l *testLoader) Load() (*clientcmdapi.Config, error) { + l.called = true + return l.config, l.err +} + +type testClientConfig struct { + config *restclient.Config + err error +} + +func (c *testClientConfig) RawConfig() (clientcmdapi.Config, error) { + return clientcmdapi.Config{}, fmt.Errorf("unexpected call") +} +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") +} +func (c *testClientConfig) ConfigAccess() ConfigAccess { + return nil +} + +type testICC struct { + testClientConfig + + possible bool + called bool +} + +func (icc *testICC) Possible() bool { + icc.called = true + return icc.possible +} + +func TestInClusterConfig(t *testing.T) { + // override direct client config for this run + originalDefault := DefaultClientConfig + defer func() { + DefaultClientConfig = originalDefault + }() + + baseDefault := &DirectClientConfig{ + overrides: &ConfigOverrides{}, + } + default1 := &DirectClientConfig{ + config: *createValidTestConfig(), + contextName: "clean", + overrides: &ConfigOverrides{}, + } + config1, err := default1.ClientConfig() + if err != nil { + t.Fatal(err) + } + config2 := &restclient.Config{Host: "config2"} + err1 := fmt.Errorf("unique error") + + testCases := map[string]struct { + clientConfig *testClientConfig + icc *testICC + defaultConfig *DirectClientConfig + + checkedICC bool + result *restclient.Config + err error + }{ + "in-cluster checked on other error": { + clientConfig: &testClientConfig{err: ErrEmptyConfig}, + icc: &testICC{}, + + checkedICC: true, + result: nil, + err: ErrEmptyConfig, + }, + + "in-cluster not checked on non-empty error": { + clientConfig: &testClientConfig{err: ErrEmptyCluster}, + icc: &testICC{}, + + checkedICC: false, + result: nil, + err: ErrEmptyCluster, + }, + + "in-cluster checked when config is default": { + defaultConfig: default1, + clientConfig: &testClientConfig{config: config1}, + icc: &testICC{}, + + checkedICC: true, + result: config1, + err: nil, + }, + + "in-cluster not checked when config is not equal to default": { + defaultConfig: default1, + clientConfig: &testClientConfig{config: config2}, + icc: &testICC{}, + + checkedICC: false, + result: config2, + err: nil, + }, + + "in-cluster checked when config is not equal to default and error is empty": { + clientConfig: &testClientConfig{config: config2, err: ErrEmptyConfig}, + icc: &testICC{}, + + checkedICC: true, + result: config2, + err: ErrEmptyConfig, + }, + + "in-cluster error returned when config is empty": { + clientConfig: &testClientConfig{err: ErrEmptyConfig}, + icc: &testICC{ + possible: true, + testClientConfig: testClientConfig{ + err: err1, + }, + }, + + checkedICC: true, + result: nil, + err: err1, + }, + + "in-cluster config returned when config is empty": { + clientConfig: &testClientConfig{err: ErrEmptyConfig}, + icc: &testICC{ + possible: true, + testClientConfig: testClientConfig{ + config: config2, + }, + }, + + checkedICC: true, + result: config2, + err: nil, + }, + } + + for name, test := range testCases { + if test.defaultConfig != nil { + DefaultClientConfig = *test.defaultConfig + } else { + DefaultClientConfig = *baseDefault + } + c := &DeferredLoadingClientConfig{icc: test.icc} + c.clientConfig = test.clientConfig + + cfg, err := c.ClientConfig() + if test.icc.called != test.checkedICC { + t.Errorf("%s: unexpected in-cluster-config call %t", name, test.icc.called) + } + if err != test.err || cfg != test.result { + t.Errorf("%s: unexpected result: %v %#v", name, err, cfg) + } + } +}