diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go index 92d8ec37..5dc0781b 100644 --- a/cmd/troubleshoot/cli/run.go +++ b/cmd/troubleshoot/cli/run.go @@ -245,35 +245,42 @@ the %s Admin Console to begin analysis.` // loadSupportBundleSpecsFromURIs loads support bundle specs from URIs func loadSupportBundleSpecsFromURIs(ctx context.Context, kinds *loader.TroubleshootKinds) error { - remoteRawSpecs := []string{} + moreKinds := loader.NewTroubleshootKinds() + + // iterate through original kinds and replace any support bundle spec with provided uri spec 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 - // TODO: Pass ctx to LoadSupportBundleSpec - rawSpec, err := supportbundle.LoadSupportBundleSpec(s.Spec.Uri) - if err != nil { - // 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 - } - remoteRawSpecs = append(remoteRawSpecs, string(rawSpec)) + if s.Spec.Uri == "" || !util.IsURL(s.Spec.Uri) { + moreKinds.SupportBundlesV1Beta2 = append(moreKinds.SupportBundlesV1Beta2, s) + continue } + + // 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 + } + k, err := loader.LoadSpecs(ctx, loader.LoadOptions{RawSpec: string(rawSpec)}) + if err != nil { + // add back original spec + moreKinds.SupportBundlesV1Beta2 = append(moreKinds.SupportBundlesV1Beta2, s) + klog.Warningf("unable to load spec: %v", err) + continue + } + + // finally append the uri spec + moreKinds.SupportBundlesV1Beta2 = append(moreKinds.SupportBundlesV1Beta2, k.SupportBundlesV1Beta2...) + } - if len(remoteRawSpecs) == 0 { - return nil - } + kinds.SupportBundlesV1Beta2 = moreKinds.SupportBundlesV1Beta2 - moreKinds, err := loader.LoadSpecs(ctx, loader.LoadOptions{ - RawSpecs: remoteRawSpecs, - }) - if err != nil { - return err - } - - kinds.Add(moreKinds) return nil } diff --git a/cmd/troubleshoot/cli/run_test.go b/cmd/troubleshoot/cli/run_test.go index 26d75c3f..dfc3b715 100644 --- a/cmd/troubleshoot/cli/run_test.go +++ b/cmd/troubleshoot/cli/run_test.go @@ -18,7 +18,8 @@ import ( testclient "k8s.io/client-go/kubernetes/fake" ) -var orig = ` +func templSpec() string { + return ` apiVersion: troubleshoot.sh/v1beta2 kind: SupportBundle metadata: @@ -30,6 +31,7 @@ spec: name: kube-root-ca.crt namespace: default ` +} func Test_loadSupportBundleSpecsFromURIs(t *testing.T) { // Run a webserver to serve the spec @@ -45,7 +47,7 @@ spec: })) defer srv.Close() - orig := strings.ReplaceAll(orig, "$MY_URI", srv.URL) + orig := strings.ReplaceAll(templSpec(), "$MY_URI", srv.URL) ctx := context.Background() kinds, err := loader.LoadSpecs(ctx, loader.LoadOptions{RawSpec: orig}) @@ -57,8 +59,73 @@ spec: err = loadSupportBundleSpecsFromURIs(ctx, kinds) require.NoError(t, err) - require.Len(t, kinds.SupportBundlesV1Beta2, 2) - assert.NotNil(t, kinds.SupportBundlesV1Beta2[1].Spec.Collectors[0].ClusterInfo) + require.Len(t, kinds.SupportBundlesV1Beta2, 1) + assert.NotNil(t, kinds.SupportBundlesV1Beta2[0].Spec.Collectors[0].ClusterInfo) +} + +func Test_loadMultipleSupportBundleSpecsWithNoURIs(t *testing.T) { + ctx := context.Background() + client := testclient.NewSimpleClientset() + specs := []string{testutils.ServeFromFilePath(t, ` +apiVersion: troubleshoot.sh/v1beta2 +kind: SupportBundle +metadata: + name: sb-1 +spec: + collectors: + - clusterInfo:{}`), testutils.ServeFromFilePath(t, ` +apiVersion: troubleshoot.sh/v1beta2 +kind: SupportBundle +metadata: + name: sb-2 + spec: + collectors: + - clusterInfo: {}`)} + + sb, _, err := loadSpecs(ctx, specs, client) + require.NoError(t, err) + require.Len(t, sb.Spec.Collectors, 2) +} + +func Test_loadMultipleSupportBundleSpecsWithURIs(t *testing.T) { + ctx := context.Background() + client := testclient.NewSimpleClientset() + + specFile := testutils.ServeFromFilePath(t, ` +apiVersion: troubleshoot.sh/v1beta2 +kind: SupportBundle +metadata: + name: sb-file +spec: + collectors: + - logs: + name: podlogs/kotsadm + selector: + - app=kotsadm +`) + + // 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-uri +spec: + collectors: + - clusterInfo: {}`)) + })) + defer srv.Close() + + orig := strings.ReplaceAll(templSpec(), "$MY_URI", srv.URL) + specUri := testutils.ServeFromFilePath(t, orig) + specs := []string{specFile, specUri} + + sb, _, err := loadSpecs(ctx, specs, client) + require.NoError(t, err) + assert.NotNil(t, sb.Spec.Collectors[0].Logs) + assert.Nil(t, sb.Spec.Collectors[1].ConfigMap) // original spec gone + assert.NotNil(t, sb.Spec.Collectors[1].ClusterInfo) // new spec from URI } func Test_loadSupportBundleSpecsFromURIs_TimeoutError(t *testing.T) { @@ -69,7 +136,7 @@ func Test_loadSupportBundleSpecsFromURIs_TimeoutError(t *testing.T) { ctx := context.Background() kinds, err := loader.LoadSpecs(ctx, loader.LoadOptions{ - RawSpec: strings.ReplaceAll(orig, "$MY_URI", srv.URL), + RawSpec: strings.ReplaceAll(templSpec(), "$MY_URI", srv.URL), }) require.NoError(t, err)