Repository navigation
Prewarm front backend before GCP cutover - #164
Conversation
📝 WalkthroughWalkthroughThe change adds a URL-map routing helper, front-slot canary preparation, evidence validation, migration-script integration, and regression coverage for routing state, timing, session continuity, and invalid canary proofs. ChangesFront-slot migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Migration as migrate-to-front-slots.sh
participant Routing as url-map-routing.py
participant Front as Front backend
participant Journal as Front-slot journal
participant Validator as validate-deploy-evidence.py
Migration->>Routing: Prepare and assert canary route
Migration->>Front: Send authenticated public canary requests
Front->>Journal: Record front-slot request
Migration->>Validator: Emit routing and stability evidence
Validator-->>Migration: Validate canary and backend-health timing
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 7
🧹 Nitpick comments (5)
cmd/subrouter/deploy_scripts_test.go (1)
21-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrong end-to-end coverage of the routing helper.
Every destination path uses a fresh
t.TempDir(), so no call violates the source-differs-from-destination rule inwrite_map. The hijacked-host case on line 141 rejects for the correct reason: the canary matcher is still present while the expected host is gone, socanary_statefails the partial-configuration check on line 233 ofdeploy/gcp/url-map-routing.py.One gap. No case uses backend URLs where one is a prefix of another. That is the scenario behind the substring reference counting raised on
deploy/gcp/url-map-routing.pylines 263-272. Add a case with URLs such as.../staging-frontand.../staging-front-v2when that fix lands.🤖 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 `@cmd/subrouter/deploy_scripts_test.go` around lines 21 - 145, Extend TestGCPURLMapCanaryRemainsReferencedAcrossActiveRouteSwitches with a backend URL prefix-collision case, using distinct URLs such as staging-front and staging-front-v2. Exercise the relevant prepare/rewrite/assert flow and verify references are counted per complete URL rather than by substring, ensuring the shorter URL is not mistaken for a reference to the longer one.deploy/gcp/url-map-routing.py (1)
339-359: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider named options for the shared arguments.
Three subcommands take six to eight positional arguments in a fixed order. A call site that swaps
canary_hostandcanary_urlfails closed on theHOSTpattern, so the current risk is low. Named options such as--active-matcherand--canary-hostwould make the call sites inmigrate-to-front-slots.sh,switch-front-migration.sh, andfinalize-legacy-retirement.shself-describing.🤖 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/gcp/url-map-routing.py` around lines 339 - 359, Update build_parser to replace the shared positional arguments for prepare-canary, rewrite-active, and assert-state with explicit named options, including --active-matcher, --expected-active-url, --canary-matcher, --canary-host, and --canary-url, while preserving command-specific source, destination, new_url, and --forbid-url behavior. Update the three shell call sites to pass these values using the new option names.cmd/subrouter-transport-observer/testdata/golden_fake_client/main.go (1)
59-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCount only registered PID records, not temporary files.
registerFakeProcesson lines 444-448 creates a.process-*temporary file in the same directory before renaming it to the PID path.os.ReadDircounts that temporary file. A session registration that overlaps this check can therefore push the count past 2 without a held session being registered yet, which weakens the guard the test is meant to enforce.The current test starts several sessions before preparation, so the count is far above the threshold and the test is not flaky. Filtering the prefix makes the assertion match its stated intent.
♻️ Proposed refactor
if operation == "migration-prepare" && os.Getenv("SUBROUTER_GOLDEN_FAKE_REQUIRE_SESSIONS_DURING_PREPARE") == "1" { entries, err := os.ReadDir(os.Getenv("SUBROUTER_GOLDEN_FAKE_PROCESS_STATE")) // The harness process and this action are already registered. Require at // least one separately held Codex session before migration preparation. - if err != nil || len(entries) <= 2 { + registered := 0 + for _, entry := range entries { + if !strings.HasPrefix(entry.Name(), ".process-") { + registered++ + } + } + if err != nil || registered <= 2 { os.Exit(9) } }🤖 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 `@cmd/subrouter-transport-observer/testdata/golden_fake_client/main.go` around lines 59 - 66, Update the migration-prepare guard in the operation check to count only registered PID record entries, filtering the os.ReadDir results to the established PID filename pattern and excluding temporary .process-* files before applying the threshold. Preserve the existing error and exit behavior while ensuring the session-count assertion reflects registered processes only.deploy/gcp/migrate-to-front-slots.sh (1)
193-200: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDistinguish an SSH failure from a missing journal entry.
canary_session_observedsends both stdout and stderr to/dev/null. A broken IAP tunnel, an expired credential, and a missing journal entry all return the same non-zero status. The caller then retries for ten minutes and fails with "did not correlate through the public load balancer", which points the operator at the load balancer instead of the SSH path. The behavior is fail-closed, so this is a diagnosability gap only.Record the SSH stderr to an artifact file so the operator can separate the two causes.
🤖 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/gcp/migrate-to-front-slots.sh` around lines 193 - 200, Update canary_session_observed to preserve the gcloud SSH command’s stderr in an artifact file instead of discarding it with >/dev/null 2>&1. Keep journal-query stdout suppressed and retain the existing exit-status behavior, ensuring the artifact captures IAP, credential, or SSH failures for diagnosis.cmd/subrouter-transport-observer/golden_acceptance_regression_test.go (1)
179-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for the twenty-minute canary cap.
Three new rules have no negative case: the unsupported instance on line 271 of
cmd/subrouter-transport-observer/golden_migration_evidence.go, the 600-attempt ceiling on line 289, and the twenty-minute canary cap on line 340. The twenty-minute cap matters most, becausedeploy/gcp/migrate-to-front-slots.shcan produce a longer window without failing. A negative case documents the boundary that the script must respect.🤖 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 `@cmd/subrouter-transport-observer/golden_acceptance_regression_test.go` at line 179, Add a negative acceptance test in the relevant golden regression test cases covering the twenty-minute canary cap defined by the migration evidence rules. Exercise a canary window longer than twenty minutes and assert that it is rejected, using the existing test structure and symbols around the canary-cap rule rather than changing the production script.
🤖 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 `@cmd/subrouter-transport-observer/golden_acceptance_regression_test.go`:
- Around line 160-169: Update the shortCanary fixture by moving
first_observed_at forward to create a sub-five-minute canary window, while
keeping canary verified_at at 2026-08-02T00:05:00Z and preserving the valid
ordering chain. Leave the validateGo and validatePython assertions focused
solely on rejecting the duration bound.
In `@deploy/gcp/migrate-to-front-slots.sh`:
- Around line 427-431: Update the canary stability validation near
canary_stable_duration_ms and the existing five-minute lower-bound check to also
reject durations above the validator’s 20-minute maximum (1,200,000 ms). Ensure
the script fails with a clear diagnostic before writing evidence, while
preserving acceptance of durations within the inclusive five-to-twenty-minute
range.
In `@deploy/gcp/switch-front-migration.sh`:
- Around line 115-117: The routing extraction in
deploy/gcp/switch-front-migration.sh lines 115-117 must default missing jq
fields to empty and immediately die when ACTIVE_MATCHER, CANARY_MATCHER, or
CANARY_HOST is empty. Apply the same change to
deploy/gcp/finalize-legacy-retirement.sh lines 73-75 for its corresponding
variables, preserving the existing validation flow.
In `@deploy/gcp/url-map-routing.py`:
- Around line 146-155: Update the host parsing in host_rule to validate every
non-field line in the item, rather than only the slice between hosts and
pathMatcher. Reject any host entry appearing after pathMatcher (and preserve
rejection of unsupported YAML), while continuing to collect valid hosts and
enforce non-empty uniqueness.
- Around line 263-272: Replace raw body.count(url) checks in the
reference-validation loops with counts derived from parsed defaultService
values, comparing complete service URLs rather than substrings. Update both
expected reference validation and forbidden URL validation while preserving the
existing failure messages and distinct-backend checks.
In `@deploy/gcp/validate-deploy-evidence.py`:
- Around line 656-661: Require the two canary session hashes to be distinct in
both validators. In deploy/gcp/validate-deploy-evidence.py lines 656-661,
capture the results of the sha() calls for first_session_sha256 and
verified_session_sha256 and fail when they match; in
cmd/subrouter-transport-observer/golden_migration_evidence.go lines 286-293, add
equality of canary.FirstSessionSHA256 and canary.VerifiedSessionSHA256 to the
migration_canary_invalid condition.
- Around line 619-634: Align the instance handling in the validator around
expected_routing with migrate-to-front-slots.sh and
golden_migration_evidence.go: support custom instances by deriving the active
matcher, canary matcher, and canary host from the declared environment variables
instead of rejecting instances outside the fixed table, while retaining the
named-instance defaults.
---
Nitpick comments:
In `@cmd/subrouter-transport-observer/golden_acceptance_regression_test.go`:
- Line 179: Add a negative acceptance test in the relevant golden regression
test cases covering the twenty-minute canary cap defined by the migration
evidence rules. Exercise a canary window longer than twenty minutes and assert
that it is rejected, using the existing test structure and symbols around the
canary-cap rule rather than changing the production script.
In `@cmd/subrouter-transport-observer/testdata/golden_fake_client/main.go`:
- Around line 59-66: Update the migration-prepare guard in the operation check
to count only registered PID record entries, filtering the os.ReadDir results to
the established PID filename pattern and excluding temporary .process-* files
before applying the threshold. Preserve the existing error and exit behavior
while ensuring the session-count assertion reflects registered processes only.
In `@cmd/subrouter/deploy_scripts_test.go`:
- Around line 21-145: Extend
TestGCPURLMapCanaryRemainsReferencedAcrossActiveRouteSwitches with a backend URL
prefix-collision case, using distinct URLs such as staging-front and
staging-front-v2. Exercise the relevant prepare/rewrite/assert flow and verify
references are counted per complete URL rather than by substring, ensuring the
shorter URL is not mistaken for a reference to the longer one.
In `@deploy/gcp/migrate-to-front-slots.sh`:
- Around line 193-200: Update canary_session_observed to preserve the gcloud SSH
command’s stderr in an artifact file instead of discarding it with >/dev/null
2>&1. Keep journal-query stdout suppressed and retain the existing exit-status
behavior, ensuring the artifact captures IAP, credential, or SSH failures for
diagnosis.
In `@deploy/gcp/url-map-routing.py`:
- Around line 339-359: Update build_parser to replace the shared positional
arguments for prepare-canary, rewrite-active, and assert-state with explicit
named options, including --active-matcher, --expected-active-url,
--canary-matcher, --canary-host, and --canary-url, while preserving
command-specific source, destination, new_url, and --forbid-url behavior. Update
the three shell call sites to pass these values using the new option names.
🪄 Autofix
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: 1a48aef3-b231-4297-b5da-6d91b2e9cb13
📒 Files selected for processing (12)
cmd/subrouter-transport-observer/golden_acceptance_regression_test.gocmd/subrouter-transport-observer/golden_local_mac_harness_test.gocmd/subrouter-transport-observer/golden_migration_cycle.gocmd/subrouter-transport-observer/golden_migration_evidence.gocmd/subrouter-transport-observer/testdata/golden_fake_client/main.gocmd/subrouter/deploy_scripts_test.godeploy/gcp/finalize-legacy-retirement.shdeploy/gcp/golden-local-mac-production-continuity.shdeploy/gcp/migrate-to-front-slots.shdeploy/gcp/switch-front-migration.shdeploy/gcp/url-map-routing.pydeploy/gcp/validate-deploy-evidence.py
1 participant
Starts held Codex sessions before migration preparation, keeps the front backend referenced through an unadvertised canary host, and requires five minutes of public canary plus backend-health evidence before active route changes. Also fixes RSS sampler waiting in a subshell.
Tests: go test -p 1 ./...; focused URL-map and golden harness regressions; shell syntax and shellcheck.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Prewarms the front backend through a hidden, authenticated canary route, hard-binds routing selectors to the target instance, and continuously samples front readiness (public canary + backend health) before switching GCP URL‑map routes. Requires five minutes of uninterrupted evidence before changing the active route.
New Features
url-map-routing.pyto keep a dedicated canary host/matcher, switch only the active route, retain the warm canary across switches, and assert URL‑map state.canary-security-policy.pyto render/verify a Cloud Armor policy that authenticates the canary via a token; migration scripts apply and pin this policy per instance.wait-for-front-readiness.pyandprobe-front-readiness.shto sample public canary responses tied to backend health; the probe forwards the canary token; shared parsing moved tobackend_health.py.routing.active_matcherandrouting.canary(host, security policy, attempts, sessions, density, timings); validators pin matcher names, canary host, and security policy per instance and ensurerouting.canary.backend_urlequals the front URL; migration scripts assert selectors, require a held Codex session before prepare, and run the continuous readiness probe during prewarm.Bug Fixes
Written for commit 456b008. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests