In this codepath we're still using the old "internal" ResourceAddress
parser to parse resource addresses from the diff, but the string being
parsed does not include a module path and so the result is interpreted
as a resource in the root module.
We need to rewrite this to be in the correct module path as part of
converting to the new address representation, in order to get a correct
absolute address.
A trivial mistake in the rework of this function meant that it was just
discarding its result rather than returning it. It will now return its
result as expected, allowing reference analysis to work for this node
type.
The final "<resource> provided by <provider>" message was showing the
original selection made by the resource itself rather than the provider
address finally resolved after inheritance, which caused the log to
misrepresent what was actually being created in the graph.
Previously this was just stubbing out provider types, but we now need to
include schema for each of the providers and resource types the tests
use in order for the references to be properly detected.
The test fixtures are adjusted slightly here so we can use the
simpleTestSchema as the schema for all of the different blocks in these
tests. The relationships between the resources are still preserved, but
the attributes are renamed to comply with this schema.
Since expression evaluation is now done via an EvalContext method, and
since these tests use the MockEvalContext, we need to provide a value for
the mock call to return. Previously expression evaluation happened at a
different layer that was not mocked here.
These transformers both construct temporary graphs using many of the same
transformers used in the apply graph, and properly doing this now requires
access to the providers and provisioners in order to obtain their schemas.
Along with this, we also update the tests here to use the
simpleMockComponentFactory helper to get a mock provider with a schema
already configured, which means we also need to update the test fixtures
and assertions to use the resource type and attributes defined in that
mock factory.
Having a missing schema is a programming error, but at the time of this
commit we're in the midst of introducing schema all over Terraform and so
there are inevitably some places -- particularly in older unit tests --
where schema isn't yet being provided.
This error allows us to catch those cases and fail gracefully, rather
than panicking further down here when we access t.Components methods.
Also includes some additional logging to aid debugging of this
transformer.
Previously we were attempting to construct a default manually here, and
actually getting it wrong by using the resource type name as a whole
rather than the expected inferrence by prefix.
Now we use the method provided in the addrs package for this purpose,
which implements the standard behavior of shaving off the first
underscore-separated word from the resource type name.
Now that any configuration processing requires schema, we need either a
standalone schema or a provider/provisioner configured with one a lot more
often in tests.
To avoid adding loads of extra boilerplate to the tests, these new helper
functions produce objects pre-configured with a schema that should be
useful for a number of different cases, and can be customized further for
more interesting situations.
A lot of our tests can then just use these pre-defined object and
attribute names in their fixtures in situations where the canned schemas
here are good enough.
The logic under test has intentionally changed here so that setting count
to any value -- even 1 -- causes Terraform to prefer to keep indexed
instances if both non-indexed and indexed are present.
Previously Terraform treated count = 1 as equivalent to count not set at
all, but we now recognize these as two different situations and treat
count = 1 as the same as any other non-zero count, to ensure that if
count is set conditionally it'll always produce indexed instances, even
if the dynamic expression ends up evaluating to 1.
This function was previously checking for a path length greater than one
because the older path format included an always present "root" element
at the start.
We now need to check for a totally-empty list, because otherwise we fail
to add the expected prefix to the front of a path with only one element.
This also includes some adjustments to the related tests and transforms
that do not change behavior but do make the test results easier to
understand and debug.
Due to some disagreement about what representation of provider addresses
we were using, the inherited providers map wasn't matching. Now we'll
consistenly use the "compact" form (just the provider name and optional
alias).
Also includes some other tweaks to make this test better-behaved.
Although the AddModule method now takes a new-style
module address as an argument, internally we're still
shimming it to a []string.
Therefore this test needs to still expect [][]string as
a result, rather tan []addrs.ModuleInstance.
Prior to the refactoring to move provider address parsing/selection up
into the frontend, there was some logic here to just-in-time default a
provider config based on the given resource type.
This is now expected to happen at a higher layer, with ImportTarget
expecting an already-valid provider configuration address.
The normal import codepath was already updated with this in mind, but some
of the provider transform tests are using ImportStateTransformer as a
shortcut for getting some resource nodes added to the graph, and so those
tests now need to include a valid provider address in their ImportTarget
values.
Also includes some adjustments to test output to make the tests easier
to debug.
The prior implementation of this function (before the rewrite to use
addrs.AbsProviderConfig values) was relying on some implicit behavior of
our provider address normalization to generate an address in the root
module. Since that wasn't explicit, I introduced a bug here when
introducing the new address type, where I generated an address in the
node's own module, rather than in the root as expected.
This new implementation is functionally equivalent to the prior, but is
written to make the intent more obvious: take _just_ the type from the
node's provider address and create an implicit configuration for it
_in the root module_.
Along with the change in approach, this new implementation also has an
updated documentation comment that better describes its current intent
(previously it was outdated) and hopefully clearer trace logging to
better communicate what it's doing.
In the change to using addrs types rather than string keys directly here
I incorrectly made this use the _relative_ provider config instead of
the absolute one, causing MissingProviderTransformer to only match
providers defined in the root module (due to ambiguity in the string
representations of these address types).
The rest of this change is improved logging and test output that helped
with debugging this issue.
Some of these tests were using an outdated form to reference a local
directory as a module. While here, I also updated the provider references
to the new unquoted form, which is preferred from v0.12 onwards.
The changes to our GraphNodeReferencable and GraphNodeReferencer
interfaces were not also reflected in our testing structs here, and so
these tests were no longer working.
This updates these implementations to the new required signatures,
adapting the simplified model used in the structs to generate local
variable references, since the reference model no longer uses the
arbitrary strings that this test originally depended on.
We also remove some of the tests here since the functionality they were
testing no longer applies: inter-module dependencies are now handled by
the graph nodes producing extra ReferencableAddrs, and the "backup" form
is no longer used because our new address model is able to distinguish
resources from resource instances without the need for the magical
backup reference forms we previously used.
Resolution of dependencies automatically from expressions now requires
a schema to be available. To avoid the need to provide mock schemas for
all of these different resource types, we instead use the depends_on
argument here to mark the dependencies explicitly.
Previously we had a bug where we'd use the resource type name instead of
the provider name. Now we use the DefaultProviderConfig helper function
to extract the provider name from the resource type name.
The ReferenceTransformer can't do its work here unless our resources have
schema attached, since otherwise it doesn't know which attributes to
search to find references.
Now that a schema is required to decode resource configuration, a lot more
tests than before need access to mock providers in order to get a schema.
These new helpers allow a mock provider with a schema to be constructed
concisely.
This test required some adaptation for the new variable handling model in
the HCL2-oriented codepaths, where more of the handling is now done in the
"command" package.
The initial reorganization of this test was not correct, so this is a
further revision to adapt the original assertions to the new model as much
as possible, removing a few tests that no longer make sense in the new
world.
A number of our test fixtures were previously using the non-idiomatic form
of including a single child attribute all on one line with the block
header and bounding braces.
This non-idiomatic form is an error in HCL2, and hclfmt has always "fixed"
it to the expected form of each attribute being on a line of its own, and
so here we just update all the affected test fixtures to canonical form
(using hclfmt), allowing them to be parsed as intended.
Since the these entire files were processed with hclfmt, there are some
other unrelated style changes included in situations where the file
layouts were non-idiomatic in other ways.
These are all things that ought to be present in normal use but can end up
being nil in incorrect tests. Test debugging is simpler if these things
return errors gracefully, rather than crashing.
While there's no good reason for this to happen in practice, it can arise
in tests if mocks aren't set up quite right, and so we'll catch it and
report it nicely to make test debugging a little easier.
There's actually no good reason for this to happen in the normal case, but
it can happen reasonably easy if a test doesn't properly configure the
MockEvalContext, and so having this check here makes test debugging a
little easier.
Since this test case is using t.Run, it must be sure to pass the nested
*testing.T over to the testDiffs function or else calls to t.Fatal will
crash the whole test process, since they would otherwise be called on the
wrong instance of *testing.T.
The only reason these cases are arising right now is because we have tests
that haven't yet been updated to properly support schema, but it can't
hurt to add some robustness here to reduce the risk of real crashes.
After the refactoring to integrate HCL2 many of the tests were no longer
using correct types, attribute names, etc.
This is a bulk update of all of the tests to make them compile again, with
minimal changes otherwise. Although the tests now compile, many of them
do not yet pass. The tests will be gradually repaired in subsequent
commits, as we continue to complete the refactoring and retrofit work.
Some of the objects that are referencable from expressions are transient
values computed only during a graph walk, and not persisted in state. In
order to support arbitrary evaluation of expressions, such as in the
"terraform console" CLI command, it's necessary to be able to evaluate
these values before we start evaluating.
This new Eval method achieves this by performing a special graph walk that
ignores resources (except for dependency resolution) and just focuses on
evaluating all of these transient values, before returning an evaluation
scope that can then resolve expressions in terms of that result.
This replaces the Context.Interpolator method, which was fraught with
various issues due to it not properly priming the state before evaluating.
Here we replace the stub implementations in evaluationStateData with real
implementations that are based on their equivalents in the old
Interpolator type.
The behavior here is a little different due to the different semantics
expected under HCL2, but the principle remains the same: the main
references are resolved from the state, using config for validation
in order to produce some helpful error messages.
I took some missteps here while doing the initial refactor for HCL2 types.
This restores the map of maps that retains all of the variable values, and
then makes it available to the evaluator.
Previously our evaluationStateData object was constructed inside
Evaluator.Scope, but this was awkward because all of the fields inside it
need to be populated from BuiltinEvalContext fields, and so the signature
of Evaluator.Scope kept growing new arguments over time.
Instead, we reassign the responsibilities here so that Evaluator.Scope
takes an already-constructed lang.Data, and then teach BuiltinEvalContext
to build this object itself from its own internal values.
Due to a logic error here we were trying to find our our module's parent
as a descendent of itself, rather than as a descendent of the root. It
turns out that we can do this even more simply by just accessing the
Parent field on the given config, avoiding the need to traverse the tree
down from the root at all.
While here, this also switches to using the path.Call helper method rather
than manually slicing the path array, since this better communicates our
intent.
This is a little awkward since we need to instantiate the providers much
earlier than before. To avoid a lot of reshuffling here we just spin each
one up and then immediately shut it down again, letting our existing init
functionality during the graph walk still do the main initialization.
We've not yet adjusted any of the state structs to reflect our new address
types because they are used with encoding/json to produce our state file
format, but the shimming here previously was incorrect because it failed
to include the special "root" string that's always required at element
zero of a module path in the state.
Due to how deeply the configuration types go into Terraform Core, there
isn't a great way to switch out to HCL2 gradually. As a consequence, this
huge commit gets us from the old state to a _compilable_ new state, but
does not yet attempt to fix any tests and has a number of known missing
parts and bugs. We will continue to iterate on this in forthcoming
commits, heading back towards passing tests and making Terraform
fully-functional again.
The three main goals here are:
- Use the configuration models from the "configs" package instead of the
older models in the "config" package, which is now deprecated and
preserved only to help us write our migration tool.
- Do expression inspection and evaluation using the functionality of the
new "lang" package, instead of the Interpolator type and related
functionality in the main "terraform" package.
- Represent addresses of various objects using types in the addrs package,
rather than hand-constructed strings. This is not critical to support
the above, but was a big help during the implementation of these other
points since it made it much more explicit what kind of address is
expected in each context.
Since our new packages are built to accommodate some future planned
features that are not yet implemented (e.g. the "for_each" argument on
resources, "count"/"for_each" on modules), and since there's still a fair
amount of functionality still using old-style APIs, there is a moderate
amount of shimming here to connect new assumptions with old, hopefully in
a way that makes it easier to find and eliminate these shims later.
I apologize in advance to the person who inevitably just found this huge
commit while spelunking through the commit history.
This is currently not very ergonomic due to the API exposed by providers.
We'll smooth this out in a later change to improve the provider API, since
we know we always want the entire schema.
Initially the intent here was to tease these apart a little more since
they don't really share much behavior in common in core, but in practice
it'll take a lot of refactoring to tease apart these assumptions in core
right now and so we'll keep these things unified at the configuration
layer in the interests of minimizing disruption at the core layer.
The two types are still kept in separate maps to help reinforce the fact
that they are separate concepts with some behaviors in common, rather than
the same concept.
For the moment this is just a lightly-adapted copy of
ModuleTreeDependencies named ConfigTreeDependencies, with the goal that
the two can live concurrently for the moment while not all callers are yet
updated and then we can drop ModuleTreeDependencies and its helper
functions altogether in a later commit.
This can then be used to make "terraform init" and "terraform providers"
work properly with the HCL2-powered configuration loader.
This is a rather-messy, complex change to get the "command" package
building again against the new backend API that was updated for
the new configuration loader.
A lot of this is mechanical rewriting to the new API, but
meta_config.go and meta_backend.go in particular saw some major
changes to interface with the new loader APIs and to deal with
the change in order of steps in the backend API.
When computing the count value, make sure to include walkDestroy with
walkApply, as the former is only a special case of the latter.
When applying a saved plan, the computed count values are lost and we
can no longer query the state for those values. The apply walk was
already considered in the `resourceCountMax` function, but the destroy
walk was not. This worked when destroying in a single operation
("terraform destroy"), since the state would still be updated with the
latest counts from the plan.
