rajinisivaram commented on a change in pull request #9378:
URL: https://github.com/apache/kafka/pull/9378#discussion_r500981702
##########
File path: tests/kafkatest/services/security/kafka_acls.py
##########
@@ -93,11 +97,13 @@ def add_cluster_acl(self, kafka, principal,
force_use_zk_connection=False):
force_use_zk_connection = force_use_zk_connection or not
kafka.all_nodes_acl_command_supports_bootstrap_server()
- cmd = "%(cmd_prefix)s --add --cluster --operation=ClusterAction
--allow-principal=%(principal)s" % {
- 'cmd_prefix': self._acl_cmd_prefix(kafka, node,
force_use_zk_connection),
- 'principal': principal
- }
- kafka.run_cli_tool(node, cmd)
+ for operation in ['ClusterAction', 'Alter', 'Create']:
Review comment:
I guess the problem is we don't have a separate principal in tests that
we can assign those to. Could we separate out `Create` and `Alter` into another
method, so that tests can set them as necessary or would that impact every
test? We expect brokers to have only `ClusterAction`, but our docs and
generally everyone expects broker to have more permissions. This sort of makes
our tests run with a combination of permissions that we never expect a
principal to have - broker principal should have only ClusterAction, no one
else should have ClusterAction. Separating out into two methods may be better
even if we end up using the same principal in tests.
----------------------------------------------------------------
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:
us...@infra.apache.org
