From 4a2a8fe28c52bf4e0dd5de4668262fda4250b0b9 Mon Sep 17 00:00:00 2001
From: jianping-shu <jshu@redhat.com>
Date: Mon, 25 Aug 2025 13:28:50 +0800
Subject: [PATCH] OCPBUGS-29900:fix the Metric cco_credentials_mode issue

---
 pkg/operator/metrics/metrics.go      | 64 +++++++++++++++++++++++++++-
 pkg/operator/metrics/metrics_test.go | 20 +++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/pkg/operator/metrics/metrics.go b/pkg/operator/metrics/metrics.go
index 2e83203694..f901882c79 100644
--- a/pkg/operator/metrics/metrics.go
+++ b/pkg/operator/metrics/metrics.go
@@ -2,6 +2,7 @@ package metrics
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"time"
 
@@ -252,11 +253,17 @@ func (a *credRequestAccumulator) processCR(cr *credreqv1.CredentialsRequest, cco
 
 	isPodIdentity, err := credRequestIsPodIdentity(cr, cloudType, a.kubeClient)
 	if err != nil {
-		a.logger.WithError(err).Error("failed to determine whether CredentialsRequest is of type STS")
+		a.logger.WithError(err).WithField("credentialsRequest", cr.Name).Error("failed to determine whether CredentialsRequest is of type pod identity")
 	}
 
 	if isPodIdentity {
 		a.podIdentityCredentials++
+		a.logger.WithFields(log.Fields{
+			"credentialsRequest": cr.Name,
+			"namespace":          cr.Namespace,
+			"cloudType":          cloudType,
+			"secretRef":          fmt.Sprintf("%s/%s", cr.Spec.SecretRef.Namespace, cr.Spec.SecretRef.Name),
+		}).Debug("detected pod identity credentials")
 	}
 
 	// Skip reporting conditions if CCO is disabled, as we shouldn't be alerting in that case, except for stale credentials.
@@ -364,14 +371,32 @@ func (a *credRequestAccumulator) setMetrics() {
 }
 
 func credRequestIsPodIdentity(cr *credreqv1.CredentialsRequest, cloudType string, kubeClient client.Client) (bool, error) {
+	// Check if SecretRef is set
+	if cr.Spec.SecretRef.Name == "" || cr.Spec.SecretRef.Namespace == "" {
+		log.WithFields(log.Fields{
+				"credentialsRequest": cr.Name,
+			}).Debug("keys Name or Namespace is null")
+		return false, nil
+	}
+
 	secretKey := types.NamespacedName{Name: cr.Spec.SecretRef.Name, Namespace: cr.Spec.SecretRef.Namespace}
 	secret := &corev1.Secret{}
 
 	err := kubeClient.Get(context.TODO(), secretKey, secret)
 	if errors.IsNotFound(err) {
 		// Secret for CredReq doesn't exist so we can't query it
+		log.WithFields(log.Fields{
+			"credentialsRequest": cr.Name,
+			"secretName":         secret.Name,
+			"secretNamespace":    secret.Namespace,
+		}).Debug("AWS secret not found")
 		return false, nil
 	} else if err != nil {
+		log.WithFields(log.Fields{
+			"credentialsRequest": cr.Name,
+			"secretName":         secret.Name,
+			"secretNamespace":    secret.Namespace,
+		}).Debug("AWS secret not loaded due to other error")
 		return false, err
 	}
 
@@ -379,6 +404,18 @@ func credRequestIsPodIdentity(cr *credreqv1.CredentialsRequest, cloudType string
 	case "AWSProviderSpec":
 		secretData, ok := secret.Data[constants.AWSSecretDataCredentialsKey]
 		if !ok {
+			// Log available keys for debugging
+			availableKeys := make([]string, 0, len(secret.Data))
+			for k := range secret.Data {
+				availableKeys = append(availableKeys, k)
+			}
+			log.WithFields(log.Fields{
+				"credentialsRequest": cr.Name,
+				"secretName":         secret.Name,
+				"secretNamespace":    secret.Namespace,
+				"expectedKey":        constants.AWSSecretDataCredentialsKey,
+				"availableKeys":      availableKeys,
+			}).Debug("AWS secret missing expected credentials key")
 			return false, nil
 		}
 
@@ -388,10 +425,35 @@ func credRequestIsPodIdentity(cr *credreqv1.CredentialsRequest, cloudType string
 			return true, nil
 		}
 
+		// Log for debugging when AWS secret doesn't contain STS indicators
+		preview := string(secretData)
+		if len(preview) > 100 {
+			preview = preview[:100] + "..."
+		}
+		log.WithFields(log.Fields{
+			"credentialsRequest": cr.Name,
+			"secretName":         secret.Name,
+			"secretNamespace":    secret.Namespace,
+			"secretDataPreview":  preview,
+		}).Debug("AWS secret does not contain web_identity_token_file, not STS credentials")
+
 		return false, nil
 	case "AzureProviderSpec":
 		_, ok := secret.Data[azure.AzureFederatedTokenFile]
 		return ok, nil
+	case "GCPProviderSpec":
+		secretData, ok := secret.Data["service_account.json"]
+		if !ok {
+			return false, nil
+		}
+
+		// external_account type is a clear indicator that the credentials
+		// are configured for GCP Workload Identity Federation (WIF)
+		if strings.Contains(string(secretData), "external_account") {
+			return true, nil
+		}
+
+		return false, nil
 	default:
 		return false, nil
 	}
diff --git a/pkg/operator/metrics/metrics_test.go b/pkg/operator/metrics/metrics_test.go
index a68f2dd05c..fb011984f7 100644
--- a/pkg/operator/metrics/metrics_test.go
+++ b/pkg/operator/metrics/metrics_test.go
@@ -256,6 +256,26 @@ func TestCredentialsRequests(t *testing.T) {
 				assert.Equal(t, 1, accumulator.podIdentityCredentials)
 			},
 		},
+		{
+			name:        "cco manual mode with GCP Workload Identity Federation",
+			ccoDisabled: true,
+			existingObjects: []runtime.Object{
+				testSecret("wif-namespace", "wif-name", map[string][]byte{
+					"service_account.json": []byte(`{"type": "external_account", "audience": "//iam.googleapis.com/projects/123/locations/global/workloadIdentityPools/pool/providers/provider"}`),
+				}),
+				testSecret("non-wif-namespace", "non-wif-name", map[string][]byte{
+					"service_account.json": []byte(`{"type": "service_account", "project_id": "my-project"}`),
+				}),
+			},
+			credReqs: []credreqv1.CredentialsRequest{
+				testCredRequestWithSecretRef(testGCPCredRequest("wif-style"), "wif-namespace", "wif-name"),
+				testCredRequestWithSecretRef(testGCPCredRequest("non-wif-style"), "non-wif-namespace", "non-wif-name"),
+			},
+			validate: func(t *testing.T, accumulator *credRequestAccumulator) {
+				assert.Equal(t, 2, accumulator.crTotals["gcp"])
+				assert.Equal(t, 1, accumulator.podIdentityCredentials)
+			},
+		},
 	}
 
 	for _, test := range tests {