Various improvements

* Improve error messages
* Util function appending elements to a nil slice that allows adding
  specs to an empty slice of collectors/analysers/redactors

cmd/analyze/cli/run.go

@@ -27,7 +27,7 @@ func runAnalyzers(v *viper.Viper, bundlePath string) error {
 	} else {
 		if !util.IsURL(specPath) {
 			// TODO: Better error message when we do not have a file/url etc
-			return fmt.Errorf("%s is not a URL and was not found (err %s)", specPath, err)
+			return fmt.Errorf("%s is not a URL and was not found", specPath)
 		}
 
 		req, err := http.NewRequest("GET", specPath, nil)

cmd/collect/cli/run.go

@@ -58,7 +58,7 @@ func runCollect(v *viper.Viper, arg string) error {
 		collectorContent = b
 	} else {
 		if !util.IsURL(arg) {
-			return fmt.Errorf("%s is not a URL and was not found (err %s)", arg, err)
+			return fmt.Errorf("%s is not a URL and was not found", arg)
 		}
 
 		req, err := http.NewRequest("GET", arg, nil)

cmd/troubleshoot/cli/analyze.go

@@ -91,7 +91,7 @@ func downloadAnalyzerSpec(specPath string) (string, error) {
 		specContent = string(b)
 	} else {
 		if !util.IsURL(specPath) {
-			return "", fmt.Errorf("%s is not a URL and was not found (err %s)", specPath, err)
+			return "", fmt.Errorf("%s is not a URL and was not found", specPath)
 		}
 
 		req, err := http.NewRequest("GET", specPath, nil)

cmd/troubleshoot/cli/run.go

@@ -233,7 +233,7 @@ the %s Admin Console to begin analysis.`
 }
 
 // loadSupportBundleSpecsFromURIs loads support bundle specs from URIs
-func loadSupportBundleSpecsFromURIs(ctx context.Context, kinds *loader.TroubleshootKinds) (*loader.TroubleshootKinds, error) {
+func loadSupportBundleSpecsFromURIs(ctx context.Context, kinds *loader.TroubleshootKinds) error {
 	remoteRawSpecs := []string{}
 	for _, s := range kinds.SupportBundlesV1Beta2 {
 		if s.Spec.Uri != "" && util.IsURL(s.Spec.Uri) {
@@ -252,12 +252,18 @@ func loadSupportBundleSpecsFromURIs(ctx context.Context, kinds *loader.Troublesh
 	}
 
 	if len(remoteRawSpecs) == 0 {
-		return kinds, nil
+		return nil
 	}
 
-	return loader.LoadSpecs(ctx, loader.LoadOptions{
+	moreKinds, err := loader.LoadSpecs(ctx, loader.LoadOptions{
 		RawSpecs: remoteRawSpecs,
 	})
+	if err != nil {
+		return err
+	}
+
+	kinds.Add(moreKinds)
+	return nil
 }
 
 func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface) (*troubleshootv1beta2.SupportBundle, *troubleshootv1beta2.Redactor, error) {
@@ -270,11 +276,9 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface)
 
 	// Load additional specs from support bundle URIs
 	if !viper.GetBool("no-uri") {
-		moreKinds, err := loadSupportBundleSpecsFromURIs(ctx, kinds)
+		err := loadSupportBundleSpecsFromURIs(ctx, kinds)
 		if err != nil {
 			klog.Warningf("unable to load support bundles from URIs: %v", err)
-		} else {
-			kinds.Add(moreKinds)
 		}
 	}
 
@@ -304,14 +308,14 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface)
 	}
 
 	for _, c := range kinds.CollectorsV1Beta2 {
-		mainBundle.Spec.Collectors = append(mainBundle.Spec.Collectors, c.Spec.Collectors...)
+		mainBundle.Spec.Collectors = util.Append(mainBundle.Spec.Collectors, c.Spec.Collectors...)
 	}
 
 	for _, hc := range kinds.HostCollectorsV1Beta2 {
-		mainBundle.Spec.HostCollectors = append(mainBundle.Spec.HostCollectors, hc.Spec.Collectors...)
+		mainBundle.Spec.HostCollectors = util.Append(mainBundle.Spec.HostCollectors, hc.Spec.Collectors...)
 	}
 
-	if len(mainBundle.Spec.Collectors) > 0 {
+	if mainBundle.Spec.Collectors != nil {
 		// If we have in-cluster 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.
@@ -336,7 +340,7 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface)
 		},
 	}
 	for _, r := range kinds.RedactorsV1Beta2 {
-		additionalRedactors.Spec.Redactors = append(additionalRedactors.Spec.Redactors, r.Spec.Redactors...)
+		additionalRedactors.Spec.Redactors = util.Append(additionalRedactors.Spec.Redactors, r.Spec.Redactors...)
 	}
 
 	return mainBundle, additionalRedactors, nil

cmd/troubleshoot/cli/run_test.go

@@ -2,6 +2,7 @@ package cli
 
 import (
 	"context"
+	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -48,11 +49,12 @@ spec:
 	require.NoError(t, err)
 	require.NotNil(t, kinds)
 
-	moreKinds, err := loadSupportBundleSpecsFromURIs(ctx, kinds)
+	assert.Len(t, kinds.SupportBundlesV1Beta2, 0)
+	err = loadSupportBundleSpecsFromURIs(ctx, kinds)
 	require.NoError(t, err)
 
-	require.Len(t, moreKinds.SupportBundlesV1Beta2, 1)
-	assert.NotNil(t, moreKinds.SupportBundlesV1Beta2[0].Spec.Collectors[0].ClusterInfo)
+	require.Len(t, kinds.SupportBundlesV1Beta2, 1)
+	assert.NotNil(t, kinds.SupportBundlesV1Beta2[0].Spec.Collectors[0].ClusterInfo)
 }
 
 func Test_loadSupportBundleSpecsFromURIs_TimeoutError(t *testing.T) {
@@ -76,8 +78,14 @@ func Test_loadSupportBundleSpecsFromURIs_TimeoutError(t *testing.T) {
 		httputil.GetHttpClient().Timeout = before
 	}()
 
-	kindsAfter, err := loadSupportBundleSpecsFromURIs(ctx, kinds)
+	beforeJSON, err := json.Marshal(kinds)
 	require.NoError(t, err)
 
-	assert.Equal(t, kinds, kindsAfter)
+	err = loadSupportBundleSpecsFromURIs(ctx, kinds)
+	require.NoError(t, err)
+
+	afterJSON, err := json.Marshal(kinds)
+	require.NoError(t, err)
+
+	assert.JSONEq(t, string(beforeJSON), string(afterJSON))
 }

internal/specs/specs.go

@@ -167,7 +167,7 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 				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))
+					return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, fmt.Errorf("%s is not a URL and was not found", v))
 				}
 
 				// Download preflight specs

internal/util/util.go

@@ -52,3 +52,13 @@ func EstimateNumberOfLines(text string) int {
 	}
 	return n
 }
+
+// Append appends the elements to the slice.
+// If the slice is nil, a new slice is created
+// as opposed to the standard library's append
+func Append[T any](slice []T, v ...T) []T {
+	if slice == nil {
+		slice = []T{}
+	}
+	return append(slice, v...)
+}

pkg/loader/loader.go

@@ -153,6 +153,10 @@ func (l *specLoader) loadFromStrings(rawSpecs ...string) (*TroubleshootKinds, er
 	// For secrets and configmaps, extract support bundle, redactor or preflight specs
 	// For troubleshoot kinds, pass them through
 	for _, rawDoc := range multiRawDocs {
+		if rawDoc == "" {
+			continue
+		}
+
 		var parsed parsedDoc
 
 		err := yaml.Unmarshal([]byte(rawDoc), &parsed)

pkg/supportbundle/load.go

@@ -225,7 +225,7 @@ func loadSpec(arg string) ([]byte, error) {
 	}
 
 	if !util.IsURL(arg) {
-		return nil, fmt.Errorf("%s is not a URL and was not found (err %s)", arg, err)
+		return nil, fmt.Errorf("%s is not a URL and was not found", arg)
 	}
 
 	spec, err := loadSpecFromURL(arg)

pkg/supportbundle/supportbundle.go

@@ -137,7 +137,10 @@ func CollectSupportBundleFromSpec(
 	} else if hostFiles != nil {
 		result = hostFiles
 	} else {
-		return nil, errors.Wrap(err, "failed to generate support bundle")
+		if len(collectorsErrs) > 0 {
+			return nil, fmt.Errorf("failed to generate support bundle: %s", strings.Join(collectorsErrs, "\n"))
+		}
+		return nil, fmt.Errorf("failed to generate support bundle")
 	}
 
 	version, err := getVersionFile()