chore(helm): generate values.schema.json from values.yaml - #41780
Merged
Merged
Conversation
Annotates values.yaml with `# @schema` comments so the helm-values-schema-json plugin can generate values.schema.json with full top-level coverage (43/43 keys vs. 4/43 previously). Dependency pass-throughs (redis, mongodb, postgresql, prometheus, mongodbOperator) use additionalProperties: true with their internals hidden — only the chart-owned `enabled` condition flag is typed. Adds a CI workflow that regenerates the schema on PR and fails on drift, ensuring values.yaml and values.schema.json stay in sync.
Replace placeholder schema $id with the actual chart repo URL.
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR implements comprehensive Helm chart values schema validation: adds a draft-2020-12 JSON Schema, annotates values.yaml, adds Helm unit tests for schema validation, enforces schema drift detection in CI, updates Helm release packaging/indexing, and documents schema regeneration. ChangesHelm Values Schema Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
13 cases covering schema enforcement: negative tests for invalid service.type, replicas, workload.kind, image.pullPolicy, service.port, mongodbCommunity.name/members, redis.enabled type; positive tests for defaults, applicationConfig extras, and dependency pass-throughs.
- values.yaml: annotate persistence.existingClaim.{enabled,name,
claimName} and persistence.efs.{enabled,driver,volumeHandle} with
multi-type schemas (boolean|null, string|null) so they accept their
real-world override types. Auto-generation had inferred bare null
from the empty defaults, which broke pre-existing unittests that
override these fields.
- helm-schema.yml: pin helm to v4.1.4 and add --verify=false to the
plugin install (Helm 4 requires it; the plugin source ships no
verification metadata).
- helm-unittest.yml: rename job from "publish" (a copy-paste from the
publish workflow) to "unittest".
The schema previously typed each enumerated APPSMITH_* key as string, rejecting natural YAML like APPSMITH_DISABLE_TELEMETRY: true or APPSMITH_MAIL_PORT: 587. Helm and the Kubernetes API stringify scalar values when rendering env vars, so this rejection was purely a schema artifact, not a runtime requirement. Hide the enumerated keys from the schema (keep them in values.yaml as in-file documentation) and broaden additionalProperties to accept any scalar type [string, boolean, integer, number]. Adds a unittest case covering the user-reported scenario.
Restructures the chart publish workflow: - Adds a release channel: pushes to release branch publish as <chart-yaml-version>-release.<short-sha>, hidden from default `helm install` (clients must pass --devel). Master keeps publishing the on-disk version verbatim as stable. - PRs touching deploy/helm/** now run a pre-flight version check that fails if the on-disk Chart.yaml version was already published as a stable release. Catches "I forgot to bump the version" before merge. - Single combined job (was two): version-collision check uses curl HEAD against the public chart URL, no AWS credentials needed, so it applies identically on PRs (including from forks) and on push. - index.yaml is regenerated from current bucket state on every publish rather than --merge appended. This decouples cleanup from publish: any S3 lifecycle rule that expires release-channel tarballs is automatically reflected in the next published index. - values.schema.json is now uploaded alongside the chart on stable publishes, enabling IDE schema validation via `# yaml-language-server: $schema=https://helm.appsmith.com/values.schema.json`. - index.yaml gets short Cache-Control (max-age=60) so `helm repo update` picks up new versions promptly even behind a CDN. - Bumps Helm to v4.1.4 (was v3.6.3) and setup-helm to v4 (was v1). - Switches HELM_S3_BUCKET / HELM_REPO_URL from secrets to repository variables so the validate-version path works on fork PRs. Naming matches what each value actually represents (bucket name vs. URL). - Drops workflow_dispatch (re-runs from the Actions UI cover the rare manual republish case) and the unused `helm repo add bitnami` step (helm dep build resolves dependencies via Chart.yaml URLs).
wyattwalter
marked this pull request as ready for review
May 7, 2026 19:27
![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: 5
🧹 Nitpick comments (2)
.github/workflows/helm-release.yml (2)
88-95: 💤 Low value
HELM_REPO_URLtrailing slash will produce//appsmith-...tgz.If anyone sets
vars.HELM_REPO_URLtohttps://helm.appsmith.com/(with the trailing/), the concatenation here yields a double slash. Most CDNs and S3 will still resolve it, but it's brittle and can confuse caching layers. Strip or normalize once at the top of the script.🧹 Suggested normalization
- base="${{ steps.chart-version.outputs.base }}" - tarball_url="${{ vars.HELM_REPO_URL }}/appsmith-${base}.tgz" + base="${{ steps.chart-version.outputs.base }}" + repo_url="${{ vars.HELM_REPO_URL }}" + tarball_url="${repo_url%/}/appsmith-${base}.tgz"🤖 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-release.yml around lines 88 - 95, Normalize vars.HELM_REPO_URL by stripping any trailing slash before it's used to build tarball_url so you don't produce a double slash; e.g., create a local variable (used where tarball_url is composed) that trims a trailing '/' from vars.HELM_REPO_URL and then construct tarball_url="${HELM_REPO_URL_NORMALIZED}/appsmith-${{ steps.chart-version.outputs.base }}.tgz" so tarball_url and the curl check never contain "//". Ensure the normalization runs once near the top of the job/step and use the normalized variable in the existing tarball_url and echo messages.
49-52: Consider upgrading toazure/setup-helm@v5. The action works as configured—azure/setup-helm@v4supports Helm 4.1.4 with explicit version pinning. However, v5 is now the latest major version and is the recommended setup going forward.🤖 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-release.yml around lines 49 - 52, The workflow step "Setup Helm" uses the action reference azure/setup-helm@v4; update that to azure/setup-helm@v5 to use the latest major release (keep the existing with: version: v4.1.4 if you want to pin the Helm binary). Locate the step that currently reads azure/setup-helm@v4 (the "Setup Helm" step) and change the action reference to azure/setup-helm@v5.
🤖 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 @.cursor/rules/regen-helm-schema.mdc:
- Line 53: The macOS-only plugin path string
`~/Library/helm/plugins/helm-values-schema-json.git/docs/README.md` should be
replaced with a cross-platform instruction: either reference `$(helm env
HELM_PLUGINS)/helm-values-schema-json.git/docs/README.md` (or `helm env
HELM_PLUGINS` in prose) or list the three platform-specific locations
(`~/Library/helm/...` for macOS, `~/.local/share/helm/plugins/...` for Linux,
and `%APPDATA%\helm\plugins\...` for Windows) so readers on Linux/Windows can
find the plugin docs.
In @.github/workflows/helm-release.yml:
- Around line 138-141: The S3 upload of values.schema.json uses aws s3 cp
without a content type, causing the file to be served as
application/octet-stream; update the aws s3 cp invocation that uploads
values.schema.json (the conditional block checking channel == "stable") to
include --content-type "application/json". Also consider adding --content-type
"application/x-yaml" (or "text/yaml") to the aws s3 cp that uploads index.yaml
to ensure IDEs and clients detect the YAML content correctly.
In `@deploy/helm/values.schema.json`:
- Line 1: The values.schema.json in deploy/helm is out of date; install the
helm-values-schema-json plugin if missing (helm plugin install ... or check with
helm plugin list), then regenerate the schema by running the helm schema command
shown (helm schema --schema-root.title "Appsmith Helm chart values"
--schema-root.id "https://helm.appsmith.com/values.schema.json" -o
values.schema.json) from the deploy/helm directory, review the changes with git
diff deploy/helm/values.schema.json to ensure only intended edits are present,
and commit the updated deploy/helm/values.schema.json.
- Around line 83-86: The schema entry for customCAcert is currently typed as
null so users cannot supply certificate objects; update deploy/helm/values.yaml
to annotate customCAcert as allowing both object and null (so the generator
emits ["object","null"] for the customCAcert schema) and then regenerate
deploy/helm/values.schema.json; specifically modify the customCAcert
default/type annotation in values.yaml (the symbol customCAcert) to be a
multi-type (object or null), run the helm/values schema generation step used in
this repo, and confirm deploy/helm/values.schema.json now permits object values
as well as null.
In `@deploy/helm/values.yaml`:
- Around line 455-460: The `customCAcert` key is being treated as null because
its YAML lacks a schema annotation; update the `customCAcert` entry to declare
it as a map of PEM strings (e.g. add schema annotations that set the type to
object and the additionalProperties/item type to string for `customCAcert`) so
the generated values.schema.json permits a mapping of filenames->PEM contents,
then regenerate the Helm chart's values.schema.json after saving the change.
---
Nitpick comments:
In @.github/workflows/helm-release.yml:
- Around line 88-95: Normalize vars.HELM_REPO_URL by stripping any trailing
slash before it's used to build tarball_url so you don't produce a double slash;
e.g., create a local variable (used where tarball_url is composed) that trims a
trailing '/' from vars.HELM_REPO_URL and then construct
tarball_url="${HELM_REPO_URL_NORMALIZED}/appsmith-${{
steps.chart-version.outputs.base }}.tgz" so tarball_url and the curl check never
contain "//". Ensure the normalization runs once near the top of the job/step
and use the normalized variable in the existing tarball_url and echo messages.
- Around line 49-52: The workflow step "Setup Helm" uses the action reference
azure/setup-helm@v4; update that to azure/setup-helm@v5 to use the latest major
release (keep the existing with: version: v4.1.4 if you want to pin the Helm
binary). Locate the step that currently reads azure/setup-helm@v4 (the "Setup
Helm" step) and change the action reference to azure/setup-helm@v5.
🪄 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: 0215da7d-6285-4779-baff-7d7d804100df
📒 Files selected for processing (7)
.cursor/rules/regen-helm-schema.mdc.github/workflows/helm-release.yml.github/workflows/helm-schema.yml.github/workflows/helm-unittest.ymldeploy/helm/tests/values_schema_test.yamldeploy/helm/values.schema.jsondeploy/helm/values.yaml
sebastianiv21
previously approved these changes
May 7, 2026
- customCAcert: annotate as type: [object, "null"] with string-valued additionalProperties so the schema accepts a map of filename->PEM, not just the bare null default. - helm-release.yml: add Content-Type to the S3 uploads of values.schema.json (application/json) and index.yaml (application/x-yaml) so IDEs and clients detect them correctly rather than seeing application/octet-stream. - helm-release.yml: strip any trailing slash from HELM_REPO_URL before building the tarball URL so the existence check never produces a doubled slash. - regen-helm-schema.mdc: replace the macOS-only plugin docs path with `helm env HELM_PLUGINS` (and platform notes) so the reference works on Linux and Windows too. Skipped: setup-helm@v5 bump (v4 still current and works; can bump in a follow-up).
sebastianiv21
self-requested a review
May 7, 2026 23:30
sebastianiv21
previously approved these changes
May 7, 2026
…ndex Several follow-ups from review: - s3 cp doesn't accept --tagging; switch the release-channel upload to s3api put-object so the channel=release tag actually applies. - Revert to append-only `helm repo index --merge` so existing entries keep their original `created` timestamps. Pruning entries for expired release-channel tarballs is left to a separate cleanup workflow. - Package into a temp dir via `helm package --destination` and run `helm repo index` against that same dir, so subchart tarballs left in ./charts/ by `helm dep build` are no longer added to index.yaml as bogus top-level entries. - Hoist the temp path into a `pkg_dir` variable rooted at $RUNNER_TEMP. - Tighten comments to forward-relevant rationale.
Minor bump to reflect the addition of comprehensive values.schema.json validation, IDE schema hosting, and the release-channel publish flow.
sebastianiv21
self-requested a review
May 8, 2026 17:07
sebastianiv21
approved these changes
May 8, 2026
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.
This PR has two coordinated parts: (1) a complete rewrite of the chart's
values.schema.json(auto-generated fromvalues.yaml), and (2) a publish-workflow restructure to add a release channel, PR pre-flight checks, and schema URL hosting.They're together because the workflow surfaces and validates the new schema, and would conflict on
helm-release.ymlif split into separate PRs.Part 1: values.schema.json — full coverage, auto-generated
What changed
deploy/helm/values.yamlwith# @schemacomments so the helm-values-schema-json plugin can generatevalues.schema.jsonwith full top-level coverage (43/43 keys vs. 4/43 previously).redis,mongodb,postgresql,prometheus,mongodbOperator) useadditionalProperties: truewith their internals hidden — only the chart-ownedenabledcondition flag is typed (boolean).applicationConfigaccepts any scalar type (string,boolean,integer,number) for any key, since Helm/K8s stringify env-var values when rendering. Earlier draft enforced string-only, which rejected natural YAML likeAPPSMITH_DISABLE_TELEMETRY: true..github/workflows/helm-schema.ymlregenerates the schema on PR and fails on drift betweenvalues.yamlandvalues.schema.json. Pinned to Helm 4.1.4.tests/values_schema_test.yamlhelm-unittest suite (14 cases) exercises rejection of invalid input and acceptance of pass-through values. Picked up automatically by the existingHelm Unit Testsworkflow..cursor/rules/regen-helm-schema.mdc) auto-attaches when editingvalues.yamlorvalues.schema.jsonto remind contributors how to regenerate.Behavior changes worth flagging
mongodbOperator.enabledwas previously the only validated key undermongodbOperator. It still is — the rest of the block stays opaque (any subchart values pass through unvalidated). Same model now applied uniformly to all five dependency pass-throughs.persistence.existingClaim.{enabled,name,claimName}andpersistence.efs.{enabled,driver,volumeHandle}had barenulldefaults invalues.yaml(e.g.enabled:with no value). The auto-generated schema initially typed these asnull-only, which would have rejected real-world overrides likepersistence.existingClaim.enabled: true. They are now annotated as multi-type ([boolean, \"null\"]or[string, \"null\"]) so they accept both the empty default and a concrete override. Tightening of a previously-empty schema, not a loosening.applicationConfig: enumeratedAPPSMITH_*keys are no longer typed individually (they're hidden from the schema and serve as in-file documentation). The block validates viaadditionalPropertiesonly, accepting any scalar value for any key. Trade-off: IDEs no longer auto-complete the specific known keys, but the block is now permissive for the real-world shapes that Helm renders fine.How to regenerate the schema locally
Plugin install (one-time):
The Cursor rule provides this as on-demand context when editing the relevant files.
Part 2: helm-release.yml — release channel + PR pre-flight + schema hosting
What changed
Adds a release channel. Pushes to the
releasebranch publish as<chart-yaml-version>-release.<short-sha>(e.g.3.8.0-release.abc1234). SemVer treats the-release.SHAsuffix as a pre-release, so defaulthelm install yourrepo/appsmithskips these — clients must pass--develto opt in. Master keeps publishing the on-disk version verbatim as stable.PR pre-flight version check. PRs touching
deploy/helm/**now run acurl HEADagainst the public chart URL for the on-disk version. If a tarball at that version already exists, CI fails with an actionable message:Catches "I forgot to bump the version" before the PR lands. No AWS creds needed — works on fork PRs.
Single combined job (was two): version-collision check is shared between PR and push paths. Same code, same logic, same failure mode.
index.yamlregenerated from bucket state on every publish rather than--mergeappended. This decouples cleanup from publish: any S3 lifecycle rule that expires-release.*tarballs is automatically reflected in the next published index, with no separate cleanup workflow needed.values.schema.jsonis now uploaded alongside the chart on stable publishes. Enables IDE schema validation via# yaml-language-server: $schema=https://helm.appsmith.com/values.schema.jsonin any values.yaml file.Cache-Control: public, max-age=60onindex.yamlandvalues.schema.jsonsohelm repo updateand IDE schema fetches pick up new versions promptly even if the bucket is fronted by a CDN. Tarballs (content-addressed via digest in index) keep default caching.Helm 4.1.4 (was v3.6.3) via
setup-helm@v4(was v1).HELM_S3_BUCKETandHELM_REPO_URLare now repository variables, not secrets. Both values are public (bucket name and chart URL). Variables are accessible to fork PRs (unlike secrets), which is required for the version-check path. Naming matches what each value represents:vars.HELM_S3_BUCKET— bucket name foraws s3 …operationsvars.HELM_REPO_URL— full URL (incl. scheme) clients fetch charts fromDrops
workflow_dispatch(Actions UI re-runs cover the rare manual case) and the unusedhelm repo add bitnamistep (helm dep buildresolves all dependencies from Chart.yaml URLs without needing repos pre-registered).Behavior matrix
refs/pull/N/mergerefs/heads/masterrefs/heads/releasebase-release.SHA)Test plan
Helm Values Schemaworkflow passes (regenerates schema, finds no drift) — passing on most recent runHelm Unit Testsworkflow passes (existing 53 tests + new 14 schema cases = 67 total)helm lint deploy/helm/succeedsapplicationConfigboolean/integer regressions that drove the multi-type fixvars.HELM_S3_BUCKETandvars.HELM_REPO_URLare set in the repo Actions settingss3:PutObjectTaggingpermission (needed for the channel tag on release-channel uploads)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores
Documentation
Warning
Tests have not run on the HEAD f2fc338 yet
Fri, 08 May 2026 17:01:09 UTC