Skip to content

Commit faf28e0

Browse files
morazowmorazow
committed
Address reviews suggestions.
Changes: - Add ZK negative tests - Don't auto generate ZK username and password - Rename sasl.configName to jaas.configName - Documentation updates - Refactor to previous methods, instead of generic methods
1 parent 004552e commit faf28e0

6 files changed

Lines changed: 119 additions & 64 deletions

File tree

‎helm/templates/_security.tpl‎

Lines changed: 34 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -130,61 +130,63 @@ Usage:
130130
{{- end -}}
131131

132132
{{/*
133-
Returns a default SASL username based on a given prefix and the release name.
133+
Validates that ZooKeeper SASL username is not empty when ZK SASL is enabled.
134+
Returns an error message if invalid, empty string otherwise.
134135
Usage:
135-
include "fluss.security.sasl.defaultUsername" (dict "prefix" "fluss-internal" "Release" .Release)
136+
include "fluss.security.sasl.validateZookeeperUsername" .
136137
*/}}
137-
{{- define "fluss.security.sasl.defaultUsername" -}}
138-
{{- printf "%s-user-%s" .prefix .Release.Name -}}
138+
{{- define "fluss.security.sasl.validateZookeeperUsername" -}}
139+
{{- if and .Values.security.zookeeper.sasl.enabled (not .Values.security.zookeeper.sasl.username) -}}
140+
{{- print "security.zookeeper.sasl.username must not be empty when security.zookeeper.sasl.enabled is true" -}}
141+
{{- end -}}
139142
{{- end -}}
140143

141144
{{/*
142-
Returns a default SASL password based on a given prefix and the release name (sha256 hashed).
145+
Validates that ZooKeeper SASL password is not empty when ZK SASL is enabled.
146+
Returns an error message if invalid, empty string otherwise.
143147
Usage:
144-
include "fluss.security.sasl.defaultPassword" (dict "prefix" "fluss-internal" "Release" .Release)
148+
include "fluss.security.sasl.validateZookeeperPassword" .
145149
*/}}
146-
{{- define "fluss.security.sasl.defaultPassword" -}}
147-
{{- printf "%s-password-%s" .prefix .Release.Name | sha256sum -}}
150+
{{- define "fluss.security.sasl.validateZookeeperPassword" -}}
151+
{{- if and .Values.security.zookeeper.sasl.enabled (not .Values.security.zookeeper.sasl.password) -}}
152+
{{- print "security.zookeeper.sasl.password must not be empty when security.zookeeper.sasl.enabled is true" -}}
153+
{{- end -}}
148154
{{- end -}}
149155

150156
{{/*
151-
Returns the resolved internal SASL username.
152-
It generates internal username if user provided is empty.
157+
Returns the default internal SASL username based on the release name.
153158
Usage:
154-
include "fluss.security.sasl.plain.internal.username" .
159+
include "fluss.security.sasl.plain.internal.defaultUsername" .
155160
*/}}
156-
{{- define "fluss.security.sasl.plain.internal.username" -}}
157-
{{- .Values.security.internal.sasl.plain.username | default (include "fluss.security.sasl.defaultUsername" (dict "prefix" "fluss-internal" "Release" .Release)) -}}
161+
{{- define "fluss.security.sasl.plain.internal.defaultUsername" -}}
162+
{{- printf "fluss-internal-user-%s" .Release.Name -}}
158163
{{- end -}}
159164

160165
{{/*
161-
Returns the resolved internal SASL password.
162-
It generates internal password if user provided is empty.
166+
Returns the default internal SASL password based on the release name (sha256 hashed).
163167
Usage:
164-
include "fluss.security.sasl.plain.internal.password" .
168+
include "fluss.security.sasl.plain.internal.defaultPassword" .
165169
*/}}
166-
{{- define "fluss.security.sasl.plain.internal.password" -}}
167-
{{- .Values.security.internal.sasl.plain.password | default (include "fluss.security.sasl.defaultPassword" (dict "prefix" "fluss-internal" "Release" .Release)) -}}
170+
{{- define "fluss.security.sasl.plain.internal.defaultPassword" -}}
171+
{{- printf "fluss-internal-password-%s" .Release.Name | sha256sum -}}
168172
{{- end -}}
169173

