feat(controller): clone a reference SandboxPool into each org namespace (#288)#746
feat(controller): clone a reference SandboxPool into each org namespace (#288)#746stubbi wants to merge 50 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds template-driven per-org SandboxPool provisioning. New controller flags and Helm values configure the template source, org pool name, and warm-min override; the reconciler clones the template into each org namespace when enabled. Tests, docs, and schema formatting are updated accordingly. ChangesPer-org pool template feature
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/threat-model.md`:
- Around line 1168-1178: Clarify the behavior of --org-default-pool-image in the
threat-model text: it should be described as provisioning a default SandboxPool
inside each already-existing org namespace, not as the feature that enables
per-org namespaces. Update the sentence in the affected paragraph to reflect
that --enable-org-tenancy is what gates namespace reconciliation, and keep the
explanation aligned with the existing terminology around SandboxPool, org
namespace, and isolation boundary.
In `@internal/controller/org_controller.go`:
- Around line 69-73: The org default pool and gateway default pool can drift
apart, causing unnamed creates to fail at runtime. Add a startup/config
validation or shared constant so `DefaultPoolName` in `org_controller` stays
aligned with the gateway’s `--default-pool` setting, and make the check explicit
in the controller and gateway main setup paths.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a1046dd4-ccc1-4f8a-b76e-49144f816a20
📒 Files selected for processing (8)
cmd/controller/main.godeploy/charts/mitos/templates/controller-deployment.yamldeploy/charts/mitos/values.schema.jsondeploy/charts/mitos/values.yamldocs/threat-model.mdinternal/controller/org_builders_test.gointernal/controller/org_controller.gointernal/controller/org_envtest_test.go
| // DefaultPoolName is the name of the default SandboxPool stamped into every | ||
| // org namespace. It MUST match the gateway's default pool name so a create | ||
| // that names no pool resolves it. Empty (with DefaultPoolImage also empty) | ||
| // disables per-org pool provisioning. | ||
| DefaultPoolName string |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate the gateway's default-pool flag/config to confirm it matches "default".
rg -n 'default-pool|DefaultPool' --iglob '*gateway*' -g '*.go'Repository: mitos-run/mitos
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant controller file and search for pool-name contract references.
git ls-files internal/controller/org_controller.go
echo '---'
cat -n internal/controller/org_controller.go | sed -n '1,180p'
echo '--- SEARCH ---'
rg -n 'DefaultPoolName|DefaultPoolImage|default pool|default-pool|SandboxPool|gateway' .Repository: mitos-run/mitos
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrowly search for the gateway-side configuration or defaults that govern unnamed-pool creation.
rg -n --hidden --glob '!**/vendor/**' --glob '!**/.git/**' \
'default[-_ ]pool|DefaultPool|pool name|sandbox pool|SandboxPool|DefaultPoolName' .Repository: mitos-run/mitos
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show surrounding code where the comment applies, plus any nearby validation logic.
sed -n '1,140p' internal/controller/org_controller.goRepository: mitos-run/mitos
Length of output: 6457
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the gateway's default-pool setting and its default value.
rg -n --hidden --glob '!**/vendor/**' --glob '!**/.git/**' \
'default-pool|defaultPool|DefaultPool|default pool' \
cmd internal docs | head -n 200Repository: mitos-run/mitos
Length of output: 15487
Keep --org-default-pool-name and --default-pool aligned (cmd/controller/main.go:114, cmd/gateway/main.go:97). Creates that omit pool/image/template fall back to the gateway default pool, so a mismatch turns the unnamed-create path into a runtime failure; add a startup/config check or shared default.
🤖 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 `@internal/controller/org_controller.go` around lines 69 - 73, The org default
pool and gateway default pool can drift apart, causing unnamed creates to fail
at runtime. Add a startup/config validation or shared constant so
`DefaultPoolName` in `org_controller` stays aligned with the gateway’s
`--default-pool` setting, and make the check explicit in the controller and
gateway main setup paths.
…ce (#288) Per-org namespace tenancy provisioned everything EXCEPT a SandboxPool, so a create in an org namespace had no pool to fork from. This closes that gap so the hosted deployment can run per-org namespaces instead of one shared namespace (which is what removes the shared-namespace referenced-object class of findings, e.g. the bare-name secret/workspace reference). When --org-pool-template-name is set (and --enable-org-tenancy), the OrgReconciler CLONES that reference pool's full spec (image, resources, placement, snapshots, network) into every org namespace with warm.min overridden (default 0), named --org-pool-name. Cloning the full spec from one source of truth is what makes the per-org pool schedule exactly where the reference does; a bare image would not carry the KVM placement the hosted nodes require. warm.min:0 means no idle warm husks, so N per-org pools cost nothing at rest (the snapshot is content-addressed and deduped per node); the first fork per org cold-starts and the pool warms on demand. The pool is cloned once and never clobbered, so autoscaler or operator warm tuning persists. Empty template name (the default) provisions no pool, so a self-host / single-tenant install is unaffected. Chart: controller.orgTenancy.poolTemplate.{name,namespace,orgPoolName,warmMin}. Threat-model 7a notes the pool lives inside the already-isolated org namespace and grants no cross-tenant reach. TDD: builder unit test (clone + deep-copy independence) + envtest (clones from a seeded reference pool when configured, skips when not). Refs #288. Signed-off-by: Jannes Stubbemann <jannes@openclaw.rocks>
cebd4ce to
57a4eed
Compare
Signed-off-by: Jannes Stubbemann <jannes@openclaw.rocks> # Conflicts: # deploy/charts/mitos/values.schema.json
|
Re-assessed against current main. Still relevant and not superseded: org_controller.go has had zero conflicting commits since #410, the merge is clean, CI is green, and per-org pools remain the missing enablement for flipping hosted prod to per-org namespaces. Three small fixes before merge:
Beyond the fixes, merging this is coupled to the operational decision to flip hosted prod to per-org tenancy, so it stays parked until that call is made; the feature is off by default and safe to land ahead of the flip if preferred. |
Thinking Path
Linked Issues or Issue Description
Refs #288 (per-org namespace tenancy). This is the enablement that lets the hosted deployment move off the single shared namespace, complementing the defense-in-depth in #739.
What Changed
internal/controller/org_controller.go:ensureOrgPool+buildOrgPoolFromTemplate. WhenPoolTemplateNameis set, CLONE that reference pool's full spec (image, resources, placement, snapshots, network) into the org namespace withwarm.minoverridden (default 0), namedOrgPoolName, org label, owner-referenced to the Org. Cloning the full spec is what makes the per-org pool schedule where the reference does (a bare image would not carry the KVM placement). Cloned once and never clobbered. No-op when unset (self-host / bring-your-own-pool).cmd/controller/main.go:--org-pool-template-name/--org-pool-template-namespace/--org-pool-name/--org-pool-warm-minflags, wired under--enable-org-tenancy, with a startup log line.controller.orgTenancy.poolTemplate.{name,namespace,orgPoolName,warmMin}values, rendered onto the controller args (gated on name being set);values.schema.jsonupdated.docs/threat-model.md7a: notes the pool lives inside the already-isolated org namespace and grants no cross-tenant reach.Verification
go build ./...eval $(setup-envtest use 1.31 -p env) && go test ./internal/controller/ -run Org(new:TestBuildOrgPoolFromTemplateincl. deep-copy independence,TestOrgReconcilerProvisionsPoolFromTemplate,TestOrgReconcilerSkipsPoolWhenUnconfigured; existing Org stack tests still green; all TDD failing-first)helm template --kube-version 1.31: pool args render only whencontroller.orgTenancy.poolTemplate.nameis set, absent otherwise; schema validatescreateonsandboxpools)Risks
Low risk. Off by default (empty image), so self-host and the current single-tenant prod are unaffected until the flag is set. The pool is created once and never overwritten, so it cannot clobber warm tuning.
warm.min:0means no idle cost. The one operational note: the cloned pool name (OrgPoolName) must match the pool name a create resolves (the SDK image/pool argument, e.g.python); documented in the values.Model Used
Claude Fable 5 (
claude-fable-5), Anthropic, via Claude Code. Extended reasoning mode.Checklist
Signed-off-bytrailerSummary by CodeRabbit
SandboxPoolby cloning a reference template, including template location, per-org pool name, andwarm.minoverride.warm.minoverride; per-org pools are created only when missing (no updates to existing).