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

Deprecate vpc_classic_link #824

Closed
marcofranssen opened this issue Aug 29, 2022 · 6 comments · Fixed by #826
Closed

Deprecate vpc_classic_link #824

marcofranssen opened this issue Aug 29, 2022 · 6 comments · Fixed by #826

Comments

@marcofranssen
Copy link

Is your request related to a new offering from AWS?

Is this functionality available in the AWS provider for Terraform? See CHANGELOG.md, too.

  • Yes ✅: 4.28.0

Is your request related to a problem? Please describe.

╷
│ Warning: Argument is deprecated
│ 
│   with module.vpc.aws_vpc.this,
│   on .terraform/modules/vpc/main.tf line 27, in resource "aws_vpc" "this":
│   27:   enable_classiclink               = var.enable_classiclink
│ 
│ With the retirement of EC2-Classic the enable_classiclink attribute has
│ been deprecated and will be removed in a future version.
│ 
│ (and 3 more similar warnings elsewhere)
╵

Describe the solution you'd like.

Implement a migration path once the classiclink is completely removed from the AWS provider.

Describe alternatives you've considered.

Additional context

Warning shows up since we bumped the AWS provider to 4.28.0.

@lorengordon
Copy link
Contributor

Fyi, since this module is setting enable_classiclink to null by default, there is a chance this is a bug. I've opened an upstream issue. Please give it a 👍 !

hashicorp/terraform#31730

@bryantbiggs
Copy link
Member

Thank you @lorengordon for that. While that is rather annoying, I'm leaning towards maybe just setting it to null on the argument here in the module. Some could view that as a breaking change, but since AWS made this decision and that is the direction (and I suspect it affects a very, very small segment of users), I'm inclined in making that change since it seems like the easiest path to resolution with the least disruption. @antonbabenko what do you think about setting

enable_classiclink = var.enable_classiclink
to a hardcoded null?

@lorengordon
Copy link
Contributor

lorengordon commented Sep 1, 2022

You might as well just remove the argument from the resource, in that case... It's the same effect to terraform and the provider and the users, and less code.

@lorengordon
Copy link
Contributor

The harder "backwards incompatible" bit for this module, imo, would be the removal of the variable, var.enable_classiclink. Since for any user that is setting that variable in their config, even if false or null, they would then need to remove it to avoid an error when upgrading.

Certainly, users that are relying on that variable and setting it to true, they're going to run into an problem with that feature being deprecated at the AWS API. They've got some work anyway.

Anyway, just my two cents. I always pin module versions and test updates, so I'm happy as long as the deprecation warning goes away. 😛

@antonbabenko
Copy link
Member

This issue has been resolved in version 3.14.4 🎉

@github-actions
Copy link

I'm going to lock this issue 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 similar to this, 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 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants