Previously, interpolation of multi-variables was returning an empty
variable if the resource count was 0. The empty variable was defined as
TypeString, Value "". This means that empty resource counts fail type
checking for interpolation functions which operate on lists.
Instead, return an empty list if the count is 0. A context test tests
this against further regression. Also add a regression test covering the
case of a single count multi-variable.
In order to make the context testing framework deal with this change it
was necessary to special case empty lists in the test diff function.
Fixes#7002
For `terraform destroy`, we currently build up the same graph we do for
`plan` and `apply` and we do a walk with a special Diff that says
"destroy everything".
We have fought the interpolation subsystem time and again through this
code path. Beginning in #2775 we gained a new feature to selectively
prune out problematic graph nodes. The past chain of destroy fixes I
have been involved with (#6557, #6599, #6753) have attempted to massage
the "noop" definitions to properly handle the edge cases reported.
"Variable is depended on by provider config" is another edge case we add
here and try to fix.
This dive only makes me more convinced that the whole `terraform
destroy` code path needs to be reworked.
For now, I went with a "surgical strike" approach to the problem
expressed in #7047. I found a couple of issues with the existing
Noop and DestroyEdgeInclude logic, especially with regards to
flattening, but I'm explicitly ignoring these for now so we can get this
particular bug fixed ahead of the 0.7 release. My hope is that we can
circle around with a fully specced initiative to refactor `terraform
destroy`'s graph to be more state-derived than config-derived.
Until then, this fixes#7407
Previously the plan phase would produce a data diff only if no state was
already present. However, this is a faulty approach because a state will
already be present in the case where the data resource depends on a
managed resource that existed in state during refresh but became
computed during plan, due to a "forces new resource" diff.
Now we will produce a data diff regardless of the presence of the state
when the configuration is computed during the plan phase.
This fixes#6824.
Earlier we had a bug where data resources would not yet removed from the
state during a destroy. This was fixed in cd0c452, and this test will
hopefully make sure it stays fixed.
Variables weren't being interpolated during the Input phase, causing a
syntax error on the interpolation string. Adding `walkInput` to the
EvalTree operations prevents skipping the interpolation step.
Apparently there's been a regression in the creation of data resource
diffs: they aren't showing up in the plan at all.
As a first step to fixing this, this is an intentionally-failing test
that proves it's broken.
Building on b10564a, adding tweaks that allow the module var count
search to act recursively, ensuring that a sitaution where something
like var.top gets passed to module middle, as var.middle, and then to
module bottom, as var.bottom, which is then used in a resource count.
A new problem was introduced by the prior fixes for destroy
interpolation messages when resources depend on module variables with
a _count_ attribute, this makes the variable crucial for properly
building the graph - even in destroys. So removing all module variables
from the graph as noops was overzealous.
By borrowing the logic in `DestroyEdgeInclude` we are able to determine
if we need to keep a given module variable relatively easily.
I'd like to overhaul the `Destroy: true` implementation so that it does
not depend on config at all, but I want to continue for now with the
targeted fixes that we can backport into the 0.6 series.
The fix that landed in #6557 was unfortunately the wrong subset of the
work I had been doing locally, and users of the attached bugs are still
reporting problems with Terraform v0.6.16.
At the very last step, I attempted to scope down both the failing test
and the implementation to their bare essentials, but ended up with a
test that did not exercise the root of the problem and a subset of the
implementation that was insufficient for a full bugfix.
The key thing I removed from the test was a _referencing output_ for the
module, which is what breaks down the #6557 solution.
I've re-tested the examples in #5440 and #3268 to verify this solution
does indeed solve the problem.
This changes the representation of maps in the interpolator from the
dotted flatmap form of a string variable named "var.variablename.key"
per map element to use native HIL maps instead.
This involves porting some of the interpolation functions in order to
keep the tests green, and adding support for map outputs.
There is one backwards incompatibility: as a result of an implementation
detail of maps, one could access an indexed map variable using the
syntax "${var.variablename.key}".
This is no longer possible - instead HIL native syntax -
"${var.variablename["key"]}" must be used. This was previously
documented, (though not heavily used) so it must be noted as a backward
compatibility issue for Terraform 0.7.
This introduces the terraform state list command to list the resources
within a state. This is the first of many state management commands to
come into 0.7.
This is the first command of many to come that is considered a
"plumbing" command within Terraform (see "plumbing vs porcelain":
http://git.661346.n2.nabble.com/what-are-plumbing-and-porcelain-td2190639.html).
As such, this PR also introduces a bunch of groundwork to support
plumbing commands.
The main changes:
- Main command output is changed to split "common" and "uncommon"
commands.
- mitchellh/cli is updated to support nested subcommands, since
terraform state list is a nested subcommand.
- terraform.StateFilter is introduced as a way in core to filter/search
the state files. This is very basic currently but I expect to make it
more advanced as time goes on.
- terraform state list command is introduced to list resources in a
state. This can take a series of arguments to filter this down.
Known issues, or things that aren't done in this PR on purpose:
- Unit tests for terraform state list are on the way. Unit tests for the
core changes are all there.
Wow this one was tricky!
This bug presents itself only when using planfiles, because when doing a
straight `terraform apply` the interpolations are left in place from the
Plan graph walk and paper over the issue. (This detail is what made it
so hard to reproduce initially.)
Basically, graph nodes for module variables are visited during the apply
walk and attempt to interpolate. During a destroy walk, no attributes
are interpolated from resource nodes, so these interpolations fail.
This scenario is supposed to be handled by the `PruneNoopTransformer` -
in fact it's described as the example use case in the comment above it!
So the bug had to do with the actual behavor of the Noop transformer.
The resource nodes were not properly reporting themselves as Noops
during a destroy, so they were being left in the graph.
This in turn triggered the module variable nodes to see that they had
another node depending on them, so they also reported that they could
not be pruned.
Therefore we had two nodes in the graph that were effectively noops but
were being visited anyways. The module variable nodes were already graph
leaves, which is why this error presented itself as just stray messages
instead of actual failure to destroy.
Fixes#5440Fixes#5708Fixes#4988Fixes#3268
A consequnce of the work done in #6185 was that variables which were in
a module but not set explicitly (i.e. the default value was relied upon)
were marked as type errors. This was reported in #6230.
This commit adds a test case for this and a patch which fixes the issue.
The flattening process was not properly drawing dependencies between provider
nodes in modules and their parent provider nodes.
Fixes#2832Fixes#4443Fixes#4865
These tests demonstrates a problem where the types to a module input are
not checked. For example, if a module - inner - defines a variable
"should_be_a_map" as a map, or with a default variable of map, we do not
fail if the user sets the variable value in the outer module to a string
value. This is also a problem in nested modules.
The implementation changes add a type checking step into the graph
evaluation process to ensure invalid types are not passed.
The ignore_changes diff filter was stripping out attributes on Create
but the diff was still making it down to the provider, so Create would
end up missing attributes, causing a full failure if any required
attributes were being ignored.
In addition, any changes that required a replacement of the resource
were causing problems with `ignore_chages`, which didn't properly filter
out the replacement when the triggering attributes were filtered out.
Refs #5627
When a user specifies `-target`s on a `terraform plan` and stores
the resulting diff in a plan file using `-out` - it usually works just
fine since the diff is scoped based on the targets.
When there are tainted resources in the state, however, graph nodes to
destroy them were popping back into the plan when it was being loaded
from a file. This was because Targets weren't being stored in the
Planfile, so Terraform didn't know to filter them out. (In the
non-Planfile scenario, we still had the Targets loaded directly from the
flags.)
By encoding Targets in with the Planfile we can ensure that the same
filters are always applied.
Backwards compatibility should be fine here, since we're just adding a
field. The gob encoder/decoder will just do the right thing (ignore/skip
the field) with planfiles stored w/ versions that don't know about
Targets.
Fixes#5183
Context:
As part of building up a Plan, Terraform needs to detect "orphaned"
resources--resources which are present in the state but not in the
config. This happens when config for those resources is removed by the
user, making it Terraform's responsibility to destroy them.
Both state and config are organized by Module into a logical tree, so
the process of finding orphans involves checking for orphaned Resources
in the current module and for orphaned Modules, which themselves will
have all their Resources marked as orphans.
Bug:
In #3114 a problem was exposed where, given a module tree that looked
like this:
```
root
|
+-- parent (empty, except for sub-modules)
|
+-- child1 (1 resource)
|
+-- child2 (1 resource)
```
If `parent` was removed, a bunch of error messages would occur during
the plan. The root cause of this was duplicate orphans appearing for the
resources in child1 and child2.
Fix:
This turned out to be a bug in orphaned module detection. When looking
for deeply nested orphaned modules, root.parent was getting added twice
as an orphaned module to the graph.
Here, we add an additional check to prevent a double add, which
addresses this scenario properly.
Fixes#3114 (the Provisioner side of it was fixed in #4877)
Fixes an interpolation race that was occurring when a tainted destroy
node and a primary destroy node both tried to interpolate a computed
count in their config. Since they were sharing a pointer to the _same_
config, depending on how the race played out one of them could catch the
config uninterpolated and would then throw a syntax error.
The `Copy()` tree implemented for this fix can probably be used
elsewhere - basically we should copy the config whenever we drop nodes
into the graph - but for now I'm just applying it to the place that
fixes this bug.
Fixes#4982 - Includes a test covering that race condition.
This commit adds support for declaring variable types in Terraform
configuration. Historically, the type has been inferred from the default
value, defaulting to string if no default was supplied. This has caused
users to devise workarounds if they wanted to declare a map but provide
values from a .tfvars file (for example).
The new syntax adds the "type" key to variable blocks:
```
variable "i_am_a_string" {
type = "string"
}
variable "i_am_a_map" {
type = "map"
}
```
This commit does _not_ extend the type system to include bools, integers
or floats - the only two types available are maps and strings.
Validation is performed if a default value is provided in order to
ensure that the default value type matches the declared type.
In the case that a type is not declared, the old logic is used for
determining the type. This allows backwards compatiblity with previous
Terraform configuration.
Instead of trying to skip non-targeted orphans as they are added to
the graph in OrphanTransformer, remove knowledge of targeting from
OrphanTransformer and instead make the orphan resource nodes properly
addressable.
That allows us to use existing logic in TargetTransformer to filter out
the nodes appropriately. This does require adding TargetTransformer to the
list of transforms that run during DynamicExpand so that targeting can
be applied to nodes with expanded counts.
Fixes#4515Fixes#2538Fixes#4462
This attempts to reproduce the issue described in #2598 whereby outputs
added after an apply are not reflected in the output. As per the issue
the outputs are described using the JSON syntax.
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
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
