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

Deprecation Warning For Input/Output Variables #18381

Open
jonbrouse opened this issue Jul 3, 2018 · 26 comments · May be fixed by #36079 or #36016
Open

Deprecation Warning For Input/Output Variables #18381

jonbrouse opened this issue Jul 3, 2018 · 26 comments · May be fixed by #36079 or #36016

Comments

@jonbrouse
Copy link

jonbrouse commented Jul 3, 2018

As open sources modules become more popular and teams in organizations create Terraform modules that depend on the remote state of another team's module, there is a need to communicate the evolution of a module.

This request is proposing the addition of an optional deprication_warning parameter to the output and input variable configuration. The depreciation notification would display when a module references the deprecated output via remote state.

Terraform Configuration Files Example

For this example we will first view the outputs.tf file from v1.0.0 of an example module.

output "route53_zone_id" {
  value = "${aws_route53_zone.route53_zone.zone_id}"
}

Below is the outputs.tf file from v1.1.0 of our example module. The output variable route53_zone_id has been updated to primary_zone_id.

output "route53_zone_id" {
  value = "${aws_route53_zone.primary_zone.zone_id}"
  deprication_warning = "The route53_zone_id output variable has been deprecated. Please use primary_zone_id"
}

output "primary_zone_id" {
  value = "${aws_route53_zone.primary_zone.zone_id}"
}

Expected Behavior

When the apply is run in the root module that references the output, a depreciation warning would be displayed

Warning: The route53_zone_id output variable has been deprecated. Please use primary_zone_id

Another option would be to use deprication_replacement instead of deprication_warning. This would allow Terraform to have more control over the message.

output "route53_zone_id" {
  value = "${aws_route53_zone.primary_zone.zone_id}"
  deprication_replacement = "primary_zone_id"
}

Output of terraform apply

Warning: module.our_example: "route53_zone_id": [DEPRECATED] Use primary_zone_id instead
@apparentlymart
Copy link
Contributor

Hi @jonbrouse! Thanks for sharing this use-case.

Being able to mark things as deprecated definitely seems like a good idea. For outputs in particular things are tricky though, because Terraform doesn't currently analyze their usage in a way that would permit the conditional warning you suggest here. That doesn't mean it's impossible, of course, but we'll need to think about the best way to achieve it within Terraform's internal architecture, and probably also to accept that the warning won't be displayed in some edge cases where the attribute is accessed dynamically.

Supporting this idea for variable blocks would be easier, I think, because we're already verifying statically that each of the arguments in a module block correspond to a variable block with suitable settings.

We generally lean towards shorter names for these built-in arguments inside variable and output blocks, so I'd lean towards naming the argument merely deprecated to keep things concise:

output "route53_zone_id" {
  value      = "${aws_route53_zone.primary_zone.zone_id}"
  deprecated = "Use primary_zone_id instead."
}

output "primary_zone_id" {
  value = "${aws_route53_zone.primary_zone.zone_id}"
}

With the new warning message style coming in the next major release, that might generate something like this:

Warning: Deprecated output value

  on example.tf line 21, in resource "aws_route53_record" "example":
  21:   zone_id = module.example.route53_zone_id

The output value route53_zone_id is deprecated. Use primary_zone_id instead.

The development branch for the next major release has some significant changes to the configuration loader already, so we'll need to hold on this for now at least until that work is completed and merged. We'll consider this some more for a subsequent release.

Thanks again for this suggestion!

@jonbrouse
Copy link
Author

Hey @apparentlymart thanks for the reply. Y'all are doing awesome work!

@jasonmcintosh
Copy link
Contributor

Assuming Hashicorp is considering adding support for deprecating parameters - are you looking to add it on variables as well? Be nice to support "module version 1.2.2 deprecates this variable, instead use X to be removed in 1.3" kinda thing :)

@slavaaaaaaaaaa
Copy link

I'm seeking this as well, more for general use than just outputs.

In my case, I'm deprecating an internal company module in favour of including its resources in another module. Procedures for this require state moves, so embedding such instructions in the to-be-deprecated module would be ideal.

@rodrigodiez
Copy link

I am also interested in this behaviour. As we evolve our infrastructure we have to change our modules. Being able to warn engineers about deprecations is a key feature to help them migrate their code base gradually

@oswalya
Copy link

oswalya commented Oct 19, 2020

I would also be interested in seeing this. Any update?

@TrueCyberAxe
Copy link

Any progress?

@ghostsquad
Copy link

Is this on the roadmap?

@bryantbiggs
Copy link

I hate to be the +1 guy but bumping for visibility and maybe Hashicorp's current thoughts on this enhancement 🙏🏽

@apparentlymart
Copy link
Contributor

As far as I'm aware, there is not yet any clear design for how to reliably determine when referring to a particular output value in a calling module, and I believe solving that would be the main blocker for implementing something like this.

Using the names from the original writeup as a placeholder, module.example.route53_zone_id is a straightforward case that I think we could handle today, but for a module using for_each we may see module.example["foo"].route53_zone_id which requires a more detailed level of static analysis than Terraform has needed before.

It's also possible to use an entire module instance as a value and pass it around as an object, which would require even more sophisticated static analysis to resolve fully. For example:

module "example" {
  # ...
}

locals {
  intermediate = {
    for a, v in module.example : a => v if contains(["route53_zone_id", "domain_name", a])
  }
}