170174
{{/*
171-
Returns the resolved ZooKeeper SASL username.
172-
It generates Zookeeper username if user provided is empty.
175+
Returns the resolved internal SASL username (user-provided or auto-generated default).
173176
Usage:
174-
include "fluss.security.zookeeper.sasl.username" .
177+
include "fluss.security.sasl.plain.internal.username" .
175178
*/}}
176-
{{- define "fluss.security.zookeeper.sasl.username" -}}
177-
{{- .Values.security.zookeeper.sasl.username | default (include "fluss.security.sasl.defaultUsername" (dict "prefix" "fluss-zookeeper" "Release" .Release)) -}}
179+
{{- define "fluss.security.sasl.plain.internal.username" -}}
180+
{{- .Values.security.internal.sasl.plain.username | default (include "fluss.security.sasl.plain.internal.defaultUsername" .) -}}
178181
{{- end -}}
179182

180183
{{/*
181-
Returns the resolved ZooKeeper SASL password.
182-
It generates Zookeeper password if user provided is empty.
184+
Returns the resolved internal SASL password (user-provided or auto-generated default).
183185
Usage:
184-
include "fluss.security.zookeeper.sasl.password" .
186+
include "fluss.security.sasl.plain.internal.password" .
185187
*/}}
186-
{{- define "fluss.security.zookeeper.sasl.password" -}}
187-
{{- .Values.security.zookeeper.sasl.password | default (include "fluss.security.sasl.defaultPassword" (dict "prefix" "fluss-zookeeper" "Release" .Release)) -}}
188+
{{- define "fluss.security.sasl.plain.internal.password" -}}
189+
{{- .Values.security.internal.sasl.plain.password | default (include "fluss.security.sasl.plain.internal.defaultPassword" .) -}}
188190
{{- end -}}
189191

190192
{{/*
@@ -214,19 +216,6 @@ Usage:
214216
{{- end -}}
215217
{{- end -}}
216218

217-
{{/*
218-
Returns a warning if the ZooKeeper SASL user is using auto-generated credentials.
219-
Usage:
220-
include "fluss.security.sasl.warnZookeeperUser" .
221-
*/}}
222-
{{- define "fluss.security.sasl.warnZookeeperUser" -}}
223-
{{- if .Values.security.zookeeper.sasl.enabled -}}
224-
{{- if and (not .Values.security.zookeeper.sasl.username) (not .Values.security.zookeeper.sasl.password) -}}
225-
{{- print "You are using AUTO-GENERATED SASL credentials for ZooKeeper authentication.\n It is strongly recommended to set the following values in production:\n - security.zookeeper.sasl.username\n - security.zookeeper.sasl.password" -}}
226-
{{- end -}}
227-
{{- end -}}
228-
{{- end -}}
229-
230219
{{/*
231220
Compile all warnings and errors into a single message.
232221
Usage:
@@ -238,13 +227,14 @@ Usage:
238227
{{- $errMessages = append $errMessages (include "fluss.security.sasl.validateMechanisms" .) -}}
239228
{{- $errMessages = append $errMessages (include "fluss.security.sasl.validateClientPlainUsers" .) -}}
240229
{{- $errMessages = append $errMessages (include "fluss.security.sasl.validateZookeeperLoginModuleClass" .) -}}
230+
{{- $errMessages = append $errMessages (include "fluss.security.sasl.validateZookeeperUsername" .) -}}
231+
{{- $errMessages = append $errMessages (include "fluss.security.sasl.validateZookeeperPassword" .) -}}
241232

242233
{{- $errMessages = without $errMessages "" -}}
243234
{{- $errMessage := join "\n" $errMessages -}}
244235

245236
{{- $warnMessages := list -}}
246237
{{- $warnMessages = append $warnMessages (include "fluss.security.sasl.warnInternalUser" .) -}}
247-
{{- $warnMessages = append $warnMessages (include "fluss.security.sasl.warnZookeeperUser" .) -}}
248238

249239
{{- $warnMessages = without $warnMessages "" -}}
250240
{{- $warnMessage := join "\n" $warnMessages -}}
@@ -262,8 +252,8 @@ Usage:
262252
{{/*
263253
Returns the SASL JAAS config name.
264254
Usage:
265-
include "fluss.security.sasl.configName" .
255+
include "fluss.security.jaas.configName" .
266256
*/}}
267-
{{- define "fluss.security.sasl.configName" -}}
257+
{{- define "fluss.security.jaas.configName" -}}
268258
{{ include "fluss.fullname" . }}-sasl-jaas-config
269259
{{- end -}}

‎helm/templates/secret-jaas-config.yaml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@
1616
# limitations under the License.
1717
#
1818

19-
{{- if (include "fluss.security.jaas.required" .) -}}
19+
{{ if (include "fluss.security.jaas.required" .) }}
2020
{{- $internalMechanism := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "internal") -}}
2121
{{- $clientMechanism := include "fluss.security.listener.mechanism" (dict "context" .Values "listener" "client") -}}
2222
{{- $internalUsername := include "fluss.security.sasl.plain.internal.username" . -}}
2323
{{- $internalPassword := include "fluss.security.sasl.plain.internal.password" . -}}
2424
apiVersion: v1
2525
kind: Secret
2626
metadata:
27-
name: {{ include "fluss.security.sasl.configName" . }}
27+
name: {{ include "fluss.security.jaas.configName" . }}
2828
labels:
2929
{{- include "fluss.labels" . | nindent 4 }}
3030
type: Opaque
@@ -54,8 +54,8 @@ stringData:
5454
{{- if .Values.security.zookeeper.sasl.enabled }}
5555
ZookeeperClient {
5656
{{ .Values.security.zookeeper.sasl.loginModuleClass }} required
57-
username="{{ include "fluss.security.zookeeper.sasl.username" . }}"
58-
password="{{ include "fluss.security.zookeeper.sasl.password" . }}";
57+
username="{{ .Values.security.zookeeper.sasl.username }}"
58+
password="{{ .Values.security.zookeeper.sasl.password }}";
5959
};
6060
{{- end }}
6161
{{- if .Values.security.zookeeper.sasl.enabled }}

‎helm/templates/sts-coordinator.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ spec:
121121
{{- if (include "fluss.security.jaas.required" .) }}
122122
- name: sasl-config
123123
secret:
124-
secretName: {{ include "fluss.security.sasl.configName" . }}
124+
secretName: {{ include "fluss.security.jaas.configName" . }}
125125
{{- end }}
126126
{{- if .Values.coordinator.storage.enabled }}
127127
volumeClaimTemplates:

‎helm/templates/sts-tablet.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ spec:
118118
{{- if (include "fluss.security.jaas.required" .) }}
119119
- name: sasl-config
120120
secret:
121-
secretName: {{ include "fluss.security.sasl.configName" . }}
121+
secretName: {{ include "fluss.security.jaas.configName" . }}
122122
{{- end }}
123123
{{- if .Values.tablet.storage.enabled }}
124124
volumeClaimTemplates:

‎helm/tests/security_test.yaml‎

Lines changed: 78 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -327,18 +327,6 @@ tests:
327327
- matchRegex:
328328
path: stringData["jaas.conf"]
329329
pattern: 'ZookeeperClient\s*\{'
330-
- it: uses auto-generated ZK credentials when user does not override them
331-
set:
332-
security.zookeeper.sasl.enabled: true
333-
asserts:
334-
- hasDocuments:
335-
count: 1
336-
- matchRegex:
337-
path: stringData["jaas.conf"]
338-
pattern: 'username="fluss-zookeeper-user-RELEASE-NAME"'
339-
- matchRegex:
340-
path: stringData["jaas.conf"]
341-
pattern: 'password="[a-f0-9]{64}"'
342330
- it: does not render SASL listener blocks when only ZK SASL is enabled
343331
set:
344332
security.zookeeper.sasl.enabled: true
@@ -429,14 +417,91 @@ tests:
429417

430418
---
431419

432-
suite: zookeeper-sasl-empty-login-module-fails
420+
suite: zookeeper-sasl-disabled
421+
templates:
422+
- templates/secret-jaas-config.yaml
423+
- templates/sts-coordinator.yaml
424+
- templates/sts-tablet.yaml
425+
- templates/configmap.yaml
426+
tests:
427+
- it: does not render ZookeeperClient JAAS block when ZK SASL is disabled
428+
template: templates/secret-jaas-config.yaml
429+
set:
430+
security.internal.sasl.mechanism: plain
431+
security.internal.sasl.plain.username: internal-user
432+
security.internal.sasl.plain.password: internal-pass
433+
asserts:
434+
- hasDocuments:
435+
count: 1
436+
- notMatchRegex:
437+
path: stringData["jaas.conf"]
438+
pattern: 'ZookeeperClient\s*\{'
439+
- isNull:
440+
path: stringData["zookeeper-client.properties"]
441+
- it: does not render JAAS secret when neither SASL nor ZK SASL is enabled
442+
template: templates/secret-jaas-config.yaml
443+
asserts:
444+
- hasDocuments:
445+
count: 0
446+
- it: does not render JAAS env var or volume mount for coordinator when ZK SASL is disabled
447+
template: templates/sts-coordinator.yaml
448+
asserts:
449+
- notContains:
450+
path: spec.template.spec.containers[0].env
451+
content:
452+
name: FLUSS_ENV_JAVA_OPTS
453+
value: "-Djava.security.auth.login.config=/etc/fluss/conf/jaas.conf"
454+
- notContains:
455+
path: spec.template.spec.volumes
456+
content:
457+
name: sasl-config
458+
any: true
459+
- it: does not render JAAS env var or volume mount for tablet when ZK SASL is disabled
460+
template: templates/sts-tablet.yaml
461+
asserts:
462+
- notContains:
463+
path: spec.template.spec.containers[0].env
464+
content:
465+
name: FLUSS_ENV_JAVA_OPTS
466+
value: "-Djava.security.auth.login.config=/etc/fluss/conf/jaas.conf"
467+
- notContains:
468+
path: spec.template.spec.volumes
469+
content:
470+
name: sasl-config
471+
any: true
472+
- it: does not render zookeeper client config path in configmap when ZK SASL is disabled
473+
template: templates/configmap.yaml
474+
asserts:
475+
- notMatchRegex:
476+
path: data["server.yaml"]
477+
pattern: 'zookeeper\.client\.config\.path'
478+
479+
---
480+
481+
suite: zookeeper-sasl-validation
433482
templates:
434483
- templates/NOTES.txt
435484
tests:
436485
- it: fails when loginModuleClass is empty and ZK SASL is enabled
437486
set:
438487
security.zookeeper.sasl.enabled: true
488+
security.zookeeper.sasl.username: zk-user
489+
security.zookeeper.sasl.password: zk-pass
439490
security.zookeeper.sasl.loginModuleClass: ""
440491
asserts:
441492
- failedTemplate:
442493
errorMessage: "VALUES VALIDATION:\nsecurity.zookeeper.sasl.loginModuleClass must not be empty when security.zookeeper.sasl.enabled is true"
494+
- it: fails when ZK SASL is enabled but username is empty
495+
set:
496+
security.zookeeper.sasl.enabled: true
497+
security.zookeeper.sasl.password: zk-pass
498+
asserts:
499+
- failedTemplate:
500+
errorMessage: "VALUES VALIDATION:\nsecurity.zookeeper.sasl.username must not be empty when security.zookeeper.sasl.enabled is true"
501+
- it: fails when ZK SASL is enabled but password is empty
502+
set:
503+
security.zookeeper.sasl.enabled: true
504+
security.zookeeper.sasl.username: zk-user
505+
asserts:
506+
- failedTemplate:
507+
errorMessage: "VALUES VALIDATION:\nsecurity.zookeeper.sasl.password must not be empty when security.zookeeper.sasl.enabled is true"

‎website/docs/install-deploy/deploying-with-helm.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ security:
347347
password: fluss-zk-password
348348
```
349349

350-
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.
350+
The `username` and `password` fields are required when ZooKeeper SASL is enabled.
351351

352352
ZooKeeper SASL can be enabled independently or together with the listeners SASL authentication.
353353

0 commit comments

Comments
 (0)