From a1ec81964b7d60407e177cd70f8f6d82e4948c4b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 17 Feb 2017 14:29:22 -0800 Subject: [PATCH] terraform: destroy ordering needs to handle destroy provisioner edges This ensures that things aren't destroyed before their values are used. --- terraform/context_apply_test.go | 70 +++++++++++++++++++ terraform/node_resource_abstract.go | 6 +- .../apply-provisioner-destroy-ref/main.tf | 12 ++++ terraform/transform_destroy_edge.go | 12 +++- 4 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 terraform/test-fixtures/apply-provisioner-destroy-ref/main.tf diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 15af89edcb..477beb3aee 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -4701,6 +4701,76 @@ module.child: } } +func TestContext2Apply_provisionerDestroyRef(t *testing.T) { + m := testModule(t, "apply-provisioner-destroy-ref") + p := testProvider("aws") + pr := testProvisioner() + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + pr.ApplyFn = func(rs *InstanceState, c *ResourceConfig) error { + val, ok := c.Config["foo"] + if !ok || val != "hello" { + return fmt.Errorf("bad value for foo: %v %#v", val, c) + } + + return nil + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "key": "hello", + }, + }, + }, + + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "bar", + }, + }, + }, + }, + }, + } + + ctx := testContext2(t, &ContextOpts{ + Module: m, + State: state, + Destroy: true, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + Provisioners: map[string]ResourceProvisionerFactory{ + "shell": testProvisionerFuncFixed(pr), + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + state, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + checkStateString(t, state, ``) + + // Verify apply was invoked + if !pr.ApplyCalled { + t.Fatalf("provisioner not invoked") + } +} + func TestContext2Apply_provisionerResourceRef(t *testing.T) { m := testModule(t, "apply-provisioner-resource-ref") p := testProvider("aws") diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index c9e9ab03a4..e4577e9dbf 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -96,8 +96,10 @@ func (n *NodeAbstractResource) References() []string { result = append(result, ReferencesFromConfig(c.RawCount)...) result = append(result, ReferencesFromConfig(c.RawConfig)...) for _, p := range c.Provisioners { - result = append(result, ReferencesFromConfig(p.ConnInfo)...) - result = append(result, ReferencesFromConfig(p.RawConfig)...) + if p.When == config.ProvisionerWhenCreate { + result = append(result, ReferencesFromConfig(p.ConnInfo)...) + result = append(result, ReferencesFromConfig(p.RawConfig)...) + } } return result diff --git a/terraform/test-fixtures/apply-provisioner-destroy-ref/main.tf b/terraform/test-fixtures/apply-provisioner-destroy-ref/main.tf new file mode 100644 index 0000000000..01561a5163 --- /dev/null +++ b/terraform/test-fixtures/apply-provisioner-destroy-ref/main.tf @@ -0,0 +1,12 @@ +resource "aws_instance" "bar" { + key = "hello" +} + +resource "aws_instance" "foo" { + foo = "bar" + + provisioner "shell" { + foo = "${aws_instance.bar.key}" + when = "destroy" + } +} diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index c837cf1725..22be1ab62a 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -159,9 +159,15 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { // This part is a little bit weird but is the best way to // find the dependencies we need to: build a graph and use the // attach config and state transformers then ask for references. - node := &NodeAbstractResource{Addr: addr} - tempG.Add(node) - tempDestroyed = append(tempDestroyed, node) + abstract := &NodeAbstractResource{Addr: addr} + tempG.Add(abstract) + tempDestroyed = append(tempDestroyed, abstract) + + // We also add the destroy version here since the destroy can + // depend on things that the creation doesn't (destroy provisioners). + destroy := &NodeDestroyResource{NodeAbstractResource: abstract} + tempG.Add(destroy) + tempDestroyed = append(tempDestroyed, destroy) } // Run the graph transforms so we have the information we need to