feat(preview): path-based per-experiment FE preview bundle (#1971) - #2123
Merged
cyberantonz merged 3 commits intoAug 3, 2026
Merged
Conversation
📝 WalkthroughWalkthroughAdded the ChangesPreview Helm deployment
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Helm
participant KubernetesIngress
participant PreviewService
participant PreviewDeployment
Helm->>KubernetesIngress: Render /exp/<name> route and rewrite
Helm->>PreviewService: Render service selector
Helm->>PreviewDeployment: Render deployment and probes
KubernetesIngress->>PreviewService: Forward preview request
PreviewService->>PreviewDeployment: Select matching pods
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
deploy/preview/tests/test_render.py (1)
118-123: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a successful OIDC render test.
This test only checks the invalid combined configuration. A regression that rejects valid OIDC-only configuration or omits its rendered authentication configuration can pass this suite. Add a test with the complete valid OIDC settings and assert the expected rendered auth configuration.
[recommendation: Verify the required OIDC fields and output in
deploy/preview/values.yamlanddeploy/preview/templates/before defining the assertion.]🤖 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 `@deploy/preview/tests/test_render.py` around lines 118 - 123, Add a separate successful OIDC-only render test alongside test_oidc_and_dev_impersonation_are_mutually_exclusive, supplying every required OIDC setting identified in deploy/preview/values.yaml and deploy/preview/templates/. Assert the render succeeds and the generated output contains the expected authentication configuration, without setting devUserEmail.
🤖 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 @.github/workflows/preview-helm.yml:
- Line 32: Update the actions/checkout step in the preview Helm workflow to
disable persisted checkout credentials by setting persist-credentials to false.
Keep the existing checkout reference unchanged, and ensure no later workflow
behavior relies on authenticated Git operations.
In `@deploy/preview/README.md`:
- Around line 29-30: Update the preview deployment documentation to accept only
image tags in the image.tag example; do not mention digest input unless the
deployment template’s image rendering is changed to handle digests separately.
- Around line 44-47: Update the authentication note in the preview README to
state that devUserEmail is mutually exclusive with both oidc.issuer and
oidc.clientId, matching the deployment template’s validation. Preserve the
existing guidance about enabling the dev-impersonation escape hatch and the real
OIDC configuration.
- Around line 3-4: Update the introductory prose in the README so “sub-issue
`#1971`).” remains on the same line, or move the issue number elsewhere within the
sentence; ensure no line begins with “#1971” while preserving the existing
wording and references.
- Around line 33-34: Update the experiment-name validation helper to enforce a
maximum length of 55 characters before resource names are generated, while
preserving the existing DNS-1123 character validation. Reject longer names
rather than allowing the subsequent preview-<experiment> truncation to collide
across Deployment, Service, and Ingress resources.
In `@deploy/preview/templates/_helpers.tpl`:
- Around line 6-14: The experiment validation in the
“insight-preview.experiment” helper must reject values longer than 55 characters
before generating resource names; enforce the maximum alongside the existing
DNS-1123 validation and update the corresponding values documentation to state
the 55-character limit.
In `@deploy/preview/templates/deployment.yaml`:
- Line 30: Update the image reference in
deploy/preview/templates/deployment.yaml:30 to render digest values with “@”
instead of “:” while preserving tag rendering, using the repository and
image.tag contract. Update the image.tag documentation in
deploy/preview/values.yaml:15-19 to clearly describe the supported tag-or-digest
format.
In `@deploy/preview/templates/ingress.yaml`:
- Around line 17-18: Add the nginx.ingress.kubernetes.io/use-regex annotation
with the value "true" alongside the existing rewrite-target annotation in the
Ingress template, enabling regex matching for the capture-group-based prefix
stripping path.
In `@docs/domain/presentation-layer/specs/DESIGN.md`:
- Line 383: Update the preview deployment description to identify the current
routing implementation as NGINX-specific, replacing “Controller-agnostic” with
wording that says it is NGINX-specific today and will migrate to Gateway API
HTTPRoute after the Envoy move.
---
Nitpick comments:
In `@deploy/preview/tests/test_render.py`:
- Around line 118-123: Add a separate successful OIDC-only render test alongside
test_oidc_and_dev_impersonation_are_mutually_exclusive, supplying every required
OIDC setting identified in deploy/preview/values.yaml and
deploy/preview/templates/. Assert the render succeeds and the generated output
contains the expected authentication configuration, without setting
devUserEmail.
🪄 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 Plus
Run ID: 426b95c5-2485-408a-893d-93bb7d4239ee
📥 Commits
Reviewing files that changed from the base of the PR and between 6ec80cf and f1493e0a7f0f7a0fa85d52cdd81d8b48665352ad.
📒 Files selected for processing (12)
.github/workflows/preview-helm.yml.pre-commit-config.yamldeploy/preview/Chart.yamldeploy/preview/README.mddeploy/preview/templates/_helpers.tpldeploy/preview/templates/deployment.yamldeploy/preview/templates/ingress.yamldeploy/preview/templates/service.yamldeploy/preview/tests/test_render.pydeploy/preview/values.yamldocs/domain/presentation-layer/specs/DESIGN.mddocs/domain/presentation-layer/specs/PRD.md
…orfabric#1971) Add the insight-preview Helm chart at deploy/preview: each experiment is one release (Deployment + Service + one prefix-strip Ingress) served under /exp/<name> on a single shared host, provisioned and removed by hand. The controller merges same-host route objects, so apply adds a path and uninstall removes it with no central config rewrite; controller-agnostic (nginx Ingress now, Gateway API HTTPRoute after the Envoy move). The experiment slug is validated as a DNS-1123 label at template time. Render-contract pytest suite plus a preview-helm.yml CI lane guard the prefix-strip route and per-experiment naming. Extend the yamlfmt exclude to cover deploy/*/templates (Go-templated, not parseable YAML). Extend the presentation PRD and DESIGN to record the shipped serving-path slice (auth constructorfabric#1972 and the synthetic-data pin constructorfabric#1973 stay open). Closes constructorfabric#1971 Part of constructorfabric#1803 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
cyberantonz
force-pushed
the
pres/1971-preview-envs
branch
from
August 3, 2026 03:33
f1493e0 to
8f8fd7f
Compare
- Ingress: add nginx.ingress.kubernetes.io/use-regex: "true" so the capture-group prefix-strip path matches as a regex (removes 404 risk). - Document image.tag as a build tag only; the template renders repository:tag, which is invalid for a digest (would need @sha256:...). - Note the 55-char experiment cap in values/README/DESIGN. - CI: set persist-credentials: false on checkout (repo convention). - DESIGN: describe the route as nginx-specific today (portable by intent), not "controller-agnostic". Part of constructorfabric#1803 Signed-off-by: Anton Zelenov <antonz@constructor.tech>
Contributor
Author
|
Addressed the CodeRabbit review in a4a78de:
Not actioned (moot): the note to document the full |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
deploy/preview/templates/ingress.yaml (1)
21-23: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winKeep chart-owned routing annotations authoritative.
Lines 21-23 render
.Values.ingress.annotationsafter the chart-owneduse-regexandrewrite-targetentries at Lines 19-20. If the values map repeats either reserved key, the rendered manifest contains two values for one annotation, so the chart cannot guarantee regex matching or the/$2prefix strip. Reject these reserved keys or merge one map with chart-owned values taking precedence. Add render-contract cases for both collisions. Helm serializes the map withtoYaml, and ingress-nginx uses these annotations for regex matching and capture-group rewrites. (helm.sh)🤖 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 `@deploy/preview/templates/ingress.yaml` around lines 21 - 23, Update the ingress template’s annotation merge around the chart-owned use-regex and rewrite-target entries so user-provided .Values.ingress.annotations cannot override or duplicate those reserved keys; make the chart-owned values authoritative. Add render-contract coverage for collisions with both reserved annotations, preserving all unrelated user annotations.
🤖 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.
Nitpick comments:
In `@deploy/preview/templates/ingress.yaml`:
- Around line 21-23: Update the ingress template’s annotation merge around the
chart-owned use-regex and rewrite-target entries so user-provided
.Values.ingress.annotations cannot override or duplicate those reserved keys;
make the chart-owned values authoritative. Add render-contract coverage for
collisions with both reserved annotations, preserving all unrelated user
annotations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ef805a7-632c-4b95-a4ad-17a1fb97270e
📥 Commits
Reviewing files that changed from the base of the PR and between f1493e0a7f0f7a0fa85d52cdd81d8b48665352ad and a4a78de.
📒 Files selected for processing (12)
.github/workflows/preview-helm.yml.pre-commit-config.yamldeploy/preview/Chart.yamldeploy/preview/README.mddeploy/preview/templates/_helpers.tpldeploy/preview/templates/deployment.yamldeploy/preview/templates/ingress.yamldeploy/preview/templates/service.yamldeploy/preview/tests/test_render.pydeploy/preview/values.yamldocs/domain/presentation-layer/specs/DESIGN.mddocs/domain/presentation-layer/specs/PRD.md
🚧 Files skipped from review as they are similar to previous changes (6)
- .pre-commit-config.yaml
- deploy/preview/Chart.yaml
- deploy/preview/README.md
- deploy/preview/templates/_helpers.tpl
- docs/domain/presentation-layer/specs/PRD.md
- .github/workflows/preview-helm.yml
cyberantonz
enabled auto-merge
August 3, 2026 05:08
ktursunov
approved these changes
Aug 3, 2026
This was referenced Aug 3, 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.
What
Adds
insight-preview, a small standalone Helm chart atdeploy/preview/that provisions one FE preview experiment per release: aDeployment+Service+ a single prefix-stripIngressserved under/exp/<name>on one shared host.Provisioning is manual (no GitOps controller):
helm upgrade --install preview-<name> ...adds the/exp/<name>path andhelm uninstall preview-<name>removes it. The controller merges same-host route objects, so no central config is ever rewritten. Only the FE image varies per experiment; the backend never does.Design points
rewrite-target: /$2,path: /exp/<name>(/|$)(.*),pathType: ImplementationSpecific, mirroring the existing fakeidp chart./api/...is emitted unprefixed by the FE and is not matched here, so it flows to the shared backend route.Ingressnow; becomes a Gateway APIHTTPRouteafter the Envoy move ([EPIC] Self-service preview environments (CI-driven provisioning) #1981).preview-<name>never truncate-collides at the 63-char limit);ingress.hostandimage.tagare required; OIDC and the dev-impersonation escape hatch are mutually exclusive.Tests
deploy/preview/tests/test_render.py— 14 render-contract assertions (prefix-strip route, per-experiment naming, service→pod selector, and every guard)..github/workflows/preview-helm.yml—helm lint+ the pytest suite ondeploy/preview/**.yamlfmtpre-commit exclude to coverdeploy/*/templates(Go-templated, not parseable YAML).Specs
Extended the presentation PRD and DESIGN to record the shipped serving-path slice. The auth return path (#1972) and the synthetic-data pin (#1973) stay open, so the
cpt-presentation-fr-preview-envsFR and thecpt-presentation-component-preview-routercomponent remain unchecked.Closes #1971
Part of #1803
Summary by CodeRabbit
New Features
/exp/<name>.Documentation
Tests