fix(kraft): match controller listener by exact name, not string prefix - #306
Open
amuraru wants to merge 1 commit into
Open
fix(kraft): match controller listener by exact name, not string prefix#306amuraru wants to merge 1 commit into
amuraru wants to merge 1 commit into
Conversation
configureBrokerKRaftMode identified the controller listener among the "listeners" entries with `listener[:len(controllerListenerName)] == strings.ToUpper(controllerListenerName)`. This slice-prefix comparison has three failure modes, none guarded by validation: - Panic (slice out of range) when controllerListenerName is longer than a listener entry string (long controller name + short-named listener). - False match when one listener name is a prefix of another (e.g. "CONTROLLER" vs "CONTROLLER-EXTERNAL"): a broker-only node would silently drop the CONTROLLER-EXTERNAL listener. - When no controller listener is defined (controllerListenerName == ""), the empty prefix matched every entry, so broker-only nodes were assigned an empty `listeners=` (a broker listening nowhere). Replace it with isControllerListenerEntry, which compares the listener NAME segment (before "://") exactly and treats an empty controllerListenerName as "no match". For every valid KRaft config (an internal listener marked usedForControllerCommunication, distinct listener names) the output is unchanged. The two golden strings that changed belong to an intentionally invalid KRaft topology in TestGenerateBrokerConfigKRaftModeSSL (the "controller" listener is declared as an external listener, so no controller listener is identified). Their previous values encoded the buggy empty-prefix behavior; such configs will be rejected by the KRaft admission validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit ad8df0a)
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
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.
Split out of #300 (2/6).
configureBrokerKRaftModeidentified the controller listener among the"listeners" entries with
listener[:len(controllerListenerName)] == strings.ToUpper(controllerListenerName). This slice-prefix comparison hasthree failure modes, none guarded by validation:
controllerListenerNameis longer than alistener entry string (long controller name + short-named listener).
(e.g. "CONTROLLER" vs "CONTROLLER-EXTERNAL"): a broker-only node would
silently drop the CONTROLLER-EXTERNAL listener.
controllerListenerName == ""),the empty prefix matched every entry, so broker-only nodes were assigned
an empty
listeners=(a broker listening nowhere).Replaces it with
isControllerListenerEntry, which compares the listenerNAME segment (before "://") exactly and treats an empty
controllerListenerNameas "no match". For every valid KRaft config (aninternal listener marked
usedForControllerCommunication, distinctlistener names) the output is unchanged.
The two golden strings that changed belong to an intentionally invalid
KRaft topology in
TestGenerateBrokerConfigKRaftModeSSL(the "controller"listener is declared as an external listener, so no controller listener is
identified). Their previous values encoded the buggy empty-prefix behavior.