From c5a6aa31d384649d67010ef6eae77a8bf1058f83 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 27 Jan 2021 15:51:40 -0500 Subject: [PATCH 1/2] cli: Add initial command views abstraction Terraform supports multiple output formats for several sub-commands. The default format is user-readable text, but many sub-commands support a `-json` flag to output a machine-readable format for the result. The output command also supports a `-raw` flag for a simpler, scripting- focused machine readable format. This commit adds a "views" abstraction, intended to help ensure consistency between the various output formats. This extracts the render specific code from the command package, and moves it into a views package. Each command is expected to create an interface for its view, and one or more implementations of that interface. By doing so, we separate the concerns of generating the sub-command result from rendering the result in the specified output format. This should make it easier to ensure that all output formats will be updated together when changes occur in the result-generating phase. There are some other consequences of this restructuring: - Views now directly access the terminal streams, rather than the now-redundant cli.Ui instance; - With the reorganization of commands, parsing CLI arguments is now the responsibility of a separate "arguments" package. For now, views are added only for the output sub-command, as an example. Because this command uses code which is shared with the apply and refresh commands, those are also partially updated. --- command/apply.go | 54 +----- command/apply_destroy_test.go | 16 ++ command/apply_test.go | 78 ++++++++- command/arguments/default.go | 16 ++ command/arguments/output.go | 88 ++++++++++ command/arguments/output_test.go | 142 ++++++++++++++++ command/arguments/types.go | 13 ++ command/command_test.go | 7 + command/meta.go | 12 +- command/meta_config.go | 3 + command/output.go | 228 ++++--------------------- command/output_test.go | 225 +++++++++---------------- command/refresh.go | 11 +- command/refresh_test.go | 34 +++- command/views/output.go | 278 +++++++++++++++++++++++++++++++ command/views/output_test.go | 60 +++++++ command/views/view.go | 122 ++++++++++++++ commands.go | 2 + 18 files changed, 998 insertions(+), 391 deletions(-) create mode 100644 command/arguments/default.go create mode 100644 command/arguments/output.go create mode 100644 command/arguments/output_test.go create mode 100644 command/arguments/types.go create mode 100644 command/views/output.go create mode 100644 command/views/output_test.go create mode 100644 command/views/view.go diff --git a/command/apply.go b/command/apply.go index d84745f19..0b4df0af6 100644 --- a/command/apply.go +++ b/command/apply.go @@ -1,15 +1,13 @@ package command import ( - "bytes" "fmt" - "sort" "strings" "github.com/hashicorp/terraform/backend" + "github.com/hashicorp/terraform/command/arguments" + "github.com/hashicorp/terraform/command/views" "github.com/hashicorp/terraform/plans/planfile" - "github.com/hashicorp/terraform/repl" - "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" ) @@ -230,9 +228,12 @@ func (c *ApplyCommand) Run(args []string) int { c.Meta.stateOutPath))) } - if !c.Destroy { - if outputs := outputsAsString(op.State, true); outputs != "" { - c.Ui.Output(c.Colorize().Color(outputs)) + if !c.Destroy && op.State != nil { + outputValues := op.State.RootModule().OutputValues + if len(outputValues) > 0 { + c.Ui.Output(c.Colorize().Color("[reset][bold][green]\nOutputs:\n\n")) + view := views.NewOutput(arguments.ViewHuman, c.View) + view.Output("", outputValues) } } @@ -366,45 +367,6 @@ Options: return strings.TrimSpace(helpText) } -func outputsAsString(state *states.State, includeHeader bool) string { - if state == nil { - return "" - } - - ms := state.RootModule() - outputs := ms.OutputValues - outputBuf := new(bytes.Buffer) - if len(outputs) > 0 { - if includeHeader { - outputBuf.WriteString("[reset][bold][green]\nOutputs:\n\n") - } - - // Output the outputs in alphabetical order - keyLen := 0 - ks := make([]string, 0, len(outputs)) - for key := range outputs { - ks = append(ks, key) - if len(key) > keyLen { - keyLen = len(key) - } - } - sort.Strings(ks) - - for _, k := range ks { - v := outputs[k] - if v.Sensitive { - outputBuf.WriteString(fmt.Sprintf("%s = \n", k)) - continue - } - - result := repl.FormatValue(v.Value, 0) - outputBuf.WriteString(fmt.Sprintf("%s = %s\n", k, result)) - } - } - - return strings.TrimSpace(outputBuf.String()) -} - const outputInterrupt = `Interrupt received. Please wait for Terraform to exit or data loss may occur. Gracefully shutting down...` diff --git a/command/apply_destroy_test.go b/command/apply_destroy_test.go index ec3b2e9ea..e4f5d8429 100644 --- a/command/apply_destroy_test.go +++ b/command/apply_destroy_test.go @@ -58,11 +58,13 @@ func TestApply_destroy(t *testing.T) { } ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Destroy: true, Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -157,11 +159,13 @@ func TestApply_destroyApproveNo(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Destroy: true, Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -206,11 +210,13 @@ func TestApply_destroyApproveYes(t *testing.T) { defaultInputWriter = new(bytes.Buffer) ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Destroy: true, Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -271,11 +277,13 @@ func TestApply_destroyLockedState(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Destroy: true, Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -306,11 +314,13 @@ func TestApply_destroyPlan(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Destroy: true, Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -337,11 +347,13 @@ func TestApply_destroyPath(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Destroy: true, Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -430,11 +442,13 @@ func TestApply_destroyTargetedDependencies(t *testing.T) { } ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Destroy: true, Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -579,11 +593,13 @@ func TestApply_destroyTargeted(t *testing.T) { } ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Destroy: true, Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } diff --git a/command/apply_test.go b/command/apply_test.go index 8857c2aaf..2980bd61d 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -38,10 +38,12 @@ func TestApply(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -73,10 +75,12 @@ func TestApply_path(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -112,10 +116,12 @@ func TestApply_approveNo(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -154,10 +160,12 @@ func TestApply_approveYes(t *testing.T) { defaultInputWriter = new(bytes.Buffer) ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -196,10 +204,12 @@ func TestApply_lockedState(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -240,10 +250,12 @@ func TestApply_lockedStateWait(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -336,10 +348,12 @@ func TestApply_parallelism(t *testing.T) { } ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: testingOverrides, Ui: ui, + View: view, }, } @@ -364,10 +378,12 @@ func TestApply_configInvalid(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -401,10 +417,12 @@ func TestApply_defaultState(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -442,10 +460,12 @@ func TestApply_error(t *testing.T) { p := testProvider() ui := cli.NewMockUi() + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -534,10 +554,12 @@ func TestApply_input(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -579,10 +601,12 @@ func TestApply_inputPartial(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -616,10 +640,12 @@ func TestApply_noArgs(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -655,10 +681,12 @@ func TestApply_plan(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -687,10 +715,12 @@ func TestApply_plan_backup(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -719,10 +749,12 @@ func TestApply_plan_noBackup(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -797,10 +829,12 @@ func TestApply_plan_remoteState(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -844,10 +878,12 @@ func TestApply_planWithVarFile(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -875,10 +911,12 @@ func TestApply_planVars(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -943,10 +981,12 @@ func TestApply_refresh(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -995,10 +1035,12 @@ func TestApply_shutdown(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, ShutdownCh: shutdownCh, }, } @@ -1107,10 +1149,12 @@ func TestApply_state(t *testing.T) { } ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1170,10 +1214,12 @@ func TestApply_stateNoExist(t *testing.T) { p := applyFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1194,10 +1240,12 @@ func TestApply_sensitiveOutput(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, done := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1212,7 +1260,7 @@ func TestApply_sensitiveOutput(t *testing.T) { t.Fatalf("bad: \n%s", ui.OutputWriter.String()) } - output := ui.OutputWriter.String() + output := done(t).Stdout() if !strings.Contains(output, "notsensitive = \"Hello world\"") { t.Fatalf("bad: output should contain 'notsensitive' output\n%s", output) } @@ -1232,10 +1280,12 @@ func TestApply_vars(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1293,10 +1343,12 @@ func TestApply_varFile(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1354,10 +1406,12 @@ func TestApply_varFileDefault(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1414,10 +1468,12 @@ func TestApply_varFileDefaultJSON(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1493,10 +1549,12 @@ func TestApply_backup(t *testing.T) { } ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1552,10 +1610,12 @@ func TestApply_disableBackup(t *testing.T) { } ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1623,10 +1683,12 @@ func TestApply_terraformEnv(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1658,8 +1720,9 @@ func TestApply_terraformEnvNonDefault(t *testing.T) { // Create new env { ui := new(cli.MockUi) + view, _ := testView(t) newCmd := &WorkspaceNewCommand{} - newCmd.Meta = Meta{Ui: ui} + newCmd.Meta = Meta{Ui: ui, View: view} if code := newCmd.Run([]string{"test"}); code != 0 { t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter) } @@ -1669,8 +1732,9 @@ func TestApply_terraformEnvNonDefault(t *testing.T) { { args := []string{"test"} ui := new(cli.MockUi) + view, _ := testView(t) selCmd := &WorkspaceSelectCommand{} - selCmd.Meta = Meta{Ui: ui} + selCmd.Meta = Meta{Ui: ui, View: view} if code := selCmd.Run(args); code != 0 { t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter) } @@ -1678,10 +1742,12 @@ func TestApply_terraformEnvNonDefault(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1728,10 +1794,12 @@ func TestApply_targeted(t *testing.T) { } ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -1763,9 +1831,11 @@ func TestApply_targetFlagsDiags(t *testing.T) { defer testChdir(t, td)() ui := new(cli.MockUi) + view, _ := testView(t) c := &ApplyCommand{ Meta: Meta{ - Ui: ui, + Ui: ui, + View: view, }, } diff --git a/command/arguments/default.go b/command/arguments/default.go new file mode 100644 index 000000000..4b7bb4024 --- /dev/null +++ b/command/arguments/default.go @@ -0,0 +1,16 @@ +package arguments + +import ( + "flag" + "io/ioutil" +) + +// defaultFlagSet creates a FlagSet with the common settings to override +// the flag package's noisy defaults. +func defaultFlagSet(name string) *flag.FlagSet { + f := flag.NewFlagSet(name, flag.ContinueOnError) + f.SetOutput(ioutil.Discard) + f.Usage = func() {} + + return f +} diff --git a/command/arguments/output.go b/command/arguments/output.go new file mode 100644 index 000000000..f77c283cc --- /dev/null +++ b/command/arguments/output.go @@ -0,0 +1,88 @@ +package arguments + +import ( + "github.com/hashicorp/terraform/tfdiags" +) + +// Output represents the command-line arguments for the output command. +type Output struct { + // Name identifies which root module output to show. If empty, show all + // outputs. + Name string + + // StatePath is an optional path to a state file, from which outputs will + // be loaded. + StatePath string + + // ViewType specifies which output format to use: human, JSON, or "raw". + ViewType ViewType +} + +// ParseOutput processes CLI arguments, returning an Output value and errors. +// If errors are encountered, an Output value is still returned representing +// the best effort interpretation of the arguments. +func ParseOutput(args []string) (*Output, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + output := &Output{} + + var jsonOutput, rawOutput bool + var statePath string + cmdFlags := defaultFlagSet("output") + cmdFlags.BoolVar(&jsonOutput, "json", false, "json") + cmdFlags.BoolVar(&rawOutput, "raw", false, "raw") + cmdFlags.StringVar(&statePath, "state", "", "path") + + if err := cmdFlags.Parse(args); err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + err.Error(), + )) + } + + args = cmdFlags.Args() + if len(args) > 1 { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unexpected argument", + "The output command expects exactly one argument with the name of an output variable or no arguments to show all outputs.", + )) + } + + if jsonOutput && rawOutput { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid output format", + "The -raw and -json options are mutually-exclusive.", + )) + + // Since the desired output format is unknowable, fall back to default + jsonOutput = false + rawOutput = false + } + + output.StatePath = statePath + + if len(args) > 0 { + output.Name = args[0] + } + + if rawOutput && output.Name == "" { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Output name required", + "You must give the name of a single output value when using the -raw option.", + )) + } + + switch { + case jsonOutput: + output.ViewType = ViewJSON + case rawOutput: + output.ViewType = ViewRaw + default: + output.ViewType = ViewHuman + } + + return output, diags +} diff --git a/command/arguments/output_test.go b/command/arguments/output_test.go new file mode 100644 index 000000000..304a156bf --- /dev/null +++ b/command/arguments/output_test.go @@ -0,0 +1,142 @@ +package arguments + +import ( + "reflect" + "testing" + + "github.com/davecgh/go-spew/spew" + "github.com/hashicorp/terraform/tfdiags" +) + +func TestParseOutput_valid(t *testing.T) { + testCases := map[string]struct { + args []string + want *Output + }{ + "defaults": { + nil, + &Output{ + Name: "", + ViewType: ViewHuman, + StatePath: "", + }, + }, + "json": { + []string{"-json"}, + &Output{ + Name: "", + ViewType: ViewJSON, + StatePath: "", + }, + }, + "raw": { + []string{"-raw", "foo"}, + &Output{ + Name: "foo", + ViewType: ViewRaw, + StatePath: "", + }, + }, + "state": { + []string{"-state=foobar.tfstate", "-raw", "foo"}, + &Output{ + Name: "foo", + ViewType: ViewRaw, + StatePath: "foobar.tfstate", + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, diags := ParseOutput(tc.args) + if len(diags) > 0 { + t.Fatalf("unexpected diags: %v", diags) + } + if *got != *tc.want { + t.Fatalf("unexpected result\n got: %#v\nwant: %#v", got, tc.want) + } + }) + } +} + +func TestParseOutput_invalid(t *testing.T) { + testCases := map[string]struct { + args []string + want *Output + wantDiags tfdiags.Diagnostics + }{ + "unknown flag": { + []string{"-boop"}, + &Output{ + Name: "", + ViewType: ViewHuman, + StatePath: "", + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + "flag provided but not defined: -boop", + ), + }, + }, + "json and raw specified": { + []string{"-json", "-raw"}, + &Output{ + Name: "", + ViewType: ViewHuman, + StatePath: "", + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Invalid output format", + "The -raw and -json options are mutually-exclusive.", + ), + }, + }, + "raw with no name": { + []string{"-raw"}, + &Output{ + Name: "", + ViewType: ViewRaw, + StatePath: "", + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Output name required", + "You must give the name of a single output value when using the -raw option.", + ), + }, + }, + "too many arguments": { + []string{"-raw", "-state=foo.tfstate", "bar", "baz"}, + &Output{ + Name: "bar", + ViewType: ViewRaw, + StatePath: "foo.tfstate", + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Unexpected argument", + "The output command expects exactly one argument with the name of an output variable or no arguments to show all outputs.", + ), + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, gotDiags := ParseOutput(tc.args) + if *got != *tc.want { + t.Fatalf("unexpected result\n got: %#v\nwant: %#v", got, tc.want) + } + if !reflect.DeepEqual(gotDiags, tc.wantDiags) { + t.Errorf("wrong result\ngot: %s\nwant: %s", spew.Sdump(gotDiags), spew.Sdump(tc.wantDiags)) + } + }) + } +} diff --git a/command/arguments/types.go b/command/arguments/types.go new file mode 100644 index 000000000..0203a04eb --- /dev/null +++ b/command/arguments/types.go @@ -0,0 +1,13 @@ +package arguments + +// ViewType represents which view layer to use for a given command. Not all +// commands will support all view types, and validation that the type is +// supported should happen in the view constructor. +type ViewType rune + +const ( + ViewNone ViewType = 0 + ViewHuman ViewType = 'H' + ViewJSON ViewType = 'J' + ViewRaw ViewType = 'R' +) diff --git a/command/command_test.go b/command/command_test.go index b592e68c9..4454a8404 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -19,8 +19,10 @@ import ( svchost "github.com/hashicorp/terraform-svchost" "github.com/hashicorp/terraform-svchost/disco" + "github.com/hashicorp/terraform/command/views" "github.com/hashicorp/terraform/internal/getproviders" "github.com/hashicorp/terraform/internal/initwd" + "github.com/hashicorp/terraform/internal/terminal" "github.com/hashicorp/terraform/registry" "github.com/hashicorp/terraform/addrs" @@ -1028,3 +1030,8 @@ func fakeRegistryHandler(resp http.ResponseWriter, req *http.Request) { resp.Write([]byte(`provider not found`)) } } + +func testView(t *testing.T) (*views.View, func(*testing.T) *terminal.TestOutput) { + streams, done := terminal.StreamsForTesting(t) + return views.NewView(streams), done +} diff --git a/command/meta.go b/command/meta.go index 033c6f39b..92ad6b85a 100644 --- a/command/meta.go +++ b/command/meta.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/backend/local" "github.com/hashicorp/terraform/command/format" + "github.com/hashicorp/terraform/command/views" "github.com/hashicorp/terraform/command/webbrowser" "github.com/hashicorp/terraform/configs/configload" "github.com/hashicorp/terraform/internal/getproviders" @@ -62,6 +63,8 @@ type Meta struct { // do some default behavior instead if so, rather than panicking. Streams *terminal.Streams + View *views.View + Color bool // True if output should be colored GlobalPluginDirs []string // Additional paths to search for plugins Ui cli.Ui // Ui for output @@ -499,6 +502,7 @@ func (m *Meta) contextOpts() (*terraform.ContextOpts, error) { } // defaultFlagSet creates a default flag set for commands. +// See also command/arguments/default.go func (m *Meta) defaultFlagSet(n string) *flag.FlagSet { f := flag.NewFlagSet(n, flag.ContinueOnError) f.SetOutput(ioutil.Discard) @@ -581,9 +585,9 @@ func (m *Meta) parseTargetFlags() tfdiags.Diagnostics { return diags } -// process will process the meta-parameters out of the arguments. This +// process will process any -no-color entries out of the arguments. This // will potentially modify the args in-place. It will return the resulting -// slice. +// slice, and update the Meta and Ui. func (m *Meta) process(args []string) []string { // We do this so that we retain the ability to technically call // process multiple times, even if we have no plans to do so @@ -617,6 +621,10 @@ func (m *Meta) process(args []string) []string { }, } + if m.View != nil { + m.View.EnableColor(m.Color) + } + return args } diff --git a/command/meta_config.go b/command/meta_config.go index d5f65355e..51ca27447 100644 --- a/command/meta_config.go +++ b/command/meta_config.go @@ -329,6 +329,9 @@ func (m *Meta) initConfigLoader() (*configload.Loader, error) { return nil, err } m.configLoader = loader + if m.View != nil { + m.View.SetConfigSources(loader.Sources) + } } return m.configLoader, nil } diff --git a/command/output.go b/command/output.go index 8cc7426f6..4eaec982a 100644 --- a/command/output.go +++ b/command/output.go @@ -1,15 +1,11 @@ package command import ( - "encoding/json" "fmt" "strings" - "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/convert" - ctyjson "github.com/zclconf/go-cty/cty/json" - - "github.com/hashicorp/terraform/repl" + "github.com/hashicorp/terraform/command/arguments" + "github.com/hashicorp/terraform/command/views" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" ) @@ -18,64 +14,54 @@ import ( // from a Terraform state and prints it. type OutputCommand struct { Meta - - // Unit tests may set rawPrint to capture the output from the -raw - // option, which would normally go to stdout directly. - rawPrint func(string) } -func (c *OutputCommand) Run(args []string) int { - args = c.Meta.process(args) - var statePath string - var jsonOutput, rawOutput bool - cmdFlags := c.Meta.defaultFlagSet("output") - cmdFlags.BoolVar(&jsonOutput, "json", false, "json") - cmdFlags.BoolVar(&rawOutput, "raw", false, "raw") - cmdFlags.StringVar(&statePath, "state", "", "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())) +func (c *OutputCommand) Run(rawArgs []string) int { + rawArgs = c.Meta.process(rawArgs) + + // Parse and validate flags + args, diags := arguments.ParseOutput(rawArgs) + if diags.HasErrors() { + c.View.Diagnostics(diags) + c.View.HelpPrompt("output") return 1 } - args = cmdFlags.Args() - if len(args) > 1 { - c.Ui.Error( - "The output command expects exactly one argument with the name\n" + - "of an output variable or no arguments to show all outputs.\n") - cmdFlags.Usage() + view := views.NewOutput(args.ViewType, c.View) + + // Fetch data from state + outputs, diags := c.Outputs(args.StatePath) + if diags.HasErrors() { + view.Diagnostics(diags) return 1 } - if jsonOutput && rawOutput { - c.Ui.Error("The -raw and -json options are mutually-exclusive.\n") - cmdFlags.Usage() + // Render the view + viewDiags := view.Output(args.Name, outputs) + diags = diags.Append(viewDiags) + + view.Diagnostics(diags) + + if diags.HasErrors() { return 1 } - if rawOutput && len(args) == 0 { - c.Ui.Error("You must give the name of a single output value when using the -raw option.\n") - cmdFlags.Usage() - return 1 - } + return 0 +} - name := "" - if len(args) > 0 { - name = args[0] - } +func (c *OutputCommand) Outputs(statePath string) (map[string]*states.OutputValue, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + // Allow state path override if statePath != "" { c.Meta.statePath = statePath } - var diags tfdiags.Diagnostics - // Load the backend b, backendDiags := c.Backend(nil) diags = diags.Append(backendDiags) - if backendDiags.HasErrors() { - c.showDiagnostics(diags) - return 1 + if diags.HasErrors() { + return nil, diags } // This is a read-only command @@ -83,20 +69,20 @@ func (c *OutputCommand) Run(args []string) int { env, err := c.Workspace() if err != nil { - c.Ui.Error(fmt.Sprintf("Error selecting workspace: %s", err)) - return 1 + diags = diags.Append(fmt.Errorf("Error selecting workspace: %s", err)) + return nil, diags } // Get the state stateStore, err := b.StateMgr(env) if err != nil { - c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err)) - return 1 + diags = diags.Append(fmt.Errorf("Failed to load state: %s", err)) + return nil, diags } if err := stateStore.RefreshState(); err != nil { - c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err)) - return 1 + diags = diags.Append(fmt.Errorf("Failed to load state: %s", err)) + return nil, diags } state := stateStore.State() @@ -104,147 +90,7 @@ func (c *OutputCommand) Run(args []string) int { state = states.NewState() } - mod := state.RootModule() - - if !jsonOutput && (state.Empty() || len(mod.OutputValues) == 0) { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Warning, - "No outputs found", - "The state file either has no outputs defined, or all the defined "+ - "outputs are empty. Please define an output in your configuration "+ - "with the `output` keyword and run `terraform refresh` for it to "+ - "become available. If you are using interpolation, please verify "+ - "the interpolated value is not empty. You can use the "+ - "`terraform console` command to assist.", - )) - c.showDiagnostics(diags) - return 0 - } - - if name == "" { - if jsonOutput { - // Due to a historical accident, the switch from state version 2 to - // 3 caused our JSON output here to be the full metadata about the - // outputs rather than just the output values themselves as we'd - // show in the single value case. We must now maintain that behavior - // for compatibility, so this is an emulation of the JSON - // serialization of outputs used in state format version 3. - type OutputMeta struct { - Sensitive bool `json:"sensitive"` - Type json.RawMessage `json:"type"` - Value json.RawMessage `json:"value"` - } - outputs := map[string]OutputMeta{} - - for n, os := range mod.OutputValues { - jsonVal, err := ctyjson.Marshal(os.Value, os.Value.Type()) - if err != nil { - diags = diags.Append(err) - c.showDiagnostics(diags) - return 1 - } - jsonType, err := ctyjson.MarshalType(os.Value.Type()) - if err != nil { - diags = diags.Append(err) - c.showDiagnostics(diags) - return 1 - } - outputs[n] = OutputMeta{ - Sensitive: os.Sensitive, - Type: json.RawMessage(jsonType), - Value: json.RawMessage(jsonVal), - } - } - - jsonOutputs, err := json.MarshalIndent(outputs, "", " ") - if err != nil { - diags = diags.Append(err) - c.showDiagnostics(diags) - return 1 - } - c.Ui.Output(string(jsonOutputs)) - return 0 - } else { - c.Ui.Output(outputsAsString(state, false)) - return 0 - } - } - - os, ok := mod.OutputValues[name] - if !ok { - c.Ui.Error(fmt.Sprintf( - "The output variable requested could not be found in the state\n" + - "file. If you recently added this to your configuration, be\n" + - "sure to run `terraform apply`, since the state won't be updated\n" + - "with new output variables until that command is run.")) - return 1 - } - v := os.Value - - switch { - case jsonOutput: - jsonOutput, err := ctyjson.Marshal(v, v.Type()) - if err != nil { - return 1 - } - - c.Ui.Output(string(jsonOutput)) - case rawOutput: - strV, err := convert.Convert(v, cty.String) - if err != nil { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Unsupported value for raw output", - fmt.Sprintf( - "The -raw option only supports strings, numbers, and boolean values, but output value %q is %s.\n\nUse the -json option for machine-readable representations of output values that have complex types.", - name, v.Type().FriendlyName(), - ), - )) - c.showDiagnostics(diags) - return 1 - } - if strV.IsNull() { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Unsupported value for raw output", - fmt.Sprintf( - "The value for output value %q is null, so -raw mode cannot print it.", - name, - ), - )) - c.showDiagnostics(diags) - return 1 - } - if !strV.IsKnown() { - // Since we're working with values from the state it would be very - // odd to end up in here, but we'll handle it anyway to avoid a - // panic in case our rules somehow change in future. - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Unsupported value for raw output", - fmt.Sprintf( - "The value for output value %q won't be known until after a successful terraform apply, so -raw mode cannot print it.", - name, - ), - )) - c.showDiagnostics(diags) - return 1 - } - // If we get out here then we should have a valid string to print. - // We're writing it directly to the output here so that a shell caller - // will get exactly the value and no extra whitespace. - str := strV.AsString() - if c.rawPrint != nil { - c.rawPrint(str) - } else { - fmt.Print(str) - } - default: - result := repl.FormatValue(v, 0) - c.Ui.Output(result) - } - - return 0 + return state.RootModule().OutputValues, nil } func (c *OutputCommand) Help() string { diff --git a/command/output_test.go b/command/output_test.go index 0faca69fd..36efa114c 100644 --- a/command/output_test.go +++ b/command/output_test.go @@ -6,7 +6,6 @@ import ( "strings" "testing" - "github.com/mitchellh/cli" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/addrs" @@ -24,11 +23,11 @@ func TestOutput(t *testing.T) { statePath := testStateFile(t, originalState) - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, + View: view, }, } @@ -36,11 +35,13 @@ func TestOutput(t *testing.T) { "-state", statePath, "foo", } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: \n%s", output.Stderr()) } - actual := strings.TrimSpace(ui.OutputWriter.String()) + actual := strings.TrimSpace(output.Stdout()) if actual != `"bar"` { t.Fatalf("bad: %#v", actual) } @@ -64,22 +65,24 @@ func TestOutput_nestedListAndMap(t *testing.T) { }) statePath := testStateFile(t, originalState) - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, + View: view, }, } args := []string{ "-state", statePath, } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: \n%s", output.Stderr()) } - actual := strings.TrimSpace(ui.OutputWriter.String()) + actual := strings.TrimSpace(output.Stdout()) expected := strings.TrimSpace(` foo = tolist([ tomap({ @@ -107,11 +110,11 @@ func TestOutput_json(t *testing.T) { statePath := testStateFile(t, originalState) - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, + View: view, }, } @@ -119,123 +122,42 @@ func TestOutput_json(t *testing.T) { "-state", statePath, "-json", } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: \n%s", output.Stderr()) } - actual := strings.TrimSpace(ui.OutputWriter.String()) + actual := strings.TrimSpace(output.Stdout()) expected := "{\n \"foo\": {\n \"sensitive\": false,\n \"type\": \"string\",\n \"value\": \"bar\"\n }\n}" if actual != expected { t.Fatalf("wrong output\ngot: %#v\nwant: %#v", actual, expected) } } -func TestOutput_raw(t *testing.T) { - originalState := states.BuildState(func(s *states.SyncState) { - s.SetOutputValue( - addrs.OutputValue{Name: "str"}.Absolute(addrs.RootModuleInstance), - cty.StringVal("bar"), - false, - ) - s.SetOutputValue( - addrs.OutputValue{Name: "multistr"}.Absolute(addrs.RootModuleInstance), - cty.StringVal("bar\nbaz"), - false, - ) - s.SetOutputValue( - addrs.OutputValue{Name: "num"}.Absolute(addrs.RootModuleInstance), - cty.NumberIntVal(2), - false, - ) - s.SetOutputValue( - addrs.OutputValue{Name: "bool"}.Absolute(addrs.RootModuleInstance), - cty.True, - false, - ) - s.SetOutputValue( - addrs.OutputValue{Name: "obj"}.Absolute(addrs.RootModuleInstance), - cty.EmptyObjectVal, - false, - ) - s.SetOutputValue( - addrs.OutputValue{Name: "null"}.Absolute(addrs.RootModuleInstance), - cty.NullVal(cty.String), - false, - ) - }) - - statePath := testStateFile(t, originalState) - - tests := map[string]struct { - WantOutput string - WantErr bool - }{ - "str": {WantOutput: "bar"}, - "multistr": {WantOutput: "bar\nbaz"}, - "num": {WantOutput: "2"}, - "bool": {WantOutput: "true"}, - "obj": {WantErr: true}, - "null": {WantErr: true}, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - var printed string - ui := cli.NewMockUi() - c := &OutputCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, - }, - rawPrint: func(s string) { - printed = s - }, - } - args := []string{ - "-state", statePath, - "-raw", - name, - } - code := c.Run(args) - - if code != 0 { - if !test.WantErr { - t.Errorf("unexpected failure\n%s", ui.ErrorWriter.String()) - } - return - } - - if test.WantErr { - t.Fatalf("succeeded, but want error") - } - - if got, want := printed, test.WantOutput; got != want { - t.Errorf("wrong result\ngot: %q\nwant: %q", got, want) - } - }) - } -} - func TestOutput_emptyOutputs(t *testing.T) { originalState := states.NewState() statePath := testStateFile(t, originalState) p := testProvider() - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), - Ui: ui, + View: view, }, } args := []string{ "-state", statePath, } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: \n%s", output.Stderr()) } - if got, want := ui.ErrorWriter.String(), "Warning: No outputs found"; !strings.Contains(got, want) { + // Warning diagnostics should go to stdout + if got, want := output.Stdout(), "Warning: No outputs found"; !strings.Contains(got, want) { t.Fatalf("bad output: expected to contain %q, got:\n%s", want, got) } } @@ -245,11 +167,11 @@ func TestOutput_jsonEmptyOutputs(t *testing.T) { statePath := testStateFile(t, originalState) p := testProvider() - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), - Ui: ui, + View: view, }, } @@ -257,11 +179,13 @@ func TestOutput_jsonEmptyOutputs(t *testing.T) { "-state", statePath, "-json", } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: \n%s", output.Stderr()) } - actual := strings.TrimSpace(ui.OutputWriter.String()) + actual := strings.TrimSpace(output.Stdout()) expected := "{}" if actual != expected { t.Fatalf("bad:\n%#v\n%#v", expected, actual) @@ -278,11 +202,11 @@ func TestOutput_badVar(t *testing.T) { }) statePath := testStateFile(t, originalState) - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, + View: view, }, } @@ -290,8 +214,10 @@ func TestOutput_badVar(t *testing.T) { "-state", statePath, "bar", } - if code := c.Run(args); code != 1 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + code := c.Run(args) + output := done(t) + if code != 1 { + t.Fatalf("bad: \n%s", output.Stderr()) } } @@ -310,11 +236,11 @@ func TestOutput_blank(t *testing.T) { }) statePath := testStateFile(t, originalState) - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, + View: view, }, } @@ -323,23 +249,24 @@ func TestOutput_blank(t *testing.T) { "", } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: \n%s", output.Stderr()) } expectedOutput := "foo = \"bar\"\nname = \"john-doe\"\n" - output := ui.OutputWriter.String() - if output != expectedOutput { - t.Fatalf("wrong output\ngot: %#v\nwant: %#v", output, expectedOutput) + if got := output.Stdout(); got != expectedOutput { + t.Fatalf("wrong output\ngot: %#v\nwant: %#v", got, expectedOutput) } } func TestOutput_manyArgs(t *testing.T) { - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, + View: view, }, } @@ -347,23 +274,27 @@ func TestOutput_manyArgs(t *testing.T) { "bad", "bad", } - if code := c.Run(args); code != 1 { - t.Fatalf("bad: \n%s", ui.OutputWriter.String()) + code := c.Run(args) + output := done(t) + if code != 1 { + t.Fatalf("bad: \n%s", output.Stdout()) } } func TestOutput_noArgs(t *testing.T) { - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, + View: view, }, } args := []string{} - if code := c.Run(args); code != 0 { - t.Fatalf("bad: \n%s", ui.OutputWriter.String()) + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: \n%s", output.Stdout()) } } @@ -371,11 +302,11 @@ func TestOutput_noState(t *testing.T) { originalState := states.NewState() statePath := testStateFile(t, originalState) - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, + View: view, }, } @@ -383,8 +314,10 @@ func TestOutput_noState(t *testing.T) { "-state", statePath, "foo", } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: \n%s", output.Stderr()) } } @@ -393,11 +326,11 @@ func TestOutput_noVars(t *testing.T) { statePath := testStateFile(t, originalState) - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, + View: view, }, } @@ -405,8 +338,10 @@ func TestOutput_noVars(t *testing.T) { "-state", statePath, "bar", } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: \n%s", output.Stderr()) } } @@ -444,22 +379,24 @@ func TestOutput_stateDefault(t *testing.T) { } defer os.Chdir(cwd) - ui := new(cli.MockUi) + view, done := testView(t) c := &OutputCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, + View: view, }, } args := []string{ "foo", } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: \n%s", output.Stderr()) } - actual := strings.TrimSpace(ui.OutputWriter.String()) + actual := strings.TrimSpace(output.Stdout()) if actual != `"bar"` { t.Fatalf("bad: %#v", actual) } diff --git a/command/refresh.go b/command/refresh.go index 5c512d711..91dd28ce1 100644 --- a/command/refresh.go +++ b/command/refresh.go @@ -5,6 +5,8 @@ import ( "strings" "github.com/hashicorp/terraform/backend" + "github.com/hashicorp/terraform/command/arguments" + "github.com/hashicorp/terraform/command/views" "github.com/hashicorp/terraform/tfdiags" ) @@ -100,8 +102,13 @@ func (c *RefreshCommand) Run(args []string) int { return op.Result.ExitStatus() } - if outputs := outputsAsString(op.State, true); outputs != "" { - c.Ui.Output(c.Colorize().Color(outputs)) + if op.State != nil { + outputValues := op.State.RootModule().OutputValues + if len(outputValues) > 0 { + c.Ui.Output(c.Colorize().Color("[reset][bold][green]\nOutputs:\n\n")) + view := views.NewOutput(arguments.ViewHuman, c.View) + view.Output("", outputValues) + } } return op.Result.ExitStatus() diff --git a/command/refresh_test.go b/command/refresh_test.go index e8e30f1bd..2175b0323 100644 --- a/command/refresh_test.go +++ b/command/refresh_test.go @@ -38,10 +38,12 @@ func TestRefresh(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -91,10 +93,12 @@ func TestRefresh_empty(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -133,10 +137,12 @@ func TestRefresh_lockedState(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -177,10 +183,12 @@ func TestRefresh_cwd(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -255,10 +263,12 @@ func TestRefresh_defaultState(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -323,10 +333,12 @@ func TestRefresh_outPath(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -382,10 +394,12 @@ func TestRefresh_var(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } p.GetSchemaResponse = refreshVarFixtureSchema() @@ -418,10 +432,12 @@ func TestRefresh_varFile(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } p.GetSchemaResponse = refreshVarFixtureSchema() @@ -459,10 +475,12 @@ func TestRefresh_varFileDefault(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } p.GetSchemaResponse = refreshVarFixtureSchema() @@ -505,10 +523,12 @@ func TestRefresh_varsUnset(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } p.GetSchemaResponse = &providers.GetSchemaResponse{ @@ -568,10 +588,12 @@ func TestRefresh_backup(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -637,10 +659,12 @@ func TestRefresh_disableBackup(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -702,10 +726,12 @@ func TestRefresh_displaysOutputs(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, done := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } p.GetSchemaResponse = &providers.GetSchemaResponse{ @@ -730,7 +756,7 @@ func TestRefresh_displaysOutputs(t *testing.T) { // Test that outputs were displayed outputValue := "foo.example.com" - actual := ui.OutputWriter.String() + actual := done(t).Stdout() if !strings.Contains(actual, outputValue) { t.Fatalf("Expected:\n%s\n\nTo include: %q", actual, outputValue) } @@ -765,10 +791,12 @@ func TestRefresh_targeted(t *testing.T) { } ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -803,9 +831,11 @@ func TestRefresh_targetFlagsDiags(t *testing.T) { defer testChdir(t, td)() ui := new(cli.MockUi) + view, _ := testView(t) c := &RefreshCommand{ Meta: Meta{ - Ui: ui, + Ui: ui, + View: view, }, } diff --git a/command/views/output.go b/command/views/output.go new file mode 100644 index 000000000..e369eed86 --- /dev/null +++ b/command/views/output.go @@ -0,0 +1,278 @@ +package views + +import ( + "bytes" + "encoding/json" + "fmt" + "sort" + "strings" + + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" + ctyjson "github.com/zclconf/go-cty/cty/json" + + "github.com/hashicorp/terraform/command/arguments" + "github.com/hashicorp/terraform/repl" + "github.com/hashicorp/terraform/states" + "github.com/hashicorp/terraform/tfdiags" +) + +// The Output view renders either one or all outputs, depending on whether or +// not the name argument is empty. +type Output interface { + Output(name string, outputs map[string]*states.OutputValue) tfdiags.Diagnostics + Diagnostics(diags tfdiags.Diagnostics) +} + +// NewOutput returns an initialized Output implementation for the given ViewType. +func NewOutput(vt arguments.ViewType, view *View) Output { + switch vt { + case arguments.ViewJSON: + return &OutputJSON{View: *view} + case arguments.ViewRaw: + return &OutputRaw{View: *view} + case arguments.ViewHuman: + return &OutputHuman{View: *view} + default: + panic(fmt.Sprintf("unknown view type %v", vt)) + } +} + +// The OutputHuman implementation renders outputs in a format equivalent to HCL +// source. This uses the same formatting logic as in the console REPL. +type OutputHuman struct { + View +} + +var _ Output = (*OutputHuman)(nil) + +func (v *OutputHuman) Output(name string, outputs map[string]*states.OutputValue) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + if len(outputs) == 0 { + diags = diags.Append(noOutputsWarning()) + return diags + } + + if name != "" { + output, ok := outputs[name] + if !ok { + diags = diags.Append(missingOutputError(name)) + return diags + } + result := repl.FormatValue(output.Value, 0) + v.output(result) + return nil + } + + outputBuf := new(bytes.Buffer) + if len(outputs) > 0 { + // Output the outputs in alphabetical order + keyLen := 0 + ks := make([]string, 0, len(outputs)) + for key := range outputs { + ks = append(ks, key) + if len(key) > keyLen { + keyLen = len(key) + } + } + sort.Strings(ks) + + for _, k := range ks { + v := outputs[k] + if v.Sensitive { + outputBuf.WriteString(fmt.Sprintf("%s = \n", k)) + continue + } + + result := repl.FormatValue(v.Value, 0) + outputBuf.WriteString(fmt.Sprintf("%s = %s\n", k, result)) + } + } + + v.output(strings.TrimSpace(outputBuf.String())) + + return nil +} + +// The OutputRaw implementation renders single string, number, or boolean +// output values directly and without quotes or other formatting. This is +// intended for use in shell scripting or other environments where the exact +// type of an output value is not important. +type OutputRaw struct { + View + + // Unit tests may set rawPrint to capture the output from the Output + // method, which would normally go to stdout directly. + rawPrint func(string) +} + +var _ Output = (*OutputRaw)(nil) + +func (v *OutputRaw) Output(name string, outputs map[string]*states.OutputValue) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + if len(outputs) == 0 { + diags = diags.Append(noOutputsWarning()) + return diags + } + + if name == "" { + diags = diags.Append(fmt.Errorf("Raw output format is only supported for single outputs")) + return diags + } + + output, ok := outputs[name] + if !ok { + diags = diags.Append(missingOutputError(name)) + return diags + } + + strV, err := convert.Convert(output.Value, cty.String) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unsupported value for raw output", + fmt.Sprintf( + "The -raw option only supports strings, numbers, and boolean values, but output value %q is %s.\n\nUse the -json option for machine-readable representations of output values that have complex types.", + name, output.Value.Type().FriendlyName(), + ), + )) + return diags + } + if strV.IsNull() { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unsupported value for raw output", + fmt.Sprintf( + "The value for output value %q is null, so -raw mode cannot print it.", + name, + ), + )) + return diags + } + if !strV.IsKnown() { + // Since we're working with values from the state it would be very + // odd to end up in here, but we'll handle it anyway to avoid a + // panic in case our rules somehow change in future. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unsupported value for raw output", + fmt.Sprintf( + "The value for output value %q won't be known until after a successful terraform apply, so -raw mode cannot print it.", + name, + ), + )) + return diags + } + // If we get out here then we should have a valid string to print. + // We're writing it directly to the output here so that a shell caller + // will get exactly the value and no extra whitespace. + str := strV.AsString() + fmt.Fprint(v.streams.Stdout.File, str) + return nil +} + +// The OutputJSON implementation renders outputs as JSON values. When rendering +// a single output, only the value is displayed. When rendering all outputs, +// the result is a JSON object with keys matching the output names and object +// values including type and sensitivity metadata. +type OutputJSON struct { + View +} + +var _ Output = (*OutputJSON)(nil) + +func (v *OutputJSON) Output(name string, outputs map[string]*states.OutputValue) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + if name != "" { + output, ok := outputs[name] + if !ok { + diags = diags.Append(missingOutputError(name)) + return diags + } + value := output.Value + + jsonOutput, err := ctyjson.Marshal(value, value.Type()) + if err != nil { + diags = diags.Append(err) + return diags + } + + v.output(string(jsonOutput)) + + return nil + } + + // Due to a historical accident, the switch from state version 2 to + // 3 caused our JSON output here to be the full metadata about the + // outputs rather than just the output values themselves as we'd + // show in the single value case. We must now maintain that behavior + // for compatibility, so this is an emulation of the JSON + // serialization of outputs used in state format version 3. + type OutputMeta struct { + Sensitive bool `json:"sensitive"` + Type json.RawMessage `json:"type"` + Value json.RawMessage `json:"value"` + } + outputMetas := map[string]OutputMeta{} + + for n, os := range outputs { + jsonVal, err := ctyjson.Marshal(os.Value, os.Value.Type()) + if err != nil { + diags = diags.Append(err) + return diags + } + jsonType, err := ctyjson.MarshalType(os.Value.Type()) + if err != nil { + diags = diags.Append(err) + return diags + } + outputMetas[n] = OutputMeta{ + Sensitive: os.Sensitive, + Type: json.RawMessage(jsonType), + Value: json.RawMessage(jsonVal), + } + } + + jsonOutputs, err := json.MarshalIndent(outputMetas, "", " ") + if err != nil { + diags = diags.Append(err) + return diags + } + + v.output(string(jsonOutputs)) + + return nil +} + +// For text and raw output modes, an empty map of outputs is considered a +// separate and higher priority failure mode than an output not being present +// in a non-empty map. This warning diagnostic explains how this might have +// happened. +func noOutputsWarning() tfdiags.Diagnostic { + return tfdiags.Sourceless( + tfdiags.Warning, + "No outputs found", + "The state file either has no outputs defined, or all the defined "+ + "outputs are empty. Please define an output in your configuration "+ + "with the `output` keyword and run `terraform refresh` for it to "+ + "become available. If you are using interpolation, please verify "+ + "the interpolated value is not empty. You can use the "+ + "`terraform console` command to assist.", + ) +} + +// Attempting to display a missing output results in this failure, which +// includes suggestions on how to rectify the problem. +func missingOutputError(name string) tfdiags.Diagnostic { + return tfdiags.Sourceless( + tfdiags.Error, + fmt.Sprintf("Output %q not found", name), + "The output variable requested could not be found in the state "+ + "file. If you recently added this to your configuration, be "+ + "sure to run `terraform apply`, since the state won't be updated "+ + "with new output variables until that command is run.", + ) +} diff --git a/command/views/output_test.go b/command/views/output_test.go new file mode 100644 index 000000000..2cfa95a33 --- /dev/null +++ b/command/views/output_test.go @@ -0,0 +1,60 @@ +package views + +import ( + "testing" + + "github.com/hashicorp/terraform/internal/terminal" + "github.com/hashicorp/terraform/states" + "github.com/zclconf/go-cty/cty" +) + +func TestOutputRaw(t *testing.T) { + values := map[string]cty.Value{ + "str": cty.StringVal("bar"), + "multistr": cty.StringVal("bar\nbaz"), + "num": cty.NumberIntVal(2), + "bool": cty.True, + "obj": cty.EmptyObjectVal, + "null": cty.NullVal(cty.String), + } + + tests := map[string]struct { + WantOutput string + WantErr bool + }{ + "str": {WantOutput: "bar"}, + "multistr": {WantOutput: "bar\nbaz"}, + "num": {WantOutput: "2"}, + "bool": {WantOutput: "true"}, + "obj": {WantErr: true}, + "null": {WantErr: true}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + streams, done := terminal.StreamsForTesting(t) + view := NewView(streams) + v := &OutputRaw{ + View: *view, + } + + value := values[name] + outputs := map[string]*states.OutputValue{ + name: {Value: value}, + } + diags := v.Output(name, outputs) + + if diags.HasErrors() { + if !test.WantErr { + t.Fatalf("unexpected diagnostics: %s", diags) + } + } else if test.WantErr { + t.Fatalf("succeeded, but want error") + } + + if got, want := done(t).Stdout(), test.WantOutput; got != want { + t.Errorf("wrong result\ngot: %q\nwant: %q", got, want) + } + }) + } +} diff --git a/command/views/view.go b/command/views/view.go new file mode 100644 index 000000000..a5dd446b6 --- /dev/null +++ b/command/views/view.go @@ -0,0 +1,122 @@ +package views + +import ( + "fmt" + + "github.com/hashicorp/terraform/command/format" + "github.com/hashicorp/terraform/internal/terminal" + "github.com/hashicorp/terraform/tfdiags" + "github.com/mitchellh/colorstring" +) + +// View is the base layer for command views, encapsulating a set of I/O +// streams, a colorize implementation, and implementing a human friendly view +// for diagnostics. +type View struct { + streams *terminal.Streams + colorize *colorstring.Colorize + + // NOTE: compactWarnings is currently always false. When implementing + // views for commands which support this flag, we will need to address this. + compactWarnings bool + + // This unfortunate wart is required to enable rendering of diagnostics which + // have associated source code in the configuration. This function pointer + // will be dereferenced as late as possible when rendering diagnostics in + // order to access the config loader cache. + configSources func() map[string][]byte +} + +// Initialize a View with the given streams, a disabled colorize object, and a +// no-op configSources callback. +func NewView(streams *terminal.Streams) *View { + return &View{ + streams: streams, + colorize: &colorstring.Colorize{ + Colors: colorstring.DefaultColors, + Disable: true, + Reset: true, + }, + configSources: func() map[string][]byte { return nil }, + } +} + +// EnableColor controls the colorize implementation used by this view (and any +// other views which depend on it). This is called by the code which processes +// the global -no-color CLI flag, which happens after the view is initialized. +func (v *View) EnableColor(color bool) { + v.colorize.Disable = !color +} + +// SetConfigSources overrides the default no-op callback with a new function +// pointer, and should be called when the config loader is initialized. +func (v *View) SetConfigSources(cb func() map[string][]byte) { + v.configSources = cb +} + +// Diagnostics renders a set of warnings and errors in human-readable form. +// Warnings are printed to stdout, and errors to stderr. +func (v *View) Diagnostics(diags tfdiags.Diagnostics) { + diags.Sort() + + if len(diags) == 0 { + return + } + + diags = diags.ConsolidateWarnings(1) + + // Since warning messages are generally competing + if v.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, v.colorize) + msg = "\n" + msg + "\nTo see the full warning notes, run Terraform without -compact-warnings.\n" + v.output(msg) + return + } + } + + for _, diag := range diags { + var msg string + if v.colorize.Disable { + msg = format.DiagnosticPlain(diag, v.configSources(), v.streams.Stderr.Columns()) + } else { + msg = format.Diagnostic(diag, v.configSources(), v.colorize, v.streams.Stderr.Columns()) + } + + if diag.Severity() == tfdiags.Error { + fmt.Fprint(v.streams.Stderr.File, msg) + } else { + fmt.Fprint(v.streams.Stdout.File, msg) + } + } +} + +// HelpPrompt is intended to be called from commands which fail to parse all +// of their CLI arguments successfully. It refers users to the full help output +// rather than rendering it directly, which can be overwhelming and confusing. +func (v *View) HelpPrompt(command string) { + fmt.Fprintf(v.streams.Stderr.File, helpPrompt, command) +} + +const helpPrompt = ` +For more help on using this command, run: + terraform %s -help +` + +// output is a shorthand for the common view operation of printing a string to +// the stdout stream, followed by a newline. +func (v *View) output(s string) { + fmt.Fprintln(v.streams.Stdout.File, s) +} diff --git a/commands.go b/commands.go index 7c692bd65..b694c6acf 100644 --- a/commands.go +++ b/commands.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/command" "github.com/hashicorp/terraform/command/cliconfig" + "github.com/hashicorp/terraform/command/views" "github.com/hashicorp/terraform/command/webbrowser" "github.com/hashicorp/terraform/internal/getproviders" "github.com/hashicorp/terraform/internal/terminal" @@ -81,6 +82,7 @@ func initCommands( meta := command.Meta{ OriginalWorkingDir: originalWorkingDir, Streams: streams, + View: views.NewView(streams), Color: true, GlobalPluginDirs: globalPluginDirs(), From 57879bfb717007e2b6a9ac21b644e1cfc5c8c413 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 10 Feb 2021 17:31:43 -0500 Subject: [PATCH 2/2] cli: Add global view arguments parser Rather than modifying and relying on the existing Meta.process argument extractor, we can more clearly handle global CLI flags using a separate parser step. This allows us to explicitly configure the view in the command. --- command/arguments/view.go | 43 +++++++++++++++++++++++ command/arguments/view_test.go | 62 ++++++++++++++++++++++++++++++++++ command/meta.go | 4 --- command/output.go | 4 ++- command/output_test.go | 1 + command/views/view.go | 10 +++--- 6 files changed, 114 insertions(+), 10 deletions(-) create mode 100644 command/arguments/view.go create mode 100644 command/arguments/view_test.go diff --git a/command/arguments/view.go b/command/arguments/view.go new file mode 100644 index 000000000..3d6372b6a --- /dev/null +++ b/command/arguments/view.go @@ -0,0 +1,43 @@ +package arguments + +// View represents the global command-line arguments which configure the view. +type View struct { + // NoColor is used to disable the use of terminal color codes in all + // output. + NoColor bool + + // CompactWarnings is used to coalesce duplicate warnings, to reduce the + // level of noise when multiple instances of the same warning are raised + // for a configuration. + CompactWarnings bool +} + +// ParseView processes CLI arguments, returning a View value and a +// possibly-modified slice of arguments. If any of the supported flags are +// found, they will be removed from the slice. +func ParseView(args []string) (*View, []string) { + common := &View{} + + // Keep track of the length of the returned slice. When we find an + // argument we support, i will not be incremented. + i := 0 + for _, v := range args { + switch v { + case "-no-color": + common.NoColor = true + case "-compact-warnings": + common.CompactWarnings = true + default: + // Unsupported argument: move left to the current position, and + // increment the index. + args[i] = v + i++ + } + } + + // Reduce the slice to the number of unsupported arguments. Any remaining + // to the right of i have already been moved left. + args = args[:i] + + return common, args +} diff --git a/command/arguments/view_test.go b/command/arguments/view_test.go new file mode 100644 index 000000000..d2e7c3f73 --- /dev/null +++ b/command/arguments/view_test.go @@ -0,0 +1,62 @@ +package arguments + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestParseView(t *testing.T) { + testCases := map[string]struct { + args []string + want *View + wantArgs []string + }{ + "nil": { + nil, + &View{NoColor: false, CompactWarnings: false}, + nil, + }, + "empty": { + []string{}, + &View{NoColor: false, CompactWarnings: false}, + []string{}, + }, + "none matching": { + []string{"-foo", "bar", "-baz"}, + &View{NoColor: false, CompactWarnings: false}, + []string{"-foo", "bar", "-baz"}, + }, + "no-color": { + []string{"-foo", "-no-color", "-baz"}, + &View{NoColor: true, CompactWarnings: false}, + []string{"-foo", "-baz"}, + }, + "compact-warnings": { + []string{"-foo", "-compact-warnings", "-baz"}, + &View{NoColor: false, CompactWarnings: true}, + []string{"-foo", "-baz"}, + }, + "both": { + []string{"-foo", "-no-color", "-compact-warnings", "-baz"}, + &View{NoColor: true, CompactWarnings: true}, + []string{"-foo", "-baz"}, + }, + "both, resulting in empty args": { + []string{"-no-color", "-compact-warnings"}, + &View{NoColor: true, CompactWarnings: true}, + []string{}, + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, gotArgs := ParseView(tc.args) + if *got != *tc.want { + t.Errorf("unexpected result\n got: %#v\nwant: %#v", got, tc.want) + } + if !cmp.Equal(gotArgs, tc.wantArgs) { + t.Errorf("unexpected args\n got: %#v\nwant: %#v", gotArgs, tc.wantArgs) + } + }) + } +} diff --git a/command/meta.go b/command/meta.go index 92ad6b85a..4f01af630 100644 --- a/command/meta.go +++ b/command/meta.go @@ -621,10 +621,6 @@ func (m *Meta) process(args []string) []string { }, } - if m.View != nil { - m.View.EnableColor(m.Color) - } - return args } diff --git a/command/output.go b/command/output.go index 4eaec982a..52e4b942a 100644 --- a/command/output.go +++ b/command/output.go @@ -17,7 +17,9 @@ type OutputCommand struct { } func (c *OutputCommand) Run(rawArgs []string) int { - rawArgs = c.Meta.process(rawArgs) + // Parse and apply global view arguments + common, rawArgs := arguments.ParseView(rawArgs) + c.View.Configure(common) // Parse and validate flags args, diags := arguments.ParseOutput(rawArgs) diff --git a/command/output_test.go b/command/output_test.go index 36efa114c..3824c0fdc 100644 --- a/command/output_test.go +++ b/command/output_test.go @@ -149,6 +149,7 @@ func TestOutput_emptyOutputs(t *testing.T) { } args := []string{ + "-no-color", "-state", statePath, } code := c.Run(args) diff --git a/command/views/view.go b/command/views/view.go index a5dd446b6..5dd75184f 100644 --- a/command/views/view.go +++ b/command/views/view.go @@ -3,6 +3,7 @@ package views import ( "fmt" + "github.com/hashicorp/terraform/command/arguments" "github.com/hashicorp/terraform/command/format" "github.com/hashicorp/terraform/internal/terminal" "github.com/hashicorp/terraform/tfdiags" @@ -41,11 +42,10 @@ func NewView(streams *terminal.Streams) *View { } } -// EnableColor controls the colorize implementation used by this view (and any -// other views which depend on it). This is called by the code which processes -// the global -no-color CLI flag, which happens after the view is initialized. -func (v *View) EnableColor(color bool) { - v.colorize.Disable = !color +// Configure applies the global view configuration flags. +func (v *View) Configure(view *arguments.View) { + v.colorize.Disable = view.NoColor + v.compactWarnings = view.CompactWarnings } // SetConfigSources overrides the default no-op callback with a new function