fix(helm): correct chart docs, examples, and dead config across the board by waleedlatif1 · Pull Request #5907 · simstudioai/sim

waleedlatif1 · 2026-07-24T00:10:18Z

Summary

Audit-driven accuracy pass over the chart's entire documentation surface — README, all 10 example values files, values.yaml/schema comments, NOTES.txt, the sim-helm skill references, and the docs-site Kubernetes page — with every claim verified by rendering the examples against the actual templates
Migrations reality: they run as an init container on the app pod, not a Job — fixed the README component description, all kubectl logs job/sim-migrations commands (README, skill refs), and removed the dead migrations-job NetworkPolicy rule that selected pods which no longer exist
Referenced-but-never-created resources (each one bricked or degraded a fresh install): GKE ManagedCertificate creation manifest documented in values-gcp; the key-file Secret mount in values-gcp commented out with Workload Identity guidance (it left every pod in ContainerCreating and overrode WI/ADC); certManager.enabled added to values-production so its postgres TLS issuerRef exists; cert-manager.io/cluster-issuer annotation added to values-azure so nginx stops serving the fake certificate
values-external-db: networkPolicy.egress was a map (rendered an invalid manifest kubectl rejects) — now a proper rule list; placeholder host/username failed the chart's own schema — now render-valid
Realtime scaling truth: >1 realtime replica requires REDIS_URL (Socket.IO Redis adapter gates on it) — examples now default to 1 replica with the scaling note, and the three autoscaling examples warn that the HPA overrides replicaCount
Silent no-ops fixed: pod anti-affinity selected label value simstudio but the chart's name label is sim; unimplemented app.secrets.existingSecret.keys removed from values + schema (secret is consumed wholesale via envFrom); dead SOCKET_SERVER_URL / GOOGLE_CLOUD_PROJECT/REGION env removed
Docs-site kubernetes.mdx: every install command failed at template time (missing required CRON_SECRET + postgresql.auth.password) — fixed all four, plus the wrong port-forward deployment name (sim-sim-app → sim-app), stale version requirements (1.19+/3.0+ → 1.25+/3.8+), and the unsupported key-remapping FAQ claim
Assorted accuracy: README PDB default was stated backwards, cronjob list missing 5 jobs, /metrics ServiceMonitor caveat, NOTES secret count, Azure-only StorageClass hardcoded in "generic" examples, ESO apiVersion mismatch, skill-reference resource-table drift, constitution-violating "Mothership" wording
Chart version bumped to 1.0.1

Type of Change

Bug fix (documentation/config accuracy)

Testing

helm lint clean; all 10 examples render via helm template (the two with intentional required-secret guards render fully with their documented --sets)
Spot-verified rendered output: external-db NetworkPolicy egress is now a valid rule list, production renders the full cert-manager chain (6 resources incl. sim-ca-issuer), anti-affinity selectors match the real sim label
Realtime/REDIS_URL claim verified against apps/realtime/src/config/socket.ts and the shared-secret envFrom wiring in all three secret modes

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)

…oard Audit-driven accuracy pass over the chart's entire documentation surface, verified by rendering every example against the templates: - migrations run as an init container on the app pod, not a Job — fix the README component list, troubleshooting commands, and sim-helm skill refs; drop the dead migrations-job NetworkPolicy ingress rule - referenced-but-never-created resources: document the GKE ManagedCertificate creation (values-gcp), comment out the key-file Secret mount that stuck all pods in ContainerCreating (values-gcp), enable certManager for the postgres TLS issuerRef (values-production), add the cert-manager cluster-issuer annotation nginx needs (values-azure) - values-external-db: networkPolicy.egress is a list, not a map (the map rendered an invalid manifest); fill schema-failing placeholder host/username - realtime >1 replica requires REDIS_URL (Socket.IO Redis adapter) — default examples to 1 replica with the scaling note, and warn where autoscaling HPAs override replicaCount - pod anti-affinity selectors matched nothing (simstudio vs sim name label) - kubernetes.mdx: install commands were missing required CRON_SECRET and postgresql password (failed at template time), wrong deployment name in port-forward, stale version requirements, unsupported key-remapping claim - remove unimplemented app.secrets.existingSecret.keys from values + schema; fix README PDB default, cronjob list, /metrics caveat, NOTES secret count, Azure-only StorageClass in generic examples, dead SOCKET_SERVER_URL and GOOGLE_CLOUD_* env, ESO apiVersion mismatch, and skill-reference drift - bump chart to 1.0.1

