Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛Support KUBECONFIG env var file paths #3047

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 26 additions & 11 deletions cmd/clusterctl/client/cluster/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,19 @@ var (
)

type proxy struct {
kubeconfig Kubeconfig
timeout time.Duration
kubeconfig Kubeconfig
timeout time.Duration
configLoadingRules *clientcmd.ClientConfigLoadingRules
}

var _ Proxy = &proxy{}

// CurrentNamespace returns the namespace for the specified context or the
// first valid context as determined by the default config loading rules.
func (k *proxy) CurrentNamespace() (string, error) {
config, err := clientcmd.LoadFromFile(k.kubeconfig.Path)
config, err := k.configLoadingRules.Load()
if err != nil {
return "", errors.Wrapf(err, "failed to load Kubeconfig file from %q", k.kubeconfig.Path)
return "", errors.Wrap(err, "failed to load Kubeconfig")
}

context := config.CurrentContext
Expand All @@ -59,7 +62,10 @@ func (k *proxy) CurrentNamespace() (string, error) {

v, ok := config.Contexts[context]
if !ok {
return "", errors.Errorf("failed to get context %q from %q", context, k.kubeconfig.Path)
if k.kubeconfig.Path != "" {
return "", errors.Errorf("failed to get context %q from %q", context, k.configLoadingRules.GetExplicitFile())
}
return "", errors.Errorf("failed to get context %q from %q", context, k.configLoadingRules.GetLoadingPrecedence())
}

if v.Namespace != "" {
Expand Down Expand Up @@ -93,10 +99,11 @@ func (k *proxy) ValidateKubernetesVersion() error {
return nil
}

// GetConfig returns the config for a kubernetes client.
func (k *proxy) GetConfig() (*rest.Config, error) {
config, err := clientcmd.LoadFromFile(k.kubeconfig.Path)
config, err := k.configLoadingRules.Load()
if err != nil {
return nil, errors.Wrapf(err, "failed to load Kubeconfig file from %q", k.kubeconfig.Path)
return nil, errors.Wrap(err, "failed to load Kubeconfig")
}

configOverrides := &clientcmd.ConfigOverrides{
Expand Down Expand Up @@ -213,14 +220,22 @@ func InjectProxyTimeout(t time.Duration) ProxyOption {
}
}

func InjectKubeconfigPaths(paths []string) ProxyOption {
return func(p *proxy) {
p.configLoadingRules.Precedence = paths
}
}

func newProxy(kubeconfig Kubeconfig, opts ...ProxyOption) Proxy {
// If a kubeconfig file isn't provided, find one in the standard locations.
if kubeconfig.Path == "" {
kubeconfig.Path = clientcmd.NewDefaultClientConfigLoadingRules().GetDefaultFilename()
rules := clientcmd.NewDefaultClientConfigLoadingRules()
if kubeconfig.Path != "" {
rules.ExplicitPath = kubeconfig.Path
}
p := &proxy{
kubeconfig: kubeconfig,
timeout: 30 * time.Second,
kubeconfig: kubeconfig,
timeout: 30 * time.Second,
configLoadingRules: rules,
}

for _, o := range opts {
Expand Down
63 changes: 63 additions & 0 deletions cmd/clusterctl/client/cluster/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,69 @@ func TestProxyGetConfig(t *testing.T) {
})
}

// These tests are emulating the files passed in via KUBECONFIG env var by
// injecting the file paths into the ClientConfigLoadingRules.Precedence
// chain.
func TestKUBECONFIGEnvVar(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote the tests this way because I preferred to not override the KUBECONFIG env var. This method of injecting the precedence path is similar to what is done in kubernetes/client-go.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit.

Suggested change
func TestKUBECONFIGEnvVar(t *testing.T) {
func TestKubeconfigEnvVar(t *testing.T) {

t.Run("CurrentNamespace", func(t *testing.T) {
// KUBECONFIG can specify multiple config files. We should be able to
// get the correct namespace for the context by parsing through all
// kubeconfig files
var (
context = "workload"
kubeconfigContents = kubeconfig("does-not-exist", "some-ns")
)

g := NewWithT(t)
dir, err := ioutil.TempDir("", "clusterctl")
g.Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(dir)
configFile := filepath.Join(dir, ".test-kubeconfig.yaml")
g.Expect(ioutil.WriteFile(configFile, []byte(kubeconfigContents), 0640)).To(Succeed())

proxy := newProxy(
// dont't give an explicit path but rather define the file in the
// configLoadingRules precedence chain.
Kubeconfig{Path: "", Context: context},
InjectKubeconfigPaths([]string{configFile}),
)
actualNS, err := proxy.CurrentNamespace()
g.Expect(err).ToNot(HaveOccurred())
g.Expect(actualNS).To(Equal("some-ns"))
})

t.Run("GetConfig", func(t *testing.T) {
// KUBECONFIG can specify multiple config files. We should be able to
// get the valid cluster context by parsing all the kubeconfig files.
var (
context = "workload"
// TODO: If we change current context to "do-not-exist", we get an
// error. See https://github.com/kubernetes/client-go/issues/797
kubeconfigContents = kubeconfig("management", "default")
expectedHost = "https://kind-server:38790"
)
g := NewWithT(t)
dir, err := ioutil.TempDir("", "clusterctl")
g.Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(dir)
configFile := filepath.Join(dir, ".test-kubeconfig.yaml")
g.Expect(ioutil.WriteFile(configFile, []byte(kubeconfigContents), 0640)).To(Succeed())

proxy := newProxy(
// dont't give an explicit path but rather define the file in the
// configLoadingRules precedence chain.
Kubeconfig{Path: "", Context: context},
InjectKubeconfigPaths([]string{configFile}),
)
conf, err := proxy.GetConfig()
g.Expect(err).ToNot(HaveOccurred())
g.Expect(conf).ToNot(BeNil())
// asserting on the host of the cluster associated with the
// context
g.Expect(conf.Host).To(Equal(expectedHost))
})
}

func TestProxyCurrentNamespace(t *testing.T) {
tests := []struct {
name string
Expand Down