feat: Add dry run flag to print support bundle specs to std out (#1337)

* Add dry-run flag

* No traces on dry run

* More refactoring

* More updates to support bundle binary

* More refactoring changes

* Different approach of loading specs from URIs

* Self review

* More changes after review and testing

* fix how we parse oci image uri

* Remove unnecessary comment

* Add missing file

* Fix failing tests

* Better error check for no collectors

* Add default collectors when parsing support bundle specs

* Add missed test fixture

* Download specs with correct headers

* Fix typo

cmd/preflight/cli/oci_fetch.go

@@ -2,9 +2,12 @@ package cli
 
 import (
 	"fmt"
+	"strings"
 
+	"github.com/replicatedhq/troubleshoot/pkg/logger"
 	"github.com/replicatedhq/troubleshoot/pkg/oci"
 	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
 )
 
 func OciFetchCmd() *cobra.Command {
@@ -12,6 +15,13 @@ func OciFetchCmd() *cobra.Command {
 		Use:   "oci-fetch [URI]",
 		Args:  cobra.MinimumNArgs(1),
 		Short: "Fetch a preflight from an OCI registry and print it to standard out",
+		PreRun: func(cmd *cobra.Command, args []string) {
+			v := viper.GetViper()
+			v.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
+			v.BindPFlags(cmd.Flags())
+
+			logger.SetupLogger(v)
+		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			uri := args[0]
 			data, err := oci.PullPreflightFromOCI(uri)
@@ -22,5 +32,9 @@ func OciFetchCmd() *cobra.Command {
 			return nil
 		},
 	}
+
+	// Initialize klog flags
+	logger.InitKlogFlags(cmd)
+
 	return cmd
 }

cmd/preflight/cli/root.go

@@ -49,7 +49,7 @@ that a cluster meets the requirements to run an application.`,
 			}
 
 			err = preflight.RunPreflights(v.GetBool("interactive"), v.GetString("output"), v.GetString("format"), args)
-			if v.GetBool("debug") || v.IsSet("v") {
+			if !v.GetBool("dry-run") && (v.GetBool("debug") || v.IsSet("v")) {
 				fmt.Printf("\n%s", traces.GetExporterInstance().GetSummary())
 			}
 

cmd/troubleshoot/cli/root.go

@@ -45,7 +45,7 @@ from a server that can be used to assist when troubleshooting a Kubernetes clust
 			}
 
 			err = runTroubleshoot(v, args)
-			if v.GetBool("debug") || v.IsSet("v") {
+			if !v.IsSet("dry-run") && (v.GetBool("debug") || v.IsSet("v")) {
 				fmt.Printf("\n%s", traces.GetExporterInstance().GetSummary())
 			}
 
@@ -74,6 +74,7 @@ from a server that can be used to assist when troubleshooting a Kubernetes clust
 	cmd.Flags().String("since", "", "force pod logs collectors to return logs newer than a relative duration like 5s, 2m, or 3h.")
 	cmd.Flags().StringP("output", "o", "", "specify the output file path for the support bundle")
 	cmd.Flags().Bool("debug", false, "enable debug logging. This is equivalent to --v=0")
+	cmd.Flags().Bool("dry-run", false, "print support bundle spec without collecting anything")
 
 	// hidden in favor of the `insecure-skip-tls-verify` flag
 	cmd.Flags().Bool("allow-insecure-connections", false, "when set, do not verify TLS certs when retrieving spec and reporting results")
@@ -81,7 +82,7 @@ from a server that can be used to assist when troubleshooting a Kubernetes clust
 
 	// `no-uri` references the `followURI` functionality where we can use an upstream spec when creating a support bundle
 	// This flag makes sure we can also disable this and fall back to the default spec.
-	cmd.Flags().Bool("no-uri", false, "When this flag is used, Troubleshoot does not attempt to retrieve the bundle referenced by the uri: field in the spec.`")
+	cmd.Flags().Bool("no-uri", false, "When this flag is used, Troubleshoot does not attempt to retrieve the spec referenced by the uri: field`")
 
 	k8sutil.AddFlags(cmd.Flags())
 

cmd/troubleshoot/cli/run.go

@@ -9,7 +9,6 @@ import (
 	"os"
 	"os/signal"
 	"path/filepath"
-	"strings"
 	"sync"
 	"time"
 
@@ -17,27 +16,65 @@ import (
 	"github.com/fatih/color"
 	"github.com/mattn/go-isatty"
 	"github.com/pkg/errors"
-	privSpecs "github.com/replicatedhq/troubleshoot/internal/specs"
+	"github.com/replicatedhq/troubleshoot/internal/specs"
+	"github.com/replicatedhq/troubleshoot/internal/util"
 	analyzer "github.com/replicatedhq/troubleshoot/pkg/analyze"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"github.com/replicatedhq/troubleshoot/pkg/collect"
+	"github.com/replicatedhq/troubleshoot/pkg/constants"
 	"github.com/replicatedhq/troubleshoot/pkg/convert"
 	"github.com/replicatedhq/troubleshoot/pkg/httputil"
 	"github.com/replicatedhq/troubleshoot/pkg/k8sutil"
 	"github.com/replicatedhq/troubleshoot/pkg/loader"
 	"github.com/replicatedhq/troubleshoot/pkg/supportbundle"
+	"github.com/replicatedhq/troubleshoot/pkg/types"
 	"github.com/spf13/viper"
 	spin "github.com/tj/go-spin"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
 )
 
-func runTroubleshoot(v *viper.Viper, arg []string) error {
+func runTroubleshoot(v *viper.Viper, args []string) error {
 	ctx := context.Background()
-	if !v.GetBool("load-cluster-specs") && len(arg) < 1 {
+	if !v.GetBool("load-cluster-specs") && len(args) < 1 {
 		return errors.New("flag load-cluster-specs must be set if no specs are provided on the command line")
 	}
 
+	restConfig, err := k8sutil.GetRESTConfig()
+	if err != nil {
+		return errors.Wrap(err, "failed to convert kube flags to rest config")
+	}
+
+	client, err := kubernetes.NewForConfig(restConfig)
+	if err != nil {
+		return errors.Wrap(err, "failed to create kubernetes client")
+	}
+
+	mainBundle, additionalRedactors, err := loadSpecs(ctx, args, client)
+	if err != nil {
+		return err
+	}
+
+	// For --dry-run, we want to print the yaml and exit
+	if v.GetBool("dry-run") {
+		k := loader.TroubleshootKinds{
+			SupportBundlesV1Beta2: []troubleshootv1beta2.SupportBundle{*mainBundle},
+		}
+		// If we have redactors, add them to the temp kinds object
+		if len(additionalRedactors.Spec.Redactors) > 0 {
+			k.RedactorsV1Beta2 = []troubleshootv1beta2.Redactor{*additionalRedactors}
+		}
+
+		out, err := k.ToYaml()
+		if err != nil {
+			return types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, errors.Wrap(err, "failed to convert specs to yaml"))
+		}
+		fmt.Printf("%s", out)
+		return nil
+	}
+
 	interactive := v.GetBool("interactive") && isatty.IsTerminal(os.Stdout.Fd())
 
 	if interactive {
@@ -55,11 +92,6 @@ func runTroubleshoot(v *viper.Viper, arg []string) error {
 		os.Exit(0)
 	}()
 
-	restConfig, err := k8sutil.GetRESTConfig()
-	if err != nil {
-		return errors.Wrap(err, "failed to convert kube flags to rest config")
-	}
-
 	var sinceTime *time.Time
 	if v.GetString("since-time") != "" || v.GetString("since") != "" {
 		sinceTime, err = parseTimeFlags(v)
@@ -74,67 +106,6 @@ func runTroubleshoot(v *viper.Viper, arg []string) error {
 		})
 	}
 
-	var mainBundle *troubleshootv1beta2.SupportBundle
-
-	additionalRedactors := &troubleshootv1beta2.Redactor{}
-
-	// Defining `v` below will render using `v` in reference to Viper unusable.
-	// Therefore refactoring `v` to `val` will make sure we can still use it.
-	for _, val := range arg {
-
-		collectorContent, err := supportbundle.LoadSupportBundleSpec(val)
-		if err != nil {
-			return errors.Wrap(err, "failed to load support bundle spec")
-		}
-		multidocs := strings.Split(string(collectorContent), "\n---\n")
-		// Referencing `ParseSupportBundle with a secondary arg of `no-uri`
-		// Will make sure we can enable or disable the use of the `Spec.uri` field for an upstream spec.
-		// This change will not have an impact on KOTS' usage of `ParseSupportBundle`
-		// As Kots uses `load.go` directly.
-		supportBundle, err := supportbundle.ParseSupportBundle([]byte(multidocs[0]), !v.GetBool("no-uri"))
-		if err != nil {
-			return errors.Wrap(err, "failed to parse support bundle spec")
-		}
-
-		mainBundle = supportbundle.ConcatSpec(mainBundle, supportBundle)
-
-		parsedRedactors, err := supportbundle.ParseRedactorsFromDocs(multidocs)
-		if err != nil {
-			return errors.Wrap(err, "failed to parse redactors from doc")
-		}
-		additionalRedactors.Spec.Redactors = append(additionalRedactors.Spec.Redactors, parsedRedactors...)
-	}
-
-	if v.GetBool("load-cluster-specs") {
-		kinds, err := loadClusterSpecs(ctx, v)
-		if err != nil {
-			return err
-		}
-		if len(kinds.SupportBundlesV1Beta2) == 0 {
-			return errors.New("no support bundle specs found in cluster")
-		}
-		for _, sb := range kinds.SupportBundlesV1Beta2 {
-			sb := sb // Why? https://golang.org/doc/faq#closures_and_goroutines
-			mainBundle = supportbundle.ConcatSpec(mainBundle, &sb)
-		}
-
-		for _, redactor := range kinds.RedactorsV1Beta2 {
-			additionalRedactors.Spec.Redactors = append(additionalRedactors.Spec.Redactors, redactor.Spec.Redactors...)
-		}
-	}
-
-	if mainBundle == nil {
-		return errors.New("no support bundle specs provided to run")
-	} else if mainBundle.Spec.Collectors == nil && mainBundle.Spec.HostCollectors == nil {
-		return errors.New("no collectors specified in support bundle")
-	}
-
-	redactors, err := supportbundle.GetRedactorsFromURIs(v.GetStringSlice("redactors"))
-	if err != nil {
-		return errors.Wrap(err, "failed to get redactors")
-	}
-	additionalRedactors.Spec.Redactors = append(additionalRedactors.Spec.Redactors, redactors...)
-
 	var wg sync.WaitGroup
 	collectorCB := func(c chan interface{}, msg string) { c <- msg }
 	progressChan := make(chan interface{})
@@ -261,18 +232,103 @@ the %s Admin Console to begin analysis.`
 	return nil
 }
 
-func loadClusterSpecs(ctx context.Context, v *viper.Viper) (*loader.TroubleshootKinds, error) {
-	config, err := k8sutil.GetRESTConfig()
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to convert kube flags to rest config")
+func loadSupportBundleSpecsFromURIs(ctx context.Context, kinds *loader.TroubleshootKinds) (*loader.TroubleshootKinds, error) {
+	remoteRawSpecs := []string{}
+	for _, s := range kinds.SupportBundlesV1Beta2 {
+		if s.Spec.Uri != "" && util.IsURL(s.Spec.Uri) {
+			// We are using LoadSupportBundleSpec function here since it handles prompting
+			// users to accept insecure connections
+			// There is an opportunity to refactor this code in favour of the Loader APIs
+			rawSpec, err := supportbundle.LoadSupportBundleSpec(s.Spec.Uri)
+			if err != nil {
+				return nil, errors.Wrapf(err, "failed to load support bundle from URI %q", s.Spec.Uri)
+			}
+			remoteRawSpecs = append(remoteRawSpecs, string(rawSpec))
+		}
 	}
 
-	client, err := kubernetes.NewForConfig(config)
+	return loader.LoadSpecs(ctx, loader.LoadOptions{
+		RawSpecs: remoteRawSpecs,
+	})
+}
+
+func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface) (*troubleshootv1beta2.SupportBundle, *troubleshootv1beta2.Redactor, error) {
+	// Append redactor uris to the args
+	allArgs := append(args, viper.GetStringSlice("redactors")...)
+	kinds, err := specs.LoadFromCLIArgs(ctx, client, allArgs, viper.GetViper())
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to convert create k8s client")
+		return nil, nil, err
 	}
 
-	return privSpecs.LoadFromCluster(ctx, client, v.GetStringSlice("selector"), v.GetString("namespace"))
+	// Load additional specs from support bundle URIs
+	if !viper.GetBool("no-uri") {
+		moreKinds, err := loadSupportBundleSpecsFromURIs(ctx, kinds)
+		if err != nil {
+			return nil, nil, err
+		}
+		kinds.Add(moreKinds)
+	}
+
+	// Check if we have any collectors to run in the troubleshoot specs
+	// TODO: Do we use the RemoteCollectors anymore?
+	if len(kinds.CollectorsV1Beta2) == 0 &&
+		len(kinds.HostCollectorsV1Beta2) == 0 &&
+		len(kinds.SupportBundlesV1Beta2) == 0 {
+		return nil, nil, errors.New("no collectors specified to run")
+	}
+
+	// Merge specs
+	// We need to add the default type information to the support bundle spec
+	// since by default these fields would be empty
+	mainBundle := &troubleshootv1beta2.SupportBundle{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "troubleshoot.sh/v1beta2",
+			Kind:       "SupportBundle",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "merged-support-bundle-spec",
+		},
+	}
+	for _, sb := range kinds.SupportBundlesV1Beta2 {
+		sb := sb
+		mainBundle = supportbundle.ConcatSpec(mainBundle, &sb)
+	}
+
+	for _, c := range kinds.CollectorsV1Beta2 {
+		mainBundle.Spec.Collectors = append(mainBundle.Spec.Collectors, c.Spec.Collectors...)
+	}
+
+	for _, hc := range kinds.HostCollectorsV1Beta2 {
+		mainBundle.Spec.HostCollectors = append(mainBundle.Spec.HostCollectors, hc.Spec.Collectors...)
+	}
+
+	// Ensure cluster info and cluster resources collectors are in the merged spec
+	// We need to add them here so when we --dry-run, these collectors are included.
+	// supportbundle.runCollectors duplicates this bit. We'll need to refactor it out later
+	// when its clearer what other code depends on this logic e.g KOTS
+	mainBundle.Spec.Collectors = collect.EnsureCollectorInList(
+		mainBundle.Spec.Collectors,
+		troubleshootv1beta2.Collect{ClusterInfo: &troubleshootv1beta2.ClusterInfo{}},
+	)
+	mainBundle.Spec.Collectors = collect.EnsureCollectorInList(
+		mainBundle.Spec.Collectors,
+		troubleshootv1beta2.Collect{ClusterResources: &troubleshootv1beta2.ClusterResources{}},
+	)
+
+	additionalRedactors := &troubleshootv1beta2.Redactor{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "troubleshoot.sh/v1beta2",
+			Kind:       "Redactor",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "merged-redactors-spec",
+		},
+	}
+	for _, r := range kinds.RedactorsV1Beta2 {
+		additionalRedactors.Spec.Redactors = append(additionalRedactors.Spec.Redactors, r.Spec.Redactors...)
+	}
+
+	return mainBundle, additionalRedactors, nil
 }
 
 func parseTimeFlags(v *viper.Viper) (*time.Time, error) {

cmd/troubleshoot/cli/run_test.go

@@ -0,0 +1,51 @@
+package cli
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/replicatedhq/troubleshoot/pkg/loader"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_loadSupportBundleSpecsFromURIs(t *testing.T) {
+	// Run a webserver to serve the spec
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte(`
+apiVersion: troubleshoot.sh/v1beta2
+kind: SupportBundle
+metadata:
+  name: sb-2
+spec:
+  collectors:
+    - clusterInfo: {}`))
+	}))
+	defer srv.Close()
+
+	orig := `
+apiVersion: troubleshoot.sh/v1beta2
+kind: SupportBundle
+metadata:
+  name: sb-1
+spec:
+  uri: ` + srv.URL + `
+  collectors:
+    - configMap:
+        name: kube-root-ca.crt
+        namespace: default
+`
+
+	ctx := context.Background()
+	kinds, err := loader.LoadSpecs(ctx, loader.LoadOptions{RawSpec: orig})
+	require.NoError(t, err)
+	require.NotNil(t, kinds)
+
+	moreKinds, err := loadSupportBundleSpecsFromURIs(ctx, kinds)
+	require.NoError(t, err)
+
+	require.Len(t, moreKinds.SupportBundlesV1Beta2, 1)
+	assert.NotNil(t, moreKinds.SupportBundlesV1Beta2[0].Spec.Collectors[0].ClusterInfo)
+}