refactor: keep support bundle concat logic to be consistent with Preflight concat (#1002)

* refactor: keep support bundle concat logic to be consistent with Preflight
* test: add tests for support bundle spec concat function

Author: yunju.lly

cmd/troubleshoot/cli/run.go

@@ -82,7 +82,7 @@ func runTroubleshoot(v *viper.Viper, arg []string) error {
 
 	// Defining `v` below will render using `v` in reference to Viper unusable.
 	// Therefore refactoring `v` to `val` will make sure we can still use it.
-	for i, val := range arg {
+	for _, val := range arg {
 
 		collectorContent, err := supportbundle.LoadSupportBundleSpec(val)
 		if err != nil {
@@ -98,11 +98,7 @@ func runTroubleshoot(v *viper.Viper, arg []string) error {
 			return errors.Wrap(err, "failed to parse support bundle spec")
 		}
 
-		if i == 0 {
-			mainBundle = supportBundle
-		} else {
-			mainBundle = supportbundle.ConcatSpec(mainBundle, supportBundle)
-		}
+		mainBundle = supportbundle.ConcatSpec(mainBundle, supportBundle)
 
 		parsedRedactors, err := supportbundle.ParseRedactorsFromDocs(multidocs)
 		if err != nil {

pkg/supportbundle/supportbundle.go

@@ -233,13 +233,21 @@ func AnalyzeSupportBundle(spec *troubleshootv1beta2.SupportBundleSpec, tmpDir st
 	return analyzeResults, nil
 }
 
-// the intention with these appends is to swap them out at a later date with more specific handlers for merging the spec fields
+// ConcatSpec the intention with these appends is to swap them out at a later date with more specific handlers for merging the spec fields
 func ConcatSpec(target *troubleshootv1beta2.SupportBundle, source *troubleshootv1beta2.SupportBundle) *troubleshootv1beta2.SupportBundle {
-	newBundle := target.DeepCopy()
-	newBundle.Spec.Collectors = append(target.Spec.Collectors, source.Spec.Collectors...)
-	newBundle.Spec.AfterCollection = append(target.Spec.AfterCollection, source.Spec.AfterCollection...)
-	newBundle.Spec.HostCollectors = append(target.Spec.HostCollectors, source.Spec.HostCollectors...)
-	newBundle.Spec.HostAnalyzers = append(target.Spec.HostAnalyzers, source.Spec.HostAnalyzers...)
-	newBundle.Spec.Analyzers = append(target.Spec.Analyzers, source.Spec.Analyzers...)
+	if source == nil {
+		return target
+	}
+	var newBundle *troubleshootv1beta2.SupportBundle
+	if target == nil {
+		newBundle = source
+	} else {
+		newBundle = target.DeepCopy()
+		newBundle.Spec.Collectors = append(target.Spec.Collectors, source.Spec.Collectors...)
+		newBundle.Spec.AfterCollection = append(target.Spec.AfterCollection, source.Spec.AfterCollection...)
+		newBundle.Spec.HostCollectors = append(target.Spec.HostCollectors, source.Spec.HostCollectors...)
+		newBundle.Spec.HostAnalyzers = append(target.Spec.HostAnalyzers, source.Spec.HostAnalyzers...)
+		newBundle.Spec.Analyzers = append(target.Spec.Analyzers, source.Spec.Analyzers...)
+	}
 	return newBundle
 }

pkg/supportbundle/supportbundle_test.go

@@ -3,6 +3,8 @@ package supportbundle
 import (
 	"reflect"
 	"testing"
+
+	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
 )
 
 func Test_LoadAndConcatSpec(t *testing.T) {
@@ -44,3 +46,27 @@ func Test_LoadAndConcatSpec(t *testing.T) {
 	}
 
 }
+
+func Test_LoadAndConcatSpec_WithNil(t *testing.T) {
+	var bundle *troubleshootv1beta2.SupportBundle
+	// both function arguments are nil
+	bundle4 := ConcatSpec(bundle, bundle)
+	if reflect.DeepEqual(bundle4, (*troubleshootv1beta2.SupportBundle)(nil)) == false {
+		t.Error("concatenating nil pointer with nil pointer has error.")
+	}
+
+	fulldoc, _ := LoadSupportBundleSpec("test/completebundle.yaml")
+	bundle, _ = ParseSupportBundleFromDoc(fulldoc)
+
+	// targetBundle is nil pointer
+	bundle5 := ConcatSpec((*troubleshootv1beta2.SupportBundle)(nil), bundle)
+	if reflect.DeepEqual(bundle5, bundle) == false {
+		t.Error("concatenating targetBundle of nil pointer has error.")
+	}
+
+	// sourceBundle is nil pointer
+	bundle6 := ConcatSpec(bundle, (*troubleshootv1beta2.SupportBundle)(nil))
+	if reflect.DeepEqual(bundle6, bundle) == false {
+		t.Error("concatenating sourceBundle of nil pointer has error.")
+	}
+}