Skip to content

Commit 3f911d0

Browse files
OIiveirraOIiveirra
authored
[fix](hudi) Keep storage credentials out of scanner logs (branch-4.1) (#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
1 parent f5905b5 commit 3f911d0

2 files changed

Lines changed: 139 additions & 1 deletion

File tree

‎fe/be-java-extensions/hadoop-hudi-scanner/src/main/java/org/apache/doris/hudi/HadoopHudiJniScanner.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,13 @@ public HadoopHudiJniScanner(int fetchSize, Map<String, String> params) {
121121
this.fieldInspectors = new ObjectInspector[requiredFields.length];
122122
this.structFields = new StructField[requiredFields.length];
123123
this.fsOptionsProps = Maps.newHashMap();
124+
LOG.debug("initialize hudi scanner for query {}", params.get("query_id"));
124125
for (Map.Entry<String, String> entry : params.entrySet()) {
125126
if (entry.getKey().startsWith(HADOOP_CONF_PREFIX)) {
126127
fsOptionsProps.put(entry.getKey().substring(HADOOP_CONF_PREFIX.length()), entry.getValue());
127128
}
128129
if (LOG.isDebugEnabled()) {
129-
LOG.debug("get hudi params {}: {}", entry.getKey(), entry.getValue());
130+
LOG.debug("get hudi parameter: {}", entry.getKey());
130131
}
131132
}
132133
this.preExecutionAuthenticator = PreExecutionAuthenticatorCache.getAuthenticator(fsOptionsProps);

‎fe/be-java-extensions/hadoop-hudi-scanner/src/test/java/org/apache/doris/hudi/HadoopHudiJniScannerLogTest.java‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.doris.hudi;
19+
20+
import org.apache.logging.log4j.Level;
21+
import org.apache.logging.log4j.LogManager;
22+
import org.apache.logging.log4j.core.LogEvent;
23+
import org.apache.logging.log4j.core.Logger;
24+
import org.apache.logging.log4j.core.appender.AbstractAppender;
25+
import org.apache.logging.log4j.core.config.Property;
26+
import org.junit.Assert;
27+
import org.junit.Test;
28+
import org.slf4j.LoggerFactory;
29+
30+
import java.lang.reflect.Field;
31+
import java.util.HashMap;
32+
import java.util.List;
33+
import java.util.Map;
34+
import java.util.concurrent.CopyOnWriteArrayList;
35+
36+
public class HadoopHudiJniScannerLogTest {
37+
@Test
38+
public void logsQueryIdAndParameterNamesWithoutCredentialValues() throws Exception {
39+
Map<String, String> hadoopProperties = new HashMap<>();
40+
hadoopProperties.put("fs.s3a.access.key", "synthetic-access-key");
41+
hadoopProperties.put("fs.s3a.secret.key", "synthetic-secret-key");
42+
hadoopProperties.put("fs.s3a.session.token", "synthetic-session-token");
43+
hadoopProperties.put("fs.s3a.bucket.test-bucket.access.key", "synthetic-bucket-access-key");
44+
hadoopProperties.put("fs.s3a.bucket.test-bucket.secret.key", "synthetic-bucket-secret-key");
45+
hadoopProperties.put("fs.s3a.bucket.test-bucket.session.token", "synthetic-bucket-session-token");
46+
hadoopProperties.put("custom.setting", "synthetic-custom-value");
47+
48+
Map<String, String> params = new HashMap<>();
49+
params.put("query_id", "0123456789abcdef-0123456789abcdef");
50+
params.put("base_path", "file:///unused");
51+
params.put("data_file_path", "file:///unused/data.parquet");
52+
params.put("data_file_length", "0");
53+
params.put("hudi_column_names", "id");
54+
params.put("hudi_column_types", "int");
55+
params.put("columns_types", "int");
56+
params.put("required_fields", "id");
57+
params.put("time_zone", "UTC");
58+
params.put("hoodie.custom.token", "synthetic-hudi-token");
59+
hadoopProperties.forEach((key, value) -> params.put("hadoop_conf." + key, value));
60+
61+
try (CapturedLog capture = new CapturedLog()) {
62+
Assert.assertTrue("The test must exercise DEBUG parameter logging",
63+
LoggerFactory.getLogger(HadoopHudiJniScanner.class).isDebugEnabled());
64+
HadoopHudiJniScanner scanner = new HadoopHudiJniScanner(16, params);
65+
String text = String.join("\n", capture.messages);
66+
Assert.assertTrue("Keep the query ID for diagnostics", text.contains(params.get("query_id")));
67+
for (String key : params.keySet()) {
68+
Assert.assertTrue("Missing parameter name: " + key, text.contains(key));
69+
}
70+
for (String value : hadoopProperties.values()) {
71+
Assert.assertFalse("Hadoop parameter values must not be logged", text.contains(value));
72+
}
73+
Assert.assertFalse("Non-Hadoop credentials must not be logged either",
74+
text.contains(params.get("hoodie.custom.token")));
75+
76+
Field properties = HadoopHudiJniScanner.class.getDeclaredField("fsOptionsProps");
77+
properties.setAccessible(true);
78+
Assert.assertEquals("Removing values from logs must preserve the actual Hadoop configuration",
79+
hadoopProperties, properties.get(scanner));
80+
}
81+
}
82+
83+
private static class CapturedLog implements AutoCloseable {
84+
private final List<String> messages = new CopyOnWriteArrayList<>();
85+
private final Logger logger = (Logger) LogManager.getLogger(HadoopHudiJniScanner.class);
86+
private final org.apache.log4j.Logger reload4j =
87+
org.apache.log4j.Logger.getLogger(HadoopHudiJniScanner.class);
88+
private final org.apache.log4j.Level previousReload4jLevel = reload4j.getLevel();
89+
private final boolean previousReload4jAdditive = reload4j.getAdditivity();
90+
private final org.apache.log4j.AppenderSkeleton reload4jAppender = new org.apache.log4j.AppenderSkeleton() {
91+
@Override
92+
protected void append(org.apache.log4j.spi.LoggingEvent event) {
93+
messages.add(event.getRenderedMessage());
94+
}
95+
96+
@Override
97+
public void close() {
98+
}
99+
100+
@Override
101+
public boolean requiresLayout() {
102+
return false;
103+
}
104+
};
105+
private final Level previousLevel = logger.getLevel();
106+
private final boolean previousAdditive = logger.isAdditive();
107+
private final AbstractAppender appender = new AbstractAppender("hudi-parameter-test", null,
108+
null, true, Property.EMPTY_ARRAY) {
109+
@Override
110+
public void append(LogEvent event) {
111+
messages.add(event.getMessage().getFormattedMessage());
112+
}
113+
};
114+
115+
CapturedLog() {
116+
reload4j.setLevel(org.apache.log4j.Level.DEBUG);
117+
reload4j.setAdditivity(false);
118+
reload4j.addAppender(reload4jAppender);
119+
appender.start();
120+
logger.setLevel(Level.DEBUG);
121+
logger.setAdditive(false);
122+
logger.addAppender(appender);
123+
}
124+
125+
@Override
126+
public void close() {
127+
logger.removeAppender(appender);
128+
logger.setLevel(previousLevel);
129+
logger.setAdditive(previousAdditive);
130+
appender.stop();
131+
reload4j.removeAppender(reload4jAppender);
132+
reload4j.setLevel(previousReload4jLevel);
133+
reload4j.setAdditivity(previousReload4jAdditive);
134+
reload4jAppender.close();
135+
}
136+
}
137+
}

0 commit comments

Comments
 (0)