From 5507a1987c02cc9755bc8bf6f7040472d8f5ef76 Mon Sep 17 00:00:00 2001 From: Akash Agrawal Date: Wed, 23 Sep 2026 17:32:46 +0530 Subject: [PATCH] fix(toolkit-lib): diff leaves a deleting shell stack behind, breaking the next deploy --- .../lib/api/deployments/cfn-api.ts | 27 ++++++++++++- .../toolkit-lib/test/actions/diff.test.ts | 40 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/cfn-api.ts b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/cfn-api.ts index 1215ead76..4722b67c4 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/cfn-api.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/cfn-api.ts @@ -61,6 +61,14 @@ export interface CreateChangeSetOptions { cfn: ICloudFormationClient; changeSetName: string; exists: boolean; + /** + * Whether a stack was present at all before this change set was created, regardless of its status. + * + * Distinct from `exists`, which reports a stack in REVIEW_IN_PROGRESS or DELETE_IN_PROGRESS as + * absent. Used to decide whether the empty stack left behind is one this change set created, and + * is therefore ours to wait for on the way out. + */ + stackExistedBefore: boolean; uuid: string; stack: cxapi.CloudFormationStackArtifact; bodyParameter: TemplateBodyParameter; @@ -80,7 +88,7 @@ export async function createDiffChangeSet( options: Omit, ): Promise { try { - const { cfn, bodyParameter, exists, executionRoleArn, diagnoser } = await prepareChangeSetEnv(ioHelper, options); + const { cfn, bodyParameter, exists, stackExistedBefore, executionRoleArn, diagnoser } = await prepareChangeSetEnv(ioHelper, options); await ioHelper.defaults.info( 'Hold on while we create a read-only change set to get a diff with accurate replacement information (use --method=template to use a less accurate but faster template-only diff)\n', @@ -91,6 +99,7 @@ export async function createDiffChangeSet( changeSetName: options.changeSetName ?? `cdk-diff-change-set-${options.uuid}`, stack: options.stack, exists, + stackExistedBefore, uuid: options.uuid, bodyParameter, parameters: options.parameters, @@ -273,6 +282,18 @@ async function createChangeSetAndCleanup( StackName: stackId, ClientRequestToken: randomUUID(), }); + // Wait for the delete to actually finish, but only for a shell this change set created. + // `DeleteStack` merely submits the request, and `DescribeStacks` keeps answering for the shell + // while it is DELETE_IN_PROGRESS. A `cdk deploy` started right after a `cdk diff` therefore + // finds a stack that "exists", calls `GetTemplate` on it, and fails with + // `Stack [] does not exist` — a REVIEW_IN_PROGRESS/deleting shell has no template. + // `Deployments.cleanupChangeSet` already waits after the equivalent delete. + // + // Gated on `stackExistedBefore` rather than `exists` so that a stack someone else already had + // in flight is never something a read-only diff blocks on. + if (!options.stackExistedBefore) { + await waitForStackDelete(options.cfn, ioHelper, stackId); + } } } } @@ -326,6 +347,10 @@ export async function createValidationChangeSet( StackName: changeSet.StackId ?? options.stack.stackName, ClientRequestToken: randomUUID(), }).catch((e) => ioHelper.defaults.warn(`Failed to clean up REVIEW_IN_PROGRESS stack: ${e}`)); + // Same reason as in `createChangeSetAndCleanup`: without this the shell stack can still be + // DELETE_IN_PROGRESS when the next operation reads it. Best-effort, to match the delete above. + await waitForStackDelete(cfn, ioHelper, changeSet.StackId ?? options.stack.stackName) + .catch((e) => ioHelper.defaults.warn(`Failed to wait for REVIEW_IN_PROGRESS stack cleanup: ${e}`)); } } } diff --git a/packages/@aws-cdk/toolkit-lib/test/actions/diff.test.ts b/packages/@aws-cdk/toolkit-lib/test/actions/diff.test.ts index 88ffa2917..b8bde773c 100644 --- a/packages/@aws-cdk/toolkit-lib/test/actions/diff.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/actions/diff.test.ts @@ -595,6 +595,46 @@ describe('diff', () => { }); }); + test('ChangeSet diff waits for the REVIEW_IN_PROGRESS stack to finish deleting', async () => { + // GIVEN - stack doesn't exist, so the CREATE change set leaves a shell stack behind + jest.spyOn(deployments.Deployments.prototype, 'stackExists').mockResolvedValue(false); + mockSSMClient.on(GetParameterCommand).resolves({ Parameter: { Value: '99' } }); + mockCloudFormationClient.on(CreateChangeSetCommand).resolves({ + Id: 'arn:aws:cloudformation:us-east-1:123456789012:changeSet/cdk-diff', + StackId: 'arn:aws:cloudformation:us-east-1:123456789012:stack/Stack1/fake-id', + }); + mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({ + Status: 'CREATE_COMPLETE', + Changes: [], + }); + + let deleteIssued = false; + let statusReadsAfterDelete = 0; + mockCloudFormationClient.on(DeleteStackCommand).callsFake(() => { + deleteIssued = true; + return {}; + }); + mockCloudFormationClient.on(DescribeStacksCommand).callsFake(() => { + if (deleteIssued) { + statusReadsAfterDelete += 1; + } + return { Stacks: [] }; + }); + + // WHEN + const cx = await cdkOutFixture(toolkit, 'stack-with-bucket'); + await toolkit.diff(cx, { + stacks: { strategy: StackSelectionStrategy.ALL_STACKS }, + method: DiffMethod.ChangeSet({ fallbackToTemplate: false }), + }); + + // THEN - the diff does not return until the shell stack's deletion has been confirmed. + // Without the wait DeleteStack is fire-and-forget, so a `cdk deploy` started straight after a + // `cdk diff` reads the still-deleting shell and fails on GetTemplate. + expect(deleteIssued).toBe(true); + expect(statusReadsAfterDelete).toBeGreaterThan(0); + }); + test('ChangeSet diff describes the change set with IncludePropertyValues so deploy-time-only changes are surfaced', async () => { // GIVEN - an existing stack jest.spyOn(deployments.Deployments.prototype, 'stackExists').mockResolvedValue(true);