K8SPSMDB-1602: address PR review comments

TineoC · TineoC · commit ff17396c9489 · 2026-06-16T08:44:59.000-04:00
- Remove explicit workloadIdentity field from API
- Follow AWS S3 pattern: empty credentialsSecret triggers ADC fallback (hors feedback)
- Remove workloadIdentity from all CRD YAMLs via make generate manifests
- Add E2E test: demand-backup-gcs-workload-identity (mayankshah1607 feedback)
- Keep PBM-side ADC fallback for when credentials are not provided
diff --git a/config/crd/bases/psmdb.percona.com_perconaservermongodbbackups.yaml b/config/crd/bases/psmdb.percona.com_perconaservermongodbbackups.yaml
@@ -139,8 +139,6 @@ spec:
                     - backoffMax
                     - backoffMultiplier
                     type: object
-                  workloadIdentity:
-                    type: boolean
                 required:
                 - bucket
                 type: object
diff --git a/config/crd/bases/psmdb.percona.com_perconaservermongodbrestores.yaml b/config/crd/bases/psmdb.percona.com_perconaservermongodbrestores.yaml
@@ -97,8 +97,6 @@ spec:
                         - backoffMax
                         - backoffMultiplier
                         type: object
-                      workloadIdentity:
-                        type: boolean
                     required:
                     - bucket
                     type: object
diff --git a/config/crd/bases/psmdb.percona.com_perconaservermongodbs.yaml b/config/crd/bases/psmdb.percona.com_perconaservermongodbs.yaml
@@ -361,8 +361,6 @@ spec:
                               - backoffMax
                               - backoffMultiplier
                               type: object
-                            workloadIdentity:
-                              type: boolean
                           required:
                           - bucket
                           type: object
diff --git a/deploy/bundle.yaml b/deploy/bundle.yaml
@@ -143,8 +143,6 @@ spec:
                     - backoffMax
                     - backoffMultiplier
                     type: object
-                  workloadIdentity:
-                    type: boolean
                 required:
                 - bucket
                 type: object
@@ -391,8 +389,6 @@ spec:
                         - backoffMax
                         - backoffMultiplier
                         type: object
-                      workloadIdentity:
-                        type: boolean
                     required:
                     - bucket
                     type: object
@@ -1317,8 +1313,6 @@ spec:
                               - backoffMax
                               - backoffMultiplier
                               type: object
-                            workloadIdentity:
-                              type: boolean
                           required:
                           - bucket
                           type: object
diff --git a/deploy/crd.yaml b/deploy/crd.yaml
@@ -143,8 +143,6 @@ spec:
                     - backoffMax
                     - backoffMultiplier
                     type: object
-                  workloadIdentity:
-                    type: boolean
                 required:
                 - bucket
                 type: object
@@ -391,8 +389,6 @@ spec:
                         - backoffMax
                         - backoffMultiplier
                         type: object
-                      workloadIdentity:
-                        type: boolean
                     required:
                     - bucket
                     type: object
@@ -1317,8 +1313,6 @@ spec:
                               - backoffMax
                               - backoffMultiplier
                               type: object
-                            workloadIdentity:
-                              type: boolean
                           required:
                           - bucket
                           type: object
diff --git a/deploy/cw-bundle.yaml b/deploy/cw-bundle.yaml
@@ -143,8 +143,6 @@ spec:
                     - backoffMax
                     - backoffMultiplier
                     type: object
-                  workloadIdentity:
-                    type: boolean
                 required:
                 - bucket
                 type: object
@@ -391,8 +389,6 @@ spec:
                         - backoffMax
                         - backoffMultiplier
                         type: object
-                      workloadIdentity:
-                        type: boolean
                     required:
                     - bucket
                     type: object
@@ -1317,8 +1313,6 @@ spec:
                               - backoffMax
                               - backoffMultiplier
                               type: object
-                            workloadIdentity:
-                              type: boolean
                           required:
                           - bucket
                           type: object
