Skip to content

[fix](hudi) Keep storage credentials out of scanner logs - #69478

Merged
yiguolei merged 1 commit into
apache:masterfrom
OIiveirra:fix/jira-sdbsec-256
Oct 23, 2026
Merged

yiguolei merged 1 commit into
apache:masterfrom
OIiveirra:fix/jira-sdbsec-256

Conversation

@OIiveirra

@OIiveirra OIiveirra commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

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

    • Regression test
    • Unit Test
    • Manual test
    • No need to test or manual 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.
    • The new log test fails on the original code because parameter values appear in DEBUG output and passes after the fix.
    • The existing plugin test writes and reads a local Hudi table through the Java scanner.
    • 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.
    • Yes. DEBUG logs retain parameter names and query IDs without printing other parameter values.
  • Does this need documentation?

    • 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: 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
Copilot AI balanced review requested due to automatic review settings October 22, 2026 17:31
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@OIiveirra

Copy link
Copy Markdown
Collaborator Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28334 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit a5e90e3ff4f46280b5773344cf89a2e888ba3b92, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17585	5192	5047	5047
q2	2133	309	289	289
q3	10200	1394	805	805
q4	4679	469	340	340
q5	7451	798	526	526
q6	177	186	156	156
q7	749	792	583	583
q8	9244	1342	1047	1047
q9	5467	4163	4171	4163
q10	6820	1314	1031	1031
q11	438	244	226	226
q12	634	408	285	285
q13	18092	3561	3047	3047
q14	274	262	247	247
q15	q16	721	737	672	672
q17	1226	753	575	575
q18	6964	6144	6078	6078
q19	1095	959	637	637
q20	373	330	229	229
q21	4932	2096	2244	2096
q22	345	294	255	255
Total cold run time: 99599 ms
Total hot run time: 28334 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5598	5568	5551	5551
q2	683	610	551	551
q3	5216	5378	4878	4878
q4	2003	2197	1546	1546
q5	5263	5122	5455	5122
q6	258	199	159	159
q7	2324	2095	1843	1843
q8	2997	2583	2626	2583
q9	8018	8006	7802	7802
q10	3762	3737	3301	3301
q11	622	433	411	411
q12	793	791	533	533
q13	3508	3835	3277	3277
q14	311	329	307	307
q15	q16	708	732	703	703
q17	8201	7547	6983	6983
q18	12973	12122	12810	12122
q19	892	762	755	755
q20	2162	2148	1920	1920
q21	5562	4584	4656	4584
q22	489	449	405	405
Total cold run time: 72343 ms
Total hot run time: 65336 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 152346 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit a5e90e3ff4f46280b5773344cf89a2e888ba3b92, data reload: false

