Repository: doitintl/kube-no-trouble
Pull request #237 (2021-11-18 18:45:05)

Add support for in-cluster auth
This PR mainly adds support for in-cluster authentication and some minor refactoring on tests.

This func. would deserve integration test, created #236.




Patch:
diff --git a/pkg/collector/kube.go b/pkg/collector/kube.go
index 9f48f7b..2d7d298 100644
--- a/pkg/collector/kube.go
+++ b/pkg/collector/kube.go
@@ -21,30 +21,48 @@ func newKubeCollector(kubeconfig string, kubecontext string, discoveryClient dis
 	if discoveryClient != nil {
 		col.discoveryClient = discoveryClient
 	} else {
-		pathOptions := clientcmd.NewDefaultPathOptions()
-		if kubeconfig != "" {
-			pathOptions.GlobalFile = kubeconfig
+		var err error
+		if col.restConfig, err = newClientRestConfig(kubeconfig, kubecontext, rest.InClusterConfig); err != nil {
+			return nil, fmt.Errorf("failed to assemble client config: %w", err)
 		}
 
-		config, err := pathOptions.GetStartingConfig()
-
-		configOverrides := clientcmd.ConfigOverrides{}
-		if kubecontext != "" {
-			configOverrides.CurrentContext = kubecontext
+		if col.discoveryClient, err = discovery.NewDiscoveryClientForConfig(col.restConfig); err != nil {
+			return nil, fmt.Errorf("failed to create client: %w", err)
 		}
+	}
 
-		clientConfig := clientcmd.NewDefaultClientConfig(*config, &configOverrides)
-		col.restConfig, err = clientConfig.ClientConfig()
-		if err != nil {
-			return nil, err
-		}
+	return col, nil
+}
 
-		if col.discoveryClient, err = discovery.NewDiscoveryClientForConfig(col.restConfig); err != nil {
-			return nil, err
+func newClientRestConfig(kubeconfig string, kubecontext string, inClusterFn func() (*rest.Config, error)) (*rest.Config, error) {
+	if kubeconfig == "" {
+		if restConfig, err := inClusterFn(); err == nil {
+			return restConfig, nil
 		}
 	}
 
-	return col, nil
+	pathOptions := clientcmd.NewDefaultPathOptions()
+	if kubeconfig != "" {
+		pathOptions.GlobalFile = kubeconfig
+	}
+
+	config, err := pathOptions.GetStartingConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	configOverrides := clientcmd.ConfigOverrides{}
+	if kubecontext != "" {
+		configOverrides.CurrentContext = kubecontext
+	}
+
+	clientConfig := clientcmd.NewDefaultClientConfig(*config, &configOverrides)
+	restConfig, err := clientConfig.ClientConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	return restConfig, nil
 }
 
 func (c *kubeCollector) GetServerVersion() (*judge.Version, error) {


Test patch:
diff --git a/cmd/kubent/main_test.go b/cmd/kubent/main_test.go
index c0568f1..363d998 100644
--- a/cmd/kubent/main_test.go
+++ b/cmd/kubent/main_test.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strconv"
 	"testing"
 
@@ -14,9 +15,11 @@ import (
 	"github.com/rs/zerolog"
 )
 
+const FIXTURES_DIR = "../../fixtures"
+
 func TestInitCollectors(t *testing.T) {
 	testConfig := config.Config{
-		Filenames:  []string{"../../fixtures/deployment-v1beta1.yaml"},
+		Filenames:  []string{filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")},
 		Cluster:    false,
 		Helm2:      false,
 		Helm3:      false,
@@ -33,7 +36,8 @@ func TestInitCollectors(t *testing.T) {
 }
 
 func TestGetCollectors(t *testing.T) {
-	fileCollector, err := collector.NewFileCollector(&collector.FileOpts{Filenames: []string{"../../fixtures/deployment-v1beta1.yaml"}})
+	fileCollector, err := collector.NewFileCollector(
+		&collector.FileOpts{Filenames: []string{filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}})
 
 	if err != nil {
 		t.Errorf("Failed to create File collector with error: %s", err)
@@ -51,7 +55,8 @@ func TestGetCollectors(t *testing.T) {
 
 func TestStoreCollector(t *testing.T) {
 	collectors := []collector.Collector{}
-	fileCollector, err := collector.NewFileCollector(&collector.FileOpts{Filenames: []string{"../../fixtures/deployment-v1beta1.yaml"}})
+	fileCollector, err := collector.NewFileCollector(
+		&collector.FileOpts{Filenames: []string{filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}})
 
 	if err != nil {
 		t.Errorf("Failed to create File collector with error: %s", err)
@@ -66,7 +71,8 @@ func TestStoreCollector(t *testing.T) {
 
 func TestStoreCollectorMultiple(t *testing.T) {
 	collectors := []collector.Collector{}
-	fileCollector, err := collector.NewFileCollector(&collector.FileOpts{Filenames: []string{"../../fixtures/deployment-v1beta1.yaml"}})
+	fileCollector, err := collector.NewFileCollector(
+		&collector.FileOpts{Filenames: []string{filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}})
 
 	if err != nil {
 		t.Errorf("Failed to create File collector with error: %s", err)
@@ -100,9 +106,9 @@ func TestMainExitCodes(t *testing.T) {
 	}{
 		{"success", []string{"-c=false", "--helm2=false", "--helm3=false"}, 0},
 		{"errorBadFlag", []string{"-c=not-boolean"}, 2},
-		{"successFound", []string{"-c=false", "--helm2=false", "--helm3=false", "-f=../../fixtures/deployment-v1beta1.yaml"}, 0},
+		{"successFound", []string{"-c=false", "--helm2=false", "--helm3=false", "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 0},
 		{"exitErrorFlagNone", []string{"-c=false", "--helm2=false", "--helm3=false", "-e"}, 0},
-		{"exitErrorFlagFound", []string{"-c=false", "--helm2=false", "--helm3=false", "-e", "-f=../../fixtures/deployment-v1beta1.yaml"}, 200},
+		{"exitErrorFlagFound", []string{"-c=false", "--helm2=false", "--helm3=false", "-e", "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 200},
 	}
 
 	if os.Getenv("TEST_EXIT_CODE") == "1" {
@@ -160,7 +166,8 @@ func TestGetServerVersionNone(t *testing.T) {
 func TestGetServerVersionNotSupported(t *testing.T) {
 	collectors := []collector.Collector{}
 
-	fileCollector, err := collector.NewFileCollector(&collector.FileOpts{Filenames: []string{"../../fixtures/deployment-v1beta1.yaml"}})
+	fileCollector, err := collector.NewFileCollector(
+		&collector.FileOpts{Filenames: []string{filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}})
 	if err != nil {
 		t.Errorf("Failed to create File collector with error: %s", err)
 	}
diff --git a/pkg/collector/cluster_test.go b/pkg/collector/cluster_test.go
index cff0a34..951c81c 100644
--- a/pkg/collector/cluster_test.go
+++ b/pkg/collector/cluster_test.go
@@ -1,8 +1,9 @@
 package collector
 
 import (
-	"encoding/json"
 	"io/ioutil"
+	"path/filepath"
+	"strings"
 	"testing"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -15,14 +16,13 @@ import (
 
 func TestNewClusterCollectorBadPath(t *testing.T) {
 	testOpts := ClusterOpts{Kubeconfig: "bad path"}
-	result, funcErr := NewClusterCollector(&testOpts, []string{})
+	_, err := NewClusterCollector(&testOpts, []string{})
 
-	if funcErr.Error() != "invalid configuration: no configuration has been provided" {
-		out, err := json.Marshal(result)
+	if !strings.Contains(err.Error(), "no configuration has been provided") {
 		if err != nil {
-			t.Errorf("Should have errored with invalid configuration error instead got: %s", string(out))
+			t.Errorf("Should have errored with invalid configuration error instead got: %s", err)
 		} else {
-			t.Errorf("Should have errored instead got: %s", funcErr)
+			t.Errorf("Should have failed but succeeded")
 		}
 	}
 }
@@ -32,7 +32,7 @@ func TestNewClusterCollectorValidEmptyCollector(t *testing.T) {
 	clientset := fake.NewSimpleDynamicClient(scheme)
 	discoveryClient := discoveryFake.FakeDiscovery{}
 	testOpts := ClusterOpts{
-		Kubeconfig:      "../../fixtures/kube.config",
+		Kubeconfig:      filepath.Join(FIXTURES_DIR, "kube.config"),
 		ClientSet:       clientset,
 		DiscoveryClient: &discoveryClient,
 	}
@@ -74,10 +74,10 @@ func TestClusterCollectorGetFake(t *testing.T) {
 		expected int      // number of manifests
 	}{
 		{"empty", []string{}, 0},
-		{"withoutAnnotation", []string{"../../fixtures/fake-deployment-v1beta1-no-annotation.yaml"}, 0},
-		{"one", []string{"../../fixtures/fake-deployment-v1beta1-with-annotation.yaml"}, 1},
-		{"multiple", []string{"../../fixtures/fake-deployment-v1beta1-with-annotation.yaml", "../../fixtures/fake-ingress-v1beta1-with-annotation.yaml"}, 2},
-		{"mixed", []string{"../../fixtures/fake-deployment-v1beta1-no-annotation.yaml", "../../fixtures/fake-ingress-v1beta1-with-annotation.yaml"}, 1},
+		{"withoutAnnotation", []string{"fake-deployment-v1beta1-no-annotation.yaml"}, 0},
+		{"one", []string{"fake-deployment-v1beta1-with-annotation.yaml"}, 1},
+		{"multiple", []string{"fake-deployment-v1beta1-with-annotation.yaml", "fake-ingress-v1beta1-with-annotation.yaml"}, 2},
+		{"mixed", []string{"fake-deployment-v1beta1-no-annotation.yaml", "fake-ingress-v1beta1-with-annotation.yaml"}, 1},
 	}
 
 	for _, tc := range testCases {
@@ -88,7 +88,7 @@ func TestClusterCollectorGetFake(t *testing.T) {
 			for _, f := range tc.input {
 				obj := &unstructured.Unstructured{}
 
-				input, err := ioutil.ReadFile(f)
+				input, err := ioutil.ReadFile(filepath.Join(FIXTURES_DIR, f))
 				dec := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme)
 				_, _, err = dec.Decode(input, nil, obj)
 				if err != nil {
diff --git a/pkg/collector/file_test.go b/pkg/collector/file_test.go
index 30eb921..74b5c1f 100644
--- a/pkg/collector/file_test.go
+++ b/pkg/collector/file_test.go
@@ -3,6 +3,7 @@ package collector
 import (
 	"io"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -28,10 +29,10 @@ func TestFileCollectorGet(t *testing.T) {
 		input    []string // file list
 		expected []string // kinds of objects
 	}{
-		{"yaml", []string{"../../fixtures/deployment-v1beta1.yaml"}, []string{"Deployment"}},
-		{"yamlMulti", []string{"../../fixtures/deployment-v1beta1-and-ingress-v1beta1.yaml"}, []string{"Deployment", "Ingress"}},
-		{"json", []string{"../../fixtures/deployment-v1beta1.json"}, []string{"Deployment"}},
-		{"mixed", []string{"../../fixtures/deployment-v1beta1.json", "../../fixtures/deployment-v1beta1.yaml"}, []string{"Deployment", "Deployment"}},
+		{"yaml", []string{filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, []string{"Deployment"}},
+		{"yamlMulti", []string{filepath.Join(FIXTURES_DIR, "deployment-v1beta1-and-ingress-v1beta1.yaml")}, []string{"Deployment", "Ingress"}},
+		{"json", []string{filepath.Join(FIXTURES_DIR, "deployment-v1beta1.json")}, []string{"Deployment"}},
+		{"mixed", []string{filepath.Join(FIXTURES_DIR, "deployment-v1beta1.json"), filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, []string{"Deployment", "Deployment"}},
 	}
 
 	for _, tc := range testCases {
@@ -61,7 +62,7 @@ func TestFileCollectorGet(t *testing.T) {
 }
 
 func TestFileCollectorGetUnknown(t *testing.T) {
-	input := []string{"../../fixtures/meow.txt"}
+	input := []string{filepath.Join(FIXTURES_DIR, "meow.txt")}
 
 	c, err := NewFileCollector(
 		&FileOpts{Filenames: input},
@@ -79,7 +80,7 @@ func TestFileCollectorGetUnknown(t *testing.T) {
 }
 
 func TestFileCollectorGetNonExistent(t *testing.T) {
-	input := []string{"../../fixtures/does-not-exist"}
+	input := []string{"does-not-exist"}
 	expected := "failed to read"
 
 	c, err := NewFileCollector(
@@ -101,7 +102,7 @@ func TestFileCollectorGetNonExistent(t *testing.T) {
 
 func TestFileCollectorGetStdin(t *testing.T) {
 	input := []string{"-"}
-	inputFilename := "../../fixtures/deployment-v1beta1.yaml"
+	inputFilename := filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")
 	expected := 1
 
 	c, err := NewFileCollector(
diff --git a/pkg/collector/kube_test.go b/pkg/collector/kube_test.go
index cd9cb33..33d183e 100644
--- a/pkg/collector/kube_test.go
+++ b/pkg/collector/kube_test.go
@@ -1,7 +1,9 @@
 package collector
 
 import (
+	"k8s.io/client-go/rest"
 	"os"
+	"path/filepath"
 	"testing"
 
 	goversion "github.com/hashicorp/go-version"
@@ -10,6 +12,8 @@ import (
 	"k8s.io/client-go/kubernetes/fake"
 )
 
+const FIXTURES_DIR = "../../fixtures"
+
 func TestNewKubeCollector(t *testing.T) {
 	clientSet := fake.NewSimpleClientset()
 	col, err := newKubeCollector("", "", clientSet.Discovery())
@@ -22,41 +26,28 @@ func TestNewKubeCollector(t *testing.T) {
 	}
 }
 
-func TestNewKubeCollectorError(t *testing.T) {
-	_, err := newKubeCollector("does-not-exist", "", nil)
-
-	if err == nil {
-		t.Errorf("Expected to fail with non-existent kubeconfig")
-	}
-}
-
 func TestNewKubeCollectorWithKubeconfigPath(t *testing.T) {
-	_, err := newKubeCollector("../../fixtures/kube.config.basic", "", nil)
+	col, err := newKubeCollector(filepath.Join(FIXTURES_DIR, "kube.config.basic"), "", nil)
+
 	if err != nil {
-		t.Fatalf("Failed with: %s", err)
+		t.Fatalf("Failed to create kubeCollector from fake discovery client")
 	}
-}
-
-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 col == nil {
+		t.Fatalf("Should return collector, got nil instead")
 	}
-
-	if _, err := newKubeCollector("", "", nil); err != nil {
-		t.Fatalf("Failed with: %s", err)
+	if col.discoveryClient == nil {
+		t.Fatalf("Collector should have discovery client, got nil instead")
 	}
-
-	var err error
-	if oldSet {
-		err = os.Setenv(kcEnvVar, oldKubeConfig)
-	} else {
-		err = os.Unsetenv(kcEnvVar)
+	if col.restConfig == nil {
+		t.Fatalf("Collector should have rest config, got nil instead")
 	}
-	if err != nil {
-		t.Fatalf("Failed so reset %s env variable after test: %s", kcEnvVar, err)
+}
+
+func TestNewKubeCollectorError(t *testing.T) {
+	_, err := newKubeCollector("does-not-exist", "", nil)
+
+	if err == nil {
+		t.Errorf("Expected to fail with non-existent kubeconfig")
 	}
 }
 
@@ -88,7 +79,7 @@ func TestContext(t *testing.T) {
 	expectedContext := "test-context"
 	expectedHost := "https://test-cluster"
 
-	collector, err := newKubeCollector("../../fixtures/kube.config.context", expectedContext, nil)
+	collector, err := newKubeCollector(filepath.Join(FIXTURES_DIR, "kube.config.context"), expectedContext, nil)
 	if err != nil {
 		t.Fatalf("Failed to create kubeCollector from fake client with context %s: %s", expectedContext, err)
 	}
@@ -102,8 +93,109 @@ func TestContext(t *testing.T) {
 func TestContextMissing(t *testing.T) {
 	expectedContext := "non-existent"
 
-	_, err := newKubeCollector("../../fixtures/kube.config.context", expectedContext, nil)
+	_, err := newKubeCollector(filepath.Join(FIXTURES_DIR, "kube.config.context"), expectedContext, nil)
 	if err == nil {
 		t.Fatalf("Expected to fail when uisng non-existent context: %s", expectedContext)
 	}
 }
+
+func TestNewClientRestConfigError(t *testing.T) {
+	_, err := newClientRestConfig("does-not-exist", "", rest.InClusterConfig)
+
+	if err == nil {
+		t.Errorf("Expected to fail with non-existent kubeconfig")
+	}
+}
+
+func TestNewClientRestConfigWithKubeconfigPath(t *testing.T) {
+	_, err := newClientRestConfig(filepath.Join(FIXTURES_DIR, "kube.config.basic"), "", rest.InClusterConfig)
+	if err != nil {
+		t.Fatalf("Failed with: %s", err)
+	}
+}
+
+func TestNewClientRestConfigMultipleFiles(t *testing.T) {
+	env := setupEnv(t, map[string]string{
+		"KUBECONFIG": filepath.Join(FIXTURES_DIR, "kube.config.empty") + ":" + filepath.Join(FIXTURES_DIR, "kube.config.basic"),
+	})
+	defer env.reset()
+
+	if _, err := newClientRestConfig("", "", rest.InClusterConfig); err != nil {
+		t.Fatalf("Failed with: %s", err)
+	}
+}
+
+func TestNewClientRestConfigWithContext(t *testing.T) {
+	expectedContext := "test-context"
+	expectedHost := "https://test-cluster"
+
+	config, err := newClientRestConfig(filepath.Join(FIXTURES_DIR, "kube.config.context"), expectedContext, rest.InClusterConfig)
+	if err != nil {
+		t.Fatalf("Failed to create kubeCollector from fake client with context %s: %s", expectedContext, err)
+	}
+
+	if config.Host != expectedHost {
+		t.Fatalf("Expected host from context %s to be: %s, got %s instead", expectedContext, expectedHost, config.Host)
+	}
+}
+
+func TestNewClientRestConfigContextMissing(t *testing.T) {
+	expectedContext := "non-existent"
+
+	_, err := newClientRestConfig(filepath.Join(FIXTURES_DIR, "kube.config.context"), expectedContext, rest.InClusterConfig)
+	if err == nil {
+		t.Fatalf("Expected to fail when uisng non-existent context: %s", expectedContext)
+	}
+}
+
+func TestNewClientRestConfigInCluster(t *testing.T) {
+	expectedHost := "test-host"
+	inClusterFn := func() (*rest.Config, error) {
+		return &rest.Config{Host: expectedHost}, nil
+	}
+
+	cfg, err := newClientRestConfig("", "", inClusterFn)
+	if err != nil {
+		t.Fatalf("Failed to create in-cluster config: %s", err)
+	}
+	if cfg.Host != expectedHost {
+		t.Fatalf("Expected %s host, instead got: %s", expectedHost, cfg.Host)
+	}
+}
+
+type env struct {
+	initialVals  map[string]string
+	initialState map[string]bool
+	t            *testing.T
+}
+
+func setupEnv(t *testing.T, vars map[string]string) *env {
+	env := env{
+		initialState: make(map[string]bool),
+		initialVals:  make(map[string]string),
+	}
+
+	for k, v := range vars {
+		env.initialVals[k], env.initialState[k] = os.LookupEnv(k)
+		if err := os.Setenv(k, v); err != nil {
+			t.Fatalf("Failed so set %s env variable for test: %s", k, err)
+		}
+	}
+
+	return &env
+}
+
+func (e *env) reset() {
+	for k, v := range e.initialState {
+		var err error
+		if v {
+			err = os.Setenv(k, e.initialVals[k])
+		} else {
+			err = os.Unsetenv(k)
+		}
+
+		if err != nil {
+			e.t.Errorf("Failed to reset %s env variable after test: %s", k, err)
+		}
+	}
+}
diff --git a/pkg/judge/rego_test.go b/pkg/judge/rego_test.go
index afec1ef..3d1feb6 100644
--- a/pkg/judge/rego_test.go
+++ b/pkg/judge/rego_test.go
@@ -2,6 +2,7 @@ package judge
 
 import (
 	"io/ioutil"
+	"path/filepath"
 	"testing"
 
 	"github.com/doitintl/kube-no-trouble/pkg/rules"
@@ -9,6 +10,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
+const FIXTURES_DIR = "../../fixtures"
+
 func TestNewRegoJudge(t *testing.T) {
 	_, err := NewRegoJudge(&RegoOpts{}, []rules.Rule{})
 	if err != nil {
@@ -40,8 +43,8 @@ func TestEvalRules(t *testing.T) {
 		inputFiles []string // file list
 		expected   []string // findings - kinds
 	}{
-		{"deprecated-1-16.rego", []string{"../../fixtures/deployment-v1beta1.yaml"}, []string{"Deployment"}},
-		{"deprecated-1-22.rego", []string{"../../fixtures/ingress-v1beta1.yaml"}, []string{"Ingress"}},
+		{"deprecated-1-16.rego", []string{"deployment-v1beta1.yaml"}, []string{"Deployment"}},
+		{"deprecated-1-22.rego", []string{"ingress-v1beta1.yaml"}, []string{"Ingress"}},
 	}
 
 	for _, tc := range testCases {
@@ -53,7 +56,7 @@ func TestEvalRules(t *testing.T) {
 				var input []byte
 				var err error
 
-				input, err = ioutil.ReadFile(f)
+				input, err = ioutil.ReadFile(filepath.Join(FIXTURES_DIR, f))
 				if err != nil {
 					t.Errorf("failed to read file %s: %v", f, err)
 				}