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

Custom resource capitalises properties - undocumented #4896

Closed
TomDufall opened this issue Nov 7, 2019 · 2 comments · Fixed by #7736
Closed

Custom resource capitalises properties - undocumented #4896

TomDufall opened this issue Nov 7, 2019 · 2 comments · Fixed by #7736
Assignees
Labels
bug This issue is a bug. p2

Comments

@TomDufall
Copy link
Contributor

When creating a custom resource, you can add in properties. These properties get capitalised. This is documented in the code, but not the documentation pages.

Documentation in code
Usage in code

/**

  • Uppercase the first letter of every property name
  • It's customary for CloudFormation properties to start with capitals, and our
  • properties to start with lowercase, so this function translates from one
  • to the other
    */

Reproduction Steps

CDK definition

db_initialiser = aws_cfn.CustomResource(
    self, "db_initialiser",
    properties={"es_endpoint": self.es_domain.attr_domain_endpoint},
    provider=aws_cfn.CustomResourceProvider.lambda_(
        aws_lambda.SingletonFunction(
            self, "db_init_singleton",
            code=aws_lambda.Code.asset("efar/elasticsearch_config"),
            handler="es_config_custom_resource.event_handler",
            runtime=aws_lambda.Runtime.PYTHON_3_7,
            timeout=core.Duration.seconds(300),
            uuid="1c23d474-74c3-4adc-8d1d-760e56c2d681",
            )
        )
    )

Error Log

Cloudwatch log

[DEBUG] 2019-11-06T16:42:19.234Z f54b36c0-da56-4f2c-9069-2769094304ce {'RequestType': 'Create', 'ServiceToken': 'arn:aws:lambda:eu-west-2:536099501702:function:efar-elasticsearch-test3-SingletonLambda1c23d47474-BWVU5Q9UOYAF', 'ResponseURL': 'https://cloudformation-custom-resource-response-euwest2.s3.eu-west-2.amazonaws.com/arn%3Aaws%3Acloudformation%3Aeu-west-2%3A536099501702%3Astack/efar-elasticsearch-test3/e5e9b9a0-00b2-11ea-b5e6-02317fefc7aa%7Cessubstackdbinitialiser8B803743%7C4a954972-49c7-461f-8578-869fae154ff1?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Date=20191106T164218Z&X-Amz-SignedHeaders=host&X-Amz-Expires=7199&X-Amz-Credential=AKIAZYWU4JB3S7SC2XN6%2F20191106%2Feu-west-2%2Fs3%2Faws4_request&X-Amz-Signature=65ca19224c3cac34e954e46dedcba9cf9a21c996117ac84349f0df6c0a549528', 'StackId': 'arn:aws:cloudformation:eu-west-2:536099501702:stack/efar-elasticsearch-test3/e5e9b9a0-00b2-11ea-b5e6-02317fefc7aa', 'RequestId': '4a954972-49c7-461f-8578-869fae154ff1', 'LogicalResourceId': 'essubstackdbinitialiser8B803743', 'ResourceType': 'AWS::CloudFormation::CustomResource', 'ResourceProperties': {'ServiceToken': 'arn:aws:lambda:eu-west-2:536099501702:function:efar-elasticsearch-test3-SingletonLambda1c23d47474-BWVU5Q9UOYAF', 'Es_endpoint': 'search-efar-db-test3-ayfzxubo47v3xtswcvdrakbnxu.eu-west-2.es.amazonaws.com'}}

Specifically:

'ResourceProperties':{
    'ServiceToken': 'arn:aws:lambda:eu-west-2:536099501702:function:efar-elasticsearch-test3-SingletonLambda1c23d47474-BWVU5Q9UOYAF',
    'Es_endpoint': 'search-efar-db-test3-ayfzxubo47v3xtswcvdrakbnxu.eu-west-2.es.amazonaws.com'
}

Environment

  • CLI Version : 1.15.0
  • Framework Version: 1.15.0
  • OS : Linux
  • Language : Python

