URI field that returns content that isn't a support bundle causes analysis to fail

[crdant]

Bug Description

I specified I URL to a non-existent object in Google Cloud storage, which led to an XML file being returned explaining the error. The caused the bundle analysis to fail when running kubectl support-bundle --load-cluster-specs.

Expected Behavior

The analyzers and collectors from the spec that is stored in the cluster will be run rather than the non-existent spec from the XML file leading to misbehavior.

Steps To Reproduce

You may be to reproduce with fewer steps, but this will make it most like what I encountered.

1. Include a uri field in a spec that points to a non-existent Google Cloud Storage object
2. Confirm a curl of that object returns an XML indicating an error
3. Store that spec in cluster with at least one other spec in the same namespace
4. Run kubectl support-bundle --load-cluster-specs
5. Observe that the TUI does not come up
6. Extract analysis.json from the bundle and notice its content is []

[hedge-sparrow]

Ah that'll be why...

troubleshoot/cmd/troubleshoot/cli/run.go

Lines 256 to 267 in 9f5f063

	// 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
	// TODO: Pass ctx to LoadSupportBundleSpec
	rawSpec, err := supportbundle.LoadSupportBundleSpec(s.Spec.Uri)
	if err != nil {
	// add back original spec
	moreKinds.SupportBundlesV1Beta2 = append(moreKinds.SupportBundlesV1Beta2, s)
	// In the event a spec can't be loaded, we'll just skip it and print a warning
	klog.Warningf("unable to load support bundle from URI: %q: %v", s.Spec.Uri, err)
	continue
	}

[hedge-sparrow]

so following this through to

troubleshoot/pkg/supportbundle/load.go

Lines 239 to 264 in 9f5f063

	func loadSpecFromURL(arg string) ([]byte, error) {
	for {
	req, err := http.NewRequest("GET", arg, 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))
	httpClient := httputil.GetHttpClient()
	resp, err := httpClient.Do(req)
	if err != nil {
	if shouldRetryRequest(err) {
	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
	}
	}

I don't think this function handles http error statuses, it looks like it only fails if the request itself fails.