Stack wide setting to use construct ids as names

[jsteinich]

Fixes #248

Just add stack.useConstructIdsAsNames() and all constructs will use their id as a name instead of generating one from their path and a hash.

This should make it much easier to convert existing Terraform stacks when a destroy/rebuild isn't feasible.

[skorfmann]

Overall, this looks interesting. A few things we'll have to consider though:

friendlyUniqueId is just a workaround for aws/constructs#120 at the moment. Ideally, this logic would move into the constructs package again at some point.

For the terraform -> cdktf migration path, you'd probably end up with a flat structure anyway which makes handling potential duplicates straightforward. However, if you would pull in 3rd party constructs for some reason, this might cause quite some headaches for the user. Perhaps it could make sense to think about this as an option on Resource and TerraformElement, so that this could be set as needed?

[skorfmann]

Couldn't this be an option for the constructor?

[jsteinich]

It could be. I was going for more of a this is something non-default to opt in to, but maybe that's just my Java background showing through.

Or maybe this is actually more of a long term different mode that people would continue to use than just as a transition.

[jsteinich]

friendlyUniqueId is just a workaround for aws/constructs#120 at the moment. Ideally, this logic would move into the constructs package again at some point.

I do like the separator, but is there a functional reason for it?
Even that change happened, I think the logic of when to use a unique id vs a construct id would fall inside cdktf. I could make a new method that is used in preparation for that though.

Perhaps it could make sense to think about this as an option on Resource and TerraformElement, so that this could be set as needed?

I agree that having a more granular way is useful. I was planning to leave that as a follow up task, but I could loop that work in with this. I have some of that in #277. I could imagine wanting control over entire categories, but that may just be overkill.

[skorfmann]

friendlyUniqueId is just a workaround for aws/constructs#120 at the moment. Ideally, this logic would move into the constructs package again at some point.

I do like the separator, but is there a functional reason for it?
Even that change happened, I think the logic of when to use a unique id vs a construct id would fall inside cdktf. I could make a new method that is used in preparation for that though.

Probably right, the switch would still be here I suppose.

Perhaps it could make sense to think about this as an option on Resource and TerraformElement, so that this could be set as needed?

I agree that having a more granular way is useful.

We should think about it a bit. Perhaps a setting like this shouldn't cascade down into nested constructs automatically?

[jsteinich]

I took a step back and searched the AWS CDK repo for details about their logical ids. While Terraform isn't the same as CloudFormation, I think all of the same arguments apply (except maybe character and length limitations), so it seems reasonable to follow their example.

Here are a few resources that I found useful:

• Thoughts about resource IDs aws/aws-cdk#1424
• https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_core.Stack.html#protected-allocate-wbr-logical-wbr-idcfnelement
• https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_core.CfnResource.html#override-wbr-logical-wbr-idnewlogicalid

As a result of my research, I opened #329. I feel that meets the more pressing need of easing transitions from Terraform to cdktf.
I think the concepts of overriding id generation strategy or individual ids still has merit and is likely something we'll want to add; however, I think that can wait.

[skorfmann]

As a result of my research, I opened #329. I feel that meets the more pressing need of easing transitions from Terraform to cdktf.

I like that proposal.

I think the concepts of overriding id generation strategy or individual ids still has merit and is likely something we'll want to add; however, I think that can wait.

Agreed 👍

[jsteinich]

Going to close this in favor or #329

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.