Skip to content

feat(controller): clone a reference SandboxPool into each org namespace (#288)#746

Open
stubbi wants to merge 50 commits into
mainfrom
feat/org-default-pool
Open

feat(controller): clone a reference SandboxPool into each org namespace (#288)#746
stubbi wants to merge 50 commits into
mainfrom
feat/org-default-pool

Conversation

@stubbi

@stubbi stubbi commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

  • Mitos runs the hosted product with a control plane that can place each org in its own Kubernetes namespace (mitos-org-<id>) via the OrgReconciler, behind --enable-org-tenancy.
  • Prod currently runs a single shared namespace instead, which is why the battle-test found shared-namespace referenced-object issues (bare-name secret/workspace refs, hardened defensively in fix(saas): refuse secretRef/workspace refs in single-tenant mode (GHSA-pgv2-9w24-j7wh) #739).
  • The blocker to flipping prod to per-org namespaces is that the reconciler provisions the namespace, quota, LimitRange, NetworkPolicy, and RBAC but NOT a SandboxPool, so a create in an org namespace has no pool to fork from.
  • It needs solving now because per-org namespaces are the root fix that removes the shared-namespace class of findings, and per-org pools are the one missing piece.
  • This pull request has the reconciler stamp a warm.min:0 pool into each org namespace when an image is configured.
  • The benefit is that the hosted deployment can run true per-org isolation with no per-org warm-husk cost (the snapshot is content-addressed and deduped; the first fork per org cold-starts).

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. When PoolTemplateName is set, CLONE that reference pool's full spec (image, resources, placement, snapshots, network) into the org namespace with warm.min overridden (default 0), named OrgPoolName, 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-min flags, wired under --enable-org-tenancy, with a startup log line.
  • Chart: controller.orgTenancy.poolTemplate.{name,namespace,orgPoolName,warmMin} values, rendered onto the controller args (gated on name being set); values.schema.json updated.
  • docs/threat-model.md 7a: 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: TestBuildOrgPoolFromTemplate incl. 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 when controller.orgTenancy.poolTemplate.name is set, absent otherwise; schema validates
  • gofmt clean; no em/en dashes; no api/CRD/RBAC drift (the controller already holds create on sandboxpools)

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:0 means 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

  • PR title is a conventional commit
  • Thinking Path traces from project context to this change
  • Model Used is filled in
  • Tests added for behavior changes, in the same commit (TDD)
  • Docs updated in the same PR (threat-model 7a, chart values)
  • Threat-model delta included
  • Benchmark run (n/a)
  • No em or en dashes introduced anywhere
  • Secret values never logged, in errors, in condition messages, or on host paths
  • No internal/instance-local references
  • Every commit carries a Signed-off-by trailer

Summary by CodeRabbit

  • New Features
    • Added org-tenancy CLI/Helm configuration to optionally provision per-org SandboxPool by cloning a reference template, including template location, per-org pool name, and warm.min override.
  • Bug Fixes
    • Cloned pools preserve template image/placement while applying the warm.min override; per-org pools are created only when missing (no updates to existing).
  • Documentation
    • Updated threat-model docs to reflect cloning behavior and boundary implications.
  • Tests
    • Added envtest coverage for provisioning from template and skipping when unconfigured.

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This 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.

Changes

Per-org pool template feature

Layer / File(s) Summary
OrgReconciler pool provisioning
internal/controller/org_controller.go, internal/controller/org_builders_test.go, internal/controller/org_envtest_test.go
Adds template-based per-org pool creation in ensureStack, deep-copies the source SandboxPool, overrides warm.min, sets org labels and ownership, and verifies the clone/skip behavior in unit and envtest coverage.
CLI and chart wiring
cmd/controller/main.go, deploy/charts/mitos/templates/controller-deployment.yaml
Adds org-tenancy pool template CLI flags, passes them into OrgReconciler, updates startup logging, and conditionally renders the matching Helm container args.
Values and documentation updates
deploy/charts/mitos/values.yaml, docs/threat-model.md, deploy/charts/mitos/values.schema.json
Adds controller.orgTenancy.poolTemplate values and behavior notes, documents the cloning behavior in the threat model, and reformats schema fragments without changing constraints.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is conventional, concise, and accurately summarizes the main change: cloning a reference SandboxPool into each org namespace.
Description check ✅ Passed The description covers the required sections with a clear thinking path, linked issue reference, change summary, verification, risks, model used, and checklist.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/org-default-pool

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 76cc54b and cebd4ce.

📒 Files selected for processing (8)
  • cmd/controller/main.go
  • deploy/charts/mitos/templates/controller-deployment.yaml
  • deploy/charts/mitos/values.schema.json
  • deploy/charts/mitos/values.yaml
  • docs/threat-model.md
  • internal/controller/org_builders_test.go
  • internal/controller/org_controller.go
  • internal/controller/org_envtest_test.go

Comment thread docs/threat-model.md Outdated
Comment thread internal/controller/org_controller.go Outdated
Comment on lines +69 to +73
// 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ 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.go

Repository: 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 200

Repository: 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>
@stubbi stubbi force-pushed the feat/org-default-pool branch from cebd4ce to 57a4eed Compare July 5, 2026 21:51
@stubbi stubbi changed the title feat(controller): auto-provision a warm.min:0 SandboxPool per org namespace (#288) feat(controller): clone a reference SandboxPool into each org namespace (#288) Jul 5, 2026
@stubbi stubbi enabled auto-merge (squash) July 5, 2026 22:06
@stubbi

stubbi commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

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:

  1. TemplateRef gap: buildOrgPoolFromTemplate copies the reference spec verbatim, so a reference pool using spec.templateRef (resolved in the pool's OWN namespace) clones a dangling bare-name ref into every org namespace. Reject a TemplateRef template at ensureOrgPool time with actionable text, or resolve it inline at clone time, plus a test.
  2. Deletion self-heal: SetupWithManager Owns the namespace/quota/netpol/rolebinding but not SandboxPool, so a deleted per-org pool only comes back on the next Org event or resync despite the doc claiming stack objects self-heal. Add Owns(&v1.SandboxPool{}); create-once semantics already skip existing pools.
  3. The two open CodeRabbit threads predate the template-clone rework, which substantively addresses both, but they need replies per repo policy. The surviving point is real: OrgPoolName must match what the gateway's --default-pool resolves or every org gets a silent dead end; at minimum document the invariant in the flag help, optionally cross-check at startup.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant