Copilot commented on code in PR #23197:
URL: https://github.com/apache/kafka/pull/23197#discussion_r4063168570
##########
test-common/test-common-runtime/src/main/java/org/apache/kafka/common/test/KafkaClusterTestKit.java:
##########
@@ -270,9 +270,13 @@ private boolean isPlainSaslMechanism(Map<String, Object>
props) {
}
private Optional<File> maybeSetupJaasFile() throws Exception {
+ // Skip the auto-generated PLAIN-only JAAS file when a test has
already configured its own
+ // JAAS login config (for example, a MiniKdc-based GSSAPI test
that also enables PLAIN),
+ // since overwriting it here would discard mechanisms other than
PLAIN.
if
((brokerSecurityProtocol.equals(SecurityProtocol.SASL_PLAINTEXT.name) ||
brokerSecurityProtocol.equals(SecurityProtocol.SASL_SSL.name)) &&
- isPlainSaslMechanism(configProps)) {
+ isPlainSaslMechanism(configProps) &&
+
System.getProperty(org.apache.kafka.common.security.JaasUtils.JAVA_LOGIN_CONFIG_PARAM)
== null) {
Review Comment:
This treats every non-null login-config property as test-owned, but this
builder itself leaves that property set after deleting its generated JAAS file
in `close()`. Consequently, after one ordinary PLAIN cluster closes, a later
PLAIN/SASL cluster in the same JVM can skip JAAS generation because of the
stale property and then use a path to a deleted file (especially after any
`Configuration.setConfiguration(null)` reload). Please distinguish an
externally supplied JAAS setting from the builder-owned setting, or
clear/restore the builder-owned property when the cluster closes.
--
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]