From 289102f16ddfab5df50ba3c93145b6e4e93269ee Mon Sep 17 00:00:00 2001
From: Gerard Nguyen <gerard@replicated.com>
Date: Fri, 18 Oct 2024 15:58:32 +1100
Subject: [PATCH] bug: fix nil check in host collector filter (#1653)

* add nil check in filter host collector
---
 pkg/supportbundle/collect.go      | 10 +++----
 pkg/supportbundle/collect_test.go | 45 +++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 5 deletions(-)
 create mode 100644 pkg/supportbundle/collect_test.go

diff --git a/pkg/supportbundle/collect.go b/pkg/supportbundle/collect.go
index 6cff37701..303b68bcc 100644
--- a/pkg/supportbundle/collect.go
+++ b/pkg/supportbundle/collect.go
@@ -290,7 +290,6 @@ func collectHost(ctx context.Context, filteredCollectors []FilteredCollector, op
 		if err != nil {
 			span.SetStatus(codes.Error, err.Error())
 			opts.ProgressChan <- fmt.Sprintf("[%s] Error: %v", collector.Title(), err)
-			return errors.Wrap(err, "failed to run host collector")
 		}
 
 		// Send progress event: completed successfully
@@ -332,13 +331,14 @@ func filterHostCollectors(ctx context.Context, collectSpecs []*troubleshootv1bet
 
 	for _, desiredCollector := range collectSpecs {
 		collector, ok := collect.GetHostCollector(desiredCollector, bundlePath)
-		_, span := otel.Tracer(constants.LIB_TRACER_NAME).Start(ctx, collector.Title())
-		span.SetAttributes(attribute.String("type", reflect.TypeOf(collector).String()))
-
 		if !ok {
-			return nil, collect.ErrHostCollectorNotFound
+			opts.ProgressChan <- "Host collector not found"
+			continue
 		}
 
+		_, span := otel.Tracer(constants.LIB_TRACER_NAME).Start(ctx, collector.Title())
+		span.SetAttributes(attribute.String("type", reflect.TypeOf(collector).String()))
+
 		isExcluded, _ := collector.IsExcluded()
 		if isExcluded {
 			opts.ProgressChan <- fmt.Sprintf("[%s] Excluding host collector", collector.Title())
diff --git a/pkg/supportbundle/collect_test.go b/pkg/supportbundle/collect_test.go
new file mode 100644
index 000000000..0dfda3e68
--- /dev/null
+++ b/pkg/supportbundle/collect_test.go
@@ -0,0 +1,45 @@
+package supportbundle
+
+import (
+	"context"
+	"testing"
+
+	v1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/trace/noop"
+)
+
+func Test_filterHostCollectors(t *testing.T) {
+	otel.SetTracerProvider(noop.NewTracerProvider())
+
+	testCases := []struct {
+		name           string
+		collectSpecs   []*v1beta2.HostCollect
+		bundlePath     string
+		opts           SupportBundleCreateOpts
+		expectedResult []FilteredCollector
+		expectedError  error
+	}{
+		{
+			name:         "nil host collectors spec",
+			collectSpecs: []*v1beta2.HostCollect{},
+			bundlePath:   "/tmp",
+			opts: SupportBundleCreateOpts{
+				ProgressChan: make(chan interface{}, 10),
+			},
+			expectedResult: []FilteredCollector{},
+			expectedError:  nil,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			filtered, err := filterHostCollectors(context.TODO(), tc.collectSpecs, tc.bundlePath, tc.opts)
+			if err != tc.expectedError {
+				t.Fatalf("expected error %v, got %v", tc.expectedError, err)
+			}
+			if len(filtered) != len(tc.expectedResult) {
+				t.Fatalf("expected %d filtered collectors, got %d", len(tc.expectedResult), len(filtered))
+			}
+		})
+	}
+}