In study of existing providers we've found a pattern we werent previously
accounting for of using a nested block type to represent a group of
arguments that relate to a particular feature that is always enabled but
where it improves configuration readability to group all of its settings
together in a nested block.
The existing NestingSingle was not a good fit for this because it is
designed under the assumption that the presence or absence of the block
has some significance in enabling or disabling the relevant feature, and
so for these always-active cases we'd generate a misleading plan where
the settings for the feature appear totally absent, rather than showing
the default values that will be selected.
NestingGroup is, therefore, a slight variation of NestingSingle where
presence vs. absence of the block is not distinguishable (it's never null)
and instead its contents are treated as unset when the block is absent.
This then in turn causes any default values associated with the nested
arguments to be honored and displayed in the plan whenever the block is
not explicitly configured.
The current SDK cannot activate this mode, but that's okay because its
"legacy type system" opt-out flag allows it to force a block to be
processed in this way anyway. We're adding this now so that we can
introduce the feature in a future SDK without causing a breaking change
to the protocol, since the set of possible block nesting modes is not
extensible.
* configs/configupgrade: detect invalid resource names and print a TODO
message
In terraform 0.11 and prior it was possible to start a resource name
with a number. This is no longer valid, as the resource name would would
be ambiguous with number values in HCL expressions.
Fixes#19919
* Update configs/configupgrade/test-fixtures/valid/invalid-resource-name/want/resource.tf
Co-Authored-By: mildwonkey <mildwonkey@users.noreply.github.com>
* configs/configupgrade: detect possible relative module sources
If a module source appears to be a relative local path but does not have
a preceding ./, print a #TODO message for the user.
* internal/initwd: limit go-getter detectors to those supported by terraform
* internal/initwd: move isMaybeRelativeLocalPath check into getWithGoGetter
To avoid making two calls to getter.Detect, which potentially makes
non-trivial API calls, the "isMaybeRelativeLocalPath" check was moved to
a later step and a custom error type was added so user-friendly
diagnostics could be displayed in the event that a possible relative local
path was detected.
Prior to Terraform v0.12 it was possible for a provider to secretly set
some default arguments for the "connection" block, which most commonly
included a hard-coded type of "ssh" and a value from "host".
In the interests of "explicit is better than implicit", Terraform 0.12 no
longer has this feature and instead requires connection settings to be
written explicitly in terms of the resource's exported attributes. For
compatibility though, the upgrade tool will insert expressions that are
as close as possible to the logic the provider formerly implemented, or
in a few rare cases a TF-UPGRADE-TODO comment to fix it up manually.
Some of the existing resource type implementations have incredibly
complicated implementations of selecting a single host IP address to use
and don't expose the result of that as an attribute, so for now we handle
those via a complicated Terraform language expression achieving the same
result. Ideally these providers would introduce a new attribute that
exports the same address formerly exported as the hostname before their
initial v0.12-compatible release, in which case we can simplify these to
just reference the attribute in question. That would be preferable also
because it would allow use of that exported attribute in other contexts,
such as in a null_resource provisioner somewhere else or in an output
to allow a caller to deal with the SSH part itself.
Aside from the two special meta-arguments "connection" and "provisioner"
this is just our standard mapping from schema to conversion rules, using
the provisioner's configuration schema.
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.
This is a temporary implementation of these rules just so that these can
be passed through verbatim (rather than generating an error) while we
do testing of other features.
A subsequent commit will finish these with their own custom rulesets.
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.
Both resource blocks and module blocks contain references to providers
that are expressed as short-form provider addresses ("aws.foo" rather than
"provider.aws.foo").
These rules call for those to be unwrapped as naked identifiers during
upgrade, rather than appearing as quoted strings. This also introduces
some further rules for other simpler meta-arguments that are required
for the test fixtures for this feature.
Some further rules are required here to deal with the meta-arguments we
accept inside these blocks, but this is good enough to pass through most
module blocks using the standard attribute-expression-based mapping.
Previously we were handling this one as a special case, effectively
duplicating most of the logic from upgradeBlockBody.
By doing some prior analysis of the block we can produce a "rules" that
just passes through all of the attributes as-is, allowing us to reuse
upgradeBlockBody. This is a little weird for the locals block since
everything in it is user-selected names, but this facility will also be
useful in a future commit for dealing with module blocks, which contain
a mixture of user-chosen and reserved argument names.
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.
Previously we were erroneously moving these out of their original block
into the surrounding body. Now we'll make sure to collect up any remaining
ad-hoc comments inside a nested block body before closing it.
Users discovered that they could exploit some missing validation in
Terraform v0.11 and prior to treat block types as if they were attributes
and assign dynamic expressions to them, with some significant caveats and
gotchas resulting from the fact that this was never intended to work.
However, since such patterns are in use in the wild we'll convert them
to a dynamic block during upgrade. With only static analysis we must
unfortunately generate a very conservative, ugly dynamic block with
every possible argument set. Users ought to then clean up the generated
configuration after confirming which arguments are actually required.
Previously we were using the line count difference between the start of
one item and the next to decide whether to insert a blank line between
two items, but that is incorrect for multi-line items.
Instead, we'll now count the difference from the final line of the
previous item to the first line of the next, as best we can with the
limited position info recorded by the HCL1 parser.
The main area of interest in upgrading is dealing with special cases for
individual block items, so this generalization allows us to use the same
overall body-processing logic for everything but to specialize just how
individual items are dealt with, which we match by their names as given
in the original input source code.
This involved some refactoring of how block bodies are migrated, which
still needs some additional work to deal with meta-arguments but is now
at least partially generalized to support both resource and provider
blocks.
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.
Martin Atkins