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]

Reply via email to