From 00ae99def8b207995c11bac4ab694523722e4d3c Mon Sep 17 00:00:00 2001 From: Kazuma Watanabe Date: Sat, 4 Jan 2025 14:35:57 +0000 Subject: [PATCH] Allow marked values in dynamic block `for_each` Follow up of https://github.com/hashicorp/hcl/pull/679 Previously, for_each in dynamic blocks did not allow marked values such as sensitive. However, https://github.com/hashicorp/hcl/pull/679 now supports this by propagating the marks to expanded children. The reason behind this is to add a new mark called "ephemeral", so we'll pull the changes to support Terraform 1.10. Note that tfhcl's dynamic block support has incomplete mark propagation since marked values resolve to unknown values. This is because in the past the marked values could not be sent over the wire protocol, and may be fixed in the near future. --- terraform/tfhcl/expand_body.go | 49 ++++++++++++++----- terraform/tfhcl/expand_body_test.go | 73 +++++++++++++++++++++++++++++ terraform/tfhcl/expand_spec.go | 24 ++++++++-- terraform/tfhcl/expr_wrap.go | 22 ++++++++- 4 files changed, 151 insertions(+), 17 deletions(-) diff --git a/terraform/tfhcl/expand_body.go b/terraform/tfhcl/expand_body.go index 3bb83fd8b..3ab0cbf19 100644 --- a/terraform/tfhcl/expand_body.go +++ b/terraform/tfhcl/expand_body.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package tfhcl import ( @@ -15,6 +18,7 @@ type expandBody struct { ctx *hcl.EvalContext dynamicIteration *dynamicIteration // non-nil if we're nested inside a "dynamic" block metaArgIteration *metaArgIteration // non-nil if we're nested inside a block with meta-arguments + valueMarks cty.ValueMarks // These are used with PartialContent to produce a "remaining items" // body to return. They are nil on all bodies fresh out of the transformer. @@ -126,7 +130,7 @@ func (b *expandBody) extendSchema(schema *hcl.BodySchema) *hcl.BodySchema { func (b *expandBody) prepareAttributes(rawAttrs hcl.Attributes) (hcl.Attributes, hcl.Diagnostics) { var diags hcl.Diagnostics - if len(b.hiddenAttrs) == 0 && b.dynamicIteration == nil && b.metaArgIteration == nil { + if len(b.hiddenAttrs) == 0 && b.dynamicIteration == nil && b.metaArgIteration == nil && len(b.valueMarks) == 0 { // Easy path: just pass through the attrs from the original body verbatim return rawAttrs, diags } @@ -143,9 +147,10 @@ func (b *expandBody) prepareAttributes(rawAttrs hcl.Attributes) (hcl.Attributes, if b.dynamicIteration != nil || b.metaArgIteration != nil { attr := *rawAttr // shallow copy so we can mutate it expr := exprWrap{ - Expression: attr.Expr, - di: b.dynamicIteration, - mi: b.metaArgIteration, + Expression: attr.Expr, + di: b.dynamicIteration, + mi: b.metaArgIteration, + resultMarks: b.valueMarks, } // Unlike hcl/ext/dynblock, wrapped expressions are evaluated immediately. // The result is bound to the expression and can be accessed without @@ -161,8 +166,18 @@ func (b *expandBody) prepareAttributes(rawAttrs hcl.Attributes) (hcl.Attributes, } attrs[name] = &attr } else { - // If we have no active iteration then no wrapping is required. - attrs[name] = rawAttr + // If we have no active iteration then no wrapping is required + // unless we have marks to apply. + if len(b.valueMarks) != 0 { + attr := *rawAttr // shallow copy so we can mutate it + attr.Expr = exprWrap{ + Expression: attr.Expr, + resultMarks: b.valueMarks, + } + attrs[name] = &attr + } else { + attrs[name] = rawAttr + } } } return attrs, diags @@ -228,14 +243,16 @@ func (b *expandBody) expandDynamicBlock(schema *hcl.BodySchema, rawBlock *hcl.Bl return hcl.Blocks{}, diags } - if !spec.forEachVal.IsKnown() { + // For dynamic blocks only, it allows marked values + forEachVal, marks := spec.forEachVal.Unmark() + if !forEachVal.IsKnown() { // If for_each is unknown, no blocks are returned return hcl.Blocks{}, diags } var blocks hcl.Blocks - for it := spec.forEachVal.ElementIterator(); it.Next(); { + for it := forEachVal.ElementIterator(); it.Next(); { key, value := it.Element() i := b.dynamicIteration.MakeChild(spec.iteratorName, key, value) @@ -244,7 +261,7 @@ func (b *expandBody) expandDynamicBlock(schema *hcl.BodySchema, rawBlock *hcl.Bl if block != nil { // Attach our new iteration context so that attributes // and other nested blocks can refer to our iterator. - block.Body = b.expandChild(block.Body, i, b.metaArgIteration) + block.Body = b.expandChild(block.Body, i, b.metaArgIteration, marks) blocks = append(blocks, block) } } @@ -278,7 +295,7 @@ func (b *expandBody) expandMetaArgBlock(schema *hcl.BodySchema, rawBlock *hcl.Bl i := MakeCountIteration(cty.NumberIntVal(int64(idx))) expandedBlock := *rawBlock // shallow copy - expandedBlock.Body = b.expandChild(rawBlock.Body, b.dynamicIteration, i) + expandedBlock.Body = b.expandChild(rawBlock.Body, b.dynamicIteration, i, nil) blocks = append(blocks, &expandedBlock) } @@ -299,7 +316,7 @@ func (b *expandBody) expandMetaArgBlock(schema *hcl.BodySchema, rawBlock *hcl.Bl i := MakeForEachIteration(it.Element()) expandedBlock := *rawBlock // shallow copy - expandedBlock.Body = b.expandChild(rawBlock.Body, b.dynamicIteration, i) + expandedBlock.Body = b.expandChild(rawBlock.Body, b.dynamicIteration, i, nil) blocks = append(blocks, &expandedBlock) } @@ -317,15 +334,16 @@ func (b *expandBody) expandStaticBlock(rawBlock *hcl.Block) *hcl.Block { // case it contains expressions that refer to our inherited // iterators, or nested "dynamic" blocks. expandedBlock := *rawBlock - expandedBlock.Body = b.expandChild(rawBlock.Body, b.dynamicIteration, b.metaArgIteration) + expandedBlock.Body = b.expandChild(rawBlock.Body, b.dynamicIteration, b.metaArgIteration, nil) return &expandedBlock } -func (b *expandBody) expandChild(child hcl.Body, i *dynamicIteration, mi *metaArgIteration) hcl.Body { +func (b *expandBody) expandChild(child hcl.Body, i *dynamicIteration, mi *metaArgIteration, valueMarks cty.ValueMarks) hcl.Body { chiCtx := i.EvalContext(mi.EvalContext(b.ctx)) ret := Expand(child, chiCtx) ret.(*expandBody).dynamicIteration = i ret.(*expandBody).metaArgIteration = mi + ret.(*expandBody).valueMarks = valueMarks return ret } @@ -339,3 +357,8 @@ func (b *expandBody) JustAttributes() (hcl.Attributes, hcl.Diagnostics) { func (b *expandBody) MissingItemRange() hcl.Range { return b.original.MissingItemRange() } + +// hcldec.MarkedBody impl +func (b *expandBody) BodyValueMarks() cty.ValueMarks { + return b.valueMarks +} diff --git a/terraform/tfhcl/expand_body_test.go b/terraform/tfhcl/expand_body_test.go index 7fdd6878e..bf85147bc 100644 --- a/terraform/tfhcl/expand_body_test.go +++ b/terraform/tfhcl/expand_body_test.go @@ -1,11 +1,16 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package tfhcl import ( "testing" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hcldec" "github.com/hashicorp/hcl/v2/hcltest" + "github.com/zclconf/go-cty-debug/ctydebug" "github.com/zclconf/go-cty/cty" ) @@ -331,3 +336,71 @@ func TestExpand(t *testing.T) { }) } + +func TestExpandMarkedForEach(t *testing.T) { + srcBody := hcltest.MockBody(&hcl.BodyContent{ + Blocks: hcl.Blocks{ + { + Type: "dynamic", + Labels: []string{"b"}, + LabelRanges: []hcl.Range{{}}, + Body: hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ + "for_each": hcltest.MockExprLiteral(cty.TupleVal([]cty.Value{ + cty.StringVal("hey"), + }).Mark("boop")), + "iterator": hcltest.MockExprTraversalSrc("dyn_b"), + }), + Blocks: hcl.Blocks{ + { + Type: "content", + Body: hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ + "val0": hcltest.MockExprLiteral(cty.StringVal("static c 1")), + "val1": hcltest.MockExprTraversalSrc("dyn_b.value"), + }), + }), + }, + }, + }), + }, + }, + }) + + // Emulate eval context because iterators are indistinguishable from any resource and effectively resolve to unknown. + ctx := &hcl.EvalContext{ + Variables: map[string]cty.Value{"dyn_b": cty.DynamicVal}, + } + + dynBody := Expand(srcBody, ctx) + + t.Run("Decode", func(t *testing.T) { + decSpec := &hcldec.BlockListSpec{ + TypeName: "b", + Nested: &hcldec.ObjectSpec{ + "val0": &hcldec.AttrSpec{ + Name: "val0", + Type: cty.String, + }, + "val1": &hcldec.AttrSpec{ + Name: "val1", + Type: cty.String, + }, + }, + } + + want := cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "val0": cty.StringVal("static c 1"), + "val1": cty.UnknownVal(cty.String), + }).Mark("boop"), + }) + got, diags := hcldec.Decode(dynBody, decSpec, ctx) + if diags.HasErrors() { + t.Fatalf("unexpected errors\n%s", diags.Error()) + } + if diff := cmp.Diff(want, got, ctydebug.CmpOptions); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) +} diff --git a/terraform/tfhcl/expand_spec.go b/terraform/tfhcl/expand_spec.go index 8846c95d0..b42ad253d 100644 --- a/terraform/tfhcl/expand_spec.go +++ b/terraform/tfhcl/expand_spec.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package tfhcl import ( @@ -42,7 +45,9 @@ func (b *expandBody) decodeDynamicSpec(blockS *hcl.BlockHeaderSchema, rawSpec *h eachVal, eachDiags := eachAttr.Expr.Value(b.ctx) diags = append(diags, eachDiags...) - if !eachVal.CanIterateElements() && eachVal.Type() != cty.DynamicPseudoType { + // For dynamic blocks only, it allows marked values + unmarkedEachVal, _ := eachVal.Unmark() + if !unmarkedEachVal.CanIterateElements() && unmarkedEachVal.Type() != cty.DynamicPseudoType { // We skip this error for DynamicPseudoType because that means we either // have a null (which is checked immediately below) or an unknown // (which is handled in the expandBody Content methods). @@ -56,7 +61,7 @@ func (b *expandBody) decodeDynamicSpec(blockS *hcl.BlockHeaderSchema, rawSpec *h }) return nil, diags } - if eachVal.IsNull() { + if unmarkedEachVal.IsNull() { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid dynamic for_each value", @@ -188,13 +193,26 @@ func (s *expandDynamicSpec) newBlock(i *dynamicIteration, ctx *hcl.EvalContext) return nil, diags } if !labelVal.IsKnown() { + // Unlike hcl/ext/dynblock, if the label is unknown + // it will not return an error and will not append a new block. return nil, diags } if labelVal.IsMarked() { + // This situation is tricky because HCL just works generically + // with marks and so doesn't have any good language to talk about + // the meaning of specific mark types, but yet we cannot allow + // marked values here because the HCL API guarantees that a block's + // labels are always known static constant Go strings. + // Therefore this is a low-quality error message but at least + // better than panicking below when we call labelVal.AsString. + // If this becomes a problem then we could potentially add a new + // option for the public function [Expand] to allow calling + // applications to specify custom label validation functions that + // could then supersede this generic message. diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid dynamic block label", - Detail: "Cannot use a marked value as a dynamic block label.", + Detail: "This value has dynamic marks that make it unsuitable for use as a block label.", Subject: labelExpr.Range().Ptr(), Expression: labelExpr, EvalContext: lCtx, diff --git a/terraform/tfhcl/expr_wrap.go b/terraform/tfhcl/expr_wrap.go index 1843e4aef..4595b46b6 100644 --- a/terraform/tfhcl/expr_wrap.go +++ b/terraform/tfhcl/expr_wrap.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package tfhcl import ( @@ -9,6 +12,13 @@ type exprWrap struct { hcl.Expression di *dynamicIteration mi *metaArgIteration + + // resultMarks is a set of marks that must be applied to whatever + // value results from this expression. We do this whenever a + // dynamic block's for_each expression produced a marked result, + // since in that case any nested expressions inside are treated + // as being derived from that for_each expression. + resultMarks cty.ValueMarks } func (e exprWrap) Variables() []hcl.Traversal { @@ -35,8 +45,14 @@ func (e exprWrap) Variables() []hcl.Traversal { } func (e exprWrap) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { + if e.di == nil && e.mi == nil { + // If we don't have an active iteration then we can just use the + // given EvalContext directly. + return e.prepareValue(e.Expression.Value(ctx)) + } + extCtx := e.di.EvalContext(e.mi.EvalContext(ctx)) - return e.Expression.Value(extCtx) + return e.prepareValue(e.Expression.Value(extCtx)) } // UnwrapExpression returns the expression being wrapped by this instance. @@ -44,3 +60,7 @@ func (e exprWrap) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { func (e exprWrap) UnwrapExpression() hcl.Expression { return e.Expression } + +func (e exprWrap) prepareValue(val cty.Value, diags hcl.Diagnostics) (cty.Value, hcl.Diagnostics) { + return val.WithMarks(e.resultMarks), diags +}