Repository navigation
[fix](hudi) Keep storage credentials out of scanner logs - #69478
Conversation
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Hudi JNI scanners logged every input parameter value at DEBUG, including explicit S3A access keys, secret keys and session tokens. Log parameter names and the query ID separately while preserving the Hadoop configuration used to read data. Add a regression test that captures DEBUG output, checks that query IDs remain available, rejects bucket-specific and custom credential values, and verifies that the scanner retains the original Hadoop properties. ### Release note Hudi JNI scanner debug logs no longer record storage credential values and continue to include query IDs for diagnostics. ### Check List (For Author) - Test: Unit Test - 16 targeted Hudi scanner, plugin and column-value tests pass; the new log test was verified to fail on the original code. Maven validate / Checkstyle and git diff --check pass. - Behavior changed: Yes - scanner debug logs show parameter names and the query ID without storage credentials. - Does this need documentation: No
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
TPC-H: Total hot run time: 28334 ms |
TPC-DS: Total hot run time: 152346 ms |
ClickBench: Total hot run time: 24.34 s |
FE Regression Coverage ReportIncrement line coverage |
|
/review |
There was a problem hiding this comment.
Reviewed both changed files at a5e90e3. No actionable findings identified. No existing inline findings require carry-forward, and no additional user focus was supplied.
Critical checkpoints:
- Goal and evidence: constructor DEBUG output now contains parameter names and a separate query ID, with parameter values omitted. The new JUnit test exercises the real constructor, checks ordinary and bucket-specific S3A credentials plus custom values, and verifies the Hadoop property map is preserved.
- Scope and clarity: the production change is limited to two logging statements; property extraction and reading behavior are unchanged.
- Concurrency and locks: no production synchronization or shared mutable state is added. The test captures one named JUL logger with a concurrent message list; no concurrent test execution is configured in this module.
- Lifecycle and initialization: the capture restores the prior logger level and parent-handler setting and removes its handler through try-with-resources. Scanner construction does not open a reader or acquire a filesystem hold.
- Configuration: no configuration items are added or changed.
- Compatibility: no public symbols, storage formats, or serialized protocols change.
- Thrift/protobuf: no definitions change; cross-branch field-ID comparison is inapplicable.
- Parallel paths: both BE Hudi reader implementations use the same plugin/scanner and supply a runtime-generated query ID. Constructor logging is therefore covered for both.
- Conditional logic: the existing DEBUG guard and Hadoop-property prefix extraction are retained; no new conditional behavior is introduced.
- Test coverage: the negative log assertions cover access keys, secret keys, session tokens, bucket overrides, a custom Hadoop value, and a non-Hadoop token. Positive assertions require DEBUG logging, all parameter names, and the query ID. The module binds SLF4J to JUL and excludes competing providers from Surefire.
- Expected results: no regression output files are added or modified; the new test asserts known synthetic values and exact property-map equality.
- Observability: parameter names remain visible and the query ID is logged separately; no new metrics are needed for this logging-only change.
- Transactions and persistence: no transaction, visibility/version, delete-bitmap, EditLog, or failover behavior changes.
- Data writes and atomicity: no data-writing path changes.
- FE/BE variable propagation: no new transmitted variables; query_id already exists in both reader paths.
- Performance and memory: one parameterized DEBUG call and map lookup per scanner; no additional collection traversal or production allocation proportional to parameter values.
- Additional review: authentication setup, Hadoop configuration handoff, plugin factory dispatch, and error propagation remain unchanged; no further actionable regression found.
Validation limits: this was a static review. Builds, unit tests, and a FE/BE/S3 end-to-end run were not executed in this review environment. Test execution claims in the PR description were not independently reproduced. No unresolved code-review candidates or uncovered changed files remain.
Review converged after 2 round(s).
…#69479) ### What problem does this PR solve? Issue Number: N/A Related PR: #69478 (backport to branch-4.2) Problem Summary: When a Hudi JNI scan uses explicit S3A credentials, enabling scanner DEBUG logging prints every parameter value, including access keys, secret keys and session tokens. Log parameter names without values and record the query ID separately so scans remain traceable. The Hadoop configuration used to read data is unchanged. Add a regression test that captures the real scanner's DEBUG output, checks that parameter names and the query ID remain visible, rejects ordinary and bucket-specific credentials and custom tokens, and verifies that Hadoop properties retain their original values. ### Release note Hudi JNI scanner debug logs no longer include storage credential values and continue to include query IDs for diagnostics. ### Check List (For Author) - Test - [ ] Regression test - [x] Unit Test - [ ] Manual test - [ ] No need to test or manual test. Validation with JDK 17: - `EXTRA_FE_MODULES=hudi=be-java-extensions/hadoop-hudi-scanner ./run-fe-ut.sh --run 'org.apache.doris.hudi.HadoopHudiJniScannerLogTest,org.apache.doris.hudi.HadoopHudiColumnValueTest,org.apache.doris.hudi.HudiUuidTransportTest'`: 9 tests passed. - The log regression test verifies query ID retention, credential-free DEBUG output and unchanged Hadoop properties. - The test captures this branch's Log4j/reload4j backends. - Maven `validate` / Checkstyle for `be-java-extensions/hadoop-hudi-scanner` and its dependencies, plus `git diff --check`: passed. - The logging regression is covered directly by JUnit. FE/BE + S3 end-to-end regression was not run locally; SQL planning, credential propagation and data-reading logic are unchanged. - Behavior changed: - [ ] No. - [x] Yes. DEBUG logs retain parameter names and query IDs without printing other parameter values. - Does this need documentation? - [x] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
…#69480) ### What problem does this PR solve? Issue Number: N/A Related PR: #69478 (backport to branch-4.1) Problem Summary: When a Hudi JNI scan uses explicit S3A credentials, enabling scanner DEBUG logging prints every parameter value, including access keys, secret keys and session tokens. Log parameter names without values and record the query ID separately so scans remain traceable. The Hadoop configuration used to read data is unchanged. Add a regression test that captures the real scanner's DEBUG output, checks that parameter names and the query ID remain visible, rejects ordinary and bucket-specific credentials and custom tokens, and verifies that Hadoop properties retain their original values. ### Release note Hudi JNI scanner debug logs no longer include storage credential values and continue to include query IDs for diagnostics. ### Check List (For Author) - Test - [ ] Regression test - [x] Unit Test - [ ] Manual test - [ ] No need to test or manual test. Validation with JDK 17: - `EXTRA_FE_MODULES=hudi=be-java-extensions/hadoop-hudi-scanner ./run-fe-ut.sh --run 'org.apache.doris.hudi.HadoopHudiJniScannerLogTest,org.apache.doris.hudi.HadoopHudiColumnValueTest'`: 5 tests passed. - The log regression test verifies query ID retention, credential-free DEBUG output and unchanged Hadoop properties. - The test captures this branch's Log4j/reload4j backends. - Maven `validate` / Checkstyle for `be-java-extensions/hadoop-hudi-scanner` and its dependencies, plus `git diff --check`: passed. - The logging regression is covered directly by JUnit. FE/BE + S3 end-to-end regression was not run locally; SQL planning, credential propagation and data-reading logic are unchanged. - Behavior changed: - [ ] No. - [x] Yes. DEBUG logs retain parameter names and query IDs without printing other parameter values. - Does this need documentation? - [x] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
What problem does this PR solve?
Issue Number: N/A
Related PR: #69479 (branch-4.2), #69480 (branch-4.1), #69481 (branch-3.1)
Problem Summary: When a Hudi JNI scan uses explicit S3A credentials, enabling scanner DEBUG logging prints every parameter value, including access keys, secret keys and session tokens. Log parameter names without values and record the query ID separately so scans remain traceable. The Hadoop configuration used to read data is unchanged.
Add a regression test that captures the real scanner's DEBUG output, checks that parameter names and the query ID remain visible, rejects ordinary and bucket-specific credentials and custom tokens, and verifies that Hadoop properties retain their original values.
Release note
Hudi JNI scanner debug logs no longer include storage credential values and continue to include query IDs for diagnostics.
Check List (For Author)
Test
Validation with JDK 17:
./run-fe-ut.sh --run 'org.apache.doris.hudi.HadoopHudiJniScannerLogTest,org.apache.doris.hudi.HadoopHudiPluginTest,org.apache.doris.hudi.HadoopHudiColumnValueTest': 16 tests passed.validate/ Checkstyle forbe-java-extensions/hadoop-hudi-scannerand its dependencies, plusgit diff --check: passed.Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)