diff --git a/scripts/generate-v0-cdk-apps.ts b/scripts/generate-v0-cdk-apps.ts index a90a0075c..ca3cff1f4 100644 --- a/scripts/generate-v0-cdk-apps.ts +++ b/scripts/generate-v0-cdk-apps.ts @@ -1,9 +1,11 @@ #!/usr/bin/env bun /** - * Regenerates src/core/project/migrate/v0CdkApps.ts: what every CLI release that - * creates v0 projects (v0.4.0 to v0.31.1, 1.0.0-preview.1 to 1.0.0-preview.33) - * vended in agentcore/cdk, read from its git tag's src/assets/cdk. Releases that - * vended identical apps share an entry. Run from a clone with the release tags: + * Migration must recognize the CDK app every v0 release generated, so it can tell + * an unmodified app (safe to replace with v1's) from one with customer changes + * (which replacing would lose). This regenerates that list, + * src/core/project/migrate/v0CdkApps.ts, from each release's git tag + * (v0.4.0 to v0.31.1, 1.0.0-preview.1 to 1.0.0-preview.33), rather than keeping + * it by hand, so a release can't be missed. Run it from a clone with the tags: * * bun scripts/generate-v0-cdk-apps.ts */ diff --git a/src/core/project/migrate/__fixtures__/physical-names-contract.agentcore.json b/src/core/project/migrate/__fixtures__/physical-names-contract.agentcore.json index 07661bcac..f53ded78a 100644 --- a/src/core/project/migrate/__fixtures__/physical-names-contract.agentcore.json +++ b/src/core/project/migrate/__fixtures__/physical-names-contract.agentcore.json @@ -291,24 +291,7 @@ "strategies": [] } ], - "knowledgeBases": [ - { - "type": "AgentCoreKnowledgeBase", - "name": "kb-docs", - "description": "kb", - "dataSources": [ - { - "type": "S3", - "uri": "s3://my-bucket/docs" - }, - { - "type": "CONFLUENCE", - "connectorConfigFile": "kb/confluence.json" - } - ], - "gateway": "gw1" - } - ], + "knowledgeBases": [], "credentials": [ { "authorizerType": "ApiKeyCredentialProvider", @@ -592,7 +575,7 @@ "name": "Retrieve", "description": "d", "parameterValues": { - "knowledgeBaseId": "kb-docs" + "knowledgeBaseId": "KBDOCS001A" }, "parameterOverrides": [ { diff --git a/src/core/project/migrate/apply.test.ts b/src/core/project/migrate/apply.test.ts index 9054844ce..8bd27dc77 100644 --- a/src/core/project/migrate/apply.test.ts +++ b/src/core/project/migrate/apply.test.ts @@ -507,3 +507,182 @@ describe("physicalIdChanges", () => { ]); }); }); + +describe("applyMigration retaining unsupported resources", () => { + const DATASET = "ApplicationDatasetGolden8AF7116D"; + const retained = { + Resources: { + OldMemory: { Type: "AWS::BedrockAgentCore::Memory" }, + [DATASET]: { + Type: "AWS::BedrockAgentCore::Dataset", + DeletionPolicy: "Retain", + UpdateReplacePolicy: "Retain", + }, + }, + }; + const stack = (status: string) => ({ + Stacks: [{ StackName: "AgentCore-Proj-default", StackStatus: status, Parameters: [] }], + }); + const policyChange = { + Type: "Resource", + ResourceChange: { + Action: "Modify", + LogicalResourceId: DATASET, + Replacement: "False", + Details: [ + { Target: { Attribute: "DeletionPolicy" } }, + { Target: { Attribute: "UpdateReplacePolicy" } }, + ], + }, + }; + + function retainingTarget(execute: unknown = {}) { + const { client, sent } = fakeCloudFormation({ + DescribeStacks: [ + stack("UPDATE_COMPLETE"), + stack("UPDATE_IN_PROGRESS"), + stack("UPDATE_COMPLETE"), + ], + CreateChangeSet: [{ Id: "cs" }], + DescribeChangeSet: [{ Status: "CREATE_COMPLETE", Changes: [policyChange] }], + ExecuteChangeSet: [execute], + GetTemplate: [{ TemplateBody: JSON.stringify(retained) }], + ListStackResources: [ + stackResources([ + ["OldMemory", "mem"], + [DATASET, "ds"], + ]), + stackResources([ + ["NewMemory", "mem"], + [DATASET, "ds"], + ]), + ], + CreateStackRefactor: [{ StackRefactorId: "rf-default" }], + DescribeStackRefactor: [ + { Status: "CREATE_COMPLETE" }, + { ExecutionStatus: "EXECUTE_COMPLETE" }, + ], + ListStackRefactorActions: [ + moveActions("AgentCore-Proj-default", [["OldMemory", "NewMemory"]]), + ], + ExecuteStackRefactor: [{}], + }); + const entry: ApplyTarget = { + target: { name: "default", account: "111111111111", region: "us-east-1" }, + stackName: "AgentCore-Proj-default", + renames: new Map([["OldMemory", "NewMemory"]]), + bridge: { + Resources: { + NewMemory: retained.Resources.OldMemory, + [DATASET]: retained.Resources[DATASET], + }, + }, + retain: { template: retained, logicalIds: new Set([DATASET]) }, + client, + templates: async () => ({ put: async () => "", remove: async () => {} }), + }; + return { entry, sent }; + } + + test("sets the retain policies before the refactor, recording the change set while it runs", async () => { + const root = tempRoot(); + const { entry, sent } = retainingTarget(); + const { applier, saved } = config(root, freshState("default"), entry); + + const result = await applyMigration(applier); + + expect(result.migrated).toBe(true); + expect(result.stacks[0]).toMatchObject({ outcome: "refactored", refactorId: "rf-default" }); + expect(result.stacks[0]?.retainChangeSet).toStartWith("agentcore-migrate-retain-"); + const names = sent.map((command) => command.name); + expect(names.indexOf("ExecuteChangeSet")).toBeLessThan(names.indexOf("CreateStackRefactor")); + const statuses = saved.flatMap((state) => state.targets.map((t) => t.status)); + expect(statuses).toContain("retaining"); + expect(statuses.indexOf("retaining")).toBeLessThan(statuses.indexOf("refactoring")); + }); + + test("an unknown retain outcome stops before the refactor, with the change set's check commands", async () => { + const root = tempRoot(); + const { entry, sent } = retainingTarget(new Error("socket hang up")); + const { applier, events } = config(root, freshState("default"), entry); + + const result = await applyMigration(applier); + + expect(result.stacks[0]?.outcome).toBe("needs-repair"); + expect(sent.some((command) => command.name === "CreateStackRefactor")).toBe(false); + // An update whose outcome isn't known may have run, so the files stay as written. + expect(result.restored).toBeUndefined(); + expect(events).toEqual(["write"]); + const recovery = result.recovery.join("\n"); + expect(recovery).toContain( + "aws cloudformation describe-change-set --stack-name AgentCore-Proj-default", + ); + expect(recovery).toContain("agentcore migrate --target default"); + expect(recovery).not.toContain("describe-stack-refactor"); + const state = await readMigrationState(root); + expect(state?.targets[0]).toMatchObject({ status: "retaining" }); + }); + + test("a retain update CloudFormation refused puts the v0 files back", async () => { + const root = tempRoot(); + const { entry } = retainingTarget(clientError("ValidationError")); + const { applier, events } = config(root, freshState("default"), entry); + + const result = await applyMigration(applier); + + expect(result.stacks[0]?.outcome).toBe("failed"); + expect(result.restored).toBe(true); + expect(events).toEqual(["write", "restore"]); + }); + + test("a successful rerun cleans up the retain change set recorded by the unknown run", async () => { + const root = tempRoot(); + const { entry, sent } = target("default"); + const state = freshState("default"); + state.files!.status = "installed"; + state.targets[0] = { + ...state.targets[0]!, + status: "retaining", + changeSetName: "retain-from-unknown-run", + }; + const { applier } = config(root, state, entry); + + const result = await applyMigration(applier); + + expect(result.migrated).toBe(true); + expect(sent).toContainEqual({ + name: "DeleteChangeSet", + input: { + StackName: "AgentCore-Proj-default", + ChangeSetName: "retain-from-unknown-run", + }, + }); + }); + + test("a successful no-refactor rerun also cleans up the recorded retain change set", async () => { + const root = tempRoot(); + const { entry, sent } = target("default"); + entry.renames = new Map(); + entry.retain = undefined; + const state = freshState("default"); + state.files!.status = "installed"; + state.targets[0] = { + ...state.targets[0]!, + status: "retaining", + changeSetName: "retain-from-unknown-run", + }; + const { applier } = config(root, state, entry); + + const result = await applyMigration(applier); + + expect(result.migrated).toBe(true); + expect(result.stacks[0]?.outcome).toBe("unchanged"); + expect(sent).toContainEqual({ + name: "DeleteChangeSet", + input: { + StackName: "AgentCore-Proj-default", + ChangeSetName: "retain-from-unknown-run", + }, + }); + }); +}); diff --git a/src/core/project/migrate/apply.ts b/src/core/project/migrate/apply.ts index b4672d766..72aaa537a 100644 --- a/src/core/project/migrate/apply.ts +++ b/src/core/project/migrate/apply.ts @@ -34,9 +34,15 @@ import { type PollOptions, type RefactorAction, } from "./refactor"; +import { deleteRetainChangeSet, retainCheckCommands, retainResources } from "./retain"; import type { CfnTemplate } from "./templates"; import type { TemplateStore } from "./templateStore"; +/** + * Applying records each step in the journal before taking it, so a migration + * stopped at any point (a crash, Ctrl-C, a failed refactor) can be finished or + * undone, and deploying can refuse a target that's only half migrated. + */ /** The one deployment target whose stack this run refactors. */ export type ApplyTarget = { target: AwsDeploymentTarget; @@ -45,6 +51,11 @@ export type ApplyTarget = { renames: ReadonlyMap; /** The deployed template with the renames applied; absent when nothing moves. */ bridge?: CfnTemplate; + /** + * The update made before the refactor: the deployed template with retain + * policies on the unsupported resources the migration keeps. + */ + retain?: { template: CfnTemplate; logicalIds: ReadonlySet }; /** A CloudFormation client for the target's account and region. */ client: CloudFormationClient; /** Where a template too big to pass inline is uploaded. */ @@ -112,6 +123,9 @@ export async function applyMigration( }; const state: MigrationState = structuredClone(config.state); const entry = config.target; + const previousRetainChangeSet = entry + ? state.targets.find((target) => target.name === entry.target.name)?.changeSetName + : undefined; const results: StackRefactorResult[] = entry ? [ { @@ -184,29 +198,67 @@ export async function applyMigration( // 3. The selected target's stack. let outcome: StackRefactorResult["outcome"] | undefined; - if (entry && entry.renames.size === 0) { + if (entry && entry.renames.size === 0 && !entry.retain) { dropTarget(state, entry.target.name); outcome = "unchanged"; } else if (entry) { if (guard.stop()) { result({ outcome: "not-started", message: INTERRUPTED }); } else { + const record = (status: JournalTarget["status"], extra: Partial = {}) => + setTarget(state, { + name: entry.target.name, + account: entry.target.account, + region: entry.target.region, + stackName: entry.stackName, + status, + ...extra, + }); try { - await refactorTarget(entry, poll, async (refactorId) => { - result({ refactorId }); - setTarget(state, { - name: entry.target.name, - account: entry.target.account, - region: entry.target.region, - stackName: entry.stackName, - status: "refactoring", - refactorId, - renames: [...entry.renames], - }); + if (entry.retain) { + // Before the refactor, so the stack it moves (and the v1 deploy after it) + // already keeps the resources v1 has no counterpart for. + await retainResources( + { + stackName: entry.stackName, + region: entry.target.region, + template: entry.retain.template, + logicalIds: entry.retain.logicalIds, + client: entry.client, + templates: entry.templates, + }, + { + ...poll, + onCreated: async (changeSetName) => { + result({ retainChangeSet: changeSetName }); + record("retaining", { changeSetName }); + await save(rootPath, state); + }, + }, + ); + record("pending"); await save(rootPath, state); - }); - result({ outcome: "refactored" }); - markRefactored(state, entry.target.name); + } + if (entry.renames.size === 0) { + dropTarget(state, entry.target.name); + result({ outcome: "unchanged" }); + } else { + await refactorTarget(entry, poll, async (refactorId) => { + result({ refactorId }); + setTarget(state, { + name: entry.target.name, + account: entry.target.account, + region: entry.target.region, + stackName: entry.stackName, + status: "refactoring", + refactorId, + renames: [...entry.renames], + }); + await save(rootPath, state); + }); + result({ outcome: "refactored" }); + markRefactored(state, entry.target.name); + } } catch (error) { if (error instanceof ActionsMismatchError) { result({ outcome: "mismatch", message: error.message }); @@ -230,6 +282,14 @@ export async function applyMigration( } outcome = results[0]!.outcome; } + if ( + entry && + previousRetainChangeSet && + outcome !== undefined && + ["refactored", "unchanged"].includes(outcome) + ) { + await deleteRetainChangeSet(entry.client, entry.stackName, previousRetainChangeSet); + } // 4. Done, undone, or recorded for later. const stackError = results @@ -513,6 +573,20 @@ async function discard(rootPath: string): Promise { } function repairSteps(entry: StackRefactorResult): string[] { + if (entry.retainChangeSet && !entry.refactorId) { + const [changeSet, stack] = retainCheckCommands( + entry.stackName, + entry.region, + entry.retainChangeSet, + ); + return [ + `Stack ${entry.stackName} (target '${entry.target}') needs a look before anything else is ` + + `done to it: ${entry.message ?? "the outcome of the update that retains its unsupported resources isn't known"}. ` + + "That update only sets retain policies; it doesn't change or replace any resource.", + `Check it with \`${changeSet}\` and \`${stack}\`. Once the stack is UPDATE_COMPLETE (or ` + + `UPDATE_ROLLBACK_COMPLETE), run \`agentcore migrate --target ${entry.target}\` to carry on.`, + ]; + } const id = entry.refactorId ?? ""; const describe = `aws cloudformation describe-stack-refactor --stack-refactor-id ${id} --region ${entry.region}`; const list = `aws cloudformation list-stack-resources --stack-name ${entry.stackName} --region ${entry.region}`; diff --git a/src/core/project/migrate/contract.test.ts b/src/core/project/migrate/contract.test.ts index 910a92689..40e91cce0 100644 --- a/src/core/project/migrate/contract.test.ts +++ b/src/core/project/migrate/contract.test.ts @@ -40,7 +40,7 @@ describe("migratedFrom contract with @aws/agentcore-cdk", () => { "2dac5fc18e6893f04276cddab70ad50cd739968f6af25a314b34e8e5b7d5168c", ); expect(sha256("physical-names-contract.agentcore.json")).toBe( - "25e3f3a77da888068fba1709a8507419c33826b429e04b8d600e9e2b7f61db5d", + "1bd381c22d9919ecb7e7ee7be6f59dbb4d3013a6c0ce8e8282b2e12d46e3f761", ); }); diff --git a/src/core/project/migrate/convertHarness.ts b/src/core/project/migrate/convertHarness.ts index 2981c2ddf..5d5e53f44 100644 --- a/src/core/project/migrate/convertHarness.ts +++ b/src/core/project/migrate/convertHarness.ts @@ -30,6 +30,11 @@ export type ConvertedHarness = { facts?: HarnessFacts; }; +/** + * v1 reads harnesses from harness.yaml in a different shape than v0's + * harness.json; converting them keeps every harness working after the migration + * without the user rewriting its configuration by hand. + */ /** Reads a harness registry entry's `harness.json` and converts it to `harness.yaml`. */ export async function convertHarness( rootPath: string, diff --git a/src/core/project/migrate/convertSpec.test.ts b/src/core/project/migrate/convertSpec.test.ts index c80ddf35f..dcd440ada 100644 --- a/src/core/project/migrate/convertSpec.test.ts +++ b/src/core/project/migrate/convertSpec.test.ts @@ -10,6 +10,46 @@ type Json = Record; const NEVER_DEPLOYED: V0DeployedState = { targets: {} }; const f1 = () => V0AgentCoreProjectSpecSchema.parse(structuredClone(F1_SPEC)); const f1State = () => structuredClone(F1_DEPLOYED_STATE) as V0DeployedState; +const KB_ID = "KBDOCS001A"; +const KB_STATE = { + "kb-docs": { + knowledgeBaseId: KB_ID, + knowledgeBaseArn: `arn:aws:bedrock:us-east-1:998846730471:knowledge-base/${KB_ID}`, + dataSources: [], + }, +}; + +function withDeployedKnowledgeBase(state: V0DeployedState): V0DeployedState { + const copy = structuredClone(state); + const target = Object.values(copy.targets)[0]; + if (!target) return copy; + target.resources = { ...target.resources, knowledgeBases: KB_STATE }; + return copy; +} + +/** The audit suite is about v0/v1 schema differences, not the migration's KB-reference blocker. */ +function withExternalKnowledgeBaseReferences( + spec: ReturnType, +): ReturnType { + const copy = structuredClone(spec); + const owned = new Set(copy.knowledgeBases.map((kb) => kb.name)); + const visit = (value: unknown): void => { + if (Array.isArray(value)) { + for (const entry of value) visit(entry); + return; + } + if (typeof value !== "object" || value === null) return; + for (const [key, child] of Object.entries(value as Json)) { + if (key === "knowledgeBaseId" && typeof child === "string" && owned.has(child)) { + (value as Json)[key] = KB_ID; + } else { + visit(child); + } + } + }; + visit(copy.agentCoreGateways); + return copy; +} /** * The audit fixtures v1 validates more strictly than v0 (the schema audit; see the design doc), @@ -70,7 +110,10 @@ const EXPECTED_PROBLEMS: Record = { describe("convertSpec on the v0 audit fixtures", () => { test.each(validV0FixtureNames())("%s converts or names exactly what v1 rejects", (name) => { - const result = convertSpec(v0Fixture(name), NEVER_DEPLOYED); + const result = convertSpec( + withExternalKnowledgeBaseReferences(v0Fixture(name)), + NEVER_DEPLOYED, + ); const expected = EXPECTED_PROBLEMS[name]; if (expected) { expect(result.problems.map((problem) => problem.path)).toEqual(expected); @@ -94,7 +137,10 @@ describe("convertSpec on the v0 audit fixtures", () => { }); test("warns about the fields v1 drops, naming the resource", () => { - const { warnings } = convertSpec(v0Fixture("baseline.json"), NEVER_DEPLOYED); + const { warnings } = convertSpec( + withExternalKnowledgeBaseReferences(v0Fixture("baseline.json")), + NEVER_DEPLOYED, + ); expect(warnings).toEqual([ "Dropped fields v1 doesn't use: credentials[my-oauth].usage, credentials[gw1-oauth].usage", ]); @@ -114,6 +160,84 @@ describe("convertSpec on the v0 audit fixtures", () => { expect("mcpRuntimeTools" in json).toBe(false); expect((json.toolRuntimes as Json[])[0]?.name).toBe("rtTool"); }); + + test("drops knowledge bases and rewrites a connector to the retained deployed ID", () => { + const result = convertSpec( + v0Fixture("baseline.json"), + withDeployedKnowledgeBase({ + targets: { default: { resources: {} } }, + }), + ); + expect(result.problems).toEqual([]); + expect(result.json.knowledgeBases).toEqual([]); + expect(JSON.stringify(result.json)).not.toContain('"knowledgeBaseId":"kb-docs"'); + expect(JSON.stringify(result.json)).toContain(`"knowledgeBaseId":"${KB_ID}"`); + expect(result.warnings).toContainEqual( + expect.stringContaining( + `Rewrote agentCoreGateways[gw1].targets[kbconn].configurations[Retrieve].parameterValues.knowledgeBaseId`, + ), + ); + }); + + test("blocks a connector whose removed knowledge base has no unique deployed ID", () => { + const result = convertSpec(v0Fixture("baseline.json"), NEVER_DEPLOYED); + expect("knowledgeBases" in result.json).toBe(false); + expect(result.problems).toContainEqual({ + file: "agentcore/agentcore.json", + path: "agentCoreGateways[gw1].targets[kbconn].configurations[Retrieve].parameterValues.knowledgeBaseId", + message: expect.stringContaining("no deployed knowledge base ID is recorded"), + }); + }); + + test("rewrites project knowledge base names in agentic retrievers too", () => { + const v0 = v0Fixture("baseline.json"); + const connector = v0.agentCoreGateways[0]!.targets.find( + (target) => + target.targetType === "connector" && target.connectorId === "bedrock-knowledge-bases", + )!; + const agentic = connector.configurations!.find( + (configuration) => configuration.name === "AgenticRetrieveStream", + )!; + const retriever = ( + agentic.parameterValues!.retrievers as Array<{ + configuration: { knowledgeBase: { knowledgeBaseId: string } }; + }> + )[0]!; + retriever.configuration.knowledgeBase.knowledgeBaseId = "kb-docs"; + + const result = convertSpec( + v0, + withDeployedKnowledgeBase({ + targets: { default: { resources: {} } }, + }), + ); + + expect(result.problems).toEqual([]); + expect(retriever.configuration.knowledgeBase.knowledgeBaseId).toBe("kb-docs"); + expect(JSON.stringify(result.json)).not.toContain('"knowledgeBaseId":"kb-docs"'); + expect( + result.warnings.some( + (warning) => warning.includes("AgenticRetrieveStream") && warning.includes(KB_ID), + ), + ).toBe(true); + }); + + test("drops insights-only online eval configs without blocking the project", () => { + const v0 = withExternalKnowledgeBaseReferences(v0Fixture("baseline.json")); + v0.capacityProviders = []; + v0.runtimes = v0.runtimes.map( + ({ + capacityProviderConfiguration: _capacity, + filesystemConfigurations: _mounts, + ...runtime + }) => runtime, + ); + const result = convertSpec(v0, NEVER_DEPLOYED); + expect(result.problems).toEqual([]); + expect((result.json.onlineEvalConfigs as Json[]).map((config) => config.name)).not.toContain( + "oe2", + ); + }); }); describe("convertSpec kept names", () => { @@ -335,7 +459,9 @@ describe("convertTargets", () => { describe("convertSpec pins for tool runtimes, gateway-target compute, and containers", () => { const recorded = (resources: Record): V0DeployedState => - ({ targets: { default: { resources } } }) as V0DeployedState; + ({ + targets: { default: { resources: { ...resources, knowledgeBases: KB_STATE } } }, + }) as V0DeployedState; test("pins deployed tool runtimes, target Lambdas, and Container runtime repositories", () => { const state = recorded({ @@ -406,7 +532,10 @@ describe("convertSpec pins for tool runtimes, gateway-target compute, and contai }, }, }; - const { problems } = convertSpec(v0Fixture("top-mcpRuntimeTools.json"), state); + const { problems } = convertSpec( + v0Fixture("top-mcpRuntimeTools.json"), + withDeployedKnowledgeBase(state), + ); expect(problems.map((problem) => problem.path)).toEqual([ expect.stringMatching(/^gateway-target-function .+\/lambdaTs$/), ]); @@ -527,7 +656,7 @@ describe("convertSpec dropped fields", () => { }, }, }; - const result = convertSpec(v0, deployed); + const result = convertSpec(v0, withDeployedKnowledgeBase(deployed)); expect(result.deployedChanges).toEqual([ { file: "agentcore/agentcore.json", diff --git a/src/core/project/migrate/convertSpec.ts b/src/core/project/migrate/convertSpec.ts index 34ad434dd..b94fd1fba 100644 --- a/src/core/project/migrate/convertSpec.ts +++ b/src/core/project/migrate/convertSpec.ts @@ -14,6 +14,11 @@ import { onlineInsightConfigs } from "./preflight"; type Json = Record; +/** + * Converting agentcore.json is done in memory and in a temporary copy first, so + * the plan can show exactly what the v1 files will be, and every problem with + * them, before the user agrees to change anything. + */ /** Project-relative paths shown to users. Always forward slashes, on every platform. */ export const SPEC_RELATIVE_PATH = "agentcore/agentcore.json"; export const AWS_TARGETS_RELATIVE_PATH = "agentcore/aws-targets.json"; @@ -56,6 +61,8 @@ type HarnessRecord = Recorded & { roleArn: string; provisioner?: string }; export type DeployedResources = { runtimes: Map; memories: Map; + /** Knowledge base IDs, used to rewrite connector references when KBs leave the project. */ + knowledgeBases: Map; evaluators: Map; onlineEvalConfigs: Map; policyEngines: Map; @@ -82,6 +89,7 @@ export function deployedResources(state: V0DeployedState): DeployedResources { const deployed: DeployedResources = { runtimes: new Map(), memories: new Map(), + knowledgeBases: new Map(), evaluators: new Map(), onlineEvalConfigs: new Map(), policyEngines: new Map(), @@ -106,6 +114,9 @@ export function deployedResources(state: V0DeployedState): DeployedResources { }; add(deployed.runtimes, resources.runtimes, (r) => ({ id: r.runtimeId })); add(deployed.memories, resources.memories, (m) => ({ id: m.memoryId })); + add(deployed.knowledgeBases, resources.knowledgeBases, (kb) => ({ + id: kb.knowledgeBaseId, + })); add(deployed.evaluators, resources.evaluators, (e) => ({ id: e.evaluatorId })); add(deployed.onlineEvalConfigs, resources.onlineEvalConfigs, (o) => ({ id: o.onlineEvaluationConfigId, @@ -150,15 +161,24 @@ export function convertSpec( const { spec } = translateV0Spec(v0); const { unassignedTargets, abTests } = v0; const candidate: Json = { ...(spec as unknown as Json), migratedFrom: MIGRATED_FROM }; - // v1 has no online insights. Preflight blocks a project that uses them; dropping - // them here lets the rest of the project convert, so one preview reports everything. + const warnings: string[] = []; + const deployed = deployedResources(state); + + // Knowledge bases and insights-only online evals stay in AWS outside the stack, + // so neither collection is carried into the v1 project. + delete candidate.knowledgeBases; const insights = new Set(onlineInsightConfigs(v0).map((config) => config.name)); if (insights.size > 0 && Array.isArray(candidate.onlineEvalConfigs)) { candidate.onlineEvalConfigs = (candidate.onlineEvalConfigs as Json[]).filter( (config) => !insights.has(String(config.name)), ); } - const warnings: string[] = []; + const knowledgeBaseReferences = rewriteKnowledgeBaseReferences( + candidate, + new Set(v0.knowledgeBases.map((kb) => kb.name)), + deployed, + ); + warnings.push(...knowledgeBaseReferences.warnings); if (unassignedTargets?.length) { warnings.push( @@ -173,14 +193,18 @@ export function convertSpec( ); } - const deployed = deployedResources(state); const everDeployed = Object.values(state.targets).some( ({ resources }) => resources !== undefined, ); const { pins, problems } = pinDeployedNames(v0.name, candidate, deployed, harnesses); + problems.unshift(...knowledgeBaseReferences.problems); problems.push(...pinCollisions(v0.name, candidate, pins)); - const parsed = ProjectSpecSchema.safeParse(candidate); + const validationCandidate = structuredClone(candidate); + for (const path of knowledgeBaseReferences.blockedPaths) { + setAtPath(validationCandidate, path, "AAAAAAAAAA"); + } + const parsed = ProjectSpecSchema.safeParse(validationCandidate); if (!parsed.success) { return { json: candidate, @@ -208,7 +232,144 @@ export function convertSpec( if (harmless.length > 0 || undeployed.length > 0) { warnings.push(`${DROPPED_FIELDS_WARNING}: ${[...harmless, ...undeployed].join(", ")}`); } - return { json: parsed.data as unknown as Json, pins, warnings, problems, deployedChanges }; + return { + json: + knowledgeBaseReferences.problems.length > 0 ? candidate : (parsed.data as unknown as Json), + pins, + warnings, + problems, + deployedChanges, + }; +} + +const REAL_KB_ID_PATTERN = /^[A-Z0-9]{10}$/; + +/** + * A gateway connector can outlive a project-owned knowledge base, but its v1 + * config cannot keep referring to the KB by project name once knowledgeBases[] + * is removed. Replace that name with the one unambiguous deployed ID; otherwise + * report the field as a blocker instead of writing a dangling reference. + */ +function rewriteKnowledgeBaseReferences( + spec: Json, + projectKnowledgeBases: ReadonlySet, + deployed: DeployedResources, +): { warnings: string[]; problems: MigrationProblem[]; blockedPaths: PropertyKey[][] } { + const warnings: string[] = []; + const problems: MigrationProblem[] = []; + const blockedPaths: PropertyKey[][] = []; + const gateways = Array.isArray(spec.agentCoreGateways) ? (spec.agentCoreGateways as Json[]) : []; + + const rewrite = ( + container: Json, + key: string, + path: PropertyKey[], + gatewayName: string, + targetName: string, + ) => { + const name = container[key]; + if (typeof name !== "string" || !projectKnowledgeBases.has(name)) return; + const ids = [ + ...new Set((deployed.knowledgeBases.get(name) ?? []).map((record) => record.id)), + ].sort(); + const field = describePath(spec, path); + if (ids.length === 1 && REAL_KB_ID_PATTERN.test(ids[0]!)) { + container[key] = ids[0]!; + warnings.push( + `Rewrote ${field} from project knowledge base '${name}' to its retained AWS ID ` + + `'${ids[0]}', so gateway connector '${gatewayName}/${targetName}' stays wired after ` + + "knowledgeBases[] is removed.", + ); + return; + } + const reason = + ids.length === 0 + ? "no deployed knowledge base ID is recorded" + : `the deployed targets record different or invalid IDs (${ids.join(", ")})`; + problems.push({ + file: SPEC_RELATIVE_PATH, + path: field, + message: + `references project knowledge base '${name}', which migrate removes from agentcore.json, ` + + `but ${reason}, so the connector can't be rewritten safely. Change this field to the ` + + "10-character AWS ID the gateway connector should keep using, then migrate again", + }); + blockedPaths.push(path); + }; + + for (const [gatewayIndex, gateway] of gateways.entries()) { + const targets = Array.isArray(gateway.targets) ? (gateway.targets as Json[]) : []; + for (const [targetIndex, target] of targets.entries()) { + if (target.targetType !== "connector" || target.connectorId !== "bedrock-knowledge-bases") { + continue; + } + const configurations = Array.isArray(target.configurations) + ? (target.configurations as Json[]) + : []; + for (const [configIndex, config] of configurations.entries()) { + const parameterValues = + typeof config.parameterValues === "object" && + config.parameterValues !== null && + !Array.isArray(config.parameterValues) + ? (config.parameterValues as Json) + : undefined; + if (!parameterValues) continue; + const base = [ + "agentCoreGateways", + gatewayIndex, + "targets", + targetIndex, + "configurations", + configIndex, + "parameterValues", + ]; + if (config.name === "Retrieve") { + rewrite( + parameterValues, + "knowledgeBaseId", + [...base, "knowledgeBaseId"], + String(gateway.name), + String(target.name), + ); + } + if (config.name !== "AgenticRetrieveStream") continue; + const retrievers = Array.isArray(parameterValues.retrievers) + ? (parameterValues.retrievers as Json[]) + : []; + for (const [retrieverIndex, retriever] of retrievers.entries()) { + const configuration = retriever.configuration as Json | undefined; + const knowledgeBase = configuration?.knowledgeBase as Json | undefined; + if (!knowledgeBase) continue; + rewrite( + knowledgeBase, + "knowledgeBaseId", + [ + ...base, + "retrievers", + retrieverIndex, + "configuration", + "knowledgeBase", + "knowledgeBaseId", + ], + String(gateway.name), + String(target.name), + ); + } + } + } + } + return { warnings, problems, blockedPaths }; +} + +/** Mutates one value in a JSON-shaped object by a path already discovered in it. */ +function setAtPath(root: Json, path: PropertyKey[], value: unknown): void { + let current: unknown = root; + for (const segment of path.slice(0, -1)) { + if (typeof current !== "object" || current === null) return; + current = (current as Record)[segment]; + } + if (typeof current !== "object" || current === null) return; + (current as Record)[path.at(-1)!] = value; } /** diff --git a/src/core/project/migrate/index.test.ts b/src/core/project/migrate/index.test.ts index 99fe93581..030244445 100644 --- a/src/core/project/migrate/index.test.ts +++ b/src/core/project/migrate/index.test.ts @@ -274,7 +274,7 @@ describe("FsProjectMigrator.plan", () => { }); }); - test("blocks on unsupported resources and an old CDK app", async () => { + test("an old CDK app blocks; datasets don't (they're kept outside the stack)", async () => { const plan = await migrator().plan( await project({ cdkPin: "0.1.0-alpha.30", @@ -291,7 +291,7 @@ describe("FsProjectMigrator.plan", () => { }), ); const failed = plan.checks.filter((check) => !check.ok).map((check) => check.name); - expect(failed).toEqual(["cdk dependencies", "supported resources"]); + expect(failed).toEqual(["cdk dependencies"]); expect(plan.ready).toBe(false); }); }); diff --git a/src/core/project/migrate/index.ts b/src/core/project/migrate/index.ts index 78223e189..82180d969 100644 --- a/src/core/project/migrate/index.ts +++ b/src/core/project/migrate/index.ts @@ -54,6 +54,7 @@ import { import { planStack, type PlannedStack } from "./plan"; import { describe as describeRefactor, type PollOptions } from "./refactor"; import { preflight } from "./preflight"; +import { RETAINED_GUIDANCE } from "./retain"; import { saveMigrationReport } from "./reports"; import { checkStack, cloudFormationStackReader } from "./stacks"; import { @@ -372,6 +373,7 @@ export class FsProjectMigrator implements ProjectMigrator { stackName, renames: planned.renames, ...(planned.bridge && { bridge: planned.bridge }), + ...(planned.retain && { retain: planned.retain }), client: entry.client!, templates: async () => (store ??= @@ -466,8 +468,11 @@ export class FsProjectMigrator implements ProjectMigrator { ok: false, message: `${unsupported}, so the first deploy after migrating would replace this project's ` + - "resources. Update the AgentCore CLI to a release with that library, then run " + - "agentcore migrate again.", + "resources. Run `agentcore update`, then run `agentcore migrate` again.", + summary: + `This CLI can't keep this project's resource names yet (it vends ${CDK_LIBRARY} ` + + `${vended ?? "(unknown)"}; migrating needs ${MIN_MIGRATED_FROM_CDK_VERSION} or later). ` + + "Run `agentcore update`, then run `agentcore migrate` again.", }, warnings: [], }; @@ -837,6 +842,16 @@ export class FsProjectMigrator implements ProjectMigrator { v1StackName: artifact.stackName, pins: pins.filter((pin) => pin.targets.includes(target.name)), }); + if (stack.plan.retained && entry.client) { + // The plan names what it keeps by physical ID, so the user can find it later. + const ids = await physicalIds(entry.client, stackName); + for (const resource of stack.plan.retained) { + const physicalId = ids.get(resource.logicalId); + if (physicalId) resource.physicalId = physicalId; + const guidance = RETAINED_GUIDANCE[resource.type]; + if (guidance) resource.guidance = guidance; + } + } planned.push({ ...entry, planned: stack }); } catch (error) { // One target that can't be planned still leaves the rest of the report. diff --git a/src/core/project/migrate/journal.ts b/src/core/project/migrate/journal.ts index 342e29fc7..cb83b0036 100644 --- a/src/core/project/migrate/journal.ts +++ b/src/core/project/migrate/journal.ts @@ -20,7 +20,9 @@ const FileChangeSchema = z.object({ /** * A deployment target whose stack still has its v0 logical IDs. `pending` hasn't - * been refactored; `refactoring` had a refactor created (with `refactorId`), whose + * been refactored; `retaining` had the change set that retains its unsupported + * resources created (`changeSetName`), so a rerun plans it afresh from what the + * stack then holds; `refactoring` had a refactor created (with `refactorId`), whose * outcome a rerun reads back before doing anything else. */ const JournalTargetSchema = z.object({ @@ -28,7 +30,9 @@ const JournalTargetSchema = z.object({ account: z.string(), region: z.string(), stackName: z.string(), - status: z.enum(["pending", "refactoring"]), + status: z.enum(["pending", "retaining", "refactoring"]), + /** The change set that retains the target's unsupported resources, recorded while `retaining`. */ + changeSetName: z.string().optional(), refactorId: z.string().optional(), /** The refactor's logical ID renames (deployed → v1), recorded with `refactorId`. */ renames: z.array(z.tuple([z.string(), z.string()])).optional(), diff --git a/src/core/project/migrate/mapping.ts b/src/core/project/migrate/mapping.ts index 6f1782a9a..42b36d6a2 100644 --- a/src/core/project/migrate/mapping.ts +++ b/src/core/project/migrate/mapping.ts @@ -11,6 +11,12 @@ import { type ResourceReference, } from "./templates"; +/** + * v1 gives the same AWS resources different logical IDs than v0 did. Pairing them + * by what they are (their physical names and how they reference each other) + * rather than by how the IDs look lets the refactor move each resource to its v1 + * ID instead of the deploy recreating it. + */ /** The path recorded for a DependsOn entry, which no property path can equal. */ const DEPENDS_ON_PATH = "(DependsOn)"; diff --git a/src/core/project/migrate/migrate-test-support.ts b/src/core/project/migrate/migrate-test-support.ts index b53bdc22d..04b01b5c5 100644 --- a/src/core/project/migrate/migrate-test-support.ts +++ b/src/core/project/migrate/migrate-test-support.ts @@ -677,6 +677,13 @@ export function maxprojDeployedState(): V0DeployedState { memoryId: maxprojId(`maxproj_${name}`), memoryArn: `${MAXPROJ_ARN}:memory/${maxprojId(`maxproj_${name}`)}`, })), + knowledgeBases: { + "kb-docs": { + knowledgeBaseId: "KBDOCS001A", + knowledgeBaseArn: "arn:aws:bedrock:us-east-1:998846730471:knowledge-base/KBDOCS001A", + dataSources: [], + }, + }, evaluators: byName(["evalNum", "evalCat", "evalCode", "evalExt"], (name) => ({ evaluatorId: maxprojId(`maxproj_${name}`), evaluatorArn: `${MAXPROJ_ARN}:evaluator/${maxprojId(`maxproj_${name}`)}`, diff --git a/src/core/project/migrate/pinCollisions.ts b/src/core/project/migrate/pinCollisions.ts index 690d0b9d3..df6dd3e7e 100644 --- a/src/core/project/migrate/pinCollisions.ts +++ b/src/core/project/migrate/pinCollisions.ts @@ -8,6 +8,11 @@ import { describePath } from "./issues"; type Json = Record; +/** + * A migrated project keeps v0's physical names, which have no target segment, so + * two resources (or two targets in one account and region) can claim the same + * name. Finding that in the plan beats a deploy failing on a name conflict. + */ /** One resource that takes an AWS physical name: its deployed name, or the one v0 naming composes. */ type NameEntry = { label: string; diff --git a/src/core/project/migrate/plan.test.ts b/src/core/project/migrate/plan.test.ts index b74266a57..d159c0c05 100644 --- a/src/core/project/migrate/plan.test.ts +++ b/src/core/project/migrate/plan.test.ts @@ -18,6 +18,127 @@ const plan = async (deployed: string, v1: string, v1StackName = STACK, pins: Pin pins, }); +describe("planStack with a dataset v1 has no counterpart for", () => { + const DATASET = "ApplicationDatasetGolden8AF7116D"; + const withDataset = (): CfnTemplate => { + const deployed = templateFixture("pinned.v0"); + deployed.Resources![DATASET] = { + Type: "AWS::BedrockAgentCore::Dataset", + Properties: { + DatasetName: "CompatMig_golden", + SchemaType: "AGENTCORE_EVALUATION_PREDEFINED_V1", + }, + }; + return deployed; + }; + const planWith = (deployed: CfnTemplate) => + planStack({ + target, + stackName: STACK, + v1StackName: STACK, + deployed, + v1: templateFixture("pinned.v1"), + pins: [], + }); + + test("is retained first, then refactored and left in the account", async () => { + const { plan: stack, retain, bridge } = await planWith(withDataset()); + expect(stack.ready).toBe(true); + expect(stack.retained).toEqual([ + { logicalId: DATASET, type: "AWS::BedrockAgentCore::Dataset" }, + ]); + expect([...retain!.logicalIds]).toEqual([DATASET]); + expect(retain!.template.Resources![DATASET]).toMatchObject({ + DeletionPolicy: "Retain", + UpdateReplacePolicy: "Retain", + }); + // The refactor starts from the retained stack, so its template must carry the policies. + expect(bridge!.Resources![DATASET]!.DeletionPolicy).toBe("Retain"); + const removed = stack.removed.find((change) => change.logicalId === DATASET)!; + expect(removed.severity).toBe("info"); + expect(removed.reason).toContain("leaves it, and its data, in the account outside the stack"); + }); + + test("a dataset the stack already retains needs no update, only the refactor", async () => { + const deployed = withDataset(); + deployed.Resources![DATASET]!.DeletionPolicy = "Retain"; + deployed.Resources![DATASET]!.UpdateReplacePolicy = "Retain"; + const { plan: stack, retain } = await planWith(deployed); + expect(stack.ready).toBe(true); + expect(retain).toBeUndefined(); + expect(stack.retained).toHaveLength(1); + }); +}); + +describe("planStack with knowledge bases and insights-only online evals", () => { + test("retains each resource and the dropped resources connected to it", async () => { + const deployed = templateFixture("pinned.v0"); + Object.assign(deployed.Resources!, { + KnowledgeBase: { + Type: "AWS::Bedrock::KnowledgeBase", + Properties: { RoleArn: { "Fn::GetAtt": ["KnowledgeBaseRole", "Arn"] } }, + }, + KnowledgeBaseDataSource: { + Type: "AWS::Bedrock::DataSource", + Properties: { KnowledgeBaseId: { Ref: "KnowledgeBase" } }, + }, + KnowledgeBaseRole: { + Type: "AWS::IAM::Role", + Properties: { AssumeRolePolicyDocument: { Statement: [] } }, + }, + KnowledgeBasePolicy: { + Type: "AWS::IAM::Policy", + Properties: { + Roles: [{ Ref: "KnowledgeBaseRole" }], + PolicyDocument: { Statement: [] }, + }, + }, + InsightsEval: { + Type: "AWS::BedrockAgentCore::OnlineEvaluationConfig", + Properties: { ExecutionRoleArn: { "Fn::GetAtt": ["InsightsRole", "Arn"] } }, + }, + InsightsRole: { + Type: "AWS::IAM::Role", + Properties: { AssumeRolePolicyDocument: { Statement: [] } }, + }, + InsightsPolicy: { + Type: "AWS::IAM::Policy", + Properties: { + Roles: [{ Ref: "InsightsRole" }], + PolicyDocument: { Statement: [] }, + }, + }, + }); + + const { plan: stack, retain } = await planStack({ + target, + stackName: STACK, + v1StackName: STACK, + deployed, + v1: templateFixture("pinned.v1"), + pins: [], + }); + + const expected = [ + "InsightsEval", + "InsightsPolicy", + "InsightsRole", + "KnowledgeBase", + "KnowledgeBaseDataSource", + "KnowledgeBasePolicy", + "KnowledgeBaseRole", + ]; + expect(stack.ready).toBe(true); + expect(stack.retained?.map((resource) => resource.logicalId)).toEqual(expected); + expect([...retain!.logicalIds]).toEqual(expected); + expect( + stack.removed + .filter((change) => expected.includes(change.logicalId)) + .every((change) => change.severity === "info"), + ).toBe(true); + }); +}); + describe("planStack", () => { test("the live CompatMig stack with pinned names: 10 moves and 0 replacements", async () => { const { plan: stack, bridge, renames } = await plan("pinned.v0", "pinned.v1"); diff --git a/src/core/project/migrate/plan.ts b/src/core/project/migrate/plan.ts index 34d7fc971..134cb4d6d 100644 --- a/src/core/project/migrate/plan.ts +++ b/src/core/project/migrate/plan.ts @@ -9,9 +9,16 @@ import type { AwsDeploymentTarget } from "../../../projectSchemas/aws-targets"; import { buildBridgeTemplate, verifyBridge } from "./bridge"; import { mapLogicalIds, NAME_PROPERTIES, renamesOf, type LogicalIdMapping } from "./mapping"; import { deployImpact } from "./replacements"; +import { resourcesToRetain, withRetain } from "./retain"; import { CDK_METADATA_TYPE, resourcesOf, type CfnTemplate } from "./templates"; import { resourcesOutsideV0App } from "./v0CdkApp"; +/** + * Planning is where a migration proves it's safe before it touches anything: it + * works out the stack refactor and predicts the first v1 deploy, so a plan that + * would replace or delete a resource the user needs is refused up front instead + * of discovered in AWS. + */ export type StackPlanInput = { target: AwsDeploymentTarget; /** The deployed stack's name. */ @@ -34,6 +41,11 @@ export type PlannedStack = { renames: Map; /** The template the refactor moves the stack to; absent when it couldn't be built. */ bridge?: CfnTemplate; + /** + * The update to make before the refactor: the deployed template with retain + * policies on the unsupported resources it keeps. Absent when nothing needs it. + */ + retain?: { template: CfnTemplate; logicalIds: Set }; }; /** @@ -99,15 +111,39 @@ export async function planStack(input: StackPlanInput): Promise { ); } + // Resources migrate deliberately leaves out of v1 are kept: the stack is first + // updated to retain them and their dropped dependencies, so the refactor and + // the deploy start from there. + const retained = resourcesToRetain( + deployed, + new Set(mapping.onlyInOld.map((resource) => resource.logicalId)), + ); + let start = deployed; + let retain: PlannedStack["retain"]; + if (retained.length > 0) { + plan.retained = retained.map(({ logicalId, type }) => ({ logicalId, type })); + start = withRetain( + deployed, + retained.map((resource) => resource.logicalId), + ); + const needed = retained.filter((resource) => !resource.alreadyRetained); + if (needed.length > 0) { + retain = { + template: start, + logicalIds: new Set(needed.map((resource) => resource.logicalId)), + }; + } + } + let bridge: CfnTemplate | undefined; try { - bridge = buildBridgeTemplate(deployed, renames); + bridge = buildBridgeTemplate(start, renames); } catch (error) { plan.blockers.push(`Couldn't build the refactor template: ${(error as Error).message}`); } if (bridge) { plan.blockers.push( - ...verifyBridge(deployed, bridge, renames).map( + ...verifyBridge(start, bridge, renames).map( (problem) => `The refactor template would change more than logical IDs: ${problem}`, ), ); @@ -120,7 +156,7 @@ export async function planStack(input: StackPlanInput): Promise { } plan.ready = bridge !== undefined && stackBlockerCount(plan) === 0; - return { plan, mapping, renames, ...(bridge && { bridge }) }; + return { plan, mapping, renames, ...(bridge && { bridge }), ...(retain && { retain }) }; } /** How a kept name's resource kind reads in a message. */ diff --git a/src/core/project/migrate/preflight.test.ts b/src/core/project/migrate/preflight.test.ts index 3391f811b..9a225f87a 100644 --- a/src/core/project/migrate/preflight.test.ts +++ b/src/core/project/migrate/preflight.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, test } from "bun:test"; +import { readFileSync, writeFileSync } from "node:fs"; import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { V0AgentCoreProjectSpecSchema } from "../../../projectSchemas/v0"; @@ -124,79 +125,89 @@ describe("checkCdkPin", () => { }); describe("checkSupportedResources", () => { + const without = (spec: ReturnType, ...drop: Array<"insights" | "capacity">) => + ({ + ...spec, + ...(drop.includes("insights") && { + onlineEvalConfigs: spec.onlineEvalConfigs.filter((config) => config.insights === undefined), + }), + ...(drop.includes("capacity") && { + capacityProviders: [], + runtimes: spec.runtimes.map( + ({ capacityProviderConfiguration: _cp, filesystemConfigurations: _fs, ...runtime }) => + runtime, + ), + }), + }) as typeof spec; + test("passes a project without unsupported resources", () => { expect(checkSupportedResources(V0AgentCoreProjectSpecSchema.parse(F1_SPEC))).toEqual({ name: "supported resources", ok: true, - message: "no knowledge bases, datasets, capacity providers, or online insights", + message: "no capacity providers or runtimes on them", }); }); - test("blocks online insights, which v1 removed", () => { - const check = checkSupportedResources(v0Fixture("baseline.json")); - expect(check.ok).toBe(false); - expect(check.message).toContain("online insights (oe2)"); + test("lets knowledge bases, datasets, and online insights through to be retained", () => { + const spec = without(v0Fixture("baseline.json"), "insights", "capacity"); + expect(spec.knowledgeBases.length).toBeGreaterThan(0); + expect(checkSupportedResources(spec).ok).toBe(true); + expect(checkSupportedResources(v0Fixture("top-datasets-nonempty.json")).message).not.toContain( + "datasets", + ); + const insightsOnly = without(v0Fixture("baseline.json"), "capacity"); + expect(insightsOnly.onlineEvalConfigs.some((config) => config.insights !== undefined)).toBe( + true, + ); + expect(checkSupportedResources(insightsOnly).ok).toBe(true); }); - test("blocks knowledge bases, naming them", () => { - const check = checkSupportedResources(v0Fixture("baseline.json")); + test("lists what still blocks on one summary line, and says why in the full message", () => { + const check = checkSupportedResources(v0Fixture("runtime-capacityProvider.json")); expect(check.ok).toBe(false); - expect(check.message).toMatch( - /^This project uses knowledge bases \(.+\), which v1 doesn't support yet/, + expect(check.summary).toStartWith( + "Migration blocked: This project uses resources that v1 doesn't support yet: ", ); + expect(check.summary).toContain("capacity providers ("); + expect(check.summary).not.toContain("online insights"); expect(check.message).toContain("compatibility mode"); + expect(check.message).toEndWith(`See ${PRESERVE_RESOURCES_DOCS_URL}`); }); - test("gives a keep-the-data path and a delete path, linking the preserve guide", () => { - const message = checkSupportedResources(v0Fixture("baseline.json")).message; - expect(message).toContain(" To keep them and their data in AWS:\n"); - expect(message).toContain( - " 1. In agentcore/cdk/lib/cdk-stack.ts, set DeletionPolicy Retain on them", - ); - expect(message).toContain( - " 2. Run `agentcore deploy` (it deploys a v0 project with the v0 CLI).\n", - ); - expect(message).toContain("They leave the stack but stay in AWS.\n"); - expect(message).toContain( - " 4. Undo the change to cdk-stack.ts, commit, and run `agentcore migrate --dry-run`.\n", - ); - expect(message).toContain("This deletes them from AWS.\n"); - expect(message).toEndWith(`See ${PRESERVE_RESOURCES_DOCS_URL}`); + test("never tells the user to delete anything", () => { + for (const fixture of ["baseline.json", "runtime-capacityProviderArn-only.json"]) { + const { message } = checkSupportedResources(v0Fixture(fixture)); + expect(message).not.toMatch(/delet|remove them/i); + } }); - test("tells online insights alone to just be removed, with no keep path", () => { - const spec = v0Fixture("baseline.json"); - const insightsOnly = { - ...spec, - knowledgeBases: [], - datasets: [], - capacityProviders: [], - runtimes: spec.runtimes.map( - ({ capacityProviderConfiguration: _cp, filesystemConfigurations: _fs, ...runtime }) => - runtime, - ), - } as typeof spec; - const check = checkSupportedResources(insightsOnly); + test("blocks capacity providers and the runtimes that use them, saying why", () => { + const check = checkSupportedResources(v0Fixture("runtime-capacityProviderArn-only.json")); expect(check.ok).toBe(false); - expect(check.message).toStartWith("This project runs online insights (oe2)"); - expect(check.message).toContain("insights hold no data to keep."); - expect(check.message).not.toContain("To keep them"); - }); - - test("blocks datasets", () => { - expect(checkSupportedResources(v0Fixture("top-datasets-nonempty.json")).message).toContain( - "datasets (", + expect(check.summary).toContain("runtimes on capacity providers ("); + expect(check.message).toContain("runtimes run on them"); + expect(checkSupportedResources(v0Fixture("runtime-capacityProvider.json")).summary).toContain( + "capacity providers (", ); }); +}); - test("blocks capacity providers and the runtimes that use them", () => { - const check = checkSupportedResources(v0Fixture("runtime-capacityProviderArn-only.json")); +describe("checkCdkPin with a local library build", () => { + test("says it's a local build, not an old one, with a next step", async () => { + const root = await project(); + const path = join(root, "agentcore", "cdk", "package.json"); + const manifest = JSON.parse(readFileSync(path, "utf8")) as { + dependencies: Record; + }; + manifest.dependencies["@aws/agentcore-cdk"] = "file:../../libs/aws-agentcore-cdk-1.0.0.tgz"; + writeFileSync(path, JSON.stringify(manifest)); + const check = await checkCdkPin(root); expect(check.ok).toBe(false); - expect(check.message).toContain("runtimes on capacity providers ("); - expect(check.message).toContain(" First move runtimes off their capacity providers:"); - expect(checkSupportedResources(v0Fixture("runtime-capacityProvider.json")).message).toContain( - "capacity providers (", + expect(check.message).toContain( + "agentcore/cdk uses a local @aws/agentcore-cdk build (file:../../libs/aws-agentcore-cdk-1.0.0.tgz)", ); + expect(check.message).not.toContain("older than"); + expect(check.message).toContain("`agentcore migrate` again"); }); }); diff --git a/src/core/project/migrate/preflight.ts b/src/core/project/migrate/preflight.ts index 8f3f34fae..8a6f46eaa 100644 --- a/src/core/project/migrate/preflight.ts +++ b/src/core/project/migrate/preflight.ts @@ -15,7 +15,13 @@ import { DEPLOYED_STATE_RELATIVE_PATH } from "../backends/cdk/deployedState"; import { checkGitGate } from "./gitGate"; import { checkCdkApp } from "./v0CdkApp"; +/** + * Checks that run before planning, so a project that can't migrate (not v0, not in + * git, an app or dependency migrate doesn't recognize, resources v1 can't keep) + * gets a clear reason up front rather than a failed or lossy migration. + */ /** The CDK app's manifest, relative to the project root. */ +export const CDK_APP_RELATIVE_PATH = "agentcore/cdk"; export const CDK_PACKAGE_RELATIVE_PATH = "agentcore/cdk/package.json"; /** The first `@aws/agentcore-cdk` release the v0.24.2 CLI vended; older apps can't be migrated. */ @@ -126,6 +132,19 @@ export async function checkCdkPin(rootPath: string): Promise { }; } const pin = manifest.dependencies?.["@aws/agentcore-cdk"]; + if (typeof pin === "string" && isLocalSpec(pin)) { + // A local build isn't old, it just isn't a release: comparing versions would mislead. + return { + name, + ok: false, + message: + `${CDK_APP_RELATIVE_PATH} uses a local @aws/agentcore-cdk build (${pin}), not the v0 ` + + "release the project was created with, so migrate can't tell what it deployed. Put " + + `the released dependency back (for example, \`git checkout -- ${CDK_PACKAGE_RELATIVE_PATH} ` + + `${CDK_APP_RELATIVE_PATH}/package-lock.json\`, then \`npm install\` in ${CDK_APP_RELATIVE_PATH}), ` + + "and run `agentcore migrate` again.", + }; + } const match = typeof pin === "string" ? CDK_PIN_PATTERN.exec(pin) : null; if (!match || Number(match[1]) < MIN_CDK_ALPHA) { return { name, ok: false, message: OLD_CDK_MESSAGE }; @@ -133,7 +152,12 @@ export async function checkCdkPin(rootPath: string): Promise { return { name, ok: true, message: `@aws/agentcore-cdk ${String(pin)}` }; } -/** v0 online eval configs that run insights, which v1 doesn't support. */ +/** A dependency spec that points at a build on disk or a tarball rather than a published release. */ +function isLocalSpec(spec: string): boolean { + return /^(file:|link:|\.{1,2}\/|\/|~\/)/.test(spec.trim()) || /\.tgz$/.test(spec.trim()); +} + +/** v0 online eval configs that run insights, which migrate leaves outside the v1 project. */ export function onlineInsightConfigs(v0: V0ProjectSpec): V0ProjectSpec["onlineEvalConfigs"] { return v0.onlineEvalConfigs.filter((config) => config.insights !== undefined); } @@ -145,7 +169,13 @@ export function onlineInsightConfigs(v0: V0ProjectSpec): V0ProjectSpec["onlineEv export const PRESERVE_RESOURCES_DOCS_URL = "https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/preserve-resources.html"; -/** Knowledge bases, datasets, capacity providers, and online insights have no v1 equivalent. */ +/** + * The resources migrating can't carry over, so the user hears about them before + * anything changes rather than finding them broken after. Knowledge bases, + * datasets, and insights-only online evals are retained outside the stack, so + * they don't block. Capacity providers do: runtimes run on them, and v1 can't run + * a runtime on one yet. + */ export function checkSupportedResources(v0: V0ProjectSpec): MigrationCheck { const name = "supported resources"; const runtimesOnCapacityProviders = v0.runtimes @@ -155,69 +185,38 @@ export function checkSupportedResources(v0: V0ProjectSpec): MigrationCheck { (runtime.filesystemConfigurations ?? []).some((mount) => "capacityProviderVolume" in mount), ) .map((runtime) => runtime.name); - // Resources AWS keeps when they leave the stack, so their data can outlive the migration. - const keepable = [ - ["knowledge bases", v0.knowledgeBases.map((kb) => kb.name)], - ["datasets", (v0.datasets ?? []).map((dataset) => dataset.name)], - ["capacity providers", (v0.capacityProviders ?? []).map((provider) => provider.name)], - ] as const; - const insights = onlineInsightConfigs(v0).map((config) => config.name); + const capacityProviders = (v0.capacityProviders ?? []).map((provider) => provider.name); - const listed = (groups: ReadonlyArray) => - groups - .filter(([, names]) => names.length > 0) - .map(([label, names]) => `${label} (${names.join(", ")})`) - .join("; "); - const resources = listed(keepable); - if (resources === "" && insights.length === 0 && runtimesOnCapacityProviders.length === 0) { - return { - name, - ok: true, - message: "no knowledge bases, datasets, capacity providers, or online insights", - }; + const listed = [ + capacityProviders.length > 0 ? `capacity providers (${capacityProviders.join(", ")})` : "", + runtimesOnCapacityProviders.length > 0 + ? `runtimes on capacity providers (${runtimesOnCapacityProviders.join(", ")})` + : "", + ].filter(Boolean); + if (listed.length === 0) { + return { name, ok: true, message: "no capacity providers or runtimes on them" }; } - const lines: string[] = []; - if (resources !== "" || runtimesOnCapacityProviders.length > 0) { - lines.push( - `This project uses ${listed([ - ...keepable, - ["runtimes on capacity providers", runtimesOnCapacityProviders], - ])}, which v1 doesn't support yet. Take them out of the project before migrating:`, - ); - if (runtimesOnCapacityProviders.length > 0) { - lines.push( - ` First move runtimes off their capacity providers: remove capacityProviderConfiguration ` + - `and capacity-provider volumes from them in ${SPEC_RELATIVE_PATH}, then ` + - "run `agentcore deploy`.", - ); - } - if (resources !== "") { - lines.push( - " To keep them and their data in AWS:", - " 1. In agentcore/cdk/lib/cdk-stack.ts, set DeletionPolicy Retain on them (for example " + - "with an Aspect that calls applyRemovalPolicy(RemovalPolicy.RETAIN)).", - " 2. Run `agentcore deploy` (it deploys a v0 project with the v0 CLI).", - ` 3. Remove them from ${SPEC_RELATIVE_PATH} and run \`agentcore deploy\` again. ` + - "They leave the stack but stay in AWS.", - " 4. Undo the change to cdk-stack.ts, commit, and run `agentcore migrate --dry-run`.", - ` If you don't need them: remove them from ${SPEC_RELATIVE_PATH} and run ` + - "`agentcore deploy`. This deletes them from AWS.", - ); - } - } - if (insights.length > 0) { - lines.push( - `This project runs online insights (${insights.join(", ")}), which v1 doesn't support. ` + - `Remove the insights settings from those online eval configs in ${SPEC_RELATIVE_PATH} ` + - "and run `agentcore deploy`; insights hold no data to keep.", + const summary = `Migration blocked: This project uses resources that v1 doesn't support yet: ${listed.join("; ")}.`; + const why: string[] = []; + if (capacityProviders.length > 0 || runtimesOnCapacityProviders.length > 0) { + why.push( + "Capacity providers: runtimes run on them, and v1 can't run a runtime on a capacity " + + "provider yet, so migrating would leave those runtimes without their compute.", ); } - lines.push( - "Or keep the project unmigrated and use it in compatibility mode.", - `See ${PRESERVE_RESOURCES_DOCS_URL}`, - ); - return { name, ok: false, message: lines.join("\n") }; + return { + name, + ok: false, + summary, + message: [ + summary, + ...why, + "Keep using this project in compatibility mode until v1 supports them; nothing in it " + + "needs to change.", + `See ${PRESERVE_RESOURCES_DOCS_URL}`, + ].join("\n"), + }; } /** The v0 deployed state, which says which resources already exist in AWS. */ diff --git a/src/core/project/migrate/refactor.ts b/src/core/project/migrate/refactor.ts index fad73c676..67fcb176c 100644 --- a/src/core/project/migrate/refactor.ts +++ b/src/core/project/migrate/refactor.ts @@ -11,6 +11,12 @@ import { import type { CfnTemplate } from "./templates"; import { templateLocation, type TemplateStore } from "./templateStore"; +/** + * A stack refactor renames logical IDs in place, so v1 takes over the deployed + * resources without recreating them. Its outcome isn't always readable (a lost + * response, a timeout), and guessing wrong would leave the files and the stack + * disagreeing, so anything not proven is reported as unknown, with how to check. + */ /** One deployed stack to refactor onto new logical IDs. */ export type RefactorStack = { target: string; diff --git a/src/core/project/migrate/replacements.ts b/src/core/project/migrate/replacements.ts index d913c97e3..7aa86e6a9 100644 --- a/src/core/project/migrate/replacements.ts +++ b/src/core/project/migrate/replacements.ts @@ -4,6 +4,7 @@ import type { StackResourceChange, } from "../../../handlers/project/migrate/types"; import { NAME_PROPERTIES } from "./mapping"; +import { RETAINABLE_TYPES } from "./retain"; import { CDK_METADATA_TYPE, isCustomResource, @@ -14,6 +15,11 @@ import { type CfnTemplate, } from "./templates"; +/** + * CloudFormation only tells you what a deploy replaces once you run it. Predicting + * it from the templates (with the AWS resource specification) lets migrate block a + * plan that would lose a runtime, memory, or dataset before anything changes. + */ /** What the first v1 deploy after the refactor does to the stack's resources. */ export type DeployImpact = { added: StackResource[]; @@ -370,6 +376,17 @@ function removal( severity, reason, }); + if (RETAINABLE_TYPES.has(type)) { + return retained + ? change( + "info", + "v1 has no counterpart; it's retained, so the v1 deploy leaves it, and its data, in the account outside the stack, where v1 no longer manages it", + ) + : change( + "blocker", + "v1 has no counterpart, and migrate couldn't plan keeping it, so the v1 deploy would delete it and its data", + ); + } if (type.startsWith(AGENTCORE_TYPE_PREFIX) || KEPT_TYPES.has(type)) { return change( "blocker", diff --git a/src/core/project/migrate/retain.test.ts b/src/core/project/migrate/retain.test.ts new file mode 100644 index 000000000..16f63e1d2 --- /dev/null +++ b/src/core/project/migrate/retain.test.ts @@ -0,0 +1,370 @@ +import { describe, expect, test } from "bun:test"; +import type { Change } from "@aws-sdk/client-cloudformation"; +import { fakeCloudFormation, NO_WAIT } from "./migrate-cfn-test-support"; +import { needsManualRepair, StackRefactorError } from "./refactor"; +import { + resourcesToRetain, + retainChangeSetName, + retainChangesMismatch, + retainCheckCommands, + retainResources, + withRetain, +} from "./retain"; +import type { CfnTemplate } from "./templates"; + +const STACK = "AgentCore-Proj-default"; +const DATASET = "ApplicationDatasetGolden8AF7116D"; + +const deployed: CfnTemplate = { + Parameters: { BootstrapVersion: { Type: "AWS::SSM::Parameter::Value" } }, + Resources: { + [DATASET]: { + Type: "AWS::BedrockAgentCore::Dataset", + Properties: { + DatasetName: "Proj_golden", + KmsKeyArn: { "Fn::GetAtt": ["DatasetKey", "Arn"] }, + }, + }, + DatasetKey: { Type: "AWS::KMS::Key", Properties: {} }, + SharedRole: { Type: "AWS::IAM::Role", Properties: {} }, + Memory: { + Type: "AWS::BedrockAgentCore::Memory", + Properties: { RoleArn: { "Fn::GetAtt": ["SharedRole", "Arn"] } }, + }, + CDKMetadata: { Type: "AWS::CDK::Metadata" }, + }, +}; + +describe("resourcesToRetain", () => { + test("keeps a dropped dataset and what it references that v1 drops too", () => { + const retained = resourcesToRetain(deployed, new Set([DATASET, "DatasetKey", "CDKMetadata"])); + expect(retained).toEqual([ + { logicalId: DATASET, type: "AWS::BedrockAgentCore::Dataset", alreadyRetained: false }, + { logicalId: "DatasetKey", type: "AWS::KMS::Key", alreadyRetained: false }, + ]); + }); + + test("leaves references v1 keeps managed", () => { + // The key stays in the v1 template, so it isn't retained, only the dataset. + expect(resourcesToRetain(deployed, new Set([DATASET])).map((r) => r.logicalId)).toEqual([ + DATASET, + ]); + }); + + test("walks references in both directions around a retained knowledge base", () => { + const knowledgeBase: CfnTemplate = { + Resources: { + KnowledgeBase: { + Type: "AWS::Bedrock::KnowledgeBase", + Properties: { RoleArn: { "Fn::GetAtt": ["KnowledgeBaseRole", "Arn"] } }, + }, + DataSource: { + Type: "AWS::Bedrock::DataSource", + Properties: { KnowledgeBaseId: { Ref: "KnowledgeBase" } }, + }, + KnowledgeBaseRole: { Type: "AWS::IAM::Role", Properties: {} }, + KnowledgeBasePolicy: { + Type: "AWS::IAM::Policy", + Properties: { Roles: [{ Ref: "KnowledgeBaseRole" }] }, + }, + Memory: { + Type: "AWS::BedrockAgentCore::Memory", + Properties: { RoleArn: { "Fn::GetAtt": ["KnowledgeBaseRole", "Arn"] } }, + }, + }, + }; + const retained = resourcesToRetain( + knowledgeBase, + new Set(["KnowledgeBase", "DataSource", "KnowledgeBaseRole", "KnowledgeBasePolicy"]), + ); + expect(retained.map((resource) => resource.logicalId)).toEqual([ + "DataSource", + "KnowledgeBase", + "KnowledgeBasePolicy", + "KnowledgeBaseRole", + ]); + }); + + test("retains a dropped insights-only online eval and its dropped support resources", () => { + const onlineEval: CfnTemplate = { + Resources: { + Eval: { + Type: "AWS::BedrockAgentCore::OnlineEvaluationConfig", + Properties: { ExecutionRoleArn: { "Fn::GetAtt": ["EvalRole", "Arn"] } }, + }, + EvalRole: { Type: "AWS::IAM::Role", Properties: {} }, + EvalPolicy: { + Type: "AWS::IAM::Policy", + Properties: { Roles: [{ Ref: "EvalRole" }] }, + }, + }, + }; + expect( + resourcesToRetain(onlineEval, new Set(["Eval", "EvalRole", "EvalPolicy"])).map( + (resource) => resource.logicalId, + ), + ).toEqual(["Eval", "EvalPolicy", "EvalRole"]); + }); + + test("retains nothing when v1 drops only other kinds of resources", () => { + expect(resourcesToRetain(deployed, new Set(["SharedRole", "CDKMetadata"]))).toEqual([]); + }); + + test("notes resources the deployed template already retains", () => { + const retainedAlready = withRetain(deployed, [DATASET]); + expect(resourcesToRetain(retainedAlready, new Set([DATASET]))).toEqual([ + { logicalId: DATASET, type: "AWS::BedrockAgentCore::Dataset", alreadyRetained: true }, + ]); + }); +}); + +describe("withRetain", () => { + test("sets both retain policies on a copy, changing nothing else", () => { + const updated = withRetain(deployed, [DATASET]); + expect(updated.Resources![DATASET]).toEqual({ + ...deployed.Resources![DATASET]!, + DeletionPolicy: "Retain", + UpdateReplacePolicy: "Retain", + }); + expect(deployed.Resources![DATASET]!.DeletionPolicy).toBeUndefined(); + expect(updated.Resources!.Memory).toEqual(deployed.Resources!.Memory!); + }); + + test("refuses a resource the template doesn't have", () => { + expect(() => withRetain(deployed, ["Missing"])).toThrow("'Missing' isn't in the template"); + }); +}); + +const modifyPolicies = (id: string, extra: object[] = []): Change => ({ + Type: "Resource", + ResourceChange: { + Action: "Modify", + LogicalResourceId: id, + Replacement: "False", + Details: [ + { Target: { Attribute: "UpdateReplacePolicy", RequiresRecreation: "Never" } }, + { Target: { Attribute: "DeletionPolicy", RequiresRecreation: "Never" } }, + ...extra, + ], + }, +}); + +describe("retainChangesMismatch", () => { + test("accepts only the retain policies on the expected resources", () => { + expect(retainChangesMismatch([modifyPolicies(DATASET)], new Set([DATASET]))).toBeUndefined(); + }); + + test("names property changes, other resources, replacements, and missing changes", () => { + const problems = retainChangesMismatch( + [ + modifyPolicies(DATASET, [{ Target: { Attribute: "Properties", Name: "DatasetName" } }]), + { + Type: "Resource", + ResourceChange: { Action: "Modify", LogicalResourceId: "Memory" }, + } as Change, + { + Type: "Resource", + ResourceChange: { Action: "Modify", LogicalResourceId: "Key", Replacement: "True" }, + } as Change, + ], + new Set([DATASET, "Key", "Other"]), + ); + expect(problems).toContain(`${DATASET} changes Properties DatasetName`); + expect(problems).toContain("unexpected Modify of Memory"); + expect(problems).toContain("Key would be replaced"); + expect(problems).toContain("no change to Other"); + }); +}); + +function retainStack(client: ReturnType["client"]) { + return { + stackName: STACK, + region: "us-east-1", + template: withRetain(deployed, [DATASET]), + logicalIds: new Set([DATASET]), + client, + templates: async () => ({ + put: async () => { + throw new Error("a small template must not be uploaded"); + }, + remove: async () => {}, + }), + }; +} + +const describedStack = (status: string) => ({ + Stacks: [ + { + StackName: STACK, + StackStatus: status, + Parameters: [{ ParameterKey: "BootstrapVersion", ParameterValue: "/cdk-bootstrap/v" }], + Capabilities: ["CAPABILITY_IAM", "CAPABILITY_NAMED_IAM"], + }, + ], +}); + +const retainedTemplate = { TemplateBody: JSON.stringify(withRetain(deployed, [DATASET])) }; + +describe("retainResources", () => { + test("creates a change set reusing parameters and capabilities, checks it, and executes it", async () => { + const created: string[] = []; + const { client, sent } = fakeCloudFormation({ + DescribeStacks: [ + describedStack("CREATE_COMPLETE"), + describedStack("UPDATE_IN_PROGRESS"), + describedStack("UPDATE_COMPLETE"), + ], + CreateChangeSet: [{ Id: "cs-arn" }], + DescribeChangeSet: [ + { Status: "CREATE_IN_PROGRESS" }, + { Status: "CREATE_COMPLETE", Changes: [modifyPolicies(DATASET)] }, + ], + ExecuteChangeSet: [{}], + GetTemplate: [retainedTemplate], + DeleteChangeSet: [{}], + }); + + await retainResources(retainStack(client), { + ...NO_WAIT, + changeSetName: "agentcore-migrate-retain-1", + onCreated: async (name) => { + created.push(name); + }, + }); + + expect(created).toEqual(["agentcore-migrate-retain-1"]); + const create = sent.find((command) => command.name === "CreateChangeSet")!; + expect(create.input.ChangeSetType).toBe("UPDATE"); + expect(create.input.Parameters).toEqual([ + { ParameterKey: "BootstrapVersion", UsePreviousValue: true }, + ]); + expect(create.input.Capabilities).toEqual(["CAPABILITY_IAM", "CAPABILITY_NAMED_IAM"]); + const body = JSON.parse(create.input.TemplateBody as string) as CfnTemplate; + expect(body.Resources![DATASET]!.DeletionPolicy).toBe("Retain"); + expect(sent.some((command) => command.name === "ExecuteChangeSet")).toBe(true); + expect(sent.some((command) => command.name === "DeleteChangeSet")).toBe(true); + }); + + test("does not report a change set ID when creating it failed before one existed", async () => { + const { client } = fakeCloudFormation({ + DescribeStacks: [describedStack("CREATE_COMPLETE")], + CreateChangeSet: [new Error("network unavailable")], + }); + + const error = await retainResources(retainStack(client), { + ...NO_WAIT, + changeSetName: "never-created", + }).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(StackRefactorError); + expect((error as StackRefactorError).details).toEqual({ + kind: "create-failed", + stackName: STACK, + region: "us-east-1", + }); + }); + + test("deletes a change set that does more than set retain policies, without executing it", async () => { + const { client, sent } = fakeCloudFormation({ + DescribeStacks: [describedStack("CREATE_COMPLETE")], + CreateChangeSet: [{ Id: "cs-arn" }], + DescribeChangeSet: [ + { + Status: "CREATE_COMPLETE", + Changes: [ + modifyPolicies(DATASET, [{ Target: { Attribute: "Properties", Name: "DatasetName" } }]), + ], + }, + ], + DeleteChangeSet: [{}], + }); + + const error = await retainResources(retainStack(client), NO_WAIT).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(StackRefactorError); + expect((error as StackRefactorError).details.kind).toBe("create-failed"); + expect((error as Error).message).toContain("would do more than set their retain policies"); + expect(sent.some((command) => command.name === "ExecuteChangeSet")).toBe(false); + expect(sent.some((command) => command.name === "DeleteChangeSet")).toBe(true); + }); + + test("a change set CloudFormation refuses to execute leaves the stack unchanged", async () => { + const refused = Object.assign(new Error("ValidationError"), { + $metadata: { httpStatusCode: 400 }, + }); + const { client } = fakeCloudFormation({ + DescribeStacks: [describedStack("CREATE_COMPLETE")], + CreateChangeSet: [{ Id: "cs-arn" }], + DescribeChangeSet: [{ Status: "CREATE_COMPLETE", Changes: [modifyPolicies(DATASET)] }], + ExecuteChangeSet: [refused], + DeleteChangeSet: [{}], + }); + + const error = await retainResources(retainStack(client), NO_WAIT).catch((e: unknown) => e); + + expect((error as StackRefactorError).details.kind).toBe("execute-failed"); + expect(needsManualRepair(error)).toBe(false); + }); + + test("an execute call that fails without CloudFormation's answer is an unknown outcome", async () => { + const { client } = fakeCloudFormation({ + DescribeStacks: [describedStack("CREATE_COMPLETE")], + CreateChangeSet: [{ Id: "cs-arn" }], + DescribeChangeSet: [{ Status: "CREATE_COMPLETE", Changes: [modifyPolicies(DATASET)] }], + ExecuteChangeSet: [new Error("socket hang up")], + }); + + const error = await retainResources(retainStack(client), { + ...NO_WAIT, + changeSetName: "cs-1", + }).catch((e: unknown) => e); + + expect(needsManualRepair(error)).toBe(true); + expect((error as StackRefactorError).details.refactorId).toBe("cs-1"); + }); + + test("an update that rolls back leaves the stack as it was", async () => { + const { client } = fakeCloudFormation({ + DescribeStacks: [ + describedStack("CREATE_COMPLETE"), + describedStack("UPDATE_ROLLBACK_COMPLETE"), + ], + CreateChangeSet: [{ Id: "cs-arn" }], + DescribeChangeSet: [{ Status: "CREATE_COMPLETE", Changes: [modifyPolicies(DATASET)] }], + ExecuteChangeSet: [{}], + }); + + const error = await retainResources(retainStack(client), NO_WAIT).catch((e: unknown) => e); + + expect((error as StackRefactorError).details.kind).toBe("execute-failed"); + expect((error as Error).message).toContain("rolled back, so the stack is unchanged"); + }); + + test("an update that finished without the retain policies in place is an unknown outcome", async () => { + const { client } = fakeCloudFormation({ + DescribeStacks: [describedStack("CREATE_COMPLETE"), describedStack("UPDATE_COMPLETE")], + CreateChangeSet: [{ Id: "cs-arn" }], + DescribeChangeSet: [{ Status: "CREATE_COMPLETE", Changes: [modifyPolicies(DATASET)] }], + ExecuteChangeSet: [{}], + GetTemplate: [{ TemplateBody: JSON.stringify(deployed) }], + }); + + const error = await retainResources(retainStack(client), NO_WAIT).catch((e: unknown) => e); + + expect(needsManualRepair(error)).toBe(true); + expect((error as Error).message).toContain(`doesn't retain ${DATASET}`); + }); +}); + +test("retainChangeSetName is unique per second and a valid change set name", () => { + expect(retainChangeSetName(new Date("2026-10-14T16:27:30.123Z"))).toBe( + "agentcore-migrate-retain-20261014162730", + ); +}); + +test("retainCheckCommands names the change set and the stack", () => { + expect(retainCheckCommands(STACK, "us-east-1", "cs-1")).toEqual([ + `aws cloudformation describe-change-set --stack-name ${STACK} --change-set-name cs-1 --region us-east-1`, + `aws cloudformation describe-stacks --stack-name ${STACK} --region us-east-1`, + ]); +}); diff --git a/src/core/project/migrate/retain.ts b/src/core/project/migrate/retain.ts new file mode 100644 index 000000000..41ea99f46 --- /dev/null +++ b/src/core/project/migrate/retain.ts @@ -0,0 +1,377 @@ +import { + CreateChangeSetCommand, + DeleteChangeSetCommand, + DescribeChangeSetCommand, + DescribeStacksCommand, + ExecuteChangeSetCommand, + GetTemplateCommand, + type Change, + type CloudFormationClient, + type DescribeChangeSetCommandOutput, + type Stack, +} from "@aws-sdk/client-cloudformation"; +import { isClientError, poll, StackRefactorError, type PollOptions } from "./refactor"; +import { + CDK_METADATA_TYPE, + referencesIn, + resourcesOf, + type CfnResource, + type CfnTemplate, +} from "./templates"; +import { templateLocation, type TemplateStore } from "./templateStore"; + +/** + * Deployed resource types migrate deliberately leaves out of the v1 project. + * Without retain policies, the first v1 deploy would delete them, so migrate + * sets the policies before the refactor and leaves them in AWS outside the stack. + */ +export const RETAINABLE_TYPES: ReadonlySet = new Set([ + "AWS::Bedrock::KnowledgeBase", + "AWS::BedrockAgentCore::Dataset", + "AWS::BedrockAgentCore::OnlineEvaluationConfig", +]); + +/** How a retained resource of each type is looked after once v1 no longer manages it. */ +export const RETAINED_GUIDANCE: Readonly> = { + "AWS::Bedrock::KnowledgeBase": + "manage it with the AWS Bedrock knowledge base APIs or AWS CLI, using its ID", + "AWS::BedrockAgentCore::Dataset": + "manage it with the `agentcore eval dataset` commands, using its ID", + "AWS::BedrockAgentCore::OnlineEvaluationConfig": + "manage it with the `agentcore eval online-eval` commands, using its ID", +}; + +/** A deployed resource the migration keeps in the account outside the stack. */ +export type RetainedResource = { + logicalId: string; + type: string; + /** Whether the deployed template already retains it, so no update is needed for it. */ + alreadyRetained: boolean; +}; + +const RETAIN = "Retain"; + +function retains(resource: CfnResource): boolean { + return resource.DeletionPolicy === RETAIN && resource.UpdateReplacePolicy === RETAIN; +} + +/** + * The resources a migration has to retain: every deployed resource of a + * retainable type that the v1 template drops, and the whole connected component + * of dropped resources around it. Walking both what a retained resource + * references and what references it preserves supporting resources such as an + * IAM policy attached to a knowledge base's service role. Resources v1 keeps + * under some logical ID are left alone: they stay managed. + */ +export function resourcesToRetain( + deployed: CfnTemplate, + droppedByV1: ReadonlySet, +): RetainedResource[] { + const resources = resourcesOf(deployed); + const dropped = new Set( + [...droppedByV1].filter( + (id) => resources[id] !== undefined && resources[id]?.Type !== CDK_METADATA_TYPE, + ), + ); + const adjacent = new Map>([...dropped].map((id) => [id, new Set()])); + const isResource = (id: string) => id in resources; + for (const id of dropped) { + const resource = resources[id]!; + const references = [ + ...referencesIn(resource.Properties, isResource).map((reference) => reference.target), + ...[resource.DependsOn ?? []].flat(), + ]; + for (const target of references) { + if (!dropped.has(target)) continue; + adjacent.get(id)!.add(target); + adjacent.get(target)!.add(id); + } + } + + const seeds = [...dropped].filter((id) => RETAINABLE_TYPES.has(resources[id]!.Type)); + const keep = new Set(); + const queue = [...seeds]; + while (queue.length > 0) { + const id = queue.shift()!; + if (keep.has(id)) continue; + keep.add(id); + queue.push(...(adjacent.get(id) ?? [])); + } + return [...keep].sort().map((logicalId) => ({ + logicalId, + type: resources[logicalId]!.Type, + alreadyRetained: retains(resources[logicalId]!), + })); +} + +/** The template with DeletionPolicy and UpdateReplacePolicy Retain on the given resources. */ +export function withRetain(template: CfnTemplate, logicalIds: Iterable): CfnTemplate { + const copy = structuredClone(template); + const resources = resourcesOf(copy); + for (const id of logicalIds) { + const resource = resources[id]; + if (!resource) throw new Error(`Resource '${id}' isn't in the template.`); + resource.DeletionPolicy = RETAIN; + resource.UpdateReplacePolicy = RETAIN; + } + return copy; +} + +const POLICY_ATTRIBUTES = new Set(["DeletionPolicy", "UpdateReplacePolicy"]); + +/** + * How a change set differs from setting only the retain policies on `expected`; + * undefined when it doesn't. Anything else (a property change, another resource) + * means the deployed template wasn't what the plan read. + */ +export function retainChangesMismatch( + changes: Change[], + expected: ReadonlySet, +): string | undefined { + const problems: string[] = []; + const seen = new Set(); + for (const change of changes) { + const resource = change.ResourceChange; + const id = resource?.LogicalResourceId ?? ""; + if (change.Type !== "Resource" || resource?.Action !== "Modify" || !expected.has(id)) { + problems.push(`unexpected ${resource?.Action ?? change.Type ?? "change"} of ${id}`); + continue; + } + if (resource.Replacement && resource.Replacement !== "False") { + problems.push(`${id} would be replaced`); + } + for (const detail of resource.Details ?? []) { + const target = detail.Target; + if (target?.Attribute !== undefined && !POLICY_ATTRIBUTES.has(target.Attribute)) { + problems.push(`${id} changes ${target.Attribute}${target.Name ? ` ${target.Name}` : ""}`); + } + } + seen.add(id); + } + for (const id of expected) if (!seen.has(id)) problems.push(`no change to ${id}`); + return problems.length > 0 ? problems.join(", ") : undefined; +} + +/** One deployed stack whose unsupported resources to retain before its refactor. */ +export type RetainStack = { + stackName: string; + region: string; + /** The deployed template with the retain policies set: what the update moves the stack to. */ + template: CfnTemplate; + /** The resources the update sets the policies on. */ + logicalIds: ReadonlySet; + client: CloudFormationClient; + templates: () => Promise; +}; + +/** A change set name unique to this run, so a rerun never trips over an old one. */ +export function retainChangeSetName(now: Date = new Date()): string { + return `agentcore-migrate-retain-${now + .toISOString() + .replace(/[^0-9]/g, "") + .slice(0, 14)}`; +} + +/** + * Sets DeletionPolicy and UpdateReplacePolicy Retain on the stack's unsupported + * resources with a change set that changes nothing else: the change set's + * changes are checked to be exactly those attributes on exactly those resources + * before it's executed, and deleted otherwise. A change set CloudFormation + * refused, or that rolled back, leaves the stack as it was; one whose outcome + * couldn't be read is `unknown`, and must be checked before anything else. + */ +export async function retainResources( + stack: RetainStack, + options: PollOptions & { + changeSetName?: string; + onCreated?: (name: string) => Promise; + } = {}, +): Promise { + const { client, stackName, region } = stack; + const changeSetName = options.changeSetName ?? retainChangeSetName(); + const details = { stackName, region }; + const createdDetails = { ...details, refactorId: changeSetName }; + const deployed = await describeStack(client, stackName); + const template = await templateLocation(stack.template, `${stackName}-retain`, stack.templates); + try { + options.onStep?.(`Retaining the unsupported resources of stack ${stackName}`); + try { + await client.send( + new CreateChangeSetCommand({ + StackName: stackName, + ChangeSetName: changeSetName, + ChangeSetType: "UPDATE", + Description: "agentcore migrate: retain resources v1 doesn't manage", + ...template.location, + Parameters: (deployed.Parameters ?? []).map((parameter) => ({ + ParameterKey: parameter.ParameterKey, + UsePreviousValue: true, + })), + Capabilities: deployed.Capabilities ?? [], + }), + ); + } catch (error) { + throw new StackRefactorError( + `Couldn't create the change set that retains resources in stack '${stackName}': ` + + `${(error as Error).message}. Nothing was changed.`, + { kind: "create-failed", ...details }, + ); + } + await options.onCreated?.(changeSetName); + const created = await poll( + () => describeChangeSet(client, stackName, changeSetName), + (state) => !state.Status?.endsWith("_PENDING") && !state.Status?.endsWith("_IN_PROGRESS"), + options, + ); + if (created.Status !== "CREATE_COMPLETE") { + await deleteRetainChangeSet(client, stackName, changeSetName); + throw new StackRefactorError( + `CloudFormation couldn't prepare the change set that retains resources in stack ` + + `'${stackName}' (${created.Status ?? "UNKNOWN"}): ${created.StatusReason ?? "no reason given"}. ` + + "Nothing was changed.", + { kind: "create-failed", ...createdDetails }, + ); + } + const mismatch = retainChangesMismatch(created.Changes ?? [], stack.logicalIds); + if (mismatch) { + await deleteRetainChangeSet(client, stackName, changeSetName); + throw new StackRefactorError( + `The change set that retains resources in stack '${stackName}' would do more than set ` + + `their retain policies (${mismatch}), so it was deleted unexecuted. Nothing was changed.`, + { kind: "create-failed", ...createdDetails }, + ); + } + try { + await client.send( + new ExecuteChangeSetCommand({ StackName: stackName, ChangeSetName: changeSetName }), + ); + } catch (error) { + // Only CloudFormation's own refusal proves the change set didn't start. + if (isClientError(error)) { + await deleteRetainChangeSet(client, stackName, changeSetName); + throw new StackRefactorError( + `CloudFormation refused to execute the change set that retains resources in stack ` + + `'${stackName}': ${(error as Error).message}. Nothing was changed.`, + { kind: "execute-failed", ...createdDetails }, + ); + } + throw unknownOutcome(stackName, region, changeSetName, (error as Error).message); + } + let finished: Stack; + try { + finished = await poll( + () => describeStack(client, stackName), + (state) => !state.StackStatus?.endsWith("_IN_PROGRESS"), + options, + ); + } catch (error) { + throw unknownOutcome(stackName, region, changeSetName, (error as Error).message); + } + if (finished.StackStatus !== "UPDATE_COMPLETE") { + const kind = + finished.StackStatus === "UPDATE_ROLLBACK_COMPLETE" ? "execute-failed" : "unknown"; + throw new StackRefactorError( + `The update that retains resources in stack '${stackName}' ended ${finished.StackStatus ?? "UNKNOWN"}` + + (kind === "execute-failed" ? " and rolled back, so the stack is unchanged." : "."), + { kind, ...createdDetails, ...(finished.StackStatus && { status: finished.StackStatus }) }, + ); + } + const missing = await notRetained(client, stackName, stack.logicalIds); + if (missing.length > 0) { + throw unknownOutcome( + stackName, + region, + changeSetName, + `the update finished, but the deployed template doesn't retain ${missing.join(", ")}`, + ); + } + await deleteRetainChangeSet(client, stackName, changeSetName); + } finally { + await template.release(); + } +} + +function unknownOutcome( + stackName: string, + region: string, + changeSetName: string, + detail: string, +): StackRefactorError { + return new StackRefactorError( + `Lost track of the update that retains resources in stack '${stackName}' (${detail}), so ` + + "whether it finished isn't known.", + { kind: "unknown", stackName, region, refactorId: changeSetName }, + ); +} + +async function describeStack(client: CloudFormationClient, stackName: string): Promise { + const response = await client.send(new DescribeStacksCommand({ StackName: stackName })); + const stack = response.Stacks?.[0]; + if (!stack) throw new Error(`CloudFormation returned no stack named '${stackName}'.`); + return stack; +} + +async function describeChangeSet( + client: CloudFormationClient, + stackName: string, + changeSetName: string, +): Promise { + const first = await client.send( + new DescribeChangeSetCommand({ StackName: stackName, ChangeSetName: changeSetName }), + ); + const changes = [...(first.Changes ?? [])]; + let token = first.NextToken; + while (token) { + const page = await client.send( + new DescribeChangeSetCommand({ + StackName: stackName, + ChangeSetName: changeSetName, + NextToken: token, + }), + ); + changes.push(...(page.Changes ?? [])); + token = page.NextToken; + } + return { ...first, Changes: changes }; +} + +export async function deleteRetainChangeSet( + client: CloudFormationClient, + stackName: string, + changeSetName: string, +): Promise { + try { + await client.send( + new DeleteChangeSetCommand({ StackName: stackName, ChangeSetName: changeSetName }), + ); + } catch { + // Cleanup is best effort: leaving any change set behind is only clutter. + } +} + +/** The given resources the deployed template doesn't retain. */ +async function notRetained( + client: CloudFormationClient, + stackName: string, + logicalIds: ReadonlySet, +): Promise { + const response = await client.send( + new GetTemplateCommand({ StackName: stackName, TemplateStage: "Original" }), + ); + const body = response.TemplateBody; + const template = (typeof body === "string" ? JSON.parse(body) : body) as CfnTemplate; + const resources = resourcesOf(template); + return [...logicalIds].filter((id) => !resources[id] || !retains(resources[id]!)); +} + +/** The commands that show whether a retain update finished. */ +export function retainCheckCommands( + stackName: string, + region: string, + changeSetName: string, +): string[] { + return [ + `aws cloudformation describe-change-set --stack-name ${stackName} --change-set-name ${changeSetName} --region ${region}`, + `aws cloudformation describe-stacks --stack-name ${stackName} --region ${region}`, + ]; +} diff --git a/src/core/project/migrate/stacks.ts b/src/core/project/migrate/stacks.ts index f006f4a79..a355668e8 100644 --- a/src/core/project/migrate/stacks.ts +++ b/src/core/project/migrate/stacks.ts @@ -9,6 +9,10 @@ import type { MigrationCheck } from "../../../handlers/project/migrate/types"; import { isStackNotFound } from "../backends/cdk/environment"; import type { CfnTemplate } from "./templates"; +/** + * Reading the deployed stack (status, template, stack policy) is all planning + * needs from AWS, and it's read-only, so a preview never changes the account. + */ /** The CloudFormation reads migration planning makes; all read-only. */ export type StackReader = { /** The stack, or undefined when it doesn't exist. */ diff --git a/src/core/project/migrate/synth.ts b/src/core/project/migrate/synth.ts index cc68d3b36..2d72b48e9 100644 --- a/src/core/project/migrate/synth.ts +++ b/src/core/project/migrate/synth.ts @@ -11,7 +11,7 @@ import { synthCommand } from "../backends/cdk/assembly"; * tarball or package directory (made absolute, so `npm pack` output in the * current directory works). Lets the planner run against an unpublished library * build; never needed with a released CLI, whose vended app already pins a - * library that understands the pinned names. + * library that supports migratedFrom. */ export { CDK_LIBRARY_OVERRIDE_ENV }; diff --git a/src/core/project/migrate/v0CdkApp.test.ts b/src/core/project/migrate/v0CdkApp.test.ts index 7ba30fed1..33c9e1993 100644 --- a/src/core/project/migrate/v0CdkApp.test.ts +++ b/src/core/project/migrate/v0CdkApp.test.ts @@ -130,6 +130,20 @@ describe("checkCdkApp", () => { " modified agentcore/cdk/README.md", " modified agentcore/cdk/cdk.json", ]); + // The summary names the files instead of cutting the long message off. + expect(check.summary).toBe( + "agentcore/cdk differs from the app v0 generated, and migrating replaces it, so these " + + "changes would be lost: added lib/uploads.ts, added notes.txt, modified README.md, " + + "modified cdk.json.", + ); + }); + + test("the summary names up to five changed files, then counts the rest", async () => { + const root = await project(); + for (const n of [1, 2, 3, 4, 5, 6, 7]) + await write(root, `agentcore/cdk/lib/extra${n}.ts`, "x\n"); + const check = await checkCdkApp(root); + expect(check.summary).toEndWith(", and 2 more."); }); test("blocks a package.json with another dependency or changed scripts", async () => { diff --git a/src/core/project/migrate/v0CdkApp.ts b/src/core/project/migrate/v0CdkApp.ts index 72bfd3110..c5ddeab68 100644 --- a/src/core/project/migrate/v0CdkApp.ts +++ b/src/core/project/migrate/v0CdkApp.ts @@ -6,6 +6,11 @@ import type { CfnTemplate } from "./templates"; import { resourcesOf } from "./templates"; import { V0_CDK_APPS, V0_CDK_DEPENDENCY_NAMES, type V0CdkApp } from "./v0CdkApps"; +/** + * Migrating replaces agentcore/cdk with v1's app. That's only safe when the app is + * exactly what a v0 release generated: anything the user added there would be + * lost, and resources it creates deleted by the first v1 deploy. + */ /** The v0 CDK app's directory, relative to the project root; migrating replaces it. */ export const CDK_APP_DIRECTORY = "agentcore/cdk"; @@ -266,9 +271,18 @@ export async function checkCdkApp(rootPath: string): Promise { ...closest.added.map((file) => ` added ${CDK_APP_DIRECTORY}/${file}`), ...closest.modified.map((file) => ` modified ${CDK_APP_DIRECTORY}/${file}`), ]; + const changed = [ + ...closest.added.map((file) => `added ${file}`), + ...closest.modified.map((file) => `modified ${file}`), + ]; + const shown = changed.slice(0, SUMMARY_FILE_LIMIT); + const more = changed.length - shown.length; return { name, ok: false, + summary: + `${CDK_APP_DIRECTORY} differs from the app v0 generated, and migrating replaces it, so ` + + `these changes would be lost: ${shown.join(", ")}${more > 0 ? `, and ${more} more` : ""}.`, message: [ `${CDK_APP_DIRECTORY} isn't the CDK app a release that creates v0 projects (v0.4.0 to ` + `v0.31.1, 1.0.0-preview.1 to 1.0.0-preview.33) created; compared with v0 ` + @@ -285,6 +299,9 @@ export async function checkCdkApp(rootPath: string): Promise { }; } +/** How many changed files the summary names before counting the rest. */ +const SUMMARY_FILE_LIMIT = 5; + /** Whether the project's v0 app builds harness containers itself (`hasDockerfile`, v0.16.0 and later). */ export async function appBuildsHarnessContainers(rootPath: string): Promise { try { diff --git a/src/core/project/migrate/workspace.ts b/src/core/project/migrate/workspace.ts index 1d23f9815..b71736760 100644 --- a/src/core/project/migrate/workspace.ts +++ b/src/core/project/migrate/workspace.ts @@ -18,6 +18,10 @@ import type { AssetSource } from "../source"; import { PROJECT_SPEC_RELATIVE_PATH } from "../fsUtils"; import { FsTreeNode } from "../templates/fsTree"; +/** + * Planning builds the v1 project in a temporary copy, so the preview can install, + * convert, and synthesize for real without changing the user's project. + */ /** * Directories never copied into or compared across a workspace: installed * dependencies (npm and Python virtual environments), caches, synth output, and git. diff --git a/src/handlers/project/migrate/index.test.ts b/src/handlers/project/migrate/index.test.ts index 27736936a..96beec916 100644 --- a/src/handlers/project/migrate/index.test.ts +++ b/src/handlers/project/migrate/index.test.ts @@ -30,6 +30,8 @@ import { import { ALREADY_V1_MESSAGE, APPLY_HINT, + applyHint, + planBlockers, CONFIRM_QUESTION, formatMigrationPlan, formatMigrationResult, @@ -394,6 +396,150 @@ function scriptedCli( }; } +describe("agentcore migrate, a not-ready summary", () => { + test("a missing CDK library shows only that line, with `agentcore update`", () => { + const blockers = planBlockers({ + ...readyPlan("/p"), + ready: false, + checks: [ + { + name: "cdk library", + ok: false, + message: "long message", + summary: + "This CLI can't keep this project's resource names yet. Run `agentcore update`, then run `agentcore migrate` again.", + }, + { name: "v1 synth", ok: false, message: "doesn't support migratedFrom" }, + ], + skippedStacks: [ + { reason: "the converted project's v1 CDK app didn't synthesize (see the v1 synth check)" }, + ], + }); + expect(blockers).toEqual([ + "cdk library: This CLI can't keep this project's resource names yet. Run `agentcore update`, then run `agentcore migrate` again.", + ]); + }); + + test("unsupported resources are one line pointing at the full plan, without their per-resource deletions", () => { + const stack: StackMigrationPlan = { + target: "default", + account: "1", + region: "us-east-1", + stackName: STACK, + moves: [], + unchanged: 0, + added: [], + removed: [ + { + logicalId: "CapacityProviderCompute", + type: "AWS::BedrockAgentCore::CapacityProvider", + severity: "blocker", + reason: "the v1 template has no counterpart, so the v1 deploy would delete it", + }, + ], + replaced: [], + updated: [], + blockers: [], + ready: false, + }; + const blockers = planBlockers({ + ...readyPlan("/p"), + ready: false, + checks: [ + { + name: "supported resources", + ok: false, + message: "full", + summary: + "Migration blocked: This project uses resources that v1 doesn't support yet: capacity providers (compute).", + }, + ], + stacks: [stack], + }); + expect(blockers).toEqual([ + "Migration blocked: This project uses resources that v1 doesn't support yet: capacity providers (compute).\n See the full plan for how to fix it.", + ]); + }); + + test("a changed CDK app names the changed files instead of cutting the line off", async () => { + await inCommittedV0Project(); + const subject = scriptedCli({ + ready: false, + plan: { + checks: [ + { + name: "cdk app", + ok: false, + message: "long", + summary: + "agentcore/cdk differs from the app v0 generated, and migrating replaces it, so these changes would be lost: added lib/a.ts, added lib/b.ts, added lib/c.ts, added lib/d.ts, modified bin/cdk.ts, and 2 more.", + }, + ], + }, + }); + await subject.run().catch(() => {}); + expect(subject.io.stdout()).toContain("modified bin/cdk.ts, and 2 more."); + }); +}); + +describe("agentcore migrate, keeping unsupported resources", () => { + const retainingStack: StackMigrationPlan = { + target: "default", + account: "998846730471", + region: "us-east-1", + stackName: STACK, + moves: [], + unchanged: 3, + added: [], + removed: [], + replaced: [], + updated: [], + blockers: [], + ready: true, + retained: [ + { + logicalId: "ApplicationDatasetGolden8AF7116D", + type: "AWS::BedrockAgentCore::Dataset", + physicalId: + "arn:aws:bedrock-agentcore:us-east-1:998846730471:dataset/CompatA_golden-ABCDEFGHIJ", + guidance: "manage it with the `agentcore eval dataset` commands, using its ID", + }, + ], + }; + + test("the apply hint repeats the flags applying needs", () => { + expect(applyHint()).toBe(APPLY_HINT); + expect(applyHint(["--target prod"])).toBe( + "Nothing was changed. Run `agentcore migrate --yes --target prod` to apply these changes.", + ); + }); + + test("the preview lists what stays in AWS outside the stack, by type and physical ID", async () => { + await inCommittedV0Project(); + const subject = scriptedCli({ plan: { stacks: [retainingStack] } }); + + await subject.run(); + + const out = subject.io.stdout(); + expect(out).toContain( + " ! Keeps 1 resource in AWS outside the stack after migrating, where v1 won't manage it: " + + "AWS::BedrockAgentCore::Dataset arn:aws:bedrock-agentcore:us-east-1:998846730471:dataset/CompatA_golden-ABCDEFGHIJ", + ); + const report = subject.calls.reports[0]!.contents; + expect(report).toContain("Kept in AWS outside the stack"); + expect(report).toContain( + "AWS::BedrockAgentCore::Dataset: manage it with the `agentcore eval dataset` commands, using its ID.", + ); + expect(report).not.toMatch(/delete it/i); + }); + + test("there's no --retain-unsupported flag: the prompt or --yes is the consent", async () => { + await inCommittedV0Project(); + const subject = scriptedCli(); + await expect(subject.run("--retain-unsupported")).rejects.toThrow(/unknown option/); + }); +}); + describe("agentcore migrate, applying", () => { test("passes --target to the plan", async () => { await inCommittedV0Project(); diff --git a/src/handlers/project/migrate/index.ts b/src/handlers/project/migrate/index.ts index 5ed11ad14..b641d9c92 100644 --- a/src/handlers/project/migrate/index.ts +++ b/src/handlers/project/migrate/index.ts @@ -24,6 +24,7 @@ import { type StackMigrationPlan, type StackRefactorResult, type StackResourceChange, + type RetainedStackResource, } from "./types"; export type { MigrationPlan, ProjectMigrator } from "./types"; @@ -53,6 +54,12 @@ export const CONFIRM_QUESTION = "Apply these changes? (y/N) "; export const APPLY_HINT = "Nothing was changed. Run `agentcore migrate --yes` to apply these changes."; +/** The apply hint for a run that needs its other flags repeated to apply. */ +export function applyHint(flags: string[] = []): string { + if (flags.length === 0) return APPLY_HINT; + return `Nothing was changed. Run \`agentcore migrate --yes ${flags.join(" ")}\` to apply these changes.`; +} + /** * `agentcore migrate`: upgrades the enclosing v0 project to v1, one flow every * time. It checks the project and prints the plan (what changes in the project @@ -141,7 +148,11 @@ export const createMigrateProjectHandler = (config: MigrateProjectHandlerConfig) } const canPrompt = !flags.yes && !json && io.stdin.isTTY && io.stdout.isTTY && io.stderr.isTTY; - if (!flags.yes && !canPrompt) return showPlan(APPLY_HINT); + if (!flags.yes && !canPrompt) { + return showPlan( + applyHint([...(flags.target !== undefined ? [`--target ${flags.target}`] : [])]), + ); + } // With --json the plan goes out once, with the result. if (!json) showPlan(); // The question is settled here, before apply's progress UI mounts: nothing @@ -289,33 +300,60 @@ function overrideWarningLines(cdkLibraryOverride: string, indent: string): strin /** One line per thing that keeps the plan from being ready. */ export function planBlockers(plan: MigrationPlan): string[] { + const failed = (name: string) => plan.checks.some((check) => check.name === name && !check.ok); + // Some failures explain others: the summary shows the cause and leaves its + // consequences to the plan file, so the user sees what to act on. + const libraryMissing = failed(LIBRARY_CHECK); + const unsupported = failed(SUPPORTED_RESOURCES_CHECK); const lines: string[] = []; for (const check of plan.checks) { - if (!check.ok) lines.push(`${check.name}: ${brief(check.message)}`); + if (check.ok) continue; + if (libraryMissing && check.name === SYNTH_CHECK) continue; + if (check.name === SUPPORTED_RESOURCES_CHECK) { + lines.push( + `${check.summary ?? brief(check.message)}\n See the full plan for how to fix it.`, + ); + continue; + } + lines.push(`${check.name}: ${check.summary ?? brief(check.message)}`); } for (const problem of [...plan.problems, ...plan.deployedChanges]) { lines.push(`${problem.file} ${problem.path}: ${brief(problem.message)}`); } for (const skipped of plan.skippedStacks) { + if (libraryMissing && skipped.reason.includes(SYNTH_CHECK)) continue; const which = skipped.target === undefined ? "" : ` (${skipped.target})`; lines.push(`stack plan skipped${which}: ${brief(skipped.reason)}`); } for (const stack of plan.stacks) { for (const blocker of stack.blockers) lines.push(`${stack.stackName}: ${brief(blocker)}`); for (const change of [...stack.removed, ...stack.replaced, ...stack.updated]) { - if (change.severity === "blocker") { - lines.push(`${shortType(change.type)} ${change.logicalId}: ${brief(change.reason)}`); - } + if (change.severity !== "blocker") continue; + // The unsupported-resources line already covers what v1 can't keep. + if (unsupported && UNSUPPORTED_TYPES.some((type) => change.type.startsWith(type))) continue; + lines.push(`${shortType(change.type)} ${change.logicalId}: ${brief(change.reason)}`); } } return lines; } +const LIBRARY_CHECK = "cdk library"; +const SYNTH_CHECK = "v1 synth"; +const SUPPORTED_RESOURCES_CHECK = "supported resources"; +/** Resource types the unsupported-resources check stands for in the summary. */ +const UNSUPPORTED_TYPES = [ + "AWS::BedrockAgentCore::CapacityProvider", + "AWS::BedrockAgentCore::OnlineEvaluationConfig", +]; + /** What the first v1 deploy does to the stack, in one line. */ function deployImpact(stack: StackMigrationPlan): string { + // Retained resources leave the stack but stay in the account: not deletions. + const kept = new Set((stack.retained ?? []).map((resource) => resource.logicalId)); + const deleted = stack.removed.filter((change) => !kept.has(change.logicalId)); const lost = [ stack.replaced.length > 0 ? `replaces ${count(stack.replaced.length, "resource")}` : "", - stack.removed.length > 0 ? `deletes ${count(stack.removed.length, "resource")}` : "", + deleted.length > 0 ? `deletes ${count(deleted.length, "resource")}` : "", ].filter(Boolean); const parts = [ lost.length === 0 ? "replaces nothing" : `${lost.join(", ")} (none that must be kept)`, @@ -384,6 +422,14 @@ export function formatMigrationSummary(plan: MigrationPlan, planPath: string): s ` Moves ${count(stack.moves.length, "resource")} in ${stack.stackName} to new logical IDs (stack refactor)`, ` First v1 deploy: ${deployImpact(stack)}`, ); + const retained = stack.retained ?? []; + if (retained.length > 0) { + lines.push( + ` ! Keeps ${count(retained.length, "resource")} in AWS outside the stack after ` + + `migrating, where v1 won't manage ${retained.length === 1 ? "it" : "them"}: ` + + `${listed(retained.map(retainedLabel))}`, + ); + } } if (!plan.finishPending && plan.changes.length > 0) { lines.push( @@ -653,6 +699,11 @@ export function formatMigrationPlan(plan: MigrationPlan): string { } /** One stack's refactor: what moves, then what the first v1 deploy changes, then the verdict. */ +/** A retained resource for the summary: its kind and the ID to find it by. */ +function retainedLabel(resource: RetainedStackResource): string { + return `${resource.type} ${resource.physicalId ?? resource.logicalId}`; +} + export function formatStackPlan(stack: StackMigrationPlan): string[] { const lines = [ `Stack ${stack.stackName} (target '${stack.target}', ${stack.account}/${stack.region})`, @@ -671,6 +722,30 @@ export function formatStackPlan(stack: StackMigrationPlan): string[] { ); } lines.push(` ${count(stack.unchanged, "resource")} keep their logical IDs`); + const retained = stack.retained ?? []; + if (retained.length > 0) { + lines.push( + " Kept in AWS outside the stack: set to retain before the refactor, so the first v1 " + + "deploy leaves them, and their data, in place. v1 doesn't manage them afterwards.", + ...indent( + table( + ["TYPE", "LOGICAL ID", "PHYSICAL ID"], + retained.map((resource) => [ + resource.type, + resource.logicalId, + resource.physicalId ?? "-", + ]), + ), + ), + ...[ + ...new Map( + retained.flatMap((resource) => + resource.guidance ? [[resource.type, resource.guidance] as const] : [], + ), + ), + ].map(([type, guidance]) => ` ${type}: ${guidance}.`), + ); + } lines.push(" Then the first v1 deploy:"); const changes = (label: string, entries: StackResourceChange[]) => { diff --git a/src/handlers/project/migrate/types.ts b/src/handlers/project/migrate/types.ts index 4e552e2f4..2f2b187a9 100644 --- a/src/handlers/project/migrate/types.ts +++ b/src/handlers/project/migrate/types.ts @@ -18,6 +18,11 @@ export type MigrationCheck = { ok: boolean; /** What was found, or what to do about it when the check failed. */ message: string; + /** + * The one line the summary shows for a failed check, when the full message is + * too long for it; the full message goes in the plan file. + */ + summary?: string; }; /** @@ -125,6 +130,13 @@ export type StackMigrationPlan = { * rejects on an existing resource (a gateway's ProtocolType) is a blocker. */ updated: StackResourceChange[]; + /** + * Deployed resources v1 has no counterpart for that the migration keeps in the + * account outside the stack: the stack is updated to retain them before its + * refactor, and the first v1 deploy then leaves them in place. v1 doesn't + * manage them afterwards. + */ + retained?: RetainedStackResource[]; /** * Problems with the stack as a whole (its name, the refactor template) that stop * it migrating. Resource-level blockers are the removed and replaced entries @@ -135,6 +147,16 @@ export type StackMigrationPlan = { ready: boolean; }; +/** A deployed resource a migration keeps in the account outside the stack. */ +export type RetainedStackResource = { + logicalId: string; + type: string; + /** Its physical ID, when the stack's resources were read. */ + physicalId?: string; + /** How to manage and delete it once v1 no longer does. */ + guidance?: string; +}; + /** Everything that stops one stack migrating: stack-level problems and blocking resource changes. */ export function stackBlockerCount(plan: StackMigrationPlan): number { return ( @@ -227,6 +249,8 @@ export type StackRefactorResult = { region: string; outcome: StackRefactorOutcome; refactorId?: string; + /** The change set that set the retain policies on the stack's unsupported resources, when one ran. */ + retainChangeSet?: string; /** Why it failed, for the failed outcomes. */ message?: string; };