lianetm commented on code in PR #18989:
URL: https://github.com/apache/kafka/pull/18989#discussion_r1969934129
##########
docs/ops.html:
##########
@@ -127,6 +127,8 @@ <h4 class="anchor-heading"><a id="basic_ops_consumer_group"
class="anchor-link">
topic2 0 460537 803290 342753
consumer1-3fc8d6f1-581a-4472-bdf3-3515b4aee8c1 /127.0.0.1 consumer1
topic3 2 243655 398812 155157
consumer4-117fe4d3-c6c1-4178-8ee9-eb4a3954bee0 /127.0.0.1
consumer4</code></pre>
+ Note that if the consumer group uses the consumer protocol, the admin client
needs the authorization to describe all the group's subscribed topics to
describe it. In contrast, the classic protocol does not require the topic
describe authorization.
Review Comment:
`needs the authorization to describe all the group's subscribed topics to
describe it` doesn't read clearly. Would it be better something like
`needs DESCRIBE access to all the topics used in the group (topics the
members are subscribed to)`
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java:
##########
@@ -16603,6 +16606,144 @@ foooTopicName, new TopicMetadata(foooTopicId,
foooTopicName, 1)
);
}
+ @Test
+ public void
testConsumerGroupMemberJoinsWithRegexWithTopicAuthorizationFailure() {
Review Comment:
Should we cover the recovery path too? (extending here or adding new one).
This test is ensuring that matching topics without auth are removed from the HB
response. But if the ACLs are added, the expectation is that when the regex is
refreshed, the hb response should include the topics that were excluded before
right?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
