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

Add --terragrunt-include-module-prefix option #2493

Merged
merged 3 commits into from
Apr 3, 2023

Conversation

maciasello
Copy link
Contributor

@maciasello maciasello commented Mar 20, 2023

Description

Fixes #1194 .

TODOs

Read the Gruntwork contribution guidelines.

  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added --terragrunt-include-module-prefix flag to include module dir prefix in Terraform output

Migration Guide

@maciasello
Copy link
Contributor Author

This PR adds a flag to enable the prepending of prefixes.

Also it may be reasonable to trim the prefix to be relative to a common parent of the modules or to cwd (or other place) - please guide me what is the suggested approach. In logs prefix key is an absolute path - I started from that.

pf.beginningOfANewLine = false
}

buf.WriteByte(b)
Copy link
Member

Choose a reason for hiding this comment

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

I was wondering why output should be buffered and not directly written to pf.writer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not want to call the underlying pf.writer for each character as it may degrade performance.

This buffering does not cross single .Write() call span so it will not create a "typical" buffered i/o effect.

Still, it needs an additional memory and one copying of the input []byte. If you feel there is more efficient way I would be more than happy to apply it.

**CLI Arg**: `--terragrunt-include-module-prefix`
**Environment Variable**: `TERRAGRUNT_INCLUDE_MODULE_PREFIX` (set to `true`)

When this flag is set output from Terraform sub-commands is prefixed with module name.
Copy link
Member

Choose a reason for hiding this comment

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

Actually, output include module path, not module name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected the doc.


### terragrunt-include-module-prefix

**CLI Arg**: `--terragrunt-include-module-prefix`
Copy link
Member

Choose a reason for hiding this comment

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

Noticed that terragrunt-include-module-prefix is not listed in the help output

$ terragrunt --help | grep terragrunt-include-module-prefix
$

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Included the info in help output. Is there any other place the help should be updated?

@maciasello maciasello force-pushed the dev/mligenza/prefix branch 3 times, most recently from 51427d4 to feed3c4 Compare March 27, 2023 09:55
@maciasello
Copy link
Contributor Author

Added tests for PrefixWriter for a complete picture. Somehow missed that initially.

@maciasello maciasello requested review from denis256 and removed request for zackproser March 27, 2023 09:57
@denis256
Copy link
Member

[INFO] Initializing environment for https://github.com/gruntwork-io/pre-commit.
Terraform fmt............................................................Passed
goimports................................................................Failed
- hook id: goimports
- files were modified by this hook

util/prefix-writer_test.go

@maciasello
Copy link
Contributor Author

[INFO] Initializing environment for https://github.com/gruntwork-io/pre-commit.
Terraform fmt............................................................Passed
goimports................................................................Failed
- hook id: goimports
- files were modified by this hook

util/prefix-writer_test.go

Fixed.

@denis256 denis256 merged commit 6d48a85 into gruntwork-io:master Apr 3, 2023
@lorengordon
Copy link
Contributor

lorengordon commented Apr 3, 2023

Also it may be reasonable to trim the prefix to be relative to a common parent of the modules or to cwd (or other place) - please guide me what is the suggested approach. In logs prefix key is an absolute path - I started from that.

Thanks! Where did this end up? Is the prefix the absolute path, or a relative path of some kind? I didn't see an example in the changed files...

@lorengordon
Copy link
Contributor

Tested to find the answer myself, it is the absolute path...

@dhirschfeld
Copy link

Tested to find the answer myself, it is the absolute path...

Yeah, it's the absolute path:

[/home/sysop/code/github/dhirschfeld/azure-infra-test/terraform/dev/network]

That takes up a lot of screen real-estate. It would be great if the path could be relative to the cwd - i.e. running in the dev folder that would be:

[./network]

szesch pushed a commit to szesch/terragrunt that referenced this pull request May 23, 2023
* refactor: modularize testCommandOutputOrder for easier reuse

* feat: prepend TerragruntOptions.OutputPrefix to subcommand's output

* feat: add TerragruntOptions.IncludeModulePrefix
@AlDemion
Copy link

AlDemion commented Jun 3, 2023

As a workaround you can utilize something like this
terragrunt run-all plan -lock=false --terragrunt-non-interactive | sed "s#$(pwd)/##g"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to prefix *-all terraform output with working-dir label
5 participants