Resource interpolation support - #1434
Merged
Merged
Conversation
jedevc
force-pushed
the
resource-interpolation
branch
from
November 29, 2022 12:31
80c290b to
ea767f1
Compare
Member
|
Looks great! Was thinking we could have a common interface for |
Signed-off-by: Justin Chadwell <me@jedevc.com>
This patch adds support for block-based interpolation, so that properties of blocks can be referenced in the current block and across other blocks. Previously, order-of-evaluation did not matter for blocks, and could be evaluated in any order. However, now that blocks can refer to each other, we split out this dynamic evaluation order into a separate resolveBlock function. Additionally, we need to support partial block evaluations - if block A refers to property X of block B, when we should only evaluate property X, and not the entire block. This ensures that we can safely evaluate blocks that refer to other properties within themselves, and allows sequences that would otherwise be co-recursive. We take special care in this logic to ensure that each property is evaluated once *and only* once - this could otherwise present inconsistencies with stateful functions, and could risk inconsistent results. Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
tonistiigi
force-pushed
the
resource-interpolation
branch
from
December 14, 2022 01:28
ea767f1 to
e51b55e
Compare
tonistiigi
reviewed
Dec 14, 2022
tonistiigi
left a comment
tonistiigi
left a comment
Member
There was a problem hiding this comment.
One thing I discovered was not handling default values. Eg. you can't do target.foo.dockerfile while if you do bake foo --print then the dockerfile value is filled in with default value.
tonistiigi
approved these changes
Dec 14, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛠️ Fixes #445.
This took a while 😄 To do this, I needed to substantially rework how we parse HCL, and hack around some of the HCL library limitations that make it more complex to perform the partial block evaluations that we need to be able to have blocks that reference themselves as in the example from the feature request:
The core of the PR reworks block resolution to be lazy, similar to how we evaluate global attributes today. Then we also need to ensure that we can perform partial evaluation correctly.
Some of the error messages aren't quite right - it seems that
hcl.Diagnosticsdon't have a wrapping behavior, so we need to detect these properly to provide good error messages in the case of typos, etc.