diff --git a/e2e/cli/run_bundle_secret_update.txtar b/e2e/cli/run_bundle_secret_update.txtar new file mode 100644 index 00000000..d4700049 --- /dev/null +++ b/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"}}} diff --git a/internal/database/database.go b/internal/database/database.go index cebbcd9b..0d99b250 100644 --- a/internal/database/database.go +++ b/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 { diff --git a/internal/database/secret_links_test.go b/internal/database/secret_links_test.go new file mode 100644 index 00000000..8af98fa6 --- /dev/null +++ b/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, + }, + } +}