From b558f4703cd1ab4a8b6e5b47b2aa063e80a33e18 Mon Sep 17 00:00:00 2001 From: Simon Gottschlag Date: Wed, 25 Jan 2023 12:53:19 +0100 Subject: [PATCH] Fix kubeconfig default path (#613) --- cmd/kubectl-azad-proxy/config.go | 35 +++++++++++++++++++-- cmd/kubectl-azad-proxy/config_test.go | 41 ++++++++++++++++++++++++- cmd/kubectl-azad-proxy/generate_test.go | 40 +++++++++++++++--------- 3 files changed, 97 insertions(+), 19 deletions(-) diff --git a/cmd/kubectl-azad-proxy/config.go b/cmd/kubectl-azad-proxy/config.go index 9d24e32..39e0f6f 100644 --- a/cmd/kubectl-azad-proxy/config.go +++ b/cmd/kubectl-azad-proxy/config.go @@ -3,6 +3,7 @@ package main import ( "fmt" "os" + "path/filepath" "github.com/alexflint/go-arg" ) @@ -18,7 +19,7 @@ type generateConfig struct { ClusterName string `arg:"--cluster-name,env:CLUSTER_NAME,required" help:"The name of the Kubernetes cluster / context"` ProxyURL string `arg:"--proxy-url,env:PROXY_URL,required" help:"The proxy url for azad-kube-proxy"` Resource string `arg:"--resource,env:RESOURCE,required" help:"The Azure AD App URI / resource"` - KubeConfig string `arg:"--kubeconfig,env:KUBECONFIG" help:"The path of the Kubernetes Config"` // FIXME: Default to fmt.Sprintf("%s/.kube/config", osUserHomeDir) + KubeConfig string `arg:"--kubeconfig,env:KUBECONFIG" help:"The path of the Kubernetes Config"` TokenCacheDir string `arg:"--token-cache-dir,env:TOKEN_CACHE_DIR" help:"The directory to where the tokens are cached, defaults to the same as KUBECONFIG"` Overwrite bool `arg:"--overwrite,env:OVERWRITE_KUBECONFIG" default:"false" help:"If the cluster already exists in the kubeconfig, should it be overwritten?"` TLSInsecureSkipVerify bool `arg:"--tls-insecure-skip-verify,env:TLS_INSECURE_SKIP_VERIFY" default:"false" help:"Should the proxy url server certificate validation be skipped?"` @@ -27,7 +28,7 @@ type generateConfig struct { type loginConfig struct { ClusterName string `arg:"--cluster-name,env:CLUSTER_NAME,required" help:"The name of the Kubernetes cluster / context"` Resource string `arg:"--resource,env:RESOURCE,required" help:"The Azure AD App URI / resource"` - KubeConfig string `arg:"--kubeconfig,env:KUBECONFIG" help:"The path of the Kubernetes Config"` // FIXME: Default to fmt.Sprintf("%s/.kube/config", osUserHomeDir) + KubeConfig string `arg:"--kubeconfig,env:KUBECONFIG" help:"The path of the Kubernetes Config"` TokenCacheDir string `arg:"--token-cache-dir,env:TOKEN_CACHE_DIR" help:"The directory to where the tokens are cached, defaults to the same as KUBECONFIG"` } @@ -39,7 +40,7 @@ type menuConfig struct { ClusterName string `arg:"--cluster-name,env:CLUSTER_NAME" help:"The name of the Kubernetes cluster / context"` ProxyURL string `arg:"--proxy-url,env:PROXY_URL" help:"The proxy url for azad-kube-proxy"` Resource string `arg:"--resource,env:RESOURCE" help:"The Azure AD App URI / resource"` - KubeConfig string `arg:"--kubeconfig,env:KUBECONFIG" help:"The path of the Kubernetes Config"` // FIXME: Default to fmt.Sprintf("%s/.kube/config", osUserHomeDir) + KubeConfig string `arg:"--kubeconfig,env:KUBECONFIG" help:"The path of the Kubernetes Config"` TokenCacheDir string `arg:"--token-cache-dir,env:TOKEN_CACHE_DIR" help:"The directory to where the tokens are cached, defaults to the same as KUBECONFIG"` Overwrite bool `arg:"--overwrite,env:OVERWRITE_KUBECONFIG" default:"false" help:"If the cluster already exists in the kubeconfig, should it be overwritten?"` TLSInsecureSkipVerify bool `arg:"--tls-insecure-skip-verify,env:TLS_INSECURE_SKIP_VERIFY" default:"false" help:"Should the proxy url server certificate validation be skipped?"` @@ -66,6 +67,29 @@ type config struct { authConfig authConfig } +func (cfg *config) setKubeConfigDefaults() error { + userHomeDir, err := os.UserHomeDir() + if err != nil { + return err + } + + defaultKubeConfig := filepath.Clean(fmt.Sprintf("%s/.kube/config", userHomeDir)) + + if cfg.Generate != nil && cfg.Generate.KubeConfig == "" { + cfg.Generate.KubeConfig = defaultKubeConfig + } + + if cfg.Login != nil && cfg.Login.KubeConfig == "" { + cfg.Login.KubeConfig = defaultKubeConfig + } + + if cfg.Menu != nil && cfg.Menu.KubeConfig == "" { + cfg.Menu.KubeConfig = defaultKubeConfig + } + + return nil +} + func (config) Version() string { return fmt.Sprintf("version=%s revision=%s created=%s\n", Version, Revision, Created) } @@ -96,5 +120,10 @@ func newConfig(args []string) (config, error) { excludeMSIAuth: cfg.ExcludeMSIAuth, } + err = cfg.setKubeConfigDefaults() + if err != nil { + return config{}, err + } + return cfg, err } diff --git a/cmd/kubectl-azad-proxy/config_test.go b/cmd/kubectl-azad-proxy/config_test.go index c4b62e5..6082207 100644 --- a/cmd/kubectl-azad-proxy/config_test.go +++ b/cmd/kubectl-azad-proxy/config_test.go @@ -1,12 +1,40 @@ package main import ( + "fmt" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/require" ) func TestNewConfig(t *testing.T) { + envVarsToClear := []string{ + "AZURE_CLIENT_ID", + "AZURE_CLIENT_SECRET", + "AZURE_TENANT_ID", + "CLUSTER_NAME", + "EXCLUDE_AZURE_CLI_AUTH", + "EXCLUDE_ENVIRONMENT_AUTH", + "EXCLUDE_MSI_AUTH", + "KUBECONFIG", + "OUTPUT", + "OVERWRITE_KUBECONFIG", + "PROXY_URL", + "RESOURCE", + "TLS_INSECURE_SKIP_VERIFY", + "TOKEN_CACHE_DIR", + } + + for _, envVar := range envVarsToClear { + restore := testTempUnsetEnv(t, envVar) + defer restore() + } + userHomeDir, err := os.UserHomeDir() + require.NoError(t, err) + defaultKubeConfig := filepath.Clean(fmt.Sprintf("%s/.kube/config", userHomeDir)) + t.Run("no subcommand", func(t *testing.T) { args := []string{ "/foo/bar/bin", @@ -90,6 +118,7 @@ func TestNewConfig(t *testing.T) { ClusterName: "ze-cluster", ProxyURL: "ze-proxy-url", Resource: "ze-resource", + KubeConfig: defaultKubeConfig, } require.Equal(t, expectedGenerateConfig, *cfg.Generate) }) @@ -125,6 +154,7 @@ func TestNewConfig(t *testing.T) { expectedLoginConfig := loginConfig{ ClusterName: "ze-cluster", Resource: "ze-resource", + KubeConfig: defaultKubeConfig, } require.Equal(t, expectedLoginConfig, *cfg.Login) }) @@ -138,8 +168,17 @@ func TestNewConfig(t *testing.T) { require.NoError(t, err) require.NotNil(t, cfg.Menu) expectedMenuConfig := menuConfig{ - Output: "TABLE", + Output: "TABLE", + KubeConfig: defaultKubeConfig, } require.Equal(t, expectedMenuConfig, *cfg.Menu) }) } + +func testTempUnsetEnv(t *testing.T, key string) func() { + t.Helper() + + oldEnv := os.Getenv(key) + os.Unsetenv(key) + return func() { os.Setenv(key, oldEnv) } +} diff --git a/cmd/kubectl-azad-proxy/generate_test.go b/cmd/kubectl-azad-proxy/generate_test.go index 462318a..0ac130f 100644 --- a/cmd/kubectl-azad-proxy/generate_test.go +++ b/cmd/kubectl-azad-proxy/generate_test.go @@ -53,6 +53,11 @@ func TestRunGenerate(t *testing.T) { func TestGenerate(t *testing.T) { ctx := logr.NewContext(context.Background(), logr.Discard()) + ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, "test response") + })) + defer ts.Close() + tmpDir, err := os.MkdirTemp("", "") require.NoError(t, err) defer os.RemoveAll(tmpDir) @@ -62,12 +67,12 @@ func TestGenerate(t *testing.T) { client := &GenerateClient{ clusterName: "test", - proxyURL: testGetURL(t, "https://www.google.com"), + proxyURL: testGetURL(t, ts.URL), resource: "https://fake", kubeConfig: kubeConfigFile, tokenCacheDir: tokenCacheDir, overwrite: false, - insecureSkipVerify: false, + insecureSkipVerify: true, defaultAzureCredentialOptions: defaultAzureCredentialOptions{ excludeAzureCLICredential: false, excludeEnvironmentCredential: false, @@ -76,21 +81,25 @@ func TestGenerate(t *testing.T) { } cases := []struct { - GenerateClient *GenerateClient - GenerateClientFunc func(oldCfg *GenerateClient) *GenerateClient + testDescription string + generateClient *GenerateClient + generateClientFunc func(oldCfg *GenerateClient) *GenerateClient expectedErrContains string }{ { - GenerateClient: client, + testDescription: "plain", + generateClient: client, expectedErrContains: "", }, { - GenerateClient: client, + testDescription: "config error", + generateClient: client, expectedErrContains: "Overwrite config error:", }, { - GenerateClient: client, - GenerateClientFunc: func(oldClient *GenerateClient) *GenerateClient { + testDescription: "ca error", + generateClient: client, + generateClientFunc: func(oldClient *GenerateClient) *GenerateClient { client := oldClient client.proxyURL = testGetURL(t, "https://localhost:12345") client.overwrite = true @@ -100,12 +109,13 @@ func TestGenerate(t *testing.T) { }, } - for _, c := range cases { - if c.GenerateClientFunc != nil { - c.GenerateClient = c.GenerateClientFunc(c.GenerateClient) + for i, c := range cases { + t.Logf("Test #%d: %s", i, c.testDescription) + if c.generateClientFunc != nil { + c.generateClient = c.generateClientFunc(c.generateClient) } - err := c.GenerateClient.Generate(ctx) + err := c.generateClient.Generate(ctx) if c.expectedErrContains != "" { require.ErrorContains(t, err, c.expectedErrContains) continue @@ -113,10 +123,10 @@ func TestGenerate(t *testing.T) { require.NoError(t, err) - kubeCfg, err := k8sclientcmd.LoadFromFile(c.GenerateClient.kubeConfig) + kubeCfg, err := k8sclientcmd.LoadFromFile(c.generateClient.kubeConfig) require.NoError(t, err) - require.Equal(t, fmt.Sprintf("%s://%s", c.GenerateClient.proxyURL.Scheme, c.GenerateClient.proxyURL.Host), kubeCfg.Clusters[c.GenerateClient.clusterName].Server) - require.NotEmpty(t, kubeCfg.Clusters[c.GenerateClient.clusterName].CertificateAuthorityData) + require.Equal(t, fmt.Sprintf("%s://%s", c.generateClient.proxyURL.Scheme, c.generateClient.proxyURL.Host), kubeCfg.Clusters[c.generateClient.clusterName].Server) + require.NotEmpty(t, kubeCfg.Clusters[c.generateClient.clusterName].CertificateAuthorityData) } }