diff --git a/e2e-tests/demand-backup-gcs-workload-identity/conf/backup-gcs-wi.yml b/e2e-tests/demand-backup-gcs-workload-identity/conf/backup-gcs-wi.yml
@@ -0,0 +1,10 @@
+apiVersion: psmdb.percona.com/v1
+kind: PerconaServerMongoDBBackup
+metadata:
+  finalizers:
+    - percona.com/delete-backup
+  name:
+spec:
+  type: physical
+  clusterName: some-name
+  storageName:
diff --git a/e2e-tests/demand-backup-gcs-workload-identity/conf/restore.yml b/e2e-tests/demand-backup-gcs-workload-identity/conf/restore.yml
@@ -0,0 +1,7 @@
+apiVersion: psmdb.percona.com/v1
+kind: PerconaServerMongoDBRestore
+metadata:
+  name:
+spec:
+  clusterName: some-name
+  backupName:
diff --git a/e2e-tests/demand-backup-gcs-workload-identity/conf/some-name.yml b/e2e-tests/demand-backup-gcs-workload-identity/conf/some-name.yml
@@ -0,0 +1,63 @@
+apiVersion: psmdb.percona.com/v1
+kind: PerconaServerMongoDB
+metadata:
+  finalizers:
+  - percona.com/delete-psmdb-pvc
+  name: some-name
+spec:
+  image:
+  imagePullPolicy: Always
+  updateStrategy: SmartUpdate
+  backup:
+    enabled: true
+    image: perconalab/percona-server-mongodb-operator:1.1.0-backup
+    serviceAccountName: percona-server-mongodb-operator
+    storages:
+      gcs-wi:
+        type: gcs
+        gcs:
+          bucket: operator-testing
+          prefix: psmdb-demand-backup-gcs-wi
+  replsets:
+  - name: rs0
+    affinity:
+      antiAffinityTopologyKey: none
+    resources:
+      limits:
+        cpu: 500m
+        memory: 1G
+      requests:
+        cpu: 100m
+        memory: 0.1G
+    volumeSpec:
+      persistentVolumeClaim:
+        resources:
+          requests:
+            storage: 3Gi
+    expose:
+      enabled: false
+      type: ClusterIP
+    size: 3
+    configuration: |
+      operationProfiling:
+        mode: slowOp
+        slowOpThresholdMs: 100
+      security:
+        enableEncryption: true
+        redactClientLogData: false
+      setParameter:
+        ttlMonitorSleepSecs: 60
+        wiredTigerConcurrentReadTransactions: 128
+        wiredTigerConcurrentWriteTransactions: 128
+      storage:
+        engine: wiredTiger
+        wiredTiger:
+          collectionConfig:
+            blockCompressor: snappy
+          engineConfig:
+            directoryForIndexes: false
+            journalCompressor: snappy
+          indexConfig:
+            prefixCompression: true
+  secrets:
+    users: some-users
diff --git a/e2e-tests/demand-backup-gcs-workload-identity/run b/e2e-tests/demand-backup-gcs-workload-identity/run
@@ -0,0 +1,70 @@
+#!/bin/bash
+
+set -o errexit
+
+test_dir=$(realpath "$(dirname "$0")")
+. "${test_dir}/../functions"
+set_debug
+
+if [ -z "$GKE" ]; then
+	desc 'Skip test. Set GKE=1 to run on GKE cluster.'
+	exit 0
+fi
+
+if [ -n "$SKIP_BACKUPS_TO_AWS_GCP_AZURE" ]; then
+	desc 'Skip tests related to GCP Cloud Storage'
+	exit 0
+fi
+
+if [ -z "$GCP_PROJECT" ] || [ -z "$GCS_WI_SERVICE_ACCOUNT" ]; then
+	desc 'Skip test. Set GCP_PROJECT and GCS_WI_SERVICE_ACCOUNT for GCS Workload Identity test.'
+	exit 0
+fi
+
+create_infra "${namespace}"
+
+desc 'create GCP workload identity binding'
+kubectl_bin annotate serviceaccount \
+	--namespace "${namespace}" \
+	"${namespace}-psmdb-db" \
+	"iam.gke.io/gcp-service-account=${GCS_WI_SERVICE_ACCOUNT}"
+
+desc 'create PSMDB cluster without GCS credentialsSecret'
+cluster="some-name"
+kubectl_bin apply -f "${test_dir}/conf/${cluster}.yml"
+kubectl_bin apply -f "${conf_dir}/client_with_tls.yml"
+
+desc 'check if all pods started'
+wait_for_running "${cluster}-rs0" 3
+wait_cluster_consistency "${cluster}"
+
+sleep 60
+wait_for_pbm_operations "${cluster}"
+
+desc 'write test data'
+run_mongo \
+	'db.createUser({user:"myApp",pwd:"myPass",roles:[{db:"myApp",role:"readWrite"}]})' \
+	"userAdmin:userAdmin123456@${cluster}-rs0.${namespace}"
+sleep 1
+run_mongo \
+	'use myApp\n db.test.insert({ x: 100500 })' \
+	"myApp:myPass@${cluster}-rs0.${namespace}"
+sleep 5
+compare_mongo_cmd "find" "myApp:myPass@${cluster}-rs0-0.${cluster}-rs0.${namespace}"
+compare_mongo_cmd "find" "myApp:myPass@${cluster}-rs0-1.${cluster}-rs0.${namespace}"
+compare_mongo_cmd "find" "myApp:myPass@${cluster}-rs0-2.${cluster}-rs0.${namespace}"
+
+desc 'run GCS backup with workload identity'
+backup_name="backup-gcs-wi"
+run_backup gcs-wi "${backup_name}" 'physical'
+wait_backup "${backup_name}"
+check_backup_in_storage "${backup_name}" gcs rs0
+
+desc 'drop and restore'
+run_mongo 'use myApp\n db.test.drop()' "myApp:myPass@${cluster}-rs0.${namespace}"
+run_restore "${backup_name}"
+run_recovery_check "${backup_name}" "${cluster}"
+
+destroy "$namespace"
+
+desc 'test passed'
diff --git a/e2e-tests/run-pr.csv b/e2e-tests/run-pr.csv
@@ -27,6 +27,7 @@ demand-backup-physical-parallel
 demand-backup-physical-aws
 demand-backup-physical-azure
 demand-backup-physical-gcp-s3
+demand-backup-gcs-workload-identity
 demand-backup-physical-gcp-native
 demand-backup-physical-minio
 demand-backup-physical-minio-native
diff --git a/e2e-tests/run-release.csv b/e2e-tests/run-release.csv
@@ -28,6 +28,7 @@ demand-backup-physical-parallel
 demand-backup-physical-aws
 demand-backup-physical-azure
 demand-backup-physical-gcp-s3
+demand-backup-gcs-workload-identity
 demand-backup-physical-gcp-native
 demand-backup-physical-minio
 demand-backup-physical-minio-native
diff --git a/e2e-tests/version-service/conf/crd.yaml b/e2e-tests/version-service/conf/crd.yaml
@@ -143,8 +143,6 @@ spec:
                     - backoffMax
                     - backoffMultiplier
                     type: object
-                  workloadIdentity:
-                    type: boolean
                 required:
                 - bucket
                 type: object
@@ -391,8 +389,6 @@ spec:
                         - backoffMax
                         - backoffMultiplier
                         type: object
-                      workloadIdentity:
-                        type: boolean
                     required:
                     - bucket
                     type: object
@@ -1317,8 +1313,6 @@ spec:
                               - backoffMax
                               - backoffMultiplier
                               type: object
-                            workloadIdentity:
-                              type: boolean
                           required:
                           - bucket
                           type: object
diff --git a/pkg/apis/psmdb/v1/psmdb_types.go b/pkg/apis/psmdb/v1/psmdb_types.go
@@ -1321,7 +1321,6 @@ type BackupStorageGCSSpec struct {
 	Bucket            string      `json:"bucket"`
 	Prefix            string      `json:"prefix,omitempty"`
 	CredentialsSecret string      `json:"credentialsSecret,omitempty"`
-	WorkloadIdentity  bool        `json:"workloadIdentity,omitempty"`
 	ChunkSize         int         `json:"chunkSize,omitempty"`
 	Retryer           *GCSRetryer `json:"retryer,omitempty"`
 }
