feat(iceberg): BYO-IAM on operator-owned S3 — inline metadata, fail-closed credentials, SecretRef rotation fix, typed storage errors, verify probe (#1500, #1497, #1498)#1505
Conversation
eee6435 to
c38d412
Compare
|
Heads-up: your base #1502 moved (review-response wave restack) — a quick restack will clear the The perf burndown stack beneath this PR (#1490 → #1502) was restacked as part of the 2026-07-15 review-response wave. Your base, This PR ( ( I did not touch this branch — flagging so you can restack on your own schedule. The rest of the stack (#1503 loadtable-meta-cache, #1506 exploration, #1507 F17) also restacked on top and is green/MERGEABLE, so #1502's new tip is stable to rebase onto. |
) Local-mode 'fluree info <gs>' and 'fluree iceberg info <gs>' printed the stored graph-source config verbatim, including catalog.auth.client_secret (a live Snowflake PAT). Route all four CLI print sites (local + remote defense-in-depth) through the existing fail-closed allowlist redactor, now re-exported as fluree_db_api::redact_graph_source_config. The issue's suggested type-level redacting Serialize is unimplementable: persistence requires lossless config serialization (CI-locked in ledger_info.rs 'storage serialization must be lossless' + server/CLI round-trip tests). Display-layer redaction via the shared allowlist is the correct seam; SecretRef-at-rest (#1500 §3) removes the secret from storage entirely.
…m S3 (#1500 §1) The REST catalog's loadTable response carries the parsed table metadata inline (LoadTableResponse.metadata — Snowflake Horizon/Polaris vend it), but the query path never read it: every cold table touch paid an S3 GET + parse for bytes the catalog already handed us (~85-98ms/table). Extract the resolution chain into resolve_table_metadata() with order: in-memory cache -> inline loadTable copy -> disk catalog cache -> S3 GET. The inline branch seeds the in-memory cache (cache-reconstructed loads carry metadata: None and hit it), writes NO disk-cache entry (that layer exists for processes never handed the bytes — Direct mode keeps it), and emits NO r2rml.read_metadata span (must fire only on a real S3 read; the live gate asserts n=0 for inline-vending catalogs). Fallback chain is unchanged for Direct mode and catalogs that omit inline metadata. Tests: inline_metadata_short_circuits_disk_and_s3 (panic-stub storage + panic-on-build disk cache prove neither is touched; mutation-verified), no_inline_metadata_falls_back_to_storage_read (exactly one storage read).
…1500 §4) Two hedges that make mechanism B (per-graph-source sts:AssumeRole) a localized edit instead of a format migration, and fix a latent full-miss: - path() now hashes the metadata_location with xxh64 (spec-defined, already a workspace dep) instead of std DefaultHasher, whose algorithm is not guaranteed across toolchains — a routine toolchain bump would silently re-key the entire cache dir. Golden test pins the value. - Filenames gain a scope segment ({hash}.{scope}.{suffix}.json), constant "shared" today. When B lands the scope must become a stable fingerprint of the catalog-auth scope (what rest_clients is keyed on) and NEVER the vended credential (rotates every loadTable => 0% hit rate). Documented at CACHE_SCOPE. - CACHE_FORMAT_VERSION 1->2; v1 orphans keep old names, are never opened, and are evicted oldest-first by the existing MAX_CACHE_BYTES mtime prune.
…owngrade (#1500 §5+§2) S3 AccessDenied previously surfaced as HTTP 400 INVALID_QUERY with the reason buried in an opaque string, forcing hosts to regex-classify and misfire (an admin missing a bucket grant gets told to rotate their PAT). And a REST source configured for vended credentials whose catalog vended none silently downgraded to the process's ambient AWS identity — a fail-open that granted a catalog-scoped source the shared role's entire S3 reach. - IcebergError::StorageAccessDenied {bucket,key,region,message}, classified at the 5 SDK send sites via error code + HTTP 403 (pure helper is unit-testable without SDK types). Display carries the s3:ListBucket caveat: no permission and object-moved are indistinguishable at the API. - QueryError/ApiError gain StorageAccessDenied + CatalogCredentialsNotVended; both map to HTTP 403 with wire codes err:storage/AccessDenied and err:catalog/CredentialsNotVended (fluree-vocab), on direct AND Query-wrapped paths. - decide_credential_source(): single tested decision matrix shared by the scan and preview paths. (true,None,REST) fails closed; ambient only via explicit vended_credentials=false; Direct mode never fails closed; cached loadTable entries preserve credentials verbatim so the guard cannot false-fire on cache hits. - build_preview_storage now enforces the same policy, so wizard preview/generate cannot green-light a credential path queries won't use. - Catalog-side 401->refresh-retry (rest.rs) untouched.
…ver injection (#1500 §3) The engine half of the credential-rotation fix (fluree/solo#782): configs can now carry an opaque secret *reference* instead of a baked literal, resolved at client-build time by a host-injected resolver. - ConfigValue::SecretRef { secret_ref } — declared BETWEEN Literal and Dynamic: untagged serde tries variants in order and Dynamic's all-default fields match any object, so a later position would silently swallow the ref (ordering canary test pins this). Wire shape {"secret_ref":"<opaque>"}; db never parses the ref. resolve() on a SecretRef fails closed with an actionable error (OSS/CLI contexts). - #[async_trait] SecretResolver + SecretResolveError{Denied,NotFound, Unavailable}: host implements authorization internally; db stays tenant-agnostic. Injected via Fluree::with_secret_resolver (cheap clone — Fluree is now Clone) or FlureeBuilder::with_secret_resolver. - AuthConfig::hydrate() rewrites SecretRef→Literal at the four create_provider_arc sites; create_provider_arc stays sync. On the scan path hydration sits INSIDE the rest_clients cache-miss arm, strictly AFTER rest_client_cache_key fingerprints the RAW reference-bearing config — hydrating first would re-key the client cache on every rotation (resolver call + OAuth exchange per query instead of once per 900s TTL). secret_ref_fingerprint_is_rotation_stable pins this. - Wrapper-boundary hydration (hydrate_conn) for browse/preview/sample/ ephemeral/generate/validate; connection-test errors before any network I/O so solo's connection_tested gate catches missing resolvers. - Builders (frozen cross-repo names) at all three levels: with_auth_oauth2_client_secret_ref / with_auth_bearer_token_ref on IcebergConnectionConfig, IcebergCreateConfig, R2rmlCreateConfig. - Display redactor preserves strict {"secret_ref":"<string>"} objects under secret keys (a reference is not a secret — same policy as env-var names); every other shape stays redacted (canary extended).
The Direct branch of load_table_context rebuilt S3IcebergStorage (from_default_chain: env -> ~/.aws -> IMDS/ECS network legs, plus a fresh connection pool) on every call, and did so even when its own metadata-location cache hit — the REST branch has used the query session's storage cache all along. Mirror it: direct_session_storage() checks session.cached_storage first and store_storage's after building; construction now sits below the 2s metadata-location cache check in both arms. Direct never calls store_load_table (no vended credentials to rotate), so the cached client is never invalidated mid-query — first build wins for the whole query. Tests: direct_session_storage_reuses_arc_across_calls (Arc::ptr_eq across two calls, distinct per table key) and a session-layer guard that cached storage persists absent a fresh loadTable.
…s gate - format/sparql.rs: semicolons on the () write_node branches (semicolon_if_nothing_returned, deny-level in this crate) - catalog/rest.rs: drop with_catalog_semaphore — cfg(test) helper whose consuming test was removed on the perf line (zero callers) - scan/topk.rs: vec! -> arrays in test fixtures (useless_vec) All pre-existing on the perf-line base; fixed here because the branch gate is cargo clippy --workspace --all-features --all-targets -D warnings at the pinned 1.96.1 toolchain.
…engine's own credential path (#1500 §6) Solo's wizard needs to prove at configure time that the stack can read a DATA file — not merely reach the catalog — and it must do so through the exact credential + storage construction queries use, or Test goes green over a broken feature. - verify_storage_access(conn, table) + Fluree::verify_iceberg_storage_access (hydrate_conn first — SecretRef-aware): loadTable honoring io.vended_credentials -> build_preview_storage (§2 fail-closed fires before any probe) -> manifest list + manifests read (proves metadata/ prefix) -> HeadObject on the first data file (proves data/ prefix). Prefix-scoped grants that cover only one prefix fail here exactly as queries would. - StorageAccessReport: credential_source/metadata_location/ data_files_listed/probed_data_file(_bytes)/data_probe_skipped/skip_reason (wire shape pinned by a golden JSON test). Empty or snapshotless table reports the data probe as skipped, not failed. - POST /v1/fluree/iceberg/catalog/verify mirroring /preview (admin-gated); response body is the report. - All probe reads map through storage_api_error: a denial surfaces as 403 err:storage/AccessDenied naming bucket/key. - Direct mode rejected identically to preview (no REST catalog to query); like preview, requires the catalog to vend inline metadata.
Pre-existing on the perf line; params mirror the exec-one CLI surface 1:1. Last lint blocking the workspace clippy -D warnings gate, which is now clean at the pinned 1.96.1 toolchain.
…allowlist Adversarial-review finding S1: the doc comments claimed 'allowlist redaction that FAILS CLOSED', but redact_graph_source_config redacts only values under SECRET_CONFIG_KEYS (a denylist) — an unrecognized secret-bearing field prints verbatim. All current secret leaves ARE covered (no live leak); the fail-closed property applies to unparseable input only. Say so accurately: a maintainer must add new secret fields to SECRET_CONFIG_KEYS.
…e-METADATA cache (#1500 x #1503) The restack onto the pointer-rung/lazy-storage chain applies textually clean but is semantically wrong in four ways: #1503 did not replace the two sites this branch hardens, it CLONED them into a deferred copy that none of the guards reached. Fixed by unifying, not by patching copies. - rest_session_storage(): ONE helper holding the §2 fail-closed decision, the #1498 session reuse, and the vended/ambient construction. The eager arm and the deferred LazyS3Storage builder now share it. Previously the builder's copy had no §2 guard at all, so any lazy-forced storage build silently downgraded a vended-configured source to the process's ambient AWS identity — the exact fail-open §2 exists to close, resurrected. - hydrated_auth_provider(): pairs hydrate() with create_provider_arc() so a client-construction site cannot ship un-hydrated. #1503's pointer rung added a second create_provider_arc that §3's hydrate never reached, so a ConfigValue::SecretRef hard-errored via resolve()'s fail-closed arm exactly when the pointer cache was warm — i.e. rotation broke on the fast path. Hydration still runs strictly AFTER rest_client_cache_key in both arms. - Typed errors now round-trip the builder's IcebergError channel (new IcebergError::CatalogCredentialsNotVended + a storage_query_error lift) instead of being flattened by a blanket to_string(), which had destroyed the 403 wire codes (err:storage/AccessDenied, err:catalog/CredentialsNotVended) on the lazy path. - §1 now seeds the disk metadata layer from the inline copy. #1503's rung serves a zero-REST resolve only when metadata_from_caches hits, and a fresh process's in-memory cache is empty — so the pre-restack §1 would have starved the rung for exactly the catalogs that vend inline metadata (Snowflake), reviving the ~1-3s loadTable GET with the regression visible only in the cache gate's load_table.n. Seeding from the free inline copy arms the rung one touch EARLIER than #1503 alone. - §4: CACHE_FORMAT_VERSION stays at #1503's 2 (a payload change); our filename re-key needs no bump and is documented as orthogonal. path()'s key param generalized for the lt_key space; the pointer layer gets an explicit scope verdict (catalog-gated + already per-graph-source). Tests: fail-closed on the shared helper, explicit ambient opt-in still resolves, typed errors survive the builder channel, inline metadata seeds both caches. All were invisible to both suites before: ours predates the rung, #1503's predates SecretRef/fail-closed/typed errors.
462c61c to
210cf48
Compare
|
Corpus perf gate (post-restack, full 118-record run): 0 hash mismatches; 6 perf "violations" diagnosed as network + noise, none attributable to this diff. Correctness is clean — full result-hash parity with the native oracles across the corpus, including the new exploration family (q055–q059). The 6 perf violations, with per-query span attribution from the run records:
For every virtual violator Nothing here touches loadTable-GET frequency or latency: §1 removes the metadata S3 read and arms the pointer rung earlier (q001 measured 3227 ms → 4 ms, Confirmatory base-vs-head A/B of just these 6 queries on the same machine available on request (deferred here for time + the host's post-OOM memory pressure). |
Iceberg on operator-owned S3 (BYO-IAM): inline metadata, fail-closed credential handling, SecretRef rotation fix, typed storage errors, verify probe. Closes #1500, #1497, #1498.
What this is
The db-side engine work for the cross-repo BYO-IAM effort (fluree/solo#782 credential rotation + fluree/solo#783 configure-time BYO-IAM wizard): a Snowflake operator whose Iceberg data files live in an S3 bucket they own configures it on a live Solo stack — no CloudFormation update per bucket. Driven by a real client configuration, live-verified against a standing Snowflake + AWS rig throughout.
Stacked on the head of the perf chain — base
perf/r2rml-f17-union-budget(#1507), i.e.#1502 → #1503 (loadTable-METADATA cache) → #1506 (corpus) → #1507 (F17). That is deliberate: the binding constraint was performance must not degrade under any credential configuration, so this had to be built and measured on top of the virtual-dataset perf work rather than reconciled at merge time.Rebasing onto the chain applied textually clean for 8 of 10 commits, and that was the trap. #1503 did not replace the two sites this branch hardens — it cloned them into a deferred (
LazyS3Storage) copy that none of the guards reached. Every break below is invisible to both test suites (ours predates the pointer rung; #1503's predates SecretRef / fail-closed / typed errors), and a plain merge ships all four silently:metadata_from_cacheshits, and its cross-process source is the diskmetadatalayer — whose only writer was the S3 read §1 removes. Fresh process ⇒ rung misses ⇒ the ~1–3 sloadTableGET returns, visible only in theload_table.ncounter (§1 still books its own win, so nothing looks wrong).create_provider_arcthat §3's hydrate never reached ⇒resolve()'s fail-closed arm ⇒ fluree/solo#782's rotation breaks on the fast path.Fixed by unifying, not by patching duplicates (commit
fix(iceberg): reconcile BYO-IAM credential handling with the loadTable-METADATA cache):rest_session_storage()— one §2 decision + Direct-modeload_table_contextrebuilds the S3 client every call (skips the session storage cache the REST branch uses) #1498 session reuse + vended/ambient construction, shared by the eager arm and the deferred builder. There is now exactly onedecide_credential_sourcecall site on the scan path, so the two arms cannot diverge again.hydrated_auth_provider()— pairs hydration with provider construction so a new client-construction site cannot ship un-hydrated. Hydration still runs strictly afterrest_client_cache_keyin both arms (pinned bysecret_ref_fingerprint_is_rotation_stable).IcebergError::CatalogCredentialsNotVended+ astorage_query_errorlift — typed errors round-trip the builder'sIcebergErrorchannel, so the lazy path reports identically to the eager one.CACHE_FORMAT_VERSIONbump: their2is a payload change (Envelope.key+ verification); our filename re-key is orthogonal and correctly needs no bump.path()'s key param generalized for the newlt_keyspace; thepointerlayer given an explicit scope verdict (catalog-gated and already per-graph-source ⇒ needs no scope).Commits (one per spec section)
654677b23info/iceberg inforoute config display through the existing fail-closed redactor (re-exported asfluree_db_api::redact_graph_source_config)dcf692484loadTablemetadata — no S3 GET for bytes we were handed;r2rml.read_metadatafires only on real reads. Seeds both cache layers, which is what keeps #1503's zero-REST rung armed (see above)6967946beEnvelope.keybump2a9d35206StorageAccessDenied(bucket/key/region → 403err:storage/AccessDenied) + fail-closed vended→ambient downgrade (err:catalog/CredentialsNotVended); one shareddecide_credential_sourcematrix for scan AND previewf305df480ConfigValue::SecretRef+ asynchydrate()+ injectedSecretResolver(tenant-agnostic);Fluree::with_secret_resolvercheap-clone injection;_refbuilders at all three config levels16e5ed489291f00749verify_storage_access+POST /v1/fluree/iceberg/catalog/verify: config-time probe through the engine's own credential path (manifest read = metadata/ prefix, HeadObject = data/ prefix)210cf4833rest_session_storage(),hydrated_auth_provider(), typed round-trip, §1 disk seeding, §4 version reconcile (see the section above; this is the review-critical commit)1b25366e2,42761a386,7b9f32d74-D warningsgate + a benchallow+ redactor doc accuracy(The pre-restack rustfmt-drift commit dropped as empty — the review wave applied the identical fmt fixes upstream, which is where they belong.)
Decisions & corrections to the issues (recorded on the issues as comments)
fluree infoprints OAuth2client_secretin plaintext — ConfigValue redacts Debug but not Serialize #1497's preferred fix (redactingSerialize) is refuted: a CI-locked contract requires lossless config persistence (ledger_info.rs"storage serialization must be lossless"); a lossySerializewould persist[redacted]and break query-time auth. The display-layer denylist redactor is the correct seam; §3's SecretRef-at-rest is the disease-level fix (no secret in storage at all). Hardening follow-up: Harden redact_graph_source_config: denylist of secret key names fails open for future fields #1504.None), so full removal would regress Direct-mode cross-process cold starts. Live outcome identical (Snowflake vends inline ⇒ zero metadata GETs).vended_credentials=true(default) whose catalog vends nothing now 403s instead of silently reading with the process's ambient AWS identity. Explicitvended_credentials=falseis the ambient opt-in. (Solo runbook copy notified.)hydrate()runs strictly afterrest_client_cache_keyfingerprints the raw, reference-bearing config, inside the client-cache miss arm — pinned bysecret_ref_fingerprint_is_rotation_stable(rotation does not re-key; hydrate-before-fingerprint would have).SecretRefsits betweenLiteralandDynamic: untagged serde tries variants in order andDynamic(all-default fields) matches any object — declared later, the ref would be silently swallowed. Ordering canary test pins it. (Both sessions found this hazard independently.)Verification
Adversarial review (independent agent over the full diff): SHIP, zero blockers. Independently recomputed the §4 golden hash, byte-diffed legacy error strings, verified resolved secrets are never persisted/fingerprinted/logged (create/map persists the original ref-bearing config), and confirmed 401→refresh untouched. Its one SHOULD-FIX (redactor doc claimed allowlist semantics) landed as 462c61c + #1504.
Machine gates (local — CI does not fire on stacked bases):
cargo +1.96.1 fmt --all --checkCLEAN;cargo +1.96.1 clippy --locked --workspace --all-features --all-targets -- -D warningsCLEAN (this branch also clears four pre-existing perf-line lints to get there); workspace test suite (--all-features --no-fail-fast): 174/176 targets pass; both failures are outside this branch's code — a pre-existingfluree-bench-virtualcorpus-drift test (error_boundary_queries_match_observed_behavior, q013 error-boundary declaration vs observed engine behavior; the crate is untouched here beyond a lint#[allow]and the test pre-dates the fork) and a testcontainers flake (fluree-db-connectionpublish_index_without_preexisting_index_item— passes on immediate re-run; crate untouched).Live rig (standing Snowflake ABACYOU-PP85756 + AWS sandbox, all under fresh isolated caches):
Table metadata used inline from loadTable responsefired exactly once;r2rml.read_metadatan=0.fluree infoprints OAuth2client_secretin plaintext — ConfigValue redacts Debug but not Serialize #1497: localfluree infoshows"client_secret": "[redacted]", zero PAT occurrences.no identity-based policy allows(no-perm role) andno permissions boundary allows(boundary-limited role). This is the distinction solo's runbook dispatch branches on (a boundary'd admin must not be told to re-apply a bucket policy).credential_source:"vended"; 200credential_source:"ambient"under the bucket-policy-only role (HeadObject needs no ListBucket — Lambda shape confirmed); 403@type: err:storage/AccessDeniedunder the no-perm role. The probe reads S3 directly (never cache-served), so it cannot be masked by a warm artifact cache.Composition proven live (Snowflake
virtual-sf01, restacked binary) — two consecutiveexec-oneprocesses, warm disk between them:r2rml.load_table.niceberg.oauth_token.nr2rml.read_metadata.nThe counters self-discriminate, so this is a proof rather than a correlation: the prime did a real loadTable (
n=1) ⇒ the rung missed ⇒ the diskmetadatalayer was empty; andread_metadata.n=0⇒ §1 served that metadata inline, with no S3 read. So the only possible writer of the entry the gate then hit is §1's seeding — and #1503's DoD gate (load_table.n=0andoauth_token.n=0) holds because of it. Pre-fix this readsload_table.n=1at ~3 s with nothing else looking wrong.Live rig re-verified at the restacked head (standing Snowflake + AWS sandbox; every credential-discriminating case under a fresh per-process cache): vended E2E ✅ (3 rows, 4.82 s) · BYO-IAM E2E under a bucket-policy-only role with zero identity-based S3 permissions ✅ (3 rows, 3.47 s) · both AWS AccessDenied strings survive verbatim to the CLI (
no identity-based policy allowspresent,no permissions boundary allowscorrectly absent for the identity variant) ✅ ·fluree inforedaction, zero PAT occurrences ✅ · zero-REST rung × BYO-IAM ✅ (second process resolves with no catalog REST at all) · on-disk proof of §1 seeding (*.shared.metadata.jsonpresent after a run that logged "used inline … (no S3 fetch)") ✅ · no cache file ever contains the PAT ✅.§2's guard is live-proven on the lazy arm: the deferred path emits
Using ambient AWS credentials, a log line that lives insiderest_session_storageafter the §2 check — so the shared guard demonstrably executes there.Benchmark corpus:
vbench compare --gateis running post-push against the new base (the chain adds q055–q059, so the pre-restack baseline is not comparable). Prior to the restack this branch gated clean (0 hash mismatches, 0 perf violations across 108 records; native median Δ +0 ms, virtual −37 s). Result will be posted here; note the chain's own baseline carries 5 known network-attributed violations (their F21).Notes for reviewers
fluree-db-querystays iceberg-agnostic (generic storage-denied variant; mapping helper lives in fluree-db-api). The 401→refresh-retry path is status-based on the catalog call and untouched.Flureeis nowClone(all fields Arc-backed or already-Clone);with_secret_resolveris the per-request-or-process-wide injection point. Non-iceberg builds unaffected (feature-gated).verify_storage_accessrequires the catalog to vend inline metadata — same as the existing preview path; Snowflake Horizon/Polaris do.main, not introduced here): a storage denial on the disk-cache-fill Parquet route surfaces as 500Internalinstead of 403 +err:storage/AccessDenied, becausesend_parquet.rs'scoalesced_fetchflattens the typed error through anio::Errortwo layers below our lift. perf(iceberg): loadTable-METADATA cache — zero-REST first-ask (q031 72s→~6s arc) + F19 residual #1503 makes that route more reachable (warm catalog + cold artifact cache ⇒ the first S3 read is a data file). §5 is a strict improvement everywhere it lands; the raw AWS chain still survives so host regex dispatch still fires. Not fixed here deliberately — it touches the Parquet reader's hot path and deserves its own review + bench.fluree-db-cli/src/main.rs:22contradicts the code (honored only with--verbose); machine-global artifact cache means previously-fetched table data remains locally readable after upstream access revocation (inherent to content-addressed caching; the §6 probe is immune by construction).