Skip to content

Commit bc91c70

Browse files
authored
fix(core): serviceTimeout for CustomResource does not work with token (#33541)
### Issue # (if applicable) Closes #33513. ### Reason for this change The `serviceTimeout` for `CustomResource` does not work with token. The way the token is handled is wrong, and the `isUnresolved` method that the `Duration` type has should be used. ### Description of changes Use the `props.serviceTimeout.isUnresolved()`. Also add a doc that the token must be specified in `Duration.seconds()` since it is converted by `toSeconds` internally. (Because it is a token and unknown MINUTES value cannot be converted to SECONDS. This is due to the token mechanism.) see: https://github.com/go-to-k/aws-cdk/blob/75e52619cd09f363882ff62561a53cd5cd79ab30/packages/aws-cdk-lib/core/lib/custom-resource.ts#L169 https://github.com/go-to-k/aws-cdk/blob/75e52619cd09f363882ff62561a53cd5cd79ab30/packages/aws-cdk-lib/core/lib/duration.ts#L332 ### Describe any new or updated permissions being added ### Description of how you validated changes Unit tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 0596824 commit bc91c70

File tree

2 files changed

+70
-3
lines changed

2 files changed

+70
-3
lines changed

packages/aws-cdk-lib/core/lib/custom-resource.ts

+14-2
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,19 @@ export interface CustomResourceProps {
6767
*
6868
* Maps to [ServiceTimeout](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-cloudformation-customresource.html#cfn-cloudformation-customresource-servicetimeout) property for the `AWS::CloudFormation::CustomResource` resource
6969
*
70+
* A token can be specified for this property, but it must be specified with `Duration.seconds()`.
71+
*
72+
* @example
73+
* const stack = new Stack();
74+
* const durToken = new CfnParameter(stack, 'MyParameter', {
75+
* type: 'Number',
76+
* default: 60,
77+
* });
78+
* new CustomResource(stack, 'MyCustomResource', {
79+
* serviceToken: 'MyServiceToken',
80+
* serviceTimeout: Duration.seconds(durToken.valueAsNumber),
81+
* });
82+
*
7083
* @default Duration.seconds(3600)
7184
*/
7285
readonly serviceTimeout?: Duration;
@@ -154,8 +167,7 @@ export class CustomResource extends Resource {
154167
const pascalCaseProperties = props.pascalCaseProperties ?? false;
155168
const properties = pascalCaseProperties ? uppercaseProperties(props.properties || {}) : (props.properties || {});
156169

157-
if (props.serviceTimeout !== undefined && !Token.isUnresolved(props.serviceTimeout)
158-
) {
170+
if (props.serviceTimeout !== undefined && !props.serviceTimeout.isUnresolved()) {
159171
const serviceTimeoutSeconds = props.serviceTimeout.toSeconds();
160172

161173
if (serviceTimeoutSeconds < 1 || serviceTimeoutSeconds > 3600) {

packages/aws-cdk-lib/core/test/custom-resource.test.ts

+56-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { toCloudFormation } from './util';
22
import { Annotations } from '../../assertions';
3-
import { CustomResource, Duration, RemovalPolicy, Stack } from '../lib';
3+
import { CfnParameter, CustomResource, Duration, RemovalPolicy, Stack } from '../lib';
44

55
describe('custom resource', () => {
66
test('simple case provider identified by service token', () => {
@@ -201,6 +201,61 @@ describe('custom resource', () => {
201201
});
202202
});
203203

204+
test('set serviceTimeout with token as seconds', () => {
205+
// GIVEN
206+
const stack = new Stack();
207+
const durToken = new CfnParameter(stack, 'MyParameter', {
208+
type: 'Number',
209+
default: 60,
210+
});
211+
212+
// WHEN
213+
new CustomResource(stack, 'MyCustomResource', {
214+
serviceToken: 'MyServiceToken',
215+
serviceTimeout: Duration.seconds(durToken.valueAsNumber),
216+
});
217+
218+
// THEN
219+
expect(toCloudFormation(stack)).toEqual({
220+
Parameters: {
221+
MyParameter: {
222+
Default: 60,
223+
Type: 'Number',
224+
},
225+
},
226+
Resources: {
227+
MyCustomResource: {
228+
Type: 'AWS::CloudFormation::CustomResource',
229+
Properties: {
230+
ServiceToken: 'MyServiceToken',
231+
ServiceTimeout: {
232+
Ref: 'MyParameter',
233+
},
234+
},
235+
UpdateReplacePolicy: 'Delete',
236+
DeletionPolicy: 'Delete',
237+
},
238+
},
239+
});
240+
});
241+
242+
test('throws error when serviceTimeout is set with token as units other than seconds', () => {
243+
// GIVEN
244+
const stack = new Stack();
245+
const durToken = new CfnParameter(stack, 'MyParameter', {
246+
type: 'Number',
247+
default: 60,
248+
});
249+
250+
// WHEN
251+
expect(() => {
252+
new CustomResource(stack, 'MyCustomResource', {
253+
serviceToken: 'MyServiceToken',
254+
serviceTimeout: Duration.minutes(durToken.valueAsNumber),
255+
});
256+
}).toThrow('Duration must be specified as \'Duration.seconds()\' here since its value comes from a token and cannot be converted (got Duration.minutes)');
257+
});
258+
204259
test.each([0, 4000])('throw an error when serviceTimeout is set to %d seconds.', (invalidSeconds: number) => {
205260
// GIVEN
206261
const stack = new Stack();

0 commit comments

Comments
 (0)