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

core: Warn about deprecated fields when they get referenced #7569

Open
radeksimko opened this issue Jul 9, 2016 · 4 comments
Open

core: Warn about deprecated fields when they get referenced #7569

radeksimko opened this issue Jul 9, 2016 · 4 comments

Comments

@radeksimko
Copy link
Member

radeksimko commented Jul 9, 2016

Terraform only prints deprecation warnings if the Deprecated field is explicitly specified in the HCL config. There are no easy mechanisms to notify users which use a Deprecated & Computed field as a reference.

e.g.

output "example" {
  value = "${aws_instance.demo.deprecated_field}"
}

Related: #6387

Expected behaviour

[WARN] aws_instance.demo.deprecated_field is deprecated, use aws_instance.demo.new_field instead.

Current behaviour

Silent failure - example output is ignored.


Possibly related to #5334

@apparentlymart
Copy link
Contributor

In the new provider protocol for Terraform v0.12 Terraform Core now has access to some schema information for a provider, but we left the idea of Deprecated to be handled entirely by the SDK as a built-in validation check and thus Terraform Core still can't see that information when validating references.

We could potentially add that to the protocol, though. Perhaps we should consider whether there ought to be the possibility for a different message for assigning a value vs. referring to an attribute, in case the provider needs to give different instructions in each case. If an attribute has been replaced directly with another name then the same message is probably fine for both, but it seems that deprecations are sometimes more complicated than that and so we might want to give some more specific instructions that would differ between these two cases.

@paultyng
Copy link
Contributor

paultyng commented Apr 8, 2020

Deprecated is now sent to core (initially just for language server usage) so this is now possible.

@bflad
Copy link
Contributor

bflad commented Jul 27, 2022

This feature request is still valid and represents a rough edge for provider developers. Both terraform-plugin-sdk and terraform-plugin-framework support the ability to configure blocks and attributes with a deprecation message, which gets translated onto the protocol version 5 and 6 deprecated boolean fields for each. However, the only validation the provider side can perform, due to the protocol, is when the attribute is configurable (Required/Optional) and has a configuration value during the Validate*Config RPCs to raise the deprecation as a warning diagnostic. Providers cannot handle anything across multiple resources without some major core and protocol enhancements, which probably fall outside any desirable design considerations.

I'm wondering if core could take that deprecated field that's being sent across the protocol and either perform some direct static validation analysis, or potentially if necessary, do something similar to sensitive values where the value receives an additional cty mark for that property of the value. This new "deprecated" cty mark would then be associated with the value and potentially checked to raise warning diagnostics for the cases providers cannot handle, such as Computed-only attributes when they get referenced. I'm guessing at that point to reduce the noise associated with it, the cty mark could be dropped from the value.

For what its worth, this sort of "deprecated" value marking might be a solution for something like #18381 as well.

@apparentlymart
Copy link
Contributor

I think the same concern as for deprecated output values applies here too: we'd need to decide what should happen in any situation other than a direct, statically-resolvable reference to the deprecated attribute.

Of course one option would be to decide we don't really care about that, but even this somewhat-common (anecdotally) pattern runs into the problem:

resource "thing_with_deprecations" "boop" {
  # Imagine that this is a resource type with at least one
  # of its Computed-only attributes marked as deprecated.
}

output "thing" {
  # This returns the entire resource instance object, rather
  # than any single attribute, so we can't tell whether the
  # deprecated computed attributes will be used without
  # "following" this value through an arbitrary amount of
  # indirection and transformation.
  value = thing_with_deprecations.boop
}

One capability we have in our pocket now, after implementing the idea of dynamically-tracked sensitive values in an earlier Terraform version, is that we can potentially "mark" a value as having some additional characteristic separate from its value. We could in principle use that same mechanism to dynamically track the flow of deprecated values, but there are a few details about that which are finicky:

  • Terraform Core immediately commits all objects returned from a provider to JSON for storage in the state, and re-inflates that JSON only if we need to resolve a reference to the resource elsewhere. Since JSON cannot natively represent "sensitive" we have to track the sensitivity of each attribute path in a sidecar data structure. We might need to do similarly for the deprecation mark, if we expect these deprecated values to pass through any of those serialized forms. (I've not verified that it would need to, but I remember running into this problem before when trying to use a similar mechanism to track the heritage of unknown values via marks.)
  • The default behavior is for marks to virally "infect" any derived results unless a particular function or operator opts out of it. That could mean that deprecation ends up propagating better than we'd prefer, and potentially being reported in many different locations as the mark propagates to increasingly-remote parts of the configuration that the user might not recognize as being derived from the deprecated attribute.
  • Due to the element coalescing behavior of sets, we cannot track marks on a per-element basis inside a set. For sensitive values that already causes us to treat an entire set as sensitive if it has any sensitive values inside it, with the justification that otherwise the coalescing behavior might inadvertently imply the cleartext value of a particular sensitive value. That seems less defensible for deprecation: treating an entire set of objects as deprecated just because there's one deprecated attribute inside it would be incorrect.

Given that, I don't think marks are the best approach for this problem and we'd instead probably need some new static analysis technique, e.g. based on refinement types. Terraform Core and HCL together are not equipped for that sort of static analysis today, and so we'd need to do some pretty significant heavy lifting to get to a point where that sort of analysis would be practical to implement, unless someone has some clever ideas about how we could get it done within the existing architecture and APIs.

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

No branches or pull requests

4 participants