diff --git a/command/apply.go b/command/apply.go index 28ddec774..76e500bf3 100644 --- a/command/apply.go +++ b/command/apply.go @@ -248,11 +248,15 @@ Usage: terraform apply [options] [DIR-OR-PLAN] Options: + -auto-approve Skip interactive approval of plan before applying. + -backup=path Path to backup the existing state file before modifying. Defaults to the "-state-out" path with ".backup" extension. Set to "-" to disable backup. - -auto-approve Skip interactive approval of plan before applying. + -compact-warnings If Terraform produces any warnings that are not + accompanied by errors, show them in a more compact + form that includes only the summary messages. -lock=true Lock the state file when locking is supported. diff --git a/command/format/diagnostic.go b/command/format/diagnostic.go index ed34ddbb9..6b8232650 100644 --- a/command/format/diagnostic.go +++ b/command/format/diagnostic.go @@ -177,6 +177,51 @@ func Diagnostic(diag tfdiags.Diagnostic, sources map[string][]byte, color *color return buf.String() } +// DiagnosticWarningsCompact is an alternative to Diagnostic for when all of +// the given diagnostics are warnings and we want to show them compactly, +// with only two lines per warning and excluding all of the detail information. +// +// The caller may optionally pre-process the given diagnostics with +// ConsolidateWarnings, in which case this function will recognize consolidated +// messages and include an indication that they are consolidated. +// +// Do not pass non-warning diagnostics to this function, or the result will +// be nonsense. +func DiagnosticWarningsCompact(diags tfdiags.Diagnostics, color *colorstring.Colorize) string { + var b strings.Builder + b.WriteString(color.Color("[bold][yellow]Warnings:[reset]\n\n")) + for _, diag := range diags { + sources := tfdiags.WarningGroupSourceRanges(diag) + b.WriteString(fmt.Sprintf("- %s\n", diag.Description().Summary)) + if len(sources) > 0 { + mainSource := sources[0] + if mainSource.Subject != nil { + if len(sources) > 1 { + b.WriteString(fmt.Sprintf( + " on %s line %d (and %d more)\n", + mainSource.Subject.Filename, + mainSource.Subject.Start.Line, + len(sources)-1, + )) + } else { + b.WriteString(fmt.Sprintf( + " on %s line %d\n", + mainSource.Subject.Filename, + mainSource.Subject.Start.Line, + )) + } + } else if len(sources) > 1 { + b.WriteString(fmt.Sprintf( + " (%d occurences of this warning)\n", + len(sources), + )) + } + } + } + + return b.String() +} + func parseRange(src []byte, rng hcl.Range) (*hcl.File, int) { filename := rng.Filename offset := rng.Start.Byte diff --git a/command/format/diagnostic_test.go b/command/format/diagnostic_test.go new file mode 100644 index 000000000..18812089c --- /dev/null +++ b/command/format/diagnostic_test.go @@ -0,0 +1,73 @@ +package format + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcl/v2" + "github.com/mitchellh/colorstring" + + "github.com/hashicorp/terraform/tfdiags" +) + +func TestDiagnosticWarningsCompact(t *testing.T) { + var diags tfdiags.Diagnostics + diags = diags.Append(tfdiags.SimpleWarning("foo")) + diags = diags.Append(tfdiags.SimpleWarning("foo")) + diags = diags.Append(tfdiags.SimpleWarning("bar")) + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "source foo", + Detail: "...", + Subject: &hcl.Range{ + Filename: "source.tf", + Start: hcl.Pos{Line: 2, Column: 1, Byte: 5}, + End: hcl.Pos{Line: 2, Column: 1, Byte: 5}, + }, + }) + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "source foo", + Detail: "...", + Subject: &hcl.Range{ + Filename: "source.tf", + Start: hcl.Pos{Line: 3, Column: 1, Byte: 7}, + End: hcl.Pos{Line: 3, Column: 1, Byte: 7}, + }, + }) + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "source bar", + Detail: "...", + Subject: &hcl.Range{ + Filename: "source2.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 1}, + End: hcl.Pos{Line: 1, Column: 1, Byte: 1}, + }, + }) + + // ConsolidateWarnings groups together the ones + // that have source location information and that + // have the same summary text. + diags = diags.ConsolidateWarnings(1) + + // A zero-value Colorize just passes all the formatting + // codes back to us, so we can test them literally. + got := DiagnosticWarningsCompact(diags, &colorstring.Colorize{}) + want := `[bold][yellow]Warnings:[reset] + +- foo +- foo +- bar +- source foo + on source.tf line 2 (and 1 more) +- source bar + on source2.tf line 1 +` + if got != want { + t.Errorf( + "wrong result\ngot:\n%s\n\nwant:\n%s\n\ndiff:\n%s", + got, want, cmp.Diff(want, got), + ) + } +} diff --git a/command/meta.go b/command/meta.go index 9f553da62..05009c999 100644 --- a/command/meta.go +++ b/command/meta.go @@ -157,6 +157,9 @@ type Meta struct { // init. // // reconfigure forces init to ignore any stored configuration. + // + // compactWarnings (-compact-warnings) selects a more compact presentation + // of warnings in the output when they are not accompanied by errors. statePath string stateOutPath string backupPath string @@ -166,6 +169,7 @@ type Meta struct { stateLockTimeout time.Duration forceInitCopy bool reconfigure bool + compactWarnings bool // Used with the import command to allow import of state when no matching config exists. allowMissingConfig bool @@ -377,6 +381,7 @@ func (m *Meta) extendedFlagSet(n string) *flag.FlagSet { f.BoolVar(&m.input, "input", true, "input") f.Var((*FlagTargetSlice)(&m.targets), "target", "resource to target") + f.BoolVar(&m.compactWarnings, "compact-warnings", false, "use compact warnings") if m.variableArgs.items == nil { m.variableArgs = newRawFlags("-var") @@ -480,8 +485,33 @@ func (m *Meta) showDiagnostics(vals ...interface{}) { diags = diags.Append(vals...) diags.Sort() + if len(diags) == 0 { + return + } + + diags = diags.ConsolidateWarnings(1) + // Since warning messages are generally competing - diags = diags.ConsolidateWarnings() + if m.compactWarnings { + // If the user selected compact warnings and all of the diagnostics are + // warnings then we'll use a more compact representation of the warnings + // that only includes their summaries. + // We show full warnings if there are also errors, because a warning + // can sometimes serve as good context for a subsequent error. + useCompact := true + for _, diag := range diags { + if diag.Severity() != tfdiags.Warning { + useCompact = false + break + } + } + if useCompact { + msg := format.DiagnosticWarningsCompact(diags, m.Colorize()) + msg = "\n" + msg + "\nTo see the full warning notes, run Terraform without -compact-warnings.\n" + m.Ui.Warn(msg) + return + } + } for _, diag := range diags { // TODO: Actually measure the terminal width and pass it here. diff --git a/command/plan.go b/command/plan.go index aad035705..e3e181566 100644 --- a/command/plan.go +++ b/command/plan.go @@ -201,6 +201,10 @@ Usage: terraform plan [options] [DIR] Options: + -compact-warnings If Terraform produces any warnings that are not + accompanied by errors, show them in a more compact form + that includes only the summary messages. + -destroy If set, a plan will be generated to destroy all resources managed by the given configuration and state. diff --git a/command/refresh.go b/command/refresh.go index db7ca9c43..dbb4ab61e 100644 --- a/command/refresh.go +++ b/command/refresh.go @@ -124,6 +124,10 @@ Options: modifying. Defaults to the "-state-out" path with ".backup" extension. Set to "-" to disable backup. + -compact-warnings If Terraform produces any warnings that are not + accompanied by errors, show them in a more compact form + that includes only the summary messages. + -input=true Ask for input for variables if not directly set. -lock=true Lock the state file when locking is supported. diff --git a/tfdiags/consolidate_warnings.go b/tfdiags/consolidate_warnings.go index 3e0983ee4..06f3d52cc 100644 --- a/tfdiags/consolidate_warnings.go +++ b/tfdiags/consolidate_warnings.go @@ -15,12 +15,9 @@ import "fmt" // The returned slice always has a separate backing array from the reciever, // but some diagnostic values themselves might be shared. // -// The definition of "unreasonable" may change in future releases. -func (diags Diagnostics) ConsolidateWarnings() Diagnostics { - // We'll start grouping when there are more than this number of warnings - // with the same summary. - const unreasonableThreshold = 2 - +// The definition of "unreasonable" is given as the threshold argument. At most +// that many warnings with the same summary will be shown. +func (diags Diagnostics) ConsolidateWarnings(threshold int) Diagnostics { if len(diags) == 0 { return nil } @@ -55,7 +52,7 @@ func (diags Diagnostics) ConsolidateWarnings() Diagnostics { } warningStats[summary]++ - if warningStats[summary] == unreasonableThreshold { + if warningStats[summary] == threshold { // Initially creating the group doesn't really change anything // visibly in the result, since a group with only one warning // is just a passthrough anyway, but once we do this any additional @@ -128,3 +125,22 @@ func (wg *warningGroup) Append(diag Diagnostic) { } wg.Warnings = append(wg.Warnings, diag) } + +// WarningGroupSourceRanges can be used in conjunction with +// Diagnostics.ConsolidateWarnings to recover the full set of original source +// locations from a consolidated warning. +// +// For convenience, this function accepts any diagnostic and will just return +// the single Source value from any diagnostic that isn't a warning group. +func WarningGroupSourceRanges(diag Diagnostic) []Source { + wg, ok := diag.(*warningGroup) + if !ok { + return []Source{diag.Source()} + } + + ret := make([]Source, len(wg.Warnings)) + for i, wrappedDiag := range wg.Warnings { + ret[i] = wrappedDiag.Source() + } + return ret +} diff --git a/tfdiags/consolidate_warnings_test.go b/tfdiags/consolidate_warnings_test.go index 6eb7eabc3..df94d4af8 100644 --- a/tfdiags/consolidate_warnings_test.go +++ b/tfdiags/consolidate_warnings_test.go @@ -53,7 +53,7 @@ func TestConsolidateWarnings(t *testing.T) { // We're using ForRPC here to force the diagnostics to be of a consistent // type that we can easily assert against below. - got := diags.ConsolidateWarnings().ForRPC() + got := diags.ConsolidateWarnings(2).ForRPC() want := Diagnostics{ // First set &rpcFriendlyDiag{ diff --git a/website/docs/commands/apply.html.markdown b/website/docs/commands/apply.html.markdown index 11c372469..239e16cfb 100644 --- a/website/docs/commands/apply.html.markdown +++ b/website/docs/commands/apply.html.markdown @@ -27,6 +27,10 @@ The command-line flags are all optional. The list of available flags are: * `-backup=path` - Path to the backup file. Defaults to `-state-out` with the ".backup" extension. Disabled by setting to "-". +* `-compact-warnings` - If Terraform produces any warnings that are not + accompanied by errors, show them in a more compact form that includes only + the summary messages. + * `-lock=true` - Lock the state file when locking is supported. * `-lock-timeout=0s` - Duration to retry a state lock. diff --git a/website/docs/commands/plan.html.markdown b/website/docs/commands/plan.html.markdown index 838daf7a2..815e237ea 100644 --- a/website/docs/commands/plan.html.markdown +++ b/website/docs/commands/plan.html.markdown @@ -37,6 +37,10 @@ inspect a planfile. The command-line flags are all optional. The list of available flags are: +* `-compact-warnings` - If Terraform produces any warnings that are not + accompanied by errors, show them in a more compact form that includes only + the summary messages. + * `-destroy` - If set, generates a plan to destroy all the known resources. * `-detailed-exitcode` - Return a detailed exit code when the command exits. diff --git a/website/docs/commands/refresh.html.markdown b/website/docs/commands/refresh.html.markdown index c03e68911..4700bf3e9 100644 --- a/website/docs/commands/refresh.html.markdown +++ b/website/docs/commands/refresh.html.markdown @@ -29,6 +29,10 @@ The command-line flags are all optional. The list of available flags are: * `-backup=path` - Path to the backup file. Defaults to `-state-out` with the ".backup" extension. Disabled by setting to "-". +* `-compact-warnings` - If Terraform produces any warnings that are not + accompanied by errors, show them in a more compact form that includes only + the summary messages. + * `-input=true` - Ask for input for variables if not directly set. * `-lock=true` - Lock the state file when locking is supported.