Repository: doitintl/kube-no-trouble
Pull request #338 (2022-08-22 13:39:29)

feat: Display correct version in User-Agent HTTP header
Signed-off-by: dark0dave <dark0dave@mykolab.com>

dark0dave: > I think it looks good in general +1 thanks.
> 
> However the `newKubeCollector` is also used in Helm v2/v3 collectors, so I assume the user agent version will need to be propagated there as well.
> 
> I would expect the tests to fail - maybe it needs rebase to catchup with some of the recent CI changes.

I will fix the tests and anything using the newKubeCollector should have the user agent set as its part of the Rest Client config.

Patch:
diff --git a/cmd/kubent/main.go b/cmd/kubent/main.go
index 093a180..fe56eb6 100644
--- a/cmd/kubent/main.go
+++ b/cmd/kubent/main.go
@@ -29,6 +29,10 @@ const (
 	EXIT_CODE_FOUND_ISSUES = 200
 )
 
+func generateUserAgent() string {
+	return fmt.Sprintf("kubent (%s/%s)", version, gitSha)
+}
+
 func getCollectors(collectors []collector.Collector) []map[string]interface{} {
 	var inputs []map[string]interface{}
 	for _, c := range collectors {
@@ -55,17 +59,17 @@ func storeCollector(collector collector.Collector, err error, collectors []colle
 func initCollectors(config *config.Config) []collector.Collector {
 	collectors := []collector.Collector{}
 	if config.Cluster {
-		collector, err := collector.NewClusterCollector(&collector.ClusterOpts{Kubeconfig: config.Kubeconfig, KubeContext: config.Context}, config.AdditionalKinds)
+		collector, err := collector.NewClusterCollector(&collector.ClusterOpts{Kubeconfig: config.Kubeconfig, KubeContext: config.Context}, config.AdditionalKinds, generateUserAgent())
 		collectors = storeCollector(collector, err, collectors)
 	}
 
 	if config.Helm2 {
-		collector, err := collector.NewHelmV2Collector(&collector.HelmV2Opts{Kubeconfig: config.Kubeconfig, KubeContext: config.Context})
+		collector, err := collector.NewHelmV2Collector(&collector.HelmV2Opts{Kubeconfig: config.Kubeconfig, KubeContext: config.Context}, generateUserAgent())
 		collectors = storeCollector(collector, err, collectors)
 	}
 
 	if config.Helm3 {
-		collector, err := collector.NewHelmV3Collector(&collector.HelmV3Opts{Kubeconfig: config.Kubeconfig, KubeContext: config.Context})
+		collector, err := collector.NewHelmV3Collector(&collector.HelmV3Opts{Kubeconfig: config.Kubeconfig, KubeContext: config.Context}, generateUserAgent())
 		collectors = storeCollector(collector, err, collectors)
 	}
 
diff --git a/pkg/collector/cluster.go b/pkg/collector/cluster.go
index 26ae0f0..3b15a78 100644
--- a/pkg/collector/cluster.go
+++ b/pkg/collector/cluster.go
@@ -2,6 +2,7 @@ package collector
 
 import (
 	"encoding/json"
+
 	"github.com/rs/zerolog/log"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -27,8 +28,8 @@ type ClusterOpts struct {
 	DiscoveryClient discovery.DiscoveryInterface
 }
 
-func NewClusterCollector(opts *ClusterOpts, additionalKinds []string) (*ClusterCollector, error) {
-	kubeCollector, err := newKubeCollector(opts.Kubeconfig, opts.KubeContext, opts.DiscoveryClient)
+func NewClusterCollector(opts *ClusterOpts, additionalKinds []string, userAgent string) (*ClusterCollector, error) {
+	kubeCollector, err := newKubeCollector(opts.Kubeconfig, opts.KubeContext, opts.DiscoveryClient, userAgent)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/collector/helm2.go b/pkg/collector/helm2.go
index 2944e3a..6ee383c 100644
--- a/pkg/collector/helm2.go
+++ b/pkg/collector/helm2.go
@@ -25,9 +25,9 @@ type HelmV2Opts struct {
 	CoreClient      corev1.CoreV1Interface
 }
 
-func NewHelmV2Collector(opts *HelmV2Opts) (*HelmV2Collector, error) {
+func NewHelmV2Collector(opts *HelmV2Opts, userAgent string) (*HelmV2Collector, error) {
 
-	kubeCollector, err := newKubeCollector(opts.Kubeconfig, opts.KubeContext, opts.DiscoveryClient)
+	kubeCollector, err := newKubeCollector(opts.Kubeconfig, opts.KubeContext, opts.DiscoveryClient, userAgent)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/collector/helm3.go b/pkg/collector/helm3.go
index a946216..44df134 100644
--- a/pkg/collector/helm3.go
+++ b/pkg/collector/helm3.go
@@ -25,8 +25,8 @@ type HelmV3Opts struct {
 	CoreClient      corev1.CoreV1Interface
 }
 
-func NewHelmV3Collector(opts *HelmV3Opts) (*HelmV3Collector, error) {
-	kubeCollector, err := newKubeCollector(opts.Kubeconfig, opts.KubeContext, opts.DiscoveryClient)
+func NewHelmV3Collector(opts *HelmV3Opts, userAgent string) (*HelmV3Collector, error) {
+	kubeCollector, err := newKubeCollector(opts.Kubeconfig, opts.KubeContext, opts.DiscoveryClient, userAgent)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/collector/kube.go b/pkg/collector/kube.go
index 2d7d298..f6e102e 100644
--- a/pkg/collector/kube.go
+++ b/pkg/collector/kube.go
@@ -15,14 +15,14 @@ type kubeCollector struct {
 	restConfig      *rest.Config
 }
 
-func newKubeCollector(kubeconfig string, kubecontext string, discoveryClient discovery.DiscoveryInterface) (*kubeCollector, error) {
+func newKubeCollector(kubeconfig string, kubecontext string, discoveryClient discovery.DiscoveryInterface, userAgent string) (*kubeCollector, error) {
 	col := &kubeCollector{}
 
 	if discoveryClient != nil {
 		col.discoveryClient = discoveryClient
 	} else {
 		var err error
-		if col.restConfig, err = newClientRestConfig(kubeconfig, kubecontext, rest.InClusterConfig); err != nil {
+		if col.restConfig, err = newClientRestConfig(kubeconfig, kubecontext, rest.InClusterConfig, userAgent); err != nil {
 			return nil, fmt.Errorf("failed to assemble client config: %w", err)
 		}
 
@@ -34,9 +34,10 @@ func newKubeCollector(kubeconfig string, kubecontext string, discoveryClient dis
 	return col, nil
 }
 
-func newClientRestConfig(kubeconfig string, kubecontext string, inClusterFn func() (*rest.Config, error)) (*rest.Config, error) {
+func newClientRestConfig(kubeconfig string, kubecontext string, inClusterFn func() (*rest.Config, error), userAgent string) (*rest.Config, error) {
 	if kubeconfig == "" {
 		if restConfig, err := inClusterFn(); err == nil {
+			restConfig.UserAgent = userAgent
 			return restConfig, nil
 		}
 	}
@@ -62,6 +63,7 @@ func newClientRestConfig(kubeconfig string, kubecontext string, inClusterFn func
 		return nil, err
 	}
 
+	restConfig.UserAgent = userAgent
 	return restConfig, nil
 }
 


Test patch:
diff --git a/pkg/collector/cluster_test.go b/pkg/collector/cluster_test.go
index 951c81c..cbac045 100644
--- a/pkg/collector/cluster_test.go
+++ b/pkg/collector/cluster_test.go
@@ -16,7 +16,7 @@ import (
 
 func TestNewClusterCollectorBadPath(t *testing.T) {
 	testOpts := ClusterOpts{Kubeconfig: "bad path"}
-	_, err := NewClusterCollector(&testOpts, []string{})
+	_, err := NewClusterCollector(&testOpts, []string{}, USER_AGENT)
 
 	if !strings.Contains(err.Error(), "no configuration has been provided") {
 		if err != nil {
@@ -36,7 +36,7 @@ func TestNewClusterCollectorValidEmptyCollector(t *testing.T) {
 		ClientSet:       clientset,
 		DiscoveryClient: &discoveryClient,
 	}
-	collector, err := NewClusterCollector(&testOpts, []string{})
+	collector, err := NewClusterCollector(&testOpts, []string{}, USER_AGENT)
 
 	if err != nil {
 		t.Fatalf("Should have parsed config instead got: %s", err)
@@ -55,7 +55,7 @@ func TestNewClusterCollectorFakeClient(t *testing.T) {
 	discoveryClient := discoveryFake.FakeDiscovery{}
 	testOpts := ClusterOpts{ClientSet: clientset, DiscoveryClient: &discoveryClient}
 
-	collector, err := NewClusterCollector(&testOpts, []string{})
+	collector, err := NewClusterCollector(&testOpts, []string{}, USER_AGENT)
 	if err != nil {
 		t.Fatalf("failed to create cluster collector from fake client: %s", err)
 	}
@@ -104,7 +104,7 @@ func TestClusterCollectorGetFake(t *testing.T) {
 			discoveryClient := discoveryFake.FakeDiscovery{}
 			testOpts := ClusterOpts{ClientSet: clientset, DiscoveryClient: &discoveryClient}
 
-			collector, err := NewClusterCollector(&testOpts, []string{})
+			collector, err := NewClusterCollector(&testOpts, []string{}, USER_AGENT)
 
 			if err != nil {
 				t.Errorf("failed to create collector from fake client: %s", err)
diff --git a/pkg/collector/helm2_test.go b/pkg/collector/helm2_test.go
index d0d63ce..1855633 100644
--- a/pkg/collector/helm2_test.go
+++ b/pkg/collector/helm2_test.go
@@ -13,7 +13,7 @@ func TestNewHelmV2Collector(t *testing.T) {
 	col, err := NewHelmV2Collector(&HelmV2Opts{
 		DiscoveryClient: clientSet.Discovery(),
 		CoreClient:      clientSet.CoreV1(),
-	})
+	}, USER_AGENT)
 	clientSet.CoreV1()
 
 	if err != nil {
diff --git a/pkg/collector/helm3_test.go b/pkg/collector/helm3_test.go
index 70742ba..3175794 100644
--- a/pkg/collector/helm3_test.go
+++ b/pkg/collector/helm3_test.go
@@ -13,7 +13,7 @@ func TestNewHelmV3Collector(t *testing.T) {
 	col, err := NewHelmV3Collector(&HelmV3Opts{
 		DiscoveryClient: clientSet.Discovery(),
 		CoreClient:      clientSet.CoreV1(),
-	})
+	}, USER_AGENT)
 
 	if err != nil {
 		t.Fatalf("Failed to create collector from fake discovery client")
diff --git a/pkg/collector/kube_test.go b/pkg/collector/kube_test.go
index 33d183e..e3f4564 100644
--- a/pkg/collector/kube_test.go
+++ b/pkg/collector/kube_test.go
@@ -1,11 +1,12 @@
 package collector
 
 import (
-	"k8s.io/client-go/rest"
 	"os"
 	"path/filepath"
 	"testing"
 
+	"k8s.io/client-go/rest"
+
 	goversion "github.com/hashicorp/go-version"
 	"k8s.io/apimachinery/pkg/version"
 	discoveryFake "k8s.io/client-go/discovery/fake"
@@ -13,10 +14,13 @@ import (
 )
 
 const FIXTURES_DIR = "../../fixtures"
+const USER_AGENT = "testUserAgent"
+const BASIC_CONFIG = "kube.config.basic"
+const CONTEXT = "kube.config.context"
 
 func TestNewKubeCollector(t *testing.T) {
 	clientSet := fake.NewSimpleClientset()
-	col, err := newKubeCollector("", "", clientSet.Discovery())
+	col, err := newKubeCollector("", "", clientSet.Discovery(), USER_AGENT)
 
 	if err != nil {
 		t.Fatalf("Failed to create kubeCollector from fake discovery client")
@@ -27,7 +31,7 @@ func TestNewKubeCollector(t *testing.T) {
 }
 
 func TestNewKubeCollectorWithKubeconfigPath(t *testing.T) {
-	col, err := newKubeCollector(filepath.Join(FIXTURES_DIR, "kube.config.basic"), "", nil)
+	col, err := newKubeCollector(filepath.Join(FIXTURES_DIR, CONTEXT), "", nil, USER_AGENT)
 
 	if err != nil {
 		t.Fatalf("Failed to create kubeCollector from fake discovery client")
@@ -44,7 +48,7 @@ func TestNewKubeCollectorWithKubeconfigPath(t *testing.T) {
 }
 
 func TestNewKubeCollectorError(t *testing.T) {
-	_, err := newKubeCollector("does-not-exist", "", nil)
+	_, err := newKubeCollector("does-not-exist", "", nil, USER_AGENT)
 
 	if err == nil {
 		t.Errorf("Expected to fail with non-existent kubeconfig")
@@ -60,7 +64,7 @@ func TestGetServerVersion(t *testing.T) {
 		GitVersion: gitVersion,
 	}
 
-	collector, err := newKubeCollector("", "", clientSet.Discovery())
+	collector, err := newKubeCollector("", "", clientSet.Discovery(), USER_AGENT)
 	if err != nil {
 		t.Fatalf("failed to create kubeCollector from fake client: %s", err)
 	}
@@ -79,7 +83,7 @@ func TestContext(t *testing.T) {
 	expectedContext := "test-context"
 	expectedHost := "https://test-cluster"
 
-	collector, err := newKubeCollector(filepath.Join(FIXTURES_DIR, "kube.config.context"), expectedContext, nil)
+	collector, err := newKubeCollector(filepath.Join(FIXTURES_DIR, CONTEXT), expectedContext, nil, USER_AGENT)
 	if err != nil {
 		t.Fatalf("Failed to create kubeCollector from fake client with context %s: %s", expectedContext, err)
 	}
@@ -93,14 +97,14 @@ func TestContext(t *testing.T) {
 func TestContextMissing(t *testing.T) {
 	expectedContext := "non-existent"
 
-	_, err := newKubeCollector(filepath.Join(FIXTURES_DIR, "kube.config.context"), expectedContext, nil)
+	_, err := newKubeCollector(filepath.Join(FIXTURES_DIR, CONTEXT), expectedContext, nil, USER_AGENT)
 	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)
+	_, err := newClientRestConfig("does-not-exist", "", rest.InClusterConfig, USER_AGENT)
 
 	if err == nil {
 		t.Errorf("Expected to fail with non-existent kubeconfig")
@@ -108,7 +112,7 @@ func TestNewClientRestConfigError(t *testing.T) {
 }
 
 func TestNewClientRestConfigWithKubeconfigPath(t *testing.T) {
-	_, err := newClientRestConfig(filepath.Join(FIXTURES_DIR, "kube.config.basic"), "", rest.InClusterConfig)
+	_, err := newClientRestConfig(filepath.Join(FIXTURES_DIR, BASIC_CONFIG), "", rest.InClusterConfig, USER_AGENT)
 	if err != nil {
 		t.Fatalf("Failed with: %s", err)
 	}
@@ -116,11 +120,11 @@ func TestNewClientRestConfigWithKubeconfigPath(t *testing.T) {
 
 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"),
+		"KUBECONFIG": filepath.Join(FIXTURES_DIR, "kube.config.empty") + ":" + filepath.Join(FIXTURES_DIR, BASIC_CONFIG),
 	})
 	defer env.reset()
 
-	if _, err := newClientRestConfig("", "", rest.InClusterConfig); err != nil {
+	if _, err := newClientRestConfig("", "", rest.InClusterConfig, USER_AGENT); err != nil {
 		t.Fatalf("Failed with: %s", err)
 	}
 }
@@ -129,7 +133,7 @@ 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)
+	config, err := newClientRestConfig(filepath.Join(FIXTURES_DIR, CONTEXT), expectedContext, rest.InClusterConfig, USER_AGENT)
 	if err != nil {
 		t.Fatalf("Failed to create kubeCollector from fake client with context %s: %s", expectedContext, err)
 	}
@@ -142,7 +146,7 @@ func TestNewClientRestConfigWithContext(t *testing.T) {
 func TestNewClientRestConfigContextMissing(t *testing.T) {
 	expectedContext := "non-existent"
 
-	_, err := newClientRestConfig(filepath.Join(FIXTURES_DIR, "kube.config.context"), expectedContext, rest.InClusterConfig)
+	_, err := newClientRestConfig(filepath.Join(FIXTURES_DIR, CONTEXT), expectedContext, rest.InClusterConfig, USER_AGENT)
 	if err == nil {
 		t.Fatalf("Expected to fail when uisng non-existent context: %s", expectedContext)
 	}
@@ -154,7 +158,7 @@ func TestNewClientRestConfigInCluster(t *testing.T) {
 		return &rest.Config{Host: expectedHost}, nil
 	}
 
-	cfg, err := newClientRestConfig("", "", inClusterFn)
+	cfg, err := newClientRestConfig("", "", inClusterFn, USER_AGENT)
 	if err != nil {
 		t.Fatalf("Failed to create in-cluster config: %s", err)
 	}