From c9d62bb2f67279754718c564c035a706aa916129 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Fri, 16 Aug 2019 08:31:21 -0400 Subject: [PATCH] command: discard output from flags package and return errs directly (#22373) Any command using meta.defaultFlagSet *might* occasionally exit before the flag package's output got written. This caused flag error messages to get lost. This PR discards the flag package output in favor of directly returning the error to the end user. --- command/console.go | 2 ++ command/fmt.go | 1 + command/get.go | 2 ++ command/graph.go | 1 + command/meta.go | 23 +---------------------- command/output.go | 1 + command/providers.go | 1 + command/providers_schema.go | 1 + command/show.go | 1 + command/state_list.go | 1 + command/state_mv.go | 3 ++- command/state_pull.go | 4 ++-- command/state_push.go | 3 ++- command/state_rm.go | 3 ++- command/state_show.go | 3 ++- command/taint.go | 1 + command/unlock.go | 1 + command/untaint.go | 1 + command/validate.go | 1 + command/workspace_delete.go | 1 + command/workspace_list.go | 2 ++ command/workspace_new.go | 1 + command/workspace_select.go | 1 + command/workspace_show.go | 2 ++ 24 files changed, 33 insertions(+), 28 deletions(-) diff --git a/command/console.go b/command/console.go index cf309621f9..7d366fb2de 100644 --- a/command/console.go +++ b/command/console.go @@ -2,6 +2,7 @@ package command import ( "bufio" + "fmt" "strings" "github.com/hashicorp/terraform/addrs" @@ -29,6 +30,7 @@ func (c *ConsoleCommand) Run(args []string) int { cmdFlags.StringVar(&c.Meta.statePath, "state", DefaultStateFilename, "path") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command line flags: %s\n", err.Error())) return 1 } diff --git a/command/fmt.go b/command/fmt.go index df51e88871..5712c22868 100644 --- a/command/fmt.go +++ b/command/fmt.go @@ -54,6 +54,7 @@ func (c *FmtCommand) Run(args []string) int { cmdFlags.BoolVar(&c.recursive, "recursive", false, "recursive") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/get.go b/command/get.go index 02269852ba..4d2cee41d8 100644 --- a/command/get.go +++ b/command/get.go @@ -1,6 +1,7 @@ package command import ( + "fmt" "strings" "github.com/hashicorp/terraform/tfdiags" @@ -24,6 +25,7 @@ func (c *GetCommand) Run(args []string) int { cmdFlags.BoolVar(&update, "update", false, "update") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/graph.go b/command/graph.go index f047e14d14..113a415271 100644 --- a/command/graph.go +++ b/command/graph.go @@ -36,6 +36,7 @@ func (c *GraphCommand) Run(args []string) int { cmdFlags.BoolVar(&verbose, "verbose", false, "verbose") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/meta.go b/command/meta.go index d56fb9da84..e95ea67ae9 100644 --- a/command/meta.go +++ b/command/meta.go @@ -1,13 +1,11 @@ package command import ( - "bufio" "bytes" "context" "errors" "flag" "fmt" - "io" "io/ioutil" "log" "os" @@ -353,26 +351,7 @@ func (m *Meta) contextOpts() *terraform.ContextOpts { // defaultFlagSet creates a default flag set for commands. func (m *Meta) defaultFlagSet(n string) *flag.FlagSet { f := flag.NewFlagSet(n, flag.ContinueOnError) - - // Create an io.Writer that writes to our Ui properly for errors. - // This is kind of a hack, but it does the job. Basically: create - // a pipe, use a scanner to break it into lines, and output each line - // to the UI. Do this forever. - errR, errW := io.Pipe() - errScanner := bufio.NewScanner(errR) - go func() { - // This only needs to be alive long enough to write the help info if - // there is a flag error. Kill the scanner after a short duriation to - // prevent these from accumulating during tests, and cluttering up the - // stack traces. - time.AfterFunc(2*time.Second, func() { - errW.Close() - }) - for errScanner.Scan() { - m.Ui.Error(errScanner.Text()) - } - }() - f.SetOutput(errW) + f.SetOutput(ioutil.Discard) // Set the default Usage to empty f.Usage = func() {} diff --git a/command/output.go b/command/output.go index ecc09b99d4..c2c0fd672d 100644 --- a/command/output.go +++ b/command/output.go @@ -36,6 +36,7 @@ func (c *OutputCommand) Run(args []string) int { cmdFlags.StringVar(&module, "module", "", "module") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/providers.go b/command/providers.go index 6730719fa3..368fd0e6b3 100644 --- a/command/providers.go +++ b/command/providers.go @@ -35,6 +35,7 @@ func (c *ProvidersCommand) Run(args []string) int { cmdFlags := c.Meta.defaultFlagSet("providers") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/providers_schema.go b/command/providers_schema.go index 73d540e599..f26a1c96fd 100644 --- a/command/providers_schema.go +++ b/command/providers_schema.go @@ -35,6 +35,7 @@ func (c *ProvidersSchemaCommand) Run(args []string) int { cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/show.go b/command/show.go index d72f97782f..bbee8d660b 100644 --- a/command/show.go +++ b/command/show.go @@ -33,6 +33,7 @@ func (c *ShowCommand) Run(args []string) int { cmdFlags.BoolVar(&jsonOutput, "json", false, "produce JSON output") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/state_list.go b/command/state_list.go index adaff6fc19..5d446dcdd0 100644 --- a/command/state_list.go +++ b/command/state_list.go @@ -28,6 +28,7 @@ func (c *StateListCommand) Run(args []string) int { cmdFlags.StringVar(&statePath, "state", "", "path") lookupId := cmdFlags.String("id", "", "Restrict output to paths with a resource having the specified ID.") if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return cli.RunResultHelp } args = cmdFlags.Args() diff --git a/command/state_mv.go b/command/state_mv.go index db76b30d1b..00b48306f0 100644 --- a/command/state_mv.go +++ b/command/state_mv.go @@ -36,7 +36,8 @@ func (c *StateMvCommand) Run(args []string) int { cmdFlags.StringVar(&c.statePath, "state", "", "path") cmdFlags.StringVar(&statePathOut, "state-out", "", "path") if err := cmdFlags.Parse(args); err != nil { - return cli.RunResultHelp + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) + return 1 } args = cmdFlags.Args() if len(args) != 2 { diff --git a/command/state_pull.go b/command/state_pull.go index fbd546b45e..f253f17c55 100644 --- a/command/state_pull.go +++ b/command/state_pull.go @@ -7,7 +7,6 @@ import ( "github.com/hashicorp/terraform/states/statefile" "github.com/hashicorp/terraform/states/statemgr" - "github.com/mitchellh/cli" ) // StatePullCommand is a Command implementation that shows a single resource. @@ -24,7 +23,8 @@ func (c *StatePullCommand) Run(args []string) int { cmdFlags := c.Meta.defaultFlagSet("state pull") if err := cmdFlags.Parse(args); err != nil { - return cli.RunResultHelp + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) + return 1 } args = cmdFlags.Args() diff --git a/command/state_push.go b/command/state_push.go index 9a3cf6292b..640329a481 100644 --- a/command/state_push.go +++ b/command/state_push.go @@ -31,7 +31,8 @@ func (c *StatePushCommand) Run(args []string) int { cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state") cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") if err := cmdFlags.Parse(args); err != nil { - return cli.RunResultHelp + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) + return 1 } args = cmdFlags.Args() diff --git a/command/state_rm.go b/command/state_rm.go index fd93636446..c4326bcfed 100644 --- a/command/state_rm.go +++ b/command/state_rm.go @@ -30,7 +30,8 @@ func (c *StateRmCommand) Run(args []string) int { cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.StringVar(&c.statePath, "state", "", "path") if err := cmdFlags.Parse(args); err != nil { - return cli.RunResultHelp + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) + return 1 } args = cmdFlags.Args() diff --git a/command/state_show.go b/command/state_show.go index e3aaf48668..bd9d2c48ba 100644 --- a/command/state_show.go +++ b/command/state_show.go @@ -27,7 +27,8 @@ func (c *StateShowCommand) Run(args []string) int { cmdFlags := c.Meta.defaultFlagSet("state show") cmdFlags.StringVar(&c.Meta.statePath, "state", "", "path") if err := cmdFlags.Parse(args); err != nil { - return cli.RunResultHelp + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) + return 1 } args = cmdFlags.Args() if len(args) != 1 { diff --git a/command/taint.go b/command/taint.go index 80ecfb70d8..538e528d56 100644 --- a/command/taint.go +++ b/command/taint.go @@ -35,6 +35,7 @@ func (c *TaintCommand) Run(args []string) int { cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/unlock.go b/command/unlock.go index 8dc7cafcce..bef3e5daca 100644 --- a/command/unlock.go +++ b/command/unlock.go @@ -29,6 +29,7 @@ func (c *UnlockCommand) Run(args []string) int { cmdFlags.BoolVar(&force, "force", false, "force") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/untaint.go b/command/untaint.go index a23f3c3ffd..d4adea5357 100644 --- a/command/untaint.go +++ b/command/untaint.go @@ -35,6 +35,7 @@ func (c *UntaintCommand) Run(args []string) int { cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/validate.go b/command/validate.go index d348800361..c190f1c066 100644 --- a/command/validate.go +++ b/command/validate.go @@ -38,6 +38,7 @@ func (c *ValidateCommand) Run(args []string) int { cmdFlags.Var(varFiles, "var-file", "variable file") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/workspace_delete.go b/command/workspace_delete.go index e363b5e6ad..a8639b3aed 100644 --- a/command/workspace_delete.go +++ b/command/workspace_delete.go @@ -34,6 +34,7 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int { cmdFlags.DurationVar(&stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/workspace_list.go b/command/workspace_list.go index 93eeb6c13b..ae13facf21 100644 --- a/command/workspace_list.go +++ b/command/workspace_list.go @@ -2,6 +2,7 @@ package command import ( "bytes" + "fmt" "strings" "github.com/hashicorp/terraform/tfdiags" @@ -24,6 +25,7 @@ func (c *WorkspaceListCommand) Run(args []string) int { cmdFlags := c.Meta.defaultFlagSet("workspace list") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/workspace_new.go b/command/workspace_new.go index e26984d8e9..0c477467c2 100644 --- a/command/workspace_new.go +++ b/command/workspace_new.go @@ -36,6 +36,7 @@ func (c *WorkspaceNewCommand) Run(args []string) int { cmdFlags.StringVar(&statePath, "state", "", "terraform state file") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/workspace_select.go b/command/workspace_select.go index 28b1755db7..8e64102157 100644 --- a/command/workspace_select.go +++ b/command/workspace_select.go @@ -25,6 +25,7 @@ func (c *WorkspaceSelectCommand) Run(args []string) int { cmdFlags := c.Meta.defaultFlagSet("workspace select") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 } diff --git a/command/workspace_show.go b/command/workspace_show.go index cccc4f5869..193a0826a7 100644 --- a/command/workspace_show.go +++ b/command/workspace_show.go @@ -1,6 +1,7 @@ package command import ( + "fmt" "strings" "github.com/posener/complete" @@ -19,6 +20,7 @@ func (c *WorkspaceShowCommand) Run(args []string) int { cmdFlags := c.Meta.extendedFlagSet("workspace show") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { + c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) return 1 }