show deprecation warning if -state is used with plan, apply, refresh (#…

…35660) * show deprecation warning if -state is used with plan, apply, refresh * show deprecation warning if -state is used with plan, apply, refresh * updated -state flag check condition * added better diagnostic details view * resolved failed test * updated the content of the -state flag warning
hashicorp · Sep 5, 2024 · 2a9a8c2 · 2a9a8c2
1 parent e312ffc
commit 2a9a8c2
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 13 deletions.
diff --git a/internal/command/apply.go b/internal/command/apply.go
@@ -65,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
@@ -57,6 +57,14 @@ func ParseApply(args []string) (*Apply, tfdiags.Diagnostics) {
 		))
 	}
 
+	if apply.State.StatePath != "" {
+		diags = diags.Append(tfdiags.Sourceless(
+			tfdiags.Warning,
+			"Deprecated flag: -state",
+			"Use `path` attribute within the `local` backend instead: https://developer.hashicorp.com/terraform/language/v1.10.x/settings/backends/local#path",
+		))
+	}
+
 	args = cmdFlags.Args()
 	if len(args) > 0 {
 		apply.PlanPath = args[0]

diff --git 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
@@ -62,6 +62,14 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) {
 		))
 	}
 
+	if plan.State.StatePath != "" {
+		diags = diags.Append(tfdiags.Sourceless(
+			tfdiags.Warning,
+			"Deprecated flag: -state",
+			"Use `path` attribute within the `local` backend instead: https://developer.hashicorp.com/terraform/language/v1.10.x/settings/backends/local#path",
+		))
+	}
+
 	args = cmdFlags.Args()
 
 	if len(args) > 0 {

diff --git 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
@@ -47,6 +47,14 @@ func ParseRefresh(args []string) (*Refresh, tfdiags.Diagnostics) {
 		))
 	}
 
+	if refresh.State.StatePath != "" {
+		diags = diags.Append(tfdiags.Sourceless(
+			tfdiags.Warning,
+			"Deprecated flag: -state",
+			"Use `path` attribute within the `local` backend instead: https://developer.hashicorp.com/terraform/language/v1.10.x/settings/backends/local#path",
+		))
+	}
+
 	args = cmdFlags.Args()
 	if len(args) > 0 {
 		diags = diags.Append(tfdiags.Sourceless(

diff --git 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
@@ -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: 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"}