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

fix: Update tags for default resources to correct spurious plan diffs #730

Merged

Conversation

bryantbiggs
Copy link
Member

@bryantbiggs bryantbiggs commented Jan 11, 2022

Description

  • default_xxx_name default values changed to null; still works the same due to use of coalesce(...)
  • Name tags for default resources changed to use coalesce(var.default_xxx_name, "${var.name}-default") except for default_vpc_name because its not tied to this module (aka - created by this module)
  • Add default_route_table_name to match other default resource conventions (tag)
  • Change default_security_group_ingress and default_security_group_egress values to []; using null will fail - updated and tested in simple-vpc - does not affect users because in order for this to work for them they will have had to set the rules so its a no-op for users. This also matches the default route table routes values as well
  • Clean up usage of format(...)
  • Fix complete example to correct generic endpoint policy to not use DynamoDB endpoint but VPC CIDR and for DynamoDB endpoint to use correct data source to get the VPC endpoint ID; closes DynamoDB VPC Endpoint Policy in the complete example denies all DynamoDB requests #680
  • Update example outputs to include full breadth of outputs to aid in testing - more broader coverage of any potential changes
  • Update local variable vpc_id to use try(); replace nearly all instances of aws_vpc.this[0].id with local.vpc_id except for the CIDR block association
  • Clean up local.nat_gateway_ips hack now that issue is resolved and we are requiring min version of 0.13.1

Motivation and Context

Breaking Changes

  • No; these are all backwards compatible

How Has This Been Tested?

  • I have tested and validated these changes using one or more of the provided examples/* projects
    • Tested with complete-vpc and simple-vpc

@bryantbiggs bryantbiggs changed the title fix: update tags for default resources to correct spurious plan diffs fix: Update tags for default resources to correct spurious plan diffs Jan 11, 2022
@bryantbiggs
Copy link
Member Author

one of these days I'll get the title right on the first go 😅

@bryantbiggs
Copy link
Member Author

should be ready for review now

Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Now, when we don't have format("%s", ...) let's hope that nobody calls their VPC true which will be interpreted as a boolean in tags :)

examples/simple-vpc/main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
@bryantbiggs
Copy link
Member Author

@antonbabenko should be all set now if you get some time to take another 👀

@antonbabenko antonbabenko merged commit d1adf74 into terraform-aws-modules:master Jan 13, 2022
antonbabenko pushed a commit that referenced this pull request Jan 13, 2022
### [3.11.3](v3.11.2...v3.11.3) (2022-01-13)

### Bug Fixes

* Update tags for default resources to correct spurious plan diffs ([#730](#730)) ([d1adf74](d1adf74))
@antonbabenko
Copy link
Member

This PR is included in version 3.11.3 🎉

@bryantbiggs bryantbiggs deleted the fix/default-names branch January 13, 2022 14:16
harrythebot pushed a commit to lolocompany/terraform-aws-vpc that referenced this pull request May 11, 2022
### [3.11.3](terraform-aws-modules/terraform-aws-vpc@v3.11.2...v3.11.3) (2022-01-13)

### Bug Fixes

* Update tags for default resources to correct spurious plan diffs ([terraform-aws-modules#730](terraform-aws-modules#730)) ([d1adf74](terraform-aws-modules@d1adf74))
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants