Repository: doitintl/kube-no-trouble
Pull request #196 (2021-07-23 11:52:05)

Fix: Helm v3 collector name and refactoring to be test-friendly
The main reason for this PR is to fix Helm v3 collector name:

```diff
pkg/collector/helm3.go
- commonCollector: newCommonCollector("Helm v2"),
+ commonCollector: newCommonCollector("Helm v3"),
```

And I've also added a test to cover this, and that needed some small refactoring too :).


Patch:
diff --git a/pkg/collector/cluster.go b/pkg/collector/cluster.go
index 7456bd2..26ae0f0 100644
--- a/pkg/collector/cluster.go
+++ b/pkg/collector/cluster.go
@@ -11,6 +11,8 @@ import (
 	"k8s.io/client-go/restmapper"
 )
 
+const CLUSTER_COLLECTOR_NAME = "Cluster"
+
 type ClusterCollector struct {
 	*commonCollector
 	*kubeCollector
@@ -33,7 +35,7 @@ func NewClusterCollector(opts *ClusterOpts, additionalKinds []string) (*ClusterC
 
 	collector := &ClusterCollector{
 		kubeCollector:   kubeCollector,
-		commonCollector: newCommonCollector("Cluster"),
+		commonCollector: newCommonCollector(CLUSTER_COLLECTOR_NAME),
 	}
 
 	if opts.ClientSet == nil {
diff --git a/pkg/collector/fake.go b/pkg/collector/fake.go
index 6770fe7..3dbf281 100644
--- a/pkg/collector/fake.go
+++ b/pkg/collector/fake.go
@@ -4,7 +4,10 @@ import (
 	goversion "github.com/hashicorp/go-version"
 )
 
-const FAKE_VERSION = "1.2.3"
+const (
+	FAKE_VERSION        = "1.2.3"
+	FAKE_COLLECTOR_NAME = "Fake"
+)
 
 type fakeCollector struct {
 	*commonCollector
@@ -16,7 +19,7 @@ func (c *fakeCollector) Get() ([]map[string]interface{}, error) {
 
 func NewFakeCollector() *fakeCollector {
 	return &fakeCollector{
-		commonCollector: newCommonCollector("Fake"),
+		commonCollector: newCommonCollector(FAKE_COLLECTOR_NAME),
 	}
 }
 
diff --git a/pkg/collector/file.go b/pkg/collector/file.go
index 286c84b..72ae2ea 100644
--- a/pkg/collector/file.go
+++ b/pkg/collector/file.go
@@ -13,6 +13,8 @@ import (
 	"helm.sh/helm/v3/pkg/releaseutil"
 )
 
+const FILE_COLLECTOR_NAME = "File"
+
 type FileCollector struct {
 	*commonCollector
 	filenames []string
@@ -29,7 +31,7 @@ func NewFileCollector(opts *FileOpts) (*FileCollector, error) {
 	}
 
 	collector := &FileCollector{
-		commonCollector: newCommonCollector("File"),
+		commonCollector: newCommonCollector(FILE_COLLECTOR_NAME),
 		filenames:       opts.Filenames,
 	}
 
diff --git a/pkg/collector/helm2.go b/pkg/collector/helm2.go
index 7ebc49a..15d7cdc 100644
--- a/pkg/collector/helm2.go
+++ b/pkg/collector/helm2.go
@@ -10,10 +10,12 @@ import (
 	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 )
 
+const HELM_V2_COLLECTOR_NAME = "Helm v2"
+
 type HelmV2Collector struct {
 	*commonCollector
 	*kubeCollector
-	client       *corev1.CoreV1Client
+	client       corev1.CoreV1Interface
 	secretsStore *storage.Storage
 	configStore  *storage.Storage
 }
@@ -22,6 +24,7 @@ type HelmV2Opts struct {
 	Kubeconfig      string
 	KubeContext     string
 	DiscoveryClient discovery.DiscoveryInterface
+	CoreClient      corev1.CoreV1Interface
 }
 
 func NewHelmV2Collector(opts *HelmV2Opts) (*HelmV2Collector, error) {
@@ -32,12 +35,13 @@ func NewHelmV2Collector(opts *HelmV2Opts) (*HelmV2Collector, error) {
 	}
 
 	collector := &HelmV2Collector{
-		commonCollector: newCommonCollector("Helm v2"),
+		commonCollector: newCommonCollector(HELM_V2_COLLECTOR_NAME),
 		kubeCollector:   kubeCollector,
 	}
 
-	collector.client, err = corev1.NewForConfig(kubeCollector.GetRestConfig())
-	if err != nil {
+	if opts.CoreClient != nil {
+		collector.client = opts.CoreClient
+	} else if collector.client, err = corev1.NewForConfig(kubeCollector.GetRestConfig()); err != nil {
 		return nil, err
 	}
 
diff --git a/pkg/collector/helm3.go b/pkg/collector/helm3.go
index c4274e6..cae1cea 100644
--- a/pkg/collector/helm3.go
+++ b/pkg/collector/helm3.go
@@ -10,10 +10,12 @@ import (
 	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 )
 
+const HELM_V3_COLLECTOR_NAME = "Helm v3"
+
 type HelmV3Collector struct {
 	*commonCollector
 	*kubeCollector
-	client       *corev1.CoreV1Client
+	client       corev1.CoreV1Interface
 	secretsStore *storage.Storage
 	configStore  *storage.Storage
 }
@@ -22,6 +24,7 @@ type HelmV3Opts struct {
 	Kubeconfig      string
 	KubeContext     string
 	DiscoveryClient discovery.DiscoveryInterface
+	CoreClient      corev1.CoreV1Interface
 }
 
 func NewHelmV3Collector(opts *HelmV3Opts) (*HelmV3Collector, error) {
@@ -31,12 +34,13 @@ func NewHelmV3Collector(opts *HelmV3Opts) (*HelmV3Collector, error) {
 	}
 
 	collector := &HelmV3Collector{
-		commonCollector: newCommonCollector("Helm v2"),
+		commonCollector: newCommonCollector(HELM_V3_COLLECTOR_NAME),
 		kubeCollector:   kubeCollector,
 	}
 
-	collector.client, err = corev1.NewForConfig(kubeCollector.GetRestConfig())
-	if err != nil {
+	if opts.CoreClient != nil {
+		collector.client = opts.CoreClient
+	} else if collector.client, err = corev1.NewForConfig(kubeCollector.GetRestConfig()); err != nil {
 		return nil, err
 	}
 
diff --git a/pkg/collector/kube.go b/pkg/collector/kube.go
index a24f8ac..ba85868 100644
--- a/pkg/collector/kube.go
+++ b/pkg/collector/kube.go
@@ -17,7 +17,9 @@ type kubeCollector struct {
 func newKubeCollector(kubeconfig string, kubecontext string, discoveryClient discovery.DiscoveryInterface) (*kubeCollector, error) {
 	col := &kubeCollector{}
 
-	if discoveryClient == nil {
+	if discoveryClient != nil {
+		col.discoveryClient = discoveryClient
+	} else {
 		pathOptions := clientcmd.NewDefaultPathOptions()
 		if kubeconfig != "" {
 			pathOptions.GlobalFile = kubeconfig
@@ -36,12 +38,9 @@ func newKubeCollector(kubeconfig string, kubecontext string, discoveryClient dis
 			return nil, err
 		}
 
-		col.discoveryClient, err = discovery.NewDiscoveryClientForConfig(col.restConfig)
-		if err != nil {
+		if col.discoveryClient, err = discovery.NewDiscoveryClientForConfig(col.restConfig); err != nil {
 			return nil, err
 		}
-	} else {
-		col.discoveryClient = discoveryClient
 	}
 
 	return col, nil


Test patch:
diff --git a/pkg/collector/helm2_test.go b/pkg/collector/helm2_test.go
new file mode 100644
index 0000000..d0d63ce
--- /dev/null
+++ b/pkg/collector/helm2_test.go
@@ -0,0 +1,28 @@
+package collector
+
+import (
+	"testing"
+
+	"k8s.io/client-go/kubernetes/fake"
+)
+
+func TestNewHelmV2Collector(t *testing.T) {
+	expectedName := "Helm v2"
+
+	clientSet := fake.NewSimpleClientset()
+	col, err := NewHelmV2Collector(&HelmV2Opts{
+		DiscoveryClient: clientSet.Discovery(),
+		CoreClient:      clientSet.CoreV1(),
+	})
+	clientSet.CoreV1()
+
+	if err != nil {
+		t.Fatalf("Failed to create collector from fake discovery client")
+	}
+	if col == nil {
+		t.Fatalf("Should return collector, got nil instead")
+	}
+	if col.Name() != expectedName {
+		t.Fatalf("Expected collector name: %s, instead got: %s", expectedName, col.Name())
+	}
+}
diff --git a/pkg/collector/helm3_test.go b/pkg/collector/helm3_test.go
new file mode 100644
index 0000000..70742ba
--- /dev/null
+++ b/pkg/collector/helm3_test.go
@@ -0,0 +1,27 @@
+package collector
+
+import (
+	"testing"
+
+	"k8s.io/client-go/kubernetes/fake"
+)
+
+func TestNewHelmV3Collector(t *testing.T) {
+	expectedName := "Helm v3"
+
+	clientSet := fake.NewSimpleClientset()
+	col, err := NewHelmV3Collector(&HelmV3Opts{
+		DiscoveryClient: clientSet.Discovery(),
+		CoreClient:      clientSet.CoreV1(),
+	})
+
+	if err != nil {
+		t.Fatalf("Failed to create collector from fake discovery client")
+	}
+	if col == nil {
+		t.Fatalf("Should return collector, got nil instead")
+	}
+	if col.Name() != expectedName {
+		t.Fatalf("Expected collector name: %s, instead got: %s", expectedName, col.Name())
+	}
+}
diff --git a/pkg/collector/kube_test.go b/pkg/collector/kube_test.go
index 3c6f8f8..cd9cb33 100644
--- a/pkg/collector/kube_test.go
+++ b/pkg/collector/kube_test.go
@@ -15,10 +15,10 @@ func TestNewKubeCollector(t *testing.T) {
 	col, err := newKubeCollector("", "", clientSet.Discovery())
 
 	if err != nil {
-		t.Errorf("Failed to create kubeCollector from fake discovery client")
+		t.Fatalf("Failed to create kubeCollector from fake discovery client")
 	}
 	if col == nil {
-		t.Errorf("Should return collector, instrad got nil")
+		t.Fatalf("Should return collector, got nil instead")
 	}
 }