Skip to content

fix: send null instead of empty string when describing default client quotas#3128

Merged
dnwe merged 3 commits intoIBM:mainfrom
petedannemann:fix/describe-default-quota
Mar 19, 2025
Merged

fix: send null instead of empty string when describing default client quotas#3128
dnwe merged 3 commits intoIBM:mainfrom
petedannemann:fix/describe-default-quota

Conversation

@petedannemann
Copy link
Copy Markdown
Contributor

@petedannemann petedannemann commented Mar 18, 2025

DescribeClientQuota currently fails with the following error message logged by the Kafka broker when trying to describe default client quotas:

org.apache.kafka.common.errors.InvalidRequestException: Request specified MATCH_TYPE_DEFAULT, but also specified a match string

This is because we are currently sending an empty string ("") instead of a null as the match string

The Java client simply ignores the match string here to avoid this issue, so I propose we do the same https://github.com/apache/kafka/blob/3ed590288fd773902f7791959e8f081ff937c144/clients/src/main/java/org/apache/kafka/common/requests/DescribeClientQuotasRequest.java#L54-L55

@petedannemann petedannemann changed the title fix: describe default client quotas fix: send null instead of empty string when describing default client quotas Mar 18, 2025
Signed-off-by: Peter Dannemann <[email protected]>
Copy link
Copy Markdown
Collaborator

@dnwe dnwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This does indeed look like a bug in the original implementation, thanks for the fix

Signed-off-by: Peter Dannemann <[email protected]>
@petedannemann petedannemann force-pushed the fix/describe-default-quota branch from 39f5433 to fa7d527 Compare March 19, 2025 19:43
@petedannemann
Copy link
Copy Markdown
Contributor Author

Thanks for the review, my mistake for missing the unit test there, I think I may have only ran the functional tests on my machine

@dnwe dnwe added the fix label Mar 19, 2025
@dnwe dnwe merged commit 86a72ef into IBM:main Mar 19, 2025
16 checks passed
@petedannemann
Copy link
Copy Markdown
Contributor Author

petedannemann commented Mar 20, 2025

@dnwe curious if you have a date in mind for the next Sarama release? I was hoping to leverage this feature here Mongey/terraform-provider-kafka#527

EDIT: nevermind, that maintainer was fine with using an untagged version of sarama

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants