From 45b04c826a40ed5c86eab16cce2cb3a1740c7378 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 15 May 2017 15:40:29 -0700 Subject: [PATCH] core: don't crash if no module state exists for multi var For child modules, a ModuleState isn't allocated until the first time a module instance is inserted into the state under the module's path. Normally interpolations of resource attributes are delayed until at least one resource has been created due to the nature of the dependency graph, but if the interpolation value is a multi-var (splat) then it is possible that the referenced resource has count=0 and thus created _no_ resource states when it was visited. Previously we would crash when trying to access the resource map for the nil module in order to count how many instances are present. Since we know there can't be any instances present in a nil module, we now preempt this crash by returning zero early. This edge-case does not apply to the root module because its ModuleState is allocated as part of initializing the main State instance. This fixes #14438. --- terraform/context_apply_test.go | 30 +++++++++++++++++++ terraform/interpolate.go | 13 ++++++++ terraform/interpolate_test.go | 28 +++++++++++++++++ .../child/child.tf | 15 ++++++++++ .../apply-multi-var-missing-state/root.tf | 7 +++++ 5 files changed, 93 insertions(+) create mode 100644 terraform/test-fixtures/apply-multi-var-missing-state/child/child.tf create mode 100644 terraform/test-fixtures/apply-multi-var-missing-state/root.tf diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 2b99ba59c..7d6a75e97 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -3447,6 +3447,36 @@ func TestContext2Apply_multiVarCountDec(t *testing.T) { } } +// Test that we can resolve a multi-var (splat) for the first resource +// created in a non-root module, which happens when the module state doesn't +// exist yet. +// https://github.com/hashicorp/terraform/issues/14438 +func TestContext2Apply_multiVarMissingState(t *testing.T) { + m := testModule(t, "apply-multi-var-missing-state") + p := testProvider("test") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + // First, apply with a count of 3 + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "test": testProviderFuncFixed(p), + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("plan failed: %s", err) + } + + // Before the relevant bug was fixed, Terraform would panic during apply. + if _, err := ctx.Apply(); err != nil { + t.Fatalf("apply failed: %s", err) + } + + // If we get here with no errors or panics then our test was successful. +} + func TestContext2Apply_nilDiff(t *testing.T) { m := testModule(t, "apply-good") p := testProvider("aws") diff --git a/terraform/interpolate.go b/terraform/interpolate.go index 19dcf21f5..0def295fa 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -731,6 +731,19 @@ func (i *Interpolater) resourceCountMax( return count, nil } + // If we have no module state in the apply walk, that suggests we've hit + // a rather awkward edge-case: the resource this variable refers to + // has count = 0 and is the only resource processed so far on this walk, + // and so we've ended up not creating any resource states yet. We don't + // create a module state until the first resource is written into it, + // so the module state doesn't exist when we get here. + // + // In this case we act as we would if we had been passed a module + // with an empty resource state map. + if ms == nil { + return 0, nil + } + // We need to determine the list of resource keys to get values from. // This needs to be sorted so the order is deterministic. We used to // use "cr.Count()" but that doesn't work if the count is interpolated diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index 60605fc33..46c0cbf8c 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -437,6 +437,34 @@ func TestInterpolater_resourceVariableMultiPartialUnknown(t *testing.T) { }) } +func TestInterpolater_resourceVariableMultiNoState(t *testing.T) { + // When evaluating a "splat" variable in a module that doesn't have + // any state yet, we should still be able to resolve to an empty + // list. + // See https://github.com/hashicorp/terraform/issues/14438 for an + // example of what we're testing for here. + lock := new(sync.RWMutex) + state := &State{ + Modules: []*ModuleState{}, + } + + i := &Interpolater{ + Module: testModule(t, "interpolate-resource-variable-multi"), + State: state, + StateLock: lock, + Operation: walkApply, + } + + scope := &InterpolationScope{ + Path: rootModulePath, + } + + testInterpolate(t, i, scope, "aws_instance.web.*.foo", ast.Variable{ + Type: ast.TypeList, + Value: []ast.Variable{}, + }) +} + // When a splat reference is made to an attribute that is a computed list, // the result should be unknown. func TestInterpolater_resourceVariableMultiList(t *testing.T) { diff --git a/terraform/test-fixtures/apply-multi-var-missing-state/child/child.tf b/terraform/test-fixtures/apply-multi-var-missing-state/child/child.tf new file mode 100644 index 000000000..928018627 --- /dev/null +++ b/terraform/test-fixtures/apply-multi-var-missing-state/child/child.tf @@ -0,0 +1,15 @@ + +# This resource gets visited first on the apply walk, but since it DynamicExpands +# to an empty subgraph it ends up being a no-op, leaving the module state +# uninitialized. +resource "test_thing" "a" { + count = 0 +} + +# This resource is visited second. During its eval walk we try to build the +# array for the null_resource.a.*.id interpolation, which involves iterating +# over all of the resource in the state. This should succeed even though the +# module state will be nil when evaluating the variable. +resource "test_thing" "b" { + a_ids = "${join(" ", null_resource.a.*.id)}" +} diff --git a/terraform/test-fixtures/apply-multi-var-missing-state/root.tf b/terraform/test-fixtures/apply-multi-var-missing-state/root.tf new file mode 100644 index 000000000..25a0a1f9b --- /dev/null +++ b/terraform/test-fixtures/apply-multi-var-missing-state/root.tf @@ -0,0 +1,7 @@ +// We test this in a child module, since the root module state exists +// very early on, even before any resources are created in it, but that is not +// true for child modules. + +module "child" { + source = "./child" +}