feat(support-bundle): add support labels for troubleshoot.io and troubleshoot.sh (#1203)

Author: DexterYan

cmd/troubleshoot/cli/root.go

@@ -68,7 +68,7 @@ from a server that can be used to assist when troubleshooting a Kubernetes clust
 	cmd.Flags().Bool("redact", true, "enable/disable default redactions")
 	cmd.Flags().Bool("interactive", true, "enable/disable interactive mode")
 	cmd.Flags().Bool("collect-without-permissions", true, "always generate a support bundle, even if it some require additional permissions")
-	cmd.Flags().StringSliceP("selector", "l", []string{"troubleshoot.io/kind=support-bundle"}, "selector to filter on for loading additional support bundle specs found in secrets within the cluster")
+	cmd.Flags().StringSliceP("selector", "l", []string{"troubleshoot.sh/kind=support-bundle"}, "selector to filter on for loading additional support bundle specs found in secrets within the cluster")
 	cmd.Flags().Bool("load-cluster-specs", false, "enable/disable loading additional troubleshoot specs found within the cluster. required when no specs are provided on the command line")
 	cmd.Flags().String("since-time", "", "force pod logs collectors to return logs after a specific date (RFC3339)")
 	cmd.Flags().String("since", "", "force pod logs collectors to return logs newer than a relative duration like 5s, 2m, or 3h.")

cmd/troubleshoot/cli/run.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"os/signal"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"sync"
 	"time"
@@ -19,6 +20,7 @@ import (
 	"github.com/pkg/errors"
 	analyzer "github.com/replicatedhq/troubleshoot/pkg/analyze"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"github.com/replicatedhq/troubleshoot/pkg/constants"
 	"github.com/replicatedhq/troubleshoot/pkg/convert"
 	"github.com/replicatedhq/troubleshoot/pkg/httputil"
 	"github.com/replicatedhq/troubleshoot/pkg/k8sutil"
@@ -257,7 +259,7 @@ the %s Admin Console to begin analysis.`
 }
 
 // loadClusterSpecs loads the support bundle and redactor specs from the cluster
-// based on troubleshoot.io/kind=support-bundle label selector. We search for secrets
+// based on the provided labels. By default this will be troubleshoot.io/kind=support-bundle and troubleshoot.sh/kind=support-bundle label selectors. We search for secrets
 // and configmaps with the label selector and parse the data as a support bundle. If the
 // user does not have sufficient permissions to list & read secrets and configmaps from
 // all namespaces, we will fallback to trying each namespace individually, and eventually
@@ -269,7 +271,13 @@ func loadClusterSpecs() (*troubleshootv1beta2.SupportBundle, *troubleshootv1beta
 
 	klog.Info("Discover troubleshoot specs from cluster")
 
-	labelSelector := strings.Join(v.GetStringSlice("selector"), ",")
+	selectors := v.GetStringSlice("selector")
+	if reflect.DeepEqual(selectors, []string{"troubleshoot.sh/kind=support-bundle"}) {
+		// Its the default selector so we append troubleshoot.io/kind=support-bundle to it due to backwards compatibility
+		selectors = append(selectors, "troubleshoot.io/kind=support-bundle")
+	}
+
+	labelSelector := strings.Join(selectors, ",")
 
 	parsedSelector, err := labels.Parse(labelSelector)
 	if err != nil {
@@ -331,28 +339,35 @@ func loadClusterSpecs() (*troubleshootv1beta2.SupportBundle, *troubleshootv1beta
 
 	var bundlesFromCluster []string
 
+	parsedSelectorStrings, err := specs.SplitTroubleshootSecretLabelSelector(client, parsedSelector)
+	if err != nil {
+		klog.Errorf("failed to parse troubleshoot labels selector %s", err)
+	}
+
 	// Search cluster for support bundle specs
-	klog.V(1).Infof("Search support bundle specs from [%q] namespaces using %q selector", strings.Join(namespaces, ", "), parsedSelector.String())
-	for _, ns := range namespaces {
-		bundlesFromSecrets, err := specs.LoadFromSecretMatchingLabel(client, parsedSelector.String(), ns, specs.SupportBundleKey)
-		if err != nil {
-			if !k8serrors.IsForbidden(err) {
-				klog.Errorf("failed to load support bundle spec from secrets: %s", err)
-			} else {
-				klog.Warningf("Reading secrets from %q namespace forbidden", ns)
+	for _, parsedSelectorString := range parsedSelectorStrings {
+		klog.V(1).Infof("Search support bundle specs from [%q] namespace using %q selector", strings.Join(namespaces, ", "), parsedSelectorString)
+		for _, ns := range namespaces {
+			bundlesFromSecrets, err := specs.LoadFromSecretMatchingLabel(client, parsedSelectorString, ns, constants.SupportBundleKey)
+			if err != nil {
+				if !k8serrors.IsForbidden(err) {
+					klog.Errorf("failed to load support bundle spec from secrets: %s", err)
+				} else {
+					klog.Warningf("Reading secrets from %q namespace forbidden", ns)
+				}
 			}
-		}
-		bundlesFromCluster = append(bundlesFromCluster, bundlesFromSecrets...)
+			bundlesFromCluster = append(bundlesFromCluster, bundlesFromSecrets...)
 
-		bundlesFromConfigMaps, err := specs.LoadFromConfigMapMatchingLabel(client, parsedSelector.String(), ns, specs.SupportBundleKey)
-		if err != nil {
-			if !k8serrors.IsForbidden(err) {
-				klog.Errorf("failed to load support bundle spec from configmap: %s", err)
-			} else {
-				klog.Warningf("Reading configmaps from %q namespace forbidden", ns)
+			bundlesFromConfigMaps, err := specs.LoadFromConfigMapMatchingLabel(client, parsedSelectorString, ns, constants.SupportBundleKey)
+			if err != nil {
+				if !k8serrors.IsForbidden(err) {
+					klog.Errorf("failed to load support bundle spec from configmap: %s", err)
+				} else {
+					klog.Warningf("Reading configmaps from %q namespace forbidden", ns)
+				}
 			}
+			bundlesFromCluster = append(bundlesFromCluster, bundlesFromConfigMaps...)
 		}
-		bundlesFromCluster = append(bundlesFromCluster, bundlesFromConfigMaps...)
 	}
 
 	parsedBundle := &troubleshootv1beta2.SupportBundle{}
@@ -379,27 +394,29 @@ func loadClusterSpecs() (*troubleshootv1beta2.SupportBundle, *troubleshootv1beta
 	var redactorsFromCluster []string
 
 	// Search cluster for redactor specs
-	klog.V(1).Infof("Search redactor specs from [%q] namespaces using %q selector", strings.Join(namespaces, ", "), parsedSelector.String())
-	for _, ns := range namespaces {
-		redactorsFromSecrets, err := specs.LoadFromSecretMatchingLabel(client, parsedSelector.String(), ns, specs.RedactorKey)
-		if err != nil {
-			if !k8serrors.IsForbidden(err) {
-				klog.Errorf("failed to load support bundle spec from secrets: %s", err)
-			} else {
-				klog.Warningf("Reading secrets from %q namespace forbidden", ns)
+	for _, parsedSelectorString := range parsedSelectorStrings {
+		klog.V(1).Infof("Search redactor specs from [%q] namespace using %q selector", strings.Join(namespaces, ", "), parsedSelectorString)
+		for _, ns := range namespaces {
+			redactorsFromSecrets, err := specs.LoadFromSecretMatchingLabel(client, parsedSelectorString, ns, constants.RedactorKey)
+			if err != nil {
+				if !k8serrors.IsForbidden(err) {
+					klog.Errorf("failed to load support bundle spec from secrets: %s", err)
+				} else {
+					klog.Warningf("Reading secrets from %q namespace forbidden", ns)
+				}
 			}
-		}
-		redactorsFromCluster = append(redactorsFromCluster, redactorsFromSecrets...)
+			redactorsFromCluster = append(redactorsFromCluster, redactorsFromSecrets...)
 
-		redactorsFromConfigMaps, err := specs.LoadFromConfigMapMatchingLabel(client, parsedSelector.String(), ns, specs.RedactorKey)
-		if err != nil {
-			if !k8serrors.IsForbidden(err) {
-				klog.Errorf("failed to load support bundle spec from configmap: %s", err)
-			} else {
-				klog.Warningf("Reading configmaps from %q namespace forbidden", ns)
+			redactorsFromConfigMaps, err := specs.LoadFromConfigMapMatchingLabel(client, parsedSelectorString, ns, constants.RedactorKey)
+			if err != nil {
+				if !k8serrors.IsForbidden(err) {
+					klog.Errorf("failed to load support bundle spec from configmap: %s", err)
+				} else {
+					klog.Warningf("Reading configmaps from %q namespace forbidden", ns)
+				}
 			}
+			redactorsFromCluster = append(redactorsFromCluster, redactorsFromConfigMaps...)
 		}
-		redactorsFromCluster = append(redactorsFromCluster, redactorsFromConfigMaps...)
 	}
 
 	for _, redactor := range redactorsFromCluster {

docs/preflight.md

@@ -28,6 +28,7 @@ preflight [url] [flags]
       --context string                 The name of the kubeconfig context to use
       --cpuprofile string              File path to write cpu profiling data
       --debug                          enable debug logging
+      --disable-compression            If true, opt-out of response compression for all requests to the server
       --format string                  output format, one of human, json, yaml. only used when interactive is set to false (default "human")
   -h, --help                           help for preflight
       --insecure-skip-tls-verify       If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure
@@ -44,10 +45,12 @@ preflight [url] [flags]
       --tls-server-name string         Server name to use for server certificate validation. If it is not provided, the hostname used to contact the server is used
       --token string                   Bearer token for authentication to the API server
       --user string                    The name of the kubeconfig user to use
+  -v, --v Level                        number for the log level verbosity
 ```
 
 ### SEE ALSO
 
+* [preflight oci-fetch](preflight_oci-fetch.md)	 - Fetch a preflight from an OCI registry and print it to standard out
 * [preflight version](preflight_version.md)	 - Print the current version and exit
 
-###### Auto generated by spf13/cobra on 3-Jan-2023
+###### Auto generated by spf13/cobra on 8-Jun-2023

docs/preflight_oci-fetch.md

@@ -0,0 +1,36 @@
+## preflight oci-fetch
+
+Fetch a preflight from an OCI registry and print it to standard out
+
+```
+preflight oci-fetch [URI] [flags]
+```
+
+### Options
+
+```
+  -h, --help   help for oci-fetch
+```
+
+### Options inherited from parent commands
+
+```
+      --collect-without-permissions   always run preflight checks even if some require permissions that preflight does not have (default true)
+      --collector-image string        the full name of the collector image to use
+      --collector-pullpolicy string   the pull policy of the collector image
+      --cpuprofile string             File path to write cpu profiling data
+      --debug                         enable debug logging
+      --format string                 output format, one of human, json, yaml. only used when interactive is set to false (default "human")
+      --interactive                   interactive preflights (default true)
+      --memprofile string             File path to write memory profiling data
+  -o, --output string                 specify the output file path for the preflight checks
+      --selector string               selector (label query) to filter remote collection nodes on.
+      --since string                  force pod logs collectors to return logs newer than a relative duration like 5s, 2m, or 3h.
+      --since-time string             force pod logs collectors to return logs after a specific date (RFC3339)
+```
+
+### SEE ALSO
+
+* [preflight](preflight.md)	 - Run and retrieve preflight checks in a cluster
+
+###### Auto generated by spf13/cobra on 8-Jun-2023

docs/support-bundle.md

@@ -25,7 +25,8 @@ support-bundle [urls...] [flags]
       --collect-without-permissions    always generate a support bundle, even if it some require additional permissions (default true)
       --context string                 The name of the kubeconfig context to use
       --cpuprofile string              File path to write cpu profiling data
-      --debug                          enable debug logging
+      --debug                          enable debug logging. This is equivalent to --v=0
+      --disable-compression            If true, opt-out of response compression for all requests to the server
   -h, --help                           help for support-bundle
       --insecure-skip-tls-verify       If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure
       --interactive                    enable/disable interactive mode (default true)
@@ -38,13 +39,14 @@ support-bundle [urls...] [flags]
       --redact                         enable/disable default redactions (default true)
       --redactors strings              names of the additional redactors to use
       --request-timeout string         The length of time to wait before giving up on a single server request. Non-zero values should contain a corresponding time unit (e.g. 1s, 2m, 3h). A value of zero means don't timeout requests. (default "0")
-  -l, --selector strings               selector to filter on for loading additional support bundle specs found in secrets within the cluster (default [troubleshoot.io/kind=support-bundle])
+  -l, --selector strings               selector to filter on for loading additional support bundle specs found in secrets within the cluster (default [troubleshoot.sh/kind=support-bundle])
   -s, --server string                  The address and port of the Kubernetes API server
       --since string                   force pod logs collectors to return logs newer than a relative duration like 5s, 2m, or 3h.
       --since-time string              force pod logs collectors to return logs after a specific date (RFC3339)
       --tls-server-name string         Server name to use for server certificate validation. If it is not provided, the hostname used to contact the server is used
       --token string                   Bearer token for authentication to the API server
       --user string                    The name of the kubeconfig user to use
+  -v, --v Level                        number for the log level verbosity
 ```
 
 ### SEE ALSO
@@ -53,4 +55,4 @@ support-bundle [urls...] [flags]
 * [support-bundle redact](support-bundle_redact.md)	 - Redact information from a generated support bundle archive
 * [support-bundle version](support-bundle_version.md)	 - Print the current version and exit
 
-###### Auto generated by spf13/cobra on 3-Jan-2023
+###### Auto generated by spf13/cobra on 8-Jun-2023

pkg/constants/constants.go

@@ -59,9 +59,11 @@ const (
 	EXIT_CODE_WARN        = 4
 
 	// Troubleshoot label constants
-	SupportBundleKey = "support-bundle-spec"
-	RedactorKey      = "redactor-spec"
-	PreflightKey     = "preflight.yaml" // Shouldn't this be "preflight-spec"?
+	SupportBundleKey       = "support-bundle-spec"
+	RedactorKey            = "redactor-spec"
+	TroubleshootIOLabelKey = "troubleshoot.io/kind"
+	TroubleshootSHLabelKey = "troubleshoot.sh/kind"
+	PreflightKey           = "preflight.yaml" // Shouldn't this be "preflight-spec"?
 
 	// Troubleshoot spec constants
 	Troubleshootv1beta2Kind = "troubleshoot.sh/v1beta2"

pkg/specs/specs.go

@@ -1,12 +1,46 @@
 package specs
 
-import "github.com/replicatedhq/troubleshoot/pkg/constants"
+import (
+	"github.com/pkg/errors"
+	"github.com/replicatedhq/troubleshoot/pkg/constants"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/klog/v2"
+)
 
-const (
-	// Preserved for backwards compatibility
+// SplitTroubleshootSecretLabelSelector splits a label selector into two selectors, if applicable:
+// 1. troubleshoot.io/kind=support-bundle and non-troubleshoot (if contains) labels selector.
+// 2. troubleshoot.sh/kind=support-bundle and non-troubleshoot (if contains) labels selector.
+func SplitTroubleshootSecretLabelSelector(client kubernetes.Interface, labelSelector labels.Selector) ([]string, error) {
 
-	// Deprecated: Use constants.SupportBundleKey instead
-	SupportBundleKey = constants.SupportBundleKey
-	// Deprecated: Use constants.RedactorKey instead
-	RedactorKey = constants.RedactorKey
-)
+	klog.V(1).Infof("Split %q selector into troubleshoot and non-troubleshoot labels selector separately, if applicable", labelSelector.String())
+
+	selectorRequirements, selectorSelectable := labelSelector.Requirements()
+	if !selectorSelectable {
+		return nil, errors.Errorf("Selector %q is not selectable", labelSelector.String())
+	}
+
+	var troubleshootReqs, otherReqs []labels.Requirement
+
+	for _, req := range selectorRequirements {
+		if req.Key() == constants.TroubleshootIOLabelKey || req.Key() == constants.TroubleshootSHLabelKey {
+			troubleshootReqs = append(troubleshootReqs, req)
+		} else {
+			otherReqs = append(otherReqs, req)
+		}
+	}
+
+	parsedSelectorStrings := make([]string, 0)
+	// Combine each troubleshoot requirement with other requirements to form new selectors
+	if len(troubleshootReqs) == 0 {
+		return []string{labelSelector.String()}, nil
+	}
+
+	for _, tReq := range troubleshootReqs {
+		reqs := append(otherReqs, tReq)
+		newSelector := labels.NewSelector().Add(reqs...)
+		parsedSelectorStrings = append(parsedSelectorStrings, newSelector.String())
+	}
+
+	return parsedSelectorStrings, nil
+}

pkg/specs/specs_test.go

@@ -0,0 +1,63 @@
+package specs
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/apimachinery/pkg/labels"
+)
+
+func Test_SplitTroubleshootSecretLabelSelector(t *testing.T) {
+	tests := []struct {
+		name              string
+		selectorString    string
+		expectedSelectors []string
+		expectedError     bool
+	}{
+		{
+			name:           "Split both troubleshoot and non-troubleshoot labels",
+			selectorString: "troubleshoot.io/kind=support-bundle,troubleshoot.sh/kind=support-bundle,a=b",
+			expectedSelectors: []string{
+				"a=b,troubleshoot.io/kind=support-bundle",
+				"a=b,troubleshoot.sh/kind=support-bundle",
+			},
+			expectedError: false,
+		},
+		{
+			name:              "Split only troubleshoot.io label",
+			selectorString:    "troubleshoot.io/kind=support-bundle",
+			expectedSelectors: []string{"troubleshoot.io/kind=support-bundle"},
+			expectedError:     false,
+		},
+		{
+			name:              "Split only troubleshoot.sh label",
+			selectorString:    "troubleshoot.sh/kind=support-bundle",
+			expectedSelectors: []string{"troubleshoot.sh/kind=support-bundle"},
+			expectedError:     false,
+		},
+		{
+			name:              "Split only non-troubleshoot label",
+			selectorString:    "a=b",
+			expectedSelectors: []string{"a=b"},
+			expectedError:     false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			selector, err := labels.Parse(tt.selectorString)
+			if err != nil {
+				t.Errorf("Error parsing selector string: %v", err)
+				return
+			}
+
+			gotSelectors, err := SplitTroubleshootSecretLabelSelector(nil, selector)
+			if (err != nil) != tt.expectedError {
+				t.Errorf("Expected error: %v, got: %v", tt.expectedError, err)
+				return
+			}
+
+			assert.ElementsMatch(t, tt.expectedSelectors, gotSelectors)
+		})
+	}
+}

test/validate-support-bundle-e2e.sh

@@ -99,3 +99,15 @@ if ! grep "labelled-support-bundle-2 \*\*\*HIDDEN\*\*\*" "$tmpdir/$bundle_direct
     echo "Hidden content not found in redacted echo-hi-2 file"
     exit 1
 fi
+
+if ! grep "labelled-support-bundle-3 \*\*\*HIDDEN\*\*\*" "$tmpdir/$bundle_directory_name/echo-hi-3"; then
+    echo "$(cat $tmpdir/$bundle_directory_name/echo-hi-3)"
+    echo "Hidden content not found in redacted echo-hi-3 file"
+    exit 1
+fi
+
+if ! grep "labelled-support-bundle-4 \*\*\*HIDDEN\*\*\*" "$tmpdir/$bundle_directory_name/echo-hi-4"; then
+    echo "$(cat $tmpdir/$bundle_directory_name/echo-hi-4)"
+    echo "Hidden content not found in redacted echo-hi-4 file"
+    exit 1
+fi