Fix for no json output of state locking actions for --json flag

[zetHannes]

Fixes #32265

Target Release

1.4.x

Draft CHANGELOG

BUG FIXES

• state locking : when a ´--json´ flag is passed to a command locking the state (such as apply, plan), the output of the state locker happens in json format, otherwise it is human-readable

[hashicorp-cla]

All committers have signed the CLA.

[crw]

Thanks for this submission. I will bring this to triage to see if it can be reviewed.

[liamcervante]

Hi @zetHannes, thanks for this! I think this is a good start, and your implementation of the state locker view itself is fine. I'm just not entirely comfortable with how you are parsing the ViewType from the arguments because different commands are now initialising the view in different ways. For example:

1. The apply command parses the view type itself, creates the view and then sets the view type onto the meta object which is then never referenced.
2. The other commands you've added -json to parse the flag using the common meta approach and set it from there
3. The output command takes a third approach, and parses the -json flag on it's own and never sets it to the meta object
4. Other commands that do not use views at all (such as the console command) will now start supporting the -json flag even though it has no meaning to them but simply because the meta object will happily parse and store it.

I do agree that this is in large part due to a weakness in Terraform with all the different commands parsing their flags in their own way, and it's hard to work within that framework.

Due to 4, I'd hesitate to add the -json flag to the Meta object at all, as all the commands rely on this and the -json flag is not meaningful to all of them. As a potentially easy workaround or short term fix, and to solve 1 and 2, you could instead just have each command that does support the -json flag manually add it. For example in the state_mv.go, you could copy apply.go and just do this:

	var json bool
	cmdFlags.BoolVar(&json, "json", false, "json")

I understand this is going to be duplicating code, but at least we're being explicit about exactly which command supports JSON output and what that means for them. Just for the clarity, this is required before I'd be willing to accept this PR.

If you're interested in a larger challenge, and something that might take a step to improving the inconsistencies, then we could try a larger refactor. Note, I'd be happy to accept the changes discussed above without this, and I think what I'm about to propose is substantially more work so I understand completely if you just implement the short term fix. But still, in case you are interested:

All the view implementations already accept a View object that provide some behavioural flags. We could add the ViewType to this structure, and then modify the constructors of the view types so they don't accept the ViewType separately and instead just read it from the provided views.View.

You could then modify the arguments.View struct and the arguments.ParseView function to support reading the -json flag and the -raw flag and setting the view type on the overall struct. I'd also modify the ParseView function signature: func ParseView(args[] string, supportedViewTypes ...ViewTypes) (*View, []string) { ... }. This would allow the various commands to specify exactly which view types they support, so the output command could say all three (human, raw, and json) while others just two (human and json).

The challenge is then how to merge the Meta.process function with the ParseView function. Some commands will need to do both of these. One option might be to have process call the ParseView function directly, and just do all the config. Or we could create a new process function (processFromView or something) that accepts the arguments.View struct, and can copy the relevant data across into the meta. And we can make commands be explicit about which process function they call based on whether that have to support the views or not.

There's probably going to be challenges I've missed, or something that will require additional thinking, but I think with something like that above we can make it so that every command is both being explicit about what kind of formats they support, and they are all sharing the same approach to parsing the UI arguments.

If you're interested in this, I'd be happy to answer any questions and can do further reviews. If not, I'd be more than happy to accept the modified short term fix I proposed first, which shouldn't be too difficult.

[zetHannes]

Hello. I am sorry for not responding for a time, currently I have a lot of stress to deal with, this is going to be until next Saturday at least. But thank you very much for the input, those will be refactoring challenges, currently I'd suggest I adapt the code to fix the short term issue and then further changes can be planned accordingly.

[zetHannes]

Finally I have got enough time now!
So, as far as I'm concerned, the only commands using JSON mode are

• output
• show
• validate
• version
• apply

You did mention the state_mv command, I guess that was more an example on using cmdline flags? Or do we also want to support JSON output for other commands?

[liamcervante]

Yes, you're correct. We shouldn't actually be adding the JSON flag anywhere new as part of this change. I hadn't considered this in my previous comment which is why I used that example. You just need to make sure that the correct ViewType is propagated onto the state locker for the commands that already support the JSON flag.

I think there are more commands than you listed, here is my list:

• apply
• output
• plan
• providers_schema
• refresh
• show
• validate
• version

But not all of these lock the state, so you will only need to pass the view type around for some of these commands. Of the ones that do lock the state file they do so through the Meta.Operation function, so you could modify that function to accept the view type alongside the backend as an argument. Any command that doesn't support the JSON flag can just pass in Human, and any flag that does can pass in the ViewType they retrieved from the flag when the parsed it.

Hopefully that all makes sense!

[liamcervante]

Looking good, just a couple of small nits.

[zetHannes]

Another topic was popping up which is the initialization process of the Backend. I did add the viewtype as a field of BackendOpts/ backendMigrateOpts so the viewtype can be passed to the Backend function when needed. If not set, we use Human mode (so it works just like a command line flag that may be present or not). That way most of the code calling the Meta.Backend function stays the same (the function is needed for all commands), just the commands supporting json have that extra viewType argument.

[liamcervante]

New changes all look fine, just still some other unwanted changes left over from the first attempt.

[liamcervante]

There's still some unwanted changes left over in this file.

[liamcervante]

and here as well

[zetHannes]

Oh, sorry about that, I misread something here. Of course, that should have been erased as well already.

[liamcervante]

All looks good, I'll merge this and update the changelog and we'll see the fix in v1.4.0!

Thanks for your contribution! Much appreciated.

[github-actions]

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

[github-actions]

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.