Copilot commented on code in PR #539:
URL: https://github.com/apache/tez/pull/539#discussion_r4046803964
##########
tez-dag/src/test/java/org/apache/tez/dag/history/utils/TestDAGUtils.java:
##########
@@ -222,4 +222,32 @@ public void testConvertDAGPlanToATSMap() throws
IOException, JSONException {
}
}
+ @Test
+ public void testConvertConfigurationToATSMapRedactsSecrets() {
+ Configuration conf = new Configuration(false);
Review Comment:
Using `new Configuration(false)` here means the test is not loading Hadoop
default resources (including defaults for
`hadoop.security.sensitive-config-keys`), so the redaction expectations can
become dependent on `ConfigRedactor` implementation details rather than the
configured defaults. Consider loading defaults (or explicitly setting the
sensitive-key pattern) so the test exercises the same behavior as production AM
configs.
##########
tez-dag/src/test/java/org/apache/tez/dag/history/utils/TestDAGUtils.java:
##########
@@ -222,4 +222,32 @@ public void testConvertDAGPlanToATSMap() throws
IOException, JSONException {
}
}
+ @Test
+ public void testConvertConfigurationToATSMapRedactsSecrets() {
+ Configuration conf = new Configuration(false);
+ // Non-sensitive properties must pass through unchanged.
+ conf.set("tez.am.dag.scheduler.class",
+ "org.apache.tez.dag.app.dag.impl.DAGSchedulerNaturalOrder");
+ conf.set("mapreduce.job.name", "normal-job");
+ // These keys are covered by Hadoop's default
+ // hadoop.security.sensitive-config-keys pattern.
+ conf.set("fs.s3a.secret.key", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYSECRET");
+ conf.set("fs.s3a.access.key", "AKIAIOSFODNN7EXAMPLE");
+ conf.set("ssl.server.keystore.password", "SuperSecretKeystorePass!");
+ conf.set("hadoop.security.credential.provider.password", "credpass");
+
+ Map<String, String> ats = DAGUtils.convertConfigurationToATSMap(conf);
+
+ assertEquals("org.apache.tez.dag.app.dag.impl.DAGSchedulerNaturalOrder",
+ ats.get("tez.am.dag.scheduler.class"));
+ assertEquals("normal-job", ats.get("mapreduce.job.name"));
+ assertFalse(ats.get("fs.s3a.secret.key").contains("SECRET"),
+ "s3a secret key must be redacted, got: " +
ats.get("fs.s3a.secret.key"));
+
assertFalse(ats.get("ssl.server.keystore.password").contains("SuperSecret"),
+ "keystore password must be redacted, got: " +
ats.get("ssl.server.keystore.password"));
+
assertFalse(ats.get("hadoop.security.credential.provider.password").contains("credpass"),
+ "credential provider password must be redacted, got: "
+ + ats.get("hadoop.security.credential.provider.password"));
Review Comment:
The test values for S3A credentials currently resemble real AWS key formats
(e.g., `AKIA...` and the well-known AWS secret-key example), which can trigger
secret-scanning/false-positive credential alerts and may violate repository
hygiene. Use clearly fake placeholders that still include the substrings you
assert against, and also assert that `fs.s3a.access.key` is redacted since it
is included in the test setup.
--
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]
