`terraform 0.12upgrade` assumes that the configuration has passed 0.11
init, but did not explicitly check that the configuration was valid.
Certain issues would not get caught because the configuration was
syntactically valid. In this case, int or float values out of range
resulted in a panic from `Value()`.
Since running a 0.11 validate command is a breaking change, this PR
merely moves the `Value()` logic for ints and floats into `configupgrade` so
the error can be returned to the user, instead of causing a panic.
Previously we were using the experimental HCL 2 repository, but now we'll
shift over to the v2 import path within the main HCL repository as part of
actually releasing HCL 2.0 as stable.
This is a mechanical search/replace to the new import paths. It also
switches to the v2.0.0 release of HCL, which includes some new code that
Terraform didn't previously have but should not change any behavior that
matters for Terraform's purposes.
For the moment the experimental HCL2 repository is still an indirect
dependency via terraform-config-inspect, so it remains in our go.sum and
vendor directories for the moment. Because terraform-config-inspect uses
a much smaller subset of the HCL2 functionality, this does still manage
to prune the vendor directory a little. A subsequent release of
terraform-config-inspect should allow us to completely remove that old
repository in a future commit.
In some cases (see #22020 for a specific example), the parsed hilNode
can be nil. This causes a series of panics. Instead, return an error and
move on.
The expression upgrade functionality mostly ignores comments because in
the old language the syntax prevented comments from appearing in the
middle of expressions, but there was one exception: object expressions.
Because HCL 1 used ObjectType both for blocks and for object expressions,
that is the one situation where something we consider to be an expression
could have inline attached comments in the old language.
We migrate these here so we don't lose these comments that don't appear
anywhere else. Other comments get gathered up into a general comments
set maintained inside the analysis object and so will be printed out as
required _between_ expressions, just as they did before.
The v0.12 language supports numeric constants only in decimal notation, as
a simplification. For rare situations where a different base is more
appropriate, such as unix-style file modes, we've found it better for
providers to accept a string containing a representation in the
appropriate base, since that way the interpretation can be validated and
it will be displayed in the same way in the rendered plan diff, in
outputs, etc.
We use tv.Value() here to mimick how HCL 1 itself would have interpreted
these, and then format them back out in the canonical form, which
implicitly converts any non-decimal constants to decimal on the way
through.
Terraform 0.11 and prior had an odd special case where a resource
attribute access for "count" would be resolved as the count for the
whole resource, rather than as an attribute of an individual instance as
for all other attributes.
Because Terraform 0.12 makes test_instance.foo appear as a list when count
is set (so it can be used in other expressions), it's no longer possible
to have an attribute in that position: lists don't have attributes.
Fortunately we don't really need that special case anymore since it
doesn't do anything we can't now do with the length(...) function.
This upgrade rule, then, detects references like test_instance.foo.count
and rewrites to length(test_instance.foo). As a special case, if
test_instance.foo doesn't have "count" set then it just rewrites as the
constant 1, which mimics what would've happened in that case in Terraform
0.11.
Although sets do not have indexed elements, in Terraform 0.11 and earlier
element(...) would work with sets because we'd automatically convert them
to lists on entry to HIL -- with an arbitrary-but-consistent ordering --
and this return an arbitrary-but-consistent element from the list.
The element(...) function in Terraform 0.12 does not allow this because it
is not safe in general, but there was an existing pattern relying on this
in Terraform 0.11 configs which this upgrade rule is intended to preserve:
resource "example" "example" {
count = "${length(any_set_attribute)}"
foo = "${element(any_set_attribute, count.index}"
}
The above works because the exact indices assigned in the conversion are
irrelevant: we're just asking Terraform to create one resource for each
distinct element in the set.
This upgrade rule therefore inserts an explicit conversion to list if it
is able to successfully provide that the given expression will return a
set type:
foo = "${element(tolist(any_set_attribute), count.index}"
This makes the conversion explicit, allowing users to decide if it is
safe and rework the configuration if not. Since our static type analysis
functionality focuses mainly on resource type attributes, in practice this
rule will only apply when the given expression is a statically-checkable
resource reference. Since sets are an SDK-only concept in Terraform 0.11
and earlier anyway, in practice that works out just right: it's not
possible for sets to appear anywhere else in older versions anyway.
Prior to Terraform 0.12, ignore_changes was implemented in a
flatmap-oriented fashion and so users found that they could (and in fact,
were often forced to) use the internal .% and .# suffixes flatmap uses to
ignore changes to the number of elements in a list or map.
Terraform 0.12 no longer uses that representation, so we'll interpret
ignoring changes to the length as ignoring changes to the entire
collection. While this is not a totally-equivalent change, in practice
this pattern was most often used in conjunction with specific keys from a
map in order to _effectively_ ignore the entire map, even though Terraform
didn't really support that.
HIL implemented its type conversions by rewriting its AST to include calls
to some undocumented builtin functions. Unfortunately those functions were
still explicitly callable if you could figure out the name for them, and
so they may have been used in the wild.
In particular, __builtin_StringToFloat was used as part of a workaround
for a HIL design flaw where it would prefer to convert strings to integers
rather than floats when performing arithmetic operations. This issue was,
indeed, the main reason for unifying int ant float into a single number
type in HCL. Since we published that as a suggested workaround, the
upgrade tool ought to fix it up.
The other cases have never been documented as a workaround, so they are
less likely to appear in the wild, but we might as well fix them up anyway
since we already have the conversion functions required to get the same
result in the new language.
To be safe/conservative, most of these convert to _two_ function calls
rather than just one, which ensures that these new expressions retain the
behavior of implicitly converting to the source type before running the
conversion. The new conversion functions only specify target type, and so
cannot guarantee identical results if the argument type does not exactly
match what was previously given as the parameter type in HIL.
HEREDOC tokens are a little more fussy than normal string sequences
because we need to preserve the whitespace within them along with the
start and end markers while we upgrade any interpolated expressions inside.
We need to do some work locally here because the HCL heredoc processing
"does too much" and throws away information we need to do a faithful
upgrade.
We also need to contend with the fact that Terraform <=0.11 had an older
version of HCL that accidentally permitted a degenerate form of heredoc
where the marker was at the end of the final line, like this:
degenerate = <<EOT
this should never have workedEOT
When we migrate this, we'll introduce the additional newline that is now
required, which will unfortunately slightly change the result string to
include a newline when parsed by 0.12, and so we'll need to call this out
as a caveat in the upgrade guide.
Previously, configupgrade would panic if it encountered a HEREDOC. For
the time being, we will simply print out the HEREDOC as-is.
Unfortunately, we discovered that terraform 0.11's version of HCL
allowed for HEREDOCs with the termination delimiter inline (instead of
on a newline, which is technically correct). Since 0.12configupgrade
needs to be bug-compatible with terraform 0.11, we must roll back to the
same version of HCL used in terraform 0.11.
In prior versions, we recommended using hash functions in conjunction with
the file function as an idiom for detecting changes to upstream blobs
without fetching and comparing the whole blob.
That approach relied on us being able to return raw binary data from
file(...). Since Terraform strings pass through intermediate
representations that are not binary-safe (e.g. the JSON state), there was
a risk of string corruption in prior versions which we have avoided for
0.12 by requiring that file(...) be used only with UTF-8 text files.
The specific case of returning a string and immediately passing it into
another function was not actually subject to that corruption risk, since
the HIL interpreter would just pass the string through verbatim, but this
is still now forbidden as a result of the stricter handling of file(...).
To avoid breaking these use-cases, here we introduce variants of the hash
functions a with "file" prefix that take a filename for a disk file to
hash rather than hashing the given string directly. The configuration
upgrade tool also now includes a rule to detect the documented idiom and
rewrite it into a single function call for one of these new functions.
This does cause a bit of function sprawl, but that seems preferable to
introducing more complex rules for when file(...) can and cannot read
binary files, making the behavior of these various functions easier to
understand in isolation.
In early versions of Terraform where the interpolation language didn't
have any real list support, list brackets around a single string was the
signal to split the string on a special uuid separator to produce a list
just in time for processing, giving expressions like this:
foo = ["${test_instance.foo.*.id}"]
Logically this is weird because it looks like it should produce a list
of lists of strings. When we added real list support in Terraform 0.7 we
retained support for this behavior by trimming off extra levels of list
during evaluation, and inadvertently continued relying on this notation
for correct type checking.
During the Terraform 0.10 line we fixed the type checker bugs (a few
remaining issues notwithstanding) so that it was finally possible to
use the more intuitive form:
foo = "${test_instance.foo.*.id}"
...but we continued trimming off extra levels of list for backward
compatibility.
Terraform 0.12 finally removes that compatibility shim, causing redundant
list brackets to be interpreted as a list of lists.
This upgrade rule attempts to identify situations that are relying on the
old compatibility behavior and trim off the redundant extra brackets. It's
not possible to do this fully-generally using only static analysis, but
we can gather enough information through or partial type inference
mechanism here to deal with the most common situations automatically and
produce a TF-UPGRADE-TODO comment for more complex scenarios where the
user intent isn't decidable with only static analysis.
In particular, this handles by far the most common situation of wrapping
list brackets around a splat expression like the first example above.
After this and the other upgrade rules are applied, the first example
above will become:
foo = test_instance.foo.*.id
The main tricky thing here is ignore_changes, which contains strings that
are better given as naked traversals in 0.12. We also handle here mapping
the old special case ["*"] value to the new "all" keyword.
Prior to v0.12 Terraform was liberal about these and allowed them to
mismatch, but now it's important to get this right so that resources
and resource instances can be used directly as object values, and so
we'll fix up any sloppy existing references so things keep working as
expected.
This is particularly important for the pattern of using count to create
conditional resources, since previously the "true" case would create one
instance and Terraform would accept an unindexed reference to that.
The reference syntax is not significantly changed, but there are some
minor additional restrictions on identifiers in HCL2 and as a special case
we need to rewrite references to data.terraform_remote_state .
Along with those mandatory upgrades, we will also switch references to
using normal index syntax where it's safe to do so, as part of
de-emphasizing the old strange integer attribute syntax (like foo.0.bar).
We're using break elsewhere in here so it was weird to have a small set
of situations that return instead, which could then cause confusion for
future maintenance if a reader doesn't notice that control doesn't always
leave the outer switch statement.
If lookup is being used with only two arguments then it is equivalent to
index syntax and more readable that way, so we'll replace it.
Ideally we'd do similarly for element(...) here but sadly we cannot
because we can't prove in static analysis that the user is not relying
on the modulo wraparound behavior of that function.
We now have native language features for declaring tuples and objects,
which are the idiomatic way to construct sequence and mapping values that
can then be converted to list, set, and map types as needed.
In the old world, lists and maps could be created either using functions
in HIL or list/object constructs in HCL. Here we ensure that in the HCL
case we'll apply any required expression transformations to the individual
items within HCL's compound constructs.
In order to properly migrate the contents of resource, data, provider and
provisioner blocks we will need the provider's schema in order to
understand what is expected, so we can resolve some ambiguities inherent
in the legacy HCL AST.
This includes an initial prototype of migrating the content of resource
blocks just to verify that the information is being gathered correctly.
As with the rest of the upgrade_native.go file, this will be reorganized
significantly once the basic end-to-end flow is established and we can
see how to organize this code better.
This covers all of the expression node types in HIL's AST, and also
includes initial support for some of our top-level blocks so that we can
easily test that.
The initial implementations of the "variable" and "output" blocks are
pretty redundant and messy, so we can hopefully improve on these in a
later pass.
Kristin Laemmert