diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/cloudformation/stack-helpers.ts b/packages/@aws-cdk/toolkit-lib/lib/api/cloudformation/stack-helpers.ts index adba4cb66..271258a26 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/cloudformation/stack-helpers.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/cloudformation/stack-helpers.ts @@ -67,16 +67,6 @@ export class CloudFormationStack { ) { } - /** - * Returns the Stack object returned from the CloudFormation service response that this decorating object wraps - */ - public get wrapped(): Stack { - if (!this.stack) { - throw new ToolkitError('NoStack', 'CloudFormationStack object does not hold a stack'); - } - return this.stack; - } - /** * Retrieve the stack's deployed template * diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deploy-stack.ts b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deploy-stack.ts index c44350a97..04b3bbc44 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deploy-stack.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deploy-stack.ts @@ -45,6 +45,7 @@ import { invalidateHotswapTemplateCache, readHotswapTemplateCache } from '../hot import type { IoHelper } from '../io/private'; import type { ResourcesToImport } from '../resource-import'; import { StackActivityMonitor } from '../stack-events'; +import type { ResourceErrors } from '../stack-events/resource-errors'; export interface DeployStackOptions { /** @@ -719,7 +720,7 @@ class FullCloudFormationDeployment { }); await monitor.start(); - let finalState = this.cloudFormationStack; + let finalState: CloudFormationStack; try { const successStack = await waitForStackDeploy(this.cfn, this.ioHelper, this.stackName, this.options.stackEventPollingInterval); @@ -729,15 +730,12 @@ class FullCloudFormationDeployment { } finalState = successStack; } catch (e: any) { - // If this is a deployment error, route the diagnosis and error reporting through the central code for that + // Deployment errors get replaced by a diagnosis of the underlying resource failures, which says more. + // Any other error, and any failure to diagnose, leaves `e` to propagate as it is. if (ToolkitError.isDeploymentError(e)) { - const diagnosis = await this.diagnoser.diagnoseFromErrorCollection(monitor.errors, finalState.wrapped, true, { - rollbackEnabled: this.options.rollback !== false, - }); - diagnosis.throwOnError(); + await this.diagnoseDeploymentFailure(stackArn, monitor.errors); } - // Otherwise rethrow the current error and hope it has enough information. throw e; } finally { await monitor.stop(); @@ -753,6 +751,31 @@ class FullCloudFormationDeployment { }; } + /** + * Throw a `DeploymentError` describing why the deployment failed, if we can establish that + * + * Returns normally when no cause could be established, leaving the caller's original error as the better + * one to report. + */ + private async diagnoseDeploymentFailure(stackArn: string, errors: ResourceErrors): Promise { + // Describe the stack as it is now. The pre-deploy description held by this class is either absent (the + // stack is being created) or describes a state the deployment has since left. + const deployedState = await this.cfn.describeStacks({ StackName: stackArn }) + .then((response) => response.Stacks?.[0]) + .catch(async (e) => { + await this.ioHelper.defaults.debug(`Could not describe ${stackArn} to diagnose the failure: ${formatErrorMessage(e)}`); + return undefined; + }); + if (!deployedState) { + return; + } + + const diagnosis = await this.diagnoser.diagnoseFromErrorCollection(errors, deployedState, true, { + rollbackEnabled: this.options.rollback !== false, + }); + diagnosis.throwOnError(); + } + /** * Return the options that are shared between CreateStack, UpdateStack and CreateChangeSet */ diff --git a/packages/@aws-cdk/toolkit-lib/test/api/deployments/deploy-stack-error-surfacing.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/deployments/deploy-stack-error-surfacing.test.ts new file mode 100644 index 000000000..c847ab0d4 --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/test/api/deployments/deploy-stack-error-surfacing.test.ts @@ -0,0 +1,149 @@ +import type { CloudFormationStackArtifact } from '@aws-cdk/cloud-assembly-api'; +import { deployStack } from '../../../lib/api/deployments/deploy-stack'; +import type { DeployStackOptions as DeployStackApiOptions } from '../../../lib/api/deployments/deploy-stack'; +import { CloudFormationStackDiagnoser } from '../../../lib/api/diagnosing/stack-diagnoser'; +import { NoBootstrapStackEnvironmentResources } from '../../../lib/api/environment'; +import { StackArtifactSourceTracer } from '../../../lib/api/source-tracing/private/stack-source-tracing'; +import { ToolkitError } from '../../../lib/toolkit/toolkit-error'; +import { testStack } from '../../_helpers/assembly'; +import { FakeCloudFormation } from '../../_helpers/fake-aws/fake-cloudformation'; +import { advanceTime } from '../../_helpers/fake-time'; +import { + mockCloudFormationClient, + mockResolvedEnvironment, + MockSdk, + MockSdkProvider, + restoreSdkMocksToDefault, +} from '../../_helpers/mock-sdk'; +import { TestIoHost } from '../../_helpers/test-io-host'; + +jest.mock('../../../lib/api/deployments/checks', () => ({ + determineAllowCrossAccountAssetPublishing: jest.fn().mockResolvedValue(true), +})); + +const ioHost = new TestIoHost(); +const ioHelper = ioHost.asHelper('deploy'); + +const FAILING_STACK = testStack({ + stackName: 'freshstack', + template: { + Resources: { + Bad: { + Type: 'Test::Fake::Resource', + Properties: { Fail: true }, + }, + }, + }, +}); + +let sdk: MockSdk; +let sdkProvider: MockSdkProvider; +const fakeCfn = new FakeCloudFormation(); + +beforeEach(() => { + fakeCfn.reset(); + sdkProvider = new MockSdkProvider(); + sdk = new MockSdk(); + sdk.getUrlSuffix = () => Promise.resolve('amazonaws.com'); + restoreSdkMocksToDefault(); + fakeCfn.installUsingAwsMock(mockCloudFormationClient); + jest.useFakeTimers(); +}); + +afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); +}); + +function standardDeployStackArguments(stack: CloudFormationStackArtifact): DeployStackApiOptions { + const resolvedEnvironment = mockResolvedEnvironment(); + return { + stack, + sdk, + sdkProvider, + resolvedEnvironment, + envResources: new NoBootstrapStackEnvironmentResources(resolvedEnvironment, sdk, ioHelper), + diagnoser: new CloudFormationStackDiagnoser({ + sdk, + sourceTracer: new StackArtifactSourceTracer(stack), + ioHelper, + topLevelStackHierarchicalId: stack.hierarchicalId, + }), + }; +} + +describe.each(['change-set', 'direct'] as const)('a failing %s deployment of a new stack', (method) => { + test('reports the CloudFormation failure rather than an internal NoStack error', async () => { + // GIVEN - a stack that does not exist yet, so the pre-deploy lookup holds no stack at all + const deployment = advanceTime(deployStack({ + ...standardDeployStackArguments(FAILING_STACK), + deploymentMethod: { method }, + }, ioHelper)); + + // THEN - either the resource-level diagnosis or the status the stack ended up in, but something + // that names the actual failure + await expect(deployment).rejects.toThrow(/freshstack\/Bad|ROLLBACK_COMPLETE/); + await expect(deployment).rejects.not.toThrow(/does not hold a stack/); + await expect(deployment).rejects.toMatchObject({ name: 'DeploymentError' }); + }); + + test('reports the CloudFormation failure when rollback is disabled', async () => { + // GIVEN - without rollback the stack stays CREATE_FAILED rather than rolling back, reaching the + // diagnoser through a different status + const deployment = advanceTime(deployStack({ + ...standardDeployStackArguments(FAILING_STACK), + deploymentMethod: { method }, + rollback: false, + }, ioHelper)); + + // THEN + await expect(deployment).rejects.toThrow(/freshstack\/Bad|CREATE_FAILED/); + await expect(deployment).rejects.not.toThrow(/does not hold a stack/); + }); +}); + +test('a failing update of an existing stack still reports the CloudFormation failure', async () => { + // GIVEN - the same failure without the absent-stack path, guarding against a fix that only works + // when the stack is missing + fakeCfn.createStackSync({ StackName: 'freshstack' }); + + // WHEN + const deployment = advanceTime(deployStack({ + ...standardDeployStackArguments(FAILING_STACK), + deploymentMethod: { method: 'change-set' }, + }, ioHelper)); + + // THEN + await expect(deployment).rejects.toThrow(/freshstack\/Bad|UPDATE_ROLLBACK_COMPLETE/); + await expect(deployment).rejects.not.toThrow(/does not hold a stack/); +}); + +test('a failed lookup while diagnosing leaves the deployment error intact', async () => { + // GIVEN - describing the deployed stack fails while we are trying to diagnose why the deployment + // failed. Diagnosis is best-effort, so this must not replace the deployment error. + const debugIoHost = new TestIoHost('debug'); + const realClient = sdk.cloudFormation(); + jest.spyOn(sdk, 'cloudFormation').mockReturnValue({ + ...realClient, + describeStacks: async (input) => { + if (input.StackName?.startsWith('arn:')) { + throw new ToolkitError('Throttling', 'Rate exceeded'); + } + return realClient.describeStacks(input); + }, + }); + + // WHEN + const deployment = advanceTime(deployStack({ + ...standardDeployStackArguments(FAILING_STACK), + deploymentMethod: { method: 'change-set' }, + }, debugIoHost.asHelper('deploy'))); + + // THEN + await expect(deployment).rejects.toThrow(/ROLLBACK_COMPLETE/); + await expect(deployment).rejects.not.toThrow(/Rate exceeded/); + await expect(deployment).rejects.toMatchObject({ name: 'DeploymentError' }); + + // ...and the swallowed lookup failure is still visible to anyone debugging + debugIoHost.expectMessage({ level: 'debug', containing: 'Rate exceeded' }); +});