From 0552c246335685ec0cc2505f19323cc4669a567d Mon Sep 17 00:00:00 2001 From: notgitika Date: Mon, 24 Aug 2026 15:58:24 -0400 Subject: [PATCH 1/5] fix(project): check the template before letting the Toolkit deploy it Two review findings from #2058, both cases of deploy trusting something it had not checked. A synthesized template with no resources makes the CDK Toolkit *delete* an existing stack of that name and return as though it deployed. #2058 caught that after the fact, by which point the stack was already gone. Check the resource count before handing the assembly to the Toolkit instead. Stack selection matched on the target-name tag alone, never on the account and region the artifact was synthesized for. Those derive from the same target today and so cannot disagree, but nothing enforced it, and the Toolkit deploys where the artifact's environment points rather than where the tag says. Both fields were also being stripped on read, since the manifest schema declared neither. stackArtifactIdForTarget becomes stackArtifactForTarget, returning the template path alongside the id so the resource check needs no second read of the manifest. --- src/core/project/backends/cdk.test.ts | 72 ++++++++-- src/core/project/backends/cdk.ts | 23 +-- .../project/backends/cdk/assembly.test.ts | 135 +++++++++++++++--- src/core/project/backends/cdk/assembly.ts | 120 +++++++++++++++- src/core/project/backends/cdk/toolkit.ts | 3 +- 5 files changed, 315 insertions(+), 38 deletions(-) diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index 288bb2199..f09204ba7 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -51,9 +51,37 @@ async function project(withDependencies = true): Promise { }; } -async function writeAssembly(project: Project, targetNames: string[]): Promise { +type AssemblyOptions = { + /** Overrides the environment every stack artifact is synthesized for. */ + environment?: string; + /** Overrides the resources every stack's template declares. */ + resources?: Record; +}; + +async function writeAssembly( + project: Project, + targetNames: string[], + options: AssemblyOptions = {}, +): Promise { const directory = assemblyDirectory(project); await mkdir(directory, { recursive: true }); + const stacks = targetNames.map((target, index) => ({ + target, + id: `AgentCore-example-${target}-${index}`, + templateFile: `AgentCore-example-${target}-${index}.template.json`, + })); + + await Promise.all( + stacks.map((stack) => + writeFile( + join(directory, stack.templateFile), + JSON.stringify({ + Resources: options.resources ?? { Runtime: { Type: "AWS::BedrockAgentCore::Runtime" } }, + }), + ), + ), + ); + await writeFile( join(directory, "manifest.json"), JSON.stringify({ @@ -61,14 +89,16 @@ async function writeAssembly(project: Project, targetNames: string[]): Promise [ - [ - `AgentCore-example-${target}-${index}`, - { - type: "aws:cloudformation:stack", - properties: { tags: { "agentcore:target-name": target } }, + stacks.map((stack) => [ + stack.id, + { + type: "aws:cloudformation:stack", + environment: options.environment ?? `aws://${TARGET.account}/${TARGET.region}`, + properties: { + templateFile: stack.templateFile, + tags: { "agentcore:target-name": stack.target }, }, - ], + }, ]), ), }, @@ -302,6 +332,32 @@ describe("CdkBackend.deploy", () => { expect(subject.runs).toEqual([]); }); + test("refuses a resource-less stack before the Toolkit can delete it", async () => { + const input = await project(); + await writeAssembly(input, [TARGET.name], { resources: {} }); + const subject = harness(); + + await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).rejects.toThrow( + /declares no resources/, + ); + // Nothing reached the Toolkit, so no stack was deleted and none bootstrapped. + expect(subject.runs).toEqual([]); + expect(subject.bootstrapRegions).toEqual([]); + }); + + test("refuses a stack synthesized for a different region than the target", async () => { + const input = await project(); + await writeAssembly(input, [TARGET.name], { + environment: `aws://${TARGET.account}/us-west-2`, + }); + const subject = harness(); + + await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).rejects.toThrow( + /built for region us-west-2 \(target expects us-east-1\)/, + ); + expect(subject.runs).toEqual([]); + }); + test.each([ ["absent", { kind: "absent" } as const], ["outdated", { kind: "outdated", version: 29 } as const], diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index ee160263e..b58fd7ff5 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -11,7 +11,7 @@ import { } from "../../../io"; import type { Logger } from "../../../logging"; import type { DeployBackendInput, ProjectBackend } from "./types"; -import { stackArtifactIdForTarget } from "./cdk/assembly"; +import { assertStackHasResources, stackArtifactForTarget } from "./cdk/assembly"; import { readDeployedState, updateTargetState } from "./cdk/deployedState"; import { probeBootstrap, @@ -108,11 +108,13 @@ export class CdkBackend implements ProjectBackend { yield* this.build(project); const assemblyDirectory = this.assemblyDirectory(project); - const stackArtifactId = await stackArtifactIdForTarget( - this.json, - assemblyDirectory, - target.name, - ); + const artifact = await stackArtifactForTarget(this.json, assemblyDirectory, target.name, { + account: target.account, + region: target.region, + }); + // Checked here rather than after the fact: the Toolkit deletes an existing + // stack whose new template has no resources, and returns as if it deployed. + await assertStackHasResources(this.json, assemblyDirectory, artifact); const options = { assemblyDirectory, credentials, region: target.region }; const bootstrap = await this.bootstrap(target.region, credentials); @@ -143,15 +145,18 @@ export class CdkBackend implements ProjectBackend { } } - yield { message: `Deploying ${stackArtifactId}` }; - const { outputs, stackArn } = await this.cdk({ kind: "deploy", stackArtifactId }, options); + yield { message: `Deploying ${artifact.id}` }; + const { outputs, stackArn } = await this.cdk( + { kind: "deploy", stackArtifactId: artifact.id }, + options, + ); // A successful deploy always has a stack ARN (CDK's DeployedStack requires // it). Its absence means a malformed result; fail loudly rather than return // success without recording the binding later commands need. if (!stackArn) { throw new MalformedServiceResponseError( - `The CDK Toolkit reported a successful deploy of '${stackArtifactId}' without a stack ARN.`, + `The CDK Toolkit reported a successful deploy of '${artifact.id}' without a stack ARN.`, ); } diff --git a/src/core/project/backends/cdk/assembly.test.ts b/src/core/project/backends/cdk/assembly.test.ts index 20180acee..8e2cb7775 100644 --- a/src/core/project/backends/cdk/assembly.test.ts +++ b/src/core/project/backends/cdk/assembly.test.ts @@ -4,11 +4,14 @@ import { join } from "node:path"; import { tmpdir } from "node:os"; import { FsReadWriteJson } from "../../../../io"; import { createSilentLogger } from "../../../../testing"; -import { stackArtifactIdForTarget } from "./assembly"; +import { assertStackHasResources, stackArtifactForTarget } from "./assembly"; const temporaryDirectories: string[] = []; const json = new FsReadWriteJson({ logger: createSilentLogger() }); +const EXPECTED = { account: "111122223333", region: "us-east-1" } as const; +const TEMPLATE_FILE = "stack.template.json"; + afterEach(async () => { await Promise.all( temporaryDirectories @@ -24,31 +27,42 @@ async function assembly(artifacts: Record): Promise { return directory; } -describe("stackArtifactIdForTarget", () => { +/** A stack artifact for `target`, bound to EXPECTED unless `overrides` says otherwise. */ +function stackArtifact(target: string, overrides: Record = {}) { + return { + type: "aws:cloudformation:stack", + environment: `aws://${EXPECTED.account}/${EXPECTED.region}`, + properties: { + tags: { "agentcore:target-name": target }, + templateFile: TEMPLATE_FILE, + }, + ...overrides, + }; +} + +async function writeTemplate(directory: string, resources: Record): Promise { + await writeFile(join(directory, TEMPLATE_FILE), JSON.stringify({ Resources: resources })); +} + +describe("stackArtifactForTarget", () => { test("selects by the target tag instead of deriving a stack name", async () => { - const directory = await assembly({ - "nested/stack-id": { - type: "aws:cloudformation:stack", - properties: { - tags: { "agentcore:target-name": "prod" }, - }, - }, - }); + const directory = await assembly({ "nested/stack-id": stackArtifact("prod") }); - expect(await stackArtifactIdForTarget(json, directory, "prod")).toBe("nested/stack-id"); + expect(await stackArtifactForTarget(json, directory, "prod", EXPECTED)).toEqual({ + id: "nested/stack-id", + templateFile: TEMPLATE_FILE, + }); }); test("ignores non-stack artifacts", async () => { const directory = await assembly({ Tree: { type: "cdk:tree", - properties: { - tags: { "agentcore:target-name": "prod" }, - }, + properties: { tags: { "agentcore:target-name": "prod" } }, }, }); - await expect(stackArtifactIdForTarget(json, directory, "prod")).rejects.toThrow( + await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( /defines 0 stack/, ); }); @@ -58,8 +72,97 @@ describe("stackArtifactIdForTarget", () => { temporaryDirectories.push(directory); await mkdir(directory, { recursive: true }); - await expect(stackArtifactIdForTarget(json, directory, "prod")).rejects.toThrow( + await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( /No synthesized cloud assembly was found/, ); }); + + test("rejects a stack tagged for the target but built for another account", async () => { + const directory = await assembly({ + Stack: stackArtifact("prod", { environment: `aws://999988887777/${EXPECTED.region}` }), + }); + + await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( + /built for account 999988887777 \(target expects 111122223333\)/, + ); + }); + + test("rejects a stack tagged for the target but built for another region", async () => { + const directory = await assembly({ + Stack: stackArtifact("prod", { environment: `aws://${EXPECTED.account}/eu-west-1` }), + }); + + await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( + /built for region eu-west-1 \(target expects us-east-1\)/, + ); + }); + + test("names both halves when neither account nor region matches", async () => { + const directory = await assembly({ + Stack: stackArtifact("prod", { environment: "aws://999988887777/eu-west-1" }), + }); + + await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( + /account 999988887777 \(target expects 111122223333\) and region eu-west-1/, + ); + }); + + test.each([ + ["no environment at all", undefined], + ["CDK's unknown-* placeholders", "aws://unknown-account/unknown-region"], + ])("accepts an environment-agnostic stack with %s", async (_label, environment) => { + const directory = await assembly({ + // An absent key and an explicit undefined both mean "agnostic" once + // serialized, which is what the manifest on disk actually looks like. + Stack: stackArtifact("prod", { environment }), + }); + + expect((await stackArtifactForTarget(json, directory, "prod", EXPECTED)).id).toBe("Stack"); + }); + + test("rejects an environment it cannot parse rather than assuming a match", async () => { + const directory = await assembly({ + Stack: stackArtifact("prod", { environment: "us-east-1" }), + }); + + await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( + /unrecognized environment 'us-east-1'/, + ); + }); +}); + +describe("assertStackHasResources", () => { + test("accepts a template that declares resources", async () => { + const directory = await assembly({ Stack: stackArtifact("prod") }); + await writeTemplate(directory, { Runtime: { Type: "AWS::BedrockAgentCore::Runtime" } }); + + expect( + await assertStackHasResources(json, directory, { id: "Stack", templateFile: TEMPLATE_FILE }), + ).toBeUndefined(); + }); + + test("rejects a resource-less template, which the Toolkit reads as a deletion", async () => { + const directory = await assembly({ Stack: stackArtifact("prod") }); + await writeTemplate(directory, {}); + + await expect( + assertStackHasResources(json, directory, { id: "Stack", templateFile: TEMPLATE_FILE }), + ).rejects.toThrow(/declares no resources, so deploying it would delete the existing stack/); + }); + + test("rejects a template the assembly names but does not contain", async () => { + const directory = await assembly({ Stack: stackArtifact("prod") }); + + await expect( + assertStackHasResources(json, directory, { id: "Stack", templateFile: TEMPLATE_FILE }), + ).rejects.toThrow(/synthesized template for stack 'Stack' is missing/); + }); + + test("rejects a stack artifact that names no template at all", async () => { + const directory = await assembly({ Stack: stackArtifact("prod") }); + + await expect( + assertStackHasResources(json, directory, { id: "Stack", templateFile: undefined }), + ).rejects.toThrow(/names no template file/); + }); }); diff --git a/src/core/project/backends/cdk/assembly.ts b/src/core/project/backends/cdk/assembly.ts index 4077da573..288a36d10 100644 --- a/src/core/project/backends/cdk/assembly.ts +++ b/src/core/project/backends/cdk/assembly.ts @@ -6,6 +6,9 @@ import type { ReadWriteJson } from "../../../../io"; const TARGET_TAG = "agentcore:target-name"; const STACK_ARTIFACT = "aws:cloudformation:stack"; +/** What CDK writes in an artifact's `environment` when the stack is env-agnostic. */ +const UNKNOWN_ACCOUNT = "unknown-account"; +const UNKNOWN_REGION = "unknown-region"; const AssemblyManifestSchema = z.object({ artifacts: z @@ -13,9 +16,11 @@ const AssemblyManifestSchema = z.object({ z.string(), z.object({ type: z.string(), + environment: z.string().optional(), properties: z .object({ tags: z.record(z.string(), z.string()).optional(), + templateFile: z.string().optional(), }) .optional(), }), @@ -23,12 +28,36 @@ const AssemblyManifestSchema = z.object({ .default({}), }); -/** Finds the one synthesized stack artifact tagged for the selected deployment target. */ -export async function stackArtifactIdForTarget( +const StackTemplateSchema = z + .object({ + Resources: z.record(z.string(), z.unknown()).default({}), + }) + .passthrough(); + +/** The synthesized stack a deploy selected, and where its template lives. */ +export interface StackArtifact { + /** Artifact id the CDK Toolkit selects the stack by. */ + id: string; + /** Assembly-relative path of the synthesized template. */ + templateFile: string | undefined; +} + +/** The account and region a deploy expects its stack to be bound to. */ +export interface StackEnvironment { + account: string; + region: string; +} + +/** + * Finds the one synthesized stack artifact tagged for the selected deployment + * target, and checks it is bound to the environment that target names. + */ +export async function stackArtifactForTarget( json: ReadWriteJson, assemblyDirectory: string, target: string, -): Promise { + expected: StackEnvironment, +): Promise { const manifestPath = join(assemblyDirectory, "manifest.json"); if (!existsSync(manifestPath)) { throw new ProjectStateError(`No synthesized cloud assembly was found at ${manifestPath}.`); @@ -54,5 +83,88 @@ export async function stackArtifactIdForTarget( `'${target}'. Exactly one stack must be tagged ${TARGET_TAG}='${target}'.`, ); } - return matches[0]![0]; + + const [id, artifact] = matches[0]!; + assertEnvironmentMatches(id, artifact.environment, expected); + return { id, templateFile: artifact.properties?.templateFile }; +} + +/** + * Refuses to deploy a synthesized template that declares no resources. + * + * The CDK Toolkit reads such a template as an instruction to *delete* an + * existing stack of that name, and reports the run as a normal success. Without + * this check `project deploy` is the only way to destroy a deployed stack, and + * there is no `project destroy` for a user to have asked for it with. + */ +export async function assertStackHasResources( + json: ReadWriteJson, + assemblyDirectory: string, + artifact: StackArtifact, +): Promise { + // The cloud assembly schema requires templateFile on a stack artifact, so its + // absence is a malformed assembly rather than a stack to deploy unchecked. + if (artifact.templateFile === undefined) { + throw new ProjectStateError( + `Stack artifact '${artifact.id}' names no template file in the cloud assembly manifest.`, + ); + } + + const templatePath = join(assemblyDirectory, artifact.templateFile); + if (!existsSync(templatePath)) { + throw new ProjectStateError( + `The synthesized template for stack '${artifact.id}' is missing from the cloud ` + + `assembly at ${templatePath}.`, + ); + } + + const template = await json.read(templatePath, StackTemplateSchema); + if (Object.keys(template.Resources).length === 0) { + throw new ProjectStateError( + `The synthesized stack '${artifact.id}' declares no resources, so deploying it would ` + + `delete the existing stack rather than update it. Check that the project spec still ` + + `declares the runtimes, gateways and memories it should before deploying.`, + ); + } +} + +// Both the target tag and the stack's environment derive from the same target in +// the synthesized app, so today they cannot disagree. Checking anyway keeps a +// correct tag from carrying a stack into the wrong account or region: the Toolkit +// deploys where the artifact's environment points, not where the tag says. +function assertEnvironmentMatches( + id: string, + environment: string | undefined, + expected: StackEnvironment, +): void { + // An artifact with no environment is environment-agnostic: it deploys into + // whatever the credentials resolve to, which the account preflight checked. + if (environment === undefined) return; + + const parsed = /^aws:\/\/([^/]+)\/(.+)$/.exec(environment); + if (!parsed) { + throw new ProjectStateError( + `Stack artifact '${id}' declares an unrecognized environment '${environment}'. ` + + `Expected the form aws:///.`, + ); + } + + const account = parsed[1]!; + const region = parsed[2]!; + // The unknown-* placeholders are the env-agnostic case spelled out. + const mismatches = [ + account !== UNKNOWN_ACCOUNT && account !== expected.account + ? `account ${account} (target expects ${expected.account})` + : undefined, + region !== UNKNOWN_REGION && region !== expected.region + ? `region ${region} (target expects ${expected.region})` + : undefined, + ].filter((mismatch) => mismatch !== undefined); + + if (mismatches.length > 0) { + throw new ProjectStateError( + `The synthesized stack '${id}' is built for ${mismatches.join(" and ")}. ` + + `Re-synthesize the project so its stack matches the deployment target.`, + ); + } } diff --git a/src/core/project/backends/cdk/toolkit.ts b/src/core/project/backends/cdk/toolkit.ts index 5bce6f503..3097d79d2 100644 --- a/src/core/project/backends/cdk/toolkit.ts +++ b/src/core/project/backends/cdk/toolkit.ts @@ -198,7 +198,8 @@ export async function performCdkOperation( // than one stack, so a missing result is not "no match": the Toolkit skips a // stack whose template has no resources, and *deletes* it if it already // exists. Both return normally, so reporting empty outputs here would call a - // deletion a successful deploy. + // deletion a successful deploy. assertStackHasResources rejects the known way + // into that state before we get here; this stays as the backstop for any other. if (result.stacks.length !== 1) { throw new AgentCoreCLIError( `The CDK Toolkit deployed no stack for '${operation.stackArtifactId}'. ` + From 3a222d4085b2a1b0fe22bfdf872a1e27d23c952c Mon Sep 17 00:00:00 2001 From: notgitika Date: Tue, 25 Aug 2026 18:11:07 -0400 Subject: [PATCH 2/5] feat(project): tear the stack down on a deploy of an empty project, with --yes The resource check added in the previous commit never fired. CDK writes an AWS::CDK::Metadata resource into every stack unless version reporting is disabled, so a project whose spec declares nothing still synthesizes a template with one resource in it: Object.keys(Resources).length === 0 is unreachable through the CLI. Verified against eight synthesized assemblies. Count only the resources the project asked for, and the check becomes real. Making it real needs somewhere for that deploy to go. main handles it -- an empty project plus deploy destroys the stack -- and refactor dropped that along the way, so refusing outright would trade a silent delete for a regression. Route it to an explicit toolkit.destroy() instead, gated on --yes, and report it as "Removed" rather than "Deployed" since the stack no longer exists. Destroying explicitly rather than deploying the empty template is what makes the outcome reportable: destroy fails loudly when the stack cannot be removed, where a deploy of an empty template succeeds either way. The existing guard in performCdkOperation stays as the backstop for reaching that state some other way. Two states are distinguished before anything is destroyed, both by probing CloudFormation for the stack: nothing to deploy and no stack to remove is a project that needs a resource added, not a teardown. Detection reads the synthesized template rather than counting spec collections the way main does, so a resource type added to the spec later is covered without anyone remembering to extend a list. Also corrects the comment nico flagged: an artifact with no environment is not the env-agnostic case -- CDK spells that out as aws://unknown-account/unknown-region -- it is a manifest field the cloud assembly schema leaves optional. --- src/core/project/backends/cdk.test.ts | 119 +++++++++++++++--- src/core/project/backends/cdk.ts | 66 +++++++++- .../project/backends/cdk/assembly.test.ts | 78 +++++++++--- src/core/project/backends/cdk/assembly.ts | 62 ++++++--- .../backends/cdk/deployedState.test.ts | 15 +++ .../project/backends/cdk/deployedState.ts | 16 +++ .../project/backends/cdk/environment.test.ts | 74 ++++++++++- src/core/project/backends/cdk/environment.ts | 47 ++++++- src/core/project/backends/cdk/toolkit.test.ts | 41 ++++++ src/core/project/backends/cdk/toolkit.ts | 30 +++-- src/core/project/backends/types.ts | 6 + src/core/project/manager.test.ts | 19 ++- src/core/project/manager.tsx | 5 +- src/handlers/project/deploy/index.test.ts | 33 ++++- src/handlers/project/deploy/index.ts | 16 ++- src/handlers/project/types.ts | 11 ++ 16 files changed, 562 insertions(+), 76 deletions(-) diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index f09204ba7..9a0144fc8 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -8,6 +8,7 @@ import { ProjectSpecSchema } from "../../../projectSchemas/project"; import { createSilentLogger } from "../../../testing"; import { CdkBackend } from "./cdk"; import { DEPLOYED_STATE_RELATIVE_PATH } from "./cdk/deployedState"; +import type { DeployBackendInput } from "./types"; import type { BootstrapState } from "./cdk/environment"; import type { CdkCredentialProvider, CdkOperation, CdkOutputs, CdkRunOptions } from "./cdk/toolkit"; @@ -17,6 +18,13 @@ const TARGET = { region: "us-east-1", } as const; +/** A template holding only what CDK adds itself, as an empty project synthesizes. */ +const METADATA_ONLY = { CDKMetadata: { Type: "AWS::CDK::Metadata" } }; + +function deployInput(overrides: Partial = {}): DeployBackendInput { + return { target: TARGET, confirmTeardown: false, ...overrides }; +} + const tempDirectories: string[] = []; afterEach(async () => { @@ -115,6 +123,8 @@ type HarnessOptions = { template?: boolean; failOperation?: CdkOperation["kind"]; bootstrapError?: Error; + /** Whether CloudFormation still holds the target's stack. Defaults to present. */ + stackExists?: boolean; }; function harness(options: HarnessOptions = {}) { @@ -125,6 +135,7 @@ function harness(options: HarnessOptions = {}) { const bootstrapCredentials: CdkCredentialProvider[] = []; const accountRegions: string[] = []; const bootstrapRegions: string[] = []; + const stackProbes: string[] = []; let templateLoads = 0; let templateCleanups = 0; const credentials: CdkCredentialProvider = async () => ({ @@ -153,6 +164,10 @@ function harness(options: HarnessOptions = {}) { if (options.bootstrapError) throw options.bootstrapError; return options.bootstrap ?? { kind: "current", version: 30 }; }, + stack: async (stackName) => { + stackProbes.push(stackName); + return options.stackExists ?? true; + }, cdk: async (operation, runOptions) => { runs.push({ operation, options: runOptions }); if (operation.kind === options.failOperation) { @@ -194,6 +209,7 @@ function harness(options: HarnessOptions = {}) { credentialRegions, credentials, runs, + stackProbes, templateLoads: () => templateLoads, templateCleanups: () => templateCleanups, }; @@ -255,7 +271,7 @@ describe("CdkBackend.deploy", () => { await writeAssembly(input, [TARGET.name]); const subject = harness({ outputs: { RuntimeArn: "arn:runtime" } }); - const deployed = await collectDeploy(subject.backend.deploy(input, { target: TARGET })); + const deployed = await collectDeploy(subject.backend.deploy(input, deployInput())); expect(deployed.events).toEqual([ { message: `Verifying AWS account ${TARGET.account}` }, @@ -293,7 +309,7 @@ describe("CdkBackend.deploy", () => { stackArn: "arn:aws:cloudformation:us-east-1:111122223333:stack/AgentCore-example-default/abc", }); - await collectDeploy(subject.backend.deploy(input, { target: TARGET })); + await collectDeploy(subject.backend.deploy(input, deployInput())); const statePath = join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH); expect(JSON.parse(await Bun.file(statePath).text())).toEqual({ @@ -311,7 +327,7 @@ describe("CdkBackend.deploy", () => { await writeAssembly(input, [TARGET.name]); const subject = harness({ outputs: { RuntimeArn: "arn:runtime" }, omitStackArn: true }); - await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).rejects.toThrow( + await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow( /without a stack ARN/, ); expect(existsSync(join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH))).toBe(false); @@ -324,27 +340,94 @@ describe("CdkBackend.deploy", () => { await writeFile(statePath, "{ not valid json"); const subject = harness({ outputs: { RuntimeArn: "arn:runtime" } }); - await expect( - collectDeploy(subject.backend.deploy(input, { target: TARGET })), - ).rejects.toThrow(); + await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow(); // Validated before synth/bootstrap/deploy, so nothing ran against AWS. expect(subject.commands).toEqual([]); expect(subject.runs).toEqual([]); }); - test("refuses a resource-less stack before the Toolkit can delete it", async () => { + test("will not remove a stack the user did not ask to remove", async () => { const input = await project(); await writeAssembly(input, [TARGET.name], { resources: {} }); const subject = harness(); - await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).rejects.toThrow( - /declares no resources/, + await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow( + /would delete stack 'AgentCore-example-default-0'.*--yes/s, ); // Nothing reached the Toolkit, so no stack was deleted and none bootstrapped. expect(subject.runs).toEqual([]); expect(subject.bootstrapRegions).toEqual([]); }); + test("treats a template holding only CDK's own metadata as nothing to deploy", async () => { + // An empty project still synthesizes this one resource, so a check for an + // empty Resources block would let the teardown case through as a deploy. + const input = await project(); + await writeAssembly(input, [TARGET.name], { resources: METADATA_ONLY }); + const subject = harness(); + + await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow( + /--yes/, + ); + expect(subject.runs).toEqual([]); + }); + + test("removes the stack when the teardown is confirmed", async () => { + const input = await project(); + await writeAssembly(input, [TARGET.name], { resources: METADATA_ONLY }); + const statePath = join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH); + await mkdir(dirname(statePath), { recursive: true }); + await writeFile( + statePath, + JSON.stringify({ + targets: { + default: { stackArn: "arn:stack:default" }, + prod: { stackArn: "arn:stack:prod" }, + }, + }), + ); + const subject = harness(); + + const deployed = await collectDeploy( + subject.backend.deploy(input, deployInput({ confirmTeardown: true })), + ); + + expect(deployed.result).toEqual({ outputs: {}, tornDown: true }); + expect(deployed.events).toContainEqual({ + message: "Removing stack AgentCore-example-default-0", + }); + // Destroyed explicitly, rather than by deploying an empty template and + // letting the Toolkit infer a deletion. + expect(subject.runs.map(({ operation }) => operation)).toEqual([ + { kind: "destroy", stackArtifactId: "AgentCore-example-default-0" }, + ]); + expect(subject.stackProbes).toEqual(["AgentCore-example-default-0"]); + expect(JSON.parse(await Bun.file(statePath).text())).toEqual({ + targets: { prod: { stackArn: "arn:stack:prod" } }, + }); + }); + + test("says to add a resource when there is no stack to remove either", async () => { + const input = await project(); + await writeAssembly(input, [TARGET.name], { resources: METADATA_ONLY }); + const subject = harness({ stackExists: false }); + + await expect( + collectDeploy(subject.backend.deploy(input, deployInput({ confirmTeardown: true }))), + ).rejects.toThrow(/no stack .* exists .* to remove.*Add a resource/s); + expect(subject.runs).toEqual([]); + }); + + test("does not probe for a stack when there is something to deploy", async () => { + const input = await project(); + await writeAssembly(input, [TARGET.name]); + const subject = harness(); + + await collectDeploy(subject.backend.deploy(input, deployInput())); + + expect(subject.stackProbes).toEqual([]); + }); + test("refuses a stack synthesized for a different region than the target", async () => { const input = await project(); await writeAssembly(input, [TARGET.name], { @@ -352,7 +435,7 @@ describe("CdkBackend.deploy", () => { }); const subject = harness(); - await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).rejects.toThrow( + await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow( /built for region us-west-2 \(target expects us-east-1\)/, ); expect(subject.runs).toEqual([]); @@ -366,7 +449,7 @@ describe("CdkBackend.deploy", () => { await writeAssembly(input, [TARGET.name]); const subject = harness({ bootstrap }); - const deployed = await collectDeploy(subject.backend.deploy(input, { target: TARGET })); + const deployed = await collectDeploy(subject.backend.deploy(input, deployInput())); expect(subject.runs.map(({ operation }) => operation)).toEqual([ { @@ -384,7 +467,7 @@ describe("CdkBackend.deploy", () => { const input = await project(); const subject = harness({ account: "999900001111" }); - await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).rejects.toThrow( + await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow( /expects AWS account 111122223333.*999900001111/, ); expect(subject.commands).toEqual([]); @@ -397,7 +480,7 @@ describe("CdkBackend.deploy", () => { await writeAssembly(input, ["other"]); const subject = harness(); - await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).rejects.toThrow( + await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow( /no stack for deployment target 'default'/, ); expect(subject.bootstrapRegions).toEqual([]); @@ -409,7 +492,7 @@ describe("CdkBackend.deploy", () => { await writeAssembly(input, [TARGET.name, TARGET.name]); const subject = harness(); - await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).rejects.toThrow( + await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow( /2 stacks for deployment target 'default'/, ); expect(subject.bootstrapRegions).toEqual([]); @@ -422,9 +505,7 @@ describe("CdkBackend.deploy", () => { const failure = new Error("CDKToolkit is UPDATE_IN_PROGRESS"); const subject = harness({ bootstrapError: failure }); - await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).rejects.toBe( - failure, - ); + await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toBe(failure); expect(subject.runs).toEqual([]); }); @@ -433,7 +514,7 @@ describe("CdkBackend.deploy", () => { await writeAssembly(input, [TARGET.name]); const subject = harness({ bootstrap: { kind: "absent" }, template: true }); - await collectDeploy(subject.backend.deploy(input, { target: TARGET })); + await collectDeploy(subject.backend.deploy(input, deployInput())); expect(subject.runs[0]?.operation).toEqual({ kind: "bootstrap", @@ -453,7 +534,7 @@ describe("CdkBackend.deploy", () => { failOperation: "bootstrap", }); - await expect(collectDeploy(subject.backend.deploy(input, { target: TARGET }))).rejects.toThrow( + await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow( "bootstrap failed", ); expect(subject.templateCleanups()).toBe(1); diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index b58fd7ff5..e8aeebb11 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -11,13 +11,19 @@ import { } from "../../../io"; import type { Logger } from "../../../logging"; import type { DeployBackendInput, ProjectBackend } from "./types"; -import { assertStackHasResources, stackArtifactForTarget } from "./cdk/assembly"; -import { readDeployedState, updateTargetState } from "./cdk/deployedState"; +import { + countDeployableResources, + stackArtifactForTarget, + type StackArtifact, +} from "./cdk/assembly"; +import { readDeployedState, removeTargetState, updateTargetState } from "./cdk/deployedState"; import { probeBootstrap, + probeStack, resolveAwsAccount, type AccountResolver, type BootstrapProbe, + type StackProbe, } from "./cdk/environment"; import { createCdkCredentialResolver, @@ -26,6 +32,7 @@ import { type BootstrapTemplateLoader, type CdkCredentialResolver, type CdkRunner, + type CdkRunOptions, } from "./cdk/toolkit"; export type CdkBackendConfig = { @@ -36,6 +43,7 @@ export type CdkBackendConfig = { cdk?: CdkRunner; resolveCredentials?: CdkCredentialResolver; bootstrap?: BootstrapProbe; + stack?: StackProbe; resolveAccount?: AccountResolver; loadBootstrapTemplate?: BootstrapTemplateLoader; }; @@ -49,6 +57,7 @@ export class CdkBackend implements ProjectBackend { private readonly cdk: CdkRunner; private readonly resolveCredentials: CdkCredentialResolver; private readonly bootstrap: BootstrapProbe; + private readonly stack: StackProbe; private readonly resolveAccount: AccountResolver; private readonly loadBootstrapTemplate: BootstrapTemplateLoader; @@ -61,6 +70,7 @@ export class CdkBackend implements ProjectBackend { this.resolveCredentials = config.resolveCredentials ?? createCdkCredentialResolver(config.logger); this.bootstrap = config.bootstrap ?? probeBootstrap; + this.stack = config.stack ?? probeStack; this.resolveAccount = config.resolveAccount ?? resolveAwsAccount; this.loadBootstrapTemplate = config.loadBootstrapTemplate ?? loadBootstrapTemplate; } @@ -112,11 +122,15 @@ export class CdkBackend implements ProjectBackend { account: target.account, region: target.region, }); - // Checked here rather than after the fact: the Toolkit deletes an existing - // stack whose new template has no resources, and returns as if it deployed. - await assertStackHasResources(this.json, assemblyDirectory, artifact); const options = { assemblyDirectory, credentials, region: target.region }; + // Decided before the Toolkit is handed the assembly: it reads a template + // with nothing in it as an instruction to delete the stack, and reports that + // as an ordinary successful deploy. + if ((await countDeployableResources(this.json, assemblyDirectory, artifact)) === 0) { + return yield* this.teardown({ project, artifact, input, options }); + } + const bootstrap = await this.bootstrap(target.region, credentials); this.logger .child({ @@ -168,6 +182,48 @@ export class CdkBackend implements ProjectBackend { return { outputs }; } + /** + * Removes the target's stack, for a deploy of a project that declares nothing + * to deploy. + * + * Destroying explicitly rather than letting the Toolkit infer it from an empty + * template is what makes this reportable: `destroy` fails loudly if the stack + * cannot be removed, where a deploy of an empty template succeeds either way. + */ + private async *teardown({ + project, + artifact, + input, + options, + }: { + project: Project; + artifact: StackArtifact; + input: DeployBackendInput; + options: CdkRunOptions; + }): AsyncGenerator { + const { target } = input; + if (!(await this.stack(artifact.stackName, target.region, options.credentials))) { + throw new ProjectStateError( + `Project '${project.name}' declares no resources to deploy, and no stack ` + + `'${artifact.stackName}' exists in ${target.account}/${target.region} to remove. ` + + `Add a resource — for example 'agentcore project add runtime' — before deploying.`, + ); + } + + if (!input.confirmTeardown) { + throw new ProjectStateError( + `Project '${project.name}' declares no resources to deploy, so deploying to target ` + + `'${target.name}' would delete stack '${artifact.stackName}' and every resource in ` + + `it. Re-run with --yes to confirm, or restore the resources the project should have.`, + ); + } + + yield { message: `Removing stack ${artifact.stackName}` }; + await this.cdk({ kind: "destroy", stackArtifactId: artifact.id }, options); + await removeTargetState(this.json, project.rootPath, target.name); + return { outputs: {}, tornDown: true }; + } + private cdkDirectory(project: Project): string { return join(project.rootPath, "agentcore", "cdk"); } diff --git a/src/core/project/backends/cdk/assembly.test.ts b/src/core/project/backends/cdk/assembly.test.ts index 8e2cb7775..d43ce1ed6 100644 --- a/src/core/project/backends/cdk/assembly.test.ts +++ b/src/core/project/backends/cdk/assembly.test.ts @@ -4,7 +4,7 @@ import { join } from "node:path"; import { tmpdir } from "node:os"; import { FsReadWriteJson } from "../../../../io"; import { createSilentLogger } from "../../../../testing"; -import { assertStackHasResources, stackArtifactForTarget } from "./assembly"; +import { countDeployableResources, stackArtifactForTarget } from "./assembly"; const temporaryDirectories: string[] = []; const json = new FsReadWriteJson({ logger: createSilentLogger() }); @@ -50,6 +50,27 @@ describe("stackArtifactForTarget", () => { expect(await stackArtifactForTarget(json, directory, "prod", EXPECTED)).toEqual({ id: "nested/stack-id", + // No stackName in the manifest, so CloudFormation knows the stack by its + // artifact id — the same fallback CDK itself applies. + stackName: "nested/stack-id", + templateFile: TEMPLATE_FILE, + }); + }); + + test("prefers the manifest's stack name over the artifact id", async () => { + const directory = await assembly({ + "nested/stack-id": stackArtifact("prod", { + properties: { + tags: { "agentcore:target-name": "prod" }, + templateFile: TEMPLATE_FILE, + stackName: "AgentCore-example-prod", + }, + }), + }); + + expect(await stackArtifactForTarget(json, directory, "prod", EXPECTED)).toEqual({ + id: "nested/stack-id", + stackName: "AgentCore-example-prod", templateFile: TEMPLATE_FILE, }); }); @@ -131,38 +152,65 @@ describe("stackArtifactForTarget", () => { }); }); -describe("assertStackHasResources", () => { - test("accepts a template that declares resources", async () => { +describe("countDeployableResources", () => { + const artifact = { id: "Stack", stackName: "Stack", templateFile: TEMPLATE_FILE }; + + test("counts the resources the project asked for", async () => { const directory = await assembly({ Stack: stackArtifact("prod") }); - await writeTemplate(directory, { Runtime: { Type: "AWS::BedrockAgentCore::Runtime" } }); + await writeTemplate(directory, { + Runtime: { Type: "AWS::BedrockAgentCore::Runtime" }, + Role: { Type: "AWS::IAM::Role" }, + }); - expect( - await assertStackHasResources(json, directory, { id: "Stack", templateFile: TEMPLATE_FILE }), - ).toBeUndefined(); + expect(await countDeployableResources(json, directory, artifact)).toBe(2); }); - test("rejects a resource-less template, which the Toolkit reads as a deletion", async () => { + test("does not count the metadata resource CDK adds on its own", async () => { + // The reason a check for an empty Resources block never fires: an empty + // project still synthesizes this. + const directory = await assembly({ Stack: stackArtifact("prod") }); + await writeTemplate(directory, { CDKMetadata: { Type: "AWS::CDK::Metadata" } }); + + expect(await countDeployableResources(json, directory, artifact)).toBe(0); + }); + + test("counts a real resource sitting alongside the metadata one", async () => { + const directory = await assembly({ Stack: stackArtifact("prod") }); + await writeTemplate(directory, { + CDKMetadata: { Type: "AWS::CDK::Metadata" }, + Runtime: { Type: "AWS::BedrockAgentCore::Runtime" }, + }); + + expect(await countDeployableResources(json, directory, artifact)).toBe(1); + }); + + test("counts an untyped resource, rather than reading a broken template as empty", async () => { + const directory = await assembly({ Stack: stackArtifact("prod") }); + await writeTemplate(directory, { Mystery: {} }); + + expect(await countDeployableResources(json, directory, artifact)).toBe(1); + }); + + test("reports an empty Resources block as nothing to deploy", async () => { const directory = await assembly({ Stack: stackArtifact("prod") }); await writeTemplate(directory, {}); - await expect( - assertStackHasResources(json, directory, { id: "Stack", templateFile: TEMPLATE_FILE }), - ).rejects.toThrow(/declares no resources, so deploying it would delete the existing stack/); + expect(await countDeployableResources(json, directory, artifact)).toBe(0); }); test("rejects a template the assembly names but does not contain", async () => { const directory = await assembly({ Stack: stackArtifact("prod") }); - await expect( - assertStackHasResources(json, directory, { id: "Stack", templateFile: TEMPLATE_FILE }), - ).rejects.toThrow(/synthesized template for stack 'Stack' is missing/); + await expect(countDeployableResources(json, directory, artifact)).rejects.toThrow( + /synthesized template for stack 'Stack' is missing/, + ); }); test("rejects a stack artifact that names no template at all", async () => { const directory = await assembly({ Stack: stackArtifact("prod") }); await expect( - assertStackHasResources(json, directory, { id: "Stack", templateFile: undefined }), + countDeployableResources(json, directory, { ...artifact, templateFile: undefined }), ).rejects.toThrow(/names no template file/); }); }); diff --git a/src/core/project/backends/cdk/assembly.ts b/src/core/project/backends/cdk/assembly.ts index 288a36d10..38af080bb 100644 --- a/src/core/project/backends/cdk/assembly.ts +++ b/src/core/project/backends/cdk/assembly.ts @@ -9,6 +9,8 @@ const STACK_ARTIFACT = "aws:cloudformation:stack"; /** What CDK writes in an artifact's `environment` when the stack is env-agnostic. */ const UNKNOWN_ACCOUNT = "unknown-account"; const UNKNOWN_REGION = "unknown-region"; +/** The resource CDK adds to every stack of its own accord, which no user asked for. */ +const METADATA_RESOURCE_TYPE = "AWS::CDK::Metadata"; const AssemblyManifestSchema = z.object({ artifacts: z @@ -21,6 +23,7 @@ const AssemblyManifestSchema = z.object({ .object({ tags: z.record(z.string(), z.string()).optional(), templateFile: z.string().optional(), + stackName: z.string().optional(), }) .optional(), }), @@ -30,7 +33,15 @@ const AssemblyManifestSchema = z.object({ const StackTemplateSchema = z .object({ - Resources: z.record(z.string(), z.unknown()).default({}), + Resources: z + .record( + z.string(), + // An untyped resource is a malformed template rather than a metadata + // resource, so leaving Type optional counts it as deployable and keeps + // the teardown path from claiming such a stack is empty. + z.object({ Type: z.string().optional() }).passthrough(), + ) + .default({}), }) .passthrough(); @@ -38,6 +49,8 @@ const StackTemplateSchema = z export interface StackArtifact { /** Artifact id the CDK Toolkit selects the stack by. */ id: string; + /** Name CloudFormation knows the deployed stack by. */ + stackName: string; /** Assembly-relative path of the synthesized template. */ templateFile: string | undefined; } @@ -86,22 +99,36 @@ export async function stackArtifactForTarget( const [id, artifact] = matches[0]!; assertEnvironmentMatches(id, artifact.environment, expected); - return { id, templateFile: artifact.properties?.templateFile }; + return { + id, + // Mirrors how CDK itself resolves a stack artifact's physical name + // (`properties.stackName || artifactId`), so the name we hand CloudFormation + // is the one the Toolkit would have used. + stackName: artifact.properties?.stackName ?? id, + templateFile: artifact.properties?.templateFile, + }; } /** - * Refuses to deploy a synthesized template that declares no resources. + * Counts the resources in a synthesized template that the project actually asked + * for, so a deploy can tell "update this stack" from "tear it down". * - * The CDK Toolkit reads such a template as an instruction to *delete* an - * existing stack of that name, and reports the run as a normal success. Without - * this check `project deploy` is the only way to destroy a deployed stack, and - * there is no `project destroy` for a user to have asked for it with. + * `AWS::CDK::Metadata` is excluded because CDK adds it to every stack unless + * version reporting is disabled: a project whose spec declares nothing still + * synthesizes a template holding that one resource. Counting raw resources would + * therefore never reach zero through the CLI, so a check for an empty + * `Resources` block would never fire — and the interesting question is whether + * anything the user asked for is left, not whether CDK's own bookkeeping is. + * + * Reading the template rather than the spec keeps this honest as the spec grows: + * a resource type added later shows up here without anyone remembering to extend + * a list of collections to check. */ -export async function assertStackHasResources( +export async function countDeployableResources( json: ReadWriteJson, assemblyDirectory: string, artifact: StackArtifact, -): Promise { +): Promise { // The cloud assembly schema requires templateFile on a stack artifact, so its // absence is a malformed assembly rather than a stack to deploy unchecked. if (artifact.templateFile === undefined) { @@ -119,13 +146,9 @@ export async function assertStackHasResources( } const template = await json.read(templatePath, StackTemplateSchema); - if (Object.keys(template.Resources).length === 0) { - throw new ProjectStateError( - `The synthesized stack '${artifact.id}' declares no resources, so deploying it would ` + - `delete the existing stack rather than update it. Check that the project spec still ` + - `declares the runtimes, gateways and memories it should before deploying.`, - ); - } + return Object.values(template.Resources).filter( + (resource) => resource.Type !== METADATA_RESOURCE_TYPE, + ).length; } // Both the target tag and the stack's environment derive from the same target in @@ -137,8 +160,11 @@ function assertEnvironmentMatches( environment: string | undefined, expected: StackEnvironment, ): void { - // An artifact with no environment is environment-agnostic: it deploys into - // whatever the credentials resolve to, which the account preflight checked. + // Not the env-agnostic case — CDK spells that out as + // aws://unknown-account/unknown-region, handled below. `environment` is + // optional in the cloud assembly schema, so a stack artifact without one is a + // manifest we cannot draw any conclusion from; the account preflight has + // already confirmed the credentials point at the target account either way. if (environment === undefined) return; const parsed = /^aws:\/\/([^/]+)\/(.+)$/.exec(environment); diff --git a/src/core/project/backends/cdk/deployedState.test.ts b/src/core/project/backends/cdk/deployedState.test.ts index 32bac0a33..6f4630140 100644 --- a/src/core/project/backends/cdk/deployedState.test.ts +++ b/src/core/project/backends/cdk/deployedState.test.ts @@ -8,6 +8,7 @@ import { createSilentLogger } from "../../../../testing"; import { DEPLOYED_STATE_RELATIVE_PATH, readDeployedState, + removeTargetState, updateTargetState, } from "./deployedState"; @@ -150,3 +151,17 @@ describe("updateTargetState", () => { }); }); }); + +describe("removeTargetState", () => { + test("removes only the destroyed target", async () => { + const root = await projectRoot(); + await updateTargetState(json, root, "default", { stackArn: "arn:stack:default" }); + await updateTargetState(json, root, "prod", { stackArn: "arn:stack:prod" }); + + await removeTargetState(json, root, "default"); + + expect(await readRaw(root)).toEqual({ + targets: { prod: { stackArn: "arn:stack:prod" } }, + }); + }); +}); diff --git a/src/core/project/backends/cdk/deployedState.ts b/src/core/project/backends/cdk/deployedState.ts index b1edc215a..a655c68b7 100644 --- a/src/core/project/backends/cdk/deployedState.ts +++ b/src/core/project/backends/cdk/deployedState.ts @@ -117,3 +117,19 @@ export async function updateTargetState( await atomicWrite(statePath, JSON.stringify(next, undefined, 2)); return next; } + +/** Removes a target's state after its CloudFormation stack is destroyed. */ +export async function removeTargetState( + json: ReadWriteJson, + projectRoot: string, + targetName: string, +): Promise { + const statePath = statePathFor(projectRoot); + const state = await readDeployedState(json, projectRoot); + if (!(targetName in state.targets)) return state; + + const { [targetName]: _removed, ...targets } = state.targets; + const next = { ...state, targets }; + await atomicWrite(statePath, JSON.stringify(next, undefined, 2)); + return next; +} diff --git a/src/core/project/backends/cdk/environment.test.ts b/src/core/project/backends/cdk/environment.test.ts index fddbbaa63..1377f3820 100644 --- a/src/core/project/backends/cdk/environment.test.ts +++ b/src/core/project/backends/cdk/environment.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import type { Stack } from "@aws-sdk/client-cloudformation"; -import { isStackNotFound, probeBootstrap, readBootstrapState } from "./environment"; +import { isStackNotFound, probeBootstrap, probeStack, readBootstrapState } from "./environment"; import type { CdkCredentialProvider } from "./toolkit"; const credentials: CdkCredentialProvider = async () => ({ @@ -110,3 +110,75 @@ describe("probeBootstrap", () => { expect(providers).toEqual([credentials]); }); }); + +describe("probeStack", () => { + test("reports a stack CloudFormation still holds as present", async () => { + expect( + await probeStack("AgentCore-orders-default", "us-east-1", credentials, async () => [ + stack("CREATE_COMPLETE"), + ]), + ).toBe(true); + }); + + test.each([ + // A stack part-way through a failed change is still a stack the user needs a + // way to remove, so status must not narrow this to "healthy stacks only". + "ROLLBACK_COMPLETE", + "UPDATE_ROLLBACK_FAILED", + "DELETE_FAILED", + ] as const)("counts a stack in %s as present", async (status) => { + expect( + await probeStack("AgentCore-orders-default", "us-east-1", credentials, async () => [ + stack(status), + ]), + ).toBe(true); + }); + + test("reports a stack CloudFormation does not know about as absent", async () => { + expect( + await probeStack("AgentCore-orders-default", "us-east-1", credentials, async () => { + throw Object.assign(new Error("Stack with id AgentCore-orders-default does not exist"), { + name: "ValidationError", + }); + }), + ).toBe(false); + }); + + test("treats an empty response as absent rather than crashing", async () => { + expect( + await probeStack("AgentCore-orders-default", "us-east-1", credentials, async () => undefined), + ).toBe(false); + }); + + // Reporting "no stack" on a permissions or throttling failure would turn a + // teardown the user confirmed into an unexplained "add a resource" error. + test("propagates a failure that is not a missing stack", async () => { + const failure = Object.assign(new Error("User is not authorized"), { + name: "AccessDeniedException", + }); + + await expect( + probeStack("AgentCore-orders-default", "us-east-1", credentials, async () => { + throw failure; + }), + ).rejects.toBe(failure); + }); + + test("looks the stack up by name in the target region with the deployment credentials", async () => { + const reads: { stackName: string; region: string; provider: CdkCredentialProvider }[] = []; + + await probeStack( + "AgentCore-orders-prod", + "eu-west-1", + credentials, + async (stackName, region, provider) => { + reads.push({ stackName, region, provider }); + return [stack("CREATE_COMPLETE")]; + }, + ); + + expect(reads).toEqual([ + { stackName: "AgentCore-orders-prod", region: "eu-west-1", provider: credentials }, + ]); + }); +}); diff --git a/src/core/project/backends/cdk/environment.ts b/src/core/project/backends/cdk/environment.ts index 55a6667b9..16b9ffe11 100644 --- a/src/core/project/backends/cdk/environment.ts +++ b/src/core/project/backends/cdk/environment.ts @@ -27,6 +27,16 @@ export type AccountResolver = ( region: string, credentials: CdkCredentialProvider, ) => Promise; +export type StackReader = ( + stackName: string, + region: string, + credentials: CdkCredentialProvider, +) => Promise; +export type StackProbe = ( + stackName: string, + region: string, + credentials: CdkCredentialProvider, +) => Promise; export function readBootstrapState(stacks?: Stack[]): Exclude { const stack = stacks?.[0]; @@ -60,7 +70,7 @@ export function readBootstrapState(stacks?: Stack[]): Exclude { + const { CloudFormationClient, DescribeStacksCommand } = + await import("@aws-sdk/client-cloudformation"); + const client = new CloudFormationClient({ credentials, region }); + try { + const response = await client.send(new DescribeStacksCommand({ StackName: stackName })); + return response.Stacks; + } finally { + client.destroy(); + } +}; + +/** + * Whether CloudFormation still holds a stack of this name, so a deploy with + * nothing left to deploy can tell "tear the stack down" from "there was never + * anything here". + * + * Any stack CloudFormation returns counts as present, whatever its status: a + * stack stuck mid-rollback is still a stack the user needs a way to remove. + * Deleted stacks are not returned when looked up by name, only by id. + */ +export async function probeStack( + stackName: string, + region: string, + credentials: CdkCredentialProvider, + read: StackReader = describeStack, +): Promise { + try { + return ((await read(stackName, region, credentials)) ?? []).length > 0; + } catch (error) { + if (isStackNotFound(error)) return false; + throw error; + } +} + export const resolveAwsAccount: AccountResolver = async (region, credentials) => { const { GetCallerIdentityCommand, STSClient } = await import("@aws-sdk/client-sts"); const client = new STSClient({ credentials, region }); diff --git a/src/core/project/backends/cdk/toolkit.test.ts b/src/core/project/backends/cdk/toolkit.test.ts index 171e07fc3..6fbfc5828 100644 --- a/src/core/project/backends/cdk/toolkit.test.ts +++ b/src/core/project/backends/cdk/toolkit.test.ts @@ -78,6 +78,10 @@ function loadedToolkit(stacks: unknown[] = [DEPLOYED_STACK]) { calls.push({ method: "deploy", args }); return { stacks } as never; }, + destroy: async (...args: Parameters) => { + calls.push({ method: "destroy", args }); + return { stacks: [] } as never; + }, }; return { calls, loaded: { lib: toolkitLib, toolkit } as LoadedCdkToolkit }; } @@ -190,6 +194,43 @@ describe("performCdkOperation", () => { ); }); + test("destroys exactly one named stack from the synthesized assembly", async () => { + const { calls, loaded } = loadedToolkit(); + + expect( + await performCdkOperation( + loaded, + { kind: "destroy", stackArtifactId: "AgentCore-orders-default" }, + runOptions({ assemblyDirectory: "/workspace/agentcore/cdk/cdk.out" }), + ), + ).toEqual({ outputs: {} }); + + // Never deploy: an empty template would also delete the stack, but reports + // success whether or not the deletion worked. + expect(calls.map(({ method }) => method)).toEqual(["fromAssemblyDirectory", "destroy"]); + expect(calls[0]!.args).toEqual(["/workspace/agentcore/cdk/cdk.out"]); + expect(calls[1]!.args[1]).toMatchObject({ + stacks: { + strategy: toolkitLib.StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE, + patterns: ["AgentCore-orders-default"], + }, + }); + }); + + // The empty `stacks` a destroy returns is the expected shape, not the missing + // stack the deploy path refuses. + test("does not read a destroy's empty stack list as a failure", async () => { + const { loaded } = loadedToolkit([]); + + expect( + await performCdkOperation( + loaded, + { kind: "destroy", stackArtifactId: "AgentCore-orders-default" }, + runOptions(), + ), + ).toEqual({ outputs: {} }); + }); + test("accepts a deployed stack that declares no outputs", async () => { const { loaded } = loadedToolkit([{ ...DEPLOYED_STACK, outputs: {} }]); diff --git a/src/core/project/backends/cdk/toolkit.ts b/src/core/project/backends/cdk/toolkit.ts index 3097d79d2..d747117c2 100644 --- a/src/core/project/backends/cdk/toolkit.ts +++ b/src/core/project/backends/cdk/toolkit.ts @@ -13,7 +13,8 @@ import type { Logger } from "../../../../logging"; export type CdkOperation = | { kind: "bootstrap"; environments: string[]; templateFile?: string } - | { kind: "deploy"; stackArtifactId: string }; + | { kind: "deploy"; stackArtifactId: string } + | { kind: "destroy"; stackArtifactId: string }; export type CdkRunOptions = { /** Synthesized cloud assembly used by deploy operations. */ @@ -36,7 +37,10 @@ export type CdkRunResult = { outputs: CdkOutputs; stackArn?: string }; export type CdkRunner = (operation: CdkOperation, options: CdkRunOptions) => Promise; -export type CdkToolkit = Pick; +export type CdkToolkit = Pick< + Toolkit, + "bootstrap" | "deploy" | "destroy" | "fromAssemblyDirectory" +>; export type CdkToolkitLib = Pick< typeof import("@aws-cdk/toolkit-lib"), @@ -187,19 +191,25 @@ export async function performCdkOperation( } const source = await toolkit.fromAssemblyDirectory(options.assemblyDirectory); - const result = await toolkit.deploy(source, { - stacks: { - strategy: lib.StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE, - patterns: [operation.stackArtifactId], - }, - }); + const stacks = { + strategy: lib.StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE, + patterns: [operation.stackArtifactId], + }; + + if (operation.kind === "destroy") { + await toolkit.destroy(source, { stacks }); + return { outputs: {} }; + } + + const result = await toolkit.deploy(source, { stacks }); // PATTERN_MUST_MATCH_SINGLE throws when the pattern matches anything other // than one stack, so a missing result is not "no match": the Toolkit skips a // stack whose template has no resources, and *deletes* it if it already // exists. Both return normally, so reporting empty outputs here would call a - // deletion a successful deploy. assertStackHasResources rejects the known way - // into that state before we get here; this stays as the backstop for any other. + // deletion a successful deploy. A deploy that would empty the stack is routed + // to an explicit, confirmed destroy before reaching this call, so this is the + // backstop for arriving in that state some other way. if (result.stacks.length !== 1) { throw new AgentCoreCLIError( `The CDK Toolkit deployed no stack for '${operation.stackArtifactId}'. ` + diff --git a/src/core/project/backends/types.ts b/src/core/project/backends/types.ts index eebeccf29..d3b1d6fac 100644 --- a/src/core/project/backends/types.ts +++ b/src/core/project/backends/types.ts @@ -4,6 +4,12 @@ import type { AwsDeploymentTarget } from "../../../projectSchemas/aws-targets"; export type DeployBackendInput = { /** Fully resolved account and region selected from aws-targets.json. */ target: AwsDeploymentTarget; + /** + * Permission to tear the target's stack down when the project no longer + * declares anything to deploy. Withheld by default: that deploy destroys + * deployed resources, so it takes saying so. + */ + confirmTeardown: boolean; }; /** Builds the deployable artifacts owned by a project's selected backend. */ diff --git a/src/core/project/manager.test.ts b/src/core/project/manager.test.ts index 45997ff8e..7e1e92203 100644 --- a/src/core/project/manager.test.ts +++ b/src/core/project/manager.test.ts @@ -385,8 +385,9 @@ describe("FsProjectManager.deploy", () => { manager: FsProjectManager, project: Project, target: string, + confirmTeardown = false, ): Promise<{ events: ProjectEvent[]; result: DeployResult }> { - const generator = manager.deploy(project, { target }); + const generator = manager.deploy(project, { target, confirmTeardown }); const events: ProjectEvent[] = []; while (true) { const next = await generator.next(); @@ -415,13 +416,27 @@ describe("FsProjectManager.deploy", () => { const deployed = await deploy(subject.manager, project, "prod"); - expect(subject.calls).toEqual([{ project, input: { target: targets[1]! } }]); + expect(subject.calls).toEqual([ + { project, input: { target: targets[1]!, confirmTeardown: false } }, + ]); expect(deployed.events).toEqual([{ message: "Backend deployment started" }]); expect(deployed.result).toEqual({ outputs: { RuntimeArn: "arn:runtime" }, }); }); + // The backend decides whether a deploy of nothing is a teardown; the manager's + // job is only to carry the user's confirmation through to it. + test("passes the teardown confirmation through to the backend", async () => { + const root = await inTempDirectory(); + const subject = deployManager(); + const project = await projectWithTargets(root, targets); + + await deploy(subject.manager, project, "prod", true); + + expect(subject.calls.map(({ input }) => input.confirmTeardown)).toEqual([true]); + }); + test("rejects an unknown target before invoking the backend", async () => { const root = await inTempDirectory(); const subject = deployManager(); diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 042c6a13a..4b5a2c514 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -477,7 +477,10 @@ export class FsProjectManager implements ProjectManager { ); } - return yield* this.backendFor(project).deploy(project, { target }); + return yield* this.backendFor(project).deploy(project, { + target, + confirmTeardown: input.confirmTeardown, + }); } private backendFor(project: Project): ProjectBackend { diff --git a/src/handlers/project/deploy/index.test.ts b/src/handlers/project/deploy/index.test.ts index 6384195b7..9cdca330f 100644 --- a/src/handlers/project/deploy/index.test.ts +++ b/src/handlers/project/deploy/index.test.ts @@ -104,7 +104,9 @@ describe("project deploy handler", () => { await subject.run(); - expect(subject.calls.map(({ input }) => input)).toEqual([{ target: DEFAULT_TARGET }]); + expect(subject.calls.map(({ input }) => input)).toEqual([ + { target: DEFAULT_TARGET, confirmTeardown: false }, + ]); expect(subject.io.stderr()).toContain("Preparing deployment\nDeploying stack"); expect(subject.io.stderr()).toContain("Deployed project 'orders' to target 'default'"); expect(subject.io.stdout()).toBe("AlphaArn: arn:alpha\nZetaUrl: https://zeta.example"); @@ -117,10 +119,37 @@ describe("project deploy handler", () => { await subject.run(["--target", "staging", "--json"]); - expect(subject.calls.map(({ input }) => input)).toEqual([{ target: STAGING_TARGET }]); + expect(subject.calls.map(({ input }) => input)).toEqual([ + { target: STAGING_TARGET, confirmTeardown: false }, + ]); expect(JSON.parse(subject.io.stdout())).toEqual(result); }); + // --yes is the only way to authorize the teardown the backend refuses without + // it, so a flag that never reaches the backend would make it unreachable. + test("carries --yes through as permission to tear the stack down", async () => { + const subject = testDeployCommand({ outputs: {} }); + await inProjectWithTargets(subject); + + await subject.run(["--yes"]); + + expect(subject.calls.map(({ input }) => input.confirmTeardown)).toEqual([true]); + }); + + test("says the project was removed when the deploy tore the stack down", async () => { + const subject = testDeployCommand({ outputs: {}, tornDown: true }, [ + { message: "Removing stack AgentCore-orders-default" }, + ]); + await inProjectWithTargets(subject); + + await subject.run(["--yes"]); + + expect(subject.io.stderr()).toContain("Removing stack AgentCore-orders-default"); + expect(subject.io.stderr()).toContain("Removed project 'orders' from target 'default'"); + // "Deployed" would be the wrong word for a stack that no longer exists. + expect(subject.io.stderr()).not.toContain("Deployed project"); + }); + test("rejects an unknown target without invoking the backend", async () => { const subject = testDeployCommand({ outputs: {} }); await inProjectWithTargets(subject); diff --git a/src/handlers/project/deploy/index.ts b/src/handlers/project/deploy/index.ts index 50fda3346..6d3eca915 100644 --- a/src/handlers/project/deploy/index.ts +++ b/src/handlers/project/deploy/index.ts @@ -16,6 +16,11 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = description: "deploy the project to AWS", flags: [ flag("target", "name of the aws-targets.json entry to deploy", z.string().default("default")), + flag( + "yes", + "confirm removing the target's stack when the project declares nothing to deploy", + z.boolean().default(false), + ), ], handle: async (ctx, flags) => { // withProject has already resolved the enclosing project. @@ -24,7 +29,10 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = // Progress goes to stderr, keeping stdout for machine output. Driven by // hand rather than `for await` because the outputs we render below are the // generator's return value, which `for await` discards. - const deployment = config.projectManager.deploy(project, { target: flags.target }); + const deployment = config.projectManager.deploy(project, { + target: flags.target, + confirmTeardown: flags.yes, + }); let next = await deployment.next(); while (!next.done) { config.io.stderr.write(`${next.value.message}\n`); @@ -32,7 +40,11 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = } const result = next.value; - config.io.stderr.write(`Deployed project '${project.name}' to target '${flags.target}'\n`); + config.io.stderr.write( + result.tornDown + ? `Removed project '${project.name}' from target '${flags.target}'\n` + : `Deployed project '${project.name}' to target '${flags.target}'\n`, + ); if (ctx.require(JsonKey)) { ctx.require(JsonRendererKey).renderJson(result); return; diff --git a/src/handlers/project/types.ts b/src/handlers/project/types.ts index c96b2e7af..a098a8cc1 100644 --- a/src/handlers/project/types.ts +++ b/src/handlers/project/types.ts @@ -108,6 +108,11 @@ export type ProjectEvent = { export type DeployProjectInput = { /** Name of the aws-targets.json entry to deploy. */ target: string; + /** + * Permission to tear the target's stack down when the project no longer + * declares anything to deploy. Withheld by default. + */ + confirmTeardown: boolean; }; export type DeployResult = { @@ -119,6 +124,12 @@ export type DeployResult = { * map rather than indexing into it. */ outputs: Record; + /** + * Set when the deploy removed the target's stack instead of updating it, + * because the project no longer declares anything to deploy. Callers report + * this differently: "deployed" is the wrong word for what happened. + */ + tornDown?: boolean; }; export type ResolveProjectInput = { From 975a25c7f47e4a4bf0d8163b5f76a22d99bb0250 Mon Sep 17 00:00:00 2001 From: notgitika Date: Thu, 27 Aug 2026 15:56:26 -0400 Subject: [PATCH 3/5] refactor(project): remove redundant stack environment check --- src/core/project/backends/cdk.test.ts | 16 ----- src/core/project/backends/cdk.ts | 5 +- .../project/backends/cdk/assembly.test.ts | 63 ++----------------- src/core/project/backends/cdk/assembly.ts | 61 +----------------- 4 files changed, 6 insertions(+), 139 deletions(-) diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index 9a0144fc8..bc6e7123e 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -60,8 +60,6 @@ async function project(withDependencies = true): Promise { } type AssemblyOptions = { - /** Overrides the environment every stack artifact is synthesized for. */ - environment?: string; /** Overrides the resources every stack's template declares. */ resources?: Record; }; @@ -101,7 +99,6 @@ async function writeAssembly( stack.id, { type: "aws:cloudformation:stack", - environment: options.environment ?? `aws://${TARGET.account}/${TARGET.region}`, properties: { templateFile: stack.templateFile, tags: { "agentcore:target-name": stack.target }, @@ -428,19 +425,6 @@ describe("CdkBackend.deploy", () => { expect(subject.stackProbes).toEqual([]); }); - test("refuses a stack synthesized for a different region than the target", async () => { - const input = await project(); - await writeAssembly(input, [TARGET.name], { - environment: `aws://${TARGET.account}/us-west-2`, - }); - const subject = harness(); - - await expect(collectDeploy(subject.backend.deploy(input, deployInput()))).rejects.toThrow( - /built for region us-west-2 \(target expects us-east-1\)/, - ); - expect(subject.runs).toEqual([]); - }); - test.each([ ["absent", { kind: "absent" } as const], ["outdated", { kind: "outdated", version: 29 } as const], diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index e8aeebb11..2bbfc20f0 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -118,10 +118,7 @@ export class CdkBackend implements ProjectBackend { yield* this.build(project); const assemblyDirectory = this.assemblyDirectory(project); - const artifact = await stackArtifactForTarget(this.json, assemblyDirectory, target.name, { - account: target.account, - region: target.region, - }); + const artifact = await stackArtifactForTarget(this.json, assemblyDirectory, target.name); const options = { assemblyDirectory, credentials, region: target.region }; // Decided before the Toolkit is handed the assembly: it reads a template diff --git a/src/core/project/backends/cdk/assembly.test.ts b/src/core/project/backends/cdk/assembly.test.ts index d43ce1ed6..1b1d32348 100644 --- a/src/core/project/backends/cdk/assembly.test.ts +++ b/src/core/project/backends/cdk/assembly.test.ts @@ -9,7 +9,6 @@ import { countDeployableResources, stackArtifactForTarget } from "./assembly"; const temporaryDirectories: string[] = []; const json = new FsReadWriteJson({ logger: createSilentLogger() }); -const EXPECTED = { account: "111122223333", region: "us-east-1" } as const; const TEMPLATE_FILE = "stack.template.json"; afterEach(async () => { @@ -31,7 +30,6 @@ async function assembly(artifacts: Record): Promise { function stackArtifact(target: string, overrides: Record = {}) { return { type: "aws:cloudformation:stack", - environment: `aws://${EXPECTED.account}/${EXPECTED.region}`, properties: { tags: { "agentcore:target-name": target }, templateFile: TEMPLATE_FILE, @@ -48,7 +46,7 @@ describe("stackArtifactForTarget", () => { test("selects by the target tag instead of deriving a stack name", async () => { const directory = await assembly({ "nested/stack-id": stackArtifact("prod") }); - expect(await stackArtifactForTarget(json, directory, "prod", EXPECTED)).toEqual({ + expect(await stackArtifactForTarget(json, directory, "prod")).toEqual({ id: "nested/stack-id", // No stackName in the manifest, so CloudFormation knows the stack by its // artifact id — the same fallback CDK itself applies. @@ -68,7 +66,7 @@ describe("stackArtifactForTarget", () => { }), }); - expect(await stackArtifactForTarget(json, directory, "prod", EXPECTED)).toEqual({ + expect(await stackArtifactForTarget(json, directory, "prod")).toEqual({ id: "nested/stack-id", stackName: "AgentCore-example-prod", templateFile: TEMPLATE_FILE, @@ -83,7 +81,7 @@ describe("stackArtifactForTarget", () => { }, }); - await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( + await expect(stackArtifactForTarget(json, directory, "prod")).rejects.toThrow( /defines 0 stack/, ); }); @@ -93,63 +91,10 @@ describe("stackArtifactForTarget", () => { temporaryDirectories.push(directory); await mkdir(directory, { recursive: true }); - await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( + await expect(stackArtifactForTarget(json, directory, "prod")).rejects.toThrow( /No synthesized cloud assembly was found/, ); }); - - test("rejects a stack tagged for the target but built for another account", async () => { - const directory = await assembly({ - Stack: stackArtifact("prod", { environment: `aws://999988887777/${EXPECTED.region}` }), - }); - - await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( - /built for account 999988887777 \(target expects 111122223333\)/, - ); - }); - - test("rejects a stack tagged for the target but built for another region", async () => { - const directory = await assembly({ - Stack: stackArtifact("prod", { environment: `aws://${EXPECTED.account}/eu-west-1` }), - }); - - await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( - /built for region eu-west-1 \(target expects us-east-1\)/, - ); - }); - - test("names both halves when neither account nor region matches", async () => { - const directory = await assembly({ - Stack: stackArtifact("prod", { environment: "aws://999988887777/eu-west-1" }), - }); - - await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( - /account 999988887777 \(target expects 111122223333\) and region eu-west-1/, - ); - }); - - test.each([ - ["no environment at all", undefined], - ["CDK's unknown-* placeholders", "aws://unknown-account/unknown-region"], - ])("accepts an environment-agnostic stack with %s", async (_label, environment) => { - const directory = await assembly({ - // An absent key and an explicit undefined both mean "agnostic" once - // serialized, which is what the manifest on disk actually looks like. - Stack: stackArtifact("prod", { environment }), - }); - - expect((await stackArtifactForTarget(json, directory, "prod", EXPECTED)).id).toBe("Stack"); - }); - - test("rejects an environment it cannot parse rather than assuming a match", async () => { - const directory = await assembly({ - Stack: stackArtifact("prod", { environment: "us-east-1" }), - }); - - await expect(stackArtifactForTarget(json, directory, "prod", EXPECTED)).rejects.toThrow( - /unrecognized environment 'us-east-1'/, - ); - }); }); describe("countDeployableResources", () => { diff --git a/src/core/project/backends/cdk/assembly.ts b/src/core/project/backends/cdk/assembly.ts index 38af080bb..0327d5ded 100644 --- a/src/core/project/backends/cdk/assembly.ts +++ b/src/core/project/backends/cdk/assembly.ts @@ -6,9 +6,6 @@ import type { ReadWriteJson } from "../../../../io"; const TARGET_TAG = "agentcore:target-name"; const STACK_ARTIFACT = "aws:cloudformation:stack"; -/** What CDK writes in an artifact's `environment` when the stack is env-agnostic. */ -const UNKNOWN_ACCOUNT = "unknown-account"; -const UNKNOWN_REGION = "unknown-region"; /** The resource CDK adds to every stack of its own accord, which no user asked for. */ const METADATA_RESOURCE_TYPE = "AWS::CDK::Metadata"; @@ -18,7 +15,6 @@ const AssemblyManifestSchema = z.object({ z.string(), z.object({ type: z.string(), - environment: z.string().optional(), properties: z .object({ tags: z.record(z.string(), z.string()).optional(), @@ -55,21 +51,11 @@ export interface StackArtifact { templateFile: string | undefined; } -/** The account and region a deploy expects its stack to be bound to. */ -export interface StackEnvironment { - account: string; - region: string; -} - -/** - * Finds the one synthesized stack artifact tagged for the selected deployment - * target, and checks it is bound to the environment that target names. - */ +/** Finds the one synthesized stack artifact tagged for the selected deployment target. */ export async function stackArtifactForTarget( json: ReadWriteJson, assemblyDirectory: string, target: string, - expected: StackEnvironment, ): Promise { const manifestPath = join(assemblyDirectory, "manifest.json"); if (!existsSync(manifestPath)) { @@ -98,7 +84,6 @@ export async function stackArtifactForTarget( } const [id, artifact] = matches[0]!; - assertEnvironmentMatches(id, artifact.environment, expected); return { id, // Mirrors how CDK itself resolves a stack artifact's physical name @@ -150,47 +135,3 @@ export async function countDeployableResources( (resource) => resource.Type !== METADATA_RESOURCE_TYPE, ).length; } - -// Both the target tag and the stack's environment derive from the same target in -// the synthesized app, so today they cannot disagree. Checking anyway keeps a -// correct tag from carrying a stack into the wrong account or region: the Toolkit -// deploys where the artifact's environment points, not where the tag says. -function assertEnvironmentMatches( - id: string, - environment: string | undefined, - expected: StackEnvironment, -): void { - // Not the env-agnostic case — CDK spells that out as - // aws://unknown-account/unknown-region, handled below. `environment` is - // optional in the cloud assembly schema, so a stack artifact without one is a - // manifest we cannot draw any conclusion from; the account preflight has - // already confirmed the credentials point at the target account either way. - if (environment === undefined) return; - - const parsed = /^aws:\/\/([^/]+)\/(.+)$/.exec(environment); - if (!parsed) { - throw new ProjectStateError( - `Stack artifact '${id}' declares an unrecognized environment '${environment}'. ` + - `Expected the form aws:///.`, - ); - } - - const account = parsed[1]!; - const region = parsed[2]!; - // The unknown-* placeholders are the env-agnostic case spelled out. - const mismatches = [ - account !== UNKNOWN_ACCOUNT && account !== expected.account - ? `account ${account} (target expects ${expected.account})` - : undefined, - region !== UNKNOWN_REGION && region !== expected.region - ? `region ${region} (target expects ${expected.region})` - : undefined, - ].filter((mismatch) => mismatch !== undefined); - - if (mismatches.length > 0) { - throw new ProjectStateError( - `The synthesized stack '${id}' is built for ${mismatches.join(" and ")}. ` + - `Re-synthesize the project so its stack matches the deployment target.`, - ); - } -} From 11db5bb9b5ceb37ad4129aa7ae2223401e55df48 Mon Sep 17 00:00:00 2001 From: notgitika Date: Thu, 27 Aug 2026 16:24:44 -0400 Subject: [PATCH 4/5] feat(project): prompt before tearing down empty deployments --- src/core/project/backends/cdk.test.ts | 45 +++++++- src/core/project/backends/cdk.ts | 11 +- src/core/project/backends/types.ts | 9 +- src/core/project/manager.test.ts | 12 +- src/core/project/manager.tsx | 3 + src/handlers/project/deploy/index.test.ts | 134 +++++++++++++++++++++- src/handlers/project/deploy/index.ts | 54 ++++++++- src/handlers/project/types.ts | 15 +++ 8 files changed, 270 insertions(+), 13 deletions(-) diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index bc6e7123e..793a35f3f 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -356,6 +356,38 @@ describe("CdkBackend.deploy", () => { expect(subject.bootstrapRegions).toEqual([]); }); + test("requests confirmation with the exact teardown details", async () => { + const input = await project(); + await writeAssembly(input, [TARGET.name], { resources: METADATA_ONLY }); + const requests: unknown[] = []; + const subject = harness(); + + await expect( + collectDeploy( + subject.backend.deploy( + input, + deployInput({ + requestTeardownConfirmation: async (request) => { + requests.push(request); + return false; + }, + }), + ), + ), + ).rejects.toThrow(/--yes/); + + expect(requests).toEqual([ + { + projectName: "example", + targetName: "default", + stackName: "AgentCore-example-default-0", + account: TARGET.account, + region: TARGET.region, + }, + ]); + expect(subject.runs).toEqual([]); + }); + test("treats a template holding only CDK's own metadata as nothing to deploy", async () => { // An empty project still synthesizes this one resource, so a check for an // empty Resources block would let the teardown case through as a deploy. @@ -384,12 +416,23 @@ describe("CdkBackend.deploy", () => { }), ); const subject = harness(); + let prompted = false; const deployed = await collectDeploy( - subject.backend.deploy(input, deployInput({ confirmTeardown: true })), + subject.backend.deploy( + input, + deployInput({ + confirmTeardown: true, + requestTeardownConfirmation: async () => { + prompted = true; + return false; + }, + }), + ), ); expect(deployed.result).toEqual({ outputs: {}, tornDown: true }); + expect(prompted).toBe(false); expect(deployed.events).toContainEqual({ message: "Removing stack AgentCore-example-default-0", }); diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index 2bbfc20f0..eb9782ba8 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -207,7 +207,16 @@ export class CdkBackend implements ProjectBackend { ); } - if (!input.confirmTeardown) { + const confirmed = + input.confirmTeardown || + (await input.requestTeardownConfirmation?.({ + projectName: project.name, + targetName: target.name, + stackName: artifact.stackName, + account: target.account, + region: target.region, + })); + if (!confirmed) { throw new ProjectStateError( `Project '${project.name}' declares no resources to deploy, so deploying to target ` + `'${target.name}' would delete stack '${artifact.stackName}' and every resource in ` + diff --git a/src/core/project/backends/types.ts b/src/core/project/backends/types.ts index d3b1d6fac..4d0653ea8 100644 --- a/src/core/project/backends/types.ts +++ b/src/core/project/backends/types.ts @@ -1,4 +1,9 @@ -import type { DeployResult, Project, ProjectEvent } from "../../../handlers/project/types"; +import type { + DeployResult, + Project, + ProjectEvent, + TeardownConfirmationHandler, +} from "../../../handlers/project/types"; import type { AwsDeploymentTarget } from "../../../projectSchemas/aws-targets"; export type DeployBackendInput = { @@ -10,6 +15,8 @@ export type DeployBackendInput = { * deployed resources, so it takes saying so. */ confirmTeardown: boolean; + /** Requests approval after synthesis identifies an otherwise unconfirmed teardown. */ + requestTeardownConfirmation?: TeardownConfirmationHandler; }; /** Builds the deployable artifacts owned by a project's selected backend. */ diff --git a/src/core/project/manager.test.ts b/src/core/project/manager.test.ts index 7e1e92203..e51020186 100644 --- a/src/core/project/manager.test.ts +++ b/src/core/project/manager.test.ts @@ -12,6 +12,7 @@ import { type DeployResult, type Project, type ProjectEvent, + type TeardownConfirmationHandler, } from "../../handlers/project/types"; import { createSilentLogger } from "../../testing"; import type { DeployBackendInput, ProjectBackend } from "./backends/types"; @@ -386,8 +387,13 @@ describe("FsProjectManager.deploy", () => { project: Project, target: string, confirmTeardown = false, + requestTeardownConfirmation?: TeardownConfirmationHandler, ): Promise<{ events: ProjectEvent[]; result: DeployResult }> { - const generator = manager.deploy(project, { target, confirmTeardown }); + const generator = manager.deploy(project, { + target, + confirmTeardown, + ...(requestTeardownConfirmation && { requestTeardownConfirmation }), + }); const events: ProjectEvent[] = []; while (true) { const next = await generator.next(); @@ -431,10 +437,12 @@ describe("FsProjectManager.deploy", () => { const root = await inTempDirectory(); const subject = deployManager(); const project = await projectWithTargets(root, targets); + const requestTeardownConfirmation: TeardownConfirmationHandler = async () => true; - await deploy(subject.manager, project, "prod", true); + await deploy(subject.manager, project, "prod", true, requestTeardownConfirmation); expect(subject.calls.map(({ input }) => input.confirmTeardown)).toEqual([true]); + expect(subject.calls[0]?.input.requestTeardownConfirmation).toBe(requestTeardownConfirmation); }); test("rejects an unknown target before invoking the backend", async () => { diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 4b5a2c514..7ce53934f 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -480,6 +480,9 @@ export class FsProjectManager implements ProjectManager { return yield* this.backendFor(project).deploy(project, { target, confirmTeardown: input.confirmTeardown, + ...(input.requestTeardownConfirmation && { + requestTeardownConfirmation: input.requestTeardownConfirmation, + }), }); } diff --git a/src/handlers/project/deploy/index.test.ts b/src/handlers/project/deploy/index.test.ts index 9cdca330f..a728ef2b1 100644 --- a/src/handlers/project/deploy/index.test.ts +++ b/src/handlers/project/deploy/index.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { tmpdir } from "node:os"; +import { UserCancellationError } from "../../../errors/errors"; import { createRootHandler } from "../../index"; import { createSilentLogger, @@ -11,7 +12,7 @@ import { } from "../../../testing"; import type { DeployBackendInput, ProjectBackend } from "../../../core/project"; import type { AwsDeploymentTarget } from "../../../projectSchemas/aws-targets"; -import type { DeployResult, Project, ProjectEvent } from "../types"; +import type { DeployResult, Project, ProjectEvent, TeardownConfirmationRequest } from "../types"; const DEFAULT_TARGET: AwsDeploymentTarget = { name: "default", @@ -24,6 +25,13 @@ const STAGING_TARGET: AwsDeploymentTarget = { region: "eu-west-1", }; const TARGETS = [DEFAULT_TARGET, STAGING_TARGET]; +const TEARDOWN: TeardownConfirmationRequest = { + projectName: "orders", + targetName: "default", + stackName: "AgentCore-orders-default-0", + account: DEFAULT_TARGET.account, + region: DEFAULT_TARGET.region, +}; /** * A ProjectBackend that deploys successfully, which CdkBackend cannot do until @@ -31,12 +39,23 @@ const TARGETS = [DEFAULT_TARGET, STAGING_TARGET]; * manager keeps the real FsProjectManager in the path, so target resolution and * withProject run for real. */ -function fakeBackend(result: DeployResult, events: ProjectEvent[] = []) { +function fakeBackend( + result: DeployResult, + events: ProjectEvent[] = [], + teardown?: TeardownConfirmationRequest, +) { const calls: { project: Project; input: DeployBackendInput }[] = []; const backend: ProjectBackend = { async *build() {}, async *deploy(project, input) { calls.push({ project, input }); + if ( + teardown && + !input.confirmTeardown && + !(await input.requestTeardownConfirmation?.(teardown)) + ) { + throw new Error("Re-run with --yes to confirm the teardown."); + } yield* events; return result; }, @@ -44,9 +63,19 @@ function fakeBackend(result: DeployResult, events: ProjectEvent[] = []) { return { calls, backend }; } -function testDeployCommand(result: DeployResult, events: ProjectEvent[] = []) { - const io = testIO(); - const fake = fakeBackend(result, events); +type TestDeployOptions = { + isTTY?: boolean; + stdin?: string; + teardown?: TeardownConfirmationRequest; +}; + +function testDeployCommand( + result: DeployResult, + events: ProjectEvent[] = [], + options: TestDeployOptions = {}, +) { + const io = testIO({ isTTY: options.isTTY, stdin: options.stdin }); + const fake = fakeBackend(result, events, options.teardown); const core = new TestCoreClient({ backends: { CDK: fake.backend } }); const root = createRootHandler(core, { io: io.io, @@ -128,12 +157,105 @@ describe("project deploy handler", () => { // --yes is the only way to authorize the teardown the backend refuses without // it, so a flag that never reaches the backend would make it unreachable. test("carries --yes through as permission to tear the stack down", async () => { - const subject = testDeployCommand({ outputs: {} }); + const subject = testDeployCommand({ outputs: {}, tornDown: true }, [], { + isTTY: true, + stdin: "\n", + teardown: TEARDOWN, + }); await inProjectWithTargets(subject); await subject.run(["--yes"]); expect(subject.calls.map(({ input }) => input.confirmTeardown)).toEqual([true]); + expect(subject.calls[0]?.input.requestTeardownConfirmation).toBeUndefined(); + expect(subject.io.stderr()).not.toContain("(y/N)"); + }); + + test("prompts before tearing down and proceeds on yes", async () => { + const subject = testDeployCommand({ outputs: {}, tornDown: true }, [], { + isTTY: true, + stdin: "yes\n", + teardown: TEARDOWN, + }); + await inProjectWithTargets(subject); + + await subject.run(); + + expect(subject.io.stderr()).toContain("Project 'orders' declares no resources to deploy."); + expect(subject.io.stderr()).toContain( + "Delete stack 'AgentCore-orders-default-0' and every resource in it from target " + + "'default' (111122223333/us-east-1)? (y/N)", + ); + expect(subject.io.stderr()).toContain("Removed project 'orders' from target 'default'"); + }); + + test.each([ + ["no", "n\n"], + ["the default", "\n"], + ])("does not tear down when the user chooses %s", async (_label, stdin) => { + const subject = testDeployCommand({ outputs: {}, tornDown: true }, [], { + isTTY: true, + stdin, + teardown: TEARDOWN, + }); + await inProjectWithTargets(subject); + + await expect(subject.run()).rejects.toBeInstanceOf(UserCancellationError); + + expect(subject.io.stderr()).toContain("(y/N)"); + expect(subject.io.stderr()).not.toContain("Removed project"); + }); + + test("cancels when interactive input closes without an answer", async () => { + const subject = testDeployCommand({ outputs: {}, tornDown: true }, [], { + isTTY: true, + stdin: "", + teardown: TEARDOWN, + }); + await inProjectWithTargets(subject); + + await expect(subject.run()).rejects.toBeInstanceOf(UserCancellationError); + + expect(subject.io.stderr()).not.toContain("Removed project"); + }); + + test("requires --yes instead of prompting in a non-interactive shell", async () => { + const subject = testDeployCommand({ outputs: {}, tornDown: true }, [], { + stdin: "yes\n", + teardown: TEARDOWN, + }); + await inProjectWithTargets(subject); + + await expect(subject.run()).rejects.toThrow(/--yes/); + + expect(subject.io.stderr()).not.toContain("(y/N)"); + expect(subject.calls[0]?.input.requestTeardownConfirmation).toBeUndefined(); + }); + + test("requires --yes instead of prompting in JSON mode", async () => { + const subject = testDeployCommand({ outputs: {}, tornDown: true }, [], { + isTTY: true, + stdin: "yes\n", + teardown: TEARDOWN, + }); + await inProjectWithTargets(subject); + + await expect(subject.run(["--json"])).rejects.toThrow(/--yes/); + + expect(subject.io.stderr()).not.toContain("(y/N)"); + expect(subject.calls[0]?.input.requestTeardownConfirmation).toBeUndefined(); + }); + + test("does not prompt for a normal deployment", async () => { + const subject = testDeployCommand({ outputs: { RuntimeArn: "arn:runtime" } }, [], { + isTTY: true, + stdin: "yes\n", + }); + await inProjectWithTargets(subject); + + await subject.run(); + + expect(subject.io.stderr()).not.toContain("(y/N)"); }); test("says the project was removed when the deploy tore the stack down", async () => { diff --git a/src/handlers/project/deploy/index.ts b/src/handlers/project/deploy/index.ts index 6d3eca915..cd2f41c7c 100644 --- a/src/handlers/project/deploy/index.ts +++ b/src/handlers/project/deploy/index.ts @@ -1,9 +1,15 @@ +import { createInterface } from "node:readline/promises"; import z from "zod"; +import { UserCancellationError } from "../../../errors/errors"; import type { AppIO } from "../../../io"; import { createHandler, flag, ProjectKey } from "../../../router"; import { JsonRendererKey } from "../../../tui"; import { JsonKey } from "../../keys"; -import type { ProjectManager } from "../types"; +import type { + ProjectManager, + TeardownConfirmationRequest, + TeardownConfirmationHandler, +} from "../types"; type DeployProjectHandlerConfig = { projectManager: ProjectManager; @@ -25,6 +31,13 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = handle: async (ctx, flags) => { // withProject has already resolved the enclosing project. const project = ctx.require(ProjectKey); + const jsonOutput = ctx.require(JsonKey); + const canPrompt = + !flags.yes && + !jsonOutput && + config.io.stdin.isTTY && + config.io.stdout.isTTY && + config.io.stderr.isTTY; // Progress goes to stderr, keeping stdout for machine output. Driven by // hand rather than `for await` because the outputs we render below are the @@ -32,6 +45,9 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = const deployment = config.projectManager.deploy(project, { target: flags.target, confirmTeardown: flags.yes, + ...(canPrompt && { + requestTeardownConfirmation: createTeardownConfirmationHandler(config.io), + }), }); let next = await deployment.next(); while (!next.done) { @@ -45,7 +61,7 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = ? `Removed project '${project.name}' from target '${flags.target}'\n` : `Deployed project '${project.name}' to target '${flags.target}'\n`, ); - if (ctx.require(JsonKey)) { + if (jsonOutput) { ctx.require(JsonRendererKey).renderJson(result); return; } @@ -56,3 +72,37 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = } }, }); + +function createTeardownConfirmationHandler(io: AppIO): TeardownConfirmationHandler { + return async (request) => { + if (!(await promptForTeardown(io, request))) { + throw new UserCancellationError(); + } + return true; + }; +} + +async function promptForTeardown( + io: AppIO, + request: TeardownConfirmationRequest, +): Promise { + const readline = createInterface({ input: io.stdin, output: io.stderr }); + try { + const cancelled = new Promise((_resolve, reject) => { + const cancel = () => reject(new UserCancellationError()); + readline.once("SIGINT", cancel); + readline.once("close", cancel); + }); + const answer = await Promise.race([ + readline.question( + `Project '${request.projectName}' declares no resources to deploy.\n` + + `Delete stack '${request.stackName}' and every resource in it from target ` + + `'${request.targetName}' (${request.account}/${request.region})? (y/N) `, + ), + cancelled, + ]); + return /^(?:y|yes)$/i.test(answer.trim()); + } finally { + readline.close(); + } +} diff --git a/src/handlers/project/types.ts b/src/handlers/project/types.ts index a098a8cc1..c80ec5e97 100644 --- a/src/handlers/project/types.ts +++ b/src/handlers/project/types.ts @@ -105,6 +105,19 @@ export type ProjectEvent = { message: string; }; +/** The destructive deployment discovered after a project has been synthesized. */ +export type TeardownConfirmationRequest = { + projectName: string; + targetName: string; + stackName: string; + account: string; + region: string; +}; + +export type TeardownConfirmationHandler = ( + request: TeardownConfirmationRequest, +) => Promise; + export type DeployProjectInput = { /** Name of the aws-targets.json entry to deploy. */ target: string; @@ -113,6 +126,8 @@ export type DeployProjectInput = { * declares anything to deploy. Withheld by default. */ confirmTeardown: boolean; + /** Requests approval after the backend discovers that this deploy is a teardown. */ + requestTeardownConfirmation?: TeardownConfirmationHandler; }; export type DeployResult = { From 552717b7c4dfb13622248bcc4f7e4dba98df804e Mon Sep 17 00:00:00 2001 From: notgitika Date: Thu, 27 Aug 2026 17:53:37 -0400 Subject: [PATCH 5/5] refactor(project): address deploy review feedback --- src/core/factories.tsx | 5 ++ src/core/index.tsx | 4 ++ src/core/project/backends/cdk.test.ts | 19 ++---- src/core/project/backends/cdk.ts | 33 ++++++---- src/core/project/backends/cdk/assembly.ts | 4 +- .../project/backends/cdk/environment.test.ts | 44 ++++++++++++- src/core/project/backends/cdk/environment.ts | 65 ++++++++++--------- src/core/project/backends/types.ts | 10 +-- src/core/project/manager.test.ts | 24 ++----- src/core/project/manager.tsx | 6 +- src/core/types.tsx | 10 +++ src/handlers/project/deploy/index.test.ts | 34 +++++----- src/handlers/project/deploy/index.ts | 16 +++-- src/handlers/project/types.ts | 10 +-- src/index.ts | 2 + src/testing/TestCoreClient.tsx | 4 +- src/testing/fixtures.tsx | 10 +++ 17 files changed, 181 insertions(+), 119 deletions(-) diff --git a/src/core/factories.tsx b/src/core/factories.tsx index 1f2364dcf..209b8da1c 100644 --- a/src/core/factories.tsx +++ b/src/core/factories.tsx @@ -2,7 +2,9 @@ import { BedrockAgentCoreControlClient } from "@aws-sdk/client-bedrock-agentcore import { BedrockAgentCoreClient } from "@aws-sdk/client-bedrock-agentcore"; import { IAMClient } from "@aws-sdk/client-iam"; import { CloudWatchLogsClient } from "@aws-sdk/client-cloudwatch-logs"; +import { CloudFormationClient } from "@aws-sdk/client-cloudformation"; import type { + CreateCloudFormationClient, CreateControlClient, CreateDataClient, CreateIamClient, @@ -24,3 +26,6 @@ export const createIamClient: CreateIamClient = (config) => new IAMClient({ ...c export const createLogsClient: CreateLogsClient = (config) => new CloudWatchLogsClient({ ...config }); + +export const createCloudFormationClient: CreateCloudFormationClient = (config) => + new CloudFormationClient({ ...config }); diff --git a/src/core/index.tsx b/src/core/index.tsx index d7275b912..87948c358 100644 --- a/src/core/index.tsx +++ b/src/core/index.tsx @@ -12,6 +12,7 @@ import type { AwsClients, ClientConfig, CoreFetch, + CreateCloudFormationClient, CreateControlClient, CreateDataClient, CreateIamClient, @@ -26,12 +27,14 @@ export type { ClientConfig, CoreFetch, CreateControlClient, + CreateCloudFormationClient, CreateDataClient, CreateIamClient, CreateLogsClient, } from "./types"; type CoreClientConfig = { + createCloudFormationClient?: CreateCloudFormationClient; createControlClient: CreateControlClient; createDataClient: CreateDataClient; createIamClient: CreateIamClient; @@ -88,6 +91,7 @@ export class CoreClient implements AwsClients { this.projectManager = new FsProjectManager({ logger: this.logger.child({ module: "projectManager" }), + createCloudFormationClient: config.createCloudFormationClient, }); } diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index 793a35f3f..8c0506628 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -22,7 +22,7 @@ const TARGET = { const METADATA_ONLY = { CDKMetadata: { Type: "AWS::CDK::Metadata" } }; function deployInput(overrides: Partial = {}): DeployBackendInput { - return { target: TARGET, confirmTeardown: false, ...overrides }; + return { target: TARGET, confirmTeardown: async () => false, ...overrides }; } const tempDirectories: string[] = []; @@ -367,7 +367,7 @@ describe("CdkBackend.deploy", () => { subject.backend.deploy( input, deployInput({ - requestTeardownConfirmation: async (request) => { + confirmTeardown: async (request) => { requests.push(request); return false; }, @@ -380,7 +380,7 @@ describe("CdkBackend.deploy", () => { { projectName: "example", targetName: "default", - stackName: "AgentCore-example-default-0", + resourceDescription: "stack 'AgentCore-example-default-0' and every resource in it", account: TARGET.account, region: TARGET.region, }, @@ -416,23 +416,16 @@ describe("CdkBackend.deploy", () => { }), ); const subject = harness(); - let prompted = false; - const deployed = await collectDeploy( subject.backend.deploy( input, deployInput({ - confirmTeardown: true, - requestTeardownConfirmation: async () => { - prompted = true; - return false; - }, + confirmTeardown: async () => true, }), ), ); expect(deployed.result).toEqual({ outputs: {}, tornDown: true }); - expect(prompted).toBe(false); expect(deployed.events).toContainEqual({ message: "Removing stack AgentCore-example-default-0", }); @@ -453,7 +446,9 @@ describe("CdkBackend.deploy", () => { const subject = harness({ stackExists: false }); await expect( - collectDeploy(subject.backend.deploy(input, deployInput({ confirmTeardown: true }))), + collectDeploy( + subject.backend.deploy(input, deployInput({ confirmTeardown: async () => true })), + ), ).rejects.toThrow(/no stack .* exists .* to remove.*Add a resource/s); expect(subject.runs).toEqual([]); }); diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index eb9782ba8..c8e7fa525 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -11,6 +11,8 @@ import { } from "../../../io"; import type { Logger } from "../../../logging"; import type { DeployBackendInput, ProjectBackend } from "./types"; +import { createCloudFormationClient } from "../../factories"; +import type { CreateCloudFormationClient } from "../../types"; import { countDeployableResources, stackArtifactForTarget, @@ -18,6 +20,8 @@ import { } from "./cdk/assembly"; import { readDeployedState, removeTargetState, updateTargetState } from "./cdk/deployedState"; import { + bootstrapStackReader, + createCloudFormationStackReader, probeBootstrap, probeStack, resolveAwsAccount, @@ -40,6 +44,7 @@ export type CdkBackendConfig = { runner?: ProcessRunner; checkTool?: typeof requireTool; json?: ReadWriteJson; + createCloudFormationClient?: CreateCloudFormationClient; cdk?: CdkRunner; resolveCredentials?: CdkCredentialResolver; bootstrap?: BootstrapProbe; @@ -69,8 +74,16 @@ export class CdkBackend implements ProjectBackend { this.cdk = config.cdk ?? createCdkRunner(config.logger); this.resolveCredentials = config.resolveCredentials ?? createCdkCredentialResolver(config.logger); - this.bootstrap = config.bootstrap ?? probeBootstrap; - this.stack = config.stack ?? probeStack; + const readStack = createCloudFormationStackReader( + config.createCloudFormationClient ?? createCloudFormationClient, + ); + const readBootstrapStack = bootstrapStackReader(readStack); + this.bootstrap = + config.bootstrap ?? + ((region, credentials) => probeBootstrap(region, credentials, readBootstrapStack)); + this.stack = + config.stack ?? + ((stackName, region, credentials) => probeStack(stackName, region, credentials, readStack)); this.resolveAccount = config.resolveAccount ?? resolveAwsAccount; this.loadBootstrapTemplate = config.loadBootstrapTemplate ?? loadBootstrapTemplate; } @@ -207,15 +220,13 @@ export class CdkBackend implements ProjectBackend { ); } - const confirmed = - input.confirmTeardown || - (await input.requestTeardownConfirmation?.({ - projectName: project.name, - targetName: target.name, - stackName: artifact.stackName, - account: target.account, - region: target.region, - })); + const confirmed = await input.confirmTeardown({ + projectName: project.name, + targetName: target.name, + resourceDescription: `stack '${artifact.stackName}' and every resource in it`, + account: target.account, + region: target.region, + }); if (!confirmed) { throw new ProjectStateError( `Project '${project.name}' declares no resources to deploy, so deploying to target ` + diff --git a/src/core/project/backends/cdk/assembly.ts b/src/core/project/backends/cdk/assembly.ts index 0327d5ded..483702d2c 100644 --- a/src/core/project/backends/cdk/assembly.ts +++ b/src/core/project/backends/cdk/assembly.ts @@ -42,14 +42,14 @@ const StackTemplateSchema = z .passthrough(); /** The synthesized stack a deploy selected, and where its template lives. */ -export interface StackArtifact { +export type StackArtifact = { /** Artifact id the CDK Toolkit selects the stack by. */ id: string; /** Name CloudFormation knows the deployed stack by. */ stackName: string; /** Assembly-relative path of the synthesized template. */ templateFile: string | undefined; -} +}; /** Finds the one synthesized stack artifact tagged for the selected deployment target. */ export async function stackArtifactForTarget( diff --git a/src/core/project/backends/cdk/environment.test.ts b/src/core/project/backends/cdk/environment.test.ts index 1377f3820..4f6bae862 100644 --- a/src/core/project/backends/cdk/environment.test.ts +++ b/src/core/project/backends/cdk/environment.test.ts @@ -1,6 +1,13 @@ import { describe, expect, test } from "bun:test"; -import type { Stack } from "@aws-sdk/client-cloudformation"; -import { isStackNotFound, probeBootstrap, probeStack, readBootstrapState } from "./environment"; +import type { CloudFormationClient, Stack } from "@aws-sdk/client-cloudformation"; +import { + bootstrapStackReader, + createCloudFormationStackReader, + isStackNotFound, + probeBootstrap, + probeStack, + readBootstrapState, +} from "./environment"; import type { CdkCredentialProvider } from "./toolkit"; const credentials: CdkCredentialProvider = async () => ({ @@ -17,6 +24,39 @@ function stack(status: Stack["StackStatus"], version?: string): Stack { }; } +test("injects and caches CloudFormation clients by credentials and region", async () => { + const otherCredentials: CdkCredentialProvider = async () => ({ + accessKeyId: "other-access-key", + secretAccessKey: "other-secret-key", + }); + const creations: { region: string; credentials: CdkCredentialProvider }[] = []; + const stackNames: string[] = []; + const read = createCloudFormationStackReader((config) => { + creations.push({ + region: config.region, + credentials: config.credentials as CdkCredentialProvider, + }); + return { + send: async (command: { input: { StackName?: string } }) => { + stackNames.push(command.input.StackName ?? ""); + return { Stacks: [stack("CREATE_COMPLETE", "30")] }; + }, + } as unknown as CloudFormationClient; + }); + + await bootstrapStackReader(read)("us-east-1", credentials); + await read("Application", "us-east-1", credentials); + await read("Regional", "eu-west-1", credentials); + await read("OtherCredentials", "us-east-1", otherCredentials); + + expect(creations).toEqual([ + { region: "us-east-1", credentials }, + { region: "eu-west-1", credentials }, + { region: "us-east-1", credentials: otherCredentials }, + ]); + expect(stackNames).toEqual(["CDKToolkit", "Application", "Regional", "OtherCredentials"]); +}); + describe("readBootstrapState", () => { test("accepts stable stacks at or above the minimum version", () => { expect(readBootstrapState([stack("CREATE_COMPLETE", "30")])).toEqual({ diff --git a/src/core/project/backends/cdk/environment.ts b/src/core/project/backends/cdk/environment.ts index 16b9ffe11..11e778389 100644 --- a/src/core/project/backends/cdk/environment.ts +++ b/src/core/project/backends/cdk/environment.ts @@ -1,5 +1,10 @@ -import type { Stack } from "@aws-sdk/client-cloudformation"; +import { + DescribeStacksCommand, + type CloudFormationClient, + type Stack, +} from "@aws-sdk/client-cloudformation"; import { MalformedServiceResponseError, ProjectStateError } from "../../../../errors/errors"; +import type { CreateCloudFormationClient } from "../../../types"; import type { CdkCredentialProvider } from "./toolkit"; const BOOTSTRAP_STACK_NAME = "CDKToolkit"; @@ -38,6 +43,34 @@ export type StackProbe = ( credentials: CdkCredentialProvider, ) => Promise; +/** Shares CloudFormation connections for calls using the same credentials and region. */ +export function createCloudFormationStackReader( + createClient: CreateCloudFormationClient, +): StackReader { + const clients = new WeakMap>(); + + return async (stackName, region, credentials) => { + let clientsByRegion = clients.get(credentials); + if (!clientsByRegion) { + clientsByRegion = new Map(); + clients.set(credentials, clientsByRegion); + } + + let client = clientsByRegion.get(region); + if (!client) { + client = createClient({ credentials, region }); + clientsByRegion.set(region, client); + } + + const response = await client.send(new DescribeStacksCommand({ StackName: stackName })); + return response.Stacks; + }; +} + +export function bootstrapStackReader(read: StackReader): BootstrapStackReader { + return (region, credentials) => read(BOOTSTRAP_STACK_NAME, region, credentials); +} + export function readBootstrapState(stacks?: Stack[]): Exclude { const stack = stacks?.[0]; if (!stack) { @@ -81,24 +114,10 @@ export function isStackNotFound(error: unknown): boolean { ); } -const describeBootstrapStack: BootstrapStackReader = async (region, credentials) => { - const { CloudFormationClient, DescribeStacksCommand } = - await import("@aws-sdk/client-cloudformation"); - const client = new CloudFormationClient({ credentials, region }); - try { - const response = await client.send( - new DescribeStacksCommand({ StackName: BOOTSTRAP_STACK_NAME }), - ); - return response.Stacks; - } finally { - client.destroy(); - } -}; - export async function probeBootstrap( region: string, credentials: CdkCredentialProvider, - read: BootstrapStackReader = describeBootstrapStack, + read: BootstrapStackReader, ): Promise { try { return readBootstrapState(await read(region, credentials)); @@ -108,18 +127,6 @@ export async function probeBootstrap( } } -const describeStack: StackReader = async (stackName, region, credentials) => { - const { CloudFormationClient, DescribeStacksCommand } = - await import("@aws-sdk/client-cloudformation"); - const client = new CloudFormationClient({ credentials, region }); - try { - const response = await client.send(new DescribeStacksCommand({ StackName: stackName })); - return response.Stacks; - } finally { - client.destroy(); - } -}; - /** * Whether CloudFormation still holds a stack of this name, so a deploy with * nothing left to deploy can tell "tear the stack down" from "there was never @@ -133,7 +140,7 @@ export async function probeStack( stackName: string, region: string, credentials: CdkCredentialProvider, - read: StackReader = describeStack, + read: StackReader, ): Promise { try { return ((await read(stackName, region, credentials)) ?? []).length > 0; diff --git a/src/core/project/backends/types.ts b/src/core/project/backends/types.ts index 4d0653ea8..fb1ef5b20 100644 --- a/src/core/project/backends/types.ts +++ b/src/core/project/backends/types.ts @@ -9,14 +9,8 @@ import type { AwsDeploymentTarget } from "../../../projectSchemas/aws-targets"; export type DeployBackendInput = { /** Fully resolved account and region selected from aws-targets.json. */ target: AwsDeploymentTarget; - /** - * Permission to tear the target's stack down when the project no longer - * declares anything to deploy. Withheld by default: that deploy destroys - * deployed resources, so it takes saying so. - */ - confirmTeardown: boolean; - /** Requests approval after synthesis identifies an otherwise unconfirmed teardown. */ - requestTeardownConfirmation?: TeardownConfirmationHandler; + /** Requests approval after synthesis identifies a teardown. */ + confirmTeardown: TeardownConfirmationHandler; }; /** Builds the deployable artifacts owned by a project's selected backend. */ diff --git a/src/core/project/manager.test.ts b/src/core/project/manager.test.ts index e51020186..a10246354 100644 --- a/src/core/project/manager.test.ts +++ b/src/core/project/manager.test.ts @@ -386,13 +386,11 @@ describe("FsProjectManager.deploy", () => { manager: FsProjectManager, project: Project, target: string, - confirmTeardown = false, - requestTeardownConfirmation?: TeardownConfirmationHandler, + confirmTeardown: TeardownConfirmationHandler = async () => false, ): Promise<{ events: ProjectEvent[]; result: DeployResult }> { const generator = manager.deploy(project, { target, confirmTeardown, - ...(requestTeardownConfirmation && { requestTeardownConfirmation }), }); const events: ProjectEvent[] = []; while (true) { @@ -422,29 +420,15 @@ describe("FsProjectManager.deploy", () => { const deployed = await deploy(subject.manager, project, "prod"); - expect(subject.calls).toEqual([ - { project, input: { target: targets[1]!, confirmTeardown: false } }, - ]); + expect(subject.calls).toHaveLength(1); + expect(subject.calls[0]?.project).toBe(project); + expect(subject.calls[0]?.input.target).toEqual(targets[1]); expect(deployed.events).toEqual([{ message: "Backend deployment started" }]); expect(deployed.result).toEqual({ outputs: { RuntimeArn: "arn:runtime" }, }); }); - // The backend decides whether a deploy of nothing is a teardown; the manager's - // job is only to carry the user's confirmation through to it. - test("passes the teardown confirmation through to the backend", async () => { - const root = await inTempDirectory(); - const subject = deployManager(); - const project = await projectWithTargets(root, targets); - const requestTeardownConfirmation: TeardownConfirmationHandler = async () => true; - - await deploy(subject.manager, project, "prod", true, requestTeardownConfirmation); - - expect(subject.calls.map(({ input }) => input.confirmTeardown)).toEqual([true]); - expect(subject.calls[0]?.input.requestTeardownConfirmation).toBe(requestTeardownConfirmation); - }); - test("rejects an unknown target before invoking the backend", async () => { const root = await inTempDirectory(); const subject = deployManager(); diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 7ce53934f..9ede69c1f 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -47,11 +47,13 @@ import { AwsDeploymentTargetsSchema } from "../../projectSchemas/aws-targets"; import type { RuntimeResourceConfig } from "../../handlers/project/add/runtime/types"; import type { TemplateRenderer } from "./templates/types"; import { HandlebarsTemplateRenderer } from "./templates/renderer"; +import type { CreateCloudFormationClient } from "../types"; const TARGETS_EXAMPLE = '[{ "name": "default", "account": "111122223333", "region": "us-east-1" }]'; type ProjectManagerConfig = { logger: Logger; + createCloudFormationClient?: CreateCloudFormationClient; source?: AssetSource; runner?: ProcessRunner; checkTool?: typeof requireTool; @@ -81,6 +83,7 @@ export class FsProjectManager implements ProjectManager { this.backends = config.backends ?? { CDK: new CdkBackend({ logger: config.logger, + createCloudFormationClient: config.createCloudFormationClient, runner: config.runner, checkTool: config.checkTool, json: config.json, @@ -480,9 +483,6 @@ export class FsProjectManager implements ProjectManager { return yield* this.backendFor(project).deploy(project, { target, confirmTeardown: input.confirmTeardown, - ...(input.requestTeardownConfirmation && { - requestTeardownConfirmation: input.requestTeardownConfirmation, - }), }); } diff --git a/src/core/types.tsx b/src/core/types.tsx index 98a2f338d..9e36a7c17 100644 --- a/src/core/types.tsx +++ b/src/core/types.tsx @@ -2,6 +2,10 @@ import type { BedrockAgentCoreControlClient } from "@aws-sdk/client-bedrock-agen import type { BedrockAgentCoreClient } from "@aws-sdk/client-bedrock-agentcore"; import type { IAMClient } from "@aws-sdk/client-iam"; import type { CloudWatchLogsClient } from "@aws-sdk/client-cloudwatch-logs"; +import type { + CloudFormationClient, + CloudFormationClientConfig, +} from "@aws-sdk/client-cloudformation"; // CoreOptions is the standard trailing argument for Core operations. It carries // the per-call settings a handler resolves from context (the AWS region and an @@ -20,6 +24,11 @@ export interface ClientConfig { endpoint?: string; } +export type CredentialedClientConfig = { + region: string; + credentials: NonNullable; +}; + // Factories construct an SDK client from a ClientConfig. Injecting these (rather // than the clients themselves) lets CoreClient create/cache one client per config // while keeping construction swappable for unit tests. @@ -27,6 +36,7 @@ export type CreateControlClient = (config: ClientConfig) => BedrockAgentCoreCont export type CreateDataClient = (config: ClientConfig) => BedrockAgentCoreClient; export type CreateIamClient = (config: ClientConfig) => IAMClient; export type CreateLogsClient = (config: ClientConfig) => CloudWatchLogsClient; +export type CreateCloudFormationClient = (config: CredentialedClientConfig) => CloudFormationClient; export type CoreFetch = ( ...args: Parameters ) => ReturnType; diff --git a/src/handlers/project/deploy/index.test.ts b/src/handlers/project/deploy/index.test.ts index a728ef2b1..1c00dafe3 100644 --- a/src/handlers/project/deploy/index.test.ts +++ b/src/handlers/project/deploy/index.test.ts @@ -28,7 +28,7 @@ const TARGETS = [DEFAULT_TARGET, STAGING_TARGET]; const TEARDOWN: TeardownConfirmationRequest = { projectName: "orders", targetName: "default", - stackName: "AgentCore-orders-default-0", + resourceDescription: "stack 'AgentCore-orders-default-0' and every resource in it", account: DEFAULT_TARGET.account, region: DEFAULT_TARGET.region, }; @@ -45,22 +45,23 @@ function fakeBackend( teardown?: TeardownConfirmationRequest, ) { const calls: { project: Project; input: DeployBackendInput }[] = []; + const confirmations: boolean[] = []; const backend: ProjectBackend = { async *build() {}, async *deploy(project, input) { calls.push({ project, input }); - if ( - teardown && - !input.confirmTeardown && - !(await input.requestTeardownConfirmation?.(teardown)) - ) { - throw new Error("Re-run with --yes to confirm the teardown."); + if (teardown) { + const confirmed = await input.confirmTeardown(teardown); + confirmations.push(confirmed); + if (!confirmed) { + throw new Error("Re-run with --yes to confirm the teardown."); + } } yield* events; return result; }, }; - return { calls, backend }; + return { calls, confirmations, backend }; } type TestDeployOptions = { @@ -133,9 +134,8 @@ describe("project deploy handler", () => { await subject.run(); - expect(subject.calls.map(({ input }) => input)).toEqual([ - { target: DEFAULT_TARGET, confirmTeardown: false }, - ]); + expect(subject.calls).toHaveLength(1); + expect(subject.calls[0]?.input.target).toEqual(DEFAULT_TARGET); expect(subject.io.stderr()).toContain("Preparing deployment\nDeploying stack"); expect(subject.io.stderr()).toContain("Deployed project 'orders' to target 'default'"); expect(subject.io.stdout()).toBe("AlphaArn: arn:alpha\nZetaUrl: https://zeta.example"); @@ -148,9 +148,8 @@ describe("project deploy handler", () => { await subject.run(["--target", "staging", "--json"]); - expect(subject.calls.map(({ input }) => input)).toEqual([ - { target: STAGING_TARGET, confirmTeardown: false }, - ]); + expect(subject.calls).toHaveLength(1); + expect(subject.calls[0]?.input.target).toEqual(STAGING_TARGET); expect(JSON.parse(subject.io.stdout())).toEqual(result); }); @@ -166,8 +165,7 @@ describe("project deploy handler", () => { await subject.run(["--yes"]); - expect(subject.calls.map(({ input }) => input.confirmTeardown)).toEqual([true]); - expect(subject.calls[0]?.input.requestTeardownConfirmation).toBeUndefined(); + expect(subject.confirmations).toEqual([true]); expect(subject.io.stderr()).not.toContain("(y/N)"); }); @@ -229,7 +227,7 @@ describe("project deploy handler", () => { await expect(subject.run()).rejects.toThrow(/--yes/); expect(subject.io.stderr()).not.toContain("(y/N)"); - expect(subject.calls[0]?.input.requestTeardownConfirmation).toBeUndefined(); + expect(subject.confirmations).toEqual([false]); }); test("requires --yes instead of prompting in JSON mode", async () => { @@ -243,7 +241,7 @@ describe("project deploy handler", () => { await expect(subject.run(["--json"])).rejects.toThrow(/--yes/); expect(subject.io.stderr()).not.toContain("(y/N)"); - expect(subject.calls[0]?.input.requestTeardownConfirmation).toBeUndefined(); + expect(subject.confirmations).toEqual([false]); }); test("does not prompt for a normal deployment", async () => { diff --git a/src/handlers/project/deploy/index.ts b/src/handlers/project/deploy/index.ts index cd2f41c7c..d904d2b0c 100644 --- a/src/handlers/project/deploy/index.ts +++ b/src/handlers/project/deploy/index.ts @@ -44,10 +44,7 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = // generator's return value, which `for await` discards. const deployment = config.projectManager.deploy(project, { target: flags.target, - confirmTeardown: flags.yes, - ...(canPrompt && { - requestTeardownConfirmation: createTeardownConfirmationHandler(config.io), - }), + confirmTeardown: createTeardownConfirmationHandler(config.io, flags.yes, canPrompt), }); let next = await deployment.next(); while (!next.done) { @@ -73,7 +70,14 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = }, }); -function createTeardownConfirmationHandler(io: AppIO): TeardownConfirmationHandler { +function createTeardownConfirmationHandler( + io: AppIO, + confirmed: boolean, + canPrompt: boolean, +): TeardownConfirmationHandler { + if (confirmed) return async () => true; + if (!canPrompt) return async () => false; + return async (request) => { if (!(await promptForTeardown(io, request))) { throw new UserCancellationError(); @@ -96,7 +100,7 @@ async function promptForTeardown( const answer = await Promise.race([ readline.question( `Project '${request.projectName}' declares no resources to deploy.\n` + - `Delete stack '${request.stackName}' and every resource in it from target ` + + `Delete ${request.resourceDescription} from target ` + `'${request.targetName}' (${request.account}/${request.region})? (y/N) `, ), cancelled, diff --git a/src/handlers/project/types.ts b/src/handlers/project/types.ts index c80ec5e97..064030279 100644 --- a/src/handlers/project/types.ts +++ b/src/handlers/project/types.ts @@ -109,7 +109,8 @@ export type ProjectEvent = { export type TeardownConfirmationRequest = { projectName: string; targetName: string; - stackName: string; + /** Human-readable description of the resources the backend will remove. */ + resourceDescription: string; account: string; region: string; }; @@ -121,13 +122,8 @@ export type TeardownConfirmationHandler = ( export type DeployProjectInput = { /** Name of the aws-targets.json entry to deploy. */ target: string; - /** - * Permission to tear the target's stack down when the project no longer - * declares anything to deploy. Withheld by default. - */ - confirmTeardown: boolean; /** Requests approval after the backend discovers that this deploy is a teardown. */ - requestTeardownConfirmation?: TeardownConfirmationHandler; + confirmTeardown: TeardownConfirmationHandler; }; export type DeployResult = { diff --git a/src/index.ts b/src/index.ts index f626f066d..c46290b6d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,6 +9,7 @@ import { join } from "path"; import { CoreClient } from "./core"; import { + createCloudFormationClient, createControlClient, createDataClient, createIamClient, @@ -66,6 +67,7 @@ process.exit( // factories (rather than instances) lets CoreClient build one client per // region on demand. const coreClient = new CoreClient({ + createCloudFormationClient, createControlClient, createDataClient, createIamClient, diff --git a/src/testing/TestCoreClient.tsx b/src/testing/TestCoreClient.tsx index e58ef5209..8fcce7a46 100644 --- a/src/testing/TestCoreClient.tsx +++ b/src/testing/TestCoreClient.tsx @@ -159,7 +159,7 @@ import type { } from "../handlers/eval/types"; import { isTerminalStatus } from "../core/batchEvaluationResults"; import { abortable } from "../core/abortable"; -import type { CoreOptions } from "../core/types"; +import type { CoreOptions, CreateCloudFormationClient } from "../core/types"; import type { ProjectManager } from "../handlers/project/types"; import type { Logger } from "../logging"; import type { ReadWriteJson } from "../io"; @@ -1208,6 +1208,7 @@ type TestCoreClientOptions = { logger?: Logger; json?: ReadWriteJson; backends?: Partial>; + createCloudFormationClient?: CreateCloudFormationClient; }; export class TestIdentityClient implements CoreIdentityClient { @@ -2238,6 +2239,7 @@ export class TestCoreClient implements Core { constructor(options?: TestCoreClientOptions) { this.projectManager = new FsProjectManager({ logger: options?.logger ?? createSilentLogger(), + createCloudFormationClient: options?.createCloudFormationClient, json: options?.json, backends: options?.backends, runner: async (command, { cwd }) => { diff --git a/src/testing/fixtures.tsx b/src/testing/fixtures.tsx index a6a4d39c6..60271ee56 100644 --- a/src/testing/fixtures.tsx +++ b/src/testing/fixtures.tsx @@ -5,8 +5,10 @@ import type { BedrockAgentCoreControlClient } from "@aws-sdk/client-bedrock-agen import type { BedrockAgentCoreClient } from "@aws-sdk/client-bedrock-agentcore"; import type { IAMClient } from "@aws-sdk/client-iam"; import type { CloudWatchLogsClient } from "@aws-sdk/client-cloudwatch-logs"; +import type { CloudFormationClient } from "@aws-sdk/client-cloudformation"; import type { ClientConfig, + CreateCloudFormationClient, CreateControlClient, CreateDataClient, CreateIamClient, @@ -14,6 +16,7 @@ import type { CoreFetch, } from "../core/types"; import { + createCloudFormationClient, createControlClient, createDataClient, createIamClient, @@ -215,12 +218,19 @@ function makeRecordingSend Promise }>( // (parsing → middleware → handler → CoreClient) against recorded data. The fake // clients only implement `.send()`, which is all CoreClient uses. export function fixtureFactories(dir: string): { + createCloudFormationClient: CreateCloudFormationClient; createControlClient: CreateControlClient; createDataClient: CreateDataClient; createIamClient: CreateIamClient; createLogsClient: CreateLogsClient; } { return { + createCloudFormationClient: (config) => { + const real = createCloudFormationClient(config); + return { + send: makeRecordingSend(real, dir), + } as unknown as CloudFormationClient; + }, createControlClient: (config: ClientConfig) => { // The real client is only constructed to satisfy record mode; in replay // mode its `.send()` is never reached.