gsm-secrets: make the updater service account per collection opt-in

pkg/group/config.go

@@ -38,6 +38,10 @@ type Target struct {
 	// The gsm-secret-sync reconciler keeps these collections' secrets alive but creates no
 	// updater service account and no IAM bindings for them until they are moved under a normal group.
 	Unclaimed bool `json:"unclaimed,omitempty" yaml:"unclaimed,omitempty"`
+	// UpdaterServiceAccounts lists the secret collections, a subset of SecretCollections, that
+	// get their own updater service account. Group members can already write to every collection
+	// the group owns; a service account is for automation that cannot authenticate as one of them.
+	UpdaterServiceAccounts []string `json:"updater_service_accounts,omitempty" yaml:"updater_service_accounts,omitempty"`
 }
 
 func (t Target) ResolveClusters(cg map[string][]string) sets.Set[string] {
@@ -90,6 +94,19 @@ func (c *Config) validate() error {
 			}
 			seen.Insert(collection)
 		}
+		withSA := sets.New[string]()
+		for _, collection := range v.UpdaterServiceAccounts {
+			if !seen.Has(collection) {
+				return fmt.Errorf("group '%s' requests an updater service account for '%s', which is not one of its secret collections", k, collection)
+			}
+			if withSA.Has(collection) {
+				return fmt.Errorf("secret collection '%s' is listed more than once under updater_service_accounts for group '%s' in the configuration file", collection, k)
+			}
+			withSA.Insert(collection)
+		}
+		if v.Unclaimed && len(v.UpdaterServiceAccounts) > 0 {
+			return fmt.Errorf("unclaimed group '%s' cannot request updater service accounts", k)
+		}
 	}
 	return nil
 }

pkg/group/config_test.go

@@ -38,6 +38,21 @@ func TestLoadConfig(t *testing.T) {
 			file:        filepath.Join("testdata", "TestLoadConfig", "duplicate_secret_collection.yaml"),
 			expectedErr: fmt.Errorf("failed to validate config file: secret collection 'wildfly-charts-secrets' is listed more than once for group 'test-platform-gsm-secrets-owners' in the configuration file"),
 		},
