Repository: replicatedhq/troubleshoot
Pull request #1056 (2023-03-20 20:14:06)

feat(analyzer): remove duplicated analyzers
## Description, Motivation and Context
- add DedupAnalyzers to remove duplicated analyzers in spec

<!--- If it relates to an open issue, please link to the issue here.
e.g.
Fixes: #414
-->

## Checklist

- [x] New and existing tests pass locally with introduced changes.
- [x] Tests for the changes have been added (for bug fixes / features)
- [ ] The commit message(s) are informative and highlight any breaking changes
- [ ] Any documentation required has been added/updated. For changes to https://troubleshoot.sh/ create a PR [here](https://github.com/replicatedhq/troubleshoot.sh/pulls)

## Does this PR introduce a breaking change?
- [ ] Yes
- [x] No

<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. -->



Patch:
diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go
index 8d9244c4..d26d3637 100644
--- a/cmd/troubleshoot/cli/run.go
+++ b/cmd/troubleshoot/cli/run.go
@@ -114,11 +114,6 @@ func runTroubleshoot(v *viper.Viper, arg []string) error {
 			return errors.Wrap(err, "unable to parse selector")
 		}
 
-		namespace := ""
-		if v.GetString("namespace") != "" {
-			namespace = v.GetString("namespace")
-		}
-
 		config, err := k8sutil.GetRESTConfig()
 		if err != nil {
 			return errors.Wrap(err, "failed to convert kube flags to rest config")
@@ -129,6 +124,22 @@ func runTroubleshoot(v *viper.Viper, arg []string) error {
 			return errors.Wrap(err, "failed to convert create k8s client")
 		}
 
+		namespace := ""
+
+		if v.GetString("namespace") != "" {
+			namespace = v.GetString("namespace")
+		} else {
+			IsNamespacedScopeRBAC, err := k8sutil.IsNamespacedScopeRBAC(client)
+			if err != nil {
+				return errors.Wrap(err, "failed to check if cluster is namespaced")
+			}
+
+			if !IsNamespacedScopeRBAC {
+				kubeconfig := k8sutil.GetKubeconfig()
+				namespace, _, _ = kubeconfig.Namespace()
+			}
+		}
+
 		var bundlesFromCluster []string
 
 		// Search cluster for Troubleshoot objects in cluster
diff --git a/pkg/analyze/analyzer.go b/pkg/analyze/analyzer.go
index 9fd72878..d6693988 100644
--- a/pkg/analyze/analyzer.go
+++ b/pkg/analyze/analyzer.go
@@ -2,6 +2,7 @@ package analyzer
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"reflect"
 	"strconv"
@@ -235,3 +236,26 @@ func getAnalyzer(analyzer *troubleshootv1beta2.Analyze) Analyzer {
 		return nil
 	}
 }