Other

Personally, I'd rather properties keep the name I give them - it's unintuitive for things to rename themselves. However, this would be a breaking change, so it would probably be best to simply change the documentation to highlight this feature.


This is 🐛 Bug Report

@TomDufall TomDufall added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 7, 2019
@eladb
Copy link
Contributor

eladb commented Nov 8, 2019

This is totally a redudant feature. We will document and also remove it under a "future flag" (a new thing).

@eladb eladb added the p2 label Nov 8, 2019
@eladb eladb self-assigned this Nov 8, 2019
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Nov 13, 2019
@ashatch
Copy link

ashatch commented Apr 22, 2020

I bumped into this today using Netlify's deployment lambda as a custom resource. They require properties with specific capitalization (e.g. build_settings, netlifyToken etc.)

eladb pushed a commit that referenced this issue May 1, 2020
This commit folds the `CustomResource` and `NestedStack` types from `@aws-cdk/aws-cloudformation` into `@aws-cdk/core` in order to allow code in `core` and other lower layers to use capabilities such as nested stacks and custom resources.

This comes at a minor sacrifice to API fidelity: the provider's service token is for custom resources is now passed as a simple `string` instead of a strongly typed `ICustomResourceProvider`. But this is negligible for this type of resource given the high involvement users require to use it anyway. Additionally, the `NestedStack` class accepts a `notificationArns` as a `string[]` instead of an `sns.ITopic[]`. In both cases the API in `@aws-cdk/aws-cloudformation` (which is considered a stable module) remains unchanged with a compatibility layer added.

We took this opportunity to change the behavior of custom resources so that it won't pascal-case property names by default. This resolves #4896 and resolves #7035 and supersedes #7034.

The API in the aws-cloudformation module are still supported for backwards compatibility but marked as deprecated.
eladb pushed a commit that referenced this issue May 1, 2020
This commit folds the `CustomResource` and `NestedStack` types from `@aws-cdk/aws-cloudformation` into `@aws-cdk/core` in order to allow code in `core` and other lower layers to use capabilities such as nested stacks and custom resources.

This comes at a minor sacrifice to API fidelity: the provider's service token is for custom resources is now passed as a simple `string` instead of a strongly typed `ICustomResourceProvider`. But this is negligible for this type of resource given the high involvement users require to use it anyway. Additionally, the `NestedStack` class accepts a `notificationArns` as a `string[]` instead of an `sns.ITopic[]`. In both cases the API in `@aws-cdk/aws-cloudformation` (which is considered a stable module) remains unchanged with a compatibility layer added.

We took this opportunity to change the behavior of custom resources so that it won't pascal-case property names by default. This resolves #4896 and resolves #7035 and supersedes #7034.

The API in the aws-cloudformation module are still supported for backwards compatibility but marked as deprecated.
@mergify mergify bot closed this as completed in #7736 May 4, 2020
mergify bot pushed a commit that referenced this issue May 4, 2020
This commit folds the `CustomResource` and `NestedStack` types from `@aws-cdk/aws-cloudformation` into `@aws-cdk/core` in order to allow code in `core` and other lower layers to use capabilities such as nested stacks and custom resources.

This comes at a minor sacrifice to API fidelity: the provider's service token is for custom resources is now passed as a simple `string` instead of a strongly typed `ICustomResourceProvider`. But this is negligible for this type of resource given the high involvement users require to use it anyway. Additionally, the `NestedStack` class accepts a `notificationArns` as a `string[]` instead of an `sns.ITopic[]`. In both cases the API in `@aws-cdk/aws-cloudformation` (which is considered a stable module) remains unchanged with a compatibility layer added.

We took this opportunity to change the behavior of custom resources so that it won't pascal-case property names by default. This resolves #4896 and resolves #7035 and supersedes #7034.

The API in the aws-cloudformation module are still supported for backwards compatibility but marked as deprecated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants