diff --git a/command/command_test.go b/command/command_test.go index 58b817c7a..f87ad7554 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -260,8 +260,10 @@ func testState() *states.State { // The weird whitespace here is reflective of how this would // get written out in a real state file, due to the indentation // of all of the containing wrapping objects and arrays. - AttrsJSON: []byte("{\n \"id\": \"bar\"\n }"), - Status: states.ObjectReady, + AttrsJSON: []byte("{\n \"id\": \"bar\"\n }"), + Status: states.ObjectReady, + Dependencies: []addrs.AbsResource{}, + DependsOn: []addrs.Referenceable{}, }, addrs.ProviderConfig{ Type: "test", diff --git a/command/refresh_test.go b/command/refresh_test.go index aec83d334..acbb68c4a 100644 --- a/command/refresh_test.go +++ b/command/refresh_test.go @@ -27,6 +27,8 @@ import ( "github.com/hashicorp/terraform/terraform" ) +var equateEmpty = cmpopts.EquateEmpty() + func TestRefresh(t *testing.T) { state := testState() statePath := testStateFile(t, state) @@ -275,7 +277,8 @@ func TestRefresh_defaultState(t *testing.T) { expected := &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, AttrsJSON: []byte("{\n \"ami\": null,\n \"id\": \"yes\"\n }"), - Dependencies: []addrs.Referenceable{}, + Dependencies: []addrs.AbsResource{}, + DependsOn: []addrs.Referenceable{}, } if !reflect.DeepEqual(actual, expected) { t.Fatalf("wrong new object\ngot: %swant: %s", spew.Sdump(actual), spew.Sdump(expected)) @@ -339,7 +342,8 @@ func TestRefresh_outPath(t *testing.T) { expected := &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, AttrsJSON: []byte("{\n \"ami\": null,\n \"id\": \"yes\"\n }"), - Dependencies: []addrs.Referenceable{}, + Dependencies: []addrs.AbsResource{}, + DependsOn: []addrs.Referenceable{}, } if !reflect.DeepEqual(actual, expected) { t.Fatalf("wrong new object\ngot: %swant: %s", spew.Sdump(actual), spew.Sdump(expected)) @@ -568,7 +572,8 @@ func TestRefresh_backup(t *testing.T) { expected := &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, AttrsJSON: []byte("{\n \"ami\": null,\n \"id\": \"changed\"\n }"), - Dependencies: []addrs.Referenceable{}, + Dependencies: []addrs.AbsResource{}, + DependsOn: []addrs.Referenceable{}, } if !reflect.DeepEqual(actual, expected) { t.Fatalf("wrong new object\ngot: %swant: %s", spew.Sdump(actual), spew.Sdump(expected)) @@ -623,8 +628,10 @@ func TestRefresh_disableBackup(t *testing.T) { } newState := testStateRead(t, statePath) - if !reflect.DeepEqual(newState, state) { - t.Fatalf("bad: %#v", newState) + if !cmp.Equal(state, newState, equateEmpty) { + spew.Config.DisableMethods = true + fmt.Println(cmp.Diff(state, newState, equateEmpty)) + t.Fatalf("bad: %s", newState) } newState = testStateRead(t, outPath) @@ -632,7 +639,8 @@ func TestRefresh_disableBackup(t *testing.T) { expected := &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, AttrsJSON: []byte("{\n \"ami\": null,\n \"id\": \"yes\"\n }"), - Dependencies: []addrs.Referenceable{}, + Dependencies: []addrs.AbsResource{}, + DependsOn: []addrs.Referenceable{}, } if !reflect.DeepEqual(actual, expected) { t.Fatalf("wrong new object\ngot: %swant: %s", spew.Sdump(actual), spew.Sdump(expected)) diff --git a/helper/resource/state_shim.go b/helper/resource/state_shim.go index 5ddd02020..afd60b318 100644 --- a/helper/resource/state_shim.go +++ b/helper/resource/state_shim.go @@ -87,7 +87,7 @@ func shimNewState(newState *states.State, providers map[string]terraform.Resourc resState.Primary.Meta["schema_version"] = i.Current.SchemaVersion } - for _, dep := range i.Current.Dependencies { + for _, dep := range i.Current.DependsOn { resState.Dependencies = append(resState.Dependencies, dep.String()) } diff --git a/helper/resource/state_shim_test.go b/helper/resource/state_shim_test.go index e87a87e9e..a9c101a45 100644 --- a/helper/resource/state_shim_test.go +++ b/helper/resource/state_shim_test.go @@ -31,7 +31,7 @@ func TestStateShim(t *testing.T) { Status: states.ObjectReady, AttrsFlat: map[string]string{"id": "foo", "bazzle": "dazzle"}, SchemaVersion: 7, - Dependencies: []addrs.Referenceable{ + DependsOn: []addrs.Referenceable{ addrs.ResourceInstance{ Resource: addrs.Resource{ Mode: 'M', @@ -52,9 +52,9 @@ func TestStateShim(t *testing.T) { Name: "baz", }.Instance(addrs.NoKey), &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsFlat: map[string]string{"id": "baz", "bazzle": "dazzle"}, - Dependencies: []addrs.Referenceable{}, + Status: states.ObjectReady, + AttrsFlat: map[string]string{"id": "baz", "bazzle": "dazzle"}, + DependsOn: []addrs.Referenceable{}, }, addrs.ProviderConfig{ Type: "test", @@ -70,9 +70,9 @@ func TestStateShim(t *testing.T) { Name: "foo", }.Instance(addrs.NoKey), &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsJSON: []byte(`{"id": "bar", "fuzzle":"wuzzle"}`), - Dependencies: []addrs.Referenceable{}, + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id": "bar", "fuzzle":"wuzzle"}`), + DependsOn: []addrs.Referenceable{}, }, addrs.ProviderConfig{ Type: "test", @@ -87,7 +87,7 @@ func TestStateShim(t *testing.T) { &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, AttrsJSON: []byte(`{"id": "bar", "fizzle":"wizzle"}`), - Dependencies: []addrs.Referenceable{ + DependsOn: []addrs.Referenceable{ addrs.ResourceInstance{ Resource: addrs.Resource{ Mode: 'D', @@ -112,7 +112,7 @@ func TestStateShim(t *testing.T) { &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, AttrsFlat: map[string]string{"id": "old", "fizzle": "wizzle"}, - Dependencies: []addrs.Referenceable{ + DependsOn: []addrs.Referenceable{ addrs.ResourceInstance{ Resource: addrs.Resource{ Mode: 'D', @@ -134,9 +134,9 @@ func TestStateShim(t *testing.T) { Name: "lots", }.Instance(addrs.IntKey(0)), &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsFlat: map[string]string{"id": "0", "bazzle": "dazzle"}, - Dependencies: []addrs.Referenceable{}, + Status: states.ObjectReady, + AttrsFlat: map[string]string{"id": "0", "bazzle": "dazzle"}, + DependsOn: []addrs.Referenceable{}, }, addrs.ProviderConfig{ Type: "test", @@ -149,9 +149,9 @@ func TestStateShim(t *testing.T) { Name: "lots", }.Instance(addrs.IntKey(1)), &states.ResourceInstanceObjectSrc{ - Status: states.ObjectTainted, - AttrsFlat: map[string]string{"id": "1", "bazzle": "dazzle"}, - Dependencies: []addrs.Referenceable{}, + Status: states.ObjectTainted, + AttrsFlat: map[string]string{"id": "1", "bazzle": "dazzle"}, + DependsOn: []addrs.Referenceable{}, }, addrs.ProviderConfig{ Type: "test", @@ -165,9 +165,9 @@ func TestStateShim(t *testing.T) { Name: "single_count", }.Instance(addrs.IntKey(0)), &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsJSON: []byte(`{"id": "single", "bazzle":"dazzle"}`), - Dependencies: []addrs.Referenceable{}, + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id": "single", "bazzle":"dazzle"}`), + DependsOn: []addrs.Referenceable{}, }, addrs.ProviderConfig{ Type: "test", diff --git a/states/instance_object.go b/states/instance_object.go index 211b4c8b3..78e1dda93 100644 --- a/states/instance_object.go +++ b/states/instance_object.go @@ -29,12 +29,17 @@ type ResourceInstanceObject struct { // it was updated. Status ObjectStatus - // Dependencies is a set of other addresses in the same module which - // this instance depended on when the given attributes were evaluated. - // This is used to construct the dependency relationships for an object - // whose configuration is no longer available, such as if it has been - // removed from configuration altogether, or is now deposed. - Dependencies []addrs.Referenceable + // Dependencies is a set of absolute address to other resources this + // instance dependeded on when it was applied. This is used to construct + // the dependency relationships for an object whose configuration is no + // longer available, such as if it has been removed from configuration + // altogether, or is now deposed. + Dependencies []addrs.AbsResource + + // DependsOn corresponds to the deprecated `depends_on` field in the state. + // This field contained the configuration `depends_on` values, and some of + // the references from within a single module. + DependsOn []addrs.Referenceable } // ObjectStatus represents the status of a RemoteObject. diff --git a/states/instance_object_src.go b/states/instance_object_src.go index 62907ab76..a18cf313c 100644 --- a/states/instance_object_src.go +++ b/states/instance_object_src.go @@ -53,7 +53,9 @@ type ResourceInstanceObjectSrc struct { // ResourceInstanceObject. Private []byte Status ObjectStatus - Dependencies []addrs.Referenceable + Dependencies []addrs.AbsResource + // deprecated + DependsOn []addrs.Referenceable } // Decode unmarshals the raw representation of the object attributes. Pass the @@ -86,6 +88,7 @@ func (os *ResourceInstanceObjectSrc) Decode(ty cty.Type) (*ResourceInstanceObjec Value: val, Status: os.Status, Dependencies: os.Dependencies, + DependsOn: os.DependsOn, Private: os.Private, }, nil } diff --git a/states/state_deepcopy.go b/states/state_deepcopy.go index 8664f3bea..7d7a7ef10 100644 --- a/states/state_deepcopy.go +++ b/states/state_deepcopy.go @@ -153,8 +153,17 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc { // Some addrs.Referencable implementations are technically mutable, but // we treat them as immutable by convention and so we don't deep-copy here. - dependencies := make([]addrs.Referenceable, len(obj.Dependencies)) - copy(dependencies, obj.Dependencies) + var dependencies []addrs.AbsResource + if obj.Dependencies != nil { + dependencies = make([]addrs.AbsResource, len(obj.Dependencies)) + copy(dependencies, obj.Dependencies) + } + + var dependsOn []addrs.Referenceable + if obj.DependsOn != nil { + dependsOn = make([]addrs.Referenceable, len(obj.DependsOn)) + copy(dependsOn, obj.DependsOn) + } return &ResourceInstanceObjectSrc{ Status: obj.Status, @@ -163,6 +172,7 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc { AttrsFlat: attrsFlat, AttrsJSON: attrsJSON, Dependencies: dependencies, + DependsOn: dependsOn, } } @@ -187,9 +197,9 @@ func (obj *ResourceInstanceObject) DeepCopy() *ResourceInstanceObject { // Some addrs.Referenceable implementations are technically mutable, but // we treat them as immutable by convention and so we don't deep-copy here. - var dependencies []addrs.Referenceable + var dependencies []addrs.AbsResource if obj.Dependencies != nil { - dependencies = make([]addrs.Referenceable, len(obj.Dependencies)) + dependencies = make([]addrs.AbsResource, len(obj.Dependencies)) copy(dependencies, obj.Dependencies) } diff --git a/states/state_test.go b/states/state_test.go index 618cfaafb..22a43859f 100644 --- a/states/state_test.go +++ b/states/state_test.go @@ -138,7 +138,7 @@ func TestStateDeepCopy(t *testing.T) { SchemaVersion: 1, AttrsJSON: []byte(`{"woozles":"confuzles"}`), Private: []byte("private data"), - Dependencies: []addrs.Referenceable{}, + Dependencies: []addrs.AbsResource{}, }, addrs.ProviderConfig{ Type: "test", @@ -155,11 +155,16 @@ func TestStateDeepCopy(t *testing.T) { SchemaVersion: 1, AttrsJSON: []byte(`{"woozles":"confuzles"}`), Private: []byte("private data"), - Dependencies: []addrs.Referenceable{addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test_thing", - Name: "baz", - }}, + Dependencies: []addrs.AbsResource{ + { + Module: addrs.RootModuleInstance, + Resource: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_thing", + Name: "baz", + }, + }, + }, }, addrs.ProviderConfig{ Type: "test", diff --git a/states/statefile/roundtrip_test.go b/states/statefile/roundtrip_test.go index 81158aa3f..693a9be96 100644 --- a/states/statefile/roundtrip_test.go +++ b/states/statefile/roundtrip_test.go @@ -2,7 +2,6 @@ package statefile import ( "bytes" - "encoding/json" "io/ioutil" "os" "path/filepath" @@ -11,8 +10,6 @@ import ( "testing" "github.com/go-test/deep" - - tfversion "github.com/hashicorp/terraform/version" ) func TestRoundtrip(t *testing.T) { @@ -22,8 +19,6 @@ func TestRoundtrip(t *testing.T) { t.Fatal(err) } - currentVersion := tfversion.Version - for _, info := range entries { const inSuffix = ".in.tfstate" const outSuffix = ".out.tfstate" @@ -39,14 +34,20 @@ func TestRoundtrip(t *testing.T) { outName := name + outSuffix t.Run(name, func(t *testing.T) { - ir, err := os.Open(filepath.Join(dir, inName)) - if err != nil { - t.Fatal(err) - } oSrcWant, err := ioutil.ReadFile(filepath.Join(dir, outName)) if err != nil { t.Fatal(err) } + oWant, diags := readStateV4(oSrcWant) + if diags.HasErrors() { + t.Fatal(diags.Err()) + } + + ir, err := os.Open(filepath.Join(dir, inName)) + if err != nil { + t.Fatal(err) + } + defer ir.Close() f, err := Read(ir) if err != nil { @@ -58,20 +59,12 @@ func TestRoundtrip(t *testing.T) { if err != nil { t.Fatal(err) } - oSrcGot := buf.Bytes() + oSrcWritten := buf.Bytes() - var oGot, oWant map[string]interface{} - err = json.Unmarshal(oSrcGot, &oGot) - if err != nil { - t.Fatalf("result isn't JSON: %s", err) + oGot, diags := readStateV4(oSrcWritten) + if diags.HasErrors() { + t.Fatal(diags.Err()) } - err = json.Unmarshal(oSrcWant, &oWant) - if err != nil { - t.Fatalf("wanted result isn't JSON: %s", err) - } - - // A newly written state should always reflect the current terraform version. - oWant["terraform_version"] = currentVersion problems := deep.Equal(oGot, oWant) sort.Strings(problems) diff --git a/states/statefile/testdata/roundtrip/v4-foreach.in.tfstate b/states/statefile/testdata/roundtrip/v4-foreach.in.tfstate new file mode 100644 index 000000000..0b5085f9a --- /dev/null +++ b/states/statefile/testdata/roundtrip/v4-foreach.in.tfstate @@ -0,0 +1,36 @@ +{ + "version": 4, + "serial": 0, + "lineage": "f2968801-fa14-41ab-a044-224f3a4adf04", + "terraform_version": "0.12.0", + "outputs": { + "numbers": { + "type": "string", + "value": "0,1" + } + }, + "resources": [ + { + "module": "module.modA", + "mode": "managed", + "type": "null_resource", + "name": "resource", + "provider": "provider.null", + "instances": [ + { + "schema_version": 0, + "attributes": { + "id": "4639265839606265182", + "triggers": { + "input": "test" + } + }, + "private": "bnVsbA==", + "depends_on": [ + "var.input" + ] + } + ] + } + ] +} diff --git a/states/statefile/testdata/roundtrip/v4-foreach.out.tfstate b/states/statefile/testdata/roundtrip/v4-foreach.out.tfstate new file mode 120000 index 000000000..d35986e2e --- /dev/null +++ b/states/statefile/testdata/roundtrip/v4-foreach.out.tfstate @@ -0,0 +1 @@ +v4-foreach.in.tfstate \ No newline at end of file diff --git a/states/statefile/testdata/roundtrip/v4-modules.in.tfstate b/states/statefile/testdata/roundtrip/v4-modules.in.tfstate new file mode 100644 index 000000000..af2e62d4a --- /dev/null +++ b/states/statefile/testdata/roundtrip/v4-modules.in.tfstate @@ -0,0 +1,88 @@ +{ + "version": 4, + "terraform_version": "0.12.0", + "serial": 0, + "lineage": "f2968801-fa14-41ab-a044-224f3a4adf04", + "outputs": { + "numbers": { + "value": "0,1", + "type": "string" + } + }, + "resources": [ + { + "mode": "managed", + "type": "null_resource", + "name": "bar", + "provider": "provider.null", + "instances": [ + { + "schema_version": 0, + "attributes_flat": { + "id": "5388490630832483079", + "triggers.%": "1", + "triggers.whaaat": "0,1" + }, + "depends_on": [ + "null_resource.foo" + ] + } + ] + }, + { + "module": "module.modB", + "mode": "managed", + "type": "null_resource", + "name": "bar", + "each": "map", + "provider": "provider.null", + "instances": [ + { + "index_key": "a", + "schema_version": 0, + "attributes_flat": { + "id": "8212585058302700791" + }, + "dependencies": [ + "module.modA.null_resource.resource" + ] + }, + { + "index_key": "b", + "schema_version": 0, + "attributes_flat": { + "id": "1523897709610803586" + }, + "dependencies": [ + "module.modA.null_resource.resource" + ] + } + ] + }, + { + "module": "module.modA", + "mode": "managed", + "type": "null_resource", + "name": "resource", + "provider": "provider.null", + "instances": [ + { + "schema_version": 0, + "attributes": { + "id": "4639265839606265182", + "triggers": { + "input": "test" + } + }, + "private": "bnVsbA==", + "dependencies": [ + "null_resource.bar" + ], + "depends_on": [ + "var.input" + ] + } + ] + } + ] +} diff --git a/states/statefile/testdata/roundtrip/v4-modules.out.tfstate b/states/statefile/testdata/roundtrip/v4-modules.out.tfstate new file mode 120000 index 000000000..009f759ed --- /dev/null +++ b/states/statefile/testdata/roundtrip/v4-modules.out.tfstate @@ -0,0 +1 @@ +v4-modules.in.tfstate \ No newline at end of file diff --git a/states/statefile/version3_upgrade.go b/states/statefile/version3_upgrade.go index fbec5477c..943e0f45c 100644 --- a/states/statefile/version3_upgrade.go +++ b/states/statefile/version3_upgrade.go @@ -313,7 +313,7 @@ func upgradeInstanceObjectV3ToV4(rsOld *resourceStateV2, isOld *instanceStateV2, Status: status, Deposed: string(deposedKey), AttributesFlat: attributes, - Dependencies: dependencies, + DependsOn: dependencies, SchemaVersion: schemaVersion, PrivateRaw: privateJSON, }, nil diff --git a/states/statefile/version4.go b/states/statefile/version4.go index ee8b65236..2cc0677ab 100644 --- a/states/statefile/version4.go +++ b/states/statefile/version4.go @@ -181,7 +181,10 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) { } { - depsRaw := isV4.Dependencies + // Allow both the deprecated `depends_on` and new + // `dependencies` to coexist for now so resources can be + // upgraded as they are refreshed. + depsRaw := isV4.DependsOn deps := make([]addrs.Referenceable, 0, len(depsRaw)) for _, depRaw := range depsRaw { ref, refDiags := addrs.ParseRefStr(depRaw) @@ -202,6 +205,20 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) { } deps = append(deps, ref.Subject) } + obj.DependsOn = deps + } + + { + depsRaw := isV4.Dependencies + deps := make([]addrs.AbsResource, 0, len(depsRaw)) + for _, depRaw := range depsRaw { + addr, addrDiags := addrs.ParseAbsResourceStr(depRaw) + diags = diags.Append(addrDiags) + if addrDiags.HasErrors() { + continue + } + deps = append(deps, addr) + } obj.Dependencies = deps } @@ -466,6 +483,11 @@ func appendInstanceObjectStateV4(rs *states.Resource, is *states.ResourceInstanc deps[i] = depAddr.String() } + depOn := make([]string, len(obj.DependsOn)) + for i, depAddr := range obj.DependsOn { + depOn[i] = depAddr.String() + } + var rawKey interface{} switch tk := key.(type) { case addrs.IntKey: @@ -491,6 +513,7 @@ func appendInstanceObjectStateV4(rs *states.Resource, is *states.ResourceInstanc AttributesRaw: obj.AttrsJSON, PrivateRaw: privateRaw, Dependencies: deps, + DependsOn: depOn, }), diags } @@ -540,7 +563,8 @@ type instanceObjectStateV4 struct { PrivateRaw []byte `json:"private,omitempty"` - Dependencies []string `json:"depends_on,omitempty"` + Dependencies []string `json:"dependencies,omitempty"` + DependsOn []string `json:"depends_on,omitempty"` } // stateVersionV4 is a weird special type we use to produce our hard-coded diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 98e60ae26..8d50a3e9e 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -26,6 +26,7 @@ import ( "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/provisioners" "github.com/hashicorp/terraform/states" + "github.com/hashicorp/terraform/states/statefile" "github.com/hashicorp/terraform/tfdiags" "github.com/zclconf/go-cty/cty" ) @@ -1328,19 +1329,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 +1357,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 +1456,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 +1484,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 @@ -2889,7 +2985,9 @@ func TestContext2Apply_orphanResource(t *testing.T) { AttrsJSON: []byte(`{}`), }, providerAddr) }) - if !cmp.Equal(state, want) { + + // compare the marshaled form to easily remove empty and nil slices + if !statefile.StatesMarshalEqual(state, want) { t.Fatalf("wrong state after step 1\n%s", cmp.Diff(want, state)) } @@ -3465,6 +3563,9 @@ module.B: provider = provider.aws foo = foo type = aws_instance + + Dependencies: + module.A.aws_instance.foo `) } @@ -8743,7 +8844,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..d8479ae6c 100644 --- a/terraform/context_refresh_test.go +++ b/terraform/context_refresh_test.go @@ -1964,3 +1964,93 @@ 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"}`), + Dependencies: []addrs.AbsResource{ + // Existing dependencies should not be removed during refresh + { + Module: addrs.RootModuleInstance, + Resource: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "baz", + }, + }, + }, + }, + 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.baz + 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_refresh.go b/terraform/eval_refresh.go index 4dfb5b4e9..fd50f873a 100644 --- a/terraform/eval_refresh.go +++ b/terraform/eval_refresh.go @@ -89,6 +89,7 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { newState := state.DeepCopy() newState.Value = resp.NewState newState.Private = resp.Private + newState.Dependencies = state.Dependencies // Call post-refresh hook err = ctx.Hook(func(h Hook) (HookAction, error) { diff --git a/terraform/eval_state.go b/terraform/eval_state.go index b611113e3..b0fcee007 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -3,6 +3,7 @@ package terraform import ( "fmt" "log" + "sort" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" @@ -200,6 +201,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 +220,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 +227,13 @@ 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 + if n.Dependencies != nil { + log.Printf("[TRACE] EvalWriteState: recording %d dependencies for %s", len(*n.Dependencies), absAddr) + 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") @@ -473,3 +484,44 @@ func (n *EvalForgetResourceState) Eval(ctx EvalContext) (interface{}, error) { return nil, nil } + +// EvalRefreshDependencies is an EvalNode implementation that appends any newly +// found dependencies to those saved in the state. The existing dependencies +// are retained, as they may be missing from the config, and will be required +// for the updates and destroys during the next apply. +type EvalRefreshDependencies struct { + // Prior State + State **states.ResourceInstanceObject + // Dependencies to write to the new state + Dependencies *[]addrs.AbsResource +} + +func (n *EvalRefreshDependencies) Eval(ctx EvalContext) (interface{}, error) { + state := *n.State + if state == nil { + // no existing state to append + return nil, nil + } + + depMap := make(map[string]addrs.AbsResource) + for _, d := range *n.Dependencies { + depMap[d.String()] = d + } + + for _, d := range state.Dependencies { + depMap[d.String()] = d + } + + deps := make([]addrs.AbsResource, 0, len(depMap)) + for _, d := range depMap { + deps = append(deps, d) + } + + sort.Slice(deps, func(i, j int) bool { + return deps[i].String() < deps[j].String() + }) + + *n.Dependencies = deps + + return nil, nil +} 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_apply_test.go b/terraform/graph_builder_apply_test.go index c0f987423..1d098a63b 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/states" + "github.com/zclconf/go-cty/cty" ) func TestApplyGraphBuilder_impl(t *testing.T) { @@ -474,6 +475,110 @@ func TestApplyGraphBuilder_targetModule(t *testing.T) { testGraphNotContains(t, g, "module.child1.output.instance_id") } +// Ensure that an update resulting from the removal of a resource happens after +// that resource is destroyed. +func TestApplyGraphBuilder_updateFromOrphan(t *testing.T) { + schemas := simpleTestSchemas() + instanceSchema := schemas.Providers["test"].ResourceTypes["test_object"] + + bBefore, _ := plans.NewDynamicValue( + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("b_id"), + "test_string": cty.StringVal("a_id"), + }), instanceSchema.ImpliedType()) + bAfter, _ := plans.NewDynamicValue( + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("b_id"), + "test_string": cty.StringVal("changed"), + }), instanceSchema.ImpliedType()) + + changes := &plans.Changes{ + Resources: []*plans.ResourceInstanceChangeSrc{ + { + Addr: mustResourceInstanceAddr("test_object.a"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Delete, + }, + }, + { + Addr: mustResourceInstanceAddr("test_object.b"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Update, + Before: bBefore, + After: bAfter, + }, + }, + }, + } + + state := states.NewState() + root := state.EnsureModule(addrs.RootModuleInstance) + root.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_object", + Name: "a", + }.Instance(addrs.NoKey), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"a_id"}`), + }, + addrs.ProviderConfig{ + Type: "test", + }.Absolute(addrs.RootModuleInstance), + ) + root.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_object", + Name: "b", + }.Instance(addrs.NoKey), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"b_id","test_string":"a_id"}`), + Dependencies: []addrs.AbsResource{ + addrs.AbsResource{ + Resource: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_object", + Name: "a", + }, + Module: root.Addr, + }, + }, + }, + addrs.ProviderConfig{ + Type: "test", + }.Absolute(addrs.RootModuleInstance), + ) + + b := &ApplyGraphBuilder{ + Config: testModule(t, "graph-builder-apply-orphan-update"), + Changes: changes, + Components: simpleMockComponentFactory(), + Schemas: schemas, + State: state, + } + + g, err := b.Build(addrs.RootModuleInstance) + if err != nil { + t.Fatalf("err: %s", err) + } + + expected := strings.TrimSpace(` +test_object.a (destroy) +test_object.b + test_object.a (destroy) +`) + + instanceGraph := filterInstances(g) + got := strings.TrimSpace(instanceGraph.String()) + + if got != expected { + t.Fatalf("expected:\n%s\ngot:\n%s", expected, got) + } +} + const testApplyGraphBuilderStr = ` meta.count-boundary (EachMode fixup) module.child.test_object.other 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..bdf62ab26 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" @@ -35,6 +34,10 @@ type ConcreteResourceInstanceNodeFunc func(*NodeAbstractResourceInstance) dag.Ve // configuration. type GraphNodeResourceInstance interface { ResourceInstanceAddr() addrs.AbsResourceInstance + + // StateDependencies returns any inter-resource dependencies that are + // stored in the state. + StateDependencies() []addrs.AbsResource } // NodeAbstractResource represents a resource that has no associated @@ -93,8 +96,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 +223,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 +236,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 +254,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 +294,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..753ef9677 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..2dd549c0b 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, @@ -203,6 +214,11 @@ func (n *NodeRefreshableManagedResourceInstance) evalTreeManagedResource() EvalN Output: &state, }, + &EvalRefreshDependencies{ + State: &state, + Dependencies: &n.Dependencies, + }, + &EvalRefresh{ Addr: addr.Resource, ProviderAddr: n.ResolvedProvider, @@ -217,6 +233,7 @@ func (n *NodeRefreshableManagedResourceInstance) evalTreeManagedResource() EvalN ProviderAddr: n.ResolvedProvider, ProviderSchema: &providerSchema, State: &state, + Dependencies: &n.Dependencies, }, }, } @@ -276,6 +293,7 @@ func (n *NodeRefreshableManagedResourceInstance) evalTreeManagedResourceNoState( ProviderAddr: n.ResolvedProvider, ProviderSchema: &providerSchema, State: &state, + Dependencies: &n.Dependencies, }, // We must also save the planned change, so that expressions in 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/testdata/graph-builder-apply-orphan-update/main.tf b/terraform/testdata/graph-builder-apply-orphan-update/main.tf new file mode 100644 index 000000000..22e7ae0f1 --- /dev/null +++ b/terraform/testdata/graph-builder-apply-orphan-update/main.tf @@ -0,0 +1,3 @@ +resource "test_object" "b" { + test_string = "changed" +} diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index be1540ffe..c25bdce7e 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -54,26 +54,39 @@ 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) + + // Record the creators, which will need to depend on the destroyers if they + // are only being updated. + creators := make(map[string]GraphNodeCreator) + + // 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 { - continue - } + switch n := v.(type) { + case GraphNodeDestroyer: + addrP := n.DestroyAddr() + if addrP == nil { + log.Printf("[WARN] DestroyEdgeTransformer: %q (%T) has no destroy address", dag.VertexName(n), v) + continue + } + addr := *addrP - addrP := dn.DestroyAddr() - if addrP == nil { - continue - } - addr := *addrP + key := addr.String() + log.Printf("[TRACE] DestroyEdgeTransformer: %q (%T) destroys %s", dag.VertexName(n), v, key) + destroyers[key] = append(destroyers[key], n) + destroyerAddrs[key] = addr - key := addr.String() - 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], n) + case GraphNodeCreator: + addr := n.CreateAddr() + creators[addr.String()] = n + } } // If we aren't destroying anything, there will be no edges to make @@ -82,6 +95,40 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { return nil } + // Connect destroy despendencies as stored in the state + for _, ds := range destroyers { + for _, des := range ds { + ri, ok := des.(GraphNodeResourceInstance) + if !ok { + continue + } + + for _, resAddr := range ri.StateDependencies() { + for _, desDep := range destroyersByResource[resAddr.String()] { + log.Printf("[TRACE] DestroyEdgeTransformer: %s has stored dependency of %s\n", dag.VertexName(desDep), dag.VertexName(des)) + g.Connect(dag.BasicEdge(desDep, des)) + + } + } + } + } + + // connect creators to any destroyers on which they may depend + for _, c := range creators { + ri, ok := c.(GraphNodeResourceInstance) + if !ok { + continue + } + + for _, resAddr := range ri.StateDependencies() { + for _, desDep := range destroyersByResource[resAddr.String()] { + log.Printf("[TRACE] DestroyEdgeTransformer: %s has stored dependency of %s\n", dag.VertexName(c), dag.VertexName(desDep)) + g.Connect(dag.BasicEdge(c, desDep)) + + } + } + } + // Go through and connect creators to destroyers. Going along with // our example, this makes: A_d => A for _, v := range g.Vertices() { @@ -95,13 +142,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 diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index f9199be0e..25e544996 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -3,12 +3,15 @@ package terraform import ( "fmt" "log" + "sort" "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 +40,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,6 +92,79 @@ 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) + } + sort.Slice(deps, func(i, j int) bool { + return deps[i].String() < deps[j].String() + }) + + log.Printf("[TRACE] AttachDependenciesTransformer: %s depends on %s", attacher.ResourceAddr(), deps) + attacher.AttachDependencies(deps) } return nil