C0urante commented on code in PR #13375: URL: https://github.com/apache/kafka/pull/13375#discussion_r1144964514
########## core/src/test/java/kafka/testkit/KafkaClusterTestKit.java: ########## @@ -174,13 +174,17 @@ private KafkaConfig createNodeConfig(TestKitNode node) { props.put(KafkaConfig$.MODULE$.LogDirsProp(), String.join(",", brokerNode.logDataDirectories())); } - props.put(KafkaConfig$.MODULE$.ListenerSecurityProtocolMapProp(), - "EXTERNAL:PLAINTEXT,CONTROLLER:PLAINTEXT"); - props.put(KafkaConfig$.MODULE$.ListenersProp(), listeners(node.id())); - props.put(KafkaConfig$.MODULE$.InterBrokerListenerNameProp(), - nodes.interBrokerListenerName().value()); - props.put(KafkaConfig$.MODULE$.ControllerListenerNamesProp(), - "CONTROLLER"); + + // listeners could be defined via Builder::setConfigProp which shouldn't be overridden + if (!props.containsKey(KafkaConfig$.MODULE$.ListenersProp())) { Review Comment: I think removing the `if` guard entirely and just doing `putIfAbsent` would be the most intuitive behavior. If someone still misconfigures their cluster, that's on them; we don't have to try to do any preflight checks here. Then, if someone wants to enable SSL, they could just tweak the `listener.security.protocol.map` property and leave everything else as-is. -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org