query5	4313	605	448	448
query6	449	215	196	196
query7	4796	478	227	227
query8	319	169	161	161
query9	8759	3090	3114	3090
query10	465	319	253	253
query11	6040	3922	3626	3626
query12	149	95	89	89
query13	1254	434	343	343
query14	6151	4052	3754	3754
query14_1	3493	3471	3548	3471
query15	213	201	186	186
query16	961	430	426	426
query17	861	682	546	546
query18	2434	431	318	318
query19	207	165	135	135
query20	90	87	90	87
query21	216	134	116	116
query22	13350	13319	13131	13131
query23	13568	12954	12344	12344
query23_1	12621	12551	12568	12551
query24	7427	834	468	468
query24_1	475	481	487	481
query25	550	426	369	369
query26	1277	255	149	149
query27	2756	448	287	287
query28	4599	1969	1958	1958
query29	1588	593	454	454
query30	316	236	199	199
query31	1009	944	840	840
query32	146	96	94	94
query33	549	307	246	246
query34	901	851	512	512
query35	743	762	646	646
query36	819	791	713	713
query37	129	104	89	89
query38	1831	1762	1698	1698
query39	797	800	756	756
query39_1	737	737	766	737
query40	274	119	116	116
query41	87	81	81	81
query42	89	86	91	86
query43	360	366	329	329
query44	1299	690	691	690
query45	182	180	172	172
query46	840	984	576	576
query47	3066	2959	2717	2717
query48	286	310	213	213
query49	584	401	292	292
query50	671	272	217	217
query51	10584	10340	10536	10340
query52	82	82	71	71
query53	191	210	151	151
query54	233	193	175	175
query55	70	67	64	64
query56	243	226	218	218
query57	1680	1624	1578	1578
query58	292	274	254	254
query59	2118	2128	1940	1940
query60	299	243	230	230
query61	145	144	139	139
query62	392	333	291	291
query63	195	163	162	162
query64	2716	966	746	746
query65	3511	3350	3349	3349
query66	1786	414	299	299
query67	20604	20494	20256	20256
query68	2967	1005	613	613
query69	383	290	258	258
query70	849	793	782	782
query71	307	233	209	209
query72	2703	2520	2273	2273
query73	520	535	299	299
query74	4642	4536	4349	4349
query75	2360	2321	1925	1925
query76	2327	1032	600	600
query77	342	356	275	275
query78	10178	9995	9447	9447
query79	997	869	523	523
query80	1236	436	347	347
query81	590	337	286	286
query82	616	134	107	107
query83	300	200	189	189
query84	311	120	101	101
query85	860	416	356	356
query86	406	233	234	233
query87	2015	1982	1857	1857
query88	3690	2709	2668	2668
query89	348	281	242	242
query90	1916	175	174	174
query91	153	151	123	123
query92	96	90	90	90
query93	1040	1005	578	578
query94	657	310	282	282
query95	584	342	310	310
query96	672	484	233	233
query97	2437	2438	2330	2330
query98	164	153	150	150
query99	758	768	642	642
Total cold run time: 237464 ms
Total hot run time: 152346 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.34 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit a5e90e3ff4f46280b5773344cf89a2e888ba3b92, data reload: false

query1	0.00	0.00	0.01
query2	0.10	0.05	0.05
query3	0.26	0.09	0.09
query4	1.61	0.10	0.11
query5	0.24	0.24	0.24
query6	1.19	0.69	0.68
query7	0.04	0.00	0.01
query8	0.05	0.04	0.04
query9	0.39	0.35	0.36
query10	0.57	0.60	0.58
query11	0.20	0.14	0.14
query12	0.18	0.15	0.14
query13	0.50	0.49	0.49
query14	1.03	1.01	1.02
query15	0.64	0.59	0.60
query16	0.35	0.36	0.37
query17	1.15	1.16	1.19
query18	0.22	0.18	0.19
query19	2.11	2.02	1.96
query20	0.02	0.01	0.02
query21	15.49	0.19	0.14
query22	4.87	0.06	0.05
query23	16.29	0.34	0.12
query24	2.68	0.58	0.40
query25	0.09	0.07	0.06
query26	0.82	0.22	0.15
query27	0.05	0.05	0.04
query28	3.71	0.85	0.35
query29	12.51	4.05	3.28
query30	0.28	0.12	0.10
query31	2.77	0.55	0.32
query32	3.22	0.60	0.50
query33	3.10	3.29	3.18
query34	15.28	3.96	3.40
query35	3.33	3.38	3.31
query36	0.55	0.46	0.43
query37	0.10	0.06	0.07
query38	0.05	0.04	0.03
query39	0.03	0.03	0.03
query40	0.17	0.15	0.14
query41	0.09	0.03	0.02
query42	0.03	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 96.4 s
Total hot run time: 24.34 s

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 100% (0/0) 🎉
Increment coverage report
Complete coverage report

@OIiveirra

Copy link
Copy Markdown
Collaborator Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

yiguolei pushed a commit that referenced this pull request Oct 23, 2026
…#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
@yiguolei
yiguolei merged commit d035f09 into apache:master Oct 23, 2026
46 checks passed
yiguolei pushed a commit that referenced this pull request Oct 23, 2026
…#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants