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

Remove custom mappings for resources that are supported by Cloud Control API #554

Open
8 tasks
xiehan opened this issue Nov 27, 2023 · 1 comment
Open
8 tasks
Labels
backlog breaking-change An issue or PR that involves a breaking change. cloudcontrol Issues related to CloudControl resources and manual mappings enhancement New feature or request

Comments

@xiehan
Copy link
Member

xiehan commented Nov 27, 2023

Description

In #547 we did another update of the list of resources supported by Cloud Control. Many of the ones added in this update already had custom mappings in https://github.com/hashicorp/cdktf-aws-cdk/tree/main/src/mapping. For now, the custom mappings will continue to take precedence over the CC implementation. We decided to keep it this way at least for this release because removing a mapping and having it be handled by CC would cause somebody's resource to be destroyed and recreated, which would be a pretty dramatic breaking change for existing users of this adapter.

We do, however, need to rip off the bandaid at some point and remove these custom mappings. My proposal is to do so before the GA release of this adapter.

Custom mappings that can/should be removed as of November 2023:

  • AWS::EC2::VPC
  • AWS::EC2::Subnet
  • AWS::EC2::Route
  • AWS::EC2::RouteTable
  • AWS::EC2::VPCGatewayAttachment
  • AWS::ElasticLoadBalancingV2::LoadBalancer
  • AWS::Events::Rule
  • AWS::IAM::Role

References

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
@xiehan xiehan added enhancement New feature or request backlog cloudcontrol Issues related to CloudControl resources and manual mappings labels Nov 27, 2023
@xiehan xiehan added this to the General Availability milestone Nov 27, 2023
@xiehan xiehan added the breaking-change An issue or PR that involves a breaking change. label Nov 27, 2023
@sapslaj
Copy link

sapslaj commented Feb 21, 2024

As of this writing, there doesn't seem to be a way to disable the built-in custom mappings as they are global. So there are two things in my mind that would make removing them more viable:

  1. Refactor mappings to be tied to a particular AwsTerraformAdapter instance instead of being global.
  2. Add a config/props type for AwsTerraformAdapter and add an option like enableBuiltinMappings that defaults to true. That way downstream consumers can just disable that config option and instantly see what the effects are without having to jump between major versions.

I'll admit it's quite a bit of overhead for a library that isn't GA yet, but I think a little bit of effort could be made to at least make the change less disruptive. Currently there isn't even a good way to revert those custom mappings since the object that holds those mappings is both global and unexported and you may only add new mappings or lookup an existing mapping by key; you cannot easily remove mappings. This is exacerbated by createGenericCCApiMapping not being exported so you can't just registerMapping('AWS::EC2::VPC', createGenericCCApiMapping('AWS::EC2::VPC')) to get the non-custom mappings yourself; it's a much more tedious process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog breaking-change An issue or PR that involves a breaking change. cloudcontrol Issues related to CloudControl resources and manual mappings enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants