Merge pull request #17241 from hashicorp/jbardin/destroy-with-locals

Fix destroy-time handling of outputs and local values

terraform/context_apply_test.go

@@ -2,6 +2,7 @@ package terraform
 
 import (
  "bytes"
+ "errors"
  "fmt"
  "reflect"
  "runtime"
@@ -2590,24 +2591,14 @@ func TestContext2Apply_moduleDestroyOrder(t *testing.T) {
  &ModuleState{
  Path: rootModulePath,
  Resources: map[string]*ResourceState{
- "aws_instance.b": &ResourceState{
- Type: "aws_instance",
- Primary: &InstanceState{
- ID: "b",
- },
- },
+ "aws_instance.b": resourceState("aws_instance", "b"),
  },
  },
 
  &ModuleState{
  Path: []string{"root", "child"},
  Resources: map[string]*ResourceState{
- "aws_instance.a": &ResourceState{
- Type: "aws_instance",
- Primary: &InstanceState{
- ID: "a",
- },
- },
+ "aws_instance.a": resourceState("aws_instance", "a"),
  },
  Outputs: map[string]*OutputState{
  "a_output": &OutputState{
@@ -7594,6 +7585,212 @@ aws_instance.bar:
  `)
 }
 
+func TestContext2Apply_destroyProvisionerWithLocals(t *testing.T) {
+ m := testModule(t, "apply-provisioner-destroy-locals")
+ p := testProvider("aws")
+ p.ApplyFn = testApplyFn
+ p.DiffFn = testDiffFn
+
+ pr := testProvisioner()
+ pr.ApplyFn = func(_ *InstanceState, rc *ResourceConfig) error {
+ cmd, ok := rc.Get("command")
+ if !ok || cmd != "local" {
+ fmt.Printf("%#v\n", rc.Config)
+ return fmt.Errorf("provisioner got %v:%s", ok, cmd)
+ }
+ return nil
+ }
+
+ ctx := testContext2(t, &ContextOpts{
+ Module: m,
+ ProviderResolver: ResourceProviderResolverFixed(
+ map[string]ResourceProviderFactory{
+ "aws": testProviderFuncFixed(p),
+ },
+ ),
+ Provisioners: map[string]ResourceProvisionerFactory{
+ "shell": testProvisionerFuncFixed(pr),
+ },
+ State: &State{
+ Modules: []*ModuleState{
+ &ModuleState{
+ Path: []string{"root"},
+ Resources: map[string]*ResourceState{
+ "aws_instance.foo": resourceState("aws_instance", "1234"),
+ },
+ },
+ },
+ },
+ Destroy: true,
+ // the test works without targeting, but this also tests that the local
+ // node isn't inadvertently pruned because of the wrong evaluation
+ // order.
+ Targets: []string{"aws_instance.foo"},
+ })
+
+ if _, err := ctx.Plan(); err != nil {
+ t.Fatal(err)
+ }
+
+ if _, err := ctx.Apply(); err != nil {
+ t.Fatal(err)
+ }
+
+ if !pr.ApplyCalled {
+ t.Fatal("provisioner not called")
+ }
+}
+
+// this also tests a local value in the config referencing a resource that
+// wasn't in the state during destroy.
+func TestContext2Apply_destroyProvisionerWithMultipleLocals(t *testing.T) {
+ m := testModule(t, "apply-provisioner-destroy-multiple-locals")
+ p := testProvider("aws")
+ p.ApplyFn = testApplyFn
+ p.DiffFn = testDiffFn
+
+ pr := testProvisioner()
+ pr.ApplyFn = func(is *InstanceState, rc *ResourceConfig) error {
+ cmd, ok := rc.Get("command")
+ if !ok {
+ return errors.New("no command in provisioner")
+ }
+
+ switch is.ID {
+ case "1234":
+ if cmd != "local" {
+ return fmt.Errorf("provisioner %q got:%q", is.ID, cmd)
+ }
+ case "3456":
+ if cmd != "1234" {
+ return fmt.Errorf("provisioner %q got:%q", is.ID, cmd)
+ }
+ default:
+ t.Fatal("unknown instance")
+ }
+ return nil
+ }
+
+ ctx := testContext2(t, &ContextOpts{
+ Module: m,
+ ProviderResolver: ResourceProviderResolverFixed(
+ map[string]ResourceProviderFactory{
+ "aws": testProviderFuncFixed(p),
+ },
+ ),
+ Provisioners: map[string]ResourceProvisionerFactory{
+ "shell": testProvisionerFuncFixed(pr),
+ },
+ State: &State{
+ Modules: []*ModuleState{
+ &ModuleState{
+ Path: []string{"root"},
+ Resources: map[string]*ResourceState{
+ "aws_instance.foo": resourceState("aws_instance", "1234"),
+ "aws_instance.bar": resourceState("aws_instance", "3456"),
+ },
+ },
+ },
+ },
+ Destroy: true,
+ })
+
+ if _, err := ctx.Plan(); err != nil {
+ t.Fatal(err)
+ }
+
+ if _, err := ctx.Apply(); err != nil {
+ t.Fatal(err)
+ }
+
+ if !pr.ApplyCalled {
+ t.Fatal("provisioner not called")
+ }
+}
+
+func TestContext2Apply_destroyProvisionerWithOutput(t *testing.T) {
+ m := testModule(t, "apply-provisioner-destroy-outputs")
+ p := testProvider("aws")
+ p.ApplyFn = testApplyFn
+ p.DiffFn = testDiffFn
+
+ pr := testProvisioner()
+ pr.ApplyFn = func(is *InstanceState, rc *ResourceConfig) error {
+ cmd, ok := rc.Get("command")
+ if !ok || cmd != "3" {
+ fmt.Printf("%#v\n", rc.Config)
+ return fmt.Errorf("provisioner for %s got %v:%s", is.ID, ok, cmd)
+ }
+ return nil
+ }
+
+ ctx := testContext2(t, &ContextOpts{
+ Module: m,
+ ProviderResolver: ResourceProviderResolverFixed(
+ map[string]ResourceProviderFactory{
+ "aws": testProviderFuncFixed(p),
+ },
+ ),
+ Provisioners: map[string]ResourceProvisionerFactory{
+ "shell": testProvisionerFuncFixed(pr),
+ },
+ State: &State{
+ Modules: []*ModuleState{
+ &ModuleState{
+ Path: []string{"root"},
+ Resources: map[string]*ResourceState{
+ "aws_instance.foo": resourceState("aws_instance", "1"),
+ },
+ Outputs: map[string]*OutputState{
+ "value": {
+ Type: "string",
+ Value: "3",
+ },
+ },
+ },
+ &ModuleState{
+ Path: []string{"root", "mod"},
+ Resources: map[string]*ResourceState{
+ "aws_instance.baz": resourceState("aws_instance", "3"),
+ },
+ // state needs to be properly initialized
+ Outputs: map[string]*OutputState{},
+ },
+ &ModuleState{
+ Path: []string{"root", "mod2"},
+ Resources: map[string]*ResourceState{
+ "aws_instance.bar": resourceState("aws_instance", "2"),
+ },
+ },
+ },
+ },
+ Destroy: true,
+
+ // targeting the source of the value used by all resources shoudl still
+ // destroy them all.
+ Targets: []string{"module.mod.aws_instance.baz"},
+ })
+
+ if _, err := ctx.Plan(); err != nil {
+ t.Fatal(err)
+ }
+
+ state, err := ctx.Apply()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !pr.ApplyCalled {
+ t.Fatal("provisioner not called")
+ }
+
+ // confirm all outputs were removed too
+ for _, mod := range state.Modules {
+ if len(mod.Outputs) > 0 {
+ t.Fatalf("output left in module state: %#v\n", mod)
+ }
+ }
+}
+
 func TestContext2Apply_targetedDestroyCountDeps(t *testing.T) {
  m := testModule(t, "apply-destroy-targeted-count")
  p := testProvider("aws")
@@ -9000,6 +9197,10 @@ func TestContext2Apply_destroyWithLocals(t *testing.T) {
  Type: "aws_instance",
  Primary: &InstanceState{
  ID: "foo",
+ // FIXME: id should only exist in one place
+ Attributes: map[string]string{
+ "id": "foo",
+ },
  },
  },
  },

terraform/context_test.go

@@ -375,6 +375,9 @@ func resourceState(resourceType, resourceID string) *ResourceState {
  Type: resourceType,
  Primary: &InstanceState{
  ID: resourceID,
+ Attributes: map[string]string{
+ "id": resourceID,
+ },
  },
  }
 }

terraform/graph_builder_apply.go

@@ -119,11 +119,19 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
  // Connect references so ordering is correct
  &ReferenceTransformer{},
 
- // Reverse the edges to outputs and locals, so that
+ // Handle destroy time transformations for output and local values.
+ // Reverse the edges from outputs and locals, so that
  // interpolations don't fail during destroy.
+ // Create a destroy node for outputs to remove them from the state.
+ // Prune unreferenced values, which may have interpolations that can't
+ // be resolved.
  GraphTransformIf(
  func() bool { return b.Destroy },
- &DestroyValueReferenceTransformer{},
+ GraphTransformMulti(
+ &DestroyValueReferenceTransformer{},
+ &DestroyOutputTransformer{},
+ &PruneUnusedValuesTransformer{},
+ ),
  ),
 
  // Add the node to fix the state count boundaries

terraform/interpolate.go

@@ -518,6 +518,16 @@ func (i *Interpolater) computeResourceVariable(
  return &v, err
  }
 
+ // special case for the "id" field which is usually also an attribute
+ if v.Field == "id" && r.Primary.ID != "" {
+ // This is usually pulled from the attributes, but is sometimes missing
+ // during destroy. We can return the ID field in this case.
+ // FIXME: there should only be one ID to rule them all.
+ log.Printf("[WARN] resource %s missing 'id' attribute", v.ResourceId())
+ v, err := hil.InterfaceToVariable(r.Primary.ID)
+ return &v, err
+ }
+
  // computed list or map attribute
  _, isList = r.Primary.Attributes[v.Field+".#"]
  _, isMap = r.Primary.Attributes[v.Field+".%"]
@@ -655,6 +665,11 @@ func (i *Interpolater) computeResourceMultiVariable(
  continue
  }
 
+ if v.Field == "id" && r.Primary.ID != "" {
+ log.Printf("[WARN] resource %s missing 'id' attribute", v.ResourceId())
+ values = append(values, r.Primary.ID)
+ }
+
  // computed list or map attribute
  _, isList := r.Primary.Attributes[v.Field+".#"]
  _, isMap := r.Primary.Attributes[v.Field+".%"]

terraform/interpolate_test.go

@@ -129,6 +129,40 @@ func TestInterpolater_localVal(t *testing.T) {
  })
 }
 
+func TestInterpolater_missingID(t *testing.T) {
+ lock := new(sync.RWMutex)
+ state := &State{
+ Modules: []*ModuleState{
+ &ModuleState{
+ Path: rootModulePath,
+ Resources: map[string]*ResourceState{
+ "aws_instance.web": &ResourceState{
+ Type: "aws_instance",
+ Primary: &InstanceState{
+ ID: "bar",
+ },
+ },
+ },
+ },
+ },
+ }
+
+ i := &Interpolater{
+ Module: testModule(t, "interpolate-resource-variable"),
+ State: state,
+ StateLock: lock,
+ }
+
+ scope := &InterpolationScope{
+ Path: rootModulePath,
+ }
+
+ testInterpolate(t, i, scope, "aws_instance.web.id", ast.Variable{
+ Value: "bar",
+ Type: ast.TypeString,
+ })
+}
+
 func TestInterpolater_pathCwd(t *testing.T) {
  i := &Interpolater{}
  scope := &InterpolationScope{}