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 288bb2199..8c0506628 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: async () => false, ...overrides }; +} + const tempDirectories: string[] = []; afterEach(async () => { @@ -51,9 +59,35 @@ async function project(withDependencies = true): Promise { }; } -async function writeAssembly(project: Project, targetNames: string[]): Promise { +type AssemblyOptions = { + /** 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 +95,15 @@ 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", + properties: { + templateFile: stack.templateFile, + tags: { "agentcore:target-name": stack.target }, }, - ], + }, ]), ), }, @@ -85,6 +120,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 = {}) { @@ -95,6 +132,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 () => ({ @@ -123,6 +161,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) { @@ -164,6 +206,7 @@ function harness(options: HarnessOptions = {}) { credentialRegions, credentials, runs, + stackProbes, templateLoads: () => templateLoads, templateCleanups: () => templateCleanups, }; @@ -225,7 +268,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}` }, @@ -263,7 +306,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({ @@ -281,7 +324,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); @@ -294,14 +337,132 @@ 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("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, 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("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({ + confirmTeardown: async (request) => { + requests.push(request); + return false; + }, + }), + ), + ), + ).rejects.toThrow(/--yes/); + + expect(requests).toEqual([ + { + projectName: "example", + targetName: "default", + resourceDescription: "stack 'AgentCore-example-default-0' and every resource in it", + 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. + 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: async () => 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: async () => 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.each([ ["absent", { kind: "absent" } as const], ["outdated", { kind: "outdated", version: 29 } as const], @@ -310,7 +471,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([ { @@ -328,7 +489,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([]); @@ -341,7 +502,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([]); @@ -353,7 +514,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([]); @@ -366,9 +527,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([]); }); @@ -377,7 +536,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", @@ -397,7 +556,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 ee160263e..c8e7fa525 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -11,13 +11,23 @@ import { } from "../../../io"; import type { Logger } from "../../../logging"; import type { DeployBackendInput, ProjectBackend } from "./types"; -import { stackArtifactIdForTarget } from "./cdk/assembly"; -import { readDeployedState, updateTargetState } from "./cdk/deployedState"; +import { createCloudFormationClient } from "../../factories"; +import type { CreateCloudFormationClient } from "../../types"; import { + countDeployableResources, + stackArtifactForTarget, + type StackArtifact, +} from "./cdk/assembly"; +import { readDeployedState, removeTargetState, updateTargetState } from "./cdk/deployedState"; +import { + bootstrapStackReader, + createCloudFormationStackReader, probeBootstrap, + probeStack, resolveAwsAccount, type AccountResolver, type BootstrapProbe, + type StackProbe, } from "./cdk/environment"; import { createCdkCredentialResolver, @@ -26,6 +36,7 @@ import { type BootstrapTemplateLoader, type CdkCredentialResolver, type CdkRunner, + type CdkRunOptions, } from "./cdk/toolkit"; export type CdkBackendConfig = { @@ -33,9 +44,11 @@ export type CdkBackendConfig = { runner?: ProcessRunner; checkTool?: typeof requireTool; json?: ReadWriteJson; + createCloudFormationClient?: CreateCloudFormationClient; cdk?: CdkRunner; resolveCredentials?: CdkCredentialResolver; bootstrap?: BootstrapProbe; + stack?: StackProbe; resolveAccount?: AccountResolver; loadBootstrapTemplate?: BootstrapTemplateLoader; }; @@ -49,6 +62,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; @@ -60,7 +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; + 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; } @@ -108,13 +131,16 @@ 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); 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({ @@ -143,15 +169,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.`, ); } @@ -163,6 +192,55 @@ 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.`, + ); + } + + 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 ` + + `'${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 20180acee..1b1d32348 100644 --- a/src/core/project/backends/cdk/assembly.test.ts +++ b/src/core/project/backends/cdk/assembly.test.ts @@ -4,11 +4,13 @@ import { join } from "node:path"; import { tmpdir } from "node:os"; import { FsReadWriteJson } from "../../../../io"; import { createSilentLogger } from "../../../../testing"; -import { stackArtifactIdForTarget } from "./assembly"; +import { countDeployableResources, stackArtifactForTarget } from "./assembly"; const temporaryDirectories: string[] = []; const json = new FsReadWriteJson({ logger: createSilentLogger() }); +const TEMPLATE_FILE = "stack.template.json"; + afterEach(async () => { await Promise.all( temporaryDirectories @@ -24,31 +26,62 @@ 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", + 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": stackArtifact("prod") }); + + 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. + 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": { - type: "aws:cloudformation:stack", + "nested/stack-id": stackArtifact("prod", { properties: { tags: { "agentcore:target-name": "prod" }, + templateFile: TEMPLATE_FILE, + stackName: "AgentCore-example-prod", }, - }, + }), }); - expect(await stackArtifactIdForTarget(json, directory, "prod")).toBe("nested/stack-id"); + expect(await stackArtifactForTarget(json, directory, "prod")).toEqual({ + id: "nested/stack-id", + stackName: "AgentCore-example-prod", + 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")).rejects.toThrow( /defines 0 stack/, ); }); @@ -58,8 +91,71 @@ describe("stackArtifactIdForTarget", () => { temporaryDirectories.push(directory); await mkdir(directory, { recursive: true }); - await expect(stackArtifactIdForTarget(json, directory, "prod")).rejects.toThrow( + await expect(stackArtifactForTarget(json, directory, "prod")).rejects.toThrow( /No synthesized cloud assembly was found/, ); }); }); + +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" }, + Role: { Type: "AWS::IAM::Role" }, + }); + + expect(await countDeployableResources(json, directory, artifact)).toBe(2); + }); + + 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, {}); + + 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(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( + 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 4077da573..483702d2c 100644 --- a/src/core/project/backends/cdk/assembly.ts +++ b/src/core/project/backends/cdk/assembly.ts @@ -6,6 +6,8 @@ import type { ReadWriteJson } from "../../../../io"; const TARGET_TAG = "agentcore:target-name"; const STACK_ARTIFACT = "aws:cloudformation:stack"; +/** 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 @@ -16,6 +18,8 @@ const AssemblyManifestSchema = z.object({ properties: z .object({ tags: z.record(z.string(), z.string()).optional(), + templateFile: z.string().optional(), + stackName: z.string().optional(), }) .optional(), }), @@ -23,12 +27,36 @@ const AssemblyManifestSchema = z.object({ .default({}), }); +const StackTemplateSchema = z + .object({ + 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(); + +/** The synthesized stack a deploy selected, and where its template lives. */ +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 stackArtifactIdForTarget( +export async function stackArtifactForTarget( json: ReadWriteJson, assemblyDirectory: string, target: string, -): Promise { +): Promise { const manifestPath = join(assemblyDirectory, "manifest.json"); if (!existsSync(manifestPath)) { throw new ProjectStateError(`No synthesized cloud assembly was found at ${manifestPath}.`); @@ -54,5 +82,56 @@ export async function stackArtifactIdForTarget( `'${target}'. Exactly one stack must be tagged ${TARGET_TAG}='${target}'.`, ); } - return matches[0]![0]; + + const [id, artifact] = matches[0]!; + 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, + }; +} + +/** + * 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". + * + * `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 countDeployableResources( + 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); + return Object.values(template.Resources).filter( + (resource) => resource.Type !== METADATA_RESOURCE_TYPE, + ).length; } 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..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, 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({ @@ -110,3 +150,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..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"; @@ -27,6 +32,44 @@ 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; + +/** 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]; @@ -60,7 +103,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: 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)); @@ -98,6 +127,29 @@ export async function probeBootstrap( } } +/** + * 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, +): 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 5bce6f503..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,18 +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. + // 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..fb1ef5b20 100644 --- a/src/core/project/backends/types.ts +++ b/src/core/project/backends/types.ts @@ -1,9 +1,16 @@ -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 = { /** Fully resolved account and region selected from aws-targets.json. */ target: AwsDeploymentTarget; + /** 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 45997ff8e..a10246354 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"; @@ -385,8 +386,12 @@ describe("FsProjectManager.deploy", () => { manager: FsProjectManager, project: Project, target: string, + confirmTeardown: TeardownConfirmationHandler = async () => 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,7 +420,9 @@ describe("FsProjectManager.deploy", () => { const deployed = await deploy(subject.manager, project, "prod"); - expect(subject.calls).toEqual([{ project, input: { target: targets[1]! } }]); + 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" }, diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 042c6a13a..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, @@ -477,7 +480,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/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 6384195b7..1c00dafe3 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", + resourceDescription: "stack 'AgentCore-orders-default-0' and every resource in it", + account: DEFAULT_TARGET.account, + region: DEFAULT_TARGET.region, +}; /** * A ProjectBackend that deploys successfully, which CdkBackend cannot do until @@ -31,22 +39,44 @@ 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 confirmations: boolean[] = []; const backend: ProjectBackend = { async *build() {}, async *deploy(project, input) { calls.push({ project, input }); + 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 }; } -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, @@ -104,7 +134,8 @@ describe("project deploy handler", () => { await subject.run(); - expect(subject.calls.map(({ input }) => input)).toEqual([{ target: DEFAULT_TARGET }]); + 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"); @@ -117,10 +148,128 @@ describe("project deploy handler", () => { await subject.run(["--target", "staging", "--json"]); - expect(subject.calls.map(({ input }) => input)).toEqual([{ target: STAGING_TARGET }]); + expect(subject.calls).toHaveLength(1); + expect(subject.calls[0]?.input.target).toEqual(STAGING_TARGET); 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: {}, tornDown: true }, [], { + isTTY: true, + stdin: "\n", + teardown: TEARDOWN, + }); + await inProjectWithTargets(subject); + + await subject.run(["--yes"]); + + expect(subject.confirmations).toEqual([true]); + 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.confirmations).toEqual([false]); + }); + + 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.confirmations).toEqual([false]); + }); + + 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 () => { + 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..d904d2b0c 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; @@ -16,15 +22,30 @@ 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. 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 // 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: createTeardownConfirmationHandler(config.io, flags.yes, canPrompt), + }); let next = await deployment.next(); while (!next.done) { config.io.stderr.write(`${next.value.message}\n`); @@ -32,8 +53,12 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = } const result = next.value; - config.io.stderr.write(`Deployed project '${project.name}' to target '${flags.target}'\n`); - if (ctx.require(JsonKey)) { + 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 (jsonOutput) { ctx.require(JsonRendererKey).renderJson(result); return; } @@ -44,3 +69,44 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = } }, }); + +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(); + } + 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 ${request.resourceDescription} 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 c96b2e7af..064030279 100644 --- a/src/handlers/project/types.ts +++ b/src/handlers/project/types.ts @@ -105,9 +105,25 @@ export type ProjectEvent = { message: string; }; +/** The destructive deployment discovered after a project has been synthesized. */ +export type TeardownConfirmationRequest = { + projectName: string; + targetName: string; + /** Human-readable description of the resources the backend will remove. */ + resourceDescription: 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; + /** Requests approval after the backend discovers that this deploy is a teardown. */ + confirmTeardown: TeardownConfirmationHandler; }; export type DeployResult = { @@ -119,6 +135,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 = { 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.