From 50444fe4f4bb758ab847da6843c4f92f933e0eae Mon Sep 17 00:00:00 2001 From: pallakatos Date: Mon, 7 Sep 2026 18:43:58 +0200 Subject: [PATCH 1/5] feat(controller): bind sandbox namespaces to workspace and instance UIDs Add atomic namespace claims, conservative legacy adoption, preconditioned cleanup, and workspace-aware guards for approval cleanup and router token reads. Preserve pre-CR credential staging through explicit namespace reservations, and preflight both upgrade paths before changing controllers. Validation: 107 focused CLI/Helm tests, TypeScript typecheck, changed-file lint, and Rust formatting pass. The 26 focused Rust tests await the coordinated shared Cargo slot; no Rust build or target directory was created. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: eb3654cd-f1e0-445a-8734-430800af1903 --- cli/src/cli.ts | 2 + cli/src/commands/add.ts | 9 +- cli/src/commands/namespace.test.ts | 39 + cli/src/commands/namespace.ts | 27 + cli/src/commands/up/fast_upgrade.ts | 2 + cli/src/commands/upgrade.ts | 2 + cli/src/lib/kube-bootstrap.ts | 2 +- cli/src/lib/namespace-ownership.test.ts | 228 +++++ cli/src/lib/namespace-ownership.ts | 275 +++++++ cli/src/testing/namespace-ownership.test.ts | 71 ++ controller/src/egress_approval_reconciler.rs | 55 +- controller/src/inference_policy_reconciler.rs | 2 +- controller/src/kars_memory_reconciler.rs | 2 +- controller/src/reconciler/mod.rs | 96 +-- .../src/reconciler/namespace_ownership.rs | 567 +++++++++++++ .../reconciler/namespace_ownership_tests.rs | 777 ++++++++++++++++++ .../src/status/router_confirmation_io.rs | 15 +- controller/src/tool_policy_reconciler.rs | 3 +- deploy/helm/kars/templates/rbac.yaml | 2 +- docs/how-to/helm-installation.md | 5 + docs/how-to/namespace-ownership.md | 159 ++++ tests/compat/fixtures/namespace-legacy.json | 77 ++ 22 files changed, 2324 insertions(+), 93 deletions(-) create mode 100644 cli/src/commands/namespace.test.ts create mode 100644 cli/src/commands/namespace.ts create mode 100644 cli/src/lib/namespace-ownership.test.ts create mode 100644 cli/src/lib/namespace-ownership.ts create mode 100644 cli/src/testing/namespace-ownership.test.ts create mode 100644 controller/src/reconciler/namespace_ownership.rs create mode 100644 controller/src/reconciler/namespace_ownership_tests.rs create mode 100644 docs/how-to/namespace-ownership.md create mode 100644 tests/compat/fixtures/namespace-legacy.json diff --git a/cli/src/cli.ts b/cli/src/cli.ts index 63940a39..e724fdcb 100644 --- a/cli/src/cli.ts +++ b/cli/src/cli.ts @@ -11,6 +11,7 @@ import { upgradeCommand } from "./commands/upgrade.js"; import { devCommand } from "./commands/dev.js"; import { addCommand } from "./commands/add.js"; import { credentialsCommand } from "./commands/credentials.js"; +import { namespaceCommand } from "./commands/namespace.js"; import { configCommand } from "./commands/config.js"; import { connectCommand } from "./commands/connect.js"; import { statusCommand } from "./commands/status.js"; @@ -71,6 +72,7 @@ export function createCli(): Command { // Configuration program.addCommand(credentialsCommand()); + program.addCommand(namespaceCommand()); program.addCommand(configCommand()); program.addCommand(modelCommand()); program.addCommand(policyCommand()); diff --git a/cli/src/commands/add.ts b/cli/src/commands/add.ts index b46b459a..55c5a01e 100644 --- a/cli/src/commands/add.ts +++ b/cli/src/commands/add.ts @@ -6,6 +6,7 @@ import chalk from "chalk"; import ora from "ora"; import { loadContext, resolveSecret } from "../config.js"; import { assertRuntimeWired, buildRuntimeBlock, flagToKind } from "../runtime.js"; +import { CLAIM, prepareCredentialNamespace } from "../lib/namespace-ownership.js"; import { buildInferencePolicy, buildToolPolicy, @@ -449,9 +450,13 @@ generating per-sandbox AGT ToolPolicy / TrustGraph CRs. }; if (Object.keys(allSecrets).length > 0) { spinner.text = "Creating credential secret..."; + const namespaceUid = await prepareCredentialNamespace(execa, name, "kars-system"); + const metadata = sandbox.metadata as Record; + metadata.annotations = { + ...(metadata.annotations as Record | undefined), + [CLAIM.namespaceUid]: namespaceUid, + }; try { - // Ensure namespace exists - await execa("kubectl", ["create", "namespace", namespace], { stdio: "pipe" }).catch(() => {}); const secretArgs = ["create", "secret", "generic", `${name}-credentials`, "-n", namespace]; for (const [envVar, value] of Object.entries(allSecrets)) { secretArgs.push(`--from-literal=${envVar}=${value}`); diff --git a/cli/src/commands/namespace.test.ts b/cli/src/commands/namespace.test.ts new file mode 100644 index 00000000..20f4913a --- /dev/null +++ b/cli/src/commands/namespace.test.ts @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { afterEach, describe, expect, it, vi } from "vitest"; +import { execa } from "execa"; +import { namespaceCommand } from "./namespace.js"; + +vi.mock("execa", () => ({ execa: vi.fn() })); +afterEach(() => vi.restoreAllMocks()); + +describe("namespace commands", () => { + it("runs a read-only generic-context preflight", async () => { + vi.mocked(execa).mockResolvedValue({ stdout: '{"items":[]}' } as never); + vi.spyOn(console, "log").mockImplementation(() => {}); + await namespaceCommand().parseAsync(["preflight"], { from: "user" }); + expect(execa).toHaveBeenCalledWith("kubectl", [ + "get", "karssandboxes", "-A", "--show-managed-fields=true", "-o", "json", + ], { stdio: "pipe" }); + }); + + it("refuses adoption unless the administrator supplies both reviewed UIDs", async () => { + const command = namespaceCommand().exitOverride().configureOutput({ + writeErr: () => {}, writeOut: () => {}, + }); + for (const child of command.commands) { + child.exitOverride().configureOutput({ writeErr: () => {}, writeOut: () => {} }); + } + await expect(command.parseAsync(["adopt", "demo", "--namespace", "workspace-a"], { + from: "user", + })).rejects.toThrow("required option"); + }); + + it("propagates API failures instead of printing preflight success", async () => { + vi.mocked(execa).mockRejectedValue(new Error("Forbidden")); + const output = vi.spyOn(console, "log").mockImplementation(() => {}); + await expect(namespaceCommand().parseAsync(["preflight"], { from: "user" })).rejects.toThrow("Forbidden"); + expect(output).not.toHaveBeenCalled(); + }); +}); diff --git a/cli/src/commands/namespace.ts b/cli/src/commands/namespace.ts new file mode 100644 index 00000000..244f8e5a --- /dev/null +++ b/cli/src/commands/namespace.ts @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { Command } from "commander"; +import { execa } from "execa"; +import { adoptNamespace, inspectNamespaceOwnership } from "../lib/namespace-ownership.js"; + +export function namespaceCommand(): Command { + const command = new Command("namespace").description("Inspect and explicitly adopt sandbox namespace ownership"); + command.command("preflight") + .description("Read-only ownership checks in the current Kubernetes context (no Secrets are read)") + .action(async () => { + for (const result of await inspectNamespaceOwnership(execa)) console.log(result); + console.log("Namespace ownership preflight passed"); + }); + command.command("adopt") + .description("Explicit administrator adoption of an unclaimed legacy namespace; preserves all workloads and data") + .argument("", "Existing Sandbox name") + .requiredOption("--namespace ", "Namespace containing the KarsSandbox CR") + .requiredOption("--sandbox-uid ", "Reviewed live KarsSandbox UID") + .requiredOption("--namespace-uid ", "Reviewed live target namespace UID") + .action(async (name: string, options: { namespace: string; sandboxUid: string; namespaceUid: string }) => { + await adoptNamespace(execa, name, options.namespace, options.sandboxUid, options.namespaceUid); + console.log("Namespace claim recorded; the controller will verify it before reconciliation"); + }); + return command; +} diff --git a/cli/src/commands/up/fast_upgrade.ts b/cli/src/commands/up/fast_upgrade.ts index 45b47633..a65e6744 100644 --- a/cli/src/commands/up/fast_upgrade.ts +++ b/cli/src/commands/up/fast_upgrade.ts @@ -13,6 +13,7 @@ import { loadContext } from "../../config.js"; import { requireBundledAsset } from "../../lib/repo-assets.js"; import { cliReleaseTag } from "../../lib/version.js"; import { rolloutRestartAll } from "../upgrade.js"; +import { inspectNamespaceOwnership } from "../../lib/namespace-ownership.js"; export interface UpOptionsForUpgrade { upgrade?: boolean; @@ -36,6 +37,7 @@ export async function runFastUpgrade(options: UpOptionsForUpgrade): Promise { diff --git a/cli/src/lib/namespace-ownership.test.ts b/cli/src/lib/namespace-ownership.test.ts new file mode 100644 index 00000000..687231d4 --- /dev/null +++ b/cli/src/lib/namespace-ownership.test.ts @@ -0,0 +1,228 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { readFileSync } from "node:fs"; +import { describe, expect, it, vi } from "vitest"; +import type { Execute } from "./deployment-target.js"; +import { + CLAIM, adoptNamespace, inspectNamespaceOwnership, legacyNamespaceProof, + namespaceClaimed, namespacePrestaged, prepareCredentialNamespace, type OwnershipObject, +} from "./namespace-ownership.js"; + +function fixture(): { sandbox: OwnershipObject; namespace: OwnershipObject; deployment: OwnershipObject } { + return JSON.parse(readFileSync(new URL("../../../tests/compat/fixtures/namespace-legacy.json", import.meta.url), "utf8")); +} + +function cluster() { + const f = fixture(); + const state = { + sandboxes: [f.sandbox], namespace: f.namespace as OwnershipObject | undefined, + deployment: f.deployment as OwnershipObject | undefined, readError: false, patchError: false, + }; + const run = vi.fn(async (command: string, args: readonly string[], options?: { input?: string }) => { + expect(command).toBe("kubectl"); + if (args[0] === "get") { + if (state.readError) throw new Error("Forbidden (403)"); + expect(args).toContain("--show-managed-fields=true"); + const value = args[1] === "karssandboxes" ? { items: state.sandboxes } + : args[1] === "namespace" ? state.namespace : state.deployment; + return { stdout: value ? JSON.stringify(value) : "" }; + } + if (args[0] === "patch") { + if (state.patchError) throw new Error("Conflict (409)"); + expect(args.slice(0, 4)).toEqual(["patch", "namespace", "kars-demo", "--type=merge"]); + const patch = JSON.parse(args[5]); + expect(patch.metadata.uid).toBe(state.namespace?.metadata.uid); + expect(patch.metadata.resourceVersion).toBe(state.namespace?.metadata.resourceVersion); + state.namespace!.metadata.annotations = { + ...state.namespace!.metadata.annotations, ...patch.metadata.annotations, + }; + return { stdout: "" }; + } + if (args[0] === "create") { + expect(state.namespace).toBeUndefined(); + const created = JSON.parse(options!.input!) as OwnershipObject; + created.metadata.uid = "reserved-namespace"; + created.metadata.resourceVersion = "101"; + created.metadata.creationTimestamp = "2026-09-01T09:59:00Z"; + state.namespace = created; + return { stdout: JSON.stringify(created) }; + } + throw new Error(`Unexpected operation ${args.join(" ")}`); + }); + return { state, run, execute: run as unknown as Execute }; +} + +describe("namespace claim v1 and shared legacy evidence", () => { + it("accepts genuine legacy deployments even when the CLI created the namespace before the CR", () => { + const f = fixture(); + expect(legacyNamespaceProof(f.namespace, f.deployment, f.sandbox)).toBe(true); + expect(namespaceClaimed(f.namespace, f.sandbox)).toBe(false); + }); + + it("does not infer ownership from labels alone", () => { + const f = fixture(); + f.namespace.metadata.managedFields = []; + expect(legacyNamespaceProof(f.namespace, f.deployment, f.sandbox)).toBe(false); + }); + + it("rejects deployment parent mismatch and newer CR incarnation", () => { + const f = fixture(); + f.deployment.metadata.labels!["kars.azure.com/parent-namespace"] = "other"; + expect(legacyNamespaceProof(f.namespace, f.deployment, f.sandbox)).toBe(false); + delete f.deployment.metadata.labels!["kars.azure.com/parent-namespace"]; + expect(legacyNamespaceProof(f.namespace, f.deployment, f.sandbox)).toBe(true); + f.sandbox.metadata.uid = "new-incarnation"; + f.sandbox.metadata.creationTimestamp = "2026-09-01T12:00:00Z"; + expect(legacyNamespaceProof(f.namespace, f.deployment, f.sandbox)).toBe(false); + f.sandbox.metadata.creationTimestamp = f.deployment.metadata.creationTimestamp; + expect(legacyNamespaceProof(f.namespace, f.deployment, f.sandbox)).toBe(false); + }); + + it.each([ + "status", "namespace-manager", "deployment-manager", "finalizer", "deployment-timestamp", "selector", "pod-label", + ])("requires every piece of legacy evidence: %s", field => { + const f = fixture(); + if (field === "status") delete f.sandbox.status; + if (field === "namespace-manager") f.namespace.metadata.managedFields![0].manager = "kubectl"; + if (field === "deployment-manager") f.deployment.metadata.managedFields![0].manager = "kubectl"; + if (field === "finalizer") f.sandbox.metadata.finalizers = []; + if (field === "deployment-timestamp") delete f.deployment.metadata.creationTimestamp; + if (field === "selector") f.deployment.spec!.selector!.matchLabels = {}; + if (field === "pod-label") f.deployment.spec!.template!.metadata!.labels = {}; + expect(legacyNamespaceProof(f.namespace, f.deployment, f.sandbox)).toBe(false); + }); + + it.each([CLAIM.uid, CLAIM.namespace, CLAIM.name, CLAIM.version])("rejects a foreign/partial claim: %s", key => { + const f = fixture(); + f.namespace.metadata.annotations![key] = "foreign"; + expect(() => namespaceClaimed(f.namespace, f.sandbox)).toThrow("NamespaceOwnershipConflict"); + }); + + it("rejects arbitrary ownerReferences and replacement namespace UIDs", () => { + const f = fixture(); + f.namespace.metadata.ownerReferences = [{ uid: "customer-owner" }]; + expect(() => namespaceClaimed(f.namespace, f.sandbox)).toThrow("ownerReferences"); + f.namespace.metadata.ownerReferences = []; + f.sandbox.metadata.annotations = { [CLAIM.namespaceUid]: "old-namespace" }; + expect(() => namespaceClaimed(f.namespace, f.sandbox)).toThrow("replaced"); + }); +}); + +describe("read-only upgrade preflight", () => { + it("reports proven legacy adoption without mutating workloads, namespaces, or data", async () => { + const { state, execute, run } = cluster(); + const original = structuredClone(state); + await expect(inspectNamespaceOwnership(execute)).resolves.toEqual([ + "workspace-a/demo: unambiguous legacy deployment (metadata-only adoption)", + ]); + expect(state).toEqual(original); + expect(run.mock.calls.every(([, args]) => args[0] === "get")).toBe(true); + expect(run.mock.calls.some(([, args]) => args.includes("secret"))).toBe(false); + }); + + it("blocks ambiguous same-name Sandboxes in different workspaces", async () => { + const { state, execute, run } = cluster(); + const other = structuredClone(state.sandboxes[0]); + other.metadata.namespace = "workspace-b"; + other.metadata.uid = "sandbox-b"; + state.sandboxes.push(other); + await expect(inspectNamespaceOwnership(execute)).rejects.toThrow("multiple possible owners"); + expect(run.mock.calls.every(([, args]) => args[0] === "get")).toBe(true); + }); + + it("rejects namespaces without workloads or valid evidence and surfaces read errors", async () => { + const { state, execute } = cluster(); + state.deployment = undefined; + await expect(inspectNamespaceOwnership(execute)).rejects.toThrow("explicit adoption required"); + state.readError = true; + await expect(inspectNamespaceOwnership(execute)).rejects.toThrow("403"); + }); + + it("recognizes empty clusters and new namespaces but not missing bound namespaces", async () => { + const { state, execute } = cluster(); + state.namespace = undefined; + await expect(inspectNamespaceOwnership(execute)).resolves.toEqual([ + "workspace-a/demo: new namespace (atomic create)", + ]); + state.sandboxes[0].metadata.annotations = { [CLAIM.namespaceUid]: "old" }; + await expect(inspectNamespaceOwnership(execute)).rejects.toThrow("missing"); + state.sandboxes = []; + await expect(inspectNamespaceOwnership(execute)).resolves.toEqual([]); + }); +}); + +describe("explicit administrator adoption", () => { + it("uses reviewed UIDs and CAS, changing only claim annotations", async () => { + const { state, execute } = cluster(); + const original = structuredClone(state); + await adoptNamespace(execute, "demo", "workspace-a", "sandbox-a", "namespace-a"); + expect(namespaceClaimed(state.namespace!, state.sandboxes[0])).toBe(true); + expect(state.namespace!.metadata.labels).toEqual(original.namespace!.metadata.labels); + expect(state.namespace!.metadata.annotations!["customer-annotation"]).toBe("keep"); + expect(state.deployment).toEqual(original.deployment); + expect(state.sandboxes).toEqual(original.sandboxes); + }); + + it("rejects stale reviewed UIDs, foreign claims, and namespace write conflicts", async () => { + const { state, execute } = cluster(); + await expect(adoptNamespace(execute, "demo", "workspace-a", "stale", "namespace-a")).rejects.toThrow("UID"); + await expect(adoptNamespace(execute, "demo", "workspace-a", "sandbox-a", "stale")).rejects.toThrow("UID"); + state.namespace!.metadata.annotations![CLAIM.uid] = "foreign"; + await expect(adoptNamespace(execute, "demo", "workspace-a", "sandbox-a", "namespace-a")).rejects.toThrow("different"); + delete state.namespace!.metadata.annotations![CLAIM.uid]; + state.patchError = true; + await expect(adoptNamespace(execute, "demo", "workspace-a", "sandbox-a", "namespace-a")).rejects.toThrow("409"); + }); +}); + +describe("add credential namespace prestaging", () => { + it("rejects path-shaped inputs before querying or creating namespaces", async () => { + const { execute, run } = cluster(); + await expect(prepareCredentialNamespace(execute, "../other", "workspace-a")).rejects.toThrow("DNS label"); + await expect(adoptNamespace(execute, "demo", "../other", "sandbox-a", "namespace-a")).rejects.toThrow("DNS label"); + expect(run).not.toHaveBeenCalled(); + }); + + it("creates a reservation before the CR and requires its UID backlink for binding", async () => { + const { state, execute, run } = cluster(); + state.sandboxes = []; + state.namespace = undefined; + expect(await prepareCredentialNamespace(execute, "demo", "workspace-a")).toBe("reserved-namespace"); + const sandbox = fixture().sandbox; + expect(namespacePrestaged(state.namespace!, sandbox)).toBe(false); + sandbox.metadata.annotations = { [CLAIM.namespaceUid]: "reserved-namespace" }; + expect(namespacePrestaged(state.namespace!, sandbox)).toBe(true); + expect(await prepareCredentialNamespace(execute, "demo", "workspace-a")).toBe("reserved-namespace"); + state.namespace!.metadata.annotations![CLAIM.uid] = "previous-owner"; + expect(namespacePrestaged(state.namespace!, sandbox)).toBe(false); + expect(run.mock.calls.filter(([, args]) => args[0] === "create")).toHaveLength(1); + }); + + it("retains the existing legacy credential namespace when its ownership is proven", async () => { + const { state, execute, run } = cluster(); + const original = structuredClone(state); + expect(await prepareCredentialNamespace(execute, "demo", "workspace-a")).toBe("namespace-a"); + expect(state).toEqual(original); + expect(run.mock.calls.every(([, args]) => args[0] === "get")).toBe(true); + }); + + it("never claims an arbitrary or foreign-workspace namespace", async () => { + const { state, execute } = cluster(); + await expect(prepareCredentialNamespace(execute, "demo", "workspace-b")).rejects.toThrow("another workspace"); + state.sandboxes = []; + await expect(prepareCredentialNamespace(execute, "demo", "workspace-a")).rejects.toThrow("explicit adoption"); + }); + + it("does not swallow a create409 race or namespace read failure", async () => { + const { state, execute, run } = cluster(); + state.sandboxes = []; + state.namespace = undefined; + run.mockImplementation(async (_command, args) => { + if (args[0] === "get") return { stdout: args[1] === "karssandboxes" ? '{"items":[]}' : "" }; + throw new Error("AlreadyExists (409)"); + }); + await expect(prepareCredentialNamespace(execute, "demo", "workspace-a")).rejects.toThrow("409"); + expect(run.mock.calls.some(([, args]) => args[0] === "patch" || args[0] === "delete")).toBe(false); + }); +}); diff --git a/cli/src/lib/namespace-ownership.ts b/cli/src/lib/namespace-ownership.ts new file mode 100644 index 00000000..77a2c947 --- /dev/null +++ b/cli/src/lib/namespace-ownership.ts @@ -0,0 +1,275 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { Execute } from "./deployment-target.js"; + +export const CLAIM = { + version: "kars.azure.com/namespace-claim-version", + namespace: "kars.azure.com/sandbox-namespace", + name: "kars.azure.com/sandbox-name", + uid: "kars.azure.com/sandbox-uid", + namespaceUid: "kars.azure.com/namespace-uid", + prestage: "kars.azure.com/namespace-prestage", +} as const; +const FINALIZER = "kars.azure.com/namespace-cleanup"; +const MANAGER = "kars-controller/karssandbox"; +const GUIDE = "docs/how-to/namespace-ownership.md"; + +export interface OwnershipObject { + apiVersion?: string; + kind?: string; + metadata: { + name?: string; namespace?: string; uid?: string; resourceVersion?: string; + creationTimestamp?: string; deletionTimestamp?: string; + annotations?: Record; labels?: Record; + ownerReferences?: unknown[]; finalizers?: string[]; + managedFields?: Array<{ + manager?: string; operation?: string; subresource?: string; + fieldsV1?: Record; + }>; + }; + status?: { namespace?: string }; + spec?: { + selector?: { matchLabels?: Record }; + template?: { metadata?: { labels?: Record } }; + }; +} + +function fail(message: string): never { + throw new Error(`NamespaceOwnershipConflict: ${message}; see ${GUIDE}`); +} + +function identity(object: OwnershipObject): void { + if (!object.metadata?.uid || !object.metadata.resourceVersion || !object.metadata.name) { + fail("API object omitted name/UID/resourceVersion"); + } +} + +function validateTarget(name: string, namespace: string): void { + for (const value of [name, namespace]) { + if (!value || value.length > 63 || !/^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/.test(value)) { + fail("target name/workspace is not a Kubernetes DNS label"); + } + } +} + +export function claimAnnotations(sandbox: OwnershipObject): Record { + identity(sandbox); + if (!sandbox.metadata.namespace || !sandbox.metadata.name) fail("Sandbox identity is incomplete"); + return { + [CLAIM.version]: "v1", [CLAIM.namespace]: sandbox.metadata.namespace, + [CLAIM.name]: sandbox.metadata.name, [CLAIM.uid]: sandbox.metadata.uid!, + }; +} + +/** Strict checks shared by diagnostics, explicit adoption, and credential prestaging. */ +export function namespaceClaimed(namespace: OwnershipObject, sandbox: OwnershipObject): boolean { + identity(namespace); + identity(sandbox); + if (namespace.metadata.name !== `kars-${sandbox.metadata.name}` + || namespace.metadata.ownerReferences?.length) fail("namespace name or ownerReferences do not match"); + const bound = sandbox.metadata.annotations?.[CLAIM.namespaceUid]; + if (bound !== undefined && bound !== namespace.metadata.uid) fail("namespace was replaced"); + const annotations = namespace.metadata.annotations ?? {}; + const keys = [CLAIM.version, CLAIM.namespace, CLAIM.name, CLAIM.uid, CLAIM.prestage]; + if (!keys.some(key => annotations[key] !== undefined)) return false; + for (const [key, value] of Object.entries(claimAnnotations(sandbox))) { + if (annotations[key] !== value) fail("namespace is reserved for a different Sandbox UID/workspace"); + } + if (annotations[CLAIM.prestage] !== undefined) fail("namespace is not bound to this Sandbox UID"); + return true; +} + +export function namespacePrestaged(namespace: OwnershipObject, sandbox: OwnershipObject): boolean { + const annotations = namespace.metadata.annotations ?? {}; + const nsTime = Date.parse(namespace.metadata.creationTimestamp ?? ""); + const sandboxTime = Date.parse(sandbox.metadata.creationTimestamp ?? ""); + return namespace.metadata.name === `kars-${sandbox.metadata.name}` + && !namespace.metadata.ownerReferences?.length + && annotations[CLAIM.version] === "v1" + && annotations[CLAIM.namespace] === sandbox.metadata.namespace + && annotations[CLAIM.name] === sandbox.metadata.name + && annotations[CLAIM.uid] === undefined + && annotations[CLAIM.prestage] === "bind-next-sandbox" + && !!namespace.metadata.uid + && sandbox.metadata.annotations?.[CLAIM.namespaceUid] === namespace.metadata.uid + && Number.isFinite(nsTime) && Number.isFinite(sandboxTime) && nsTime <= sandboxTime; +} + +function appliedField(object: OwnershipObject, path: string[]): boolean { + return object.metadata.managedFields?.some(entry => { + if (entry.manager !== MANAGER || entry.operation !== "Apply" || entry.subresource) return false; + let value: unknown = entry.fieldsV1; + for (const key of path) { + if (!value || typeof value !== "object" || !(key in value)) return false; + value = (value as Record)[key]; + } + return true; + }) ?? false; +} + +/** Keep this conjunctive test in sync with controller namespace_ownership::legacy_proof. */ +export function legacyNamespaceProof( + namespace: OwnershipObject, deployment: OwnershipObject, sandbox: OwnershipObject, +): boolean { + const name = sandbox.metadata.name; + const target = `kars-${name}`; + const parent = deployment.metadata.labels?.["kars.azure.com/parent-namespace"]; + const deployed = Date.parse(deployment.metadata.creationTimestamp ?? ""); + const created = Date.parse(sandbox.metadata.creationTimestamp ?? ""); + return sandbox.status?.namespace === target + && (sandbox.metadata.finalizers?.includes(FINALIZER) ?? false) + && namespace.metadata.labels?.["kars.azure.com/sandbox"] === name + && namespace.metadata.labels?.["kars.azure.com/role"] === "sandbox" + && appliedField(namespace, ["f:metadata", "f:labels", "f:kars.azure.com/sandbox"]) + && deployment.metadata.name === name && deployment.metadata.namespace === target + && !deployment.metadata.ownerReferences?.length && !deployment.metadata.deletionTimestamp + && deployment.metadata.labels?.["kars.azure.com/sandbox"] === name + && (parent === undefined || parent === sandbox.metadata.namespace) + && appliedField(deployment, ["f:spec"]) + && Number.isFinite(deployed) && Number.isFinite(created) && deployed > created + && deployment.spec?.selector?.matchLabels?.["kars.azure.com/sandbox"] === name + && deployment.spec?.template?.metadata?.labels?.["kars.azure.com/sandbox"] === name; +} + +async function get(execute: Execute, kind: string, name: string, namespace?: string): Promise { + // --ignore-not-found suppresses only 404. Auth, transport, and parsing errors + // must stop the operation; they are never interpreted as an empty cluster. + const { stdout } = await execute("kubectl", [ + "get", kind, name, ...(namespace ? ["-n", namespace] : []), + "--ignore-not-found", "--show-managed-fields=true", "-o", "json", + ], { stdio: "pipe" }); + if (!String(stdout).trim()) return undefined; + const object = JSON.parse(String(stdout)) as OwnershipObject; + identity(object); + return object; +} + +async function sandboxes(execute: Execute): Promise { + const { stdout } = await execute("kubectl", [ + "get", "karssandboxes", "-A", "--show-managed-fields=true", "-o", "json", + ], { stdio: "pipe" }); + const list = JSON.parse(String(stdout)) as { items?: OwnershipObject[] }; + if (!Array.isArray(list.items)) fail("Sandbox inventory is incomplete"); + for (const sandbox of list.items) { + identity(sandbox); + if (!sandbox.metadata.namespace) fail("Sandbox inventory omitted its workspace namespace"); + } + return list.items; +} + +export async function inspectNamespaceOwnership(execute: Execute): Promise { + const inventory = await sandboxes(execute); + const diagnostics: string[] = []; + for (const sandbox of inventory) { + const name = sandbox.metadata.name!; + const label = `${sandbox.metadata.namespace}/${name}`; + const target = await get(execute, "namespace", `kars-${name}`); + const unique = inventory.filter(s => s.metadata.name === name).length === 1; + if (!target) { + if (!unique) fail(`${label}: same-name Sandboxes exist in different workspaces`); + if (sandbox.metadata.annotations?.[CLAIM.namespaceUid] && !sandbox.metadata.deletionTimestamp) { + fail(`${label}: previously bound namespace is missing`); + } + diagnostics.push(`${label}: ${sandbox.metadata.deletionTimestamp ? "cleanup pending" : "new namespace (atomic create)"}`); + continue; + } + if (target.metadata.deletionTimestamp && !sandbox.metadata.deletionTimestamp) fail(`${label}: namespace is terminating`); + if (namespacePrestaged(target, sandbox)) { + if (!unique) fail(`${label}: prestage target is ambiguous`); + diagnostics.push(`${label}: explicit prestage (will bind UID)`); + continue; + } + if (namespaceClaimed(target, sandbox)) { + diagnostics.push(`${label}: namespace claim v1 verified`); + continue; + } + if (!unique) fail(`${label}: legacy namespace has multiple possible owners`); + const deployment = await get(execute, "deployment", name, `kars-${name}`); + if (!deployment || !legacyNamespaceProof(target, deployment, sandbox)) { + fail(`${label}: legacy ownership is unproven; resources preserved; explicit adoption required`); + } + diagnostics.push(`${label}: unambiguous legacy deployment (metadata-only adoption)`); + } + return diagnostics; +} + +/** Explicit administrator decision, never a preflight side effect or a force takeover. */ +export async function adoptNamespace( + execute: Execute, name: string, sourceNamespace: string, sandboxUid: string, namespaceUid: string, +): Promise { + validateTarget(name, sourceNamespace); + const inventory = await sandboxes(execute); + const candidates = inventory.filter(s => s.metadata.name === name); + const sandbox = candidates[0]; + if (candidates.length !== 1 || sandbox.metadata.namespace !== sourceNamespace + || sandbox.metadata.uid !== sandboxUid || sandbox.metadata.deletionTimestamp) { + fail("reviewed Sandbox UID/workspace is no longer the unique live owner"); + } + const namespace = await get(execute, "namespace", `kars-${name}`); + if (!namespace || namespace.metadata.uid !== namespaceUid || namespace.metadata.deletionTimestamp) { + fail("reviewed namespace UID is missing, replaced, or terminating"); + } + // A partial/foreign claim is not an invitation to overwrite it. + namespaceClaimed(namespace, sandbox); + await execute("kubectl", ["patch", "namespace", `kars-${name}`, "--type=merge", "-p", + JSON.stringify({ metadata: { + uid: namespaceUid, resourceVersion: namespace.metadata.resourceVersion, + annotations: claimAnnotations(sandbox), + } }), + ], { stdio: "pipe" }); +} + +/** Preserve add's pre-CR credential staging, with an explicit two-way namespace + * reservation. Returning the namespace UID lets add include it in the CR CREATE. + * Interrupted explicit reservations can be resumed, but unmarked namespaces + * are never claimed merely because their name matches. */ +export async function prepareCredentialNamespace( + execute: Execute, name: string, sourceNamespace: string, +): Promise { + validateTarget(name, sourceNamespace); + const inventory = await sandboxes(execute); + const candidates = inventory.filter(s => s.metadata.name === name); + if (candidates.some(s => s.metadata.namespace !== sourceNamespace)) fail("sandbox name belongs to another workspace"); + const sandbox = candidates[0]; + const existing = await get(execute, "namespace", `kars-${name}`); + if (existing) { + const annotations = existing.metadata.annotations ?? {}; + if (!sandbox && !existing.metadata.deletionTimestamp && !existing.metadata.ownerReferences?.length + && annotations[CLAIM.version] === "v1" + && annotations[CLAIM.namespace] === sourceNamespace && annotations[CLAIM.name] === name + && annotations[CLAIM.uid] === undefined && annotations[CLAIM.prestage] === "bind-next-sandbox") { + return existing.metadata.uid!; + } + if (!sandbox || existing.metadata.deletionTimestamp) fail("existing namespace needs explicit adoption before credential staging"); + if (!namespaceClaimed(existing, sandbox)) { + const deployment = await get(execute, "deployment", name, `kars-${name}`); + if (!deployment || !legacyNamespaceProof(existing, deployment, sandbox)) fail("credential target ownership is unproven"); + } + if (sandbox.metadata.deletionTimestamp) fail("Sandbox is terminating"); + return existing.metadata.uid!; + } + if (sandbox) fail("existing Sandbox has no namespace; wait for the controller"); + const { stdout } = await execute("kubectl", ["create", "-f", "-", "-o", "json"], { + input: JSON.stringify({ + apiVersion: "v1", kind: "Namespace", metadata: { + name: `kars-${name}`, + annotations: { + [CLAIM.version]: "v1", [CLAIM.namespace]: sourceNamespace, + [CLAIM.name]: name, [CLAIM.prestage]: "bind-next-sandbox", + }, + labels: { + "app.kubernetes.io/name": "kars", "app.kubernetes.io/component": "sandbox", + "kars.azure.com/sandbox": name, "kars.azure.com/role": "sandbox", + "kars.azure.com/isolated": "strict", + "pod-security.kubernetes.io/enforce": "privileged", + "pod-security.kubernetes.io/audit": "baseline", + "pod-security.kubernetes.io/warn": "baseline", + }, + }, + }), stdio: "pipe", + }); + const created = JSON.parse(String(stdout)) as OwnershipObject; + identity(created); + return created.metadata.uid!; +} diff --git a/cli/src/testing/namespace-ownership.test.ts b/cli/src/testing/namespace-ownership.test.ts new file mode 100644 index 00000000..fc9d9b3f --- /dev/null +++ b/cli/src/testing/namespace-ownership.test.ts @@ -0,0 +1,71 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; +import { parseAllDocuments } from "yaml"; + +const root = new URL("../../../", import.meta.url); +const source = (path: string) => readFileSync(new URL(path, root), "utf8"); + +describe("namespace ownership wiring", () => { + it("checks ownership before either upgrade path invokes Helm", () => { + const upgrade = source("cli/src/commands/upgrade.ts"); + expect(upgrade.indexOf("await inspectNamespaceOwnership(execa)")) + .toBeLessThan(upgrade.indexOf('stepper.step(`Importing ${target}')); + const fast = source("cli/src/commands/up/fast_upgrade.ts"); + expect(fast.indexOf("await inspectNamespaceOwnership(execa)")) + .toBeLessThan(fast.indexOf('const helmArgs = [')); + }); + + it("claims before controller side effects and retains namespace cleanup ordering", () => { + const reconciler = source("controller/src/reconciler/mod.rs"); + expect(reconciler.indexOf("namespace_ownership::ensure")) + .toBeLessThan(reconciler.indexOf("if sandbox.metadata.deletion_timestamp.is_some()")); + expect(reconciler.indexOf("namespace_ownership::delete")) + .toBeLessThan(reconciler.indexOf("namespace_ownership::remove_finalizer")); + expect(reconciler).not.toContain("ns_api.delete"); + expect(source("controller/src/reconciler/namespace_ownership.rs")).not.toContain(".force()"); + expect(reconciler.split("\n").length - 1).toBeLessThanOrEqual(3700); + }); + + it("registers generic diagnostics with the existing kube context safeguard", () => { + expect(source("cli/src/cli.ts")).toContain("program.addCommand(namespaceCommand())"); + expect(source("cli/src/lib/kube-bootstrap.ts")).toContain('"namespace"'); + const add = source("cli/src/commands/add.ts"); + expect(add.indexOf("await prepareCredentialNamespace")) + .toBeLessThan(add.indexOf('const secretArgs = ["create", "secret"')); + expect(add).toContain("[CLAIM.namespaceUid]: namespaceUid"); + }); + + it("guards approval cleanup and admin-token reads with workspace-aware ownership", () => { + const approvals = source("controller/src/egress_approval_reconciler.rs"); + expect(approvals.indexOf("namespace_ownership::verify_target")) + .toBeLessThan(approvals.indexOf("return finalize(&api")); + const confirmation = source("controller/src/status/router_confirmation_io.rs"); + expect(confirmation.indexOf("namespace_ownership::verify_target")) + .toBeLessThan(confirmation.indexOf('api.get_opt("router-admin-token")')); + expect(confirmation).toContain("read_admin_token(client, source_namespace, sandbox)"); + expect(confirmation).toContain("namespace_ownership::lock(sandbox)"); + expect(approvals).toContain("drop(namespace_lock)"); + expect(approvals).toContain('"uid": approval.metadata.uid'); + expect(approvals).toContain('"resourceVersion": approval.metadata.resource_version'); + }); + + it("grants the controller the namespace watch needed by the new secondary watch", () => { + const output = execFileSync("helm", [ + "template", "kars", fileURLToPath(new URL("deploy/helm/kars", root)), + "--namespace", "kars-system", "--show-only", "templates/rbac.yaml", + ], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], timeout: 30_000 }); + const docs = parseAllDocuments(output).map(document => { + if (document.errors.length) throw document.errors[0]; + return document.toJSON(); + }); + const controller = docs.find(doc => doc?.kind === "ClusterRole" && doc.metadata?.name === "kars-controller"); + expect(controller).toBeDefined(); + const rule = controller.rules.find((rule: { resources: string[] }) => rule.resources.includes("namespaces")); + expect(rule.verbs).toContain("watch"); + }); +}); diff --git a/controller/src/egress_approval_reconciler.rs b/controller/src/egress_approval_reconciler.rs index 5472e631..a9b7439e 100644 --- a/controller/src/egress_approval_reconciler.rs +++ b/controller/src/egress_approval_reconciler.rs @@ -110,6 +110,8 @@ const POLICY_KIND_WIRE: &str = "EgressApproval"; #[derive(Debug, thiserror::Error)] enum ReconcileError { + #[error(transparent)] + NamespaceOwnership(#[from] crate::reconciler::namespace_ownership::Error), #[error("Kubernetes API error: {0}")] Kube(#[from] kube::Error), #[error("JSON serialization error: {0}")] @@ -121,6 +123,7 @@ impl ReconcileError { match self { ReconcileError::Kube(_) => "kube_api", ReconcileError::SerdeJson(_) => "serde", + ReconcileError::NamespaceOwnership(_) => "namespace_ownership", } } } @@ -499,26 +502,29 @@ async fn remove_finalizer( approval: &EgressApproval, name: &str, ) -> Result<(), ReconcileError> { + if approval.metadata.uid.is_none() || approval.metadata.resource_version.is_none() { + return Err(crate::reconciler::namespace_ownership::Error::Conflict( + "approval cleanup requires UID/resourceVersion".into(), + ) + .into()); + } let remaining: Vec = approval .metadata .finalizers .as_ref() .map(|v| v.iter().filter(|f| *f != FINALIZER).cloned().collect()) .unwrap_or_default(); - // SSA-applying just the metadata.finalizers field with our - // manager so the API server treats removal correctly without - // forcing whole-object ownership. let patch = json!({ "apiVersion": "kars.azure.com/v1alpha1", "kind": "EgressApproval", - "metadata": { "finalizers": remaining }, + "metadata": { + "uid": approval.metadata.uid, + "resourceVersion": approval.metadata.resource_version, + "finalizers": remaining + }, }); - api.patch( - name, - &PatchParams::apply(FIELD_MANAGER).force(), - &Patch::Apply(patch), - ) - .await?; + api.patch(name, &PatchParams::default(), &Patch::Merge(patch)) + .await?; Ok(()) } @@ -643,6 +649,33 @@ async fn reconcile(approval: Arc, ctx: Arc) -> Result = Api::namespaced(ctx.client.clone(), &sandbox_pod_ns); let sandboxes: Api = Api::namespaced(ctx.client.clone(), &ns); + let namespace_lock = crate::reconciler::namespace_ownership::lock(&approval.spec.sandbox).await; + // Namespace GC owns the local ConfigMap once this very namespace is + // terminating. Only release this approval's UID-guarded finalizer; requiring + // its already-deleted sibling Sandbox would deadlock namespace GC. + if approval.metadata.deletion_timestamp.is_some() + && ns == sandbox_pod_ns + && Api::::all(ctx.client.clone()) + .get_opt(&ns) + .await? + .is_some_and(|namespace| namespace.metadata.deletion_timestamp.is_some()) + { + remove_finalizer(&api, &approval, &name).await?; + return Ok(Action::await_change()); + } + let target_exists = crate::reconciler::namespace_ownership::verify_target( + &ctx.client, + &ns, + &approval.spec.sandbox, + ) + .await?; + if !target_exists { + if approval.metadata.deletion_timestamp.is_some() { + remove_finalizer(&api, &approval, &name).await?; + return Ok(Action::await_change()); + } + return Ok(Action::requeue(REQUEUE_AWAITING)); + } if approval.metadata.deletion_timestamp.is_some() { return finalize(&api, &configmaps, &approval, &name).await; @@ -817,9 +850,11 @@ async fn reconcile(approval: Arc, ctx: Arc) -> Result, ctx: Arc) -> Result, ctx: Arc) -> Result Result, ctx: Arc) -> Result (Arc::new(live), namespace), + Err(error) => { + if matches!(error, namespace_ownership::Error::Conflict(_)) { + namespace_ownership::report_conflict(client, &sandbox, &error.to_string()).await?; + } + return Err(error.into()); + } + }; // Detect SRE-mode sandbox via the kars.azure.com/role=sre label. // Computed once at the top of reconcile and threaded through the @@ -219,23 +232,10 @@ async fn reconcile(sandbox: Arc, ctx: Arc) -> Result = Api::all(client.clone()); - match ns_api.delete(&sandbox_ns, &DeleteParams::default()).await { - Ok(_) => tracing::info!("Namespace {sandbox_ns} deletion initiated"), - Err(kube::Error::Api(ae)) if ae.code == 404 => { - tracing::info!("Namespace {sandbox_ns} already gone"); - } - Err(e) => { - tracing::error!(error = %e, "Failed to delete namespace {sandbox_ns}"); - return Ok(Action::requeue(Duration::from_secs(10))); - } - } + namespace_ownership::delete(client, &sandbox, owned_namespace.as_ref()).await?; // Clean up the spawner ClusterRoleBinding let crb_api: Api = Api::all(client.clone()); @@ -243,7 +243,7 @@ async fn reconcile(sandbox: Arc, ctx: Arc) -> Result tracing::info!("ClusterRoleBinding {crb_name} deleted"), Err(kube::Error::Api(ae)) if ae.code == 404 => {} - Err(e) => tracing::warn!(error = %e, "Failed to delete ClusterRoleBinding {crb_name}"), + Err(e) => return Err(e.into()), } // Clean up the Azure federated identity credential. @@ -305,41 +305,12 @@ async fn reconcile(sandbox: Arc, ctx: Arc) -> Result = - Api::namespaced(client.clone(), &sandbox.namespace().unwrap_or_default()); - let patch = json!({ - "metadata": { - "finalizers": sandbox.metadata.finalizers.as_ref() - .map(|f| f.iter().filter(|x| x.as_str() != FINALIZER).collect::>()) - .unwrap_or_default() - } - }); - let _ = sandbox_api - .patch(&name, &PatchParams::default(), &Patch::Merge(patch)) - .await; + namespace_ownership::remove_finalizer(client, &sandbox).await?; tracing::info!("KarsSandbox {name} cleanup complete"); return Ok(Action::await_change()); } - // Ensure our finalizer is present (add it if missing) - let has_finalizer = sandbox - .metadata - .finalizers - .as_ref() - .is_some_and(|f| f.iter().any(|x| x == FINALIZER)); - if !has_finalizer { - let sandbox_api: Api = - Api::namespaced(client.clone(), &sandbox.namespace().unwrap_or_default()); - let mut finalizers = sandbox.metadata.finalizers.clone().unwrap_or_default(); - finalizers.push(FINALIZER.to_string()); - let patch = json!({ "metadata": { "finalizers": finalizers } }); - sandbox_api - .patch(&name, &PatchParams::default(), &Patch::Merge(patch)) - .await?; - tracing::info!("Added finalizer to KarsSandbox {name}"); - } - let spec = sandbox.spec.clone(); let sandbox_config = spec.sandbox.unwrap_or_default(); let sandbox_self_ns = sandbox.namespace().unwrap_or_default(); @@ -689,33 +660,6 @@ async fn reconcile(sandbox: Arc, ctx: Arc) -> Result = Api::all(client.clone()); - let ns: Namespace = serde_json::from_value(json!({ - "apiVersion": "v1", - "kind": "Namespace", - "metadata": { - "name": sandbox_ns, - "labels": { - "app.kubernetes.io/name": "kars", - "app.kubernetes.io/component": "sandbox", - "kars.azure.com/sandbox": name, - "kars.azure.com/role": "sandbox", - "kars.azure.com/isolated": "strict", - "pod-security.kubernetes.io/enforce": "privileged", - "pod-security.kubernetes.io/audit": "baseline", - "pod-security.kubernetes.io/warn": "baseline" - } - } - }))?; - ns_api - .patch( - &sandbox_ns, - &PatchParams::apply(crate::field_managers::CLAWSANDBOX).force(), - &Patch::Apply(ns), - ) - .await?; - // ── Step 2: Create ServiceAccount with Workload Identity ───────────── let sa_api: Api = Api::namespaced(client.clone(), &sandbox_ns); let sa: ServiceAccount = serde_json::from_value(json!({ @@ -3233,7 +3177,7 @@ fn error_requeue_duration(error: &ReconcileError) -> Duration { let base = match error { // Transient kube API errors (throttling, connection reset, 5xx): // retry soon so we don't starve legitimate work. - ReconcileError::Kube(_) => 30, + ReconcileError::Kube(_) | ReconcileError::NamespaceOwnership(_) => 30, // Serde errors are deterministic — the same body will fail again. // Back off longer so we don't spam logs while a human fixes the // bad CR. @@ -3248,6 +3192,7 @@ fn error_policy(sandbox: Arc, error: &ReconcileError, _ctx: Arc "kube_api", ReconcileError::SerdeJson(_) => "serde", ReconcileError::Configuration(_) => "configuration", + ReconcileError::NamespaceOwnership(_) => "namespace_ownership", }; crate::metrics::record_reconcile_error("KarsSandbox", class); tracing::error!( @@ -3425,6 +3370,11 @@ pub async fn run(client: Client) -> Result<()> { }); Controller::new(sandboxes, crate::watch_config::bounded()) + .watches( + Api::::all(ctx.client.clone()), + crate::watch_config::bounded(), + namespace_ownership::to_sandbox_ref, + ) .watches( Api::::all(ctx.client.clone()), crate::watch_config::bounded(), diff --git a/controller/src/reconciler/namespace_ownership.rs b/controller/src/reconciler/namespace_ownership.rs new file mode 100644 index 00000000..9ec356ed --- /dev/null +++ b/controller/src/reconciler/namespace_ownership.rs @@ -0,0 +1,567 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Namespace claim v1. Namespace annotations are cluster-admin/controller +//! authority, not tenant-supplied labels. See docs/how-to/namespace-ownership.md. +//! Never read Secrets to establish ownership. + +use k8s_openapi::{ + api::{apps::v1::Deployment, core::v1::Namespace}, + apimachinery::pkg::apis::meta::v1::ObjectMeta, +}; +use kube::{ + Api, Client, ResourceExt, + api::{DeleteParams, ListParams, Patch, PatchParams, PostParams, Preconditions}, + runtime::reflector::ObjectRef, +}; +use serde_json::{Value, json}; +use std::sync::LazyLock; +use tokio::sync::{Mutex, MutexGuard}; + +use crate::{crd::KarsSandbox, field_managers::CLAWSANDBOX}; + +pub const FINALIZER: &str = "kars.azure.com/namespace-cleanup"; +pub const VERSION: &str = "kars.azure.com/namespace-claim-version"; +pub const SOURCE_NAMESPACE: &str = "kars.azure.com/sandbox-namespace"; +pub const SOURCE_NAME: &str = "kars.azure.com/sandbox-name"; +pub const SOURCE_UID: &str = "kars.azure.com/sandbox-uid"; +pub const NAMESPACE_UID: &str = "kars.azure.com/namespace-uid"; +pub const PRESTAGE: &str = "kars.azure.com/namespace-prestage"; + +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("Kubernetes namespace ownership API error: {0}")] + Kube(#[from] kube::Error), + #[error("NamespaceOwnershipConflict: {0}; see docs/how-to/namespace-ownership.md")] + Conflict(String), + #[error("Namespace ownership serialization error: {0}")] + Json(#[from] serde_json::Error), +} + +fn conflict(message: &str) -> Error { + Error::Conflict(message.into()) +} + +// Same-named CRs have different controller queue keys. Serialize their entire +// reconcile (including cleanup), not only the claim operation. Bounded stripes +// avoid an unbounded map of tenant-controlled names. +static LOCKS: LazyLock>> = + LazyLock::new(|| (0..64).map(|_| Mutex::new(())).collect()); + +pub async fn lock(name: &str) -> MutexGuard<'static, ()> { + let stripe = name.bytes().fold(0usize, |sum, byte| sum + byte as usize) % LOCKS.len(); + LOCKS[stripe].lock().await +} + +/// Watch routing is only a hint; ensure() always re-reads the authoritative CR +/// and namespace. Neither labels nor watch payloads authorize a write. +pub fn to_sandbox_ref(namespace: Namespace) -> Option> { + let meta = &namespace.metadata; + let name = annotation(meta, SOURCE_NAME)?; + let source_namespace = annotation(meta, SOURCE_NAMESPACE)?; + if annotation(meta, VERSION) != Some("v1") + || source_namespace.is_empty() + || name.is_empty() + || namespace.name_any() != format!("kars-{name}") + { + return None; + } + Some(ObjectRef::new(name).within(source_namespace)) +} + +fn annotation<'a>(meta: &'a ObjectMeta, key: &str) -> Option<&'a str> { + meta.annotations.as_ref()?.get(key).map(String::as_str) +} + +fn identity(meta: &ObjectMeta) -> Result<(&str, &str), Error> { + match (meta.uid.as_deref(), meta.resource_version.as_deref()) { + (Some(uid), Some(rv)) if !uid.is_empty() && !rv.is_empty() => Ok((uid, rv)), + _ => Err(conflict( + "object is missing its API-server UID/resourceVersion", + )), + } +} + +fn owner_annotations(sandbox: &KarsSandbox) -> Value { + json!({ + VERSION: "v1", + SOURCE_NAMESPACE: sandbox.namespace(), + SOURCE_NAME: sandbox.name_any(), + SOURCE_UID: sandbox.metadata.uid, + }) +} + +/// Only metadata is written when adopting; existing labels, workloads, Secrets, +/// and pod templates are never modified by this module. +fn claim_patch(namespace: &Namespace, sandbox: &KarsSandbox) -> Result { + let (uid, rv) = identity(&namespace.metadata)?; + let mut annotations = owner_annotations(sandbox); + annotations[PRESTAGE] = Value::Null; + Ok(json!({"metadata": { + "uid": uid, "resourceVersion": rv, "annotations": annotations, + }})) +} + +fn has_foreign_owner(meta: &ObjectMeta) -> bool { + meta.owner_references + .as_ref() + .is_some_and(|refs| !refs.is_empty()) +} + +/// `true` means already claimed; `false` means legacy evidence is needed. +pub fn claimed(namespace: &Namespace, sandbox: &KarsSandbox) -> Result { + let (ns_uid, _) = identity(&namespace.metadata)?; + identity(&sandbox.metadata)?; + if namespace.name_any() != format!("kars-{}", sandbox.name_any()) + || has_foreign_owner(&namespace.metadata) + { + return Err(conflict("namespace name or ownerReferences do not match")); + } + if let Some(bound_uid) = annotation(&sandbox.metadata, NAMESPACE_UID) + && bound_uid != ns_uid + { + return Err(conflict( + "namespace was replaced; recorded namespace UID differs", + )); + } + let meta = &namespace.metadata; + let has_claim = [VERSION, SOURCE_NAMESPACE, SOURCE_NAME, SOURCE_UID, PRESTAGE] + .iter() + .any(|key| annotation(meta, key).is_some()); + if !has_claim { + return Ok(false); + } + if annotation(meta, VERSION) != Some("v1") + || annotation(meta, SOURCE_NAMESPACE) != sandbox.metadata.namespace.as_deref() + || annotation(meta, SOURCE_NAME) != sandbox.metadata.name.as_deref() + { + return Err(conflict( + "namespace is reserved for a different sandbox/workspace", + )); + } + if annotation(meta, SOURCE_UID) != sandbox.metadata.uid.as_deref() + || annotation(meta, PRESTAGE).is_some() + { + return Err(conflict("namespace is not bound to this Sandbox UID")); + } + Ok(true) +} + +fn prestaged(namespace: &Namespace, sandbox: &KarsSandbox) -> bool { + let meta = &namespace.metadata; + namespace.name_any() == format!("kars-{}", sandbox.name_any()) + && annotation(meta, VERSION) == Some("v1") + && annotation(meta, SOURCE_NAMESPACE) == sandbox.metadata.namespace.as_deref() + && annotation(meta, SOURCE_NAME) == sandbox.metadata.name.as_deref() + && annotation(meta, SOURCE_UID).is_none() + && annotation(meta, PRESTAGE) == Some("bind-next-sandbox") + && annotation(&sandbox.metadata, NAMESPACE_UID) == meta.uid.as_deref() + && !has_foreign_owner(meta) + && meta + .creation_timestamp + .as_ref() + .zip(sandbox.metadata.creation_timestamp.as_ref()) + .is_some_and(|(ns_time, sandbox_time)| ns_time <= sandbox_time) +} + +fn applied_field(meta: &ObjectMeta, path: &[&str]) -> bool { + meta.managed_fields.as_ref().is_some_and(|fields| { + fields.iter().any(|entry| { + entry.manager.as_deref() == Some(CLAWSANDBOX) + && entry.operation.as_deref() == Some("Apply") + && entry.subresource.as_deref().is_none_or(str::is_empty) + && entry.fields_v1.as_ref().is_some_and(|fields| { + let mut value = &fields.0; + for segment in path { + let Some(next) = value.get(*segment) else { + return false; + }; + value = next; + } + true + }) + }) + }) +} + +/// Legacy proof is conjunctive: current CR status + controller-managed namespace +/// and Deployment + Deployment creation strictly after this CR incarnation. +/// Equal timestamps are ambiguous (Kubernetes timestamps have second precision). +/// Namespace creation may precede the CR: the legacy CLI prestages credentials. +pub fn legacy_proof(namespace: &Namespace, deployment: &Deployment, sandbox: &KarsSandbox) -> bool { + let name = sandbox.name_any(); + let ns_name = format!("kars-{name}"); + let label = |meta: &ObjectMeta, key: &str, value: &str| { + meta.labels + .as_ref() + .and_then(|labels| labels.get(key)) + .is_some_and(|v| v == value) + }; + sandbox + .status + .as_ref() + .and_then(|status| status.namespace.as_deref()) + == Some(&ns_name) + && sandbox + .metadata + .finalizers + .as_ref() + .is_some_and(|fs| fs.iter().any(|f| f == FINALIZER)) + && label(&namespace.metadata, "kars.azure.com/sandbox", &name) + && label(&namespace.metadata, "kars.azure.com/role", "sandbox") + && applied_field( + &namespace.metadata, + &["f:metadata", "f:labels", "f:kars.azure.com/sandbox"], + ) + && deployment.name_any() == name + && deployment.namespace().as_deref() == Some(&ns_name) + && !has_foreign_owner(&deployment.metadata) + && deployment.metadata.deletion_timestamp.is_none() + && label(&deployment.metadata, "kars.azure.com/sandbox", &name) + && deployment + .metadata + .labels + .as_ref() + .and_then(|labels| labels.get("kars.azure.com/parent-namespace")) + .is_none_or(|parent| Some(parent.as_str()) == sandbox.metadata.namespace.as_deref()) + && applied_field(&deployment.metadata, &["f:spec"]) + && deployment + .metadata + .creation_timestamp + .as_ref() + .zip(sandbox.metadata.creation_timestamp.as_ref()) + .is_some_and(|(deployment_time, sandbox_time)| deployment_time > sandbox_time) + && deployment.spec.as_ref().is_some_and(|spec| { + spec.selector + .match_labels + .as_ref() + .and_then(|labels| labels.get("kars.azure.com/sandbox")) + == Some(&name) + && spec + .template + .metadata + .as_ref() + .is_some_and(|meta| label(meta, "kars.azure.com/sandbox", &name)) + }) +} + +async fn unique_sandbox(client: &Client, sandbox: &KarsSandbox) -> Result<(), Error> { + let api: Api = Api::all(client.clone()); + let list = api + .list(&ListParams::default().fields(&format!("metadata.name={}", sandbox.name_any()))) + .await?; + if list.items.len() != 1 + || list.items[0].metadata.uid != sandbox.metadata.uid + || list.items[0].metadata.namespace != sandbox.metadata.namespace + { + return Err(conflict( + "sandbox name is not unique across workspaces; no namespace was claimed", + )); + } + Ok(()) +} + +async fn live_sandbox(client: &Client, sandbox: &KarsSandbox) -> Result { + identity(&sandbox.metadata)?; + let ns = sandbox + .namespace() + .filter(|ns| !ns.is_empty()) + .ok_or_else(|| conflict("Sandbox source namespace is missing"))?; + let api: Api = Api::namespaced(client.clone(), &ns); + let live = api.get(&sandbox.name_any()).await?; + if live.metadata.uid != sandbox.metadata.uid { + return Err(conflict("Sandbox was recreated during reconciliation")); + } + Ok(live) +} + +async fn patch_sandbox( + client: &Client, + sandbox: &KarsSandbox, + mut metadata: Value, +) -> Result { + let (uid, rv) = identity(&sandbox.metadata)?; + metadata["uid"] = json!(uid); + metadata["resourceVersion"] = json!(rv); + let api: Api = Api::namespaced(client.clone(), &sandbox.namespace().unwrap()); + Ok(api + .patch( + &sandbox.name_any(), + &PatchParams::default(), + &Patch::Merge(json!({"metadata": metadata})), + ) + .await?) +} + +fn new_namespace(sandbox: &KarsSandbox) -> Result { + Ok(serde_json::from_value(json!({ + "apiVersion": "v1", "kind": "Namespace", + "metadata": { + "name": format!("kars-{}", sandbox.name_any()), + "annotations": owner_annotations(sandbox), + "labels": { + "app.kubernetes.io/name": "kars", + "app.kubernetes.io/component": "sandbox", + "kars.azure.com/sandbox": sandbox.name_any(), + "kars.azure.com/role": "sandbox", + "kars.azure.com/isolated": "strict", + "pod-security.kubernetes.io/enforce": "privileged", + "pod-security.kubernetes.io/audit": "baseline", + "pod-security.kubernetes.io/warn": "baseline" + } + } + }))?) +} + +/// Establish authority before *any* target-namespace operations. HTTP conflicts +/// propagate to the controller's retry queue; every retry re-reads all evidence. +pub async fn ensure( + client: &Client, + observed: &KarsSandbox, +) -> Result<(KarsSandbox, Option), Error> { + let mut sandbox = live_sandbox(client, observed).await?; + let api: Api = Api::all(client.clone()); + let name = format!("kars-{}", sandbox.name_any()); + let mut namespace = api.get_opt(&name).await?; + if namespace.is_none() { + unique_sandbox(client, &sandbox).await?; + if sandbox.metadata.deletion_timestamp.is_some() { + return Ok((sandbox, None)); + } + if annotation(&sandbox.metadata, NAMESPACE_UID).is_some() { + return Err(conflict( + "bound namespace is missing; explicit operator recovery is required", + )); + } + } + if let Some(ns) = &namespace { + if ns.metadata.deletion_timestamp.is_some() && sandbox.metadata.deletion_timestamp.is_none() + { + return Err(conflict( + "namespace is terminating; workload reconciliation is stopped", + )); + } + if !prestaged(ns, &sandbox) && !claimed(ns, &sandbox)? { + unique_sandbox(client, &sandbox).await?; + let deployments: Api = Api::namespaced(client.clone(), &name); + let deployment = deployments.get_opt(&sandbox.name_any()).await?; + if !deployment + .as_ref() + .is_some_and(|d| legacy_proof(ns, d, &sandbox)) + { + return Err(conflict( + "legacy namespace ownership is unproven; preserve it and run upgrade preflight", + )); + } + } else if prestaged(ns, &sandbox) { + unique_sandbox(client, &sandbox).await?; + } + } + if sandbox.metadata.deletion_timestamp.is_none() + && !sandbox + .metadata + .finalizers + .as_ref() + .is_some_and(|fs| fs.iter().any(|f| f == FINALIZER)) + { + let mut finalizers = sandbox.metadata.finalizers.clone().unwrap_or_default(); + finalizers.push(FINALIZER.into()); + sandbox = patch_sandbox(client, &sandbox, json!({"finalizers": finalizers})).await?; + } + if namespace.is_none() { + match api + .create(&PostParams::default(), &new_namespace(&sandbox)?) + .await + { + Ok(created) => namespace = Some(created), + Err(kube::Error::Api(error)) if error.code == 409 => { + // A concurrent creator won. Never turn this into an apply/adopt. + let existing = api.get(&name).await?; + if !claimed(&existing, &sandbox)? { + return Err(conflict("namespace creation raced an unclaimed namespace")); + } + namespace = Some(existing); + } + Err(error) => return Err(error.into()), + } + } + let mut namespace = namespace.unwrap(); + if prestaged(&namespace, &sandbox) || !claimed(&namespace, &sandbox)? { + namespace = api + .patch( + &name, + &PatchParams::default(), + &Patch::Merge(claim_patch(&namespace, &sandbox)?), + ) + .await?; + } + // Re-read the CR before persisting the backlink. If its UID or deletion + // state changed, do not proceed into any workload operations. + let live = live_sandbox(client, &sandbox).await?; + if live.metadata.deletion_timestamp != sandbox.metadata.deletion_timestamp { + return Err(conflict("Sandbox deletion started during namespace claim")); + } + sandbox = live; + if annotation(&sandbox.metadata, NAMESPACE_UID) + .is_some_and(|uid| Some(uid) != namespace.metadata.uid.as_deref()) + { + return Err(conflict("Sandbox namespace binding changed during claim")); + } + if annotation(&sandbox.metadata, NAMESPACE_UID) != namespace.metadata.uid.as_deref() { + sandbox = patch_sandbox( + client, + &sandbox, + json!({"annotations": {NAMESPACE_UID: namespace.metadata.uid}}), + ) + .await?; + } + recheck(client, &sandbox, &namespace).await?; + Ok((sandbox, Some(namespace))) +} + +pub async fn recheck( + client: &Client, + sandbox: &KarsSandbox, + expected: &Namespace, +) -> Result { + let api: Api = Api::all(client.clone()); + let live = api.get(&expected.name_any()).await?; + if live.metadata.uid != expected.metadata.uid || !claimed(&live, sandbox)? { + return Err(conflict( + "namespace ownership changed during reconciliation", + )); + } + if live.metadata.deletion_timestamp.is_some() && sandbox.metadata.deletion_timestamp.is_none() { + return Err(conflict("namespace deletion started during reconciliation")); + } + Ok(live) +} + +/// Read-only gate for other controllers consuming sandbox-local resources. +/// A missing namespace is not authority to read or create anything there. +/// Legacy namespaces wait for the Sandbox reconciler to establish their claim. +pub async fn verify_target( + client: &Client, + source_namespace: &str, + name: &str, +) -> Result { + let valid_label = |value: &str| { + !value.is_empty() + && value.len() <= 63 + && value + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-') + && value.as_bytes()[0].is_ascii_alphanumeric() + && value.as_bytes()[value.len() - 1].is_ascii_alphanumeric() + }; + if !valid_label(source_namespace) || !valid_label(name) { + return Err(conflict( + "target name/workspace is not a Kubernetes DNS label", + )); + } + let namespaces: Api = Api::all(client.clone()); + let Some(namespace) = namespaces.get_opt(&format!("kars-{name}")).await? else { + return Ok(false); + }; + let sandboxes: Api = Api::namespaced(client.clone(), source_namespace); + let sandbox = sandboxes + .get_opt(name) + .await? + .ok_or_else(|| conflict("target namespace has no Sandbox owner in this workspace"))?; + if sandbox.name_any() != name || sandbox.namespace().as_deref() != Some(source_namespace) { + return Err(conflict( + "target Sandbox identity differs from the requested workspace/name", + )); + } + if !claimed(&namespace, &sandbox)? { + return Err(conflict( + "target namespace ownership has not been established", + )); + } + Ok(true) +} + +/// Accepted deletion is sufficient; namespace claims prevent reuse while it +/// terminates. Waiting for disappearance would deadlock a CR inside its own +/// runtime namespace, whose deletion itself waits for this CR's finalizer. +pub async fn delete( + client: &Client, + sandbox: &KarsSandbox, + namespace: Option<&Namespace>, +) -> Result<(), Error> { + let Some(namespace) = namespace else { + return Ok(()); + }; + let live = match recheck(client, sandbox, namespace).await { + Err(Error::Kube(kube::Error::Api(error))) if error.code == 404 => return Ok(()), + result => result?, + }; + let (uid, rv) = identity(&live.metadata)?; + let api: Api = Api::all(client.clone()); + if live.metadata.deletion_timestamp.is_none() { + match api + .delete( + &live.name_any(), + &DeleteParams { + preconditions: Some(Preconditions { + uid: Some(uid.into()), + resource_version: Some(rv.into()), + }), + ..Default::default() + }, + ) + .await + { + Ok(_) => {} + Err(kube::Error::Api(error)) if error.code == 404 => {} + Err(error) => return Err(error.into()), + } + } + Ok(()) +} + +pub async fn remove_finalizer(client: &Client, sandbox: &KarsSandbox) -> Result<(), Error> { + let sandbox = live_sandbox(client, sandbox).await?; + let finalizers: Vec<_> = sandbox + .metadata + .finalizers + .iter() + .flatten() + .filter(|f| f.as_str() != FINALIZER) + .cloned() + .collect(); + patch_sandbox(client, &sandbox, json!({"finalizers": finalizers})).await?; + Ok(()) +} + +pub async fn report_conflict( + client: &Client, + sandbox: &KarsSandbox, + message: &str, +) -> Result<(), Error> { + let sandbox = live_sandbox(client, sandbox).await?; + let (uid, rv) = identity(&sandbox.metadata)?; + let mut patch = + crate::status::build_degraded_status_patch(&sandbox, "NamespaceOwnershipConflict", message); + let current_status = serde_json::to_value(&sandbox.status)?; + if patch["status"].as_object().is_some_and(|fields| { + fields + .iter() + .all(|(key, value)| current_status.get(key) == Some(value)) + }) { + return Ok(()); + } + patch["metadata"] = json!({"uid": uid, "resourceVersion": rv}); + let api: Api = Api::namespaced(client.clone(), &sandbox.namespace().unwrap()); + api.patch_status( + &sandbox.name_any(), + &PatchParams::default(), + &Patch::Merge(patch), + ) + .await?; + Ok(()) +} + +#[cfg(test)] +#[path = "namespace_ownership_tests.rs"] +mod tests; diff --git a/controller/src/reconciler/namespace_ownership_tests.rs b/controller/src/reconciler/namespace_ownership_tests.rs new file mode 100644 index 00000000..49a5b849 --- /dev/null +++ b/controller/src/reconciler/namespace_ownership_tests.rs @@ -0,0 +1,777 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use super::*; +use std::sync::{Arc, Mutex as StdMutex}; +use wiremock::{Mock, MockServer, Request, Respond, ResponseTemplate}; + +const NS_PATH: &str = "/api/v1/namespaces/kars-demo"; +const CRS_PATH: &str = "/apis/kars.azure.com/v1alpha1/karssandboxes"; +const CR_PATH: &str = "/apis/kars.azure.com/v1alpha1/namespaces/workspace-a/karssandboxes/demo"; + +fn fixture() -> Value { + serde_json::from_str(include_str!( + "../../../tests/compat/fixtures/namespace-legacy.json" + )) + .unwrap() +} + +fn sandbox(value: &Value) -> KarsSandbox { + serde_json::from_value(value.clone()).unwrap() +} + +fn namespace(value: &Value) -> Namespace { + serde_json::from_value(value.clone()).unwrap() +} + +#[test] +fn legacy_evidence_accepts_cli_prestaged_namespace_without_changing_data() { + let f = fixture(); + assert!(legacy_proof( + &namespace(&f["namespace"]), + &serde_json::from_value(f["deployment"].clone()).unwrap(), + &sandbox(&f["sandbox"]) + )); +} + +#[test] +fn legacy_evidence_rejects_labels_alone_foreign_parents_and_recreated_uids() { + for pointer in [ + "/namespace/metadata/managedFields", + "/deployment/metadata/managedFields", + "/sandbox/status/namespace", + "/sandbox/metadata/finalizers", + "/deployment/spec/template/metadata/labels", + ] { + let mut f = fixture(); + *f.pointer_mut(pointer).unwrap() = Value::Null; + assert!( + !legacy_proof( + &namespace(&f["namespace"]), + &serde_json::from_value(f["deployment"].clone()).unwrap(), + &sandbox(&f["sandbox"]) + ), + "{pointer}" + ); + } + let mut f = fixture(); + f["deployment"]["metadata"]["labels"]["kars.azure.com/parent-namespace"] = json!("workspace-b"); + assert!(!legacy_proof( + &namespace(&f["namespace"]), + &serde_json::from_value(f["deployment"].clone()).unwrap(), + &sandbox(&f["sandbox"]) + )); + let mut f = fixture(); + f["sandbox"]["metadata"]["uid"] = json!("recreated"); + f["sandbox"]["metadata"]["creationTimestamp"] = json!("2026-09-01T11:00:00Z"); + assert!(!legacy_proof( + &namespace(&f["namespace"]), + &serde_json::from_value(f["deployment"].clone()).unwrap(), + &sandbox(&f["sandbox"]) + )); +} + +#[test] +fn claims_reject_foreign_workspace_uid_ownerrefs_and_namespace_replacement() { + let f = fixture(); + let sb = sandbox(&f["sandbox"]); + let mut ns = namespace(&f["namespace"]); + ns.metadata.annotations = Some(serde_json::from_value(owner_annotations(&sb)).unwrap()); + assert!(claimed(&ns, &sb).unwrap()); + for key in [SOURCE_NAMESPACE, SOURCE_NAME, SOURCE_UID, VERSION] { + let mut foreign = ns.clone(); + foreign + .metadata + .annotations + .as_mut() + .unwrap() + .insert(key.into(), "foreign".into()); + assert!(claimed(&foreign, &sb).is_err(), "{key}"); + } + let mut other = sb.clone(); + other + .metadata + .annotations + .get_or_insert_default() + .insert(NAMESPACE_UID.into(), "replaced".into()); + assert!(claimed(&ns, &other).is_err()); + let mut value = serde_json::to_value(&ns).unwrap(); + value["metadata"]["ownerReferences"] = json!([{ + "apiVersion": "v1", "kind": "Namespace", "name": "foreign", "uid": "foreign", + }]); + assert!(claimed(&namespace(&value), &sb).is_err()); +} + +#[test] +fn prestage_requires_explicit_two_way_intent_and_binds_only_once() { + let f = fixture(); + let mut sb = sandbox(&f["sandbox"]); + let mut ns = namespace(&f["namespace"]); + let annotations = ns.metadata.annotations.get_or_insert_default(); + annotations.insert(VERSION.into(), "v1".into()); + annotations.insert(SOURCE_NAMESPACE.into(), "workspace-a".into()); + annotations.insert(SOURCE_NAME.into(), "demo".into()); + annotations.insert(PRESTAGE.into(), "bind-next-sandbox".into()); + assert!(!prestaged(&ns, &sb)); + sb.metadata + .annotations + .get_or_insert_default() + .insert(NAMESPACE_UID.into(), "namespace-a".into()); + assert!(prestaged(&ns, &sb)); + ns.metadata + .annotations + .as_mut() + .unwrap() + .insert(SOURCE_UID.into(), "previous-incarnation".into()); + assert!(!prestaged(&ns, &sb)); + assert!(claimed(&ns, &sb).is_err()); +} + +#[test] +fn same_second_legacy_creation_is_ambiguous() { + let mut f = fixture(); + f["sandbox"]["metadata"]["creationTimestamp"] = + f["deployment"]["metadata"]["creationTimestamp"].clone(); + assert!(!legacy_proof( + &namespace(&f["namespace"]), + &serde_json::from_value(f["deployment"].clone()).unwrap(), + &sandbox(&f["sandbox"]), + )); +} + +#[test] +fn namespace_watch_maps_only_the_exact_reserved_target_and_source() { + let f = fixture(); + let mut ns = namespace(&f["namespace"]); + assert!(to_sandbox_ref(ns.clone()).is_none()); + ns.metadata.annotations = + Some(serde_json::from_value(owner_annotations(&sandbox(&f["sandbox"]))).unwrap()); + let reference = to_sandbox_ref(ns.clone()).unwrap(); + assert_eq!(reference.name, "demo"); + assert_eq!(reference.namespace.as_deref(), Some("workspace-a")); + ns.metadata.name = Some("unrelated".into()); + assert!(to_sandbox_ref(ns).is_none()); +} + +#[derive(Clone)] +struct Store { + sandboxes: Vec, + namespace: Option, + deployment: Option, + version: u64, + race_create: Option, + replace_on_patch: bool, + fail_namespace_get: u16, + fail_delete: u16, + fail_cr_patch_once: bool, +} + +impl Default for Store { + fn default() -> Self { + let f = fixture(); + Self { + sandboxes: vec![f["sandbox"].clone()], + namespace: Some(f["namespace"].clone()), + deployment: Some(f["deployment"].clone()), + version: 100, + race_create: None, + replace_on_patch: false, + fail_namespace_get: 0, + fail_delete: 0, + fail_cr_patch_once: false, + } + } +} + +fn response(code: u16, value: Value) -> ResponseTemplate { + ResponseTemplate::new(code).set_body_json(value) +} + +fn failure(code: u16) -> ResponseTemplate { + response( + code, + json!({ + "apiVersion": "v1", "kind": "Status", "status": "Failure", + "reason": if code == 404 { "NotFound" } else { "Conflict" }, "code": code, + }), + ) +} + +fn merge(target: &mut Value, patch: Value) { + if let Value::Object(patch) = patch { + if !target.is_object() { + *target = json!({}); + } + for (key, value) in patch { + if value.is_null() { + target.as_object_mut().unwrap().remove(&key); + } else { + merge( + target + .as_object_mut() + .unwrap() + .entry(key) + .or_insert(Value::Null), + value, + ); + } + } + } else { + *target = patch; + } +} + +#[derive(Clone)] +struct Server(Arc>); + +impl Respond for Server { + fn respond(&self, request: &Request) -> ResponseTemplate { + let mut store = self.0.lock().unwrap(); + let path = request.url.path(); + let method = request.method.as_str(); + if method == "GET" { + if path == CRS_PATH { + assert!(request.url.query().unwrap().contains("fieldSelector")); + return response( + 200, + json!({ + "apiVersion": "kars.azure.com/v1alpha1", "kind": "KarsSandboxList", + "metadata": {}, "items": store.sandboxes, + }), + ); + } + if path.starts_with("/apis/kars.azure.com/v1alpha1/namespaces/") + && path.ends_with("/karssandboxes/demo") + { + return store + .sandboxes + .iter() + .find(|s| s["metadata"]["namespace"].as_str() == path.split('/').nth(5)) + .cloned() + .map_or_else(|| failure(404), |s| response(200, s)); + } + if path == NS_PATH { + if store.fail_namespace_get != 0 { + return failure(store.fail_namespace_get); + } + return store + .namespace + .clone() + .map_or_else(|| failure(404), |ns| response(200, ns)); + } + if path == "/apis/apps/v1/namespaces/kars-demo/deployments/demo" { + return store + .deployment + .clone() + .map_or_else(|| failure(404), |d| response(200, d)); + } + } + if method == "POST" && path == "/api/v1/namespaces" { + if let Some(winner) = store.race_create.take() { + store.namespace = Some(winner); + return failure(409); + } + if store.namespace.is_some() { + return failure(409); + } + let mut body: Value = serde_json::from_slice(&request.body).unwrap(); + body["metadata"]["uid"] = json!("namespace-new"); + store.version += 1; + body["metadata"]["resourceVersion"] = json!(store.version.to_string()); + body["metadata"]["creationTimestamp"] = json!("2026-09-01T12:00:00Z"); + store.namespace = Some(body.clone()); + return response(201, body); + } + if method == "PATCH" + && (path == NS_PATH || path == CR_PATH || path == format!("{CR_PATH}/status")) + { + assert_eq!( + request.headers.get("content-type").unwrap(), + "application/merge-patch+json" + ); + let body: Value = serde_json::from_slice(&request.body).unwrap(); + if path == NS_PATH && store.replace_on_patch { + store.replace_on_patch = false; + let ns = store.namespace.as_mut().unwrap(); + ns["metadata"]["uid"] = json!("replacement"); + ns["metadata"]["resourceVersion"] = json!("999"); + } + if path == CR_PATH && store.fail_cr_patch_once { + store.fail_cr_patch_once = false; + return failure(409); + } + store.version += 1; + let rv = store.version.to_string(); + let existing = if path == NS_PATH { + store.namespace.as_mut().unwrap() + } else { + &mut store.sandboxes[0] + }; + if body["metadata"]["uid"] != existing["metadata"]["uid"] + || body["metadata"]["resourceVersion"] != existing["metadata"]["resourceVersion"] + { + return failure(409); + } + merge(existing, body); + existing["metadata"]["resourceVersion"] = json!(rv); + return response(200, existing.clone()); + } + if method == "DELETE" && path == NS_PATH { + if store.fail_delete != 0 { + return failure(store.fail_delete); + } + let body: Value = serde_json::from_slice(&request.body).unwrap(); + let Some(existing) = store.namespace.as_ref() else { + return failure(404); + }; + if body["preconditions"]["uid"] != existing["metadata"]["uid"] + || body["preconditions"]["resourceVersion"] + != existing["metadata"]["resourceVersion"] + { + return failure(409); + } + return response(200, store.namespace.take().unwrap()); + } + panic!("unexpected request (must not read/write Secrets or workloads): {method} {path}"); + } +} + +async fn setup(store: Store) -> (MockServer, Client, Arc>) { + let server = MockServer::start().await; + let store = Arc::new(StdMutex::new(store)); + Mock::given(wiremock::matchers::any()) + .respond_with(Server(store.clone())) + .mount(&server) + .await; + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + let client = Client::try_from(kube::Config::new(server.uri().parse().unwrap())).unwrap(); + (server, client, store) +} + +#[tokio::test] +async fn adopts_unambiguous_legacy_metadata_only_and_is_idempotent() { + let initial = Store::default(); + let original_namespace = initial.namespace.clone().unwrap(); + let original_deployment = initial.deployment.clone(); + let sb = sandbox(&initial.sandboxes[0]); + let (server, client, store) = setup(initial).await; + let (bound, ns) = ensure(&client, &sb).await.unwrap(); + assert!(claimed(&ns.unwrap(), &bound).unwrap()); + let after_first = server.received_requests().await.unwrap().len(); + ensure(&client, &bound).await.unwrap(); + assert!( + server.received_requests().await.unwrap()[after_first..] + .iter() + .all(|r| r.method == "GET") + ); + let state = store.lock().unwrap(); + assert_eq!(state.deployment, original_deployment); + let ns = state.namespace.as_ref().unwrap(); + assert_eq!( + ns["metadata"]["labels"], + original_namespace["metadata"]["labels"] + ); + assert_eq!(ns["spec"], original_namespace["spec"]); + assert_eq!(ns["metadata"]["annotations"]["customer-annotation"], "keep"); +} + +#[tokio::test] +async fn rejects_arbitrary_namespace_without_any_writes() { + let mut initial = Store::default(); + initial.namespace.as_mut().unwrap()["metadata"]["managedFields"] = Value::Null; + let sb = sandbox(&initial.sandboxes[0]); + let (server, client, _) = setup(initial).await; + assert!(ensure(&client, &sb).await.is_err()); + assert!( + server + .received_requests() + .await + .unwrap() + .iter() + .all(|r| r.method == "GET") + ); +} + +#[tokio::test] +async fn duplicate_name_in_another_workspace_cannot_adopt_or_create() { + for existing_namespace in [true, false] { + let mut initial = Store::default(); + let mut other = initial.sandboxes[0].clone(); + other["metadata"]["uid"] = json!("sandbox-b"); + other["metadata"]["namespace"] = json!("workspace-b"); + initial.sandboxes.push(other); + if !existing_namespace { + initial.namespace = None; + } + let sb = sandbox(&initial.sandboxes[0]); + let (server, client, _) = setup(initial).await; + assert!(ensure(&client, &sb).await.is_err()); + assert!( + server + .received_requests() + .await + .unwrap() + .iter() + .all(|r| r.method == "GET") + ); + } +} + +#[tokio::test] +async fn new_namespace_creation_is_atomic_and_new_uid_never_adopts_old_claim() { + let mut initial = Store { + namespace: None, + ..Store::default() + }; + initial.sandboxes[0]["status"] = Value::Null; + initial.sandboxes[0]["metadata"]["finalizers"] = Value::Null; + let sb = sandbox(&initial.sandboxes[0]); + let (server, client, store) = setup(initial).await; + let (bound, ns) = ensure(&client, &sb).await.unwrap(); + assert_eq!(ns.unwrap().metadata.uid.as_deref(), Some("namespace-new")); + assert_eq!( + annotation(&bound.metadata, NAMESPACE_UID), + Some("namespace-new") + ); + let requests = server.received_requests().await.unwrap(); + assert!( + requests + .iter() + .position(|r| r.method == "PATCH" && r.url.path() == CR_PATH) + .unwrap() + < requests.iter().position(|r| r.method == "POST").unwrap() + ); + let recreated = { + let mut store = store.lock().unwrap(); + store.sandboxes[0]["metadata"]["uid"] = json!("recreated"); + sandbox(&store.sandboxes[0]) + }; + assert!(ensure(&client, &recreated).await.is_err()); +} + +#[tokio::test] +async fn creation_409_never_force_adopts_a_winner() { + let f = fixture(); + let initial = Store { + namespace: None, + race_create: Some(f["namespace"].clone()), + ..Store::default() + }; + let sb = sandbox(&initial.sandboxes[0]); + let (server, client, store) = setup(initial).await; + assert!(ensure(&client, &sb).await.is_err()); + assert_eq!( + store.lock().unwrap().namespace.as_ref().unwrap(), + &f["namespace"] + ); + assert!( + !server + .received_requests() + .await + .unwrap() + .iter() + .any(|r| r.method == "PATCH" && r.url.path() == NS_PATH) + ); +} + +#[tokio::test] +async fn cas_retry_completes_a_partially_persisted_claim_without_overwrites() { + let initial = Store { + fail_cr_patch_once: true, + ..Store::default() + }; + let sb = sandbox(&initial.sandboxes[0]); + let (_server, client, store) = setup(initial).await; + assert!(ensure(&client, &sb).await.is_err()); + assert_eq!( + store.lock().unwrap().namespace.as_ref().unwrap()["metadata"]["annotations"][SOURCE_UID], + "sandbox-a" + ); + let (bound, ns) = ensure(&client, &sb).await.unwrap(); + assert!(claimed(&ns.unwrap(), &bound).unwrap()); +} + +#[tokio::test] +async fn namespace_replacement_between_read_and_claim_fails_cas() { + let initial = Store { + replace_on_patch: true, + ..Store::default() + }; + let sb = sandbox(&initial.sandboxes[0]); + let (_server, client, store) = setup(initial).await; + assert!(ensure(&client, &sb).await.is_err()); + assert!( + store.lock().unwrap().namespace.as_ref().unwrap()["metadata"]["annotations"] + .get(SOURCE_UID) + .is_none() + ); +} + +#[tokio::test] +async fn cleanup_accepts_deletion_and_non404_errors_preserve_the_finalizer() { + for error in [0, 403, 409, 500] { + let mut initial = Store { + fail_delete: error, + ..Store::default() + }; + initial.sandboxes[0]["metadata"]["deletionTimestamp"] = json!("2026-09-01T12:00:00Z"); + let sb = sandbox(&initial.sandboxes[0]); + let (_server, client, store) = setup(initial).await; + let (bound, ns) = ensure(&client, &sb).await.unwrap(); + let result = delete(&client, &bound, ns.as_ref()).await; + if error == 0 { + result.unwrap(); + let (live, gone) = ensure(&client, &bound).await.unwrap(); + delete(&client, &live, gone.as_ref()).await.unwrap(); + } else { + assert!(result.is_err()); + } + assert_eq!( + store.lock().unwrap().sandboxes[0]["metadata"]["finalizers"], + json!([FINALIZER]) + ); + } +} + +#[tokio::test] +async fn read_errors_do_not_become_absence_or_trigger_creation() { + for error in [403, 500] { + let initial = Store { + fail_namespace_get: error, + ..Store::default() + }; + let sb = sandbox(&initial.sandboxes[0]); + let (server, client, _) = setup(initial).await; + assert!(ensure(&client, &sb).await.is_err()); + assert!( + server + .received_requests() + .await + .unwrap() + .iter() + .all(|r| r.method == "GET") + ); + } +} + +#[tokio::test] +async fn full_prestage_flow_binds_uid_atomically_and_preserves_existing_data() { + let mut initial = Store::default(); + initial.deployment = None; + initial.sandboxes[0]["status"] = Value::Null; + initial.sandboxes[0]["metadata"]["annotations"] = json!({NAMESPACE_UID: "namespace-a"}); + let annotations = &mut initial.namespace.as_mut().unwrap()["metadata"]["annotations"]; + annotations[VERSION] = json!("v1"); + annotations[SOURCE_NAME] = json!("demo"); + annotations[SOURCE_NAMESPACE] = json!("workspace-a"); + annotations[PRESTAGE] = json!("bind-next-sandbox"); + let sb = sandbox(&initial.sandboxes[0]); + let (_server, client, store) = setup(initial).await; + let (live, ns) = ensure(&client, &sb).await.unwrap(); + assert!(claimed(&ns.unwrap(), &live).unwrap()); + let state = store.lock().unwrap(); + let annotations = &state.namespace.as_ref().unwrap()["metadata"]["annotations"]; + assert!(annotations.get(PRESTAGE).is_none()); + assert_eq!(annotations[SOURCE_UID], "sandbox-a"); + assert_eq!(annotations["customer-annotation"], "keep"); +} + +#[tokio::test] +async fn a_conflicting_new_cr_does_not_disable_the_already_claimed_owner() { + let mut initial = Store::default(); + let sb = sandbox(&initial.sandboxes[0]); + merge( + initial.namespace.as_mut().unwrap(), + json!({"metadata": {"annotations": owner_annotations(&sb)}}), + ); + let mut other = initial.sandboxes[0].clone(); + other["metadata"]["uid"] = json!("sandbox-b"); + other["metadata"]["namespace"] = json!("workspace-b"); + initial.sandboxes.push(other); + let (_server, client, _) = setup(initial).await; + assert!(ensure(&client, &sb).await.is_ok()); +} + +#[tokio::test] +async fn an_existing_foreign_claim_is_rejected_before_mutation() { + let mut initial = Store::default(); + let sb = sandbox(&initial.sandboxes[0]); + let mut annotations = owner_annotations(&sb); + annotations[SOURCE_NAMESPACE] = json!("workspace-b"); + annotations[SOURCE_UID] = json!("sandbox-b"); + merge( + initial.namespace.as_mut().unwrap(), + json!({"metadata": {"annotations": annotations}}), + ); + let (server, client, _) = setup(initial).await; + assert!(ensure(&client, &sb).await.is_err()); + assert!( + server + .received_requests() + .await + .unwrap() + .iter() + .all(|r| r.method == "GET") + ); +} + +#[tokio::test] +async fn namespace_replacement_before_cleanup_is_never_deleted() { + let mut initial = Store::default(); + initial.sandboxes[0]["metadata"]["deletionTimestamp"] = json!("2026-09-01T12:00:00Z"); + let sb = sandbox(&initial.sandboxes[0]); + let (server, client, store) = setup(initial).await; + let (bound, ns) = ensure(&client, &sb).await.unwrap(); + store.lock().unwrap().namespace.as_mut().unwrap()["metadata"]["uid"] = json!("replacement"); + assert!(delete(&client, &bound, ns.as_ref()).await.is_err()); + assert!( + !server + .received_requests() + .await + .unwrap() + .iter() + .any(|r| r.method == "DELETE") + ); +} + +#[tokio::test] +async fn stale_cr_uid_and_missing_bound_namespace_are_rejected() { + let mut initial = Store::default(); + let old = sandbox(&initial.sandboxes[0]); + initial.sandboxes[0]["metadata"]["uid"] = json!("new-incarnation"); + let (server, client, store) = setup(initial).await; + assert!(ensure(&client, &old).await.is_err()); + let current = { + let mut state = store.lock().unwrap(); + state.namespace = None; + state.sandboxes[0]["metadata"]["annotations"] = json!({NAMESPACE_UID: "missing"}); + sandbox(&state.sandboxes[0]) + }; + assert!(ensure(&client, ¤t).await.is_err()); + assert!( + server + .received_requests() + .await + .unwrap() + .iter() + .all(|r| r.method == "GET") + ); +} + +#[tokio::test] +async fn finalizer_removal_retries_cas_and_preserves_unrelated_finalizers() { + let mut initial = Store { + namespace: None, + ..Store::default() + }; + initial.sandboxes[0]["metadata"]["deletionTimestamp"] = json!("2026-09-01T12:00:00Z"); + initial.sandboxes[0]["metadata"]["finalizers"] = json!([FINALIZER, "customer.example/cleanup"]); + let sb = sandbox(&initial.sandboxes[0]); + let (_server, client, store) = setup(initial).await; + store.lock().unwrap().fail_cr_patch_once = true; + assert!(remove_finalizer(&client, &sb).await.is_err()); + assert_eq!( + store.lock().unwrap().sandboxes[0]["metadata"]["finalizers"], + json!([FINALIZER, "customer.example/cleanup"]) + ); + remove_finalizer(&client, &sb).await.unwrap(); + assert_eq!( + store.lock().unwrap().sandboxes[0]["metadata"]["finalizers"], + json!(["customer.example/cleanup"]) + ); +} + +#[tokio::test] +async fn conflicts_are_visible_without_a_status_hot_loop_or_target_mutations() { + let initial = Store::default(); + let original_namespace = initial.namespace.clone(); + let sb = sandbox(&initial.sandboxes[0]); + let (server, client, store) = setup(initial).await; + report_conflict(&client, &sb, "ownership cannot be proven") + .await + .unwrap(); + report_conflict(&client, &sb, "ownership cannot be proven") + .await + .unwrap(); + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.iter().filter(|r| r.method == "PATCH").count(), 1); + let state = store.lock().unwrap(); + assert_eq!(state.namespace, original_namespace); + assert_eq!(state.sandboxes[0]["status"]["phase"], "Degraded"); + assert!( + state.sandboxes[0]["status"]["conditions"] + .as_array() + .unwrap() + .iter() + .any(|c| c["type"] == "Ready" && c["status"] == "False") + ); +} + +#[tokio::test] +async fn shared_target_gate_rejects_unclaimed_foreign_and_missing_source_owners() { + let initial = Store::default(); + let sb = sandbox(&initial.sandboxes[0]); + let (_server, client, store) = setup(initial).await; + assert!(verify_target(&client, "workspace-a", "demo").await.is_err()); + ensure(&client, &sb).await.unwrap(); + assert!(verify_target(&client, "workspace-a", "demo").await.unwrap()); + store.lock().unwrap().sandboxes[0]["metadata"]["uid"] = json!("foreign-uid"); + assert!(verify_target(&client, "workspace-a", "demo").await.is_err()); + store.lock().unwrap().sandboxes.clear(); + assert!(verify_target(&client, "workspace-a", "demo").await.is_err()); + store.lock().unwrap().namespace = None; + assert!(!verify_target(&client, "workspace-a", "demo").await.unwrap()); +} + +#[tokio::test] +async fn target_gate_rejects_path_shaped_names_before_api_access() { + let (server, client, _) = setup(Store::default()).await; + assert!( + verify_target(&client, "workspace-a", "../other") + .await + .is_err() + ); + assert!( + verify_target(&client, "../workspace-b", "demo") + .await + .is_err() + ); + assert!(server.received_requests().await.unwrap().is_empty()); +} + +#[tokio::test] +async fn token_reader_does_not_read_secrets_for_an_unclaimed_namespace() { + let (server, client, _) = setup(Store::default()).await; + let _lock = lock("demo").await; + assert!( + crate::status::router_confirmation_io::read_admin_token(&client, "workspace-a", "demo",) + .await + .is_err() + ); + assert!( + !server + .received_requests() + .await + .unwrap() + .iter() + .any(|request| request.url.path().contains("/secrets/")) + ); +} + +#[tokio::test] +async fn self_hosted_cr_cleanup_does_not_wait_for_its_own_namespace_to_disappear() { + let mut initial = Store::default(); + initial.sandboxes[0]["metadata"]["namespace"] = json!("kars-demo"); + initial.sandboxes[0]["metadata"]["deletionTimestamp"] = json!("2026-09-01T12:00:00Z"); + initial.namespace.as_mut().unwrap()["metadata"]["deletionTimestamp"] = + json!("2026-09-01T12:00:01Z"); + let sb = sandbox(&initial.sandboxes[0]); + merge( + initial.namespace.as_mut().unwrap(), + json!({"metadata": {"annotations": owner_annotations(&sb)}}), + ); + let ns = namespace(initial.namespace.as_ref().unwrap()); + let (_server, client, store) = setup(initial).await; + delete(&client, &sb, Some(&ns)).await.unwrap(); + assert!(store.lock().unwrap().namespace.is_some()); +} diff --git a/controller/src/status/router_confirmation_io.rs b/controller/src/status/router_confirmation_io.rs index b13dd44c..ad17bada 100644 --- a/controller/src/status/router_confirmation_io.rs +++ b/controller/src/status/router_confirmation_io.rs @@ -95,11 +95,18 @@ where /// awaiting-router condition rather than a hard failure (the /// sandbox reconciler may not yet have completed its first pass). /// An empty token string is folded into `Ok(None)` for the same -/// reason. +/// reason. The caller holds the namespace lifecycle lock across this +/// read and the authenticated request, as poll_referencing_sandboxes does. pub async fn read_admin_token( client: &Client, + source_namespace: &str, sandbox: &str, -) -> Result, kube::Error> { +) -> Result, crate::reconciler::namespace_ownership::Error> { + if !crate::reconciler::namespace_ownership::verify_target(client, source_namespace, sandbox) + .await? + { + return Ok(None); + } let secret_ns = format!("kars-{sandbox}"); let api: Api = Api::namespaced(client.clone(), &secret_ns); let secret = match api.get_opt("router-admin-token").await? { @@ -131,6 +138,7 @@ pub async fn read_admin_token( pub async fn poll_referencing_sandboxes( client: &Client, http: &reqwest::Client, + source_namespace: &str, sandboxes: &[String], ) -> Vec<( String, @@ -138,7 +146,8 @@ pub async fn poll_referencing_sandboxes( )> { let mut out = Vec::with_capacity(sandboxes.len()); for sandbox in sandboxes { - let token = match read_admin_token(client, sandbox).await { + let _namespace_lock = crate::reconciler::namespace_ownership::lock(sandbox).await; + let token = match read_admin_token(client, source_namespace, sandbox).await { Ok(Some(t)) => t, Ok(None) => { out.push((sandbox.clone(), Err(ConfirmError::HttpStatus(0)))); diff --git a/controller/src/tool_policy_reconciler.rs b/controller/src/tool_policy_reconciler.rs index 38a72cc8..8dd64ecc 100644 --- a/controller/src/tool_policy_reconciler.rs +++ b/controller/src/tool_policy_reconciler.rs @@ -235,7 +235,8 @@ async fn reconcile(tp: Arc, ctx: Arc) -> Result RouterEnforcementState::NotApplicable, diff --git a/deploy/helm/kars/templates/rbac.yaml b/deploy/helm/kars/templates/rbac.yaml index 14ac8f1d..49bf0084 100644 --- a/deploy/helm/kars/templates/rbac.yaml +++ b/deploy/helm/kars/templates/rbac.yaml @@ -76,7 +76,7 @@ rules: # Create and manage sandbox namespaces - apiGroups: [""] resources: ["namespaces"] - verbs: ["get", "list", "create", "update", "patch", "delete"] + verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] # Manage pods, services, configmaps in sandbox namespaces - apiGroups: [""] resources: ["pods", "services", "configmaps", "secrets", "serviceaccounts"] diff --git a/docs/how-to/helm-installation.md b/docs/how-to/helm-installation.md index 21fe0517..0ecc9400 100644 --- a/docs/how-to/helm-installation.md +++ b/docs/how-to/helm-installation.md @@ -4,6 +4,11 @@ Use the Helm chart when the Kubernetes cluster, image access, inference backend, and required identity configuration already exist. The chart can manage AgentMesh or use an existing external deployment. +Before upgrading an existing controller, run `kars namespace preflight` with the +updated CLI and resolve all ownership conflicts. See +[namespace ownership migration and adoption](namespace-ownership.md). The check +does not read Secrets or change namespaces, workloads, or Helm values. + ## Local kind ```bash diff --git a/docs/how-to/namespace-ownership.md b/docs/how-to/namespace-ownership.md new file mode 100644 index 00000000..53b068b9 --- /dev/null +++ b/docs/how-to/namespace-ownership.md @@ -0,0 +1,159 @@ +# Sandbox namespace ownership (claim v1) + +KarsSandbox CRs are namespaced, but their runtime namespace remains +`kars-`. Names must therefore be unique across workspaces when +claiming an unowned namespace. This feature adds ownership checks; it does not +rename namespaces, migrate workloads, change images, or introduce credential +sources. + +## Before upgrading the controller + +Use the CLI containing this feature **before** replacing the controller: + +```sh +kubectl config current-context +kars namespace preflight +``` + +Preflight is read-only and works on generic Kubernetes as well as AKS. It reads +Sandbox CRs, namespace metadata, and Deployments; it never reads Secrets. API, +RBAC, malformed-response, and transport errors fail the check rather than +appearing as an empty cluster. + +`kars upgrade` (including `--dry-run`) and `kars up --upgrade` run the same check +before changing the controller. Direct Helm/GitOps upgrades must run it explicitly. +No `up` provisioning or operator defaults change. A healthy controller Deployment +alone does **not** prove that sandbox ownership migration will succeed. + +The preflight must succeed for all existing Sandbox CRs. Resolve every failure +before upgrading. A conflict discovered later sets `Ready=False` and +`Degraded=True`, reason `NamespaceOwnershipConflict`; it leaves target resources +untouched. Existing running pods are not stopped by a failed adoption. + +### Automatic legacy adoption + +For an unclaimed namespace, the controller requires **all** of: + +* Exactly one current Sandbox of that name across all workspaces. +* Its existing `status.namespace` identifies the target namespace, and its + namespace-cleanup finalizer is already present. +* The namespace has controller-managed sandbox labels, including an `Apply` + managedFields entry by `kars-controller/karssandbox` owning the sandbox label. +* Its same-named Deployment has matching sandbox labels and selector/pod labels, + controller-managed spec fields, and no foreign ownerReferences. If the + Deployment carries `kars.azure.com/parent-namespace`, it must match the CR. +* The Deployment was created **strictly after** this Sandbox incarnation. + Equal timestamps are ambiguous because Kubernetes creation timestamps have + second precision. A newer recreated Sandbox cannot adopt an older Deployment. + +The namespace itself may predate the Sandbox: the legacy CLI created namespaces +to stage credentials before creating CRs. Proven deployments retain their +namespace UID, labels, workloads, pod templates, and all data. Adoption changes +only claim metadata and the Sandbox's namespace-UID backlink, without a rollout. + +Names, labels, managedFields, or CR status **alone** are not ownership proof. +Missing evidence, older field-manager formats, same-second creation, overlay-only +deployments, and unfinished legacy prestaging require an explicit administrator +decision. Preflight rejects these rather than silently disabling them on upgrade. + +### Explicit adoption of an ambiguous legacy namespace + +First verify that the namespace and **all** its contents belong to the intended +Sandbox. Check workspace, deployment history, creation times, and audit records. +Back up customer data through your normal secure process. Do not use adoption to +transfer a previous Sandbox incarnation's credentials to a new agent. + +Obtain the current identifiers without printing any Secret: + +```sh +kubectl get karssandbox demo -n workspace-a \ + -o jsonpath='{.metadata.uid}{"\n"}' +kubectl get namespace kars-demo -o jsonpath='{.metadata.uid}{"\n"}' +kars namespace adopt demo --namespace workspace-a \ + --sandbox-uid --namespace-uid +kars namespace preflight +``` + +The explicit adoption command is a privileged, metadata-only write. It requires a +unique live Sandbox, rejects foreign/partial claims, ownerReferences, terminating +resources, and stale reviewed UIDs, and uses namespace UID/resourceVersion +preconditions. It never deletes/recreates a namespace or copies credential data. +Only a namespace administrator should receive this permission. + +If a namespace already has a claim for another UID/workspace, stop and resolve +the original owner's lifecycle. Do not remove or overwrite its claim annotations. +If a *bound* namespace disappears or is replaced, recovery also requires an +administrator to investigate: the controller will not silently create another +namespace under an existing backlink. For failed same-name contenders with +finalizers, verify that they never owned the namespace before manually removing +only their own cleanup finalizer; never delete the winning namespace. + +## Claim contract + +The namespace carries these reserved annotations: + +| Annotation | Value | +|---|---| +| `kars.azure.com/namespace-claim-version` | `v1` | +| `kars.azure.com/sandbox-namespace` | Namespace containing the Sandbox CR | +| `kars.azure.com/sandbox-name` | Sandbox CR name | +| `kars.azure.com/sandbox-uid` | Exact live Sandbox UID | + +The Sandbox carries `kars.azure.com/namespace-uid`, the exact target namespace +UID. These are namespace-controller/administrator authority, not tenant labels. +Namespace create is atomic. Existing claims must match both identities; +adoption and backlink writes use UID/resourceVersion checks. A 409 retries from +fresh API reads, never through force apply. Same-name reconciles are serialized +inside the controller, in addition to API-server concurrency checks. + +Namespace watch events enqueue the annotated source CR, but are only routing +hints: live identity checks still authorize every reconciliation. Periodic +requeues provide a backstop. Egress-approval target writes/cleanup and router +admin-token reads use the same read-only claim check; policy confirmation keeps +the referring workspace rather than treating a bare Sandbox name as authority. +Run a single elected controller as usual; these +checks do not defend against a cluster administrator deliberately rewriting +namespace claims or bypassing Kubernetes lifecycle controls. + +## Credential prestaging compatibility + +Updated `kars add` still creates credentials **before** the Sandbox CR/pod. For a +new namespace it writes an explicit v1 reservation with source namespace/name, +no sandbox UID yet, and +`kars.azure.com/namespace-prestage: bind-next-sandbox`. The CR CREATE includes +that namespace's UID backlink. Only that two-way reservation permits initial +binding. The controller atomically replaces the prestage intent with the live +Sandbox UID before touching workloads. Subsequent CR recreation cannot reuse it. +An interrupted explicit reservation can be resumed by `kars add`; unrelated or +unmarked namespaces cannot. + +Existing running legacy deployments remain valid staging targets when the same +legacy proof succeeds. Unfinished **unmarked** prestaging from older clients +cannot be attributed safely: complete/verify those Sandbox workflows and record +an explicit administrator claim before upgrading. Inventory outstanding +pre-CR reservations separately (`kubectl get namespaces`); without a CR they are +not included in `namespace preflight`. Update provisioning clients to the v1 +reservation contract before creating further pre-CR namespaces. Do not resolve +this gap by granting broader Secret permissions. + +This feature does not change `credentials update/remove`, Secret contents, +EnvFrom behavior, or workspace/team credential merging. It is the namespace +safety prerequisite for a separate future credential-source feature. + +## Cleanup and rollback + +Deletion rechecks ownership and namespace UID, then sends UID/resourceVersion +preconditions. The CR cleanup finalizer remains until deletion is accepted (or +the namespace is already gone) and the remaining required cleanup succeeds. +The controller does not wait for namespace disappearance: a CR inside that +namespace would otherwise deadlock its own deletion. Terminating namespaces +retain their claim and cannot be adopted by another Sandbox. +Non-404 failures retain the CR finalizer and retry; +a stuck namespace is diagnosed, not force-finalized. Other CR finalizers are +preserved. + +Rolling back to a controller without claim-v1 support removes these protections: +older binaries ignore the claim metadata. Avoid provisioning, deleting, or +reusing sandbox names during such a rollback; resolve ownership and run preflight +before returning to the protected controller. No claim or namespace is +Helm-owned by this feature. diff --git a/tests/compat/fixtures/namespace-legacy.json b/tests/compat/fixtures/namespace-legacy.json new file mode 100644 index 00000000..51e67a84 --- /dev/null +++ b/tests/compat/fixtures/namespace-legacy.json @@ -0,0 +1,77 @@ +{ + "sandbox": { + "apiVersion": "kars.azure.com/v1alpha1", + "kind": "KarsSandbox", + "metadata": { + "name": "demo", + "namespace": "workspace-a", + "uid": "sandbox-a", + "resourceVersion": "10", + "generation": 1, + "creationTimestamp": "2026-09-01T10:00:00Z", + "finalizers": ["kars.azure.com/namespace-cleanup"] + }, + "spec": {"inferenceRef": {"name": "demo-inference"}}, + "status": {"namespace": "kars-demo", "phase": "Running"} + }, + "namespace": { + "apiVersion": "v1", + "kind": "Namespace", + "metadata": { + "name": "kars-demo", + "uid": "namespace-a", + "resourceVersion": "20", + "creationTimestamp": "2026-09-01T09:59:00Z", + "labels": { + "kars.azure.com/sandbox": "demo", + "kars.azure.com/role": "sandbox", + "customer-label": "keep" + }, + "annotations": {"customer-annotation": "keep"}, + "managedFields": [{ + "manager": "kars-controller/karssandbox", + "operation": "Apply", + "apiVersion": "v1", + "fieldsType": "FieldsV1", + "fieldsV1": {"f:metadata": {"f:labels": {"f:kars.azure.com/sandbox": {}}}} + }] + }, + "spec": {"finalizers": ["kubernetes"]} + }, + "deployment": { + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "name": "demo", + "namespace": "kars-demo", + "uid": "deployment-a", + "resourceVersion": "30", + "creationTimestamp": "2026-09-01T10:00:01Z", + "labels": { + "kars.azure.com/sandbox": "demo", + "kars.azure.com/parent-namespace": "workspace-a" + }, + "managedFields": [{ + "manager": "kars-controller/karssandbox", + "operation": "Apply", + "apiVersion": "apps/v1", + "fieldsType": "FieldsV1", + "fieldsV1": {"f:spec": {}} + }] + }, + "spec": { + "replicas": 1, + "selector": {"matchLabels": {"kars.azure.com/sandbox": "demo"}}, + "template": { + "metadata": {"labels": {"kars.azure.com/sandbox": "demo"}}, + "spec": { + "containers": [{ + "name": "openclaw", + "image": "customer.example/agent:latest", + "envFrom": [{"secretRef": {"name": "demo-credentials", "optional": true}}] + }] + } + } + } + } +} From e587063143ad30b24a02579b4a6a236798ebf297 Mon Sep 17 00:00:00 2001 From: pallakatos Date: Mon, 7 Sep 2026 18:50:14 +0200 Subject: [PATCH 2/5] test(controller): qualify namespace ownership lifecycle Keep the prestage fixture initializer compliant with strict Clippy. Qualified the actual namespace adoption, preservation, collision, UID recreation, CAS retry, and cleanup cases: all 107 affected controller tests pass, including 26 namespace ownership tests. Controller all-targets Clippy passes with warnings denied, using only the shared Cargo target and CARGO_INCREMENTAL=0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: eb3654cd-f1e0-445a-8734-430800af1903 --- controller/src/reconciler/namespace_ownership_tests.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/controller/src/reconciler/namespace_ownership_tests.rs b/controller/src/reconciler/namespace_ownership_tests.rs index 49a5b849..aecb917b 100644 --- a/controller/src/reconciler/namespace_ownership_tests.rs +++ b/controller/src/reconciler/namespace_ownership_tests.rs @@ -556,8 +556,10 @@ async fn read_errors_do_not_become_absence_or_trigger_creation() { #[tokio::test] async fn full_prestage_flow_binds_uid_atomically_and_preserves_existing_data() { - let mut initial = Store::default(); - initial.deployment = None; + let mut initial = Store { + deployment: None, + ..Store::default() + }; initial.sandboxes[0]["status"] = Value::Null; initial.sandboxes[0]["metadata"]["annotations"] = json!({NAMESPACE_UID: "namespace-a"}); let annotations = &mut initial.namespace.as_mut().unwrap()["metadata"]["annotations"]; From 62093414cb8d5d9937c1d9974504047c84669d6c Mon Sep 17 00:00:00 2001 From: pallakatos Date: Mon, 7 Sep 2026 20:12:06 +0200 Subject: [PATCH 3/5] fix(namespace): preserve producer compatibility and fence handoff credentials Complete the dev/SRE namespace producer migration, retain legacy Helm resources, preserve cancellation during claim binding, and fence existing handoff credential writes by the created sandbox and Secret identities. Keep genuine audit sign-offs and final independent closure pending. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: eb3654cd-f1e0-445a-8734-430800af1903 --- .../commands/dev/local-k8s-namespace.test.ts | 125 ++++ cli/src/commands/dev/local-k8s.ts | 25 +- cli/src/commands/sre.test.ts | 132 +++++ cli/src/commands/sre.ts | 61 +- cli/src/lib/namespace-ownership.ts | 4 +- .../testing/sre-namespace-ownership.test.ts | 192 +++++++ controller/src/reconciler/mod.rs | 7 + .../src/reconciler/namespace_ownership.rs | 45 ++ .../namespace_ownership_finalization_tests.rs | 544 ++++++++++++++++++ controller/src/reconciler/sre_writer.rs | 183 ++++++ controller/src/reconciler/sre_writer_tests.rs | 464 +++++++++++++++ deploy/helm/kars/templates/sre.yaml | 64 ++- deploy/helm/kars/values.yaml | 2 +- docs/how-to/namespace-ownership.md | 48 +- .../2026-09-07-sandbox-namespace-ownership.md | 157 +++++ inference-router/src/spawn/credentials.rs | 346 +++++++++++ .../src/spawn/credentials/test_server.rs | 299 ++++++++++ .../src/spawn/credentials/tests.rs | 387 +++++++++++++ inference-router/src/spawn/mod.rs | 125 +--- tests/e2e/namespace-ownership.sh | 61 ++ tests/e2e/run.sh | 4 + 21 files changed, 3102 insertions(+), 173 deletions(-) create mode 100644 cli/src/commands/dev/local-k8s-namespace.test.ts create mode 100644 cli/src/commands/sre.test.ts create mode 100644 cli/src/testing/sre-namespace-ownership.test.ts create mode 100644 controller/src/reconciler/namespace_ownership_finalization_tests.rs create mode 100644 controller/src/reconciler/sre_writer.rs create mode 100644 controller/src/reconciler/sre_writer_tests.rs create mode 100644 docs/security-audits/2026-09-07-sandbox-namespace-ownership.md create mode 100644 inference-router/src/spawn/credentials.rs create mode 100644 inference-router/src/spawn/credentials/test_server.rs create mode 100644 inference-router/src/spawn/credentials/tests.rs create mode 100644 tests/e2e/namespace-ownership.sh diff --git a/cli/src/commands/dev/local-k8s-namespace.test.ts b/cli/src/commands/dev/local-k8s-namespace.test.ts new file mode 100644 index 00000000..eeb2bcaf --- /dev/null +++ b/cli/src/commands/dev/local-k8s-namespace.test.ts @@ -0,0 +1,125 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { parseAllDocuments } from "yaml"; +import { autoCreateSandbox, type LocalK8sOptions } from "./local-k8s.js"; +import { CLAIM, namespacePrestaged, type OwnershipObject } from "../../lib/namespace-ownership.js"; + +const { execute } = vi.hoisted(() => ({ + execute: vi.fn<(file: string, args: readonly string[], options?: { input?: string }) => Promise<{ stdout: string }>>(), +})); +vi.mock("execa", () => ({ execa: execute })); +vi.mock("../../config.js", () => ({ + loadConfig: vi.fn(), + getSecret: (name: string) => name === "telegram-token" ? "test-channel-token" : undefined, +})); +vi.mock("../../refs.js", () => ({ loadAgtProfile: () => "version: 1\n" })); + +const tools = { + kind: "kind", kubectl: "/test/bin/kubectl", helm: "helm", + runtime: "docker", runtimeName: "docker" as const, env: {}, +}; +const options: LocalK8sOptions = { + name: "demo", clusterName: "isolated-test", image: "example.test/sandbox:latest", + ephemeral: false, noBuild: true, channels: "telegram", +}; +const credentials = { endpoint: "https://example.test", model: "test-model", apiKey: "" }; + +function cluster() { + const state = { + sandboxes: [] as OwnershipObject[], + namespace: undefined as OwnershipObject | undefined, + applied: [] as OwnershipObject[], + error: "", + }; + execute.mockImplementation(async (file, args, commandOptions) => { + expect(file).toBe(tools.kubectl); + expect(args.slice(0, 2)).toEqual(["--context", "kind-isolated-test"]); + const operation = args[2]; + if ((operation === "get" && state.error === "Forbidden (403)") + || (operation === "create" && state.error === "AlreadyExists (409)")) { + throw new Error(state.error); + } + if (operation === "get") { + const result = args[3] === "karssandboxes" ? { items: state.sandboxes } : state.namespace; + return { stdout: result ? JSON.stringify(result) : "" }; + } + if (operation === "create") { + expect(state.namespace).toBeUndefined(); + state.namespace = JSON.parse(commandOptions!.input!) as OwnershipObject; + state.namespace.metadata.uid = "reserved-uid"; + state.namespace.metadata.resourceVersion = "1"; + state.namespace.metadata.creationTimestamp = "2026-09-07T10:00:00Z"; + return { stdout: JSON.stringify(state.namespace) }; + } + if (operation === "apply") { + state.applied = parseAllDocuments(commandOptions!.input!).map(document => { + if (document.errors.length) throw document.errors[0]; + return document.toJSON(); + }).filter(Boolean); + return { stdout: "" }; + } + throw new Error(`Unexpected operation ${operation}`); + }); + return state; +} + +beforeEach(() => { execute.mockReset(); }); + +describe("local-k8s first-party namespace producer", () => { + it("reserves atomically before credentials and puts the exact UID on the Sandbox", async () => { + const state = cluster(); + await autoCreateSandbox(tools, options, credentials); + expect(execute.mock.calls.map(([, args]) => args[2])).toEqual(["get", "get", "create", "apply"]); + expect(state.applied.some(resource => resource.kind === "Namespace")).toBe(false); + const sandbox = state.applied.find(resource => resource.kind === "KarsSandbox")!; + sandbox.metadata.uid = "sandbox-uid"; + sandbox.metadata.resourceVersion = "2"; + sandbox.metadata.creationTimestamp = "2026-09-07T10:00:00Z"; + expect(sandbox.metadata.annotations?.[CLAIM.namespaceUid]).toBe("reserved-uid"); + expect(namespacePrestaged(state.namespace!, sandbox)).toBe(true); + const secret = state.applied.find(resource => resource.kind === "Secret")!; + expect(secret.metadata).toMatchObject({ name: "demo-credentials", namespace: "kars-demo" }); + expect(state.applied.indexOf(secret)).toBeLessThan(state.applied.indexOf(sandbox)); + + state.sandboxes = [sandbox]; + state.namespace!.metadata.annotations![CLAIM.uid] = "sandbox-uid"; + delete state.namespace!.metadata.annotations![CLAIM.prestage]; + await autoCreateSandbox(tools, options, credentials); + expect(execute.mock.calls.filter(([, args]) => args[2] === "create")).toHaveLength(1); + expect(state.namespace!.metadata.uid).toBe("reserved-uid"); + }); + + it("resumes only the explicit reservation after an interrupted creation", async () => { + const state = cluster(); + await autoCreateSandbox(tools, options, credentials); + await autoCreateSandbox(tools, options, credentials); + expect(execute.mock.calls.filter(([, args]) => args[2] === "create")).toHaveLength(1); + expect(state.applied.find(resource => resource.kind === "KarsSandbox")?.metadata.annotations) + .toEqual({ [CLAIM.namespaceUid]: "reserved-uid" }); + }); + + it("never stages credentials into an arbitrary pre-existing namespace", async () => { + const state = cluster(); + state.namespace = { metadata: { name: "kars-demo", uid: "customer-uid", resourceVersion: "1" } }; + await expect(autoCreateSandbox(tools, options, credentials)).rejects.toThrow("explicit adoption"); + expect(execute.mock.calls.every(([, args]) => args[2] === "get")).toBe(true); + }); + + it("rejects a same-name Sandbox in another workspace before any writes", async () => { + const state = cluster(); + state.sandboxes = [{ + metadata: { name: "demo", namespace: "other", uid: "other-uid", resourceVersion: "1" }, + }]; + await expect(autoCreateSandbox(tools, options, credentials)).rejects.toThrow("another workspace"); + expect(execute.mock.calls).toHaveLength(1); + }); + + it.each(["Forbidden (403)", "AlreadyExists (409)"])("surfaces %s without applying credentials", async error => { + const state = cluster(); + state.error = error; + await expect(autoCreateSandbox(tools, options, credentials)).rejects.toThrow(error); + expect(execute.mock.calls.some(([, args]) => args[2] === "apply")).toBe(false); + }); +}); diff --git a/cli/src/commands/dev/local-k8s.ts b/cli/src/commands/dev/local-k8s.ts index b0677daf..c3aadd38 100644 --- a/cli/src/commands/dev/local-k8s.ts +++ b/cli/src/commands/dev/local-k8s.ts @@ -30,6 +30,7 @@ import { stageMeshPlugin } from "../../lib/stage-mesh-plugin.js"; import { ensureAgtRepo, ensureAgtWheels } from "../../lib/agt-bootstrap.js"; import { resolveBundledAsset, requireBundledAsset, findRepoRootOrNull } from "../../lib/repo-assets.js"; import { buildCopilotFallbackChain } from "../../github-copilot.js"; +import { CLAIM, prepareCredentialNamespace } from "../../lib/namespace-ownership.js"; export interface LocalK8sOptions { /** Sandbox / agent name. Reused as Helm release name suffix. */ @@ -2581,10 +2582,9 @@ function resolveTelegramAllowFrom(channels: string | undefined): string | undefi } /** - * Auto-create the sandbox in the cluster: a one-shot YAML bundle with - * the namespace, optional credentials Secret (telegram/slack/discord - * tokens), the InferencePolicy CR, and the KarsSandbox CR. Server-side - * apply so re-running `kars dev` is idempotent. + * Reserve the runtime namespace before staging credentials, then apply the + * policies and Sandbox with the namespace UID backlink. Re-running `kars dev` + * preserves an existing, proven Sandbox namespace. * * The InferencePolicy `provider` field is just a tag — the actual * upstream is governed by the controller env (set by the per-run @@ -2592,7 +2592,7 @@ function resolveTelegramAllowFrom(channels: string | undefined): string | undefi * (Foundry / GitHub Models / GitHub Copilot) end up in the same * `azure-openai` provider tag here. */ -async function autoCreateSandbox( +export async function autoCreateSandbox( tools: Tooling, opts: LocalK8sOptions, creds: KarsConfig, @@ -2600,6 +2600,12 @@ async function autoCreateSandbox( ): Promise { const ns = `kars-${opts.name}`; const policyName = `${opts.name}-inference`; + const namespaceUid = await prepareCredentialNamespace( + (_file, args, options) => execa(tools.kubectl, [ + "--context", `kind-${opts.clusterName}`, ...args, + ], options), + opts.name, "kars-system", + ); // Channels: convert tokens to a base64-encoded Secret block. The // controller mounts `-credentials` via `envFrom: secretRef` @@ -2662,13 +2668,6 @@ async function autoCreateSandbox( const memoryStoreName = `memory-${opts.name.substring(0, 56)}`; const yaml = [ - "---", - "apiVersion: v1", - "kind: Namespace", - "metadata:", - ` name: ${ns}`, - " labels:", - ` kars.azure.com/sandbox: ${opts.name}`, credsBlock, "---", "apiVersion: kars.azure.com/v1alpha1", @@ -2805,6 +2804,8 @@ async function autoCreateSandbox( "metadata:", ` name: ${opts.name}`, " namespace: kars-system", + " annotations:", + ` ${CLAIM.namespaceUid}: ${JSON.stringify(namespaceUid)}`, ...(mcpGithub.enabled ? [" labels:", " mcp-github: allow"] : []), diff --git a/cli/src/commands/sre.test.ts b/cli/src/commands/sre.test.ts new file mode 100644 index 00000000..436c98fa --- /dev/null +++ b/cli/src/commands/sre.test.ts @@ -0,0 +1,132 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { sreCommand } from "./sre.js"; + +const { execute } = vi.hoisted(() => ({ + execute: vi.fn<(file: string, args: readonly string[], options?: unknown) => Promise<{ stdout: string }>>(), +})); +vi.mock("execa", () => ({ execa: execute })); +vi.mock("../lib/repo-assets.js", () => ({ requireBundledAsset: () => "/test/chart" })); + +const releases = JSON.stringify([{ name: "kars", namespace: "kars-system" }]); +const controller = JSON.stringify({ + apiVersion: "apps/v1", kind: "Deployment", + metadata: { name: "kars-controller", namespace: "kars-system", uid: "controller-uid" }, +}); + +beforeEach(() => { + execute.mockReset(); + vi.spyOn(console, "log").mockImplementation(() => {}); +}); +afterEach(() => vi.restoreAllMocks()); + +describe("SRE controller upgrade namespace preflight", () => { + it.each(["upgrade", "template"])("preflights %s mode in the selected context before mutations", async mode => { + execute.mockImplementation(async (file, args) => { + if (file === "helm" && args[0] === "list") return { stdout: mode === "upgrade" ? releases : "[]" }; + if (file === "kubectl" && args.includes("deployment")) { + expect(args).toContain("--ignore-not-found"); + return { stdout: controller }; + } + if (file === "kubectl" && args.includes("karssandboxes")) { + expect(args.slice(0, 2)).toEqual(["--context", "test-context"]); + return { stdout: '{"items":[]}' }; + } + return { stdout: "" }; + }); + await sreCommand().parseAsync(["node", "sre", "install", "--no-wait", "--context", "test-context"]); + const calls = execute.mock.calls; + const inspected = calls.findIndex(([file, args]) => file === "kubectl" && args.includes("karssandboxes")); + const mutation = calls.findIndex(([file, args]) => file === "helm" && args[0] === mode); + expect(inspected).toBeGreaterThanOrEqual(0); + expect(mutation).toBeGreaterThan(inspected); + }); + + it("stops an existing release upgrade when ownership cannot be inspected", async () => { + execute.mockImplementation(async (file, args) => { + if (file === "helm" && args[0] === "list") return { stdout: releases }; + throw new Error("Forbidden (403)"); + }); + await expect(sreCommand().parseAsync(["node", "sre", "install", "--no-wait"])) + .rejects.toThrow("403"); + expect(execute.mock.calls.some(([file, args]) => file === "helm" && args[0] === "upgrade")).toBe(false); + }); + + it.each(["timeout", "Forbidden (403)"])("never treats controller discovery %s as a fresh install", async error => { + execute.mockImplementation(async (file, args) => { + if (file === "helm" && args[0] === "list") return { stdout: "[]" }; + throw new Error(error); + }); + await expect(sreCommand().parseAsync(["node", "sre", "install", "--no-wait"])) + .rejects.toThrow(error); + expect(execute.mock.calls).toHaveLength(2); + expect(execute.mock.calls.some(([file, args]) => file === "helm" && args[0] !== "list")).toBe(false); + }); + + it("propagates Helm discovery errors without attempting another install path", async () => { + execute.mockRejectedValue(new Error("Helm authorization failed")); + await expect(sreCommand().parseAsync(["node", "sre", "install", "--no-wait"])) + .rejects.toThrow("authorization"); + expect(execute.mock.calls).toHaveLength(1); + }); + + it.each(["{}", "null", '[{"name":"kars","namespace":"other"}]'])("rejects invalid release inventory %s", async stdout => { + execute.mockResolvedValue({ stdout }); + await expect(sreCommand().parseAsync(["node", "sre", "install", "--no-wait"])) + .rejects.toThrow("invalid release inventory"); + expect(execute.mock.calls).toHaveLength(1); + }); + + it.each(["null", "{}", '{"kind":"Deployment","metadata":{"name":"foreign"}}'])("rejects invalid controller identity %s", async stdout => { + execute.mockImplementation(async (file, args) => ({ + stdout: file === "helm" && args[0] === "list" ? "[]" : stdout, + })); + await expect(sreCommand().parseAsync(["node", "sre", "install", "--no-wait"])) + .rejects.toThrow("invalid controller identity"); + expect(execute.mock.calls).toHaveLength(2); + }); + + it("permits fresh installation only after successful inventory and explicit controller absence", async () => { + execute.mockImplementation(async (file, args) => ({ + stdout: file === "helm" && args[0] === "list" ? "[]" : "", + })); + await sreCommand().parseAsync(["node", "sre", "install", "--no-wait"]); + expect(execute.mock.calls.map(([file, args]) => [file, args[0]])).toEqual([ + ["helm", "list"], ["kubectl", "-n"], ["helm", "install"], + ]); + expect(execute.mock.calls[1][1]).toContain("--ignore-not-found"); + }); + + it.each([ + { target: "kars.prod", names: ["kars.prod"] }, + { target: "kars", names: ["other.release", "kars"] }, + { target: "kars", names: ["other.release"] }, + { target: "a".repeat(53), names: ["a".repeat(53)] }, + ])("accepts Helm-compatible release inventory for $target", async ({ target, names }) => { + execute.mockImplementation(async (file, args) => { + if (file === "helm" && args[0] === "list") { + return { stdout: JSON.stringify(names.map(name => ({ name, namespace: "kars-system" }))) }; + } + if (file === "kubectl" && args.includes("deployment")) return { stdout: controller }; + if (file === "kubectl" && args.includes("karssandboxes")) return { stdout: '{"items":[]}' }; + return { stdout: "" }; + }); + await sreCommand().parseAsync(["node", "sre", "install", "--no-wait", "--release", target]); + const mode = names.includes(target) ? "upgrade" : "template"; + expect(execute.mock.calls.some(([file, args]) => file === "helm" && args[0] === mode && args[1] === target)) + .toBe(true); + expect(execute.mock.calls.some(([, args]) => args.includes("karssandboxes"))).toBe(true); + }); + + it.each(["", ".kars", "kars.", "kars..prod", "kars.-prod", "Kars", "a".repeat(54)])( + "rejects the Helm-invalid release name %s without installation", + async name => { + execute.mockResolvedValue({ stdout: JSON.stringify([{ name, namespace: "kars-system" }]) }); + await expect(sreCommand().parseAsync(["node", "sre", "install", "--no-wait"])) + .rejects.toThrow("invalid release inventory"); + expect(execute.mock.calls).toHaveLength(1); + }, + ); +}); diff --git a/cli/src/commands/sre.ts b/cli/src/commands/sre.ts index bb49c509..09e8e71e 100644 --- a/cli/src/commands/sre.ts +++ b/cli/src/commands/sre.ts @@ -5,6 +5,9 @@ import { Command } from "commander"; import chalk from "chalk"; import { execa } from "execa"; import { requireBundledAsset } from "../lib/repo-assets.js"; +import { inspectNamespaceOwnership } from "../lib/namespace-ownership.js"; + +const HELM_RELEASE_NAME = /^[a-z0-9](?:[-a-z0-9]*[a-z0-9])?(?:\.[a-z0-9](?:[-a-z0-9]*[a-z0-9])?)*$/; /** * `kars sre` — autonomous incident triage controller. @@ -77,42 +80,50 @@ export function sreCommand(): Command { // C. no chart at all → `helm install` with --take-ownership + // a fallback workload-identity client-id (local dev). let mode: "upgrade" | "template" | "install" = "install"; - const listArgs = ["list", "-n", options.namespace, "-q"]; + const listArgs = ["list", "-n", options.namespace, "--all", "-o", "json"]; if (options.context) listArgs.push("--kube-context", options.context); - try { - const { stdout } = await execa("helm", listArgs, { stdio: "pipe" }); - if ( - stdout - .split(/\r?\n/) - .map(s => s.trim()) - .includes(options.release) - ) { - mode = "upgrade"; - } - } catch { - // helm list errored — treat as "not installed" + const { stdout: releasesOutput } = await execa("helm", listArgs, { stdio: "pipe", timeout: 30_000 }); + const releases: unknown = JSON.parse(releasesOutput); + if (!Array.isArray(releases) || releases.some(release => + !release || typeof release !== "object" || typeof release.name !== "string" + || release.name.length > 53 || !HELM_RELEASE_NAME.test(release.name) + || release.namespace !== options.namespace)) { + throw new Error("Helm returned an invalid release inventory; SRE installation stopped before making changes."); + } + if (releases.some(release => release.name === options.release)) { + mode = "upgrade"; } if (mode === "install") { // Check whether the controller already runs in the namespace. // Presence implies `kars dev` deployed it via `helm template // | kubectl apply` — adopting via plain `helm install` would // fail on every pre-existing resource. Take the template path. - try { - await execa( - "kubectl", - [ - ...(options.context ? ["--context", options.context] : []), - "-n", options.namespace, - "get", "deploy/kars-controller", - ], - { stdio: "ignore" }, - ); + // Only a successful --ignore-not-found empty result proves absence. + // A failed read must never select the forced fresh-install path. + const { stdout } = await execa("kubectl", [ + ...(options.context ? ["--context", options.context] : []), + "-n", options.namespace, "get", "deployment", "kars-controller", + "--ignore-not-found", "-o", "json", + ], { stdio: "pipe", timeout: 30_000 }); + if (stdout.trim()) { + const deployment: unknown = JSON.parse(stdout); + if (!deployment || typeof deployment !== "object" + || !("kind" in deployment) || deployment.kind !== "Deployment" + || !("metadata" in deployment) || !deployment.metadata || typeof deployment.metadata !== "object" + || !("name" in deployment.metadata) || deployment.metadata.name !== "kars-controller" + || !("namespace" in deployment.metadata) || deployment.metadata.namespace !== options.namespace + || !("uid" in deployment.metadata) || typeof deployment.metadata.uid !== "string" || !deployment.metadata.uid) { + throw new Error("Kubernetes returned an invalid controller identity; SRE installation stopped before making changes."); + } mode = "template"; - } catch { - // Controller missing → fresh cluster → safe to helm install. } } + if (mode !== "install") { + await inspectNamespaceOwnership((file, args, commandOptions) => + execa(file, [...(options.context ? ["--context", options.context] : []), ...args], commandOptions)); + } + const helmArgs = mode === "upgrade" ? [ diff --git a/cli/src/lib/namespace-ownership.ts b/cli/src/lib/namespace-ownership.ts index 77a2c947..1af8d2ea 100644 --- a/cli/src/lib/namespace-ownership.ts +++ b/cli/src/lib/namespace-ownership.ts @@ -1,7 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import type { Execute } from "./deployment-target.js"; +type Execute = ( + file: string, args: readonly string[], options: { stdio: "pipe"; input?: string }, +) => Promise<{ stdout: string }>; export const CLAIM = { version: "kars.azure.com/namespace-claim-version", diff --git a/cli/src/testing/sre-namespace-ownership.test.ts b/cli/src/testing/sre-namespace-ownership.test.ts new file mode 100644 index 00000000..b10a8ef6 --- /dev/null +++ b/cli/src/testing/sre-namespace-ownership.test.ts @@ -0,0 +1,192 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { createServer } from "node:http"; +import { copyFileSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { execa } from "execa"; +import { describe, expect, it } from "vitest"; +import { parseAllDocuments } from "yaml"; + +const chart = fileURLToPath(new URL("../../../deploy/helm/kars", import.meta.url)); +interface Resource { + kind: string; + metadata: { + name: string; namespace?: string; + labels?: Record; annotations?: Record; + }; + automountServiceAccountToken?: boolean; +} +function documents(output: string): Resource[] { + return parseAllDocuments(output).map(document => { + if (document.errors.length) throw document.errors[0]; + return document.toJSON(); + }).filter(Boolean); +} +function legacy(kind: string, name: string, release = "kars"): Resource { + return { + kind, + metadata: { + name, ...(kind === "ServiceAccount" ? { namespace: "kars-sre" } : {}), + labels: { "customer.example/label": "preserve", "app.kubernetes.io/managed-by": "Helm" }, + annotations: { + "meta.helm.sh/release-name": release, + "meta.helm.sh/release-namespace": "kars-system", + "customer.example/annotation": "preserve", + "kars.azure.com/sandbox-uid": "existing-sandbox-uid", + }, + }, + ...(kind === "ServiceAccount" ? { automountServiceAccountToken: false } : {}), + }; +} + +async function upgrade( + namespace: Resource | undefined, writer: Resource | undefined, enabled = true, forbidden = false, +): Promise { + const server = createServer((request, response) => { + const path = new URL(request.url!, "http://localhost").pathname; + response.setHeader("content-type", "application/json"); + if (request.method !== "GET") { + response.writeHead(405); + response.end(JSON.stringify({ kind: "Status", reason: "MethodNotAllowed", code: 405 })); + return; + } + let result: unknown; + if (path === "/version") result = { major: "1", minor: "32", gitVersion: "v1.32.0" }; + if (path === "/api") result = { kind: "APIVersions", apiVersion: "v1", versions: ["v1"] }; + const groups = [ + { name: "kars.azure.com", version: "v1alpha1" }, + { name: "rbac.authorization.k8s.io", version: "v1" }, + ].map(({ name, version }) => ({ + name, versions: [{ groupVersion: `${name}/${version}`, version }], + preferredVersion: { groupVersion: `${name}/${version}`, version }, + })); + if (path === "/apis") result = { kind: "APIGroupList", apiVersion: "v1", groups }; + if (path === "/api/v1") result = { + kind: "APIResourceList", apiVersion: "v1", groupVersion: "v1", + resources: [ + { name: "namespaces", kind: "Namespace", namespaced: false, verbs: ["get", "list"] }, + { name: "serviceaccounts", kind: "ServiceAccount", namespaced: true, verbs: ["get", "list"] }, + ], + }; + if (path === "/api/v1/namespaces/kars-sre") result = namespace && { apiVersion: "v1", ...namespace }; + if (path === "/apis/kars.azure.com/v1alpha1") result = { + kind: "APIResourceList", apiVersion: "v1", groupVersion: "kars.azure.com/v1alpha1", + resources: [ + { name: "karssandboxes", kind: "KarsSandbox", namespaced: true, verbs: ["get", "list"] }, + { name: "inferencepolicies", kind: "InferencePolicy", namespaced: true, verbs: ["get", "list"] }, + { name: "toolpolicies", kind: "ToolPolicy", namespaced: true, verbs: ["get", "list"] }, + ], + }; + if (path === "/apis/rbac.authorization.k8s.io/v1") result = { + kind: "APIResourceList", apiVersion: "v1", groupVersion: "rbac.authorization.k8s.io/v1", + resources: [ + { name: "clusterroles", kind: "ClusterRole", namespaced: false, verbs: ["get", "list"] }, + { name: "clusterrolebindings", kind: "ClusterRoleBinding", namespaced: false, verbs: ["get", "list"] }, + ], + }; + if (path === "/api/v1/namespaces/kars-sre/serviceaccounts/sre-writer") { + result = writer && { apiVersion: "v1", ...writer }; + } + if (forbidden && path === "/api/v1/namespaces/kars-sre") { + response.writeHead(403); + response.end(JSON.stringify({ + kind: "Status", apiVersion: "v1", reason: "Forbidden", code: 403, + message: "Forbidden: namespace lookup denied", + })); + return; + } + if (!result) { + response.writeHead(404); + result = { kind: "Status", apiVersion: "v1", reason: "NotFound", code: 404 }; + } + response.end(JSON.stringify(result)); + }); + const directory = mkdtempSync(join(tmpdir(), "kars-sre-lookup-")); + try { + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("Expected TCP test API address"); + const kubeconfig = join(directory, "config"); + const fixtureChart = join(directory, "chart"); + mkdirSync(join(fixtureChart, "templates"), { recursive: true }); + for (const file of ["Chart.yaml", "values.yaml", "templates/sre.yaml"]) { + copyFileSync(join(chart, file), join(fixtureChart, file)); + } + writeFileSync(kubeconfig, JSON.stringify({ + apiVersion: "v1", kind: "Config", + clusters: [{ name: "fixture", cluster: { server: `http://127.0.0.1:${address.port}` } }], + users: [{ name: "fixture", user: {} }], + contexts: [{ name: "fixture", context: { cluster: "fixture", user: "fixture" } }], + "current-context": "fixture", + }), { mode: 0o600 }); + const { stdout } = await execa("helm", [ + "template", "kars", fixtureChart, "--namespace", "kars-system", + "--kubeconfig", kubeconfig, "--dry-run=server", "--is-upgrade", + // The fixture serves discovery and live lookup, not an OpenAPI schema. + "--disable-openapi-validation", + "--set", `sre.enabled=${enabled}`, "--show-only", "templates/sre.yaml", + ], { timeout: 20_000 }); + return documents(stdout); + } finally { + server.closeAllConnections(); + await new Promise((resolve, reject) => server.close(error => error ? reject(error) : resolve())); + rmSync(directory, { recursive: true, force: true }); + } +} + +describe("SRE namespace ownership (actual Helm lookup against an isolated test API)", () => { + it("leaves fresh runtime namespaces and writer accounts to the controller", async () => { + const { stdout } = await execa("helm", [ + "template", "kars", chart, "--namespace", "kars-system", + "--set", "sre.enabled=true", "--show-only", "templates/sre.yaml", + ]); + const resources = documents(stdout); + expect(resources.some(resource => resource.kind === "Namespace" || resource.kind === "ServiceAccount")).toBe(false); + const namespaced = resources.filter(resource => resource.metadata.namespace); + expect(namespaced).toHaveLength(3); + expect(namespaced.every(resource => resource.metadata.namespace === "kars-system")).toBe(true); + expect(resources.some(resource => resource.kind === "KarsSandbox" && resource.metadata.name === "sre")).toBe(true); + }); + + it.each([true, false])("retains a legacy namespace without deleting its data when enabled=%s", async enabled => { + const namespace = legacy("Namespace", "kars-sre"); + const writer = legacy("ServiceAccount", "sre-writer"); + const resources = await upgrade(namespace, writer, enabled); + const retained = resources.find(resource => resource.kind === "Namespace")!; + expect(retained.metadata.labels).toEqual(namespace.metadata.labels); + expect(retained.metadata.annotations).toEqual({ + ...namespace.metadata.annotations, "helm.sh/resource-policy": "keep", + }); + const account = resources.find(resource => resource.kind === "ServiceAccount"); + if (enabled) { + expect(account?.metadata.annotations).toEqual({ + ...writer.metadata.annotations, "kars.azure.com/no-automount": "true", + }); + expect(account?.metadata.labels).toEqual(writer.metadata.labels); + expect(account?.automountServiceAccountToken).toBe(false); + } else { + expect(account).toBeUndefined(); + expect(resources).toHaveLength(1); + } + }); + + it("does not claim another release's namespace or service account", async () => { + const resources = await upgrade( + legacy("Namespace", "kars-sre", "other-release"), + legacy("ServiceAccount", "sre-writer", "other-release"), + ); + expect(resources.some(resource => ["Namespace", "ServiceAccount"].includes(resource.kind))).toBe(false); + }); + + it("does not invent a namespace when upgrading a release that never enabled SRE", async () => { + const resources = await upgrade(undefined, undefined); + expect(resources.some(resource => ["Namespace", "ServiceAccount"].includes(resource.kind))).toBe(false); + }); + + it("propagates namespace lookup failures instead of omitting a possibly owned resource", async () => { + await expect(upgrade(undefined, undefined, true, true)).rejects.toThrow(/error calling lookup/); + }); +}); diff --git a/controller/src/reconciler/mod.rs b/controller/src/reconciler/mod.rs index 81c24f57..38bcdfcd 100644 --- a/controller/src/reconciler/mod.rs +++ b/controller/src/reconciler/mod.rs @@ -39,6 +39,7 @@ pub(crate) mod governance_mounts; mod inference; mod mcp_egress; pub(crate) mod namespace_ownership; +mod sre_writer; pub(crate) mod trustgraph_mount; use mcp_egress::mcp_egress_rule; @@ -181,6 +182,9 @@ async fn reconcile(sandbox: Arc, ctx: Arc) -> Result (Arc::new(live), namespace), Err(error) => { @@ -683,6 +687,9 @@ async fn reconcile(sandbox: Arc, ctx: Arc) -> Result Result { }))?) } +/// Namespace GC may remove a legacy Deployment before deleting the self-hosted +/// Sandbox CR. No ownership adoption is needed to release only our finalizer on +/// that live CR: namespace deletion is already in progress independently of us. +/// A true result must stop reconciliation, including all other cleanup. +pub async fn finalize_legacy_namespace_gc( + client: &Client, + observed: &KarsSandbox, +) -> Result { + let namespace_name = format!("kars-{}", observed.name_any()); + if observed.metadata.deletion_timestamp.is_none() + || observed.namespace().as_deref() != Some(namespace_name.as_str()) + { + return Ok(false); + } + let sandbox = live_sandbox(client, observed).await?; + if sandbox.metadata.deletion_timestamp.is_none() + || sandbox.namespace().as_deref() != Some(namespace_name.as_str()) + { + return Ok(false); + } + let namespaces: Api = Api::all(client.clone()); + let Some(namespace) = namespaces.get_opt(&namespace_name).await? else { + return Ok(false); + }; + if namespace.metadata.deletion_timestamp.is_none() + || prestaged(&namespace, &sandbox) + || claimed(&namespace, &sandbox)? + { + return Ok(false); + } + // claimed() rejects conflicting/partial claims, ownerReferences, and any + // recorded namespace-UID mismatch. Only its unclaimed legacy case gets here. + let mut finalizers = sandbox.metadata.finalizers.clone().unwrap_or_default(); + let before = finalizers.len(); + finalizers.retain(|finalizer| finalizer != FINALIZER); + if finalizers.len() != before { + patch_sandbox(client, &sandbox, json!({"finalizers": finalizers})).await?; + } + Ok(true) +} + /// Establish authority before *any* target-namespace operations. HTTP conflicts /// propagate to the controller's retry queue; every retry re-reads all evidence. pub async fn ensure( @@ -565,3 +606,7 @@ pub async fn report_conflict( #[cfg(test)] #[path = "namespace_ownership_tests.rs"] mod tests; + +#[cfg(test)] +#[path = "namespace_ownership_finalization_tests.rs"] +mod finalization_tests; diff --git a/controller/src/reconciler/namespace_ownership_finalization_tests.rs b/controller/src/reconciler/namespace_ownership_finalization_tests.rs new file mode 100644 index 00000000..54f42bc2 --- /dev/null +++ b/controller/src/reconciler/namespace_ownership_finalization_tests.rs @@ -0,0 +1,544 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use super::*; +use crate::reconciler::{Context, error_policy, reconcile}; +use kube::runtime::controller::Action; +use std::sync::{Arc, Mutex as StdMutex}; +use wiremock::{Mock, MockServer, Request, Respond, ResponseTemplate}; + +const CR_PATH: &str = "/apis/kars.azure.com/v1alpha1/namespaces/kars-demo/karssandboxes/demo"; +const NS_PATH: &str = "/api/v1/namespaces/kars-demo"; +const OTHER_FINALIZER: &str = "customer.example/cleanup"; +const CONCURRENT_FINALIZER: &str = "customer.example/concurrent-cleanup"; + +#[derive(Clone, Copy)] +enum Fault { + None, + SourceGet(u16), + NamespaceGet(u16), + Patch(u16), + RecreateOnPatch, + BeginDeletionOnNamespacePatch, +} + +struct State { + sandbox: Value, + namespace: Value, + fault: Fault, +} + +impl State { + fn legacy(fault: Fault) -> Self { + let fixture: Value = serde_json::from_str(include_str!( + "../../../tests/compat/fixtures/namespace-legacy.json" + )) + .unwrap(); + let mut sandbox = fixture["sandbox"].clone(); + sandbox["metadata"]["namespace"] = json!("kars-demo"); + sandbox["metadata"]["deletionTimestamp"] = json!("2026-09-01T12:00:01Z"); + sandbox["metadata"]["finalizers"] = json!([FINALIZER, OTHER_FINALIZER]); + sandbox["metadata"]["annotations"] = json!({"customer.example/setting": "keep"}); + let mut namespace = fixture["namespace"].clone(); + namespace["metadata"]["deletionTimestamp"] = json!("2026-09-01T12:00:00Z"); + namespace["status"] = json!({"phase": "Terminating"}); + Self { + sandbox, + namespace, + fault, + } + } + + fn observed(&self) -> Arc { + Arc::new(serde_json::from_value(self.sandbox.clone()).unwrap()) + } + + fn prestaged(fault: Fault) -> Self { + let mut state = Self::legacy(fault); + state.sandbox["metadata"]["annotations"][NAMESPACE_UID] = + state.namespace["metadata"]["uid"].clone(); + state.namespace["metadata"]["annotations"] = json!({ + VERSION: "v1", SOURCE_NAME: "demo", SOURCE_NAMESPACE: "kars-demo", + PRESTAGE: "bind-next-sandbox", "customer.example/setting": "keep", + }); + state + } +} + +fn context(client: Client) -> Arc { + Arc::new(Context { + client, + wi_client_id: String::new(), + inference_router_image: String::new(), + sandbox_image: String::new(), + openai_endpoint: String::new(), + foundry_endpoint: String::new(), + foundry_project_endpoint: String::new(), + foundry_deployments: String::new(), + imds_client_id: String::new(), + content_safety_endpoint: String::new(), + fedcred: None, + byo_strict: false, + dev_openai_api_key: String::new(), + dev_provider: String::new(), + dev_copilot_github_token: String::new(), + anthropic_api_key: String::new(), + anthropic_endpoint: String::new(), + ollama_endpoint: String::new(), + openai_moderation_api_key: String::new(), + openai_moderation_endpoint: String::new(), + dev_profile: false, + cluster_name: None, + cluster_uid: String::new(), + agent_id_cache: Arc::new(crate::agent_id_provisioning::ProvisionerCache::new()), + }) +} + +fn response(code: u16, body: Value) -> ResponseTemplate { + ResponseTemplate::new(code).set_body_json(body) +} + +fn failure(code: u16) -> ResponseTemplate { + response( + code, + json!({ + "apiVersion": "v1", "kind": "Status", "status": "Failure", + "reason": if code == 404 { "NotFound" } else { "Conflict" }, "code": code, + }), + ) +} + +#[derive(Clone)] +struct Server(Arc>); + +impl Respond for Server { + fn respond(&self, request: &Request) -> ResponseTemplate { + let mut state = self.0.lock().unwrap(); + let path = request.url.path(); + match (request.method.as_str(), path) { + ("GET", CR_PATH) => { + if let Fault::SourceGet(code) = state.fault { + state.fault = Fault::None; + return failure(code); + } + response(200, state.sandbox.clone()) + } + ("GET", NS_PATH) => { + if let Fault::NamespaceGet(code) = state.fault { + state.fault = Fault::None; + return failure(code); + } + response(200, state.namespace.clone()) + } + ("GET", "/apis/kars.azure.com/v1alpha1/karssandboxes") => response( + 200, + json!({ + "apiVersion": "kars.azure.com/v1alpha1", "kind": "KarsSandboxList", + "metadata": {}, "items": [state.sandbox], + }), + ), + ("GET", "/apis/apps/v1/namespaces/kars-demo/deployments/demo") => failure(404), + ( + "DELETE", + "/apis/rbac.authorization.k8s.io/v1/clusterrolebindings/kars-spawner-demo", + ) => failure(404), + ("PATCH", NS_PATH) => { + let patch: Value = serde_json::from_slice(&request.body).unwrap(); + assert_eq!(patch["metadata"]["uid"], state.namespace["metadata"]["uid"]); + assert_eq!( + patch["metadata"]["resourceVersion"], + state.namespace["metadata"]["resourceVersion"] + ); + if matches!(state.fault, Fault::BeginDeletionOnNamespacePatch) { + state.fault = Fault::None; + assert!( + state.sandbox["metadata"]["finalizers"] + .as_array() + .unwrap() + .contains(&json!(FINALIZER)) + ); + state.namespace["metadata"]["deletionTimestamp"] = + json!("2026-09-01T12:00:00Z"); + state.namespace["metadata"]["resourceVersion"] = json!("31"); + state.sandbox["metadata"]["deletionTimestamp"] = json!("2026-09-01T12:00:01Z"); + state.sandbox["metadata"]["resourceVersion"] = json!("23"); + return failure(409); + } + let annotations = state.namespace["metadata"]["annotations"] + .as_object_mut() + .unwrap(); + for (key, value) in patch["metadata"]["annotations"].as_object().unwrap() { + if value.is_null() { + annotations.remove(key); + } else { + annotations.insert(key.clone(), value.clone()); + } + } + state.namespace["metadata"]["resourceVersion"] = json!("32"); + response(200, state.namespace.clone()) + } + ("PATCH", CR_PATH) => { + let patch: Value = serde_json::from_slice(&request.body).unwrap(); + assert_eq!( + request.headers.get("content-type").unwrap(), + "application/merge-patch+json" + ); + assert_eq!(patch.as_object().unwrap().len(), 1); + assert_eq!(patch["metadata"].as_object().unwrap().len(), 3); + match state.fault { + Fault::Patch(code) => { + state.fault = Fault::None; + if code == 409 { + state.sandbox["metadata"]["resourceVersion"] = json!("21"); + state.sandbox["metadata"]["finalizers"] + .as_array_mut() + .unwrap() + .push(json!(CONCURRENT_FINALIZER)); + } + return failure(code); + } + Fault::RecreateOnPatch => { + state.fault = Fault::None; + state.sandbox["metadata"]["uid"] = json!("recreated-sandbox"); + state.sandbox["metadata"]["resourceVersion"] = json!("21"); + } + _ => {} + } + if patch["metadata"]["uid"] != state.sandbox["metadata"]["uid"] + || patch["metadata"]["resourceVersion"] + != state.sandbox["metadata"]["resourceVersion"] + { + return failure(409); + } + state.sandbox["metadata"]["finalizers"] = patch["metadata"]["finalizers"].clone(); + state.sandbox["metadata"]["resourceVersion"] = json!("22"); + response(200, state.sandbox.clone()) + } + _ => failure(500), + } + } +} + +async fn setup(state: State) -> (MockServer, Client, Arc>) { + let server = MockServer::start().await; + let state = Arc::new(StdMutex::new(state)); + Mock::given(wiremock::matchers::any()) + .respond_with(Server(state.clone())) + .mount(&server) + .await; + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + let client = Client::try_from(kube::Config::new(server.uri().parse().unwrap())).unwrap(); + (server, client, state) +} + +fn only_finalizer_io(requests: &[Request]) { + assert!(requests.iter().all(|request| { + matches!( + (request.method.as_str(), request.url.path()), + ("GET", CR_PATH) | ("GET", NS_PATH) | ("PATCH", CR_PATH) + ) + })); +} + +#[tokio::test] +async fn reconciler_entry_finalizes_terminating_legacy_self_host_without_deployment_or_adoption() { + let state = State::legacy(Fault::None); + let original_namespace = state.namespace.clone(); + let mut expected_sandbox = state.sandbox.clone(); + let mut observed = (*state.observed()).clone(); + observed.metadata.resource_version = Some("stale-cache-version".into()); + let (server, client, state) = setup(state).await; + let result = reconcile(Arc::new(observed), context(client)) + .await + .unwrap(); + assert_eq!(result, Action::await_change()); + let requests = server.received_requests().await.unwrap(); + only_finalizer_io(&requests); + assert_eq!(requests.len(), 3); + let patch: Value = serde_json::from_slice(&requests[2].body).unwrap(); + assert_eq!( + patch["metadata"]["uid"], + expected_sandbox["metadata"]["uid"] + ); + assert_eq!( + patch["metadata"]["resourceVersion"], + expected_sandbox["metadata"]["resourceVersion"] + ); + expected_sandbox["metadata"]["finalizers"] = json!([OTHER_FINALIZER]); + expected_sandbox["metadata"]["resourceVersion"] = json!("22"); + let state = state.lock().unwrap(); + assert_eq!(state.namespace, original_namespace); + assert_eq!(state.sandbox, expected_sandbox); +} + +#[tokio::test] +async fn reconciler_entry_cancels_prestage_when_deletion_races_binding_after_finalizer_creation() { + let mut state = State::prestaged(Fault::BeginDeletionOnNamespacePatch); + state.sandbox["metadata"] + .as_object_mut() + .unwrap() + .remove("deletionTimestamp"); + state.namespace["metadata"] + .as_object_mut() + .unwrap() + .remove("deletionTimestamp"); + state.namespace["status"]["phase"] = json!("Active"); + state.sandbox["metadata"]["finalizers"] = json!([OTHER_FINALIZER]); + let observed = state.observed(); + let namespace_uid = state.namespace["metadata"]["uid"].clone(); + let sandbox_uid = state.sandbox["metadata"]["uid"].clone(); + let (server, client, state) = setup(state).await; + let ctx = context(client); + + let error = reconcile(observed.clone(), ctx.clone()).await.unwrap_err(); + assert_ne!( + error_policy(observed, &error, ctx.clone()), + Action::await_change() + ); + let deleting = { + let state = state.lock().unwrap(); + assert!( + state.sandbox["metadata"]["finalizers"] + .as_array() + .unwrap() + .contains(&json!(FINALIZER)) + ); + assert!(state.sandbox["metadata"]["deletionTimestamp"].is_string()); + assert!(state.namespace["metadata"]["deletionTimestamp"].is_string()); + assert_eq!( + state.namespace["metadata"]["annotations"][PRESTAGE], + "bind-next-sandbox" + ); + assert!(state.namespace["metadata"]["annotations"][SOURCE_UID].is_null()); + state.observed() + }; + assert_eq!( + reconcile(deleting, ctx).await.unwrap(), + Action::await_change() + ); + { + let state = state.lock().unwrap(); + assert_eq!( + state.sandbox["metadata"]["finalizers"], + json!([OTHER_FINALIZER]) + ); + assert_eq!(state.namespace["metadata"]["uid"], namespace_uid); + assert_eq!( + state.namespace["metadata"]["annotations"][SOURCE_UID], + sandbox_uid + ); + assert!(state.namespace["metadata"]["annotations"][PRESTAGE].is_null()); + assert_eq!( + state.namespace["metadata"]["annotations"]["customer.example/setting"], + "keep" + ); + } + let requests = server.received_requests().await.unwrap(); + assert!(requests.iter().all(|request| { + !(request.url.path().contains("/deployments/") + || request.method == "DELETE" && request.url.path() == NS_PATH) + })); +} + +#[tokio::test] +async fn reconciler_entry_retries_non404_and_cas_errors_without_losing_other_finalizers() { + for fault in [ + Fault::SourceGet(403), + Fault::NamespaceGet(500), + Fault::Patch(403), + Fault::Patch(500), + Fault::Patch(409), + ] { + let state = State::legacy(fault); + let observed = state.observed(); + let original_namespace = state.namespace.clone(); + let (server, client, state) = setup(state).await; + let ctx = context(client); + let error = reconcile(observed.clone(), ctx.clone()).await.unwrap_err(); + assert_ne!( + error_policy(observed.clone(), &error, ctx.clone()), + Action::await_change() + ); + assert!( + state.lock().unwrap().sandbox["metadata"]["finalizers"] + .as_array() + .unwrap() + .contains(&json!(FINALIZER)) + ); + assert_eq!( + reconcile(observed, ctx).await.unwrap(), + Action::await_change() + ); + let requests = server.received_requests().await.unwrap(); + only_finalizer_io(&requests); + let state = state.lock().unwrap(); + assert_eq!(state.namespace, original_namespace); + let expected = if matches!(fault, Fault::Patch(409)) { + json!([OTHER_FINALIZER, CONCURRENT_FINALIZER]) + } else { + json!([OTHER_FINALIZER]) + }; + assert_eq!(state.sandbox["metadata"]["finalizers"], expected); + } +} + +#[tokio::test] +async fn reconciler_entry_rejects_recreated_cr_before_read_and_between_read_and_patch() { + for during_patch in [false, true] { + let mut state = State::legacy(if during_patch { + Fault::RecreateOnPatch + } else { + Fault::None + }); + let observed = state.observed(); + if !during_patch { + state.sandbox["metadata"]["uid"] = json!("recreated-sandbox"); + } + let original_namespace = state.namespace.clone(); + let (server, client, state) = setup(state).await; + let ctx = context(client); + assert!(reconcile(observed.clone(), ctx.clone()).await.is_err()); + assert!(reconcile(observed, ctx).await.is_err()); + only_finalizer_io(&server.received_requests().await.unwrap()); + let state = state.lock().unwrap(); + assert_eq!(state.namespace, original_namespace); + assert_eq!(state.sandbox["metadata"]["uid"], "recreated-sandbox"); + assert_eq!( + state.sandbox["metadata"]["finalizers"], + json!([FINALIZER, OTHER_FINALIZER]) + ); + } +} + +#[tokio::test] +async fn reconciler_entry_rejects_foreign_claims_ownerrefs_and_namespace_uid_replacement() { + for variant in [ + "foreign-workspace", + "foreign-uid", + "partial-claim", + "owner-ref", + "namespace-uid", + "prestage-foreign-workspace", + "prestage-namespace-uid", + "prestage-missing-backlink", + ] { + let mut state = if variant.starts_with("prestage-") { + State::prestaged(Fault::None) + } else { + State::legacy(Fault::None) + }; + match variant { + "foreign-workspace" | "foreign-uid" => { + let sandbox = state.observed(); + let mut annotations = owner_annotations(&sandbox); + annotations[if variant == "foreign-workspace" { + SOURCE_NAMESPACE + } else { + SOURCE_UID + }] = json!("foreign"); + state.namespace["metadata"]["annotations"] = annotations; + } + "partial-claim" => state.namespace["metadata"]["annotations"][VERSION] = json!("v1"), + "prestage-foreign-workspace" => { + state.namespace["metadata"]["annotations"][SOURCE_NAMESPACE] = + json!("other-workspace"); + } + "prestage-missing-backlink" => { + state.sandbox["metadata"]["annotations"] + .as_object_mut() + .unwrap() + .remove(NAMESPACE_UID); + } + "owner-ref" => { + state.namespace["metadata"]["ownerReferences"] = json!([{ + "apiVersion": "v1", "kind": "Namespace", "name": "foreign", "uid": "foreign", + }]) + } + _ => { + state.sandbox["metadata"]["annotations"][NAMESPACE_UID] = + json!("previous-namespace") + } + } + let observed = state.observed(); + let original_source = state.sandbox.clone(); + let original_namespace = state.namespace.clone(); + let (server, client, state) = setup(state).await; + assert!( + reconcile(observed, context(client)).await.is_err(), + "{variant}" + ); + let requests = server.received_requests().await.unwrap(); + assert!( + requests.iter().all(|request| request.method == "GET"), + "{variant}" + ); + only_finalizer_io(&requests); + let state = state.lock().unwrap(); + assert_eq!(state.sandbox, original_source, "{variant}"); + assert_eq!(state.namespace, original_namespace, "{variant}"); + } +} + +#[tokio::test] +async fn namespace_gc_path_never_handles_foreign_namespace_active_or_already_claimed_objects() { + for variant in [ + "foreign-namespace", + "active-source", + "active-namespace", + "claimed", + ] { + let mut state = State::legacy(Fault::None); + match variant { + "foreign-namespace" => state.sandbox["metadata"]["namespace"] = json!("workspace-a"), + "active-source" => { + let _ = state.sandbox["metadata"] + .as_object_mut() + .unwrap() + .remove("deletionTimestamp"); + } + "active-namespace" => { + let _ = state.namespace["metadata"] + .as_object_mut() + .unwrap() + .remove("deletionTimestamp"); + } + _ => state.namespace["metadata"]["annotations"] = owner_annotations(&state.observed()), + } + let observed = state.observed(); + let original_source = state.sandbox.clone(); + let original_namespace = state.namespace.clone(); + let (server, client, state) = setup(state).await; + assert!( + !finalize_legacy_namespace_gc(&client, &observed) + .await + .unwrap(), + "{variant}" + ); + let requests = server.received_requests().await.unwrap(); + assert!( + requests.iter().all(|request| request.method == "GET"), + "{variant}" + ); + only_finalizer_io(&requests); + let state = state.lock().unwrap(); + assert_eq!(state.sandbox, original_source, "{variant}"); + assert_eq!(state.namespace, original_namespace, "{variant}"); + } +} + +#[tokio::test] +async fn reconciler_entry_does_not_rewrite_a_cr_whose_own_finalizer_is_already_gone() { + let mut state = State::legacy(Fault::None); + state.sandbox["metadata"]["finalizers"] = json!([OTHER_FINALIZER]); + let observed = state.observed(); + let original_source = state.sandbox.clone(); + let (server, client, state) = setup(state).await; + assert_eq!( + reconcile(observed, context(client)).await.unwrap(), + Action::await_change() + ); + let requests = server.received_requests().await.unwrap(); + assert!(requests.iter().all(|request| request.method == "GET")); + only_finalizer_io(&requests); + assert_eq!(state.lock().unwrap().sandbox, original_source); +} diff --git a/controller/src/reconciler/sre_writer.rs b/controller/src/reconciler/sre_writer.rs new file mode 100644 index 00000000..a67f37af --- /dev/null +++ b/controller/src/reconciler/sre_writer.rs @@ -0,0 +1,183 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Materialize the permissionless SRE writer identity only in a claimed runtime +//! namespace. Compatible Helm accounts are left byte-for-byte unchanged. + +use k8s_openapi::api::core::v1::{Namespace, ServiceAccount}; +use kube::{Api, Client, ResourceExt, api::PostParams}; +use serde_json::json; + +use super::namespace_ownership::{ + self, Error, NAMESPACE_UID, SOURCE_NAME, SOURCE_NAMESPACE, SOURCE_UID, +}; +use crate::crd::KarsSandbox; + +const NAME: &str = "sre-writer"; +const NAMESPACE: &str = "kars-sre"; +const MANAGED_BY: &str = "app.kubernetes.io/managed-by"; +const HELM_NAME: &str = "meta.helm.sh/release-name"; +const HELM_NAMESPACE: &str = "meta.helm.sh/release-namespace"; + +fn conflict() -> Error { + Error::Conflict("SRE writer ServiceAccount has conflicting data or unproven ownership".into()) +} + +fn desired(sandbox: &KarsSandbox, namespace: &Namespace) -> Result { + Ok(serde_json::from_value(json!({ + "apiVersion": "v1", + "kind": "ServiceAccount", + "metadata": { + "name": NAME, + "namespace": NAMESPACE, + "labels": { + "app.kubernetes.io/name": "kars", + "app.kubernetes.io/component": "sre", + MANAGED_BY: "kars-controller", + "kars.azure.com/role": NAME + }, + "annotations": { + "kars.azure.com/no-automount": "true", + SOURCE_NAME: sandbox.name_any(), + SOURCE_NAMESPACE: sandbox.namespace(), + SOURCE_UID: sandbox.metadata.uid, + NAMESPACE_UID: namespace.metadata.uid + }, + "ownerReferences": [{ + "apiVersion": "v1", "kind": "Namespace", "name": NAMESPACE, + "uid": namespace.metadata.uid, "controller": true, "blockOwnerDeletion": false + }] + }, + "automountServiceAccountToken": false + }))?) +} + +fn compatible( + account: &ServiceAccount, + sandbox: &KarsSandbox, + namespace: &Namespace, +) -> Result<(), Error> { + if account.name_any() != NAME + || account.namespace().as_deref() != Some(NAMESPACE) + || account.metadata.uid.as_deref().is_none_or(str::is_empty) + || account + .metadata + .resource_version + .as_deref() + .is_none_or(str::is_empty) + || account.metadata.deletion_timestamp.is_some() + || account.automount_service_account_token != Some(false) + || account + .secrets + .as_ref() + .is_some_and(|refs| !refs.is_empty()) + || account + .image_pull_secrets + .as_ref() + .is_some_and(|refs| !refs.is_empty()) + || account + .labels() + .get("kars.azure.com/role") + .map(String::as_str) + != Some(NAME) + || account + .annotations() + .get("kars.azure.com/no-automount") + .is_some_and(|value| value != "true") + { + return Err(conflict()); + } + let expected = desired(sandbox, namespace)?; + let annotations = account.annotations(); + for key in [SOURCE_NAME, SOURCE_NAMESPACE, SOURCE_UID, NAMESPACE_UID] { + if annotations + .get(key) + .is_some_and(|value| expected.annotations().get(key) != Some(value)) + { + return Err(conflict()); + } + } + let has_helm_owner = annotations.contains_key(HELM_NAME) + || annotations.contains_key(HELM_NAMESPACE) + || account + .labels() + .get(MANAGED_BY) + .is_some_and(|value| value == "Helm"); + if has_helm_owner { + // CLI-created SRE CRs may coexist with a retained Helm-owned namespace. + // Either explicit source ownership or that verified namespace must + // identify the exact release; never infer ownership from the SA alone. + let owner = if sandbox.annotations().contains_key(HELM_NAME) + || sandbox.annotations().contains_key(HELM_NAMESPACE) + { + sandbox.annotations() + } else { + namespace.annotations() + }; + let release = owner.get(HELM_NAME).filter(|name| !name.is_empty()); + if release.is_none() + || annotations.get(HELM_NAME) != release + || annotations.get(HELM_NAMESPACE).map(String::as_str) + != sandbox.metadata.namespace.as_deref() + || owner.get(HELM_NAMESPACE).map(String::as_str) + != sandbox.metadata.namespace.as_deref() + || account.labels().get(MANAGED_BY).map(String::as_str) != Some("Helm") + || account + .metadata + .owner_references + .as_ref() + .is_some_and(|refs| !refs.is_empty()) + { + return Err(conflict()); + } + } else if account.labels().get(MANAGED_BY).map(String::as_str) != Some("kars-controller") + || account.metadata.owner_references != expected.metadata.owner_references + || [SOURCE_NAME, SOURCE_NAMESPACE, SOURCE_UID, NAMESPACE_UID] + .iter() + .any(|key| annotations.get(*key) != expected.annotations().get(*key)) + { + return Err(conflict()); + } + Ok(()) +} + +pub async fn ensure( + client: &Client, + sandbox: &KarsSandbox, + accounts: &Api, + namespace: Option<&Namespace>, +) -> Result<(), Error> { + if sandbox.name_any() != "sre" + || sandbox + .labels() + .get("kars.azure.com/role") + .map(String::as_str) + != Some("sre") + { + return Ok(()); + } + if sandbox.metadata.deletion_timestamp.is_some() + || accounts.resource_url() != "/api/v1/namespaces/kars-sre/serviceaccounts" + { + return Err(conflict()); + } + let namespace = namespace.ok_or_else(conflict)?; + let namespace = namespace_ownership::recheck(client, sandbox, namespace).await?; + if let Some(existing) = accounts.get_opt(NAME).await? { + return compatible(&existing, sandbox, &namespace); + } + accounts + .create( + &PostParams { + field_manager: Some(crate::field_managers::CLAWSANDBOX.into()), + ..Default::default() + }, + &desired(sandbox, &namespace)?, + ) + .await?; + Ok(()) +} + +#[cfg(test)] +#[path = "sre_writer_tests.rs"] +mod tests; diff --git a/controller/src/reconciler/sre_writer_tests.rs b/controller/src/reconciler/sre_writer_tests.rs new file mode 100644 index 00000000..660817c0 --- /dev/null +++ b/controller/src/reconciler/sre_writer_tests.rs @@ -0,0 +1,464 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use super::*; +use crate::reconciler::namespace_ownership::VERSION; +use serde_json::Value; +use std::sync::{Arc, Mutex}; +use wiremock::{Mock, MockServer, Request, Respond, ResponseTemplate}; + +const NS_PATH: &str = "/api/v1/namespaces/kars-sre"; +const ACCOUNTS_PATH: &str = "/api/v1/namespaces/kars-sre/serviceaccounts"; +const WRITER_PATH: &str = "/api/v1/namespaces/kars-sre/serviceaccounts/sre-writer"; + +fn sandbox() -> KarsSandbox { + serde_json::from_value(json!({ + "apiVersion": "kars.azure.com/v1alpha1", "kind": "KarsSandbox", + "metadata": { + "name": "sre", "namespace": "workspace-a", "uid": "sandbox-sre", + "resourceVersion": "10", + "labels": {"kars.azure.com/role": "sre"}, + "annotations": { + HELM_NAME: "customer-release", HELM_NAMESPACE: "workspace-a", + NAMESPACE_UID: "namespace-sre" + } + }, + "spec": {"inferenceRef": {"name": "sre-inference"}} + })) + .unwrap() +} + +fn namespace(sandbox: &KarsSandbox) -> Namespace { + serde_json::from_value(json!({ + "apiVersion": "v1", "kind": "Namespace", + "metadata": { + "name": NAMESPACE, "uid": "namespace-sre", "resourceVersion": "20", + "annotations": { + VERSION: "v1", SOURCE_NAME: "sre", + SOURCE_NAMESPACE: sandbox.namespace(), SOURCE_UID: sandbox.metadata.uid + } + } + })) + .unwrap() +} + +fn helm_account() -> ServiceAccount { + serde_json::from_value(json!({ + "apiVersion": "v1", "kind": "ServiceAccount", + "metadata": { + "name": NAME, "namespace": NAMESPACE, "uid": "legacy-writer", "resourceVersion": "30", + "labels": { + "app.kubernetes.io/name": "kars", "app.kubernetes.io/component": "sre", + MANAGED_BY: "Helm", "kars.azure.com/role": NAME, "customer-label": "keep" + }, + "annotations": { + HELM_NAME: "customer-release", HELM_NAMESPACE: "workspace-a", + "kars.azure.com/no-automount": "true", "customer.example/note": "keep" + } + }, + "automountServiceAccountToken": false + })) + .unwrap() +} + +struct State { + namespace: Namespace, + account: Option, + get_error: u16, + create_error: u16, + race_account: Option, +} + +fn response(code: u16, body: Value) -> ResponseTemplate { + ResponseTemplate::new(code).set_body_json(body) +} + +fn failure(code: u16) -> ResponseTemplate { + response( + code, + json!({ + "apiVersion": "v1", "kind": "Status", "status": "Failure", + "reason": if code == 404 { "NotFound" } else { "Conflict" }, "code": code, + }), + ) +} + +#[derive(Clone)] +struct Server(Arc>); + +impl Respond for Server { + fn respond(&self, request: &Request) -> ResponseTemplate { + let mut state = self.0.lock().unwrap(); + match (request.method.as_str(), request.url.path()) { + ("GET", NS_PATH) => response(200, serde_json::to_value(&state.namespace).unwrap()), + ("GET", WRITER_PATH) => { + if state.get_error != 0 { + return failure(state.get_error); + } + state.account.as_ref().map_or_else( + || failure(404), + |account| response(200, serde_json::to_value(account).unwrap()), + ) + } + ("POST", ACCOUNTS_PATH) => { + if let Some(winner) = state.race_account.take() { + state.account = Some(winner); + return failure(409); + } + if state.create_error != 0 { + return failure(state.create_error); + } + if state.account.is_some() { + return failure(409); + } + let mut account: ServiceAccount = serde_json::from_slice(&request.body).unwrap(); + account.metadata.uid = Some("created-writer".into()); + account.metadata.resource_version = Some("31".into()); + let body = serde_json::to_value(&account).unwrap(); + state.account = Some(account); + response(201, body) + } + _ => failure(500), + } + } +} + +async fn setup( + sandbox: &KarsSandbox, + account: Option, +) -> (MockServer, Client, Api, Arc>) { + let server = MockServer::start().await; + let state = Arc::new(Mutex::new(State { + namespace: namespace(sandbox), + account, + get_error: 0, + create_error: 0, + race_account: None, + })); + Mock::given(wiremock::matchers::any()) + .respond_with(Server(state.clone())) + .mount(&server) + .await; + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + let client = Client::try_from(kube::Config::new(server.uri().parse().unwrap())).unwrap(); + let accounts = Api::namespaced(client.clone(), NAMESPACE); + (server, client, accounts, state) +} + +#[tokio::test] +async fn creates_writer_only_after_verified_namespace_claim_with_automount_disabled() { + let sandbox = sandbox(); + let ns = namespace(&sandbox); + let (server, client, accounts, state) = setup(&sandbox, None).await; + ensure(&client, &sandbox, &accounts, Some(&ns)) + .await + .unwrap(); + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.len(), 3); + assert_eq!( + (requests[0].method.as_str(), requests[0].url.path()), + ("GET", NS_PATH) + ); + assert_eq!( + (requests[1].method.as_str(), requests[1].url.path()), + ("GET", WRITER_PATH) + ); + assert_eq!( + (requests[2].method.as_str(), requests[2].url.path()), + ("POST", ACCOUNTS_PATH) + ); + let body: Value = serde_json::from_slice(&requests[2].body).unwrap(); + assert_eq!(body["automountServiceAccountToken"], false); + assert_eq!(body["metadata"]["name"], "sre-writer"); + assert_eq!(body["metadata"]["namespace"], "kars-sre"); + assert_eq!(body["metadata"]["annotations"][SOURCE_UID], "sandbox-sre"); + assert_eq!( + body["metadata"]["ownerReferences"][0]["uid"], + "namespace-sre" + ); + let original = state.lock().unwrap().account.clone(); + ensure(&client, &sandbox, &accounts, Some(&ns)) + .await + .unwrap(); + assert_eq!(state.lock().unwrap().account, original); + assert!( + server.received_requests().await.unwrap()[3..] + .iter() + .all(|r| r.method == "GET") + ); +} + +#[tokio::test] +async fn ordinary_sandboxes_and_noncanonical_sre_names_are_unchanged() { + for (name, role) in [("demo", "agent"), ("demo", "sre"), ("sre", "agent")] { + let mut sandbox = sandbox(); + sandbox.metadata.name = Some(name.into()); + sandbox + .metadata + .labels + .as_mut() + .unwrap() + .insert("kars.azure.com/role".into(), role.into()); + let (server, client, accounts, state) = setup(&sandbox, None).await; + ensure(&client, &sandbox, &accounts, None).await.unwrap(); + assert!(server.received_requests().await.unwrap().is_empty()); + assert!(state.lock().unwrap().account.is_none()); + } +} + +#[tokio::test] +async fn unverified_foreign_and_replaced_namespaces_never_read_or_create_writer_accounts() { + for variant in [ + "missing-proof", + "unclaimed", + "foreign", + "replacement", + "wrong-api", + ] { + let sandbox = sandbox(); + let ns = namespace(&sandbox); + let (server, client, accounts, state) = setup(&sandbox, None).await; + match variant { + "unclaimed" => state.lock().unwrap().namespace.metadata.annotations = None, + "foreign" => { + state + .lock() + .unwrap() + .namespace + .metadata + .annotations + .as_mut() + .unwrap() + .insert(SOURCE_UID.into(), "foreign-sandbox".into()); + } + "replacement" => { + state.lock().unwrap().namespace.metadata.uid = Some("replacement".into()) + } + _ => {} + } + let accounts = if variant == "wrong-api" { + Api::namespaced(client.clone(), "customer-namespace") + } else { + accounts + }; + assert!( + ensure( + &client, + &sandbox, + &accounts, + if variant == "missing-proof" { + None + } else { + Some(&ns) + }, + ) + .await + .is_err(), + "{variant}" + ); + assert!( + server + .received_requests() + .await + .unwrap() + .iter() + .all(|r| r.method == "GET" && r.url.path() == NS_PATH) + ); + assert!(state.lock().unwrap().account.is_none()); + } +} + +#[tokio::test] +async fn compatible_legacy_helm_account_keeps_its_uid_fields_and_annotations_without_writes() { + let sandbox = sandbox(); + let ns = namespace(&sandbox); + let original = helm_account(); + let (server, client, accounts, state) = setup(&sandbox, Some(original.clone())).await; + ensure(&client, &sandbox, &accounts, Some(&ns)) + .await + .unwrap(); + assert!( + server + .received_requests() + .await + .unwrap() + .iter() + .all(|r| r.method == "GET") + ); + assert_eq!(state.lock().unwrap().account, Some(original)); +} + +#[test] +fn cli_source_can_reuse_retained_namespace_release_but_not_unproven_helm_ownership() { + let mut sandbox = sandbox(); + sandbox + .metadata + .annotations + .as_mut() + .unwrap() + .remove(HELM_NAME); + sandbox + .metadata + .annotations + .as_mut() + .unwrap() + .remove(HELM_NAMESPACE); + let mut ns = namespace(&sandbox); + let account = helm_account(); + assert!(compatible(&account, &sandbox, &ns).is_err()); + ns.metadata + .annotations + .as_mut() + .unwrap() + .insert(HELM_NAME.into(), "customer-release".into()); + ns.metadata + .annotations + .as_mut() + .unwrap() + .insert(HELM_NAMESPACE.into(), "workspace-a".into()); + compatible(&account, &sandbox, &ns).unwrap(); + sandbox + .metadata + .annotations + .as_mut() + .unwrap() + .insert(HELM_NAME.into(), "other-release".into()); + assert!(compatible(&account, &sandbox, &ns).is_err()); +} + +#[test] +fn conflicting_account_data_and_foreign_ownership_are_rejected() { + let sandbox = sandbox(); + let ns = namespace(&sandbox); + for variant in [ + "automount-true", + "automount-default", + "foreign-release", + "foreign-release-namespace", + "foreign-owner", + "unknown-owner", + "foreign-binding", + "token-secret", + "pull-secret", + ] { + let mut account = serde_json::to_value(helm_account()).unwrap(); + match variant { + "automount-true" => account["automountServiceAccountToken"] = json!(true), + "automount-default" => account["automountServiceAccountToken"] = Value::Null, + "foreign-release" => { + account["metadata"]["annotations"][HELM_NAME] = json!("other-release") + } + "foreign-release-namespace" => { + account["metadata"]["annotations"][HELM_NAMESPACE] = json!("other-workspace") + } + "foreign-owner" => { + account["metadata"]["ownerReferences"] = json!([{ + "apiVersion": "v1", "kind": "Namespace", "name": NAMESPACE, "uid": "foreign", + }]) + } + "unknown-owner" => { + account["metadata"]["annotations"] = json!({}); + account["metadata"]["labels"][MANAGED_BY] = json!("customer"); + } + "foreign-binding" => { + account["metadata"]["annotations"][SOURCE_UID] = json!("previous-sandbox") + } + "token-secret" => account["secrets"] = json!([{"name": "old-token"}]), + _ => account["imagePullSecrets"] = json!([{"name": "customer-registry"}]), + } + let account: ServiceAccount = serde_json::from_value(account).unwrap(); + assert!(compatible(&account, &sandbox, &ns).is_err(), "{variant}"); + } +} + +#[tokio::test] +async fn incompatible_account_is_preserved_instead_of_force_overwritten() { + let sandbox = sandbox(); + let ns = namespace(&sandbox); + let mut original = helm_account(); + original.automount_service_account_token = Some(true); + let (server, client, accounts, state) = setup(&sandbox, Some(original.clone())).await; + assert!( + ensure(&client, &sandbox, &accounts, Some(&ns)) + .await + .is_err() + ); + assert!( + server + .received_requests() + .await + .unwrap() + .iter() + .all(|r| r.method == "GET") + ); + assert_eq!(state.lock().unwrap().account, Some(original)); +} + +#[tokio::test] +async fn api_failures_and_create_races_never_force_take_over_an_account() { + let sandbox = sandbox(); + let ns = namespace(&sandbox); + for (get_error, create_error) in [(403, 0), (500, 0), (0, 403), (0, 409), (0, 500)] { + let (server, client, accounts, state) = setup(&sandbox, None).await; + { + let mut state = state.lock().unwrap(); + state.get_error = get_error; + state.create_error = create_error; + } + assert!( + ensure(&client, &sandbox, &accounts, Some(&ns)) + .await + .is_err() + ); + assert!(state.lock().unwrap().account.is_none()); + assert!( + server + .received_requests() + .await + .unwrap() + .iter() + .all(|r| r.method != "PATCH" && r.method != "PUT" && r.method != "DELETE") + ); + } + let (server, client, accounts, state) = setup(&sandbox, None).await; + let mut foreign = helm_account(); + foreign + .metadata + .annotations + .as_mut() + .unwrap() + .insert(HELM_NAME.into(), "other-release".into()); + state.lock().unwrap().race_account = Some(foreign.clone()); + assert!( + ensure(&client, &sandbox, &accounts, Some(&ns)) + .await + .is_err() + ); + assert!( + ensure(&client, &sandbox, &accounts, Some(&ns)) + .await + .is_err() + ); + assert_eq!(state.lock().unwrap().account, Some(foreign)); + assert!( + server + .received_requests() + .await + .unwrap() + .iter() + .all(|r| r.method != "PATCH" && r.method != "PUT" && r.method != "DELETE") + ); +} + +#[test] +fn reconciler_wires_writer_after_namespace_ensure_and_primary_service_account() { + let source = include_str!("mod.rs"); + let writer = source.find("sre_writer::ensure(").unwrap(); + assert!(source.find("namespace_ownership::ensure(").unwrap() < writer); + assert!( + source[..writer] + .rfind(".patch(\n \"sandbox\",") + .is_some() + ); + assert!(source[..writer].ends_with("if is_sre_sandbox && name == \"sre\" {\n ")); +} diff --git a/deploy/helm/kars/templates/sre.yaml b/deploy/helm/kars/templates/sre.yaml index 96c0c6e7..6f87944d 100644 --- a/deploy/helm/kars/templates/sre.yaml +++ b/deploy/helm/kars/templates/sre.yaml @@ -16,7 +16,7 @@ What this template creates (when sre.enabled=true): - ClusterRole `kars-sre-reader` — kars-CR read scope (Slice 1) - ClusterRoleBinding `kars-sre-reader` — bound to the SA `sandbox` in namespace `kars-sre` (the controller-created default) - - ToolPolicy `sre-tools` (kars-sre) — gates the sre_* tool surface + - ToolPolicy `sre-tools` (Release.Namespace) — gates the sre_* tool surface Per design (docs/blueprints/07-kars-sre-proposal.md §7.8 — privilege containment): @@ -31,27 +31,43 @@ containment): the deregistration were bypassed, there's no network path to the relay */}} +{{- $legacyNamespace := dict }} +{{- $retainNamespace := false }} +{{- $legacyWriter := dict }} +{{- $retainWriter := false }} +{{- if .Release.IsUpgrade }} +{{- $legacyNamespace = lookup "v1" "Namespace" "" "kars-sre" }} +{{- if $legacyNamespace }} +{{- $annotations := default dict $legacyNamespace.metadata.annotations }} +{{- $retainNamespace = and + (eq (index $annotations "meta.helm.sh/release-name") .Release.Name) + (eq (index $annotations "meta.helm.sh/release-namespace") .Release.Namespace) }} +{{- end }} {{- if (.Values.sre | default dict).enabled }} +{{- $legacyWriter = lookup "v1" "ServiceAccount" "kars-sre" "sre-writer" }} +{{- if $legacyWriter }} +{{- $annotations := default dict $legacyWriter.metadata.annotations }} +{{- $retainWriter = and + (eq (index $annotations "meta.helm.sh/release-name") .Release.Name) + (eq (index $annotations "meta.helm.sh/release-namespace") .Release.Namespace) }} +{{- end }} +{{- end }} +{{- end }} +{{- if $retainNamespace }} --- -# kars-sre Namespace — created by the chart so the ToolPolicy below -# (which lives in this ns by design — see proposal §7.6 + the -# ToolPolicy "cross-namespace refs deliberately not supported" rule) -# has a namespace to land in BEFORE the controller has reconciled -# the KarsSandbox. -# -# The controller's own namespace reconcile path uses server-side -# apply with field manager `kars-controller`, so it will harmlessly -# co-own this namespace (adding its labels + annotations) once it -# reaches step 1 of reconcile/mod.rs. No conflict. +# Retain an earlier release's Namespace even when disabling SRE. Dropping it +# from an upgrade would bypass the Sandbox controller's guarded cleanup. +# Fresh installs let the controller create and claim the runtime namespace. apiVersion: v1 kind: Namespace metadata: name: kars-sre + annotations: + {{- toYaml (mergeOverwrite (deepCopy (default dict $legacyNamespace.metadata.annotations)) (dict "helm.sh/resource-policy" "keep")) | nindent 4 }} labels: - kars.azure.com/role: sre - app.kubernetes.io/name: kars - app.kubernetes.io/component: sre - app.kubernetes.io/managed-by: {{ .Release.Service }} + {{- toYaml (default dict $legacyNamespace.metadata.labels) | nindent 4 }} +{{- end }} +{{- if (.Values.sre | default dict).enabled }} --- # kars-sre InferencePolicy — the model the SRE agent uses for diagnosis. # Default model is configurable via .Values.sre.model; the policy applies @@ -397,8 +413,9 @@ subjects: # executes via that token, and tears down the binding. # # The pieces below provide: -# 1. SA `sre-writer` (kars-sre) — the identity the controller mints -# tokens for. No auto-mount; controller-only path. +# 1. Legacy chart-owned SA `sre-writer` (kars-sre), retained on upgrades. +# Fresh accounts are created by the Sandbox controller after claiming +# the namespace. No auto-mount; controller-only path. # 2. Two narrow writer ClusterRoles — one for `resourcequotas`, one # for the workload kinds the typed actions cover. The one-shot # ClusterRoleBinding the controller mints binds the RIGHT one @@ -408,24 +425,19 @@ subjects: # 4. ClusterRole `kars:sre-approver` — for human / group # bindings (operator-facing). Cluster admin binds it manually. # --------------------------------------------------------------------- +{{- if $retainWriter }} apiVersion: v1 kind: ServiceAccount metadata: name: sre-writer namespace: kars-sre labels: - app.kubernetes.io/name: kars - app.kubernetes.io/component: sre - app.kubernetes.io/managed-by: {{ .Release.Service }} - kars.azure.com/role: sre-writer + {{- toYaml (default dict $legacyWriter.metadata.labels) | nindent 4 }} annotations: - # No auto-mount. The controller mints tokens via TokenRequest - # (in a future hardening pass — Slice 3 today uses the - # controller's own SA for the action execution; the writer SA - # structure lands the §7.8.4 architecture). - kars.azure.com/no-automount: "true" + {{- toYaml (mergeOverwrite (deepCopy (default dict $legacyWriter.metadata.annotations)) (dict "kars.azure.com/no-automount" "true")) | nindent 4 }} automountServiceAccountToken: false --- +{{- end }} apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: diff --git a/deploy/helm/kars/values.yaml b/deploy/helm/kars/values.yaml index 94b29da8..08bf3e63 100644 --- a/deploy/helm/kars/values.yaml +++ b/deploy/helm/kars/values.yaml @@ -482,7 +482,7 @@ entraSidecar: # When enabled, deploy/helm/kars/templates/sre.yaml provisions: # - InferencePolicy `sre-inference` (Release.Namespace) # - KarsSandbox `sre` (Release.Namespace) -# - ToolPolicy `sre-tools` (kars-sre) +# - ToolPolicy `sre-tools` (Release.Namespace) # - ClusterRole `kars-sre-reader` (cluster-scope) # - ClusterRoleBinding `kars-sre-reader` (cluster-scope → kars-sre/sandbox SA) # diff --git a/docs/how-to/namespace-ownership.md b/docs/how-to/namespace-ownership.md index 53b068b9..e8eb1d23 100644 --- a/docs/how-to/namespace-ownership.md +++ b/docs/how-to/namespace-ownership.md @@ -20,8 +20,9 @@ Sandbox CRs, namespace metadata, and Deployments; it never reads Secrets. API, RBAC, malformed-response, and transport errors fail the check rather than appearing as an empty cluster. -`kars upgrade` (including `--dry-run`) and `kars up --upgrade` run the same check -before changing the controller. Direct Helm/GitOps upgrades must run it explicitly. +`kars upgrade` (including `--dry-run`), `kars up --upgrade`, and `kars sre install` +against an existing controller run the same check before changing the controller. +Direct Helm/GitOps upgrades must run it explicitly. No `up` provisioning or operator defaults change. A healthy controller Deployment alone does **not** prove that sandbox ownership migration will succeed. @@ -117,7 +118,8 @@ namespace claims or bypassing Kubernetes lifecycle controls. ## Credential prestaging compatibility -Updated `kars add` still creates credentials **before** the Sandbox CR/pod. For a +Updated `kars add` and local-Kubernetes `kars dev` still create credentials +**before** the Sandbox CR/pod. For a new namespace it writes an explicit v1 reservation with source namespace/name, no sandbox UID yet, and `kars.azure.com/namespace-prestage: bind-next-sandbox`. The CR CREATE includes @@ -140,6 +142,36 @@ This feature does not change `credentials update/remove`, Secret contents, EnvFrom behavior, or workspace/team credential merging. It is the namespace safety prerequisite for a separate future credential-source feature. +The existing handoff credential writer also waits for the created Sandbox's +exact workspace/UID claim and namespace backlink. It establishes a metadata-only +Secret anchor, rechecks ownership, and writes values with Secret +UID/resourceVersion preconditions. Namespace or Secret replacement cannot turn +that update into a blind write to a new target. Failed or incomplete claims do +not authorize credential copying; errors are surfaced without credential values. + +## Built-in SRE installation + +Fresh `kars sre install` and Helm installations with `sre.enabled=true` let the +controller create and claim `kars-sre`. The chart's InferencePolicy, ToolPolicy +and Sandbox already reside in the Helm release namespace; they do not require +precreating the runtime namespace. The controller creates the existing +`sre-writer` ServiceAccount only after the namespace is claimed, with token +automount disabled. The SRE role/binding targets are unchanged. + +On a real Helm upgrade, live lookup retains a Namespace and writer account owned +by that exact release. The Namespace also receives `helm.sh/resource-policy: +keep`, even when the upgrade disables SRE, so removing an old chart resource +cannot bypass controller cleanup. Customer metadata is preserved. Namespace +lookup errors stop rendering rather than silently dropping a possibly owned +resource. Other releases' resources are never imported. + +Client-only `helm template` cannot discover previous Helm ownership. Do not +replace a legacy Helm release with a pruning GitOps deployment using a fresh +render: first perform the live Helm migration and preserve the retained namespace +in the GitOps ownership/pruning policy. Existing template/apply installations +must likewise keep previously managed runtime namespaces out of pruning; run +preflight and resolve any ambiguous legacy claim before updating the controller. + ## Cleanup and rollback Deletion rechecks ownership and namespace UID, then sends UID/resourceVersion @@ -152,6 +184,16 @@ Non-404 failures retain the CR finalizer and retry; a stuck namespace is diagnosed, not force-finalized. Other CR finalizers are preserved. +A narrow legacy exception handles a Sandbox stored inside its own runtime +namespace when both are already terminating. Namespace GC may have removed the +Deployment before claim-v1 adoption. The controller then uses the live CR UID +and resourceVersion to release only its own CR cleanup finalizer and stops +reconciling; it does not adopt, write, or delete the unproven namespace or other +resources. Foreign/partial claims and recorded namespace-UID mismatches still +fail closed. This prevents a namespace-GC deadlock without inventing ownership. +Valid two-way v1 reservations are not legacy namespaces: cancellation during +binding follows the normal guarded cleanup path instead of the legacy shortcut. + Rolling back to a controller without claim-v1 support removes these protections: older binaries ignore the claim metadata. Avoid provisioning, deleting, or reusing sandbox names during such a rollback; resolve ownership and run preflight diff --git a/docs/security-audits/2026-09-07-sandbox-namespace-ownership.md b/docs/security-audits/2026-09-07-sandbox-namespace-ownership.md new file mode 100644 index 00000000..01d18d82 --- /dev/null +++ b/docs/security-audits/2026-09-07-sandbox-namespace-ownership.md @@ -0,0 +1,157 @@ +# Security Audit - Sandbox namespace ownership + +Date: 2026-09-07 +Review status: Implementation evidence prepared; independent review and genuine +author/reviewer sign-offs are pending. + +## Scope + +This bounded slice follows the inference foundation in Azure/kars#547. It adds +claim-v1 namespace ownership, guarded namespace cleanup, read-only migration +preflight, explicit administrator adoption, and compatible CLI credential +prestaging. The candidate integrates parent `45cfa009` at `0c28a49f`; subsequent +review repairs also cover local-development and built-in SRE creation paths, +plus the existing handoff credential writer in `inference-router/src/spawn/`. + +Affected surfaces are `controller/src/reconciler/namespace_ownership.rs`, +Sandbox reconciliation, egress-approval target access, router-token reads, +`cli/src/lib/namespace-ownership.ts`, the namespace/add/upgrade commands, and the +controller namespace RBAC rule. The CLI command changes are capability-gated +paths. The migration guide is `docs/how-to/namespace-ownership.md`. + +This slice does not implement generic credential sources, shared task budgets, +Bridge permissions, or new runtime execution. It does not rename namespaces, +broaden existing credential propagation, alter default images, or introduce Helm +ownership of customer Sandbox namespaces. No live cluster migration is claimed. + +## T1: New capability / attack surface? YES + +- Namespace claims bind the source workspace, Sandbox name and exact Sandbox + UID. A backlink binds the Sandbox to the exact Namespace UID. +- `kars namespace preflight` reads Sandbox, Namespace and Deployment metadata + across the cluster. It does not read Secrets or mutate resources. +- `kars namespace adopt` is a privileged, metadata-only administrator operation. + It requires explicitly reviewed Sandbox and Namespace UIDs, a unique live + Sandbox, and compare-and-swap writes. Foreign or partial claims, terminating + resources and stale identities are rejected, not overwritten. +- CLI credential prestaging in both `add` and local-Kubernetes `dev` uses a + two-way reservation and Namespace UID backlink. The controller consumes the + reservation for one Sandbox incarnation before touching workloads. + +Claim annotations are controller/namespace-administrator authority, not tenant +assertions. Kubernetes authorization remains the boundary against changing that +authority. This design does not protect against a cluster administrator rewriting +claims or bypassing lifecycle controls. + +## T2: Security-control change? YES + +- Namespace creation is atomic. Existing resources are not adopted through + force apply merely because their names match. +- Same-name Sandboxes in different source workspaces cannot claim or clean up + each other's runtime namespaces. +- Cleanup rechecks the live claim and uses Namespace UID/resourceVersion + preconditions. Non-404 failures retain the Sandbox cleanup finalizer for retry; + unrelated finalizers are preserved. +- Egress-approval target writes/cleanup and router admin-token reads use the + shared read-only ownership guard. Policy confirmation retains the originating + workspace instead of treating the bare Sandbox name as authority. +- Legacy adoption requires unique live identity, existing status/finalizer, + controller-managed namespace/deployment evidence, matching selectors, and a + Deployment created strictly after the Sandbox incarnation. Names, labels or + status alone are not sufficient. +- Adoption preserves Namespace UID, data and pod templates. It does not copy + another Sandbox incarnation's credentials or trigger a workload rollout. +- Existing handoff credential propagation uses the created Sandbox's actual + workspace and UID, waits for a matching claim/backlink, and rejects changed or + terminating targets. A metadata-only Secret anchor is established before a + fresh ownership check; credential values are written with Secret + UID/resourceVersion fencing rather than blind apply. Metadata-only responses + and sanitized errors avoid exposing credential values in logs. Existing + credential sources and RBAC are unchanged. + +## T3: Availability / fail-open risk? MIXED, migration-gated + +Controller claim conflicts leave target resources untouched and report +`NamespaceOwnershipConflict`. They are not treated as a successful empty +namespace. API/RBAC/transport/malformed-response failures remain errors. + +`kars upgrade`, `kars up --upgrade`, and SRE installation against an existing +controller run the read-only preflight before controller replacement. +Direct Helm/GitOps upgrades must run it explicitly. +Ambiguous legacy installations require an administrator decision before upgrade: +same-second creation timestamps, older field-manager formats, overlay-only +deployments and unfinished unmarked prestaging are not automatically trusted. +Existing running pods are not stopped by a rejected adoption, but successful +controller reconciliation must not be claimed until ownership is resolved. + +Pre-CR namespaces cannot be inventoried through Sandbox CRs alone. Operators must +inventory unfinished reservations separately and update provisioning clients to +claim-v1 before further credential prestaging. The migration guide records this +limitation rather than silently broadening Secret permissions. + +The cleanup finalizer waits for accepted namespace deletion, not namespace +disappearance, avoiding a deadlock when the Sandbox CR resides inside its own +runtime namespace. Terminating namespaces retain their claims. Rollback to older +controllers removes these guards because older binaries ignore claim-v1; +provisioning/deletion/name reuse must be paused during such a rollback. + +## Verification + +The candidate integrates parent `45cfa009` at `0c28a49f`. The complete local +repair batch has the following evidence: + +- 42 focused controller tests passed, covering namespace ownership, SRE writer + materialization and actual reconciler-entry finalization races. +- 32 spawn tests passed, including 11 credential-claim regressions. They cover + normal propagation and zero credential-value writes for foreign workspaces, + recreated CR/Namespace/Secret UIDs, missing claims/backlinks, terminating + targets and API failures. +- 161 combined CLI/Helm/dev tests passed, including first-party creation, + preflight, true absence versus failed discovery, and Helm-compatible dotted + release names with the 53-character limit. +- Strict all-target Clippy passed for both crates; CLI typecheck/scoped lint, + formatting and existing LOC/crypto/no-stub gates passed. + +Independent automated review found three blockers in `e5870631`: first-party +local-development/SRE namespace prestagers, the existing handoff credential +writer bypassing claims, and namespace GC deadlocking an unclaimed self-hosted +legacy Sandbox. Bounded follow-ups identified SRE failed-discovery handling, +dotted Helm names, and cancellation during v1 reservation binding. The original +legacy-GC repair, SRE writer companion and dotted-release repair have automated +closure; final closure of the reservation-cancellation follow-up and handoff +credential writer is pending. Automated technical review is not human approval. + +Actual Helm rendering against an isolated read-only API fixture covers +legacy Namespace/account retention, disabled SRE, foreign releases, missing +resources and lookup errors. Fresh SRE namespaces are left to the controller; +old exact-release Namespace manifests remain present with retention metadata. +Client-only rendering cannot discover prior ownership: the migration guide +requires retaining legacy namespaces when moving to pruning GitOps. + +The SRE discovery repair validates Helm's JSON inventory and controller +identity, treats only a successful empty `--ignore-not-found` response as +absence, bounds reads to 30 seconds, and propagates failures without installing. +Existing compatible Helm writer accounts are left unchanged; conflicting data +or ownership is not force-overwritten. + +The GC regression models namespace deletion beginning after CR finalizer +creation but before the namespace claim patch. The first patch conflicts; the +next real reconciler entry recognizes the valid two-way prestage, completes +normal guarded cleanup and preserves other finalizers. Foreign/partial +reservations and UID mismatches remain rejected. The legacy-only shortcut still +releases just its own CR finalizer without adopting an unproven namespace. + +The existing disposable Kind harness now includes SRE installation, exact +Sandbox/Namespace UID binding, writer-account materialization with automount +disabled, and controller cleanup preserving the core namespace. Shell syntax +has been checked; execution of that new lifecycle gate awaits hosted CI. + +Local TypeScript checks use the existing authorized dependency cache, not a +lockfile-exact installation; hosted CI must supply exact-lockfile evidence. +No live customer or dedicated test-cluster upgrade was performed. + +## Verdict + +Pending independent review and migration-compatibility closure. This document is +not an approval or a sign-off. Genuine author and independent reviewer sign-offs +must be supplied before the capability-audit gate can pass. diff --git a/inference-router/src/spawn/credentials.rs b/inference-router/src/spawn/credentials.rs new file mode 100644 index 00000000..3dd9aba5 --- /dev/null +++ b/inference-router/src/spawn/credentials.rs @@ -0,0 +1,346 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Existing handoff credential propagation, authorized only by namespace claim +//! v1. Keep these keys aligned with controller/reconciler/namespace_ownership.rs +//! and cli/src/lib/namespace-ownership.ts. Labels/prestages never grant access. + +use k8s_openapi::{ + api::core::v1::{Namespace, Secret}, + apimachinery::pkg::apis::meta::v1::ObjectMeta, +}; +use kube::{ + Api, Client, + api::{DynamicObject, Patch, PatchParams}, +}; +use serde_json::json; +use std::{collections::BTreeMap, time::Duration}; + +const VERSION: &str = "kars.azure.com/namespace-claim-version"; +const SOURCE_NAMESPACE: &str = "kars.azure.com/sandbox-namespace"; +const SOURCE_NAME: &str = "kars.azure.com/sandbox-name"; +const SOURCE_UID: &str = "kars.azure.com/sandbox-uid"; +const NAMESPACE_UID: &str = "kars.azure.com/namespace-uid"; +const PRESTAGE: &str = "kars.azure.com/namespace-prestage"; + +const CREDENTIAL_ENV_VARS: &[&str] = &[ + "TELEGRAM_BOT_TOKEN", + "TELEGRAM_ALLOW_FROM", + "SLACK_BOT_TOKEN", + "DISCORD_BOT_TOKEN", + "WHATSAPP_ENABLED", + "BRAVE_API_KEY", + "TAVILY_API_KEY", + "EXA_API_KEY", + "FIRECRAWL_API_KEY", + "PERPLEXITY_API_KEY", +]; + +pub(super) struct Target { + workspace: String, + name: String, + sandbox_uid: String, + namespace_uid: Option, +} + +impl Target { + pub(super) fn from_created( + created: &DynamicObject, + workspace: &str, + name: &str, + ) -> Result { + let meta = &created.metadata; + let actual_workspace = meta + .namespace + .as_deref() + .ok_or("Created Sandbox has no workspace")?; + let actual_name = meta.name.as_deref().ok_or("Created Sandbox has no name")?; + if actual_workspace != workspace + || actual_name != name + || !valid_label(workspace) + || !valid_label(name) + || meta.deletion_timestamp.is_some() + { + return Err( + "Created Sandbox has an invalid or changed workspace/name/lifecycle".into(), + ); + } + let sandbox_uid = meta + .uid + .as_ref() + .filter(|uid| !uid.is_empty()) + .ok_or("Created Sandbox has no API-server UID")? + .clone(); + let namespace_uid = annotation(meta, NAMESPACE_UID).map(str::to_string); + if namespace_uid.as_deref() == Some("") { + return Err("Created Sandbox has an empty namespace UID backlink".into()); + } + Ok(Self { + workspace: actual_workspace.into(), + name: actual_name.into(), + sandbox_uid, + namespace_uid, + }) + } + + fn namespace(&self) -> String { + format!("kars-{}", self.name) + } +} + +fn valid_label(value: &str) -> bool { + !value.is_empty() + && value.len() <= 63 + && value + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-') + && value.as_bytes()[0].is_ascii_alphanumeric() + && value.as_bytes()[value.len() - 1].is_ascii_alphanumeric() +} + +fn annotation<'a>(meta: &'a ObjectMeta, name: &str) -> Option<&'a str> { + meta.annotations.as_ref()?.get(name).map(String::as_str) +} + +fn identity(meta: &ObjectMeta) -> Result<(&str, &str), String> { + match (meta.uid.as_deref(), meta.resource_version.as_deref()) { + (Some(uid), Some(version)) if !uid.is_empty() && !version.is_empty() => Ok((uid, version)), + _ => Err("Ownership evidence omitted its API-server UID/resourceVersion".into()), + } +} + +fn api_error(stage: &str, error: kube::Error) -> String { + // Admission errors may echo stringData. Never log API bodies or values. + match error { + kube::Error::Api(status) => format!("{stage}: Kubernetes API status {}", status.code), + _ => format!("{stage}: Kubernetes client/transport failure"), + } +} + +fn validate_sandbox(target: &Target, live: &ObjectMeta) -> Result<(), String> { + identity(live)?; + if live.namespace.as_deref() != Some(target.workspace.as_str()) + || live.name.as_deref() != Some(target.name.as_str()) + || live.uid.as_deref() != Some(target.sandbox_uid.as_str()) + || live.deletion_timestamp.is_some() + { + return Err("Created Sandbox was replaced, moved, or is terminating".into()); + } + if let Some(expected) = target.namespace_uid.as_deref() + && annotation(live, NAMESPACE_UID) != Some(expected) + { + return Err("Created Sandbox namespace UID backlink changed".into()); + } + Ok(()) +} + +fn claim_ready( + target: &Target, + sandbox: &ObjectMeta, + namespace: &Namespace, + pinned_namespace_uid: &mut Option, +) -> Result { + let meta = &namespace.metadata; + let (uid, _) = identity(meta)?; + if meta.name.as_deref() != Some(target.namespace().as_str()) + || meta.deletion_timestamp.is_some() + || namespace + .status + .as_ref() + .and_then(|status| status.phase.as_deref()) + == Some("Terminating") + || meta + .owner_references + .as_ref() + .is_some_and(|owners| !owners.is_empty()) + { + return Err("Target namespace name/ownership/lifecycle is invalid".into()); + } + if pinned_namespace_uid + .as_deref() + .is_some_and(|expected| expected != uid) + { + return Err("Target namespace was recreated during credential propagation".into()); + } + *pinned_namespace_uid = Some(uid.to_string()); + let mut complete = true; + for (key, expected) in [ + (VERSION, "v1"), + (SOURCE_NAMESPACE, target.workspace.as_str()), + (SOURCE_NAME, target.name.as_str()), + (SOURCE_UID, target.sandbox_uid.as_str()), + ] { + match annotation(meta, key) { + Some(value) if value == expected => {} + Some(_) => { + return Err( + "Target namespace claim belongs to another Sandbox UID/workspace".into(), + ); + } + None => complete = false, + } + } + if let Some(prestage) = annotation(meta, PRESTAGE) { + if prestage != "bind-next-sandbox" { + return Err("Target namespace has an invalid prestage marker".into()); + } + complete = false; + } + match annotation(sandbox, NAMESPACE_UID) { + Some(backlink) if backlink == uid => {} + Some(_) => { + return Err("Sandbox namespace UID backlink does not match the live namespace".into()); + } + None => complete = false, + } + Ok(complete) +} + +async fn probe( + client: &Client, + target: &Target, + namespace_uid: &mut Option, +) -> Result { + let sandboxes: Api = Api::namespaced_with( + client.clone(), + &target.workspace, + &super::kars_sandbox_api_resource(), + ); + let live = sandboxes + .get_metadata(&target.name) + .await + .map_err(|error| api_error("Read created Sandbox", error))?; + validate_sandbox(target, &live.metadata)?; + let namespaces: Api = Api::all(client.clone()); + match namespaces + .get_opt(&target.namespace()) + .await + .map_err(|error| api_error("Read target namespace", error))? + { + Some(namespace) => claim_ready(target, &live.metadata, &namespace, namespace_uid), + None if namespace_uid.is_none() && annotation(&live.metadata, NAMESPACE_UID).is_none() => { + Ok(false) + } + None => Err("Previously observed/bound target namespace is missing".into()), + } +} + +#[derive(Clone, Copy)] +struct Wait { + attempts: usize, + delay: Duration, +} + +async fn write_credentials( + client: &Client, + target: &Target, + credentials: &BTreeMap, + predecessor: &str, + wait: Wait, +) -> Result<(), String> { + if credentials.is_empty() { + return Ok(()); + } + let mut namespace_uid = target.namespace_uid.clone(); + let mut ready = false; + for attempt in 0..wait.attempts { + if probe(client, target, &mut namespace_uid).await? { + ready = true; + break; + } + if attempt + 1 < wait.attempts { + tokio::time::sleep(wait.delay).await; + } + } + if !ready { + return Err( + "Namespace claim/backlink did not converge; no credentials were written".into(), + ); + } + let namespace = target.namespace(); + let secret_name = format!("{}-credentials", target.name); + let secrets: Api = Api::namespaced(client.clone(), &namespace); + + // Reserve only metadata, with a different manager from the historical + // credential writer so SSA cannot prune its existing data. No values cross + // the namespace boundary until its incarnation is rechecked below. + let anchor = secrets + .patch_metadata( + &secret_name, + &PatchParams::apply("kars-handoff-credential-anchor"), + &Patch::Apply(json!({ + "apiVersion": "v1", "kind": "Secret", + "metadata": { "name": secret_name, "namespace": namespace }, + })), + ) + .await + .map_err(|error| api_error("Reserve credential metadata", error))?; + let (secret_uid, secret_version) = identity(&anchor.metadata)?; + if anchor.metadata.name.as_deref() != Some(secret_name.as_str()) + || anchor.metadata.namespace.as_deref() != Some(namespace.as_str()) + || anchor.metadata.deletion_timestamp.is_some() + { + return Err("Credential metadata target is invalid or terminating".into()); + } + if !probe(client, target, &mut namespace_uid).await? { + return Err("Namespace claim/backlink disappeared before the credential write".into()); + } + + // A merge patch cannot create an absent Secret. UID/resourceVersion fence + // the existing object, so deleting/recreating the namespace or Secret after + // revalidation cannot redirect credential values into a new incarnation. + // Metadata-only responses prevent reading existing credential values. + secrets + .patch_metadata( + &secret_name, + &PatchParams::default(), + &Patch::Merge(json!({ + "metadata": { + "uid": secret_uid, "resourceVersion": secret_version, + "labels": { + "kars.azure.com/managed-by": "handoff", + "kars.azure.com/predecessor": predecessor, + }, + }, + "type": "Opaque", "stringData": credentials, + })), + ) + .await + .map_err(|error| api_error("Write handoff credentials", error))?; + Ok(()) +} + +pub(super) async fn propagate(client: &Client, target: &Target) -> Result<(), String> { + let credentials: BTreeMap = CREDENTIAL_ENV_VARS + .iter() + .filter_map(|name| { + std::env::var(name) + .ok() + .filter(|value| !value.is_empty()) + .map(|value| ((*name).into(), value)) + }) + .collect(); + if credentials.is_empty() { + tracing::info!(child = %target.name, "No channel/plugin credentials to propagate"); + return Ok(()); + } + write_credentials( + client, + target, + &credentials, + &std::env::var("SANDBOX_NAME").unwrap_or_default(), + Wait { + attempts: 15, + delay: Duration::from_secs(2), + }, + ) + .await?; + tracing::info!(child = %target.name, workspace = %target.workspace, + credential_count = credentials.len(), "Propagated credentials to the claimed handoff namespace"); + Ok(()) +} + +#[cfg(test)] +mod test_server; +#[cfg(test)] +mod tests; diff --git a/inference-router/src/spawn/credentials/test_server.rs b/inference-router/src/spawn/credentials/test_server.rs new file mode 100644 index 00000000..ec47d256 --- /dev/null +++ b/inference-router/src/spawn/credentials/test_server.rs @@ -0,0 +1,299 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use super::*; +use serde_json::Value; +use std::sync::{Arc, Mutex}; +use wiremock::{Mock, MockServer, Request, ResponseTemplate}; + +pub(super) const NS_PATH: &str = "/api/v1/namespaces/kars-demo"; +pub(super) const SECRET_PATH: &str = "/api/v1/namespaces/kars-demo/secrets/demo-credentials"; +pub(super) const SENTINEL: &str = "sensitive-handoff-value"; + +#[derive(Clone, Copy, Default)] +pub(super) enum Scenario { + #[default] + Stable, + ConvergeMissing, + ConvergePartial, + ReplaceNamespaceWhileWaiting, + ReplaceSandboxWhileWaiting, + ReplaceNamespaceAtAnchor, + ReplaceSandboxAtAnchor, + TerminateNamespaceAtAnchor, + TerminateSandboxAtAnchor, + RemoveClaimAtAnchor, + ReplaceSecretAtWrite, + ReplaceNamespaceAtWrite, + ConflictAtWrite, + SandboxError(u16), + NamespaceError(u16), + AnchorError(u16), + WriteError(u16), +} + +pub(super) struct State { + pub sandbox: Option, + pub namespace: Option, + pub secret: Option, + pub scenario: Scenario, + pub requests: Vec, + pub namespace_reads: usize, + pub value_attempts: usize, + pub value_writes: usize, +} + +pub(super) fn sandbox() -> Value { + json!({ + "apiVersion":"kars.azure.com/v1alpha1", "kind":"KarsSandbox", + "metadata": { + "name":"demo", "namespace":"workspace-a", "uid":"created-uid", "resourceVersion":"1", + "annotations": { NAMESPACE_UID: "namespace-uid" }, + }, + "spec": {}, + }) +} + +pub(super) fn namespace() -> Value { + json!({ + "apiVersion":"v1", "kind":"Namespace", + "metadata": { + "name":"kars-demo", "uid":"namespace-uid", "resourceVersion":"1", + "annotations": { + VERSION:"v1", SOURCE_NAMESPACE:"workspace-a", SOURCE_NAME:"demo", SOURCE_UID:"created-uid", + }, + }, + "status": { "phase":"Active" }, + }) +} + +fn secret() -> Value { + json!({ + "apiVersion":"v1", "kind":"Secret", "type":"Opaque", + "metadata": { + "name":"demo-credentials", "namespace":"kars-demo", "uid":"secret-uid", "resourceVersion":"1", + "labels": {"existing":"preserve"}, + }, + "data": { "EXISTING":"preserve" }, + }) +} + +impl Default for State { + fn default() -> Self { + Self { + sandbox: Some(sandbox()), + namespace: Some(namespace()), + secret: None, + scenario: Scenario::Stable, + requests: Vec::new(), + namespace_reads: 0, + value_attempts: 0, + value_writes: 0, + } + } +} + +fn api_error(code: u16) -> ResponseTemplate { + let reason = match code { + 403 => "Forbidden", + 404 => "NotFound", + 409 => "Conflict", + 422 => "Invalid", + 500 => "InternalError", + 503 => "ServiceUnavailable", + _ => "Failure", + }; + ResponseTemplate::new(code).set_body_json(json!({ + "apiVersion":"v1", "kind":"Status", "status":"Failure", "code":code, + "reason":reason, "message":SENTINEL, + })) +} + +fn metadata_response(object: &Value) -> ResponseTemplate { + ResponseTemplate::new(200).set_body_json(json!({ + "apiVersion":"meta.k8s.io/v1", "kind":"PartialObjectMetadata", "metadata":object["metadata"], + })) +} + +fn replace_namespace(state: &mut State) { + state.namespace = Some(namespace()); + state.namespace.as_mut().unwrap()["metadata"]["uid"] = "replacement-namespace".into(); + if let Some(sandbox) = &mut state.sandbox { + sandbox["metadata"]["annotations"][NAMESPACE_UID] = "replacement-namespace".into(); + } + state.secret = None; +} + +impl State { + pub fn with_existing_secret(mut self) -> Self { + self.secret = Some(secret()); + self + } + + fn respond(&mut self, request: &Request) -> ResponseTemplate { + self.requests.push(request.clone()); + let path = request.url.path(); + if request.method == "GET" && path.starts_with("/apis/kars.azure.com/") { + if let Scenario::SandboxError(code) = self.scenario { + return api_error(code); + } + let Some(sandbox) = &self.sandbox else { + return api_error(404); + }; + let workspace = sandbox["metadata"]["namespace"].as_str().unwrap(); + if path + != format!( + "/apis/kars.azure.com/v1alpha1/namespaces/{workspace}/karssandboxes/demo" + ) + { + return api_error(404); + } + assert!( + request.headers["accept"] + .to_str() + .unwrap() + .contains("PartialObjectMetadata") + ); + return metadata_response(sandbox); + } + if request.method == "GET" && path == NS_PATH { + self.namespace_reads += 1; + if let Scenario::NamespaceError(code) = self.scenario { + return api_error(code); + } + let reply = self.namespace.as_ref().map_or_else( + || api_error(404), + |namespace| ResponseTemplate::new(200).set_body_json(namespace), + ); + if self.namespace_reads == 1 { + match self.scenario { + Scenario::ConvergeMissing | Scenario::ConvergePartial => { + self.namespace = Some(namespace()); + self.sandbox = Some(sandbox()); + } + Scenario::ReplaceNamespaceWhileWaiting => replace_namespace(self), + Scenario::ReplaceSandboxWhileWaiting => { + self.sandbox = Some(sandbox()); + self.sandbox.as_mut().unwrap()["metadata"]["uid"] = + "recreated-sandbox".into(); + self.namespace = Some(namespace()); + self.namespace.as_mut().unwrap()["metadata"]["annotations"][SOURCE_UID] = + "recreated-sandbox".into(); + } + _ => {} + } + } + return reply; + } + if request.method != "PATCH" || path != SECRET_PATH { + return api_error(404); + } + assert!( + request.headers["accept"] + .to_str() + .unwrap() + .contains("PartialObjectMetadata") + ); + assert!( + !request + .url + .query() + .unwrap_or_default() + .contains("force=true") + ); + let body: Value = request.body_json().unwrap(); + if body.get("stringData").is_none() { + assert!(body.get("data").is_none()); + assert!( + request + .url + .query() + .unwrap_or_default() + .contains("fieldManager=kars-handoff-credential-anchor") + ); + if let Scenario::AnchorError(code) = self.scenario { + return api_error(code); + } + match self.scenario { + Scenario::ReplaceNamespaceAtAnchor => replace_namespace(self), + Scenario::ReplaceSandboxAtAnchor => { + self.sandbox.as_mut().unwrap()["metadata"]["uid"] = "recreated-sandbox".into(); + } + Scenario::TerminateNamespaceAtAnchor => { + self.namespace.as_mut().unwrap()["metadata"]["deletionTimestamp"] = + "2026-09-07T00:00:00Z".into(); + } + Scenario::TerminateSandboxAtAnchor => { + self.sandbox.as_mut().unwrap()["metadata"]["deletionTimestamp"] = + "2026-09-07T00:00:00Z".into(); + } + Scenario::RemoveClaimAtAnchor => { + self.namespace.as_mut().unwrap()["metadata"]["annotations"] = json!({}); + } + _ => {} + } + let secret = self.secret.get_or_insert_with(|| { + let mut created = secret(); + created["data"] = json!({}); + created["metadata"]["labels"] = json!({}); + created + }); + let version = secret["metadata"]["resourceVersion"] + .as_str() + .unwrap() + .parse::() + .unwrap() + + 1; + secret["metadata"]["resourceVersion"] = version.to_string().into(); + return metadata_response(secret); + } + self.value_attempts += 1; + assert_eq!( + request.headers["content-type"], + "application/merge-patch+json" + ); + assert!(body["metadata"]["uid"].is_string()); + assert!(body["metadata"]["resourceVersion"].is_string()); + match self.scenario { + Scenario::WriteError(code) => return api_error(code), + Scenario::ReplaceNamespaceAtWrite => replace_namespace(self), + Scenario::ReplaceSecretAtWrite => { + self.secret = Some(secret()); + self.secret.as_mut().unwrap()["metadata"]["uid"] = "replacement-secret".into(); + } + Scenario::ConflictAtWrite => { + self.secret.as_mut().unwrap()["metadata"]["resourceVersion"] = "changed".into(); + } + _ => {} + } + let Some(secret) = &mut self.secret else { + return api_error(404); + }; + if secret["metadata"]["uid"] != body["metadata"]["uid"] + || secret["metadata"]["resourceVersion"] != body["metadata"]["resourceVersion"] + { + return api_error(409); + } + for (key, value) in body["stringData"].as_object().unwrap() { + secret["data"][key] = value.clone(); + } + for (key, value) in body["metadata"]["labels"].as_object().unwrap() { + secret["metadata"]["labels"][key] = value.clone(); + } + self.value_writes += 1; + metadata_response(secret) + } +} + +pub(super) async fn start(state: State) -> (MockServer, Client, Arc>) { + let server = MockServer::start().await; + let state = Arc::new(Mutex::new(state)); + let handler = state.clone(); + Mock::given(|_: &Request| true) + .respond_with(move |request: &Request| handler.lock().unwrap().respond(request)) + .mount(&server) + .await; + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + let client = Client::try_from(kube::Config::new(server.uri().parse().unwrap())).unwrap(); + (server, client, state) +} diff --git a/inference-router/src/spawn/credentials/tests.rs b/inference-router/src/spawn/credentials/tests.rs new file mode 100644 index 00000000..3676ae31 --- /dev/null +++ b/inference-router/src/spawn/credentials/tests.rs @@ -0,0 +1,387 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use super::test_server::{NS_PATH, SECRET_PATH, SENTINEL, Scenario, State, sandbox, start}; +use super::*; + +fn target() -> Target { + let mut created = sandbox(); + created["metadata"]["annotations"] = json!({}); + Target::from_created( + &serde_json::from_value(created).unwrap(), + "workspace-a", + "demo", + ) + .unwrap() +} + +fn values() -> BTreeMap { + BTreeMap::from([("TELEGRAM_BOT_TOKEN".into(), SENTINEL.into())]) +} + +async fn write(client: &Client, target: &Target) -> Result<(), String> { + write_credentials( + client, + target, + &values(), + "parent", + Wait { + attempts: 3, + delay: Duration::ZERO, + }, + ) + .await +} + +#[test] +fn target_uses_the_created_response_identity_not_a_later_same_name_lookup() { + let created: DynamicObject = serde_json::from_value(sandbox()).unwrap(); + let target = Target::from_created(&created, "workspace-a", "demo").unwrap(); + assert_eq!(target.workspace, "workspace-a"); + assert_eq!(target.sandbox_uid, "created-uid"); + assert_eq!(target.namespace_uid.as_deref(), Some("namespace-uid")); + assert!(Target::from_created(&created, "workspace-b", "demo").is_err()); + assert!(Target::from_created(&created, "workspace-a", "another").is_err()); + let mut missing = created.clone(); + missing.metadata.uid = None; + assert!(Target::from_created(&missing, "workspace-a", "demo").is_err()); + missing = created; + missing.metadata.namespace = None; + assert!(Target::from_created(&missing, "workspace-a", "demo").is_err()); +} + +#[tokio::test] +async fn matching_claim_propagates_with_metadata_only_reads_and_a_uid_version_fence() { + for existing in [false, true] { + let initial = if existing { + State::default().with_existing_secret() + } else { + State::default() + }; + let (_server, client, state) = start(initial).await; + write(&client, &target()).await.unwrap(); + let state = state.lock().unwrap(); + assert_eq!(state.value_writes, 1); + assert_eq!(state.value_attempts, 1); + let secret = state.secret.as_ref().unwrap(); + assert_eq!(secret["data"]["TELEGRAM_BOT_TOKEN"], SENTINEL); + if existing { + assert_eq!(secret["metadata"]["labels"]["existing"], "preserve"); + assert_eq!(secret["data"]["EXISTING"], "preserve"); + } + assert_eq!( + secret["metadata"]["labels"]["kars.azure.com/predecessor"], + "parent" + ); + let patches: Vec<_> = state + .requests + .iter() + .filter(|request| request.method == "PATCH") + .collect(); + assert_eq!(patches.len(), 2); + let anchor: serde_json::Value = patches[0].body_json().unwrap(); + assert!(anchor.get("data").is_none() && anchor.get("stringData").is_none()); + let update: serde_json::Value = patches[1].body_json().unwrap(); + assert_eq!(update["metadata"]["uid"], "secret-uid"); + assert_eq!(update["metadata"]["resourceVersion"], "2"); + assert!( + state + .requests + .iter() + .all(|request| { request.url.path() != SECRET_PATH || request.method == "PATCH" }) + ); + } +} + +#[tokio::test] +async fn same_named_sandbox_in_another_workspace_cannot_write_the_claimed_namespace() { + let mut initial = State::default(); + initial.sandbox.as_mut().unwrap()["metadata"]["namespace"] = "workspace-b".into(); + initial.sandbox.as_mut().unwrap()["metadata"]["uid"] = "workspace-b-sandbox".into(); + let created: DynamicObject = serde_json::from_value(initial.sandbox.clone().unwrap()).unwrap(); + let requested = Target::from_created(&created, "workspace-b", "demo").unwrap(); + let (_server, client, state) = start(initial).await; + assert!(write(&client, &requested).await.is_err()); + let state = state.lock().unwrap(); + assert_eq!(state.value_writes, 0); + assert!(state.requests.iter().all(|request| request.method == "GET")); + assert!( + state.requests[0] + .url + .path() + .contains("/namespaces/workspace-b/") + ); +} + +#[tokio::test] +async fn missing_namespace_and_partial_claims_wait_for_full_claim_and_backlink() { + for scenario in [Scenario::ConvergeMissing, Scenario::ConvergePartial] { + let mut initial = State { + scenario, + ..Default::default() + }; + initial.sandbox.as_mut().unwrap()["metadata"]["annotations"] = json!({}); + if matches!(scenario, Scenario::ConvergeMissing) { + initial.namespace = None; + } else { + let annotations = initial.namespace.as_mut().unwrap()["metadata"]["annotations"] + .as_object_mut() + .unwrap(); + annotations.remove(SOURCE_UID); + annotations.insert(PRESTAGE.into(), "bind-next-sandbox".into()); + } + let (_server, client, state) = start(initial).await; + write(&client, &target()).await.unwrap(); + let state = state.lock().unwrap(); + assert_eq!(state.value_writes, 1); + let anchor = state + .requests + .iter() + .position(|request| request.method == "PATCH") + .unwrap(); + assert_eq!( + state.requests[..anchor] + .iter() + .filter(|request| request.url.path() == NS_PATH) + .count(), + 2 + ); + } +} + +#[tokio::test] +async fn unclaimed_labels_prestage_or_missing_backlink_never_authorize_a_write() { + for shape in ["labels", "partial", "prestage", "backlink"] { + let mut initial = State::default(); + match shape { + "labels" => { + initial.namespace.as_mut().unwrap()["metadata"]["annotations"] = json!({}); + initial.namespace.as_mut().unwrap()["metadata"]["labels"] = + json!({"kars.azure.com/sandbox":"demo"}); + } + "partial" => { + initial.namespace.as_mut().unwrap()["metadata"]["annotations"] + .as_object_mut() + .unwrap() + .remove(VERSION); + } + "prestage" => { + let annotations = initial.namespace.as_mut().unwrap()["metadata"]["annotations"] + .as_object_mut() + .unwrap(); + annotations.remove(SOURCE_UID); + annotations.insert(PRESTAGE.into(), "bind-next-sandbox".into()); + } + _ => initial.sandbox.as_mut().unwrap()["metadata"]["annotations"] = json!({}), + } + let (_server, client, state) = start(initial).await; + assert!( + write(&client, &target()) + .await + .unwrap_err() + .contains("did not converge") + ); + let state = state.lock().unwrap(); + assert_eq!(state.value_attempts, 0); + assert!(state.requests.iter().all(|request| request.method == "GET")); + } +} + +#[tokio::test] +async fn replaced_uids_foreign_owners_and_terminating_objects_fail_before_secret_access() { + for shape in [ + "cr-uid", + "ns-uid", + "claim-uid", + "claim-workspace", + "claim-name", + "version", + "owner", + "ns-delete", + "ns-phase", + "cr-delete", + "ns-missing", + "ns-no-uid", + "ns-no-rv", + "cr-no-rv", + ] { + let mut initial = State::default(); + match shape { + "cr-uid" => initial.sandbox.as_mut().unwrap()["metadata"]["uid"] = "recreated".into(), + "ns-uid" => initial.namespace.as_mut().unwrap()["metadata"]["uid"] = "recreated".into(), + "claim-uid" => { + initial.namespace.as_mut().unwrap()["metadata"]["annotations"][SOURCE_UID] = + "foreign".into() + } + "claim-workspace" => { + initial.namespace.as_mut().unwrap()["metadata"]["annotations"][SOURCE_NAMESPACE] = + "workspace-b".into() + } + "claim-name" => { + initial.namespace.as_mut().unwrap()["metadata"]["annotations"][SOURCE_NAME] = + "other".into() + } + "version" => { + initial.namespace.as_mut().unwrap()["metadata"]["annotations"][VERSION] = + "v2".into() + } + "owner" => { + initial.namespace.as_mut().unwrap()["metadata"]["ownerReferences"] = + json!([{"apiVersion":"v1","kind":"Namespace","name":"foreign","uid":"foreign"}]) + } + "ns-delete" => { + initial.namespace.as_mut().unwrap()["metadata"]["deletionTimestamp"] = + "2026-09-07T00:00:00Z".into() + } + "ns-phase" => { + initial.namespace.as_mut().unwrap()["status"]["phase"] = "Terminating".into() + } + "cr-delete" => { + initial.sandbox.as_mut().unwrap()["metadata"]["deletionTimestamp"] = + "2026-09-07T00:00:00Z".into() + } + "ns-no-uid" => { + initial.namespace.as_mut().unwrap()["metadata"]["uid"] = serde_json::Value::Null + } + "ns-no-rv" => { + initial.namespace.as_mut().unwrap()["metadata"]["resourceVersion"] = + serde_json::Value::Null + } + "cr-no-rv" => { + initial.sandbox.as_mut().unwrap()["metadata"]["resourceVersion"] = + serde_json::Value::Null + } + _ => initial.namespace = None, + } + let (_server, client, state) = start(initial).await; + assert!(write(&client, &target()).await.is_err(), "{shape}"); + let state = state.lock().unwrap(); + assert_eq!(state.value_attempts, 0); + assert!(state.requests.iter().all(|request| request.method == "GET")); + } +} + +#[tokio::test] +async fn uid_replacement_during_convergence_is_not_rebound_to_the_new_objects() { + for scenario in [ + Scenario::ReplaceNamespaceWhileWaiting, + Scenario::ReplaceSandboxWhileWaiting, + ] { + let mut initial = State { + scenario, + ..Default::default() + }; + initial.sandbox.as_mut().unwrap()["metadata"]["annotations"] = json!({}); + initial.namespace.as_mut().unwrap()["metadata"]["annotations"] = json!({}); + let (_server, client, state) = start(initial).await; + assert!(write(&client, &target()).await.is_err()); + assert!( + state + .lock() + .unwrap() + .requests + .iter() + .all(|request| request.method == "GET") + ); + } +} + +#[tokio::test] +async fn changed_claim_or_lifecycle_after_the_anchor_never_receives_credential_values() { + for scenario in [ + Scenario::ReplaceNamespaceAtAnchor, + Scenario::ReplaceSandboxAtAnchor, + Scenario::TerminateNamespaceAtAnchor, + Scenario::TerminateSandboxAtAnchor, + Scenario::RemoveClaimAtAnchor, + ] { + let (_server, client, state) = start(State { + scenario, + ..Default::default() + }) + .await; + assert!(write(&client, &target()).await.is_err()); + let state = state.lock().unwrap(); + assert_eq!(state.value_attempts, 0); + assert_eq!(state.value_writes, 0); + assert!( + state + .requests + .iter() + .all(|request| !String::from_utf8_lossy(&request.body).contains(SENTINEL)) + ); + } +} + +#[tokio::test] +async fn uid_version_fence_rejects_replacement_after_the_last_claim_read() { + for scenario in [ + Scenario::ReplaceSecretAtWrite, + Scenario::ReplaceNamespaceAtWrite, + Scenario::ConflictAtWrite, + ] { + let (_server, client, state) = start(State { + scenario, + ..Default::default() + }) + .await; + let error = write(&client, &target()).await.unwrap_err(); + assert!(!error.contains(SENTINEL)); + let state = state.lock().unwrap(); + assert_eq!(state.value_attempts, 1); + assert_eq!(state.value_writes, 0); + assert_eq!( + state + .requests + .iter() + .filter(|request| request.method == "PATCH") + .count(), + 2 + ); + } +} + +#[tokio::test] +async fn api_failures_are_not_convergence_and_never_echo_secret_values() { + for scenario in [ + Scenario::SandboxError(403), + Scenario::SandboxError(404), + Scenario::NamespaceError(403), + Scenario::NamespaceError(500), + Scenario::NamespaceError(503), + Scenario::AnchorError(403), + Scenario::WriteError(403), + Scenario::WriteError(409), + Scenario::WriteError(422), + ] { + let (_server, client, state) = start(State { + scenario, + ..Default::default() + }) + .await; + let error = write(&client, &target()).await.unwrap_err(); + assert!(error.contains("Kubernetes API status")); + assert!(!error.contains(SENTINEL)); + let state = state.lock().unwrap(); + assert_eq!(state.value_writes, 0); + assert!(state.namespace_reads <= 2); + } +} + +#[tokio::test] +async fn no_credentials_needs_no_cluster_access() { + let (_server, client, state) = start(State::default()).await; + write_credentials( + &client, + &target(), + &BTreeMap::new(), + "parent", + Wait { + attempts: 1, + delay: Duration::ZERO, + }, + ) + .await + .unwrap(); + assert!(state.lock().unwrap().requests.is_empty()); +} diff --git a/inference-router/src/spawn/mod.rs b/inference-router/src/spawn/mod.rs index b50c6726..9114012f 100644 --- a/inference-router/src/spawn/mod.rs +++ b/inference-router/src/spawn/mod.rs @@ -7,15 +7,15 @@ //! HTTP endpoints that the plugin's `/kars-spawn` slash command calls to //! manage sub-agent sandboxes through the pod's ServiceAccount. -use k8s_openapi::api::core::v1::{Namespace, Secret}; use kube::{ Api, Client, ResourceExt, - api::{DynamicObject, ListParams, Patch, PatchParams, PostParams}, + api::{DynamicObject, ListParams, PostParams}, discovery::ApiResource, }; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; +mod credentials; mod docker; #[cfg(test)] @@ -165,7 +165,7 @@ pub async fn create_sandbox( let namespace = std::env::var("KARS_NAMESPACE").unwrap_or_else(|_| "kars-system".into()); let api: Api = - Api::namespaced_with(client, &namespace, &kars_sandbox_api_resource()); + Api::namespaced_with(client.clone(), &namespace, &kars_sandbox_api_resource()); // Sub-agents inherit the parent's model unless the spawn request explicitly // overrides it. The controller plumbs the parent's resolved @@ -274,23 +274,29 @@ pub async fn create_sandbox( serde_json::from_value(crd).map_err(|e| format!("Failed to build CRD: {e}"))?; match api.create(&PostParams::default(), &obj).await { - Ok(_created) => { + Ok(created) => { tracing::info!(parent = %parent_name, child = %req.agent_id, "Sub-agent sandbox created"); // For handoff targets, propagate channel/plugin credentials to the // target namespace so the cloud agent gets Telegram, Slack, etc. if req.handoff.is_some() { let child_name = req.agent_id.clone(); - let client_clone = Client::try_default().await.ok(); - if let Some(kc) = client_clone { - tokio::spawn(async move { - if let Err(e) = propagate_credentials(&kc, &child_name).await { - tracing::warn!( - child = %child_name, - "Credential propagation failed (non-fatal): {e}" - ); - } - }); + match credentials::Target::from_created(&created, &namespace, &child_name) { + Ok(target) => { + let kc = client.clone(); + tokio::spawn(async move { + if let Err(e) = credentials::propagate(&kc, &target).await { + tracing::warn!( + child = %child_name, + "Credential propagation failed (non-fatal): {e}" + ); + } + }); + } + Err(e) => tracing::warn!( + child = %child_name, + "Credential propagation not scheduled (non-fatal): {e}" + ), } } @@ -326,97 +332,6 @@ pub async fn create_sandbox( } } -// ── Credential propagation for handoff targets ────────────────────────────── -// -// The controller mounts `{name}-credentials` secret as envFrom (optional: true). -// For handoff targets we propagate channel/plugin credentials from the source's -// environment so the cloud agent inherits Telegram, Slack, etc. - -/// Env vars that carry channel and plugin credentials (safe to propagate). -const CREDENTIAL_ENV_VARS: &[&str] = &[ - "TELEGRAM_BOT_TOKEN", - "TELEGRAM_ALLOW_FROM", - "SLACK_BOT_TOKEN", - "DISCORD_BOT_TOKEN", - "WHATSAPP_ENABLED", - "BRAVE_API_KEY", - "TAVILY_API_KEY", - "EXA_API_KEY", - "FIRECRAWL_API_KEY", - "PERPLEXITY_API_KEY", -]; - -async fn propagate_credentials(client: &Client, child_name: &str) -> Result<(), String> { - // Collect credential env vars that are set in the current environment - let mut creds: BTreeMap = BTreeMap::new(); - for &var in CREDENTIAL_ENV_VARS { - if let Ok(val) = std::env::var(var) { - if !val.is_empty() { - creds.insert(var.to_string(), val); - } - } - } - if creds.is_empty() { - tracing::info!(child = %child_name, "No channel/plugin credentials to propagate"); - return Ok(()); - } - - let target_ns = format!("kars-{}", child_name); - let secret_name = format!("{}-credentials", child_name); - - // Wait for the namespace to be created by the controller (up to 30s) - let ns_api: Api = Api::all(client.clone()); - let mut ns_ready = false; - for i in 0..15 { - if ns_api.get_opt(&target_ns).await.ok().flatten().is_some() { - ns_ready = true; - break; - } - if i == 0 { - tracing::info!(child = %child_name, "Waiting for namespace '{target_ns}' before creating credentials secret"); - } - tokio::time::sleep(std::time::Duration::from_secs(2)).await; - } - if !ns_ready { - return Err(format!("Namespace '{target_ns}' not created within 30s")); - } - - // Build and apply the credentials secret - let secret: Secret = serde_json::from_value(serde_json::json!({ - "apiVersion": "v1", - "kind": "Secret", - "metadata": { - "name": secret_name, - "namespace": target_ns, - "labels": { - "kars.azure.com/managed-by": "handoff", - "kars.azure.com/predecessor": std::env::var("SANDBOX_NAME").unwrap_or_default(), - } - }, - "type": "Opaque", - "stringData": creds, - })) - .map_err(|e| format!("Failed to build credentials secret: {e}"))?; - - let secret_api: Api = Api::namespaced(client.clone(), &target_ns); - secret_api - .patch( - &secret_name, - &PatchParams::apply("kars-handoff"), - &Patch::Apply(secret), - ) - .await - .map_err(|e| format!("Failed to create credentials secret: {e}"))?; - - tracing::info!( - child = %child_name, - creds = creds.len(), - "Propagated {} credential(s) to {target_ns}/{secret_name}", - creds.len() - ); - Ok(()) -} - /// List sub-agents spawned by a parent sandbox. pub async fn list_sandboxes(parent_name: &str) -> Result, String> { let client = Client::try_default() diff --git a/tests/e2e/namespace-ownership.sh b/tests/e2e/namespace-ownership.sh new file mode 100644 index 00000000..3972742d --- /dev/null +++ b/tests/e2e/namespace-ownership.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +# Sourced only by the existing disposable Kind harness. +test_sre_namespace_ownership() { + local context="kind-kars-e2e" system_uid sandbox_uid namespace_uid claimed_uid backlink source_ns + local k=(kubectl --context "$context") + system_uid=$("${k[@]}" get namespace kars-system -o jsonpath='{.metadata.uid}') || { + fail "Cannot read the disposable Kind namespace"; return 1; + } + if ! helm upgrade kars "$ROOT_DIR/deploy/helm/kars" \ + --kube-context "$context" --namespace kars-system --reuse-values \ + --set sre.enabled=true --set-string runtimes.hermes.image=kars-sandbox-e2e:dev \ + --wait --timeout 3m; then + fail "Fresh SRE enable failed"; return 1; + fi + local deadline=$(($(date +%s) + 120)) + local materialized=0 + while [ "$(date +%s)" -lt "$deadline" ]; do + if "${k[@]}" get deployment sre -n kars-sre >/dev/null 2>&1 \ + && "${k[@]}" get serviceaccount sre-writer -n kars-sre >/dev/null 2>&1; then + materialized=1 + break + fi + sleep 2 + done + if [ "$materialized" -ne 1 ]; then + "${k[@]}" get karssandbox sre -n kars-system -o yaml || true + fail "SRE did not materialize its deployment and writer account after namespace claiming" + return 1 + fi + sandbox_uid=$("${k[@]}" get karssandbox sre -n kars-system -o jsonpath='{.metadata.uid}') || return 1 + namespace_uid=$("${k[@]}" get namespace kars-sre -o jsonpath='{.metadata.uid}') || return 1 + claimed_uid=$("${k[@]}" get namespace kars-sre -o go-template='{{index .metadata.annotations "kars.azure.com/sandbox-uid"}}') || return 1 + source_ns=$("${k[@]}" get namespace kars-sre -o go-template='{{index .metadata.annotations "kars.azure.com/sandbox-namespace"}}') || return 1 + backlink=$("${k[@]}" get karssandbox sre -n kars-system -o go-template='{{index .metadata.annotations "kars.azure.com/namespace-uid"}}') || return 1 + if [ -z "$sandbox_uid" ] || [ -z "$namespace_uid" ] \ + || [ "$claimed_uid" != "$sandbox_uid" ] || [ "$backlink" != "$namespace_uid" ] \ + || [ "$source_ns" != "kars-system" ]; then + fail "Fresh SRE runtime lacks exact two-way namespace ownership"; return 1; + fi + if [ "$("${k[@]}" get serviceaccount sre-writer -n kars-sre -o jsonpath='{.automountServiceAccountToken}')" != "false" ]; then + fail "SRE writer account does not disable token automount"; return 1; + fi + pass "Fresh SRE install materializes a UID-bound namespace, deployment and non-automounting writer" + + if ! helm upgrade kars "$ROOT_DIR/deploy/helm/kars" \ + --kube-context "$context" --namespace kars-system --reuse-values \ + --set sre.enabled=false --wait --timeout 3m; then + fail "SRE disable failed"; return 1; + fi + if ! "${k[@]}" wait --for=delete karssandbox/sre -n kars-system --timeout=120s \ + || ! "${k[@]}" wait --for=delete namespace/kars-sre --timeout=120s; then + fail "SRE namespace cleanup did not complete"; return 1; + fi + if [ "$("${k[@]}" get namespace kars-system -o jsonpath='{.metadata.uid}')" != "$system_uid" ]; then + fail "SRE removal changed the core namespace identity"; return 1; + fi + pass "SRE removal completes guarded cleanup and preserves the core namespace" +} diff --git a/tests/e2e/run.sh b/tests/e2e/run.sh index 3016b030..34536369 100755 --- a/tests/e2e/run.sh +++ b/tests/e2e/run.sh @@ -3014,6 +3014,8 @@ EOF # ─── Main ───────────────────────────────────────────────────────────────────── +source "$SCRIPT_DIR/namespace-ownership.sh" + main() { echo "" echo "═══════════════════════════════════════════════════════" @@ -3112,6 +3114,8 @@ main() { ;; esac + test_sre_namespace_ownership || fail "SRE namespace lifecycle gate failed" + echo "" echo "═══════════════════════════════════════════════════════" echo -e " Results: ${GREEN}${PASS} passed${NC}, ${RED}${FAIL} failed${NC}" From 4ba663b77967571a2e33ccd01212b415a71e2602 Mon Sep 17 00:00:00 2001 From: pallakatos Date: Tue, 8 Sep 2026 15:48:58 +0200 Subject: [PATCH 4/5] docs(security): record maintainer sign-off for PR 548 Record the explicit 2026-09-08 maintainer approval for the qualified source. Preserve the requirement for a separate independent human sign-off and all release boundaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: eb3654cd-f1e0-445a-8734-430800af1903 --- .../2026-09-07-sandbox-namespace-ownership.md | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/docs/security-audits/2026-09-07-sandbox-namespace-ownership.md b/docs/security-audits/2026-09-07-sandbox-namespace-ownership.md index 01d18d82..25c69974 100644 --- a/docs/security-audits/2026-09-07-sandbox-namespace-ownership.md +++ b/docs/security-audits/2026-09-07-sandbox-namespace-ownership.md @@ -1,8 +1,8 @@ # Security Audit - Sandbox namespace ownership Date: 2026-09-07 -Review status: Implementation evidence prepared; independent review and genuine -author/reviewer sign-offs are pending. +Review status: Maintainer sign-off received; independent human review and +sign-off remain pending. ## Scope @@ -152,6 +152,18 @@ No live customer or dedicated test-cluster upgrade was performed. ## Verdict -Pending independent review and migration-compatibility closure. This document is -not an approval or a sign-off. Genuine author and independent reviewer sign-offs -must be supplied before the capability-audit gate can pass. +The earlier pending technical evidence above is superseded by closure at +`62093414cb8d5d9937c1d9974504047c84669d6c` in Azure/kars#548. Automated review +closed the bounded findings, and hosted technical gates passed, including 105 +disposable Kind cases with zero failures and the real SRE ownership/removal +lifecycle. The hosted lifecycle evidence is +https://github.com/Azure/kars/actions/runs/34150756641/job/101838058446. +This is not a live customer migration or a second human review. + +The maintainer explicitly signed off and approved that qualified source on +2026-09-08. The approval does not extend to later functional changes, other +unresolved slices, a customer deployment or promotion to `main`. +An independent person's review and sign-off remain required; this one approval +does not satisfy both positions in the two-person capability-audit gate. + +Signed-off-by: pallakatos <191481949+pallakatos@users.noreply.github.com> From 3aed0df7a73a5b39e9a9fa54ce50a0519afe35aa Mon Sep 17 00:00:00 2001 From: pallakatos Date: Tue, 8 Sep 2026 16:00:47 +0200 Subject: [PATCH 5/5] docs(security): record author integration waiver for PR 548 Record the author's explicit waiver of the second-person signature for this qualified integration slice only. Preserve truthful CI results, all other gates, exact source scope and immediate protection restoration. No main promotion or customer deployment is authorized. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: eb3654cd-f1e0-445a-8734-430800af1903 --- .../2026-09-07-sandbox-namespace-ownership.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/security-audits/2026-09-07-sandbox-namespace-ownership.md b/docs/security-audits/2026-09-07-sandbox-namespace-ownership.md index 25c69974..6af028e1 100644 --- a/docs/security-audits/2026-09-07-sandbox-namespace-ownership.md +++ b/docs/security-audits/2026-09-07-sandbox-namespace-ownership.md @@ -167,3 +167,19 @@ An independent person's review and sign-off remain required; this one approval does not satisfy both positions in the two-person capability-audit gate. Signed-off-by: pallakatos <191481949+pallakatos@users.noreply.github.com> + +## Explicit author waiver for integration assembly + +On 2026-09-08, Kars author `pallakatos` explicitly waived the second-person +sign-off for Azure/kars#548: "just push them and say I waived it". +This supersedes the independent-signature landing requirement above only for +assembling the already-qualified source into `Azure/kars:kars-bridge`. +It does not assert that an independent human review occurred. + +All other required technical and security gates must pass on the landing head. +The signature-only branch-protection exception and the already-authorized +account-specific review allowance must be restored immediately after the merge, +including on failure. No CI result is rewritten as successful. +The waiver does not cover functional changes beyond the qualified source, +later unresolved slices, `main` promotion, customer deployments or public +publication of the private Bridge application.