Copilot commented on code in PR #4401:
URL: https://github.com/apache/iggy/pull/4401#discussion_r4180031493
##########
gateways/kafka/src/group/state.rs:
##########
@@ -708,6 +819,26 @@ fn join_request_error(config: &GroupCoordinatorConfig,
request: &JoinRequest) ->
None
}
+/// `protocol_type`, a client-supplied `member_id`, and `group_instance_id`
are echoed as legacy
+/// strings. When the member id is generated, the instance id is the prefix
and the `-{uuid}`
+/// suffix has to fit in the same cap.
+fn legacy_strings_too_long(request: &JoinRequest) -> bool {
+ if request.protocol_type.len() > MAX_LEGACY_STRING_BYTES
+ || request.member_id.len() > MAX_LEGACY_STRING_BYTES
+ {
Review Comment:
Protocol names also need this legacy-string cap. JoinGroup v6+ can decode a
compact protocol name longer than `i16::MAX`, and the 64 KiB member-blob limit
still admits such a name. If that name is selected, DescribeGroups v0-v4 writes
it as `protocol_data` using a legacy string, so encoding fails and the
connection closes for an otherwise valid stored group. Reject overlong protocol
names here as well.
##########
gateways/kafka/tests/consumer_group_tests.rs:
##########
@@ -1416,6 +1432,493 @@ async fn
given_a_transaction_key_type_when_finding_the_coordinator_should_return
assert_eq!(decoder.remaining(), 0);
}
+// ── DescribeGroups / ListGroups ─────────────────────────────────────────────
+
+async fn describe(state: &GatewayState, version: i16, groups: &[&str]) ->
DescribeResponse {
+ let body = build_describe_groups_request(version, groups, false);
+ let response = handle_request_bounded(state, API_KEY_DESCRIBE_GROUPS,
version, body)
+ .await
+ .expect_response("DescribeGroups must answer");
+ DescribeResponse::decode(version, response)
Review Comment:
The linked issue requires an integration test with a running consumer group
and verification through Kafka admin tooling, but all new
DescribeGroups/ListGroups assertions call `handle_request_bounded` directly.
This bypasses TCP dispatch, response headers, and the
`kafka-consumer-groups.sh` compatibility that motivates the change. Add an
end-to-end test that creates a live group through the listener and
inspects/lists it through the admin client (or the repository's equivalent
real-wire harness).
--
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]
