From bc91c70129f4946f0a9b26a61c9cbf3a6fd8282d Mon Sep 17 00:00:00 2001 From: "Kenta Goto (k.goto)" <24818752+go-to-k@users.noreply.github.com> Date: Sat, 22 Feb 2025 13:32:38 +0900 Subject: [PATCH] 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* --- .../aws-cdk-lib/core/lib/custom-resource.ts | 16 +++++- .../core/test/custom-resource.test.ts | 57 ++++++++++++++++++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk-lib/core/lib/custom-resource.ts b/packages/aws-cdk-lib/core/lib/custom-resource.ts index d5a8bb5d78d73..25ba85793394c 100644 --- a/packages/aws-cdk-lib/core/lib/custom-resource.ts +++ b/packages/aws-cdk-lib/core/lib/custom-resource.ts @@ -67,6 +67,19 @@ export interface CustomResourceProps { * * 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 * + * A token can be specified for this property, but it must be specified with `Duration.seconds()`. + * + * @example + * const stack = new Stack(); + * const durToken = new CfnParameter(stack, 'MyParameter', { + * type: 'Number', + * default: 60, + * }); + * new CustomResource(stack, 'MyCustomResource', { + * serviceToken: 'MyServiceToken', + * serviceTimeout: Duration.seconds(durToken.valueAsNumber), + * }); + * * @default Duration.seconds(3600) */ readonly serviceTimeout?: Duration; @@ -154,8 +167,7 @@ export class CustomResource extends Resource { const pascalCaseProperties = props.pascalCaseProperties ?? false; const properties = pascalCaseProperties ? uppercaseProperties(props.properties || {}) : (props.properties || {}); - if (props.serviceTimeout !== undefined && !Token.isUnresolved(props.serviceTimeout) - ) { + if (props.serviceTimeout !== undefined && !props.serviceTimeout.isUnresolved()) { const serviceTimeoutSeconds = props.serviceTimeout.toSeconds(); if (serviceTimeoutSeconds < 1 || serviceTimeoutSeconds > 3600) { diff --git a/packages/aws-cdk-lib/core/test/custom-resource.test.ts b/packages/aws-cdk-lib/core/test/custom-resource.test.ts index 3334a0782c296..ac2df44be15b5 100644 --- a/packages/aws-cdk-lib/core/test/custom-resource.test.ts +++ b/packages/aws-cdk-lib/core/test/custom-resource.test.ts @@ -1,6 +1,6 @@ import { toCloudFormation } from './util'; import { Annotations } from '../../assertions'; -import { CustomResource, Duration, RemovalPolicy, Stack } from '../lib'; +import { CfnParameter, CustomResource, Duration, RemovalPolicy, Stack } from '../lib'; describe('custom resource', () => { test('simple case provider identified by service token', () => { @@ -201,6 +201,61 @@ describe('custom resource', () => { }); }); + test('set serviceTimeout with token as seconds', () => { + // GIVEN + const stack = new Stack(); + const durToken = new CfnParameter(stack, 'MyParameter', { + type: 'Number', + default: 60, + }); + + // WHEN + new CustomResource(stack, 'MyCustomResource', { + serviceToken: 'MyServiceToken', + serviceTimeout: Duration.seconds(durToken.valueAsNumber), + }); + + // THEN + expect(toCloudFormation(stack)).toEqual({ + Parameters: { + MyParameter: { + Default: 60, + Type: 'Number', + }, + }, + Resources: { + MyCustomResource: { + Type: 'AWS::CloudFormation::CustomResource', + Properties: { + ServiceToken: 'MyServiceToken', + ServiceTimeout: { + Ref: 'MyParameter', + }, + }, + UpdateReplacePolicy: 'Delete', + DeletionPolicy: 'Delete', + }, + }, + }); + }); + + test('throws error when serviceTimeout is set with token as units other than seconds', () => { + // GIVEN + const stack = new Stack(); + const durToken = new CfnParameter(stack, 'MyParameter', { + type: 'Number', + default: 60, + }); + + // WHEN + expect(() => { + new CustomResource(stack, 'MyCustomResource', { + serviceToken: 'MyServiceToken', + serviceTimeout: Duration.minutes(durToken.valueAsNumber), + }); + }).toThrow('Duration must be specified as \'Duration.seconds()\' here since its value comes from a token and cannot be converted (got Duration.minutes)'); + }); + test.each([0, 4000])('throw an error when serviceTimeout is set to %d seconds.', (invalidSeconds: number) => { // GIVEN const stack = new Stack();