codope commented on code in PR #8027:
URL: https://github.com/apache/hudi/pull/8027#discussion_r1116542520
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -620,6 +620,12 @@ public class HoodieWriteConfig extends HoodieConfig {
.withDocumentation("Whether to enable commit conflict checking or not
during early "
+ "conflict detection.");
+ public static final ConfigProperty<String> SENSITIVE_CONFIG_KEYS_FILTER =
ConfigProperty
+ .key("hoodie.sensitive.config.keys")
+ .defaultValue("ssl,tls,sasl,auth,credentials")
+ .withDocumentation("Comma separated list of filters for sensitive config
keys. Delta Streamer "
+ + "avoids printing any configurations which contains the configured
filter.");
Review Comment:
Instead of saying `avoids`, be more explicit and say `... will not print any
configuration...`.
Also, you could add an example.
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/HoodieDeltaStreamer.java:
##########
@@ -528,7 +533,10 @@ public String toString() {
}
}
- private static String toSortedTruncatedString(TypedProperties props) {
+ static String toSortedTruncatedString(TypedProperties props) {
+ List<String> sensitiveConfigList =
props.getStringList(HoodieWriteConfig.SENSITIVE_CONFIG_KEYS_FILTER.key(),
Review Comment:
Why can't it be a Set? Not that it matters for few handful of elements, but
Set feels more intuitive.
##########
hudi-utilities/src/test/java/org/apache/hudi/utilities/deltastreamer/TestHoodieDeltaStreamer.java:
##########
@@ -2319,6 +2319,22 @@ public void testDeletePartitions() throws Exception {
TestHelpers.assertNoPartitionMatch(tableBasePath, sqlContext,
HoodieTestDataGenerator.DEFAULT_FIRST_PARTITION_PATH);
}
+ @Test
+ public void testToSortedTruncatedStringSecretsMasked() {
+ TypedProperties props =
+ new DFSPropertiesConfiguration(fs.getConf(), new Path(basePath + "/" +
PROPS_FILENAME_TEST_SOURCE)).getProps();
+ props.put("ssl.trustore.location", "SSL SECRET KEY");
+ props.put("sasl.jaas.config", "SASL SECRET KEY");
+ props.put("auth.credentials", "AUTH CREDENTIALS");
+ props.put("auth.user.info", "AUTH USER INFO");
+
+ String truncatedKeys = HoodieDeltaStreamer.toSortedTruncatedString(props);
+ assertFalse(truncatedKeys.contains("SSL SECRET KEY"));
Review Comment:
Shouldn't the assertion be that truncatedKeys contains
`SENSITIVE_INFO_MASKED`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
