diff --git a/command/format/object_id.go b/command/format/object_id.go new file mode 100644 index 000000000..85ebbfec5 --- /dev/null +++ b/command/format/object_id.go @@ -0,0 +1,123 @@ +package format + +import ( + "github.com/zclconf/go-cty/cty" +) + +// ObjectValueID takes a value that is assumed to be an object representation +// of some resource instance object and attempts to heuristically find an +// attribute of it that is likely to be a unique identifier in the remote +// system that it belongs to which will be useful to the user. +// +// If such an attribute is found, its name and string value intended for +// display are returned. Both returned strings are empty if no such attribute +// exists, in which case the caller should assume that the resource instance +// address within the Terraform configuration is the best available identifier. +// +// This is only a best-effort sort of thing, relying on naming conventions in +// our resource type schemas. The result is not guaranteed to be unique, but +// should generally be suitable for display to an end-user anyway. +// +// This function will panic if the given value is not of an object type. +func ObjectValueID(obj cty.Value) (k, v string) { + if obj.IsNull() || !obj.IsKnown() { + return "", "" + } + + atys := obj.Type().AttributeTypes() + + switch { + + case atys["id"] == cty.String: + v := obj.GetAttr("id") + if v.IsKnown() && !v.IsNull() { + return "id", v.AsString() + } + + case atys["name"] == cty.String: + // "name" isn't always globally unique, but if there isn't also an + // "id" then it _often_ is, in practice. + v := obj.GetAttr("name") + if v.IsKnown() && !v.IsNull() { + return "name", v.AsString() + } + } + + return "", "" +} + +// ObjectValueName takes a value that is assumed to be an object representation +// of some resource instance object and attempts to heuristically find an +// attribute of it that is likely to be a human-friendly name in the remote +// system that it belongs to which will be useful to the user. +// +// If such an attribute is found, its name and string value intended for +// display are returned. Both returned strings are empty if no such attribute +// exists, in which case the caller should assume that the resource instance +// address within the Terraform configuration is the best available identifier. +// +// This is only a best-effort sort of thing, relying on naming conventions in +// our resource type schemas. The result is not guaranteed to be unique, but +// should generally be suitable for display to an end-user anyway. +// +// Callers that use both ObjectValueName and ObjectValueID at the same time +// should be prepared to get the same attribute key and value from both in +// some cases, since there is overlap betweek the id-extraction and +// name-extraction heuristics. +// +// This function will panic if the given value is not of an object type. +func ObjectValueName(obj cty.Value) (k, v string) { + if obj.IsNull() || !obj.IsKnown() { + return "", "" + } + + atys := obj.Type().AttributeTypes() + + switch { + + case atys["name"] == cty.String: + v := obj.GetAttr("name") + if v.IsKnown() && !v.IsNull() { + return "name", v.AsString() + } + + case atys["tags"].IsMapType() && atys["tags"].ElementType() == cty.String: + tags := obj.GetAttr("tags") + if tags.IsNull() || !tags.IsWhollyKnown() { + break + } + + switch { + case tags.HasIndex(cty.StringVal("name")).RawEquals(cty.True): + v := tags.Index(cty.StringVal("name")) + if v.IsKnown() && !v.IsNull() { + return "tags.name", v.AsString() + } + case tags.HasIndex(cty.StringVal("Name")).RawEquals(cty.True): + // AWS-style naming convention + v := tags.Index(cty.StringVal("Name")) + if v.IsKnown() && !v.IsNull() { + return "tags.Name", v.AsString() + } + } + } + + return "", "" +} + +// ObjectValueIDOrName is a convenience wrapper around both ObjectValueID +// and ObjectValueName (in that preference order) to try to extract some sort +// of human-friendly descriptive string value for an object as additional +// context about an object when it is being displayed in a compact way (where +// not all of the attributes are visible.) +// +// Just as with the two functions it wraps, it is a best-effort and may return +// two empty strings if no suitable attribute can be found for a given object. +func ObjectValueIDOrName(obj cty.Value) (k, v string) { + k, v = ObjectValueID(obj) + if k != "" { + return + } + k, v = ObjectValueName(obj) + return +} diff --git a/command/format/object_id_test.go b/command/format/object_id_test.go new file mode 100644 index 000000000..b562234fe --- /dev/null +++ b/command/format/object_id_test.go @@ -0,0 +1,194 @@ +package format + +import ( + "fmt" + "testing" + + "github.com/zclconf/go-cty/cty" +) + +func TestObjectValueIDOrName(t *testing.T) { + tests := []struct{ + obj cty.Value + id [2]string + name [2]string + either [2]string + }{ + { + cty.NullVal(cty.EmptyObject), + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + { + cty.UnknownVal(cty.EmptyObject), + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + { + cty.EmptyObjectVal, + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo-123"), + }), + [...]string{"id", "foo-123"}, + [...]string{"", ""}, + [...]string{"id", "foo-123"}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo-123"), + "name": cty.StringVal("awesome-foo"), + }), + [...]string{"id", "foo-123"}, + [...]string{"name", "awesome-foo"}, + [...]string{"id", "foo-123"}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("awesome-foo"), + }), + [...]string{"name", "awesome-foo"}, + [...]string{"name", "awesome-foo"}, + [...]string{"name", "awesome-foo"}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("awesome-foo"), + "tags": cty.MapVal(map[string]cty.Value{ + "Name": cty.StringVal("My Awesome Foo"), + }), + }), + [...]string{"name", "awesome-foo"}, + [...]string{"name", "awesome-foo"}, + [...]string{"name", "awesome-foo"}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "tags": cty.MapVal(map[string]cty.Value{ + "Name": cty.StringVal("My Awesome Foo"), + "name": cty.StringVal("awesome-foo"), + }), + }), + [...]string{"", ""}, + [...]string{"tags.name", "awesome-foo"}, + [...]string{"tags.name", "awesome-foo"}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "tags": cty.MapVal(map[string]cty.Value{ + "Name": cty.StringVal("My Awesome Foo"), + }), + }), + [...]string{"", ""}, + [...]string{"tags.Name", "My Awesome Foo"}, + [...]string{"tags.Name", "My Awesome Foo"}, + }, + + // The following are degenerate cases, included to make sure we don't + // crash when we encounter them. If you're here fixing a reported panic + // in this formatter, this is the place to add a new test case. + { + cty.ObjectVal(map[string]cty.Value{ + "id": cty.True, + }), + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + }), + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + }), + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "tags": cty.StringVal("foo"), + }), + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "tags": cty.NullVal(cty.Map(cty.String)), + }), + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "tags": cty.UnknownVal(cty.Map(cty.String)), + }), + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "tags": cty.MapVal(map[string]cty.Value{ + "Name": cty.True, + }), + }), + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "tags": cty.MapVal(map[string]cty.Value{ + "Name": cty.UnknownVal(cty.String), + }), + }), + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "tags": cty.MapVal(map[string]cty.Value{ + "Name": cty.NullVal(cty.String), + }), + }), + [...]string{"", ""}, + [...]string{"", ""}, + [...]string{"", ""}, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%#v", test.obj), func (t *testing.T) { + obj := test.obj + gotIDKey, gotIDVal := ObjectValueID(obj) + gotNameKey, gotNameVal := ObjectValueName(obj) + gotEitherKey, gotEitherVal := ObjectValueIDOrName(obj) + + if got, want := [...]string{gotIDKey, gotIDVal}, test.id; got != want { + t.Errorf("wrong ObjectValueID result\ngot: %#v\nwant: %#v", got, want) + } + if got, want := [...]string{gotNameKey, gotNameVal}, test.name; got != want { + t.Errorf("wrong ObjectValueName result\ngot: %#v\nwant: %#v", got, want) + } + if got, want := [...]string{gotEitherKey, gotEitherVal}, test.either; got != want { + t.Errorf("wrong ObjectValueIDOrName result\ngot: %#v\nwant: %#v", got, want) + } + }) + } +} diff --git a/command/hook_ui.go b/command/hook_ui.go index e90573042..f105930d7 100644 --- a/command/hook_ui.go +++ b/command/hook_ui.go @@ -15,6 +15,7 @@ import ( "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/command/format" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" @@ -41,10 +42,10 @@ var _ terraform.Hook = (*UiHook)(nil) // uiResourceState tracks the state of a single resource type uiResourceState struct { - Name string - ResourceId string - Op uiResourceOp - Start time.Time + DispAddr string + IDKey, IDValue string + Op uiResourceOp + Start time.Time DoneCh chan struct{} // To be used for cancellation @@ -71,6 +72,7 @@ func (h *UiHook) PreApply(addr addrs.AbsResourceInstance, gen states.Generation, var operation string var op uiResourceOp + idKey, idValue := format.ObjectValueIDOrName(priorState) switch action { case plans.Delete: operation = "Destroying..." @@ -101,11 +103,6 @@ func (h *UiHook) PreApply(addr addrs.AbsResourceInstance, gen states.Generation, dAttrs := map[string]terraform.ResourceAttrDiff{} keys := make([]string, 0, len(dAttrs)) for key, _ := range dAttrs { - // Skip the ID since we do that specially - if key == "id" { - continue - } - keys = append(keys, key) if len(key) > keyLen { keyLen = len(key) @@ -141,7 +138,15 @@ func (h *UiHook) PreApply(addr addrs.AbsResourceInstance, gen states.Generation, attrString = "\n " + attrString } - var stateId, stateIdSuffix string + var stateIdSuffix string + if idKey != "" && idValue != "" { + stateIdSuffix = fmt.Sprintf(" [%s=%s]", idKey, idValue) + } else { + // Make sure they are both empty so we can deal with this more + // easily in the other hook methods. + idKey = "" + idValue = "" + } h.ui.Output(h.Colorize.Color(fmt.Sprintf( "[reset][bold]%s: %s%s[reset]%s", @@ -151,10 +156,11 @@ func (h *UiHook) PreApply(addr addrs.AbsResourceInstance, gen states.Generation, attrString, ))) - id := addr.String() + key := addr.String() uiState := uiResourceState{ - Name: id, - ResourceId: stateId, + DispAddr: key, + IDKey: idKey, + IDValue: idValue, Op: op, Start: time.Now().Round(time.Second), DoneCh: make(chan struct{}), @@ -162,7 +168,7 @@ func (h *UiHook) PreApply(addr addrs.AbsResourceInstance, gen states.Generation, } h.l.Lock() - h.resources[id] = uiState + h.resources[key] = uiState h.l.Unlock() // Start goroutine that shows progress @@ -195,13 +201,13 @@ func (h *UiHook) stillApplying(state uiResourceState) { } idSuffix := "" - if v := state.ResourceId; v != "" { - idSuffix = fmt.Sprintf("ID: %s, ", truncateId(v, maxIdLen)) + if state.IDKey != "" { + idSuffix = fmt.Sprintf("%s=%s, ", state.IDKey, truncateId(state.IDValue, maxIdLen)) } h.ui.Output(h.Colorize.Color(fmt.Sprintf( - "[reset][bold]%s: %s (%s%s elapsed)[reset]", - state.Name, + "[reset][bold]%s: %s [%s%s elapsed][reset]", + state.DispAddr, msg, idSuffix, time.Now().Round(time.Second).Sub(state.Start), @@ -223,6 +229,9 @@ func (h *UiHook) PostApply(addr addrs.AbsResourceInstance, gen states.Generation h.l.Unlock() var stateIdSuffix string + if k, v := format.ObjectValueID(newState); k != "" && v != "" { + stateIdSuffix = fmt.Sprintf(" [%s=%s]", k, v) + } var msg string switch state.Op { @@ -283,6 +292,9 @@ func (h *UiHook) PreRefresh(addr addrs.AbsResourceInstance, gen states.Generatio h.once.Do(h.init) var stateIdSuffix string + if k, v := format.ObjectValueID(priorState); k != "" && v != "" { + stateIdSuffix = fmt.Sprintf(" [%s=%s]", k, v) + } h.ui.Output(h.Colorize.Color(fmt.Sprintf( "[reset][bold]%s: Refreshing state...%s", diff --git a/command/hook_ui_test.go b/command/hook_ui_test.go index cce88d03c..0a871cf6a 100644 --- a/command/hook_ui_test.go +++ b/command/hook_ui_test.go @@ -41,11 +41,7 @@ func TestUiHookPreApply_periodicTimer(t *testing.T) { Name: "available", }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) - priorState := cty.NullVal(cty.Object(map[string]cty.Type{ - "id": cty.String, - "names": cty.List(cty.String), - })) - plannedNewState := cty.ObjectVal(map[string]cty.Value{ + priorState := cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("2017-03-05 10:56:59.298784526 +0000 UTC"), "names": cty.ListVal([]cty.Value{ cty.StringVal("us-east-1a"), @@ -54,6 +50,10 @@ func TestUiHookPreApply_periodicTimer(t *testing.T) { cty.StringVal("us-east-1d"), }), }) + plannedNewState := cty.NullVal(cty.Object(map[string]cty.Type{ + "id": cty.String, + "names": cty.List(cty.String), + })) action, err := h.PreApply(addr, states.CurrentGen, plans.Delete, priorState, plannedNewState) if err != nil { @@ -70,10 +70,10 @@ func TestUiHookPreApply_periodicTimer(t *testing.T) { close(uiState.DoneCh) <-uiState.done - expectedOutput := `data.aws_availability_zones.available: Destroying... (ID: 2017-03-05 10:56:59.298784526 +0000 UTC) -data.aws_availability_zones.available: Still destroying... (ID: 2017-03-05 10:56:59.298784526 +0000 UTC, 1s elapsed) -data.aws_availability_zones.available: Still destroying... (ID: 2017-03-05 10:56:59.298784526 +0000 UTC, 2s elapsed) -data.aws_availability_zones.available: Still destroying... (ID: 2017-03-05 10:56:59.298784526 +0000 UTC, 3s elapsed) + expectedOutput := `data.aws_availability_zones.available: Destroying... [id=2017-03-05 10:56:59.298784526 +0000 UTC] +data.aws_availability_zones.available: Still destroying... [id=2017-03-05 10:56:59.298784526 +0000 UTC, 1s elapsed] +data.aws_availability_zones.available: Still destroying... [id=2017-03-05 10:56:59.298784526 +0000 UTC, 2s elapsed] +data.aws_availability_zones.available: Still destroying... [id=2017-03-05 10:56:59.298784526 +0000 UTC, 3s elapsed] ` output := ui.OutputWriter.String() if output != expectedOutput { @@ -111,11 +111,7 @@ func TestUiHookPreApply_destroy(t *testing.T) { Name: "available", }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) - priorState := cty.NullVal(cty.Object(map[string]cty.Type{ - "id": cty.String, - "names": cty.List(cty.String), - })) - plannedNewState := cty.ObjectVal(map[string]cty.Value{ + priorState := cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("2017-03-05 10:56:59.298784526 +0000 UTC"), "names": cty.ListVal([]cty.Value{ cty.StringVal("us-east-1a"), @@ -124,6 +120,10 @@ func TestUiHookPreApply_destroy(t *testing.T) { cty.StringVal("us-east-1d"), }), }) + plannedNewState := cty.NullVal(cty.Object(map[string]cty.Type{ + "id": cty.String, + "names": cty.List(cty.String), + })) action, err := h.PreApply(addr, states.CurrentGen, plans.Delete, priorState, plannedNewState) if err != nil { @@ -138,7 +138,7 @@ func TestUiHookPreApply_destroy(t *testing.T) { close(uiState.DoneCh) <-uiState.done - expectedOutput := "data.aws_availability_zones.available: Destroying... (ID: 2017-03-05 10:56:59.298784526 +0000 UTC)\n" + expectedOutput := "data.aws_availability_zones.available: Destroying... [id=2017-03-05 10:56:59.298784526 +0000 UTC]\n" output := ui.OutputWriter.String() if output != expectedOutput { t.Fatalf("Output didn't match.\nExpected: %q\nGiven: %q", expectedOutput, output)