Re: [PR] [helm] Enable SASL authenticated connection to Zookeeper nodes [fluss]

["affo (via GitHub)" <gi...@apache.org>]

affo commented on code in PR #2700:
URL: https://github.com/apache/fluss/pull/2700#discussion_r2989278228


##########
website/docs/install-deploy/deploying-with-helm.md:
##########
@@ -193,6 +193,15 @@ The following table lists the configurable parameters of the Fluss chart, and th
 | `security.internal.sasl.plain.username` | Internal listener PLAIN username | `""` |
 | `security.internal.sasl.plain.password` | Internal listener PLAIN password | `""` |
 
+#### ZooKeeper SASL Parameters

Review Comment:
   I think this is going in-between a previous section breaking it 🤔 



##########
website/docs/install-deploy/deploying-with-helm.md:
##########
@@ -314,6 +323,23 @@ security:
         password: internal-password
 ```
 
+### Enabling ZooKeeper SASL Authentication
+
+You can enable ZooKeeper ensemble SASL authentication, with the following values in the Fluss Helm chart:
+
+```yaml
+security:
+  zookeeper:
+    sasl:
+      enabled: true
+      username: fluss-zk-user
+      password: fluss-zk-password
+```
+
+If the username or password is left empty, the chart automatically generates credentials based on the Helm release name. It is recommended to set these explicitly in production.

Review Comment:
   we should not generate those credentials, but simply fail.
   Fluss does not manage ZK.



##########
helm/tests/security_test.yaml:
##########
@@ -282,3 +282,161 @@ tests:
       - matchRegex:
           path: spec.template.spec.containers[0].command[2]
           pattern: '>> \$FLUSS_HOME/conf/server\.yaml'
+

Review Comment:
   Should we add tests to check that the zookeper SASL is not enabled when it is not enabled?



##########
helm/templates/_security.tpl:
##########
@@ -118,39 +118,84 @@ Usage:
 {{- end -}}
 
 {{/*
-Returns the default internal SASL username based on the release name.
+Validates that ZooKeeper SASL loginModuleClass is not empty when ZK SASL is enabled.
+Returns an error message if invalid, empty string otherwise.
+Usage:
+  include "fluss.security.sasl.validateZookeeperLoginModuleClass" .
+*/}}
+{{- define "fluss.security.sasl.validateZookeeperLoginModuleClass" -}}
+{{- if and .Values.security.zookeeper.sasl.enabled (not .Values.security.zookeeper.sasl.loginModuleClass) -}}
+  {{- print "security.zookeeper.sasl.loginModuleClass must not be empty when security.zookeeper.sasl.enabled is true" -}}
+{{- end -}}
+{{- end -}}
+
+{{/*
+Returns a default SASL username based on a given prefix and the release name.
 Usage:
-  include "fluss.security.sasl.plain.internal.defaultUsername" .
+  include "fluss.security.sasl.defaultUsername" (dict "prefix" "fluss-internal" "Release" .Release)
 */}}
-{{- define "fluss.security.sasl.plain.internal.defaultUsername" -}}
-{{- printf "fluss-internal-user-%s" .Release.Name -}}
+{{- define "fluss.security.sasl.defaultUsername" -}}
+{{- printf "%s-user-%s" .prefix .Release.Name -}}
 {{- end -}}
 
 {{/*
-Returns the default internal SASL password based on the release name (sha256 hashed).
+Returns a default SASL password based on a given prefix and the release name (sha256 hashed).
 Usage:
-  include "fluss.security.sasl.plain.internal.defaultPassword" .
+  include "fluss.security.sasl.defaultPassword" (dict "prefix" "fluss-internal" "Release" .Release)
 */}}
-{{- define "fluss.security.sasl.plain.internal.defaultPassword" -}}
-{{- printf "fluss-internal-password-%s" .Release.Name | sha256sum -}}
+{{- define "fluss.security.sasl.defaultPassword" -}}
+{{- printf "%s-password-%s" .prefix .Release.Name | sha256sum -}}
 {{- end -}}
 
 {{/*
-Returns the resolved internal SASL username (user-provided or auto-generated default).
+Returns the resolved internal SASL username.
+It generates internal username if user provided is empty.
 Usage:
   include "fluss.security.sasl.plain.internal.username" .
 */}}
 {{- define "fluss.security.sasl.plain.internal.username" -}}
-{{- .Values.security.internal.sasl.plain.username | default (include "fluss.security.sasl.plain.internal.defaultUsername" .) -}}
+{{- .Values.security.internal.sasl.plain.username | default (include "fluss.security.sasl.defaultUsername" (dict "prefix" "fluss-internal" "Release" .Release)) -}}
 {{- end -}}
 
 {{/*
-Returns the resolved internal SASL password (user-provided or auto-generated default).
+Returns the resolved internal SASL password.
+It generates internal password if user provided is empty.
 Usage:
   include "fluss.security.sasl.plain.internal.password" .
 */}}
 {{- define "fluss.security.sasl.plain.internal.password" -}}
-{{- .Values.security.internal.sasl.plain.password | default (include "fluss.security.sasl.plain.internal.defaultPassword" .) -}}
+{{- .Values.security.internal.sasl.plain.password | default (include "fluss.security.sasl.defaultPassword" (dict "prefix" "fluss-internal" "Release" .Release)) -}}
+{{- end -}}
+
+{{/*
+Returns the resolved ZooKeeper SASL username.
+It generates Zookeeper username if user provided is empty.
+Usage:
+  include "fluss.security.zookeeper.sasl.username" .
+*/}}
+{{- define "fluss.security.zookeeper.sasl.username" -}}
+{{- .Values.security.zookeeper.sasl.username | default (include "fluss.security.sasl.defaultUsername" (dict "prefix" "fluss-zookeeper" "Release" .Release)) -}}

Review Comment:
   no need for username/password generation here.
   We are not the owner of the ZK server



##########
helm/templates/_security.tpl:
##########
@@ -118,39 +118,84 @@ Usage:
 {{- end -}}
 
 {{/*
-Returns the default internal SASL username based on the release name.
+Validates that ZooKeeper SASL loginModuleClass is not empty when ZK SASL is enabled.
+Returns an error message if invalid, empty string otherwise.
+Usage:
+  include "fluss.security.sasl.validateZookeeperLoginModuleClass" .
+*/}}
+{{- define "fluss.security.sasl.validateZookeeperLoginModuleClass" -}}
+{{- if and .Values.security.zookeeper.sasl.enabled (not .Values.security.zookeeper.sasl.loginModuleClass) -}}
+  {{- print "security.zookeeper.sasl.loginModuleClass must not be empty when security.zookeeper.sasl.enabled is true" -}}
+{{- end -}}
+{{- end -}}
+
+{{/*
+Returns a default SASL username based on a given prefix and the release name.
 Usage:
-  include "fluss.security.sasl.plain.internal.defaultUsername" .
+  include "fluss.security.sasl.defaultUsername" (dict "prefix" "fluss-internal" "Release" .Release)
 */}}
-{{- define "fluss.security.sasl.plain.internal.defaultUsername" -}}
-{{- printf "fluss-internal-user-%s" .Release.Name -}}
+{{- define "fluss.security.sasl.defaultUsername" -}}
+{{- printf "%s-user-%s" .prefix .Release.Name -}}
 {{- end -}}
 
 {{/*
-Returns the default internal SASL password based on the release name (sha256 hashed).
+Returns a default SASL password based on a given prefix and the release name (sha256 hashed).
 Usage:
-  include "fluss.security.sasl.plain.internal.defaultPassword" .
+  include "fluss.security.sasl.defaultPassword" (dict "prefix" "fluss-internal" "Release" .Release)
 */}}
-{{- define "fluss.security.sasl.plain.internal.defaultPassword" -}}
-{{- printf "fluss-internal-password-%s" .Release.Name | sha256sum -}}
+{{- define "fluss.security.sasl.defaultPassword" -}}
+{{- printf "%s-password-%s" .prefix .Release.Name | sha256sum -}}
 {{- end -}}
 
 {{/*
-Returns the resolved internal SASL username (user-provided or auto-generated default).
+Returns the resolved internal SASL username.
+It generates internal username if user provided is empty.
 Usage:
   include "fluss.security.sasl.plain.internal.username" .
 */}}
 {{- define "fluss.security.sasl.plain.internal.username" -}}
-{{- .Values.security.internal.sasl.plain.username | default (include "fluss.security.sasl.plain.internal.defaultUsername" .) -}}
+{{- .Values.security.internal.sasl.plain.username | default (include "fluss.security.sasl.defaultUsername" (dict "prefix" "fluss-internal" "Release" .Release)) -}}
 {{- end -}}
 
 {{/*
-Returns the resolved internal SASL password (user-provided or auto-generated default).
+Returns the resolved internal SASL password.
+It generates internal password if user provided is empty.
 Usage:
   include "fluss.security.sasl.plain.internal.password" .
 */}}
 {{- define "fluss.security.sasl.plain.internal.password" -}}
-{{- .Values.security.internal.sasl.plain.password | default (include "fluss.security.sasl.plain.internal.defaultPassword" .) -}}
+{{- .Values.security.internal.sasl.plain.password | default (include "fluss.security.sasl.defaultPassword" (dict "prefix" "fluss-internal" "Release" .Release)) -}}
+{{- end -}}
+
+{{/*
+Returns the resolved ZooKeeper SASL username.
+It generates Zookeeper username if user provided is empty.
+Usage:
+  include "fluss.security.zookeeper.sasl.username" .
+*/}}
+{{- define "fluss.security.zookeeper.sasl.username" -}}
+{{- .Values.security.zookeeper.sasl.username | default (include "fluss.security.sasl.defaultUsername" (dict "prefix" "fluss-zookeeper" "Release" .Release)) -}}
+{{- end -}}
+
+{{/*
+Returns the resolved ZooKeeper SASL password.
+It generates Zookeeper password if user provided is empty.
+Usage:
+  include "fluss.security.zookeeper.sasl.password" .
+*/}}
+{{- define "fluss.security.zookeeper.sasl.password" -}}
+{{- .Values.security.zookeeper.sasl.password | default (include "fluss.security.sasl.defaultPassword" (dict "prefix" "fluss-zookeeper" "Release" .Release)) -}}

Review Comment:
   no need for username/password generation here.
   We are not the owner of the ZK server



##########
helm/templates/sts-tablet.yaml:
##########
@@ -115,7 +115,7 @@ spec:
         - name: data
           emptyDir: {}
         {{- end }}
-        {{- if (include "fluss.security.sasl.plain.enabled" .) }}
+        {{- if (include "fluss.security.jaas.required" .) }}
         - name: sasl-config
           secret:
             secretName: {{ include "fluss.security.sasl.configName" . }}

Review Comment:
   should this be rename to `fluss.security.jaas.configName`?



-- 
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: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org