Skip to content

Commit 095061b

Browse files
waleedlatif1claude
andcommitted
fix: address the second review round
Two chart defects, both the same shape as the uploads-path bug this branch already documents: a path that looks right but sits outside where the app actually serves from. - `branding.mountPath` defaulted to `/app/public/branding`, while the standalone build places the public directory at `/app/apps/sim/public`. Every branded asset the docs tell operators to reference would have 404'd. Corrected in the chart and the example. - The device plugin was the only `sim.image` call site not passing `chartAppVersion`, so `tag: ""` — a natural way to ask for the chart default — errored instead of falling back. Docs, each verified against source: - Local disk is selected unless a backend's *activating* variables are set; credentials alone activate nothing. - Six backoff retries follow the initial failure, so up to seven attempts. - The migrations init container reads only the database Secret, and the chart's Secret templates carry just the password — so `MIGRATION_DATABASE_URL` needs an operator-supplied Secret, not the generated one. - The Ollama stack's missing Redis also breaks CLI authentication outright, while the idempotency store and progress markers fall back to Postgres. - `sim-setup reset` leaves the PostgreSQL PVC on Kubernetes. - The copilot ingress route renders only when `copilot.enabled` is set. - The Postgres exposure applies to whichever Compose file started the install. - The GKE annotate command resolves the Service name from the release rather than assuming it. - `API_ENCRYPTION_KEY` warns on every use, not once, and the whitelabeling coverage claim now describes the two surfaces that only partially honour it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015BwsJTEQRzWJaY4BRCkPZt
1 parent 4f9c34f commit 095061b

10 files changed

Lines changed: 16 additions & 10 deletions

File tree

apps/docs/content/docs/platform/enterprise/whitelabeling.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ Once enabled, configure branding through **Settings → Organization → White-l
106106

107107
## Instance-wide branding
108108

109-
Instance-wide branding is a separate mechanism from the organization settings above. It is read from environment variables at every render, needs **no organization** and **no** `WHITELABELING_ENABLED`, and applies across the deployment, including surfaces organization settings never reach — though not uniformly: the PWA manifest ignores the configured logo and favicon URLs, and public file-share pages drop the Sim logo without substituting yours. It covers the surfaces organization white-labeling never reaches: the login modal, the PWA manifest, and public file-share pages.
109+
Instance-wide branding is a separate mechanism from the organization settings above. It is read from environment variables at every render, needs **no organization** and **no** `WHITELABELING_ENABLED`, and applies across the deployment, including surfaces organization settings never reach — though not uniformly. The PWA manifest keeps the built-in icons, and public file-share pages only hide the Sim wordmark rather than rendering yours. It covers the surfaces organization white-labeling never reaches: the login modal, the PWA manifest, and public file-share pages.
110110

111111
Where both are set, an organization's saved settings take precedence for members of that organization; the instance-wide values are the deployment's baseline.
112112

@@ -157,7 +157,7 @@ The `NEXT_PUBLIC_BRAND_*_URL` variables point at files the app serves — they d
157157
branding:
158158
enabled: true
159159
# Serving path for the mounted files
160-
mountPath: /app/public/branding
160+
mountPath: /app/apps/sim/public/branding
161161
# Text files (CSS, JSON, SVG) as plain text
162162
files:
163163
custom.css: |

apps/docs/content/docs/platform/self-hosting/background-jobs.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ cronjobs:
7171
enabled: true
7272
```
7373
74-
Each job runs a small `curlimages/curl` pod that calls the app's **in-cluster Service** (not the ingress), with `concurrencyPolicy: Forbid` so a slow run never overlaps itself. The Service load-balances that single call to one replica, so a job never fans out across pods. The chart keeps three completed and one failed Job per CronJob, but `ttlSecondsAfterFinished` defaults to 600, so a finished Job is deleted ten minutes after it ends — inspect a failure promptly or collect the logs. It sets no `backoffLimit`, so the Kubernetes default of six applies and the chart exposes no key to change it. `cronjobs.activeDeadlineSeconds` (default `300`) bounds the **whole Job** — retries and the controller's back-off included — so whichever limit is reached first ends the Job — six attempts, or five minutes of attempts plus back-off. The same deadline cuts off a single hung request. One attempt is already several HTTP calls: the container retries three times, ten seconds apart, with `curl --retry 2` inside each.
74+
Each job runs a small `curlimages/curl` pod that calls the app's **in-cluster Service** (not the ingress), with `concurrencyPolicy: Forbid` so a slow run never overlaps itself. The Service load-balances that single call to one replica, so a job never fans out across pods. The chart keeps three completed and one failed Job per CronJob, but `ttlSecondsAfterFinished` defaults to 600, so a finished Job is deleted ten minutes after it ends — inspect a failure promptly or collect the logs. It sets no `backoffLimit`, so the Kubernetes default of six applies and the chart exposes no key to change it. `cronjobs.activeDeadlineSeconds` (default `300`) bounds the **whole Job** — retries and the controller's back-off included — so whichever limit is reached first ends the Job — six retries after the initial failure, up to seven attempts, or five minutes of attempts plus back-off. The same deadline cuts off a single hung request. One attempt is already several HTTP calls: the container retries three times, ten seconds apart, with `curl --retry 2` inside each.
7575
7676
Disable individual jobs you do not need — billing reconciliation is the obvious one on a self-hosted install:
7777

apps/docs/content/docs/platform/self-hosting/docker.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ For nginx, Traefik, or a cloud load balancer — and for the GKE websocket timeo
100100

101101
## Ollama
102102

103-
`docker-compose.ollama.yml` is a **development** stack, not a drop-in swap for `docker-compose.prod.yml`. It builds `simstudio`, `realtime`, and `migrations` from the source checkout instead of pulling published images, and it ships neither a `redis` service nor a `cron` service — so live cross-process status and every [background job](/platform/self-hosting/background-jobs) are missing. For production with local models, keep `docker-compose.prod.yml` and point `OLLAMA_URL` at an Ollama instance, as under [External Ollama](#external-ollama) below.
103+
`docker-compose.ollama.yml` is a **development** stack, not a drop-in swap for `docker-compose.prod.yml`. It builds `simstudio`, `realtime`, and `migrations` from the source checkout instead of pulling published images, and it ships neither a `redis` service nor a `cron` service. Without an external Redis, pub/sub falls back to a process-local emitter and CLI authentication stops working entirely, since its approval store has no fallback; the idempotency store and execution progress markers do fall back to Postgres. And with no cron service, live cross-process status and every [background job](/platform/self-hosting/background-jobs) are missing. For production with local models, keep `docker-compose.prod.yml` and point `OLLAMA_URL` at an Ollama instance, as under [External Ollama](#external-ollama) below.
104104

105105
Always pass a server profile alongside `setup`. With `--profile setup` on its own no Ollama server starts and `model-setup` waits forever.
106106

apps/docs/content/docs/platform/self-hosting/environment-variables.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { Callout } from 'fumadocs-ui/components/callout'
2626

2727
| Variable | Description |
2828
|----------|-------------|
29-
| `API_ENCRYPTION_KEY` | Encrypts user-generated Sim API keys at rest, exactly 64 hex characters (32 bytes): `openssl rand -hex 32`. **When unset, API keys are stored in plain text** — the app logs one warning and keeps working |
29+
| `API_ENCRYPTION_KEY` | Encrypts user-generated Sim API keys at rest, exactly 64 hex characters (32 bytes): `openssl rand -hex 32`. **When unset, API keys are stored in plain text** — the app logs a warning each time it encrypts or decrypts one and keeps working |
3030
| `CRON_SECRET` | Bearer token for the background job endpoints: `openssl rand -hex 32`. **Required whenever background jobs are enabled** — the Helm chart's default, and the chart will not render without it, unless it comes from `app.secrets.existingSecret` or External Secrets. See [Background Jobs](/platform/self-hosting/background-jobs) |
3131
| `REDIS_URL` | Redis connection string. Optional on a single replica; **required** past one app or realtime replica — see [Redis](/platform/self-hosting/redis) |
3232
| `REDIS_TLS_SERVERNAME` | TLS SNI override. Required when `REDIS_URL` uses `rediss://` with a bare IP, or the app throws the first time it opens a Redis connection |

apps/docs/content/docs/platform/self-hosting/networking.mdx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ When `realtime.host` equals `app.host`, the realtime paths are folded into the a
8383
Both `ingress` and `ingressInternal` accept an optional `copilot` block, commented out in `values.yaml` because the `copilot` service is off by default. It follows the same host-sharing rule:
8484

