Skip to content

Commit 28613b3

Browse files
rayne-Lirayne-Li
authored
chart: optimize redis passwd logic in helper.tpl (vllm-project#2234)
1 parent a1663b4 commit 28613b3

4 files changed

Lines changed: 160 additions & 63 deletions

File tree

‎dist/chart/README.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,27 @@ resolve the issue and retry until you get:
4242
1 chart(s) linted, 0 chart(s) failed
4343
```
4444

45+
### helm unittest
46+
47+
test cases in dist/chart/tests
48+
```
49+
helm plugin install https://github.com/helm-unittest/helm-unittest.git
50+
51+
helm unittest dist/chart
52+
### Chart [ aibrix ] dist/chart
53+
54+
PASS Test Redis Configuration Logic dist/chart/tests/redis_config_test.yaml
55+
PASS Test Redis Dependency Passwd Logic dist/chart/tests/redis_config_test.yaml
56+
PASS Test Redis Shared Passwd Conflict dist/chart/tests/redis_config_test.yaml
57+
PASS Test Redis Shared Passwd dist/chart/tests/redis_config_test.yaml
58+
59+
Charts: 1 passed, 1 total
60+
Test Suites: 4 passed, 4 total
61+
Tests: 4 passed, 4 total
62+
Snapshot: 0 passed, 0 total
63+
Time: 14.85875ms
64+
```
65+
4566
### Render yaml files
4667

4768
Render all manifests using:

‎dist/chart/templates/_helpers.tpl‎

Lines changed: 35 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -130,84 +130,71 @@ Create the name of the metadata service service account
130130
{{- if and .Values.gateway.enable .Values.gateway.envoyAsSideCar -}}
131131
{{- fail "gateway.enable and gateway.envoyAsSideCar are mutually exclusive and cannot both be true." -}}
132132
{{- end -}}
133-
{{- $metadata := get .Values "metadata" | default dict -}}
134-
{{- $metadataRedis := get $metadata "redis" | default dict -}}
135-
{{- $metadataService := get $metadata "service" | default dict -}}
136-
{{- $metadataServiceRedis := get $metadataService "redis" | default dict -}}
137-
{{- $gatewayPlugin := get .Values "gatewayPlugin" | default dict -}}
138-
{{- $gatewayDependencies := get $gatewayPlugin "dependencies" | default dict -}}
139-
{{- $gatewayRedis := get $gatewayDependencies "redis" | default dict -}}
140-
{{- $gpuOptimizer := get .Values "gpuOptimizer" | default dict -}}
141-
{{- $gpuDependencies := get $gpuOptimizer "dependencies" | default dict -}}
142-
{{- $gpuRedis := get $gpuDependencies "redis" | default dict -}}
143-
{{- $builtInRedisEnabled := true -}}
144-
{{- if hasKey $metadataRedis "enabled" -}}
145-
{{- $builtInRedisEnabled = get $metadataRedis "enabled" -}}
146-
{{- end -}}
147-
{{- $sharedEnablePassword := get $metadataRedis "enablePassword" | default false -}}
148-
{{- $sharedPassword := get $metadataRedis "password" | default "" -}}
133+
134+
{{- $builtInRedisEnabled := dig "redis" "enabled" true .Values.metadata -}}
135+
{{- $sharedEnablePassword := dig "redis" "enablePassword" false .Values.metadata -}}
136+
{{- $sharedPassword := dig "redis" "password" "" .Values.metadata -}}
137+
138+
{{- /* Shared Redis password must be non-empty when enablePassword is true */ -}}
149139
{{- if and $sharedEnablePassword (empty (trim $sharedPassword)) -}}
150140
{{- fail "metadata.redis.enablePassword=true requires a non-empty metadata.redis.password." -}}
151141
{{- end -}}
142+
143+
{{- /* When builtIn Redis is disabled, all components must declare an external Redis host */ -}}
152144
{{- if not $builtInRedisEnabled -}}
153145
{{- $missingHosts := list -}}
154-
{{- if empty (trim (get $metadataServiceRedis "host" | default "")) -}}
155-
{{- $missingHosts = append $missingHosts "metadata.service.redis.host" -}}
146+
{{- if empty (trim (dig "service" "redis" "host" "" .Values.metadata)) -}}
147+
{{- $missingHosts = append $missingHosts "metadata.service.redis.host" -}}
156148
{{- end -}}
157-
{{- if empty (trim (get $gatewayRedis "host" | default "")) -}}
158-
{{- $missingHosts = append $missingHosts "gatewayPlugin.dependencies.redis.host" -}}
149+
{{- if empty (trim (dig "dependencies" "redis" "host" "" .Values.gatewayPlugin)) -}}
150+
{{- $missingHosts = append $missingHosts "gatewayPlugin.dependencies.redis.host" -}}
159151
{{- end -}}
160-
{{- if empty (trim (get $gpuRedis "host" | default "")) -}}
161-
{{- $missingHosts = append $missingHosts "gpuOptimizer.dependencies.redis.host" -}}
152+
{{- if empty (trim (dig "dependencies" "redis" "host" "" .Values.gpuOptimizer)) -}}
153+
{{- $missingHosts = append $missingHosts "gpuOptimizer.dependencies.redis.host" -}}
162154
{{- end -}}
155+
163156
{{- if gt (len $missingHosts) 0 -}}
164157
{{- fail (printf "metadata.redis.enabled=false requires non-empty values for %s." (join ", " $missingHosts)) -}}
165158
{{- end -}}
166159
{{- end -}}
160+
161+
{{- /*
162+
Prevent the use of component-specific Redis passwords when the shared builtIn Redis is enabled.
163+
This OR block checks whether any custom password is set across the three components.
164+
*/ -}}
167165
{{- if and $builtInRedisEnabled (or
168-
(get $metadataServiceRedis "password" | default "")
169-
(get $gatewayRedis "password" | default "")
170-
(get $gpuRedis "password" | default "")) -}}
166+
(dig "service" "redis" "password" "" .Values.metadata)
167+
(dig "dependencies" "redis" "password" "" .Values.gatewayPlugin)
168+
(dig "dependencies" "redis" "password" "" .Values.gpuOptimizer)) -}}
171169
{{- fail "built-in Redis does not support component-specific passwords; use metadata.redis.enablePassword/password for shared built-in Redis auth or disable built-in Redis for external Redis passwords." -}}
172170
{{- end -}}
171+
173172
{{- end -}}
174173

175174
{{/*
176175
Return whether the shared Redis password config is enabled.
177176
*/}}
178177
{{- define "aibrix.redis.sharedHasPassword" -}}
179-
{{- $metadata := get .Values "metadata" | default dict -}}
180-
{{- $redis := get $metadata "redis" | default dict -}}
181-
{{- if or (get $redis "enablePassword" | default false) (get $redis "password" | default "") -}}true{{- end -}}
178+
{{- if or (dig "redis" "enablePassword" false .Values.metadata) (dig "redis" "password" "" .Values.metadata) -}}true{{- end -}}
182179
{{- end -}}
183180

184181
{{/*
185182
Return whether a component Redis connection should use password auth.
186183
Supported components: metadataService, gatewayPlugin, gpuOptimizer.
187184
*/}}
188185
{{- define "aibrix.redis.connectionHasPassword" -}}
189-
{{- $values := .Values -}}
190-
{{- $component := .component -}}
191-
{{- $metadata := get $values "metadata" | default dict -}}
192-
{{- $metadataRedis := get $metadata "redis" | default dict -}}
193-
{{- $sharedEnabled := get $metadataRedis "enablePassword" | default false -}}
194-
{{- $sharedPassword := get $metadataRedis "password" | default "" -}}
186+
{{- $sharedEnabled := dig "redis" "enablePassword" false .Values.metadata -}}
187+
{{- $sharedPassword := dig "redis" "password" "" .Values.metadata -}}
188+
195189
{{- $componentPassword := "" -}}
196-
{{- if eq $component "metadataService" -}}
197-
{{- $metadataService := get $metadata "service" | default dict -}}
198-
{{- $metadataServiceRedis := get $metadataService "redis" | default dict -}}
199-
{{- $componentPassword = get $metadataServiceRedis "password" | default "" -}}
200-
{{- else if eq $component "gatewayPlugin" -}}
201-
{{- $gatewayPlugin := get $values "gatewayPlugin" | default dict -}}
202-
{{- $dependencies := get $gatewayPlugin "dependencies" | default dict -}}
203-
{{- $redis := get $dependencies "redis" | default dict -}}
204-
{{- $componentPassword = get $redis "password" | default "" -}}
205-
{{- else if eq $component "gpuOptimizer" -}}
206-
{{- $gpuOptimizer := get $values "gpuOptimizer" | default dict -}}
207-
{{- $dependencies := get $gpuOptimizer "dependencies" | default dict -}}
208-
{{- $redis := get $dependencies "redis" | default dict -}}
209-
{{- $componentPassword = get $redis "password" | default "" -}}
190+
{{- if eq .component "metadataService" -}}
191+
{{- $componentPassword = dig "service" "redis" "password" "" .Values.metadata -}}
192+
{{- else if eq .component "gatewayPlugin" -}}
193+
{{- $componentPassword = dig "dependencies" "redis" "password" "" .Values.gatewayPlugin -}}
194+
{{- else if eq .component "gpuOptimizer" -}}
195+
{{- $componentPassword = dig "dependencies" "redis" "password" "" .Values.gpuOptimizer -}}
210196
{{- end -}}
197+
211198
{{- if or $sharedEnabled $sharedPassword $componentPassword -}}true{{- end -}}
212199
{{- end -}}
213200

‎dist/chart/templates/metadata-service/redis.yaml‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ spec:
5252

5353
{{- end }}
5454

55-
{{- $sharedPassword := .Values.metadata.redis.password -}}
5655
{{- if include "aibrix.redis.anyPasswordsConfigured" . }}
5756
---
5857
apiVersion: v1
@@ -61,22 +60,22 @@ metadata:
6160
name: {{ include "aibrix.fullname" . }}-redis
6261
type: Opaque
6362
data:
64-
{{- if $sharedPassword }}
65-
redis-password: {{ $sharedPassword | b64enc }}
63+
{{- $sharedPass := dig "redis" "password" "" .Values.metadata -}}
64+
{{- $gatewayPass := dig "dependencies" "redis" "password" "" .Values.gatewayPlugin -}}
65+
{{- $gpuPass := dig "dependencies" "redis" "password" "" .Values.gpuOptimizer -}}
66+
{{- $metaPass := dig "service" "redis" "password" "" .Values.metadata -}}
67+
68+
{{- if $sharedPass }}
69+
redis-password: {{ $sharedPass | b64enc }}
6670
{{- end }}
67-
{{- if .Values.gatewayPlugin.dependencies.redis.password }}
68-
gateway-plugin-redis-password: {{ .Values.gatewayPlugin.dependencies.redis.password | b64enc }}
69-
{{- else if $sharedPassword }}
70-
gateway-plugin-redis-password: {{ $sharedPassword | b64enc }}
71+
72+
{{- if or $gatewayPass $sharedPass }}
73+
gateway-plugin-redis-password: {{ default $sharedPass $gatewayPass | b64enc }}
7174
{{- end }}
72-
{{- if .Values.gpuOptimizer.dependencies.redis.password }}
73-
gpu-optimizer-redis-password: {{ .Values.gpuOptimizer.dependencies.redis.password | b64enc }}
74-
{{- else if $sharedPassword }}
75-
gpu-optimizer-redis-password: {{ $sharedPassword | b64enc }}
75+
{{- if or $gpuPass $sharedPass }}
76+
gpu-optimizer-redis-password: {{ default $sharedPass $gpuPass | b64enc }}
7677
{{- end }}
77-
{{- if .Values.metadata.service.redis.password }}
78-
metadata-service-redis-password: {{ .Values.metadata.service.redis.password | b64enc }}
79-
{{- else if $sharedPassword }}
80-
metadata-service-redis-password: {{ $sharedPassword | b64enc }}
78+
{{- if or $metaPass $sharedPass }}
79+
metadata-service-redis-password: {{ default $sharedPass $metaPass | b64enc }}
8180
{{- end }}
8281
{{- end }}

‎dist/chart/tests/redis_config_test.yaml‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
suite: Test Redis Configuration Logic
2+
templates:
3+
- gateway-plugin/deployment.yaml
4+
- metadata-service/deployment.yaml
5+
- gpu-optimizer/deployment.yaml
6+
tests:
7+
- it: should configure external redis when builtIn is disabled
8+
set:
9+
metadata.redis.enabled: false
10+
gatewayPlugin.dependencies.redis.host: "external-redis.cluster.local"
11+
gatewayPlugin.dependencies.redis.port: 3796
12+
metadata.service.redis.host: "external-redis.cluster.local"
13+
metadata.service.redis.port: 3796
14+
gpuOptimizer.dependencies.redis.host: "external-redis.cluster.local"
15+
gpuOptimizer.dependencies.redis.port: 3796
16+
asserts:
17+
- contains:
18+
path: spec.template.spec.containers[0].env
19+
content:
20+
name: REDIS_HOST
21+
value: "external-redis.cluster.local"
22+
- contains:
23+
path: spec.template.spec.containers[0].env
24+
content:
25+
name: REDIS_PORT
26+
value: "3796"
27+
---
28+
suite: Test Redis Dependency Passwd Logic
29+
templates:
30+
- metadata-service/redis.yaml
31+
tests:
32+
- it: should set external redis passwd when builtIn is disabled
33+
set:
34+
metadata.redis.enabled: false
35+
gatewayPlugin.dependencies.redis.password: "abcd"
36+
metadata.service.redis.password: "abdc"
37+
gpuOptimizer.dependencies.redis.password: "acbd"
38+
asserts:
39+
- notExists:
40+
path: data.redis-password
41+
- equal:
42+
path: data.gateway-plugin-redis-password
43+
value: "YWJjZA=="
44+
- equal:
45+
path: data.gpu-optimizer-redis-password
46+
value: "YWNiZA=="
47+
- equal:
48+
path: data.metadata-service-redis-password
49+
value: "YWJkYw=="
50+
---
51+
suite: Test Redis Shared Passwd Conflict
52+
templates:
53+
tests:
54+
- it: should conflict
55+
set:
56+
metadata.redis.enabled: true
57+
gatewayPlugin.dependencies.redis.password: "abcd"
58+
metadata.service.redis.password: "abdc"
59+
gpuOptimizer.dependencies.redis.password: "acbd"
60+
asserts:
61+
- failedTemplate:
62+
errorMessage: "built-in Redis does not support component-specific passwords; use metadata.redis.enablePassword/password for shared built-in Redis auth or disable built-in Redis for external Redis passwords."
63+
---
64+
suite: Test Redis Shared Passwd
65+
templates:
66+
- metadata-service/redis.yaml
67+
tests:
68+
- it: should configure shared redis passwd
69+
set:
70+
metadata.redis.enabled: true
71+
metadata.redis.enablePassword: false
72+
metadata.redis.password: "dddd"
73+
asserts:
74+
- equal:
75+
path: data.redis-password
76+
value: "ZGRkZA=="
77+
documentIndex: 2
78+
- equal:
79+
path: data.gateway-plugin-redis-password
80+
value: "ZGRkZA=="
81+
documentIndex: 2
82+
- equal:
83+
path: data.gpu-optimizer-redis-password
84+
value: "ZGRkZA=="
85+
documentIndex: 2
86+
- equal:
87+
path: data.metadata-service-redis-password
88+
value: "ZGRkZA=="
89+
documentIndex: 2
90+

0 commit comments

Comments
 (0)