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

[IkeNefcy]

Issue

Closes #32468

Reason for this change

When using a custom resource, the values for serviceToken and serviceTimeout are added to the construct prop properties during the synth. Thus passing those values through to the lambda. The issue is that these values can be overwritten if you also include the exact keys in your own properties argument.

So if I include serviceToken, which is a required arg, then I set properties

properties: {
  ServiceToken: 'something else',
},

the value of serviceToken is set to ServiceToken, then my property I wrote to ServiceToken takes over and replaces the value with my own.

This change is to add a warning to the user so they can understand that what they are doing is overwriting that key, as well as add some more detailed flavor text to the properties and readme to help convey this.

Description of changes

Previously the props like serviceToken were being written directly to properties, along with the user provided properties broken out with ... notation.

I moved the automatically added props out of this

    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: {
        ...constructPropertiesPassed,
        ...properties,
      },
    });

This allowed for a simple comparison between the 2 dicts, which allows for the warning to be initiated from.

Description of how you validated changes

I added a test to check if this warning is being generated.

I did not change any integs because the actual synth in the end is the exact same as before.

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

(This review is outdated)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.17%. Comparing base (8728d5b) to head (f5034f2).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #33507      +/-   ##
==========================================
+ Coverage   82.16%   82.17%   +0.01%     
==========================================
  Files         119      119              
  Lines        6857     6862       +5     
  Branches     1157     1158       +1     
==========================================
+ Hits         5634     5639       +5     
  Misses       1120     1120              
  Partials      103      103              

Flag	Coverage Δ
suite.unit	82.17% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	∅ <ø> (∅)
packages/aws-cdk-lib/core	82.17% <100.00%> (+0.01%)	⬆️

[IkeNefcy]

Exemption Request : No integs need to be changed, this is just adding a warning test during synth. As far as I'm aware this is not something we can test with integs. The actual Synth'ed template should be the same as before.

[IkeNefcy]

Open to any verbiage changes

[samson-keung]

A few minor comments on the wording. Thank you for making the change.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

Pull request has been modified.

[samson-keung]

LGTM. Thanks!

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: f5034f2
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.