From c1903678aba31ca5b23a3bebb84249921e15dd5c Mon Sep 17 00:00:00 2001 From: Patrick Florek Date: Sat, 26 Feb 2022 03:36:06 +0100 Subject: [PATCH 1/2] fix(custom-resources): physical resource id must be determined before isComplete (#18630) For some resource on event can only generate an intermediary physical resource id. On isComplete the actual physical resource id can be retrieved from the created resource. Therefor it's useful to override the temporary one. Fixes https://github.com/aws/aws-cdk/issues/18670 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/provider-framework/runtime/framework.ts | 1 + .../lib/provider-framework/types.d.ts | 7 ++++++- .../test/provider-framework/runtime.test.ts | 17 +++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts b/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts index 181968611d354..9c2d8a4bfc0fc 100644 --- a/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts +++ b/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts @@ -73,6 +73,7 @@ async function isComplete(event: AWSCDKAsyncCustomResource.IsCompleteRequest) { const response = { ...event, + ...isCompleteResult, Data: { ...event.Data, ...isCompleteResult.Data, diff --git a/packages/@aws-cdk/custom-resources/lib/provider-framework/types.d.ts b/packages/@aws-cdk/custom-resources/lib/provider-framework/types.d.ts index 9a9536eac078b..288dc217c7f82 100644 --- a/packages/@aws-cdk/custom-resources/lib/provider-framework/types.d.ts +++ b/packages/@aws-cdk/custom-resources/lib/provider-framework/types.d.ts @@ -4,7 +4,7 @@ /** * these types can be accessed without needing to `import` the module. * e.g. `AWSCDKAsyncCustomResource.OnEventRequest` - */ + */ export as namespace AWSCDKAsyncCustomResource; /** @@ -105,6 +105,11 @@ export interface IsCompleteResponse { */ readonly IsComplete: boolean; + /** + * If present, overrides the PhysicalResourceId of OnEventResponse with the PhysicalResourceId of IsCompleteResponse. + */ + readonly PhysicalResourceId?: string; + /** * Additional/changes to resource attributes. This hash will be merged with the one returned from `OnEventResponse`. */ diff --git a/packages/@aws-cdk/custom-resources/test/provider-framework/runtime.test.ts b/packages/@aws-cdk/custom-resources/test/provider-framework/runtime.test.ts index d2af0a4fafd2d..751e33e6408e0 100644 --- a/packages/@aws-cdk/custom-resources/test/provider-framework/runtime.test.ts +++ b/packages/@aws-cdk/custom-resources/test/provider-framework/runtime.test.ts @@ -169,6 +169,23 @@ describe('PhysicalResourceId', () => { }); }); + test('UPDATE: can override the physical ID with the actual on isComplete', async () => { + // GIVEN + mocks.onEventImplMock = async () => ({ PhysicalResourceId: 'TemporaryPhysicalId' }); + mocks.isCompleteImplMock = async () => ({ IsComplete: true, PhysicalResourceId: 'NewPhysicalId' }); + + // WHEN + await simulateEvent({ + RequestType: 'Update', + PhysicalResourceId: 'CurrentPhysicalId', + }); + + // THEN + expectCloudFormationSuccess({ + PhysicalResourceId: 'NewPhysicalId', + }); + }); + test('DELETE: cannot change the physical resource ID during a delete', async () => { // GIVEN mocks.onEventImplMock = async () => ({ PhysicalResourceId: 'NewPhysicalId' }); From b0e0155f87a65c34a75e11776f98d55b83d2b220 Mon Sep 17 00:00:00 2001 From: Visa de Bruijn Date: Sat, 26 Feb 2022 05:20:13 +0200 Subject: [PATCH 2/2] fix: construct paths are not printed for nested stacks in CLI output (#18725) This fixes issue https://github.com/aws/aws-cdk/issues/18724 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/artifacts/cloudformation-artifact.ts | 11 +++---- .../cx-api/test/cloud-assembly.test.ts | 17 ++++++++++- .../test/fixtures/nested-stacks/manifest.json | 30 +++++++++++++++++++ .../nested-stacks/nestedStack.template.json | 0 .../nestedStackWithStackName.template.json | 0 .../nested-stacks/topLevelStack.template.json | 0 6 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/manifest.json create mode 100644 packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/nestedStack.template.json create mode 100644 packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/nestedStackWithStackName.template.json create mode 100644 packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/topLevelStack.template.json diff --git a/packages/@aws-cdk/cx-api/lib/artifacts/cloudformation-artifact.ts b/packages/@aws-cdk/cx-api/lib/artifacts/cloudformation-artifact.ts index 3e6f395b5c99d..f601131d9f532 100644 --- a/packages/@aws-cdk/cx-api/lib/artifacts/cloudformation-artifact.ts +++ b/packages/@aws-cdk/cx-api/lib/artifacts/cloudformation-artifact.ts @@ -37,9 +37,10 @@ export class CloudFormationStackArtifact extends CloudArtifact { public readonly stackName: string; /** - * A string that represents this stack. Should only be used in user interfaces. - * If the stackName and artifactId are the same, it will just return that. Otherwise, - * it will return something like " ()" + * A string that represents this stack. Should only be used in user + * interfaces. If the stackName has not been set explicitly, or has been set + * to artifactId, it will return the hierarchicalId of the stack. Otherwise, + * it will return something like " ()" */ public readonly displayName: string; @@ -148,8 +149,8 @@ export class CloudFormationStackArtifact extends CloudArtifact { this.assets = this.findMetadataByType(cxschema.ArtifactMetadataEntryType.ASSET).map(e => e.data as cxschema.AssetMetadataEntry); this.displayName = this.stackName === artifactId - ? this.stackName - : `${artifactId} (${this.stackName})`; + ? this.hierarchicalId + : `${this.hierarchicalId} (${this.stackName})`; this.name = this.stackName; // backwards compat this.originalName = this.stackName; diff --git a/packages/@aws-cdk/cx-api/test/cloud-assembly.test.ts b/packages/@aws-cdk/cx-api/test/cloud-assembly.test.ts index 2151e2345620c..804d1e43e16b6 100644 --- a/packages/@aws-cdk/cx-api/test/cloud-assembly.test.ts +++ b/packages/@aws-cdk/cx-api/test/cloud-assembly.test.ts @@ -132,7 +132,22 @@ test('getStackArtifact retrieves a stack by artifact id', () => { expect(assembly.getStackArtifact('stack1').id).toEqual('stack1'); }); -test('displayName shows both artifact ID and stack name if needed', () => { +test('displayName shows hierarchical ID for nested stack without explicit stackName', () => { + const assembly = new CloudAssembly(path.join(FIXTURES, 'nested-stacks')); + const stackArtifact = assembly.getStackArtifact('topLevelStackNestedStackDAC87084'); + expect(stackArtifact.hierarchicalId).toStrictEqual('topLevelStack/nestedStack'); + expect(stackArtifact.displayName).toStrictEqual('topLevelStack/nestedStack'); +}); + +test('displayName shows hierarchical ID and stackName for nested stack with explicit stackName', () => { + const assembly = new CloudAssembly(path.join(FIXTURES, 'nested-stacks')); + const nestedStack = assembly.getStackArtifact('topLevelStackNestedStackWithStackName6D28EAEF'); + expect(nestedStack.hierarchicalId).toStrictEqual('topLevelStack/nestedStackWithStackName'); + expect(nestedStack.stackName).toStrictEqual('explicitStackName'); + expect(nestedStack.displayName).toStrictEqual('topLevelStack/nestedStackWithStackName (explicitStackName)'); +}); + +test('displayName shows both hierarchical ID and stack name if needed', () => { const a1 = new CloudAssembly(path.join(FIXTURES, 'multiple-stacks-same-name')); expect(a1.getStackArtifact('stack1').displayName).toStrictEqual('stack1 (the-physical-name-of-the-stack)'); expect(a1.getStackArtifact('stack2').displayName).toStrictEqual('stack2 (the-physical-name-of-the-stack)'); diff --git a/packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/manifest.json b/packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/manifest.json new file mode 100644 index 0000000000000..8d732d2105e2d --- /dev/null +++ b/packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/manifest.json @@ -0,0 +1,30 @@ +{ + "version": "0.0.0", + "artifacts": { + "topLevelStack": { + "type": "aws:cloudformation:stack", + "environment": "aws://111111111111/us-east-1", + "properties": { + "templateFile": "topLevelStack.template.json" + }, + "displayName": "topLevelStack" + }, + "topLevelStackNestedStackDAC87084": { + "type": "aws:cloudformation:stack", + "environment": "aws://111111111111/us-east-1", + "properties": { + "templateFile": "nestedStack.template.json" + }, + "displayName": "topLevelStack/nestedStack" + }, + "topLevelStackNestedStackWithStackName6D28EAEF": { + "type": "aws:cloudformation:stack", + "environment": "aws://111111111111/us-east-1", + "properties": { + "templateFile": "nestedStackWithStackName.template.json", + "stackName": "explicitStackName" + }, + "displayName": "topLevelStack/nestedStackWithStackName" + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/nestedStack.template.json b/packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/nestedStack.template.json new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/nestedStackWithStackName.template.json b/packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/nestedStackWithStackName.template.json new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/topLevelStack.template.json b/packages/@aws-cdk/cx-api/test/fixtures/nested-stacks/topLevelStack.template.json new file mode 100644 index 0000000000000..e69de29bb2d1d