Prior to Terraform 0.12 there were certain behaviors we expected from
providers that were actually just details of the SDK and not part of the
enforced contract.
For 0.12 we're now codifying some of these behaviors explicitly via safety
checks in core, thus ensuring that all future providers will behave in a
consistent way that users can rely on.
Unfortunately, due to the hand-written nature of the mock provider
implementations we use in tests, they have been getting away with some
unusual behaviors that don't match our usual expectations, and our safety
checks now detect those as incorrect behaviors.
To address this, we make the minimal changes to each test to ensure that
its mock provider behaves in a consistent way, which requires that values
set in config be represented correctly in the plan and ultimately saved
in the new state, without any changes along the way. In particular, the
common testDiffFn implementation has historically used a number of special
hidden attributes to trigger special behaviors, and our new rules require
that these special settings propagate properly through the plan and into
the state.
Because Terraform Core has traditionally not checked that the final apply
result is consistent with what was planned, some of our apply tests were
producing inconsistent results.
Here we fix all of that so that they produce something compatible with
what they planned. This doesn't actually achieve anything in isolation,
but we're about to start enforcing this consistency in a subsequent
commit.
There are a few constructs from 0.11 and prior that cause 0.12 parsing to
fail altogether, which previously created a chicken/egg problem because
we need to install the providers in order to run "terraform 0.12upgrade"
and thus fix the problem.
This changes "terraform init" to use the new "early configuration" loader
for module and provider installation. This is built on the more permissive
parser in the terraform-config-inspect package, and so it allows us to
read out the top-level blocks from the configuration while accepting
legacy HCL syntax.
In the long run this will let us do version compatibility detection before
attempting a "real" config load, giving us better error messages for any
future syntax additions, but in the short term the key thing is that it
allows us to install the dependencies even if the configuration isn't
fully valid.
Because backend init still requires full configuration, this introduces a
new mode of terraform init where it detects heuristically if it seems like
we need to do a configuration upgrade and does a partial init if so,
before finally directing the user to run "terraform 0.12upgrade" before
running any other commands.
The heuristic here is based on two assumptions:
- If the "early" loader finds no errors but the normal loader does, the
configuration is likely to be valid for Terraform 0.11 but not 0.12.
- If there's already a version constraint in the configuration that
excludes Terraform versions prior to v0.12 then the configuration is
probably _already_ upgraded and so it's just a normal syntax error,
even if the early loader didn't detect it.
Once the upgrade process is removed in 0.13.0 (users will be required to
go stepwise 0.11 -> 0.12 -> 0.13 to upgrade after that), some of this can
be simplified to remove that special mode, but the idea of doing the
dependency version checks against the liberal parser will remain valuable
to increase our chances of reporting version-based incompatibilities
rather than syntax errors as we add new features in future.
Due to a quirk in how testDiffFn constructs its diff, compute doesn't
actually get included in the final result anymore under the new shims.
This result is still correct, nonetheless.
The prior behavior being asserted by this test was incorrect, since the
configuration calls for there to be two instances of the resource at the
end.
We also now assert on the generated plan since it's important to verify
that we are indeed planning to replace the zeroth instance but not the
first instance (which doesn't yet exist).
We now include attribute changes in destroy diffs, so the expected output
of this test includes these changes.
Also includes a fix to legacyDiffComparisonString to actually sort the
attribute changes by name in the rendered diff.
The testDiffFn doesn't include "compute" in the diff it produces and so
it no longer appears in the shimmed output.
This is just a quirk of this weird mock implementation; real providers
always copy all of the values from thec config into the diff before adding
in any other changes.
This was assuming a previous buggy behavior of failing to prune out an
empty module in the state. The new state code doesn't have this bug, so
we must update the expected result to reflect that.
This test's original outcome was invalid because it didn't actually
configure a mock apply function, and so _any_ apply operation would appear
to have destroyed the object it was given. This was visible in the fact
that the configuration contains aws_instance.bar but yet it was not
present in the expected state string.
Now we use the standard testApplyFn, and update the expected output to
include the aws_instance.bar object that is created by a Create change.
Now that we're marking errored creates as tainted in the state, the
object created by this test will get marked as tainted due to the error
about it containing an unknown value even after apply.
Since this test sets its own special schema for aws_instance, its expected
output must now be adjusted to only expect values that conform to that
schema. The extraneous attributes like "type" which testDiffFn produces
are no longer visible unless declared in schema.
We also need to now declare "id" as a computed attribute in order for our
state stringer shim to properly populate the formerly-special "ID".
Since StateReferences was implemented on NodeAbstractResource rather than
NodeAbstractResourceInstance it wasn't properly detecting references to
the same instance as self-references.
Now that we are using "seen" to filter out duplicates we can also simplify
how we handle these self-references by just pretending we saw them before
we even start the loop.
This change is confirmed by
TestContext2Apply_provisionerMultiSelfRefSingle
An earlier update to make this not use info.HumanId selected the wrong
fake "ami" name in the branch here.
Also, the error message for this failure was terrible. :(
Some earlier updates to it changed some things in our expected state
string. This doesn't fully fix it since there seems to still be a bug
related to recording dependencies.
Due to how often the state and plan types are referenced throughout
Terraform, there isn't a great way to switch them out gradually. As a
consequence, this huge commit gets us from the old world to a _compilable_
new world, but still has a large number of known test failures due to
key functionality being stubbed out.
The stubs here are for anything that interacts with providers, since we
now need to do the follow-up work to similarly replace the old
terraform.ResourceProvider interface with its replacement in the new
"providers" package. That work, along with work to fix the remaining
failing tests, will follow in subsequent commits.
The aim here was to replace all references to terraform.State and its
downstream types with states.State, terraform.Plan with plans.Plan,
state.State with statemgr.State, and switch to the new implementations of
the state and plan file formats. However, due to the number of times those
types are used, this also ended up affecting numerous other parts of core
such as terraform.Hook, the backend.Backend interface, and most of the CLI
commands.
Just as with 5861dbf3fc49b19587a31816eb06f511ab861bb4 before, I apologize
in advance to the person who inevitably just found this huge commit while
spelunking through the commit history.
Update test fixtures to work in our new world.
This is mostly changing out attribute names for those in the schema,
adding Providers to states, and updating the test-fixture
configurations.
Previously this test's fixture was depending on the fact that attribute
access of an unknown value would always succeed and return another unknown
value, but under the new language interpreter an unknown value still
retains type information and so accessing this "bar" attribute would fail
the semantic check.
We also have to fuss a bit here to work around the limitations of the
testDiffFn implementation, which doesn't have enough context to understand
that "list" in the ResourceConfig is the same as "list.#" in its result.
Since this part of the provider API will change soon to use cty values
directly, this change just accepts a slightly-odd-looking diff in the mean
time, with both "list" and "list.#" populated.
The behavior of the "count" meta-argument has changed so that its presence
(rather than its value) chooses whether the associated resource has
indexes. As a consequence, these tests which set count = 1 now produce
a single indexed instance with index 0.
Previously Terraform Core was unaware of the structure of a resource type
schema and so the strange behavior of our testDiffFn caused some
attributes to not appear at all in the result. With core now more aware,
it "fills in" these missing items before calling, and as a result they
now appear in the diff even with the testDiffFn.
In real code, where helper/schema is constructing diffs, this situation
doesn't arise because the framework always produces schema-compatible
diffs.
Mostly this is about updating ctx.Plan callers to expect diags instead of
err, but also includes a few light updates to test fixtures, and a fix to
testModuleInline.
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.
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.
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.
The provisionerFail_createBeforeDestroy test was verifying the incorrect
output. The create_before_destroy instance in the state has an ID of
"bar" with require_new="abc", and a new instance would get an ID of
"foo" with require_new="xyz". The existing test was expecting the
following state:
aws_instance.bar: (1 deposed)
ID = bar
provider = provider.aws
require_new = abc
Deposed ID 1 = foo (tainted)
Which showed "bar" still the primary instance in the state, with the new
instance "foo" as being the deposed instance, though properly tainted.
The new output is:
aws_instance.bar: (tainted) (1 deposed)
ID = foo
provider = provider.aws
require_new = xyz
type = aws_instance
Deposed ID 1 = bar
Showing the new "foo instance as being the primary instance in the
state, with "bar" as the deposed instance.
Remove the module entry from the state if a module is no longer in the
configuration. Modules are not removed if there are any existing
resources with the module path as a prefix. The only time this should be
the case is if a module was removed in the config, but the apply didn't
target that module.
Create a NodeModuleRemoved and an associated EvalDeleteModule to track
the module in the graph then remove it from the state. The
NodeModuleRemoved dependencies are simply any other node which contains
the module path as a prefix in its path.
This could have probably been done much easier as a step in pruning the
state, but modules are going to have to be promoted to full graph nodes
anyway in order to support count.
