From 530b04ae3fddd434e64d61a40c155f23c8e1f193 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 16 Jun 2020 11:44:45 +0200 Subject: [PATCH] cli/cmd: refactor getKubeconfig() to return kubeconfig file content Rather than kubeconfig file path. This will allow changing this function to pull kubeconfig content from Terraform output rather than from assets directory, which will resolve #608. Signed-off-by: Mateusz Gozdek --- cli/cmd/cluster-apply.go | 13 +++--- cli/cmd/component-apply.go | 8 +--- cli/cmd/component-delete.go | 8 +--- cli/cmd/health.go | 8 +--- cli/cmd/utils.go | 31 +++++++------- cli/cmd/utils_internal_test.go | 76 ++++++++++++++++++++++++++++------ 6 files changed, 87 insertions(+), 57 deletions(-) diff --git a/cli/cmd/cluster-apply.go b/cli/cmd/cluster-apply.go index 2cb2065e5..287971086 100644 --- a/cli/cmd/cluster-apply.go +++ b/cli/cmd/cluster-apply.go @@ -16,7 +16,6 @@ package cmd import ( "fmt" - "io/ioutil" "github.com/pkg/errors" log "github.com/sirupsen/logrus" @@ -80,14 +79,12 @@ func runClusterApply(cmd *cobra.Command, args []string) { fmt.Printf("\nYour configurations are stored in %s\n", assetDir) - kubeconfigPath := assetsKubeconfig(assetDir) - - kubeconfigContent, err := ioutil.ReadFile(kubeconfigPath) // #nosec G304 + kubeconfig, err := getKubeconfig() if err != nil { - ctxLogger.Fatalf("Failed to read kubeconfig file: %q: %v", kubeconfigPath, err) + ctxLogger.Fatalf("Failed to get kubeconfig: %v", err) } - if err := verifyCluster(kubeconfigContent, p.Meta().ExpectedNodes); err != nil { + if err := verifyCluster(kubeconfig, p.Meta().ExpectedNodes); err != nil { ctxLogger.Fatalf("Verify cluster: %v", err) } @@ -96,7 +93,7 @@ func runClusterApply(cmd *cobra.Command, args []string) { fmt.Printf("\nEnsuring that cluster controlplane is up to date.\n") cu := controlplaneUpdater{ - kubeconfig: kubeconfigContent, + kubeconfig: kubeconfig, assetDir: assetDir, ctxLogger: *ctxLogger, ex: *ex, @@ -125,7 +122,7 @@ func runClusterApply(cmd *cobra.Command, args []string) { ctxLogger.Println("Applying component configuration") if len(componentsToApply) > 0 { - if err := applyComponents(lokoConfig, kubeconfigContent, componentsToApply...); err != nil { + if err := applyComponents(lokoConfig, kubeconfig, componentsToApply...); err != nil { ctxLogger.Fatalf("Applying component configuration failed: %v", err) } } diff --git a/cli/cmd/component-apply.go b/cli/cmd/component-apply.go index fe28bf154..fb80a4f39 100644 --- a/cli/cmd/component-apply.go +++ b/cli/cmd/component-apply.go @@ -16,7 +16,6 @@ package cmd import ( "fmt" - "io/ioutil" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -64,12 +63,7 @@ func runApply(cmd *cobra.Command, args []string) { contextLogger.Fatalf("Error in finding kubeconfig file: %s", err) } - kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 - if err != nil { - contextLogger.Fatalf("Failed to read kubeconfig file: %q: %v", kubeconfig, err) - } - - if err := applyComponents(lokoConfig, kubeconfigContent, componentsToApply...); err != nil { + if err := applyComponents(lokoConfig, kubeconfig, componentsToApply...); err != nil { contextLogger.Fatal(err) } } diff --git a/cli/cmd/component-delete.go b/cli/cmd/component-delete.go index b1587193e..ab1fe0ebb 100644 --- a/cli/cmd/component-delete.go +++ b/cli/cmd/component-delete.go @@ -16,7 +16,6 @@ package cmd import ( "fmt" - "io/ioutil" "strings" log "github.com/sirupsen/logrus" @@ -92,12 +91,7 @@ func runDelete(cmd *cobra.Command, args []string) { contextLogger.Fatalf("Error in finding kubeconfig file: %s", err) } - kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 - if err != nil { - contextLogger.Fatalf("Failed to read kubeconfig file: %q: %v", kubeconfig, err) - } - - if err := deleteComponents(kubeconfigContent, componentsObjects...); err != nil { + if err := deleteComponents(kubeconfig, componentsObjects...); err != nil { contextLogger.Fatal(err) } } diff --git a/cli/cmd/health.go b/cli/cmd/health.go index a2c71ce72..1779c07d0 100644 --- a/cli/cmd/health.go +++ b/cli/cmd/health.go @@ -16,7 +16,6 @@ package cmd import ( "fmt" - "io/ioutil" "os" "text/tabwriter" @@ -48,12 +47,7 @@ func runHealth(cmd *cobra.Command, args []string) { contextLogger.Fatalf("Error in finding kubeconfig file: %s", err) } - kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 - if err != nil { - contextLogger.Fatalf("Failed to read kubeconfig file: %v", err) - } - - cs, err := k8sutil.NewClientset(kubeconfigContent) + cs, err := k8sutil.NewClientset(kubeconfig) if err != nil { contextLogger.Fatalf("Error in creating setting up Kubernetes client: %q", err) } diff --git a/cli/cmd/utils.go b/cli/cmd/utils.go index aa9a3655e..1cd8b00e1 100644 --- a/cli/cmd/utils.go +++ b/cli/cmd/utils.go @@ -16,6 +16,7 @@ package cmd import ( "fmt" + "io/ioutil" "os" "path/filepath" @@ -92,17 +93,20 @@ func getAssetDir() (string, error) { return cfg.Meta().AssetDir, nil } -// expandKubeconfigPath tries to expand ~ in the given kubeconfig path. -// However, if that fails, it just returns original path as the best effort. -func expandKubeconfigPath(path string) string { +func getKubeconfig() ([]byte, error) { + path, err := getKubeconfigPath() + if err != nil { + return nil, fmt.Errorf("failed getting kubeconfig path: %w", err) + } + if expandedPath, err := homedir.Expand(path); err == nil { - return expandedPath + path = expandedPath } // homedir.Expand is too restrictive for the ~ prefix, // i.e., it errors on "~somepath" which is a valid path, - // so just return the original path. - return path + // so just read from the original path. + return ioutil.ReadFile(path) // #nosec G304 } // getKubeconfig finds the kubeconfig to be used. The precedence is the following: @@ -112,7 +116,7 @@ func expandKubeconfigPath(path string) string { // - Asset directory from cluster configuration. // - KUBECONFIG environment variable. // - ~/.kube/config path, which is the default for kubectl. -func getKubeconfig() (string, error) { +func getKubeconfigPath() (string, error) { assetKubeconfig, err := assetsKubeconfigPath() if err != nil { return "", fmt.Errorf("reading kubeconfig path from configuration failed: %w", err) @@ -125,18 +129,13 @@ func getKubeconfig() (string, error) { defaultKubeconfigPath, } - return expandKubeconfigPath(pickString(paths...)), nil -} - -// pickString returns first non-empty string. -func pickString(options ...string) string { - for _, option := range options { - if option != "" { - return option + for _, path := range paths { + if path != "" { + return path, nil } } - return "" + return "", nil } // assetsKubeconfigPath reads the lokocfg configuration and returns diff --git a/cli/cmd/utils_internal_test.go b/cli/cmd/utils_internal_test.go index 9982e7caa..541f67125 100644 --- a/cli/cmd/utils_internal_test.go +++ b/cli/cmd/utils_internal_test.go @@ -18,6 +18,7 @@ import ( "io/ioutil" "os" "path/filepath" + "reflect" "testing" "github.com/spf13/viper" @@ -29,6 +30,10 @@ type kubeconfigSources struct { configFile string } +const ( + tmpPattern = "lokoctl-tests-" +) + func prepareKubeconfigSource(t *testing.T, k *kubeconfigSources) { // Ensure viper flag is NOT empty. viper.Set(kubeconfigFlag, k.flag) @@ -48,7 +53,7 @@ func prepareKubeconfigSource(t *testing.T, k *kubeconfigSources) { } // Ensure there is no lokocfg configuration in working directory. - tmpDir, err := ioutil.TempDir("", "lokoctl-tests-") + tmpDir, err := ioutil.TempDir("", tmpPattern) if err != nil { t.Fatalf("creating tmp dir: %v", err) } @@ -71,7 +76,54 @@ func prepareKubeconfigSource(t *testing.T, k *kubeconfigSources) { } } -func TestGetKubeconfigFlag(t *testing.T) { +func TestGetKubeconfigBadConfig(t *testing.T) { + k := &kubeconfigSources{ + configFile: `cluster "packet" { + asset_dir = "/foo" +}`, + } + + prepareKubeconfigSource(t, k) + + kubeconfig, err := getKubeconfig() + if err == nil { + t.Errorf("getting kubeconfig with bad configuration should fail") + } + + if kubeconfig != nil { + t.Fatalf("if getting kubeconfig fails, empty content should be returned") + } +} + +func TestGetKubeconfig(t *testing.T) { + expectedContent := []byte("foo") + + f, err := ioutil.TempFile("", tmpPattern) + if err != nil { + t.Fatalf("creating temp file should succeed, got: %v", err) + } + + if err := ioutil.WriteFile(f.Name(), expectedContent, 0600); err != nil { + t.Fatalf("writing temp file %q should succeed, got: %v", f.Name(), err) + } + + k := &kubeconfigSources{ + env: f.Name(), + } + + prepareKubeconfigSource(t, k) + + kubeconfig, err := getKubeconfig() + if err != nil { + t.Fatalf("getting kubeconfig: %v", err) + } + + if !reflect.DeepEqual(kubeconfig, expectedContent) { + t.Fatalf("expected %q, got %q", expectedContent, kubeconfig) + } +} + +func TestGetKubeconfigPathFlag(t *testing.T) { expectedPath := "/foo" k := &kubeconfigSources{ @@ -99,7 +151,7 @@ func TestGetKubeconfigFlag(t *testing.T) { prepareKubeconfigSource(t, k) - kubeconfig, err := getKubeconfig() + kubeconfig, err := getKubeconfigPath() if err != nil { t.Fatalf("getting kubeconfig: %v", err) } @@ -109,7 +161,7 @@ func TestGetKubeconfigFlag(t *testing.T) { } } -func TestGetKubeconfigConfigFile(t *testing.T) { +func TestGetKubeconfigPathConfigFile(t *testing.T) { expectedPath := assetsKubeconfig("/foo") k := &kubeconfigSources{ @@ -136,7 +188,7 @@ func TestGetKubeconfigConfigFile(t *testing.T) { prepareKubeconfigSource(t, k) - kubeconfig, err := getKubeconfig() + kubeconfig, err := getKubeconfigPath() if err != nil { t.Fatalf("getting kubeconfig: %v", err) } @@ -146,7 +198,7 @@ func TestGetKubeconfigConfigFile(t *testing.T) { } } -func TestGetKubeconfigBadConfigFile(t *testing.T) { +func TestGetKubeconfigPathBadConfigFile(t *testing.T) { expectedPath := "" k := &kubeconfigSources{ @@ -157,7 +209,7 @@ func TestGetKubeconfigBadConfigFile(t *testing.T) { prepareKubeconfigSource(t, k) - kubeconfig, err := getKubeconfig() + kubeconfig, err := getKubeconfigPath() if err == nil { t.Errorf("getting kubeconfig with bad configuration should fail") } @@ -167,7 +219,7 @@ func TestGetKubeconfigBadConfigFile(t *testing.T) { } } -func TestGetKubeconfigEnvVariable(t *testing.T) { +func TestGetKubeconfigPathEnvVariable(t *testing.T) { expectedPath := "/foo" k := &kubeconfigSources{ @@ -176,7 +228,7 @@ func TestGetKubeconfigEnvVariable(t *testing.T) { prepareKubeconfigSource(t, k) - kubeconfig, err := getKubeconfig() + kubeconfig, err := getKubeconfigPath() if err != nil { t.Fatalf("getting kubeconfig: %v", err) } @@ -186,14 +238,14 @@ func TestGetKubeconfigEnvVariable(t *testing.T) { } } -func TestGetKubeconfigDefault(t *testing.T) { - expectedPath := expandKubeconfigPath(defaultKubeconfigPath) +func TestGetKubeconfigPathDefault(t *testing.T) { + expectedPath := defaultKubeconfigPath k := &kubeconfigSources{} prepareKubeconfigSource(t, k) - kubeconfig, err := getKubeconfig() + kubeconfig, err := getKubeconfigPath() if err != nil { t.Fatalf("getting kubeconfig: %v", err) }