You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
* fix(controlplane): raise STS freshness floor to 35min; surface expiry on config-store aws path
DuckDB statements capture S3 credentials when they start executing (secret
lookups resolve against the statement's MVCC snapshot), so the credential-
refresh scheduler's CREATE OR REPLACE SECRET push never reaches an in-flight
statement. With a 10min cache safety margin, a statement could begin on
near-expiry cached STS creds and die with ExpiredToken after as little as
10 minutes — which is what killed a long-running events-backfill DELETE.
- Raise stsCacheSafetyMargin 10min -> 35min: every credential capture point
(worker activation, scheduler refresh push) now hands out creds with at
least 35min of runway, and since the margin exceeds the scheduler's 30min
lookahead, a refresh push always stamps the worker out of the "due"
window — eliminating the 5s identical-creds re-push churn observed in
prod (~700 push events / 15min).
- Move credentialRefreshLookahead to package scope and add a compile-time
guard that fails the build if the margin ever drops to/below the
lookahead.
- Fix a latent bug: the config-store "aws" provider path brokers STS creds
but returned no expiration, leaving S3CredentialsExpiresAt nil — the
refresh scheduler never refreshed those workers, so any session
outliving the 1h token died. The path now returns the STS expiry.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(s3): pin no-mid-statement-credential-recovery; STS session-duration knob; always pin SESSION_TOKEN
Investigation result (REFUTES the rotating-credential-file design explored on
this branch): DuckDB v1.5.3's refresh-on-403 hook (TryRefreshS3Secret) lives
only in S3FileHandle::Initialize and fires only when the file open performs a
network request. It never does for scan workloads: the S3 glob (ListObjectsV2),
DuckLake, and Iceberg file lists all pre-populate file_size/etag/last_modified
in extended_info, which marks the handle initialized and skips the HEAD — the
first auth failure surfaces on a range GET in the read path, which has no
refresh handling, and the statement dies. Verified empirically against MinIO
(an in-flight 48-file scan died on a newly-opened file's first GET despite the
credential file being rewritten and the secret CREATE OR REPLACE'd first) and
at source level in PostHog/duckdb-httpfs v1.5.3-stoi-fix, duckdb/ducklake, and
duckdb/duckdb-iceberg v1.5. The credential_chain + rotating-file mechanism
would therefore only guarantee freshness for NEW statements — which plain
CREATE OR REPLACE SECRET rotation already achieves — so it is not included;
the 35-min freshness floor (26fede7) remains the only (and honest) guarantee.
What this commit ships instead:
- tests/integration/credential_rotation_pin_test.go: pins the limitation the
freshness floor is designed around — an in-flight scan does NOT survive
credential rotation (MVCC secret snapshot + no HEAD at open), while a fresh
statement on the rotated secret does. If a DuckDB upgrade ever makes the
in-flight scan survive, the test fails loudly telling us to revisit the
floor.
- DUCKGRES_STS_SESSION_DURATION (env-only): overrides the 1h AssumeRole
session duration (clamped to AWS's 900s floor) so a soak test can exercise
real in-statement expiry. credentialRefreshLookahead (duration/2) and
stsCacheSafetyMargin (lookahead+5m, the historical 35m at default) now
derive from it; the margin>lookahead invariant holds by construction and is
unit-tested.
- buildConfigSecret always emits SESSION_TOKEN (empty pins "no token"):
httpfs copies a host AWS_SESSION_TOKEN env var into the global
s3_session_token setting at load and secret lookup falls back to settings
for omitted keys, signing with a mismatched (key, token) pair.
- tests/e2e-mw-dev/README.md: documents why the freshness floor cannot be
asserted in-Job and where its coverage lives.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(s3): prove mid-statement credential recovery with patched httpfs fork
The PostHog httpfs fork (PostHog/duckdb-httpfs branch
cred-refresh-read-path) adds what DuckDB v1.5.3 lacks: on an auth
failure (400/403) in any S3 request path it re-resolves the LATEST
COMMITTED secret — bypassing the statement's MVCC snapshot, so the
credential-refresh scheduler's CREATE OR REPLACE SECRET pushes are
picked up mid-statement — and retries once with the new credentials.
That removes the statement-length ceiling the STS freshness floor could
only push to ~35-60min.
Restructure the credential-rotation pin test into a shared scenario
(rotate to user B on a side connection, then disable user A while a
glob scan is in flight) driving two tests:
- TestInFlightScanDiesOnCredentialRotation (unchanged semantics): the
STOCK extension still dies on the first post-revocation range GET —
keeps pinning the upstream limitation so we notice if DuckDB gains
recovery natively.
- TestInFlightScanSurvivesRotationWithPatchedHTTPFS (new): with
DUCKGRES_TEST_PATCHED_HTTPFS pointing at a locally-built patched
extension, the SAME in-flight scan survives rotation + revocation
with an exact row count (no loss or duplication on retry).
Verified locally against MinIO: stock dies at T+4.7s with 403; patched
completes 1,920,000 rows at T+25s with the starting credentials revoked
at T+4.2s.
Also update the stsCacheSafetyMargin doc: once a fork release with the
patch is pinned via HTTPFS_EXTENSION_TAG in Dockerfile.worker, the
freshness floor becomes defense-in-depth rather than the only guarantee
for long statements.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(controlplane): clamp DUCKGRES_STS_SESSION_DURATION to the 1h role-chaining ceiling
Review finding (codex): values above 1h passed straight through to
AssumeRole. STS does not truncate an oversized DurationSeconds — it
REJECTS the call (role chaining via EKS Pod Identity caps at 3600s, and
every duckling-* role has MaxSessionDuration=3600), so an accidentally
high env value (e.g. "2h") would break activation for every org.
Clamp down to maxSTSSessionDuration=1h with a loud warning, mirroring
the existing 900s floor clamp; TestResolveSTSSessionDuration covers
both bounds.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
0 commit comments