fix(gitops): make environments/local/values.yaml a wizard-generated a…#1379
Conversation
|
Warning Review limit reached
More reviews will be available in 5 minutes and 10 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughIntroduces a ChangesdevUserEmail Feature, Wizard Template Handling, and Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@compose/seed/silver.py`:
- Around line 88-90: The exception message on lines 88-90 that mentions the
PLACEHOLDERS_SQL path uses relative-path conventions that are working-directory
dependent and may not align with documentation examples, causing confusion for
users. Update this error message to use cwd-agnostic wording such as "set to an
existing file path" rather than specifying relative paths like
"compose/seed/sql/placeholders.sql", and optionally provide one concrete example
that matches the CONTRIBUTING documentation. Apply the same fix to the similar
error message at lines 102-104.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 296fb1bf-b673-4fd0-9c5e-0e6fcf8b2da1
📒 Files selected for processing (7)
CONTRIBUTING.mdcharts/insight/templates/_helpers.tplcompose/insight-init.shcompose/seed/silver.pydeploy/gitops/environments/local/values.yaml.templatesrc/frontend/helm/templates/deployment.yamlsrc/frontend/helm/values.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- compose/insight-init.sh
- CONTRIBUTING.md
…ror messages Per CodeRabbit on constructorfabric#1379: the hints embedded one relative-path example that only worked when CWD was compose/seed, mismatching anything run from a different directory. Reword to "set to an existing X path, e.g. <path> when running from compose/seed" — the example is now explicitly anchored to the doc'd CWD instead of being presented as a universally-correct fragment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
…ror messages Per CodeRabbit on constructorfabric#1379: the hints embedded one relative-path example that only worked when CWD was compose/seed, mismatching anything run from a different directory. Reword to "set to an existing X path, e.g. <path> when running from compose/seed" — the example is now explicitly anchored to the doc'd CWD instead of being presented as a universally-correct fragment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
7ec6cca to
92508fa
Compare
…rtifact `make deploy ENV=local`'s sync-clean step aborted whenever the wizard re-injected `.global.tenantDefaultId` into the committed `environments/local/values.yaml` via `yq -i`. Same root cause as the inventory.yaml issue (constructorfabric#1378): a committed file mutated per developer. Convert to the same pattern that already works for inventory.yaml: - rename committed `environments/local/values.yaml` → `environments/local/values.yaml.template` - wizard cp's the template to the live `values.yaml` on first run, then `yq -i` injects the tenant id collected from the operator - live `environments/local/values.yaml` is gitignored - "Adding a new env" walkthrough copies from the template - CONTRIBUTING + README updated for the new artifact list `make deploy ENV=local` re-runs cleanly without sync-clean noise; the template stays a stable, committed sandbox config. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
Adds a "Non-interactive deploy (CI, scripted, headless)" subsection to CONTRIBUTING.md under "Beyond compose". The wizard refuses to run without a TTY, so CI and any automation needs the explicit "what to copy from each template, what to edit, what to do for airbyte" recipe. The chain itself is unchanged — `make deploy ENV=local` already skips the wizard whenever inventory.yaml exists. This just documents the pre-population steps so operators don't have to reverse-engineer the wizard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
Rewrites CONTRIBUTING.md end-to-end:
- Adds a contents list at the top so readers jump straight to the
section they need (compose vs k8s, non-interactive CI, daily edit
loop, troubleshooting).
- Promotes the k8s + non-interactive CI paths from a sub-section of
"Beyond compose" to first-class deployment paths alongside compose,
with a single comparison table at the top.
- Collapses the editing prose ("Edit Rust → build → watchexec") into
a compact "what triggers what" table.
- Drops repeated prose from "Auto-reload", "External MariaDB /
ClickHouse", "Frontend modes" — kept tables and bullets, removed
the surrounding setup paragraphs that paraphrased them.
- Merges "Common tasks" / "Wipe everything" / "Prune" / "Run a
one-off Rust build" into one "Common operations" code block + a
paragraph on prune's interactive contract.
725 → 488 lines (−237, ~33%). No technical content removed; the file
now reads top-to-bottom as a single linear walkthrough.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
The Prerequisites section overstated the requirement — the frontend checkout is only needed for compose with `FRONTEND_MODE=dev` (Vite HMR) or `built`. The default wizard mode (`ghcr`) and the entire k8s path both pull the published `insight-front` image from GHCR, no local clone required. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
0.1.79 is the first published chart that contains the identity
wait-for-mariadb fix (envFrom → `{release}-platform` ConfigMap +
`$MARIADB_HOST` env-var resolution instead of the hardcoded
`{release}-mariadb` template). On 0.1.78 the init container blocks
forever when `mariadb.deploy=false` because `insight-mariadb` does not
resolve in the cluster.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
Without an Ingress the deployed FE returns 404 on /api/* because the chart's frontend nginx only serves the SPA bundle — the chart relies on ingress-nginx to path-route `/api/*` → api-gateway and `/*` → frontend on the same host. Enable both Ingress objects in the local sandbox overlay (template). Host left empty so the Ingress matches any Host header — on OrbStack/k3d/kind the ingress-nginx LoadBalancer takes a host-reachable IP, so `http://<that-ip>/` lands on the FE and `/api/*` on the gateway without any /etc/hosts gymnastics. Real envs set `ingress.host:` to an FQDN + add TLS via cert-manager. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
The FE container's `docker-entrypoint.sh` writes a runtime
`oidc-config.js` with `window.__DEV_CONFIG__={devUserEmail}` when
`DEV_USER_EMAIL` env is set, so the browser builds an unsigned-JWT
bearer per /api/* call. The compose path drives this via
`.env.compose`; on k8s the chart had no path to wire it in — operators
had to `kubectl set env deployment/insight-frontend` by hand.
Add `frontend.devUserEmail` to the umbrella + subchart values; the
subchart's Deployment template emits `DEV_USER_EMAIL` when set, in the
same env block that handles OIDC. Mutually exclusive with OIDC at two
layers:
- The umbrella's `insight.validate` template fails the install when
`frontend.devUserEmail` is set together with
`apiGateway.authDisabled=false` (forgotten in a prod overlay = every
visitor silently impersonated as that email).
- The subchart fails the install when `devUserEmail` is set together
with `oidc.{issuer,clientId}` directly under `frontend.*`.
- The entrypoint still has its own runtime check as the last line of
defence.
Wizard `compose/insight-init.sh` injects the collected
`DEV_USER_EMAIL` into `environments/local/values.yaml` under
`.frontend.devUserEmail`. Sandbox template default is
`dev@company.nonpresent` matching the wizard.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
The Seeding section was compose-centric; the k8s path got a one-liner
"port-forward and run from the host" with no actual commands. Replaced
with two subsections — Compose and Kubernetes — sharing the
content-overview prose (identity / silver contents) at the top.
The k8s recipe spells out:
1. port-forward MariaDB + ClickHouse from insight-infra,
2. invoke compose/seed/seed.py with cluster creds in env,
3. roll analytics-api to refresh its schema validator (the
cf/insight#1307 gotcha applies here too).
TOC gets the two new sub-section links.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
The seed package hardcoded `/app/sql/placeholders.sql` and `/migrations` — the compose seed-sample container's bind-mount paths. Running `seed.py` from the host (the k8s deploy path) crashed at startup because those paths don't exist outside the container. Read both from env; defaults stay the compose container paths so the compose seed-sample service keeps working unchanged. CONTRIBUTING.md k8s recipe now sets: PLACEHOLDERS_SQL=./sql/placeholders.sql MIGRATIONS_DIR=../../src/ingestion/scripts/migrations Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
The 20260611000000_support-bullet-rows.sql gold migration reads from silver.class_support_activity, but the seed package's placeholders.sql was extracted before that table existed. `seed.py all` against a fresh ClickHouse crashed at the support migration with `UNKNOWN_TABLE`. Add the placeholder matching the migration's column references (person_key, date, updates, public_comments, private_comments, solved, csat_good, csat_total) using the same shape as the other class_collab_*_activity placeholders. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
…rt team Added the placeholder table in 7dab3dc but left it empty — the 20260611000000_support-bullet-rows.sql gold view rendered with 0 rows, so the FE's support bullets showed "no peer data" for the seeded support team even though the persons existed. New generators/support.py mirrors the crm/hr generator shape: filter roster by team_profile.weights[zendesk-placeholder] > 0 (only the support team), emit one row per person per day with poisson-distributed updates / public_comments / private_comments / solved counters and a ~30%-of-solved CSAT (75% good). Wired into silver.run() after task. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
…ror messages Per CodeRabbit on constructorfabric#1379: the hints embedded one relative-path example that only worked when CWD was compose/seed, mismatching anything run from a different directory. Reword to "set to an existing X path, e.g. <path> when running from compose/seed" — the example is now explicitly anchored to the doc'd CWD instead of being presented as a universally-correct fragment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
92508fa to
494a4c9
Compare
…rtifact
make deploy ENV=local's sync-clean step aborted whenever the wizard re-injected.global.tenantDefaultIdinto the committedenvironments/local/values.yamlviayq -i. Same root cause as the inventory.yaml issue (#1378): a committed file mutated per developer.Convert to the same pattern that already works for inventory.yaml:
environments/local/values.yaml→environments/local/values.yaml.templatevalues.yamlon first run, thenyq -iinjects the tenant id collected from the operatorenvironments/local/values.yamlis gitignoredmake deploy ENV=localre-runs cleanly without sync-clean noise; the template stays a stable, committed sandbox config.Summary by CodeRabbit
Documentation
New Features
Chores