hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.7k stars 9.55k forks source link

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

Open radeksimko opened 8 years ago

radeksimko commented 8 years ago

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: https://github.com/hashicorp/terraform/pull/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 https://github.com/hashicorp/terraform/issues/5334

apparentlymart commented 5 years ago

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 commented 4 years ago

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

bflad commented 2 years ago

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 https://github.com/hashicorp/terraform/issues/18381 as well.

apparentlymart commented 2 years ago

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:

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.