vercel · 2026-07-24T00:10:23Z

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

Project	Deployment	Actions	Updated (UTC)
docs	Ready	Preview, Comment	Jul 24, 2026 1:30am

cursor · 2026-07-24T00:10:28Z

PR Summary

Low Risk
Changes are primarily documentation and example values; template tweaks trigger one-time pod rolls and fix previously broken Jaeger/ESO behaviors rather than altering app runtime logic.

Overview
Helm chart 1.1.0 is mostly an audit pass that aligns docs, examples, and a few templates with how the chart actually behaves today.

Documentation (README, all example values, sim-helm skill refs, docs-site kubernetes.mdx, NOTES.txt) now describe migrations as an app init container (not a Job), require CRON_SECRET and postgresql.auth.password in install commands, fix port-forward target sim-app, and document cloud install caveats (helm upgrade --install, secret reuse, DB name). Examples default realtime to 1 replica with REDIS_URL / autoscaling.realtime.enabled guidance, fix anti-affinity label sim, external-db NetworkPolicy egress as a valid rule list, and cloud TLS prerequisites (GKE ManagedCertificate, Azure cert-manager annotation, ALB vs cert-manager).

Template / values behavior: app and realtime rollouts hash inline Secret + ExternalSecret for ESO mapping changes; realtime replicaCount stays unless both app autoscaling and autoscaling.realtime.enabled are on; Jaeger export switches to otlp/jaeger on port 4317; unused existingSecret.keys / passwordKey remapping is removed (standard key names only); Postgres NetworkPolicy drops the obsolete migrations-job rule.

Upgrade notes in the README cover the above for 1.1.0.

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

greptile-apps · 2026-07-24T00:13:02Z

Greptile Summary

This PR corrects Helm chart behavior, examples, and documentation across deployment, secrets, networking, autoscaling, telemetry, and cloud installation workflows.

Aligns migration documentation and NetworkPolicy rules with the app init-container implementation.
Repairs cloud and external-database examples, required-secret instructions, TLS configuration, and realtime scaling defaults.
Removes unused secret-key configuration and environment values while updating schema, tests, and rendered-resource documentation.
Updates HPA ownership, ExternalSecret rollout checksums, telemetry configuration, and the chart version.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains within the scope of the previous review threads.

Important Files Changed

Filename	Overview
apps/docs/content/docs/en/platform/self-hosting/kubernetes.mdx	Corrects installation prerequisites, required variables, cloud-path guidance, conversion caveats, deployment naming, and secret-management documentation.
helm/sim/examples/values-external-db.yaml	Supplies complete required-secret setup and a structurally valid, destination-scoped external-database egress rule.
helm/sim/templates/deployment-realtime.yaml	Preserves explicit realtime replica counts when its HPA is disabled and includes ExternalSecret configuration in rollout checksums.
helm/sim/templates/hpa.yaml	Renders the realtime HPA only when realtime-specific autoscaling is enabled.
helm/sim/templates/networkpolicy.yaml	Removes the obsolete migrations Job ingress selector and aligns comments with current traffic behavior.
helm/sim/templates/telemetry.yaml	Updates the collector configuration to use the supported OTLP exporter and completes memory-limiter configuration.
helm/sim/values.schema.json	Aligns the public values contract with implemented secret handling, autoscaling, and telemetry options.

_{Reviews (14): Last reviewed commit: "fix(helm+docs): explicit secret exports ..." | Re-trigger Greptile}

…, copilot Job wording - external-db example egress scopes to a placeholder database CIDR instead of to: [] (which allowed every destination on 5432, defeating the isolation the example teaches) - kubernetes.mdx cloud tabs state explicitly that they reuse the variables generated in the Installation block - Copilot migrations really do run as a Helm-hook Job — restore Job wording there (only the app migrations are an init container)

waleedlatif1 · 2026-07-24T00:16:26Z

@greptile

waleedlatif1 · 2026-07-24T00:16:27Z

@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 dd2872a. Configure here.}

…cksums, dead passwordKey knob - telemetry: memory_limiter gets the required check_interval (collector failed startup validation whenever telemetry.enabled=true); the jaeger exporter was removed from collector-contrib in v0.86 — export to Jaeger via its native OTLP endpoint instead (otlp/jaeger, default port 4317) - app/realtime rollout checksums now hash the ExternalSecret manifest too, mirroring the copilot pattern — with ESO enabled the inline Secret renders empty, so remoteRefs changes never rolled the pods - remove the unimplemented existingSecret.passwordKey knob (values, schema, README, dead helpers): nothing consumed it, and a non-default value silently produced a DATABASE_URL with an unexpandable placeholder; secrets must use the standard POSTGRES_PASSWORD / EXTERNAL_DB_PASSWORD keys - drop the orphaned sim.migrations.labels helper (its only consumer was the dead NetworkPolicy rule removed earlier) - helm test pod image resolves through sim.image so global.imageRegistry mirroring applies; NetworkPolicy realtime-ingress comment reflects actual traffic direction; smoke unittest suite loads the newly referenced external-secret template

waleedlatif1 · 2026-07-24T00:22:34Z

@greptile

waleedlatif1 · 2026-07-24T00:22:35Z

@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 a07ecf6. Configure here.}

Removing (inert) documented values keys and changing the rollout-checksum inputs is a values-surface change — per SemVer chart conventions that is more than a patch. Adds an Upgrading section documenting the one-time pod roll, the removed no-op keys, and the Jaeger-over-OTLP change.

waleedlatif1 · 2026-07-24T00:27:11Z

@greptile

waleedlatif1 · 2026-07-24T00:27:12Z

@cursor review

…flow, prod Jaeger OTLP endpoint - external-db example ships networkPolicy disabled so a verbatim install always reaches the database; the scoped egress rule stays as the documented opt-in (set your CIDR first, then enable) - values-production still pointed telemetry.jaeger at the legacy 14250 collector port — now Jaeger's OTLP gRPC endpoint to match the otlp/jaeger exporter

waleedlatif1 · 2026-07-24T00:33:16Z

@greptile

waleedlatif1 · 2026-07-24T00:33:17Z

@cursor review

… the app without unsafe realtime replicas Cursor correctly flagged that comment-level warnings didn't stop a verbatim production/external-db install from running the realtime HPA at minReplicas 2 without REDIS_URL (silent cross-pod event loss). Adds an opt-out toggle (default true — existing deployments unchanged): the realtime HPA renders only when autoscaling.realtime.enabled, and the realtime Deployment keeps spec.replicas under its control when the HPA is excluded. The three autoscaling examples set it false with the Redis rationale; README and upgrade notes document the toggle.

waleedlatif1 · 2026-07-24T00:38:22Z

@greptile

waleedlatif1 · 2026-07-24T00:38:23Z

@cursor review

waleedlatif1 · 2026-07-24T01:02:20Z

@greptile

waleedlatif1 · 2026-07-24T01:02:21Z

@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 5c83c2b. Configure here.}

…isting install The cloud values rename the bundled Postgres database to simstudio, which Postgres only applies at first initialization — an in-place conversion of a generic install would point DATABASE_URL at a nonexistent database. Document the two safe paths: keep the original name via --set, or uninstall + delete PVCs and install fresh.

waleedlatif1 · 2026-07-24T01:07:41Z

@greptile

waleedlatif1 · 2026-07-24T01:07:42Z

@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 b1c7995. Configure here.}

…d includes CRON_SECRET The primary documented command failed the chart's required-value validation (missing app.env.CRON_SECRET with cronjobs default-on) and referenced an undeclared DB_PASSWORD. It now shows the export lines for every variable it uses and sets CRON_SECRET; both commands verified with bash -n.

waleedlatif1 · 2026-07-24T01:13:02Z

@greptile

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

@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 f453313. Configure here.}

…ns only The earlier programmatic edit reformatted the whole file (~390 lines of whitespace churn hiding the 12 real deleted lines). Re-applied the removal of the dead keys/passwordKey properties as text-level deletions preserving the original style.

waleedlatif1 · 2026-07-24T01:20:39Z

@greptile

waleedlatif1 · 2026-07-24T01:20:40Z

@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 ff0ecff. Configure here.}

…sion must reuse original secrets - all five export lines are written out (three were only named in a trailing comment, so a verbatim copy passed empty required values) - the cloud-conversion caveat now leads with reusing the original secret values (helm get values) — a regenerated ENCRYPTION_KEY makes previously encrypted credentials undecryptable

waleedlatif1 · 2026-07-24T01:25:02Z

@greptile

waleedlatif1 · 2026-07-24T01:25:03Z

@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 338636b. Configure here.}

greptile-appsBot reviewed Jul 24, 2026
View reviewed changes

Comment threadapps/docs/content/docs/en/platform/self-hosting/kubernetes.mdx
Comment threadhelm/sim/examples/values-external-db.yaml
Comment threadhelm/sim/values.yaml Outdated

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