+
+// deduplicates a list of troubleshootv1beta2.Analyze objects
+// marshals object to json and then uses its string value to check for uniqueness
+// there is no sorting of the keys in the analyze object's spec so if the spec isn't an exact match line for line as written, no dedup will occur
+func DedupAnalyzers(allAnalyzers []*troubleshootv1beta2.Analyze) []*troubleshootv1beta2.Analyze {
+	uniqueAnalyzers := make(map[string]bool)
+	finalAnalyzers := []*troubleshootv1beta2.Analyze{}
+
+	for _, analyzer := range allAnalyzers {
+		data, err := json.Marshal(analyzer)
+		if err != nil {
+			// return analyzer as is if for whatever reason it can't be marshalled into json
+			finalAnalyzers = append(finalAnalyzers, analyzer)
+		} else {
+			stringData := string(data)
+			if _, value := uniqueAnalyzers[stringData]; !value {
+				uniqueAnalyzers[stringData] = true
+				finalAnalyzers = append(finalAnalyzers, analyzer)
+			}
+		}
+	}
+	return finalAnalyzers
+}
diff --git a/pkg/k8sutil/namespace.go b/pkg/k8sutil/namespace.go
new file mode 100644
index 00000000..0d6ec0b3
--- /dev/null
+++ b/pkg/k8sutil/namespace.go
@@ -0,0 +1,34 @@
+package k8sutil
+
+import (
+	"context"
+
+	authorizationv1 "k8s.io/api/authorization/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+)
+
+func IsNamespacedScopeRBAC(client kubernetes.Interface) (bool, error) {
+	ctx := context.Background()
+
+	sar := &authorizationv1.SelfSubjectAccessReview{
+		Spec: authorizationv1.SelfSubjectAccessReviewSpec{
+			ResourceAttributes: &authorizationv1.ResourceAttributes{
+				Namespace: "",
+				Verb:      "list",
+				Resource:  "secrets,configmaps",
+			},
+		},
+	}
+
+	resp, err := client.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, sar, metav1.CreateOptions{})
+	if err != nil {
+		return false, err
+	}
+
+	if resp.Status.Allowed {
+		return true, nil
+	} else {
+		return false, nil
+	}
+}
diff --git a/pkg/supportbundle/supportbundle.go b/pkg/supportbundle/supportbundle.go
index 9783b923..019fa30f 100644
--- a/pkg/supportbundle/supportbundle.go
+++ b/pkg/supportbundle/supportbundle.go
@@ -255,6 +255,7 @@ func AnalyzeSupportBundle(ctx context.Context, spec *troubleshootv1beta2.Support
 	if len(spec.Analyzers) == 0 && len(spec.HostAnalyzers) == 0 {
 		return nil, nil
 	}
+	spec.Analyzers = analyzer.DedupAnalyzers(spec.Analyzers)
 	analyzeResults, err := analyzer.AnalyzeLocal(ctx, tmpDir, spec.Analyzers, spec.HostAnalyzers)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to analyze support bundle")


Test patch:
diff --git a/pkg/analyze/analyzer_test.go b/pkg/analyze/analyzer_test.go
index 59739664..1832cb73 100644
--- a/pkg/analyze/analyzer_test.go
+++ b/pkg/analyze/analyzer_test.go
@@ -72,3 +72,97 @@ func TestAnalyzeWithNilAnalyzer(t *testing.T) {
 	assert.Error(t, err)
 	assert.Nil(t, got)
 }
+
+func TestCollector_DedupCollectors(t *testing.T) {
+	tests := []struct {
+		name      string
+		Analyzers []*troubleshootv1beta2.Analyze
+		want      []*troubleshootv1beta2.Analyze
+	}{
+		{
+			name: "multiple ClusterVersion",
+			Analyzers: []*troubleshootv1beta2.Analyze{
+				{
+					ClusterVersion: &troubleshootv1beta2.ClusterVersion{},
+				},
+				{
+					ClusterVersion: &troubleshootv1beta2.ClusterVersion{},
+				},
+			},
+			want: []*troubleshootv1beta2.Analyze{
+				{
+					ClusterVersion: &troubleshootv1beta2.ClusterVersion{},
+				},
+			},
+		},
+		{
+			name: "multiple TextAnalyze",
+			Analyzers: []*troubleshootv1beta2.Analyze{
+				{
+					TextAnalyze: &troubleshootv1beta2.TextAnalyze{
+						AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{
+							CheckName: "hi",
+						},
+					},
+				},
+				{
+					TextAnalyze: &troubleshootv1beta2.TextAnalyze{
+						AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{
+							CheckName: "hi",
+						},
+					},
+				},
+			},
+			want: []*troubleshootv1beta2.Analyze{
+				{
+					TextAnalyze: &troubleshootv1beta2.TextAnalyze{
+						AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{
+							CheckName: "hi",
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "multiple TextAnalyze with different CheckName",
+			Analyzers: []*troubleshootv1beta2.Analyze{
+				{
+					TextAnalyze: &troubleshootv1beta2.TextAnalyze{
+						AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{
+							CheckName: "hi",
+						},
+					},
+				},
+				{
+					TextAnalyze: &troubleshootv1beta2.TextAnalyze{
+						AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{
+							CheckName: "test",
+						},
+					},
+				},
+			},
+			want: []*troubleshootv1beta2.Analyze{
+				{
+					TextAnalyze: &troubleshootv1beta2.TextAnalyze{
+						AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{
+							CheckName: "hi",
+						},
+					},
+				},
+				{
+					TextAnalyze: &troubleshootv1beta2.TextAnalyze{
+						AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{
+							CheckName: "test",
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := DedupAnalyzers(tc.Analyzers)
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}