From 007774a0ffa563f6af6ca01a476f941c49346503 Mon Sep 17 00:00:00 2001 From: epolon Date: Mon, 2 Mar 2020 17:11:16 +0200 Subject: [PATCH 01/10] rename catchErrorPattern to ignoreErrorCodesMatching --- .../lib/aws-custom-resource/aws-custom-resource.ts | 2 +- .../custom-resources/lib/aws-custom-resource/runtime/index.ts | 2 +- .../aws-custom-resource/aws-custom-resource-provider.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 4ebcf8e9e1bbf..339c551f34804 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 @@ -83,7 +83,7 @@ export interface AwsSdkCall { * * @default - do not catch errors */ - readonly catchErrorPattern?: string; + readonly ignoreErrorCodesMatching?: string; /** * API version to use for the service 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 9507a8f9388b1..36f5cb3ba3819 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 @@ -122,7 +122,7 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent ? filterKeys(flatData, k => k.startsWith(call.outputPath!)) : flatData; } catch (e) { - if (!call.catchErrorPattern || !new RegExp(call.catchErrorPattern).test(e.code)) { + if (!call.ignoreErrorCodesMatching || !new RegExp(call.ignoreErrorCodesMatching).test(e.code)) { throw e; } } 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 9fa7fbfd936d6..d492dc7a8f26e 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 @@ -237,7 +237,7 @@ test('catch errors', async () => { Bucket: 'my-bucket' }, physicalResourceId: PhysicalResourceId.of('physicalResourceId'), - catchErrorPattern: 'NoSuchBucket' + ignoreErrorCodesMatching: 'NoSuchBucket' } as AwsSdkCall } }; From 737617eb7e964f9f587a9168bbbbe6ff7a64d6c3 Mon Sep 17 00:00:00 2001 From: epolon Date: Mon, 2 Mar 2020 17:29:21 +0200 Subject: [PATCH 02/10] added validation that PhysicalResourceId.fromResponse is not used along with ignoreErrorCodesMatching --- .../aws-custom-resource.ts | 6 +++ .../aws-custom-resource.test.ts | 44 +++++++++++++++++++ 2 files changed, 50 insertions(+) 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 339c551f34804..79f0c65379ca7 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 @@ -209,6 +209,12 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { } } + for (const call of [props.onCreate, props.onUpdate, props.onDelete]) { + if (call && call.ignoreErrorCodesMatching && call.physicalResourceId?.responsePath) { + throw new Error('`PhysicalResourceId.fromResponse` cannot be used along with `ignoreErrorCodesMatching`.'); + } + } + const provider = new lambda.SingletonFunction(this, 'Provider', { code: lambda.Code.fromAsset(path.join(__dirname, 'runtime')), runtime: lambda.Runtime.NODEJS_12_X, 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 38fca5e4b5757..6cfeb1f78b7a1 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 @@ -342,6 +342,50 @@ test('getData', () => { }); }); +test('fail when `PhysicalResourceId.fromResponse` is used with `ignoreErrorCodesMatching', () => { + + const stack = new cdk.Stack(); + expect(() => new AwsCustomResource(stack, 'AwsSdkOnUpdate', { + onUpdate: { + service: 'CloudWatchLogs', + action: 'putRetentionPolicy', + parameters: { + logGroupName: '/aws/lambda/loggroup', + retentionInDays: 90 + }, + ignoreErrorCodesMatching: ".*", + physicalResourceId: PhysicalResourceId.fromResponse("Response") + } + })).toThrow(/`PhysicalResourceId.fromResponse`.+`ignoreErrorCodesMatching`/); + + expect(() => new AwsCustomResource(stack, 'AwsSdkOnCreate', { + onCreate: { + service: 'CloudWatchLogs', + action: 'putRetentionPolicy', + parameters: { + logGroupName: '/aws/lambda/loggroup', + retentionInDays: 90 + }, + ignoreErrorCodesMatching: ".*", + physicalResourceId: PhysicalResourceId.fromResponse("Response") + } + })).toThrow(/`PhysicalResourceId.fromResponse`.+`ignoreErrorCodesMatching`/); + + expect(() => new AwsCustomResource(stack, 'AwsSdkOnDelete', { + onDelete: { + service: 'CloudWatchLogs', + action: 'putRetentionPolicy', + parameters: { + logGroupName: '/aws/lambda/loggroup', + retentionInDays: 90 + }, + ignoreErrorCodesMatching: ".*", + physicalResourceId: PhysicalResourceId.fromResponse("Response") + } + })).toThrow(/`PhysicalResourceId.fromResponse`.+`ignoreErrorCodesMatching`/); + +}); + test('getDataString', () => { // GIVEN const stack = new cdk.Stack(); From 71d45a5ab8d30524557b8fb12e0bf42bae32e088 Mon Sep 17 00:00:00 2001 From: epolon Date: Tue, 3 Mar 2020 00:17:19 +0200 Subject: [PATCH 03/10] added section in readme about property --- packages/@aws-cdk/custom-resources/README.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/@aws-cdk/custom-resources/README.md b/packages/@aws-cdk/custom-resources/README.md index d8ba5bb9205e7..688667957e5cf 100644 --- a/packages/@aws-cdk/custom-resources/README.md +++ b/packages/@aws-cdk/custom-resources/README.md @@ -369,6 +369,15 @@ const awsCustom2 = new AwsCustomResource(this, 'API2', { }) ``` +### Error Handling + +Every error produced by the API call is treated as is and will cause a "FAILED" response to be submitted to CloudFormation. You can ignore some errors by specifying the `ignoreErrorCodesMatching` property, which accepts a regular expression that is tested against the `code` property of the response. If matched, a "SUCCESS" response is submitted. Note that in such a case, the call response data, and the `Data` key submitted to CloudFormation, will both be an empty JSON object. This presents us with some limitations: + +- `PhysicalResourceId.fromResponse` - Since the call response data is empty, we cannot use it to extract the physical id. +- `getData` and `getDataString` - Since the `Data` key is empty, the resource will not have any attributes, and therefore, invoking these functions will result in an error. + +In both the cases, you will get a synth time error if you attempt to use it in conjunction with `ignoreErrorCodesMatching`. + ### Examples #### Verify a domain with SES From e57f5c5723b4f4b6b77ab2732863b1dd0d94eecb Mon Sep 17 00:00:00 2001 From: epolon Date: Tue, 3 Mar 2020 00:18:05 +0200 Subject: [PATCH 04/10] added validation that is not used with getData and getDataString --- .../aws-custom-resource.ts | 23 +++++++++ .../aws-custom-resource.test.ts | 48 +++++++++++++++++++ 2 files changed, 71 insertions(+) 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 79f0c65379ca7..6df3aacb33744 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 @@ -193,6 +193,7 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { public readonly grantPrincipal: iam.IPrincipal; private readonly customResource: CustomResource; + private readonly props: AwsCustomResourceProps; // 'props' cannot be optional, even though all its properties are optional. // this is because at least one sdk call must be provided. @@ -215,6 +216,8 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { } } + this.props = props; + const provider = new lambda.SingletonFunction(this, 'Provider', { code: lambda.Code.fromAsset(path.join(__dirname, 'runtime')), runtime: lambda.Runtime.NODEJS_12_X, @@ -261,9 +264,14 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { * Use `Token.asXxx` to encode the returned `Reference` as a specific type or * use the convenience `getDataString` for string attributes. * + * Note that you cannot use this method if `ignoreErrorCodesMatching` + * is configured for any of the SDK calls. This is because in such a case, + * the response data might not exist, and will cause a CloudFormation deploy time error. + * * @param dataPath the path to the data */ public getData(dataPath: string) { + this.breakIgnoreErrorsCircuit("getData"); return this.customResource.getAtt(dataPath); } @@ -272,11 +280,26 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { * * Example for S3 / listBucket : 'Buckets.0.Name' * + * Note that you cannot use this method if `ignoreErrorCodesMatching` + * is configured for any of the SDK calls. This is because in such a case, + * the response data might not exist, and will cause a CloudFormation deploy time error. + * * @param dataPath the path to the data */ public getDataString(dataPath: string): string { + this.breakIgnoreErrorsCircuit("getDataString"); return this.customResource.getAttString(dataPath); } + + private breakIgnoreErrorsCircuit(caller: string) { + + for (const call of [this.props.onCreate, this.props.onUpdate, this.props.onDelete]) { + if (call && call.ignoreErrorCodesMatching) { + throw new Error(`\`${caller}\`` + ' cannot be called along with `ignoreErrorCodesMatching`.'); + } + } + + } } /** 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 6cfeb1f78b7a1..500f626b45e41 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 @@ -342,6 +342,54 @@ test('getData', () => { }); }); +test('fails when getData is used with `ignoreErrorCodesMatching`', () => { + + const stack = new cdk.Stack(); + expect(() => { + + const resource = new AwsCustomResource(stack, 'AwsSdk', { + onUpdate: { + service: 'CloudWatchLogs', + action: 'putRetentionPolicy', + parameters: { + logGroupName: '/aws/lambda/loggroup', + retentionInDays: 90 + }, + ignoreErrorCodesMatching: ".*", + physicalResourceId: PhysicalResourceId.of("Id") + } + }); + + resource.getData("ShouldFail"); + + }).toThrow(/`getData`.+`ignoreErrorCodesMatching`/); + +}); + +test('fails when getDataString is used with `ignoreErrorCodesMatching`', () => { + + const stack = new cdk.Stack(); + expect(() => { + + const resource = new AwsCustomResource(stack, 'AwsSdk', { + onUpdate: { + service: 'CloudWatchLogs', + action: 'putRetentionPolicy', + parameters: { + logGroupName: '/aws/lambda/loggroup', + retentionInDays: 90 + }, + ignoreErrorCodesMatching: ".*", + physicalResourceId: PhysicalResourceId.of("Id") + } + }); + + resource.getDataString("ShouldFail"); + + }).toThrow(/`getDataString`.+`ignoreErrorCodesMatching`/); + +}); + test('fail when `PhysicalResourceId.fromResponse` is used with `ignoreErrorCodesMatching', () => { const stack = new cdk.Stack(); From 90c2f4ef33c20a5304e93ed7e1a175caf0e9aa91 Mon Sep 17 00:00:00 2001 From: epolon Date: Tue, 3 Mar 2020 11:32:56 +0200 Subject: [PATCH 05/10] amend expected template for integ test --- .../integ.aws-custom-resource.expected.json | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) 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 68bebdc9bc22d..6c3436e07d7fc 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 @@ -113,7 +113,7 @@ "Properties": { "Code": { "S3Bucket": { - "Ref": "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3BucketFC283D1B" + "Ref": "AssetParameters4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313fS3Bucket71E9DB75" }, "S3Key": { "Fn::Join": [ @@ -126,7 +126,7 @@ "Fn::Split": [ "||", { - "Ref": "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3VersionKey7E916B81" + "Ref": "AssetParameters4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313fS3VersionKeyADF76D6F" } ] } @@ -139,7 +139,7 @@ "Fn::Split": [ "||", { - "Ref": "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3VersionKey7E916B81" + "Ref": "AssetParameters4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313fS3VersionKeyADF76D6F" } ] } @@ -242,17 +242,17 @@ } }, "Parameters": { - "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3BucketFC283D1B": { + "AssetParameters4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313fS3Bucket71E9DB75": { "Type": "String", - "Description": "S3 bucket for asset \"6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748\"" + "Description": "S3 bucket for asset \"4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313f\"" }, - "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3VersionKey7E916B81": { + "AssetParameters4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313fS3VersionKeyADF76D6F": { "Type": "String", - "Description": "S3 key for asset version \"6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748\"" + "Description": "S3 key for asset version \"4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313f\"" }, - "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748ArtifactHashB96EE827": { + "AssetParameters4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313fArtifactHash7FE12709": { "Type": "String", - "Description": "Artifact hash for asset \"6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748\"" + "Description": "Artifact hash for asset \"4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313f\"" } }, "Outputs": { From 656725b533d0bd1e463d70ccc169fc43eef7d497 Mon Sep 17 00:00:00 2001 From: epolon Date: Tue, 3 Mar 2020 15:20:26 +0200 Subject: [PATCH 06/10] code review comments --- packages/@aws-cdk/custom-resources/README.md | 8 ++++-- .../aws-custom-resource.ts | 28 ++++++++++--------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/packages/@aws-cdk/custom-resources/README.md b/packages/@aws-cdk/custom-resources/README.md index 688667957e5cf..ce795d791ffbd 100644 --- a/packages/@aws-cdk/custom-resources/README.md +++ b/packages/@aws-cdk/custom-resources/README.md @@ -371,9 +371,13 @@ const awsCustom2 = new AwsCustomResource(this, 'API2', { ### Error Handling -Every error produced by the API call is treated as is and will cause a "FAILED" response to be submitted to CloudFormation. You can ignore some errors by specifying the `ignoreErrorCodesMatching` property, which accepts a regular expression that is tested against the `code` property of the response. If matched, a "SUCCESS" response is submitted. Note that in such a case, the call response data, and the `Data` key submitted to CloudFormation, will both be an empty JSON object. This presents us with some limitations: +Every error produced by the API call is treated as is and will cause a "FAILED" response to be submitted to CloudFormation. +You can ignore some errors by specifying the `ignoreErrorCodesMatching` property, which accepts a regular expression that is +tested against the `code` property of the response. If matched, a "SUCCESS" response is submitted. +Note that in such a case, the call response data and the `Data` key submitted to CloudFormation would both be an empty JSON object. +Since a successful resource provisioning might or might not produce outputs, this presents us with some limitations: -- `PhysicalResourceId.fromResponse` - Since the call response data is empty, we cannot use it to extract the physical id. +- `PhysicalResourceId.fromResponse` - Since the call response data might be empty, we cannot use it to extract the physical id. - `getData` and `getDataString` - Since the `Data` key is empty, the resource will not have any attributes, and therefore, invoking these functions will result in an error. In both the cases, you will get a synth time error if you attempt to use it in conjunction with `ignoreErrorCodesMatching`. 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 6df3aacb33744..f9d4d2bce0108 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 @@ -190,6 +190,17 @@ export interface AwsCustomResourceProps { * */ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { + + private static breakIgnoreErrorsCircuit(sdkCalls: Array, caller: string) { + + for (const call of sdkCalls) { + if (call && call.ignoreErrorCodesMatching) { + throw new Error(`\`${caller}\`` + ' cannot be called along with `ignoreErrorCodesMatching`.'); + } + } + + } + public readonly grantPrincipal: iam.IPrincipal; private readonly customResource: CustomResource; @@ -211,8 +222,8 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { } for (const call of [props.onCreate, props.onUpdate, props.onDelete]) { - if (call && call.ignoreErrorCodesMatching && call.physicalResourceId?.responsePath) { - throw new Error('`PhysicalResourceId.fromResponse` cannot be used along with `ignoreErrorCodesMatching`.'); + if (call && call.physicalResourceId?.responsePath) { + AwsCustomResource.breakIgnoreErrorsCircuit([call], "PhysicalResourceId.fromResponse"); } } @@ -271,7 +282,7 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { * @param dataPath the path to the data */ public getData(dataPath: string) { - this.breakIgnoreErrorsCircuit("getData"); + AwsCustomResource.breakIgnoreErrorsCircuit([this.props.onCreate, this.props.onUpdate], "getData"); return this.customResource.getAtt(dataPath); } @@ -287,19 +298,10 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { * @param dataPath the path to the data */ public getDataString(dataPath: string): string { - this.breakIgnoreErrorsCircuit("getDataString"); + AwsCustomResource.breakIgnoreErrorsCircuit([this.props.onCreate, this.props.onUpdate], "getDataString"); return this.customResource.getAttString(dataPath); } - private breakIgnoreErrorsCircuit(caller: string) { - - for (const call of [this.props.onCreate, this.props.onUpdate, this.props.onDelete]) { - if (call && call.ignoreErrorCodesMatching) { - throw new Error(`\`${caller}\`` + ' cannot be called along with `ignoreErrorCodesMatching`.'); - } - } - - } } /** From 555d2abb1881b92cdff03e5751b8890ef227f7a4 Mon Sep 17 00:00:00 2001 From: epolon Date: Wed, 4 Mar 2020 11:25:12 +0200 Subject: [PATCH 07/10] use conditional chaining instead of null check --- .../lib/aws-custom-resource/aws-custom-resource.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 f9d4d2bce0108..12e35cb4b7364 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 @@ -194,7 +194,7 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { private static breakIgnoreErrorsCircuit(sdkCalls: Array, caller: string) { for (const call of sdkCalls) { - if (call && call.ignoreErrorCodesMatching) { + if (call?.ignoreErrorCodesMatching) { throw new Error(`\`${caller}\`` + ' cannot be called along with `ignoreErrorCodesMatching`.'); } } @@ -216,13 +216,13 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { } for (const call of [props.onCreate, props.onUpdate]) { - if (call && !call.physicalResourceId) { + if (!call?.physicalResourceId) { throw new Error('`physicalResourceId` must be specified for onCreate and onUpdate calls.'); } } for (const call of [props.onCreate, props.onUpdate, props.onDelete]) { - if (call && call.physicalResourceId?.responsePath) { + if (call?.physicalResourceId?.responsePath) { AwsCustomResource.breakIgnoreErrorsCircuit([call], "PhysicalResourceId.fromResponse"); } } From 1e1ac8308a3d2421219b435be830cca0ee4cfb46 Mon Sep 17 00:00:00 2001 From: epolon Date: Wed, 4 Mar 2020 11:25:42 +0200 Subject: [PATCH 08/10] extract resource creation outside of 'expect' to make sure the right invocation fails --- .../aws-custom-resource.test.ts | 58 +++++++++---------- 1 file changed, 26 insertions(+), 32 deletions(-) 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 500f626b45e41..755c0ae22e5b8 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 @@ -345,48 +345,42 @@ test('getData', () => { test('fails when getData is used with `ignoreErrorCodesMatching`', () => { const stack = new cdk.Stack(); - expect(() => { - const resource = new AwsCustomResource(stack, 'AwsSdk', { - onUpdate: { - service: 'CloudWatchLogs', - action: 'putRetentionPolicy', - parameters: { - logGroupName: '/aws/lambda/loggroup', - retentionInDays: 90 - }, - ignoreErrorCodesMatching: ".*", - physicalResourceId: PhysicalResourceId.of("Id") - } - }); - - resource.getData("ShouldFail"); + const resource = new AwsCustomResource(stack, 'AwsSdk', { + onUpdate: { + service: 'CloudWatchLogs', + action: 'putRetentionPolicy', + parameters: { + logGroupName: '/aws/lambda/loggroup', + retentionInDays: 90 + }, + ignoreErrorCodesMatching: ".*", + physicalResourceId: PhysicalResourceId.of("Id") + } + }); - }).toThrow(/`getData`.+`ignoreErrorCodesMatching`/); + expect(() => resource.getData("ShouldFail")).toThrow(/`getData`.+`ignoreErrorCodesMatching`/); }); test('fails when getDataString is used with `ignoreErrorCodesMatching`', () => { const stack = new cdk.Stack(); - expect(() => { - const resource = new AwsCustomResource(stack, 'AwsSdk', { - onUpdate: { - service: 'CloudWatchLogs', - action: 'putRetentionPolicy', - parameters: { - logGroupName: '/aws/lambda/loggroup', - retentionInDays: 90 - }, - ignoreErrorCodesMatching: ".*", - physicalResourceId: PhysicalResourceId.of("Id") - } - }); - - resource.getDataString("ShouldFail"); + const resource = new AwsCustomResource(stack, 'AwsSdk', { + onUpdate: { + service: 'CloudWatchLogs', + action: 'putRetentionPolicy', + parameters: { + logGroupName: '/aws/lambda/loggroup', + retentionInDays: 90 + }, + ignoreErrorCodesMatching: ".*", + physicalResourceId: PhysicalResourceId.of("Id") + } + }); - }).toThrow(/`getDataString`.+`ignoreErrorCodesMatching`/); + expect(() => resource.getDataString("ShouldFail")).toThrow(/`getDataString`.+`ignoreErrorCodesMatching`/); }); From 61051b11e35e35656674e13bdb15fe04d98d4eb8 Mon Sep 17 00:00:00 2001 From: epolon Date: Wed, 4 Mar 2020 11:31:40 +0200 Subject: [PATCH 09/10] oops - revert wrong condition --- .../lib/aws-custom-resource/aws-custom-resource.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 12e35cb4b7364..c9c03f79770d9 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 @@ -216,7 +216,7 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { } for (const call of [props.onCreate, props.onUpdate]) { - if (!call?.physicalResourceId) { + if (call && !call.physicalResourceId) { throw new Error('`physicalResourceId` must be specified for onCreate and onUpdate calls.'); } } From 12212f8869c24ba075afb1420124216b221d3548 Mon Sep 17 00:00:00 2001 From: epolon Date: Wed, 4 Mar 2020 13:13:16 +0200 Subject: [PATCH 10/10] fix test compilation due to API change merged from master --- .../aws-custom-resource.test.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) 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 43f3b9a9cf63d..dcfbfa15d194c 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 @@ -367,7 +367,8 @@ test('fails when getData is used with `ignoreErrorCodesMatching`', () => { }, ignoreErrorCodesMatching: ".*", physicalResourceId: PhysicalResourceId.of("Id") - } + }, + policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE}), }); expect(() => resource.getData("ShouldFail")).toThrow(/`getData`.+`ignoreErrorCodesMatching`/); @@ -387,8 +388,9 @@ test('fails when getDataString is used with `ignoreErrorCodesMatching`', () => { retentionInDays: 90 }, ignoreErrorCodesMatching: ".*", - physicalResourceId: PhysicalResourceId.of("Id") - } + physicalResourceId: PhysicalResourceId.of("Id"), + }, + policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE}), }); expect(() => resource.getDataString("ShouldFail")).toThrow(/`getDataString`.+`ignoreErrorCodesMatching`/); @@ -408,7 +410,8 @@ test('fail when `PhysicalResourceId.fromResponse` is used with `ignoreErrorCodes }, ignoreErrorCodesMatching: ".*", physicalResourceId: PhysicalResourceId.fromResponse("Response") - } + }, + policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE}), })).toThrow(/`PhysicalResourceId.fromResponse`.+`ignoreErrorCodesMatching`/); expect(() => new AwsCustomResource(stack, 'AwsSdkOnCreate', { @@ -421,7 +424,8 @@ test('fail when `PhysicalResourceId.fromResponse` is used with `ignoreErrorCodes }, ignoreErrorCodesMatching: ".*", physicalResourceId: PhysicalResourceId.fromResponse("Response") - } + }, + policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE}), })).toThrow(/`PhysicalResourceId.fromResponse`.+`ignoreErrorCodesMatching`/); expect(() => new AwsCustomResource(stack, 'AwsSdkOnDelete', { @@ -434,7 +438,8 @@ test('fail when `PhysicalResourceId.fromResponse` is used with `ignoreErrorCodes }, ignoreErrorCodesMatching: ".*", physicalResourceId: PhysicalResourceId.fromResponse("Response") - } + }, + policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE}), })).toThrow(/`PhysicalResourceId.fromResponse`.+`ignoreErrorCodesMatching`/); });