improvement(helm): hygiene pass — CI gating, strict values schema, ESO v1 default by waleedlatif1 · Pull Request #5939 · simstudioai/sim

waleedlatif1 · 2026-07-24T20:03:06Z

Summary

Closes the gaps from a best-practices audit of the chart against the official Helm guidance and current ecosystem tooling norms (template-level conformance was already clean: full app.kubernetes.io/* label coverage, 82 unit tests, kubeconform-valid renders, complete Chart.yaml):

CI gating (was: none): new Helm Chart workflow on helm/sim/** changes — helm lint, the 82 helm-unittest cases, kubeconform validation (k8s 1.29 strict + CRD catalog) of the default and all-components renders, and a render of all 10 example values files. Until now the chart's test suite only ran when someone remembered to run it locally.
helm/sim/ci/ values files (chart-testing convention) with dummy secrets, so the chart lints/templates cleanly out of the box — previously helm template with no values errored on the required-secret guards.
Strict values schema: all 30 top-level keys declared (16 — networkPolicy, monitoring, serviceAccount, migrations, cronjobs, etc. — were invisible to validation) plus root additionalProperties: false, so a top-level typo like networkPolciy: fails at install time instead of being silently ignored.
ESO default v1beta1 → v1 (time-sensitive): External Secrets Operator removed the v1beta1 compatibility path upstream in 2026, so the old default produced rejected manifests for anyone on a current ESO. v1beta1 remains available via externalSecrets.apiVersion for ESO < 0.17; NOTES and values comments updated.
wait-for-postgres init container gets requests/limits — the only container in the chart without them (broke ResourceQuota'd namespaces).
Dead telemetry scrape config removed — the collector's Prometheus receiver scraped /metrics on app/realtime, which neither exposes, and rendered a realtime target even with realtime disabled; OTLP ingestion unchanged.
Chart bumped to 1.2.0 with README upgrade notes for the ESO default and schema strictness.

Deliberately deferred (documented in the audit): helm.sh/resource-policy: keep on standalone PVCs (README documents uninstall behavior), HPA null-metrics guard (unreachable with defaults), helm-docs table + OCI publishing (chart is git-clone-consumed by design), telemetry collector image bump (needs its own config-migration change).

Type of Change

Improvement

Testing

Every workflow step dry-run locally: lint clean, 82/82 unit tests, kubeconform 30/30 (default) and 61/61 (all components) valid, all 10 examples render through the exact CI command (which caught and fixed a wrong kubeconform template variable before it ever hit CI)
Strict schema verified both ways: a deliberate top-level typo is rejected; all 10 examples still validate

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…O v1 default, ci values Closes the gaps from a best-practices audit of the chart (template-level conformance was already clean: full label set, 82 unit tests, kubeconform- valid renders): - new Helm Chart workflow gates every chart change: helm lint, the 82 helm-unittest cases, kubeconform validation of default + all-components renders (k8s 1.29 strict, CRD catalog), and a render of all 10 example values files - helm/sim/ci/ values files (chart-testing convention) so the chart lints and templates cleanly out of the box with dummy secrets - values.schema.json declares all 30 top-level keys (16 were invisible) and sets root additionalProperties: false, so top-level typos fail fast - externalSecrets.apiVersion defaults to v1: current ESO releases removed the v1beta1 compatibility path in 2026, so the old default produced rejected manifests on new installs; NOTES/values comments updated - wait-for-postgres init container gets requests/limits (the only container in the chart without them; broke ResourceQuota'd namespaces) - drop the telemetry Prometheus scrape config for app/realtime — neither exposes /metrics, so it was dead config that also rendered a realtime target with realtime disabled - chart 1.2.0 with README upgrade notes

vercel · 2026-07-24T20:03:12Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
docs	Ready	Preview, Comment	Jul 24, 2026 9:14pm

waleedlatif1 · 2026-07-24T20:03:13Z

@greptile

waleedlatif1 · 2026-07-24T20:03:14Z

@cursor review

cursor · 2026-07-24T20:03:17Z

PR Summary

Medium Risk
Upgrades may fail schema validation on stray values keys or reject ExternalSecrets on ESO < 0.17 unless externalSecrets.apiVersion is pinned; default image tag change affects unpinned installs.

Overview
Introduces a Helm Chart GitHub workflow on helm/sim/** that runs helm lint, helm-unittest (Docker image), kubeconform on default and full renders, templates all example values files, enforces a Chart.yaml version bump when chart files change on PRs, and installs on kind with helm test.

Ships helm/sim/ci/ dummy values (default, full, kind overlays) so lint/template/install tests render without real secrets.

Chart 1.2.0 sets appVersion to v0.7.44 (replacing a non-existent default tag) and documents upgrade notes in the README.

externalSecrets.apiVersion defaults to v1 (was v1beta1) across ExternalSecret templates, values.yaml, and NOTES — for current ESO; older clusters can still set v1beta1.

values.schema.json now lists previously missing top-level keys and sets additionalProperties: false, so unknown top-level values keys fail at install; unit tests cover naming overrides and relaxed schema error patterns.

Removes the telemetry collector’s Prometheus scrape jobs for app/realtime (no /metrics); metrics pipeline uses OTLP only. Adds resource requests/limits on the copilot migrations wait-for-postgres init container.

^{Reviewed by Cursor Bugbot for commit 1cfaf0f. Configure here.}

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 187e266. Configure here.}

…gth suite Helm v4 phrases schema rejections as 'minLength: got N, want 32' while v3 says 'String length must be greater than or equal to 32'. The suite grepped for 'minLength' only, so it passed on local helm v4 and failed on CI's v3.16.4 — the enforcement itself works on both. Patterns now assert the key name plus either wording. Caught by the new Helm Chart workflow on its very first run.

waleedlatif1 · 2026-07-24T20:14:09Z

@greptile

waleedlatif1 · 2026-07-24T20:14:10Z

@cursor review

greptile-apps · 2026-07-24T20:18:08Z

Greptile Summary

The follow-up changes complete the previously requested Helm CI and strict-schema fixes.

Pins GitHub Actions and the helm-unittest image to immutable revisions and verifies the kubeconform archive checksum.
Declares nameOverride and fullnameOverride as root schema properties and adds naming-override render coverage.
Adds chart validation and installation jobs, updates the ESO default to v1, and refreshes chart configuration and documentation.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failures remain in the fixes covered by the previous review threads.

No blocking failure remains.

Important Files Changed

Filename	Overview
.github/workflows/helm.yml	Adds Helm linting, unit tests, schema validation, version gating, and kind installation with immutable CI dependencies.
helm/sim/values.schema.json	Enables strict root-key validation while declaring the standard Helm naming overrides.
helm/sim/tests/smoke_test.yaml	Adds render coverage confirming that both naming overrides affect generated resource names.
helm/sim/values.yaml	Changes the default External Secrets API version to v1 and updates image-tag guidance.
helm/sim/templates/telemetry.yaml	Removes the unused Prometheus scrape receiver while retaining OTLP metrics ingestion.

_{Reviews (7): Last reviewed commit: "fix(helm): appVersion points at a real G..." | Re-trigger Greptile}

greptile-apps · 2026-07-24T20:18:15Z

Greptile Summary

The PR adds Helm-chart CI validation and stricter values checking while updating ESO and chart runtime defaults.

Adds lint, unit-test, kubeconform, and example-rendering workflow gates with dedicated CI values.
Enables strict root-level values-schema validation and declares previously omitted chart sections.
Changes ExternalSecret resources to use external-secrets.io/v1 by default.
Adds resources to the Copilot PostgreSQL readiness init container.
Removes obsolete Prometheus scrape receivers from the telemetry collector.
Bumps the chart to 1.2.0 and documents upgrade behavior.

Confidence Score: 3/5

The PR should not merge until the strict schema accepts the chart's supported name overrides; the CI dependency pinning concern is non-blocking.

Root schema strictness deterministically rejects nameOverride and fullnameOverride before templates can consume them, breaking installs and upgrades that use those supported Helm naming controls.

helm/sim/values.schema.json and .github/workflows/helm.yml

Security Review

The workflow's action tags and downloaded kubeconform executable are not immutably pinned or integrity-verified, leaving a non-blocking CI supply-chain hardening gap. The scanner-reported values are clearly labeled dummy CI credentials and have no documented deployment path, so they are not treated as leaked secrets.

Important Files Changed

Filename	Overview
helm/sim/values.schema.json	Adds root strictness but omits the template-supported nameOverride and fullnameOverride keys, breaking those installations.
.github/workflows/helm.yml	Adds comprehensive chart validation, with a non-blocking integrity gap from mutable action references and an unchecked binary download.
helm/sim/templates/telemetry.yaml	Removes dead Prometheus scraping while retaining OTLP ingestion and configured metrics exporters.
helm/sim/templates/external-secret-app.yaml	Updates the generated ExternalSecret API default to v1 consistently with chart values and documentation.
helm/sim/templates/job-copilot-migrations.yaml	Adds fixed requests and limits to the PostgreSQL readiness init container.
helm/sim/values.yaml	Changes the External Secrets Operator API default to v1 and documents the legacy override.

_{Reviews (1): Last reviewed commit: "fix(helm): version-agnostic schema-error..." | Re-trigger Greptile}

Benchmarked against the flagship OSS charts (ingress-nginx, argo-cd, kube-prometheus-stack, grafana, bitnami, cert-manager): sim already exceeds most of them on validation rigor (strict schema — 4 of 6 ship none; 82 unit tests vs argo-cd's zero; kubeconform manifest validation none of them run), but every top community chart repo actually installs the chart on a kind cluster in chart CI — the one majority practice we lacked. Adds: - install job: kind cluster, helm install with ci/default + a new small-footprint ci/kind-values.yaml overlay (default app requests of 4Gi can't schedule on a CI node), --wait, then the chart's helm test hook, with pod/event/log diagnostics on failure - version-bump job (PR-only): fails when helm/sim/** changes without a Chart.yaml version increment — argo/kps/grafana all enforce this

…actions - nameOverride/fullnameOverride are consumed by sim.name/sim.fullname but were never in values.yaml, so root additionalProperties: false rejected Helm's standard naming overrides — both now declared (a helper-wide sweep confirmed they were the only template-read keys missing), with a smoke regression test that installs under the strict schema and asserts the override lands in resource names - CI supply-chain hardening: checkout/setup-helm/kind-action pinned to full commit SHAs (tag comments retained) and the kubeconform archive verified against its published sha256

waleedlatif1 · 2026-07-24T20:24:03Z

@greptile

waleedlatif1 · 2026-07-24T20:24:04Z

@cursor review

greptile-apps · 2026-07-24T20:29:07Z

Greptile Summary

Adds comprehensive Helm-chart CI and tightens chart defaults and validation.

Adds lint, unit-test, kubeconform, and example-render workflow gates.
Declares all supported top-level values and rejects unknown top-level keys.
Defaults External Secrets resources to v1 while retaining an explicit legacy option.
Adds init-container resources and removes an ineffective Prometheus scrape receiver.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking opportunity to integrity-pin the executable tooling introduced in the Helm workflow.

The chart behavior and strict-schema changes remain internally consistent, while the only accepted concern is conditional exposure of the read-only CI runner if an unverified upstream executable artifact is compromised.

.github/workflows/helm.yml

Security Review

The new workflow executes remotely fetched Helm and kubeconform tooling without immutable pinning or artifact-integrity verification, leaving a non-blocking CI supply-chain hardening gap.

Important Files Changed

Filename	Overview
.github/workflows/helm.yml	Adds useful chart validation gates, but executable third-party tooling is fetched without immutable or integrity-verified pinning.
helm/sim/values.schema.json	Adds compatible declarations for previously omitted top-level chart values and rejects unknown root keys.
helm/sim/templates/telemetry.yaml	Removes the ineffective app/realtime Prometheus receiver while preserving OTLP ingestion and remote-write exporting.
helm/sim/templates/external-secret-app.yaml	Switches ExternalSecret rendering to the current v1 API while retaining configurable legacy-version support.
helm/sim/templates/job-copilot-migrations.yaml	Adds explicit resource requests and limits to the PostgreSQL readiness init container.

_{Reviews (3): Last reviewed commit: "fix(helm): declare naming overrides in t..." | Re-trigger Greptile}

- helm-unittest runs via the project's official docker image pinned by immutable sha256 digest instead of a plugin install from a mutable git tag - the version-bump gate fetches the full base ref (a --depth=1 fetch could leave no merge base), computes the merge-base and diff outside the if so any git failure fails the job instead of falling into the skip branch

waleedlatif1 · 2026-07-24T20:35:50Z

@greptile

waleedlatif1 · 2026-07-24T20:35:51Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit e4b34ca. Configure here.}

The digest-pinned image runs as a non-root user that cannot write into the runner-owned bind mount (it creates tests/__snapshot__, absent from the checkout since empty dirs aren't tracked). Standard bind-mount pattern: --user "$(id -u):$(id -g)" with HOME=/tmp for helm's cache.

The kind install job caught this on its first full run: the default image tag (Chart.AppVersion 0.6.73) returns 404 on GHCR for all three images — the registry's tags are v-prefixed — so an unpinned default install could never pull. Updated appVersion to v0.7.44 (verified 200 for simstudio, realtime, and migrations manifests), stale values comment refreshed, and an upgrade note added. The CI kind values deliberately stay tag-free so the job keeps exercising the true default path.

waleedlatif1 · 2026-07-24T21:21:13Z

@greptile

waleedlatif1 · 2026-07-24T21:21:14Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 1cfaf0f. Configure here.}

waleedlatif1 · 2026-07-24T21:26:24Z

@greptile

cursorBot reviewed Jul 24, 2026
View reviewed changes

waleedlatif1 mentioned this pull request Jul 24, 2026
v0.7.45: access control groups enhancements, sso domain verification, agent thinking/tool-calls/prompt-caching, opus 5, daytona failover #5938
Merged

vercelBotdeployed to Preview July 24, 2026 20:16 View deployment

cursorBot reviewed Jul 24, 2026
View reviewed changes

Comment threadhelm/sim/values.schema.json

greptile-appsBot reviewed Jul 24, 2026
View reviewed changes

Comment threadhelm/sim/values.schema.json
Comment thread.github/workflows/helm.yml Outdated

greptile-appsBot reviewed Jul 24, 2026
View reviewed changes

Comment threadhelm/sim/values.schema.json
Comment thread.github/workflows/helm.yml Outdated

waleedlatif1 added 2 commits July 24, 2026 13:19

cursorBot reviewed Jul 24, 2026
View reviewed changes

Comment thread.github/workflows/helm.yml

vercelBotdeployed to Preview July 24, 2026 20:29 View deployment

greptile-appsBot reviewed Jul 24, 2026
View reviewed changes

Comment thread.github/workflows/helm.yml

cursorBot reviewed Jul 24, 2026
View reviewed changes

vercelBotdeployed to Preview July 24, 2026 20:40 View deployment

vercelBotdeployed to Preview July 24, 2026 20:55 View deployment

This comment has been minimized.
Sign in to view

vercelBotdeployed to Preview July 24, 2026 21:14 View deployment

cursorBot reviewed Jul 24, 2026
View reviewed changes

waleedlatif1 merged commit 90f6708 into stagingJul 24, 2026
22 of 23 checks passed

waleedlatif1 deleted the helm-hygiene branch July 24, 2026 21:25

Uh oh!

Conversation

waleedlatif1 commented Jul 24, 2026

Summary

Type of Change

Testing

Checklist

Uh oh!

vercelBot commented Jul 24, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

waleedlatif1 commented Jul 24, 2026

Uh oh!

waleedlatif1 commented Jul 24, 2026

Uh oh!

cursorBot commented Jul 24, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

cursorBot left a comment

Choose a reason for hiding this comment

Uh oh!

waleedlatif1 commented Jul 24, 2026

Uh oh!

waleedlatif1 commented Jul 24, 2026

Uh oh!

Uh oh!

greptile-appsBot commented Jul 24, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

greptile-appsBot commented Jul 24, 2026

Greptile Summary

Confidence Score: 3/5

Security Review

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Jul 24, 2026

Uh oh!

waleedlatif1 commented Jul 24, 2026

Uh oh!

Uh oh!

greptile-appsBot commented Jul 24, 2026

Greptile Summary

Confidence Score: 4/5

Security Review

Important Files Changed

Uh oh!

Uh oh!

waleedlatif1 commented Jul 24, 2026

Uh oh!

waleedlatif1 commented Jul 24, 2026

Uh oh!

cursorBot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

waleedlatif1 commented Jul 24, 2026

Uh oh!

waleedlatif1 commented Jul 24, 2026

Uh oh!

cursorBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

waleedlatif1 commented Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

vercelBot commented Jul 24, 2026•
edited
Loading

cursorBot commented Jul 24, 2026•
edited
Loading

greptile-appsBot commented Jul 24, 2026•
edited
Loading