Skip to content

Commit

Permalink
fix(core): serviceTimeout for CustomResource does not work with t…
Browse files Browse the repository at this point in the history
…oken (#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*
  • Loading branch information
go-to-k authored Feb 22, 2025
1 parent 0596824 commit bc91c70
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 3 deletions.
16 changes: 14 additions & 2 deletions packages/aws-cdk-lib/core/lib/custom-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
57 changes: 56 additions & 1 deletion packages/aws-cdk-lib/core/test/custom-resource.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit bc91c70

Please sign in to comment.