Repository navigation
[feature](fe) Prepare encrypted Iceberg HMS metadata reads - #70192
Merged
hello-stephen merged 4 commits intoNov 5, 2026
Merged
Conversation
### What problem does this PR solve? Related PR: apache#70159 Problem Summary: The authentication FileIO wrapper inherits typed defaults that reject encrypted descriptors before the Iceberg SDK delegate can decrypt them. Forward data, delete, manifest and manifest-list descriptors inside the authentication scope. Add reproducible v3 encrypted HMS samples backed by disposable KMS and MinIO services, verified independently with Iceberg SDK and Spark SQL across two snapshots. ### Release note Preserve Iceberg encryption metadata through authenticated FileIO. Encrypted SQL execution remains protected by the existing admission guard. ### Check List (For Author) - Test: Connector reactor package: 1,913 passed, 14 existing skips; full FE build and Checkstyle passed; encrypted HMS SQL regression passed with 48 rejection assertions and plaintext row/count comparisons. - Behavior changed: Yes, typed FileIO descriptors preserve SDK encryption metadata. - Does this need documentation: Yes, connector contract and fixture instructions updated in this commit.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
TPC-H: Total hot run time: 28859 ms |
Contributor
TPC-DS: Total hot run time: 151373 ms |
Contributor
ClickBench: Total hot run time: 24.33 s |
Contributor
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
### What problem does this PR solve? Related PR: apache#70192 Problem Summary: Reference jars downloaded through mktemp retained owner-only permissions, preventing the non-root Spark fixture from compiling on a fresh Linux setup. Set readable permissions after checksum verification and before atomic publication, and repair valid cached artifacts on reuse. ### Release note None ### Check List (For Author) - Test: Reproduced the original Permission denied failure in the Spark 4.0.0 container; verified fixture compilation as its default non-root user after clean downloads and after repairing cached mode-0600 jars with an unavailable download source. Shell syntax and diff checks passed. - Behavior changed: Yes. Verified reference jars are readable by the fixture container on fresh downloads and cache hits. - Does this need documentation: No. This restores the documented fixture startup behavior.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
### What problem does this PR solve? Related PR: apache#70192 Problem Summary: Unconditionally changing cached jar permissions rejects valid readable artifacts owned by another user. Inspect the target other-read bit before repairing permissions so Spark can reuse shared caches without requiring ownership, while restrictive caches are still repaired. ### Release note None ### Check List (For Author) - Test: Reproduced the cache ownership failure. Verified actual fixture compilation as the default non-root Spark user with fresh downloads, repaired mode-0600 caches, and root-owned mode-0644 caches. Cache checks used an unavailable source; shared cache metadata remained unchanged. Shell syntax and diff checks passed. - Behavior changed: Yes. Already-readable shared caches no longer require ownership to be reused. - Does this need documentation: No. This restores the documented fixture startup behavior.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
TPC-H: Total hot run time: 28853 ms |
Contributor
TPC-DS: Total hot run time: 151523 ms |
5 of 9 tasks
Contributor
ClickBench: Total hot run time: 24.31 s |
Contributor
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#70192
commit: 7e8fa587a222dbdf9bb39a70b58f37257e253ae9
base: 0eb5131db62cd7939788508377fc148df6ac1ef1
reviewed_at: 2026-11-04T19:05+08:00
reviewer: Gabriel39
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: trueNotes for maintainers
Reviewed locally with the |
924060929
previously approved these changes
Nov 4, 2026
924060929
left a comment
924060929
left a comment
Contributor
There was a problem hiding this comment.
Iceberg encrypted reads require typed file descriptors to preserve key metadata and manifest-list key IDs. This PR forwards all four descriptor types through the existing authentication scope and adds an isolated HMS/KMS fixture; encrypted SQL execution remains blocked by the existing admission checks.
Independent static review of the 17 changed files and affected FE callers found no blocking P0/P1/P2 issues. The fix belongs in the shared FileIO decorator, preserves descriptor semantics, and does not bypass admission for cached reads, COUNT pushdown, or metadata tables. Resource ownership and production wire protocols are unchanged.
Approve for the reviewed FE scope at 7e8fa58. No builds or tests were run during this review. The supplied snapshot shows compilation and Checkstyle success, while P0, FE UT, and external regression remain pending. This recommendation does not establish merge readiness.
Automated FE-focused static review (gpt-6.1-sol, high). No build or tests were run.
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
### What problem does this PR solve? Related PR: apache#70192 Problem Summary: The external regression setup rewrites hms_port with an unanchored substitution that also matches iceberg_encryption_hms_port. The suite then connects to the default Hive metastore instead of its isolated encrypted-metadata fixture and fails with a missing database. Rename the regression key to iceberg_encryption_metastore_port in both configurations, the suite and the fixture documentation. Preserve the Docker port mapping. ### Release note None ### Check List (For Author) - Test: Reproduced the original substitution failure. Verified both configuration files with legacy HMS replacements of 9083 and 9183. Ran the full enabled encrypted-HMS suite after the same substitution: 1 suite passed, 0 failures, 0 skips, including plaintext checks, 48 encrypted admission rejections, SDK/Spark snapshots and all four KMS/key failure checks. Reused existing local FE/BE binaries; no production source changes. Diff checks passed. - Behavior changed: No. Correct the regression fixture endpoint selection. - Does this need documentation: Yes. The fixture configuration mapping is updated.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
TPC-H: Total hot run time: 28238 ms |
Contributor
TPC-DS: Total hot run time: 150584 ms |
Contributor
ClickBench: Total hot run time: 24.21 s |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
924060929
approved these changes
Nov 5, 2026
924060929
left a comment
924060929
left a comment
Contributor
There was a problem hiding this comment.
Iceberg encrypted reads require typed descriptors to retain key metadata and manifest-list key IDs. This PR forwards all four descriptor types through the shared authenticated FileIO decorator and adds an isolated HMS/KMS fixture. Existing admission checks still reject encrypted SQL before manifest planning and BE task generation.
Independent static review of all 17 changed files and affected FE callers found no blocking P0/P1/P2 issues. The fix is placed at the shared abstraction, preserves authentication and descriptor semantics, and leaves resource ownership and production protocols unchanged. The increment since the previously reviewed commit only renames the fixture metastore configuration key consistently across both configurations, the suite, and documentation.
APPROVE for the FE scope at 3ac88c5. No builds or tests were run during this review. The supplied head snapshot reports compilation, Checkstyle, FE UT, P0, and external regression success; FE coverage failed and automated AI review remains incomplete. This recommendation does not establish merge readiness.
Automated FE-focused static review (gpt-6.1-sol, high). No build or tests were run.
Gabriel39
added a commit
that referenced
this pull request
Nov 5, 2026
### What problem does this PR solve? Related PRs: #70159, #70192 Prepare the file metadata and diagnostic protection required for native Iceberg encrypted reads. Data files and each delete file need independent binary keys and AAD. Generated Thrift strings and JSON diagnostics must not expose those values. - Define `TFileEncryptionInfo` directly in `PlanNodes.thrift`, alongside its data/delete consumers. The optional carrier contains a payload version, binary file key and exact AAD bytes; malformed payloads are rejected. There is no separate encryption IDL file. - Assume FE and BE are deployed together. This PR adds no encryption heartbeat fields, receiver capability negotiation, capability persistence or downgrade/replay handling. - Validate assigned fragment parameters/ranges and every lazy split response. FE and BE uniformly reject encrypted execution with `READ_UNAVAILABLE` before readers, caches or metadata COUNT shortcuts. A denied final split batch stops producers and remains an error on repeated fetches. - Keep the BE file-encryption validator inside `file_scan_range_utils.h`, alongside its only production caller, with no separate validator header. - Redact generated Java/C++ strings, nested output, ThriftDebugString, Gson and Thrift JSON diagnostics. Binary/Compact retain the real payload. The generation transform targets only the secret carrier inside `PlanNodes_types.cpp`, leaves other plan diagnostics unchanged, is idempotent and fails on compiler-shape drift. FE, regression-framework and C++ generation use the same protection. - Keep encrypted SQL reads disabled until native decryption and authenticated, confidential transport for both fragment dispatch and lazy split responses are implemented. The Iceberg option cannot bypass these requirements. The read and wire contracts document this scope. File-key extraction and native decryption are subsequent roadmap work. ### Validation - Full FE build and FE Checkstyle passed with Maven build cache disabled. - Official focused FE tests: 55 passed (10 protocol tests and 45 FE tests), zero failures/errors/skips. Covers independent data/delete keys, diagnostics, malformed payloads, plain scans, direct/lazy dispatch and terminal last-batch retries. - SQL normal comparisons passed: the wire guard suite executed 80 expected rejections with plaintext rows/counts; the existing D1 admission suite executed 96 expected rejections with plaintext comparisons. Covers v2/v3, actual lazy planning, COUNT pushdown, repeated queries, malformed versions and key lengths. Existing generated result files remain valid and unchanged. - Compiled the actual regenerated `PlanNodes_types.cpp` with the existing C++ contract suite: 4 tests passed. Generator self-tests: 4 passed, including idempotence, compiler drift and preservation of other plan diagnostics. Regression-framework generation also passed with the pinned Thrift compiler. - Clang-format 16, clang-tidy and BE build hygiene passed on affected files. Packaged heartbeat classes no longer contain either removed field or the removed backend state class. - The updated guard expectations failed against the old implementation, then passed after simplification. - Reproduced all three `PluginDrivenScanNodePartitionBatchTest` failures: its fixture omitted the required `scan_range`, so lazy dispatch validation dereferenced a missing range. Completed the fixture with the same plaintext file-scan envelope used by production and neighboring streaming tests, preserving split identities and all existing assertions. The official expanded FE run then passed 383 tests across 46 classes, with zero failures/errors/skips, including all nine partition-batch tests, plugin scan, split producer/consumer/assignment/compaction/concurrency, and encryption protocol/guard coverage. FE Checkstyle passed with zero violations. This follow-up changes only the unit-test fixture. - Inlined the unchanged BE validator and documented build-time redaction versus runtime wire/diagnostic serialization. Four standalone ASAN C++ contract tests passed against freshly regenerated `PlanNodes` code, and all four Python generator tests passed. Three relevant BE test translation units passed syntax checks using the existing ASAN compile database. Clang-format 16, clang-tidy and header hygiene passed. Python transform logic and Thrift field definitions are unchanged. The SQL tests use the updated FE and an existing ASAN BE. Native encrypted SQL interoperability is outside this protocol-preparation stage. Full BE ASAN unit execution was not rerun in this update: the earlier attempt stopped in the unchanged baseline `vparquet_writer_test.cpp` because its `WriteInfo` initializer still uses removed path members. The standalone C++ tests do not replace a full ASAN run. ### Release note Prepare per-file Iceberg encryption metadata and redacted diagnostics. Encrypted SQL reads remain disabled. ### Check List (For Author) - Test - [x] Regression test - [x] Unit Test - [x] Manual test (full FE build and local external stack) - Behavior changed: - [x] Yes. Encrypted file payloads are validated and uniformly rejected until execution is implemented. - Does this need documentation? - [x] Yes. Read and wire contracts are included. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
This was referenced Nov 5, 2026
Gabriel39
added a commit
to Gabriel39/incubator-doris
that referenced
this pull request
Nov 5, 2026
…anning ### Background Iceberg decrypts encrypted manifests before native scan planning. Doris already has a per-file encryption Thrift descriptor, but the Iceberg planner does not populate its key and AAD fields. ### What problem does this PR solve? Issue Number: apache#70245 Related PR: apache#70192 (metadata planning and fixture), apache#70213 (file-key wire contract) PR Type: Feature Problem Summary: Extract each data file's StandardKeyMetadata into every scan split so native readers can receive the correct file key and AAD. Preserve plaintext files and reject malformed metadata without exposing binary payloads. Also fix the encrypted metadata fixture's first reset after KMS restart: retained HMS tables previously caused purge to fail while reading manifests encrypted with the lost key. ### Details - **Change map:** 220 production lines changed, 280 test lines, 29 documentation lines and 2 Checkstyle configuration lines; no generated files. Production changes add a bounded SDK metadata bridge, an immutable per-file carrier and split population; tests cover SDK interoperability and split ownership, plus a real fixture restart; documentation describes the stage boundary and lifecycle test. - **Scope:** Parquet data-file key transport. Encrypted SQL admission remains disabled; encrypted deletes and native reader integration are subsequent stages. Reject independently encrypted deletes in merge-on-read and native position_deletes metadata-table planning until delete decryption is integrated. - **Design:** Validate metadata size and encoded field lengths before the SDK codec can allocate. Parse one immutable snapshot, reuse the carrier only within a file, and copy mutable Thrift buffers for each split. Null per-file metadata remains plaintext. Errors contain static classifications without codec causes. - **COUNT guard:** Both metadata COUNT algorithms fall back to normal per-file ranges when any live data file carries key metadata. Manifest totals do not certify per-file encryption absence, so the aggregate path now validates all live file metadata in O(files) with bounded retained heap, without reading data files. Encrypted delete files also prevent position-delete netting from bypassing the guard. - **Usage:** No new configuration. Existing encrypted-read admission and BE dispatch guards remain in place. - **Default behavior:** Encrypted SQL remains rejected. Plaintext ranges retain their previous wire shape. - **Compatibility:** Reuses existing optional Thrift fields and the Iceberg SDK's StandardKeyMetadata v1 codec; no protocol or plugin SPI change. - **Behavior change:** Native scan splits now carry per-file encryption parameters; the test fixture can reset retained tables after its KMS loses keys. Encrypted SQL admission is unchanged. - **Fixture fix:** Drop only the two dedicated fixture HMS entries without purge, then delete and verify their fixed object prefixes. Cleanup therefore needs no old KMS key and leaves no orphan objects in those prefixes. The restart regression retains HMS/storage, restarts KMS/fixture, and requires the first reset to succeed without retrying it. ### Testing - Locally: 286 FE tests across scan planning/ranges, SDK encrypted metadata I/O, admission and the new adapter; zero failures. Final adapter/split rerun: 5 tests, zero failures. - Review-fix validation: both reported holes reproduced in two failing tests, then 280 focused FE tests passed with zero failures/errors. Coverage includes both COUNT algorithms with the cache enabled/disabled, position/equality delete rejection, native position_deletes and netted COUNT. Existing plaintext COUNT behavior remains covered. - Locally: Maven reactor validation / FE Checkstyle passed, including the review fixes. - Before the fix: the first fixture reset after restarting the disposable KMS returned HTTP 500. - After the fix: the committed lifecycle regression passed; SDK and Spark both read historical snapshots `[1,2]` and `[1,2,3,4]`, with two file keys and encrypted manifests/manifest lists. Missing KMS, invalid credentials, denied access and wrong keys remained rejected. - Shell syntax and diff whitespace checks passed. Full encrypted Doris SQL is intentionally unavailable at this stage. ### Release note None. This prepares file-key transport while encrypted query execution remains disabled, and makes the external regression fixture restartable. ### Check List (For Author) - Behavior changed: Yes (see Details: per-file transport and fixture reset) - Does this need documentation: No user-facing feature enablement; internal contract and fixture instructions updated. - Pre-submission status: Ready ### AI Disclosure - Tool: Codex (GPT-6) - Generated: Code, tests and this description. - Human review: Pending maintainer review; agent self-review completed.
Gabriel39
added a commit
that referenced
this pull request
Nov 5, 2026
…70246) ### Background Iceberg decrypts encrypted manifests before native scan planning. Doris already has a per-file encryption Thrift descriptor, but the Iceberg planner does not populate its key and AAD fields. ### What problem does this PR solve? Issue Number: #70245 Related PR: #70192 (metadata planning and fixture), #70213 (file-key wire contract) PR Type: Feature Problem Summary: Extract each data file's StandardKeyMetadata into every scan split so native readers can receive the correct file key and AAD. Preserve plaintext files and reject malformed metadata without exposing binary payloads. Reject mixed-file metadata COUNT during bounded validation so unsupported encryption cannot trigger eager full-table task and range allocation before dispatch fails. Also fix the encrypted metadata fixture's first reset after KMS restart: retained HMS tables previously caused purge to fail while reading manifests encrypted with the lost key. ### Details - **Change map:** 238 production lines changed, 325 test lines, 38 documentation lines and 2 Checkstyle configuration lines; no generated files. Production changes add a bounded SDK metadata bridge, an immutable per-file carrier and split population; tests cover SDK interoperability, split ownership, bounded COUNT rejection and a real fixture restart. - **Scope:** Parquet data-file key transport. Encrypted SQL admission remains disabled; encrypted deletes and native reader integration are subsequent stages. Reject independently encrypted deletes in merge-on-read and native position_deletes metadata-table planning until delete decryption is integrated. - **Design:** Validate metadata size and encoded field lengths before the SDK codec can allocate. Parse one immutable snapshot, reuse the carrier only within a file, and copy mutable Thrift buffers for each split. Null per-file metadata remains plaintext. Errors contain static classifications without codec causes. - **COUNT guard:** Both metadata COUNT algorithms validate live file metadata in O(files) with bounded retained heap, without reading data files. A per-file encryption marker rejects immediately with `COUNT_READ_UNAVAILABLE`, including when the table has no encryption declaration. Returning to ordinary scan planning would retain all tasks and splits after COUNT scheduling had disabled streaming. Deterministic rejection bypasses the manifest cache retry; actual cache failures retain the SDK retry. Encrypted delete files also prevent position-delete netting from bypassing the guard. - **Usage:** No new configuration. Existing encrypted-read admission and BE dispatch guards remain in place. - **Default behavior:** Encrypted SQL remains rejected. Plaintext ranges retain their previous wire shape. - **Compatibility:** Reuses existing optional Thrift fields and the Iceberg SDK's StandardKeyMetadata v1 codec; no protocol or plugin SPI change. - **Behavior change:** Native scan splits now carry per-file encryption parameters; the test fixture can reset retained tables after its KMS loses keys. Encrypted SQL admission is unchanged. - **Fixture fix:** Drop only the two dedicated fixture HMS entries without purge, then delete and verify their fixed object prefixes. Cleanup therefore needs no old KMS key and leaves no orphan objects in those prefixes. The restart regression retains HMS/storage, restarts KMS/fixture, and requires the first reset to succeed without retrying it. ### Testing - Final local verification: **295 FE tests, zero failures/errors/skips**, covering `IcebergScanPlanProviderTest`, `IcebergScanRangeTest`, `IcebergFileEncryptionTest`, `IcebergEncryptionReadTest` and `IcebergEncryptedMetadataIOTest`. Maven reactor validation and FE Checkstyle passed. - Red/green proof: the initial four-case COUNT matrix failed before the fix. The final eight-case matrix passes across SDK/cache, present/missing manifest row aggregates and manifest concurrency 1/8. A batch threshold of 1 proves ordinary scanning would stream while COUNT stays synchronous; assertions verify early rejection, no second SDK planning pass, SDK iterable closure, no cache-failure accounting, and stopping before the tail manifest in serial mode. - Existing plaintext COUNT collapse, delete handling and both real cache-failure SDK retries pass. Final verification uses `-XX:ActiveProcessorCount=2 -XX:TieredStopAtLevel=1`, matching the FE unit-test JVM setting. A diagnostic run without the processor cap hit three existing cancellation-test fixture-count assertions caused by automatic manifest merging; those test implementations were not changed. - Previously verified, unchanged fixture lifecycle: the first reset after a disposable KMS restart failed before the fixture fix and passes afterward. SDK and Spark read historical snapshots `[1,2]` and `[1,2,3,4]`; missing KMS, invalid credentials, denied access and wrong keys remain rejected. - Diff whitespace checks passed. Encrypted Doris SQL execution remains unavailable at this stage; this update validates the planner guard rather than enabling encrypted execution. ### Release note None. This prepares file-key transport while encrypted query execution remains disabled, and makes the external regression fixture restartable. ### Check List (For Author) - Behavior changed: Yes (see Details: per-file transport and fixture reset) - Does this need documentation: No user-facing feature enablement; internal contract and fixture instructions updated. - Pre-submission status: Ready ### AI Disclosure - Tool: Codex (GPT-6) - Generated: Code, tests and this description. - Human review: Pending maintainer review; agent self-review completed.
yiguolei
pushed a commit
that referenced
this pull request
Nov 6, 2026
### Background Doris has per-file Iceberg encryption parameters and a native Parquet reader, but that reader cannot consume encrypted footers or pages. Parquet modular encryption authenticates each module with file AAD and row-group, physical-column and data-page ordinals. ### What problem does this PR solve? Issue Number: #70245 Related PR: #70087 (PME dependency build), #70192 (Iceberg metadata reads), #70213 (file-key wire contract), #70246 (per-file key extraction) PR Type: Feature Problem Summary: Add the physical reader needed to read AES_GCM_V1 Parquet files with encrypted footers and footer-key columns. Authenticate and decrypt modules through parquet-cpp before passing bytes into the existing native metadata and value decoders. Java/Iceberg interoperability, bounded parsing and cache isolation require changes beyond the crypto wrapper. Enforce the native container budget independently of unverified serialized element types, so malformed authenticated metadata cannot undercount its allocation cost. Encrypted SQL admission remains closed while index, delete-file and final scan integration are completed separately. ### Details - **Change map:** 1300 production lines changed, 1420 test/generator lines, 57 generated result lines, 81 documentation lines, 9 scanner configuration lines and 4 small synthetic binary fixtures/twins/copies (about 10 KB total). Production changes comprise module crypto/parsing, footer ownership, native page/column plumbing and sequential scan guards. The native column factory now takes four arguments through a creation context and explicit read options, with separate complex-type helpers and one scalar initialization path. Tests cover independent Java/Iceberg interoperability, validation/authentication boundaries and real S3 SQL scans. - **Scope:** AES_GCM_V1 encrypted footers, encrypted page headers, dictionary pages and Data Page V1/V2; 128/192/256-bit footer keys; scalar and nested physical decoding. AES_GCM_CTR_V1, plaintext-footer encryption and separate column keys are explicitly unsupported. - **Design:** Keep physical decoding in format_v2. The crypto wrapper uses parquet-cpp AES and module-AAD helpers, retains an immutable SecureString file key, and creates mutable decryptors per physical column reader. Track ciphertext disk sizes separately from decrypted compressed sizes and count only data pages in AAD ordinals. Authenticate metadata before accepting Parquet Java's zero-filled serialization padding. Bound encrypted modules and decompressed pages to 64 MiB and impose Thrift container, recursion and aggregate allocation limits before decoding. Bypass shared decrypted metadata/page/condition caches and index/dictionary/bloom pruning for the sequential stage. Recursive column construction shares immutable file/row-group inputs by reference, while keeping in_collection local to each node and mutable decryptors local to each physical leaf. Pass cache policy directly instead of creating a temporary RuntimeState to override it. - **Metadata budget:** Generated Thrift readers use statically declared C++ vector elements without validating the serialized element type. Charge each list/set element and both sides of each map entry at the largest reachable native element size. This is a conservative budget; very large primitive/string lists may reach the limit sooner. No schema mirror or generated source change is needed. - **Scratch reclamation:** Include decrypted page capacity and active bytes in the existing scalar scratch policy. A page that grows to 32 MiB can relinquish its unused capacity after ordinary small batches, while release waits for a page boundary so live payload and V2 level views stay valid. The existing idle thresholds remain unchanged. Page activity is tracked separately from vector size, preserving initialized bytes for same-size reuse without an additional full-page initialization. - **GCC unity builds:** Keep the shared secure-key implementation type in a named namespace. The externally linked PImpl types must not contain an anonymous-namespace type when this source is included by a unity translation unit. - **Usage:** Controlled physical-reader callers can supply the existing TFileEncryptionInfo as an optional constructor parameter. No new engine configuration, Thrift field, capability negotiation or user SQL entry is added. Encrypted indexes and delete-file integration remain follow-up work. - **Default behavior:** Existing plaintext callers retain default parameters and their existing cache/pruning paths. FE encrypted-table admission and BE scan dispatch guards remain closed. - **Compatibility:** Reuses the existing descriptor. PME requires the internal headers and actual encryption implementation from the previously enabled third-party build; older prebuilt bundles compile and explicitly report PME_DEPENDENCY_UNAVAILABLE. No storage format or dependency version changes. - **Behavior change:** The native physical reader can decrypt the supported layout when explicitly given file parameters; encrypted files without keys and unsupported layouts fail explicitly. This PR does not enable encrypted SQL queries. ### Testing - Latest full local ASAN BE build through `build.sh --be -j 6` passed, including the GLIBC_2.17 compatibility gate. - Latest final run: 249 focused ASAN tests passed across ten Parquet/native-decoder, page-cache and existing file-encryption/scan-range guard suites, including all 24 encryption tests. Used `run-be-ut.sh` with a local CMake test-source filter; production libraries were built normally. This is not the full BE unit-test suite. - Decrypted-page scratch red/green coverage: the new real-reader test fails before the repair because a 32 MiB page is absent from retained scratch accounting. After the fix, Page V1/V2 x UNCOMPRESSED/SNAPPY all preserve active page data and NULL values, defer release while page views remain live, and reduce retained scratch to at most 4 MiB after small-page transitions. SNAPPY cases assert that decompression actually runs. - GCC 15.1 unity compilation: a syntax check that includes the actual production source reproduces both subobject-linkage errors before the fix and passes after it. This is a focused compiler check, not a full GCC release build. - Allocation-budget red/green coverage: the new real-parser regression fails on the old implementation and passes after the fix for byte-labeled LIST/SET/MAP containers. It uses sub-MiB inputs and skippable unknown fields to validate cumulative accounting without allocating large native vectors. The MAP threshold detects accounting for only one side of an entry. Existing Java/Arrow fixtures cover successful normal metadata decoding. - Factory refactor coverage: all four collection/offset-index combinations read the selected middle-page rows after the temporary creation context is destroyed. Explicit page-cache policy is verified cold/warm with no RuntimeState and with both session settings, including opposing settings. Both new tests also pass twice in one process. Existing nested projection and INT96 timezone tests continue to pass. - Positive physical-reader coverage: dictionary/plain encoding, Data Page V1/V2, three AES key sizes, multiple pages and row groups, null/empty nested shapes, levels-only reads and the ParquetReader block interface. The checked-in Iceberg 1.12.0 Java sample is compared field-by-field with the independent parquet-cpp reader. - Boundary coverage: descriptor fields, unsupported ciphers/plaintext footers/column keys, AAD policy and authentication, module/row/column/page AAD binding, ordinal/length limits, tampering/truncation, another key after cache warmup, independent decryptor state and avoiding encrypted index reads. Errors do not expose keys or AAD. - Added `test_iceberg_encryption_native_reader`: on a local cluster with a freshly built PR ASAN BE and an existing compatible FE, generation and a separate normal comparison run both passed; the normal comparison also passed against the refactored BE. Each run checks 21 expected errors and seven positive result sets/aggregates across page-cache and COUNT-pushdown settings. A plaintext first file allows schema discovery; actual mixed-file SELECT/COUNT must still reject the later encrypted file with MISSING_KEY. Repeated failures and a truncated input preserve subsequent plaintext nested values/counts. The suite needs standard MinIO, without KMS or a new encryption-specific test flag, and deletes its unique bucket/objects. This SQL regression was verified earlier on the PR and was not rerun for the metadata-budget, GCC linkage or scratch-reclamation updates. - Before implementation, the SQL missing-key assertion fails on the old BE with Invalid Parquet v2 footer magic. The initial missing-key unit contract also failed before implementation. - Positive crypto tests use PME rebuilt with the Doris toolchain. Original prebuilt dependencies previously passed four explicit dependency/metadata contracts and rejected decryption. - Gitleaks 8.30.0 reproduced the reported three fixture false positives; the complete PR commit history, including the budget fix, has zero findings. The exact-path/exact-value exception still detects a different value in that file and all three original occurrences outside that path. - Fixture generator compilation, clang-format 16, changed-line clang-tidy, build hygiene and source whitespace checks passed. SQL results are runner-generated; SQL binary copies byte-match their BE originals. - Additional diagnostic: repeating the whole selection in one process passes the first round, then two unchanged cold-cache profile tests fail second-round counter assertions because the global page cache is already warm (UnknownMtimeUsesPageCacheForImmutableFile and DictionaryPruningPublishesColdAndWarmNativePageProfile). The standard single-round selection passes; new cases use independent cache identities and pass repeated execution. - Full BE CI and positive encrypted SQL end-to-end validation are not claimed; encrypted SQL admission remains intentionally gated. ### Notes for Reviewers Start with parquet_crypto, then encrypted footer parsing and native page_reader. The direct format_common include fixes a pre-existing unity-neighbor include dependency exposed by adding a source file. The fixture README includes its generator, public synthetic parameters and test matrix. CI scans the full PR commit range; the Gitleaks exception requires both the exact native test path and one of the two documented public fixture values. ### Release note None. This prepares the native encrypted Parquet reader; encrypted SQL admission remains disabled. ### Check List (For Author) - Behavior changed: Yes (see Details). - Does this need documentation: No user-facing enablement; fixture and stage limitations documented alongside the tests. - Pre-submission status: Ready; full CI pending. ### AI Disclosure - Tool: Codex (GPT-6). - Generated: Code, tests, synthetic fixture generator and this description. - Human review: Pending maintainer review; agent self-review completed.
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
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.
What problem does this PR solve?
Related PR: #70159
The authenticated Iceberg FileIO wrapper inherits typed defaults that reject encrypted data, delete, manifest and manifest-list descriptors before the SDK encryption delegate can handle them. Forward all four descriptors inside the authentication scope, preserving their key metadata and manifest-list key IDs.
This also provides the reproducible encrypted HMS metadata fixture needed for the read roadmap:
[1, 2]and[1, 2, 3, 4].The existing plugin already packages AWS KMS dependencies; this PR verifies that closure rather than adding a second dependency. Encrypted SQL execution remains blocked by the existing admission guard. Native encrypted file execution is subsequent work.
Validation
test_iceberg_encrypted_hms_metadata: generated results using the standard runner, then normal comparisons passed twice, including the final fixture source. Each run executed 48 expected encrypted rejections and verified plaintext rows/counts.hms_portsubstitution also rewrites the encrypted-fixture port, causing the suite to query the wrong metastore and report a missing database. Renamed the regression key toiceberg_encryption_metastore_portin both configuration entry points, the suite and the fixture documentation; the Docker mapping is unchanged. Verified the exact substitution with both legacy port values on both configurations, then passed the full enabled suite after applying it: 1 suite, 0 failures, 0 skips, including all 48 expected rejections and SDK/Spark/KMS/plaintext checks. This config-only rerun reused existing local FE/BE binaries.Release note
Preserve SDK encryption metadata through authenticated Iceberg FileIO. Encrypted SQL execution remains guarded.
Check List (For Author)
Check List (For Reviewer who merge this PR)