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

CustomResource: ServiceToken clobbered by properties #32468

Open
1 task
Shwaring opened this issue Dec 11, 2024 · 2 comments
Open
1 task

CustomResource: ServiceToken clobbered by properties #32468

Shwaring opened this issue Dec 11, 2024 · 2 comments
Labels
@aws-cdk/custom-resources Related to AWS CDK Custom Resources bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@Shwaring
Copy link

Shwaring commented Dec 11, 2024

Describe the bug

The properties construct props implies that all the properties are sent to the lambda. However if a key ServiceToken is added to the properties object it will set the service token for the custom resource, and even overwrite the value provided in the seviceToken constructor prop.

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

The service token of the custom resource should always be the value provided in serviceToken

Current Behavior

The ServiceToken property of the custom resource is overwritten with the value in the property.

Reproduction Steps

Warning

Deploying this stack will fail and take 2 hours too resolve due to custom resource timeout, Only synth is needed to verify the bug.

Create a basic typescript cdk app,

mkdir testApp
cd testApp
cdk init app --language=typescript

update ./bin/test_app.ts

#!/usr/bin/env node
import * as cdk from 'aws-cdk-lib'
import { Code, Function, Runtime } from 'aws-cdk-lib/aws-lambda';
import { Provider } from 'aws-cdk-lib/custom-resources';
import { Construct } from 'constructs';
const app = new cdk.App();
export class TestCdkAppStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);
Function
    const myFunction = new Function(this, 'MyFunction', {
      runtime: Runtime.NODEJS_18_X,
      handler: 'index.handler',
      code: Code.fromInline(`
        exports.handler = async (event) => {
          return(JSON.stringify(event, undefined, 2));
        };
      `),
    });
    const provider = new Provider(this, 'Provider', {
      onEventHandler: myFunction,
    });
    const invoker = new cdk.CustomResource(this, 'Invoker', {
      serviceToken: provider.serviceToken,
      properties: {
        ServiceToken: myFunction.functionArn
      },
    });
  }
}
new TestCdkAppStack(app, 'TestCdkAppStack', {

});

Run CDK synth and observe the cdk.out/TestCdkAppStack.template.json. See that the invoker has the service token of the lambda function instead of the provider.

  "Invoker": {
   "Type": "AWS::CloudFormation::CustomResource",
   "Properties": {
    "ServiceToken": {
     "Fn::GetAtt": [
      "MyFunction3BAA72D1",
      "Arn"
     ]
    }
   },
   "UpdateReplacePolicy": "Delete",
   "DeletionPolicy": "Delete",
   "Metadata": {
    "aws:cdk:path": "TestCdkAppStack/Invoker/Default"
    }

Comment out the properties attribute of the custom resource and synth again. Observe that it is now correctly has the service token of the provider,

    const invoker = new cdk.CustomResource(this, 'Invoker', {
      serviceToken: provider.serviceToken,
      // properties: {
      //   ServiceToken: myFunction.functionArn
      // },
    });
  "Invoker": {
   "Type": "AWS::CloudFormation::CustomResource",
   "Properties": {
    "ServiceToken": {
     "Fn::GetAtt": [
      "ProviderframeworkonEvent83C1D0A7",
      "Arn"
     ]
    }
   },
   "UpdateReplacePolicy": "Delete",
   "DeletionPolicy": "Delete",
   "Metadata": {
    "aws:cdk:path": "TestCdkAppStack/Invoker/Default"
   }
  },

Possible Solution

properties object overwrites the invokers base properties. Potentially provide a warning of this.

Additional Information/Context

No response

CDK CLI Version

2.171.1 (build a95560c)

Framework Version

No response

Node.js Version

v20.12.2

OS

linux

Language

TypeScript

Language Version

5.6.3

Other information

No response

@Shwaring Shwaring added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 11, 2024
@github-actions github-actions bot added the @aws-cdk/custom-resources Related to AWS CDK Custom Resources label Dec 11, 2024
@ashishdhingra ashishdhingra self-assigned this Dec 12, 2024
@ashishdhingra ashishdhingra added p2 investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Dec 12, 2024
@ashishdhingra
Copy link
Contributor

@Shwaring Good morning. Thanks for opening the issue. Yes, per CustomRessource implementation here, properties are applied after ServiceToken on a CfnResource. So if CustomResourceProps.properties has a ServiceToken key, it would replace ServiceToken property. Kindly note that the behavior could also be impacted by CustomResourceProps.pascalCaseProperties boolean flag and CustomResourceProps.properties has a key serviceToken. This is also affect on the order of property processing, where duplicate property later would replace a property already set earlier.

Could you please confirm if there any specific use case where ServiceToken could be set in CustomResourceProps.properties? Or your example just demonstrates the user case where it might be replaced later?

Either the case, I also think that there should be a check for ServiceToken property, which if explicitly set at CustomResourceProps.serviceToken, should not consider replaced by value set in CustomResourceProps.properties as ServiceToken key (or serviceToken key with CustomResourceProps.pascalCaseProperties as true). I would seek inputs from team if we need to implement this check since end user is aware of possible side-effects of replacement.

@ashishdhingra ashishdhingra added effort/small Small work item – less than a day of effort and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Dec 12, 2024
@ashishdhingra ashishdhingra removed their assignment Dec 12, 2024
@Shwaring
Copy link
Author

Shwaring commented Dec 12, 2024

The serviceToken is a required field. So I think most most people would reasonable expect the service token to be the one set in that field not in properties. There fore I cannot see a use case where someone would want to set the service token on the properties parameters.

Furthermore, the documentation on the properties explicitly says it is passed to the lambda, not that it sets properties for the custom resource. So I was not expecting my properties to have any effects on the custom resource.

"properties?
Type: { [string]: any } (optional, default: No properties.)

Properties to pass to the Lambda."

Perhaps if we were seeking a side affect free fix for this issue a simple rewording of this documentation to convey that the properties will clobber the explicitly set values of the custom resource.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/custom-resources Related to AWS CDK Custom Resources bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
Development

No branches or pull requests

2 participants