PRD: reprovision on gateway startup - bottle ca (chunk 1/5) #519
Open
didericis-claude
wants to merge 9 commits from
prd-0081-reconcile-1-ca into prd-gateway-reprovision-on-bringup
pull from: prd-0081-reconcile-1-ca
merge into: didericis:prd-gateway-reprovision-on-bringup
didericis:main
didericis:prd-0081-reconcile-2-git-gate
didericis:production
didericis:staging
didericis:prd-gateway-reprovision-on-bringup
didericis:prd-forge-subroles
didericis:fix/shared-storage-permissions
didericis:fix/authoritative-cleanup-identities
didericis:fix/bounded-heavy-operations
didericis:fix/cleanup-execution-integrity
didericis:fix/gateway-output-pump-shutdown
didericis:fix/gateway-request-boundaries
didericis:fix/authoritative-cleanup-revalidation
didericis:refactor/egress-policy-pipeline
didericis:refactor/orchestrator-fastapi
didericis:refactor/supervisor-mcp-dispatch
didericis:chore/misc-quality-cleanup
didericis:host-control-server-2-secret
didericis:host-control-server-3-list-live
didericis:host-control-server-1-transport
didericis:prd-new-host-control-server
didericis:fix/gitgate-persist-512
didericis:fix/firecracker-gateway-ca-persist-510
didericis:didericis/disable-agit-refs
didericis:didericis/prd-audit-event-schema
didericis:fix/pr-495-audit-contract
didericis:prd-egress-control-plane
didericis:prd-macos-container-ci-runner
didericis:refactor/control-plane-auth-provisioning-476
didericis:fix/db-off-data-plane-469
didericis:feat/encrypted-egress-secrets
didericis:spike/rootless-docker-macos
didericis:fix/ci-coverage-artifact-paths
didericis:claude-forward-host-credentials-rebased
didericis:fix-gateway-gitleaks-arch
didericis:fix/websocket-response-dlp-multitenant
didericis:orchestrator-agent-compose
didericis:orchestrator-gateway-ca
didericis:orchestrator-consolidated-launch
didericis:orchestrator-gateway-provision
didericis:orchestrator-gateway-network
didericis:orchestrator-client
didericis:orchestrator-gateway-net
didericis:orchestrator-gitgate-provision
didericis:orchestrator-registration
didericis:orchestrator-lifecycle
didericis:orchestrator-supervise-writers
didericis:orchestrator-supervise-multitenant
didericis:orchestrator-gitgate-multitenant
didericis:orchestrator-rename-gateway
didericis:orchestrator-slice8
didericis:orchestrator-slice7
didericis:orchestrator-slice6
didericis:prd-orchestrator
didericis:orchestrator-slice5
didericis:orchestrator-slice4
didericis:orchestrator-slice3
didericis:orchestrator-slice2
didericis:firecracker-backend
didericis:forge-native-integration
didericis:prd-smolmachines-linux
didericis:manifest-break-import-cycle
didericis:dlp-supervise-quality-fixes
didericis:table-drive-dlp-tests
didericis:fix-integration-test-failures
didericis:fix/macos-container-relative-dockerfile
didericis:prd-0054-install-script
didericis:commit-bottle-state
didericis:pr-211
didericis:move-codex-auth-to-contrib
didericis:feat/pipelock-skip-scan-extensions
didericis:prd-0049-named-labelled-agents
didericis:harden-git-gate-shell-rendering
Dismiss Review
Are you sure you want to dismiss this review?
Labels
Clear labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Status/Needs Triage
Breaking change that won't be backward compatible
Something is not working
Documentation changes
Improve existing functionality
New functionality
This is security issue
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
Awaiting initial classification
No Label
Milestone
No items
No Milestone
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: didericis/bot-bottle#519
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Delete Branch "prd-0081-reconcile-1-ca"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Stacked on #517. First of the bring-up reconcile PRs tracked by #516.
Implements the reconcile contract + its first step (CA):
attach_bottled_agents_to_gateway()as an@abc.abstractmethodonBottleBackend(backend/base.py). Reconciling running bottles is a backend responsibility — only the backend can enumerate its agents and reach them (firecracker over SSH, docker/macOS overexec/cp) — so every backend must implement it. Cross-backend by construction, no per-backend follow-up.SSL certificate verification failed— #510); this is what distributes the fresh CA, so a routine gateway rebuild now doubles as a free CA rotation.Gateway.connect_to_orchestratornow returns a cold-boot bool (True when it actually (re)brought the gateway up, False when a healthy current gateway was left untouched). Each infraensure_runninggates the reconcile on it, so it fires exactly on cold boot and never on an adopt.Still to come on this branch (see #516)
_reprovision_running_bottles.host_gateway_ca_dirbind-mount so they rotate the CA like firecracker (maintainer confirmed "consistent across backends" on #517).Tests: ABC + CA push per backend (
test_backend_secret_reprovision), the cold-boot bool on eachconnect_to_orchestrator(test_orchestrator_gateway/test_firecracker_gateway/test_macos_gateway), and the cold-boot-gated wiring in each infra (test_*_infra). Full unit suite green (the sole failure istest_wheel_install, an environment-only missing-build-module issue, unrelated).Refs #516. Addresses #510.
🤖 Generated with Claude Code
P1 — CA lookup failures abort gateway startup on Docker and macOS. In
bot_bottle/backend/docker/consolidated_launch.py:123-124andbot_bottle/backend/macos_container/consolidated_launch.py:144-145,ca_cert_pem()is called outside the best-effort exception handling. It can raiseGatewayErrorafter its polling timeout. Because these methods run synchronously fromensure_running(), a temporarily unavailable CA makes the whole bring-up command fail after the gateway has already started. This contradicts the stated contract that reconciliation never blocks the gateway coming up. Firecracker already handles this correctly. Please catchGatewayErroraround CA retrieval, log the skipped reconciliation, and return.I could not run the focused tests because
pytestis not installed in the review environment. Static inspection otherwise found no additional correctness issues.Same inconsistency @didericis-codex mentioned, but solve should be in the opposite direction (purposefully fail gateway startup). We don't want hidden broken behavior. If PRD needs updating for that do it. Also noticed some other issues.
@@ -507,0 +524,4 @@fingerprint match is needed), which rotates the CA for free on everybring-up. Git-gate re-provision and egress-token restore fold into thissame reconcile on the same trigger (see #516)."""We should enforce the flow more in here/only overwrite specific methods so things are more consistent. This is a larger theme we should have as an ADR somewhere... as much as possible, try to encode shared behavior in the base backend logic. We want to lean towards LESS freedom in the specific backend implementations, and MORE consistency.
Basic structure is I think uniform. Rough shape
@@ -77,0 +139,4 @@_push_ca_to_container(name, ca_pem)reconciled += 1except (OSError, ConsolidatedLaunchError) as exc:log.info(f"CA reconcile skipped for {name}: {exc}")Same comment as below, never skip
@@ -92,0 +106,4 @@input=ca_pem, capture_output=True, text=True, check=False,)if proc.returncode != 0:raise ConsolidatedLaunchError(Nit, but should rename this "InfraLaunchError" and have a consistent
infra_launchfile across backends. "consolidated launch" is unclear. In glossary explain "infra" refers to the gateway and orchestrator services.@@ -92,0 +124,4 @@try:ca_pem = gateway.ca_cert_pem()except GatewayError as exc:info(f"bring-up reconcile skipped: gateway CA unavailable: {exc}")Don't skip, do a hard fail here
@@ -92,0 +136,4 @@_push_gateway_ca_to_agent(private_key, guest_ip, ca_pem)reconciled += 1except (OSError, ConsolidatedLaunchError) as exc:info(f"CA reconcile skipped for {guest_ip}: {exc}")Same, do a hard fail/never skip. If the attachment doesn't work we want to vocalize it
@@ -101,0 +146,4 @@try:agents = list(enumerate_active())except (EnumerationError, OSError) as exc:info(f"CA reconcile skipped: {exc}")same issue as before/should fail hard
Thanks both. Going with @didericis's fail-hard direction over the skip in the codex review: a bottle that silently can't reach the fresh gateway (TLS just starts failing) is exactly the hidden-broken-behavior we don't want, so a loud bring-up failure beats limping on. Concrete plan:
Fail hard, never skip (6139 / 6140 / 6142 / 6144, + codex's P1). Every attach step raises instead of log-and-continue — CA lookup and each per-bottle push, on all three backends. If a running bottle can't be reconciled against the fresh gateway, bring-up fails and names the bottle. This reverses the current PRD 0081 goal "per-bottle failures are tolerated / one unreachable bottle does not block the others" — I'll flip that goal in the PRD to say the opposite (fail loud on any attach failure).
Encode the flow in the base backend (6143). Make
attach_bottled_agents_to_gateway()a concrete method onBottleBackendthat owns the shared shape + the fail-hard policy, with backends overriding only the primitives:_gateway_attach_resources()→ what's needed to attach a bottle (CA now; egress secrets / git-gate state as the later stacked PRs land)_running_bottles()→ enumerate the live bottles_attach_bottle_to_gateway(bottle, resources)→ push the resources into one bottle (SSH for firecracker, exec+cp for docker/macOS)The base runs steps 2–3 and raises on any failure, so a backend can't drift onto a bespoke loop or a silent skip. I'll capture the theme — shared behavior lives in the base backend; subclasses override primitives, not control flow — as an ADR under
docs/decisions/.Rename (6141).
ConsolidatedLaunchError→InfraLaunchErrorandconsolidated_launch.py→infra_launch.pyacross the three backends, plus a glossary entry defining infra = the gateway + orchestrator services. Heads-up thatconsolidated_launchis referenced fairly widely (including other in-flight branches), so I'll keep the rename mechanical and in its own commit to keep the diff readable.Since 6143 is architectural and you called for an ADR, I'll start on the base-class refactor + fail-hard now unless you'd rather shape the primitive split first.
Pushed
de80aaf..3c0d2fbaddressing the review — two commits on top of the original:4f2ccaerename (6141).consolidated_launch.py→infra_launch.pyandConsolidatedLaunchError→InfraLaunchErroracross all three backends + callers/tests. Unified the three identical per-backend error classes into one sharedInfraLaunchErrorinbackend/base.py. Added a glossary entry defining infra = the per-host gateway + orchestrator pair. No behaviour change.3c0d2fbbase owns the flow + fail hard (6143 + the fail-hard direction).attach_bottled_agents_to_gateway()is now concrete onBottleBackend, delegating togateway_attach.reconcile_running_bottles. Backends override only three primitives:_gateway_attach_resources()→_running_bottles()→_attach_bottle_to_gateway()— exactly the shape you sketched (find resources → list running bottles → attach each). The flow +GatewayAttachResourceslive in a newbackend/gateway_attach.pysobase.pystays the contract.InfraLaunchErrornaming them all) so a bring-up surfaces the full blast radius. Resource gathering (the CA fetch) fails hard too.base.pysize guardrail 580→600 for the new contract surface (delegator + 3 abstract primitives); the flow itself is ingateway_attach.py, notbase.py.Verification: full unit suite green except
test_wheel_install(environment missing thebuildmodule, unrelated); pyright clean; pylint 9.76. Ready for another look.PRD 0081: reconcile running bottles' CA on gateway bring-upto PRD: reprovision on gateway startup - bottle ca (chunk 1/5)b315a127b8to75957b75b875957b75b8toec45dd2764ec45dd2764to606a57b7ed606a57b7edtoa401310865Implemented the requested fail-closed fix.
@@ -0,0 +89,4 @@"docker", "network", "inspect", "--format","{{range .Containers}}{{.Name}}\n{{end}}", network,])if proc.returncode != 0:Fixed in
e49f9a4: nonzerodocker network inspectnow raisesInfraLaunchErrorwith stderr (or the exit code), and the regression test covers a completed process returning failure.The blocking Docker enumeration issue is fixed, the custom-infra regression is covered, and all CI checks pass.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.