diff --git a/cmd/analyze/cli/run.go b/cmd/analyze/cli/run.go index 6fe4e0355..447e630ef 100644 --- a/cmd/analyze/cli/run.go +++ b/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) diff --git a/cmd/collect/cli/run.go b/cmd/collect/cli/run.go index ddbad4f1d..7daf02206 100644 --- a/cmd/collect/cli/run.go +++ b/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) diff --git a/cmd/troubleshoot/cli/analyze.go b/cmd/troubleshoot/cli/analyze.go index 7ddba118b..a69aefe89 100644 --- a/cmd/troubleshoot/cli/analyze.go +++ b/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) diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go index 08a30abc3..4cb4def08 100644 --- a/cmd/troubleshoot/cli/run.go +++ b/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 diff --git a/cmd/troubleshoot/cli/run_test.go b/cmd/troubleshoot/cli/run_test.go index 645e12f2d..2a0c91a0b 100644 --- a/cmd/troubleshoot/cli/run_test.go +++ b/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)) } diff --git a/internal/specs/specs.go b/internal/specs/specs.go index 1fd50c55a..2f14c3d5c 100644 --- a/internal/specs/specs.go +++ b/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 diff --git a/internal/util/util.go b/internal/util/util.go index 90f28e1c5..df45666a5 100644 --- a/internal/util/util.go +++ b/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...) +} diff --git a/pkg/loader/loader.go b/pkg/loader/loader.go index e3a95e5fa..1b3b31d90 100644 --- a/pkg/loader/loader.go +++ b/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) diff --git a/pkg/supportbundle/load.go b/pkg/supportbundle/load.go index d804979e9..bbbaf275f 100644 --- a/pkg/supportbundle/load.go +++ b/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) diff --git a/pkg/supportbundle/supportbundle.go b/pkg/supportbundle/supportbundle.go index 99bf2e02c..3f59a88a3 100644 --- a/pkg/supportbundle/supportbundle.go +++ b/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()