feat(helm): add Redis auth support with password bootstrap#41874
Open
sebastianiv21 wants to merge 14 commits into
Open
feat(helm): add Redis auth support with password bootstrap#41874sebastianiv21 wants to merge 14 commits into
sebastianiv21 wants to merge 14 commits into
Conversation
Enable authenticated Redis in the Helm chart. A pre-install/pre-upgrade hook Job generates the Redis password Secret (referenced by redis.auth.existingSecret) when it doesn't already exist, keeping the password stable across upgrades and ArgoCD re-syncs. - Add redisMasterHost/redisSecretName/redisPasswordInitImage helpers - Assemble an authenticated APPSMITH_REDIS_URL from the Secret, injected by reference so cleartext never lands in the ConfigMap; skipped when the user supplies their own APPSMITH_REDIS_URL - Gate the init-container ping and ConfigMap URL on redis.auth.enabled - Add redisAuth.passwordInit image config and values.schema.json entries - Add redis_auth_test snapshot/test coverage Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds Helm-based Redis authentication and bootstrapping: default values and schema for redisAuth, helper templates for secret name/host/image, a pre-install hook Job+RBAC to create the Redis password Secret if missing, deployment/ConfigMap wiring to consume the secret, and Helm tests covering the flow. ChangesRedis authentication setup and wiring
Sequence DiagramsequenceDiagram
participant HookJob as Pre-install Hook Job
participant KubeAPI as Kubernetes API
participant Secret as Redis Auth Secret
participant AppDep as Appsmith Deployment
participant Redis as Redis Master
HookJob->>KubeAPI: kubectl get secret (redisSecretName)
KubeAPI-->>HookJob: found / not found
alt not found
HookJob->>KubeAPI: kubectl create secret (redis-password)
KubeAPI->>Secret: create secret
end
AppDep->>Secret: read APPSMITH_REDIS_PASSWORD / REDISCLI_AUTH
AppDep->>Redis: connect using APPSMITH_REDIS_URL
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deploy/helm/templates/deployment.yaml (1)
186-215:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t inject the bundled Redis URL when any env source can already supply
APPSMITH_REDIS_URL.This guard only skips on
applicationConfig.APPSMITH_REDIS_URL, but the chart also imports env vars fromsecretName,secrets, andexternalSecretsa few lines below. Because an explicitenv:entry takes precedence overenvFrom, a user-provided secret-backedAPPSMITH_REDIS_URLwill be silently ignored whenever bundled Redis auth is enabled. Please gate this the same wayappsmith.useOperatorMongodoes and treat all alternate env sources as authoritative.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/templates/deployment.yaml` around lines 186 - 215, The APPSMITH_REDIS_URL env entry for the bundled Redis is being injected even when envFrom sources (secretName, secrets, externalSecrets) could supply APPSMITH_REDIS_URL and would be overridden; update the guard around the APPSMITH_REDIS_PASSWORD/APPSMITH_REDIS_URL block (the conditional that currently checks .Values.redis.enabled, .Values.redis.auth.enabled and not .Values.applicationConfig.APPSMITH_REDIS_URL) to also require that no alternate env sources are configured (i.e. add checks for not .Values.secretName, not .Values.secrets, and not .Values.externalSecrets.enabled) following the same pattern used by appsmith.useOperatorMongo so the chart skips injecting the URL whenever any envFrom-backed secret source might provide APPSMITH_REDIS_URL.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/helm/templates/_helpers.tpl`:
- Around line 240-248: The helper appsmith.redisMasterHost currently hardcodes
the host using .Release.Name which can be wrong when the Bitnami redis subchart
alters its fullname; change appsmith.redisMasterHost to derive the service
hostname from the redis subchart's fullname helper instead (use the subchart's
fullname include, e.g. call include "redis.fullname" . or include
"appsmith.redis.fullname" . as appropriate) and use that result to build the
FQDN (e.g. "%s.%s.svc.cluster.local" with the included fullname and include
"appsmith.namespace" .) so the host matches the actual service name the subchart
creates.
In `@deploy/helm/values.schema.json`:
- Around line 566-592: The values.schema.json is out of sync with the Helm
values file—regenerate deploy/helm/values.schema.json from
deploy/helm/values.yaml using Helm's schema generator exactly as the CI expects:
run the helm schema command pointed at deploy/helm/values.yaml with flags
--schema-root.title "Appsmith Helm chart values", --schema-root.id
"https://helm.appsmith.com/values.schema.json" and -o values.schema.json
(matching .github/workflows/helm-schema.yml), overwrite the current
deploy/helm/values.schema.json with the generated file, verify the schema job
passes locally/CI, and commit the updated values.schema.json.
---
Outside diff comments:
In `@deploy/helm/templates/deployment.yaml`:
- Around line 186-215: The APPSMITH_REDIS_URL env entry for the bundled Redis is
being injected even when envFrom sources (secretName, secrets, externalSecrets)
could supply APPSMITH_REDIS_URL and would be overridden; update the guard around
the APPSMITH_REDIS_PASSWORD/APPSMITH_REDIS_URL block (the conditional that
currently checks .Values.redis.enabled, .Values.redis.auth.enabled and not
.Values.applicationConfig.APPSMITH_REDIS_URL) to also require that no alternate
env sources are configured (i.e. add checks for not .Values.secretName, not
.Values.secrets, and not .Values.externalSecrets.enabled) following the same
pattern used by appsmith.useOperatorMongo so the chart skips injecting the URL
whenever any envFrom-backed secret source might provide APPSMITH_REDIS_URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3bf6cbe9-c1d0-4c19-adb7-2ecabd6cebbe
⛔ Files ignored due to path filters (1)
deploy/helm/tests/__snapshot__/defaults_snapshot_test.yaml.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
deploy/helm/templates/_helpers.tpldeploy/helm/templates/configMap.yamldeploy/helm/templates/deployment.yamldeploy/helm/templates/hooks/redis.yamldeploy/helm/tests/redis_auth_test.yamldeploy/helm/values.schema.jsondeploy/helm/values.yaml
Comment on lines
+240
to
+248
| {{/* | ||
| Redis: master service hostname (FQDN inside the cluster). | ||
| Derived from the release name to stay uniform with the chart's other components. | ||
| Assumes the release name does not contain "redis" (otherwise the Bitnami subchart | ||
| collapses its fullname to just the release name and this host would not match). | ||
| */}} | ||
| {{- define "appsmith.redisMasterHost" -}} | ||
| {{- printf "%s-redis-master.%s.svc.cluster.local" .Release.Name (include "appsmith.namespace" .) -}} | ||
| {{- end -}} |
Contributor
There was a problem hiding this comment.
Derive the Redis host from the subchart fullname rules, not from .Release.Name.
This helper hardcodes <release>-redis-master, but the Bitnami Redis subchart does not always name its master service that way. If the release name already contains redis (the case called out in the PR notes), or if the subchart name is overridden, the actual Service name differs and every consumer of this helper points at a non-existent host. That breaks the init container, ConfigMap URL generation, and the app env wiring together.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/helm/templates/_helpers.tpl` around lines 240 - 248, The helper
appsmith.redisMasterHost currently hardcodes the host using .Release.Name which
can be wrong when the Bitnami redis subchart alters its fullname; change
appsmith.redisMasterHost to derive the service hostname from the redis
subchart's fullname helper instead (use the subchart's fullname include, e.g.
call include "redis.fullname" . or include "appsmith.redis.fullname" . as
appropriate) and use that result to build the FQDN (e.g.
"%s.%s.svc.cluster.local" with the included fullname and include
"appsmith.namespace" .) so the host matches the actual service name the subchart
creates.
…41875) ## Description Batch Yarn resolution upgrades and direct dependency bumps to fix 22 open Dependabot alerts across 12 packages. All are minor/patch version bumps with no breaking API changes. | Package | Old Version | New Version | Type | |---|---|---|---| | tar | various | 7.5.11 | resolution | | vite | various | 6.4.2 | resolution | | yaml (v1) | 1.10.2 | 1.10.3 | resolution | | yaml (v2) | 2.3.1 / 2.7.0 | 2.8.3 | resolution | | js-yaml | various | 3.14.2 | resolution | | mdast-util-to-hast | various | 13.2.1 | resolution | | ajv (v6) | 6.12.6 | 6.14.0 | resolution | | @babel/plugin-transform-modules-systemjs | various | 7.29.4 | resolution | | rollup (v4) | 4.43.0 | 4.59.0 | resolution | | glob (v10) | 10.4.5 | 10.5.0 | resolution | | glob (v11) | 11.0.0 | 11.1.0 | resolution | | mammoth | 1.9.0 | 1.12.0 | direct bump | | svgo | 3.2.0 | 3.3.3 | direct bump | Fixes https://linear.app/appsmith/issue/APP-15271 Slack thread: https://theappsmith.slack.com/archives/C09NG5BJ18S/p1780304878402439 ## Dependabot Alerts Resolved #620 @babel/plugin-transform-modules-systemjs (high) #573 vite (medium), #572 vite (high), #437 vite (medium), #429 vite (low), #428 vite (low) #538 yaml (medium), #535 yaml (medium) #524 tar (high), #519 tar (high), #493 tar (high), #482 tar (high), #469 tar (high), #466 tar (high) #517 svgo (high) #507 rollup (high) #497 ajv (medium) #454 mdast-util-to-hast (medium) #447 glob (high), #445 glob (high) #443 js-yaml (medium) #435 mammoth (medium) ## Automation /ok-to-test tags="@tag.All" ## Communication Should the DevRel and Marketing teams be notified? - [ ] Yes - [x] No ## Test Verification - Ran `yarn test:unit:ci` — **474 suites passed, 3583 tests passed** - 1 pre-existing failure in `QueryRender.test.tsx` (7 tests) — unrelated to these changes - No new test failures introduced by the dependency upgrades <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Upgraded mammoth dependency to version 1.11.0 * Updated svgo development dependency to version 3.3.3 * Added dependency resolution overrides to ensure consistent transitive package versioning across the project <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/26972153558> > Commit: e55e6a8 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=26972153558&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Thu, 04 Jun 2026 20:07:24 UTC <!-- end of auto-generated comment: Cypress test results -->
## Description Single-line version bump of `com.microsoft.sqlserver:mssql-jdbc` from `11.2.1.jre17` to `11.2.4.jre11` in the mssqlPlugin to resolve CVE-2025-59250 (improper input validation). **Why `jre11` instead of `jre17`?** Microsoft never published `11.2.4.jre17` — they stopped releasing the `jre17` classifier after `11.2.3`. The `jre11` classifier denotes the *minimum* supported JRE version and is fully compatible with JRE 17+. ## Verification - Maven compilation passes: `mvn compile -pl appsmith-plugins/mssqlPlugin -am` - Only 1 file changed: `app/server/appsmith-plugins/mssqlPlugin/pom.xml` ## References - Resolves Dependabot alert [#440](https://github.com/appsmithorg/appsmith/security/dependabot/440) (high severity) - CVE-2025-59250: JDBC Driver for SQL Server has improper input validation issue - Fixes https://linear.app/appsmith/issue/APP-15271 - Slack thread: https://theappsmith.slack.com/archives/C09NG5BJ18S/p1780304878402439 ## Automation /ok-to-test tags="@tag.All" ## Communication Should the Dev No-Code or No-Code App Devs Slack channel be notified of this change? - **NO** — internal dependency bump only <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated Microsoft SQL Server database driver for improved compatibility and stability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/27135835566> > Commit: 4c99c19 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=27135835566&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Mon, 08 Jun 2026 13:22:40 UTC <!-- end of auto-generated comment: Cypress test results -->
…ts (#41881) ## Description Upgrade TinyMCE from 6.8.5 to 7.9.3 and `@tinymce/tinymce-react` from ^6.0.0 to ^6.3.0 to resolve **8 high-severity XSS vulnerabilities** (CVSS 8.7) in the RichTextEditor widget. ### Changes Made | Package | Before | After | |---------|--------|-------| | `tinymce` | 6.8.5 | 7.9.3 | | `@tinymce/tinymce-react` | ^6.0.0 | ^6.3.0 | **RichTextEditor component updates:** - Added `license_key: 'gpl'` — required in TinyMCE 7 for self-hosted open-source deployments - Added `sandbox_iframes: true` — explicit opt-in to TinyMCE 7's new iframe sandboxing security default - Removed `print` from toolbar config — the print plugin was removed in TinyMCE 6 and was already non-functional **Dependabot config:** - Removed outdated ignore rule for `tinymce` version `6.8.3` ### TinyMCE 6 → 7 Breaking Changes Assessment | Breaking Change | Impact | Action | |----------------|--------|--------| | `license_key` now mandatory for self-hosted | Would show notification banner | Added `license_key: 'gpl'` | | `sandbox_iframes` defaults to true | Iframes in editor content get sandboxed | Explicit in config (security improvement) | | `convert_unsafe_embeds` defaults to true | Object/embed tags converted | Already set to `true` in existing code | | `text_patterns` trigger on Space (not Enter) | Markdown shortcuts fire on space | Better UX, no code change needed | | `template` plugin removed | N/A | Not used in our codebase | | `print` plugin removed (since v6) | Was in toolbar string | Removed from toolbar | | InsertOrderedList/UnorderedList from lists plugin only | Already using lists plugin | No change needed | All plugins currently imported (`advlist`, `autolink`, `lists`, `link`, `image`, `charmap`, `preview`, `anchor`, `searchreplace`, `visualblocks`, `code`, `fullscreen`, `insertdatetime`, `media`, `table`, `help`, `emoticons`) remain available in TinyMCE 7.9.3. ### Dependabot Alerts Resolved | Alert | Severity | CVE | Summary | First Patched | |-------|----------|-----|---------|---------------| | #643 | High | XSS via nested SVGs | Sanitization bypass through nested SVGs | 7.1.0 | | #644 | High | XSS via nested SVGs | Sanitization bypass through nested SVGs | 7.1.0 | | #645 | High | XSS via data-mce- attributes | XSS through data-mce- prefixed src/href/style | 7.9.3 | | #646 | High | XSS via data-mce- attributes | XSS through data-mce- prefixed src/href/style | 7.9.3 | | #647 | High | XSS via mce:protected | XSS through mce:protected comments | 7.9.3 | | #648 | High | XSS via mce:protected | XSS through mce:protected comments | 7.9.3 | | #649 | High | XSS via media plugin | XSS using media plugin data-mce-object injection | 7.9.3 | | #650 | High | XSS via media plugin | XSS using media plugin data-mce-object injection | 7.9.3 | ## Testing - [x] `yarn install` succeeds with new versions - [x] All 147 widget test suites pass (1318 tests, 0 failures) - [x] All imported plugins verified available in TinyMCE 7.9.3 - [x] Editor init config reviewed for deprecated/removed options - [x] No API surface changes required (Editor component, ui.registry, notificationManager all unchanged) ## Impact - **Widget affected:** RichTextEditor (single usage file) - **Risk:** Low — TinyMCE 7.x is API-compatible with 6.x for our usage pattern - **User-visible change:** Markdown shortcuts now trigger on Space instead of Enter (improved UX) Fixes https://linear.app/appsmith/issue/APP-15271 Slack thread: https://theappsmith.slack.com/archives/C09NG5BJ18S/p1780304878402439 /ok-to-test tags="@tag.Widget" **Communication** Should the Dev Rel team be notified of this change? NO <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Upgraded TinyMCE rich text editor to version 7.9.3 and its React wrapper to version 6.3.0 * Removed print and print preview options from the editor toolbar <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/27205029178> > Commit: af2f105 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=27205029178&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Widget` > Spec: > <hr>Tue, 09 Jun 2026 14:17:18 UTC <!-- end of auto-generated comment: Cypress test results -->
…#41885) ## Description Add Yarn resolution for `shell-quote@1.8.4` to patch "shell-quote quote() does not escape newlines in object .op values" (critical severity). This is a transitive dependency — not directly imported in source code. The resolution pins all transitive `shell-quote` references to `1.8.4`, which includes the fix. ## References - Fixes https://linear.app/appsmith/issue/APP-15271 - Dependabot alert: https://github.com/appsmithorg/appsmith/security/dependabot/651 - Slack thread: https://theappsmith.slack.com/archives/C09NG5BJ18S/p1780304878402439 ## Changes - `app/client/package.json`: added `"shell-quote": "1.8.4"` to `resolutions` - `app/client/yarn.lock`: regenerated (shell-quote now resolves to 1.8.4) ## Testing - `yarn install` succeeds - `rg "^\"shell-quote@" yarn.lock` confirms version 1.8.4 - `npx prettier --check package.json` passes /ok-to-test tags="@tag.Sanity" ## Communication Should the DevRel and Communication team be notified? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated package dependency versions to resolve compatibility requirements. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/27214112938> > Commit: e908dcc > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=27214112938&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Tue, 09 Jun 2026 17:05:57 UTC <!-- end of auto-generated comment: Cypress test results -->
…ues in sync (#41842) - Adds `README.md.gotmpl` template and generates the chart README via [helm-docs](https://github.com/norwoodj/helm-docs) instead of maintaining it by hand - Backfills `# --` (helm-docs description) and `# @section -- SectionName` annotations on all values in `values.yaml`, including subchart pass-through values - Organizes values into categorized tables: **Appsmith configuration**, **Global parameters**, **Workload configuration**, Networking, Persistence, Service account, External Secrets, MongoDB Community Operator, Redis/MongoDB/PostgreSQL (Bitnami subcharts), Monitoring - Reorders top-level keys so the most commonly edited sections (Appsmith config, global params, workload) appear first - No changes to templates, Chart.yaml, chart logic, or `values.schema.json` Three comment layers now coexist on each value (all are plain YAML comments, no functional impact): | Layer | Purpose | Consumed by | |-------|---------|-------------| | `## @param key.name Description` | Bitnami readme-generator compat | readme-generator-for-helm | | `# -- Description` | helm-docs value description | helm-docs | | `# @section -- SectionName` | helm-docs section grouping | helm-docs | | `# @Schema ...` | JSON schema generation | helm-values-schema-json | ```bash helm-docs --sort-values-order file ``` - [x] `helm-docs --sort-values-order file` produces a clean README with all values in their correct section tables - [x] `helm schema` produces an identical `values.schema.json` (no diff) - [x] `helm template appsmith .` still renders correctly - [x] Existing `helm-unittest` tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: Cypress test results --> > [!WARNING] > Tests have not run on the HEAD dc41fb2 yet > <hr>Tue, 09 Jun 2026 20:26:25 UTC <!-- end of auto-generated comment: Cypress test results --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **Documentation** * Major rewrite of the Helm chart README, added a documentation template and a guide on regenerating derived artifacts; expanded and reorganized the Values reference and annotation guidance. * **Chores** * CI workflow renamed and updated to install pinned tooling and enforce that generated Helm schema and README are kept in sync (PRs fail on drift). <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…oy/docker (#41873) Bumps golang from 1.26.3-alpine to 1.26.4-alpine. [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> <!-- This is an auto-generated comment: Cypress test results --> > [!WARNING] > Tests have not run on the HEAD a7b3610 yet > <hr>Wed, 03 Jun 2026 16:56:10 UTC <!-- end of auto-generated comment: Cypress test results --> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Enable authenticated Redis in the Helm chart. A pre-install/pre-upgrade hook Job generates the Redis password Secret (referenced by redis.auth.existingSecret) when it doesn't already exist, keeping the password stable across upgrades and ArgoCD re-syncs. - Add redisMasterHost/redisSecretName/redisPasswordInitImage helpers - Assemble an authenticated APPSMITH_REDIS_URL from the Secret, injected by reference so cleartext never lands in the ConfigMap; skipped when the user supplies their own APPSMITH_REDIS_URL - Gate the init-container ping and ConfigMap URL on redis.auth.enabled - Add redisAuth.passwordInit image config and values.schema.json entries - Add redis_auth_test snapshot/test coverage Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
deploy/helm/AGENTS.md (1)
24-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
AGENTS.mdmarks##@param`` optional, but repo rules require it.Line 28 conflicts with the chart annotation standard and can cause repeated drift in
values.yamldocs metadata. Please make##@param`` required in this guidance.As per coding guidelines, “Include the
##@paramkey.name Descriptioncomment line invalues.yamlfor Bitnami readme-generator compatibility.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/AGENTS.md` around lines 24 - 29, Update the guidance text so the `## `@param` key.name Description` line is required (not optional) when adding new values; specifically, change the sentence that currently marks `## `@param`` optional to mandate including `## `@param` key.name Description` for Bitnami readme-generator compatibility and keep the note to include `# -- Description` and `# `@section` -- Section Name` as well so helm-docs groups and documents values correctly; ensure the wording now reads that all three comment lines are required (with `## `@param`` mandatory) to prevent docs metadata drift.Source: Coding guidelines
.github/workflows/helm-docs.yml (1)
34-37:⚠️ Potential issue | 🟠 MajorPin the
helm-values-schema-jsonplugin version for deterministic CI.Line 37 installs the plugin from the repo without a version constraint; Helm will pull whatever the repo’s latest release is, which can change schema generation output and create noisy/flaky diffs. Pin it to
v2.4.0(latest stable).Suggested change
- run: helm plugin install --verify=false https://github.com/losisin/helm-values-schema-json.git + run: helm plugin install --verify=false https://github.com/losisin/helm-values-schema-json.git --version <pinned-tag>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/helm-docs.yml around lines 34 - 37, The helm plugin install step uses the bare repo URL for helm-values-schema-json which pulls the repo HEAD and makes CI non-deterministic; update the run line that calls "helm plugin install ... https://github.com/losisin/helm-values-schema-json.git" to pin the plugin to v2.4.0 (for example by using the git ref in the URL, e.g. append `@v2.4.0` or point to the v2.4.0 release archive) so the workflow always installs that exact version.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/helm/README.md`:
- Around line 62-80: The README's Redis table is stale relative to values.yaml:
update the documented keys (e.g., redis.auth.enabled) and add the new Redis
secret/bootstrap keys to match deploy/helm/values.yaml; regenerate the README
and JSON schema using the provided helm-docs command (run in deploy/helm:
helm-docs --sort-values-order file) so the table, default values, and new keys
are synchronized with values.yaml and will satisfy the CI
`.github/workflows/helm-docs.yml` check.
In `@deploy/helm/values.yaml`:
- Around line 358-366: The values.yaml is missing the required Bitnami
readme-generator `## `@param`` comments for the Redis keys; add `## `@param`` lines
immediately above the documented keys for enabled, existingSecret, and
existingSecretPasswordKey so they follow the pattern `## `@param` <key> <brief
description>` (e.g., describe that `enabled` toggles the Redis subchart,
`existingSecret` is the name of the Secret that holds the Redis password, and
`existingSecretPasswordKey` is the key in that Secret for the password) ensuring
the `## `@param`` exactly precedes the existing `# --`/`# `@section`` blocks to
restore readme-generator compatibility.
---
Outside diff comments:
In @.github/workflows/helm-docs.yml:
- Around line 34-37: The helm plugin install step uses the bare repo URL for
helm-values-schema-json which pulls the repo HEAD and makes CI
non-deterministic; update the run line that calls "helm plugin install ...
https://github.com/losisin/helm-values-schema-json.git" to pin the plugin to
v2.4.0 (for example by using the git ref in the URL, e.g. append `@v2.4.0` or
point to the v2.4.0 release archive) so the workflow always installs that exact
version.
In `@deploy/helm/AGENTS.md`:
- Around line 24-29: Update the guidance text so the `## `@param` key.name
Description` line is required (not optional) when adding new values;
specifically, change the sentence that currently marks `## `@param`` optional to
mandate including `## `@param` key.name Description` for Bitnami readme-generator
compatibility and keep the note to include `# -- Description` and `# `@section` --
Section Name` as well so helm-docs groups and documents values correctly; ensure
the wording now reads that all three comment lines are required (with `##
`@param`` mandatory) to prevent docs metadata drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5725ed61-4ccd-4e0c-aa39-1889207ff41b
⛔ Files ignored due to path filters (1)
app/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
.github/dependabot.yml.github/workflows/helm-docs.ymlapp/client/cypress/support/Objects/CommonLocators.tsapp/client/package.jsonapp/client/packages/icons/package.jsonapp/client/src/widgets/RichTextEditorWidget/component/index.tsxapp/server/appsmith-plugins/mssqlPlugin/pom.xmldeploy/docker/base.dockerfiledeploy/helm/AGENTS.mddeploy/helm/README.mddeploy/helm/README.md.gotmpldeploy/helm/values.yaml
💤 Files with no reviewable changes (1)
- .github/dependabot.yml
✅ Files skipped from review due to trivial changes (3)
- app/client/packages/icons/package.json
- app/server/appsmith-plugins/mssqlPlugin/pom.xml
- deploy/helm/README.md.gotmpl
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
deploy/helm/values.yaml (1)
359-367:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd missing
##@param`` lines for newly documented values.The values
redis.auth.enabled(line 362),redis.auth.existingSecret(line 365), andredis.auth.existingSecretPasswordKey(line 368) are documented with# --and#@sectioncomments but are missing the required `## `@paramlines. Per coding guidelines, documented values invalues.yamlmust use the three-layer comment format.📝 Suggested pattern
# `@schema` hidden: true auth: + ## `@param` redis.auth.enabled Enable Redis authentication # -- Enable Redis authentication # `@section` -- Redis (Bitnami subchart) enabled: true + ## `@param` redis.auth.existingSecret Name of the Secret holding the Redis password # -- Name of the Secret holding the Redis password. Bootstrapped by the redisAuth.passwordInit Job when absent (see redisAuth below) # `@section` -- Redis (Bitnami subchart) existingSecret: appsmith-redis-secret + ## `@param` redis.auth.existingSecretPasswordKey Key within existingSecret that holds the Redis password # -- Key within existingSecret that holds the Redis password # `@section` -- Redis (Bitnami subchart) existingSecretPasswordKey: redis-passwordAs per coding guidelines, use the three-layer comment format (
##@param``,# --, and `# `@section` --`) for documented values in `values.yaml`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/values.yaml` around lines 359 - 367, Add the missing top-layer doc comments for the three documented values by inserting a corresponding "## `@param`" line above each "# --" comment for redis.auth.enabled, redis.auth.existingSecret, and redis.auth.existingSecretPasswordKey; for each param include the parameter name, short description, and type/default (e.g., boolean/default true for redis.auth.enabled, string/default appsmith-redis-secret for redis.auth.existingSecret, and string/default redis-password for redis.auth.existingSecretPasswordKey) so the file follows the three-layer comment format (## `@param`, # --, # `@section`).Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@deploy/helm/values.yaml`:
- Around line 359-367: Add the missing top-layer doc comments for the three
documented values by inserting a corresponding "## `@param`" line above each "#
--" comment for redis.auth.enabled, redis.auth.existingSecret, and
redis.auth.existingSecretPasswordKey; for each param include the parameter name,
short description, and type/default (e.g., boolean/default true for
redis.auth.enabled, string/default appsmith-redis-secret for
redis.auth.existingSecret, and string/default redis-password for
redis.auth.existingSecretPasswordKey) so the file follows the three-layer
comment format (## `@param`, # --, # `@section`).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aed1b83f-367a-4274-9ff6-f016b3778c52
📒 Files selected for processing (1)
deploy/helm/values.yaml
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds authenticated Redis support to the Helm chart. A pre-install/pre-upgrade hook Job generates the Redis password Secret (referenced by
redis.auth.existingSecret) when it does not already exist, keeping the password stable across upgrades and ArgoCD re-syncs. The password is injected into the Appsmith deployment by Secret reference, so cleartext never lands in the ConfigMap.What changed
appsmith.redisMasterHost,appsmith.redisSecretName, andappsmith.redisPasswordInitImage.APPSMITH_REDIS_URLfrom the Secret (password var precedes the URL that references it via$(VAR)); skipped when the user supplies their ownapplicationConfig.APPSMITH_REDIS_URL.pingnow passesREDISCLI_AUTHwhen auth is enabled.redis.auth.enabledand routed through the shared host helper.redisAuth.passwordInitimage config (defaults toalpine/kubectl) plus a pre-install/pre-upgrade hook (templates/hooks/redis.yaml) that bootstraps the password Secret.values.yamlnow enablesredis.authby default withexistingSecret: appsmith-redis-secret/existingSecretPasswordKey: redis-password.values.schema.jsonentries and snapshot/test coverage (tests/redis_auth_test.yaml, updated default snapshot).Reviewer notes
redisAuthis intentionally kept separate fromredisso it is never forwarded to the Bitnami subchart.passwordInit.image.tagdefaults to a floatinglatestbecause upstreamalpine/kubectlperiodically retires older patch tags; pin it for byte-level reproducibility.Automation
/ok-to-test tags=""
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Improvements
Tests
Documentation
Chores
Warning
Tests have not run on the HEAD 4e79144 yet
Tue, 09 Jun 2026 22:03:13 UTC