[GitHub] [kafka] cmccabe commented on a change in pull request #10550: MINOR: Add support for ZK Authorizer with KRaft

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

cmccabe commented on a change in pull request #10550:
URL: https://github.com/apache/kafka/pull/10550#discussion_r630598753



##########
File path: tests/kafkatest/services/kafka/kafka.py
##########
@@ -863,7 +878,7 @@ def kafka_configs_cmd_with_optional_security_settings(self, node, force_use_zk_c
                 # configure JAAS to provide the typical client credentials
                 jaas_conf_prop = KafkaService.JAAS_CONF_PROPERTY
                 use_inter_broker_mechanism_for_client = False
-            optional_jass_krb_system_props_prefix = "KAFKA_OPTS='-D%s -D%s' " % (jaas_conf_prop, KafkaService.KRB5_CONF)
+            optional_jass_krb_system_props_prefix = "KAFKA_OPTS='-D%s -D%s' " % (jaas_conf_prop, KafkaService.KRB5_CONF) if security_protocol_to_use != "SSL" else ""

Review comment:
       I'm confused by the logic here.  If we have security_protocol==SSL then we do not define the jaas properties in KAFKA_OPTS?  Seems a bit weird -- why define this when we're using PLAINTEXT or when we're using SASL_SSL, but not when using SSL?  Can you add a comment about how this works?




-- 
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.

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