From 80b184e67e8c32e50657f8882737771d07782463 Mon Sep 17 00:00:00 2001 From: melsonic Date: Sun, 1 Sep 2024 15:55:24 +0530 Subject: [PATCH 1/6] show deprecation warning if -state is used with plan, apply, refresh --- ;w | 211 ++++++++++++++++++ internal/command/apply.go | 5 + internal/command/arguments/apply.go | 3 + internal/command/arguments/apply_test.go | 12 +- internal/command/arguments/plan.go | 3 + internal/command/arguments/plan_test.go | 6 +- internal/command/arguments/refresh.go | 3 + internal/command/arguments/refresh_test.go | 6 +- .../command/testdata/apply/output.jsonlog | 1 + internal/command/testdata/plan/output.jsonlog | 1 + 10 files changed, 239 insertions(+), 12 deletions(-) create mode 100644 ;w diff --git a/;w b/;w new file mode 100644 index 000000000000..2bac7e2a9f01 --- /dev/null +++ b/;w @@ -0,0 +1,211 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package arguments + +import ( + "fmt" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/plans" +) + +func TestParsePlan_basicValid(t *testing.T) { + testCases := map[string]struct { + args []string + want *Plan + }{ + "defaults": { + nil, + &Plan{ + DetailedExitCode: false, + InputEnabled: true, + OutPath: "", + ViewType: ViewHuman, + State: &State{Lock: true}, + Vars: &Vars{}, + Operation: &Operation{ + PlanMode: plans.NormalMode, + Parallelism: 10, + Refresh: true, + }, + }, + }, + "setting all options": { + []string{"-destroy", "-detailed-exitcode", "-input=false", "-out=saved.tfplan"}, + &Plan{ + DetailedExitCode: true, + InputEnabled: false, + OutPath: "saved.tfplan", + ViewType: ViewHuman, + State: &State{Lock: true}, + Vars: &Vars{}, + Operation: &Operation{ + PlanMode: plans.DestroyMode, + Parallelism: 10, + Refresh: true, + }, + }, + }, + "JSON view disables input": { + []string{"-json"}, + &Plan{ + DetailedExitCode: false, + InputEnabled: false, + OutPath: "", + ViewType: ViewJSON, + State: &State{Lock: true}, + Vars: &Vars{}, + Operation: &Operation{ + PlanMode: plans.NormalMode, + Parallelism: 10, + Refresh: true, + }, + }, + }, + } + + cmpOpts := cmpopts.IgnoreUnexported(Operation{}, Vars{}, State{}) + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, diags := ParsePlan(tc.args) + if len(diags) > 0 && diags.HasErrors() { + t.Fatalf("unexpected diags: %v", diags) + } + if diff := cmp.Diff(tc.want, got, cmpOpts); diff != "" { + t.Errorf("unexpected result\n%s", diff) + } + }) + } +} + +func TestParsePlan_invalid(t *testing.T) { + got, diags := ParsePlan([]string{"-frob"}) + if len(diags) == 0 { + t.Fatal("expected diags but got none") + } + if got, want := diags.Err().Error(), "flag provided but not defined"; !strings.Contains(got, want) { + t.Fatalf("wrong diags\n got: %s\nwant: %s", got, want) + } + if got.ViewType != ViewHuman { + t.Fatalf("wrong view type, got %#v, want %#v", got.ViewType, ViewHuman) + } +} + +func TestParsePlan_tooManyArguments(t *testing.T) { + got, diags := ParsePlan([]string{"saved.tfplan"}) + if len(diags) == 0 { + t.Fatal("expected diags but got none") + } + if got, want := diags.Err().Error(), "Too many command line arguments"; !strings.Contains(got, want) { + t.Fatalf("wrong diags\n got: %s\nwant: %s", got, want) + } + if got.ViewType != ViewHuman { + t.Fatalf("wrong view type, got %#v, want %#v", got.ViewType, ViewHuman) + } +} + +func TestParsePlan_targets(t *testing.T) { + foobarbaz, _ := addrs.ParseTargetStr("foo_bar.baz") + boop, _ := addrs.ParseTargetStr("module.boop") + testCases := map[string]struct { + args []string + want []addrs.Targetable + wantErr string + }{ + "no targets by default": { + args: nil, + want: nil, + }, + "one target": { + args: []string{"-target=foo_bar.baz"}, + want: []addrs.Targetable{foobarbaz.Subject}, + }, + "two targets": { + args: []string{"-target=foo_bar.baz", "-target", "module.boop"}, + want: []addrs.Targetable{foobarbaz.Subject, boop.Subject}, + }, + "invalid traversal": { + args: []string{"-target=foo."}, + want: nil, + wantErr: "Dot must be followed by attribute name", + }, + "invalid target": { + args: []string{"-target=data[0].foo"}, + want: nil, + wantErr: "A data source name is required", + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, diags := ParsePlan(tc.args) + if len(diags) > 0 { + if tc.wantErr == "" && diags.HasErrors() { + t.Fatalf("unexpected diags: %v", diags) + } else if got := diags.Err().Error(); !strings.Contains(got, tc.wantErr) { + t.Fatalf("wrong diags\n got: %s\nwant: %s", got, tc.wantErr) + } + } + if !cmp.Equal(got.Operation.Targets, tc.want) { + t.Fatalf("unexpected result\n%s", cmp.Diff(got.Operation.Targets, tc.want)) + } + }) + } +} + +func TestParsePlan_vars(t *testing.T) { + testCases := map[string]struct { + args []string + want []FlagNameValue + }{ + "no var flags by default": { + args: nil, + want: nil, + }, + "one var": { + args: []string{"-var", "foo=bar"}, + want: []FlagNameValue{ + {Name: "-var", Value: "foo=bar"}, + }, + }, + "one var-file": { + args: []string{"-var-file", "cool.tfvars"}, + want: []FlagNameValue{ + {Name: "-var-file", Value: "cool.tfvars"}, + }, + }, + "ordering preserved": { + args: []string{ + "-var", "foo=bar", + "-var-file", "cool.tfvars", + "-var", "boop=beep", + }, + want: []FlagNameValue{ + {Name: "-var", Value: "foo=bar"}, + {Name: "-var-file", Value: "cool.tfvars"}, + {Name: "-var", Value: "boop=beep"}, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, diags := ParsePlan(tc.args) + if len(diags) > 0 && diags.HasErrors() { + t.Fatalf("unexpected diags: %v", diags) + } + if vars := got.Vars.All(); !cmp.Equal(vars, tc.want) { + t.Fatalf("unexpected result\n%s", cmp.Diff(vars, tc.want)) + } + if got, want := got.Vars.Empty(), len(tc.want) == 0; got != want { + t.Fatalf("expected Empty() to return %t, but was %t", want, got) + } + }) + } +} diff --git a/internal/command/apply.go b/internal/command/apply.go index 5f4ce3dd56d0..705aa050fc53 100644 --- a/internal/command/apply.go +++ b/internal/command/apply.go @@ -56,6 +56,11 @@ func (c *ApplyCommand) Run(rawArgs []string) int { return 1 } + if diags.HasWarnings() { + view.Diagnostics(diags) + diags = nil + } + // Check for user-supplied plugin path var err error if c.pluginPath, err = c.loadPluginPath(); err != nil { diff --git a/internal/command/arguments/apply.go b/internal/command/arguments/apply.go index 07bb9eb35000..2eb9830a9902 100644 --- a/internal/command/arguments/apply.go +++ b/internal/command/arguments/apply.go @@ -43,6 +43,9 @@ func ParseApply(args []string) (*Apply, tfdiags.Diagnostics) { } cmdFlags := extendedFlagSet("apply", apply.State, apply.Operation, apply.Vars) + if apply.State != nil { + diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) + } cmdFlags.BoolVar(&apply.AutoApprove, "auto-approve", false, "auto-approve") cmdFlags.BoolVar(&apply.InputEnabled, "input", true, "input") diff --git a/internal/command/arguments/apply_test.go b/internal/command/arguments/apply_test.go index 09e7f4fdedf1..80483d62ce40 100644 --- a/internal/command/arguments/apply_test.go +++ b/internal/command/arguments/apply_test.go @@ -89,7 +89,7 @@ func TestParseApply_basicValid(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, diags := ParseApply(tc.args) - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { t.Fatalf("unexpected diags: %v", diags) } if diff := cmp.Diff(tc.want, got, cmpOpts); diff != "" { @@ -123,7 +123,7 @@ func TestParseApply_json(t *testing.T) { got, diags := ParseApply(tc.args) if tc.wantSuccess { - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { t.Errorf("unexpected diags: %v", diags) } } else { @@ -200,7 +200,7 @@ func TestParseApply_targets(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, diags := ParseApply(tc.args) - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { if tc.wantErr == "" { t.Fatalf("unexpected diags: %v", diags) } else if got := diags.Err().Error(); !strings.Contains(got, tc.wantErr) { @@ -259,7 +259,7 @@ func TestParseApply_replace(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, diags := ParseApply(tc.args) - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { if tc.wantErr == "" { t.Fatalf("unexpected diags: %v", diags) } else if got := diags.Err().Error(); !strings.Contains(got, tc.wantErr) { @@ -311,7 +311,7 @@ func TestParseApply_vars(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, diags := ParseApply(tc.args) - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { t.Fatalf("unexpected diags: %v", diags) } if vars := got.Vars.All(); !cmp.Equal(vars, tc.want) { @@ -366,7 +366,7 @@ func TestParseApplyDestroy_basicValid(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, diags := ParseApplyDestroy(tc.args) - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { t.Fatalf("unexpected diags: %v", diags) } if diff := cmp.Diff(tc.want, got, cmpOpts); diff != "" { diff --git a/internal/command/arguments/plan.go b/internal/command/arguments/plan.go index 0e3c87c6ccc5..4b4a59f7a080 100644 --- a/internal/command/arguments/plan.go +++ b/internal/command/arguments/plan.go @@ -46,6 +46,9 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) { } cmdFlags := extendedFlagSet("plan", plan.State, plan.Operation, plan.Vars) + if plan.State != nil { + diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) + } cmdFlags.BoolVar(&plan.DetailedExitCode, "detailed-exitcode", false, "detailed-exitcode") cmdFlags.BoolVar(&plan.InputEnabled, "input", true, "input") cmdFlags.StringVar(&plan.OutPath, "out", "", "out") diff --git a/internal/command/arguments/plan_test.go b/internal/command/arguments/plan_test.go index d020e9e4c2e2..e7fe6f401fdd 100644 --- a/internal/command/arguments/plan_test.go +++ b/internal/command/arguments/plan_test.go @@ -73,7 +73,7 @@ func TestParsePlan_basicValid(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, diags := ParsePlan(tc.args) - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { t.Fatalf("unexpected diags: %v", diags) } if diff := cmp.Diff(tc.want, got, cmpOpts); diff != "" { @@ -144,7 +144,7 @@ func TestParsePlan_targets(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, diags := ParsePlan(tc.args) - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { if tc.wantErr == "" { t.Fatalf("unexpected diags: %v", diags) } else if got := diags.Err().Error(); !strings.Contains(got, tc.wantErr) { @@ -196,7 +196,7 @@ func TestParsePlan_vars(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, diags := ParsePlan(tc.args) - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { t.Fatalf("unexpected diags: %v", diags) } if vars := got.Vars.All(); !cmp.Equal(vars, tc.want) { diff --git a/internal/command/arguments/refresh.go b/internal/command/arguments/refresh.go index 53d4ccc86919..bcb827f677f6 100644 --- a/internal/command/arguments/refresh.go +++ b/internal/command/arguments/refresh.go @@ -34,6 +34,9 @@ func ParseRefresh(args []string) (*Refresh, tfdiags.Diagnostics) { } cmdFlags := extendedFlagSet("refresh", refresh.State, refresh.Operation, refresh.Vars) + if refresh.State != nil { + diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) + } cmdFlags.BoolVar(&refresh.InputEnabled, "input", true, "input") var json bool diff --git a/internal/command/arguments/refresh_test.go b/internal/command/arguments/refresh_test.go index 9bf454054af4..32cbb5af7c29 100644 --- a/internal/command/arguments/refresh_test.go +++ b/internal/command/arguments/refresh_test.go @@ -42,7 +42,7 @@ func TestParseRefresh_basicValid(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, diags := ParseRefresh(tc.args) - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { t.Fatalf("unexpected diags: %v", diags) } // Ignore the extended arguments for simplicity @@ -117,7 +117,7 @@ func TestParseRefresh_targets(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, diags := ParseRefresh(tc.args) - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { if tc.wantErr == "" { t.Fatalf("unexpected diags: %v", diags) } else if got := diags.Err().Error(); !strings.Contains(got, tc.wantErr) { @@ -169,7 +169,7 @@ func TestParseRefresh_vars(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, diags := ParseRefresh(tc.args) - if len(diags) > 0 { + if len(diags) > 0 && diags.HasErrors() { t.Fatalf("unexpected diags: %v", diags) } if vars := got.Vars.All(); !cmp.Equal(vars, tc.want) { diff --git a/internal/command/testdata/apply/output.jsonlog b/internal/command/testdata/apply/output.jsonlog index 64cf5cc4761b..a46f15d7c7d9 100644 --- a/internal/command/testdata/apply/output.jsonlog +++ b/internal/command/testdata/apply/output.jsonlog @@ -1,4 +1,5 @@ {"@level":"info","@message":"Terraform 0.15.0-dev","@module":"terraform.ui","terraform":"0.15.0-dev","type":"version","ui":"0.1.0"} +{"@level":"warn","@message":"Warning: state is deprecated","@module":"terraform.ui","diagnostic":{"detail":"","severity":"warning","summary":"state is deprecated"},"type":"diagnostic"} {"@level":"info","@message":"test_instance.foo: Plan to create","@module":"terraform.ui","change":{"resource":{"addr":"test_instance.foo","module":"","resource":"test_instance.foo","implied_provider":"test","resource_type":"test_instance","resource_name":"foo","resource_key":null},"action":"create"},"type":"planned_change"} {"@level":"info","@message":"Plan: 1 to add, 0 to change, 0 to destroy.","@module":"terraform.ui","changes":{"add":1,"import":0,"change":0,"remove":0,"operation":"plan"},"type":"change_summary"} {"@level":"info","@message":"test_instance.foo: Creating...","@module":"terraform.ui","hook":{"resource":{"addr":"test_instance.foo","module":"","resource":"test_instance.foo","implied_provider":"test","resource_type":"test_instance","resource_name":"foo","resource_key":null},"action":"create"},"type":"apply_start"} diff --git a/internal/command/testdata/plan/output.jsonlog b/internal/command/testdata/plan/output.jsonlog index 7f42c6eca512..0650b6ef0231 100644 --- a/internal/command/testdata/plan/output.jsonlog +++ b/internal/command/testdata/plan/output.jsonlog @@ -1,4 +1,5 @@ {"@level":"info","@message":"Terraform 1.3.0-dev","@module":"terraform.ui","terraform":"1.3.0-dev","type":"version","ui":"1.0"} +{"@level":"warn","@message":"Warning: state is deprecated","@module":"terraform.ui","diagnostic":{"detail":"","severity":"warning","summary":"state is deprecated"},"type":"diagnostic"} {"@level":"info","@message":"data.test_data_source.a: Refreshing...","@module":"terraform.ui","hook":{"resource":{"addr":"data.test_data_source.a","module":"","resource":"data.test_data_source.a","implied_provider":"test","resource_type":"test_data_source","resource_name":"a","resource_key":null},"action":"read"},"type":"apply_start"} {"@level":"info","@message":"data.test_data_source.a: Refresh complete after 0s [id=zzzzz]","@module":"terraform.ui","hook":{"resource":{"addr":"data.test_data_source.a","module":"","resource":"data.test_data_source.a","implied_provider":"test","resource_type":"test_data_source","resource_name":"a","resource_key":null},"action":"read","id_key":"id","id_value":"zzzzz","elapsed_seconds":0},"type":"apply_complete"} {"@level":"info","@message":"test_instance.foo: Plan to create","@module":"terraform.ui","change":{"resource":{"addr":"test_instance.foo","module":"","resource":"test_instance.foo","implied_provider":"test","resource_type":"test_instance","resource_name":"foo","resource_key":null},"action":"create"},"type":"planned_change"} From 10c07f37f94b12f8035687c4559a88aa74d91bce Mon Sep 17 00:00:00 2001 From: melsonic Date: Sun, 1 Sep 2024 15:58:24 +0530 Subject: [PATCH 2/6] show deprecation warning if -state is used with plan, apply, refresh --- ;w | 211 ------------------------------------------------------------- 1 file changed, 211 deletions(-) delete mode 100644 ;w diff --git a/;w b/;w deleted file mode 100644 index 2bac7e2a9f01..000000000000 --- a/;w +++ /dev/null @@ -1,211 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: BUSL-1.1 - -package arguments - -import ( - "fmt" - "strings" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/plans" -) - -func TestParsePlan_basicValid(t *testing.T) { - testCases := map[string]struct { - args []string - want *Plan - }{ - "defaults": { - nil, - &Plan{ - DetailedExitCode: false, - InputEnabled: true, - OutPath: "", - ViewType: ViewHuman, - State: &State{Lock: true}, - Vars: &Vars{}, - Operation: &Operation{ - PlanMode: plans.NormalMode, - Parallelism: 10, - Refresh: true, - }, - }, - }, - "setting all options": { - []string{"-destroy", "-detailed-exitcode", "-input=false", "-out=saved.tfplan"}, - &Plan{ - DetailedExitCode: true, - InputEnabled: false, - OutPath: "saved.tfplan", - ViewType: ViewHuman, - State: &State{Lock: true}, - Vars: &Vars{}, - Operation: &Operation{ - PlanMode: plans.DestroyMode, - Parallelism: 10, - Refresh: true, - }, - }, - }, - "JSON view disables input": { - []string{"-json"}, - &Plan{ - DetailedExitCode: false, - InputEnabled: false, - OutPath: "", - ViewType: ViewJSON, - State: &State{Lock: true}, - Vars: &Vars{}, - Operation: &Operation{ - PlanMode: plans.NormalMode, - Parallelism: 10, - Refresh: true, - }, - }, - }, - } - - cmpOpts := cmpopts.IgnoreUnexported(Operation{}, Vars{}, State{}) - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - got, diags := ParsePlan(tc.args) - if len(diags) > 0 && diags.HasErrors() { - t.Fatalf("unexpected diags: %v", diags) - } - if diff := cmp.Diff(tc.want, got, cmpOpts); diff != "" { - t.Errorf("unexpected result\n%s", diff) - } - }) - } -} - -func TestParsePlan_invalid(t *testing.T) { - got, diags := ParsePlan([]string{"-frob"}) - if len(diags) == 0 { - t.Fatal("expected diags but got none") - } - if got, want := diags.Err().Error(), "flag provided but not defined"; !strings.Contains(got, want) { - t.Fatalf("wrong diags\n got: %s\nwant: %s", got, want) - } - if got.ViewType != ViewHuman { - t.Fatalf("wrong view type, got %#v, want %#v", got.ViewType, ViewHuman) - } -} - -func TestParsePlan_tooManyArguments(t *testing.T) { - got, diags := ParsePlan([]string{"saved.tfplan"}) - if len(diags) == 0 { - t.Fatal("expected diags but got none") - } - if got, want := diags.Err().Error(), "Too many command line arguments"; !strings.Contains(got, want) { - t.Fatalf("wrong diags\n got: %s\nwant: %s", got, want) - } - if got.ViewType != ViewHuman { - t.Fatalf("wrong view type, got %#v, want %#v", got.ViewType, ViewHuman) - } -} - -func TestParsePlan_targets(t *testing.T) { - foobarbaz, _ := addrs.ParseTargetStr("foo_bar.baz") - boop, _ := addrs.ParseTargetStr("module.boop") - testCases := map[string]struct { - args []string - want []addrs.Targetable - wantErr string - }{ - "no targets by default": { - args: nil, - want: nil, - }, - "one target": { - args: []string{"-target=foo_bar.baz"}, - want: []addrs.Targetable{foobarbaz.Subject}, - }, - "two targets": { - args: []string{"-target=foo_bar.baz", "-target", "module.boop"}, - want: []addrs.Targetable{foobarbaz.Subject, boop.Subject}, - }, - "invalid traversal": { - args: []string{"-target=foo."}, - want: nil, - wantErr: "Dot must be followed by attribute name", - }, - "invalid target": { - args: []string{"-target=data[0].foo"}, - want: nil, - wantErr: "A data source name is required", - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - got, diags := ParsePlan(tc.args) - if len(diags) > 0 { - if tc.wantErr == "" && diags.HasErrors() { - t.Fatalf("unexpected diags: %v", diags) - } else if got := diags.Err().Error(); !strings.Contains(got, tc.wantErr) { - t.Fatalf("wrong diags\n got: %s\nwant: %s", got, tc.wantErr) - } - } - if !cmp.Equal(got.Operation.Targets, tc.want) { - t.Fatalf("unexpected result\n%s", cmp.Diff(got.Operation.Targets, tc.want)) - } - }) - } -} - -func TestParsePlan_vars(t *testing.T) { - testCases := map[string]struct { - args []string - want []FlagNameValue - }{ - "no var flags by default": { - args: nil, - want: nil, - }, - "one var": { - args: []string{"-var", "foo=bar"}, - want: []FlagNameValue{ - {Name: "-var", Value: "foo=bar"}, - }, - }, - "one var-file": { - args: []string{"-var-file", "cool.tfvars"}, - want: []FlagNameValue{ - {Name: "-var-file", Value: "cool.tfvars"}, - }, - }, - "ordering preserved": { - args: []string{ - "-var", "foo=bar", - "-var-file", "cool.tfvars", - "-var", "boop=beep", - }, - want: []FlagNameValue{ - {Name: "-var", Value: "foo=bar"}, - {Name: "-var-file", Value: "cool.tfvars"}, - {Name: "-var", Value: "boop=beep"}, - }, - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - got, diags := ParsePlan(tc.args) - if len(diags) > 0 && diags.HasErrors() { - t.Fatalf("unexpected diags: %v", diags) - } - if vars := got.Vars.All(); !cmp.Equal(vars, tc.want) { - t.Fatalf("unexpected result\n%s", cmp.Diff(vars, tc.want)) - } - if got, want := got.Vars.Empty(), len(tc.want) == 0; got != want { - t.Fatalf("expected Empty() to return %t, but was %t", want, got) - } - }) - } -} From b379120f604dab01042827b075e3eb5504b53929 Mon Sep 17 00:00:00 2001 From: melsonic Date: Tue, 3 Sep 2024 23:24:35 +0530 Subject: [PATCH 3/6] updated -state flag check condition --- internal/command/apply.go | 8 ++------ internal/command/arguments/apply.go | 7 ++++--- internal/command/arguments/plan.go | 7 ++++--- internal/command/arguments/refresh.go | 7 ++++--- internal/command/testdata/plan/output.jsonlog | 1 - 5 files changed, 14 insertions(+), 16 deletions(-) diff --git a/internal/command/apply.go b/internal/command/apply.go index 705aa050fc53..d889f92345da 100644 --- a/internal/command/apply.go +++ b/internal/command/apply.go @@ -56,11 +56,6 @@ func (c *ApplyCommand) Run(rawArgs []string) int { return 1 } - if diags.HasWarnings() { - view.Diagnostics(diags) - diags = nil - } - // Check for user-supplied plugin path var err error if c.pluginPath, err = c.loadPluginPath(); err != nil { @@ -70,7 +65,8 @@ func (c *ApplyCommand) Run(rawArgs []string) int { } // Attempt to load the plan file, if specified - planFile, diags := c.LoadPlanFile(args.PlanPath) + planFile, loadPlanFileDiags := c.LoadPlanFile(args.PlanPath) + diags = diags.Append(loadPlanFileDiags) if diags.HasErrors() { view.Diagnostics(diags) return 1 diff --git a/internal/command/arguments/apply.go b/internal/command/arguments/apply.go index 2eb9830a9902..ada0a8626f1b 100644 --- a/internal/command/arguments/apply.go +++ b/internal/command/arguments/apply.go @@ -43,9 +43,6 @@ func ParseApply(args []string) (*Apply, tfdiags.Diagnostics) { } cmdFlags := extendedFlagSet("apply", apply.State, apply.Operation, apply.Vars) - if apply.State != nil { - diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) - } cmdFlags.BoolVar(&apply.AutoApprove, "auto-approve", false, "auto-approve") cmdFlags.BoolVar(&apply.InputEnabled, "input", true, "input") @@ -60,6 +57,10 @@ func ParseApply(args []string) (*Apply, tfdiags.Diagnostics) { )) } + if apply.State.StatePath != "" { + diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) + } + args = cmdFlags.Args() if len(args) > 0 { apply.PlanPath = args[0] diff --git a/internal/command/arguments/plan.go b/internal/command/arguments/plan.go index 4b4a59f7a080..63b7e1fd55e3 100644 --- a/internal/command/arguments/plan.go +++ b/internal/command/arguments/plan.go @@ -46,9 +46,6 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) { } cmdFlags := extendedFlagSet("plan", plan.State, plan.Operation, plan.Vars) - if plan.State != nil { - diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) - } cmdFlags.BoolVar(&plan.DetailedExitCode, "detailed-exitcode", false, "detailed-exitcode") cmdFlags.BoolVar(&plan.InputEnabled, "input", true, "input") cmdFlags.StringVar(&plan.OutPath, "out", "", "out") @@ -65,6 +62,10 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) { )) } + if plan.State.StatePath != "" { + diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) + } + args = cmdFlags.Args() if len(args) > 0 { diff --git a/internal/command/arguments/refresh.go b/internal/command/arguments/refresh.go index bcb827f677f6..d32fb2245e6f 100644 --- a/internal/command/arguments/refresh.go +++ b/internal/command/arguments/refresh.go @@ -34,9 +34,6 @@ func ParseRefresh(args []string) (*Refresh, tfdiags.Diagnostics) { } cmdFlags := extendedFlagSet("refresh", refresh.State, refresh.Operation, refresh.Vars) - if refresh.State != nil { - diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) - } cmdFlags.BoolVar(&refresh.InputEnabled, "input", true, "input") var json bool @@ -50,6 +47,10 @@ func ParseRefresh(args []string) (*Refresh, tfdiags.Diagnostics) { )) } + if refresh.State.StatePath != "" { + diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) + } + args = cmdFlags.Args() if len(args) > 0 { diags = diags.Append(tfdiags.Sourceless( diff --git a/internal/command/testdata/plan/output.jsonlog b/internal/command/testdata/plan/output.jsonlog index 0650b6ef0231..7f42c6eca512 100644 --- a/internal/command/testdata/plan/output.jsonlog +++ b/internal/command/testdata/plan/output.jsonlog @@ -1,5 +1,4 @@ {"@level":"info","@message":"Terraform 1.3.0-dev","@module":"terraform.ui","terraform":"1.3.0-dev","type":"version","ui":"1.0"} -{"@level":"warn","@message":"Warning: state is deprecated","@module":"terraform.ui","diagnostic":{"detail":"","severity":"warning","summary":"state is deprecated"},"type":"diagnostic"} {"@level":"info","@message":"data.test_data_source.a: Refreshing...","@module":"terraform.ui","hook":{"resource":{"addr":"data.test_data_source.a","module":"","resource":"data.test_data_source.a","implied_provider":"test","resource_type":"test_data_source","resource_name":"a","resource_key":null},"action":"read"},"type":"apply_start"} {"@level":"info","@message":"data.test_data_source.a: Refresh complete after 0s [id=zzzzz]","@module":"terraform.ui","hook":{"resource":{"addr":"data.test_data_source.a","module":"","resource":"data.test_data_source.a","implied_provider":"test","resource_type":"test_data_source","resource_name":"a","resource_key":null},"action":"read","id_key":"id","id_value":"zzzzz","elapsed_seconds":0},"type":"apply_complete"} {"@level":"info","@message":"test_instance.foo: Plan to create","@module":"terraform.ui","change":{"resource":{"addr":"test_instance.foo","module":"","resource":"test_instance.foo","implied_provider":"test","resource_type":"test_instance","resource_name":"foo","resource_key":null},"action":"create"},"type":"planned_change"} From bfa66b44bcba315fd6ad1a12ffbe14567f34cb15 Mon Sep 17 00:00:00 2001 From: melsonic Date: Wed, 4 Sep 2024 00:20:41 +0530 Subject: [PATCH 4/6] added better diagnostic details view --- internal/command/arguments/apply.go | 6 +++++- internal/command/arguments/plan.go | 8 +++++++- internal/command/arguments/refresh.go | 8 +++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/internal/command/arguments/apply.go b/internal/command/arguments/apply.go index ada0a8626f1b..a4d9462f72c3 100644 --- a/internal/command/arguments/apply.go +++ b/internal/command/arguments/apply.go @@ -58,7 +58,11 @@ func ParseApply(args []string) (*Apply, tfdiags.Diagnostics) { } if apply.State.StatePath != "" { - diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "state is deprecated", + fmt.Sprintf("use path variable of local backend instead : https://developer.hashicorp.com/terraform/language/settings/backends/local\n"), + )) } args = cmdFlags.Args() diff --git a/internal/command/arguments/plan.go b/internal/command/arguments/plan.go index 63b7e1fd55e3..9c9e7d5f19de 100644 --- a/internal/command/arguments/plan.go +++ b/internal/command/arguments/plan.go @@ -4,6 +4,8 @@ package arguments import ( + "fmt" + "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -63,7 +65,11 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) { } if plan.State.StatePath != "" { - diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "state is deprecated", + fmt.Sprintf("use path variable of local backend instead : https://developer.hashicorp.com/terraform/language/settings/backends/local\n"), + )) } args = cmdFlags.Args() diff --git a/internal/command/arguments/refresh.go b/internal/command/arguments/refresh.go index d32fb2245e6f..b8a8ac6f3b99 100644 --- a/internal/command/arguments/refresh.go +++ b/internal/command/arguments/refresh.go @@ -4,6 +4,8 @@ package arguments import ( + "fmt" + "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -48,7 +50,11 @@ func ParseRefresh(args []string) (*Refresh, tfdiags.Diagnostics) { } if refresh.State.StatePath != "" { - diags = append(diags, tfdiags.SimpleWarning("state is deprecated")) + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "state is deprecated", + fmt.Sprintf("Use the path variable of the local backend instead : https://developer.hashicorp.com/terraform/language/settings/backends/local\n"), + )) } args = cmdFlags.Args() From 49124553498a063b0533074e85c4e4be2900e3a9 Mon Sep 17 00:00:00 2001 From: melsonic Date: Wed, 4 Sep 2024 00:42:55 +0530 Subject: [PATCH 5/6] resolved failed test --- internal/command/testdata/apply/output.jsonlog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/command/testdata/apply/output.jsonlog b/internal/command/testdata/apply/output.jsonlog index a46f15d7c7d9..eb67c19891b3 100644 --- a/internal/command/testdata/apply/output.jsonlog +++ b/internal/command/testdata/apply/output.jsonlog @@ -1,5 +1,5 @@ {"@level":"info","@message":"Terraform 0.15.0-dev","@module":"terraform.ui","terraform":"0.15.0-dev","type":"version","ui":"0.1.0"} -{"@level":"warn","@message":"Warning: state is deprecated","@module":"terraform.ui","diagnostic":{"detail":"","severity":"warning","summary":"state is deprecated"},"type":"diagnostic"} +{"@level":"warn","@message":"Warning: state is deprecated","@module":"terraform.ui","diagnostic":{"detail":"use path variable of local backend instead : https://developer.hashicorp.com/terraform/language/settings/backends/local\n","severity":"warning","summary":"state is deprecated"},"type":"diagnostic"} {"@level":"info","@message":"test_instance.foo: Plan to create","@module":"terraform.ui","change":{"resource":{"addr":"test_instance.foo","module":"","resource":"test_instance.foo","implied_provider":"test","resource_type":"test_instance","resource_name":"foo","resource_key":null},"action":"create"},"type":"planned_change"} {"@level":"info","@message":"Plan: 1 to add, 0 to change, 0 to destroy.","@module":"terraform.ui","changes":{"add":1,"import":0,"change":0,"remove":0,"operation":"plan"},"type":"change_summary"} {"@level":"info","@message":"test_instance.foo: Creating...","@module":"terraform.ui","hook":{"resource":{"addr":"test_instance.foo","module":"","resource":"test_instance.foo","implied_provider":"test","resource_type":"test_instance","resource_name":"foo","resource_key":null},"action":"create"},"type":"apply_start"} From 3a7075f8b3efb88469f08ef420412c7bb831465c Mon Sep 17 00:00:00 2001 From: melsonic Date: Wed, 4 Sep 2024 22:04:24 +0530 Subject: [PATCH 6/6] updated the content of the -state flag warning --- internal/command/arguments/apply.go | 4 ++-- internal/command/arguments/plan.go | 6 ++---- internal/command/arguments/refresh.go | 6 ++---- internal/command/testdata/apply/output.jsonlog | 2 +- 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/internal/command/arguments/apply.go b/internal/command/arguments/apply.go index a4d9462f72c3..30896fd8e61e 100644 --- a/internal/command/arguments/apply.go +++ b/internal/command/arguments/apply.go @@ -60,8 +60,8 @@ func ParseApply(args []string) (*Apply, tfdiags.Diagnostics) { if apply.State.StatePath != "" { diags = diags.Append(tfdiags.Sourceless( tfdiags.Warning, - "state is deprecated", - fmt.Sprintf("use path variable of local backend instead : https://developer.hashicorp.com/terraform/language/settings/backends/local\n"), + "Deprecated flag: -state", + "Use `path` attribute within the `local` backend instead: https://developer.hashicorp.com/terraform/language/v1.10.x/settings/backends/local#path", )) } diff --git a/internal/command/arguments/plan.go b/internal/command/arguments/plan.go index 9c9e7d5f19de..e36ed978f2e2 100644 --- a/internal/command/arguments/plan.go +++ b/internal/command/arguments/plan.go @@ -4,8 +4,6 @@ package arguments import ( - "fmt" - "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -67,8 +65,8 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) { if plan.State.StatePath != "" { diags = diags.Append(tfdiags.Sourceless( tfdiags.Warning, - "state is deprecated", - fmt.Sprintf("use path variable of local backend instead : https://developer.hashicorp.com/terraform/language/settings/backends/local\n"), + "Deprecated flag: -state", + "Use `path` attribute within the `local` backend instead: https://developer.hashicorp.com/terraform/language/v1.10.x/settings/backends/local#path", )) } diff --git a/internal/command/arguments/refresh.go b/internal/command/arguments/refresh.go index b8a8ac6f3b99..b55e9e2c206a 100644 --- a/internal/command/arguments/refresh.go +++ b/internal/command/arguments/refresh.go @@ -4,8 +4,6 @@ package arguments import ( - "fmt" - "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -52,8 +50,8 @@ func ParseRefresh(args []string) (*Refresh, tfdiags.Diagnostics) { if refresh.State.StatePath != "" { diags = diags.Append(tfdiags.Sourceless( tfdiags.Warning, - "state is deprecated", - fmt.Sprintf("Use the path variable of the local backend instead : https://developer.hashicorp.com/terraform/language/settings/backends/local\n"), + "Deprecated flag: -state", + "Use `path` attribute within the `local` backend instead: https://developer.hashicorp.com/terraform/language/v1.10.x/settings/backends/local#path", )) } diff --git a/internal/command/testdata/apply/output.jsonlog b/internal/command/testdata/apply/output.jsonlog index eb67c19891b3..96ce709a8cde 100644 --- a/internal/command/testdata/apply/output.jsonlog +++ b/internal/command/testdata/apply/output.jsonlog @@ -1,5 +1,5 @@ {"@level":"info","@message":"Terraform 0.15.0-dev","@module":"terraform.ui","terraform":"0.15.0-dev","type":"version","ui":"0.1.0"} -{"@level":"warn","@message":"Warning: state is deprecated","@module":"terraform.ui","diagnostic":{"detail":"use path variable of local backend instead : https://developer.hashicorp.com/terraform/language/settings/backends/local\n","severity":"warning","summary":"state is deprecated"},"type":"diagnostic"} +{"@level":"warn","@message":"Warning: Deprecated flag: -state","@module":"terraform.ui","diagnostic":{"detail":"Use `path` attribute within the `local` backend instead: https://developer.hashicorp.com/terraform/language/v1.10.x/settings/backends/local#path","severity":"warning","summary":"Deprecated flag: -state"},"type":"diagnostic"} {"@level":"info","@message":"test_instance.foo: Plan to create","@module":"terraform.ui","change":{"resource":{"addr":"test_instance.foo","module":"","resource":"test_instance.foo","implied_provider":"test","resource_type":"test_instance","resource_name":"foo","resource_key":null},"action":"create"},"type":"planned_change"} {"@level":"info","@message":"Plan: 1 to add, 0 to change, 0 to destroy.","@module":"terraform.ui","changes":{"add":1,"import":0,"change":0,"remove":0,"operation":"plan"},"type":"change_summary"} {"@level":"info","@message":"test_instance.foo: Creating...","@module":"terraform.ui","hook":{"resource":{"addr":"test_instance.foo","module":"","resource":"test_instance.foo","implied_provider":"test","resource_type":"test_instance","resource_name":"foo","resource_key":null},"action":"create"},"type":"apply_start"}