Allow variables to be marked as deprecated to communicate variable removal to module consumers

[reegnz]

trafficstars

OpenTofu Version

1.6.0-beta4

Use Cases

Over the lifecycle of an opentofu module, one might need to remove variables from a module. It would be great to have a mechanism to notify users that they are using a deprecated variable that is scheduled for a future removal.

Attempted Solutions

• keep maintaining backward compatibility of your modules, never break clients (works a lot of the time)
• notify module users out of band using semver version bumps and release notes (difficult to pre-alert programmatically, major version bump results in instant abandonment of the module user if the bugfixes are not ported back to multiple release branches)
• build a deprecation solution based on comments - needs to fetch modules, parse hcl, be aware of terraform semantics of module variable usage, etc. too big task for me

Proposal

Introduce a new field into the variable declaration that is used to define a deprecation message:

variable "this_is_my_variable" {
  type = string
  description = "This is a variable for the old way of configuring things"
  deprecation = "This variable will be removed on 2024-12-31. Use that_is_my_variable instead".
}

Whenever a user is using a deprecated variable of the module, they will get a warning message on tofu validate, tofu plan and tofu apply invocations as long as the variable is still being passed to the module.

The deprecation message is freeform text, so it could contain a URL to migration instructions too.

References

Other languages support communicating deprecation to software component users.

• java has the @Deprecated annotation and even had @deprecated javadoc comments in Java 1.1, 26 years ago.
• go has a comment-based deprecation mechanism: https://zchee.github.io/golang-wiki/Deprecated/
• jsdoc for javascript/typescript also solves this in comments
• terraform providers already have a way to signal deprecations to users, it would make sense to make that feature a 1st class citizen for modules as well: https://developer.hashicorp.com/terraform/plugin/sdkv2/best-practices/deprecations

[reegnz]

Just noticed there's a 5 year old feature request for this in terraform: https://github.com/hashicorp/terraform/issues/18381

[janosdebugs]

Hi @reegnz this is a cool idea, I like it. However, I believe this would make module code incompatible with Terraform, which some module authors probably would have an issue with.

We are currently working towards a stable OpenTofu release, I think this proposal could be revisited after that release is out. I added a label to that effect.

[cam72cam]

I think it's a neat idea and would be useful, but share the same concern as @janosdebugs. We need to figure out how to handle that compatibility going forward. I wonder if a comment annotation would be a reasonable workaround, though it would definitely be a bit of a hack.

[stephen-dahl]

Just noticed there's a 5 year old feature request for this in terraform: hashicorp/terraform#18381

That issues title and comments specifically reference output values not variables. I think separate issues for these things is good since we know how to implement this for variables but still don't know how to handle outputs. Let variables be implemented then circle back to outputs when someone figures out a plan.

[janosdebugs]

Maybe a good way to implement this would be to detect a deprecated line in the description field like this:

variable "this_is_my_variable" {
  type = string
  description = "This is a variable for the old way of configuring things. Deprecated: This variable will be removed on 2024-12-31. Use that_is_my_variable instead."
}

The rule could be that the deprecation notice must always be at the end of the description and must have the format of:

Deprecated: explanation.

OpenTofu could then extract this text and display it as a warning to the user. This would work for both variables and outputs and could be implemented in one go.

@reegnz what do you think?

[lorengordon]

I think parsing descriptions is bound to be flaky. I'd maybe suggest a custom "deprecation" provider, to maintain compatibility with Hashicorp Terraform.

[janosdebugs]

@lorengordon how would that work?

[lorengordon]

Just the way providers work. Write a resource with the behavior and workflow you want. Something like, take the deprecated variable value and an deprecation message as an input, generate an error with the deprecation message if the variable value is not null. Or take inspiration from variable validation, and take a condition argument that let's the user decide when to trigger the error.

[janosdebugs]

@lorengordon this wouldn't work in on outputs, and it also wouldn't work if the user passed in the default value of a variable explicitly. Both of these would trip up someone if the variable/output in question went away.

[lorengordon]

This issue is about variable deprecation, not outputs. That would be different anyway. Would probably want the warning to occur when a user accesses the deprecated output. That's going to require changes to terraform core, no matter what.

