fix(rustfs): give each identity its own SecretStore, and catch the gap statically #453

Merged

binjovi-bot merged 1 commit from fix/rustfs-identity-reader into trunk 2026-09-08 10:44:34 +00:00

Conversation 0 Commits 1 Files changed 2 +111 -3

sean commented 2026-09-08 10:06:10 +00:00

Owner

Copy link

The tempo identity deployed and its convergence Job succeeded in 6 seconds — bucket, policy, user, attach, and both halves of the isolation check all passed — but the credential never reached a Secret:

rustfs-tempo-s3-credentials   SecretSyncedError   could not get secret data from provider

The round-trip ExternalSecret — the one that reads the frozen generator output back out and shapes it into minio_access_key/minio_secret_key — used the shared kubernetes SecretStore from the rustfs component. That store's reader Role is scoped by resourceNames to [rustfs-access-key, rustfs-secret-key]: the two root secrets. It could not see this identity's key at all.

ESO's message names neither the Secret nor the Role, which is what made this worth closing statically rather than just fixing.

The fix

Each identity now carries its own namespaced SecretStore, and its reader Role covers both read paths:

path	Secret
in-namespace round-trip	rustfs-tempo-secret-key
ClusterSecretStore → ns olly	rustfs-tempo-s3-credentials

Adding a consumer now touches one file and never has to widen a Role belonging to the store component.

The guard, and why its first version was useless

tests/rustfs-identities.sh gains the check that would have caught this: for every ExternalSecret, the remote key it reads must be inside its store's reader resourceNames.

The first version did not catch the shipped bug. It skipped any store not declared in the identity file — "not checkable here" — and the bug's whole shape was a store from another component. I verified this by reproducing the shipped manifest and watching the check pass:

=== RED: reproduce the shipped bug exactly ===
  rustfs-identities: inner check passed        <- useless

So the rule is now that an identity must be self-contained: an ExternalSecret referencing a store this file does not declare is a failure, because its reader scope cannot be verified. Re-run against the same reproduction:

FAIL: tempo: ExternalSecret rustfs-tempo-s3-credentials uses SecretStore/kubernetes,
which is not declared in this file -- its reader scope cannot be verified, and a
shared store may not be able to read this identity's Secret

Two negative controls cover it: a key moved outside the reader scope, and the exact shipped shape.

bash tests/check.sh exits 0. Server-side dry run applies all 9 objects clean.

https://claude.ai/code/session_01KZoQin34jeyt6nDGqvJA76

The tempo identity deployed and its convergence Job succeeded in 6 seconds — bucket, policy, user, attach, and **both halves of the isolation check** all passed — but the credential never reached a Secret: ``` rustfs-tempo-s3-credentials SecretSyncedError could not get secret data from provider ``` The round-trip ExternalSecret — the one that reads the frozen generator output back out and shapes it into `minio_access_key`/`minio_secret_key` — used the shared `kubernetes` SecretStore from the `rustfs` component. That store's reader Role is scoped by `resourceNames` to `[rustfs-access-key, rustfs-secret-key]`: the two **root** secrets. It could not see this identity's key at all. ESO's message names neither the Secret nor the Role, which is what made this worth closing statically rather than just fixing. ## The fix Each identity now carries its **own** namespaced SecretStore, and its reader Role covers both read paths: | path | Secret | |---|---| | in-namespace round-trip | `rustfs-tempo-secret-key` | | ClusterSecretStore → ns `olly` | `rustfs-tempo-s3-credentials` | Adding a consumer now touches one file and never has to widen a Role belonging to the store component. ## The guard, and why its first version was useless `tests/rustfs-identities.sh` gains the check that would have caught this: for every ExternalSecret, the remote key it reads must be inside its store's reader `resourceNames`. **The first version did not catch the shipped bug.** It skipped any store not declared in the identity file — "not checkable here" — and the bug's whole shape was a store from another component. I verified this by reproducing the shipped manifest and watching the check pass: ``` === RED: reproduce the shipped bug exactly === rustfs-identities: inner check passed <- useless ``` So the rule is now that an identity must be **self-contained**: an ExternalSecret referencing a store this file does not declare is a failure, because its reader scope cannot be verified. Re-run against the same reproduction: ``` FAIL: tempo: ExternalSecret rustfs-tempo-s3-credentials uses SecretStore/kubernetes, which is not declared in this file -- its reader scope cannot be verified, and a shared store may not be able to read this identity's Secret ``` Two negative controls cover it: a key moved outside the reader scope, and the exact shipped shape. `bash tests/check.sh` exits 0. Server-side dry run applies all 9 objects clean. https://claude.ai/code/session_01KZoQin34jeyt6nDGqvJA76

sean added 1 commit 2026-09-08 10:06:10 +00:00

fix(rustfs): give each identity its own SecretStore, and catch the gap statically ... e529996fe6

The tempo identity deployed and its convergence Job succeeded in 6 seconds --
bucket, policy, user, attach, and both halves of the isolation check all passed
-- but the credential never reached a Secret:

  rustfs-tempo-s3-credentials   SecretSyncedError   could not get secret data from provider

The round-trip ExternalSecret, the one that reads the frozen generator output
back out and shapes it into minio_access_key/minio_secret_key, used the shared
`kubernetes` SecretStore from the rustfs component. That store's reader Role is
scoped by resourceNames to [rustfs-access-key, rustfs-secret-key] -- the two ROOT
secrets -- so it could not see this identity's key at all.

ESO's message names neither the Secret nor the Role, which is what made it
worth closing statically rather than just fixing.

Each identity now carries its OWN namespaced SecretStore and its reader Role
covers both read paths: the in-namespace round-trip and the ClusterSecretStore
that delivers to the consuming namespace. Adding a consumer now touches one file
and never has to widen a Role that belongs to the store component.

tests/rustfs-identities.sh gains the check that would have caught this: for every
ExternalSecret, the remote key it reads must be inside its store's reader
resourceNames.

The first version of that check did NOT catch the shipped bug. It skipped any
store not declared in the identity file -- "not checkable here" -- and the bug's
whole shape was a store from another component. Verified by reproducing the
shipped manifest and watching the check pass, which is why the rule is now that
an identity must be self-contained: an ExternalSecret referencing a store this
file does not declare is a failure, because its reader scope cannot be verified.

Two negative controls cover it: a key moved outside the reader scope, and the
exact shipped shape.

Claude-Session: https://claude.ai/code/session_01KZoQin34jeyt6nDGqvJA76

binjovi-bot merged commit e529996fe6 into trunk 2026-09-08 10:44:34 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

sean/seanfarm!453

No description provided.