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 <mateusz@kinvolk.io>

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)
 		}
 	}

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)
 	}
 }

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)
 	}
 }

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)
 	}

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

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)
 	}