From 5aa088e385d85901ce0a377c131dd9cebd1598ca Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 14 Dec 2022 11:09:32 -0500 Subject: [PATCH] remove -always-out Make writing a plan file the default. We already create plans which have no changes so the plan result would need to be checked in automation, so having plans with errors should not pose a problem. If we find workflows which cannot handle a plan that can't be applied, we can reevaluate the need for a specialized flag. In the meantime, it feels more logical that the plan output would always describe the result of the plan, even if that included errors. --- internal/backend/backend.go | 1 - internal/backend/local/backend_plan.go | 7 +---- internal/command/arguments/plan.go | 29 +++------------------ internal/command/arguments/plan_test.go | 34 +------------------------ internal/command/plan.go | 4 +-- 5 files changed, 6 insertions(+), 69 deletions(-) diff --git a/internal/backend/backend.go b/internal/backend/backend.go index cef391a6d2..48a049615d 100644 --- a/internal/backend/backend.go +++ b/internal/backend/backend.go @@ -230,7 +230,6 @@ type Operation struct { PlanRefresh bool // PlanRefresh will do a refresh before a plan PlanOutPath string // PlanOutPath is the path to save the plan PlanOutBackend *plans.Backend - PlanOutAlways bool // Produce PlanOutPath even if plan is incomplete // ConfigDir is the path to the directory containing the configuration's // root module. diff --git a/internal/backend/local/backend_plan.go b/internal/backend/local/backend_plan.go index b827db5927..1513aaa43e 100644 --- a/internal/backend/local/backend_plan.go +++ b/internal/backend/local/backend_plan.go @@ -103,13 +103,8 @@ func (b *Local) opPlan( // Record whether this plan includes any side-effects that could be applied. runningOp.PlanEmpty = !plan.CanApply() - planErrored := false - if plan != nil { - planErrored = plan.Errored - } - // Save the plan to disk - if path := op.PlanOutPath; path != "" && plan != nil && (op.PlanOutAlways || !planErrored) { + if path := op.PlanOutPath; path != "" && plan != nil { if op.PlanOutBackend == nil { // This is always a bug in the operation caller; it's not valid // to set PlanOutPath without also setting PlanOutBackend. diff --git a/internal/command/arguments/plan.go b/internal/command/arguments/plan.go index d9c63d2403..2300dc7a5d 100644 --- a/internal/command/arguments/plan.go +++ b/internal/command/arguments/plan.go @@ -19,12 +19,8 @@ type Plan struct { // variable and backend config values. Default is true. InputEnabled bool - // OutPath contains an optional path to store the plan file, while - // AlwaysOut means that we'll write to OutPath even if the plan is - // incomplete, so that it's still possible to inspect it with - // "terraform show". AlwaysOut is irrelevant if OutPath isn't set. - OutPath string - AlwaysOut bool + // OutPath contains an optional path to store the plan file + OutPath string // ViewType specifies which output format to use ViewType ViewType @@ -41,13 +37,10 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) { Vars: &Vars{}, } - var outPath, alwaysOutPath string - cmdFlags := extendedFlagSet("plan", plan.State, plan.Operation, plan.Vars) cmdFlags.BoolVar(&plan.DetailedExitCode, "detailed-exitcode", false, "detailed-exitcode") cmdFlags.BoolVar(&plan.InputEnabled, "input", true, "input") - cmdFlags.StringVar(&outPath, "out", "", "out") - cmdFlags.StringVar(&alwaysOutPath, "always-out", "", "always-out") + cmdFlags.StringVar(&plan.OutPath, "out", "", "out") var json bool cmdFlags.BoolVar(&json, "json", false, "json") @@ -60,22 +53,6 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) { )) } - switch { - case outPath != "": - if alwaysOutPath != "" { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Incompatible command line options", - "The -out=... and -always-out=... options are mutually-exclusive.", - )) - } - plan.OutPath = outPath - plan.AlwaysOut = false - case alwaysOutPath != "": - plan.OutPath = alwaysOutPath - plan.AlwaysOut = true - } - args = cmdFlags.Args() if len(args) > 0 { diff --git a/internal/command/arguments/plan_test.go b/internal/command/arguments/plan_test.go index 25e5d2de3b..b547d3f7ab 100644 --- a/internal/command/arguments/plan_test.go +++ b/internal/command/arguments/plan_test.go @@ -31,30 +31,12 @@ func TestParsePlan_basicValid(t *testing.T) { }, }, }, - "setting all options with -out": { + "setting all options": { []string{"-destroy", "-detailed-exitcode", "-input=false", "-out=saved.tfplan"}, &Plan{ DetailedExitCode: true, InputEnabled: false, OutPath: "saved.tfplan", - AlwaysOut: false, - ViewType: ViewHuman, - State: &State{Lock: true}, - Vars: &Vars{}, - Operation: &Operation{ - PlanMode: plans.DestroyMode, - Parallelism: 10, - Refresh: true, - }, - }, - }, - "setting all options with -always-out": { - []string{"-destroy", "-detailed-exitcode", "-input=false", "-always-out=saved.tfplan"}, - &Plan{ - DetailedExitCode: true, - InputEnabled: false, - OutPath: "saved.tfplan", - AlwaysOut: true, ViewType: ViewHuman, State: &State{Lock: true}, Vars: &Vars{}, @@ -111,20 +93,6 @@ func TestParsePlan_invalid(t *testing.T) { } } -func TestParsePlan_conflictingOutPath(t *testing.T) { - got, diags := ParsePlan([]string{"-out=foo", "-always-out=bar"}) - if len(diags) == 0 { - t.Fatal("expected diags but got none") - } - const wantSubstr = "Incompatible command line options: The -out=... and -always-out=... options are mutually-exclusive." - if got, want := diags.Err().Error(), wantSubstr; !strings.Contains(got, want) { - t.Fatalf("wrong diags\ngot: %s\nwant: %s", got, want) - } - if got.ViewType != ViewHuman { - t.Fatalf("wrong view type\ngot: %#v\nwant: %#v", got.ViewType, ViewHuman) - } -} - func TestParsePlan_tooManyArguments(t *testing.T) { got, diags := ParsePlan([]string{"saved.tfplan"}) if len(diags) == 0 { diff --git a/internal/command/plan.go b/internal/command/plan.go index 59b99f6d76..d5ffedbff6 100644 --- a/internal/command/plan.go +++ b/internal/command/plan.go @@ -72,7 +72,7 @@ func (c *PlanCommand) Run(rawArgs []string) int { } // Build the operation request - opReq, opDiags := c.OperationRequest(be, view, args.Operation, args.OutPath, args.AlwaysOut) + opReq, opDiags := c.OperationRequest(be, view, args.Operation, args.OutPath) diags = diags.Append(opDiags) if diags.HasErrors() { view.Diagnostics(diags) @@ -139,7 +139,6 @@ func (c *PlanCommand) OperationRequest( view views.Plan, args *arguments.Operation, planOutPath string, - planOutAlways bool, ) (*backend.Operation, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics @@ -150,7 +149,6 @@ func (c *PlanCommand) OperationRequest( opReq.Hooks = view.Hooks() opReq.PlanRefresh = args.Refresh opReq.PlanOutPath = planOutPath - opReq.PlanOutAlways = planOutAlways opReq.Targets = args.Targets opReq.ForceReplace = args.ForceReplace opReq.Type = backend.OperationTypePlan