Skip to content

Commit 9a64c17

Browse files
dynblock: Preserve marks from for_each expression into result
Previously if the for_each expression was marked then expansion would fail because marked expressions are never directly iterable. Now instead we'll allow marked for_each and preserve the marks into the values produced by the resulting block as much as we can. This runs into the classic problem that HCL blocks are not values themselves and so cannot carry marks directly, but we can at least make sure that the values of any leaf arguments end up marked.
1 parent bc75765 commit 9a64c17

File tree

5 files changed

+157
-20
lines changed

5 files changed

+157
-20
lines changed

ext/dynblock/expand_body.go

+28-11
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ type expandBody struct {
1616
original hcl.Body
1717
forEachCtx *hcl.EvalContext
1818
iteration *iteration // non-nil if we're nested inside another "dynamic" block
19+
valueMarks cty.ValueMarks
1920

2021
checkForEach []func(cty.Value, hcl.Expression, *hcl.EvalContext) hcl.Diagnostics
2122

@@ -125,7 +126,7 @@ func (b *expandBody) extendSchema(schema *hcl.BodySchema) *hcl.BodySchema {
125126
}
126127

127128
func (b *expandBody) prepareAttributes(rawAttrs hcl.Attributes) hcl.Attributes {
128-
if len(b.hiddenAttrs) == 0 && b.iteration == nil {
129+
if len(b.hiddenAttrs) == 0 && b.iteration == nil && len(b.valueMarks) == 0 {
129130
// Easy path: just pass through the attrs from the original body verbatim
130131
return rawAttrs
131132
}
@@ -142,13 +143,24 @@ func (b *expandBody) prepareAttributes(rawAttrs hcl.Attributes) hcl.Attributes {
142143
if b.iteration != nil {
143144
attr := *rawAttr // shallow copy so we can mutate it
144145
attr.Expr = exprWrap{
145-
Expression: attr.Expr,
146-
i: b.iteration,
146+
Expression: attr.Expr,
147+
i: b.iteration,
148+
resultMarks: b.valueMarks,
147149
}
148150
attrs[name] = &attr
149151
} else {
150-
// If we have no active iteration then no wrapping is required.
151-
attrs[name] = rawAttr
152+
// If we have no active iteration then no wrapping is required
153+
// unless we have marks to apply.
154+
if len(b.valueMarks) != 0 {
155+
attr := *rawAttr // shallow copy so we can mutate it
156+
attr.Expr = exprWrap{
157+
Expression: attr.Expr,
158+
resultMarks: b.valueMarks,
159+
}
160+
attrs[name] = &attr
161+
} else {
162+
attrs[name] = rawAttr
163+
}
152164
}
153165
}
154166
return attrs
@@ -192,8 +204,9 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks,
192204
continue
193205
}
194206

195-
if spec.forEachVal.IsKnown() {
196-
for it := spec.forEachVal.ElementIterator(); it.Next(); {
207+
forEachVal, marks := spec.forEachVal.Unmark()
208+
if forEachVal.IsKnown() {
209+
for it := forEachVal.ElementIterator(); it.Next(); {
197210
key, value := it.Element()
198211
i := b.iteration.MakeChild(spec.iteratorName, key, value)
199212

@@ -202,7 +215,7 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks,
202215
if block != nil {
203216
// Attach our new iteration context so that attributes
204217
// and other nested blocks can refer to our iterator.
205-
block.Body = b.expandChild(block.Body, i)
218+
block.Body = b.expandChild(block.Body, i, marks)
206219
blocks = append(blocks, block)
207220
}
208221
}
@@ -214,7 +227,10 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks,
214227
block, blockDiags := spec.newBlock(i, b.forEachCtx)
215228
diags = append(diags, blockDiags...)
216229
if block != nil {
217-
block.Body = unknownBody{b.expandChild(block.Body, i)}
230+
block.Body = unknownBody{
231+
template: b.expandChild(block.Body, i, marks),
232+
valueMarks: marks,
233+
}
218234
blocks = append(blocks, block)
219235
}
220236
}
@@ -226,7 +242,7 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks,
226242
// case it contains expressions that refer to our inherited
227243
// iterators, or nested "dynamic" blocks.
228244
expandedBlock := *rawBlock // shallow copy
229-
expandedBlock.Body = b.expandChild(rawBlock.Body, b.iteration)
245+
expandedBlock.Body = b.expandChild(rawBlock.Body, b.iteration, nil)
230246
blocks = append(blocks, &expandedBlock)
231247
}
232248
}
@@ -235,11 +251,12 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks,
235251
return blocks, diags
236252
}
237253

238-
func (b *expandBody) expandChild(child hcl.Body, i *iteration) hcl.Body {
254+
func (b *expandBody) expandChild(child hcl.Body, i *iteration, valueMarks cty.ValueMarks) hcl.Body {
239255
chiCtx := i.EvalContext(b.forEachCtx)
240256
ret := Expand(child, chiCtx)
241257
ret.(*expandBody).iteration = i
242258
ret.(*expandBody).checkForEach = b.checkForEach
259+
ret.(*expandBody).valueMarks = valueMarks
243260
return ret
244261
}
245262

ext/dynblock/expand_body_test.go

+64
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"testing"
99

1010
"github.com/davecgh/go-spew/spew"
11+
"github.com/google/go-cmp/cmp"
1112
"github.com/hashicorp/hcl/v2"
1213
"github.com/hashicorp/hcl/v2/hcldec"
1314
"github.com/hashicorp/hcl/v2/hcltest"
@@ -710,6 +711,69 @@ func TestExpandUnknownBodies(t *testing.T) {
710711

711712
}
712713

714+
func TestExpandMarkedForEach(t *testing.T) {
715+
srcBody := hcltest.MockBody(&hcl.BodyContent{
716+
Blocks: hcl.Blocks{
717+
{
718+
Type: "dynamic",
719+
Labels: []string{"b"},
720+
LabelRanges: []hcl.Range{{}},
721+
Body: hcltest.MockBody(&hcl.BodyContent{
722+
Attributes: hcltest.MockAttrs(map[string]hcl.Expression{
723+
"for_each": hcltest.MockExprLiteral(cty.TupleVal([]cty.Value{
724+
cty.StringVal("hey"),
725+
}).Mark("boop")),
726+
"iterator": hcltest.MockExprTraversalSrc("dyn_b"),
727+
}),
728+
Blocks: hcl.Blocks{
729+
{
730+
Type: "content",
731+
Body: hcltest.MockBody(&hcl.BodyContent{
732+
Attributes: hcltest.MockAttrs(map[string]hcl.Expression{
733+
"val0": hcltest.MockExprLiteral(cty.StringVal("static c 1")),
734+
"val1": hcltest.MockExprTraversalSrc("dyn_b.value"),
735+
}),
736+
}),
737+
},
738+
},
739+
}),
740+
},
741+
},
742+
})
743+
744+
dynBody := Expand(srcBody, nil)
745+
746+
t.Run("Decode", func(t *testing.T) {
747+
decSpec := &hcldec.BlockListSpec{
748+
TypeName: "b",
749+
Nested: &hcldec.ObjectSpec{
750+
"val0": &hcldec.AttrSpec{
751+
Name: "val0",
752+
Type: cty.String,
753+
},
754+
"val1": &hcldec.AttrSpec{
755+
Name: "val1",
756+
Type: cty.String,
757+
},
758+
},
759+
}
760+
761+
want := cty.ListVal([]cty.Value{
762+
cty.ObjectVal(map[string]cty.Value{
763+
"val0": cty.StringVal("static c 1").Mark("boop"),
764+
"val1": cty.StringVal("hey").Mark("boop"),
765+
}),
766+
})
767+
got, diags := hcldec.Decode(dynBody, decSpec, nil)
768+
if diags.HasErrors() {
769+
t.Fatalf("unexpected errors\n%s", diags.Error())
770+
}
771+
if diff := cmp.Diff(want, got, ctydebug.CmpOptions); diff != "" {
772+
t.Errorf("wrong result\n%s", diff)
773+
}
774+
})
775+
}
776+
713777
func TestExpandInvalidIteratorError(t *testing.T) {
714778
srcBody := hcltest.MockBody(&hcl.BodyContent{
715779
Blocks: hcl.Blocks{

ext/dynblock/expand_spec.go

+25-2
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ func (b *expandBody) decodeSpec(blockS *hcl.BlockHeaderSchema, rawSpec *hcl.Bloc
7777
}
7878
}
7979

80-
if !eachVal.CanIterateElements() && eachVal.Type() != cty.DynamicPseudoType {
80+
unmarkedEachVal, _ := eachVal.Unmark()
81+
if !unmarkedEachVal.CanIterateElements() && unmarkedEachVal.Type() != cty.DynamicPseudoType {
8182
// We skip this error for DynamicPseudoType because that means we either
8283
// have a null (which is checked immediately below) or an unknown
8384
// (which is handled in the expandBody Content methods).
@@ -91,7 +92,7 @@ func (b *expandBody) decodeSpec(blockS *hcl.BlockHeaderSchema, rawSpec *hcl.Bloc
9192
})
9293
return nil, diags
9394
}
94-
if eachVal.IsNull() {
95+
if unmarkedEachVal.IsNull() {
9596
diags = append(diags, &hcl.Diagnostic{
9697
Severity: hcl.DiagError,
9798
Summary: "Invalid dynamic for_each value",
@@ -212,6 +213,28 @@ func (s *expandSpec) newBlock(i *iteration, ctx *hcl.EvalContext) (*hcl.Block, h
212213
})
213214
return nil, diags
214215
}
216+
if labelVal.IsMarked() {
217+
// This situation is tricky because HCL just works generically
218+
// with marks and so doesn't have any good language to talk about
219+
// the meaning of specific mark types, but yet we cannot allow
220+
// marked values here because the HCL API guarantees that a block's
221+
// labels are always known static constant Go strings.
222+
// Therefore this is a low-quality error message but at least
223+
// better than panicking below when we call labelVal.AsString.
224+
// If this becomes a problem then we could potentially add a new
225+
// option for the public function [Expand] to allow calling
226+
// applications to specify custom label validation functions that
227+
// could then supersede this generic message.
228+
diags = append(diags, &hcl.Diagnostic{
229+
Severity: hcl.DiagError,
230+
Summary: "Invalid dynamic block label",
231+
Detail: "This value has dynamic marks that make it unsuitable for use as a block label.",
232+
Subject: labelExpr.Range().Ptr(),
233+
Expression: labelExpr,
234+
EvalContext: lCtx,
235+
})
236+
return nil, diags
237+
}
215238

216239
labels = append(labels, labelVal.AsString())
217240
labelRanges = append(labelRanges, labelExpr.Range())

ext/dynblock/expr_wrap.go

+23-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,19 @@ import (
1111
type exprWrap struct {
1212
hcl.Expression
1313
i *iteration
14+
15+
// resultMarks is a set of marks that must be applied to whatever
16+
// value results from this expression. We do this whenever a
17+
// dynamic block's for_each expression produced a marked result,
18+
// since in that case any nested expressions inside are treated
19+
// as being derived from that for_each expression.
20+
//
21+
// (calling applications might choose to reject marks by passing
22+
// an [OptCheckForEach] to [Expand] and returning an error when
23+
// marks are present, but this mechanism is here to help achieve
24+
// reasonable behavior for situations where marks are permitted,
25+
// which is the default.)
26+
resultMarks cty.ValueMarks
1427
}
1528

1629
func (e exprWrap) Variables() []hcl.Traversal {
@@ -34,12 +47,21 @@ func (e exprWrap) Variables() []hcl.Traversal {
3447
}
3548

3649
func (e exprWrap) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
50+
if e.i == nil {
51+
// If we don't have an active iteration then we can just use the
52+
// given EvalContext directly.
53+
return e.prepareValue(e.Expression.Value(ctx))
54+
}
3755
extCtx := e.i.EvalContext(ctx)
38-
return e.Expression.Value(extCtx)
56+
return e.prepareValue(e.Expression.Value(extCtx))
3957
}
4058

4159
// UnwrapExpression returns the expression being wrapped by this instance.
4260
// This allows the original expression to be recovered by hcl.UnwrapExpression.
4361
func (e exprWrap) UnwrapExpression() hcl.Expression {
4462
return e.Expression
4563
}
64+
65+
func (e exprWrap) prepareValue(val cty.Value, diags hcl.Diagnostics) (cty.Value, hcl.Diagnostics) {
66+
return val.WithMarks(e.resultMarks), diags
67+
}

ext/dynblock/unknown_body.go

+17-6
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package dynblock
55

66
import (
77
"github.com/hashicorp/hcl/v2"
8+
"github.com/hashicorp/hcl/v2/hcldec"
89
"github.com/zclconf/go-cty/cty"
910
)
1011

@@ -18,16 +19,26 @@ import (
1819
// we instead arrange for everything _inside_ the block to be unknown instead,
1920
// to give the best possible approximation.
2021
type unknownBody struct {
21-
template hcl.Body
22+
template hcl.Body
23+
valueMarks cty.ValueMarks
2224
}
2325

2426
var _ hcl.Body = unknownBody{}
2527

26-
// hcldec.UnkownBody impl
28+
// hcldec.UnknownBody impl
2729
func (b unknownBody) Unknown() bool {
2830
return true
2931
}
3032

33+
// hcldec.MarkedBody impl
34+
func (b unknownBody) BodyValueMarks() cty.ValueMarks {
35+
// We'll pass through to our template if it is a MarkedBody
36+
if t, ok := b.template.(hcldec.MarkedBody); ok {
37+
return t.BodyValueMarks()
38+
}
39+
return nil
40+
}
41+
3142
func (b unknownBody) Content(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Diagnostics) {
3243
content, diags := b.template.Content(schema)
3344
content = b.fixupContent(content)
@@ -41,7 +52,7 @@ func (b unknownBody) Content(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Diag
4152
func (b unknownBody) PartialContent(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Body, hcl.Diagnostics) {
4253
content, remain, diags := b.template.PartialContent(schema)
4354
content = b.fixupContent(content)
44-
remain = unknownBody{remain} // remaining content must also be wrapped
55+
remain = unknownBody{template: remain, valueMarks: b.valueMarks} // remaining content must also be wrapped
4556

4657
// We're intentionally preserving the diagnostics reported from the
4758
// inner body so that we can still report where the template body doesn't
@@ -69,8 +80,8 @@ func (b unknownBody) fixupContent(got *hcl.BodyContent) *hcl.BodyContent {
6980
if len(got.Blocks) > 0 {
7081
ret.Blocks = make(hcl.Blocks, 0, len(got.Blocks))
7182
for _, gotBlock := range got.Blocks {
72-
new := *gotBlock // shallow copy
73-
new.Body = unknownBody{gotBlock.Body} // nested content must also be marked unknown
83+
new := *gotBlock // shallow copy
84+
new.Body = unknownBody{template: gotBlock.Body, valueMarks: b.valueMarks} // nested content must also be marked unknown
7485
ret.Blocks = append(ret.Blocks, &new)
7586
}
7687
}
@@ -85,7 +96,7 @@ func (b unknownBody) fixupAttrs(got hcl.Attributes) hcl.Attributes {
8596
ret := make(hcl.Attributes, len(got))
8697
for name, gotAttr := range got {
8798
new := *gotAttr // shallow copy
88-
new.Expr = hcl.StaticExpr(cty.DynamicVal, gotAttr.Expr.Range())
99+
new.Expr = hcl.StaticExpr(cty.DynamicVal.WithMarks(b.valueMarks), gotAttr.Expr.Range())
89100
ret[name] = &new
90101
}
91102
return ret

0 commit comments

Comments
 (0)