cloudposse / terraform-aws-dynamic-subnets

Terraform module for public and private subnets provisioning in existing VPC
https://cloudposse.com/accelerate
Apache License 2.0
191 stars 165 forks source link

Fixed deprecated warning, using network_interface_id instead #156

Closed ktasper closed 2 years ago

ktasper commented 2 years ago

what

Using the current version you get a deprecated warning:

│ Warning: Argument is deprecated
│ 
│   with module.dynamic-subnets.aws_route.nat_instance,
│   on .terraform/modules/dynamic-subnets/nat-instance.tf line 130, in resource "aws_route" "nat_instance":
│  130:   instance_id            = element(aws_instance.nat_instance.*.id, count.index)
│ 
│ Use network_interface_id instead

I have updated the code to use the recommended network_interface_id, as a result I have also had to point it to primary_network_interface_id.

Gowiem commented 2 years ago

@ktasper Thanks for the fix. Can you please lookup when network_interface_id was introduced / when instance_id was deprecated? We'll need to check if we need to bump the required version of the AWS provider alongside these changes because I could see this not being backwards compatible.

Gowiem commented 2 years ago

/test all

ktasper commented 2 years ago

@ktasper Thanks for the fix. Can you please lookup when network_interface_id was introduced / when instance_id was deprecated? We'll need to check if we need to bump the required version of the AWS provider alongside these changes because I could see this not being backwards compatible.

Sure.It was deprecated this release:

I cannot see when network_interface_id was added but a git blame [shows commits from 2017-03-14:] (https://github.com/hashicorp/terraform-provider-aws/blob/main/internal/service/ec2/vpc_route.go#L128)

ee1410d3a86 resource_aws_route.go                      (Paul Stack              2017-03-14 12:41:40 +0200 128)                  "network_interface_id": {

Not sure if that helps.

Gowiem commented 2 years ago

@ktasper That's great. network_interface_id has been around forever as you've found and instance_id is deprecated only in 4.0+ which is fine. I think that makes this a very backwards compatible change, so we should be good to just merge. I'll do so after tests pass.

Gowiem commented 2 years ago

/test all

Gowiem commented 2 years ago

@ktasper Thanks for the contribution! Released as https://github.com/cloudposse/terraform-aws-dynamic-subnets/releases/tag/0.40.1