diff --git a/pkg/psmdb/backup/pbm.go b/pkg/psmdb/backup/pbm.go
@@ -505,33 +505,25 @@ func GetPBMStorageGCSConfig(
 		},
 	}
 
-	// When WorkloadIdentity is enabled, skip credential secret loading entirely.
-	// PBM will use Application Default Credentials (ADC) provided by GKE Workload Identity.
-	if stg.GCS.WorkloadIdentity {
-		return storageConf, nil
-	}
-
-	if stg.GCS.CredentialsSecret == "" {
-		return config.StorageConf{}, errors.New("no credentials specified for the secret name")
-	}
-
-	gcsSecret, err := getSecret(ctx, k8sclient, cluster.Namespace, stg.GCS.CredentialsSecret)
-	if err != nil {
-		return config.StorageConf{}, errors.Wrap(err, "get GCS credentials secret")
-	}
+	if stg.GCS.CredentialsSecret != "" {
+		gcsSecret, err := getSecret(ctx, k8sclient, cluster.Namespace, stg.GCS.CredentialsSecret)
+		if err != nil {
+			return config.StorageConf{}, errors.Wrap(err, "get GCS credentials secret")
+		}
 
-	if _, ok := gcsSecret.Data[GCSClientEmailSecretKey]; ok {
-		storageConf.GCS.Credentials = gcs.Credentials{
-			ClientEmail: storage.MaskedString(string(gcsSecret.Data[GCSClientEmailSecretKey])),
-			PrivateKey:  storage.MaskedString(string(gcsSecret.Data[GCSPrivateKeySecretKey])),
+		if _, ok := gcsSecret.Data[GCSClientEmailSecretKey]; ok {
+			storageConf.GCS.Credentials = gcs.Credentials{
+				ClientEmail: storage.MaskedString(string(gcsSecret.Data[GCSClientEmailSecretKey])),
+				PrivateKey:  storage.MaskedString(string(gcsSecret.Data[GCSPrivateKeySecretKey])),
+			}
 		}
-	}
 
-	// s3 compatibility
-	if _, ok := gcsSecret.Data[AWSAccessKeySecretKey]; ok {
-		storageConf.GCS.Credentials = gcs.Credentials{
-			HMACAccessKey: storage.MaskedString(string(gcsSecret.Data[AWSAccessKeySecretKey])),
-			HMACSecret:    storage.MaskedString(string(gcsSecret.Data[AWSSecretAccessKeySecretKey])),
+		// s3 compatibility
+		if _, ok := gcsSecret.Data[AWSAccessKeySecretKey]; ok {
+			storageConf.GCS.Credentials = gcs.Credentials{
+				HMACAccessKey: storage.MaskedString(string(gcsSecret.Data[AWSAccessKeySecretKey])),
+				HMACSecret:    storage.MaskedString(string(gcsSecret.Data[AWSSecretAccessKeySecretKey])),
+			}
 		}
 	}
 
diff --git a/pkg/psmdb/backup/pbm_test.go b/pkg/psmdb/backup/pbm_test.go
@@ -346,15 +346,14 @@ func TestPBMStorageConfig(t *testing.T) {
 				},
 			},
 		},
-		"gcs workload identity": {
+		"gcs without credentials (ADC / Workload Identity)": {
 			[]client.Object{},
 			api.BackupStorageSpec{
 				Type: api.BackupStorageGCS,
 				GCS: api.BackupStorageGCSSpec{
-					Bucket:           "operator-testing",
-					Prefix:           "psmdb",
-					WorkloadIdentity: true,
-					ChunkSize:        1024 * 1024 * 10,
+					Bucket:    "operator-testing",
+					Prefix:    "psmdb",
+					ChunkSize: 1024 * 1024 * 10,
 				},
 			},
 			config.StorageConf{
