From 12e0d1f31b9eba8e34dba8f33791035f458d3476 Mon Sep 17 00:00:00 2001
From: kimorris27 <kimorris@redhat.com>
Date: Thu, 21 Nov 2024 10:33:38 -0600
Subject: [PATCH] Skip service principal enrichment for MIWI clusters + add
 unit test case

---
 pkg/util/clusterdata/clusterdata.go      | 11 +++-
 pkg/util/clusterdata/clusterdata_test.go | 81 ++++++++++++++++--------
 2 files changed, 62 insertions(+), 30 deletions(-)

diff --git a/pkg/util/clusterdata/clusterdata.go b/pkg/util/clusterdata/clusterdata.go
index 09635c27761..9b499ff03eb 100644
--- a/pkg/util/clusterdata/clusterdata.go
+++ b/pkg/util/clusterdata/clusterdata.go
@@ -98,10 +98,17 @@ func (p ParallelEnricher) enrichOne(ctx context.Context, log *logrus.Entry, oc *
 		return
 	}
 
-	errors := make(chan error, len(p.enrichers))
+	numEnrichers := len(p.enrichers)
+
+	// We skip the service principal enricher for workload identity clusters
+	if oc.UsesWorkloadIdentity() {
+		numEnrichers = numEnrichers - 1
+	}
+
+	errors := make(chan error, numEnrichers)
 	expectedResults := 0
 	for name, enricher := range p.enrichers {
-		if unsuccessfulEnrichers[name] || enricher == nil {
+		if unsuccessfulEnrichers[name] || enricher == nil || (name == servicePrincipal && oc.UsesWorkloadIdentity()) {
 			continue
 		}
 		p.emitter.EmitGauge("enricher.tasks.count", 1, nil)
diff --git a/pkg/util/clusterdata/clusterdata_test.go b/pkg/util/clusterdata/clusterdata_test.go
index 728b32c4bbc..69057a40799 100644
--- a/pkg/util/clusterdata/clusterdata_test.go
+++ b/pkg/util/clusterdata/clusterdata_test.go
@@ -23,52 +23,69 @@ func TestEnrichOne(t *testing.T) {
 	enricherName := "enricherName"
 
 	for _, tt := range []struct {
-		name                string
-		failedEnrichers     map[string]bool
-		taskCount           int
-		taskDuration        int
-		timeoutCount        int
-		errorCount          int
-		enricherCallCount   int
-		enricherReturnValue error
-		enricherIsNil       bool
+		name                 string
+		failedEnrichers      map[string]bool
+		taskCount            int
+		taskDuration         int
+		timeoutCount         int
+		errorCount           int
+		enricherCallCount    int
+		enricherReturnValue  error
+		enricherIsNil        bool
+		usesWorkloadIdentity bool
 	}{
 		{
-			name:                "enricher called",
-			enricherCallCount:   1,
+			name:                "all enrichers called for service principal cluster",
+			enricherCallCount:   2,
 			enricherReturnValue: nil,
-			taskCount:           1,
-			taskDuration:        1,
+			taskCount:           2,
+			taskDuration:        2,
 			failedEnrichers:     map[string]bool{enricherName: false},
 		},
 		{
-			name:              "enricher not called because failed",
-			enricherCallCount: 0,
-			failedEnrichers:   map[string]bool{enricherName: true},
+			name:                 "service principal enricher skipped for workload identity cluster",
+			enricherCallCount:    1,
+			enricherReturnValue:  nil,
+			taskCount:            1,
+			taskDuration:         1,
+			failedEnrichers:      map[string]bool{enricherName: false},
+			usesWorkloadIdentity: true,
+		},
+		{
+			name:                "enricher not called because failed",
+			enricherCallCount:   1,
+			enricherReturnValue: nil,
+			taskCount:           1,
+			taskDuration:        1,
+			failedEnrichers:     map[string]bool{enricherName: true},
 		},
 		{
 			//should just not panic
-			name:            "enricher not called because nil",
-			failedEnrichers: map[string]bool{enricherName: false},
-			enricherIsNil:   true,
+			name:                "enricher not called because nil",
+			enricherCallCount:   1,
+			enricherReturnValue: nil,
+			taskCount:           1,
+			taskDuration:        1,
+			failedEnrichers:     map[string]bool{enricherName: false},
+			enricherIsNil:       true,
 		},
 		{
 			name:                "enricher timeout",
-			enricherCallCount:   1,
+			enricherCallCount:   2,
 			enricherReturnValue: context.DeadlineExceeded,
 			failedEnrichers:     map[string]bool{enricherName: false},
-			taskCount:           1,
-			taskDuration:        1,
+			taskCount:           2,
+			taskDuration:        2,
 			timeoutCount:        1,
 		},
 		{
 			name:                "enricher error",
-			enricherCallCount:   1,
+			enricherCallCount:   2,
 			enricherReturnValue: errors.New("some error"),
 			failedEnrichers:     map[string]bool{enricherName: false},
-			taskCount:           1,
-			taskDuration:        1,
-			errorCount:          1,
+			taskCount:           2,
+			taskDuration:        2,
+			errorCount:          2,
 		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
@@ -89,7 +106,8 @@ func TestEnrichOne(t *testing.T) {
 			e := ParallelEnricher{
 				emitter: metricsMock,
 				enrichers: map[string]ClusterEnricher{
-					enricherName: enricherMock,
+					enricherName:     enricherMock,
+					servicePrincipal: enricherMock,
 				},
 				metricsWG: &sync.WaitGroup{},
 			}
@@ -97,8 +115,15 @@ func TestEnrichOne(t *testing.T) {
 				e.enrichers[enricherName] = nil
 			}
 
+			oc := &api.OpenShiftCluster{}
+			if tt.usesWorkloadIdentity {
+				oc.Properties.PlatformWorkloadIdentityProfile = &api.PlatformWorkloadIdentityProfile{}
+			} else {
+				oc.Properties.ServicePrincipalProfile = &api.ServicePrincipalProfile{}
+			}
+
 			ctx := context.Background()
-			e.enrichOne(ctx, log, &api.OpenShiftCluster{}, clients{}, tt.failedEnrichers)
+			e.enrichOne(ctx, log, oc, clients{}, tt.failedEnrichers)
 
 			e.metricsWG.Wait()
 		})