Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

store absolute addresses for resource dependencies in the state #23252

Merged
merged 8 commits into from
Nov 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions command/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
20 changes: 14 additions & 6 deletions command/refresh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -623,16 +628,19 @@ 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)
actual := newState.RootModule().Resources["test_instance.foo"].Instances[addrs.NoKey].Current
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))
Expand Down
2 changes: 1 addition & 1 deletion helper/resource/state_shim.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand Down
36 changes: 18 additions & 18 deletions helper/resource/state_shim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down
17 changes: 11 additions & 6 deletions states/instance_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion states/instance_object_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
18 changes: 14 additions & 4 deletions states/state_deepcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -163,6 +172,7 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc {
AttrsFlat: attrsFlat,
AttrsJSON: attrsJSON,
Dependencies: dependencies,
DependsOn: dependsOn,
}
}

Expand All @@ -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)
}

Expand Down
17 changes: 11 additions & 6 deletions states/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
31 changes: 12 additions & 19 deletions states/statefile/roundtrip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package statefile

import (
"bytes"
"encoding/json"
"io/ioutil"
"os"
"path/filepath"
Expand All @@ -11,8 +10,6 @@ import (
"testing"

"github.com/go-test/deep"

tfversion "github.com/hashicorp/terraform/version"
)

func TestRoundtrip(t *testing.T) {
Expand All @@ -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"
Expand All @@ -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))
oSrcWant, err := ioutil.ReadFile(filepath.Join(dir, outName))
if err != nil {
t.Fatal(err)
}
oSrcWant, err := ioutil.ReadFile(filepath.Join(dir, outName))
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 {
Expand All @@ -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)
Expand Down
Loading