feat!: Support enabling NAU metrics in "aws_vpc" resource

[tcharewicz]

Description

List of backwards incompatible changes

• The minimum required Terraform version is now 1.0
• The minimum required AWS provider version is now 4.x (4.35.0 at time of writing)
• assign_ipv6_address_on_creation has been removed; use the respective subnet type equivalent instead (i.e. - public_subnet_assign_ipv6_address_on_creation)
• enable_classiclink has been removed; it is no longer supported by AWS Subtle issue representing null as unset value, resulting in deprecation warning hashicorp/terraform#31730
• enable_classiclink_dns_support has been removed; it is no longer supported by AWS Subtle issue representing null as unset value, resulting in deprecation warning hashicorp/terraform#31730

Additional changes

Modified

• map_public_ip_on_launch now defaults to false
• enable_dns_hostnames now defaults to true
• enable_dns_support now defaults to true
• manage_default_security_group now defaults to true
• manage_default_route_table now defaults to true
• manage_default_network_acl now defaults to true
• The default name for the default security group, route table, and network ACL has changed to fallback to append -default to the VPC name if a specific name is not provided
• The default fallback value for outputs has changed from an empty string to null

Motivation and Context

• Resolves Support enabling NAU metrics in "aws_vpc" resource #837
• Resolves Ability turn on subnet enable_resource_name_dns_a_record_on_launch #891
• Resolves resource-name based hostnames #906

Breaking Changes

• Yes; see UPGRADE-4.0.md guide

How Has This Been Tested?

• I have updated at least one of the examples/* to demonstrate and validate my change(s)
• I have tested and validated these changes using one or more of the provided examples/* projects
• I have executed pre-commit run -a on my pull request

[jczerniak]

looks fine

[lorengordon]

Looks like 4.35.0 was just released, so this should be good to go now?

[bryantbiggs]

this is now a breaking change

[tcharewicz]

What can I do to get this PR reviewed?

[github-actions]

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

[tcharewicz]

@antonbabenko What can I do to get this PR reviewed?

[hopkinsnji]

Any eta on this?

[jonathan-mothership]

@antonbabenko 🙏

[lorengordon]

Could this be updated to also support this attribute on the aws_default_vpc resource, probably through a separate variable, default_vpc_enable_network_address_usage_metrics? For reasoning, see hashicorp/terraform-provider-aws#27841...

[yotixify]

Is there any update on when this could be merged?

[bryantbiggs]

apologies that this hasn't been addressed earlier. I was holding out for https://github.com/clowdhaus/terraform-aws-vpc-v4 but I have not had the time recently to dedicate to finishing and plotting the path from v3 to v4. However, I think its time we unblock some of these newer features for VPC and I will revisit the larger, architectural changes after

Let me check over a few of the past issues/PRs and see what else we might want to incorporate in this breaking change. The goal will be a "light" breaking change where state moves are not required - mostly just a breaking change because the min required versions of Terraform and AWS provider have been raised and some default values changed. Once I have gone through that I'll pass it over to Anton for a final review and we should be able to get this shipped out. Thank you all for your patience!

[Constantin07]

Hi @antonbabenko, conscious you are busy but please could you review this PR ?

[antonbabenko]

examples/complete works well, including the upgrade phase.

[bryantbiggs]

@antonbabenko if you get some time would you mind taking another look at this PR - thank you 🙏🏽

[antonbabenko]

@bryantbiggs I plan to review this one and the one in the ECS module during the live stream tomorrow (Friday, 7th of April).

Thank you very much for the work on all of these!

[antonbabenko]

LGTM. Please merge whenever you can or want.

[antonbabenko]

This PR is included in version 4.0.0 🎉

[adamretter]

I am not sure if this is the correct place, but I just want to add a note that we have been severely bitten by the change of the default value of:

manage_default_security_group now defaults to true

Previously we were using version 3.16.1 of terraform-aws-modules/vpc/aws (where the default for manage_default_security_group was false).

We bumped the version number to 4.0.1 and ran terraform init -upgrade. All went well.

We later ran terraform apply and I don't recall seeing any concerning messages in the plan about changes to the default Security Group. However, after that we found out that we could no longer send traffic between the subnets in our VPC.

With version 3.16.1, when the VPC is created by Terraform, the default Security Group for the VPC (named -) has one Inbound rule:

Name	Security group rule ID	IP version	Type	Protocol	Port range	Source	Description
–	sgr-09f96200588e82c90	-	All traffic	All	All	sg-0a72e8cefc737b2e8 / default	–

and one Outbound rule:

Name	Security group rule ID	IP version	Type	Protocol	Port range	Destination	Description
–	sgr-05e09415ca9a9246e	IPv4	All traffic	All	All	0.0.0.0/0	-

After upgrading to version 4.0.1 the default Security Group for the VPC (now named my_vpc-default) has exactly zero Inbound and Outbound rules.

Flipping the manage_default_security_group from true to false and back again makes no difference, the default Security Group for the VPC remains with zero inbound and outbound rules.

---

Whilst I don't mind the default value of manage_default_security_group changing per-se, I did not however expect Terraform to delete the default Inbound and Outbound rules (that it previously created via the VPC module) for the default Security Group of the default VPC - thus breaking our operations until I was able to diagnose the problem.

---

I was able to reinstate the default Security Groups that Terraform had previously created for us in VPC Module 3.16.1 by adding the following to our VPC Module Config:

  manage_default_security_group = true

  default_security_group_ingress = [
    {
      description = "Allow all"
      protocol = -1
      self = true
    }
  ]

  default_security_group_egress = [
    {
      description = "Allow all"
      protocol = -1
      from_port = 0
      to_port = 0
      cidr_blocks = "0.0.0.0/0"
    }
  ]

p.s. the documentation shows that I should write cidr_blocks = ["0.0.0.0/0"]. However this raises a type error at runtime:

The given value is not suitable for module.vpc.var.default_security_group_egress declared at .terraform/modules/vpc/variables.tf:1345,1-41: element 0: element "cidr_blocks": string required

[github-actions]

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.