8585
```yaml
86+
# The route renders only when the service itself is enabled.
87+
copilot:
88+
enabled: true
89+
8690
ingress:
8791
copilot:
8892
host: sim.yourdomain.com

apps/docs/content/docs/platform/self-hosting/object-storage.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Set `STORAGE_PROVIDER` to `local`, `s3`, `azure`, or `gcs` to select a backend e
2727
1. **Azure Blob** — used if `AZURE_STORAGE_CONTAINER_NAME` is set **and** either (`AZURE_ACCOUNT_NAME` + `AZURE_ACCOUNT_KEY`) or `AZURE_CONNECTION_STRING` is set.
2828
2. **AWS S3** — used if `S3_BUCKET_NAME` **and** `AWS_REGION` are set (and Azure is not configured).
2929
3. **Google Cloud Storage** — used if `GCS_BUCKET_NAME` is set (and neither Azure nor S3 is configured).
30-
4. **Local disk** — used only when no cloud backend has *any* of its variables set.
30+
4. **Local disk** — used unless a cloud backend's activating variables are set: `S3_BUCKET_NAME` with `AWS_REGION` for S3, `AZURE_STORAGE_CONTAINER_NAME` for Blob, `GCS_BUCKET_NAME` for GCS. Credentials alone activate nothing.
3131

3232
If `STORAGE_PROVIDER` is unset, Sim uses the first backend whose configuration is complete, in that order. An explicit `STORAGE_PROVIDER` takes precedence and must be valid and complete.
3333

apps/docs/content/docs/platform/self-hosting/security.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ The service bundles ~2.2 GB of spaCy models, so first start takes around three m
257257
## The shipped Compose file publishes Postgres
258258

259259
<Callout type="error">
260-
`docker-compose.prod.yml` maps the database to the host: `${POSTGRES_PORT:-5432}:5432`, with `POSTGRES_USER` and `POSTGRES_PASSWORD` both defaulting to `postgres`. A plain `docker compose -f docker-compose.prod.yml up -d` on a machine with a public interface therefore exposes an open Postgres on 5432 with credentials anyone can guess.
260+
`docker-compose.prod.yml` maps the database to the host: `${POSTGRES_PORT:-5432}:5432`, with `POSTGRES_USER` and `POSTGRES_PASSWORD` both defaulting to `postgres`. A plain `docker compose up -d` against that file, on a machine with a public interface, therefore exposes an open Postgres on 5432 with credentials anyone can guess. The Ollama stack maps the database the same way, so apply the fix to whichever file started your install.
261261

262262
The [Docker guide](/platform/self-hosting/docker#1-configure-environment) tells you to generate `POSTGRES_PASSWORD` before the first start — do that, and additionally close the port:
263263

apps/docs/content/docs/platform/self-hosting/upgrades.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ These control the migrator and are set on the `migrations` container, not the ap
9595
<Callout type="warn">
9696
Set `MIGRATION_DATABASE_URL` if anything sits between Sim and Postgres. Session advisory locks and session-level `SET`s do not survive PgBouncer transaction pooling — the lock silently does not hold, and consecutive statements can land on different backends. Point it at the database directly, bypassing the pooler.
9797

98-
The chart has no values key for it: the migrations init container builds `DATABASE_URL` itself and reads only the database Secret, so setting it under `app.env` does not reach the migrator. On Helm it has to live in whichever database Secret applies. The chart-generated Secrets are re-rendered on every upgrade, so add it through `externalDatabase.existingSecret` or External Secrets rather than editing the generated one — otherwise run the migration image yourself against the direct DSN before upgrading. Compose is the same: its `migrations` service declares an explicit `environment:` list, so a value in `.env` alone does not reach it — add it to that service.
98+
The chart has no values key for it: the migrations init container builds `DATABASE_URL` itself and reads only the database Secret, so setting it under `app.env` does not reach the migrator. On Helm the init container reads only the database Secret, and the chart's own Secret templates carry just the password — there is no supported key for this. Supply your own Secret through `postgresql.auth.existingSecret` (bundled) or `externalDatabase.existingSecret` (external) with the variable included, or run the migration image yourself against the direct DSN before upgrading. Compose is the same: its `migrations` service declares an explicit `environment:` list, so a value in `.env` alone does not reach it — add it to that service.
9999
</Callout>
100100

101101
Behavior that is fixed in the migrator and not configurable, but that explains what you see in the logs:

helm/sim/templates/gpu-device-plugin.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ spec:
8585
{{- end }}
8686
containers:
8787
- name: nvidia-device-plugin
88-
image: {{ include "sim.image" (dict "imageRoot" .Values.ollama.gpu.devicePlugin.image "global" .Values.global) }}
88+
image: {{ include "sim.image" (dict "imageRoot" .Values.ollama.gpu.devicePlugin.image "global" .Values.global "chartAppVersion" .Chart.AppVersion) }}
8989
imagePullPolicy: {{ .Values.ollama.gpu.devicePlugin.image.pullPolicy }}
9090
args:
9191
- "--config-file=/etc/device-plugin/config.yaml"

helm/sim/values.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1284,7 +1284,9 @@ branding:
12841284
enabled: false
12851285

12861286
# Mount path in the container where branding files will be available
1287-
mountPath: "/app/public/branding"
1287+
# Must sit inside the image's public directory, which the standalone build
1288+
# places at /app/apps/sim/public — a path outside it serves nothing.
1289+
mountPath: "/app/apps/sim/public/branding"
12881290

12891291
# Text files (CSS, JSON, HTML, etc.) - values are plain text
12901292
# Example:

0 commit comments

Comments
 (0)