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

Implement --terragrunt-source-map #1674

Merged
merged 3 commits into from
May 15, 2021
Merged

Conversation

yorinasub17
Copy link
Contributor

Fixes #1138

This implements the --terragrunt-source-map feature described in the ticket linked above. Note the following differences with the issue description:

  • --terragrunt-source-map uses standard unix CLI kv definition. That is, it uses --terragrunt-source-map source=dest instead of --terragrunt-source-map source:dest.
  • Does not implement file based configuration. The file based configuration can be implemented as an improvement in a future PR if there is demand.

@yorinasub17 yorinasub17 requested a review from brikis98 as a code owner May 14, 2021 15:14
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Very nice!

Just a few questions in the PR. Also, two more here:

  1. Does it make sense to deprecate --terragrunt-source?
  2. Should we support an env var like TERRAGRUNT_SOURCE_MAP?

config/config.go Show resolved Hide resolved
docs/_docs/04_reference/cli-options.md Outdated Show resolved Hide resolved
@yorinasub17
Copy link
Contributor Author

Does it make sense to deprecate --terragrunt-source?

Let's wait a little bit for --terragrunt-source-map to get some "in the wild" usage. I am worried there are a few corner cases that are covered by --terragrunt-source that --terragrunt-source-map doesn't cover.

Should we support an env var like TERRAGRUNT_SOURCE_MAP?

What would be the syntax for specifying multiple source mappings via env var?

@brikis98
Copy link
Member

Does it make sense to deprecate --terragrunt-source?

Let's wait a little bit for --terragrunt-source-map to get some "in the wild" usage. I am worried there are a few corner cases that are covered by --terragrunt-source that --terragrunt-source-map doesn't cover.

That's a good point.

Should we support an env var like TERRAGRUNT_SOURCE_MAP?

What would be the syntax for specifying multiple source mappings via env var?

Delimiter separated list? The delimiter could be space or semicolon or something else not commonly found in URLs.

Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Impl LGTM! Feel free to make the call on env var support (or add in another PR or file issue to track).

@yorinasub17
Copy link
Contributor Author

Thanks for review! I'll go ahead and merge this in + release it, and then open a new PR for the env var fix.

@yorinasub17 yorinasub17 merged commit 9e59ad5 into master May 15, 2021
@yorinasub17 yorinasub17 deleted the yori-terragrunt-sourcemap branch May 15, 2021 05:03
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.

Add support for --terragrunt-source-map as a more consistent, predictable way to override source URLs
2 participants