🐛 Clean up stale secret links

e2e/cli/run_bundle_secret_update.txtar

@@ -0,0 +1,40 @@
+! exec $OPACTL run --addr ./ocp.sock --config config.yaml --data-dir data &opactl&
+
+exec curl --retry 5 --retry-all-errors --unix-socket ocp.sock http://localhost/health
+
+exec curl -fsS --unix-socket ocp.sock -X PUT -H 'Authorization: bearer admin-token' -H 'Content-Type: application/json' -d @old-secret.json http://localhost/v1/secrets/old-secret
+exec curl -fsS --unix-socket ocp.sock -X PUT -H 'Authorization: bearer admin-token' -H 'Content-Type: application/json' -d @new-secret.json http://localhost/v1/secrets/new-secret
+
+exec curl -fsS --unix-socket ocp.sock -X PUT -H 'Authorization: bearer admin-token' -H 'Content-Type: application/json' -d @bundle-old-secret.json http://localhost/v1/bundles/test
+exec curl -fsS --unix-socket ocp.sock -X PUT -H 'Authorization: bearer admin-token' -H 'Content-Type: application/json' -d @bundle-new-secret.json http://localhost/v1/bundles/test
+
+exec curl -fsS --unix-socket ocp.sock -H 'Authorization: bearer admin-token' http://localhost/v1/bundles/test
+stdout '"credentials":"new-secret"'
+
+exec curl -fsS --unix-socket ocp.sock -X DELETE -H 'Authorization: bearer admin-token' http://localhost/v1/secrets/old-secret
+
+exec curl -fsS --unix-socket ocp.sock -X PUT -H 'Authorization: bearer admin-token' -H 'Content-Type: application/json' -d @bundle-without-secret.json http://localhost/v1/bundles/test
+exec curl -fsS --unix-socket ocp.sock -H 'Authorization: bearer admin-token' http://localhost/v1/bundles/test
+! stdout '"credentials"'
+
+exec curl -fsS --unix-socket ocp.sock -X DELETE -H 'Authorization: bearer admin-token' http://localhost/v1/secrets/new-secret
+
+kill opactl
+wait opactl
+
+-- config.yaml --
+tokens:
+  admin:
+    api_key: admin-token
+    scopes:
+    - role: administrator
+-- old-secret.json --
+{"value":{"type":"aws_auth","access_key_id":"old","secret_access_key":"old"}}
+-- new-secret.json --
+{"value":{"type":"aws_auth","access_key_id":"new","secret_access_key":"new"}}
+-- bundle-old-secret.json --
+{"object_storage":{"aws":{"region":"us-east-1","bucket":"test-bucket","key":"test-key","credentials":"old-secret"}}}
+-- bundle-new-secret.json --
+{"object_storage":{"aws":{"region":"us-east-1","bucket":"test-bucket","key":"test-key","credentials":"new-secret"}}}
+-- bundle-without-secret.json --
+{"object_storage":{"aws":{"region":"us-east-1","bucket":"test-bucket","key":"test-key"}}}

internal/database/database.go

@@ -1741,6 +1741,7 @@ func (d *Database) UpsertBundle(ctx context.Context, principal, tenant string, b
 			return err
 		}
 
+		var secretIDs []int
 		if bundle.ObjectStorage.AmazonS3 != nil {
 			if cred := bundle.ObjectStorage.AmazonS3.Credentials; cred != nil {
 				secretID, err := d.lookupID(ctx, tx, tenant, "secrets", cred.Name)
@@ -1751,6 +1752,7 @@ func (d *Database) UpsertBundle(ctx context.Context, principal, tenant string, b
 					id, secretID, "aws"); err != nil {
 					return fmt.Errorf("table bundles_secrets: %w", err)
 				}
+				secretIDs = append(secretIDs, secretID)
 			}
 		}
 
@@ -1764,6 +1766,7 @@ func (d *Database) UpsertBundle(ctx context.Context, principal, tenant string, b
 					id, secretID, "gcp"); err != nil {
 					return fmt.Errorf("table bundles_secrets: %w", err)
 				}
