Description
The migration guide tells the operator that credentials retained under non-deleting management policies need a rotation or revocation plan after the target cluster takes ownership, and then stops. There is no procedure. That is the point in the migration where the source cluster still holds live credentials it no longer owns and nothing tells the operator what to do with them, which is the highest-consequence gap left in the handoff: an unrevoked retained access policy outlives the cluster that vended it. The plan has to work for the retained policy, its rotating token and any delivered output secret, and it has to say what order those come apart in relative to the target cluster proving it has minted its own.
Acceptance Criteria
- #1 The migration guide carries an ordered procedure for retiring source-held credentials after target ownership is proven
- #2 The procedure names each retained object class it covers and states what evidence proves the target has replaced it before anything is revoked
- #3 The procedure states the failure mode of revoking too early and of never revoking at all
- #4 Any step that cannot be verified from this repository is marked as an operator prerequisite rather than presented as proven
Definition of Done
- #1 just check passes locally
- #2 hosted Validate workflow passes on the completing commit
Implementation Plan
- Read the ownership-transfer procedure and identify every credential class the source still holds once the target owns the stack.
- Write the retirement procedure as an ordered subsection of that transfer section, and make step 6 point at it instead of only warning.
- State both failure modes explicitly, since revoking too early and never revoking are opposite errors with the same root cause.
- Mark every step this repository cannot verify as an operator prerequisite rather than presenting it as proven.
Implementation Notes
CodeRabbit review record for this procedure. It went through ten passes, which is far past the usual ceiling; the reason is worth recording because it is a property of the work and not of the tool. The procedure is a credential-destruction runbook written from scratch, so almost every finding was of the form “this instruction destroys something under a case you did not name”. Those are exactly the findings worth taking, and each fix lengthened the prose the next pass then read.
Command each time: coderabbit review --agent --base main. Every pass emitted a complete event.
Severity totals across all passes: 0 critical, 24 major, 4 minor. Applied 25; not applied 3, each recorded below with its reason.
The substantive defects it caught, all of which were mine and all of which were real:
- Step 4 deleted the remote secret-store document unconditionally, while step 7 of the transfer procedure above explicitly permits the target to keep the source output path. On that path the instruction deleted the target live credential document. This was the worst one.
- The
PushSecretdeletion-policy trap: this repository rendersdeletionPolicy: Deletewhenever deletion is armed (platform/function/access.go:47-51), so removing the writer takes the remote document with it. The check has to happen during inventory, because step 4 of the transfer procedure removes output-document writers before the retirement section is reached. - Consumer evidence: reading the target path is a configuration fact. A process that loaded the source token at start-up keeps presenting it, and keeps succeeding, right up to revocation. A post-write successful call proves nothing on its own either.
- Plurality throughout. One policy can have several rotating tokens, one service account several tokens, and a token issued out of band never appears in the source cluster at all. Every comparison is set against set.
remoteKeyis unique only within its store, so the ownership comparison is on the (store identity, key) pair.- ESO
PushSecretreportsReady=Truewith reasonSynced, which is not the Crossplane Ready/Synced pair used elsewhere in this guide, and neither the remote document nor the source Secret carries conditions at all. - An adopted policy still leaves its tokens and delivered copies to retire; the earlier text said there was nothing to retire.
Not applied, with reasons:
- Replace the documented key formats (
region:tokenID,stackSlug:serviceAccountID) with a general instruction to read the external-name annotation. The formats are correct, verified against pinned provider source, and this guide already documents them in the adoption section. The inventory instruction already says to read the actual annotations rather than construct keys, so this would remove accurate information without changing any action. - Per-executable checksums for the envtest binaries (raised on GCV-0065). Upstream publishes one archive sidecar and no per-binary digests; the archive hash already fail-closes on any tampered member.
- An etcd version assertion in the cached-asset reuse path (GCV-0065). The justfile pins the Kubernetes version only; there is no etcd pin to assert against and inventing one would be a fabricated control.
Final Summary
Added ### Retire the source-held credentials to the ownership-transfer section of the migration guide, replacing the bare warning that step 6 previously carried.
It inventories the four retained classes (the AccessPolicy, every rotating token that policy issued, every StackServiceAccount with every token issued against it, and the delivered copies), recording each remoteKey with the store identity it is written through, because a key is unique only within its store. It requires set-against-set proof that the target minted its own provider identities and that consumers have demonstrably reloaded them, then retires in an order that removes the Kubernetes owner before revoking, so the source Composition cannot re-mint what was just revoked. Both failure modes are stated: revoking too early is an outage with no rollback because a provider identity cannot be restored, and never revoking leaves a full-scope credential that outlives its cluster and appears in no cluster inventory. Everything unverifiable from this repository is marked as an operator prerequisite.
Two data-loss traps the review surfaced are now named explicitly: this repository renders deletionPolicy: Delete on PushSecrets whenever deletion is armed (platform/function/access.go:47-51), so removing a writer takes the remote document with it and the check must happen during inventory before the transfer procedure removes output-document writers; and step 7 of that procedure permits the target to keep the source output path, so cleanup deletes only source-only (store, key) pairs with no remaining live writer.
Verified as documentation: ten CodeRabbit passes with the disposition of all 28 findings recorded in the notes, 25 applied and 3 declined with reasons; field names checked against this repository own rendering (spec.selector.secret.name, spec.data[].match.remoteRef.remoteKey at platform/function/stackconsumer.go:240,249) and both service-account kinds confirmed present in the pinned managed-kind map; full gate green.
Completing SHA 70a896c7a63b222bcbbbfad041d605b8b2643abe; hosted Validate public reference run 34413634975 success.