MINOR: Do not advertise describe APIs on the controller listener - #23237
Open
stasimus wants to merge 1 commit into
Open
MINOR: Do not advertise describe APIs on the controller listener#23237stasimus wants to merge 1 commit into
stasimus wants to merge 1 commit into
Conversation
DescribeDelegationToken and DescribeUserScramCredentials both declared the controller listener, so the controller advertised them in its ApiVersions response. ControllerApis.handle has no case for either, so they fell through to the "Unsupported ApiKey" branch and clients got UNKNOWN_SERVER_ERROR for an API the broker said it supported. Both were added along with their KRaft features (KAFKA-15219 and KAFKA-14084), which wired up the mutating APIs but not the describes. Drop the controller listener so the advertised set matches what is implemented.
github-actions
Bot
added
triage
Author
|
changes to the wire protocol "generally require a KIP", and this does change what the controller returns in its ApiVersions response - I take it a bugfix rather than a protocol change, since the API never worked on the controller and no functioning client behaviour change |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DescribeDelegationToken and DescribeUserScramCredentials declare the controller listener, so the controller advertises them in ApiVersions, but
ControllerApis.handlehas no case for either and clients get UNKNOWN_SERVER_ERROR for an API that was just advertised.Dropped the controller listener so the advertised set matches what is implemented. Adding handlers instead would be new functionality. Happy to route this through a KIP if the ApiVersions change counts as a protocol change.
Both were introduced with their KRaft features (KAFKA-15219, KAFKA-14084), which wired up the mutating APIs but not the describes. Added two regression tests in
ControllerApisTest.