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

Prevent duplicate plan output for TFE releases that don't support SRO for CLI #33018

Merged
merged 3 commits into from
Apr 14, 2023

Conversation

sebasslash
Copy link
Contributor

A customer reported seeing duplicate plan output when using Terraform v1.4.x against a TFE release that did not support enabling structured logging for runs created via the CLI. They see duplicate plan output because these releases will silently disable the setting when a run is created, resulting in plan output being written in human-readable format to the streamed log file. The only requirement to call the new JSON plan renderer (which generates plan output) was that the remote workspace needs to have the structured run output setting enabled. Therefore, plan output would be read from the streamed log file and subsequently re-generated by the plan renderer.

In short, this fix makes the check "TFE-aware". The requirement to call the JSON plan renderer for a cloud backend configured against TFE is:

  • If it is a knowable TFE version (i.e, TFE releases that include the X-TFE-Version header), then the release must be equal to or later than 202302-1. This is the release version that adds SRO support for CLI-driven runs.
  • If it is an unknowable version, we automatically know SRO will be silently disabled. This would indicate at the very least a version that predates the X-TFE-Version change.

NOTE: The check still requires structured run output to be enabled on a TFE workspace for the renderer to be called.

Target Release

1.4.5

Draft CHANGELOG entry

BUG FIXES

  • cloud: Fixes duplicate plan output for TFE releases that don't support enabling structured logging for CLI runs.

Testing

In order to smoke test this, you will need a TFE release older than 202302-1.

  1. Make sure you're using a release version of Terraform v1.4, tfenv install 1.4.0 && tfenv use 1.4.0
  2. Create any config. Make sure you've configured the cloud backend against the TFE installation. And terraform plan (your workspace, if created on terraform init, should have SRO enabled by default).
  3. You should see duplicate plan output.
  4. Great! Now checkout this branch locally and go install. Run a plan using the installed binary: $GOPATH/bin/terraform plan
  5. You should no longer see duplicate plan output.
  6. Feel free to test this against a release of TFE that supports SRO for CLI runs or TFC. Both cases should not yield duplicate plan output.

@sebasslash
Copy link
Contributor Author

Heads up! This PR also contains the dependency upgrades that fix #31895 as well as fixes errors when applying from within a symlinked working directory.

@liamcervante
Copy link
Member

liamcervante commented Apr 13, 2023

First glance looks good! Will take a deeper soon, but could you do a flyby fix of the type in the error message that was highlighted?

Edit, this typo:
Warning: This plan was generated using a different version of Terraform, the diff presented here **maybe** missing representations of recent features.

Copy link
Member

@liamcervante liamcervante left a comment

Choose a reason for hiding this comment

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

Looks good to me, pending fixing the error message!

@sebasslash
Copy link
Contributor Author

Will wait for an approval from @Mralbert93 before merging 👍

@Mralbert93
Copy link
Member

The plan succeeded without the warning or duplicate plan output, but it looks like there might be a slight issue parsing the TFE version. Please see below:

Matthew Albert@Hashi86BSTQ3 MINGW64 ~/go/bin
$ ./terraform.exe plan 
Running plan in Terraform Cloud. Output will stream here. Pressing Ctrl-C
will stop streaming the logs, but will not stop the plan running remotely.

Preparing the remote plan...

To view this run in a browser, visit:
https://set-salmon.tf-support.hashicorpdemo.com/app/test/test/runs/run-62pLnMmkkn7gd6Je

Waiting for the plan to start...

Terraform v1.4.2
on linux_amd64
Configuring remote state backend...
Initializing Terraform configuration...
null_resource.test: Refreshing state... [id=8623903424720925447]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration
and found no differences, so no changes are needed.

│ Error: strconv.Atoi: parsing "v202211": invalid syntax
│
│

@sebasslash

@Mralbert93
Copy link
Member

Mralbert93 commented Apr 14, 2023

LGTM now 👍

Matthew Albert@Hashi86BSTQ3 MINGW64 ~/go/bin
$ ./terraform.exe version
Terraform v1.5.0-dev
on windows_amd64
+ provider registry.terraform.io/hashicorp/null v3.2.1

Matthew Albert@Hashi86BSTQ3 MINGW64 ~/go/bin
$ ./terraform.exe plan
Running plan in Terraform Cloud. Output will stream here. Pressing Ctrl-C 
will stop streaming the logs, but will not stop the plan running remotely.

Preparing the remote plan...

To view this run in a browser, visit:
https://set-salmon.tf-support.hashicorpdemo.com/app/test/test/runs/run-5NPA2ENJnRw3iNj3

Waiting for the plan to start...

Terraform v1.4.2
on linux_amd64
Configuring remote state backend...
Initializing Terraform configuration...
null_resource.test: Refreshing state... [id=8623903424720925447]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration
and found no differences, so no changes are needed.

@sebasslash

@sebasslash sebasslash merged commit e6c3aab into main Apr 14, 2023
@github-actions
Copy link

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

@Mralbert93
Copy link
Member

I think this missed the deadline for 1.4.5. Will this ship out in 1.4.6?

@liamcervante
Copy link
Member

Yes, let me backport it now to make sure.

liamcervante added a commit that referenced this pull request Apr 17, 2023
* Refactor SRO check to prevent duplicate plan output

* Unit tests to ensure renderer is appropriately called

* Fix typo in format version check

---------

Co-authored-by: Sebastian Rivera <sebas.rivera0047@gmail.com>
Co-authored-by: Sebastian Rivera <sebastian.rivera@hashicorp.com>
@github-actions
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, 2023
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.

None yet

3 participants