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

fix: Wrap TF stdout and stderr in JSON #3602

Merged
merged 21 commits into from
Nov 27, 2024
Merged

fix: Wrap TF stdout and stderr in JSON #3602

merged 21 commits into from
Nov 27, 2024

Conversation

levkohimins
Copy link
Contributor

@levkohimins levkohimins commented Nov 25, 2024

Description

Fixes #3603
Fixes #3540

Deprecate the --terragrunt-tf-logs-to-json flag. TF stdout and stderr will be wrapped in JSON by default if --terragrunt-forward-tf-stdout flag is not specified.

TODOs

Read the Gruntwork contribution guidelines.

  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.
  • Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added / Removed / Updated [X].

Migration Guide

@levkohimins levkohimins marked this pull request as ready for review November 27, 2024 13:46
// If the Terraform error matches `httpStatusCacheProviderReg` we ignore it and hide the log from users, otherwise we process the error as is.
if output, err := shell.RunTerraformCommandWithOutput(ctx, cloneOpts, cloneOpts.TerraformCliArgs...); err != nil && len(errWriter.Msgs()) == 0 {
return output, err
if err != nil && httpStatusCacheProviderReg.Match(output.Stderr.Bytes()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice 👍🏽

@@ -56,7 +56,7 @@ Placeholders have preset names:

* `%prefix` - Path to the working directory were Terragrunt is running.

* `%tfpath` - Path to the OpenTofu/Terraform executable (as defined by [terragrunt-tfpath](https://terragrunt.gruntwork.io/docs/reference/cli-options/#terragrunt-tfpath)).
* `%tf-path` - Path to the OpenTofu/Terraform executable (as defined by [terragrunt-tfpath](https://terragrunt.gruntwork.io/docs/reference/cli-options/#terragrunt-tfpath)).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make sure to call out this change in the release notes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will call this out. Thanks.

@@ -83,45 +85,49 @@ type Controls map[string]Control
//nolint:lll,gochecknoglobals,stylecheck
var StrictControls = Controls{
SpinUp: {
Error: errors.Errorf("The `%s` command is no longer supported. Use `terragrunt run-all apply` instead.", SpinUp),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because this error is user facing, and it's more likely to involve multiple sentences don't you think it's better to have them be full sentences? If so, we should just add a nolint for whatever made you adjust this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alternatively, maybe you know a special way to convert the error messages to proper casing before they are reported to users?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I did what you suggested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, maybe you know a special way to convert the error messages to proper casing before they are reported to users?

Ah no, I don't know of any ready-made function that could do this, only manual message conversion, but I don't think it's worth it.

@levkohimins levkohimins merged commit 8e65d1e into main Nov 27, 2024
5 checks passed
@levkohimins levkohimins deleted the fix/tf-logs-json branch November 27, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants