[GitHub] [kafka] rondagostino commented on a change in pull request #11503: KAFKA-13456: controller.listener.names required in KRaft

[GitBox <gi...@apache.org>]

rondagostino commented on a change in pull request #11503:
URL: https://github.com/apache/kafka/pull/11503#discussion_r750751689



##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -2007,8 +2007,13 @@ class KafkaConfig private(doLog: Boolean, val props: java.util.Map[_, _], dynami
       "offsets.commit.required.acks must be greater or equal -1 and less or equal to offsets.topic.replication.factor")
     require(BrokerCompressionCodec.isValid(compressionType), "compression.type : " + compressionType + " is not valid." +
       " Valid options are " + BrokerCompressionCodec.brokerCompressionOptions.mkString(","))
-    require(!processRoles.contains(ControllerRole) || controllerListeners.nonEmpty,
-      s"${KafkaConfig.ControllerListenerNamesProp} cannot be empty if the server has the controller role")
+    if (usesSelfManagedQuorum) {

Review comment:
       I'm in agreement with not allowing `listeners` to contain the `controller.listener.names` values(s).  I'll rework tghe PR to do that.
   
   FYI, just as a sanity check, I confirmed that I was able to get a separate controller and broker working using our QuickStart files with the following changes:
   
   **controller.properties**
   ```
   listeners=SASL_PLAINTEXT://:9093
   listener.name.sasl_plaintext.oauthbearer.sasl.jaas.config=org.apache.kafka.common.security.oauthbearer.OAuthBearerLoginModule Required unsecuredLoginStringClaim_sub="broker";
   sasl.enabled.mechanisms=OAUTHBEARER
   sasl.mechanism.controller.protocol=OAUTHBEARER
   controller.listener.names=SASL_PLAINTEXT
   ```
   
   **broker.properties**
   ```
   listener.security.protocol.map=CONTROLLER:SASL_PLAINTEXT,PLAINTEXT:PLAINTEXT,SSL:SSL,SASL_PLAINTEXT:SASL_PLAINTEXT,SASL_SSL:SASL_SSL
   sasl.mechanism.controller.protocol=OAUTHBEARER
   listener.name.controller.oauthbearer.sasl.jaas.config=org.apache.kafka.common.security.oauthbearer.OAuthBearerLoginModule Required unsecuredLoginStringClaim_sub="broker";
   ```
   
   I was able to do this both with requiring the additional listener on the broker (the initial version of this PR) and not (the way the code works without this PR).
   
   So while the "listener" terminology is unfortunate in that the broker is not "listening" on it, I have confirmed that everything works as expected.
   
   I'll work on this more tomorrow.
   
   




-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org