From f7cd61ac04cc66dcbb42ba84dfe640c976762021 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 22 Apr 2024 14:20:01 +0200 Subject: [PATCH 01/10] Add additional function for parsing traversals with [*] keys (#673) * Add additional function for parsing traversals with [*] keys * add more context around skipped test cases --- hclsyntax/parse_traversal_test.go | 81 ++++++++++++++++++++++++++++++- hclsyntax/parser_traversal.go | 51 ++++++++++++++++++- hclsyntax/public.go | 31 ++++++++++++ 3 files changed, 161 insertions(+), 2 deletions(-) diff --git a/hclsyntax/parse_traversal_test.go b/hclsyntax/parse_traversal_test.go index 3ca5fc2b..5d8d24ea 100644 --- a/hclsyntax/parse_traversal_test.go +++ b/hclsyntax/parse_traversal_test.go @@ -4,11 +4,13 @@ package hclsyntax import ( + "fmt" "testing" "github.com/go-test/deep" - "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/hcl/v2" ) func TestParseTraversalAbs(t *testing.T) { @@ -208,10 +210,63 @@ func TestParseTraversalAbs(t *testing.T) { }, 1, // extra junk after traversal }, + + { + "foo[*]", + hcl.Traversal{ + hcl.TraverseRoot{ + Name: "foo", + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 4, Byte: 3}, + }, + }, + hcl.TraverseSplat{ + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 4, Byte: 3}, + End: hcl.Pos{Line: 1, Column: 7, Byte: 6}, + }, + }, + }, + 0, + }, + { + "foo.*", // Still not supporting this. + hcl.Traversal{ + hcl.TraverseRoot{ + Name: "foo", + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 4, Byte: 3}, + }, + }, + }, + 1, + }, + { + "foo[*].bar", // Run this through the unsupported function. + hcl.Traversal{ + hcl.TraverseRoot{ + Name: "foo", + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 4, Byte: 3}, + }, + }, + }, + 1, + }, } for _, test := range tests { t.Run(test.src, func(t *testing.T) { + if test.src == "foo[*]" { + // The foo[*] test will fail because the function we test in + // this branch does not support the splat syntax. So we will + // skip this test case here. + t.Skip("skipping test for unsupported splat syntax") + } + got, diags := ParseTraversalAbs([]byte(test.src), "", hcl.Pos{Line: 1, Column: 1}) if len(diags) != test.diagCount { for _, diag := range diags { @@ -226,5 +281,29 @@ func TestParseTraversalAbs(t *testing.T) { } } }) + + t.Run(fmt.Sprintf("partial_%s", test.src), func(t *testing.T) { + if test.src == "foo[*].bar" { + // The foo[*].bar test will fail because the function we test in + // this branch does support the splat syntax and this test is + // designed to make sure that the other branch still fails with + // the splat syntax. So we will skip this test case here. + t.Skip("skipping test that fails for splat syntax") + } + + got, diags := ParseTraversalPartial([]byte(test.src), "", hcl.Pos{Line: 1, Column: 1}) + if len(diags) != test.diagCount { + for _, diag := range diags { + t.Logf(" - %s", diag.Error()) + } + t.Errorf("wrong number of diagnostics %d; want %d", len(diags), test.diagCount) + } + + if diff := deep.Equal(got, test.want); diff != nil { + for _, problem := range diff { + t.Error(problem) + } + } + }) } } diff --git a/hclsyntax/parser_traversal.go b/hclsyntax/parser_traversal.go index 3afa6ab0..f7d4062f 100644 --- a/hclsyntax/parser_traversal.go +++ b/hclsyntax/parser_traversal.go @@ -4,8 +4,9 @@ package hclsyntax import ( - "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/hcl/v2" ) // ParseTraversalAbs parses an absolute traversal that is assumed to consume @@ -13,6 +14,26 @@ import ( // behavior is not supported here because traversals are not expected to // be parsed as part of a larger program. func (p *parser) ParseTraversalAbs() (hcl.Traversal, hcl.Diagnostics) { + return p.parseTraversal(false) +} + +// ParseTraversalPartial parses an absolute traversal that is permitted +// to contain splat ([*]) expressions. Only splat expressions within square +// brackets are permitted ([*]); splat expressions within attribute names are +// not permitted (.*). +// +// The meaning of partial here is that the traversal may be incomplete, in that +// any splat expression indicates reference to a potentially unknown number of +// elements. +// +// Traversals that include splats cannot be automatically traversed by HCL using +// the TraversalAbs or TraversalRel methods. Instead, the caller must handle +// the traversals manually. +func (p *parser) ParseTraversalPartial() (hcl.Traversal, hcl.Diagnostics) { + return p.parseTraversal(true) +} + +func (p *parser) parseTraversal(allowSplats bool) (hcl.Traversal, hcl.Diagnostics) { var ret hcl.Traversal var diags hcl.Diagnostics @@ -127,6 +148,34 @@ func (p *parser) ParseTraversalAbs() (hcl.Traversal, hcl.Diagnostics) { return ret, diags } + case TokenStar: + if allowSplats { + + p.Read() // Eat the star. + close := p.Read() + if close.Type != TokenCBrack { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unclosed index brackets", + Detail: "Index key must be followed by a closing bracket.", + Subject: &close.Range, + Context: hcl.RangeBetween(open.Range, close.Range).Ptr(), + }) + } + + ret = append(ret, hcl.TraverseSplat{ + SrcRange: hcl.RangeBetween(open.Range, close.Range), + }) + + if diags.HasErrors() { + return ret, diags + } + + continue + } + + // Otherwise, return the error below for the star. + fallthrough default: if next.Type == TokenStar { diags = append(diags, &hcl.Diagnostic{ diff --git a/hclsyntax/public.go b/hclsyntax/public.go index d56f8e50..17dc1ed4 100644 --- a/hclsyntax/public.go +++ b/hclsyntax/public.go @@ -118,6 +118,37 @@ func ParseTraversalAbs(src []byte, filename string, start hcl.Pos) (hcl.Traversa return expr, diags } +// ParseTraversalPartial matches the behavior of ParseTraversalAbs except +// that it allows splat expressions ([*]) to appear in the traversal. +// +// The returned traversals are "partial" in that the splat expression indicates +// an unknown value for the index. +// +// Traversals that include splats cannot be automatically traversed by HCL using +// the TraversalAbs or TraversalRel methods. Instead, the caller must handle +// the traversals manually. +func ParseTraversalPartial(src []byte, filename string, start hcl.Pos) (hcl.Traversal, hcl.Diagnostics) { + tokens, diags := LexExpression(src, filename, start) + peeker := newPeeker(tokens, false) + parser := &parser{peeker: peeker} + + // Bare traverals are always parsed in "ignore newlines" mode, as if + // they were wrapped in parentheses. + parser.PushIncludeNewlines(false) + + expr, parseDiags := parser.ParseTraversalPartial() + diags = append(diags, parseDiags...) + + parser.PopIncludeNewlines() + + // Panic if the parser uses incorrect stack discipline with the peeker's + // newlines stack, since otherwise it will produce confusing downstream + // errors. + peeker.AssertEmptyIncludeNewlinesStack() + + return expr, diags +} + // LexConfig performs lexical analysis on the given buffer, treating it as a // whole HCL config file, and returns the resulting tokens. // From 1c5ae8fc88a656ab7bd46da4ff27a20c5a97497b Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 22 Apr 2024 14:24:19 +0200 Subject: [PATCH 02/10] Update CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eebedbc..a33c5a68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # HCL Changelog +## v2.21.0 (Unreleased) + +### Enhancements + +* Introduce `ParseTraversalPartial`, which allows traversals that include the splat (`[*]`) index operator. ([#673](https://github.com/hashicorp/hcl/pull/673)) + ## v2.20.1 (March 26, 2024) ### Bugs Fixed From 4521ae92f155259e24118bddbe5fc4ef23d2a7ff Mon Sep 17 00:00:00 2001 From: "hashicorp-tsccr[bot]" <129506189+hashicorp-tsccr[bot]@users.noreply.github.com> Date: Wed, 8 May 2024 12:56:20 +0200 Subject: [PATCH 03/10] github: Pin action refs to latest trusted by TSCCR (#677) Co-authored-by: hashicorp-tsccr[bot] --- .github/workflows/checks.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 51e279ae..5102728d 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -30,9 +30,9 @@ jobs: run: | git config --global core.autocrlf false - name: "Fetch source code" - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 + uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 - name: Install Go - uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0 + uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: go-version-file: go.mod - name: Go test @@ -44,9 +44,9 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 + uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 - name: Install Go - uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0 + uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: go-version-file: go.mod - name: "copyright headers check" @@ -58,9 +58,9 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 + uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 - name: Install Go - uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0 + uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: go-version-file: go.mod - name: "go vet" @@ -72,9 +72,9 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 + uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 - name: Install Go - uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0 + uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: go-version-file: go.mod - name: "gofmt" From bf546973d01a1ec4a306cbcc789bd86ea70e44be Mon Sep 17 00:00:00 2001 From: Nara Kasbergen Kwon <855115+xiehan@users.noreply.github.com> Date: Wed, 8 May 2024 13:00:10 +0200 Subject: [PATCH 04/10] github: Set up Dependabot to manage HashiCorp-owned Actions versioning --- .github/dependabot.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..61e0aff8 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,22 @@ +version: 2 +updates: + - package-ecosystem: github-actions + directory: / + schedule: + interval: monthly + labels: + - dependencies + - automated + reviewers: + - hashicorp/terraform-core + # only update HashiCorp actions, external actions managed by TSCCR + allow: + - dependency-name: hashicorp/* + groups: + github-actions-breaking: + update-types: + - major + github-actions-backward-compatible: + update-types: + - minor + - patch From bc757658ca11c5d6d17f328d5672ac447c3efcff Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 8 May 2024 16:09:28 -0700 Subject: [PATCH 05/10] hclsyntax: Don't panic if splat operand is unknown and marked We were calling .Range() on any unknown sourceVal, without first checking whether it was marked. That method panics if called on a marked value, so we need to strip that off first. While testing this I found some return paths that weren't properly transferring the source value's marks to the output, and so this also addresses those so that all return paths preserve whatever markings are present on the source value. In particular, if a non-list/set/tuple value gets "upgraded" into a tuple then we must transfer its marks onto the tuple, because the decision about constructing that value was based on characteristics of the source value. --- hclsyntax/expression.go | 15 ++++++++------- hclsyntax/expression_test.go | 10 ++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/hclsyntax/expression.go b/hclsyntax/expression.go index 81597399..577a50fa 100644 --- a/hclsyntax/expression.go +++ b/hclsyntax/expression.go @@ -1780,7 +1780,7 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { if sourceVal.IsNull() { if autoUpgrade { - return cty.EmptyTupleVal, diags + return cty.EmptyTupleVal.WithSameMarks(sourceVal), diags } diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, @@ -1798,7 +1798,7 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { // If we don't even know the _type_ of our source value yet then // we'll need to defer all processing, since we can't decide our // result type either. - return cty.DynamicVal, diags + return cty.DynamicVal.WithSameMarks(sourceVal), diags } upgradedUnknown := false @@ -1813,13 +1813,14 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { // list of a single attribute, but we still need to check if that // attribute actually exists. if !sourceVal.IsKnown() { - sourceRng := sourceVal.Range() + unmarkedVal, _ := sourceVal.Unmark() + sourceRng := unmarkedVal.Range() if sourceRng.CouldBeNull() { upgradedUnknown = true } } - sourceVal = cty.TupleVal([]cty.Value{sourceVal}) + sourceVal = cty.TupleVal([]cty.Value{sourceVal}).WithSameMarks(sourceVal) sourceTy = sourceVal.Type() } @@ -1900,14 +1901,14 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { e.Item.clearValue(ctx) // clean up our temporary value if upgradedUnknown { - return cty.DynamicVal, diags + return cty.DynamicVal.WithMarks(marks), diags } if !isKnown { // We'll ingore the resultTy diagnostics in this case since they // will just be the same errors we saw while iterating above. ty, _ := resultTy() - return cty.UnknownVal(ty), diags + return cty.UnknownVal(ty).WithMarks(marks), diags } switch { @@ -1915,7 +1916,7 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { if len(vals) == 0 { ty, tyDiags := resultTy() diags = append(diags, tyDiags...) - return cty.ListValEmpty(ty.ElementType()), diags + return cty.ListValEmpty(ty.ElementType()).WithMarks(marks), diags } return cty.ListVal(vals).WithMarks(marks), diags default: diff --git a/hclsyntax/expression_test.go b/hclsyntax/expression_test.go index b18214cd..b50dc137 100644 --- a/hclsyntax/expression_test.go +++ b/hclsyntax/expression_test.go @@ -1457,6 +1457,16 @@ upper( cty.UnknownVal(cty.List(cty.Bool)).RefineNotNull().Mark("sensitive"), 0, }, + { // splat with sensitive non-collection that's unknown + `not_a_list.*`, + &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "not_a_list": cty.UnknownVal(cty.EmptyObject).RefineNotNull().Mark("sensitive"), + }, + }, + cty.TupleVal([]cty.Value{cty.UnknownVal(cty.EmptyObject).RefineNotNull().Mark("sensitive")}).Mark("sensitive"), + 0, + }, { // splat with sensitive collection that's unknown and not null `maps.*.enabled`, &hcl.EvalContext{ From 9a64c17c75059d9c8f5d94f2265c00026ac48781 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 8 May 2024 17:52:51 -0700 Subject: [PATCH 06/10] 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. --- ext/dynblock/expand_body.go | 39 +++++++++++++------ ext/dynblock/expand_body_test.go | 64 ++++++++++++++++++++++++++++++++ ext/dynblock/expand_spec.go | 27 +++++++++++++- ext/dynblock/expr_wrap.go | 24 +++++++++++- ext/dynblock/unknown_body.go | 23 +++++++++--- 5 files changed, 157 insertions(+), 20 deletions(-) diff --git a/ext/dynblock/expand_body.go b/ext/dynblock/expand_body.go index 2734e937..e630f184 100644 --- a/ext/dynblock/expand_body.go +++ b/ext/dynblock/expand_body.go @@ -16,6 +16,7 @@ type expandBody struct { original hcl.Body forEachCtx *hcl.EvalContext iteration *iteration // non-nil if we're nested inside another "dynamic" block + valueMarks cty.ValueMarks checkForEach []func(cty.Value, hcl.Expression, *hcl.EvalContext) hcl.Diagnostics @@ -125,7 +126,7 @@ func (b *expandBody) extendSchema(schema *hcl.BodySchema) *hcl.BodySchema { } func (b *expandBody) prepareAttributes(rawAttrs hcl.Attributes) hcl.Attributes { - if len(b.hiddenAttrs) == 0 && b.iteration == nil { + if len(b.hiddenAttrs) == 0 && b.iteration == nil && len(b.valueMarks) == 0 { // Easy path: just pass through the attrs from the original body verbatim return rawAttrs } @@ -142,13 +143,24 @@ func (b *expandBody) prepareAttributes(rawAttrs hcl.Attributes) hcl.Attributes { if b.iteration != nil { attr := *rawAttr // shallow copy so we can mutate it attr.Expr = exprWrap{ - Expression: attr.Expr, - i: b.iteration, + Expression: attr.Expr, + i: b.iteration, + resultMarks: b.valueMarks, } 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 @@ -192,8 +204,9 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks, continue } - if spec.forEachVal.IsKnown() { - for it := spec.forEachVal.ElementIterator(); it.Next(); { + forEachVal, marks := spec.forEachVal.Unmark() + if forEachVal.IsKnown() { + for it := forEachVal.ElementIterator(); it.Next(); { key, value := it.Element() i := b.iteration.MakeChild(spec.iteratorName, key, value) @@ -202,7 +215,7 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks, 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) + block.Body = b.expandChild(block.Body, i, marks) blocks = append(blocks, block) } } @@ -214,7 +227,10 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks, block, blockDiags := spec.newBlock(i, b.forEachCtx) diags = append(diags, blockDiags...) if block != nil { - block.Body = unknownBody{b.expandChild(block.Body, i)} + block.Body = unknownBody{ + template: b.expandChild(block.Body, i, marks), + valueMarks: marks, + } blocks = append(blocks, block) } } @@ -226,7 +242,7 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks, // case it contains expressions that refer to our inherited // iterators, or nested "dynamic" blocks. expandedBlock := *rawBlock // shallow copy - expandedBlock.Body = b.expandChild(rawBlock.Body, b.iteration) + expandedBlock.Body = b.expandChild(rawBlock.Body, b.iteration, nil) blocks = append(blocks, &expandedBlock) } } @@ -235,11 +251,12 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks, return blocks, diags } -func (b *expandBody) expandChild(child hcl.Body, i *iteration) hcl.Body { +func (b *expandBody) expandChild(child hcl.Body, i *iteration, valueMarks cty.ValueMarks) hcl.Body { chiCtx := i.EvalContext(b.forEachCtx) ret := Expand(child, chiCtx) ret.(*expandBody).iteration = i ret.(*expandBody).checkForEach = b.checkForEach + ret.(*expandBody).valueMarks = valueMarks return ret } diff --git a/ext/dynblock/expand_body_test.go b/ext/dynblock/expand_body_test.go index bec6c210..3b245ee8 100644 --- a/ext/dynblock/expand_body_test.go +++ b/ext/dynblock/expand_body_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/davecgh/go-spew/spew" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hcldec" "github.com/hashicorp/hcl/v2/hcltest" @@ -710,6 +711,69 @@ func TestExpandUnknownBodies(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"), + }), + }), + }, + }, + }), + }, + }, + }) + + dynBody := Expand(srcBody, nil) + + 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").Mark("boop"), + "val1": cty.StringVal("hey").Mark("boop"), + }), + }) + got, diags := hcldec.Decode(dynBody, decSpec, nil) + 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) + } + }) +} + func TestExpandInvalidIteratorError(t *testing.T) { srcBody := hcltest.MockBody(&hcl.BodyContent{ Blocks: hcl.Blocks{ diff --git a/ext/dynblock/expand_spec.go b/ext/dynblock/expand_spec.go index 0231c4aa..55a69ba9 100644 --- a/ext/dynblock/expand_spec.go +++ b/ext/dynblock/expand_spec.go @@ -77,7 +77,8 @@ func (b *expandBody) decodeSpec(blockS *hcl.BlockHeaderSchema, rawSpec *hcl.Bloc } } - if !eachVal.CanIterateElements() && eachVal.Type() != cty.DynamicPseudoType { + 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). @@ -91,7 +92,7 @@ func (b *expandBody) decodeSpec(blockS *hcl.BlockHeaderSchema, rawSpec *hcl.Bloc }) return nil, diags } - if eachVal.IsNull() { + if unmarkedEachVal.IsNull() { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid dynamic for_each value", @@ -212,6 +213,28 @@ func (s *expandSpec) newBlock(i *iteration, ctx *hcl.EvalContext) (*hcl.Block, h }) 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: "This value has dynamic marks that make it unsuitable for use as a block label.", + Subject: labelExpr.Range().Ptr(), + Expression: labelExpr, + EvalContext: lCtx, + }) + return nil, diags + } labels = append(labels, labelVal.AsString()) labelRanges = append(labelRanges, labelExpr.Range()) diff --git a/ext/dynblock/expr_wrap.go b/ext/dynblock/expr_wrap.go index 625bf9cc..57636411 100644 --- a/ext/dynblock/expr_wrap.go +++ b/ext/dynblock/expr_wrap.go @@ -11,6 +11,19 @@ import ( type exprWrap struct { hcl.Expression i *iteration + + // 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. + // + // (calling applications might choose to reject marks by passing + // an [OptCheckForEach] to [Expand] and returning an error when + // marks are present, but this mechanism is here to help achieve + // reasonable behavior for situations where marks are permitted, + // which is the default.) + resultMarks cty.ValueMarks } func (e exprWrap) Variables() []hcl.Traversal { @@ -34,8 +47,13 @@ func (e exprWrap) Variables() []hcl.Traversal { } func (e exprWrap) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { + if e.i == 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.i.EvalContext(ctx) - return e.Expression.Value(extCtx) + return e.prepareValue(e.Expression.Value(extCtx)) } // UnwrapExpression returns the expression being wrapped by this instance. @@ -43,3 +61,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 +} diff --git a/ext/dynblock/unknown_body.go b/ext/dynblock/unknown_body.go index 6cd59c77..b1fdfc07 100644 --- a/ext/dynblock/unknown_body.go +++ b/ext/dynblock/unknown_body.go @@ -5,6 +5,7 @@ package dynblock import ( "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hcldec" "github.com/zclconf/go-cty/cty" ) @@ -18,16 +19,26 @@ import ( // we instead arrange for everything _inside_ the block to be unknown instead, // to give the best possible approximation. type unknownBody struct { - template hcl.Body + template hcl.Body + valueMarks cty.ValueMarks } var _ hcl.Body = unknownBody{} -// hcldec.UnkownBody impl +// hcldec.UnknownBody impl func (b unknownBody) Unknown() bool { return true } +// hcldec.MarkedBody impl +func (b unknownBody) BodyValueMarks() cty.ValueMarks { + // We'll pass through to our template if it is a MarkedBody + if t, ok := b.template.(hcldec.MarkedBody); ok { + return t.BodyValueMarks() + } + return nil +} + func (b unknownBody) Content(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Diagnostics) { content, diags := b.template.Content(schema) content = b.fixupContent(content) @@ -41,7 +52,7 @@ func (b unknownBody) Content(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Diag func (b unknownBody) PartialContent(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Body, hcl.Diagnostics) { content, remain, diags := b.template.PartialContent(schema) content = b.fixupContent(content) - remain = unknownBody{remain} // remaining content must also be wrapped + remain = unknownBody{template: remain, valueMarks: b.valueMarks} // remaining content must also be wrapped // We're intentionally preserving the diagnostics reported from the // 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 { if len(got.Blocks) > 0 { ret.Blocks = make(hcl.Blocks, 0, len(got.Blocks)) for _, gotBlock := range got.Blocks { - new := *gotBlock // shallow copy - new.Body = unknownBody{gotBlock.Body} // nested content must also be marked unknown + new := *gotBlock // shallow copy + new.Body = unknownBody{template: gotBlock.Body, valueMarks: b.valueMarks} // nested content must also be marked unknown ret.Blocks = append(ret.Blocks, &new) } } @@ -85,7 +96,7 @@ func (b unknownBody) fixupAttrs(got hcl.Attributes) hcl.Attributes { ret := make(hcl.Attributes, len(got)) for name, gotAttr := range got { new := *gotAttr // shallow copy - new.Expr = hcl.StaticExpr(cty.DynamicVal, gotAttr.Expr.Range()) + new.Expr = hcl.StaticExpr(cty.DynamicVal.WithMarks(b.valueMarks), gotAttr.Expr.Range()) ret[name] = &new } return ret From 318bbfebb5e1eeea09765b1edb1aec303f75a268 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 8 May 2024 17:55:39 -0700 Subject: [PATCH 07/10] hcldec: Allow body-derived values to be marked Similar to the previously-added UnknownBody, the new optional interface MarkedBody allows hcl.Body implementations to suggest a set of marks that ought to be applied to any value that's generated to represent the content of that body. The dynblock extension then uses this to get hcldec to mark the whole object representing any block that was generated by a dynamic block whose for_each was marked, for a better representation of the fact that a block's existence was decided based on a marked value. --- ext/dynblock/expand_body.go | 5 +++++ ext/dynblock/expand_body_test.go | 2 +- go.mod | 2 +- go.sum | 4 ++++ hcldec/spec.go | 19 +++++++++++++++++++ 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/ext/dynblock/expand_body.go b/ext/dynblock/expand_body.go index e630f184..2cafae5f 100644 --- a/ext/dynblock/expand_body.go +++ b/ext/dynblock/expand_body.go @@ -270,3 +270,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/ext/dynblock/expand_body_test.go b/ext/dynblock/expand_body_test.go index 3b245ee8..0941f291 100644 --- a/ext/dynblock/expand_body_test.go +++ b/ext/dynblock/expand_body_test.go @@ -762,7 +762,7 @@ func TestExpandMarkedForEach(t *testing.T) { cty.ObjectVal(map[string]cty.Value{ "val0": cty.StringVal("static c 1").Mark("boop"), "val1": cty.StringVal("hey").Mark("boop"), - }), + }).Mark("boop"), }) got, diags := hcldec.Decode(dynBody, decSpec, nil) if diags.HasErrors() { diff --git a/go.mod b/go.mod index d9d4da45..56c414fa 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7 github.com/spf13/pflag v1.0.2 github.com/zclconf/go-cty v1.13.0 - github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b + github.com/zclconf/go-cty-debug v0.0.0-20240509010212-0d6042c53940 golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167 golang.org/x/tools v0.6.0 ) diff --git a/go.sum b/go.sum index 08ed23ed..53611d83 100644 --- a/go.sum +++ b/go.sum @@ -27,6 +27,10 @@ github.com/zclconf/go-cty v1.13.0 h1:It5dfKTTZHe9aeppbNOda3mN7Ag7sg6QkBNm6TkyFa0 github.com/zclconf/go-cty v1.13.0/go.mod h1:YKQzy/7pZ7iq2jNFzy5go57xdxdWoLLpaEp4u238AE0= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b h1:FosyBZYxY34Wul7O/MSKey3txpPYyCqVO5ZyceuQJEI= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8= +github.com/zclconf/go-cty-debug v0.0.0-20240417160409-8c45e122ae1a h1:/o/Emn22dZIQ7AhyA0aLOKo528WG/WRAM5tqzIoQIOs= +github.com/zclconf/go-cty-debug v0.0.0-20240417160409-8c45e122ae1a/go.mod h1:CmBdvvj3nqzfzJ6nTCIwDTPZ56aVGvDrmztiO5g3qrM= +github.com/zclconf/go-cty-debug v0.0.0-20240509010212-0d6042c53940 h1:4r45xpDWB6ZMSMNJFMOjqrGHynW3DIBuR2H9j0ug+Mo= +github.com/zclconf/go-cty-debug v0.0.0-20240509010212-0d6042c53940/go.mod h1:CmBdvvj3nqzfzJ6nTCIwDTPZ56aVGvDrmztiO5g3qrM= golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167 h1:O8uGbHCqlTp2P6QJSLmCojM4mN6UemYv8K+dCnmHmu0= golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8= diff --git a/hcldec/spec.go b/hcldec/spec.go index 2bebc433..b07a5534 100644 --- a/hcldec/spec.go +++ b/hcldec/spec.go @@ -73,6 +73,12 @@ type UnknownBody interface { Unknown() bool } +// MarkedBody can be optionally implemented by an hcl.Body instance to +// indicate that a value created from it ought to be marked. +type MarkedBody interface { + BodyValueMarks() cty.ValueMarks +} + func (s ObjectSpec) visitSameBodyChildren(cb visitFunc) { for _, c := range s { cb(c) @@ -473,6 +479,7 @@ func (s *BlockListSpec) decode(content *hcl.BodyContent, blockLabels []blockLabe val, _, childDiags := decode(childBlock.Body, labelsForBlock(childBlock), ctx, s.Nested, false) diags = append(diags, childDiags...) + val = prepareBodyVal(val, childBlock.Body) if u, ok := childBlock.Body.(UnknownBody); ok { if u.Unknown() { @@ -635,6 +642,7 @@ func (s *BlockTupleSpec) decode(content *hcl.BodyContent, blockLabels []blockLab val, _, childDiags := decode(childBlock.Body, labelsForBlock(childBlock), ctx, s.Nested, false) diags = append(diags, childDiags...) + val = prepareBodyVal(val, childBlock.Body) if u, ok := childBlock.Body.(UnknownBody); ok { if u.Unknown() { @@ -758,6 +766,7 @@ func (s *BlockSetSpec) decode(content *hcl.BodyContent, blockLabels []blockLabel val, _, childDiags := decode(childBlock.Body, labelsForBlock(childBlock), ctx, s.Nested, false) diags = append(diags, childDiags...) + val = prepareBodyVal(val, childBlock.Body) if u, ok := childBlock.Body.(UnknownBody); ok { if u.Unknown() { @@ -928,6 +937,7 @@ func (s *BlockMapSpec) decode(content *hcl.BodyContent, blockLabels []blockLabel childLabels := labelsForBlock(childBlock) val, _, childDiags := decode(childBlock.Body, childLabels[len(s.LabelNames):], ctx, s.Nested, false) + val = prepareBodyVal(val, childBlock.Body) targetMap := elems for _, key := range childBlock.Labels[:len(s.LabelNames)-1] { if _, exists := targetMap[key]; !exists { @@ -1082,6 +1092,7 @@ func (s *BlockObjectSpec) decode(content *hcl.BodyContent, blockLabels []blockLa childLabels := labelsForBlock(childBlock) val, _, childDiags := decode(childBlock.Body, childLabels[len(s.LabelNames):], ctx, s.Nested, false) + val = prepareBodyVal(val, childBlock.Body) targetMap := elems for _, key := range childBlock.Labels[:len(s.LabelNames)-1] { if _, exists := targetMap[key]; !exists { @@ -1720,3 +1731,11 @@ func (s noopSpec) sourceRange(content *hcl.BodyContent, blockLabels []blockLabel Filename: "noopSpec", } } + +func prepareBodyVal(decodeResult cty.Value, body hcl.Body) cty.Value { + if m, ok := body.(MarkedBody); ok { + marks := m.BodyValueMarks() + return decodeResult.WithMarks(marks) + } + return decodeResult +} From 212a40e528766634a1aa6dd1e820d7936762196e Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Thu, 9 May 2024 11:50:35 -0700 Subject: [PATCH 08/10] Update CHANGELOG.md --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a33c5a68..e8d06e47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ ### Enhancements * Introduce `ParseTraversalPartial`, which allows traversals that include the splat (`[*]`) index operator. ([#673](https://github.com/hashicorp/hcl/pull/673)) +* ext/dynblock: Now accepts marked values in `for_each`, and will transfer those marks (as much as technically possible) to values in the generated blocks. ([#679](https://github.com/hashicorp/hcl/pull/679)) + +### Bugs Fixed + +* Expression evaluation will no longer panic if the splat operator is applied to an unknown value that has cty marks. ([#678](https://github.com/hashicorp/hcl/pull/678)) ## v2.20.1 (March 26, 2024) From f7e093a382df24049d3ebbaeb059550b2b755b6f Mon Sep 17 00:00:00 2001 From: "hashicorp-tsccr[bot]" <129506189+hashicorp-tsccr[bot]@users.noreply.github.com> Date: Fri, 14 Jun 2024 15:41:56 +0200 Subject: [PATCH 09/10] github: Pin action refs to latest trusted by TSCCR (#683) Co-authored-by: hashicorp-tsccr[bot] --- .github/workflows/checks.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 5102728d..a5f6b7af 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -30,7 +30,7 @@ jobs: run: | git config --global core.autocrlf false - name: "Fetch source code" - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 + uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - name: Install Go uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: @@ -44,7 +44,7 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 + uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - name: Install Go uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: @@ -58,7 +58,7 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 + uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - name: Install Go uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: @@ -72,7 +72,7 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 + uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - name: Install Go uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: From 360ae579460fab69e9939599af85c8f255a59007 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Wed, 19 Jun 2024 13:34:52 +0200 Subject: [PATCH 10/10] prepare for v2.21.0 release --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8d06e47..f81110dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # HCL Changelog -## v2.21.0 (Unreleased) +## v2.21.0 (June 19, 2024) ### Enhancements