Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(custom-resources): rename catchErrorPattern to ignoreErrorCodesMatching #6553

Merged
merged 11 commits into from
Mar 4, 2020
Merged
13 changes: 13 additions & 0 deletions packages/@aws-cdk/custom-resources/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,19 @@ 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 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 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`.

### Examples

#### Verify a domain with SES
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -190,9 +190,21 @@ export interface AwsCustomResourceProps {
*
*/
export class AwsCustomResource extends cdk.Construct implements iam.IGrantable {

private static breakIgnoreErrorsCircuit(sdkCalls: Array<AwsSdkCall | undefined>, caller: string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personal taste... why not a non exported function outside the class for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just prefer starting with the minimal visibility necessary. The function is used to validate specific configuration that can (currently) only arise in the AwsCustomResource class. I'd like additional usages of it to be very intentional and not just treat it as a sort of utility function.


for (const call of sdkCalls) {
if (call && call.ignoreErrorCodesMatching) {
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
throw new Error(`\`${caller}\`` + ' cannot be called along with `ignoreErrorCodesMatching`.');
}
}

}

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.
Expand All @@ -209,6 +221,14 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable {
}
}

for (const call of [props.onCreate, props.onUpdate, props.onDelete]) {
if (call && call.physicalResourceId?.responsePath) {
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
AwsCustomResource.breakIgnoreErrorsCircuit([call], "PhysicalResourceId.fromResponse");
}
}

this.props = props;
iliapolo marked this conversation as resolved.
Show resolved Hide resolved

const provider = new lambda.SingletonFunction(this, 'Provider', {
code: lambda.Code.fromAsset(path.join(__dirname, 'runtime')),
runtime: lambda.Runtime.NODEJS_12_X,
Expand Down Expand Up @@ -255,9 +275,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) {
AwsCustomResource.breakIgnoreErrorsCircuit([this.props.onCreate, this.props.onUpdate], "getData");
return this.customResource.getAtt(dataPath);
}

Expand All @@ -266,11 +291,17 @@ 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 {
AwsCustomResource.breakIgnoreErrorsCircuit([this.props.onCreate, this.props.onUpdate], "getDataString");
return this.customResource.getAttString(dataPath);
}

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ test('catch errors', async () => {
Bucket: 'my-bucket'
},
physicalResourceId: PhysicalResourceId.of('physicalResourceId'),
catchErrorPattern: 'NoSuchBucket'
ignoreErrorCodesMatching: 'NoSuchBucket'
} as AwsSdkCall
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,98 @@ test('getData', () => {
});
});

test('fails when getData is used with `ignoreErrorCodesMatching`', () => {

const stack = new cdk.Stack();
expect(() => {

const resource = new AwsCustomResource(stack, 'AwsSdk', {
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
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', {
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
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();
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
"Properties": {
"Code": {
"S3Bucket": {
"Ref": "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3BucketFC283D1B"
"Ref": "AssetParameters4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313fS3Bucket71E9DB75"
},
"S3Key": {
"Fn::Join": [
Expand All @@ -126,7 +126,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3VersionKey7E916B81"
"Ref": "AssetParameters4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313fS3VersionKeyADF76D6F"
}
]
}
Expand All @@ -139,7 +139,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParameters6bf0eac2bfd1c5dcd41b3cc53f24814f9dba9cce0cb7fe2e34d0ded661481748S3VersionKey7E916B81"
"Ref": "AssetParameters4317eb4c8ccce73f91d75018d965c243c175c64101884e0f202d4a280e23313fS3VersionKeyADF76D6F"
}
]
}
Expand Down Expand Up @@ -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": {
Expand Down