affo commented on code in PR #2506: URL: https://github.com/apache/fluss/pull/2506#discussion_r2923336555
########## helm/templates/_security.tpl: ########## @@ -0,0 +1,173 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +{{/* +Returns the authentication mechanism value of a given listener. +Allowed mechanism values: '', 'plain' +Usage: + include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "client") +*/}} +{{- define "fluss.security.listener.mechanism" -}} +{{- $listener := index .context.security .listener | default (dict) -}} +{{- $sasl := $listener.sasl | default (dict) -}} +{{- $mechanism := lower (default "" $sasl.mechanism) -}} +{{- $mechanism -}} +{{- end -}} + +{{/* +Returns true if any of the listeners uses SASL based authentication mechanism ('plain' for now). +Usage: + include "fluss.security.sasl.enabled" . +*/}} +{{- define "fluss.security.sasl.enabled" -}} +{{- $internal := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "internal") -}} +{{- $client := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "client") -}} +{{- if or (ne $internal "") (ne $client "") -}}true{{- end -}} +{{- end -}} + +{{/* +Returns true if any of the listeners uses 'plain' authentication mechanism. +Usage: + include "fluss.security.sasl.plain.enabled" . +*/}} +{{- define "fluss.security.sasl.plain.enabled" -}} +{{- $internal := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "internal") -}} +{{- $client := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "client") -}} +{{- if or (eq $internal "plain") (eq $client "plain") -}}true{{- end -}} +{{- end -}} + +{{/* +Returns protocol value derived from listener mechanism. +Usage: + include "fluss.security.listener.protocol" (dict "context" .Values "listener" "internal") +*/}} +{{- define "fluss.security.listener.protocol" -}} +{{- $mechanism := include "fluss.security.listener.mechanism" (dict "context" .context "listener" .listener) -}} +{{- if eq $mechanism "" -}}PLAINTEXT{{- else -}}SASL{{- end -}} +{{- end -}} + +{{/* +Returns comma separated list of enabled mechanisms. +Usage: + include "fluss.security.sasl.enabledMechanisms" . +*/}} +{{- define "fluss.security.sasl.enabledMechanisms" -}} +{{- $mechanisms := list -}} +{{- range $listener := list "internal" "client" -}} + {{- $current := include "fluss.security.listener.mechanism" (dict "context" $.Values "listener" $listener) -}} + {{- if and (ne $current "") (not (has (upper $current) $mechanisms)) -}} + {{- $mechanisms = append $mechanisms (upper $current) -}} + {{- end -}} +{{- end -}} +{{- join "," $mechanisms -}} +{{- end -}} + +{{/* +Validates that SASL mechanisms are valid. +Returns an error message if invalid, empty string otherwise. +Usage: + include "fluss.security.sasl.validateMechanisms" . +*/}} +{{- define "fluss.security.sasl.validateMechanisms" -}} +{{- $allowedMechanisms := list "" "plain" -}} +{{- range $listener := list "internal" "client" -}} + {{- $listenerValues := index $.Values.security $listener | default (dict) -}} + {{- $sasl := $listenerValues.sasl | default (dict) -}} + {{- $mechanism := lower (default "" $sasl.mechanism) -}} + {{- if not (has $mechanism $allowedMechanisms) -}} + {{- printf "security.%s.sasl.mechanism must be one of: empty, plain" $listener -}} Review Comment: ```suggestion {{- printf "security.%s.sasl.mechanism must be empty or: plain" $listener -}} ``` ########## helm/templates/_security.tpl: ########## @@ -0,0 +1,173 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +{{/* +Returns the authentication mechanism value of a given listener. +Allowed mechanism values: '', 'plain' +Usage: + include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "client") +*/}} +{{- define "fluss.security.listener.mechanism" -}} +{{- $listener := index .context.security .listener | default (dict) -}} +{{- $sasl := $listener.sasl | default (dict) -}} +{{- $mechanism := lower (default "" $sasl.mechanism) -}} +{{- $mechanism -}} +{{- end -}} + +{{/* +Returns true if any of the listeners uses SASL based authentication mechanism ('plain' for now). +Usage: + include "fluss.security.sasl.enabled" . +*/}} +{{- define "fluss.security.sasl.enabled" -}} +{{- $internal := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "internal") -}} +{{- $client := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "client") -}} +{{- if or (ne $internal "") (ne $client "") -}}true{{- end -}} +{{- end -}} + +{{/* +Returns true if any of the listeners uses 'plain' authentication mechanism. +Usage: + include "fluss.security.sasl.plain.enabled" . +*/}} +{{- define "fluss.security.sasl.plain.enabled" -}} +{{- $internal := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "internal") -}} +{{- $client := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "client") -}} +{{- if or (eq $internal "plain") (eq $client "plain") -}}true{{- end -}} +{{- end -}} + +{{/* +Returns protocol value derived from listener mechanism. +Usage: + include "fluss.security.listener.protocol" (dict "context" .Values "listener" "internal") +*/}} +{{- define "fluss.security.listener.protocol" -}} +{{- $mechanism := include "fluss.security.listener.mechanism" (dict "context" .context "listener" .listener) -}} +{{- if eq $mechanism "" -}}PLAINTEXT{{- else -}}SASL{{- end -}} +{{- end -}} + +{{/* +Returns comma separated list of enabled mechanisms. +Usage: + include "fluss.security.sasl.enabledMechanisms" . +*/}} +{{- define "fluss.security.sasl.enabledMechanisms" -}} +{{- $mechanisms := list -}} +{{- range $listener := list "internal" "client" -}} + {{- $current := include "fluss.security.listener.mechanism" (dict "context" $.Values "listener" $listener) -}} + {{- if and (ne $current "") (not (has (upper $current) $mechanisms)) -}} + {{- $mechanisms = append $mechanisms (upper $current) -}} + {{- end -}} +{{- end -}} +{{- join "," $mechanisms -}} +{{- end -}} + +{{/* +Validates that SASL mechanisms are valid. +Returns an error message if invalid, empty string otherwise. +Usage: + include "fluss.security.sasl.validateMechanisms" . +*/}} +{{- define "fluss.security.sasl.validateMechanisms" -}} +{{- $allowedMechanisms := list "" "plain" -}} +{{- range $listener := list "internal" "client" -}} + {{- $listenerValues := index $.Values.security $listener | default (dict) -}} + {{- $sasl := $listenerValues.sasl | default (dict) -}} + {{- $mechanism := lower (default "" $sasl.mechanism) -}} + {{- if not (has $mechanism $allowedMechanisms) -}} + {{- printf "security.%s.sasl.mechanism must be one of: empty, plain" $listener -}} + {{- end -}} +{{- end -}} +{{- end -}} + +{{/* +Validates that the client PLAIN mechanism block contains the required users. +Returns an error message if invalid, empty string otherwise. +Usage: + include "fluss.security.sasl.validateClientPlainUsers" . +*/}} +{{- define "fluss.security.sasl.validateClientPlainUsers" -}} +{{- $clientMechanism := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "client") -}} +{{- if eq $clientMechanism "plain" -}} + {{- $users := .Values.security.client.sasl.plain.users | default (list) -}} + {{- if eq (len $users) 0 -}} + {{- print "security.client.sasl.plain.users must contain at least one user when security.client.sasl.mechanism is plain" -}} + {{- else -}} + {{- range $idx, $user := $users -}} + {{- if or (empty $user.username) (empty $user.password) -}} + {{- printf "security.client.sasl.plain.users[%d] must set both username and password" $idx -}} + {{- end -}} + {{- end -}} + {{- end -}} +{{- end -}} +{{- end -}} + +{{/* +Returns a warning if the internal SASL user is using default credentials. +Usage: + include "fluss.security.sasl.warnInternalUser" . +*/}} +{{- define "fluss.security.sasl.warnInternalUser" -}} +{{- if (include "fluss.security.sasl.enabled" .) -}} + {{- $internalMechanism := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "internal") -}} + {{- if eq $internalMechanism "plain" -}} + {{- if and (eq .Values.security.internal.sasl.plain.username "fluss-internal-user") (eq .Values.security.internal.sasl.plain.password "fluss-internal-password") -}} Review Comment: Maybe: ``` {{- define "fluss.security.sasl.plain.internal.defaultUsername" -}} {{- printf "fluss-internal-user-%s" .Release.Name -}} {{- end -}} {{- define "fluss.security.sasl.plain.internal.defaultPassword" -}} {{- printf "fluss-internal-password-%s" .Release.Name | sha256sum -}} {{- end -}} {{- define "fluss.security.sasl.plain.internal.username" -}} {{ .Values.security.internal.sasl.plain.username | default (include "fluss.security.sasl.plain.internal.defaultUsername" .) }} {{- end -}} {{- define "fluss.security.sasl.plain.internal.password" -}} {{ .Values.security.internal.sasl.plain.password | default (include "fluss.security.sasl.plain.internal.defaultPassword" .) }} {{- end -}} ``` Then you can simplify quite havily the check, as the defaults would be empty (you would also don't need to check for emptyness as if empty we use the default: ```suggestion {{- if and (not .Values.security.internal.sasl.plain.username) (not .Values.security.internal.sasl.plain.password) -}} ``` The more I think about this, the more I think we may introduce on top of this using an existing secret, but that should be part of another PR imho 🤝 ########## helm/values.yaml: ########## @@ -58,6 +58,23 @@ listeners: client: port: 9124 +# Fluss security configurations +security: + client: + sasl: + # "" | plain + mechanism: "" + plain: + users: [] + + internal: + sasl: + # "" | plain + mechanism: "" + plain: + username: "fluss-internal-user" + password: "fluss-internal-password" Review Comment: ```suggestion username: "" password: "" ``` As per the reasoning above this should now default to empty -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
