feat: [sc-106759] Troubleshoot: uri field only download when we're not downloading (#1567)

* remove uri: when url is provided

Author: nvanthao

cmd/troubleshoot/cli/run.go

@@ -293,6 +293,7 @@ func loadSpecs(ctx context.Context, args []string, client kubernetes.Interface)
 	}
 
 	// Load additional specs from support bundle URIs
+	// only when no-uri flag is not set and no URLs are provided in the args
 	if !viper.GetBool("no-uri") {
 		err := loadSupportBundleSpecsFromURIs(ctx, kinds)
 		if err != nil {

cmd/troubleshoot/cli/run_test.go

@@ -371,3 +371,49 @@ spec:
 	assert.Len(t, sb.Spec.HostCollectors, 1)
 	assert.Len(t, sb.Spec.HostAnalyzers, 1)
 }
+
+func Test_loadSpecsFromURL(t *testing.T) {
+	// Run a webserver to serve the URI 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:
+    - logs:
+        name: podlogs/kotsadm
+        selector:
+          - app=kotsadm`))
+	}))
+	defer srv.Close()
+
+	// update URI spec with the server URL
+	orig := strings.ReplaceAll(templSpec(), "$MY_URI", srv.URL)
+
+	// now create a webserver to serve the spec with URI
+	srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte(orig))
+	}))
+	defer srv.Close()
+
+	fileSpec := testutils.ServeFromFilePath(t, `
+apiVersion: troubleshoot.sh/v1beta2
+kind: SupportBundle
+metadata:
+  name: sb
+spec:
+  collectors:
+    - helm: {}`)
+
+	// test and ensure that URI spec is not loaded
+	ctx := context.Background()
+	client := testclient.NewSimpleClientset()
+	sb, _, err := loadSpecs(ctx, []string{fileSpec, srv.URL}, client)
+	require.NoError(t, err)
+	assert.Len(t, sb.Spec.Collectors, 2+2)            // default + clusterInfo + clusterResources
+	assert.NotNil(t, sb.Spec.Collectors[0].Helm)      // come from the file spec
+	assert.NotNil(t, sb.Spec.Collectors[1].ConfigMap) // come from the original spec
+	assert.Nil(t, sb.Spec.Collectors[1].Logs)         // come from the URI spec
+}

internal/specs/specs.go

@@ -78,6 +78,7 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 		ctx = context.Background()
 	}
 
+	var kindsFromURL *loader.TroubleshootKinds
 	rawSpecs := []string{}
 
 	for _, v := range args {
@@ -174,22 +175,35 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 				if err != nil {
 					return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, err)
 				}
+
+				var specFromURL string
 				if parsedURL.Host == "kots.io" {
 					// To download specs from kots.io, we need to set the User-Agent header
-					rawSpec, err := downloadFromHttpURL(ctx, v, map[string]string{
+					specFromURL, err = downloadFromHttpURL(ctx, v, map[string]string{
 						"User-Agent": "Replicated_Troubleshoot/v1beta1",
 					})
 					if err != nil {
 						return nil, err
 					}
-					rawSpecs = append(rawSpecs, rawSpec)
 				} else {
-					rawSpec, err := downloadFromHttpURL(ctx, v, nil)
+					specFromURL, err = downloadFromHttpURL(ctx, v, nil)
 					if err != nil {
 						return nil, err
 					}
-					rawSpecs = append(rawSpecs, rawSpec)
 				}
+
+				// load URL spec first to remove URI key from the spec
+				kindsFromURL, err = loader.LoadSpecs(ctx, loader.LoadOptions{
+					RawSpec: specFromURL,
+				})
+				if err != nil {
+					return nil, err
+				}
+				// remove URI key from the spec if any
+				for i := range kindsFromURL.SupportBundlesV1Beta2 {
+					kindsFromURL.SupportBundlesV1Beta2[i].Spec.Uri = ""
+				}
+
 			}
 		}
 	}
@@ -200,6 +214,9 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
 	if err != nil {
 		return nil, err
 	}
+	if kindsFromURL != nil {
+		kinds.Add(kindsFromURL)
+	}
 
 	if vp.GetBool("load-cluster-specs") {
 		clusterKinds, err := LoadFromCluster(ctx, client, vp.GetStringSlice("selector"), vp.GetString("namespace"))