Skip to content

Conversation

@jedevc
Copy link
Collaborator

@jedevc jedevc commented Nov 29, 2022

🛠️ 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:

target "front" {
  context = target.front.name
  tags    = [target.front.context]
}

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.Diagnostics don't have a wrapping behavior, so we need to detect these properly to provide good error messages in the case of typos, etc.

@jedevc jedevc added kind/enhancement New feature or request area/bake labels Nov 29, 2022
@jedevc jedevc force-pushed the resource-interpolation branch from 80c290b to ea767f1 Compare November 29, 2022 12:31
@tonistiigi tonistiigi added this to the v0.10 milestone Nov 30, 2022
@crazy-max
Copy link
Member

Looks great! Was thinking we could have a common interface for Blocks, Attributes and maybe Validators if we implement this in the future: #1346 (comment)

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 <[email protected]>
@tonistiigi tonistiigi force-pushed the resource-interpolation branch from ea767f1 to e51b55e Compare December 14, 2022 01:28
Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/bake kind/enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide support for resources interpolation

4 participants