Skip to content

adoption: use per-role label keys and indexes to fix multi-secret conflicts#417

Merged
ecordell merged 1 commit into
mainfrom
secretindexing
May 4, 2026
Merged

adoption: use per-role label keys and indexes to fix multi-secret conflicts#417
ecordell merged 1 commit into
mainfrom
secretindexing

Conversation

@ecordell
Copy link
Copy Markdown
Contributor

@ecordell ecordell commented May 1, 2026

Description

When a SpiceDBCluster references the same secret for multiple credential roles, the previous single-index / single-label approach caused adoption handlers to treat each other's secrets as stale and remove labels.

Each credential role now gets its own label key
(authzed.com/credential-type-{role}) and its own cache index, so handlers only see secrets that carry their specific label. A shared secret carries all applicable role labels simultaneously, appearing in every relevant index.

Role-qualified SSA field managers (spicedb-operator-{role}) prevent the second apply from releasing ownership of the first role's label.

Testing

See tests

References

Fixes #409

…flicts

When a SpiceDBCluster references the same secret for multiple
credential roles, the previous single-index / single-label approach caused
adoption handlers to treat each other's secrets as stale and remove labels.

Each credential role now gets its own label key
(authzed.com/credential-type-{role}) and its own cache index, so handlers
only see secrets that carry their specific label. A shared secret carries
all applicable role labels simultaneously, appearing in every relevant index.

Role-qualified SSA field managers (spicedb-operator-{role}) prevent the
second apply from releasing ownership of the first role's label.
Copy link
Copy Markdown
Contributor

@tstirrat15 tstirrat15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally makes sense

Comment thread pkg/metadata/keys.go
Comment on lines +114 to +119
case CredentialTypeDatastoreURI:
return CredentialTypeDatastoreURILabelKey
case CredentialTypePresharedKey:
return CredentialTypePresharedKeyLabelKey
case CredentialTypeMigrationSecrets:
return CredentialTypeMigrationSecretsLabelKey
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth making static maps out of these?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in this case I prefer the switch just for the simpler defaulting. But no strong opinion.

@ecordell ecordell added this pull request to the merge queue May 4, 2026
Merged via the queue into main with commit e48aa66 May 4, 2026
11 checks passed
@ecordell ecordell deleted the secretindexing branch May 4, 2026 17:40
@github-actions github-actions Bot locked and limited conversation to collaborators May 4, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

secret adoption handlers conflict with each other

2 participants