MINOR: system tests - avoid 'sasl.enabled.mechanisms' in listener overrides#7018
MINOR: system tests - avoid 'sasl.enabled.mechanisms' in listener overrides#7018rajinisivaram merged 2 commits intoapache:trunkfrom
Conversation
|
|
||
| {% for k, v in listener_security_config.client_listener_overrides.iteritems() %} | ||
| {% if k.startswith('sasl.') %} | ||
| {% if k.startswith('sasl.') and k != 'sasl.enabled.mechanisms' %} |
There was a problem hiding this comment.
It may be better to check the list of configs that actually get prefixed with mechanism. I think only a small subset of SASL configs are actually prefixed with mechanism. The config docs for these indicate if they are mechanism-prefixed.
There was a problem hiding this comment.
docs say this:
prefixed with listener.<name>.:
connections.max.reauth.ms- all
ssl...configs
prefixed with listener.<name>.<sasl-mechanism>.:
connections.max.reauth.mssasl.jaas.configsasl.login.callback.handler.classsasl.login.classsasl.server.callback.handler.class
also from the docs it seems to imply that sasl.enabled.mechanisms should instead be at the global-level, not per listener... does that sound correct? @rajinisivaram
There was a problem hiding this comment.
I have tried running with the global sasl.enabled.mechanisms instead of per-listener and it seems to not work for me...
maybe we need to update the kafka docs?
There was a problem hiding this comment.
All listener configs can be prefixed with listener prefix (listener.name.{name}.). So docs probably dont explicitly say this. Only a few SASL configs can be prefixed with mechanism. I think the docs do explicitly state this and the five you listed above look correct. All listener-prefixed configs can also be at global level, but listener-prefixed ones have precedence. So you can't override listener configs with global configs.
There was a problem hiding this comment.
I've added a check that looks for each of these 5 configs specifically.
|
I just kicked off a branch builder job to ensure tests aren't broken with this |
rajinisivaram
left a comment
rajinisivaram
left a comment
There was a problem hiding this comment.
@brianbushree Thanks for the PR, LGTM
|
Checked the system test results. The only failures were the 12 ConnectDistributed tests, which were fixed under #7023. Merging to trunk. |
…rrides (apache#7018) Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
|
thanks @rajinisivaram ! |
2 participants
named listener config
sasl.enabled.mechanismsshould not be prefixed with sasl mechanismCommitter Checklist (excluded from commit message)