From bdea92bccdc0937f169c8b608581bdddf3120c28 Mon Sep 17 00:00:00 2001 From: Fabiano Franz Date: Tue, 20 Dec 2016 03:17:37 -0200 Subject: [PATCH] In-cluster configs must take flag overrides into account --- .../unversioned/clientcmd/client_config.go | 43 +++++-- .../clientcmd/client_config_test.go | 115 ++++++++++++++++++ .../clientcmd/merged_client_builder.go | 4 +- pkg/client/unversioned/clientcmd/overrides.go | 42 +++---- test/e2e/kubectl.go | 42 +++++++ 5 files changed, 215 insertions(+), 31 deletions(-) diff --git a/pkg/client/unversioned/clientcmd/client_config.go b/pkg/client/unversioned/clientcmd/client_config.go index f564228c4e8..8fd01923e8b 100644 --- a/pkg/client/unversioned/clientcmd/client_config.go +++ b/pkg/client/unversioned/clientcmd/client_config.go @@ -439,19 +439,46 @@ func (config *DirectClientConfig) getCluster() (clientcmdapi.Cluster, error) { } // inClusterClientConfig makes a config that will work from within a kubernetes cluster container environment. -type inClusterClientConfig struct{} +// Can take options overrides for flags explicitly provided to the command inside the cluster container. +type inClusterClientConfig struct { + overrides *ConfigOverrides + inClusterConfigProvider func() (*restclient.Config, error) +} -var _ ClientConfig = inClusterClientConfig{} +var _ ClientConfig = &inClusterClientConfig{} -func (inClusterClientConfig) RawConfig() (clientcmdapi.Config, error) { +func (config *inClusterClientConfig) RawConfig() (clientcmdapi.Config, error) { return clientcmdapi.Config{}, fmt.Errorf("inCluster environment config doesn't support multiple clusters") } -func (inClusterClientConfig) ClientConfig() (*restclient.Config, error) { - return restclient.InClusterConfig() +func (config *inClusterClientConfig) ClientConfig() (*restclient.Config, error) { + if config.inClusterConfigProvider == nil { + config.inClusterConfigProvider = restclient.InClusterConfig + } + + icc, err := config.inClusterConfigProvider() + if err != nil { + return nil, err + } + + // in-cluster configs only takes a host, token, or CA file + // if any of them were individually provided, ovewrite anything else + if config.overrides != nil { + if server := config.overrides.ClusterInfo.Server; len(server) > 0 { + icc.Host = server + } + if token := config.overrides.AuthInfo.Token; len(token) > 0 { + icc.BearerToken = token + } + if certificateAuthorityFile := config.overrides.ClusterInfo.CertificateAuthority; len(certificateAuthorityFile) > 0 { + icc.TLSClientConfig.CAFile = certificateAuthorityFile + } + } + + return icc, err } -func (inClusterClientConfig) Namespace() (string, bool, error) { +func (config *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 != "" { @@ -468,12 +495,12 @@ func (inClusterClientConfig) Namespace() (string, bool, error) { return "default", false, nil } -func (inClusterClientConfig) ConfigAccess() ConfigAccess { +func (config *inClusterClientConfig) ConfigAccess() ConfigAccess { return NewDefaultClientConfigLoadingRules() } // Possible returns true if loading an inside-kubernetes-cluster is possible. -func (inClusterClientConfig) Possible() bool { +func (config *inClusterClientConfig) Possible() bool { fi, err := os.Stat("/var/run/secrets/kubernetes.io/serviceaccount/token") return os.Getenv("KUBERNETES_SERVICE_HOST") != "" && os.Getenv("KUBERNETES_SERVICE_PORT") != "" && diff --git a/pkg/client/unversioned/clientcmd/client_config_test.go b/pkg/client/unversioned/clientcmd/client_config_test.go index 232ff480d80..e42a94378f8 100644 --- a/pkg/client/unversioned/clientcmd/client_config_test.go +++ b/pkg/client/unversioned/clientcmd/client_config_test.go @@ -24,6 +24,7 @@ import ( "testing" "github.com/imdario/mergo" + "k8s.io/kubernetes/pkg/client/restclient" clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api" ) @@ -372,6 +373,120 @@ func TestCreateMissingContext(t *testing.T) { } } +func TestInClusterClientConfigPrecedence(t *testing.T) { + tt := []struct { + overrides *ConfigOverrides + }{ + { + overrides: &ConfigOverrides{ + ClusterInfo: clientcmdapi.Cluster{ + Server: "https://host-from-overrides.com", + }, + }, + }, + { + overrides: &ConfigOverrides{ + AuthInfo: clientcmdapi.AuthInfo{ + Token: "https://host-from-overrides.com", + }, + }, + }, + { + overrides: &ConfigOverrides{ + ClusterInfo: clientcmdapi.Cluster{ + CertificateAuthority: "/path/to/ca-from-overrides.crt", + }, + }, + }, + { + overrides: &ConfigOverrides{ + ClusterInfo: clientcmdapi.Cluster{ + Server: "https://host-from-overrides.com", + }, + AuthInfo: clientcmdapi.AuthInfo{ + Token: "https://host-from-overrides.com", + }, + }, + }, + { + overrides: &ConfigOverrides{ + ClusterInfo: clientcmdapi.Cluster{ + Server: "https://host-from-overrides.com", + CertificateAuthority: "/path/to/ca-from-overrides.crt", + }, + }, + }, + { + overrides: &ConfigOverrides{ + ClusterInfo: clientcmdapi.Cluster{ + CertificateAuthority: "/path/to/ca-from-overrides.crt", + }, + AuthInfo: clientcmdapi.AuthInfo{ + Token: "https://host-from-overrides.com", + }, + }, + }, + { + overrides: &ConfigOverrides{ + ClusterInfo: clientcmdapi.Cluster{ + Server: "https://host-from-overrides.com", + CertificateAuthority: "/path/to/ca-from-overrides.crt", + }, + AuthInfo: clientcmdapi.AuthInfo{ + Token: "https://host-from-overrides.com", + }, + }, + }, + { + overrides: &ConfigOverrides{}, + }, + } + + for _, tc := range tt { + expectedServer := "https://host-from-cluster.com" + expectedToken := "token-from-cluster" + expectedCAFile := "/path/to/ca-from-cluster.crt" + + icc := &inClusterClientConfig{ + inClusterConfigProvider: func() (*restclient.Config, error) { + return &restclient.Config{ + Host: expectedServer, + BearerToken: expectedToken, + TLSClientConfig: restclient.TLSClientConfig{ + CAFile: expectedCAFile, + }, + }, nil + }, + overrides: tc.overrides, + } + + clientConfig, err := icc.ClientConfig() + if err != nil { + t.Fatalf("Unxpected error: %v", err) + } + + if overridenServer := tc.overrides.ClusterInfo.Server; len(overridenServer) > 0 { + expectedServer = overridenServer + } + if overridenToken := tc.overrides.AuthInfo.Token; len(overridenToken) > 0 { + expectedToken = overridenToken + } + if overridenCAFile := tc.overrides.ClusterInfo.CertificateAuthority; len(overridenCAFile) > 0 { + expectedCAFile = overridenCAFile + } + + if clientConfig.Host != expectedServer { + t.Errorf("Expected server %v, got %v", expectedServer, clientConfig.Host) + } + if clientConfig.BearerToken != expectedToken { + t.Errorf("Expected token %v, got %v", expectedToken, clientConfig.BearerToken) + } + if clientConfig.TLSClientConfig.CAFile != expectedCAFile { + t.Errorf("Expected Certificate Authority %v, got %v", expectedCAFile, clientConfig.TLSClientConfig.CAFile) + } + } +} + func matchBoolArg(expected, got bool, t *testing.T) { if expected != got { t.Errorf("Expected %v, got %v", expected, got) diff --git a/pkg/client/unversioned/clientcmd/merged_client_builder.go b/pkg/client/unversioned/clientcmd/merged_client_builder.go index 4c3645121b1..75cdd270455 100644 --- a/pkg/client/unversioned/clientcmd/merged_client_builder.go +++ b/pkg/client/unversioned/clientcmd/merged_client_builder.go @@ -51,12 +51,12 @@ type InClusterConfig interface { // NewNonInteractiveDeferredLoadingClientConfig creates a ConfigClientClientConfig using the passed context name func NewNonInteractiveDeferredLoadingClientConfig(loader ClientConfigLoader, overrides *ConfigOverrides) ClientConfig { - return &DeferredLoadingClientConfig{loader: loader, overrides: overrides, icc: inClusterClientConfig{}} + return &DeferredLoadingClientConfig{loader: loader, overrides: overrides, icc: &inClusterClientConfig{overrides: overrides}} } // 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, icc: inClusterClientConfig{}, fallbackReader: fallbackReader} + return &DeferredLoadingClientConfig{loader: loader, overrides: overrides, icc: &inClusterClientConfig{overrides: overrides}, fallbackReader: fallbackReader} } func (config *DeferredLoadingClientConfig) createClientConfig() (ClientConfig, error) { diff --git a/pkg/client/unversioned/clientcmd/overrides.go b/pkg/client/unversioned/clientcmd/overrides.go index 626cdeaae42..38de6a6cdd9 100644 --- a/pkg/client/unversioned/clientcmd/overrides.go +++ b/pkg/client/unversioned/clientcmd/overrides.go @@ -126,6 +126,18 @@ const ( FlagTimeout = "request-timeout" ) +// RecommendedConfigOverrideFlags is a convenience method to return recommended flag names prefixed with a string of your choosing +func RecommendedConfigOverrideFlags(prefix string) ConfigOverrideFlags { + return ConfigOverrideFlags{ + AuthOverrideFlags: RecommendedAuthOverrideFlags(prefix), + ClusterOverrideFlags: RecommendedClusterOverrideFlags(prefix), + ContextOverrideFlags: RecommendedContextOverrideFlags(prefix), + + CurrentContext: FlagInfo{prefix + FlagContext, "", "", "The name of the kubeconfig context to use"}, + Timeout: FlagInfo{prefix + FlagTimeout, "", "0", "The length of time to wait before giving up on a single server request. Non-zero values should contain a corresponding time unit (e.g. 1s, 2m, 3h). A value of zero means don't timeout requests."}, + } +} + // RecommendedAuthOverrideFlags is a convenience method to return recommended flag names prefixed with a string of your choosing func RecommendedAuthOverrideFlags(prefix string) AuthOverrideFlags { return AuthOverrideFlags{ @@ -148,18 +160,6 @@ func RecommendedClusterOverrideFlags(prefix string) ClusterOverrideFlags { } } -// RecommendedConfigOverrideFlags is a convenience method to return recommended flag names prefixed with a string of your choosing -func RecommendedConfigOverrideFlags(prefix string) ConfigOverrideFlags { - return ConfigOverrideFlags{ - AuthOverrideFlags: RecommendedAuthOverrideFlags(prefix), - ClusterOverrideFlags: RecommendedClusterOverrideFlags(prefix), - ContextOverrideFlags: RecommendedContextOverrideFlags(prefix), - - CurrentContext: FlagInfo{prefix + FlagContext, "", "", "The name of the kubeconfig context to use"}, - Timeout: FlagInfo{prefix + FlagTimeout, "", "0", "The length of time to wait before giving up on a single server request. Non-zero values should contain a corresponding time unit (e.g. 1s, 2m, 3h). A value of zero means don't timeout requests."}, - } -} - // RecommendedContextOverrideFlags is a convenience method to return recommended flag names prefixed with a string of your choosing func RecommendedContextOverrideFlags(prefix string) ContextOverrideFlags { return ContextOverrideFlags{ @@ -169,6 +169,15 @@ func RecommendedContextOverrideFlags(prefix string) ContextOverrideFlags { } } +// BindOverrideFlags is a convenience method to bind the specified flags to their associated variables +func BindOverrideFlags(overrides *ConfigOverrides, flags *pflag.FlagSet, flagNames ConfigOverrideFlags) { + BindAuthInfoFlags(&overrides.AuthInfo, flags, flagNames.AuthOverrideFlags) + BindClusterFlags(&overrides.ClusterInfo, flags, flagNames.ClusterOverrideFlags) + BindContextFlags(&overrides.Context, flags, flagNames.ContextOverrideFlags) + flagNames.CurrentContext.BindStringFlag(flags, &overrides.CurrentContext) + flagNames.Timeout.BindStringFlag(flags, &overrides.Timeout) +} + // BindAuthInfoFlags is a convenience method to bind the specified flags to their associated variables func BindAuthInfoFlags(authInfo *clientcmdapi.AuthInfo, flags *pflag.FlagSet, flagNames AuthOverrideFlags) { flagNames.ClientCertificate.BindStringFlag(flags, &authInfo.ClientCertificate) @@ -189,15 +198,6 @@ func BindClusterFlags(clusterInfo *clientcmdapi.Cluster, flags *pflag.FlagSet, f flagNames.InsecureSkipTLSVerify.BindBoolFlag(flags, &clusterInfo.InsecureSkipTLSVerify) } -// BindOverrideFlags is a convenience method to bind the specified flags to their associated variables -func BindOverrideFlags(overrides *ConfigOverrides, flags *pflag.FlagSet, flagNames ConfigOverrideFlags) { - BindAuthInfoFlags(&overrides.AuthInfo, flags, flagNames.AuthOverrideFlags) - BindClusterFlags(&overrides.ClusterInfo, flags, flagNames.ClusterOverrideFlags) - BindContextFlags(&overrides.Context, flags, flagNames.ContextOverrideFlags) - flagNames.CurrentContext.BindStringFlag(flags, &overrides.CurrentContext) - flagNames.Timeout.BindStringFlag(flags, &overrides.Timeout) -} - // BindFlags is a convenience method to bind the specified flags to their associated variables func BindContextFlags(contextInfo *clientcmdapi.Context, flags *pflag.FlagSet, flagNames ContextOverrideFlags) { flagNames.ClusterName.BindStringFlag(flags, &contextInfo.Cluster) diff --git a/test/e2e/kubectl.go b/test/e2e/kubectl.go index 64b9d19fab7..0a6bccea62a 100644 --- a/test/e2e/kubectl.go +++ b/test/e2e/kubectl.go @@ -564,6 +564,48 @@ var _ = framework.KubeDescribe("Kubectl client", func() { framework.Failf("Container port output missing expected value. Wanted:'%s', got: %s", nginxDefaultOutput, body) } }) + + It("should handle in-cluster config", func() { + By("overriding icc with values provided by flags") + kubectlPath := framework.TestContext.KubectlPath + + inClusterHost := strings.TrimSpace(framework.RunHostCmdOrDie(ns, simplePodName, "printenv KUBERNETES_SERVICE_HOST")) + inClusterPort := strings.TrimSpace(framework.RunHostCmdOrDie(ns, simplePodName, "printenv KUBERNETES_SERVICE_PORT")) + framework.RunKubectlOrDie("cp", kubectlPath, ns+"/"+simplePodName+":/") + + By("getting pods with in-cluster configs") + execOutput := framework.RunHostCmdOrDie(ns, simplePodName, "/kubectl get pods") + if matched, err := regexp.MatchString("nginx +1/1 +Running", execOutput); err != nil || !matched { + framework.Failf("Unexpected kubectl exec output: ", execOutput) + } + + By("trying to use kubectl with invalid token") + _, err := framework.RunHostCmd(ns, simplePodName, "/kubectl get pods --token=invalid --v=7 2>&1") + framework.Logf("got err %v", err) + Expect(err).To(HaveOccurred()) + Expect(err).To(ContainSubstring("User \"system:anonymous\" cannot list pods in the namespace")) + Expect(err).To(ContainSubstring("Using in-cluster namespace")) + Expect(err).To(ContainSubstring("Using in-cluster configuration")) + Expect(err).To(ContainSubstring("Authorization: Bearer invalid")) + Expect(err).To(ContainSubstring("Response Status: 403 Forbidden")) + + By("trying to use kubectl with invalid server") + _, err = framework.RunHostCmd(ns, simplePodName, "/kubectl get pods --server=invalid --v=6 2>&1") + framework.Logf("got err %v", err) + Expect(err).To(HaveOccurred()) + Expect(err).To(ContainSubstring("Unable to connect to the server")) + Expect(err).To(ContainSubstring("GET http://invalid/api")) + + By("trying to use kubectl with invalid namespace") + output, _ := framework.RunHostCmd(ns, simplePodName, "/kubectl get pods --namespace=invalid --v=6 2>&1") + Expect(output).To(ContainSubstring("No resources found")) + Expect(output).ToNot(ContainSubstring("Using in-cluster namespace")) + Expect(output).To(ContainSubstring("Using in-cluster configuration")) + if matched, _ := regexp.MatchString(fmt.Sprintf("GET http[s]?://%s:%s/api/v1/namespaces/invalid/pods", inClusterHost, inClusterPort), output); !matched { + framework.Failf("Unexpected kubectl exec output: ", output) + } + + }) }) framework.KubeDescribe("Kubectl api-versions", func() {