-
-
Notifications
You must be signed in to change notification settings - Fork 18
feat: support deleting default VPCs in multiple regions #94
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
base: main
Are you sure you want to change the base?
Conversation
Add support for specifying a region in the awsutils_default_vpc_deletion resource, allowing deletion of default VPCs outside the provider's default region. Example usage for multiple regions and for_each is included. Updates resource schema and logic to handle the new region argument.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request adds multi-region support to the default VPC deletion resource. A new optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/service/ec2/resource_default_vpc_deletion_test.go`:
- Around line 92-94: The test currently dereferences conn.Config.Region without
checking for nil; update the assertion around the EC2 client region (the block
that uses conn.Config.Region) to first verify conn.Config and conn.Config.Region
are not nil and fail the test with a clear message if they are (e.g., t.Fatalf
or t.Errorf), otherwise proceed to compare *conn.Config.Region with
resourceRegion; reference the conn variable and its Config.Region field when
adding the nil checks.
🧹 Nitpick comments (2)
examples/resources/awsutils_default_vpc_deletion/resource.tf (1)
26-47: Consider documenting the duplicate region overlap.
eu-west-1appears in both the single-region example (line 23) and thevar.regionslist (line 35). When both resources are applied together, this would attempt to delete the same default VPC twice. Consider adding a comment noting this is intentional for demonstration purposes, or removing the duplicate fromvar.regions.internal/service/ec2/resource_default_vpc_deletion.go (1)
129-132: Consider adding region context to the delete log message.For consistency with the Create log (line 92), include region information in the Delete log message.
Proposed enhancement
func resourceDefaultVpcDeletionDelete(d *schema.ResourceData, meta any) error { - log.Printf("[INFO] Removing default VPC deletion state") + region := d.Get("region").(string) + log.Printf("[INFO] Removing default VPC deletion state for region %s", region) return nil }
The CodeQL workflow now analyzes only Go code, removing javascript-typescript from the language matrix. This streamlines security analysis to focus on the project's only language.
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
what
This pull request enhances the
awsutils_default_vpc_deletionresource to support deleting default VPCs in multiple or specific AWS regions, rather than being limited to the provider's default region. It also refactors the implementation to allow specifying the region per resource and improves documentation and logging.Multi-region and region-specific support:
regionargument to theawsutils_default_vpc_deletionresource, allowing users to specify which AWS region's default VPC to delete. If not specified, it defaults to the provider's region.resource.tf) to show how to delete the default VPC in a specific region, and how to delete default VPCs in multiple regions usingfor_eachand aregionsvariable.Implementation refactoring:
getEC2ConnForRegionto obtain an EC2 client for a specific region, supporting the newregionargument in resource operations.Create,Read, andDelete) to use the region-aware EC2 connection, and to store the region in Terraform state. [1] [2] [3] [4]Documentation and logging:
why
awsutils_default_vpc_deletionresource, allowing deletion of default VPCs outside the provider's default region. Example usage for multiple regions and for_each is included. Updates resource schema and logic to handle the new region argument.references
Closes #92