[GitHub] [kafka] rondagostino commented on a change in pull request #10694: MINOR: fix system test TestSecurityRollingUpgrade

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

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



##########
File path: tests/kafkatest/tests/core/security_rolling_upgrade_test.py
##########
@@ -89,18 +89,21 @@ def add_sasl_mechanism(self, new_client_sasl_mechanism):
         self.bounce()
 
     def roll_in_sasl_mechanism(self, security_protocol, new_sasl_mechanism):
-        # Roll cluster to update inter-broker SASL mechanism. This disables the old mechanism.
+        # Roll cluster to update inter-broker SASL mechanism.
+        # We need the inter-broker SASL mechanism to still be enabled through this roll.
+        self.kafka.client_sasl_mechanism = "%s,%s" % (self.kafka.interbroker_sasl_mechanism, new_sasl_mechanism)
         self.kafka.interbroker_sasl_mechanism = new_sasl_mechanism
         self.bounce()
 
-        # Bounce again with ACLs for new mechanism.
+        # Bounce again with ACLs for new mechanism. Use old SimpleAclAuthorizer here to ensure that is also tested.
+        self.kafka.client_sasl_mechanism = new_sasl_mechanism # Removes old SASL mechanism completely
         self.set_authorizer_and_bounce(security_protocol, security_protocol)
 
     def add_separate_broker_listener(self, broker_security_protocol, broker_sasl_mechanism):
         # Enable the new internal listener on all brokers first
         self.kafka.open_port(self.kafka.INTERBROKER_LISTENER_NAME)
         self.kafka.port_mappings[self.kafka.INTERBROKER_LISTENER_NAME].security_protocol = broker_security_protocol
-        self.kafka.client_sasl_mechanism = broker_sasl_mechanism
+        self.kafka.port_mappings[self.kafka.INTERBROKER_LISTENER_NAME].sasl_mechanism = broker_sasl_mechanism

Review comment:
       We can't rely on the client SASL mechanism here with the KRaft-related changes that were made in https://github.com/apache/kafka/pull/10199/ because those KRaft changes made the code very explicit about which SASL mechanisms to add: if the client security protocol is PLAINTEXT then the client SASL mechanism isn't listed as an enabled mechanism.  And in fact for this test the client security protocol is PLAINTEXT.  So we need another way to transmit the SASL mechanism, and that's what the new optional `sas_mechanism` on the KafkaListener in the port mapping is for.




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