Deep merge strategy #1759
Deep merge strategy #1759
Conversation
| if sourceConfig.RetryableErrors != nil { | ||
| targetConfig.RetryableErrors = append(targetConfig.RetryableErrors, sourceConfig.RetryableErrors...) | ||
| } |
There was a problem hiding this comment.
Does this work OK if targetConfig.RetryableErrors is nil?
There was a problem hiding this comment.
Yes. Append handles nil correctly. See https://play.golang.org/p/5RRq6Owj_sS
| // MAINTAINER'S NOTE: The following structs cannot be deep merged due to an implementation detail (they do not | ||
| // support nil attributes, so we can't determine if an attribute was intentionally set, or was defaulted from | ||
| // unspecified - this is especially problematic for bool attributes). |
There was a problem hiding this comment.
Ah, I see... So in the future, we'd need to update these to support nil?
There was a problem hiding this comment.
Yea because we can't tell if a false bool attribute was intentionally set in the code in the child terragrunt.hcl config, or if it was because it was unspecified. If unspecified, then we want to inherit from the parent, but if it was intentionally set, then it needs to override the parent. Since the resulting behavior is different, it's important that we can differentiate the two cases.
I decided to avoid addressing this for now in this PR since it was already getting big, but we will definitely want to address this as it feels limiting to not be able to deep merge an important block like remote_state.
| @@ -393,6 +394,63 @@ inputs = { | |||
| } | |||
| ``` | |||
|
|
|||
| **What is deep merge?** | |||
There was a problem hiding this comment.
NIT: consider adding an example showing two terragrunt.hcl files and what a merged output would look like.
There was a problem hiding this comment.
Added example: #1764
| When the `merge_strategy` for the `include` block is set to `deep`, Terragrunt will perform a deep merge of the included | ||
| config. For Terragrunt config, deep merge is defined as follows: | ||
|
|
||
| - For simple types, the source overrides the target. |
There was a problem hiding this comment.
NIT: What is source and what is target? Would help to clarify, perhaps with the example mentioned above.
There was a problem hiding this comment.
Switched to child and parent language: #1764
| Finally, `dependency` blocks have special treatment. When doing a `deep` merge, `dependency` blocks from **both** child | ||
| and parent config are accessible in **both** places. For example, consider the following setup: |
There was a problem hiding this comment.
I don't really have a good answer to this other than that it naturally happened when I coded the deep merge. I actually realized this fact when I started testing it.
I remember chasing the reasoning when I realized it during implementation, but it's been about 2 months since I coded this up so I have forgotten it now...
|
Thanks for review! I'll merge now, and then open a follow up PR with the docs nit fixes. |
This implements deep merge strategy for
include. Refer to the updated docs for more info on what this means.