diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 4e64109bb..317369bcf 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -1328,19 +1328,7 @@ func testContext2Apply_destroyDependsOn(t *testing.T) { // Test that destroy ordering is correct with dependencies only // in the state. func TestContext2Apply_destroyDependsOnStateOnly(t *testing.T) { - // It is possible for this to be racy, so we loop a number of times - // just to check. - for i := 0; i < 10; i++ { - testContext2Apply_destroyDependsOnStateOnly(t) - } -} - -func testContext2Apply_destroyDependsOnStateOnly(t *testing.T) { - m := testModule(t, "empty") - p := testProvider("aws") - p.ApplyFn = testApplyFn - p.DiffFn = testDiffFn - state := MustShimLegacyState(&State{ + legacyState := MustShimLegacyState(&State{ Modules: []*ModuleState{ &ModuleState{ Path: rootModulePath, @@ -1368,6 +1356,65 @@ func testContext2Apply_destroyDependsOnStateOnly(t *testing.T) { }, }) + newState := states.NewState() + root := newState.EnsureModule(addrs.RootModuleInstance) + root.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + }.Instance(addrs.NoKey), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"foo"}`), + Dependencies: []addrs.AbsResource{}, + }, + addrs.ProviderConfig{ + Type: "aws", + }.Absolute(addrs.RootModuleInstance), + ) + root.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "bar", + }.Instance(addrs.NoKey), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"bar"}`), + Dependencies: []addrs.AbsResource{ + addrs.AbsResource{ + Resource: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + }, + Module: root.Addr, + }, + }, + }, + addrs.ProviderConfig{ + Type: "aws", + }.Absolute(addrs.RootModuleInstance), + ) + + // It is possible for this to be racy, so we loop a number of times + // just to check. + for i := 0; i < 10; i++ { + t.Run("legacy", func(t *testing.T) { + testContext2Apply_destroyDependsOnStateOnly(t, legacyState) + }) + t.Run("new", func(t *testing.T) { + testContext2Apply_destroyDependsOnStateOnly(t, newState) + }) + } +} + +func testContext2Apply_destroyDependsOnStateOnly(t *testing.T, state *states.State) { + m := testModule(t, "empty") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn // Record the order we see Apply var actual []string var actualLock sync.Mutex @@ -1408,19 +1455,7 @@ func testContext2Apply_destroyDependsOnStateOnly(t *testing.T) { // Test that destroy ordering is correct with dependencies only // in the state within a module (GH-11749) func TestContext2Apply_destroyDependsOnStateOnlyModule(t *testing.T) { - // It is possible for this to be racy, so we loop a number of times - // just to check. - for i := 0; i < 10; i++ { - testContext2Apply_destroyDependsOnStateOnlyModule(t) - } -} - -func testContext2Apply_destroyDependsOnStateOnlyModule(t *testing.T) { - m := testModule(t, "empty") - p := testProvider("aws") - p.ApplyFn = testApplyFn - p.DiffFn = testDiffFn - state := MustShimLegacyState(&State{ + legacyState := MustShimLegacyState(&State{ Modules: []*ModuleState{ &ModuleState{ Path: []string{"root", "child"}, @@ -1448,6 +1483,66 @@ func testContext2Apply_destroyDependsOnStateOnlyModule(t *testing.T) { }, }) + newState := states.NewState() + child := newState.EnsureModule(addrs.RootModuleInstance.Child("child", addrs.NoKey)) + child.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + }.Instance(addrs.NoKey), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"foo"}`), + Dependencies: []addrs.AbsResource{}, + }, + addrs.ProviderConfig{ + Type: "aws", + }.Absolute(addrs.RootModuleInstance), + ) + child.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "bar", + }.Instance(addrs.NoKey), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"bar"}`), + Dependencies: []addrs.AbsResource{ + addrs.AbsResource{ + Resource: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + }, + Module: child.Addr, + }, + }, + }, + addrs.ProviderConfig{ + Type: "aws", + }.Absolute(addrs.RootModuleInstance), + ) + + // It is possible for this to be racy, so we loop a number of times + // just to check. + for i := 0; i < 10; i++ { + t.Run("legacy", func(t *testing.T) { + testContext2Apply_destroyDependsOnStateOnlyModule(t, legacyState) + }) + t.Run("new", func(t *testing.T) { + testContext2Apply_destroyDependsOnStateOnlyModule(t, newState) + }) + } +} + +func testContext2Apply_destroyDependsOnStateOnlyModule(t *testing.T, state *states.State) { + m := testModule(t, "empty") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + // Record the order we see Apply var actual []string var actualLock sync.Mutex @@ -3466,6 +3561,9 @@ module.B: provider = provider.aws foo = foo type = aws_instance + + Dependencies: + module.A.aws_instance.foo `) } @@ -8706,7 +8804,7 @@ aws_instance.foo: type = aws_instance Dependencies: - module.child + module.child.aws_instance.mod module.child: aws_instance.mod: diff --git a/terraform/context_refresh_test.go b/terraform/context_refresh_test.go index dbd57eb53..e956e7791 100644 --- a/terraform/context_refresh_test.go +++ b/terraform/context_refresh_test.go @@ -1964,3 +1964,81 @@ func TestContext2Refresh_dataResourceDependsOn(t *testing.T) { t.Fatalf("unexpected errors: %s", diags.Err()) } } + +// verify that dependencies are updated in the state during refresh +func TestRefresh_updateDependencies(t *testing.T) { + state := states.NewState() + root := state.EnsureModule(addrs.RootModuleInstance) + root.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + }.Instance(addrs.NoKey), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"foo"}`), + }, + addrs.ProviderConfig{ + Type: "aws", + }.Absolute(addrs.RootModuleInstance), + ) + root.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "bar", + }.Instance(addrs.NoKey), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"bar","foo":"foo"}`), + }, + addrs.ProviderConfig{ + Type: "aws", + }.Absolute(addrs.RootModuleInstance), + ) + + m := testModuleInline(t, map[string]string{ + "main.tf": ` +resource "aws_instance" "bar" { + foo = aws_instance.foo.id +} + +resource "aws_instance" "foo" { +}`, + }) + + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + ctx := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[string]providers.Factory{ + "aws": testProviderFuncFixed(p), + }, + ), + State: state, + }) + + result, diags := ctx.Refresh() + if diags.HasErrors() { + t.Fatalf("plan errors: %s", diags.Err()) + } + + expect := strings.TrimSpace(` +aws_instance.bar: + ID = bar + provider = provider.aws + foo = foo + + Dependencies: + aws_instance.foo +aws_instance.foo: + ID = foo + provider = provider.aws +`) + + checkStateString(t, result, expect) +} diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index 6b839fcaf..cbabaefb1 100644 --- a/terraform/eval_apply.go +++ b/terraform/eval_apply.go @@ -24,7 +24,6 @@ import ( type EvalApply struct { Addr addrs.ResourceInstance Config *configs.Resource - Dependencies []addrs.Referenceable State **states.ResourceInstanceObject Change **plans.ResourceInstanceChange ProviderAddr addrs.AbsProviderConfig @@ -271,10 +270,9 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) { var newState *states.ResourceInstanceObject if !newVal.IsNull() { // null value indicates that the object is deleted, so we won't set a new state in that case newState = &states.ResourceInstanceObject{ - Status: states.ObjectReady, - Value: newVal, - Private: resp.Private, - Dependencies: n.Dependencies, // Should be populated by the caller from the StateDependencies method on the resource instance node + Status: states.ObjectReady, + Value: newVal, + Private: resp.Private, } } diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index 4999480f5..e58ec7c6e 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -21,7 +21,6 @@ import ( type EvalReadData struct { Addr addrs.ResourceInstance Config *configs.Resource - Dependencies []addrs.Referenceable Provider *providers.Interface ProviderAddr addrs.AbsProviderConfig ProviderSchema **ProviderSchema @@ -161,9 +160,8 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { } if n.OutputState != nil { state := &states.ResourceInstanceObject{ - Value: change.After, - Status: states.ObjectPlanned, // because the partial value in the plan must be used for now - Dependencies: n.Dependencies, + Value: change.After, + Status: states.ObjectPlanned, // because the partial value in the plan must be used for now } *n.OutputState = state } @@ -275,9 +273,8 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { }, } state := &states.ResourceInstanceObject{ - Value: change.After, - Status: states.ObjectReady, // because we completed the read from the provider - Dependencies: n.Dependencies, + Value: change.After, + Status: states.ObjectReady, // because we completed the read from the provider } err = ctx.Hook(func(h Hook) (HookAction, error) { @@ -306,14 +303,13 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { // EvalReadDataApply is an EvalNode implementation that executes a data // resource's ReadDataApply method to read data from the data source. type EvalReadDataApply struct { - Addr addrs.ResourceInstance - Provider *providers.Interface - ProviderAddr addrs.AbsProviderConfig - ProviderSchema **ProviderSchema - Output **states.ResourceInstanceObject - Config *configs.Resource - Change **plans.ResourceInstanceChange - StateReferences []addrs.Referenceable + Addr addrs.ResourceInstance + Provider *providers.Interface + ProviderAddr addrs.AbsProviderConfig + ProviderSchema **ProviderSchema + Output **states.ResourceInstanceObject + Config *configs.Resource + Change **plans.ResourceInstanceChange } func (n *EvalReadDataApply) Eval(ctx EvalContext) (interface{}, error) { @@ -385,9 +381,8 @@ func (n *EvalReadDataApply) Eval(ctx EvalContext) (interface{}, error) { if n.Output != nil { *n.Output = &states.ResourceInstanceObject{ - Value: newVal, - Status: states.ObjectReady, - Dependencies: n.StateReferences, + Value: newVal, + Status: states.ObjectReady, } } diff --git a/terraform/eval_state.go b/terraform/eval_state.go index b611113e3..270507051 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -200,6 +200,10 @@ type EvalWriteState struct { // ProviderAddr is the address of the provider configuration that // produced the given object. ProviderAddr addrs.AbsProviderConfig + + // Dependencies are the inter-resource dependencies to be stored in the + // state. + Dependencies []addrs.AbsResource } func (n *EvalWriteState) Eval(ctx EvalContext) (interface{}, error) { @@ -215,7 +219,6 @@ func (n *EvalWriteState) Eval(ctx EvalContext) (interface{}, error) { if n.ProviderAddr.ProviderConfig.Type == "" { return nil, fmt.Errorf("failed to write state for %s, missing provider type", absAddr) } - obj := *n.State if obj == nil || obj.Value.IsNull() { // No need to encode anything: we'll just write it directly. @@ -223,6 +226,10 @@ func (n *EvalWriteState) Eval(ctx EvalContext) (interface{}, error) { log.Printf("[TRACE] EvalWriteState: removing state object for %s", absAddr) return nil, nil } + + // store the new deps in the state + obj.Dependencies = n.Dependencies + if n.ProviderSchema == nil || *n.ProviderSchema == nil { // Should never happen, unless our state object is nil panic("EvalWriteState used with pointer to nil ProviderSchema object") diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 73b6b9a56..615731328 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -155,6 +155,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Connect references so ordering is correct &ReferenceTransformer{}, + &AttachDependenciesTransformer{}, // Destruction ordering &DestroyEdgeTransformer{ diff --git a/terraform/graph_builder_refresh.go b/terraform/graph_builder_refresh.go index 0342cdbe8..1c7ae4898 100644 --- a/terraform/graph_builder_refresh.go +++ b/terraform/graph_builder_refresh.go @@ -165,6 +165,7 @@ func (b *RefreshGraphBuilder) Steps() []GraphTransformer { // Connect so that the references are ready for targeting. We'll // have to connect again later for providers and so on. &ReferenceTransformer{}, + &AttachDependenciesTransformer{}, // Target &TargetsTransformer{ diff --git a/terraform/node_data_refresh.go b/terraform/node_data_refresh.go index 2a4327f91..7133d42bf 100644 --- a/terraform/node_data_refresh.go +++ b/terraform/node_data_refresh.go @@ -169,7 +169,6 @@ func (n *NodeRefreshableDataResourceInstance) EvalTree() EvalNode { &EvalReadData{ Addr: addr.Resource, Config: n.Config, - Dependencies: n.StateReferences(), Provider: &provider, ProviderAddr: n.ResolvedProvider, ProviderSchema: &providerSchema, diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index d147b42e4..a1a4fc0b5 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -3,7 +3,6 @@ package terraform import ( "fmt" "log" - "sort" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" @@ -93,8 +92,8 @@ type NodeAbstractResourceInstance struct { // The fields below will be automatically set using the Attach // interfaces if you're running those transforms, but also be explicitly // set if you already have that information. - ResourceState *states.Resource + Dependencies []addrs.AbsResource } var ( @@ -220,7 +219,8 @@ func (n *NodeAbstractResource) References() []*addrs.Reference { func (n *NodeAbstractResourceInstance) References() []*addrs.Reference { // If we have a configuration attached then we'll delegate to our // embedded abstract resource, which knows how to extract dependencies - // from configuration. + // from configuration. If there is no config, then the dependencies will + // be connected during destroy from those stored in the state. if n.Config != nil { if n.Schema == nil { // We'll produce a log message about this out here so that @@ -232,8 +232,10 @@ func (n *NodeAbstractResourceInstance) References() []*addrs.Reference { return n.NodeAbstractResource.References() } - // Otherwise, if we have state then we'll use the values stored in state - // as a fallback. + // FIXME: remove once the deprecated DependsOn values have been removed from state + // The state dependencies are now connected in a separate transformation as + // absolute addresses, but we need to keep this here until we can be sure + // that no state will need to use the old depends_on references. if rs := n.ResourceState; rs != nil { if s := rs.Instance(n.InstanceKey); s != nil { // State is still storing dependencies as old-style strings, so we'll @@ -248,23 +250,23 @@ func (n *NodeAbstractResourceInstance) References() []*addrs.Reference { // https://github.com/hashicorp/terraform/issues/21407 if s.Current == nil { log.Printf("[WARN] no current state found for %s", n.Name()) - } else { - for _, addr := range s.Current.Dependencies { - if addr == nil { - // Should never happen; indicates a bug in the state loader - panic(fmt.Sprintf("dependencies for current object on %s contains nil address", n.ResourceInstanceAddr())) - } - - // This is a little weird: we need to manufacture an addrs.Reference - // with a fake range here because the state isn't something we can - // make source references into. - result = append(result, &addrs.Reference{ - Subject: addr, - SourceRange: tfdiags.SourceRange{ - Filename: "(state file)", - }, - }) + return nil + } + for _, addr := range s.Current.DependsOn { + if addr == nil { + // Should never happen; indicates a bug in the state loader + panic(fmt.Sprintf("dependencies for current object on %s contains nil address", n.ResourceInstanceAddr())) } + + // This is a little weird: we need to manufacture an addrs.Reference + // with a fake range here because the state isn't something we can + // make source references into. + result = append(result, &addrs.Reference{ + Subject: addr, + SourceRange: tfdiags.SourceRange{ + Filename: "(state file)", + }, + }) } return result } @@ -288,67 +290,17 @@ func dottedInstanceAddr(tr addrs.ResourceInstance) string { return tr.Resource.String() + suffix } -// StateReferences returns the dependencies to put into the state for -// this resource. -func (n *NodeAbstractResourceInstance) StateReferences() []addrs.Referenceable { - selfAddrs := n.ReferenceableAddrs() - - // Since we don't include the source location references in our - // results from this method, we'll also filter out duplicates: - // there's no point in listing the same object twice without - // that additional context. - seen := map[string]struct{}{} - - // Pretend that we've already "seen" all of our own addresses so that we - // won't record self-references in the state. This can arise if, for - // example, a provisioner for a resource refers to the resource itself, - // which is valid (since provisioners always run after apply) but should - // not create an explicit dependency edge. - for _, selfAddr := range selfAddrs { - seen[selfAddr.String()] = struct{}{} - if riAddr, ok := selfAddr.(addrs.ResourceInstance); ok { - seen[riAddr.ContainingResource().String()] = struct{}{} +// StateDependencies returns the dependencies saved in the state. +func (n *NodeAbstractResourceInstance) StateDependencies() []addrs.AbsResource { + if rs := n.ResourceState; rs != nil { + if s := rs.Instance(n.InstanceKey); s != nil { + if s.Current != nil { + return s.Current.Dependencies + } } } - depsRaw := n.References() - deps := make([]addrs.Referenceable, 0, len(depsRaw)) - for _, d := range depsRaw { - subj := d.Subject - if mco, isOutput := subj.(addrs.ModuleCallOutput); isOutput { - // For state dependencies, we simplify outputs to just refer - // to the module as a whole. It's not really clear why we do this, - // but this logic is preserved from before the 0.12 rewrite of - // this function. - subj = mco.Call - } - - k := subj.String() - if _, exists := seen[k]; exists { - continue - } - seen[k] = struct{}{} - switch tr := subj.(type) { - case addrs.ResourceInstance: - deps = append(deps, tr) - case addrs.Resource: - deps = append(deps, tr) - case addrs.ModuleCallInstance: - deps = append(deps, tr) - default: - // No other reference types are recorded in the state. - } - } - - // We'll also sort them, since that'll avoid creating changes in the - // serialized state that make no semantic difference. - sort.Slice(deps, func(i, j int) bool { - // Simple string-based sort because we just care about consistency, - // not user-friendliness. - return deps[i].String() < deps[j].String() - }) - - return deps + return nil } func (n *NodeAbstractResource) SetProvider(p addrs.AbsProviderConfig) { diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index d79532467..2bd25174a 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -28,12 +28,13 @@ type NodeApplyableResourceInstance struct { } var ( - _ GraphNodeResource = (*NodeApplyableResourceInstance)(nil) - _ GraphNodeResourceInstance = (*NodeApplyableResourceInstance)(nil) - _ GraphNodeCreator = (*NodeApplyableResourceInstance)(nil) - _ GraphNodeReferencer = (*NodeApplyableResourceInstance)(nil) - _ GraphNodeDeposer = (*NodeApplyableResourceInstance)(nil) - _ GraphNodeEvalable = (*NodeApplyableResourceInstance)(nil) + _ GraphNodeResource = (*NodeApplyableResourceInstance)(nil) + _ GraphNodeResourceInstance = (*NodeApplyableResourceInstance)(nil) + _ GraphNodeCreator = (*NodeApplyableResourceInstance)(nil) + _ GraphNodeReferencer = (*NodeApplyableResourceInstance)(nil) + _ GraphNodeDeposer = (*NodeApplyableResourceInstance)(nil) + _ GraphNodeEvalable = (*NodeApplyableResourceInstance)(nil) + _ GraphNodeAttachDependencies = (*NodeApplyableResourceInstance)(nil) ) // GraphNodeAttachDestroyer @@ -97,6 +98,11 @@ func (n *NodeApplyableResourceInstance) References() []*addrs.Reference { return ret } +// GraphNodeAttachDependencies +func (n *NodeApplyableResourceInstance) AttachDependencies(deps []addrs.AbsResource) { + n.Dependencies = deps +} + // GraphNodeEvalable func (n *NodeApplyableResourceInstance) EvalTree() EvalNode { addr := n.ResourceInstanceAddr() @@ -171,7 +177,6 @@ func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResou &EvalReadData{ Addr: addr.Resource, Config: n.Config, - Dependencies: n.StateReferences(), Planned: &change, // setting this indicates that the result must be complete Provider: &provider, ProviderAddr: n.ResolvedProvider, @@ -341,7 +346,6 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe &EvalApply{ Addr: addr.Resource, Config: n.Config, - Dependencies: n.StateReferences(), State: &state, Change: &diffApply, Provider: &provider, @@ -363,6 +367,7 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe ProviderAddr: n.ResolvedProvider, ProviderSchema: &providerSchema, State: &state, + Dependencies: n.Dependencies, }, &EvalApplyProvisioners{ Addr: addr.Resource, @@ -384,6 +389,7 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe ProviderAddr: n.ResolvedProvider, ProviderSchema: &providerSchema, State: &state, + Dependencies: n.Dependencies, }, &EvalIf{ If: func(ctx EvalContext) (bool, error) { diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 0f74bbe61..05ccefc34 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -78,8 +78,8 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou // Check and see if any of our dependencies have changes. changes := ctx.Changes() - for _, d := range n.StateReferences() { - ri, ok := d.(addrs.ResourceInstance) + for _, d := range n.References() { + ri, ok := d.Subject.(addrs.ResourceInstance) if !ok { continue } @@ -114,7 +114,6 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou &EvalReadData{ Addr: addr.Resource, Config: n.Config, - Dependencies: n.StateReferences(), Provider: &provider, ProviderAddr: n.ResolvedProvider, ProviderSchema: &providerSchema, diff --git a/terraform/node_resource_refresh.go b/terraform/node_resource_refresh.go index 9daeabfa6..7a1ebc94a 100644 --- a/terraform/node_resource_refresh.go +++ b/terraform/node_resource_refresh.go @@ -18,6 +18,10 @@ import ( // NodeRefreshableManagedResourceInstance. Resource count orphans are also added. type NodeRefreshableManagedResource struct { *NodeAbstractResource + + // We attach dependencies to the Resource during refresh, since the + // instances are instantiated during DynamicExpand. + Dependencies []addrs.AbsResource } var ( @@ -27,8 +31,14 @@ var ( _ GraphNodeReferencer = (*NodeRefreshableManagedResource)(nil) _ GraphNodeResource = (*NodeRefreshableManagedResource)(nil) _ GraphNodeAttachResourceConfig = (*NodeRefreshableManagedResource)(nil) + _ GraphNodeAttachDependencies = (*NodeRefreshableManagedResource)(nil) ) +// GraphNodeAttachDependencies +func (n *NodeRefreshableManagedResource) AttachDependencies(deps []addrs.AbsResource) { + n.Dependencies = deps +} + // GraphNodeDynamicExpandable func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph, error) { var diags tfdiags.Diagnostics @@ -58,6 +68,7 @@ func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph, // Add the config and state since we don't do that via transforms a.Config = n.Config a.ResolvedProvider = n.ResolvedProvider + a.Dependencies = n.Dependencies return &NodeRefreshableManagedResourceInstance{ NodeAbstractResourceInstance: a, diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 0d8cc0900..0baa2a204 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -608,9 +608,6 @@ aws_instance.bar: foo = true type = aws_instance - Dependencies: - module.child - module.child: Outputs: @@ -666,6 +663,9 @@ module.child: provider = provider.aws type = aws_instance value = bar + + Dependencies: + aws_instance.foo ` const testTerraformApplyOutputOrphanStr = ` @@ -784,17 +784,11 @@ aws_instance.foo.1: provider = provider.aws foo = number 1 type = aws_instance - - Dependencies: - aws_instance.foo[0] aws_instance.foo.2: ID = foo provider = provider.aws foo = number 2 type = aws_instance - - Dependencies: - aws_instance.foo[0] ` const testTerraformApplyProvisionerDiffStr = ` @@ -859,7 +853,7 @@ aws_instance.a: type = aws_instance Dependencies: - module.child + module.child.aws_instance.child module.child: aws_instance.child: @@ -877,7 +871,7 @@ aws_instance.a: type = aws_instance Dependencies: - module.child + module.child.module.grandchild.aws_instance.c module.child.grandchild: aws_instance.c: @@ -897,7 +891,7 @@ module.child: type = aws_instance Dependencies: - module.grandchild + module.child.module.grandchild.aws_instance.c module.child.grandchild: aws_instance.c: ID = foo @@ -1305,9 +1299,6 @@ data.null_data_source.bar: ID = foo provider = provider.null bar = yes - - Dependencies: - data.null_data_source.foo data.null_data_source.foo: ID = foo provider = provider.null diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index be1540ffe..0ad66a713 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -54,10 +54,14 @@ type DestroyEdgeTransformer struct { func (t *DestroyEdgeTransformer) Transform(g *Graph) error { // Build a map of what is being destroyed (by address string) to - // the list of destroyers. Usually there will be at most one destroyer - // per node, but we allow multiple if present for completeness. + // the list of destroyers. destroyers := make(map[string][]GraphNodeDestroyer) destroyerAddrs := make(map[string]addrs.AbsResourceInstance) + + // destroyersByResource records each destroyer by the AbsResourceAddress. + // We use this because dependencies are only referenced as resources, but we + // will want to connect all the individual instances for correct ordering. + destroyersByResource := make(map[string][]GraphNodeDestroyer) for _, v := range g.Vertices() { dn, ok := v.(GraphNodeDestroyer) if !ok { @@ -66,6 +70,7 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { addrP := dn.DestroyAddr() if addrP == nil { + log.Printf("[WARN] DestroyEdgeTransformer: %q (%T) has no destroy address", dag.VertexName(dn), v) continue } addr := *addrP @@ -74,6 +79,9 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { log.Printf("[TRACE] DestroyEdgeTransformer: %q (%T) destroys %s", dag.VertexName(dn), v, key) destroyers[key] = append(destroyers[key], dn) destroyerAddrs[key] = addr + + resAddr := addr.Resource.Absolute(addr.Module).String() + destroyersByResource[resAddr] = append(destroyersByResource[resAddr], dn) } // If we aren't destroying anything, there will be no edges to make @@ -82,6 +90,29 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { return nil } + type withDeps interface { + StateDependencies() []addrs.AbsResource + } + + // Connect destroy dependencies directly as stored in the state in case + // there's no configuration to create the edges otherwise. + for _, ds := range destroyers { + for _, des := range ds { + ri, ok := des.(withDeps) + if !ok { + continue + } + + for _, resAddr := range ri.StateDependencies() { + for _, desDep := range destroyersByResource[resAddr.String()] { + log.Printf("[TRACE] DestroyEdgeTransformer: %s depends on %s\n", dag.VertexName(des), dag.VertexName(desDep)) + g.Connect(dag.BasicEdge(desDep, des)) + + } + } + } + } + // Go through and connect creators to destroyers. Going along with // our example, this makes: A_d => A for _, v := range g.Vertices() { @@ -95,13 +126,7 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { continue } - key := addr.String() - ds := destroyers[key] - if len(ds) == 0 { - continue - } - - for _, d := range ds { + for _, d := range destroyers[addr.String()] { // For illustrating our example a_d := d.(dag.Vertex) a := v @@ -124,6 +149,8 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { } } + // FIXME: connect StateDependencies in here somewhere + // This is strange but is the easiest way to get the dependencies // of a node that is being destroyed. We use another graph to make sure // the resource is in the graph and ask for references. We have to do this diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index f9199be0e..11d261e33 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -6,9 +6,11 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/dag" "github.com/hashicorp/terraform/lang" + "github.com/hashicorp/terraform/states" ) // GraphNodeReferenceable must be implemented by any node that represents @@ -37,6 +39,11 @@ type GraphNodeReferencer interface { References() []*addrs.Reference } +type GraphNodeAttachDependencies interface { + GraphNodeResource + AttachDependencies([]addrs.AbsResource) +} + // GraphNodeReferenceOutside is an interface that can optionally be implemented. // A node that implements it can specify that its own referenceable addresses // and/or the addresses it references are in a different module than the @@ -84,11 +91,81 @@ func (t *ReferenceTransformer) Transform(g *Graph) error { for _, parent := range parents { g.Connect(dag.BasicEdge(v, parent)) } + + if len(parents) > 0 { + continue + } } return nil } +// AttachDependenciesTransformer records all resource dependencies for each +// instance, and attaches the addresses to the node itself. Managed resource +// will record these in the state for proper ordering of destroy operations. +type AttachDependenciesTransformer struct { + Config *configs.Config + State *states.State + Schemas *Schemas +} + +func (t AttachDependenciesTransformer) Transform(g *Graph) error { + for _, v := range g.Vertices() { + attacher, ok := v.(GraphNodeAttachDependencies) + if !ok { + continue + } + selfAddr := attacher.ResourceAddr() + + // Data sources don't need to track destroy dependencies + if selfAddr.Resource.Mode == addrs.DataResourceMode { + continue + } + + ans, err := g.Ancestors(v) + if err != nil { + return err + } + + // dedupe addrs when there's multiple instances involved, or + // multiple paths in the un-reduced graph + depMap := map[string]addrs.AbsResource{} + for _, d := range ans.List() { + var addr addrs.AbsResource + + switch d := d.(type) { + case GraphNodeResourceInstance: + instAddr := d.ResourceInstanceAddr() + addr = instAddr.Resource.Resource.Absolute(instAddr.Module) + case GraphNodeResource: + addr = d.ResourceAddr() + default: + continue + } + + // Data sources don't need to track destroy dependencies + if addr.Resource.Mode == addrs.DataResourceMode { + continue + } + + if addr.Equal(selfAddr) { + continue + } + depMap[addr.String()] = addr + } + + deps := make([]addrs.AbsResource, 0, len(depMap)) + for _, d := range depMap { + deps = append(deps, d) + } + + log.Printf("[TRACE] AttachDependenciesTransformer: %s depends on %s", attacher.ResourceAddr(), deps) + attacher.AttachDependencies(deps) + + } + return nil +} + // DestroyReferenceTransformer is a GraphTransformer that reverses the edges // for locals and outputs that depend on other nodes which will be // removed during destroy. If a destroy node is evaluated before the local or