diff --git a/landing-zone-configurator/app/apps/worker/src/observation.ts b/landing-zone-configurator/app/apps/worker/src/observation.ts index cb63880..4c47c26 100644 --- a/landing-zone-configurator/app/apps/worker/src/observation.ts +++ b/landing-zone-configurator/app/apps/worker/src/observation.ts @@ -1,8 +1,72 @@ +import { watch } from "node:fs"; +import { open } from "node:fs/promises"; +import { resolve } from "node:path"; import { applicationPlanJobSchema, applicationRunnerBindingSchema, + openTofuProgress, } from "@lzc/contracts"; +// Read only the bounded first version record; observation failure never fails a Plan. +export function watchPlanVersion( + directory: string, + active: () => boolean, + observed: () => void, +) { + let closed = false; + let reading = false; + let close = () => { + closed = true; + }; + const check = async () => { + if (closed || reading || !active()) return; + reading = true; + try { + const file = await open(resolve(directory, "plan-events.log"), "r"); + try { + const bytes = Buffer.alloc(16384); + const { bytesRead } = await file.read(bytes, 0, bytes.length, 0); + const content = bytes.subarray(0, bytesRead).toString("utf8"); + const newline = content.indexOf("\n"); + if (newline < 0) return; + const progress = openTofuProgress(content.slice(0, newline + 1)); + if ( + !progress.invalid && + /^OpenTofu 1\.12\.6\n/.test(progress.text) && + !closed && + active() + ) { + close(); + observed(); + } + } finally { + await file.close(); + } + } catch { + // A partial/unavailable file is retried by the existing output publication loop. + } finally { + reading = false; + } + }; + try { + const watcher = watch( + directory, + { persistent: false }, + (_event, filename) => { + if (filename === "plan-events.log") void check(); + }, + ); + close = () => { + closed = true; + watcher.close(); + }; + watcher.on("error", close); + } catch { + /* Keep the existing bounded publication fallback. */ + } + return () => close(); +} + // Operator logs contain identifiers only, never inputs, engine messages or State. export function applicationPlanObservation( jobId: unknown, diff --git a/landing-zone-configurator/app/apps/worker/src/runner.ts b/landing-zone-configurator/app/apps/worker/src/runner.ts index e649fc0..4ffd6b1 100644 --- a/landing-zone-configurator/app/apps/worker/src/runner.ts +++ b/landing-zone-configurator/app/apps/worker/src/runner.ts @@ -29,7 +29,7 @@ import { s3RunnerBackendSchema, summarizePlan, } from "@lzc/contracts"; -import { applicationPlanObservation } from "./observation.js"; +import { applicationPlanObservation, watchPlanVersion } from "./observation.js"; export const brokerOrigin = "https://lzc-dev-configurator.apps.01.cf.eu01.stackit.cloud"; @@ -610,62 +610,77 @@ export async function runWorker( outputTimer = setTimeout(pollOutput, 500); try { activeCommand = phase; - await new Promise((done, fail) => { - const child = spawn( - "/bin/bash", - [ - resolve(root, "run-plan.sh"), - phase, - applying - ? "platform-apply" - : applicationBinding?.purpose === "destroy" - ? "application-destroy-plan" - : applicationBinding?.purpose === "drift" - ? "application-drift-plan" - : mode === "initial-plan-only" - ? mode - : "platform-plan", - ], - { - cwd: work as string, - env, - stdio: ["ignore", "ignore", "ignore"], - detached: true, - }, - ); - let killTimer: ReturnType | undefined; - const stop = (signal: NodeJS.Signals) => { - try { - if (child.pid) process.kill(-child.pid, signal); - } catch {} - }; - const timeout = setTimeout( - () => { - errorCode = "timed_out"; - stop("SIGTERM"); - killTimer = setTimeout(() => stop("SIGKILL"), 10000); - }, - Math.max(1, deadline - Date.now()), - ); - const clear = () => { - clearTimeout(timeout); - if (killTimer) clearTimeout(killTimer); - }; - child.once("error", () => { - clear(); - fail(new Error("Runner process failed")); - }); - child.once("exit", (code) => { - activeCommand = undefined; - if (phase === "planning" && planObserved) - observePlan("plan-command-finished"); - if (errorCode === "timed_out") stop("SIGKILL"); - clear(); - code === 0 && errorCode !== "timed_out" - ? done() - : fail(new Error("Runner phase failed")); + const stopObservation = + phase === "planning" && mode === "application-plan" + ? watchPlanVersion( + work as string, + () => activeCommand === "planning" && !planObserved, + () => { + planObserved = true; + observePlan("native-plan-observed"); + }, + ) + : () => {}; + try { + await new Promise((done, fail) => { + const child = spawn( + "/bin/bash", + [ + resolve(root, "run-plan.sh"), + phase, + applying + ? "platform-apply" + : applicationBinding?.purpose === "destroy" + ? "application-destroy-plan" + : applicationBinding?.purpose === "drift" + ? "application-drift-plan" + : mode === "initial-plan-only" + ? mode + : "platform-plan", + ], + { + cwd: work as string, + env, + stdio: ["ignore", "ignore", "ignore"], + detached: true, + }, + ); + let killTimer: ReturnType | undefined; + const stop = (signal: NodeJS.Signals) => { + try { + if (child.pid) process.kill(-child.pid, signal); + } catch {} + }; + const timeout = setTimeout( + () => { + errorCode = "timed_out"; + stop("SIGTERM"); + killTimer = setTimeout(() => stop("SIGKILL"), 10000); + }, + Math.max(1, deadline - Date.now()), + ); + const clear = () => { + clearTimeout(timeout); + if (killTimer) clearTimeout(killTimer); + }; + child.once("error", () => { + clear(); + fail(new Error("Runner process failed")); + }); + child.once("exit", (code) => { + activeCommand = undefined; + if (phase === "planning" && planObserved) + observePlan("plan-command-finished"); + if (errorCode === "timed_out") stop("SIGKILL"); + clear(); + code === 0 && errorCode !== "timed_out" + ? done() + : fail(new Error("Runner phase failed")); + }); }); - }); + } finally { + stopObservation(); + } } finally { activeCommand = undefined; outputStopped = true; diff --git a/landing-zone-configurator/app/tests/worker-observation.test.ts b/landing-zone-configurator/app/tests/worker-observation.test.ts new file mode 100644 index 0000000..445fa22 --- /dev/null +++ b/landing-zone-configurator/app/tests/worker-observation.test.ts @@ -0,0 +1,72 @@ +import { appendFile, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, expect, it, vi } from "vitest"; +import { watchPlanVersion } from "../apps/worker/src/observation.js"; + +const roots: string[] = []; +const version = JSON.stringify({ + type: "version", + ui: "1.2", + "@message": "OpenTofu 1.12.6", +}); +afterEach(async () => { + await Promise.all( + roots.splice(0).map((root) => rm(root, { recursive: true, force: true })), + ); +}); + +it("observes a completed version record once without forwarding native contents", async () => { + const root = await mkdtemp(join(tmpdir(), "lzc-observation-")); + roots.push(root); + const observed = vi.fn(); + const stop = watchPlanVersion(root, () => true, observed); + try { + const file = join(root, "plan-events.log"); + await writeFile(file, version.slice(0, 20)); + await new Promise((done) => setTimeout(done, 40)); + expect(observed).not.toHaveBeenCalled(); + await appendFile( + file, + `${version.slice(20)}\n${JSON.stringify({ type: "diagnostic", "@message": "private-engine-value" })}\n`, + ); + await vi.waitFor(() => expect(observed).toHaveBeenCalledExactlyOnceWith()); + await appendFile(file, `${version}\n`); + await new Promise((done) => setTimeout(done, 40)); + expect(observed).toHaveBeenCalledTimes(1); + } finally { + stop(); + } +}); + +it.each(["closed", "inactive"])( + "does not observe a %s command", + async (mode) => { + const root = await mkdtemp(join(tmpdir(), "lzc-observation-")); + roots.push(root); + const observed = vi.fn(); + const stop = watchPlanVersion(root, () => mode !== "inactive", observed); + try { + if (mode === "closed") stop(); + await writeFile(join(root, "plan-events.log"), `${version}\n`); + await new Promise((done) => setTimeout(done, 40)); + expect(observed).not.toHaveBeenCalled(); + } finally { + stop(); + } + }, +); + +it("leaves Plan execution available if the optional watcher cannot be created", async () => { + const root = await mkdtemp(join(tmpdir(), "lzc-observation-")); + roots.push(root); + await mkdir(join(root, "existing")); + const observed = vi.fn(); + const stop = watchPlanVersion( + join(root, "unavailable"), + () => true, + observed, + ); + expect(() => stop()).not.toThrow(); + expect(observed).not.toHaveBeenCalled(); +}); diff --git a/landing-zone-configurator/infra/recovery-lab/README.de.md b/landing-zone-configurator/infra/recovery-lab/README.de.md index 7bd2b4d..17e72ad 100644 --- a/landing-zone-configurator/infra/recovery-lab/README.de.md +++ b/landing-zone-configurator/infra/recovery-lab/README.de.md @@ -127,6 +127,18 @@ von einmaligen oder warmen Runner-Apps und ignoriert andere Instanzen, ungültig IDs, Meldungen vor dem Scharfschalten sowie über drei Sekunden alte Ereignisse. Seine Umgebung übernimmt keine sonstigen Cloud-Zugangsdaten. +Die Organisations-, Space- und Template-Grenze wird einmal beim Scharfschalten +geprüft. Unmittelbar vor dem Stop prüft eine einzelne Abfrage erneut die genaue +Runner-App-GUID und ihren Space; einmalige Tasks benötigen zusätzlich die +eindeutig aktive Task. Eine ungeprüfte Grenze kann keinen Stop autorisieren. +Wiederholte Organisations-, Template- und vollständige App-Listen-Abfragen im +kurzen Plan-Zeitfenster werden gezielt vermieden. Der Worker beobachtet den +begrenzten ersten nativen Versionsdatensatz beim Schreiben seiner Datei; die +normale Ausgabeübertragung bleibt als Rückfall erhalten. Beide Mechanismen +verzögern den Plan nicht und ändern keine Eingaben. Private Nachweise enthalten +Zeitstempel für Absicht und Antwort zum Vergleich von tatsächlichem Stop und +nativem Abschluss. + Bei einem ausführbaren Template mit direkter Bereitstellung startet die normale Bestelloberfläche automatisch den ersten Plan. Diesen Lauf beenden lassen und sein Ergebnis prüfen. Für den Abbruchtest dieselbe entbehrliche Instanz verwenden: diff --git a/landing-zone-configurator/infra/recovery-lab/README.md b/landing-zone-configurator/infra/recovery-lab/README.md index 38a23b8..e2f98a1 100644 --- a/landing-zone-configurator/infra/recovery-lab/README.md +++ b/landing-zone-configurator/infra/recovery-lab/README.md @@ -120,6 +120,16 @@ CF, follows bounded log streams only from one-shot/warm Runner apps, and ignores other instances, malformed IDs, observations before arming and events older than three seconds. Its environment carries no ambient cloud credentials. +The organization/space/template boundary is verified once when arming. Immediately +before stopping, a single lookup rechecks the exact Runner app GUID and its space; +one-shot tasks additionally require the unique active task. An unverified boundary +cannot authorize a stop. Repeating organization, template and full app-list reads +inside the short Plan window is deliberately avoided. The worker observes the +bounded first native version record as its file is written, with normal output +publication retained as a fallback. Neither mechanism delays the Plan or changes +its inputs. Private receipts include intent and response timestamps for comparing +the actual stop with native completion. + For an executable template with direct deployment policy, the ordinary ordering UI automatically starts the first Plan. Let that initial Plan finish and review its outcome. Reuse the same disposable instance for the interruption drill: diff --git a/landing-zone-configurator/infra/recovery-lab/hosted_plan.py b/landing-zone-configurator/infra/recovery-lab/hosted_plan.py index 3bc1fd5..972b644 100644 --- a/landing-zone-configurator/infra/recovery-lab/hosted_plan.py +++ b/landing-zone-configurator/infra/recovery-lab/hosted_plan.py @@ -70,6 +70,7 @@ def __init__(self, cf, cf_home, space, template, instance, confirm=None, receipt self.env = {"PATH": "/usr/bin:/bin", "LANG": "C.UTF-8", "CF_HOME": str(home.resolve())} self.confirm = confirm self.stopped = False + self.boundary_verified = False def call(self, args): result = subprocess.run([self.cf, *args], env=self.env, capture_output=True, timeout=15) @@ -88,6 +89,7 @@ def api(self, path, method="GET"): return value def boundary(self): + self.boundary_verified = False space = self.api(f"/v3/spaces/{self.space}") org = self.api("/v3/organizations/" + guid(space["relationships"]["organization"]["data"]["guid"])) if org["name"] != "lzc-dev-runners" or space["name"] != "plans": @@ -95,6 +97,7 @@ def boundary(self): template = self.api(f"/v3/apps/{self.template}") if template["name"] != "lzc-plan-template" or template["relationships"]["space"]["data"]["guid"] != self.space: raise RuntimeError("Runner template binding mismatch") + self.boundary_verified = True def apps(self): result = self.api(f"/v3/apps?space_guids={self.space}&per_page=100") @@ -104,13 +107,12 @@ def apps(self): def stop(self, app_id, event, finished): guid(app_id) - if not self.confirm or self.stopped or event["jobId"] in finished: + if not self.confirm or self.stopped or not self.boundary_verified or event["jobId"] in finished: raise RuntimeError("No stop authorization or observation is already finished") if observation(json.dumps(event, separators=(",", ":")), self.instance, self.armed_at, time.time()) is None or event["observation"] != "native-plan-observed": raise RuntimeError("Fresh matching native Plan observation required") - self.boundary() - app = self.apps().get(app_id) - if not app: + app = self.api(f"/v3/apps/{app_id}") + if app_id == self.template or app.get("guid") != app_id or app.get("relationships", {}).get("space", {}).get("data", {}).get("guid") != self.space: raise RuntimeError("Observed Runner is outside the allowed space") # Single-use guard is set before the first mutation, including API failure. self.stopped = True @@ -120,11 +122,11 @@ def fresh(): def request(path, result): fresh() with Receipt(self.receipt_path) as receipt: - record = {"phase": "stop-intent", "instanceId": self.instance, "jobId": event["jobId"], "observedAt": event["observedAt"], "qualifiesRecovery": False, **result} + record = {"phase": "stop-intent", "instanceId": self.instance, "jobId": event["jobId"], "observedAt": event["observedAt"], "intentAt": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"), "qualifiesRecovery": False, **result} receipt.write(record) fresh() self.api(path, "POST") - receipt.write({**record, "phase": "stop-requested"}) + receipt.write({**record, "phase": "stop-requested", "responseAt": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z")}) return result if app["name"] == "lzc-plan-" + event["jobId"]: tasks = self.api(f"/v3/apps/{app_id}/tasks?per_page=100") diff --git a/landing-zone-configurator/infra/recovery-lab/test_hosted_plan.py b/landing-zone-configurator/infra/recovery-lab/test_hosted_plan.py index a9cd0b5..23d1715 100644 --- a/landing-zone-configurator/infra/recovery-lab/test_hosted_plan.py +++ b/landing-zone-configurator/infra/recovery-lab/test_hosted_plan.py @@ -23,17 +23,19 @@ def event(instance=INSTANCE, age=0): class HostedBoundaries(unittest.TestCase): + def app(self, warm=True): + return {"guid": APP, "name": "lzc-warm-"+OTHER if warm else "lzc-plan-"+JOB, "state": "STARTED", "relationships": {"space": {"data": {"guid": SPACE}}}} + def controller(self, confirm=INSTANCE, warm=True): controller = object.__new__(hosted.Controller) controller.instance, controller.space, controller.template = INSTANCE, SPACE, TEMPLATE controller.confirm, controller.stopped = confirm, False + controller.boundary_verified = True private = tempfile.TemporaryDirectory() self.addCleanup(private.cleanup) controller.receipt_path = str(Path(private.name) / "receipt.json") controller.armed_at = time.time()-1 - controller.boundary = lambda: None - controller.apps = lambda: {APP: {"name": "lzc-warm-"+OTHER if warm else "lzc-plan-"+JOB, "state": "STARTED"}} - controller.api = lambda path, method="GET": {"pagination": {"next": None}, "resources": [{"guid": OTHER, "state": "RUNNING", "name": "initial-plan"}]} + controller.api = lambda path, method="GET": self.app(warm) if path == f"/v3/apps/{APP}" else {"pagination": {"next": None}, "resources": [{"guid": OTHER, "state": "RUNNING", "name": "initial-plan"}]} return controller def test_foreign_instance_stale_and_prearmed_events_are_ignored(self): @@ -50,11 +52,11 @@ def test_observe_mode_finished_and_foreign_jobs_cannot_mutate(self): def test_only_one_exact_warm_runner_can_be_stopped(self): controller = self.controller() - with patch.object(controller, "api", return_value={}) as api: + with patch.object(controller, "api", side_effect=controller.api) as api: controller.stop(APP, event(), set()) - api.assert_called_once_with(f"/v3/apps/{APP}/actions/stop", "POST") + self.assertEqual([call.args for call in api.call_args_list], [(f"/v3/apps/{APP}",), (f"/v3/apps/{APP}/actions/stop", "POST")]) with self.assertRaises(RuntimeError): controller.stop(APP, event(), set()) - self.assertEqual(api.call_count, 1) + self.assertEqual(api.call_count, 2) def test_cold_runner_cancels_only_unique_matching_active_task(self): controller = self.controller(warm=False) @@ -63,26 +65,39 @@ def test_cold_runner_cancels_only_unique_matching_active_task(self): controller.stop(APP, event(), set()) self.assertEqual(api.call_args.args, (f"/v3/tasks/{OTHER}/actions/cancel", "POST")) controller = self.controller(warm=False) - with patch.object(controller, "api", return_value={"pagination": {"next": None}, "resources": []}) as api: + with patch.object(controller, "api", side_effect=lambda path: self.app(False) if path == f"/v3/apps/{APP}" else {"pagination": {"next": None}, "resources": []}) as api: with self.assertRaises(RuntimeError): controller.stop(APP, event(), set()) self.assertTrue(all(len(call.args) == 1 for call in api.call_args_list)) def test_target_disappearance_or_expiry_during_validation_prevents_stop(self): controller = self.controller() - controller.apps = lambda: {} - with patch.object(controller, "api") as api: + with patch.object(controller, "api", return_value={}) as api: with self.assertRaises(RuntimeError): controller.stop(APP, event(), set()) - api.assert_not_called() + self.assertTrue(all(len(call.args) == 1 for call in api.call_args_list)) controller = self.controller() value = event() - def expire(): + def expire(path): value["observedAt"] = event(age=10)["observedAt"] - return {APP: {"name": "lzc-warm-"+OTHER, "state": "STARTED"}} - controller.apps = expire - with patch.object(controller, "api") as api: + return self.app() + with patch.object(controller, "api", side_effect=expire) as api: with self.assertRaises(RuntimeError): controller.stop(APP, value, set()) + self.assertTrue(all(len(call.args) == 1 for call in api.call_args_list)) + + def test_boundary_must_have_been_verified_before_a_stop(self): + controller = self.controller() + controller.boundary_verified = False + with patch.object(controller, "api") as api: + with self.assertRaises(RuntimeError): controller.stop(APP, event(), set()) api.assert_not_called() + def test_current_app_in_another_space_cannot_be_stopped(self): + controller = self.controller() + app = self.app() + app["relationships"]["space"]["data"]["guid"] = OTHER + with patch.object(controller, "api", return_value=app) as api: + with self.assertRaises(RuntimeError): controller.stop(APP, event(), set()) + self.assertTrue(all(len(call.args) == 1 for call in api.call_args_list)) + def test_other_organization_and_public_credential_directory_are_rejected(self): controller = self.controller() controller.api = lambda path: {"relationships": {"organization": {"data": {"guid": OTHER}}}, "name": "production"} @@ -120,7 +135,9 @@ def test_private_cf_errors_are_not_forwarded(self): def test_durable_intent_precedes_stop_and_receipt_cannot_be_reused(self): controller = self.controller() - def request(path, method): + original = controller.api + def request(path, method="GET"): + if method == "GET": return original(path) stored = json.loads(Path(controller.receipt_path).read_text()) self.assertEqual(stored["phase"], "stop-intent") self.assertEqual(stored["jobId"], JOB) @@ -132,13 +149,17 @@ def request(path, method): self.assertEqual(Path(controller.receipt_path).stat().st_mode & 0o777, 0o600) retry = self.controller() retry.receipt_path = controller.receipt_path - with patch.object(retry, "api") as api: + with patch.object(retry, "api", side_effect=retry.api) as api: with self.assertRaises(FileExistsError): retry.stop(APP, event(), set()) - api.assert_not_called() + self.assertTrue(all(len(call.args) == 1 for call in api.call_args_list)) def test_ambiguous_stop_retains_intent_and_prevents_another_mutation(self): controller = self.controller() - with patch.object(controller, "api", side_effect=RuntimeError("ambiguous request")): + original = controller.api + def ambiguous(path, method="GET"): + if method == "GET": return original(path) + raise RuntimeError("ambiguous request") + with patch.object(controller, "api", side_effect=ambiguous): with self.assertRaises(RuntimeError): controller.stop(APP, event(), set()) self.assertEqual(json.loads(Path(controller.receipt_path).read_text())["phase"], "stop-intent") with self.assertRaises(FileExistsError): hosted.Receipt(controller.receipt_path) @@ -146,9 +167,9 @@ def test_ambiguous_stop_retains_intent_and_prevents_another_mutation(self): def test_public_receipt_directory_cannot_reach_cloud_mutation(self): controller = self.controller() controller.receipt_path = "/tmp/never-created-hosted-stop.json" - with patch.object(controller, "api") as api: + with patch.object(controller, "api", side_effect=controller.api) as api: with self.assertRaises(ValueError): controller.stop(APP, event(), set()) - api.assert_not_called() + self.assertTrue(all(len(call.args) == 1 for call in api.call_args_list)) if __name__ == "__main__": unittest.main()