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.

Sign up for GitHub

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

Jump to bottom

feat(custom-resources): AwsCustomResource copy physicalResourceId from request when omit it in onUpdate #24194

Merged
merged 7 commits into from Mar 8, 2023
Merged

feat(custom-resources): AwsCustomResource copy physicalResourceId from request when omit it in onUpdate #24194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aecf8c6
feat(custom-resources): AwsCustomResource passthrough physicalResourceId
konokenj Jan 26, 2023
4ed9fda
update readme
konokenj Feb 16, 2023
3f79c30
add unit tests
konokenj Mar 2, 2023
c198aeb
Apply suggestions from code review
konokenj Mar 6, 2023
1d78eb6
revert style fix
konokenj Mar 6, 2023
1880369
use single quotes in error message
konokenj Mar 6, 2023
7627dad
Merge branch 'main' into konokenj/passthrough-physical-resource-id
mergify[bot] Mar 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/@aws-cdk/custom-resources/README.md
View file
Open in desktop
Expand Up @@ -502,6 +502,10 @@ const awsCustom = new cr.AwsCustomResource(this, 'aws-custom', {
})
```

You can omit `PhysicalResourceId` property in `onUpdate` to passthrough the value in `onCreate`. This behavior is useful when using Update APIs that response with an empty body.

> AwsCustomResource.getResponseField() and .getResponseFieldReference() will not work if the Create and Update APIs don't consistently return the same fields.

### Handling Custom Resource Errors

Every error produced by the API call is treated as is and will cause a "FAILED" response to be submitted to CloudFormation.
Expand Down
13 changes: 8 additions & 5 deletions packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts
View file
Open in desktop
Expand Up @@ -86,7 +86,8 @@ export interface AwsSdkCall {

/**
* The physical resource id of the custom resource for this call.
* Mandatory for onCreate or onUpdate calls.
* Mandatory for onCreate call.
* In onUpdate, you can omit this to passthrough it from request.
*
* @default - no physical resource id
*/
Expand Down Expand Up @@ -384,10 +385,12 @@ export class AwsCustomResource extends Construct implements iam.IGrantable {
throw new Error('At least one of `policy` or `role` (or both) must be specified.');
}

for (const call of [props.onCreate, props.onUpdate]) {
if (call && !call.physicalResourceId) {
throw new Error('`physicalResourceId` must be specified for onCreate and onUpdate calls.');
}
if (props.onCreate && !props.onCreate.physicalResourceId) {
throw new Error("'physicalResourceId' must be specified for 'onCreate' call.");
}

if (!props.onCreate && props.onUpdate && !props.onUpdate.physicalResourceId) {
throw new Error("'physicalResourceId' must be specified for 'onUpdate' call when 'onCreate' is omitted.");
}

for (const call of [props.onCreate, props.onUpdate, props.onDelete]) {
Expand Down
316 changes: 304 additions & 12 deletions packages/@aws-cdk/custom-resources/test/aws-custom-resource/aws-custom-resource.test.ts
View file
Open in desktop
Expand Up @@ -171,20 +171,312 @@ test('fails when no calls are specified', () => {
})).toThrow(/`onCreate`.+`onUpdate`.+`onDelete`/);
});

test('fails when no physical resource method is specified', () => {
const stack = new cdk.Stack();
// test patterns for physicalResourceId
// | # | onCreate.physicalResourceId | onUpdate.physicalResourceId | Error thrown? |
// |---|-----------------------------------|----------------------------------|---------------|
// | 1 | ANY_VALUE | ANY_VALUE | no |
// | 2 | ANY_VALUE | undefined | no |
// | 3 | undefined | ANY_VALLUE | yes |
// | 4 | undefined | undefined | yes |
// | 5 | ANY_VALUE | undefined (*omit whole onUpdate) | no |
// | 6 | undefined | undefined (*omit whole onUpdate) | yes |
// | 7 | ANY_VALUE (*copied from onUpdate) | ANY_VALUE | no |
// | 8 | undefined (*copied from onUpdate) | undefined | yes |
describe('physicalResourceId patterns', () => {
// physicalResourceId pattern #1
test('physicalResourceId is specified both in onCreate and onUpdate then success', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new AwsCustomResource(stack, 'AwsSdk', {
resourceType: 'Custom::AthenaNotebook',
onCreate: {
service: 'Athena',
action: 'createNotebook',
physicalResourceId: PhysicalResourceId.of('id'),
parameters: {
WorkGroup: 'WorkGroupA',
Name: 'Notebook1',
},
},
onUpdate: {
service: 'Athena',
action: 'updateNotebookMetadata',
physicalResourceId: PhysicalResourceId.of('id'),
parameters: {
Name: 'Notebook1',
NotebookId: new PhysicalResourceIdReference(),
},
},
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
});

expect(() => new AwsCustomResource(stack, 'AwsSdk', {
onUpdate: {
service: 'CloudWatchLogs',
action: 'putRetentionPolicy',
parameters: {
logGroupName: '/aws/lambda/loggroup',
retentionInDays: 90,
// THEN
Template.fromStack(stack).hasResourceProperties('Custom::AthenaNotebook', {
Create: JSON.stringify({
service: 'Athena',
action: 'createNotebook',
physicalResourceId: {
id: 'id',
},
parameters: {
WorkGroup: 'WorkGroupA',
Name: 'Notebook1',
},
}),
Update: JSON.stringify({
service: 'Athena',
action: 'updateNotebookMetadata',
physicalResourceId: {
id: 'id',
},
parameters: {
Name: 'Notebook1',
NotebookId: 'PHYSICAL:RESOURCEID:',
},
}),
});
});

// physicalResourceId pattern #2
test('physicalResourceId is specified in onCreate, is not in onUpdate then absent', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new AwsCustomResource(stack, 'AwsSdk', {
resourceType: 'Custom::AthenaNotebook',
onCreate: {
service: 'Athena',
action: 'createNotebook',
physicalResourceId: PhysicalResourceId.fromResponse('NotebookId'),
parameters: {
WorkGroup: 'WorkGroupA',
Name: 'Notebook1',
},
},
},
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
})).toThrow(/`physicalResourceId`/);
onUpdate: {
service: 'Athena',
action: 'updateNotebookMetadata',
parameters: {
Name: 'Notebook1',
NotebookId: new PhysicalResourceIdReference(),
},
},
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
});

// THEN
Template.fromStack(stack).hasResourceProperties('Custom::AthenaNotebook', {
Create: JSON.stringify({
service: 'Athena',
action: 'createNotebook',
physicalResourceId: {
responsePath: 'NotebookId',
},
parameters: {
WorkGroup: 'WorkGroupA',
Name: 'Notebook1',
},
}),
Update: JSON.stringify({
service: 'Athena',
action: 'updateNotebookMetadata',
parameters: {
Name: 'Notebook1',
NotebookId: 'PHYSICAL:RESOURCEID:',
},
}),
});
});

// physicalResourceId pattern #3
test('physicalResourceId is not specified in onCreate but onUpdate then fail', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
expect(() => {
new AwsCustomResource(stack, 'AwsSdk', {
resourceType: 'Custom::AthenaNotebook',
onCreate: {
service: 'Athena',
action: 'createNotebook',
parameters: {
WorkGroup: 'WorkGroupA',
Name: 'Notebook1',
},
},
onUpdate: {
service: 'Athena',
action: 'updateNotebookMetadata',
physicalResourceId: PhysicalResourceId.of('id'),
parameters: {
Name: 'Notebook1',
NotebookId: new PhysicalResourceIdReference(),
},
},
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
});
}).toThrow(/'physicalResourceId' must be specified for 'onCreate' call./);
});

// physicalResourceId pattern #4
test('physicalResourceId is not specified both in onCreate and onUpdate then fail', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
expect(() => {
new AwsCustomResource(stack, 'AwsSdk', {
resourceType: 'Custom::AthenaNotebook',
onCreate: {
service: 'Athena',
action: 'createNotebook',
parameters: {
WorkGroup: 'WorkGroupA',
Name: 'Notebook1',
},
},
onUpdate: {
service: 'Athena',
action: 'updateNotebookMetadata',
parameters: {
Name: 'Notebook1',
NotebookId: new PhysicalResourceIdReference(),
},
},
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
});
}).toThrow(/'physicalResourceId' must be specified for 'onCreate' call./);
});

// physicalResourceId pattern #5
test('physicalResourceId is specified in onCreate with empty onUpdate then success', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new AwsCustomResource(stack, 'AwsSdk', {
resourceType: 'Custom::AthenaNotebook',
onCreate: {
service: 'Athena',
action: 'createNotebook',
physicalResourceId: PhysicalResourceId.of('id'),
parameters: {
WorkGroup: 'WorkGroupA',
Name: 'Notebook1',
},
},
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
});

// THEN
Template.fromStack(stack).hasResourceProperties('Custom::AthenaNotebook', {
Create: JSON.stringify({
service: 'Athena',
action: 'createNotebook',
physicalResourceId: {
id: 'id',
},
parameters: {
WorkGroup: 'WorkGroupA',
Name: 'Notebook1',
},
}),
});
});

// physicalResourceId pattern #6
test('physicalResourceId is not specified onCreate with empty onUpdate then fail', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
expect(() => {
new AwsCustomResource(stack, 'AwsSdk', {
resourceType: 'Custom::AthenaNotebook',
onCreate: {
service: 'Athena',
action: 'createNotebook',
parameters: {
WorkGroup: 'WorkGroupA',
Name: 'Notebook1',
},
},
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
});
}).toThrow(/'physicalResourceId' must be specified for 'onCreate' call./);
});

// physicalResourceId pattern #7
test('onCreate and onUpdate both have physicalResourceId when physicalResourceId is specified in onUpdate, even when onCreate is unspecified', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new AwsCustomResource(stack, 'AwsSdk', {
resourceType: 'Custom::AthenaNotebook',
onUpdate: {
service: 'Athena',
action: 'updateNotebookMetadata',
physicalResourceId: PhysicalResourceId.of('id'),
parameters: {
Name: 'Notebook1',
NotebookId: 'XXXX',
},
},
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
});

Template.fromStack(stack).hasResourceProperties('Custom::AthenaNotebook', {
Create: JSON.stringify({
service: 'Athena',
action: 'updateNotebookMetadata',
physicalResourceId: {
id: 'id',
},
parameters: {
Name: 'Notebook1',
NotebookId: 'XXXX',
},
}),
Update: JSON.stringify({
service: 'Athena',
action: 'updateNotebookMetadata',
physicalResourceId: {
id: 'id',
},
parameters: {
Name: 'Notebook1',
NotebookId: 'XXXX',
},
}),
});
});

// physicalResourceId pattern #8
test('Omitting physicalResourceId in onCreate when onUpdate is undefined throws an error', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
expect(() => {
new AwsCustomResource(stack, 'AwsSdk', {
resourceType: 'Custom::AthenaNotebook',
onUpdate: {
service: 'Athena',
action: 'updateNotebookMetadata',
parameters: {
Name: 'Notebook1',
NotebookId: 'XXXX',
},
},
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
});
}).toThrow(/'physicalResourceId' must be specified for 'onUpdate' call when 'onCreate' is omitted./);
});
});

test('booleans are encoded in the stringified parameters object', () => {
Expand Down
19 changes: 19 additions & 0 deletions ...source-athena.js.snapshot/CustomResourceAthenaDefaultTestDeployAssert7AE6A475.assets.json
View file
Open in desktop
@@ -0,0 +1,19 @@
{
"version": "29.0.0",
"files": {
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
"source": {
"path": "CustomResourceAthenaDefaultTestDeployAssert7AE6A475.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}