+				secretIDs = append(secretIDs, secretID)
 			}
 		}
 
@@ -1777,9 +1780,14 @@ func (d *Database) UpsertBundle(ctx context.Context, principal, tenant string, b
 					id, secretID, "azure"); err != nil {
 					return fmt.Errorf("table bundles_secrets: %w", err)
 				}
+				secretIDs = append(secretIDs, secretID)
 			}
 		}
 
+		if err := d.deleteNotIn(ctx, tx, "bundles_secrets", "bundle_id", id, "secret_id", secretIDs); err != nil {
+			return fmt.Errorf("delete stale bundle secrets: %w", err)
+		}
+
 		sources := make([]int, 0, len(bundle.Requirements))
 		for _, req := range bundle.Requirements {
 			if req.Source != nil {
@@ -1843,6 +1851,7 @@ func (d *Database) UpsertSource(ctx context.Context, principal, tenant string, s
 			return err
 		}
 
+		var secretIDs []int
 		if source.Git.Credentials != nil {
 			secretID, err := d.lookupID(ctx, tx, tenant, "secrets", source.Git.Credentials.Name)
 			if err != nil && !errors.Is(err, sql.ErrNoRows) {
@@ -1853,9 +1862,13 @@ func (d *Database) UpsertSource(ctx context.Context, principal, tenant string, s
 					id, secretID, "git_credentials"); err != nil {
 					return fmt.Errorf("upsert of secret link %s: %w", source.Git.Credentials.Name, err)
 				}
+				secretIDs = append(secretIDs, secretID)
 			}
 			// If secret not found in DB (sql.ErrNoRows), skip — it will be resolved by the secret provider at sync time
 		}
+		if err := d.deleteNotIn(ctx, tx, "sources_secrets", "source_id", id, "secret_id", secretIDs); err != nil {
+			return fmt.Errorf("delete stale source secrets: %w", err)
+		}
 
 		// Upsert data sources
 		for _, datasource := range source.Datasources {

internal/database/secret_links_test.go

@@ -0,0 +1,167 @@
+package database_test
+
+import (
+	"testing"
+
+	"github.com/testcontainers/testcontainers-go"
+
+	"github.com/open-policy-agent/opa-control-plane/internal/config"
+	"github.com/open-policy-agent/opa-control-plane/internal/migrations"
+	"github.com/open-policy-agent/opa-control-plane/internal/test/dbs"
+)
+
+func TestSecretReferenceSwitch(t *testing.T) {
+	ctx := t.Context()
+
+	for databaseType, databaseConfig := range dbs.Configs(t) {
+		t.Run(databaseType, func(t *testing.T) {
+			t.Parallel()
+
+			var ctr testcontainers.Container
+			if databaseConfig.Setup != nil {
+				ctr = databaseConfig.Setup(t)
+				if databaseConfig.Cleanup != nil {
+					t.Cleanup(databaseConfig.Cleanup(t, ctr))
+				}
+			}
+
+			db, err := migrations.New().
+				WithConfig(databaseConfig.Database(t, ctr).Database).
+				WithMigrate(true).
+				Run(ctx)
+			if err != nil {
+				t.Fatal(err)
+			}
+			t.Cleanup(db.CloseDB)
+
+			if err := db.UpsertPrincipal(ctx, principal); err != nil {
+				t.Fatal(err)
+			}
+
+			t.Run("bundle", func(t *testing.T) {
+				for _, name := range []string{"bundle-old-secret", "bundle-new-secret"} {
+					if err := db.UpsertSecret(ctx, "admin", tenant, awsSecret(name)); err != nil {
+						t.Fatal(err)
+					}
+				}
+
+				bundle := func(secretName string) *config.Bundle {
+					var credentials *config.SecretRef
+					if secretName != "" {
+						credentials = &config.SecretRef{Name: secretName}
+					}
+					return &config.Bundle{
+						Name: "bundle-secret-switch",
+						ObjectStorage: config.ObjectStorage{
+							AmazonS3: &config.AmazonS3{
+								Bucket:      "bucket",
+								Key:         "bundle.tar.gz",
+								Region:      "eu-west-1",
+								Credentials: credentials,
+							},
+						},
+					}
+				}
+
+				if err := db.UpsertBundle(ctx, "admin", tenant, bundle("bundle-old-secret")); err != nil {
+					t.Fatal(err)
+				}
+				if err := db.UpsertBundle(ctx, "admin", tenant, bundle("bundle-new-secret")); err != nil {
+					t.Fatal(err)
+				}
+
+				got, err := db.GetBundle(ctx, "admin", tenant, "bundle-secret-switch")
+				if err != nil {
+					t.Fatal(err)
+				}
+				if got.ObjectStorage.AmazonS3 == nil || got.ObjectStorage.AmazonS3.Credentials == nil {
+					t.Fatal("expected bundle credentials")
+				}
+				if got.ObjectStorage.AmazonS3.Credentials.Name != "bundle-new-secret" {
+					t.Errorf("credentials after switch = %q, want %q", got.ObjectStorage.AmazonS3.Credentials.Name, "bundle-new-secret")
+				}
+				if err := db.DeleteSecret(ctx, "admin", tenant, "bundle-old-secret"); err != nil {
+					t.Errorf("delete old, unreferenced secret: %v", err)
+				}
+
+				if err := db.UpsertBundle(ctx, "admin", tenant, bundle("")); err != nil {
+					t.Fatal(err)
+				}
+				if err := db.DeleteSecret(ctx, "admin", tenant, "bundle-new-secret"); err != nil {
+					t.Errorf("delete removed credentials secret: %v", err)
+				}
+			})
+
+			t.Run("source", func(t *testing.T) {
+				for _, name := range []string{"source-old-secret", "source-new-secret"} {
+					if err := db.UpsertSecret(ctx, "admin", tenant, tokenSecret(name)); err != nil {
+						t.Fatal(err)
+					}
+				}
+
+				source := func(secretName string) *config.Source {
+					var credentials *config.SecretRef
+					if secretName != "" {
+						credentials = &config.SecretRef{Name: secretName}
+					}
+					return &config.Source{
+						Name: "source-secret-switch",
+						Git: config.Git{
+							Repo:        "https://github.com/example/repo",
+							Credentials: credentials,
+						},
+					}
+				}
+
+				if err := db.UpsertSource(ctx, "admin", tenant, source("source-old-secret")); err != nil {
+					t.Fatal(err)
+				}
+				if err := db.UpsertSource(ctx, "admin", tenant, source("source-new-secret")); err != nil {
+					t.Fatal(err)
+				}
+
+				got, err := db.GetSource(ctx, "admin", tenant, "source-secret-switch")
+				if err != nil {
+					t.Fatal(err)
+				}
+				if got.Git.Credentials == nil {
+					t.Fatal("expected source credentials")
+				}
+				if got.Git.Credentials.Name != "source-new-secret" {
+					t.Errorf("credentials after switch = %q, want %q", got.Git.Credentials.Name, "source-new-secret")
+				}
+				if err := db.DeleteSecret(ctx, "admin", tenant, "source-old-secret"); err != nil {
+					t.Errorf("delete old, unreferenced secret: %v", err)
+				}
+
+				if err := db.UpsertSource(ctx, "admin", tenant, source("")); err != nil {
+					t.Fatal(err)
+				}
+				if err := db.DeleteSecret(ctx, "admin", tenant, "source-new-secret"); err != nil {
+					t.Errorf("delete removed credentials secret: %v", err)
+				}
+			})
+		})
+	}
+}
+
+func awsSecret(name string) *config.Secret {
+	return &config.Secret{
+		Name: name,
+		Value: map[string]any{
+			"type":              "aws_auth",
+			"access_key_id":     name,
+			"secret_access_key": "secret",
+		},
+	}
+}
+
+func tokenSecret(name string) *config.Secret {
+	return &config.Secret{
+		Name: name,
+		Value: map[string]any{
+			"type":  "token_auth",
+			"token": name,
+		},
+	}
+}