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

feature: support aws_vpc_ipam cascade for easier deletes #23714

Closed
drewmullen opened this issue Mar 16, 2022 · 8 comments · Fixed by #23973
Closed

feature: support aws_vpc_ipam cascade for easier deletes #23714

drewmullen opened this issue Mar 16, 2022 · 8 comments · Fixed by #23973
Labels
enhancement Requests to existing resources that expand the functionality or scope. good first issue Call to action for new contributors looking for a place to start. Smaller or straightforward issues. service/ec2 Issues and PRs that pertain to the ec2 service.

Comments

@drewmullen
Copy link
Collaborator

drewmullen commented Mar 16, 2022

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

Description

AWS IPAM released a new argument cascade which works like a force_delete. Add argument to the resource schema

Default should be false

New or Affected Resource(s)

  • aws_vpc_ipam

Potential Terraform Configuration

resource "aws_vpc_ipam" "test" {
  operating_region {
    region_name = "us-east-1"
  }
  cascade = true
}

References

@drewmullen drewmullen added the enhancement Requests to existing resources that expand the functionality or scope. label Mar 16, 2022
@drewmullen drewmullen changed the title feature: support aws_vpc_ipam cascade for easy delets feature: support aws_vpc_ipam cascade for easier deletes Mar 16, 2022
@drewmullen drewmullen added the good first issue Call to action for new contributors looking for a place to start. Smaller or straightforward issues. label Mar 22, 2022
@gmichelo
Copy link
Contributor

Anybody working on this?

@drewmullen
Copy link
Collaborator Author

drewmullen commented Mar 26, 2022

@BigMikes not me! At least not for a couple weeks based on priority

@AurelienNober
Copy link
Contributor

Hello @drewmullen ,
I wanted to have a first look at the internals and thought that this feature could be a good way to do that. Just fyi I use the aws provider on a weekly basis and not for IPAM.
I locally added the cascade option and made it work but I had a few thoughts about the use case. If everything is defined within terraform state, the option is quite useless because all the vpc_ipam dependencies will be destroyed before the vpc_ipam is itself destroyed. Then the main use case for the cascade option would be to ensure resources created outside of terraform are destroyed too. Am I correct? If yes then for the acceptance test, does calling the create ipam scope api directly (to make sure there is a scope created outside of terraform state) sound like a good idea?

@drewmullen
Copy link
Collaborator Author

drewmullen commented Mar 29, 2022

@AurelienNober

If everything is defined within terraform state, the option is quite useless because all the vpc_ipam dependencies will be destroyed before the vpc_ipam is itself destroyed. Then the main use case for the cascade option would be to ensure resources created outside of terraform are destroyed too

You're 100% correct. The option has very low use-case. In fact, I would personally use the cli command instead if i wanted to use cascade. I considered not opening the issue but ended up opening it because it is a feature of the API and there is a usecase, just a small one.

does calling the create ipam scope api directly (to make sure there is a scope created outside of terraform state) sound like a good idea

No if the feature is included in the provider then it should mimic the API behaviors. Scopes are not the only resource that can be a blocker to deleting: pools, provisioned CIDRs in those pools, or allocations to VPCs can also be a hinder an IPAM deletion.

@AurelienNober
Copy link
Contributor

Thanks for you reply!

No if the feature is included in the provider then it should mimic the API behaviors. Scopes are not the only resource that can be a blocker to deleting: pools, provisioned CIDRs in those pools, or allocations to VPCs can also be a hinder an IPAM deletion.

Ok if I understand correctly, for the acceptance test, creating a scope by api (to create a potential deletion blocker) is ok but not enough as all blockers should be part of the test.

@drewmullen
Copy link
Collaborator Author

Oh, i'm sorry I misunderstood! Yes an extra scope is a perfectly valid test of the functionality! You should only need to test 1 type so Scope is valid

@AurelienNober
Copy link
Contributor

Thanks for your input, tried to do something consistent

@ewbankkit ewbankkit added the service/ec2 Issues and PRs that pertain to the ec2 service. label Mar 31, 2022
@github-actions
Copy link

github-actions bot commented May 5, 2022

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. good first issue Call to action for new contributors looking for a place to start. Smaller or straightforward issues. service/ec2 Issues and PRs that pertain to the ec2 service.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants