In #2884, Terraform would hang on graphs with an orphaned resource
depended on an orphaned module.
This is because orphan module nodes (which are dependable) were getting
expanded (replaced) with GraphNodeBasicSubgraph nodes (which are _not_
dependable).
The old `graph.Replace()` code resulted in GraphNodeBasicSubgraph being
entered into the lookaside table, even though it is not dependable.
This resulted in an untraversable edge in the graph, so the graph would
hang and wait forever.
Now, we remove entries from the lookaside table when a dependable node
is being replaced with a non-dependable node. This means we lose an
edge, but we can move forward. It's ~probably~ never correct to be
replacing depenable nodes with non-dependable ones, but this tweak
seemed preferable to tossing a panic in there.
When a provider validation only returns a warning, we were cutting the
evaltree short by returning an error. This is fine during a
`walkValidate` but was causing trouble during `walkPlan` and
`walkApply`.
fixes#2870
I was worried about the implications of deeply nested orphaned modules
in the parent commit, so I added a test. It's failing but not quite like
I expected it to. Perhaps I've uncovered an unrelated bug here?
/cc @mitchellh
The `CloseProviderTransformer` relies on the `ProvidedBy()` interface to
look up the proper dependency for the the graph nodes it adds. This
interface needs to yield the name of a provider, _AND_ for flattened
nodes it needs to yield the full path to a provider.
Destroy nodes did not implement this second part, which resulted in
"provider X couldn't be found" when both of these were true:
* A module included a resource that dependend on a provider
* The root did _NOT_ include a provider config
Implementing a proper ProvidedBy() on the flattened version of
destroy nodes solves the issue.
fixes#2581
The context_test file has gotten pretty unruly. Let's split it up into
a few files so we can be nicer to our editors and our own sanity.
Definitely lots more we can do to clean up, but with changes like this
I'd rather do small, focused, clear steps instead of one big "cleaned up
lots of stuff" PR.
By prefixing them with `cmd /c` it will work with both `winner` and
`ssh` connection types.
This PR also reverts some bad stringer changes made in PR #2673
In `helper/schema` we already makes a distinction between `Default`
which is always applied and `InputDefault` which is displayed to the
user for an empty field.
But for variables we just have `Default` which is treated like
`InputDefault`. This changes it to _not_ prompt the user for a value
when the variable declaration includes a default.
Treating this as a UX bugfix and the "don't prompt for variables w/
defaults set" behavior as the originally expected behavior we were
failing to honor.
Added an already-passing test to verify and cover the `helper/schema`
behavior.
Perhaps down the road we can add a `input_default` attribute to
variables to allow similar behavior to `helper/schema` in variables, but
for now just sticking with the fix.
Fixes#2592
Allows target dependencies to be properly calculated across module
boundaries, fixing scenarios where a target depends on something inside
a module, but the contents of the module are not included in the
targeted resources.
fixes#1858
When targeting prunes out all the resource nodes between a provider and
its close node, there was no dependency to ensure the close happened
after the configure. Needed to add an explicit dependency from the close
to the provider.
This tweak highlighted the fact that CloseProviderTransformer needed to
happen after DisableProviderTransformer, since
DisableProviderTransformer inspects up-edges to decide what to disable,
and CloseProviderTransformer adds an up-edge.
fixes#2495
Had to handle a lot of implicit leaning on a few properties of the old
representation:
* Old representation allowed plain strings to be treated as lists
without problem (i.e. shoved into strings.Split), now strings need to
be checked whether they are a list before they are treated as one
(i.e. shoved into StringList(s).Slice()).
* Tested behavior of 0 and 1 length lists in formatlist() was a side
effect of the representation. Needs to be special cased now to
maintain the behavior.
* Found a pretty old context test failure that was wrong in several
different ways. It's covered by TestContext2Apply_multiVar so I
removed it.
This is the initial pure "all tests passing without a diff" stage. The
plan is to change the internal representation of StringList to include a
suffix delimiter, which will allow us to recognize empty and
single-element lists.
The provider input before wasn't scoped by path, which caused
non-descendant parts of the graph to grab the configuration of another
sub-tree. The result is that you'd often get copied provider
configurations across the module barriers.
See GH-2024
Without this 12 line function it’s impossible to use any of the
Terraform code without the need for having the files on disk. As more
and more people are using (parts of) Terraform in other software, this
seems to be a very welcome addition. It has no negative impact on
Terraform itself whatsoever (the function is never called), but it
opens up a lot of other use cases.
Next to the single new function, I renamed the existing function (and
related tests) to better reflect what the function does. So now there
is a `LoadDir` function which calls `LoadFile` for each file, which
kind of made sense to me, especially when now adding a `LoadJSON`
function as well.
But of course if the rename is a problem, I can revert that part as
it’s not related to the added `LoadJSON` function.
Thanks!
Currently Terraform is leaking goroutines and with that memory. I know
strictly speaking this maybe isn’t a real concern for Terraform as it’s
mostly used as a short running command line executable.
But there are a few of us out there that are using Terraform in some
long running processes and then this starts to become a problem.
Next to that it’s of course good programming practise to clean up
resources when they're not needed anymore. So even for the standard
command line use case, this seems an improvement in resource management.
Personally I see no downsides as the primary connection to the plugin
is kept alive (the plugin is not killed) and only unused connections
that will never be used again are closed to free up any related
goroutines and memory.
Because CBD now runs after a RootTransformer, it's now operating on a
graph that _may_ have had a graphNodeRoot added to it (a noop node whose
only purpose is to be a root).
CBD includes a step that tells the destroy node to depend on any parents
of the create node. When one of those parents was "root", this was
causing the destroy node to depend on "root", making it cease to be an
actual root node.
Because graphNodeRoot is a singleton, the follow-up RootTransformer was
not sufficient to slap another root on top - it wasn't being seen as a
fresh node, so edges were just accumulating, and we ended up in a state
with "no roots".
refs #1903 (not sure if this will fix all the "no root found" cases, or
just the one I bumped into)
fixes#1947
Root cause was a bad edge being made by the CBD transform going from the
flattened destroy node to the unflattened create node, which was no
longer in the graph. The destroy node therefore had a dependency that
could never be satisfied, which locked up the walk.
Got this while playing around in a module:
> * unflattenable node: aws_security_group.internal (orphan)
> *terraform.graphNodeOrphanResource
Basically just copied implementation from
d503cc2d82