But the provider idea certainly would work if the user passed in the default value explicitly. The module author would be responsible for determining what happens in that scenario. There are options. The resource could also simply take another input for the default value, and decide how to handle the scenario when the value passed in matches the default. Or the condition expression could handle that itself, up to the author.

[Yantrio]

Now that the stable release has been out for some time we are happy to accept contributions on this issue. We feel it is valid.

In my personal opinion, I feel it may be best to ensure that we as a community agree on the approach before development starts though.

We should also be careful to make sure that we narrow down the scope to only variable blocks as described in the original issue.

[sanskruti-shahu]

Hey, I would like to take up this issue.

[janosdebugs]

Hey @sanskruti-shahu thank you for volunteering, I've assigned you. Before you begin coding, please read this section of the contribution guide.

[cube2222]

Other than the above, I propose that we use a more "special string" than just "Deprecated: ".

E.g.

variable "this_is_my_variable" {
  type = string
  description = "This is a variable for the old way of configuring things. @deprecated: This variable will be removed on 2024-12-31. Use that_is_my_variable instead."
}

To display the warnings, I think you can just return them as part of the diags struct (as a warning) during the planning phase and they will be propagated up and displayed.

I don't think there needs to be any special apply-time handling of this.

[cam72cam]

Perhaps @Deprecated{message} or some other enclosing mark?

[sanskruti-shahu]

I agree with @cube2222 , I will go through the codebase once and will let you know about changes I'll be making before implementing them.

[skyzyx]

@cam72cam said:

I think it's a neat idea and would be useful, but share the same concern as @janosdebugs. We need to figure out how to handle that compatibility going forward. I wonder if a comment annotation would be a reasonable workaround, though it would definitely be a bit of a hack.

I actually really like this idea, and if an update to the HCL parser would enable first-class support for features-by-comment, this could open some new doors. (I've used the HCL parser, but never for trying to read comments, so I don't recall if it's possible at-present.)

The current reality is that we live in a mixed ecosystem where people/orgs/companies will be spending the next few years transitioning tools (assuming they plan to come over to OpenTofu). If anyone remembers Mark Pilgrim (Dive into Mark, assassinated his digital presence in 2010), he wrote a really great maxim:

Be liberal in what you accept, but conservative in what you produce.

The ability to opt-in and enable new features that are not broadly compatible is interesting.

# @deprecated: This variable will be removed on 2024-12-31. Use that_is_my_variable instead.
variable "this_is_my_variable" {
  type = string
  description = "This is a variable for the old way of configuring things."
}

I think that a well-defined syntax for this would be in order.

`@` + KEYWORD + COLON + SPACE + DATA
# @deprecated: This variable will be removed on 2024-12-31.

And if we look at the pattern used by conventional commits, we can do a little more.

`@` + KEYWORD + [ `(` + ALT [ , ALT ] + `)` ] + COLON + SPACE + DATA
# @terratest(custom_param): abcd1234
# @spacelift(otherCustomParam): zyxw9876
# @other_community_tool(other-other-custom-param): lmnop654

I would caution, however, that we take the time to learn from the W3C's mistakes with -webkit, -moz, and -ms prefixes in CSS. Many vendors implemented things in slightly different ways, and web devs made the mistake of just -webkit-ing everything. (i.e., Don't @spacelift something if @env0, @terragrunt, or others plan to implement the same thing.)

I would hope that with a spec, we could get community + tooling support (across HashiCorp, the OTF, and others) for adding features via comments that could eventually make their way into the core HCL parser(s), like browser engines have.

Something like a @deprecated comment (experimental, incubating) could eventually graduate to a proper deprecated = "" property supported by all tools in the ecosystem.

I would love to be able to say — for tools like tofuenv and tenv — something like:

terraform {
  # @opentofu(required_version): ~> 1.6.0
  required_version = ">= 0.13, < 2.0"

  required_providers {
    # https://github.com/hashicorp/terraform-provider-aws/releases/latest
    aws = {
      source  = "hashicorp/aws"
      version = ">= 4.0, < 6.0"
    }
  }
}

tfenv and tfswitch would read required_version while tofuenv and tenv would first read @opentofu(required_version) if it exists, and if not, fall back to required_version. Nobody's HCL parser freaks out and crashes.

---

(CSS had it easy because browsers simply ignored things it didn't understand. The HCL parser throws errors/panics.)

[sanskruti-shahu]

Hey @skyzyx, I got your point but I think reading comment for deprecation message would be quite complex than directly checking out variable's description.

[cube2222]

Alright, we've discussed this internally, and have a hard time deciding between the two approaches:

1. Adding it as an explicit deprecated field in the variable block.
2. Adding it as part of the description.

The comment approach would most likely require us to for the HCL repo, which isn't something we want to do right now.

Thus, we've decided to submit this to the steering committee for decision. This might take a week or two to resolve, until the steering committee will meet.

[reegnz]

TBQH the solution that I find likely the easiest to implement and produces the least disruptive change is adding that @depracated into the description itself.

I suggest going with that and add it as an experiment, and gather further input from the community based on that. One important note about that is that HCL supports multi-line heredocs, so that needs to be considered when parsing the description, it can contain newlines.

Eg.:

variable "this_is_my_variable" {
  type        = string
  description = <<-END
  @deprecated 
  use that_is_my_variable instead
  END
}

output "this_is_my_output" {
  value       = "x"
  description = <<-END
  @deprecated 
  use that_is_my_output instead
  END
}

}

[cube2222]

The steering committee has yesterday rejected the approach of using implicit keywords in the description string.

Instead, this issue should be frozen for now, until we at least have an RFC for #1275. Afterwards, we should reevaluate the approach of using an explicit field for this.

[reegnz]

So until then the next best thing is to try and build the deprecation warnings into a plugin for tflint: https://github.com/terraform-linters/tflint/blob/master/docs/user-guide/plugins.md

[janosdebugs]

Hey folks, we have discussed this with the core team and believe that this should only be done once #1328 is done.

[sftim]

I hope if we come up with a design, it also works for deprecating locals.

[cam72cam]

@sftim can you elaborate on deprecating locals? The main goal behind this issue is to communicate to consumers of a module that the inputs (and probably outputs) have changed are will not be supported in a future version.

[sftim]

Deprecation could look, for example, like this:

-locals {
-   foo = data.foo_bar.json_value
-   
-   foo_json = jsonencode(data.foo.value)
-}
+#[deprecated]
+locals {
+   # use local.foo_json instead
+   # the values are NOT equivalent, see JIRA-42
+   foo = data.foo_bar.json_value
+}
+locals {
+   foo_json = jsonencode(data.foo.value)
+}

Having two different deprecation mechanisms would be annoying to coders and to people explaining it all.

[sftim]

This could work, I think:

variable "this_is_my_variable" {
  lifecycle {
    deprecated = true
  }
  type = string
  description = "This is a variable for the old way of configuring things."
}

[cam72cam]

@sftim I want to understand why you want to deprecate locals? The main goal of this issue is to communicate to consumers of a module that the inputs have changes?

I'm also a fan of keeping the description the same and adding an additional "deprecation" field as discussed above.

[sftim]

When we design how to deprecate module inputs, we should consider how we'll mark other things duplicated (eg: provider-managed functions, locals, outputs, entire modules, inner modules).

For example, in Python you can mark any internal method as deprecated and gradually update your code. You can even lint on adding new uses of deprecated things.

[cam72cam]

Going back to your python example, that's still a "function call" interface. Are you looking for something that can hint to IDEs that a particular local in a module should eventually removed? Why wouldn't a comment suffice there?

The biggest value IMO of deprecated is at the module interface. Saying "This Input" or "This Output" won't be supported much longer provides clear messaging between distinct teams and organizations (module writers and maintainers vs consumers).

Given that locals are module-internal, I don't see the need for any of that information to be provided to consumers of the module. Any modern editor can easily and will happy identify and point out //TODO and //FIXME in code.

To make it more concrete, I would not want tofu warning me that a module I use has internal stuff that's deprecated. That would just be confusing to me. I would only want to know that I need to update my calling code when I use a module directly.

This does bring up a scenario on the original issue that we have yet to consider: Would you only want deprecated warnings from the root module -> first level child modules? Does it ever make sense to warn about child -> child module deprecation?