Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

terraform init -json #34886

Merged
merged 13 commits into from
Apr 17, 2024
Merged

terraform init -json #34886

merged 13 commits into from
Apr 17, 2024

Conversation

Uk1288
Copy link
Contributor

@Uk1288 Uk1288 commented Mar 26, 2024

This PR makes changes to allow the support of json output for terraform init cmd. The output would be similar to that generated by terraform plan -json.

New command option: terraform init -json

NOTE: since terraform cannot ask for interactive approval when -json is set, an error is returned if both -migrate-state and -json options are set

Target Release

1.8.x

Draft CHANGELOG entry

NEW FEATURES

  • terraform init -json: Displays machine readable format for terraform init command.

Human View (terraform init ) - success

Screenshot 2024-03-28 at 1 20 19 AM

JSON View (terraform init -json) - success

Screenshot 2024-04-05 at 1 32 41 AM

Human View (terraform init ) - with error

Screenshot 2024-03-28 at 1 23 50 AM

JSON View (terraform init -json) - with error

Screenshot 2024-04-05 at 5 32 10 PM

terraform init -help

Screenshot 2024-03-28 at 1 52 19 AM

@@ -63,6 +65,8 @@ func (c *InitCommand) Run(args []string) int {
cmdFlags.StringVar(&flagLockfile, "lockfile", "", "Set a dependency lockfile mode")
cmdFlags.BoolVar(&c.Meta.ignoreRemoteVersion, "ignore-remote-version", false, "continue even if remote and local Terraform versions are incompatible")
cmdFlags.StringVar(&testsDirectory, "test-directory", "tests", "test-directory")
cmdFlags.BoolVar(&flagJson, "json", false, "json")

cmdFlags.Usage = func() { c.Ui.Error(c.Help()) }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice that few instances of c.Ui.Error still exists in peculiar cases, e.g

  • where errors are caught before the view struct is even instantiated, (lines#70)
  • in cases where an error is appended to diagnostics & also printed with colourize

Any thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean that no json output will get printed in those cases, only the colorized error, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, however I am now revising this to display exclusively json or human format depending on the view type

Copy link
Contributor

@sebasslash sebasslash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work so far, diagnostics render as expected ✅ A few thoughts for discussion as I'm smoke testing the changes:

  • Specifying the -json flag still renders human readable output, which seems like a gotcha if some process is expecting to be able to parse output as JSON. Should we consider mapping init's output to JSON structs under views/json? This might be a tall ask given just how much machinery operates under terraform init. One example I can think of would be an init_summary message type:
{
  "@level":"info",
  "@message":"Terraform Cloud has been successfully initialized!",
  "@module":"terraform.ui",
  "@timestamp":"2024-04-01T10:24:56.597166-04:00",
  // maybe include some metadata about the backend
  "cloud": {
    "organization": "my_org",
    "hostname": "app.terraform.io",
    "workspace": {
      "name": "my_workspace"
    },
  },
  "type":"init_summary"
}
  • We should error when init -json is attempting to perform a state migration. Similarly apply -json will prevent an apply from starting unless the user specifies -auto-approve:
{"@level":"error","@message":"Error: Plan file or auto-approve required","@module":"terraform.ui","@timestamp":"2024-04-01T10:30:35.770211-04:00","diagnostic":{"severity":"error","summary":"Plan file or auto-approve required","detail":"Terraform cannot ask for interactive approval when -json is set. You can either apply a saved plan file, or enable the -auto-approve option."},"type":"diagnostic"}

We currently don't support non-interactive state migrations AFAIK.

@uturunku1
Copy link
Contributor

Just curious, how did you get that output shown in your "JSON View (terraform init -json) - with error" screenshot?
When I reproduce that same error, I get a different output:

Screenshot 2024-04-01 at 9 17 48 PM

@Uk1288
Copy link
Contributor Author

Uk1288 commented Apr 2, 2024

Just curious, how did you get that output shown in your "JSON View (terraform init -json) - with error" screenshot? When I reproduce that same error, I get a different output:

Screenshot 2024-04-01 at 9 17 48 PM

To get that error, I added both cloud and backend config initializers in the same main.tf file.

@uturunku1
Copy link
Contributor

@Uk1288 I don't think you were able to upload the screenshot you intended to. But , anyways, I also had added both cloud and backend config initializers in the same main.tf file, and yet I got a different output than yours 😄 . That's why I was confused.

Copy link
Contributor

@alisdair alisdair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! I think the approach you're taking is solid. I have a few inline comments about improving the JSON output slightly, and some other minor tweaks, but otherwise this looks good to me.

internal/command/init.go Outdated Show resolved Hide resolved
internal/command/init.go Outdated Show resolved Hide resolved
internal/command/init.go Outdated Show resolved Hide resolved
internal/command/views/init.go Outdated Show resolved Hide resolved
internal/command/views/init.go Outdated Show resolved Hide resolved
{"@level":"info","@message":"Terraform 1.9.0-dev","@module":"terraform.ui","terraform":"1.9.0-dev","type":"version","ui":"1.2"}
{"@level":"info","@message":"Initializing the backend...","@module":"terraform.ui","type":"init_output"}
{"@level":"info","@message":"Initializing modules...","@module":"terraform.ui","type":"init_output"}
{"@level":"info","@message":"- foo in foo","@module":"terraform.ui","type":"log"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to have more useful semantic output for module and provider installations, so that clients could build useful UI on top of it. Not necessary for this PR, but if you can sketch out how that might work in the PR description it'd help make sure we're not painting ourselves into a corner.

internal/command/testdata/init-get/output.jsonlog Outdated Show resolved Hide resolved
Copy link
Contributor

@alisdair alisdair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good to me! 🎉 I'm interested to see if you can easily get more semantic information out of the few remaining log calls from the module/provider installers, but even if not this is a big improvement to machine readability of init output.

@Uk1288 Uk1288 merged commit d956ed4 into main Apr 17, 2024
10 checks passed
Copy link

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

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants