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.

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

fix(core): adding warnings to alert when properties will be overwritten #33507

Merged
merged 6 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/aws-cdk-lib/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,8 @@ new CustomResource(this, 'MyMagicalResource', {
resourceType: 'Custom::MyCustomResource', // must start with 'Custom::'

// the resource properties
// properties like serviceToken serviceTimeout or are ported into properties automatically
// try not to use key names similar to these or there will be a risk of overwriting those values
properties: {
Property1: 'foo',
Property2: 'bar',
Expand Down
23 changes: 21 additions & 2 deletions packages/aws-cdk-lib/core/lib/custom-resource.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Construct } from 'constructs';
import { Annotations } from './annotations';
import { CfnResource } from './cfn-resource';
import { Duration } from './duration';
import { addConstructMetadata, MethodMetadata } from './metadata-resource';
Expand Down Expand Up @@ -54,6 +55,8 @@ export interface CustomResourceProps {
* serviceToken: myTopic.topicArn,
* });
* ```
*
* serviceToken is passed to properties with key "ServiceToken"
*/
readonly serviceToken: string;

Expand All @@ -62,13 +65,19 @@ export interface CustomResourceProps {
*
* The value must be between 1 second and 3600 seconds.
*
* serviceTimeout is passed to properties with key "ServiceTimeout"
*
* @default Duration.seconds(3600)
*/
readonly serviceTimeout?: Duration;

/**
* Properties to pass to the Lambda
*
* Some propery values are passed into this dictionary automatically.
* Please keep in mind that these keys should not be repeated in this prop or they will be overwritten.
* E.g. `ServiceToken`, `ServiceTimeout`
*
* @default - No properties.
*/
readonly properties?: { [key: string]: any };
Expand Down Expand Up @@ -154,11 +163,21 @@ export class CustomResource extends Resource {
}
}

const constructPropertiesPassed = {
ServiceToken: props.serviceToken,
ServiceTimeout: props.serviceTimeout?.toSeconds().toString(),
};

const hasCommonKeys = Object.keys(properties).some(key => key in constructPropertiesPassed);

if (hasCommonKeys) {
Annotations.of(this).addWarningV2('@aws-cdk/core:customResourcePropDuplicate', `CustomResource properties should not contain keys that are automatically added by the CDK. Found: ${Object.keys(properties).filter(key => key in constructPropertiesPassed)}`);
}

this.resource = new CfnResource(this, 'Default', {
type,
properties: {
ServiceToken: props.serviceToken,
ServiceTimeout: props.serviceTimeout?.toSeconds().toString(),
...constructPropertiesPassed,
...properties,
},
});
Expand Down
17 changes: 17 additions & 0 deletions packages/aws-cdk-lib/core/test/custom-resource.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { toCloudFormation } from './util';
import { Annotations } from '../../assertions';
import { CustomResource, Duration, RemovalPolicy, Stack } from '../lib';

describe('custom resource', () => {
Expand Down Expand Up @@ -212,4 +213,20 @@ describe('custom resource', () => {
});
}).toThrow(`serviceTimeout must either be between 1 and 3600 seconds, got ${invalidSeconds}`);
});

test('send warning if customResource construct property key is added to properties', () => {
// GIVEN
const stack = new Stack();

// WHEN
new CustomResource(stack, 'MyCustomResource', {
serviceToken: 'MyServiceToken',
properties: {
ServiceToken: 'RepeatedToken', // this is repeated because serviceToken prop above will resolve as property ServiceToken
},
});

// THEN
Annotations.fromStack(stack).hasWarning('/Default/MyCustomResource', 'CustomResource properties should not contain keys that are automatically added by the CDK. Found: ServiceToken [ack: @aws-cdk/core:customResourcePropDuplicate]');
});
});
Loading