Conversation
A gcs backend that sets serviceAccounts.project lets a Buckety
declare parameters.serviceAccount; the driver then maintains a GCP
service account with roles/storage.objectAdmin on that bucket only,
and each BucketyAccess Secret additionally carries
serviceAccountKey/serviceAccountEmail/serviceAccountKeyId - native
OAuth2 bearer-token auth with bucket-scoped blast radius, alongside
the (backend-wide) static HMAC pair.
Grant machinery, driver-agnostic: GrantRequest gains
BucketyParameters and ExistingSecretData, and the access reconciler
runs the SecretConflict gate BEFORE GrantAccess. Existing-data
reuse (verified against keys.list + validBeforeTime) is what makes
minting idempotent for create-only-retrievable credentials, the
blocker that had deferred per-access minting; it doubles as
self-healing, so server-side key deletion is the manual rotation
runbook until scheduled rotation lands (SPEC non-goal reworded as
deferred by intent). DeleteBuckety takes DeleteRequest{Name,
Parameters} so teardown finds the SA.
Safety posture: an ownership marker in the SA description is
verified before every bind/mint/delete - a tenant naming a foreign
SA is refused, never handed its keys. serviceAccounts.project
SHOULD be a dedicated identity project, confining the controller's
serviceAccountAdmin/serviceAccountKeyAdmin grants to identities
that exist only for buckety bindings. parameters.serviceAccount is
set-at-create immutable, scoping existing Bucketys out.
The parameter is template-resolved with a restricted grammar
(${name}/${namespace}/${backend.X}, no labels - parameters
re-resolve every reconcile, so inputs must be immutable), declared
per driver via the TemplatedParameters capability. Backend
parameter defaults move behind json.RawMessage so templated
defaults pass the loader's envsubst policy scan; Load re-imposes
the no-${...} rule for non-templated keys.
fake-gcs-server implements neither iam.googleapis.com nor bucket
IAM, so the SA lifecycle is unit-tested against an httptest fake
and documented as not-e2e-gated, per the existing posture.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The v0.1 directory was kept for pinned raw-URL stability, but the gcs 0.2 upgrade is being rolled out clean-slate (existing Bucketys deleted first), so there is nothing left to consult it. Release tags still serve the historical files for anyone pinning by ref. kadm/s3/objectstore stay at v0.1 - their drivers are still 0.1.x. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three leak paths from the checkit review of gcs-sa, all sharing one invariant - a minted principal is revoked exactly once, and released only after revocation: - A re-mint that changes status.principal (Secret lost or hand-edited, key invalidated) now revokes the replaced key after the successor Secret is written (review finding 1). Without this the 10-keys-per-SA cap bounded CUMULATIVE mints, wedging Keys.Create permanently; status.principal advances only past a successful revoke, so failures retry. - Buckety deletion drives the implicit access's deletion and waits for its finalizer BEFORE releasing its own (finding 3 sharpened): owner-ref GC ran after the Buckety was gone, and the access finalizer skipped RevokeAccess with no Buckety to resolve the backend from - a live key on a Retain-surviving SA. - Access deletion with the backend missing from config blocks with a condition while a principal exists, mirroring Buckety deletion under retentionPolicy=Delete, instead of silently releasing the finalizer; transient Buckety reads retry instead of skipping revocation. SPEC pins the Retain semantics these fixes assume: Retain keeps the data unit (bucket, IAM policy, keyless SA - deliberate, for recreate-with-adoption and against dangling bindings and the 30-day SA tombstone), never credentials, and the key cap bounds concurrent accesses, not lifecycle churn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two more checkit review findings. A soft-deleted service account reserves its ID for ~30 days - Create conflicts while Get sees nothing - and that state never converges on retries, so the create/re-fetch race now detects the pattern and names the tombstone, the cool-down and the ways out, instead of wrapping a misleading NotFound in a create error (finding 2; automated undelete is out - it needs the numeric unique ID nothing retains). serviceAccounts.endpoint no longer implies WithoutAuthentication: an endpoint override keeps ADC (legitimate for Private Service Connect), and only the new explicit serviceAccounts.insecure flag - which requires endpoint - disables auth, so a production typo cannot silently turn credentials off (finding 4). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
parameters.hmac="false" (mutable, per CR or as a backend parameter default) mints Secrets without the backend-wide static pair - typically with serviceAccount, so consumers hold only the bucket-scoped identity instead of having its blast-radius win undone by the shared pair riding along; without serviceAccount it yields a coordinates-only Secret for ambient-credential consumers. The DRIVER default stays pair-included: the pair is the incumbent family-portable contract and a minor bump must not remove Secret keys; a backend imposes the opt-in posture by declaring hmac "false" in its parameter defaults. serviceAccount was already settable per CR (defaults merge under the CR, CR wins per key); the missing direction was clearing a backend default, so the empty string is now the defined per-CR opt-out - valid regardless of the serviceAccounts gate, treated as absent by the driver, and still immutable post-create. SPEC additionally warns that ADDING a backend parameter default reaches existing resources on the next rollout without an admission gate - the checkit config review misread this as recreate-gated, so it is now stated where the convention is documented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
scripts/bump-release.sh after the serviceAccounts feature and review fixes changed the controller binary; CI asserts the pinned digest matches its own reproducible build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The release-pin assertion failed twice in a row with distinct digests: the pin was built locally with go1.26.5 while CI's setup-go installs from the go.mod go directive (1.26.1), and the binary - hence the image digest - is a function of the compiler. Verified both ways: GOTOOLCHAIN=go1.26.1 reproduces CI's digest byte-for-byte, and with the toolchain directive in place the pinned sha256:809b6b41 reproduces locally. The toolchain directive is what go-version-file actually honours when present, so CI now compiles with the same toolchain as the machine that minted the pin. bump-release.sh additionally refuses to run when the local toolchain differs from the directive, naming the remedy - future Go upgrades fail at bump time instead of in CI's assertion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The previous commit's deletion gate treated ANY non-empty principal as must-revoke, but v1alpha1 static principals (gcs-static, kadm-root, ...) are no-op revokes - blocking them regressed access teardown under renamed backends, which is exactly what every backend-stickiness e2e scenario does: the implicit access wedged, the Buckety sat waiting for it, and the asserted 'deletion blocked' condition never surfaced. The distinction is driver-owned, so drivers now declare it: GrantResult.Revocable (gcs: true for minted SA keys, false for the static pair; kadm/s3 unchanged at the zero value), stamped into status.principalRevocable. The missing-backend gate blocks only revocable principals; static ones release exactly as in v1alpha1. The Buckety's implicit-access wait also surfaces a RevokingAccesses condition instead of looping silently. Release pin re-bumped for the changed binary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.