-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
refactor: remove existing vpc endpoint configurations from base module and move into sub-module #635
refactor: remove existing vpc endpoint configurations from base module and move into sub-module #635
Conversation
…e and move into sub-module BREAKING: VPC endpoint resources are removed in favor of sub-module and `for_each` creation of any VPC endpoints BREAKING: AWS provider has been updated from v2.70 to v3.15 for VPC endpoint changes and available attributes
bac41ed
to
bb60b18
Compare
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.
Looks very good. Large cleanup. I commented just a few small things.
@@ -0,0 +1,34 @@ | |||
variable "vpc_id" { | |||
description = "The ID of the VPC in which the endpoint will be used" | |||
type = string |
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.
Let's add create
boolean flag to be able to disable this module the same way as we do in the VPC. For this reason there should be default
in every variable.
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.
I added the variable in 5c6a231 but its not working as I think it should. Its giving me an error as shown below, not sure why this isn't working, thoughts? (note - the complete-vpc
example as its currently written does not work with the create functionality)
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.
This is one of my favorite error messages :) objects vs objects battle...
Try writing like:
endpoints = var.create ? tomap(var.endpoints) : tomap({})
Ref: https://github.com/terraform-aws-modules/terraform-aws-route53/blob/master/modules/zones/main.tf#L2
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.
ah, perfect - thank you. I was able to resolve with 0416cec - check it out when you have some time
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.
Yes, this is the best way. :)
@antonbabenko friendly reminder 😉 |
examples/complete-vpc/main.tf
Outdated
@@ -1,20 +1,28 @@ | |||
provider "aws" { | |||
region = "eu-west-1" | |||
region = local.region |
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.
I just hit this issue when using Terraform 0.15.0 - hashicorp/terraform-provider-aws#8040 (comment)
Works well when I put a value there as a string:
region = "eu-west-1"
2. Remove the gateway endpoint route table association separate resources. The route table associations are now managed in the VPC endpoint resource itself via the map of maps provided to the VPC endpoint sub-module. Perform the necessary removals for each route table association and for S3 and/or DynamoDB depending on your configuration: | ||
|
||
``` | ||
terraform state rm 'module.vpc.aws_vpc_endpoint_route_table_association.intra_dynamodb[0]' |
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.
I also verified all these terraform state
commands. Works as expected.
v3.0.0 has been just released. 🎉 🎉 🎉 🎉 🎉 🎉 |
So if I am reading this correctly, the migration from 2.x to 3.x requires manual admin intervention to modify state for every use of this module, with the potential that a mis-typed Or if we don't use state commands, we take a traffic double-blip on all endpoints as they are destroyed and then re-created. This is a very disruptive breaking change. I hope the 2.x line will continue to get updates if they required. |
Version 2.x won't be getting updates. We support just one major release at a time. The process of upgrading from 2.x to 3.x indeed requires manual runs of I am pretty confident that Upgrade Notes are correct, too. |
Ok, thank you for confirming. I do want to express that this is quite painful for a large system where infrastructure-as-code has been implemented strictly, and virtually no one can run manual state commands. I realize there was some pain in maintaining this in the module, but the fix has pushed that pain onto every invocation of this module by every consumer. |
hi @tabletcorry - we do our best to avoid breaking changes but as with all software projects, you reach a point where you have to make a significant change to continue making forward progress and best supporting the needs of those who use the module(s). the prior versions are all there for use and can be pinned (recommended) to avoid any breaking updates, but ultimately, it is up to each individual if they want to take on the challenge in updating in order to receive the latest changes. we do feel that this change was a long time coming - AWS releases VPC endpoints all the time and the prior solution was not scalable nor was it flexible enough for most folks (based on issues and PRs that we received). again, pinning the module version is going to be the safest route to avoid any disruptions until folks are ready to make the necessary changes to update (it is an opt in, its not something that is forced upon anyone) |
How does How do we map the state shifts? |
So, not sure how we migrate individual security groups for endpoints still.. However for others: SES Example:
I cannot figure out how to provide both |
In addition to ses / email-smtp, I ran into “efs” to “elasticfilesystem”. It seems like it might be useful to have a list of some of the more common ones since |
@Routhinator you cannot associate the VPC endpoint with more than one subnet in the same AZ and why I suspect your permutations are failing. if you are creating a VPCe for SES, its one association in each AZ (typically via the private subnets) |
* Fixed formatting * Updated CHANGELOG * Add Output Of Subnet ARNs (terraform-aws-modules#242) * Add Output Of Subnet ARNs Facilitates resource access manager, subnet sharing across accounts * Update Readme For Subnet ARN Output * Fixed formatting * Updated CHANGELOG * Improving DHCP options docs (terraform-aws-modules#260) * Improving DHCP options docs * generating README from variables description * Updated CHANGELOG * ECS endpoint (terraform-aws-modules#261) * add ecs vpc endpoints * add ecs vpcendpoints outputs * add ecs vpc endpoints to readme inputs/outputs table * add ecs vpc endpoints to readme endpoint list * Added VPC endpoints for SQS (closes terraform-aws-modules#248) * Updated CHANGELOG * Finally, Terraform 0.12 support (terraform-aws-modules#266) * run terraform 0.12upgrade * Cleanup for Terraform 0.12 (closes terraform-aws-modules#265, terraform-aws-modules#228) * Fixed merge conflicts * Updated CHANGELOG * Upgrade Docker Image to fix CI (terraform-aws-modules#270) * Added VPC Endpoints for SNS, Cloudtrail, ELB, Cloudwatch (terraform-aws-modules#269) * Updated CHANGELOG * Updated Terraform versions in README * Updated CHANGELOG * Fixed opportunity to create the vpc, vpn gateway routes (bug during upgrade to 0.12) * Updated CHANGELOG * Fixed broken 2.3.0 * Updated CHANGELOG * Updated CHANGELOG * Update tflint to 0.8.2 for circleci task (terraform-aws-modules#280) * Updated VPC endpoint example (fixed terraform-aws-modules#249) * Updated CHANGELOG * Updated pre-commit-terraform to support terraform-docs and Terraform 0.12 (terraform-aws-modules#288) * Updated CHANGELOG * Updated version of pre-commit-terraform * adding secrets manager vpc end point support * adding config vpc end point support * adding codebuild, codecommit and git-codecommit vpc end point support * adding transfer server vpc end point support * Added Kinesis streams and firehose VPC endpoints (terraform-aws-modules#301) * Fixed README after merge * Updated CHANGELOG * Output var.name (terraform-aws-modules#303) * Fixed README after merge * Updated CHANGELOG * Add IPv6 support (terraform-aws-modules#317) * IPv6 support Add variable "enable_ipv6" to allow enabling IPv6 support (resulting in passing "assign_generated_ipv6_cidr_block" to aws_vpc. Enabling IPv6 support further results in an Egress-only internet gateway being provisioned and routing tables of subnets being adjusted. Additional variables allow to choose the indices out of the /64 subnets based on the assigned /56 range. * Add example for IPv6 usage * Remove redundant parameter assign_generated_ipv6_cidr_block This is needed exactly when var.enable_ipv6 is true. * Set subnet ipv6_cidr_block to null if unused * Be picky about spelling * Revert unrelated change * More IPv6 spelling * Added IPv6 support to VPC module * Added IPv6 support to VPC module * Updated CHANGELOG * Added more VPC endpoints (Glue, STS, Sagemaker Notebook), and all missing outputs (terraform-aws-modules#311) * AWS Glue VPC endpoint * sagemaker notebook vpc endpoint * correct service name for notebook endpoint * added outputs kinesis * added endpoints output glue, fix readme * endpoint for STS * endpoint for STS (variables) * Added more VPC endpoints (Glue, STS, Sagemaker Notebook), and all missing outputs * Updated CHANGELOG * Added tags to VPC Endpoints (terraform-aws-modules#292) * Add tags to VPC Endpoints * Update variables.tf add new line between variables * centralize vpce tag param * fix s3 tags * Updated README * Updated README * Updated README * Updated CHANGELOG * Added support for ICMP rules in Network ACL (terraform-aws-modules#286) * Added icmp_code and icmp_type values to non default acl rules. * Added support for both ICMP and non-ICMP rules in NACL * Updated CHANGELOG * Added support for EC2 ClassicLink (terraform-aws-modules#322) * Updated CHANGELOG * Add VPC endpoints for CloudFormation, CodePipeline, Storage Gateway, AppMesh, Transfer, Service Catalog & SageMaker(Runtime & API) (terraform-aws-modules#324) * add new endpoints * merge change from master * Updated CHANGELOG * Added VPC Endpoints for AppStream, Athena & Rekognition (terraform-aws-modules#335) * Updated CHANGELOG * Added support for `ipv6_cidr_block` in network acls (terraform-aws-modules#329) Add support for `ipv6_cidr_block` in `*_{in|out}bound_acl_rules`. As a conseqeunce, the (ipv4) `cidr_block` is made optional. * Updated network-acls example with IPv6 rules * Updated CHANGELOG * Fixed spelling mistakes * Updated CHANGELOG * Add Elastic File System & Cloud Directory VPC Endpoints (terraform-aws-modules#355) * Updated CHANGELOG * Update TFLint to v0.12.1 for circleci (terraform-aws-modules#351) Update TFLint to v0.12.1 * Added Customer Gateway resource (terraform-aws-modules#360) * Updated CHANGELOG * fix ipv6 enable (terraform-aws-modules#340) * Updated CHANGELOG * Added note about Transit Gateway integration (terraform-aws-modules#386) * Updated CHANGELOG * Updated pre-commit-terraform with terraform-docs 0.8.0 support (terraform-aws-modules#388) * Updated CHANGELOG * Set minimum terraform version to 0.12.6 (fixes circleci) (terraform-aws-modules#390) * Updated CHANGELOG * Added support for both types of values in azs (names and ids) (terraform-aws-modules#370) * Updated CHANGELOG * Add VPC Flow Logs capabilities (terraform-aws-modules#316) * Updated CHANGELOG * Fixed output of aws_flow_log * Updated CHANGELOG * Add support for specifying AZ in VPN Gateway (terraform-aws-modules#401) * Updated CHANGELOG * Added tagging for VPC Flow Logs (terraform-aws-modules#407) * Updated CHANGELOG * [ci skip] Create "LICENSE". * [ci skip] Create ".pre-commit-config.yaml". * [ci skip] Create "Makefile". * [ci skip] Create ".gitignore". * [ci skip] Create ".editorconfig". * [ci skip] Create ".chglog/CHANGELOG.tpl.md". * Updated CHANGELOG * chore: Add badge for latest version number (terraform-aws-modules#384) * Updated CHANGELOG * feat: Add intra subnet VPN route propagation (terraform-aws-modules#421) * Updated CHANGELOG * docs: Document create_database_subnet_group requiring database_subnets (terraform-aws-modules#424) I ran into the same issue as terraform-aws-modules#126 because there's nothing in the documentation making it clear that `create_database_subnet_group` only has an effect if you also specify `database_subnets`. * feat: Add EC2 Auto Scaling VPC endpoint (terraform-aws-modules#374) * docs: Updated required versions of Terraform * Updated CHANGELOG * added owner_id output (#1) * fix: Updated outputs in ipv6 example (terraform-aws-modules#375) * feat: Add routes table association and route attachment outputs (terraform-aws-modules#398) * Updated CHANGELOG * feat: Add VPC Endpoint for SES (terraform-aws-modules#449) * Updated CHANGELOG * feat: Added support for more VPC endpoints (terraform-aws-modules#369) * Updated CHANGELOG * fix: Fix wrong ACM PCA output (terraform-aws-modules#450) * Updated CHANGELOG * Updated description of vpc_owner_id * docs: Fixed README * Updated CHANGELOG * feat: Enable support for Terraform 0.13 as a valid version by setting minimum version required (terraform-aws-modules#455) * Updated CHANGELOG * feat: Add support for tagging egress only internet gateway (terraform-aws-modules#430) * feat: Add support for VPC flow log max_aggregation_interval (terraform-aws-modules#431) * Updated pre-commit hooks * Updated CHANGELOG * fix: Output list of external_nat_ips when using external eips (terraform-aws-modules#432) * Updated CHANGELOG * fix: Reorder tags to allow overriding Name tag in route tables (terraform-aws-modules#458) * Updated CHANGELOG * feat: add support for disabling IGW for public subnets (terraform-aws-modules#457) * Updated CHANGELOG * feat: manage default security group (terraform-aws-modules#382) * Updated CHANGELOG * docs: Fix typo in nat_public_ips (terraform-aws-modules#460) * fix: bumping terraform version from 0.12.6 to 0.12.7 in circleci to include regexall function (terraform-aws-modules#474) * feat: bump version of aws provider version to support 3.* (terraform-aws-modules#479) * Updated CHANGELOG * fix: InvalidServiceName for elasticbeanstalk_health (terraform-aws-modules#484) * Updated CHANGELOG * feat: add arn outputs for: igw, cgw, vgw, default vpc, acls (terraform-aws-modules#471) * Updated CHANGELOG * fix: Use database route table instead of private route table for NAT gateway route (terraform-aws-modules#476) * Updated CHANGELOG * feat: Add ability to create RDS endpoint to VPC (terraform-aws-modules#499) * Updated CHANGELOG * feat: Add ability to create CodeDeploy endpoint to VPC (terraform-aws-modules#501) * Updated CHANGELOG * feat: add enable_public_s3_endpoint variable for S3 VPC Endpoint for public subnets (terraform-aws-modules#502) * Updated CHANGELOG * feat: Added tflint as pre-commit hook (terraform-aws-modules#507) * Updated CHANGELOG * feat: Add support for security groups ids in default sg's rules (terraform-aws-modules#491) * Updated CHANGELOG * fix: Split appstream to appstream_api and appstream_streaming (terraform-aws-modules#508) * Updated CHANGELOG * feat: Added Textract vpc endpoint (terraform-aws-modules#509) * docs: Updated docs with pre-commit * Updated CHANGELOG * fix: Create only required number of NAT gateways (terraform-aws-modules#492) * Updated CHANGELOG * revert: Create only required number of NAT gateways (terraform-aws-modules#492) (terraform-aws-modules#517) * Updated CHANGELOG * feat: Added support for Terraform 0.14 (terraform-aws-modules#525) * Updated CHANGELOG * fix: Removed ignore_changes to work with Terraform 0.14 (terraform-aws-modules#526) * Updated CHANGELOG * fix: Resource aws_default_network_acl orphaned subnet_ids (terraform-aws-modules#530) * Updated CHANGELOG * Fixed circleci configs * fix: Updated min required version of Terraform to 0.12.21 (terraform-aws-modules#532) * Updated CHANGELOG * feat: Added Codeartifact API/Repo vpc endpoints (terraform-aws-modules#515) * Updated README * Updated CHANGELOG * feat: Lambda VPC Endpoint (terraform-aws-modules#534) * Updated CHANGELOG * docs: Updated README and pre-commit (terraform-aws-modules#537) * Updated CHANGELOG * feat: Adding vpc_flow_log_permissions_boundary (terraform-aws-modules#536) * Updated CHANGELOG * docs: Clarifies default_vpc attributes (terraform-aws-modules#552) * Updated CHANGELOG * fix: Adding missing RDS endpoint to output.tf (terraform-aws-modules#563) * feat: Adding VPC endpoint for DMS (terraform-aws-modules#564) * Updated CHANGELOG * fix: Fixed wrong count in DMS endpoint (terraform-aws-modules#566) * Updated CHANGELOG * fix: Specify an endpoint type for S3 VPC endpoint (terraform-aws-modules#573) * Updated CHANGELOG * feat: Upgraded minimum required versions of AWS provider to 3.10 (terraform-aws-modules#574) * Updated CHANGELOG * chore: update documentation based on latest `terraform-docs` which includes module and resource sections (terraform-aws-modules#594) * Updated CHANGELOG * chore: add ci-cd workflow for pre-commit checks (terraform-aws-modules#598) * fix: Correctly manage route tables for database subnets when multiple NAT gateways present (terraform-aws-modules#518) * Updated CHANGELOG * fix: aws_default_security_group was always dirty when manage_default_security_group was set (terraform-aws-modules#591) * chore: Adds database_subnet_group_name as an output variable (terraform-aws-modules#592) * Updated CHANGELOG * chore: Updated the conditional creation section of the README (terraform-aws-modules#584) * fix: use filter for getting service type for S3 endpoint and update to allow s3 to use interface endpoint types (terraform-aws-modules#597) * Updated CHANGELOG * feat: add vpc endpoint policies to supported services (terraform-aws-modules#601) * feat: add vpc endpoint policies to supported services * chore: empty commit to re-run * chore: Run pre-commit terraform_docs hook Co-authored-by: Anton Babenko <[email protected]> * Updated CHANGELOG * fix: Remove CreateLogGroup permission from service role (terraform-aws-modules#550) * Updated CHANGELOG * feat: add default route table resource to manage default route table, its tags, routes, etc. (terraform-aws-modules#599) * Updated CHANGELOG * chore: align ci-cd static checks to use individual minimum Terraform versions (terraform-aws-modules#606) * chore: update documentation and pin `terraform_docs` version to avoid future changes (terraform-aws-modules#619) * feat: Add outpost support (subnet, NACL, IPv6) (terraform-aws-modules#542) * Updated CHANGELOG * refactor: remove existing vpc endpoint configurations from base module and move into sub-module (terraform-aws-modules#635) * Updated CHANGELOG * chore: update CI/CD to use stable `terraform-docs` release artifact and discoverable Apache2.0 license (terraform-aws-modules#643) * chore: Private DNS cannot be used with S3 endpoint (terraform-aws-modules#651) * chore: Removed link to cloudcraft * Updated CHANGELOG * feat: Added database_subnet_group_name variable (terraform-aws-modules#656) * Updated CHANGELOG * fix: Fixed SID for assume role policy for flow logs (terraform-aws-modules#670) * fix: Fixed mistake in separate private route tables example (terraform-aws-modules#664) * docs: Added ID of aws_vpc_dhcp_options to outputs (terraform-aws-modules#669) Co-authored-by: Anton Babenko <[email protected]> * Updated CHANGELOG * fix: Update the terraform to support new provider signatures (terraform-aws-modules#678) * Updated CHANGELOG * fix: Return correct route table when enable_public_redshift is set (terraform-aws-modules#337) * Updated CHANGELOG * feat: Added device_name to customer gateway object. (terraform-aws-modules#681) * Updated CHANGELOG * feat: Add support for naming and tagging subnet groups (terraform-aws-modules#688) * Updated CHANGELOG * Updated pre-commit * chore: Updated outputs in example (terraform-aws-modules#690) * docs: Fixed docs in simple-vpc * Updated CHANGELOG * feat: Added support for VPC Flow Logs in Parquet format (terraform-aws-modules#700) * Updated CHANGELOG * feat: Added timeout block to aws_default_route_table resource (terraform-aws-modules#701) * Updated CHANGELOG * fix: Enabled destination_options only for VPC Flow Logs on S3 (terraform-aws-modules#703) * Updated CHANGELOG * feat: Add tags to VPC flow logs IAM policy (terraform-aws-modules#706) * Updated CHANGELOG * fix: update CI/CD process to enable auto-release workflow (terraform-aws-modules#711) * Downgraded provider version to match current version in main repo * Removing Error Blocks * Renamed VPC * Upgraded AWS provider version * Downgraded AWS provider version Co-authored-by: Anton Babenko <[email protected]> Co-authored-by: Blaine Schanfeldt <[email protected]> Co-authored-by: Andreas Wittig <[email protected]> Co-authored-by: Ilia Lazebnik <[email protected]> Co-authored-by: Niklas Wagner <[email protected]> Co-authored-by: Christian Kemper <[email protected]> Co-authored-by: Edward Viaene <[email protected]> Co-authored-by: Ben Sykes <[email protected]> Co-authored-by: Edward Viaene <[email protected]> Co-authored-by: Thomas Baumann <[email protected]> Co-authored-by: Bas Zoetekouw <[email protected]> Co-authored-by: Günter Grodotzki <[email protected]> Co-authored-by: Eytan Hanig <[email protected]> Co-authored-by: Miguel Ferreira <[email protected]> Co-authored-by: betajobot <[email protected]> Co-authored-by: Betajobot <[email protected]> Co-authored-by: Christoph Bünte <[email protected]> Co-authored-by: Chris Adams <[email protected]> Co-authored-by: Pablo Serrano <[email protected]> Co-authored-by: V Malinics <[email protected]> Co-authored-by: Allan Simon <[email protected]> Co-authored-by: quentin9696 <[email protected]> Co-authored-by: Jarosław Wygoda <[email protected]> Co-authored-by: Bryant Biggs <[email protected]> Co-authored-by: Clément L <[email protected]> Co-authored-by: Jonas Kint <[email protected]> Co-authored-by: Michal Schott <[email protected]> Co-authored-by: Xaaris <[email protected]> Co-authored-by: Bradley Mickunas <[email protected]> Co-authored-by: Hao CHEN <[email protected]> Co-authored-by: Volodymyr Zahorniak <[email protected]> Co-authored-by: Yauheni Batsianouski <[email protected]> Co-authored-by: Hao CHEN <[email protected]> Co-authored-by: Pablo Serrano <[email protected]> Co-authored-by: Jeremy Ciak <[email protected]> Co-authored-by: Larry Aiello <[email protected]> Co-authored-by: Kelsey M <[email protected]> Co-authored-by: Lucas Albertine de Godoi <[email protected]> Co-authored-by: Andor Markus <[email protected]> Co-authored-by: Mohamed El Mouctar Haidara <[email protected]> Co-authored-by: Diego Rodriguez <[email protected]> Co-authored-by: Oliver L Schoenborn <[email protected]> Co-authored-by: Damien Gustave <[email protected]> Co-authored-by: ae-ou <[email protected]> Co-authored-by: Léo Gillot-Lamure <[email protected]> Co-authored-by: Alex Bryant <[email protected]> Co-authored-by: Rob Lazzurs <[email protected]> Co-authored-by: Yoni Leitersdorf <[email protected]> Co-authored-by: hieultan <[email protected]> Co-authored-by: Dmytro Oboznyi <[email protected]> Co-authored-by: Jamie Starke <[email protected]> Co-authored-by: Sebastian Korfmann <[email protected]> Co-authored-by: Joao Gilberto Magalhaes <[email protected]> Co-authored-by: Juho Majasaari <[email protected]> Co-authored-by: itsmeremz <[email protected]> Co-authored-by: drewmullen <[email protected]> Co-authored-by: Mukesh Sharma <[email protected]> Co-authored-by: Kamil Aliev <[email protected]> Co-authored-by: rajgandhi9 <[email protected]> Co-authored-by: Mayank Aggarwal <[email protected]>
…e and move into sub-module (terraform-aws-modules#635)
…e and move into sub-module (terraform-aws-modules#635)
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 have 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. |
Description
for_each
creation of any VPC endpointsMotivation and Context
Closes #454
Closes #462
Closes #560
Closes #582
Closes #603
Closes #604
Closes #607
Closes #608
Closes #611
Closes #613
Closes #618
Closes #626
Closes #630
Closes #631
Closes #633
Closes #634
Closes #638
Closes #640
Breaking Changes
Yes:
for_each
creation of any VPC endpointsHow Has This Been Tested?
examples/*
projectscomplete-vpc
example