Skip to content

Commit c659b7b

Browse files
sebastianiv21claude
andcommitted
fix(helm): derive redis master host from subchart fullname + tests
Address CodeRabbit review on appsmith.redisMasterHost (PR #41874): the host was hardcoded as "{release}-redis-master", which is wrong whenever the Bitnami subchart names its master Service differently (release name contains "redis", or nameOverride/fullnameOverride set). Derive it from common.names.fullname evaluated in the redis subchart context (.Subcharts.redis) so it always matches the rendered Service. - Tighten the helper comment (name common.names.fullname; note the nameOverride/fullnameOverride edge cases). - Add redis_auth_test cases: redis.auth.password guard (reject/accept), self-managed path skips bootstrap hook, init container skips REDISCLI_AUTH on operator URL, and a regression case asserting the master host follows the subchart fullname for a "redis"-named release. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent cf9fa0d commit c659b7b

2 files changed

Lines changed: 84 additions & 4 deletions

File tree

deploy/helm/templates/_helpers.tpl

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,12 +260,15 @@ every `helm template`/install/upgrade.
260260

261261
{{/*
262262
Redis: master service hostname (FQDN inside the cluster).
263-
Derived from the release name to stay uniform with the chart's other components.
264-
Assumes the release name does not contain "redis" (otherwise the Bitnami subchart
265-
collapses its fullname to just the release name and this host would not match).
263+
Reuses the bundled `common.names.fullname` helper (the one the subchart's master
264+
Service uses), evaluated in the redis subchart's context (.Subcharts.redis), so the
265+
host always matches the Service it renders — including the edge cases where the
266+
release name contains "redis" (the subchart collapses its fullname to just the
267+
release name) or redis.nameOverride / redis.fullnameOverride is set.
268+
Only valid when redis.enabled (the subchart context exists); all callers gate on it.
266269
*/}}
267270
{{- define "appsmith.redisMasterHost" -}}
268-
{{- printf "%s-redis-master.%s.svc.cluster.local" .Release.Name (include "appsmith.namespace" .) -}}
271+
{{- printf "%s-master.%s.svc.cluster.local" (include "common.names.fullname" .Subcharts.redis) (include "appsmith.namespace" .) -}}
269272
{{- end -}}
270273

271274
{{/*

deploy/helm/tests/redis_auth_test.yaml

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,86 @@ tests:
197197
path: data.APPSMITH_REDIS_URL
198198
value: redis://RELEASE-NAME-redis-master.NAMESPACE.svc.cluster.local:6379
199199

200+
# Regression: the host must track the subchart's fullname, not a naive
201+
# "{release}-redis-master". When the release name contains "redis" the subchart
202+
# collapses its fullname to just the release name, so the master Service is
203+
# "{release}-master" (no doubled -redis-). See appsmith.redisMasterHost.
204+
- name: host follows subchart fullname when release name contains "redis"
205+
release:
206+
name: myredis
207+
set:
208+
redis.auth.enabled: false
209+
asserts:
210+
- equal:
211+
path: data.APPSMITH_REDIS_URL
212+
value: redis://myredis-master.NAMESPACE.svc.cluster.local:6379
213+
200214
- name: no bundled redis URL when the subchart is disabled
201215
set:
202216
redis.enabled: false
203217
asserts:
204218
- notExists:
205219
path: data.APPSMITH_REDIS_URL
220+
221+
---
222+
# Guard: redis.auth.password is only allowed on the fully self-managed path
223+
# (existingSecret: "" + a matching applicationConfig.APPSMITH_REDIS_URL).
224+
# Validation lives in appsmith.validateRedisAuth, invoked from configMap.yaml.
225+
suite: Redis auth - password guard (validateRedisAuth)
226+
templates:
227+
- configMap.yaml
228+
tests:
229+
- name: rejects password with the default bootstrap secret still set
230+
set:
231+
redis.auth.password: hunter2
232+
asserts:
233+
- failedTemplate:
234+
errorMessage: ""
235+
236+
- name: rejects password when no APPSMITH_REDIS_URL is supplied
237+
set:
238+
redis.auth.password: hunter2
239+
redis.auth.existingSecret: ""
240+
asserts:
241+
- failedTemplate:
242+
errorMessage: ""
243+
244+
- name: accepts the self-managed path (password + empty existingSecret + URL)
245+
set:
246+
redis.auth.password: hunter2
247+
redis.auth.existingSecret: ""
248+
applicationConfig.APPSMITH_REDIS_URL: redis://:hunter2@RELEASE-NAME-redis-master:6379
249+
asserts:
250+
- notFailedTemplate: {}
251+
252+
---
253+
suite: Redis auth - self-managed path skips chart bootstrap
254+
templates:
255+
- hooks/redis.yaml
256+
tests:
257+
- name: no bootstrap hook when redis.auth.password is set (self-managed)
258+
set:
259+
redis.auth.password: hunter2
260+
redis.auth.existingSecret: ""
261+
applicationConfig.APPSMITH_REDIS_URL: redis://:hunter2@RELEASE-NAME-redis-master:6379
262+
asserts:
263+
- hasDocuments:
264+
count: 0
265+
266+
---
267+
suite: Redis auth - init container skips REDISCLI_AUTH on self-managed URL
268+
templates:
269+
- deployment.yaml
270+
tests:
271+
- name: no REDISCLI_AUTH when the operator supplies APPSMITH_REDIS_URL
272+
set:
273+
applicationConfig.APPSMITH_REDIS_URL: redis://custom.example:6379
274+
asserts:
275+
- notContains:
276+
path: spec.template.spec.initContainers[0].env
277+
content:
278+
name: REDISCLI_AUTH
279+
valueFrom:
280+
secretKeyRef:
281+
name: appsmith-redis-secret
282+
key: redis-password

0 commit comments

Comments
 (0)