Repository: doitintl/kube-no-trouble
Pull request #191 (2021-07-15 15:02:25)

feat: Support multiple kubeconfig files
This PR addresses multiple kubeconfig files support via `KUBECONFIG` env variable, as per #123.

It removes parsing of `KUBECONFIG` env variable and relies on functionality in `client-go` packaged. This in turn, apart from multiple files support, should also make behaviour more consistent with `kubectl` and other tools.

_I'm planning to add support for context in separate PR._


Patch:
diff --git a/fixtures/kube.config.basic b/fixtures/kube.config.basic
new file mode 100644
index 0000000..176755e
--- /dev/null
+++ b/fixtures/kube.config.basic
@@ -0,0 +1,13 @@
+apiVersion: v1
+clusters:
+- cluster:
+    server: https://172.17.0.2:8443
+  name: minikube
+contexts:
+- context:
+    cluster: minikube
+    user: minikube
+  name: minikube
+current-context: minikube
+kind: Config
+preferences: {}
diff --git a/fixtures/kube.config.empty b/fixtures/kube.config.empty
new file mode 100644
index 0000000..e69de29
diff --git a/pkg/collector/cluster.go b/pkg/collector/cluster.go
index 475e19f..0431eb9 100644
--- a/pkg/collector/cluster.go
+++ b/pkg/collector/cluster.go
@@ -2,8 +2,6 @@ package collector
 
 import (
 	"encoding/json"
-	"k8s.io/client-go/tools/clientcmd"
-
 	"github.com/rs/zerolog/log"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -38,12 +36,7 @@ func NewClusterCollector(opts *ClusterOpts, additionalKinds []string) (*ClusterC
 	}
 
 	if opts.ClientSet == nil {
-		config, err := clientcmd.BuildConfigFromFlags("", opts.Kubeconfig)
-		if err != nil {
-			return nil, err
-		}
-
-		collector.clientSet, err = dynamic.NewForConfig(config)
+		collector.clientSet, err = dynamic.NewForConfig(kubeCollector.GetRestConfig())
 		if err != nil {
 			return nil, err
 		}
diff --git a/pkg/collector/helm2.go b/pkg/collector/helm2.go
index 4ea0c74..0543351 100644
--- a/pkg/collector/helm2.go
+++ b/pkg/collector/helm2.go
@@ -1,8 +1,6 @@
 package collector
 
 import (
-	"k8s.io/client-go/tools/clientcmd"
-
 	"github.com/ghodss/yaml"
 	"github.com/rs/zerolog/log"
 	"helm.sh/helm/pkg/storage"
@@ -37,12 +35,7 @@ func NewHelmV2Collector(opts *HelmV2Opts) (*HelmV2Collector, error) {
 		kubeCollector:   kubeCollector,
 	}
 
-	config, err := clientcmd.BuildConfigFromFlags("", opts.Kubeconfig)
-	if err != nil {
-		return nil, err
-	}
-
-	collector.client, err = corev1.NewForConfig(config)
+	collector.client, err = corev1.NewForConfig(kubeCollector.GetRestConfig())
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/collector/helm3.go b/pkg/collector/helm3.go
index c8b8963..df00179 100644
--- a/pkg/collector/helm3.go
+++ b/pkg/collector/helm3.go
@@ -1,8 +1,6 @@
 package collector
 
 import (
-	"k8s.io/client-go/tools/clientcmd"
-
 	"github.com/ghodss/yaml"
 	"github.com/rs/zerolog/log"
 	"helm.sh/helm/v3/pkg/releaseutil"
@@ -36,12 +34,7 @@ func NewHelmV3Collector(opts *HelmV3Opts) (*HelmV3Collector, error) {
 		kubeCollector:   kubeCollector,
 	}
 
-	config, err := clientcmd.BuildConfigFromFlags("", opts.Kubeconfig)
-	if err != nil {
-		return nil, err
-	}
-
-	collector.client, err = corev1.NewForConfig(config)
+	collector.client, err = corev1.NewForConfig(kubeCollector.GetRestConfig())
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/collector/kube.go b/pkg/collector/kube.go
index 35d64aa..ecd11f4 100644
--- a/pkg/collector/kube.go
+++ b/pkg/collector/kube.go
@@ -5,23 +5,32 @@ import (
 
 	goversion "github.com/hashicorp/go-version"
 	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 )
 
 type kubeCollector struct {
 	discoveryClient discovery.DiscoveryInterface
+	restConfig      *rest.Config
 }
 
 func newKubeCollector(kubeconfig string, discoveryClient discovery.DiscoveryInterface) (*kubeCollector, error) {
 	col := &kubeCollector{}
 
 	if discoveryClient == nil {
-		config, err := clientcmd.BuildConfigFromFlags("", kubeconfig)
+		pathOptions := clientcmd.NewDefaultPathOptions()
+		if kubeconfig != "" {
+			pathOptions.GlobalFile = kubeconfig
+		}
+
+		config, err := pathOptions.GetStartingConfig()
+		clientConfig := clientcmd.NewDefaultClientConfig(*config, &clientcmd.ConfigOverrides{})
+		col.restConfig, err = clientConfig.ClientConfig()
 		if err != nil {
 			return nil, err
 		}
 
-		col.discoveryClient, err = discovery.NewDiscoveryClientForConfig(config)
+		col.discoveryClient, err = discovery.NewDiscoveryClientForConfig(col.restConfig)
 		if err != nil {
 			return nil, err
 		}
@@ -40,3 +49,7 @@ func (c *kubeCollector) GetServerVersion() (*goversion.Version, error) {
 
 	return goversion.NewVersion(version.String())
 }
+
+func (c *kubeCollector) GetRestConfig() *rest.Config {
+	return c.restConfig
+}
diff --git a/pkg/config/config.go b/pkg/config/config.go
index d94f395..db4061b 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -2,8 +2,6 @@ package config
 
 import (
 	"fmt"
-	"os"
-	"path/filepath"
 	"strings"
 	"unicode"
 
@@ -11,7 +9,6 @@ import (
 
 	"github.com/rs/zerolog"
 	flag "github.com/spf13/pflag"
-	"k8s.io/client-go/util/homedir"
 )
 
 type Config struct {
@@ -33,14 +30,13 @@ func NewFromFlags() (*Config, error) {
 		TargetVersion: *NewVersion(),
 	}
 
-	home := homedir.HomeDir()
 	flag.StringSliceVarP(&config.AdditionalKinds, "additional-kind", "a", []string{}, "additional kinds of resources to report in Kind.version.group.com format")
 	flag.BoolVarP(&config.Cluster, "cluster", "c", true, "enable Cluster collector")
 	flag.BoolVarP(&config.ExitError, "exit-error", "e", false, "exit with non-zero code when issues are found")
 	flag.BoolVar(&config.Helm2, "helm2", true, "enable Helm v2 collector")
 	flag.BoolVar(&config.Helm3, "helm3", true, "enable Helm v3 collector")
 	flag.StringSliceVarP(&config.Filenames, "filename", "f", []string{}, "manifests to check, use - for stdin")
-	flag.StringVarP(&config.Kubeconfig, "kubeconfig", "k", envOrString("KUBECONFIG", filepath.Join(home, ".kube", "config")), "path to the kubeconfig file")
+	flag.StringVarP(&config.Kubeconfig, "kubeconfig", "k", "", "path to the kubeconfig file")
 	flag.StringVarP(&config.Output, "output", "o", "text", "output format - [text|json]")
 	flag.VarP(&config.LogLevel, "log-level", "l", "set log level (trace, debug, info, warn, error, fatal, panic, disabled)")
 	flag.VarP(&config.TargetVersion, "target-version", "t", "target K8s version in SemVer format (autodetected by default)")
@@ -58,14 +54,6 @@ func NewFromFlags() (*Config, error) {
 	return &config, nil
 }
 
-func envOrString(env string, def string) string {
-	val, ok := os.LookupEnv(env)
-	if ok {
-		return val
-	}
-	return def
-}
-
 // validateAdditionalResources check that all resources are provided in full form
 // resource.version.group.com. E.g. managedcertificate.v1beta1.networking.gke.io
 func validateAdditionalResources(resources []string) error {


Test patch:
diff --git a/pkg/collector/cluster_test.go b/pkg/collector/cluster_test.go
index c00007b..cff0a34 100644
--- a/pkg/collector/cluster_test.go
+++ b/pkg/collector/cluster_test.go
@@ -17,12 +17,12 @@ func TestNewClusterCollectorBadPath(t *testing.T) {
 	testOpts := ClusterOpts{Kubeconfig: "bad path"}
 	result, funcErr := NewClusterCollector(&testOpts, []string{})
 
-	if funcErr.Error() != "stat bad path: no such file or directory" {
+	if funcErr.Error() != "invalid configuration: no configuration has been provided" {
 		out, err := json.Marshal(result)
 		if err != nil {
-			t.Errorf("Should have crashed with path error instead got: %s", string(out))
+			t.Errorf("Should have errored with invalid configuration error instead got: %s", string(out))
 		} else {
-			t.Errorf("Should have crashed instead got un-parseable error: %s", funcErr)
+			t.Errorf("Should have errored instead got: %s", funcErr)
 		}
 	}
 }
@@ -39,7 +39,7 @@ func TestNewClusterCollectorValidEmptyCollector(t *testing.T) {
 	collector, err := NewClusterCollector(&testOpts, []string{})
 
 	if err != nil {
-		t.Errorf("Should have parsed config instead got: %s", err)
+		t.Fatalf("Should have parsed config instead got: %s", err)
 	}
 
 	result, err := collector.Get()
diff --git a/pkg/collector/kube_test.go b/pkg/collector/kube_test.go
index ef769ce..a6b719b 100644
--- a/pkg/collector/kube_test.go
+++ b/pkg/collector/kube_test.go
@@ -1,6 +1,7 @@
 package collector
 
 import (
+	"os"
 	"testing"
 
 	goversion "github.com/hashicorp/go-version"
@@ -14,7 +15,7 @@ func TestNewKubeCollector(t *testing.T) {
 	col, err := newKubeCollector("", clientSet.Discovery())
 
 	if err != nil {
-		t.Errorf("Failed to create kubeCollector form fake discovery client")
+		t.Errorf("Failed to create kubeCollector from fake discovery client")
 	}
 	if col == nil {
 		t.Errorf("Should return collector, instrad got nil")
@@ -29,6 +30,36 @@ func TestNewKubeCollectorError(t *testing.T) {
 	}
 }
 
+func TestNewKubeCollectorWithKubeconfigPath(t *testing.T) {
+	_, err := newKubeCollector("../../fixtures/kube.config.basic", nil)
+	if err != nil {
+		t.Fatalf("Failed with: %s", err)
+	}
+}
+
+func TestNewKubeCollectorMultipleFiles(t *testing.T) {
+	kcEnvVar := "KUBECONFIG"
+	oldKubeConfig, oldSet := os.LookupEnv(kcEnvVar)
+
+	if err := os.Setenv(kcEnvVar, "../../fixtures/kube.config.empty:../../fixtures/kube.config.basic"); err != nil {
+		t.Fatalf("Failed so set %s env variable for test: %s", kcEnvVar, err)
+	}
+
+	if _, err := newKubeCollector("", nil); err != nil {
+		t.Fatalf("Failed with: %s", err)
+	}
+
+	var err error
+	if oldSet {
+		err = os.Setenv(kcEnvVar, oldKubeConfig)
+	} else {
+		err = os.Unsetenv(kcEnvVar)
+	}
+	if err != nil {
+		t.Fatalf("Failed so reset %s env variable after test: %s", kcEnvVar, err)
+	}
+}
+
 func TestGetServerVersion(t *testing.T) {
 	gitVersion := "v1.2.3"
 	expectedVersion, _ := goversion.NewVersion(gitVersion)
diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
index 90096e6..0505fbc 100644
--- a/pkg/config/config_test.go
+++ b/pkg/config/config_test.go
@@ -6,7 +6,6 @@ import (
 	"testing"
 
 	"github.com/spf13/pflag"
-	"k8s.io/client-go/util/homedir"
 )
 
 func TestValidLogLevelFromFlags(t *testing.T) {
@@ -57,73 +56,6 @@ func TestNewFromFlags(t *testing.T) {
 	}
 }
 
-func TestNewFromFlagsKubeconfigEnv(t *testing.T) {
-	// reset for testing
-	pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)
-
-	testVal := "my-file.conf"
-
-	err := os.Setenv("KUBECONFIG", testVal)
-	if err != nil {
-		t.Errorf("failed to set env variable: %s", err)
-	}
-
-	config, err := NewFromFlags()
-
-	if err != nil {
-		t.Errorf("Formatting flags failed %s", err)
-	}
-
-	if config.Kubeconfig != testVal {
-		t.Errorf("kubeconfig option not loaded correctly from ebv variable, expected: %s, got: %s", testVal, config.Kubeconfig)
-	}
-}
-
-func TestNewFromFlagsKubeconfigHome(t *testing.T) {
-	// reset for testing
-	pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)
-
-	expected := homedir.HomeDir() + "/.kube/config"
-	err := os.Unsetenv("KUBECONFIG")
-	if err != nil {
-		t.Errorf("failed to unset KUBECONFIG env variable: %s", err)
-	}
-
-	config, err := NewFromFlags()
-
-	if err != nil {
-		t.Errorf("Formatting flags failed %s", err)
-	}
-
-	if config.Kubeconfig != expected {
-		t.Errorf("kubeconfig option not set to expected default, expected: %s, got: %s", expected, config.Kubeconfig)
-	}
-}
-
-func TestEnvOrStringVariable(t *testing.T) {
-	err := os.Setenv("FOO", "1")
-	if err != nil {
-		t.Errorf("failed to set env variable: %e", err)
-	}
-
-	i := envOrString("FOO", "default")
-	if i != "1" {
-		t.Errorf("Expected to get env variable, got %s instead", i)
-	}
-}
-
-func TestEnvOrStringDefault(t *testing.T) {
-	err := os.Unsetenv("FOO")
-	if err != nil {
-		t.Errorf("failed to unset env variable: %e", err)
-	}
-
-	i := envOrString("FOO", "default")
-	if i != "default" {
-		t.Errorf("Expected to get default string, got %s instead", i)
-	}
-}
-
 func TestValidateAdditionalResources(t *testing.T) {
 	resources := []string{
 		"Test.v1.example.com",