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

melsonic · 2024-09-01T10:44:24Z

This PR adds the functionality of showing a deprecation warning when -state is used along with plan, apply, and refresh. So, to add this functionality, I pushed a warning saying state is deprecated in the diags slice if the state field is not nil in all those three files i.e. plan.go, apply.go and refresh.go.

Fixes #35562

Target Release

1.8.x

Draft CHANGELOG entry

shows deprecation warning for the -state flag when used with plan, apply, and refresh.

ENHANCEMENTS

terraform plan -state=mstate.tfstate shows **Warning**: state is deprecated.
terraform refresh -state=mstate.tfstate shows **Warning**: state is deprecated.
terraform apply -state=mstate.tfstate shows **Warning**: state is deprecated.

Output

hashicorp-cla-app · 2024-09-01T10:44:36Z

All committers have signed the CLA.

liamcervante · 2024-09-02T09:07:32Z

internal/command/apply.go

+  if diags.HasWarnings() {
+    view.Diagnostics(diags)
+    diags = nil
+  }
+


I don't think we need this, the warning should still be printed whenever the diagnostics are chosen to be delayed.

Is there something else that is dropping / ignoring the warning diagnostics later?

Actually, it is required to print the diagnostics before, since on line number 73 planFile, diags := c.LoadPlanFile(args.PlanPath) returns a new diagnostic slice, it doesn't append to the already available diags variable.

Cool, I think we can change the other part instead of doing this 👍 It's more in keeping with the way the diagnostics are intended to work to just gather them up and report once.

You can change line 73 to something like:

planFile, moreDiags := c.LoadPlanFile(args.PlanPath) diags = diags.Append(moreDiags) if moreDiags.HasErrors() { view.Diagnostics(diags) return 1 }

Cool, I think we can change the other part instead of doing this 👍 It's more in keeping with the way the diagnostics are intended to work to just gather them up and report once.

You can change line 73 to something like:

planFile, moreDiags := c.LoadPlanFile(args.PlanPath) diags = diags.Append(moreDiags) if moreDiags.HasErrors() { view.Diagnostics(diags) return 1 }

Yeah, this can be done.

liamcervante · 2024-09-02T09:16:50Z

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"))


It'd be nice if this error message could present an alternative instead.

The correct thing to do is update the configuration to point to the chosen state file: #35562 (comment)

Maybe we could make this a proper diagnostic with the detail stating to use the path variable in the local backend?

Sure, will update and add a commit. And also, I have found a better way to check if the state flag is specified, will add this also in the next commit.

Hi @liamcervante

Does this align with your thoughts? I also added the link to the resource for easier navigation.

liamcervante

Thanks! Looking good, just some recommendations over the content of the error!

liamcervante · 2024-09-04T07:13:45Z

internal/command/arguments/apply.go

+			"state is deprecated",
+			fmt.Sprintf("use path variable of local backend instead : https://developer.hashicorp.com/terraform/language/settings/backends/local\n"),


Suggested change

"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",

fmt.Sprintf("Use `path` attribute within the `local` backend instead: https://developer.hashicorp.com/terraform/language/v1.10.x/settings/backends/local#path"),

Just a couple of nits over the language here.

I'd want to call out that it's the flag specifically that is deprecated.

Also, the documentation can change over time so I think it's better for us to link to the v1.10.x documentation directly given we're adding this deprecation warning for the 1.10 release. That way if we change the local backend in the future, people using this version will get a direct link to something that makes sense.

Can you copy these changes over to the other places we raise the warning?

Thanks!

Yeah, that makes sense.
Thanks

liamcervante · 2024-09-04T07:14:57Z

Oh yeah, the consistency checks make the point that you don't need to use Sprintf for the detail part of the error since the string is constant.

liamcervante

Nice, looks good! Will merge after the checks have finished!

github-actions · 2024-09-05T08:55:49Z

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

liamcervante · 2024-09-05T09:01:00Z

Merged, and CHANGELOG updated. Will be released as part of 1.10.0, thanks for your contribution!

melsonic · 2024-09-05T16:24:20Z

Merged, and CHANGELOG updated. Will be released as part of 1.10.0, thanks for your contribution!

Thanks for your guidance throughout the PR :)

github-actions · 2024-10-06T02:15:29Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

melsonic added 2 commits September 1, 2024 15:55

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

80b184e

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

10c07f3

liamcervante reviewed Sep 2, 2024

View reviewed changes

crw added the enhancement label Sep 3, 2024

melsonic added 3 commits September 3, 2024 23:24

updated -state flag check condition

b379120

added better diagnostic details view

bfa66b4

resolved failed test

4912455

liamcervante reviewed Sep 4, 2024

View reviewed changes

updated the content of the -state flag warning

3a7075f

liamcervante approved these changes Sep 5, 2024

View reviewed changes

vercel bot deployed to Preview September 5, 2024 08:46 View deployment

liamcervante merged commit 2a9a8c2 into hashicorp:main Sep 5, 2024
6 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Oct 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

melsonic commented Sep 1, 2024 •

edited

Loading

hashicorp-cla-app bot commented Sep 1, 2024 •

edited

Loading

liamcervante Sep 2, 2024

melsonic Sep 3, 2024

liamcervante Sep 3, 2024

melsonic Sep 3, 2024

liamcervante Sep 2, 2024

melsonic Sep 3, 2024

melsonic Sep 3, 2024 •

edited

Loading

liamcervante left a comment

liamcervante Sep 4, 2024

melsonic Sep 4, 2024

liamcervante commented Sep 4, 2024

liamcervante left a comment

github-actions bot commented Sep 5, 2024

liamcervante commented Sep 5, 2024

melsonic commented Sep 5, 2024

github-actions bot commented Oct 6, 2024

		"state is deprecated",
		fmt.Sprintf("use path variable of local backend instead : https://developer.hashicorp.com/terraform/language/settings/backends/local\n"),

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

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

Conversation

melsonic commented Sep 1, 2024 • edited Loading

Target Release

Draft CHANGELOG entry

ENHANCEMENTS

Output

hashicorp-cla-app bot commented Sep 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

melsonic Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

liamcervante left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liamcervante commented Sep 4, 2024

liamcervante left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 5, 2024

liamcervante commented Sep 5, 2024

melsonic commented Sep 5, 2024

github-actions bot commented Oct 6, 2024

melsonic commented Sep 1, 2024 •

edited

Loading

hashicorp-cla-app bot commented Sep 1, 2024 •

edited

Loading

melsonic Sep 3, 2024 •

edited

Loading