Fix unnecessary secret updates and logging in STS flow

The operator was repeatedly logging "Secret already exists, updating"
and "Following standardized STS workflow, secret created successfully"
even when the secret content hadn't changed. This was happening because
the CloudStorage controller calls STSStandardizedFlow() on every
reconciliation, which always attempted to create the secret first,
then caught the AlreadyExists error and performed an update.

Changed the approach to:
- First check if the secret exists
- Compare existing data with desired data
- Only update when there are actual differences
- Skip updates and avoid logging when content is identical
- Changed CloudStorage controller to use Debug level and more accurate
  message when STS secret is available (not necessarily created)

This eliminates unnecessary API calls to the Kubernetes cluster and
reduces noise in the operator logs.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>

Author: kaovilai

internal/controller/cloudstorage_controller.go

@@ -132,8 +132,8 @@ func (b CloudStorageReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
 	}
 	if secretName != "" {
-		// Secret was created successfully by STSStandardizedFlow
-		logger.Info(fmt.Sprintf("Following standardized STS workflow, secret %s created successfully", secretName))
+		// Secret exists after STSStandardizedFlow (may have been created, updated, or unchanged)
+		logger.V(1).Info(fmt.Sprintf("Following standardized STS workflow, secret %s is available", secretName))
 	}
 	// Now continue with bucket creation as secret exists and we are good to go !!!
 	if ok, err = clnt.Exists(); !ok && err == nil {

pkg/credentials/stsflow/stsflow.go

@@ -216,7 +216,7 @@ func CreateOrUpdateSTSSecretWithClients(setupLog logr.Logger, secretName string,
 func CreateOrUpdateSTSSecretWithClientsAndWait(setupLog logr.Logger, secretName string, credStringData map[string]string, secretNS string, clientInstance client.Client, clientset kubernetes.Interface, waitForSecret bool) error {
 	// Create a secret with the appropriate credentials format for STS/WIF authentication
 	// Secret format follows standard patterns used by cloud providers
-	secret := corev1.Secret{
+	desiredSecret := corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      secretName,
 			Namespace: secretNS,
@@ -226,48 +226,88 @@ func CreateOrUpdateSTSSecretWithClientsAndWait(setupLog logr.Logger, secretName
 		},
 		StringData: credStringData,
 	}
+
+	// First, try to get the existing secret
+	existingSecret := corev1.Secret{}
+	err := clientInstance.Get(context.Background(), types.NamespacedName{Name: secretName, Namespace: secretNS}, &existingSecret)
+
 	verb := "created"
-	if err := clientInstance.Create(context.Background(), &secret); err != nil {
-		if errors.IsAlreadyExists(err) {
-			verb = "updated"
-			setupLog.Info("Secret already exists, updating")
-			fromCluster := corev1.Secret{}
-			err = clientInstance.Get(context.Background(), types.NamespacedName{Name: secret.Name, Namespace: secret.Namespace}, &fromCluster)
-			if err != nil {
-				setupLog.Error(err, "unable to get existing secret resource")
+	if err != nil {
+		if errors.IsNotFound(err) {
+			// Secret doesn't exist, create it
+			if err := clientInstance.Create(context.Background(), &desiredSecret); err != nil {
+				setupLog.Error(err, "unable to create secret resource")
 				return err
 			}
-			// update StringData - preserve existing Data that's not being replaced
-			// This is safe because STS credentials are only updated during install/reconfiguration,
-			// and any BSL-specific patches (like region) should be preserved
-			updatedFromCluster := fromCluster.DeepCopy()
+		} else {
+			// Some other error occurred while getting the secret
+			setupLog.Error(err, "unable to get secret resource")
+			return err
+		}
+	} else {
+		// Secret exists, check if update is needed
+		needsUpdate := false
+
+		// Check if labels need updating
+		if existingSecret.Labels == nil || existingSecret.Labels["oadp.openshift.io/secret-type"] != "sts-credentials" {
+			needsUpdate = true
+		}
+
+		// Check if data needs updating
+		// Convert existing Data to string for comparison
+		existingData := make(map[string]string)
+		for key, value := range existingSecret.Data {
+			existingData[key] = string(value)
+		}
+
+		// Compare each key in credStringData
+		for key, desiredValue := range credStringData {
+			if existingValue, exists := existingData[key]; !exists || existingValue != desiredValue {
+				needsUpdate = true
+				break
+			}
+		}
+
+		if needsUpdate {
+			verb = "updated"
+			setupLog.Info("Secret content differs, updating")
+
+			// Update the secret
+			updatedSecret := existingSecret.DeepCopy()
+
 			// Initialize StringData if not present
-			if updatedFromCluster.StringData == nil {
-				updatedFromCluster.StringData = make(map[string]string)
+			if updatedSecret.StringData == nil {
+				updatedSecret.StringData = make(map[string]string)
 			}
+
 			// Update only the new StringData fields, preserving existing Data
-			for key, value := range secret.StringData {
-				updatedFromCluster.StringData[key] = value
+			for key, value := range credStringData {
+				updatedSecret.StringData[key] = value
 			}
+
 			// Ensure labels are set
-			if updatedFromCluster.Labels == nil {
-				updatedFromCluster.Labels = make(map[string]string)
+			if updatedSecret.Labels == nil {
+				updatedSecret.Labels = make(map[string]string)
 			}
-			updatedFromCluster.Labels["oadp.openshift.io/secret-type"] = "sts-credentials"
-			if err := clientInstance.Patch(context.Background(), updatedFromCluster, client.MergeFrom(&fromCluster)); err != nil {
+			updatedSecret.Labels["oadp.openshift.io/secret-type"] = "sts-credentials"
+
+			if err := clientInstance.Patch(context.Background(), updatedSecret, client.MergeFrom(&existingSecret)); err != nil {
 				setupLog.Error(err, fmt.Sprintf("unable to update secret resource: %v", err))
 				return err
 			}
 		} else {
-			setupLog.Error(err, "unable to create secret resource")
-			return err
+			// No update needed
+			verb = "unchanged"
 		}
 	}
-	setupLog.Info("Secret " + secret.Name + " " + verb + " successfully")
 
-	if waitForSecret {
-		// Wait for the Secret to be available
-		setupLog.Info(fmt.Sprintf("Waiting for %s Secret to be available", secret.Name))
+	if verb != "unchanged" {
+		setupLog.Info("Secret " + desiredSecret.Name + " " + verb + " successfully")
+	}
+
+	if waitForSecret && verb == "created" {
+		// Wait for the Secret to be available (only needed for newly created secrets)
+		setupLog.Info(fmt.Sprintf("Waiting for %s Secret to be available", desiredSecret.Name))
 		_, err := WaitForSecret(clientset, secretNS, secretName)
 		if err != nil {
 			setupLog.Error(err, "error waiting for credentials Secret")

pkg/credentials/stsflow/stsflow_test.go

@@ -284,18 +284,11 @@ func TestCreateOrUpdateSTSSecret_ErrorScenarios(t *testing.T) {
 	testSecretName := "test-secret"
 	testLogger := zap.New(zap.UseDevMode(true))
 
-	t.Run("Get error during update", func(t *testing.T) {
-		// Create a client that returns an error on Get
-		fakeClient := &mockErrorClient{
-			Client: fake.NewClientBuilder().
-				WithRuntimeObjects(&corev1.Secret{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      testSecretName,
-						Namespace: testNamespace,
-					},
-				}).
-				Build(),
-			getError: true,
+	t.Run("Get error (non-NotFound) during initial check", func(t *testing.T) {
+		// Create a client that returns a non-NotFound error on Get
+		// This simulates a real error (not just secret not existing)
+		fakeClient := &mockErrorClientGenericGetError{
+			Client: fake.NewClientBuilder().Build(),
 		}
 		fakeClientset := k8sfake.NewSimpleClientset()
 
@@ -304,7 +297,7 @@ func TestCreateOrUpdateSTSSecret_ErrorScenarios(t *testing.T) {
 		}, testNamespace, fakeClient, fakeClientset, false)
 
 		assert.Error(t, err)
-		assert.Contains(t, err.Error(), "not found")
+		assert.Contains(t, err.Error(), "unable to get secret resource")
 	})
 
 	t.Run("Patch error during update", func(t *testing.T) {
@@ -329,6 +322,42 @@ func TestCreateOrUpdateSTSSecret_ErrorScenarios(t *testing.T) {
 		assert.Error(t, err)
 		assert.Contains(t, err.Error(), "patch error")
 	})
+
+	t.Run("No update when content is identical", func(t *testing.T) {
+		// Create a secret with the same data we'll try to update with
+		existingSecret := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      testSecretName,
+				Namespace: testNamespace,
+				Labels: map[string]string{
+					"oadp.openshift.io/secret-type": "sts-credentials",
+				},
+			},
+			Data: map[string][]byte{
+				"key": []byte("value"),
+			},
+		}
+		fakeClient := fake.NewClientBuilder().
+			WithRuntimeObjects(existingSecret).
+			Build()
+		fakeClientset := k8sfake.NewSimpleClientset()
+
+		// Try to update with the same content
+		err := CreateOrUpdateSTSSecretWithClientsAndWait(testLogger, testSecretName, map[string]string{
+			"key": "value",
+		}, testNamespace, fakeClient, fakeClientset, false)
+
+		assert.NoError(t, err)
+		// Verify the secret wasn't modified
+		secretResult := &corev1.Secret{}
+		err = fakeClient.Get(context.Background(), client.ObjectKey{
+			Name:      testSecretName,
+			Namespace: testNamespace,
+		}, secretResult)
+		assert.NoError(t, err)
+		// The Data should remain unchanged (no StringData should be set)
+		assert.Equal(t, []byte("value"), secretResult.Data["key"])
+	})
 }
 
 // Mock client that can simulate errors
@@ -357,6 +386,15 @@ func (m *mockErrorClient) Patch(ctx context.Context, obj client.Object, patch cl
 	return m.Client.Patch(ctx, obj, patch, opts...)
 }
 
+// New mock client that returns a generic error on Get (not NotFound)
+type mockErrorClientGenericGetError struct {
+	client.Client
+}
+
+func (m *mockErrorClientGenericGetError) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
+	return errors.NewServiceUnavailable("unable to get secret resource")
+}
+
 func TestSTSStandardizedFlow(t *testing.T) {
 	// Save original env values
 	originalWatchNS := os.Getenv("WATCH_NAMESPACE")