Skip to content

Commit 21ad7a7

Browse files
konokenjkonokenj
authored
feat(custom-resources): AwsCustomResource copy physicalResourceId from request when omit it in onUpdate (#24194)
AwsCustomResource is now able to omit `physicalResourceId` in `onUpdate` to copy it from request. Some `UPDATE` AWS APIs responses with an empty body. When users want to call these APIs using AwsCustomResource, users can't specify physicalResourceId by `PhysicalResourceId.fromResponse()`. Furthermore, when the Create API generates an unpredictable ID and this must be passed to the Update API, this Construct could not be used. For example, following APIs match this situation: - https://docs.aws.amazon.com/athena/latest/APIReference/API_UpdateNotebook.html - https://docs.aws.amazon.com/singlesignon/latest/IdentityStoreAPIReference/API_UpdateUser.html Closes #23843. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent c897f44 commit 21ad7a7

File tree

13 files changed

+1612
-17
lines changed

13 files changed

+1612
-17
lines changed

‎packages/@aws-cdk/custom-resources/README.md

+4
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,10 @@ const awsCustom = new cr.AwsCustomResource(this, 'aws-custom', {
502502
})
503503
```
504504

505+
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.
506+
507+
> AwsCustomResource.getResponseField() and .getResponseFieldReference() will not work if the Create and Update APIs don't consistently return the same fields.
508+
505509
### Handling Custom Resource Errors
506510

507511
Every error produced by the API call is treated as is and will cause a "FAILED" response to be submitted to CloudFormation.

‎packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts

+8-5
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ export interface AwsSdkCall {
8686

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

387-
for (const call of [props.onCreate, props.onUpdate]) {
388-
if (call && !call.physicalResourceId) {
389-
throw new Error('`physicalResourceId` must be specified for onCreate and onUpdate calls.');
390-
}
388+
if (props.onCreate && !props.onCreate.physicalResourceId) {
389+
throw new Error("'physicalResourceId' must be specified for 'onCreate' call.");
390+
}
391+
392+
if (!props.onCreate && props.onUpdate && !props.onUpdate.physicalResourceId) {
393+
throw new Error("'physicalResourceId' must be specified for 'onUpdate' call when 'onCreate' is omitted.");
391394
}
392395

393396
for (const call of [props.onCreate, props.onUpdate, props.onDelete]) {

‎packages/@aws-cdk/custom-resources/test/aws-custom-resource/aws-custom-resource.test.ts

+304-12
Original file line numberDiff line numberDiff line change
@@ -171,20 +171,312 @@ test('fails when no calls are specified', () => {
171171
})).toThrow(/`onCreate`.+`onUpdate`.+`onDelete`/);
172172
});
173173

174-
test('fails when no physical resource method is specified', () => {
175-
const stack = new cdk.Stack();
174+
// test patterns for physicalResourceId
175+
// | # | onCreate.physicalResourceId | onUpdate.physicalResourceId | Error thrown? |
176+
// |---|-----------------------------------|----------------------------------|---------------|
177+
// | 1 | ANY_VALUE | ANY_VALUE | no |
178+
// | 2 | ANY_VALUE | undefined | no |
179+
// | 3 | undefined | ANY_VALLUE | yes |
180+
// | 4 | undefined | undefined | yes |
181+
// | 5 | ANY_VALUE | undefined (*omit whole onUpdate) | no |
182+
// | 6 | undefined | undefined (*omit whole onUpdate) | yes |
183+
// | 7 | ANY_VALUE (*copied from onUpdate) | ANY_VALUE | no |
184+
// | 8 | undefined (*copied from onUpdate) | undefined | yes |
185+
describe('physicalResourceId patterns', () => {
186+
// physicalResourceId pattern #1
187+
test('physicalResourceId is specified both in onCreate and onUpdate then success', () => {
188+
// GIVEN
189+
const stack = new cdk.Stack();
190+
191+
// WHEN
192+
new AwsCustomResource(stack, 'AwsSdk', {
193+
resourceType: 'Custom::AthenaNotebook',
194+
onCreate: {
195+
service: 'Athena',
196+
action: 'createNotebook',
197+
physicalResourceId: PhysicalResourceId.of('id'),
198+
parameters: {
199+
WorkGroup: 'WorkGroupA',
200+
Name: 'Notebook1',
201+
},
202+
},
203+
onUpdate: {
204+
service: 'Athena',
205+
action: 'updateNotebookMetadata',
206+
physicalResourceId: PhysicalResourceId.of('id'),
207+
parameters: {
208+
Name: 'Notebook1',
209+
NotebookId: new PhysicalResourceIdReference(),
210+
},
211+
},
212+
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
213+
});
176214

177-
expect(() => new AwsCustomResource(stack, 'AwsSdk', {
178-
onUpdate: {
179-
service: 'CloudWatchLogs',
180-
action: 'putRetentionPolicy',
181-
parameters: {
182-
logGroupName: '/aws/lambda/loggroup',
183-
retentionInDays: 90,
215+
// THEN
216+
Template.fromStack(stack).hasResourceProperties('Custom::AthenaNotebook', {
217+
Create: JSON.stringify({
218+
service: 'Athena',
219+
action: 'createNotebook',
220+
physicalResourceId: {
221+
id: 'id',
222+
},
223+
parameters: {
224+
WorkGroup: 'WorkGroupA',
225+
Name: 'Notebook1',
226+
},
227+
}),
228+
Update: JSON.stringify({
229+
service: 'Athena',
230+
action: 'updateNotebookMetadata',
231+
physicalResourceId: {
232+
id: 'id',
233+
},
234+
parameters: {
235+
Name: 'Notebook1',
236+
NotebookId: 'PHYSICAL:RESOURCEID:',
237+
},
238+
}),
239+
});
240+
});
241+
242+
// physicalResourceId pattern #2
243+
test('physicalResourceId is specified in onCreate, is not in onUpdate then absent', () => {
244+
// GIVEN
245+
const stack = new cdk.Stack();
246+
247+
// WHEN
248+
new AwsCustomResource(stack, 'AwsSdk', {
249+
resourceType: 'Custom::AthenaNotebook',
250+
onCreate: {
251+
service: 'Athena',
252+
action: 'createNotebook',
253+
physicalResourceId: PhysicalResourceId.fromResponse('NotebookId'),
254+
parameters: {
255+
WorkGroup: 'WorkGroupA',
256+
Name: 'Notebook1',
257+
},
184258
},
185-
},
186-
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
187-
})).toThrow(/`physicalResourceId`/);
259+
onUpdate: {
260+
service: 'Athena',
261+
action: 'updateNotebookMetadata',
262+
parameters: {
263+
Name: 'Notebook1',
264+
NotebookId: new PhysicalResourceIdReference(),
265+
},
266+
},
267+
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
268+
});
269+
270+
// THEN
271+
Template.fromStack(stack).hasResourceProperties('Custom::AthenaNotebook', {
272+
Create: JSON.stringify({
273+
service: 'Athena',
274+
action: 'createNotebook',
275+
physicalResourceId: {
276+
responsePath: 'NotebookId',
277+
},
278+
parameters: {
279+
WorkGroup: 'WorkGroupA',
280+
Name: 'Notebook1',
281+
},
282+
}),
283+
Update: JSON.stringify({
284+
service: 'Athena',
285+
action: 'updateNotebookMetadata',
286+
parameters: {
287+
Name: 'Notebook1',
288+
NotebookId: 'PHYSICAL:RESOURCEID:',
289+
},
290+
}),
291+
});
292+
});
293+
294+
// physicalResourceId pattern #3
295+
test('physicalResourceId is not specified in onCreate but onUpdate then fail', () => {
296+
// GIVEN
297+
const stack = new cdk.Stack();
298+
299+
// WHEN
300+
expect(() => {
301+
new AwsCustomResource(stack, 'AwsSdk', {
302+
resourceType: 'Custom::AthenaNotebook',
303+
onCreate: {
304+
service: 'Athena',
305+
action: 'createNotebook',
306+
parameters: {
307+
WorkGroup: 'WorkGroupA',
308+
Name: 'Notebook1',
309+
},
310+
},
311+
onUpdate: {
312+
service: 'Athena',
313+
action: 'updateNotebookMetadata',
314+
physicalResourceId: PhysicalResourceId.of('id'),
315+
parameters: {
316+
Name: 'Notebook1',
317+
NotebookId: new PhysicalResourceIdReference(),
318+
},
319+
},
320+
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
321+
});
322+
}).toThrow(/'physicalResourceId' must be specified for 'onCreate' call./);
323+
});
324+
325+
// physicalResourceId pattern #4
326+
test('physicalResourceId is not specified both in onCreate and onUpdate then fail', () => {
327+
// GIVEN
328+
const stack = new cdk.Stack();
329+
330+
// WHEN
331+
expect(() => {
332+
new AwsCustomResource(stack, 'AwsSdk', {
333+
resourceType: 'Custom::AthenaNotebook',
334+
onCreate: {
335+
service: 'Athena',
336+
action: 'createNotebook',
337+
parameters: {
338+
WorkGroup: 'WorkGroupA',
339+
Name: 'Notebook1',
340+
},
341+
},
342+
onUpdate: {
343+
service: 'Athena',
344+
action: 'updateNotebookMetadata',
345+
parameters: {
346+
Name: 'Notebook1',
347+
NotebookId: new PhysicalResourceIdReference(),
348+
},
349+
},
350+
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
351+
});
352+
}).toThrow(/'physicalResourceId' must be specified for 'onCreate' call./);
353+
});
354+
355+
// physicalResourceId pattern #5
356+
test('physicalResourceId is specified in onCreate with empty onUpdate then success', () => {
357+
// GIVEN
358+
const stack = new cdk.Stack();
359+
360+
// WHEN
361+
new AwsCustomResource(stack, 'AwsSdk', {
362+
resourceType: 'Custom::AthenaNotebook',
363+
onCreate: {
364+
service: 'Athena',
365+
action: 'createNotebook',
366+
physicalResourceId: PhysicalResourceId.of('id'),
367+
parameters: {
368+
WorkGroup: 'WorkGroupA',
369+
Name: 'Notebook1',
370+
},
371+
},
372+
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
373+
});
374+
375+
// THEN
376+
Template.fromStack(stack).hasResourceProperties('Custom::AthenaNotebook', {
377+
Create: JSON.stringify({
378+
service: 'Athena',
379+
action: 'createNotebook',
380+
physicalResourceId: {
381+
id: 'id',
382+
},
383+
parameters: {
384+
WorkGroup: 'WorkGroupA',
385+
Name: 'Notebook1',
386+
},
387+
}),
388+
});
389+
});
390+
391+
// physicalResourceId pattern #6
392+
test('physicalResourceId is not specified onCreate with empty onUpdate then fail', () => {
393+
// GIVEN
394+
const stack = new cdk.Stack();
395+
396+
// WHEN
397+
expect(() => {
398+
new AwsCustomResource(stack, 'AwsSdk', {
399+
resourceType: 'Custom::AthenaNotebook',
400+
onCreate: {
401+
service: 'Athena',
402+
action: 'createNotebook',
403+
parameters: {
404+
WorkGroup: 'WorkGroupA',
405+
Name: 'Notebook1',
406+
},
407+
},
408+
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
409+
});
410+
}).toThrow(/'physicalResourceId' must be specified for 'onCreate' call./);
411+
});
412+
413+
// physicalResourceId pattern #7
414+
test('onCreate and onUpdate both have physicalResourceId when physicalResourceId is specified in onUpdate, even when onCreate is unspecified', () => {
415+
// GIVEN
416+
const stack = new cdk.Stack();
417+
418+
// WHEN
419+
new AwsCustomResource(stack, 'AwsSdk', {
420+
resourceType: 'Custom::AthenaNotebook',
421+
onUpdate: {
422+
service: 'Athena',
423+
action: 'updateNotebookMetadata',
424+
physicalResourceId: PhysicalResourceId.of('id'),
425+
parameters: {
426+
Name: 'Notebook1',
427+
NotebookId: 'XXXX',
428+
},
429+
},
430+
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
431+
});
432+
433+
Template.fromStack(stack).hasResourceProperties('Custom::AthenaNotebook', {
434+
Create: JSON.stringify({
435+
service: 'Athena',
436+
action: 'updateNotebookMetadata',
437+
physicalResourceId: {
438+
id: 'id',
439+
},
440+
parameters: {
441+
Name: 'Notebook1',
442+
NotebookId: 'XXXX',
443+
},
444+
}),
445+
Update: JSON.stringify({
446+
service: 'Athena',
447+
action: 'updateNotebookMetadata',
448+
physicalResourceId: {
449+
id: 'id',
450+
},
451+
parameters: {
452+
Name: 'Notebook1',
453+
NotebookId: 'XXXX',
454+
},
455+
}),
456+
});
457+
});
458+
459+
// physicalResourceId pattern #8
460+
test('Omitting physicalResourceId in onCreate when onUpdate is undefined throws an error', () => {
461+
// GIVEN
462+
const stack = new cdk.Stack();
463+
464+
// WHEN
465+
expect(() => {
466+
new AwsCustomResource(stack, 'AwsSdk', {
467+
resourceType: 'Custom::AthenaNotebook',
468+
onUpdate: {
469+
service: 'Athena',
470+
action: 'updateNotebookMetadata',
471+
parameters: {
472+
Name: 'Notebook1',
473+
NotebookId: 'XXXX',
474+
},
475+
},
476+
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
477+
});
478+
}).toThrow(/'physicalResourceId' must be specified for 'onUpdate' call when 'onCreate' is omitted./);
479+
});
188480
});
189481

190482
test('booleans are encoded in the stringified parameters object', () => {

‎packages/@aws-cdk/custom-resources/test/aws-custom-resource/integ.aws-custom-resource-athena.js.snapshot/CustomResourceAthenaDefaultTestDeployAssert7AE6A475.assets.json

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"version": "29.0.0",
3+
"files": {
4+
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
5+
"source": {
6+
"path": "CustomResourceAthenaDefaultTestDeployAssert7AE6A475.template.json",
7+
"packaging": "file"
8+
},
9+
"destinations": {
10+
"current_account-current_region": {
11+
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
12+
"objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json",
13+
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
14+
}
15+
}
16+
}
17+
},
18+
"dockerImages": {}
19+
}

0 commit comments

Comments
 (0)