[bitnami/kafka] Allow setting ssl.client.auth separately for inter-broker, controller, and client listeners #43135
Conversation
… listeners Signed-off-by: Robin <hi@rablet.dev>
Rablet
changed the title
| kafka_server_conf_set "listener.name.${listener_lower}.ssl.client.auth" "$KAFKA_TLS_INTER_BROKER_AUTH" | ||
| if [[ "$listener" = "${KAFKA_CFG_INTER_BROKER_LISTENER_NAME:-INTERNAL}" ]]; then | ||
| kafka_server_conf_set "listener.name.${listener_lower}.ssl.client.auth" "$KAFKA_TLS_INTER_BROKER_AUTH" | ||
| elif [[ "${KAFKA_CFG_CONTROLLER_LISTENER_NAMES:-CONTROLLER}" =~ $listener ]]; then |
There was a problem hiding this comment.
Please use the same the syntax than in the first if
| elif [[ "${KAFKA_CFG_CONTROLLER_LISTENER_NAMES:-CONTROLLER}" =~ $listener ]]; then | |
| elif [[ "$listener" = "${KAFKA_CFG_CONTROLLER_LISTENER_NAMES:-CONTROLLER}" ]]; then |
There was a problem hiding this comment.
Please use the same the syntax than in the first if
I used that format in order to be consistent with how the controller listener is detected on line 995:
elif [[ "${KAFKA_CFG_CONTROLLER_LISTENER_NAMES:-CONTROLLER}" =~ $listener ]]; then
My assumption is it was done that way because the config is a comma separated list rather than just a single name.
In light of that, would you still like me to change it?
|
@jotamartos just checking in to see if you're happy with the PR as-is based on my last comment or if you would still like me to change it. Thank you! |
|
Looks like this was implemented in a different way recently: |
Description of the change
The kafka container already has different settings for client auth for client, inter-broker, and controller listeners but it currently uses the same setting for all listeners.
This change allows users to set it separately for inter-broker, controller, and client listeners.
Benefits
This allows ssl client auth for different categories of listeners
Possible drawbacks
An even better change might be to make it configurable per listener (for example, you might have a CLIENT listener which doesn't need it, and an EXTERNAL listener which does need it). This change keeps it simple by retaining the existing settings.
Applicable issues
N/A
Additional information
N/A