From aabd36d0a966bd045f9a930fd4e5fa3e3a004161 Mon Sep 17 00:00:00 2001 From: Eli Polonsky Date: Sun, 1 Mar 2020 17:29:00 +0200 Subject: [PATCH] feat(custom-resources): physical resource id union type (#6518) * added additional stack to 'ls' test so it doesn't fail * combine physicalResourceId and physicalResourceIdPath to a union type * fix tests according to new api * added doc strings and rename argument * fix tests * fixing some tests * enhance 'fromResponsePath' docstring * fix references to physicalResourceId in README * fix integ expected template and rename fromResponsePath to fromResponse * Rephrase docstring for `fromResponse` Co-Authored-By: Elad Ben-Israel * Rephrase docstring for `of` Co-Authored-By: Elad Ben-Israel * fix README reference to fromResponsePath Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Elad Ben-Israel --- packages/@aws-cdk/custom-resources/README.md | 26 +++++------ .../aws-custom-resource.ts | 46 +++++++++++++------ .../lib/aws-custom-resource/runtime/index.ts | 12 ++--- .../aws-custom-resource-provider.test.ts | 18 ++++---- .../aws-custom-resource.test.ts | 46 +++++++++++-------- .../integ.aws-custom-resource.expected.json | 42 +++++++++++------ .../integ.aws-custom-resource.ts | 8 ++-- 7 files changed, 118 insertions(+), 80 deletions(-) diff --git a/packages/@aws-cdk/custom-resources/README.md b/packages/@aws-cdk/custom-resources/README.md index 56d496856e039..d8ba5bb9205e7 100644 --- a/packages/@aws-cdk/custom-resources/README.md +++ b/packages/@aws-cdk/custom-resources/README.md @@ -63,7 +63,7 @@ The following example is a skeleton for a Python implementation of `onEvent`: ```py def on_event(event, context): - print(event) + print(event) request_type = event['RequestType'] if request_type == 'Create': return on_create(event) if request_type == 'Update': return on_update(event) @@ -76,9 +76,9 @@ def on_create(event): # add your create code here... physical_id = ... - + return { 'PhysicalResourceId': physical_id } - + def on_update(event): physical_id = event["PhysicalResourceId"] props = event["ResourceProperties"] @@ -104,8 +104,8 @@ def is_complete(event, context): request_type = event["RequestType"] # check if resource is stable based on request_type - is_ready = ... - + is_ready = ... + return { 'IsComplete': is_ready } ``` @@ -121,7 +121,7 @@ If `onEvent` returns successfully, the framework will submit a "SUCCESS" respons to AWS CloudFormation for this resource operation. If the provider is [asynchronous](#asynchronous-providers-iscomplete) (`isCompleteHandler` is defined), the framework will only submit a response based on the result of -`isComplete`. +`isComplete`. If `onEvent` throws an error, the framework will submit a "FAILED" response to AWS CloudFormation. @@ -153,10 +153,10 @@ The return value from `onEvent` must be a JSON object with the following fields: It is not uncommon for the provisioning of resources to be an asynchronous operation, which means that the operation does not immediately finish, and we -need to "wait" until the resource stabilizes. +need to "wait" until the resource stabilizes. The provider framework makes it easy to implement "waiters" by allowing users to -specify an additional AWS Lambda function in `isCompleteHandler`. +specify an additional AWS Lambda function in `isCompleteHandler`. The framework will repeatedly invoke the handler every `queryInterval`. When `isComplete` returns with `IsComplete: true`, the framework will submit a @@ -230,7 +230,7 @@ lifecycle events: ### Execution Policy Similarly to any AWS Lambda function, if the user-defined handlers require -access to AWS resources, you will have to define these permissions +access to AWS resources, you will have to define these permissions by calling "grant" methods such as `myBucket.grantRead(myHandler)`), using `myHandler.addToRolePolicy` or specifying an `initialPolicy` when defining the function. @@ -353,7 +353,7 @@ const awsCustom1 = new AwsCustomResource(this, 'API1', { onCreate: { service: '...', action: '...', - physicalResourceId: '...' + physicalResourceId: PhysicalResourceId.of('...') } }); @@ -364,7 +364,7 @@ const awsCustom2 = new AwsCustomResource(this, 'API2', { parameters: { text: awsCustom1.getDataString('Items.0.text') }, - physicalResourceId: '...' + physicalResourceId: PhysicalResourceId.of('...') } }) ``` @@ -381,7 +381,7 @@ const verifyDomainIdentity = new AwsCustomResource(this, 'VerifyDomainIdentity', parameters: { Domain: 'example.com' }, - physicalResourceIdPath: 'VerificationToken' // Use the token returned by the call as physical id + physicalResourceId: PhysicalResourceId.fromResponse('VerificationToken') // Use the token returned by the call as physical id } }); @@ -403,7 +403,7 @@ const getParameter = new AwsCustomResource(this, 'GetParameter', { Name: 'my-parameter', WithDecryption: true }, - physicalResourceId: Date.now().toString() // Update physical id to always fetch the latest version + physicalResourceId: PhysicalResourceId.of(Date.now().toString()) // Update physical id to always fetch the latest version } }); diff --git a/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts b/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts index f8c17c3db0aef..46fc02f013ec8 100644 --- a/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts +++ b/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts @@ -16,6 +16,32 @@ export type AwsSdkMetadata = {[key: string]: any}; const awsSdkMetadata: AwsSdkMetadata = metadata; +/** + * Physical ID of the custom resource. + */ +export class PhysicalResourceId { + + /** + * Extract the physical resource id from the path (dot notation) to the data in the API call response. + */ + public static fromResponse(responsePath: string): PhysicalResourceId { + return new PhysicalResourceId(responsePath, undefined); + } + + /** + * Explicit physical resource id. + */ + public static of(id: string): PhysicalResourceId { + return new PhysicalResourceId(undefined, id); + } + + /** + * @param responsePath Path to a response data element to be used as the physical id. + * @param id Literal string to be used as the physical id. + */ + private constructor(public readonly responsePath?: string, public readonly id?: string) { } +} + /** * An AWS SDK call. */ @@ -42,22 +68,12 @@ export interface AwsSdkCall { readonly parameters?: any; /** - * The path to the data in the API call response to use as the physical - * resource id. Either `physicalResourceId` or `physicalResourceIdPath` - * must be specified for onCreate or onUpdate calls. - * - * @default - no path - */ - readonly physicalResourceIdPath?: string; - - /** - * The physical resource id of the custom resource for this call. Either - * `physicalResourceId` or `physicalResourceIdPath` must be specified for - * onCreate or onUpdate calls. + * The physical resource id of the custom resource for this call. + * Mandatory for onCreate or onUpdate calls. * * @default - no physical resource id */ - readonly physicalResourceId?: string; + readonly physicalResourceId?: PhysicalResourceId; /** * The regex pattern to use to catch API errors. The `code` property of the @@ -174,8 +190,8 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { } for (const call of [props.onCreate, props.onUpdate]) { - if (call && !call.physicalResourceId && !call.physicalResourceIdPath) { - throw new Error('Either `physicalResourceId` or `physicalResourceIdPath` must be specified for onCreate and onUpdate calls.'); + if (call && !call.physicalResourceId) { + throw new Error('`physicalResourceId` must be specified for onCreate and onUpdate calls.'); } } diff --git a/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/runtime/index.ts b/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/runtime/index.ts index 5794ad3f98e15..9507a8f9388b1 100644 --- a/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/runtime/index.ts +++ b/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/runtime/index.ts @@ -90,14 +90,14 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent let physicalResourceId: string; switch (event.RequestType) { case 'Create': - physicalResourceId = event.ResourceProperties.Create?.physicalResourceId ?? - event.ResourceProperties.Update?.physicalResourceId ?? - event.ResourceProperties.Delete?.physicalResourceId ?? + physicalResourceId = event.ResourceProperties.Create?.physicalResourceId?.id ?? + event.ResourceProperties.Update?.physicalResourceId?.id ?? + event.ResourceProperties.Delete?.physicalResourceId?.id ?? event.LogicalResourceId; break; case 'Update': case 'Delete': - physicalResourceId = event.ResourceProperties[event.RequestType]?.physicalResourceId ?? event.PhysicalResourceId; + physicalResourceId = event.ResourceProperties[event.RequestType]?.physicalResourceId?.id ?? event.PhysicalResourceId; break; } @@ -127,8 +127,8 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent } } - if (call.physicalResourceIdPath) { - physicalResourceId = flatData[call.physicalResourceIdPath]; + if (call.physicalResourceId?.responsePath) { + physicalResourceId = flatData[call.physicalResourceId.responsePath]; } } diff --git a/packages/@aws-cdk/custom-resources/test/aws-custom-resource/aws-custom-resource-provider.test.ts b/packages/@aws-cdk/custom-resources/test/aws-custom-resource/aws-custom-resource-provider.test.ts index 179fc07dbbd14..9fa7fbfd936d6 100644 --- a/packages/@aws-cdk/custom-resources/test/aws-custom-resource/aws-custom-resource-provider.test.ts +++ b/packages/@aws-cdk/custom-resources/test/aws-custom-resource/aws-custom-resource-provider.test.ts @@ -3,7 +3,7 @@ import * as AWS from 'aws-sdk-mock'; import * as fs from 'fs-extra'; import * as nock from 'nock'; import * as sinon from 'sinon'; -import { AwsSdkCall } from '../../lib'; +import { AwsSdkCall, PhysicalResourceId } from '../../lib'; import { flatten, handler } from '../../lib/aws-custom-resource/runtime'; AWS.setSDK(require.resolve('aws-sdk')); @@ -62,7 +62,7 @@ test('create event with physical resource id path', async () => { parameters: { Bucket: 'my-bucket' }, - physicalResourceIdPath: 'Contents.1.ETag' + physicalResourceId: PhysicalResourceId.fromResponse('Contents.1.ETag') } as AwsSdkCall } }; @@ -101,7 +101,7 @@ test('update event with physical resource id', async () => { Message: 'hello', TopicArn: 'topicarn' }, - physicalResourceId: 'topicarn' + physicalResourceId: PhysicalResourceId.of('topicarn') } as AwsSdkCall } }; @@ -133,7 +133,7 @@ test('delete event', async () => { parameters: { Bucket: 'my-bucket' }, - physicalResourceIdPath: 'Contents.1.ETag' + physicalResourceId: PhysicalResourceId.fromResponse('Contents.1.ETag') } as AwsSdkCall } }; @@ -236,7 +236,7 @@ test('catch errors', async () => { parameters: { Bucket: 'my-bucket' }, - physicalResourceId: 'physicalResourceId', + physicalResourceId: PhysicalResourceId.of('physicalResourceId'), catchErrorPattern: 'NoSuchBucket' } as AwsSdkCall } @@ -283,7 +283,7 @@ test('decodes booleans', async () => { }, } }, - physicalResourceId: 'put-item' + physicalResourceId: PhysicalResourceId.of('put-item') } as AwsSdkCall } }; @@ -342,7 +342,7 @@ test('restrict output path', async () => { parameters: { Bucket: 'my-bucket' }, - physicalResourceId: 'id', + physicalResourceId: PhysicalResourceId.of('id'), outputPath: 'Contents.0' } as AwsSdkCall } @@ -379,7 +379,7 @@ test('can specify apiVersion and region', async () => { }, apiVersion: '2010-03-31', region: 'eu-west-1', - physicalResourceId: 'id', + physicalResourceId: PhysicalResourceId.of('id'), } as AwsSdkCall } }; @@ -433,7 +433,7 @@ test('installs the latest SDK', async () => { Message: 'message', TopicArn: 'topic' }, - physicalResourceId: 'id', + physicalResourceId: PhysicalResourceId.of('id'), } as AwsSdkCall } }; diff --git a/packages/@aws-cdk/custom-resources/test/aws-custom-resource/aws-custom-resource.test.ts b/packages/@aws-cdk/custom-resources/test/aws-custom-resource/aws-custom-resource.test.ts index 5527f2923ab33..38fca5e4b5757 100644 --- a/packages/@aws-cdk/custom-resources/test/aws-custom-resource/aws-custom-resource.test.ts +++ b/packages/@aws-cdk/custom-resources/test/aws-custom-resource/aws-custom-resource.test.ts @@ -1,7 +1,7 @@ import '@aws-cdk/assert/jest'; import * as iam from '@aws-cdk/aws-iam'; import * as cdk from '@aws-cdk/core'; -import { AwsCustomResource } from '../../lib'; +import { AwsCustomResource, PhysicalResourceId } from '../../lib'; // tslint:disable:object-literal-key-quotes @@ -19,7 +19,7 @@ test('aws sdk js custom resource with onCreate and onDelete', () => { logGroupName: '/aws/lambda/loggroup', retentionInDays: 90 }, - physicalResourceId: 'loggroup' + physicalResourceId: PhysicalResourceId.of('loggroup') }, onDelete: { service: 'CloudWatchLogs', @@ -39,7 +39,9 @@ test('aws sdk js custom resource with onCreate and onDelete', () => { "logGroupName": "/aws/lambda/loggroup", "retentionInDays": 90 }, - "physicalResourceId": "loggroup" + "physicalResourceId": { + "id": "loggroup" + } }, "Delete": { "service": "CloudWatchLogs", @@ -84,7 +86,7 @@ test('onCreate defaults to onUpdate', () => { Key: 'my-key', Body: 'my-body' }, - physicalResourceIdPath: 'ETag' + physicalResourceId: PhysicalResourceId.fromResponse('ETag') }, }); @@ -98,7 +100,9 @@ test('onCreate defaults to onUpdate', () => { "Key": "my-key", "Body": "my-body" }, - "physicalResourceIdPath": "ETag" + "physicalResourceId": { + "responsePath": "ETag" + } }, "Update": { "service": "s3", @@ -108,7 +112,9 @@ test('onCreate defaults to onUpdate', () => { "Key": "my-key", "Body": "my-body" }, - "physicalResourceIdPath": "ETag" + "physicalResourceId": { + "responsePath": "ETag" + } }, }); }); @@ -127,7 +133,7 @@ test('with custom policyStatements', () => { Key: 'my-key', Body: 'my-body' }, - physicalResourceIdPath: 'ETag' + physicalResourceId: PhysicalResourceId.fromResponse('ETag') }, policyStatements: [ new iam.PolicyStatement({ @@ -169,7 +175,7 @@ test('fails when no physical resource method is specified', () => { retentionInDays: 90 } } - })).toThrow(/`physicalResourceId`.+`physicalResourceIdPath`/); + })).toThrow(/`physicalResourceId`/); }); test('encodes booleans', () => { @@ -188,7 +194,7 @@ test('encodes booleans', () => { falseBoolean: false, falseString: 'false' }, - physicalResourceId: 'id' + physicalResourceId: PhysicalResourceId.of('id') }, }); @@ -203,7 +209,9 @@ test('encodes booleans', () => { "falseBoolean": "FALSE:BOOLEAN", "falseString": "false" }, - "physicalResourceId": "id" + "physicalResourceId": { + "id": "id" + } }, }); }); @@ -217,7 +225,7 @@ test('timeout defaults to 2 minutes', () => { onCreate: { service: 'service', action: 'action', - physicalResourceId: 'id' + physicalResourceId: PhysicalResourceId.of('id') } }); @@ -236,7 +244,7 @@ test('can specify timeout', () => { onCreate: { service: 'service', action: 'action', - physicalResourceId: 'id' + physicalResourceId: PhysicalResourceId.of('id') }, timeout: cdk.Duration.minutes(15) }); @@ -257,7 +265,7 @@ test('implements IGrantable', () => { onCreate: { service: 'service', action: 'action', - physicalResourceId: 'id' + physicalResourceId: PhysicalResourceId.of('id') } }); @@ -298,7 +306,7 @@ test('can use existing role', () => { onCreate: { service: 'service', action: 'action', - physicalResourceId: 'id' + physicalResourceId: PhysicalResourceId.of('id') }, role }); @@ -318,7 +326,7 @@ test('getData', () => { onCreate: { service: 'service', action: 'action', - physicalResourceId: 'id' + physicalResourceId: PhysicalResourceId.of('id') } }); @@ -341,7 +349,7 @@ test('getDataString', () => { onCreate: { service: 'service', action: 'action', - physicalResourceId: 'id' + physicalResourceId: PhysicalResourceId.of('id') } }); @@ -353,7 +361,7 @@ test('getDataString', () => { parameters: { a: awsSdk.getDataString('Data') }, - physicalResourceId: 'id' + physicalResourceId: PhysicalResourceId.of('id') } }); @@ -370,7 +378,9 @@ test('getDataString', () => { ] } }, - physicalResourceId: 'id' + physicalResourceId: { + "id": 'id' + } } }); }); diff --git a/packages/@aws-cdk/custom-resources/test/aws-custom-resource/integ.aws-custom-resource.expected.json b/packages/@aws-cdk/custom-resources/test/aws-custom-resource/integ.aws-custom-resource.expected.json index 3bd2b19270e40..68bebdc9bc22d 100644 --- a/packages/@aws-cdk/custom-resources/test/aws-custom-resource/integ.aws-custom-resource.expected.json +++ b/packages/@aws-cdk/custom-resources/test/aws-custom-resource/integ.aws-custom-resource.expected.json @@ -22,7 +22,9 @@ } }, "physicalResourceId": { - "Ref": "TopicBFC7AF6E" + "id": { + "Ref": "TopicBFC7AF6E" + } } }, "Update": { @@ -35,7 +37,9 @@ } }, "physicalResourceId": { - "Ref": "TopicBFC7AF6E" + "id": { + "Ref": "TopicBFC7AF6E" + } } } }, @@ -109,7 +113,7 @@ "Properties": { "Code": { "S3Bucket": { - "Ref": "AssetParameters66e21edb638c508d42940ac5560288d0e9666e592c8e3a4908b224ebf979d80cS3Bucket66DE4CD6" + "Ref": "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3BucketFC283D1B" }, "S3Key": { "Fn::Join": [ @@ -122,7 +126,7 @@ "Fn::Split": [ "||", { - "Ref": "AssetParameters66e21edb638c508d42940ac5560288d0e9666e592c8e3a4908b224ebf979d80cS3VersionKeyE6A30774" + "Ref": "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3VersionKey7E916B81" } ] } @@ -135,7 +139,7 @@ "Fn::Split": [ "||", { - "Ref": "AssetParameters66e21edb638c508d42940ac5560288d0e9666e592c8e3a4908b224ebf979d80cS3VersionKeyE6A30774" + "Ref": "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3VersionKey7E916B81" } ] } @@ -172,12 +176,16 @@ "Create": { "service": "SNS", "action": "listTopics", - "physicalResourceIdPath": "Topics.0.TopicArn" + "physicalResourceId": { + "responsePath": "Topics.0.TopicArn" + } }, "Update": { "service": "SNS", "action": "listTopics", - "physicalResourceIdPath": "Topics.0.TopicArn" + "physicalResourceId": { + "responsePath": "Topics.0.TopicArn" + } } }, "DependsOn": [ @@ -211,7 +219,9 @@ }, "WithDecryption": "TRUE:BOOLEAN" }, - "physicalResourceIdPath": "Parameter.ARN" + "physicalResourceId": { + "responsePath": "Parameter.ARN" + } }, "Update": { "service": "SSM", @@ -222,7 +232,9 @@ }, "WithDecryption": "TRUE:BOOLEAN" }, - "physicalResourceIdPath": "Parameter.ARN" + "physicalResourceId": { + "responsePath": "Parameter.ARN" + } } }, "UpdateReplacePolicy": "Delete", @@ -230,17 +242,17 @@ } }, "Parameters": { - "AssetParameters66e21edb638c508d42940ac5560288d0e9666e592c8e3a4908b224ebf979d80cS3Bucket66DE4CD6": { + "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3BucketFC283D1B": { "Type": "String", - "Description": "S3 bucket for asset \"66e21edb638c508d42940ac5560288d0e9666e592c8e3a4908b224ebf979d80c\"" + "Description": "S3 bucket for asset \"6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748\"" }, - "AssetParameters66e21edb638c508d42940ac5560288d0e9666e592c8e3a4908b224ebf979d80cS3VersionKeyE6A30774": { + "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3VersionKey7E916B81": { "Type": "String", - "Description": "S3 key for asset version \"66e21edb638c508d42940ac5560288d0e9666e592c8e3a4908b224ebf979d80c\"" + "Description": "S3 key for asset version \"6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748\"" }, - "AssetParameters66e21edb638c508d42940ac5560288d0e9666e592c8e3a4908b224ebf979d80cArtifactHash89BF1AD1": { + "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748ArtifactHashB96EE827": { "Type": "String", - "Description": "Artifact hash for asset \"66e21edb638c508d42940ac5560288d0e9666e592c8e3a4908b224ebf979d80c\"" + "Description": "Artifact hash for asset \"6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748\"" } }, "Outputs": { diff --git a/packages/@aws-cdk/custom-resources/test/aws-custom-resource/integ.aws-custom-resource.ts b/packages/@aws-cdk/custom-resources/test/aws-custom-resource/integ.aws-custom-resource.ts index f736237e5bce1..3fd4f1b21f10a 100644 --- a/packages/@aws-cdk/custom-resources/test/aws-custom-resource/integ.aws-custom-resource.ts +++ b/packages/@aws-cdk/custom-resources/test/aws-custom-resource/integ.aws-custom-resource.ts @@ -2,7 +2,7 @@ import * as sns from '@aws-cdk/aws-sns'; import * as ssm from '@aws-cdk/aws-ssm'; import * as cdk from '@aws-cdk/core'; -import { AwsCustomResource } from '../../lib'; +import { AwsCustomResource, PhysicalResourceId } from '../../lib'; const app = new cdk.App(); @@ -19,7 +19,7 @@ const snsPublish = new AwsCustomResource(stack, 'Publish', { Message: 'hello', TopicArn: topic.topicArn }, - physicalResourceId: topic.topicArn, + physicalResourceId: PhysicalResourceId.of(topic.topicArn), } }); @@ -27,7 +27,7 @@ const listTopics = new AwsCustomResource(stack, 'ListTopics', { onUpdate: { service: 'SNS', action: 'listTopics', - physicalResourceIdPath: 'Topics.0.TopicArn' + physicalResourceId: PhysicalResourceId.fromResponse('Topics.0.TopicArn') } }); listTopics.node.addDependency(topic); @@ -44,7 +44,7 @@ const getParameter = new AwsCustomResource(stack, 'GetParameter', { Name: ssmParameter.parameterName, WithDecryption: true }, - physicalResourceIdPath: 'Parameter.ARN' + physicalResourceId: PhysicalResourceId.fromResponse('Parameter.ARN') } });