+		{
+			name:        "an updater service account can only be requested for a collection the group owns",
+			file:        filepath.Join("testdata", "TestLoadConfig", "updater_sa_not_a_collection.yaml"),
+			expectedErr: fmt.Errorf("failed to validate config file: group 'test-platform-gsm-secrets-owners' requests an updater service account for 'not-mine', which is not one of its secret collections"),
+		},
+		{
+			name:        "a collection cannot be listed twice under updater_service_accounts",
+			file:        filepath.Join("testdata", "TestLoadConfig", "duplicate_updater_sa.yaml"),
+			expectedErr: fmt.Errorf("failed to validate config file: secret collection 'test-platform-infra' is listed more than once under updater_service_accounts for group 'test-platform-gsm-secrets-owners' in the configuration file"),
+		},
+		{
+			name:        "an unclaimed group cannot request updater service accounts",
+			file:        filepath.Join("testdata", "TestLoadConfig", "unclaimed_with_updater_sa.yaml"),
+			expectedErr: fmt.Errorf("failed to validate config file: unclaimed group 'test-platform-gsm-unclaimed-secrets' cannot request updater service accounts"),
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {

pkg/group/testdata/TestLoadConfig/duplicate_updater_sa.yaml

@@ -0,0 +1,7 @@
+groups:
+  test-platform-gsm-secrets-owners:
+    secret_collections:
+    - test-platform-infra
+    updater_service_accounts:
+    - test-platform-infra
+    - test-platform-infra

pkg/group/testdata/TestLoadConfig/unclaimed_with_updater_sa.yaml

@@ -0,0 +1,7 @@
+groups:
+  test-platform-gsm-unclaimed-secrets:
+    unclaimed: true
+    secret_collections:
+    - orphan-secrets
+    updater_service_accounts:
+    - orphan-secrets

pkg/group/testdata/TestLoadConfig/updater_sa_not_a_collection.yaml

@@ -0,0 +1,6 @@
+groups:
+  test-platform-gsm-secrets-owners:
+    secret_collections:
+    - test-platform-infra
+    updater_service_accounts:
+    - not-mine

pkg/gsm-secrets/config.go

@@ -14,10 +14,13 @@ import (
 
 // GetDesiredState parses the configuration file and builds the desired state specifications.
 //
-// Collections owned by a normal ("claimed") group each get an updater service account, its SA
-// secret, an index secret, and service-account-scoped viewer/updater bindings limited to that
-// single collection. Each owning group additionally gets one viewer and one updater binding
-// covering all of its collections.
+// Collections owned by a normal ("claimed") group each get an index secret, and their owning
+// group gets one viewer and one updater binding covering all of its collections.
+//
+// A collection additionally gets an updater service account, its SA secret, and
+// service-account-scoped viewer/updater bindings only if the group opted into one for it via
+// group.Target.UpdaterServiceAccounts. Group members are unaffected either way: the group
+// bindings already cover every collection the group owns.
 //
 // Collections owned only by an "unclaimed" group (see group.Target.Unclaimed) are kept in the
 // active-collection set so their migrated data secrets are not deleted, but they get no service
@@ -38,13 +41,15 @@ func GetDesiredState(configFile string, config Config) ([]ServiceAccountInfo, ma
 
 	claimedCollections := sets.New[string]()
 	unclaimedCollections := sets.New[string]()
+	collectionsWithSA := sets.New[string]()
 	for _, name := range groupNames {
 		groupCfg := groupConfig.Groups[name]
 		if groupCfg.Unclaimed {
 			unclaimedCollections.Insert(groupCfg.SecretCollections...)
 			continue
 		}
 		claimedCollections.Insert(groupCfg.SecretCollections...)
+		collectionsWithSA.Insert(groupCfg.UpdaterServiceAccounts...)
 	}
 
 	var desiredSAs []ServiceAccountInfo
@@ -58,30 +63,32 @@ func GetDesiredState(configFile string, config Config) ([]ServiceAccountInfo, ma
 		desiredCollections[collection] = true
 	}
 
-	// Per claimed collection: an updater service account, its SA secret, an index secret, and
-	// service-account-scoped viewer/updater bindings limited to that single collection. The
-	// service account is given its own bindings rather than being grouped with the owning group,
-	// so its access stays scoped to exactly one collection.
 	for _, collection := range sets.List(claimedCollections) {
+		desiredSecrets[GetIndexSecretName(collection)] = GCPSecret{
+			Name:       GetIndexSecretName(collection),
+			Type:       SecretTypeIndex,
+			Collection: collection,
+		}
+
+		if !collectionsWithSA.Has(collection) {
+			continue
+		}
+
 		desiredSAs = append(desiredSAs, ServiceAccountInfo{
 			Email:       GetUpdaterSAEmail(collection, config),
 			DisplayName: GetUpdaterSADisplayName(collection),
 			ID:          GetUpdaterSAId(collection),
 			Collection:  collection,
 			Description: GetUpdaterSADescription(collection),
 		})
-
 		desiredSecrets[GetUpdaterSASecretName(collection)] = GCPSecret{
 			Name:       GetUpdaterSASecretName(collection),
 			Type:       SecretTypeSA,
 			Collection: collection,
 		}
-		desiredSecrets[GetIndexSecretName(collection)] = GCPSecret{
-			Name:       GetIndexSecretName(collection),
-			Type:       SecretTypeIndex,
-			Collection: collection,
-		}
 
+		// The service account gets its own bindings rather than joining the owning group's, so
+		// its access stays scoped to exactly one collection.
 		saMembers := []string{fmt.Sprintf("serviceAccount:%s", GetUpdaterSAEmail(collection, config))}
 		desiredIAMBindings = append(desiredIAMBindings, &iampb.Binding{
 			Role:    config.GetSecretAccessorRole(),

pkg/gsm-secrets/diff.go

@@ -68,12 +68,16 @@ func DiffSecrets(desiredSecrets, actualSecrets map[string]GCPSecret, desiredColl
 	}
 
 	for _, secret := range actualSecrets {
-		if desiredCollections[secret.Collection] {
+		if !desiredCollections[secret.Collection] {
+			toDelete = append(toDelete, secret)
+			logrus.Debugf("Scheduling secret '%s' for deletion (collection '%s' not in config)", secret.Name, secret.Collection)
 			continue
 		}
 
-		toDelete = append(toDelete, secret)
-		logrus.Debugf("Scheduling secret '%s' for deletion (collection '%s' not in config)", secret.Name, secret.Collection)
+		if _, wanted := desiredSecrets[secret.Name]; !wanted && secret.Type == SecretTypeSA {
+			toDelete = append(toDelete, secret)
+			logrus.Debugf("Scheduling secret '%s' for deletion (collection '%s' has no updater service account)", secret.Name, secret.Collection)
+		}
 	}
 	slices.SortFunc(toDelete, func(a, b GCPSecret) int {
 		return strings.Compare(a.Name, b.Name)

pkg/gsm-secrets/diff_test.go

@@ -319,6 +319,22 @@ func TestDiffSecrets(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:               "collection gives up its updater service account",
+			desiredCollections: map[string]bool{testCollection: true},
+			desiredSecrets:     map[string]GCPSecret{indexSecret.Name: indexSecret},
+			actualSecrets: map[string]GCPSecret{
+				SAsecret.Name:    SAsecret,
+				indexSecret.Name: indexSecret,
+				"test-collection__group__data": {
+					Name:       "test-collection__group__data",
+					Type:       SecretTypeGeneric,
+					Collection: testCollection,
+				},
+			},
+			expectedToCreate: map[string]GCPSecret{},
+			expectedToDelete: []GCPSecret{SAsecret},
+		},
 		{
 			name: "mixed operations - create some, delete others",
 			desiredCollections: map[string]bool{

pkg/gsm-secrets/testdata/basic-config.yaml

@@ -10,6 +10,8 @@ groups:
     secret_collections:
     - collection-alpha
     - shared-collection
+    updater_service_accounts:
+    - collection-alpha
   team-beta:
     secret_collections:
     - some-collection-with-a-very-long-name

pkg/gsm-secrets/testdata/complex-config.yaml

@@ -45,3 +45,5 @@ groups:
     rename_to: yet-another-name
     secret_collections:
     - epsilon-secrets
+    updater_service_accounts:
+    - epsilon-secrets

pkg/gsm-secrets/testdata/zz_fixture_bindings_TestGetDesiredState_basic_config.yaml

@@ -11,32 +11,6 @@
   members:
   - serviceAccount:collection-alpha-updater@test-project.iam.gserviceaccount.com
   role: projects/test-project/roles/openshift_ci_secrets_updater
-- condition:
-    expression: resource.name.extract("secrets/{s}/versions/") in ["shared-collection__updater-service-account",
-      "shared-collection____index"]
-    title: 'tp-viewer: shared-collection'
-  members:
-  - serviceAccount:shared-collection-updater@test-project.iam.gserviceaccount.com
-  role: projects/test-project/roles/openshift_ci_secrets_viewer
-- condition:
-    expression: resource.name.extract("secrets/{c}__") in ["shared-collection"]
-    title: 'tp-updater: shared-collection'
-  members:
-  - serviceAccount:shared-collection-updater@test-project.iam.gserviceaccount.com
-  role: projects/test-project/roles/openshift_ci_secrets_updater
-- condition:
-    expression: resource.name.extract("secrets/{s}/versions/") in ["some-collection-with-a-very-long-name__updater-service-account",
-      "some-collection-with-a-very-long-name____index"]
-    title: 'tp-viewer: some-collection-with-a-very-long-name'
-  members:
-  - serviceAccount:ynbbgzp1q3u3iwp8edyqix-updater@test-project.iam.gserviceaccount.com
-  role: projects/test-project/roles/openshift_ci_secrets_viewer
-- condition:
-    expression: resource.name.extract("secrets/{c}__") in ["some-collection-with-a-very-long-name"]
-    title: 'tp-updater: some-collection-with-a-very-long-name'
-  members:
-  - serviceAccount:ynbbgzp1q3u3iwp8edyqix-updater@test-project.iam.gserviceaccount.com
-  role: projects/test-project/roles/openshift_ci_secrets_updater
 - condition:
     expression: resource.name.extract("secrets/{s}/versions/") in ["collection-alpha__updater-service-account",
       "collection-alpha____index", "shared-collection__updater-service-account", "shared-collection____index"]