Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion src/core/project/backends/cdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,43 @@ describe("CdkBackend.resolveDeployedResources", () => {
expect(subject.stackReads).toHaveLength(1);
});

test("fails without reading AWS when the target has no deployed stack ARN", async () => {
test("resolves resources from a legacy stack name when the target has no stack ARN", async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These tests appear to be asserting on implementations rather than behaviors. Would it be possible to test at the handler level, such that we create a project, insert the legacy stackName field into deployedState, then ensure the status command works as expected?

const input = await project();
input.spec = ProjectSpecSchema.parse({
...input.spec,
harnesses: [{ name: "support", path: "app/support" }],
});
await updateTargetState(json, input.rootPath, TARGET.name, {
resources: { stackName: "AgentCore-example-default" },
});
const subject = harness({
describedStack: {
StackName: "AgentCore-example-default",
CreationTime: new Date(0),
StackStatus: "CREATE_COMPLETE",
Outputs: [
{
ExportName: "AgentCore-example-default-Harness-support-Id",
OutputValue: "support-AbCdEf1234",
},
],
},
});

await expect(
subject.backend.resolveDeployedResources(input, { target: TARGET }),
).resolves.toEqual([
{
resourceType: "harness",
name: "support",
id: "support-AbCdEf1234",
target: TARGET,
},
]);
expect(subject.stackReads[0]?.stackName).toBe("AgentCore-example-default");
});

test("fails without reading AWS when the target has no recorded stack", async () => {
const input = await project();
const subject = harness({ describedStack: null });

Expand Down
19 changes: 13 additions & 6 deletions src/core/project/backends/cdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ import {
stackArtifactForTarget,
type StackArtifact,
} from "./cdk/assembly";
import { readDeployedState, removeTargetState, updateTargetState } from "./cdk/deployedState";
import {
readDeployedState,
removeTargetState,
stackReferenceOf,
updateTargetState,
} from "./cdk/deployedState";
import {
bootstrapStackReader,
createCloudFormationStackReader,
Expand Down Expand Up @@ -402,16 +407,17 @@ export class CdkBackend implements ProjectBackend {
): Promise<ResolvedDeployedResource[]> {
const { target } = input;
const deployedState = await readDeployedState(this.json, project.rootPath);
const stackArn = deployedState.targets[target.name]?.stackArn;
if (!stackArn) {
const recorded = deployedState.targets[target.name];
const stackReference = stackReferenceOf(recorded);
if (!stackReference) {
throw new ProjectStateError(
`Project '${project.name}' is not deployed to target '${target.name}'. ` +
`Run 'agentcore project deploy --target ${target.name}' first.`,
);
}

const credentials = await this.credentialsForTarget(target);
const stack = await this.describeStack(target.region, credentials, stackArn);
const stack = await this.describeStack(target.region, credentials, stackReference);
if (!stack) {
throw new ProjectStateError(
`Project '${project.name}' is not deployed to target '${target.name}'. ` +
Expand All @@ -437,14 +443,15 @@ export class CdkBackend implements ProjectBackend {
const { spec } = project;
const deployedState = await readDeployedState(this.json, project.rootPath);
const recorded = deployedState.targets[target.name];
const stackReference = stackReferenceOf(recorded);

// No recorded stack means nothing was ever deployed to this target, which
// every resource below reports as local-only.
const stack = recorded?.stackArn
const stack = stackReference
? await this.describeStack(
target.region,
await this.credentialsForTarget(target),
recorded.stackArn,
stackReference,
)
: undefined;

Expand Down
25 changes: 25 additions & 0 deletions src/core/project/backends/cdk/deployedState.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
DEPLOYED_STATE_RELATIVE_PATH,
readDeployedState,
removeTargetState,
stackReferenceOf,
updateTargetState,
} from "./deployedState";

Expand Down Expand Up @@ -58,6 +59,30 @@ describe("readDeployedState", () => {
});
});

describe("stackReferenceOf", () => {
test("prefers the exact stack ARN over the legacy stack name", () => {
expect(
stackReferenceOf({
stackArn: "arn:stack:default",
resources: { stackName: "AgentCore-example-default" },
}),
).toBe("arn:stack:default");
});

test("falls back to the legacy stack name", () => {
expect(
stackReferenceOf({
resources: { stackName: "AgentCore-example-default" },
}),
).toBe("AgentCore-example-default");
});

test("returns undefined when no stack was recorded", () => {
expect(stackReferenceOf(undefined)).toBeUndefined();
expect(stackReferenceOf({})).toBeUndefined();
});
});

describe("updateTargetState", () => {
test("creates the file with the target entry", async () => {
const root = await projectRoot();
Expand Down
13 changes: 13 additions & 0 deletions src/core/project/backends/cdk/deployedState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ const CredentialStateSchema = z
const ResourceStateSchema = z
.object({
credentials: z.record(z.string(), CredentialStateSchema).optional(),
// The legacy deployer recorded the CloudFormation stack name
// here. New deploys record the stack ARN instead. Keep this
// field so projects can be correctly inspected after upgrading.
stackName: z.string().optional(),
})
.passthrough();

Expand All @@ -61,6 +65,15 @@ export const DeployedStateSchema = z
export type DeployedState = z.infer<typeof DeployedStateSchema>;
export type TargetState = z.infer<typeof TargetStateSchema>;

/**
* Returns the CloudFormation reference recorded for a target. New deploys bind
* to the exact stack ARN; legacy deploys recorded only the stack name under
* resources, which CloudFormation also accepts when describing the stack.
*/
export function stackReferenceOf(state: TargetState | undefined): string | undefined {
return state?.stackArn ?? state?.resources?.stackName;
}

function statePathFor(projectRoot: string): string {
return join(projectRoot, DEPLOYED_STATE_RELATIVE_PATH);
}
Expand Down
78 changes: 70 additions & 8 deletions src/handlers/project/status/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
import { mkdtemp, rm, writeFile } from "node:fs/promises";
import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { createRootHandler } from "../../index";
import {
createSilentLogger,
TestCoreClient,
TestGlobalConfigAccessor,
TestIdentityClient,
testIO,
ttyTestIO,
waitFor,
} from "../../../testing";
import type { ProjectBackend } from "../../../core/project";
import { CdkBackend, type ProjectBackend } from "../../../core/project";
import { ProjectStateError } from "../../../errors";
import type { AwsDeploymentTarget } from "../../../projectSchemas/aws-targets";
import type { ResolvedProjectResource } from "../types";
Expand Down Expand Up @@ -48,23 +49,26 @@ function fakeBackend(deployed: ResolvedProjectResource[]) {
return { targets, backend };
}

function testStatusCommand(deployed: ResolvedProjectResource[] = [], io = testIO()) {
const fake = fakeBackend(deployed);
const root = createRootHandler(new TestCoreClient({ backends: { CDK: fake.backend } }), {
function statusCommand(backend: ProjectBackend, io = testIO()) {
const root = createRootHandler(new TestCoreClient({ backends: { CDK: backend } }), {
io: io.io,
globalConfigAccessor: new TestGlobalConfigAccessor(),
logger: createSilentLogger(),
});

return {
...fake,
io,
json: () => JSON.parse(io.stdout()),
run: (args: string[] = []) => root.route(["node", "agentcore", "project", "status", ...args]),
create: (args: string[]) => root.route(["node", "agentcore", "project", ...args]),
};
}

function testStatusCommand(deployed: ResolvedProjectResource[] = [], io = testIO()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: are both testStatusCommand and statusCommand necessary? Feels like a lot of indirection and the names aren't clearly conveying the difference to me.

const fake = fakeBackend(deployed);
return { ...fake, ...statusCommand(fake.backend, io) };
}

const originalCwd = process.cwd();
const tempDirectories: string[] = [];

Expand All @@ -88,10 +92,10 @@ afterEach(() => {
});

async function inProject(
subject: ReturnType<typeof testStatusCommand>,
subject: ReturnType<typeof statusCommand>,
spec: Record<string, unknown> = {},
targets: AwsDeploymentTarget[] = TARGETS,
): Promise<void> {
): Promise<string> {
const directory = await mkdtemp(join(tmpdir(), "agentcore-status-"));
tempDirectories.push(directory);
process.chdir(directory);
Expand All @@ -102,6 +106,7 @@ async function inProject(
const current = JSON.parse(await Bun.file(specPath).text());
await writeFile(specPath, JSON.stringify({ ...current, ...spec }));
process.chdir(projectRoot);
return projectRoot;
}

const deployed = (
Expand Down Expand Up @@ -133,6 +138,63 @@ const HARNESS_ROW = localOnly("harness", "orders");
const memory = (name: string) => ({ name, eventExpiryDuration: 30 });
const policy = (name: string) => ({ name, statement: "permit(principal, action, resource);" });
describe("project status handler", () => {
test("reports resources deployed by a legacy CLI using resources.stackName", async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice! I like this test, it gives me very high confidence this won't regress and works as expected.

const stackName = "AgentCore-orders-default";
const backend = new CdkBackend({
logger: createSilentLogger(),
identity: new TestIdentityClient(),
resolveCredentials: async () => async () => ({
accessKeyId: "access-key",
secretAccessKey: "secret-key",
}),
resolveAccount: async () => DEFAULT_TARGET.account,
describeStack: async (_region, _credentials, reference) =>
reference === stackName
? {
StackName: stackName,
CreationTime: new Date(0),
StackStatus: "CREATE_COMPLETE",
Outputs: [
{
ExportName: `${stackName}-Harness-orders-Arn`,
OutputValue: `${ARN}:harness/orders-1`,
},
],
}
: undefined,
});
const subject = statusCommand(backend);
const projectRoot = await inProject(subject);
const stateDirectory = join(projectRoot, "agentcore", ".cli");
await mkdir(stateDirectory, { recursive: true });
await Bun.write(
join(stateDirectory, "deployed-state.json"),
JSON.stringify({
targets: {
default: {
resources: { stackName },
},
},
}),
);

await subject.run(["--json"]);

expect(subject.json()).toEqual({
projectName: "orders",
target: "default",
region: DEFAULT_TARGET.region,
resources: [
{
resourceType: "harness",
name: "orders",
deploymentState: "deployed",
id: `${ARN}:harness/orders-1`,
},
],
});
});

test("reports deployed resources by ARN, nesting children under their owner", async () => {
const subject = testStatusCommand([
HARNESS_ROW,
Expand Down
Loading