Fix kubeconfig default path (#613)

XenitAB · Jan 25, 2023 · b558f47 · b558f47
1 parent b4e78b7
commit b558f47
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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,23 +109,24 @@ 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
 		}
 
 		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)
 
 	}
 }