From 1e80c402bf96b87e0cc1a13ae138936511487d57 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 1 Mar 2017 11:48:38 -0500 Subject: [PATCH 1/3] add failing test for GH-12253 --- terraform/interpolate_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index 54f762fe7..e161b5e37 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -876,6 +876,31 @@ func TestInterpolator_sets(t *testing.T) { interfaceToVariableSwallowError(set)) } +// When a splat reference is made to a resource that is unknown, we should +// return an error. +func TestInterpolater_resourceUnknownVariableList(t *testing.T) { + i := &Interpolater{ + Module: testModule(t, "plan-computed-data-resource"), + State: NewState(), // state, + StateLock: new(sync.RWMutex), + } + + scope := &InterpolationScope{ + Path: rootModulePath, + } + + // missing "data" from the reference here + v, err := config.NewInterpolatedVariable("aws_vpc.bar.*.foo") + if err != nil { + t.Fatalf("err: %s", err) + } + + _, err = i.Values(scope, map[string]config.InterpolatedVariable{"foo": v}) + if err == nil { + t.Fatal("expected error interpolating invalid resource") + } +} + func testInterpolate( t *testing.T, i *Interpolater, scope *InterpolationScope, From 6fa4a591a0420881481e32b1a77750418e4d15a9 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 1 Mar 2017 11:49:41 -0500 Subject: [PATCH 2/3] prevent calling Count() on non-existent resources It turns out that a few use cases depend on not finding a resource without an error. The other code paths had sufficient nil checks for this, but there was one place where we called Count() that needed to be checked. If the existence of the resource matters, it would be caught at a higher level and still return an "unknown resource" error to the user. --- terraform/interpolate.go | 4 ++++ terraform/interpolate_test.go | 14 +++----------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/terraform/interpolate.go b/terraform/interpolate.go index a59af6e83..11d5a53dc 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -698,6 +698,10 @@ func (i *Interpolater) resourceCountMax( // from the state. Plan and so on may not have any state yet so // we do a full interpolation. if i.Operation != walkApply { + if cr == nil { + return 0, nil + } + count, err := cr.Count() if err != nil { return 0, err diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index e161b5e37..b85f2aad4 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -877,7 +877,7 @@ func TestInterpolator_sets(t *testing.T) { } // When a splat reference is made to a resource that is unknown, we should -// return an error. +// return an empty list rather than panicking. func TestInterpolater_resourceUnknownVariableList(t *testing.T) { i := &Interpolater{ Module: testModule(t, "plan-computed-data-resource"), @@ -889,16 +889,8 @@ func TestInterpolater_resourceUnknownVariableList(t *testing.T) { Path: rootModulePath, } - // missing "data" from the reference here - v, err := config.NewInterpolatedVariable("aws_vpc.bar.*.foo") - if err != nil { - t.Fatalf("err: %s", err) - } - - _, err = i.Values(scope, map[string]config.InterpolatedVariable{"foo": v}) - if err == nil { - t.Fatal("expected error interpolating invalid resource") - } + testInterpolate(t, i, scope, "aws_vpc.bar.*.foo", + interfaceToVariableSwallowError([]interface{}{})) } func testInterpolate( From ff445d3c2631b0dab8f33ffee993771d4de77785 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 1 Mar 2017 13:54:10 -0500 Subject: [PATCH 3/3] fix test that were relying on non-matching configs A couple interpolation tests were using invalid state that didn't match the config. These will still pass but were flushed out by an attempt to make this an error. The repl however still required interpolation without a config, and tests there will provide a indication if this behavior changes. --- terraform/interpolate_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index b85f2aad4..bdadedc4f 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -680,7 +680,7 @@ func TestInterpolator_interpolatedListOrder(t *testing.T) { &ModuleState{ Path: rootModulePath, Resources: map[string]*ResourceState{ - "aws_route53_zone.list": &ResourceState{ + "aws_route53_zone.yada": &ResourceState{ Type: "aws_route53_zone", Dependencies: []string{}, Primary: &InstanceState{ @@ -719,7 +719,7 @@ func TestInterpolator_interpolatedListOrder(t *testing.T) { list := []interface{}{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l"} - testInterpolate(t, i, scope, "aws_route53_zone.list.foo", + testInterpolate(t, i, scope, "aws_route53_zone.yada.foo", interfaceToVariableSwallowError(list)) } @@ -844,7 +844,7 @@ func TestInterpolator_sets(t *testing.T) { &ModuleState{ Path: rootModulePath, Resources: map[string]*ResourceState{ - "aws_network_interface.set": &ResourceState{ + "aws_route53_zone.yada": &ResourceState{ Type: "aws_network_interface", Dependencies: []string{}, Primary: &InstanceState{ @@ -872,7 +872,7 @@ func TestInterpolator_sets(t *testing.T) { set := []interface{}{"10.42.16.179"} - testInterpolate(t, i, scope, "aws_network_interface.set.private_ips", + testInterpolate(t, i, scope, "aws_route53_zone.yada.private_ips", interfaceToVariableSwallowError(set)) }