-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Introduce concept of merge strategy for includes #1724
Conversation
0d756e6
to
f69e4da
Compare
f8c48d7
to
815da32
Compare
815da32
to
5f18f90
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -203,6 +204,29 @@ func (cfg *IncludeConfig) GetExpose() bool { | |||
return *cfg.Expose | |||
} | |||
|
|||
func (cfg *IncludeConfig) GetMergeStrategy() (MergeStrategyType, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary because you can't set the type of MergeStrategy
to *MergeStrategyType
in IncludeConfig
? I guess the HCL parser doesn't know how to parse a string into that type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup... unfortunately.
Thanks for review! Will merge now. |
This adds a new attribute
merge_strategy
toinclude
blocks, allowing you to specify aninclude
block without merging the config in. This can be used in conjunction with the newexpose
attribute to simulateread_terragrunt_config
for certain use cases. Note that due to the limitation of single include blocks, it is not a full replacement forread_terragrunt_config
(yet).This PR also refactors the merge functionality of the config into a new
include.go
file. This is in preparation for the next PR after this one, which will introduce deep merge strategy.