command: New -compact-warnings option

When warnings appear in isolation (not accompanied by an error) it's
reasonable to want to defer resolving them for a while because they are
not actually blocking immediate work.

However, our warning messages tend to be long by default in order to
include all of the necessary context to understand the implications of
the warning, and that can make them overwhelming when combined with other
output.

As a compromise, this adds a new CLI option -compact-warnings which is
supported for all the main operation commands and which uses a more
compact format to print out warnings as long as they aren't also
accompanied by errors.

The default remains unchanged except that the threshold for consolidating
warning messages is reduced to one so that we'll now only show one of
each distinct warning summary.

Full warning messages are always shown if there's at least one error
included in the diagnostic set too, because in that case the warning
message could contain additional context to help understand the error.
This commit is contained in:
Martin Atkins 2019-12-10 11:06:06 -08:00
parent 5421a62eae
commit c06675c616
11 changed files with 198 additions and 10 deletions

View File

@ -248,11 +248,15 @@ Usage: terraform apply [options] [DIR-OR-PLAN]
Options: Options:
-auto-approve Skip interactive approval of plan before applying.
-backup=path Path to backup the existing state file before -backup=path Path to backup the existing state file before
modifying. Defaults to the "-state-out" path with modifying. Defaults to the "-state-out" path with
".backup" extension. Set to "-" to disable backup. ".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. -lock=true Lock the state file when locking is supported.

View File

@ -177,6 +177,51 @@ func Diagnostic(diag tfdiags.Diagnostic, sources map[string][]byte, color *color
return buf.String() 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) { func parseRange(src []byte, rng hcl.Range) (*hcl.File, int) {
filename := rng.Filename filename := rng.Filename
offset := rng.Start.Byte offset := rng.Start.Byte

View File

@ -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),
)
}
}

View File

@ -157,6 +157,9 @@ type Meta struct {
// init. // init.
// //
// reconfigure forces init to ignore any stored configuration. // 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 statePath string
stateOutPath string stateOutPath string
backupPath string backupPath string
@ -166,6 +169,7 @@ type Meta struct {
stateLockTimeout time.Duration stateLockTimeout time.Duration
forceInitCopy bool forceInitCopy bool
reconfigure bool reconfigure bool
compactWarnings bool
// Used with the import command to allow import of state when no matching config exists. // Used with the import command to allow import of state when no matching config exists.
allowMissingConfig bool allowMissingConfig bool
@ -377,6 +381,7 @@ func (m *Meta) extendedFlagSet(n string) *flag.FlagSet {
f.BoolVar(&m.input, "input", true, "input") f.BoolVar(&m.input, "input", true, "input")
f.Var((*FlagTargetSlice)(&m.targets), "target", "resource to target") f.Var((*FlagTargetSlice)(&m.targets), "target", "resource to target")
f.BoolVar(&m.compactWarnings, "compact-warnings", false, "use compact warnings")
if m.variableArgs.items == nil { if m.variableArgs.items == nil {
m.variableArgs = newRawFlags("-var") m.variableArgs = newRawFlags("-var")
@ -480,8 +485,33 @@ func (m *Meta) showDiagnostics(vals ...interface{}) {
diags = diags.Append(vals...) diags = diags.Append(vals...)
diags.Sort() diags.Sort()
if len(diags) == 0 {
return
}
diags = diags.ConsolidateWarnings(1)
// Since warning messages are generally competing // 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 { for _, diag := range diags {
// TODO: Actually measure the terminal width and pass it here. // TODO: Actually measure the terminal width and pass it here.

View File

@ -201,6 +201,10 @@ Usage: terraform plan [options] [DIR]
Options: 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 -destroy If set, a plan will be generated to destroy all resources
managed by the given configuration and state. managed by the given configuration and state.

View File

@ -124,6 +124,10 @@ Options:
modifying. Defaults to the "-state-out" path with modifying. Defaults to the "-state-out" path with
".backup" extension. Set to "-" to disable backup. ".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. -input=true Ask for input for variables if not directly set.
-lock=true Lock the state file when locking is supported. -lock=true Lock the state file when locking is supported.

View File

@ -15,12 +15,9 @@ import "fmt"
// The returned slice always has a separate backing array from the reciever, // The returned slice always has a separate backing array from the reciever,
// but some diagnostic values themselves might be shared. // but some diagnostic values themselves might be shared.
// //
// The definition of "unreasonable" may change in future releases. // The definition of "unreasonable" is given as the threshold argument. At most
func (diags Diagnostics) ConsolidateWarnings() Diagnostics { // that many warnings with the same summary will be shown.
// We'll start grouping when there are more than this number of warnings func (diags Diagnostics) ConsolidateWarnings(threshold int) Diagnostics {
// with the same summary.
const unreasonableThreshold = 2
if len(diags) == 0 { if len(diags) == 0 {
return nil return nil
} }
@ -55,7 +52,7 @@ func (diags Diagnostics) ConsolidateWarnings() Diagnostics {
} }
warningStats[summary]++ warningStats[summary]++
if warningStats[summary] == unreasonableThreshold { if warningStats[summary] == threshold {
// Initially creating the group doesn't really change anything // Initially creating the group doesn't really change anything
// visibly in the result, since a group with only one warning // visibly in the result, since a group with only one warning
// is just a passthrough anyway, but once we do this any additional // 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) 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
}

View File

@ -53,7 +53,7 @@ func TestConsolidateWarnings(t *testing.T) {
// We're using ForRPC here to force the diagnostics to be of a consistent // We're using ForRPC here to force the diagnostics to be of a consistent
// type that we can easily assert against below. // type that we can easily assert against below.
got := diags.ConsolidateWarnings().ForRPC() got := diags.ConsolidateWarnings(2).ForRPC()
want := Diagnostics{ want := Diagnostics{
// First set // First set
&rpcFriendlyDiag{ &rpcFriendlyDiag{

View File

@ -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 * `-backup=path` - Path to the backup file. Defaults to `-state-out` with
the ".backup" extension. Disabled by setting to "-". 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=true` - Lock the state file when locking is supported.
* `-lock-timeout=0s` - Duration to retry a state lock. * `-lock-timeout=0s` - Duration to retry a state lock.

View File

@ -37,6 +37,10 @@ inspect a planfile.
The command-line flags are all optional. The list of available flags are: 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. * `-destroy` - If set, generates a plan to destroy all the known resources.
* `-detailed-exitcode` - Return a detailed exit code when the command exits. * `-detailed-exitcode` - Return a detailed exit code when the command exits.

View File

@ -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 * `-backup=path` - Path to the backup file. Defaults to `-state-out` with
the ".backup" extension. Disabled by setting to "-". 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. * `-input=true` - Ask for input for variables if not directly set.
* `-lock=true` - Lock the state file when locking is supported. * `-lock=true` - Lock the state file when locking is supported.