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.

Sign up for GitHub

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

Jump to bottom

Top-level L2 resources should have unmangled logical IDs #1687

Closed
eladb opened this issue Feb 6, 2019 · 2 comments
Closed

Top-level L2 resources should have unmangled logical IDs #1687

eladb opened this issue Feb 6, 2019 · 2 comments
Assignees
@eladb
Labels
@aws-cdk/core Related to core CDK functionality effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. package/cfn Related to the CFN layer (L1) pr/breaking-change This PR is a breaking change. It needs to be modified to be allowed in the current major version.

Comments

@eladb
Copy link
Contributor

eladb commented Feb 6, 2019 •
edited
Loading

Customers expect that when they add L2 resources directly to the Stack, the id they provide for the L2 will be the logical ID of the underlying L1 resource. This will also clean up the logical IDs of deCDK resource (#1618).

Aside from this being a huge breaking change, and we should probably allow people to disable this behavior for backwards compat. I don't see why we shouldn't implement this heuristic.

Change logical ID to "Default" (or ".") and then defaultChild (#2290) should be the way to access these resources.

@rix0rrr @RomainMuller @sam-goodwin what do you guys think?
@eladb eladb added pr/breaking-change This PR is a breaking change. It needs to be modified to be allowed in the current major version. @aws-cdk/core Related to core CDK functionality feature-request A feature should be added or improved. labels Feb 6, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 6, 2019 •
edited
Loading

I like it (honestly I would have liked this from the start), but this will be a MASSIVE breaking change.

Easy fix is to disregard "Resource" path part, just like "Defafult" currently is.

@eladb eladb added the package/cfn Related to the CFN layer (L1) label Apr 15, 2019
@eladb eladb removed the package/awscl Cross-cutting issues related to the AWS Construct Library label May 1, 2019
@eladb eladb self-assigned this Aug 12, 2019
@eladb
Copy link
Contributor Author

eladb commented Nov 11, 2019

This is something we should consider for v2.0 (added to the v2.0 issue)

@eladb eladb added the effort/small Small work item – less than a day of effort label Jan 23, 2020
@eladb eladb mentioned this issue Jan 23, 2020
Closed
12 tasks
@eladb eladb mentioned this issue Feb 4, 2020
Closed
18 tasks
@eladb eladb closed this as completed Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@eladb eladb

Labels
@aws-cdk/core Related to core CDK functionality effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. package/cfn Related to the CFN layer (L1) pr/breaking-change This PR is a breaking change. It needs to be modified to be allowed in the current major version.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rix0rrr @eladb