diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index 9b6c7e3a44e4..4463431815d3 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -96,47 +96,27 @@ func (b *Local) opApply( return } - trivialPlan := plan.Diff == nil || plan.Diff.Empty() + dispPlan := format.NewPlan(plan) + trivialPlan := dispPlan.Empty() hasUI := op.UIOut != nil && op.UIIn != nil - if hasUI && ((op.Destroy && !op.DestroyForce) || - (!op.Destroy && !op.AutoApprove && !trivialPlan)) { + mustConfirm := hasUI && ((op.Destroy && !op.DestroyForce) || (!op.Destroy && !op.AutoApprove && !trivialPlan)) + if mustConfirm { var desc, query string if op.Destroy { // Default destroy message - desc = "Terraform will delete all your managed infrastructure, as shown above.\n" + + desc = "Terraform will destroy all your managed infrastructure, as shown above.\n" + "There is no undo. Only 'yes' will be accepted to confirm." - - // If targets are specified, list those to user - if op.Targets != nil { - var descBuffer bytes.Buffer - descBuffer.WriteString("Terraform will delete the following infrastructure:\n") - for _, target := range op.Targets { - descBuffer.WriteString("\t") - descBuffer.WriteString(target) - descBuffer.WriteString("\n") - } - descBuffer.WriteString("There is no undo. Only 'yes' will be accepted to confirm") - desc = descBuffer.String() - } query = "Do you really want to destroy?" } else { - desc = "Terraform will apply the changes described above.\n" + + desc = "Terraform will perform the actions described above.\n" + "Only 'yes' will be accepted to approve." - query = "Do you want to apply these changes?" + query = "Do you want to perform these actions?" } if !trivialPlan { // Display the plan of what we are going to apply/destroy. - if op.Destroy { - op.UIOut.Output("\n" + strings.TrimSpace(approveDestroyPlanHeader) + "\n") - } else { - op.UIOut.Output("\n" + strings.TrimSpace(approvePlanHeader) + "\n") - } - op.UIOut.Output(format.Plan(&format.PlanOpts{ - Plan: plan, - Color: b.Colorize(), - ModuleDepth: -1, - })) + b.renderPlan(dispPlan) + b.CLI.Output("") } v, err := op.UIIn.Input(&terraform.InputOpts{ @@ -353,17 +333,3 @@ Terraform encountered an error attempting to save the state before canceling the current operation. Once the operation is complete another attempt will be made to save the final state. ` - -const approvePlanHeader = ` -The Terraform execution plan has been generated and is shown below. -Resources are shown in alphabetical order for quick scanning. Green resources -will be created (or destroyed and then created if an existing resource -exists), yellow resources are being changed in-place, and red resources -will be destroyed. Cyan entries are data sources to be read. -` - -const approveDestroyPlanHeader = ` -The Terraform destroy plan has been generated and is shown below. -Resources are shown in alphabetical order for quick scanning. -Resources shown in red will be destroyed. -` diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index 42a56eb291c9..905f89f1a917 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -1,6 +1,7 @@ package local import ( + "bytes" "context" "fmt" "log" @@ -95,6 +96,9 @@ func (b *Local) opPlan( runningOp.Err = errwrap.Wrapf("Error refreshing state: {{err}}", err) return } + if b.CLI != nil { + b.CLI.Output("\n------------------------------------------------------------------------") + } } // Perform the plan @@ -133,32 +137,62 @@ func (b *Local) opPlan( // Perform some output tasks if we have a CLI to output to. if b.CLI != nil { - if plan.Diff.Empty() { - b.CLI.Output(b.Colorize().Color(strings.TrimSpace(planNoChanges))) + dispPlan := format.NewPlan(plan) + if dispPlan.Empty() { + b.CLI.Output("\n" + b.Colorize().Color(strings.TrimSpace(planNoChanges))) return } + b.renderPlan(dispPlan) + + b.CLI.Output("\n------------------------------------------------------------------------") + if path := op.PlanOutPath; path == "" { - b.CLI.Output(strings.TrimSpace(planHeaderNoOutput) + "\n") + b.CLI.Output(fmt.Sprintf( + "\n" + strings.TrimSpace(planHeaderNoOutput) + "\n", + )) } else { b.CLI.Output(fmt.Sprintf( - strings.TrimSpace(planHeaderYesOutput)+"\n", - path)) + "\n"+strings.TrimSpace(planHeaderYesOutput)+"\n", + path, path, + )) } + } +} + +func (b *Local) renderPlan(dispPlan *format.Plan) { - b.CLI.Output(format.Plan(&format.PlanOpts{ - Plan: plan, - Color: b.Colorize(), - ModuleDepth: -1, - })) - - b.CLI.Output(b.Colorize().Color(fmt.Sprintf( - "[reset][bold]Plan:[reset] "+ - "%d to add, %d to change, %d to destroy.", - countHook.ToAdd+countHook.ToRemoveAndAdd, - countHook.ToChange, - countHook.ToRemove+countHook.ToRemoveAndAdd))) + headerBuf := &bytes.Buffer{} + fmt.Fprintf(headerBuf, "\n%s\n", strings.TrimSpace(planHeaderIntro)) + counts := dispPlan.ActionCounts() + if counts[terraform.DiffCreate] > 0 { + fmt.Fprintf(headerBuf, "%s create\n", format.DiffActionSymbol(terraform.DiffCreate)) + } + if counts[terraform.DiffUpdate] > 0 { + fmt.Fprintf(headerBuf, "%s update in-place\n", format.DiffActionSymbol(terraform.DiffUpdate)) + } + if counts[terraform.DiffDestroy] > 0 { + fmt.Fprintf(headerBuf, "%s destroy\n", format.DiffActionSymbol(terraform.DiffDestroy)) } + if counts[terraform.DiffDestroyCreate] > 0 { + fmt.Fprintf(headerBuf, "%s destroy and then create replacement\n", format.DiffActionSymbol(terraform.DiffDestroyCreate)) + } + if counts[terraform.DiffRefresh] > 0 { + fmt.Fprintf(headerBuf, "%s read (data resources)\n", format.DiffActionSymbol(terraform.DiffRefresh)) + } + + b.CLI.Output(b.Colorize().Color(headerBuf.String())) + + b.CLI.Output("Terraform will perform the following actions:\n") + + b.CLI.Output(dispPlan.Format(b.Colorize())) + + stats := dispPlan.Stats() + b.CLI.Output(b.Colorize().Color(fmt.Sprintf( + "[reset][bold]Plan:[reset] "+ + "%d to add, %d to change, %d to destroy.", + stats.ToAdd, stats.ToChange, stats.ToDestroy, + ))) } const planErrNoConfig = ` @@ -171,37 +205,30 @@ flag or create a single empty configuration file. Otherwise, please create a Terraform configuration file in the path being executed and try again. ` +const planHeaderIntro = ` +An execution plan has been generated and is shown below. +Resource actions are indicated with the following symbols: +` + const planHeaderNoOutput = ` -The Terraform execution plan has been generated and is shown below. -Resources are shown in alphabetical order for quick scanning. Green resources -will be created (or destroyed and then created if an existing resource -exists), yellow resources are being changed in-place, and red resources -will be destroyed. Cyan entries are data sources to be read. - -Note: You didn't specify an "-out" parameter to save this plan, so when -"apply" is called, Terraform can't guarantee this is what will execute. +Note: You didn't specify an "-out" parameter to save this plan, so Terraform +can't guarantee that exactly these actions will be performed if +"terraform apply" is subsequently run. ` const planHeaderYesOutput = ` -The Terraform execution plan has been generated and is shown below. -Resources are shown in alphabetical order for quick scanning. Green resources -will be created (or destroyed and then created if an existing resource -exists), yellow resources are being changed in-place, and red resources -will be destroyed. Cyan entries are data sources to be read. - -Your plan was also saved to the path below. Call the "apply" subcommand -with this plan file and Terraform will exactly execute this execution -plan. +This plan was saved to: %s -Path: %s +To perform exactly these actions, run the following command to apply: + terraform apply %q ` const planNoChanges = ` [reset][bold][green]No changes. Infrastructure is up-to-date.[reset][green] This means that Terraform did not detect any differences between your -configuration and real physical resources that exist. As a result, Terraform -doesn't need to do anything. +configuration and real physical resources that exist. As a result, no +actions need to be performed. ` const planRefreshing = ` diff --git a/command/format/plan.go b/command/format/plan.go index 6bc699e398bd..728345e0a8b9 100644 --- a/command/format/plan.go +++ b/command/format/plan.go @@ -11,232 +11,339 @@ import ( "github.com/mitchellh/colorstring" ) -// PlanOpts are the options for formatting a plan. -type PlanOpts struct { - // Plan is the plan to format. This is required. - Plan *terraform.Plan +// Plan is a representation of a plan optimized for display to +// an end-user, as opposed to terraform.Plan which is for internal use. +// +// DisplayPlan excludes implementation details that may otherwise appear +// in the main plan, such as destroy actions on data sources (which are +// there only to clean up the state). +type Plan struct { + Resources []*InstanceDiff +} + +// InstanceDiff is a representation of an instance diff optimized +// for display, in conjunction with DisplayPlan. +type InstanceDiff struct { + Addr *terraform.ResourceAddress + Action terraform.DiffChangeType - // Color is the colorizer. This is optional. - Color *colorstring.Colorize + // Attributes describes changes to the attributes of the instance. + // + // For destroy diffs this is always nil. + Attributes []*AttributeDiff - // ModuleDepth is the depth of the modules to expand. By default this - // is zero which will not expand modules at all. - ModuleDepth int + Tainted bool + Deposed bool } -// Plan takes a plan and returns a -func Plan(opts *PlanOpts) string { - p := opts.Plan - if p.Diff == nil || p.Diff.Empty() { - return "This plan does nothing." - } +// AttributeDiff is a representation of an attribute diff optimized +// for display, in conjunction with DisplayInstanceDiff. +type AttributeDiff struct { + // Path is a dot-delimited traversal through possibly many levels of list and map structure, + // intended for display purposes only. + Path string - if opts.Color == nil { - opts.Color = &colorstring.Colorize{ - Colors: colorstring.DefaultColors, - Reset: false, - } - } + Action terraform.DiffChangeType - buf := new(bytes.Buffer) - for _, m := range p.Diff.Modules { - if len(m.Path)-1 <= opts.ModuleDepth || opts.ModuleDepth == -1 { - formatPlanModuleExpand(buf, m, opts) - } else { - formatPlanModuleSingle(buf, m, opts) - } - } + OldValue string + NewValue string - return strings.TrimSpace(buf.String()) + NewComputed bool + Sensitive bool + ForcesNew bool } -// formatPlanModuleExpand will output the given module and all of its -// resources. -func formatPlanModuleExpand( - buf *bytes.Buffer, m *terraform.ModuleDiff, opts *PlanOpts) { - // Ignore empty diffs - if m.Empty() { - return - } +// PlanStats gives summary counts for a Plan. +type PlanStats struct { + ToAdd, ToChange, ToDestroy int +} - var modulePath []string - if !m.IsRoot() { - modulePath = m.Path[1:] +// NewPlan produces a display-oriented Plan from a terraform.Plan. +func NewPlan(plan *terraform.Plan) *Plan { + ret := &Plan{} + if plan == nil || plan.Diff == nil || plan.Diff.Empty() { + // Nothing to do! + return ret } - // We want to output the resources in sorted order to make things - // easier to scan through, so get all the resource names and sort them. - names := make([]string, 0, len(m.Resources)) - addrs := map[string]*terraform.ResourceAddress{} - for name := range m.Resources { - names = append(names, name) - var err error - addrs[name], err = terraform.ParseResourceAddressForInstanceDiff(modulePath, name) - if err != nil { - // should never happen; indicates invalid diff - panic("invalid resource address in diff") + for _, m := range plan.Diff.Modules { + var modulePath []string + if !m.IsRoot() { + // trim off the leading "root" path segment, since it's implied + // when we use a path in a resource address. + modulePath = m.Path[1:] } - } - sort.Slice(names, func(i, j int) bool { - return addrs[names[i]].Less(addrs[names[j]]) - }) - // Go through each sorted name and start building the output - for _, name := range names { - rdiff := m.Resources[name] - if rdiff.Empty() { - continue - } + for k, r := range m.Resources { + if r.Empty() { + continue + } - addr := addrs[name] - addrStr := addr.String() - dataSource := addr.Mode == config.DataResourceMode - - // Determine the color for the text (green for adding, yellow - // for change, red for delete), and symbol, and output the - // resource header. - color := "yellow" - symbol := " ~" - oldValues := true - switch rdiff.ChangeType() { - case terraform.DiffDestroyCreate: - color = "yellow" - symbol = "[red]-[reset]/[green]+[reset][yellow]" - case terraform.DiffCreate: - color = "green" - symbol = " +" - oldValues = false - - // If we're "creating" a data resource then we'll present it - // to the user as a "read" operation, so it's clear that this - // operation won't change anything outside of the Terraform state. - // Unfortunately by the time we get here we only have the name - // to work with, so we need to cheat and exploit knowledge of the - // naming scheme for data resources. - if dataSource { - symbol = " <=" - color = "cyan" + addr, err := terraform.ParseResourceAddressForInstanceDiff(modulePath, k) + if err != nil { + // should never happen; indicates invalid diff + panic("invalid resource address in diff") } - case terraform.DiffDestroy: - color = "red" - symbol = " -" - } - var extraAttr []string - if rdiff.DestroyTainted { - extraAttr = append(extraAttr, "tainted") - } - if rdiff.DestroyDeposed { - extraAttr = append(extraAttr, "deposed") - } - var extraStr string - if len(extraAttr) > 0 { - extraStr = fmt.Sprintf(" (%s)", strings.Join(extraAttr, ", ")) - } - if rdiff.ChangeType() == terraform.DiffDestroyCreate { - extraStr = extraStr + opts.Color.Color(" [red][bold](new resource required)") - } + dataSource := addr.Mode == config.DataResourceMode - buf.WriteString(opts.Color.Color(fmt.Sprintf( - "[%s]%s %s%s\n", - color, symbol, addrStr, extraStr))) - - // Get all the attributes that are changing, and sort them. Also - // determine the longest key so that we can align them all. - keyLen := 0 - keys := make([]string, 0, len(rdiff.Attributes)) - for key, _ := range rdiff.Attributes { - // Skip the ID since we do that specially - if key == "id" { + // We create "destroy" actions for data resources so we can clean + // up their entries in state, but this is an implementation detail + // that users shouldn't see. + if dataSource && r.ChangeType() == terraform.DiffDestroy { continue } - keys = append(keys, key) - if len(key) > keyLen { - keyLen = len(key) + did := &InstanceDiff{ + Addr: addr, + Action: r.ChangeType(), + Tainted: r.DestroyTainted, + Deposed: r.DestroyDeposed, } - } - sort.Strings(keys) - - // Go through and output each attribute - for _, attrK := range keys { - attrDiff := rdiff.Attributes[attrK] - v := attrDiff.New - if v == "" && attrDiff.NewComputed { - v = "" + if dataSource && did.Action == terraform.DiffCreate { + // Use "refresh" as the action for display, since core + // currently uses Create for this. + did.Action = terraform.DiffRefresh } - if attrDiff.Sensitive { - v = "" + ret.Resources = append(ret.Resources, did) + + if did.Action == terraform.DiffDestroy { + // Don't show any outputs for destroy actions + continue } - updateMsg := "" - if attrDiff.RequiresNew && rdiff.Destroy { - updateMsg = opts.Color.Color(" [red](forces new resource)") - } else if attrDiff.Sensitive && oldValues { - updateMsg = opts.Color.Color(" [yellow](attribute changed)") + for k, a := range r.Attributes { + var action terraform.DiffChangeType + switch { + case a.NewRemoved: + action = terraform.DiffDestroy + case did.Action == terraform.DiffCreate: + action = terraform.DiffCreate + default: + action = terraform.DiffUpdate + } + + did.Attributes = append(did.Attributes, &AttributeDiff{ + Path: k, + Action: action, + + OldValue: a.Old, + NewValue: a.New, + + Sensitive: a.Sensitive, + ForcesNew: a.RequiresNew, + NewComputed: a.NewComputed, + }) } - if oldValues { - var u string - if attrDiff.Sensitive { - u = "" - } else { - u = attrDiff.Old + // Sort the attributes by their paths for display + sort.Slice(did.Attributes, func(i, j int) bool { + iPath := did.Attributes[i].Path + jPath := did.Attributes[j].Path + + // as a special case, "id" is always first + switch { + case iPath != jPath && (iPath == "id" || jPath == "id"): + return iPath == "id" + default: + return iPath < jPath } - buf.WriteString(fmt.Sprintf( - " %s:%s %#v => %#v%s\n", - attrK, - strings.Repeat(" ", keyLen-len(attrK)), - u, - v, - updateMsg)) - } else { - buf.WriteString(fmt.Sprintf( - " %s:%s %#v%s\n", - attrK, - strings.Repeat(" ", keyLen-len(attrK)), - v, - updateMsg)) + }) + + } + } + + // Sort the instance diffs by their addresses for display. + sort.Slice(ret.Resources, func(i, j int) bool { + iAddr := ret.Resources[i].Addr + jAddr := ret.Resources[j].Addr + return iAddr.Less(jAddr) + }) + + return ret +} + +// Format produces and returns a text representation of the receiving plan +// intended for display in a terminal. +// +// If color is not nil, it is used to colorize the output. +func (p *Plan) Format(color *colorstring.Colorize) string { + if p.Empty() { + return "This plan does nothing." + } + + if color == nil { + color = &colorstring.Colorize{ + Colors: colorstring.DefaultColors, + Reset: false, + } + } + + // Find the longest path length of all the paths that are changing, + // so we can align them all. + keyLen := 0 + for _, r := range p.Resources { + for _, attr := range r.Attributes { + key := attr.Path + + if len(key) > keyLen { + keyLen = len(key) } } + } + + buf := new(bytes.Buffer) + for _, r := range p.Resources { + formatPlanInstanceDiff(buf, r, keyLen, color) + } + + return strings.TrimSpace(buf.String()) +} + +// Stats returns statistics about the plan +func (p *Plan) Stats() PlanStats { + var ret PlanStats + for _, r := range p.Resources { + switch r.Action { + case terraform.DiffCreate: + ret.ToAdd++ + case terraform.DiffUpdate: + ret.ToChange++ + case terraform.DiffDestroyCreate: + ret.ToAdd++ + ret.ToDestroy++ + case terraform.DiffDestroy: + ret.ToDestroy++ + } + } + return ret +} - // Write the reset color so we don't overload the user's terminal - buf.WriteString(opts.Color.Color("[reset]\n")) +// ActionCounts returns the number of diffs for each action type +func (p *Plan) ActionCounts() map[terraform.DiffChangeType]int { + ret := map[terraform.DiffChangeType]int{} + for _, r := range p.Resources { + ret[r.Action]++ } + return ret } -// formatPlanModuleSingle will output the given module and all of its -// resources. -func formatPlanModuleSingle( - buf *bytes.Buffer, m *terraform.ModuleDiff, opts *PlanOpts) { - // Ignore empty diffs - if m.Empty() { - return +// Empty returns true if there is at least one resource diff in the receiving plan. +func (p *Plan) Empty() bool { + return len(p.Resources) == 0 +} + +// DiffActionSymbol returns a string that, once passed through a +// colorstring.Colorize, will produce a result that can be written +// to a terminal to produce a symbol made of three printable +// characters, possibly interspersed with VT100 color codes. +func DiffActionSymbol(action terraform.DiffChangeType) string { + switch action { + case terraform.DiffDestroyCreate: + return "[red]-[reset]/[green]+[reset]" + case terraform.DiffCreate: + return " [green]+[reset]" + case terraform.DiffDestroy: + return " [red]-[reset]" + case terraform.DiffRefresh: + return " [cyan]<=[reset]" + default: + return " [yellow]~[reset]" } +} - moduleName := fmt.Sprintf("module.%s", strings.Join(m.Path[1:], ".")) +// formatPlanInstanceDiff writes the text representation of the given instance diff +// to the given buffer, using the given colorizer. +func formatPlanInstanceDiff(buf *bytes.Buffer, r *InstanceDiff, keyLen int, colorizer *colorstring.Colorize) { + addrStr := r.Addr.String() // Determine the color for the text (green for adding, yellow // for change, red for delete), and symbol, and output the // resource header. color := "yellow" - symbol := "~" - switch m.ChangeType() { + symbol := DiffActionSymbol(r.Action) + oldValues := true + switch r.Action { + case terraform.DiffDestroyCreate: + color = "yellow" case terraform.DiffCreate: color = "green" - symbol = "+" + oldValues = false case terraform.DiffDestroy: color = "red" - symbol = "-" + case terraform.DiffRefresh: + color = "cyan" + oldValues = false + } + + var extraStr string + if r.Tainted { + extraStr = extraStr + " (tainted)" + } + if r.Deposed { + extraStr = extraStr + " (deposed)" + } + if r.Action == terraform.DiffDestroyCreate { + extraStr = extraStr + colorizer.Color(" [red][bold](new resource required)") + } + + buf.WriteString( + colorizer.Color(fmt.Sprintf( + "[%s]%s [%s]%s%s\n", + color, symbol, color, addrStr, extraStr, + )), + ) + + for _, attr := range r.Attributes { + + v := attr.NewValue + var dispV string + switch { + case v == "" && attr.NewComputed: + dispV = "" + case attr.Sensitive: + dispV = "" + default: + dispV = fmt.Sprintf("%q", v) + } + + updateMsg := "" + switch { + case attr.ForcesNew && r.Action == terraform.DiffDestroy: + updateMsg = colorizer.Color(" [red](forces new resource)") + case attr.Sensitive && oldValues: + updateMsg = colorizer.Color(" [yellow](attribute changed)") + } + + if oldValues { + u := attr.OldValue + var dispU string + switch { + case attr.Sensitive: + dispU = "" + default: + dispU = fmt.Sprintf("%q", u) + } + buf.WriteString(fmt.Sprintf( + " %s:%s %s => %s%s\n", + attr.Path, + strings.Repeat(" ", keyLen-len(attr.Path)), + dispU, dispV, + updateMsg, + )) + } else { + buf.WriteString(fmt.Sprintf( + " %s:%s %s%s\n", + attr.Path, + strings.Repeat(" ", keyLen-len(attr.Path)), + dispV, + updateMsg, + )) + } } - buf.WriteString(opts.Color.Color(fmt.Sprintf( - "[%s]%s %s\n", - color, symbol, moduleName))) - buf.WriteString(fmt.Sprintf( - " %d resource(s)", - len(m.Resources))) - buf.WriteString(opts.Color.Color("[reset]\n")) + // Write the reset color so we don't bleed color into later text + buf.WriteString(colorizer.Color("[reset]\n")) } diff --git a/command/format/plan_test.go b/command/format/plan_test.go index 82390d3d9d31..7ee3802617e2 100644 --- a/command/format/plan_test.go +++ b/command/format/plan_test.go @@ -1,14 +1,452 @@ package format import ( + "reflect" "strings" "testing" + "github.com/davecgh/go-spew/spew" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/colorstring" ) -// Test that a root level data source gets a special plan output on create +var disabledColorize = &colorstring.Colorize{ + Colors: colorstring.DefaultColors, + Disable: true, +} + +func TestNewPlan(t *testing.T) { + tests := map[string]struct { + Input *terraform.Plan + Want *Plan + }{ + "nil input": { + Input: nil, + Want: &Plan{ + Resources: nil, + }, + }, + "nil diff": { + Input: &terraform.Plan{}, + Want: &Plan{ + Resources: nil, + }, + }, + "empty diff": { + Input: &terraform.Plan{ + Diff: &terraform.Diff{ + Modules: []*terraform.ModuleDiff{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.InstanceDiff{}, + }, + }, + }, + }, + Want: &Plan{ + Resources: nil, + }, + }, + "create managed resource": { + Input: &terraform.Plan{ + Diff: &terraform.Diff{ + Modules: []*terraform.ModuleDiff{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.InstanceDiff{ + "test_resource.foo": { + Attributes: map[string]*terraform.ResourceAttrDiff{ + "id": { + NewComputed: true, + RequiresNew: true, + }, + }, + }, + }, + }, + }, + }, + }, + Want: &Plan{ + Resources: []*InstanceDiff{ + { + Addr: mustParseResourceAddress("test_resource.foo"), + Action: terraform.DiffCreate, + Attributes: []*AttributeDiff{ + { + Path: "id", + Action: terraform.DiffCreate, + NewComputed: true, + ForcesNew: true, + }, + }, + }, + }, + }, + }, + "create managed resource in child module": { + Input: &terraform.Plan{ + Diff: &terraform.Diff{ + Modules: []*terraform.ModuleDiff{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.InstanceDiff{ + "test_resource.foo": { + Attributes: map[string]*terraform.ResourceAttrDiff{ + "id": { + NewComputed: true, + RequiresNew: true, + }, + }, + }, + }, + }, + { + Path: []string{"root", "foo"}, + Resources: map[string]*terraform.InstanceDiff{ + "test_resource.foo": { + Attributes: map[string]*terraform.ResourceAttrDiff{ + "id": { + NewComputed: true, + RequiresNew: true, + }, + }, + }, + }, + }, + }, + }, + }, + Want: &Plan{ + Resources: []*InstanceDiff{ + { + Addr: mustParseResourceAddress("test_resource.foo"), + Action: terraform.DiffCreate, + Attributes: []*AttributeDiff{ + { + Path: "id", + Action: terraform.DiffCreate, + NewComputed: true, + ForcesNew: true, + }, + }, + }, + { + Addr: mustParseResourceAddress("module.foo.test_resource.foo"), + Action: terraform.DiffCreate, + Attributes: []*AttributeDiff{ + { + Path: "id", + Action: terraform.DiffCreate, + NewComputed: true, + ForcesNew: true, + }, + }, + }, + }, + }, + }, + "create data resource": { + Input: &terraform.Plan{ + Diff: &terraform.Diff{ + Modules: []*terraform.ModuleDiff{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.InstanceDiff{ + "data.test_data_source.foo": { + Attributes: map[string]*terraform.ResourceAttrDiff{ + "id": { + NewComputed: true, + RequiresNew: true, + }, + }, + }, + }, + }, + }, + }, + }, + Want: &Plan{ + Resources: []*InstanceDiff{ + { + Addr: mustParseResourceAddress("data.test_data_source.foo"), + Action: terraform.DiffRefresh, + Attributes: []*AttributeDiff{ + { + Path: "id", + Action: terraform.DiffUpdate, + NewComputed: true, + ForcesNew: true, + }, + }, + }, + }, + }, + }, + "destroy managed resource": { + Input: &terraform.Plan{ + Diff: &terraform.Diff{ + Modules: []*terraform.ModuleDiff{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.InstanceDiff{ + "test_resource.foo": { + Destroy: true, + }, + }, + }, + }, + }, + }, + Want: &Plan{ + Resources: []*InstanceDiff{ + { + Addr: mustParseResourceAddress("test_resource.foo"), + Action: terraform.DiffDestroy, + }, + }, + }, + }, + "destroy data resource": { + Input: &terraform.Plan{ + Diff: &terraform.Diff{ + Modules: []*terraform.ModuleDiff{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.InstanceDiff{ + "data.test_data_source.foo": { + Destroy: true, + }, + }, + }, + }, + }, + }, + Want: &Plan{ + // Data source destroys are not shown + Resources: nil, + }, + }, + "destroy many instances of a resource": { + Input: &terraform.Plan{ + Diff: &terraform.Diff{ + Modules: []*terraform.ModuleDiff{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.InstanceDiff{ + "test_resource.foo.0": { + Destroy: true, + }, + "test_resource.foo.1": { + Destroy: true, + }, + "test_resource.foo.10": { + Destroy: true, + }, + "test_resource.foo.2": { + Destroy: true, + }, + "test_resource.foo.3": { + Destroy: true, + }, + "test_resource.foo.4": { + Destroy: true, + }, + "test_resource.foo.5": { + Destroy: true, + }, + "test_resource.foo.6": { + Destroy: true, + }, + "test_resource.foo.7": { + Destroy: true, + }, + "test_resource.foo.8": { + Destroy: true, + }, + "test_resource.foo.9": { + Destroy: true, + }, + }, + }, + }, + }, + }, + Want: &Plan{ + Resources: []*InstanceDiff{ + { + Addr: mustParseResourceAddress("test_resource.foo[0]"), + Action: terraform.DiffDestroy, + }, + { + Addr: mustParseResourceAddress("test_resource.foo[1]"), + Action: terraform.DiffDestroy, + }, + { + Addr: mustParseResourceAddress("test_resource.foo[2]"), + Action: terraform.DiffDestroy, + }, + { + Addr: mustParseResourceAddress("test_resource.foo[3]"), + Action: terraform.DiffDestroy, + }, + { + Addr: mustParseResourceAddress("test_resource.foo[4]"), + Action: terraform.DiffDestroy, + }, + { + Addr: mustParseResourceAddress("test_resource.foo[5]"), + Action: terraform.DiffDestroy, + }, + { + Addr: mustParseResourceAddress("test_resource.foo[6]"), + Action: terraform.DiffDestroy, + }, + { + Addr: mustParseResourceAddress("test_resource.foo[7]"), + Action: terraform.DiffDestroy, + }, + { + Addr: mustParseResourceAddress("test_resource.foo[8]"), + Action: terraform.DiffDestroy, + }, + { + Addr: mustParseResourceAddress("test_resource.foo[9]"), + Action: terraform.DiffDestroy, + }, + { + Addr: mustParseResourceAddress("test_resource.foo[10]"), + Action: terraform.DiffDestroy, + }, + }, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + got := NewPlan(test.Input) + if !reflect.DeepEqual(got, test.Want) { + t.Errorf( + "wrong result\ninput: %sgot: %swant:%s", + spew.Sdump(test.Input), + spew.Sdump(got), + spew.Sdump(test.Want), + ) + } + }) + } +} + +func TestPlanStats(t *testing.T) { + tests := map[string]struct { + Input *Plan + Want PlanStats + }{ + "empty": { + &Plan{}, + PlanStats{}, + }, + "destroy": { + &Plan{ + Resources: []*InstanceDiff{ + { + Addr: mustParseResourceAddress("test_resource.foo"), + Action: terraform.DiffDestroy, + }, + { + Addr: mustParseResourceAddress("test_resource.bar"), + Action: terraform.DiffDestroy, + }, + }, + }, + PlanStats{ + ToDestroy: 2, + }, + }, + "create": { + &Plan{ + Resources: []*InstanceDiff{ + { + Addr: mustParseResourceAddress("test_resource.foo"), + Action: terraform.DiffCreate, + }, + { + Addr: mustParseResourceAddress("test_resource.bar"), + Action: terraform.DiffCreate, + }, + }, + }, + PlanStats{ + ToAdd: 2, + }, + }, + "update": { + &Plan{ + Resources: []*InstanceDiff{ + { + Addr: mustParseResourceAddress("test_resource.foo"), + Action: terraform.DiffUpdate, + }, + { + Addr: mustParseResourceAddress("test_resource.bar"), + Action: terraform.DiffUpdate, + }, + }, + }, + PlanStats{ + ToChange: 2, + }, + }, + "data source refresh": { + &Plan{ + Resources: []*InstanceDiff{ + { + Addr: mustParseResourceAddress("data.test.foo"), + Action: terraform.DiffRefresh, + }, + }, + }, + PlanStats{ + // data resource refreshes are not counted in our stats + }, + }, + "replace": { + &Plan{ + Resources: []*InstanceDiff{ + { + Addr: mustParseResourceAddress("test_resource.foo"), + Action: terraform.DiffDestroyCreate, + }, + { + Addr: mustParseResourceAddress("test_resource.bar"), + Action: terraform.DiffDestroyCreate, + }, + }, + }, + PlanStats{ + ToDestroy: 2, + ToAdd: 2, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + got := test.Input.Stats() + if !reflect.DeepEqual(got, test.Want) { + t.Errorf( + "wrong result\ninput: %sgot: %swant:%s", + spew.Sdump(test.Input), + spew.Sdump(got), + spew.Sdump(test.Want), + ) + } + }) + } +} + +// Test that deposed instances are marked as such func TestPlan_destroyDeposed(t *testing.T) { plan := &terraform.Plan{ Diff: &terraform.Diff{ @@ -24,16 +462,8 @@ func TestPlan_destroyDeposed(t *testing.T) { }, }, } - opts := &PlanOpts{ - Plan: plan, - Color: &colorstring.Colorize{ - Colors: colorstring.DefaultColors, - Disable: true, - }, - ModuleDepth: 1, - } - - actual := Plan(opts) + dispPlan := NewPlan(plan) + actual := dispPlan.Format(disabledColorize) expected := strings.TrimSpace(` - aws_instance.foo (deposed) @@ -64,16 +494,8 @@ func TestPlan_displayInterpolations(t *testing.T) { }, }, } - opts := &PlanOpts{ - Plan: plan, - Color: &colorstring.Colorize{ - Colors: colorstring.DefaultColors, - Disable: true, - }, - ModuleDepth: 1, - } - - out := Plan(opts) + dispPlan := NewPlan(plan) + out := dispPlan.Format(disabledColorize) lines := strings.Split(out, "\n") if len(lines) != 2 { t.Fatal("expected 2 lines of output, got:\n", out) @@ -108,16 +530,8 @@ func TestPlan_rootDataSource(t *testing.T) { }, }, } - opts := &PlanOpts{ - Plan: plan, - Color: &colorstring.Colorize{ - Colors: colorstring.DefaultColors, - Disable: true, - }, - ModuleDepth: 1, - } - - actual := Plan(opts) + dispPlan := NewPlan(plan) + actual := dispPlan.Format(disabledColorize) expected := strings.TrimSpace(` <= data.type.name @@ -149,16 +563,8 @@ func TestPlan_nestedDataSource(t *testing.T) { }, }, } - opts := &PlanOpts{ - Plan: plan, - Color: &colorstring.Colorize{ - Colors: colorstring.DefaultColors, - Disable: true, - }, - ModuleDepth: 2, - } - - actual := Plan(opts) + dispPlan := NewPlan(plan) + actual := dispPlan.Format(disabledColorize) expected := strings.TrimSpace(` <= module.nested.data.type.name @@ -168,3 +574,11 @@ func TestPlan_nestedDataSource(t *testing.T) { t.Fatalf("expected:\n\n%s\n\ngot:\n\n%s", expected, actual) } } + +func mustParseResourceAddress(s string) *terraform.ResourceAddress { + addr, err := terraform.ParseResourceAddress(s) + if err != nil { + panic(err) + } + return addr +} diff --git a/command/hook_ui.go b/command/hook_ui.go index 5d4690677cae..76b1ca59cff9 100644 --- a/command/hook_ui.go +++ b/command/hook_ui.go @@ -65,6 +65,7 @@ func (h *UiHook) PreApply( } id := n.HumanId() + addr := n.ResourceAddress() op := uiResourceModify if d.Destroy { @@ -142,7 +143,7 @@ func (h *UiHook) PreApply( h.ui.Output(h.Colorize.Color(fmt.Sprintf( "[reset][bold]%s: %s%s[reset]%s", - id, + addr, operation, stateIdSuffix, attrString))) @@ -210,6 +211,7 @@ func (h *UiHook) PostApply( applyerr error) (terraform.HookAction, error) { id := n.HumanId() + addr := n.ResourceAddress() h.l.Lock() state := h.resources[id] @@ -244,7 +246,7 @@ func (h *UiHook) PostApply( colorized := h.Colorize.Color(fmt.Sprintf( "[reset][bold]%s: %s after %s%s[reset]", - id, msg, time.Now().Round(time.Second).Sub(state.Start), stateIdSuffix)) + addr, msg, time.Now().Round(time.Second).Sub(state.Start), stateIdSuffix)) h.ui.Output(colorized) @@ -260,10 +262,10 @@ func (h *UiHook) PreDiff( func (h *UiHook) PreProvision( n *terraform.InstanceInfo, provId string) (terraform.HookAction, error) { - id := n.HumanId() + addr := n.ResourceAddress() h.ui.Output(h.Colorize.Color(fmt.Sprintf( "[reset][bold]%s: Provisioning with '%s'...[reset]", - id, provId))) + addr, provId))) return terraform.HookActionContinue, nil } @@ -271,11 +273,11 @@ func (h *UiHook) ProvisionOutput( n *terraform.InstanceInfo, provId string, msg string) { - id := n.HumanId() + addr := n.ResourceAddress() var buf bytes.Buffer buf.WriteString(h.Colorize.Color("[reset]")) - prefix := fmt.Sprintf("%s (%s): ", id, provId) + prefix := fmt.Sprintf("%s (%s): ", addr, provId) s := bufio.NewScanner(strings.NewReader(msg)) s.Split(scanLines) for s.Scan() { @@ -293,7 +295,7 @@ func (h *UiHook) PreRefresh( s *terraform.InstanceState) (terraform.HookAction, error) { h.once.Do(h.init) - id := n.HumanId() + addr := n.ResourceAddress() var stateIdSuffix string // Data resources refresh before they have ids, whereas managed @@ -304,7 +306,7 @@ func (h *UiHook) PreRefresh( h.ui.Output(h.Colorize.Color(fmt.Sprintf( "[reset][bold]%s: Refreshing state...%s", - id, stateIdSuffix))) + addr, stateIdSuffix))) return terraform.HookActionContinue, nil } @@ -313,9 +315,10 @@ func (h *UiHook) PreImportState( id string) (terraform.HookAction, error) { h.once.Do(h.init) + addr := n.ResourceAddress() h.ui.Output(h.Colorize.Color(fmt.Sprintf( "[reset][bold]%s: Importing from ID %q...", - n.HumanId(), id))) + addr, id))) return terraform.HookActionContinue, nil } @@ -324,9 +327,9 @@ func (h *UiHook) PostImportState( s []*terraform.InstanceState) (terraform.HookAction, error) { h.once.Do(h.init) - id := n.HumanId() + addr := n.ResourceAddress() h.ui.Output(h.Colorize.Color(fmt.Sprintf( - "[reset][bold][green]%s: Import complete!", id))) + "[reset][bold][green]%s: Import complete!", addr))) for _, s := range s { h.ui.Output(h.Colorize.Color(fmt.Sprintf( "[reset][green] Imported %s (ID: %s)", diff --git a/command/show.go b/command/show.go index 254644863be8..d8d1c84efef7 100644 --- a/command/show.go +++ b/command/show.go @@ -110,11 +110,8 @@ func (c *ShowCommand) Run(args []string) int { } if plan != nil { - c.Ui.Output(format.Plan(&format.PlanOpts{ - Plan: plan, - Color: c.Colorize(), - ModuleDepth: moduleDepth, - })) + dispPlan := format.NewPlan(plan) + c.Ui.Output(dispPlan.Format(c.Colorize())) return 0 } diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 48d8a9f490b8..fc45c6ac032e 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -1519,6 +1519,7 @@ func TestContext2Apply_destroyData(t *testing.T) { }, }, } + hook := &testHook{} ctx := testContext2(t, &ContextOpts{ Module: m, ProviderResolver: ResourceProviderResolverFixed( @@ -1528,6 +1529,7 @@ func TestContext2Apply_destroyData(t *testing.T) { ), State: state, Destroy: true, + Hooks: []Hook{hook}, }) if p, err := ctx.Plan(); err != nil { @@ -1548,6 +1550,15 @@ func TestContext2Apply_destroyData(t *testing.T) { if got := len(newState.Modules[0].Resources); got != 0 { t.Fatalf("state has %d resources after destroy; want 0", got) } + + wantHookCalls := []*testHookCall{ + {"PreDiff", "data.null_data_source.testing"}, + {"PostDiff", "data.null_data_source.testing"}, + {"PostStateUpdate", ""}, + } + if !reflect.DeepEqual(hook.Calls, wantHookCalls) { + t.Errorf("wrong hook calls\ngot: %swant: %s", spew.Sdump(hook.Calls), spew.Sdump(wantHookCalls)) + } } // https://github.com/hashicorp/terraform/pull/5096 diff --git a/terraform/diff.go b/terraform/diff.go index fd1687e7ed6c..9cbd5d88fa7a 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -23,6 +23,12 @@ const ( DiffUpdate DiffDestroy DiffDestroyCreate + + // DiffRefresh is only used in the UI for displaying diffs. + // Managed resource reads never appear in plan, and when data source + // reads appear they are represented as DiffCreate in core before + // transforming to DiffRefresh in the UI layer. + DiffRefresh // TODO: Actually use DiffRefresh in core too, for less confusion ) // multiVal matches the index key to a flatmapped set, list or map diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index 2f6a4973e454..36c98458eb96 100644 --- a/terraform/eval_apply.go +++ b/terraform/eval_apply.go @@ -112,7 +112,7 @@ func (n *EvalApplyPre) Eval(ctx EvalContext) (interface{}, error) { } state.init() - { + if resourceHasUserVisibleApply(n.Info) { // Call post-apply hook err := ctx.Hook(func(h Hook) (HookAction, error) { return h.PreApply(n.Info, state, diff) @@ -136,7 +136,7 @@ type EvalApplyPost struct { func (n *EvalApplyPost) Eval(ctx EvalContext) (interface{}, error) { state := *n.State - { + if resourceHasUserVisibleApply(n.Info) { // Call post-apply hook err := ctx.Hook(func(h Hook) (HookAction, error) { return h.PostApply(n.Info, state, *n.Error) @@ -149,6 +149,22 @@ func (n *EvalApplyPost) Eval(ctx EvalContext) (interface{}, error) { return nil, *n.Error } +// resourceHasUserVisibleApply returns true if the given resource is one where +// apply actions should be exposed to the user. +// +// Certain resources do apply actions only as an implementation detail, so +// these should not be advertised to code outside of this package. +func resourceHasUserVisibleApply(info *InstanceInfo) bool { + addr := info.ResourceAddress() + + // Only managed resources have user-visible apply actions. + // In particular, this excludes data resources since we "apply" these + // only as an implementation detail of removing them from state when + // they are destroyed. (When reading, they don't get here at all because + // we present them as "Refresh" actions.) + return addr.Mode == config.ManagedResourceMode +} + // EvalApplyProvisioners is an EvalNode implementation that executes // the provisioners for a resource. // diff --git a/terraform/hook_test.go b/terraform/hook_test.go index a16923f42167..caa4f80878e2 100644 --- a/terraform/hook_test.go +++ b/terraform/hook_test.go @@ -7,3 +7,89 @@ import ( func TestNilHook_impl(t *testing.T) { var _ Hook = new(NilHook) } + +// testHook is a Hook implementation that logs the calls it receives. +// It is intended for testing that core code is emitting the correct hooks +// for a given situation. +type testHook struct { + Calls []*testHookCall +} + +// testHookCall represents a single call in testHook. +// This hook just logs string names to make it easy to write "want" expressions +// in tests that can DeepEqual against the real calls. +type testHookCall struct { + Action string + InstanceID string +} + +func (h *testHook) PreApply(i *InstanceInfo, s *InstanceState, d *InstanceDiff) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PreApply", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) PostApply(i *InstanceInfo, s *InstanceState, err error) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PostApply", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) PreDiff(i *InstanceInfo, s *InstanceState) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PreDiff", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) PostDiff(i *InstanceInfo, d *InstanceDiff) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PostDiff", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) PreProvisionResource(i *InstanceInfo, s *InstanceState) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PreProvisionResource", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) PostProvisionResource(i *InstanceInfo, s *InstanceState) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PostProvisionResource", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) PreProvision(i *InstanceInfo, n string) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PreProvision", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) PostProvision(i *InstanceInfo, n string, err error) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PostProvision", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) ProvisionOutput(i *InstanceInfo, n string, m string) { + h.Calls = append(h.Calls, &testHookCall{"ProvisionOutput", i.ResourceAddress().String()}) +} + +func (h *testHook) PreRefresh(i *InstanceInfo, s *InstanceState) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PreRefresh", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) PostRefresh(i *InstanceInfo, s *InstanceState) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PostRefresh", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) PreImportState(i *InstanceInfo, n string) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PreImportState", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) PostImportState(i *InstanceInfo, ss []*InstanceState) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PostImportState", i.ResourceAddress().String()}) + return HookActionContinue, nil +} + +func (h *testHook) PostStateUpdate(s *State) (HookAction, error) { + h.Calls = append(h.Calls, &testHookCall{"PostStateUpdate", ""}) + return HookActionContinue, nil +} + +var _ Hook = new(testHook) diff --git a/terraform/resource.go b/terraform/resource.go index 0acf0beb2a17..a8cd8dd9f00c 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -88,6 +88,46 @@ func (i *InstanceInfo) HumanId() string { i.Id) } +// ResourceAddress returns the address of the resource that the receiver is describing. +func (i *InstanceInfo) ResourceAddress() *ResourceAddress { + // GROSS: for tainted and deposed instances, their status gets appended + // to i.Id to create a unique id for the graph node. Historically these + // ids were displayed to the user, so it's designed to be human-readable: + // "aws_instance.bar.0 (deposed #0)" + // + // So here we detect such suffixes and try to interpret them back to + // their original meaning so we can then produce a ResourceAddress + // with a suitable InstanceType. + id := i.Id + instanceType := TypeInvalid + if idx := strings.Index(id, " ("); idx != -1 { + remain := id[idx:] + id = id[:idx] + + switch { + case strings.Contains(remain, "tainted"): + instanceType = TypeTainted + case strings.Contains(remain, "deposed"): + instanceType = TypeDeposed + } + } + + addr, err := parseResourceAddressInternal(id) + if err != nil { + // should never happen, since that would indicate a bug in the + // code that constructed this InstanceInfo. + panic(fmt.Errorf("InstanceInfo has invalid Id %s", id)) + } + if len(i.ModulePath) > 1 { + addr.Path = i.ModulePath[1:] // trim off "root" prefix, which is implied + } + if instanceType != TypeInvalid { + addr.InstanceTypeSet = true + addr.InstanceType = instanceType + } + return addr +} + func (i *InstanceInfo) uniqueId() string { prefix := i.HumanId() if v := i.uniqueExtra; v != "" { diff --git a/terraform/resource_address.go b/terraform/resource_address.go index 8badca8053c6..353725844556 100644 --- a/terraform/resource_address.go +++ b/terraform/resource_address.go @@ -362,40 +362,41 @@ func (addr *ResourceAddress) Less(other *ResourceAddress) bool { switch { - case len(addr.Path) < len(other.Path): - return true + case len(addr.Path) != len(other.Path): + return len(addr.Path) < len(other.Path) case !reflect.DeepEqual(addr.Path, other.Path): // If the two paths are the same length but don't match, we'll just // cheat and compare the string forms since it's easier than - // comparing all of the path segments in turn. + // comparing all of the path segments in turn, and lexicographic + // comparison is correct for the module path portion. addrStr := addr.String() otherStr := other.String() return addrStr < otherStr - case addr.Mode == config.DataResourceMode && other.Mode != config.DataResourceMode: - return true + case addr.Mode != other.Mode: + return addr.Mode == config.DataResourceMode - case addr.Type < other.Type: - return true + case addr.Type != other.Type: + return addr.Type < other.Type - case addr.Name < other.Name: - return true + case addr.Name != other.Name: + return addr.Name < other.Name - case addr.Index < other.Index: + case addr.Index != other.Index: // Since "Index" is -1 for an un-indexed address, this also conveniently // sorts unindexed addresses before indexed ones, should they both // appear for some reason. - return true + return addr.Index < other.Index - case other.InstanceTypeSet && !addr.InstanceTypeSet: - return true + case addr.InstanceTypeSet != other.InstanceTypeSet: + return !addr.InstanceTypeSet - case addr.InstanceType < other.InstanceType: + case addr.InstanceType != other.InstanceType: // InstanceType is actually an enum, so this is just an arbitrary // sort based on the enum numeric values, and thus not particularly // meaningful. - return true + return addr.InstanceType < other.InstanceType default: return false diff --git a/terraform/resource_address_test.go b/terraform/resource_address_test.go index a2f9cb9ff068..0fe561528834 100644 --- a/terraform/resource_address_test.go +++ b/terraform/resource_address_test.go @@ -1233,6 +1233,11 @@ func TestResourceAddressLess(t *testing.T) { "module.baz.foo.bar", true, }, + { + "module.baz.foo.bar", + "zzz.bar", // would sort after "module" in lexicographical sort + false, + }, { "module.baz.foo.bar", "module.baz.foo.bar", @@ -1243,6 +1248,11 @@ func TestResourceAddressLess(t *testing.T) { "module.boz.foo.bar", true, }, + { + "module.boz.foo.bar", + "module.baz.foo.bar", + false, + }, { "a.b", "b.c", @@ -1253,11 +1263,21 @@ func TestResourceAddressLess(t *testing.T) { "a.c", true, }, + { + "c.b", + "b.c", + false, + }, { "a.b[9]", "a.b[10]", true, }, + { + "b.b[9]", + "a.b[10]", + false, + }, { "a.b", "a.b.deposed", diff --git a/terraform/resource_test.go b/terraform/resource_test.go index 750e7060d18c..31d511e5c5cd 100644 --- a/terraform/resource_test.go +++ b/terraform/resource_test.go @@ -3,6 +3,7 @@ package terraform import ( "fmt" "reflect" + "strconv" "testing" "github.com/hashicorp/hil" @@ -46,6 +47,63 @@ func TestInstanceInfo(t *testing.T) { } } +func TestInstanceInfoResourceAddress(t *testing.T) { + tests := []struct { + Input *InstanceInfo + Want string + }{ + { + &InstanceInfo{ + Id: "test_resource.baz", + }, + "test_resource.baz", + }, + { + &InstanceInfo{ + Id: "test_resource.baz", + ModulePath: rootModulePath, + }, + "test_resource.baz", + }, + { + &InstanceInfo{ + Id: "test_resource.baz", + ModulePath: []string{"root", "foo"}, + }, + "module.foo.test_resource.baz", + }, + { + &InstanceInfo{ + Id: "test_resource.baz", + ModulePath: []string{"root", "foo", "bar"}, + }, + "module.foo.module.bar.test_resource.baz", + }, + { + &InstanceInfo{ + Id: "test_resource.baz (tainted)", + }, + "test_resource.baz.tainted", + }, + { + &InstanceInfo{ + Id: "test_resource.baz (deposed #0)", + }, + "test_resource.baz.deposed", + }, + } + + for i, test := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + gotAddr := test.Input.ResourceAddress() + got := gotAddr.String() + if got != test.Want { + t.Fatalf("wrong result\ngot: %s\nwant: %s", got, test.Want) + } + }) + } +} + func TestResourceConfigGet(t *testing.T) { cases := []struct { Config map[string]interface{}