feat(cloudformation): Shorten/simplify deployment prompts by promptin… (#8367)

…g for deployment mode first

## Problem
When customer wants to validate/deploy in REVERT_DRIFT mode, they have
to answer pre-requisite prompts correctly before they get prompted for
deployment mode.

## Solution
Ask for deployment mode first and don't ask the other prompts with
incompatible answers.

---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

---------

Co-authored-by: Akila Tennakoon <[email protected]>

Author: atennak1

packages/core/src/awsService/cloudformation/commands/cfnCommands.ts

@@ -22,7 +22,7 @@ import { Command } from 'vscode-languageclient/node'
 import * as yaml from 'js-yaml'
 
 import { Deployment } from '../stacks/actions/deploymentWorkflow'
-import { Parameter, Capability, OnStackFailure, Stack } from '@aws-sdk/client-cloudformation'
+import { Parameter, Capability, OnStackFailure, Stack, StackStatus } from '@aws-sdk/client-cloudformation'
 import {
     getParameterValues,
     getStackName,
@@ -210,13 +210,13 @@ type OptionalFlagSelection = ChangeSetOptionalFlags & {
     shouldSaveOptions?: boolean
 }
 
-function shouldPromptForDeploymentMode(
+function isRevertDriftEligible(
     stackDetails: Stack | undefined,
     importExistingResources: boolean | undefined,
     includeNestedStacks: boolean | undefined,
     onStackFailure: OnStackFailure | undefined
 ): boolean {
-    const isCreate = !stackDetails
+    const isCreate = !stackDetails || stackDetails.StackStatus === StackStatus.REVIEW_IN_PROGRESS
     const hasDisableRollback = onStackFailure === OnStackFailure.DO_NOTHING
 
     return !isCreate && !importExistingResources && !includeNestedStacks && !hasDisableRollback
@@ -247,7 +247,7 @@ export async function promptForOptionalFlags(
                 // default to REVERT_DRIFT if possible because it's generally useful
                 deploymentMode:
                     fileFlags?.deploymentMode ??
-                    (shouldPromptForDeploymentMode(
+                    (isRevertDriftEligible(
                         stackDetails,
                         fileFlags?.importExistingResources,
                         fileFlags?.includeNestedStacks,
@@ -260,21 +260,21 @@ export async function promptForOptionalFlags(
 
             break
         case OptionalFlagMode.Input: {
-            const onStackFailure = fileFlags?.onStackFailure ?? (await getOnStackFailure(!!stackDetails))
-            const includeNestedStacks = fileFlags?.includeNestedStacks ?? (await getIncludeNestedStacks())
-            const importExistingResources = fileFlags?.importExistingResources ?? (await getImportExistingResources())
-
-            let deploymentMode = fileFlags?.deploymentMode
-            if (
-                !deploymentMode &&
-                shouldPromptForDeploymentMode(
-                    stackDetails,
-                    importExistingResources,
-                    includeNestedStacks,
-                    onStackFailure
-                )
-            ) {
-                deploymentMode = await getDeploymentMode()
+            // Only available for UPDATE stack and is incompatible with the other options
+            const deploymentMode =
+                fileFlags?.deploymentMode ??
+                (isRevertDriftEligible(stackDetails, undefined, undefined, undefined)
+                    ? await getDeploymentMode()
+                    : undefined)
+
+            let onStackFailure: OnStackFailure | undefined
+            let includeNestedStacks: boolean | undefined
+            let importExistingResources: boolean | undefined
+
+            if (deploymentMode !== DeploymentMode.REVERT_DRIFT) {
+                onStackFailure = fileFlags?.onStackFailure ?? (await getOnStackFailure(stackDetails))
+                includeNestedStacks = fileFlags?.includeNestedStacks ?? (await getIncludeNestedStacks())
+                importExistingResources = fileFlags?.importExistingResources ?? (await getImportExistingResources())
             }
 
             optionalFlags = {

packages/core/src/awsService/cloudformation/ui/inputBox.ts

@@ -9,7 +9,7 @@ import {
     validateParameterValue,
     validateChangeSetName,
 } from '../stacks/actions/stackActionInputValidation'
-import { Parameter, Capability, Tag, OnStackFailure } from '@aws-sdk/client-cloudformation'
+import { Parameter, Capability, Tag, OnStackFailure, Stack, StackStatus } from '@aws-sdk/client-cloudformation'
 import {
     TemplateParameter,
     ResourceToImport,
@@ -255,13 +255,13 @@ export async function getImportExistingResources(): Promise<boolean | undefined>
     )?.value
 }
 
-export async function getOnStackFailure(stackExists?: boolean): Promise<OnStackFailure | undefined> {
+export async function getOnStackFailure(stackDetails?: Stack): Promise<OnStackFailure | undefined> {
     const options: Array<{ label: string; description: string; value: OnStackFailure }> = [
         { label: 'Do nothing', description: 'Leave stack in failed state', value: OnStackFailure.DO_NOTHING },
         { label: 'Rollback', description: 'Rollback to previous state', value: OnStackFailure.ROLLBACK },
     ]
 
-    if (!stackExists) {
+    if (!stackDetails || stackDetails.StackStatus === StackStatus.REVIEW_IN_PROGRESS) {
         // only a valid option for CREATE
         options.unshift({ label: 'Delete', description: 'Delete the stack on failure', value: OnStackFailure.DELETE })
     }
@@ -276,7 +276,7 @@ export async function getDeploymentMode(): Promise<DeploymentMode | undefined> {
             [
                 {
                     label: 'Revert Drift',
-                    description: 'Revert drift during deployment',
+                    description: 'Revert drift during deployment (disables dev friendly flags)',
                     value: DeploymentMode.REVERT_DRIFT,
                 },
                 { label: 'Standard', description: 'No special handling during deployment', value: undefined },

packages/core/src/test/awsService/cloudformation/commands/cfnCommands.test.ts

@@ -112,6 +112,7 @@ describe('CfnCommands', function () {
 
         it('should set shouldSaveOptions to true when input mode collects new values', async function () {
             chooseOptionalFlagModeStub.resolves(OptionalFlagMode.Input)
+            getDeploymentModeStub.resolves(undefined)
             getOnStackFailureStub.resolves(OnStackFailure.DELETE)
             getIncludeNestedStacksStub.resolves(true)
             getTagsStub.resolves([{ Key: 'Environment', Value: 'prod' }])
@@ -129,53 +130,80 @@ describe('CfnCommands', function () {
             })
         })
 
-        it('should prompt for deployment mode on stack update when conditions are met', async function () {
+        it('should not prompt for deployment mode for CREATE stack', async function () {
             chooseOptionalFlagModeStub.resolves(OptionalFlagMode.Input)
             getOnStackFailureStub.resolves(OnStackFailure.ROLLBACK)
             getIncludeNestedStacksStub.resolves(false)
             getTagsStub.resolves(undefined)
             getImportExistingResourcesStub.resolves(false)
-            getDeploymentModeStub.resolves('INCREMENTAL')
 
-            const stackDetails = { StackName: 'test-stack' }
-            const result = await promptForOptionalFlags(undefined, stackDetails as any)
+            const result = await promptForOptionalFlags()
 
-            assert.ok(getDeploymentModeStub.calledOnce)
+            assert.ok(getDeploymentModeStub.notCalled)
+            assert.ok(getOnStackFailureStub.calledOnce)
+            assert.ok(getIncludeNestedStacksStub.calledOnce)
+            assert.ok(getImportExistingResourcesStub.calledOnce)
             assert.deepStrictEqual(result, {
                 onStackFailure: OnStackFailure.ROLLBACK,
                 includeNestedStacks: false,
                 tags: undefined,
                 importExistingResources: false,
-                deploymentMode: 'INCREMENTAL',
+                deploymentMode: undefined,
                 shouldSaveOptions: true,
             })
         })
 
-        it('should not prompt for deployment mode on stack create', async function () {
+        it('should not prompt for deployment mode when stack is REVIEW_IN_PROGRESS', async function () {
             chooseOptionalFlagModeStub.resolves(OptionalFlagMode.Input)
             getOnStackFailureStub.resolves(OnStackFailure.ROLLBACK)
             getIncludeNestedStacksStub.resolves(false)
             getTagsStub.resolves(undefined)
             getImportExistingResourcesStub.resolves(false)
 
-            const result = await promptForOptionalFlags()
+            const stackDetails = { StackName: 'test-stack', StackStatus: 'REVIEW_IN_PROGRESS' as any }
+            const result = await promptForOptionalFlags(undefined, stackDetails as any)
 
             assert.ok(getDeploymentModeStub.notCalled)
             assert.strictEqual(result?.deploymentMode, undefined)
         })
 
-        it('should not prompt for deployment mode when importExistingResources is true', async function () {
+        it('should prompt for deployment mode and other flags when not REVERT_DRIFT', async function () {
             chooseOptionalFlagModeStub.resolves(OptionalFlagMode.Input)
-            getOnStackFailureStub.resolves(OnStackFailure.ROLLBACK)
-            getIncludeNestedStacksStub.resolves(false)
+            getDeploymentModeStub.resolves(undefined)
+            getOnStackFailureStub.resolves(OnStackFailure.DELETE)
+            getIncludeNestedStacksStub.resolves(true)
             getTagsStub.resolves(undefined)
             getImportExistingResourcesStub.resolves(true)
 
+            const stackDetails = { StackName: 'test-stack' }
+            await promptForOptionalFlags(undefined, stackDetails as any)
+
+            assert.ok(getDeploymentModeStub.calledOnce)
+            assert.ok(getOnStackFailureStub.calledOnce)
+            assert.ok(getIncludeNestedStacksStub.calledOnce)
+            assert.ok(getImportExistingResourcesStub.calledOnce)
+        })
+
+        it('should skip other prompts when deploymentMode is REVERT_DRIFT', async function () {
+            chooseOptionalFlagModeStub.resolves(OptionalFlagMode.Input)
+            getDeploymentModeStub.resolves('REVERT_DRIFT')
+            getTagsStub.resolves(undefined)
+
             const stackDetails = { StackName: 'test-stack' }
             const result = await promptForOptionalFlags(undefined, stackDetails as any)
 
-            assert.ok(getDeploymentModeStub.notCalled)
-            assert.strictEqual(result?.deploymentMode, undefined)
+            assert.ok(getDeploymentModeStub.calledOnce)
+            assert.ok(getOnStackFailureStub.notCalled)
+            assert.ok(getIncludeNestedStacksStub.notCalled)
+            assert.ok(getImportExistingResourcesStub.notCalled)
+            assert.deepStrictEqual(result, {
+                onStackFailure: undefined,
+                includeNestedStacks: undefined,
+                tags: undefined,
+                importExistingResources: undefined,
+                deploymentMode: 'REVERT_DRIFT',
+                shouldSaveOptions: true,
+            })
         })
 
         it('should include deploymentMode from fileFlags in skip mode', async function () {
@@ -246,6 +274,29 @@ describe('CfnCommands', function () {
             })
         })
 
+        it('should not default to REVERT_DRIFT in skip mode when stack is REVIEW_IN_PROGRESS', async function () {
+            chooseOptionalFlagModeStub.resolves(OptionalFlagMode.Skip)
+
+            const fileFlags = {
+                onStackFailure: OnStackFailure.ROLLBACK,
+                includeNestedStacks: false,
+                tags: undefined,
+                importExistingResources: false,
+            }
+
+            const stackDetails = { StackName: 'test-stack', StackStatus: 'REVIEW_IN_PROGRESS' as any }
+            const result = await promptForOptionalFlags(fileFlags, stackDetails as any)
+
+            assert.deepStrictEqual(result, {
+                onStackFailure: OnStackFailure.ROLLBACK,
+                includeNestedStacks: false,
+                tags: undefined,
+                importExistingResources: false,
+                deploymentMode: undefined,
+                shouldSaveOptions: false,
+            })
+        })
+
         it('should not default to REVERT_DRIFT in skip mode when includeNestedStacks is true', async function () {
             chooseOptionalFlagModeStub.resolves(OptionalFlagMode.Skip)
 

packages/toolkit/.changes/next-release/Feature-412b3e72-e203-47fe-a851-c454d56b86a8.json

@@ -0,0 +1,4 @@
+{
+	"type": "Feature",
+	"description": "CloudFormation: Shorten/simplify deployment prompts by prompting for deployment mode first"
+}