fix(cli): unable to update stacks in UPDATE_ROLLBACK_COMPLETE (#8948)

Supersedes #8779

The CLI determined that a stack in `UPDATE_ROLLBACK_COMPLETE`
status is not updateable. related [comment](#8779 (comment))

This change modifies this logic by splitting up `waitForStack` into `waitForStackDelete`
and `waitForStackDeploy` which evaluate stack status after it reaches a stable state
depending on the operation that was performed.



Closes #8126 #5151

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

packages/aws-cdk/lib/api/deploy-stack.ts

@@ -10,7 +10,7 @@ import { publishAssets } from '../util/asset-publishing';
 import { contentHash } from '../util/content-hash';
 import { ISDK, SdkProvider } from './aws-auth';
 import { ToolkitInfo } from './toolkit-info';
-import { changeSetHasNoChanges, CloudFormationStack, StackParameters, TemplateParameters, waitForChangeSet, waitForStack } from './util/cloudformation';
+import { changeSetHasNoChanges, CloudFormationStack, StackParameters, TemplateParameters, waitForChangeSet, waitForStackDeploy, waitForStackDelete } from './util/cloudformation';
 import { StackActivityMonitor } from './util/cloudformation/stack-activity-monitor';
 
 // We need to map regions to domain suffixes, and the SDK already has a function to do this.
@@ -175,7 +175,7 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
   if (cloudFormationStack.stackStatus.isCreationFailure) {
     debug(`Found existing stack ${deployName} that had previously failed creation. Deleting it before attempting to re-create it.`);
     await cfn.deleteStack({ StackName: deployName }).promise();
-    const deletedStack = await waitForStack(cfn, deployName, false);
+    const deletedStack = await waitForStackDelete(cfn, deployName);
     if (deletedStack && deletedStack.stackStatus.name !== 'DELETE_COMPLETE') {
       throw new Error(`Failed deleting stack ${deployName} that had previously failed creation (current state: ${deletedStack.stackStatus})`);
     }
@@ -263,7 +263,7 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
     }).start();
     debug('Execution of changeset %s on stack %s has started; waiting for the update to complete...', changeSetName, deployName);
     try {
-      const finalStack = await waitForStack(cfn, deployName);
+      const finalStack = await waitForStackDeploy(cfn, deployName);
 
       // This shouldn't really happen, but catch it anyway. You never know.
       if (!finalStack) { throw new Error('Stack deploy failed (the stack disappeared while we were deploying it)'); }
@@ -361,7 +361,7 @@ export async function destroyStack(options: DestroyStackOptions) {
 
   try {
     await cfn.deleteStack({ StackName: deployName, RoleARN: options.roleArn }).promise();
-    const destroyedStack = await waitForStack(cfn, deployName, false);
+    const destroyedStack = await waitForStackDelete(cfn, deployName);
     if (destroyedStack && destroyedStack.stackStatus.name !== 'DELETE_COMPLETE') {
       throw new Error(`Failed to destroy ${deployName}: ${destroyedStack.stackStatus}`);
     }

packages/aws-cdk/lib/api/util/cloudformation.ts

@@ -241,36 +241,62 @@ export function changeSetHasNoChanges(description: CloudFormation.DescribeChange
 }
 
 /**
- * Waits for a CloudFormation stack to stabilize in a complete/available state.
+ * Waits for a CloudFormation stack to stabilize in a complete/available state
+ * after a delete operation is issued.
  *
- * Fails if the stacks is not in a SUCCESSFUL state.
+ * Fails if the stack is in a FAILED state. Will not fail if the stack was
+ * already deleted.
  *
  * @param cfn        a CloudFormation client
- * @param stackName      the name of the stack to wait for
- * @param failOnDeletedStack whether to fail if the awaited stack is deleted.
+ * @param stackName      the name of the stack to wait for after a delete
  *
- * @returns     the CloudFormation description of the stabilized stack
+ * @returns     the CloudFormation description of the stabilized stack after the delete attempt
  */
-export async function waitForStack(
+export async function waitForStackDelete(
   cfn: CloudFormation,
-  stackName: string,
-  failOnDeletedStack: boolean = true): Promise<CloudFormationStack | undefined> {
+  stackName: string): Promise<CloudFormationStack | undefined> {
 
   const stack = await stabilizeStack(cfn, stackName);
   if (!stack) { return undefined; }
 
   const status = stack.stackStatus;
-  if (status.isCreationFailure) {
-    throw new Error(`The stack named ${stackName} failed creation, it may need to be manually deleted from the AWS console: ${status}`);
-  } else if (!status.isSuccess) {
-    throw new Error(`The stack named ${stackName} is in a failed state: ${status}`);
+  if (status.isFailure) {
+    throw new Error(`The stack named ${stackName} is in a failed state. You may need to delete it from the AWS console : ${status}`);
   } else if (status.isDeleted) {
-    if (failOnDeletedStack) { throw new Error(`The stack named ${stackName} was deleted`); }
     return undefined;
   }
   return stack;
 }
 
+/**
+ * Waits for a CloudFormation stack to stabilize in a complete/available state
+ * after an update/create operation is issued.
+ *
+ * Fails if the stack is in a FAILED state, ROLLBACK state, or DELETED state.
+ *
+ * @param cfn        a CloudFormation client
+ * @param stackName      the name of the stack to wait for after an update
+ *
+ * @returns     the CloudFormation description of the stabilized stack after the update attempt
+ */
+export async function waitForStackDeploy(
+  cfn: CloudFormation,
+  stackName: string): Promise<CloudFormationStack | undefined> {
+
+  const stack = await stabilizeStack(cfn, stackName);
+  if (!stack) { return undefined; }
+
+  const status = stack.stackStatus;
+
+  if (status.isCreationFailure) {
+    throw new Error(`The stack named ${stackName} failed creation, it may need to be manually deleted from the AWS console: ${status}`);
+  } else if (!status.isDeploySuccess) {
+    throw new Error(`The stack named ${stackName} failed to deploy: ${status}`);
+  }
+
+  return stack;
+}
+
 /**
  * Wait for a stack to become stable (no longer _IN_PROGRESS), returning it
  */
@@ -283,8 +309,8 @@ export async function stabilizeStack(cfn: CloudFormation, stackName: string) {
       return null;
     }
     const status = stack.stackStatus;
-    if (!status.isStable) {
-      debug('Stack %s is still not stable (%s)', stackName, status);
+    if (status.isInProgress) {
+      debug('Stack %s has an ongoing operation in progress and is not stable (%s)', stackName, status);
       return undefined;
     }
 

packages/aws-cdk/lib/api/util/cloudformation/stack-status.ts

@@ -25,20 +25,16 @@ export class StackStatus {
     return this.name.endsWith('FAILED');
   }
 
-  get isRollback(): boolean {
-    return this.name.indexOf('ROLLBACK') !== -1;
-  }
-
-  get isStable(): boolean {
-    return !this.name.endsWith('_IN_PROGRESS');
+  get isInProgress(): boolean {
+    return this.name.endsWith('_IN_PROGRESS');
   }
 
   get isNotFound(): boolean {
     return this.name === 'NOT_FOUND';
   }
 
-  get isSuccess(): boolean {
-    return !this.isNotFound && !this.isRollback && !this.isFailure;
+  get isDeploySuccess(): boolean {
+    return !this.isNotFound && (this.name === 'CREATE_COMPLETE' || this.name === 'UPDATE_COMPLETE');
   }
 
   public toString(): string {

packages/aws-cdk/test/api/deploy-stack.test.ts

@@ -401,6 +401,29 @@ test('deploy not skipped if template did not change but one tag removed', async
   expect(cfnMocks.getTemplate).toHaveBeenCalledWith({ StackName: 'withouterrors', TemplateStage: 'Original' });
 });
 
+test('existing stack in UPDATE_ROLLBACK_COMPLETE state can be updated', async () => {
+  // GIVEN
+  givenStackExists(
+    { StackStatus: 'UPDATE_ROLLBACK_COMPLETE' },    // This is for the initial check
+    { StackStatus: 'UPDATE_COMPLETE' },      // Poll the update
+  );
+  givenTemplateIs({ changed: 123 });
+
+  // WHEN
+  await deployStack({
+    stack: FAKE_STACK,
+    sdk,
+    sdkProvider,
+    resolvedEnvironment: mockResolvedEnvironment(),
+  });
+
+  // THEN
+  expect(cfnMocks.deleteStack).not.toHaveBeenCalled();
+  expect(cfnMocks.createChangeSet).toHaveBeenCalledWith(expect.objectContaining({
+    ChangeSetType: 'UPDATE',
+  }));
+});
+
 test('deploy not skipped if template changed', async () => {
   // GIVEN
   givenStackExists();

packages/aws-cdk/test/integ/cli/cli.integtest.ts

@@ -212,6 +212,96 @@ integTest('deploy with parameters', async () => {
   ]);
 });
 
+integTest('update to stack in ROLLBACK_COMPLETE state will delete stack and create a new one', async () => {
+  // GIVEN
+  await expect(cdkDeploy('param-test-1', {
+    options: [
+      '--parameters', `TopicNameParam=${STACK_NAME_PREFIX}@aww`,
+    ],
+    captureStderr: false,
+  })).rejects.toThrow('exited with error');
+
+  const response = await cloudFormation('describeStacks', {
+    StackName: fullStackName('param-test-1'),
+  });
+
+  const stackArn = response.Stacks?.[0].StackId;
+  expect(response.Stacks?.[0].StackStatus).toEqual('ROLLBACK_COMPLETE');
+
+  // WHEN
+  const newStackArn = await cdkDeploy('param-test-1', {
+    options: [
+      '--parameters', `TopicNameParam=${STACK_NAME_PREFIX}allgood`,
+    ],
+    captureStderr: false,
+  });
+
+  const newStackResponse = await cloudFormation('describeStacks', {
+    StackName: newStackArn,
+  });
+
+  // THEN
+  expect (stackArn).not.toEqual(newStackArn); // new stack was created
+  expect(newStackResponse.Stacks?.[0].StackStatus).toEqual('CREATE_COMPLETE');
+  expect(newStackResponse.Stacks?.[0].Parameters).toEqual([
+    {
+      ParameterKey: 'TopicNameParam',
+      ParameterValue: `${STACK_NAME_PREFIX}allgood`,
+    },
+  ]);
+});
+
+integTest('stack in UPDATE_ROLLBACK_COMPLETE state can be updated', async () => {
+  // GIVEN
+  const stackArn = await cdkDeploy('param-test-1', {
+    options: [
+      '--parameters', `TopicNameParam=${STACK_NAME_PREFIX}nice`,
+    ],
+    captureStderr: false,
+  });
+
+  let response = await cloudFormation('describeStacks', {
+    StackName: stackArn,
+  });
+
+  expect(response.Stacks?.[0].StackStatus).toEqual('CREATE_COMPLETE');
+
+  // bad parameter name with @ will put stack into UPDATE_ROLLBACK_COMPLETE
+  await expect(cdkDeploy('param-test-1', {
+    options: [
+      '--parameters', `TopicNameParam=${STACK_NAME_PREFIX}@aww`,
+    ],
+    captureStderr: false,
+  })).rejects.toThrow('exited with error');;
+
+  response = await cloudFormation('describeStacks', {
+    StackName: stackArn,
+  });
+
+  expect(response.Stacks?.[0].StackStatus).toEqual('UPDATE_ROLLBACK_COMPLETE');
+
+  // WHEN
+  await cdkDeploy('param-test-1', {
+    options: [
+      '--parameters', `TopicNameParam=${STACK_NAME_PREFIX}allgood`,
+    ],
+    captureStderr: false,
+  });
+
+  response = await cloudFormation('describeStacks', {
+    StackName: stackArn,
+  });
+
+  // THEN
+  expect(response.Stacks?.[0].StackStatus).toEqual('UPDATE_COMPLETE');
+  expect(response.Stacks?.[0].Parameters).toEqual([
+    {
+      ParameterKey: 'TopicNameParam',
+      ParameterValue: `${STACK_NAME_PREFIX}allgood`,
+    },
+  ]);
+});
+
 integTest('deploy with wildcard and parameters', async () => {
   await cdkDeploy('param-test-*', {
     options: [