From 07d3aa74e6c1a7b3b7ddf298cf3cc4b7ff180b48 Mon Sep 17 00:00:00 2001 From: Cory Hall <43035978+corymhall@users.noreply.github.com> Date: Mon, 27 Mar 2023 17:42:39 -0400 Subject: [PATCH] fix(integ-runner): update workflow doesn't support resource replacement (#24720) For the update workflow we deploy the stack update with the `--no-rollback` flag so that the assertion stack will deploy all the way through instead of failing on the first assertion failure. This causes issues if you are deploying an update that includes resource replacement since resource replacement is not allowed when using `--no-rollback` To fix this, this PR splits the stack update deployment into two separate deployments. The first deploys the test case stack and the second deploys the assertion stack with `--no-rollback`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/runner/integ-test-runner.ts | 28 ++++++++-- .../test/runner/integ-test-runner.test.ts | 54 ++++++++++++++++--- .../integ.json | 2 + .../integ.json | 2 + 4 files changed, 75 insertions(+), 11 deletions(-) diff --git a/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts b/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts index 1865912adc906..30bfca0138d30 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts @@ -285,23 +285,41 @@ export class IntegTestRunner extends IntegRunner { lookups: this.expectedTestSuite?.enableLookups, }); } - // now deploy the "actual" test. If there are any assertions - // deploy the assertion stack as well + // now deploy the "actual" test. this.cdk.deploy({ ...deployArgs, lookups: this.actualTestSuite.enableLookups, stacks: [ ...actualTestCase.stacks, - ...actualTestCase.assertionStack ? [actualTestCase.assertionStack] : [], ], - rollback: false, output: path.relative(this.directory, this.cdkOutDir), ...actualTestCase?.cdkCommandOptions?.deploy?.args, - ...actualTestCase.assertionStack ? { outputsFile: path.relative(this.directory, path.join(this.cdkOutDir, 'assertion-results.json')) } : undefined, context: this.getContext(actualTestCase?.cdkCommandOptions?.deploy?.args?.context), app: this.cdkApp, }); + // If there are any assertions + // deploy the assertion stack as well + // This is separate from the above deployment because we want to + // set `rollback: false`. This allows the assertion stack to deploy all the + // assertions instead of failing at the first failed assertion + // combining it with the above deployment would prevent any replacement updates + if (actualTestCase.assertionStack) { + this.cdk.deploy({ + ...deployArgs, + lookups: this.actualTestSuite.enableLookups, + stacks: [ + actualTestCase.assertionStack, + ], + rollback: false, + output: path.relative(this.directory, this.cdkOutDir), + ...actualTestCase?.cdkCommandOptions?.deploy?.args, + outputsFile: path.relative(this.directory, path.join(this.cdkOutDir, 'assertion-results.json')), + context: this.getContext(actualTestCase?.cdkCommandOptions?.deploy?.args?.context), + app: this.cdkApp, + }); + } + if (actualTestCase.hooks?.postDeploy) { actualTestCase.hooks.postDeploy.forEach(cmd => { exec(chunks(cmd), { diff --git a/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts b/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts index 20b4def9f12ff..fa9f247390353 100644 --- a/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts +++ b/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts @@ -51,7 +51,7 @@ describe('IntegTest runIntegTests', () => { }); // THEN - expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(2); + expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3); expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({ @@ -83,7 +83,6 @@ describe('IntegTest runIntegTests', () => { }), versionReporting: false, lookups: false, - rollback: false, stacks: ['test-stack', 'new-test-stack'], }); expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({ @@ -130,7 +129,6 @@ describe('IntegTest runIntegTests', () => { context: expect.not.objectContaining({ [AVAILABILITY_ZONE_FALLBACK_CONTEXT_KEY]: ['test-region-1a', 'test-region-1b', 'test-region-1c'], }), - rollback: false, lookups: false, stacks: ['stack1'], output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot', @@ -176,7 +174,6 @@ describe('IntegTest runIntegTests', () => { }), versionReporting: false, lookups: true, - rollback: false, stacks: ['test-stack'], output: 'cdk-integ.out.xxxxx.test-with-snapshot-assets-diff.js.snapshot', profile: undefined, @@ -206,6 +203,52 @@ describe('IntegTest runIntegTests', () => { }); }); + test('with an assertion stack', () => { + // WHEN + const integTest = new IntegTestRunner({ + cdk: cdkMock.cdk, + test: new IntegTest({ + fileName: 'test/test-data/xxxxx.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }), + }); + integTest.runIntegTestCase({ + testCaseName: 'xxxxx.test-with-snapshot', + }); + + // THEN + expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3); + expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1); + expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1); + expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(1, expect.objectContaining({ + app: 'xxxxx.test-with-snapshot.js.snapshot', + context: expect.any(Object), + stacks: ['test-stack'], + })); + expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(2, expect.not.objectContaining({ + rollback: false, + })); + expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(3, expect.objectContaining({ + app: 'node xxxxx.test-with-snapshot.js', + stacks: ['Bundling/DefaultTest/DeployAssert'], + rollback: false, + })); + expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({ + app: 'node xxxxx.test-with-snapshot.js', + pathMetadata: false, + assetMetadata: false, + context: expect.not.objectContaining({ + 'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({ + vpcId: 'vpc-60900905', + }), + }), + versionReporting: false, + force: true, + all: true, + output: 'cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot', + }); + }); + test('no clean', () => { // WHEN const integTest = new IntegTestRunner({ @@ -298,7 +341,6 @@ describe('IntegTest runIntegTests', () => { }), }), profile: 'test-profile', - rollback: false, lookups: false, stacks: ['stack1'], output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot', @@ -564,7 +606,7 @@ describe('IntegTest runIntegTests', () => { }); // THEN - expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(2); + expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3); expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1); expect(cdkMock.mocks.deploy).toHaveBeenCalledWith(expect.objectContaining({ diff --git a/packages/@aws-cdk/integ-runner/test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot/integ.json b/packages/@aws-cdk/integ-runner/test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot/integ.json index aefc17a9ec3b7..f9d5c63c8170b 100644 --- a/packages/@aws-cdk/integ-runner/test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot/integ.json +++ b/packages/@aws-cdk/integ-runner/test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot/integ.json @@ -3,6 +3,8 @@ "testCases": { "xxxxx.test-with-snapshot": { "stacks": ["test-stack", "new-test-stack"], + "assertionStack": "Bundling/DefaultTest/DeployAssert", + "assertionStackName": "BundlingDefaultTestDeployAssertAACA0CAF", "diffAssets": true, "allowDestroy": [ "AWS::IAM::Role" diff --git a/packages/@aws-cdk/integ-runner/test/test-data/xxxxx.test-with-snapshot.js.snapshot/integ.json b/packages/@aws-cdk/integ-runner/test/test-data/xxxxx.test-with-snapshot.js.snapshot/integ.json index 25baeac5899fd..8d0b1a4060057 100644 --- a/packages/@aws-cdk/integ-runner/test/test-data/xxxxx.test-with-snapshot.js.snapshot/integ.json +++ b/packages/@aws-cdk/integ-runner/test/test-data/xxxxx.test-with-snapshot.js.snapshot/integ.json @@ -3,6 +3,8 @@ "testCases": { "xxxxx.test-with-snapshot": { "stacks": ["test-stack"], + "assertionStack": "Bundling/DefaultTest/DeployAssert", + "assertionStackName": "BundlingDefaultTestDeployAssertAACA0CAF", "diffAssets": true, "allowDestroy": [ "AWS::IAM::Role"