output "zone_id" {
  value = local.intermediate.route53_zone_id
}

If is of course debatable how many of these more complicated cases are important to handle, but deciding that will itself require research, and so will be a part of formulating a design for this.

@bryantbiggs
Copy link

I wonder if there is a more generic way that this could be handled. a construct like the move block that has been added, where users can simply provide a deprecation notice and its up to them to provide the text. I agree that with the deeply nested constructs of for_each maps and count it can be hard to tell whats what, but I can see a very clear use case today where I simply want to output a notice of pending changes in a plan/apply. I guess this also raise another question of verbosity as well in terms of plan/apply outputs

@md5
Copy link

md5 commented Apr 4, 2022

It this issue about deprecation of outputs only or variables as well? I see that #19138 has been marked as a duplicate of this issue, but that issue is about variables, not outputs. What we're interested on the team I work with is marking inputs as deprecated, not so much outputs.

I feel like the two issues are separate (as is deprecation of modules), but if this issue is about both it seems like the description should be updated to reflect that.

@j-flat
Copy link

j-flat commented Jun 6, 2022

It this issue about deprecation of outputs only or variables as well? I see that #19138 has been marked as a duplicate of this issue, but that issue is about variables, not outputs. What we're interested on the team I work with is marking inputs as deprecated, not so much outputs.

I feel like the two issues are separate (as is deprecation of modules), but if this issue is about both it seems like the description should be updated to reflect that.

Like @md5 I would also be more interested in marking input variables as deprecated, where migrating from old input variables to new ones would be made more clear with these warnings.

@quentin9696
Copy link

Hi,

We are also interest on that feature on the variables. Do you have any update on this?

Thanks for your great job!

@pasathish
Copy link

Hi, We are also interested in marking input variables as deprecated. Any updates on this?

@InsanesTheName
Copy link

Throwing my hat into the ring as well - I also would love to have this functionality (for both variables and outputs)!

@crw
Copy link
Contributor

crw commented Nov 3, 2022

Just a reminder to please use the 👍 reaction on the original post to upvote issues - we do sort by most upvoted to understand which issues are the most important. This also reduces "noise" in the notification feed for folks following this issue. Thanks!

Unfortunately there are no other updates to share on this feature request. Please keep upvoting, we do look at that to help prioritize community requests.

@apparentlymart apparentlymart changed the title Deprecation Warning For Output Variables Deprecation Warning For Output Values Nov 3, 2022
@rquadling
Copy link

If consideration is ever given to this, one particular use case I'm having to deal with is how to advertise a deprecated root module output that would be accessed via the remote state provider of Terraform. This is in addition to module consumers.

We've found the remote state provider very useful in reducing the amount of lookups (and so performance improvements) when small projects need to deal with some infrastructure, and then share out the ID/ARNs or some elements in a structured way (for example, ALBs, what listeners are active and expected to be used, etc.).

It's not a major headache, but a simple "you are using a deprecated output" would be SO useful.

@daodennis-ri
Copy link

Does this issue cover deprecation warnings for output values or does it include inputs too?

@tom-reinders
Copy link

The closed as duplicate issue #19138 would make me assume it includes inputs too, but the title of #18381 issue makes it confusing

@wzooff
Copy link

wzooff commented May 15, 2024

I agree with @tom-reinders. And because I don't have a proper issue, I will post my workaround here.

locals {
  deprecated_variables = toset(compact([
    length(var.list_name) > 0 ? "list_name" : "",
    var.string_name != "" ? "string_name" : "",
  ]))
}

resource "null_resource" "deprecated_variable" {
  for_each = local.deprecated_variables
}

If someone used/specified deprecated variables, respective null_resources will be created.

It works in conjunction with CI on the consumer side. You can parse terraform plan and notify in PR (tested in Spacelift with notification/plan policies) that it would be nice to migrate from deprecated parameters. I have mentioned deprecations in the variable description as well.

@jonbrouse
Copy link
Author

Updated the issue to explicitly include input and output variables.

@jonbrouse jonbrouse changed the title Deprecation Warning For Output Values Deprecation Warning For Input/Output Variables Sep 11, 2024
@fatbasstard
Copy link

Any progress on this? Doesn't seem to be all too complicated to implement.

My case is that I'm building a lot of (private and public) modules used by development teams. And features/changes can move pretty fast. It's really a burden now to update/migrate/remove certain code because there is no way to announce it in the code itself.

I'd love to mark both variables as outputs as deprecated as (another way of) communicating that things will change

@dbanck dbanck linked a pull request Nov 15, 2024 that will close this issue
@DanielMSchmidt DanielMSchmidt linked a pull request Nov 22, 2024 that will close this issue
@ferricoxide
Copy link

Came here from #19138, too. Looking in the thread, not seeing that this has been addressed, yet (and not seeing anything in the TF documentation to indicate that it might have been adressed but just not noted as such, here).

Absent an actual deprecation-warning capability, anyone got a method for using input-variable's validation {} logic to "fake it" that they could point me to?

@mk-armah
Copy link

Hey Hashicorp, any update on this feature request ? super important

@lkwg82
Copy link

lkwg82 commented Nov 29, 2024

Have a look at opentofu: https://www.env0.com/blog/opentofu-v1-8-a-detailed-look-at-the-upcoming-release

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet