From c78e943f5bee82671e52474f9116d3679250976b Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Tue, 12 Sep 2023 15:04:26 +0100
Subject: [PATCH 01/17] Add dry-run flag

---
 .gitignore                   |  3 +++
 cmd/troubleshoot/cli/root.go |  1 +
 cmd/troubleshoot/cli/run.go  | 39 ++++++++++++++++++++++++++++--------
 pkg/preflight/flags.go       |  1 -
 4 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/.gitignore b/.gitignore
index 6249328eb..61ca6e377 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,7 +7,10 @@
 *.dylib
 bin
 .DS_Store
+
+# npm generated files
 node_modules/
+package*.json
 
 # Test binary, build with `go test -c`
 *.test
diff --git a/cmd/troubleshoot/cli/root.go b/cmd/troubleshoot/cli/root.go
index a54c928ec..b1853d674 100644
--- a/cmd/troubleshoot/cli/root.go
+++ b/cmd/troubleshoot/cli/root.go
@@ -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 the 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")
diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go
index 45077c5de..d770909a8 100644
--- a/cmd/troubleshoot/cli/run.go
+++ b/cmd/troubleshoot/cli/run.go
@@ -17,14 +17,17 @@ import (
 	"github.com/fatih/color"
 	"github.com/mattn/go-isatty"
 	"github.com/pkg/errors"
+	"github.com/replicatedhq/troubleshoot/internal/specs"
 	privSpecs "github.com/replicatedhq/troubleshoot/internal/specs"
 	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"
 	"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"
 	"k8s.io/client-go/kubernetes"
@@ -32,9 +35,34 @@ import (
 	"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 {
+
+	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")
+	}
+
+	kinds, err := specs.LoadFromCLIArgs(ctx, client, args, viper.GetViper())
+	if err != nil {
+		return err
+	}
+
+	if v.GetBool("dry-run") {
+		out, err := kinds.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
+	}
+
+	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")
 	}
 
@@ -55,11 +83,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)
@@ -80,7 +103,7 @@ func runTroubleshoot(v *viper.Viper, arg []string) error {
 
 	// 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 {
+	for _, val := range args {
 
 		collectorContent, err := supportbundle.LoadSupportBundleSpec(val)
 		if err != nil {
diff --git a/pkg/preflight/flags.go b/pkg/preflight/flags.go
index 1ea4e7a65..f5c85187a 100644
--- a/pkg/preflight/flags.go
+++ b/pkg/preflight/flags.go
@@ -16,7 +16,6 @@ const (
 	flagSince                     = "since"
 	flagOutput                    = "output"
 	flagDebug                     = "debug"
-	flagDryRun                    = "dry-run"
 )
 
 type PreflightFlags struct {

From eb4a5c7a7d0c5ee95ed29c20cbbf2e382a37e2ef Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Thu, 14 Sep 2023 17:50:15 +0100
Subject: [PATCH 02/17] No traces on dry run

---
 cmd/troubleshoot/cli/root.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/troubleshoot/cli/root.go b/cmd/troubleshoot/cli/root.go
index b1853d674..0b348bd9e 100644
--- a/cmd/troubleshoot/cli/root.go
+++ b/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())
 			}
 

From a55d61e848c76c550ca95d29311accd1a1506c1e Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Thu, 21 Sep 2023 12:29:34 +0100
Subject: [PATCH 03/17] More refactoring

---
 bin/watch.js                 | 15 +--------------
 bin/watchrsync.js            |  8 +-------
 cmd/troubleshoot/cli/root.go |  2 +-
 cmd/troubleshoot/cli/run.go  | 19 +++++++++++++++----
 internal/specs/specs.go      | 19 +++++++++++++++++--
 pkg/constants/constants.go   |  7 ++++---
 pkg/loader/loader.go         | 14 ++++++++++++++
 7 files changed, 53 insertions(+), 31 deletions(-)

diff --git a/bin/watch.js b/bin/watch.js
index 45a71b3d5..b2d535661 100755
--- a/bin/watch.js
+++ b/bin/watch.js
@@ -2,19 +2,6 @@
 
 const gri = require('gaze-run-interrupt');
 
-const binList = [
-  // 'bin/analyze',
-  // 'bin/preflight',
-  'bin/support-bundle',
-  // 'bin/collect'
-]
-const makeList = [
-  // 'analyze',
-  // 'preflight',
-  'support-bundle',
-  // 'collect'
-]
-
 const commands = [
   // {
   //   command: 'rm',
@@ -22,7 +9,7 @@ const commands = [
   // },
   {
     command: 'make',
-    args: makeList,
+    args: ['build'],
   },
 ];
 
diff --git a/bin/watchrsync.js b/bin/watchrsync.js
index 640cb1014..a77826a39 100755
--- a/bin/watchrsync.js
+++ b/bin/watchrsync.js
@@ -16,12 +16,6 @@ const binList = [
   'bin/support-bundle',
   // 'bin/collect'
 ]
-const makeList = [
-  // 'analyze',
-  // 'preflight',
-  'support-bundle',
-  // 'collect'
-]
 
 const commands = [
   // {
@@ -30,7 +24,7 @@ const commands = [
   // },
   {
     command: 'make',
-    args: makeList,
+    args: ['build'],
   },
 ];
 
diff --git a/cmd/troubleshoot/cli/root.go b/cmd/troubleshoot/cli/root.go
index 0b348bd9e..18816c3dc 100644
--- a/cmd/troubleshoot/cli/root.go
+++ b/cmd/troubleshoot/cli/root.go
@@ -74,7 +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 the support bundle spec without collecting anything")
+	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")
diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go
index d770909a8..702b30088 100644
--- a/cmd/troubleshoot/cli/run.go
+++ b/cmd/troubleshoot/cli/run.go
@@ -53,6 +53,21 @@ func runTroubleshoot(v *viper.Viper, args []string) error {
 		return err
 	}
 
+	// Check if we have any collectors to run in the support bundle specs
+	if len(kinds.SupportBundlesV1Beta2) == 0 {
+		return errors.New("no support bundle specs provided to run")
+	}
+	collectorsCount := 0
+	for _, sb := range kinds.SupportBundlesV1Beta2 {
+		collectorsCount += len(sb.Spec.Collectors)
+		collectorsCount += len(sb.Spec.HostCollectors)
+	}
+
+	if collectorsCount == 0 {
+		return errors.New("no collectors specified in support bundle specs")
+	}
+
+	// For --dry-run, we want to print the yaml and exit
 	if v.GetBool("dry-run") {
 		out, err := kinds.ToYaml()
 		if err != nil {
@@ -62,10 +77,6 @@ func runTroubleshoot(v *viper.Viper, args []string) error {
 		return nil
 	}
 
-	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")
-	}
-
 	interactive := v.GetBool("interactive") && isatty.IsTerminal(os.Stdout.Fd())
 
 	if interactive {
diff --git a/internal/specs/specs.go b/internal/specs/specs.go
index 0eeeedf3e..36a80e4c0 100644
--- a/internal/specs/specs.go
+++ b/internal/specs/specs.go
@@ -9,6 +9,7 @@ import (
 	"os"
 	"reflect"
 	"strings"
+	"time"
 
 	"github.com/pkg/errors"
 	"github.com/replicatedhq/troubleshoot/internal/util"
@@ -63,7 +64,20 @@ func SplitTroubleshootSecretLabelSelector(ctx context.Context, labelSelector lab
 	return parsedSelectorStrings, nil
 }
 
+var httpClient = &http.Client{
+	Timeout: 30 * time.Second,
+}
+
+// LoadFromCLIArgs loads troubleshoot specs from args passed to a CLI command.
+// This loader function is meant for troubleshoot CLI commands only, hence not making it public.
+// It will contain opinionated logic for CLI commands such as interpreting viper flags,
+// supporting secret/ uri format, downloading from OCI registry and other URLs, etc.
 func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []string, vp *viper.Viper) (*loader.TroubleshootKinds, error) {
+	// Let's always ensure we have a context
+	if ctx == nil {
+		ctx = context.Background()
+	}
+
 	rawSpecs := []string{}
 
 	for _, v := range args {
@@ -100,6 +114,7 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 			}
 
 			if u.Scheme == "oci" {
+				// TODO: We need to also pull support-bundle images from OCI
 				content, err := oci.PullPreflightFromOCI(v)
 				if err != nil {
 					if err == oci.ErrNoRelease {
@@ -115,13 +130,13 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 					return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, fmt.Errorf("%s is not a URL and was not found (err %s)", v, err))
 				}
 
-				req, err := http.NewRequest("GET", v, nil)
+				req, err := http.NewRequestWithContext(ctx, "GET", v, nil)
 				if err != nil {
 					// exit code: should this be catch all or spec issues...?
 					return nil, types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, err)
 				}
 				req.Header.Set("User-Agent", "Replicated_Preflight/v1beta2")
-				resp, err := http.DefaultClient.Do(req)
+				resp, err := httpClient.Do(req)
 				if err != nil {
 					// exit code: should this be catch all or spec issues...?
 					return nil, types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, err)
diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go
index ad9703ca3..e69a6986f 100644
--- a/pkg/constants/constants.go
+++ b/pkg/constants/constants.go
@@ -70,11 +70,12 @@ const (
 	EXIT_CODE_WARN        = 4
 
 	// Troubleshoot label constants
-	SupportBundleKey       = "support-bundle-spec"
-	RedactorKey            = "redactor-spec"
 	TroubleshootIOLabelKey = "troubleshoot.io/kind"
 	TroubleshootSHLabelKey = "troubleshoot.sh/kind"
-	PreflightKey           = "preflight.yaml" // Shouldn't this be "preflight-spec"?
+	SupportBundleKey       = "support-bundle-spec"
+	RedactorKey            = "redactor-spec"
+	PreflightKey           = "preflight.yaml"
+	PreflightKey2          = "preflight-spec"
 
 	// Troubleshoot spec constants
 	Troubleshootv1beta2Kind = "troubleshoot.sh/v1beta2"
diff --git a/pkg/loader/loader.go b/pkg/loader/loader.go
index 0244dc319..7f8fb4e69 100644
--- a/pkg/loader/loader.go
+++ b/pkg/loader/loader.go
@@ -268,6 +268,7 @@ func isConfigMap(parsedDocHead parsedDoc) bool {
 
 // getSpecFromConfigMap extracts multiple troubleshoot specs from a secret
 func (l *specLoader) getSpecFromConfigMap(cm *v1.ConfigMap) ([]string, error) {
+	// TODO: Consider not checking for the existence of the key and just trying to decode
 	specs := []string{}
 
 	str, ok := cm.Data[constants.SupportBundleKey]
@@ -282,12 +283,17 @@ func (l *specLoader) getSpecFromConfigMap(cm *v1.ConfigMap) ([]string, error) {
 	if ok {
 		specs = append(specs, util.SplitYAML(str)...)
 	}
+	str, ok = cm.Data[constants.PreflightKey2]
+	if ok {
+		specs = append(specs, util.SplitYAML(str)...)
+	}
 
 	return specs, nil
 }
 
 // getSpecFromSecret extracts multiple troubleshoot specs from a secret
 func (l *specLoader) getSpecFromSecret(secret *v1.Secret) ([]string, error) {
+	// TODO: Consider not checking for the existence of the key and just trying to decode
 	specs := []string{}
 
 	specBytes, ok := secret.Data[constants.SupportBundleKey]
@@ -302,6 +308,10 @@ func (l *specLoader) getSpecFromSecret(secret *v1.Secret) ([]string, error) {
 	if ok {
 		specs = append(specs, util.SplitYAML(string(specBytes))...)
 	}
+	specBytes, ok = secret.Data[constants.PreflightKey2]
+	if ok {
+		specs = append(specs, util.SplitYAML(string(specBytes))...)
+	}
 	str, ok := secret.StringData[constants.SupportBundleKey]
 	if ok {
 		specs = append(specs, util.SplitYAML(str)...)
@@ -314,5 +324,9 @@ func (l *specLoader) getSpecFromSecret(secret *v1.Secret) ([]string, error) {
 	if ok {
 		specs = append(specs, util.SplitYAML(str)...)
 	}
+	str, ok = secret.StringData[constants.PreflightKey2]
+	if ok {
+		specs = append(specs, util.SplitYAML(str)...)
+	}
 	return specs, nil
 }

From 1791b33d2eb759300d6f0d7565c7cf03c471923a Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Thu, 21 Sep 2023 14:47:07 +0100
Subject: [PATCH 04/17] More updates to support bundle binary

---
 cmd/troubleshoot/cli/root.go |   2 +-
 cmd/troubleshoot/cli/run.go  | 141 +++++++++++++++++------------------
 internal/specs/configmaps.go |  13 +---
 internal/specs/secrets.go    |  12 +--
 internal/specs/specs.go      |  28 ++++++-
 pkg/loader/loader.go         | 106 +++++++++++++++++++++++++-
 pkg/loader/loader_test.go    |  47 ++++++++++++
 pkg/specs/configmaps.go      |  13 +++-
 pkg/specs/secrets.go         |  12 ++-
 9 files changed, 273 insertions(+), 101 deletions(-)

diff --git a/cmd/troubleshoot/cli/root.go b/cmd/troubleshoot/cli/root.go
index 18816c3dc..f02712b7d 100644
--- a/cmd/troubleshoot/cli/root.go
+++ b/cmd/troubleshoot/cli/root.go
@@ -82,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())
 
diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go
index 702b30088..b35bef46f 100644
--- a/cmd/troubleshoot/cli/run.go
+++ b/cmd/troubleshoot/cli/run.go
@@ -9,7 +9,6 @@ import (
 	"os"
 	"os/signal"
 	"path/filepath"
-	"strings"
 	"sync"
 	"time"
 
@@ -18,14 +17,12 @@ import (
 	"github.com/mattn/go-isatty"
 	"github.com/pkg/errors"
 	"github.com/replicatedhq/troubleshoot/internal/specs"
-	privSpecs "github.com/replicatedhq/troubleshoot/internal/specs"
 	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"
-	"github.com/replicatedhq/troubleshoot/pkg/loader"
 	"github.com/replicatedhq/troubleshoot/pkg/supportbundle"
 	"github.com/replicatedhq/troubleshoot/pkg/types"
 	"github.com/spf13/viper"
@@ -48,7 +45,8 @@ func runTroubleshoot(v *viper.Viper, args []string) error {
 		return errors.Wrap(err, "failed to create kubernetes client")
 	}
 
-	kinds, err := specs.LoadFromCLIArgs(ctx, client, args, viper.GetViper())
+	argWithRedactors := append(args, v.GetStringSlice("redactors")...)
+	kinds, err := specs.LoadFromCLIArgs(ctx, client, argWithRedactors, viper.GetViper())
 	if err != nil {
 		return err
 	}
@@ -108,66 +106,67 @@ func runTroubleshoot(v *viper.Viper, args []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 args {
-
-		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...)
-		}
+	mainBundle := &troubleshootv1beta2.SupportBundle{}
+	for _, sb := range kinds.SupportBundlesV1Beta2 {
+		sb := sb
+		mainBundle = supportbundle.ConcatSpec(mainBundle, &sb)
 	}
 
-	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")
+	additionalRedactors := &troubleshootv1beta2.Redactor{}
+	for _, r := range kinds.RedactorsV1Beta2 {
+		additionalRedactors.Spec.Redactors = append(additionalRedactors.Spec.Redactors, r.Spec.Redactors...)
 	}
 
-	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...)
+	// // 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 args {
+
+	// 	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...)
+	// 	}
+	// }
+
+	// 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 }
@@ -295,19 +294,19 @@ 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 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")
+// 	}
 
-	client, err := kubernetes.NewForConfig(config)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to convert create k8s client")
-	}
+// 	client, err := kubernetes.NewForConfig(config)
+// 	if err != nil {
+// 		return nil, errors.Wrap(err, "failed to convert create k8s client")
+// 	}
 
-	return privSpecs.LoadFromCluster(ctx, client, v.GetStringSlice("selector"), v.GetString("namespace"))
-}
+// 	return privSpecs.LoadFromCluster(ctx, client, v.GetStringSlice("selector"), v.GetString("namespace"))
+// }
 
 func parseTimeFlags(v *viper.Viper) (*time.Time, error) {
 	var (
diff --git a/internal/specs/configmaps.go b/internal/specs/configmaps.go
index d587b7fc2..485775ae1 100644
--- a/internal/specs/configmaps.go
+++ b/internal/specs/configmaps.go
@@ -9,22 +9,17 @@ import (
 	"k8s.io/klog/v2"
 )
 
-func LoadFromConfigMap(ctx context.Context, client kubernetes.Interface, ns string, name string, key string) ([]byte, error) {
+func LoadFromConfigMap(ctx context.Context, client kubernetes.Interface, ns string, name string) (map[string]string, error) {
 	foundConfigMap, err := client.CoreV1().ConfigMaps(ns).Get(ctx, name, metav1.GetOptions{})
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get configmap")
 	}
 
-	spec, ok := foundConfigMap.Data[key]
-	if !ok {
-		return nil, errors.Errorf("spec not found in configmap %s", name)
-	}
-
-	klog.V(1).InfoS("Loaded spec from config map", "name",
-		foundConfigMap.Name, "namespace", foundConfigMap.Namespace, "data key", key,
+	klog.V(1).InfoS("Loaded data from config map", "name",
+		foundConfigMap.Name, "namespace", foundConfigMap.Namespace,
 	)
 
-	return []byte(spec), nil
+	return foundConfigMap.Data, nil
 }
 
 func LoadFromConfigMapMatchingLabel(ctx context.Context, client kubernetes.Interface, label string, ns string, key string) ([]string, error) {
diff --git a/internal/specs/secrets.go b/internal/specs/secrets.go
index 455c827af..0d517446e 100644
--- a/internal/specs/secrets.go
+++ b/internal/specs/secrets.go
@@ -9,21 +9,13 @@ import (
 	"k8s.io/klog/v2"
 )
 
-func LoadFromSecret(ctx context.Context, client kubernetes.Interface, ns string, name string, key string) ([]byte, error) {
+func LoadFromSecret(ctx context.Context, client kubernetes.Interface, ns string, name string) (map[string][]byte, error) {
 	foundSecret, err := client.CoreV1().Secrets(ns).Get(ctx, name, metav1.GetOptions{})
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get secret")
 	}
 
-	spec, ok := foundSecret.Data[key]
-	if !ok {
-		return nil, errors.Errorf("spec not found in secret %s", name)
-	}
-
-	klog.V(1).InfoS("Loaded spec from secret", "name",
-		foundSecret.Name, "namespace", foundSecret.Namespace, "data key", key,
-	)
-	return spec, nil
+	return foundSecret.Data, nil
 }
 
 func LoadFromSecretMatchingLabel(ctx context.Context, client kubernetes.Interface, label string, ns string, key string) ([]string, error) {
diff --git a/internal/specs/specs.go b/internal/specs/specs.go
index 36a80e4c0..52090c476 100644
--- a/internal/specs/specs.go
+++ b/internal/specs/specs.go
@@ -85,15 +85,34 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 			// format secret/namespace-name/secret-name
 			pathParts := strings.Split(v, "/")
 			if len(pathParts) != 3 {
-				return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, errors.Errorf("path %s must have 3 components", v))
+				return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, errors.Errorf("secret path %q must have 3 components", v))
 			}
 
-			spec, err := LoadFromSecret(ctx, client, pathParts[1], pathParts[2], "preflight-spec")
+			data, err := LoadFromSecret(ctx, client, pathParts[1], pathParts[2])
 			if err != nil {
 				return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, errors.Wrap(err, "failed to get spec from secret"))
 			}
 
-			rawSpecs = append(rawSpecs, string(spec))
+			// Append all data in the secret. Some may not be specs, but that's ok. They will be ignored.
+			for _, spec := range data {
+				rawSpecs = append(rawSpecs, string(spec))
+			}
+		} else if strings.HasPrefix(v, "configmap/") {
+			// format configmap/namespace-name/configmap-name
+			pathParts := strings.Split(v, "/")
+			if len(pathParts) != 3 {
+				return nil, errors.Errorf("configmap path %q must have 3 components", v)
+			}
+
+			data, err := LoadFromConfigMap(ctx, client, pathParts[1], pathParts[2])
+			if err != nil {
+				return nil, errors.Wrap(err, "failed to get spec from configmap")
+			}
+
+			// Append all data in the configmap. Some may not be specs, but that's ok. They will be ignored.
+			for _, spec := range data {
+				rawSpecs = append(rawSpecs, spec)
+			}
 		} else if _, err := os.Stat(v); err == nil {
 			b, err := os.ReadFile(v)
 			if err != nil {
@@ -154,7 +173,8 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 	}
 
 	kinds, err := loader.LoadSpecs(ctx, loader.LoadOptions{
-		RawSpecs: rawSpecs,
+		RawSpecs:              rawSpecs,
+		IgnoreUpdateDownloads: vp.GetBool("no-uri"),
 	})
 	if err != nil {
 		return nil, err
diff --git a/pkg/loader/loader.go b/pkg/loader/loader.go
index 7f8fb4e69..b05bd93de 100644
--- a/pkg/loader/loader.go
+++ b/pkg/loader/loader.go
@@ -2,9 +2,17 @@ package loader
 
 import (
 	"context"
+	"crypto/tls"
+	"fmt"
+	"io"
+	"net/http"
+	"os"
 	"reflect"
 	"strings"
+	"time"
 
+	"github.com/manifoldco/promptui"
+	"github.com/mattn/go-isatty"
 	"github.com/pkg/errors"
 	"github.com/replicatedhq/troubleshoot/internal/util"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
@@ -40,6 +48,11 @@ type LoadOptions struct {
 	// If true, the loader will return an error if any of the specs are not valid
 	// else the invalid specs will be ignored
 	Strict bool
+
+	// Ignore downloading specs from URIs defined in the specs e.g in supportbundle,
+	// a Uri field can be defined to download an additional spec from a remote
+	// location. If this flag is set to true, the loader will ignore downloading it
+	IgnoreUpdateDownloads bool
 }
 
 // LoadSpecs takes sources to load specs from and returns a TroubleshootKinds object
@@ -59,11 +72,38 @@ func LoadSpecs(ctx context.Context, opt LoadOptions) (*TroubleshootKinds, error)
 	l := specLoader{
 		strict: opt.Strict,
 	}
-	return l.loadFromStrings(opt.RawSpecs...)
-}
 
-type specLoader struct {
-	strict bool
+	kinds, err := l.loadFromStrings(opt.RawSpecs...)
+	if err != nil {
+		return nil, err
+	}
+
+	// Do not download spec updates from remote URIs
+	if opt.IgnoreUpdateDownloads {
+		return kinds, nil
+	}
+
+	// Follow support bundle URIs to download more specs
+	// Clients can later on merge all the available specs
+	moreRawSpecs := []string{}
+	for _, spec := range kinds.SupportBundlesV1Beta2 {
+		if spec.Spec.Uri != "" && util.IsURL(spec.Spec.Uri) {
+			raw, err := loadSpecFromURL(ctx, spec.Spec.Uri)
+			if err != nil {
+				return nil, errors.Wrapf(err, "failed to load support bundle from uri %q", spec.Spec.Uri)
+			}
+			moreRawSpecs = append(moreRawSpecs, string(raw))
+		}
+	}
+
+	moreKinds, err := l.loadFromStrings(moreRawSpecs...)
+	if err != nil {
+		return nil, err
+	}
+
+	kinds.Add(moreKinds)
+
+	return kinds, nil
 }
 
 type TroubleshootKinds struct {
@@ -130,6 +170,10 @@ func NewTroubleshootKinds() *TroubleshootKinds {
 	return &TroubleshootKinds{}
 }
 
+type specLoader struct {
+	strict bool
+}
+
 // loadFromStrings accepts a list of strings (exploded) which should be yaml documents
 func (l *specLoader) loadFromStrings(rawSpecs ...string) (*TroubleshootKinds, error) {
 	splitdocs := []string{}
@@ -330,3 +374,57 @@ func (l *specLoader) getSpecFromSecret(secret *v1.Secret) ([]string, error) {
 	}
 	return specs, nil
 }
+
+func loadSpecFromURL(ctx context.Context, uri string) ([]byte, error) {
+	// TODO: DRY ME - Copied from supportbundle package
+	httpClient := &http.Client{
+		Transport: http.DefaultTransport,
+		Timeout:   30 * time.Second,
+	}
+	for {
+		req, err := http.NewRequestWithContext(ctx, "GET", uri, nil)
+		if err != nil {
+			return nil, errors.Wrap(err, "make request")
+		}
+		req.Header.Set("User-Agent", "Replicated_Troubleshoot/v1beta1")
+		req.Header.Set("Bundle-Upload-Host", fmt.Sprintf("%s://%s", req.URL.Scheme, req.URL.Host))
+		resp, err := httpClient.Do(req)
+		if err != nil {
+			if shouldRetryRequest(err, httpClient) {
+				continue
+			}
+			return nil, errors.Wrap(err, "execute request")
+		}
+		defer resp.Body.Close()
+
+		body, err := io.ReadAll(resp.Body)
+		if err != nil {
+			return nil, errors.Wrap(err, "read responce body")
+		}
+
+		return body, nil
+	}
+}
+
+func shouldRetryRequest(err error, client *http.Client) bool {
+	// TODO: DRY ME - Copied from supportbundle package
+	if strings.Contains(err.Error(), "x509") && canTryInsecure() {
+		client.Transport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+		return true
+	}
+	return false
+}
+
+func canTryInsecure() bool {
+	// TODO: DRY ME - Copied from supportbundle package
+	if !isatty.IsTerminal(os.Stdout.Fd()) {
+		return false
+	}
+	prompt := promptui.Prompt{
+		Label:     "Connection appears to be insecure. Would you like to attempt to create a support bundle anyway?",
+		IsConfirm: true,
+	}
+
+	_, err := prompt.Run()
+	return err == nil
+}
diff --git a/pkg/loader/loader_test.go b/pkg/loader/loader_test.go
index 5ae629377..b6f402de4 100644
--- a/pkg/loader/loader_test.go
+++ b/pkg/loader/loader_test.go
@@ -2,6 +2,8 @@ package loader
 
 import (
 	"context"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/replicatedhq/troubleshoot/internal/testutils"
@@ -579,3 +581,48 @@ spec:
       ignoreRBAC: true`)
 	assert.Contains(t, string(y), "message: Cluster is up to date")
 }
+
+func TestDownloadingSpecUpdates(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 := LoadSpecs(ctx, LoadOptions{RawSpec: orig})
+	require.NoError(t, err)
+	require.NotNil(t, kinds)
+
+	assert.Len(t, kinds.SupportBundlesV1Beta2, 2)
+	assert.Equal(t, srv.URL, kinds.SupportBundlesV1Beta2[0].Spec.Uri)
+	assert.Equal(t, "kube-root-ca.crt", kinds.SupportBundlesV1Beta2[0].Spec.Collectors[0].ConfigMap.Name)
+	assert.NotNil(t, kinds.SupportBundlesV1Beta2[1].Spec.Collectors[0].ClusterInfo)
+
+	// Ignore updates
+	kinds, err = LoadSpecs(ctx, LoadOptions{RawSpec: orig, IgnoreUpdateDownloads: true})
+	require.NoError(t, err)
+	require.NotNil(t, kinds)
+
+	assert.Len(t, kinds.SupportBundlesV1Beta2, 1)
+}
diff --git a/pkg/specs/configmaps.go b/pkg/specs/configmaps.go
index 94f33c3a3..bd64e821d 100644
--- a/pkg/specs/configmaps.go
+++ b/pkg/specs/configmaps.go
@@ -22,7 +22,18 @@ func LoadFromConfigMap(namespace string, configMapName string, key string) ([]by
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to convert create k8s client")
 	}
-	return specs.LoadFromConfigMap(context.TODO(), client, namespace, configMapName, key)
+
+	data, err := specs.LoadFromConfigMap(context.TODO(), client, namespace, configMapName)
+	if err != nil {
+		return nil, err
+	}
+
+	spec, ok := data[key]
+	if !ok {
+		return nil, errors.Errorf("spec not found in configmap %q: key=%s", configMapName, key)
+	}
+
+	return []byte(spec), nil
 }
 
 // LoadFromConfigMapMatchingLabel reads data from a configmap and returns the list of values extracted from the key
diff --git a/pkg/specs/secrets.go b/pkg/specs/secrets.go
index 4e06f5d32..1fb8f2e9c 100644
--- a/pkg/specs/secrets.go
+++ b/pkg/specs/secrets.go
@@ -23,7 +23,17 @@ func LoadFromSecret(namespace string, secretName string, key string) ([]byte, er
 		return nil, errors.Wrap(err, "failed to convert create k8s client")
 	}
 
-	return specs.LoadFromSecret(context.TODO(), client, namespace, secretName, key)
+	data, err := specs.LoadFromSecret(context.TODO(), client, namespace, secretName)
+	if err != nil {
+		return nil, err
+	}
+
+	spec, ok := data[key]
+	if !ok {
+		return nil, errors.Errorf("spec not found in secret %q: key=%q", secretName, key)
+	}
+
+	return spec, nil
 }
 
 // LoadFromSecretMatchingLabel reads data from a secret in the cluster using a label selector

From 4b6701feb8849dce7f72cecae33e0a24cd7b83b5 Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Thu, 21 Sep 2023 17:04:15 +0100
Subject: [PATCH 05/17] More refactoring changes

---
 cmd/troubleshoot/cli/run.go        | 150 ++++++++++++-----------------
 pkg/supportbundle/load.go          |  17 +++-
 pkg/supportbundle/supportbundle.go |   1 +
 3 files changed, 75 insertions(+), 93 deletions(-)

diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go
index b35bef46f..a09a8ec12 100644
--- a/cmd/troubleshoot/cli/run.go
+++ b/cmd/troubleshoot/cli/run.go
@@ -23,10 +23,12 @@ import (
 	"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"
@@ -45,29 +47,22 @@ func runTroubleshoot(v *viper.Viper, args []string) error {
 		return errors.Wrap(err, "failed to create kubernetes client")
 	}
 
-	argWithRedactors := append(args, v.GetStringSlice("redactors")...)
-	kinds, err := specs.LoadFromCLIArgs(ctx, client, argWithRedactors, viper.GetViper())
+	mainBundle, additionalRedactors, err := loadSpecs(ctx, args, client)
 	if err != nil {
 		return err
 	}
 
-	// Check if we have any collectors to run in the support bundle specs
-	if len(kinds.SupportBundlesV1Beta2) == 0 {
-		return errors.New("no support bundle specs provided to run")
-	}
-	collectorsCount := 0
-	for _, sb := range kinds.SupportBundlesV1Beta2 {
-		collectorsCount += len(sb.Spec.Collectors)
-		collectorsCount += len(sb.Spec.HostCollectors)
-	}
-
-	if collectorsCount == 0 {
-		return errors.New("no collectors specified in support bundle specs")
-	}
-
 	// For --dry-run, we want to print the yaml and exit
 	if v.GetBool("dry-run") {
-		out, err := kinds.ToYaml()
+		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"))
 		}
@@ -75,6 +70,11 @@ func runTroubleshoot(v *viper.Viper, args []string) error {
 		return nil
 	}
 
+	// Check if we have any collectors to run in the support bundle specs
+	if len(mainBundle.Spec.Collectors) == 0 || len(mainBundle.Spec.HostCollectors) == 0 {
+		return errors.New("no collectors specified to run")
+	}
+
 	interactive := v.GetBool("interactive") && isatty.IsTerminal(os.Stdout.Fd())
 
 	if interactive {
@@ -106,68 +106,6 @@ func runTroubleshoot(v *viper.Viper, args []string) error {
 		})
 	}
 
-	mainBundle := &troubleshootv1beta2.SupportBundle{}
-	for _, sb := range kinds.SupportBundlesV1Beta2 {
-		sb := sb
-		mainBundle = supportbundle.ConcatSpec(mainBundle, &sb)
-	}
-
-	additionalRedactors := &troubleshootv1beta2.Redactor{}
-	for _, r := range kinds.RedactorsV1Beta2 {
-		additionalRedactors.Spec.Redactors = append(additionalRedactors.Spec.Redactors, r.Spec.Redactors...)
-	}
-
-	// // 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 args {
-
-	// 	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...)
-	// 	}
-	// }
-
-	// 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{})
@@ -294,19 +232,51 @@ 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 loadSpecs(ctx context.Context, args []string, client kubernetes.Interface) (*troubleshootv1beta2.SupportBundle, *troubleshootv1beta2.Redactor, error) {
+	argsWithRedactors := append(args, viper.GetStringSlice("redactors")...)
+	kinds, err := specs.LoadFromCLIArgs(ctx, client, argsWithRedactors, viper.GetViper())
+	if err != nil {
+		return nil, nil, err
+	}
+
+	// Merge specs
+	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...)
+	}
 
-// 	client, err := kubernetes.NewForConfig(config)
-// 	if err != nil {
-// 		return nil, errors.Wrap(err, "failed to convert create k8s client")
-// 	}
+	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 privSpecs.LoadFromCluster(ctx, client, v.GetStringSlice("selector"), v.GetString("namespace"))
-// }
+	return mainBundle, additionalRedactors, nil
+}
 
 func parseTimeFlags(v *viper.Viper) (*time.Time, error) {
 	var (
diff --git a/pkg/supportbundle/load.go b/pkg/supportbundle/load.go
index 6409f4232..d804979e9 100644
--- a/pkg/supportbundle/load.go
+++ b/pkg/supportbundle/load.go
@@ -2,7 +2,7 @@ package supportbundle
 
 import (
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"net/url"
 	"os"
@@ -20,6 +20,7 @@ import (
 	"k8s.io/klog/v2"
 )
 
+// GetSupportBundleFromURI downloads and parses a support bundle from a URI and returns a SupportBundle object
 func GetSupportBundleFromURI(bundleURI string) (*troubleshootv1beta2.SupportBundle, error) {
 	collectorContent, err := LoadSupportBundleSpec(bundleURI)
 	if err != nil {
@@ -36,6 +37,8 @@ func GetSupportBundleFromURI(bundleURI string) (*troubleshootv1beta2.SupportBund
 	return supportbundle, nil
 }
 
+// ParseSupportBundle parses a support bundle from a byte array into a SupportBundle object
+// Deprecated: use loader.LoadSpecs instead
 func ParseSupportBundle(doc []byte, followURI bool) (*troubleshootv1beta2.SupportBundle, error) {
 	doc, err := docrewrite.ConvertToV1Beta2(doc)
 	if err != nil {
@@ -98,10 +101,14 @@ func ParseSupportBundle(doc []byte, followURI bool) (*troubleshootv1beta2.Suppor
 	return nil, errors.New("spec was not parseable as a troubleshoot kind")
 }
 
+// ParseSupportBundle parses a support bundle from a byte array into a SupportBundle object
+// Deprecated: use loader.LoadSpecs instead
 func ParseSupportBundleFromDoc(doc []byte) (*troubleshootv1beta2.SupportBundle, error) {
 	return ParseSupportBundle(doc, true)
 }
 
+// GetRedactorFromURI parses a redactor from a URI into a Redactor object
+// Deprecated: use loader.LoadSpecs instead
 func GetRedactorFromURI(redactorURI string) (*troubleshootv1beta2.Redactor, error) {
 	redactorContent, err := LoadRedactorSpec(redactorURI)
 	if err != nil {
@@ -119,6 +126,8 @@ func GetRedactorFromURI(redactorURI string) (*troubleshootv1beta2.Redactor, erro
 	return redactor, nil
 }
 
+// GetRedactorsFromURIs parses redactors from a URIs Redactor objects
+// Deprecated: use loader.LoadSpecs instead
 func GetRedactorsFromURIs(redactorURIs []string) ([]*troubleshootv1beta2.Redact, error) {
 	redactors := []*troubleshootv1beta2.Redact{}
 	for _, redactor := range redactorURIs {
@@ -189,7 +198,7 @@ func LoadRedactorSpec(arg string) ([]byte, error) {
 func loadSpec(arg string) ([]byte, error) {
 	var err error
 	if _, err = os.Stat(arg); err == nil {
-		b, err := ioutil.ReadFile(arg)
+		b, err := os.ReadFile(arg)
 		if err != nil {
 			return nil, errors.Wrap(err, "read spec file")
 		}
@@ -244,7 +253,7 @@ func loadSpecFromURL(arg string) ([]byte, error) {
 		}
 		defer resp.Body.Close()
 
-		body, err := ioutil.ReadAll(resp.Body)
+		body, err := io.ReadAll(resp.Body)
 		if err != nil {
 			return nil, errors.Wrap(err, "read responce body")
 		}
@@ -253,6 +262,8 @@ func loadSpecFromURL(arg string) ([]byte, error) {
 	}
 }
 
+// ParseRedactorsFromDocs parses a slice of YAML docs and returns a slice of Redactors
+// Deprecated: use loader.LoadSpecs instead
 func ParseRedactorsFromDocs(docs []string) ([]*troubleshootv1beta2.Redact, error) {
 	var redactors []*troubleshootv1beta2.Redact
 
diff --git a/pkg/supportbundle/supportbundle.go b/pkg/supportbundle/supportbundle.go
index 019fa30f0..99bf2e02c 100644
--- a/pkg/supportbundle/supportbundle.go
+++ b/pkg/supportbundle/supportbundle.go
@@ -278,6 +278,7 @@ func ConcatSpec(target *troubleshootv1beta2.SupportBundle, source *troubleshootv
 		newBundle.Spec.HostCollectors = append(target.Spec.HostCollectors, source.Spec.HostCollectors...)
 		newBundle.Spec.HostAnalyzers = append(target.Spec.HostAnalyzers, source.Spec.HostAnalyzers...)
 		newBundle.Spec.Analyzers = append(target.Spec.Analyzers, source.Spec.Analyzers...)
+		// TODO: What to do with the Uri field?
 	}
 	return newBundle
 }

From 3baada1f22726a8329d21699bc1f96162b790a07 Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Fri, 22 Sep 2023 14:34:14 +0100
Subject: [PATCH 06/17] Different approach of loading specs from URIs

---
 cmd/troubleshoot/cli/run.go      |  32 ++++++++-
 cmd/troubleshoot/cli/run_test.go |  51 +++++++++++++++
 internal/specs/specs.go          |   3 +-
 pkg/loader/loader.go             | 107 +++----------------------------
 pkg/loader/loader_test.go        |  47 --------------
 5 files changed, 91 insertions(+), 149 deletions(-)
 create mode 100644 cmd/troubleshoot/cli/run_test.go

diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go
index a09a8ec12..7c3cfc026 100644
--- a/cmd/troubleshoot/cli/run.go
+++ b/cmd/troubleshoot/cli/run.go
@@ -17,6 +17,7 @@ import (
 	"github.com/mattn/go-isatty"
 	"github.com/pkg/errors"
 	"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/constants"
@@ -232,13 +233,40 @@ the %s Admin Console to begin analysis.`
 	return nil
 }
 
+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) {
+			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))
+		}
+	}
+
+	return loader.LoadSpecs(ctx, loader.LoadOptions{
+		RawSpecs: remoteRawSpecs,
+	})
+}
+
 func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface) (*troubleshootv1beta2.SupportBundle, *troubleshootv1beta2.Redactor, error) {
-	argsWithRedactors := append(args, viper.GetStringSlice("redactors")...)
-	kinds, err := specs.LoadFromCLIArgs(ctx, client, argsWithRedactors, viper.GetViper())
+	// 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, nil, err
 	}
 
+	// 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)
+	}
+
 	// Merge specs
 	mainBundle := &troubleshootv1beta2.SupportBundle{
 		TypeMeta: metav1.TypeMeta{
diff --git a/cmd/troubleshoot/cli/run_test.go b/cmd/troubleshoot/cli/run_test.go
new file mode 100644
index 000000000..a02e12c0d
--- /dev/null
+++ b/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)
+}
diff --git a/internal/specs/specs.go b/internal/specs/specs.go
index 52090c476..c93a7785e 100644
--- a/internal/specs/specs.go
+++ b/internal/specs/specs.go
@@ -173,8 +173,7 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 	}
 
 	kinds, err := loader.LoadSpecs(ctx, loader.LoadOptions{
-		RawSpecs:              rawSpecs,
-		IgnoreUpdateDownloads: vp.GetBool("no-uri"),
+		RawSpecs: rawSpecs,
 	})
 	if err != nil {
 		return nil, err
diff --git a/pkg/loader/loader.go b/pkg/loader/loader.go
index b05bd93de..e3a95e5fa 100644
--- a/pkg/loader/loader.go
+++ b/pkg/loader/loader.go
@@ -2,17 +2,9 @@ package loader
 
 import (
 	"context"
-	"crypto/tls"
-	"fmt"
-	"io"
-	"net/http"
-	"os"
 	"reflect"
 	"strings"
-	"time"
 
-	"github.com/manifoldco/promptui"
-	"github.com/mattn/go-isatty"
 	"github.com/pkg/errors"
 	"github.com/replicatedhq/troubleshoot/internal/util"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
@@ -48,13 +40,16 @@ type LoadOptions struct {
 	// If true, the loader will return an error if any of the specs are not valid
 	// else the invalid specs will be ignored
 	Strict bool
-
-	// Ignore downloading specs from URIs defined in the specs e.g in supportbundle,
-	// a Uri field can be defined to download an additional spec from a remote
-	// location. If this flag is set to true, the loader will ignore downloading it
-	IgnoreUpdateDownloads bool
 }
 
+// TODO: Additional requirements needed in this package
+// * Downloading specs from remote locations e.g oci, s3, http etc
+// 	 * Remote connection error handing
+//   * Support various auth methods
+//   * Retry logic and how to handle timeouts
+// * Support for loading specs from paths e.g directory, file, stdin, tarballs, zips etc
+// * Support for loading specs from a kubernetes cluster - concrete use case of remote location
+
 // LoadSpecs takes sources to load specs from and returns a TroubleshootKinds object
 // that contains all the parsed troubleshoot specs.
 //
@@ -73,37 +68,7 @@ func LoadSpecs(ctx context.Context, opt LoadOptions) (*TroubleshootKinds, error)
 		strict: opt.Strict,
 	}
 
-	kinds, err := l.loadFromStrings(opt.RawSpecs...)
-	if err != nil {
-		return nil, err
-	}
-
-	// Do not download spec updates from remote URIs
-	if opt.IgnoreUpdateDownloads {
-		return kinds, nil
-	}
-
-	// Follow support bundle URIs to download more specs
-	// Clients can later on merge all the available specs
-	moreRawSpecs := []string{}
-	for _, spec := range kinds.SupportBundlesV1Beta2 {
-		if spec.Spec.Uri != "" && util.IsURL(spec.Spec.Uri) {
-			raw, err := loadSpecFromURL(ctx, spec.Spec.Uri)
-			if err != nil {
-				return nil, errors.Wrapf(err, "failed to load support bundle from uri %q", spec.Spec.Uri)
-			}
-			moreRawSpecs = append(moreRawSpecs, string(raw))
-		}
-	}
-
-	moreKinds, err := l.loadFromStrings(moreRawSpecs...)
-	if err != nil {
-		return nil, err
-	}
-
-	kinds.Add(moreKinds)
-
-	return kinds, nil
+	return l.loadFromStrings(opt.RawSpecs...)
 }
 
 type TroubleshootKinds struct {
@@ -374,57 +339,3 @@ func (l *specLoader) getSpecFromSecret(secret *v1.Secret) ([]string, error) {
 	}
 	return specs, nil
 }
-
-func loadSpecFromURL(ctx context.Context, uri string) ([]byte, error) {
-	// TODO: DRY ME - Copied from supportbundle package
-	httpClient := &http.Client{
-		Transport: http.DefaultTransport,
-		Timeout:   30 * time.Second,
-	}
-	for {
-		req, err := http.NewRequestWithContext(ctx, "GET", uri, nil)
-		if err != nil {
-			return nil, errors.Wrap(err, "make request")
-		}
-		req.Header.Set("User-Agent", "Replicated_Troubleshoot/v1beta1")
-		req.Header.Set("Bundle-Upload-Host", fmt.Sprintf("%s://%s", req.URL.Scheme, req.URL.Host))
-		resp, err := httpClient.Do(req)
-		if err != nil {
-			if shouldRetryRequest(err, httpClient) {
-				continue
-			}
-			return nil, errors.Wrap(err, "execute request")
-		}
-		defer resp.Body.Close()
-
-		body, err := io.ReadAll(resp.Body)
-		if err != nil {
-			return nil, errors.Wrap(err, "read responce body")
-		}
-
-		return body, nil
-	}
-}
-
-func shouldRetryRequest(err error, client *http.Client) bool {
-	// TODO: DRY ME - Copied from supportbundle package
-	if strings.Contains(err.Error(), "x509") && canTryInsecure() {
-		client.Transport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
-		return true
-	}
-	return false
-}
-
-func canTryInsecure() bool {
-	// TODO: DRY ME - Copied from supportbundle package
-	if !isatty.IsTerminal(os.Stdout.Fd()) {
-		return false
-	}
-	prompt := promptui.Prompt{
-		Label:     "Connection appears to be insecure. Would you like to attempt to create a support bundle anyway?",
-		IsConfirm: true,
-	}
-
-	_, err := prompt.Run()
-	return err == nil
-}
diff --git a/pkg/loader/loader_test.go b/pkg/loader/loader_test.go
index b6f402de4..5ae629377 100644
--- a/pkg/loader/loader_test.go
+++ b/pkg/loader/loader_test.go
@@ -2,8 +2,6 @@ package loader
 
 import (
 	"context"
-	"net/http"
-	"net/http/httptest"
 	"testing"
 
 	"github.com/replicatedhq/troubleshoot/internal/testutils"
@@ -581,48 +579,3 @@ spec:
       ignoreRBAC: true`)
 	assert.Contains(t, string(y), "message: Cluster is up to date")
 }
-
-func TestDownloadingSpecUpdates(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 := LoadSpecs(ctx, LoadOptions{RawSpec: orig})
-	require.NoError(t, err)
-	require.NotNil(t, kinds)
-
-	assert.Len(t, kinds.SupportBundlesV1Beta2, 2)
-	assert.Equal(t, srv.URL, kinds.SupportBundlesV1Beta2[0].Spec.Uri)
-	assert.Equal(t, "kube-root-ca.crt", kinds.SupportBundlesV1Beta2[0].Spec.Collectors[0].ConfigMap.Name)
-	assert.NotNil(t, kinds.SupportBundlesV1Beta2[1].Spec.Collectors[0].ClusterInfo)
-
-	// Ignore updates
-	kinds, err = LoadSpecs(ctx, LoadOptions{RawSpec: orig, IgnoreUpdateDownloads: true})
-	require.NoError(t, err)
-	require.NotNil(t, kinds)
-
-	assert.Len(t, kinds.SupportBundlesV1Beta2, 1)
-}

From a9d152c3762702ef3dc9d38660ba3fe633079efd Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Fri, 22 Sep 2023 14:45:18 +0100
Subject: [PATCH 07/17] Self review

---
 cmd/troubleshoot/cli/run.go | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go
index 7c3cfc026..b06a21d85 100644
--- a/cmd/troubleshoot/cli/run.go
+++ b/cmd/troubleshoot/cli/run.go
@@ -37,6 +37,9 @@ import (
 
 func runTroubleshoot(v *viper.Viper, args []string) error {
 	ctx := context.Background()
+	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 {
@@ -237,6 +240,9 @@ func loadSupportBundleSpecsFromURIs(ctx context.Context, kinds *loader.Troublesh
 	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)
@@ -268,6 +274,8 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface)
 	}
 
 	// 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",

From 9ed6c4cf8d8a4eece283f47dd2e3af96aea717f7 Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Fri, 22 Sep 2023 16:45:08 +0100
Subject: [PATCH 08/17] More changes after review and testing

---
 cmd/preflight/cli/oci_fetch.go | 14 ++++++++++
 cmd/preflight/cli/root.go      |  2 +-
 internal/specs/specs.go        |  4 +--
 pkg/oci/pull.go                | 51 ++++++++++++++++++++++++++++++----
 4 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/cmd/preflight/cli/oci_fetch.go b/cmd/preflight/cli/oci_fetch.go
index f9bd147aa..7dbc8a175 100644
--- a/cmd/preflight/cli/oci_fetch.go
+++ b/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
 }
diff --git a/cmd/preflight/cli/root.go b/cmd/preflight/cli/root.go
index 10070b1c1..19f8b81d9 100644
--- a/cmd/preflight/cli/root.go
+++ b/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())
 			}
 
diff --git a/internal/specs/specs.go b/internal/specs/specs.go
index c93a7785e..f57fdaef3 100644
--- a/internal/specs/specs.go
+++ b/internal/specs/specs.go
@@ -134,7 +134,7 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 
 			if u.Scheme == "oci" {
 				// TODO: We need to also pull support-bundle images from OCI
-				content, err := oci.PullPreflightFromOCI(v)
+				content, err := oci.PullSpecsFromOCI(ctx, v)
 				if err != nil {
 					if err == oci.ErrNoRelease {
 						return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, errors.Errorf("no release found for %s.\nCheck the oci:// uri for errors or contact the application vendor for support.", v))
@@ -143,7 +143,7 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 					return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, err)
 				}
 
-				rawSpecs = append(rawSpecs, string(content))
+				rawSpecs = append(rawSpecs, content...)
 			} else {
 				if !util.IsURL(v) {
 					return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, fmt.Errorf("%s is not a URL and was not found (err %s)", v, err))
diff --git a/pkg/oci/pull.go b/pkg/oci/pull.go
index 6618262a7..364a46d82 100644
--- a/pkg/oci/pull.go
+++ b/pkg/oci/pull.go
@@ -11,6 +11,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/replicatedhq/troubleshoot/internal/util"
 	"github.com/replicatedhq/troubleshoot/pkg/version"
+	"k8s.io/klog/v2"
 	"oras.land/oras-go/pkg/auth"
 	dockerauth "oras.land/oras-go/pkg/auth/docker"
 	"oras.land/oras-go/pkg/content"
@@ -27,14 +28,52 @@ var (
 )
 
 func PullPreflightFromOCI(uri string) ([]byte, error) {
-	return pullFromOCI(uri, "replicated.preflight.spec", "replicated-preflight")
+	return pullFromOCI(context.Background(), uri, "replicated.preflight.spec", "replicated-preflight")
 }
 
 func PullSupportBundleFromOCI(uri string) ([]byte, error) {
-	return pullFromOCI(uri, "replicated.supportbundle.spec", "replicated-supportbundle")
+	return pullFromOCI(context.Background(), uri, "replicated.supportbundle.spec", "replicated-supportbundle")
 }
 
-func pullFromOCI(uri string, mediaType string, imageName string) ([]byte, error) {
+// PullSpecsFromOCI pulls both the preflight and support bundle specs from the given URI
+//
+// The URI is expected to be the same as the one used to install your KOTS application
+// Example oci://registry.replicated.com/thanos-reloaded/unstable will endup pulling
+// preflights from "registry.replicated.com/thanos-reloaded/unstable/replicated-preflight:latest"
+// and support bundles from "registry.replicated.com/thanos-reloaded/unstable/replicated-supportbundle:latest"
+// Both images have their own media types created when publishing KOTS OCI image.
+// NOTE: This only works with replicated registries for now and for KOTS applications only
+func PullSpecsFromOCI(ctx context.Context, uri string) ([]string, error) {
+	// TODOs (API is opinionated, but we should be able to support these):
+	// - Pulling from generic OCI registries (not just replicated)
+	// - Pulling from registries that require authentication
+	// - Passing in a complete URI including tags and image name
+
+	rawSpecs := []string{}
+
+	// First try to pull the preflight spec
+	rawPreflight, err := pullFromOCI(ctx, uri, "replicated.preflight.spec", "replicated-preflight")
+	if err != nil {
+		// Ignore "not found" error and continue fetching the support bundle spec
+		if !errors.Is(err, ErrNoRelease) {
+			return nil, err
+		}
+	} else {
+		rawSpecs = append(rawSpecs, string(rawPreflight))
+	}
+
+	// Then try to pull the support bundle spec
+	rawSupportBundle, err := pullFromOCI(ctx, uri, "replicated.supportbundle.spec", "replicated-supportbundle")
+	// If we had found a preflight spec, do not return an error
+	if err != nil && len(rawSpecs) == 0 {
+		return nil, err
+	}
+	rawSpecs = append(rawSpecs, string(rawSupportBundle))
+
+	return rawSpecs, nil
+}
+
+func pullFromOCI(ctx context.Context, uri string, mediaType string, imageName string) ([]byte, error) {
 	// helm credentials
 	helmCredentialsFile := filepath.Join(util.HomeDir(), HelmCredentialsFileBasename)
 	dockerauthClient, err := dockerauth.NewClientWithDockerFallback(helmCredentialsFile)
@@ -77,7 +116,9 @@ func pullFromOCI(uri string, mediaType string, imageName string) ([]byte, error)
 		return nil, errors.Wrap(err, "failed to parse reference")
 	}
 
-	manifest, err := oras.Copy(context.TODO(), registryStore, parsedRef.String(), memoryStore, "",
+	klog.V(1).Infof("Pulling spec from %q OCI uri", parsedRef.String())
+
+	manifest, err := oras.Copy(ctx, registryStore, parsedRef.String(), memoryStore, "",
 		oras.WithPullEmptyNameAllowed(),
 		oras.WithAllowedMediaTypes(allowedMediaTypes),
 		oras.WithLayerDescriptors(func(l []ocispec.Descriptor) {
@@ -94,7 +135,7 @@ func pullFromOCI(uri string, mediaType string, imageName string) ([]byte, error)
 	descriptors = append(descriptors, manifest)
 	descriptors = append(descriptors, layers...)
 
-	// expect 1 descriptor
+	// expect 2 descriptors
 	if len(descriptors) != 2 {
 		return nil, fmt.Errorf("expected 2 descriptor, got %d", len(descriptors))
 	}

From a8155b7b6696a5cefed6cb8ea094963141d8342b Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Fri, 22 Sep 2023 17:20:04 +0100
Subject: [PATCH 09/17] fix how we parse oci image uri

---
 pkg/oci/pull.go | 58 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 19 deletions(-)

diff --git a/pkg/oci/pull.go b/pkg/oci/pull.go
index 364a46d82..e67f250c8 100644
--- a/pkg/oci/pull.go
+++ b/pkg/oci/pull.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"net/url"
 	"path/filepath"
 	"strings"
 
@@ -38,9 +39,9 @@ func PullSupportBundleFromOCI(uri string) ([]byte, error) {
 // PullSpecsFromOCI pulls both the preflight and support bundle specs from the given URI
 //
 // The URI is expected to be the same as the one used to install your KOTS application
-// Example oci://registry.replicated.com/thanos-reloaded/unstable will endup pulling
-// preflights from "registry.replicated.com/thanos-reloaded/unstable/replicated-preflight:latest"
-// and support bundles from "registry.replicated.com/thanos-reloaded/unstable/replicated-supportbundle:latest"
+// Example oci://registry.replicated.com/app-slug/unstable will endup pulling
+// preflights from "registry.replicated.com/app-slug/unstable/replicated-preflight:latest"
+// and support bundles from "registry.replicated.com/app-slug/unstable/replicated-supportbundle:latest"
 // Both images have their own media types created when publishing KOTS OCI image.
 // NOTE: This only works with replicated registries for now and for KOTS applications only
 func PullSpecsFromOCI(ctx context.Context, uri string) ([]string, error) {
@@ -99,26 +100,14 @@ func pullFromOCI(ctx context.Context, uri string, mediaType string, imageName st
 	var descriptors, layers []ocispec.Descriptor
 	registryStore := content.Registry{Resolver: resolver}
 
-	// remove the oci://
-	uri = strings.TrimPrefix(uri, "oci://")
-
-	uriParts := strings.Split(uri, ":")
-	uri = fmt.Sprintf("%s/%s", uriParts[0], imageName)
-
-	if len(uriParts) > 1 {
-		uri = fmt.Sprintf("%s:%s", uri, uriParts[1])
-	} else {
-		uri = fmt.Sprintf("%s:latest", uri)
-	}
-
-	parsedRef, err := registry.ParseReference(uri)
+	parsedRef, err := parseURI(uri, imageName)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to parse reference")
+		return nil, err
 	}
 
-	klog.V(1).Infof("Pulling spec from %q OCI uri", parsedRef.String())
+	klog.V(1).Infof("Pulling spec from %q OCI uri", parsedRef)
 
-	manifest, err := oras.Copy(ctx, registryStore, parsedRef.String(), memoryStore, "",
+	manifest, err := oras.Copy(ctx, registryStore, parsedRef, memoryStore, "",
 		oras.WithPullEmptyNameAllowed(),
 		oras.WithAllowedMediaTypes(allowedMediaTypes),
 		oras.WithLayerDescriptors(func(l []ocispec.Descriptor) {
@@ -161,3 +150,34 @@ func pullFromOCI(ctx context.Context, uri string, mediaType string, imageName st
 
 	return matchingSpec, nil
 }
+
+func parseURI(in, imageName string) (string, error) {
+	u, err := url.Parse(in)
+	if err != nil {
+		return "", err
+	}
+
+	// Always check the scheme. If more schemes need to be supported
+	// we need to compare u.Scheme against a list of supported schemes.
+	// url.Parse(raw) will not return an error if a scheme is not present.
+	if u.Scheme != "oci" {
+		return "", fmt.Errorf("%q is an invalid OCI registry scheme", u.Scheme)
+	}
+
+	// remove unnecessary bits (oci://, tags)
+	uriParts := strings.Split(u.EscapedPath(), ":")
+
+	tag := "latest"
+	if len(uriParts) > 1 {
+		tag = uriParts[1]
+	}
+
+	uri := fmt.Sprintf("%s%s/%s:%s", u.Host, uriParts[0], imageName, tag) // <host>:<port>/path/<imageName>:tag
+
+	parsedRef, err := registry.ParseReference(uri)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to parse OCI uri reference")
+	}
+
+	return parsedRef.String(), nil
+}

From 2c9bcda06298a1b8b1e46e5ea460ee90861e5e8a Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Fri, 22 Sep 2023 17:33:31 +0100
Subject: [PATCH 10/17] Remove unnecessary comment

---
 internal/specs/specs.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/internal/specs/specs.go b/internal/specs/specs.go
index f57fdaef3..a90372095 100644
--- a/internal/specs/specs.go
+++ b/internal/specs/specs.go
@@ -133,7 +133,6 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 			}
 
 			if u.Scheme == "oci" {
-				// TODO: We need to also pull support-bundle images from OCI
 				content, err := oci.PullSpecsFromOCI(ctx, v)
 				if err != nil {
 					if err == oci.ErrNoRelease {

From bfc6e4f5dc1647c04554b10466a2382466289860 Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Fri, 22 Sep 2023 18:31:49 +0100
Subject: [PATCH 11/17] Add missing file

---
 pkg/oci/pull_test.go | 66 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 pkg/oci/pull_test.go

diff --git a/pkg/oci/pull_test.go b/pkg/oci/pull_test.go
new file mode 100644
index 000000000..560e93794
--- /dev/null
+++ b/pkg/oci/pull_test.go
@@ -0,0 +1,66 @@
+package oci
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_parseURI(t *testing.T) {
+	tests := []struct {
+		name      string
+		uri       string
+		imageName string
+		wantUri   string
+		wantErr   bool
+	}{
+		{
+			name:      "happy path",
+			uri:       "oci://registry.replicated.com/app-slug/unstable",
+			imageName: "replicated-preflight",
+			wantUri:   "registry.replicated.com/app-slug/unstable/replicated-preflight:latest",
+		},
+		{
+			name:      "no path in uri",
+			uri:       "oci://registry.replicated.com",
+			imageName: "replicated-preflight",
+			wantUri:   "registry.replicated.com/replicated-preflight:latest",
+		},
+		{
+			name:      "hostname with port",
+			uri:       "oci://localhost:5000/some/path",
+			imageName: "replicated-preflight",
+			wantUri:   "localhost:5000/some/path/replicated-preflight:latest",
+		},
+		{
+			name:      "uri with tags",
+			uri:       "oci://localhost:5000/some/path:tag",
+			imageName: "replicated-preflight",
+			wantUri:   "localhost:5000/some/path/replicated-preflight:tag",
+		},
+		{
+			name:    "empty uri",
+			wantErr: true,
+		},
+		{
+			name:      "invalid uri",
+			uri:       "registry.replicated.com/app-slug/unstable",
+			imageName: "replicated-preflight",
+			wantErr:   true,
+		},
+		{
+			name:      "invalid uri",
+			uri:       "https://registry.replicated.com/app-slug/unstable",
+			imageName: "replicated-preflight",
+			wantErr:   true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := parseURI(tt.uri, tt.imageName)
+			require.Equalf(t, tt.wantErr, err != nil, "parseURI() error = %v, wantErr %v", err, tt.wantErr)
+			assert.Equalf(t, tt.wantUri, got, "parseURI() = %v, want %v", got, tt.wantUri)
+		})
+	}
+}

From d9269e28512529d689eb448c006a6b9f01fe2062 Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Fri, 22 Sep 2023 18:36:26 +0100
Subject: [PATCH 12/17] Fix failing tests

---
 cmd/troubleshoot/cli/run.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go
index b06a21d85..528ba0aaa 100644
--- a/cmd/troubleshoot/cli/run.go
+++ b/cmd/troubleshoot/cli/run.go
@@ -75,7 +75,7 @@ func runTroubleshoot(v *viper.Viper, args []string) error {
 	}
 
 	// Check if we have any collectors to run in the support bundle specs
-	if len(mainBundle.Spec.Collectors) == 0 || len(mainBundle.Spec.HostCollectors) == 0 {
+	if len(mainBundle.Spec.Collectors) == 0 && len(mainBundle.Spec.HostCollectors) == 0 {
 		return errors.New("no collectors specified to run")
 	}
 

From 0fbca5713202b713ffe3440d6c50927a046a2451 Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Mon, 25 Sep 2023 11:02:56 +0100
Subject: [PATCH 13/17] Better error check for no collectors

---
 cmd/troubleshoot/cli/run.go | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go
index 528ba0aaa..963dbac48 100644
--- a/cmd/troubleshoot/cli/run.go
+++ b/cmd/troubleshoot/cli/run.go
@@ -74,11 +74,6 @@ func runTroubleshoot(v *viper.Viper, args []string) error {
 		return nil
 	}
 
-	// Check if we have any collectors to run in the support bundle specs
-	if len(mainBundle.Spec.Collectors) == 0 && len(mainBundle.Spec.HostCollectors) == 0 {
-		return errors.New("no collectors specified to run")
-	}
-
 	interactive := v.GetBool("interactive") && isatty.IsTerminal(os.Stdout.Fd())
 
 	if interactive {
@@ -273,6 +268,14 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface)
 		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

From 2b0469417b06915617cf658d49f28f9e82cada7f Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Mon, 25 Sep 2023 12:21:56 +0100
Subject: [PATCH 14/17] Add default collectors when parsing support bundle
 specs

---
 cmd/troubleshoot/cli/run.go | 14 ++++++++++++++
 pkg/loader/loader_test.go   | 21 +++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go
index 963dbac48..bed7c9e35 100644
--- a/cmd/troubleshoot/cli/run.go
+++ b/cmd/troubleshoot/cli/run.go
@@ -20,6 +20,7 @@ import (
 	"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"
@@ -301,6 +302,19 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface)
 		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",
diff --git a/pkg/loader/loader_test.go b/pkg/loader/loader_test.go
index 5ae629377..a1d12d287 100644
--- a/pkg/loader/loader_test.go
+++ b/pkg/loader/loader_test.go
@@ -579,3 +579,24 @@ spec:
       ignoreRBAC: true`)
 	assert.Contains(t, string(y), "message: Cluster is up to date")
 }
+
+func TestLoadingEmptySpec(t *testing.T) {
+	s := testutils.GetTestFixture(t, "supportbundle/empty.yaml")
+	kinds, err := LoadSpecs(context.Background(), LoadOptions{RawSpec: s})
+	require.NoError(t, err)
+	require.NotNil(t, kinds)
+
+	assert.Equal(t, &TroubleshootKinds{
+		SupportBundlesV1Beta2: []troubleshootv1beta2.SupportBundle{
+			{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "SupportBundle",
+					APIVersion: "troubleshoot.sh/v1beta2",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "empty",
+				},
+			},
+		},
+	}, kinds)
+}

From ef61b65f47d0eadbcc55123edb6dc03a20f17226 Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Mon, 25 Sep 2023 12:24:06 +0100
Subject: [PATCH 15/17] Add missed test fixture

---
 testdata/supportbundle/empty.yaml | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 testdata/supportbundle/empty.yaml

diff --git a/testdata/supportbundle/empty.yaml b/testdata/supportbundle/empty.yaml
new file mode 100644
index 000000000..8f1403455
--- /dev/null
+++ b/testdata/supportbundle/empty.yaml
@@ -0,0 +1,5 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: SupportBundle
+metadata:
+  name: empty
+spec: {}

From 35f2187ba47155fb6fecbd319d900a73ab03b289 Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Mon, 25 Sep 2023 13:18:06 +0100
Subject: [PATCH 16/17] Download specs with correct headers

---
 internal/specs/specs.go | 56 ++++++++++++++++++++++++++++++-----------
 1 file changed, 42 insertions(+), 14 deletions(-)

diff --git a/internal/specs/specs.go b/internal/specs/specs.go
index a90372095..9a02bf5f9 100644
--- a/internal/specs/specs.go
+++ b/internal/specs/specs.go
@@ -148,25 +148,22 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 					return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, fmt.Errorf("%s is not a URL and was not found (err %s)", v, err))
 				}
 
-				req, err := http.NewRequestWithContext(ctx, "GET", v, nil)
+				// Download preflight specs
+				rawSpec, err := downloadFromHttpURL(ctx, v, map[string]string{"User-Agent": "Replicated_Preflight/v1beta2"})
 				if err != nil {
-					// exit code: should this be catch all or spec issues...?
-					return nil, types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, err)
+					return nil, err
 				}
-				req.Header.Set("User-Agent", "Replicated_Preflight/v1beta2")
-				resp, err := httpClient.Do(req)
-				if err != nil {
-					// exit code: should this be catch all or spec issues...?
-					return nil, types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, err)
-				}
-				defer resp.Body.Close()
+				rawSpecs = append(rawSpecs, rawSpec)
 
-				body, err := io.ReadAll(resp.Body)
+				// Download support bundle specs
+				rawSpec, err = downloadFromHttpURL(ctx, v, map[string]string{
+					"User-Agent":         "Replicated_Troubleshoot/v1beta1",
+					"Bundle-Upload-Host": fmt.Sprintf("%s://%s", u.Scheme, u.Host),
+				})
 				if err != nil {
-					return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, err)
+					return nil, err
 				}
-
-				rawSpecs = append(rawSpecs, string(body))
+				rawSpecs = append(rawSpecs, rawSpec)
 			}
 		}
 	}
@@ -190,6 +187,37 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 	return kinds, nil
 }
 
+func downloadFromHttpURL(ctx context.Context, url string, headers map[string]string) (string, error) {
+	hs := []string{}
+	for k, v := range headers {
+		hs = append(hs, fmt.Sprintf("%s: %s", k, v))
+	}
+
+	klog.V(1).Infof("Downloading troubleshoot specs: usr=%s, headers=[%v]", url, strings.Join(hs, ", "))
+	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
+	if err != nil {
+		// exit code: should this be catch all or spec issues...?
+		return "", types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, err)
+	}
+	for k, v := range headers {
+		req.Header.Set(k, v)
+	}
+
+	resp, err := httpClient.Do(req)
+	if err != nil {
+		// exit code: should this be catch all or spec issues...?
+		return "", types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, err)
+	}
+	defer resp.Body.Close()
+
+	klog.V(1).Infof("Response status: %s", resp.Status)
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, err)
+	}
+	return string(body), nil
+}
+
 // LoadFromCluster loads troubleshoot specs from the cluster based on the provided labels.
 // By default this will be troubleshoot.io/kind=support-bundle and troubleshoot.sh/kind=support-bundle
 // labels. We search for secrets and configmaps with the label selector and extract the raw data.

From d1f951c19661080de62febf81e240e8e46a90aa4 Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Tue, 26 Sep 2023 12:51:25 +0100
Subject: [PATCH 17/17] Fix typo

---
 internal/specs/specs.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/specs/specs.go b/internal/specs/specs.go
index 9a02bf5f9..218888b12 100644
--- a/internal/specs/specs.go
+++ b/internal/specs/specs.go
@@ -193,7 +193,7 @@ func downloadFromHttpURL(ctx context.Context, url string, headers map[string]str
 		hs = append(hs, fmt.Sprintf("%s: %s", k, v))
 	}
 
-	klog.V(1).Infof("Downloading troubleshoot specs: usr=%s, headers=[%v]", url, strings.Join(hs, ", "))
+	klog.V(1).Infof("Downloading troubleshoot specs: url=%s, headers=[%v]", url, strings.Join(hs, ", "))
 	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
 	if err != nil {
 		// exit code: should this be catch all or spec issues...?