Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce separate testing scope for reference validation #33339

Merged
merged 2 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions internal/addrs/output_value.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,28 @@ import (
// that is defining it.
//
// This is related to but separate from ModuleCallOutput, which represents
// a module output from the perspective of its parent module. Since output
// values cannot be represented from the module where they are defined,
// OutputValue is not Referenceable, while ModuleCallOutput is.
// a module output from the perspective of its parent module. Outputs are
// referencable from the testing scope, in general terraform operation users
// will be referencing ModuleCallOutput.
type OutputValue struct {
referenceable
Copy link
Member

Choose a reason for hiding this comment

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

I cannot find any cases of this, but something to look out for is code which is meant to exclude things like outputs by checking if the address is referenceable.

Name string
}

func (v OutputValue) String() string {
return "output." + v.Name
}

func (v OutputValue) Equal(o OutputValue) bool {
return v.Name == o.Name
}

func (v OutputValue) UniqueKey() UniqueKey {
return v // An OutputValue is its own UniqueKey
}

func (v OutputValue) uniqueKeySigil() {}

// Absolute converts the receiver into an absolute address within the given
// module instance.
func (v OutputValue) Absolute(m ModuleInstance) AbsOutputValue {
Expand Down Expand Up @@ -82,7 +93,7 @@ func (v AbsOutputValue) String() string {
}

func (v AbsOutputValue) Equal(o AbsOutputValue) bool {
return v.OutputValue == o.OutputValue && v.Module.Equal(o.Module)
return v.OutputValue.Equal(o.OutputValue) && v.Module.Equal(o.Module)
}

func (v AbsOutputValue) ConfigOutputValue() ConfigOutputValue {
Expand Down
60 changes: 59 additions & 1 deletion internal/addrs/parse_ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import (

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty"

"github.com/hashicorp/terraform/internal/tfdiags"
)

// Reference describes a reference to an address with source location
Expand Down Expand Up @@ -82,6 +83,47 @@ func ParseRef(traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) {
return ref, diags
}

// ParseRefFromTestingScope adds check blocks and outputs into the available
// references returned by ParseRef.
//
// The testing files and functionality have a slightly expanded referencing
// scope and so should use this function to retrieve references.
func ParseRefFromTestingScope(traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) {
root := traversal.RootName()

var diags tfdiags.Diagnostics
var reference *Reference

switch root {
case "output":
name, rng, remain, outputDiags := parseSingleAttrRef(traversal)
reference = &Reference{
Subject: OutputValue{Name: name},
SourceRange: tfdiags.SourceRangeFromHCL(rng),
Remaining: remain,
}
diags = outputDiags
case "check":
name, rng, remain, checkDiags := parseSingleAttrRef(traversal)
reference = &Reference{
Subject: Check{Name: name},
SourceRange: tfdiags.SourceRangeFromHCL(rng),
Remaining: remain,
}
diags = checkDiags
}

if reference != nil {
if len(reference.Remaining) == 0 {
reference.Remaining = nil
}
return reference, diags
}

// If it's not an output or a check block, then just parse it as normal.
return ParseRef(traversal)
}

// ParseRefStr is a helper wrapper around ParseRef that takes a string
// and parses it with the HCL native syntax traversal parser before
// interpreting it.
Expand Down Expand Up @@ -111,6 +153,22 @@ func ParseRefStr(str string) (*Reference, tfdiags.Diagnostics) {
return ref, diags
}

// ParseRefStrFromTestingScope matches ParseRefStr except it supports the
// references supported by ParseRefFromTestingScope.
func ParseRefStrFromTestingScope(str string) (*Reference, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics

traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(str), "", hcl.Pos{Line: 1, Column: 1})
diags = diags.Append(parseDiags)
if parseDiags.HasErrors() {
return nil, diags
}

ref, targetDiags := ParseRefFromTestingScope(traversal)
diags = diags.Append(targetDiags)
return ref, diags
}

func parseRef(traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics

Expand Down
141 changes: 140 additions & 1 deletion internal/addrs/parse_ref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,117 @@ import (
"github.com/go-test/deep"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty"

"github.com/hashicorp/terraform/internal/tfdiags"
)

func TestParseRefInTestingScope(t *testing.T) {
tests := []struct {
Input string
Want *Reference
WantErr string
}{
{
`output.value`,
&Reference{
Subject: OutputValue{
Name: "value",
},
SourceRange: tfdiags.SourceRange{
Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0},
End: tfdiags.SourcePos{Line: 1, Column: 13, Byte: 12},
},
},
``,
},
{
`output`,
nil,
`The "output" object cannot be accessed directly. Instead, access one of its attributes.`,
},
{
`output["foo"]`,
nil,
`The "output" object does not support this operation.`,
},

{
`check.health`,
&Reference{
Subject: Check{
Name: "health",
},
SourceRange: tfdiags.SourceRange{
Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0},
End: tfdiags.SourcePos{Line: 1, Column: 13, Byte: 12},
},
},
``,
},
{
`check`,
nil,
`The "check" object cannot be accessed directly. Instead, access one of its attributes.`,
},
{
`check["foo"]`,
nil,
`The "check" object does not support this operation.`,
},

// Sanity check at least one of the others works to verify it does
// fall through to the core function.
{
`count.index`,
&Reference{
Subject: CountAttr{
Name: "index",
},
SourceRange: tfdiags.SourceRange{
Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0},
End: tfdiags.SourcePos{Line: 1, Column: 12, Byte: 11},
},
},
``,
},
}
for _, test := range tests {
t.Run(test.Input, func(t *testing.T) {
traversal, travDiags := hclsyntax.ParseTraversalAbs([]byte(test.Input), "", hcl.Pos{Line: 1, Column: 1})
if travDiags.HasErrors() {
t.Fatal(travDiags.Error())
}

got, diags := ParseRefFromTestingScope(traversal)

switch len(diags) {
case 0:
if test.WantErr != "" {
t.Fatalf("succeeded; want error: %s", test.WantErr)
}
case 1:
if test.WantErr == "" {
t.Fatalf("unexpected diagnostics: %s", diags.Err())
}
if got, want := diags[0].Description().Detail, test.WantErr; got != want {
t.Fatalf("wrong error\ngot: %s\nwant: %s", got, want)
}
default:
t.Fatalf("too many diagnostics: %s", diags.Err())
}

if diags.HasErrors() {
return
}

for _, problem := range deep.Equal(got, test.Want) {
t.Errorf(problem)
}
})
}
}

func TestParseRef(t *testing.T) {
tests := []struct {
Input string
Expand Down Expand Up @@ -719,6 +826,38 @@ func TestParseRef(t *testing.T) {
nil,
`A reference to a resource type must be followed by at least one attribute access, specifying the resource name.`,
},

// Should interpret checks and outputs as resource types.
{
`output.value`,
&Reference{
Subject: Resource{
Mode: ManagedResourceMode,
Type: "output",
Name: "value",
},
SourceRange: tfdiags.SourceRange{
Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0},
End: tfdiags.SourcePos{Line: 1, Column: 13, Byte: 12},
},
},
``,
},
{
`check.health`,
&Reference{
Subject: Resource{
Mode: ManagedResourceMode,
Type: "check",
Name: "health",
},
SourceRange: tfdiags.SourceRange{
Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0},
End: tfdiags.SourcePos{Line: 1, Column: 13, Byte: 12},
},
},
``,
},
}

for _, test := range tests {
Expand Down
7 changes: 4 additions & 3 deletions internal/command/jsonconfig/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import (

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hcldec"
"github.com/zclconf/go-cty/cty"
ctyjson "github.com/zclconf/go-cty/cty/json"

"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/configs/configschema"
"github.com/hashicorp/terraform/internal/lang"
"github.com/hashicorp/terraform/internal/lang/blocktoattr"
"github.com/zclconf/go-cty/cty"
ctyjson "github.com/zclconf/go-cty/cty/json"
)

// expression represents any unparsed expression
Expand Down Expand Up @@ -47,7 +48,7 @@ func marshalExpression(ex hcl.Expression) expression {
ret.ConstantValue = valJSON
}

refs, _ := lang.ReferencesInExpr(ex)
refs, _ := lang.ReferencesInExpr(addrs.ParseRef, ex)
if len(refs) > 0 {
var varString []string
for _, ref := range refs {
Expand Down
2 changes: 1 addition & 1 deletion internal/configs/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (cr *CheckRule) validateSelfReferences(checkType string, addr addrs.Resourc
if expr == nil {
continue
}
refs, _ := lang.References(expr.Variables())
refs, _ := lang.References(addrs.ParseRef, expr.Variables())
for _, ref := range refs {
var refAddr addrs.Resource

Expand Down
2 changes: 1 addition & 1 deletion internal/configs/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ func decodeReplaceTriggeredBy(expr hcl.Expression) ([]hcl.Expression, hcl.Diagno
exprs[i] = expr
}

refs, refDiags := lang.ReferencesInExpr(expr)
refs, refDiags := lang.ReferencesInExpr(addrs.ParseRef, expr)
for _, diag := range refDiags {
severity := hcl.DiagError
if diag.Severity() == tfdiags.Warning {
Expand Down
5 changes: 4 additions & 1 deletion internal/lang/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
package lang

import (
"github.com/zclconf/go-cty/cty"

"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty"
)

// Data is an interface whose implementations can provide cty.Value
Expand All @@ -33,4 +34,6 @@ type Data interface {
GetPathAttr(addrs.PathAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
GetTerraformAttr(addrs.TerraformAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
GetInputVariable(addrs.InputVariable, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
GetOutput(addrs.OutputValue, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
GetCheckBlock(addrs.Check, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
}
13 changes: 12 additions & 1 deletion internal/lang/data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,23 @@
package lang

import (
"github.com/zclconf/go-cty/cty"

"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty"
)

type dataForTests struct {
CountAttrs map[string]cty.Value
ForEachAttrs map[string]cty.Value
Resources map[string]cty.Value
LocalValues map[string]cty.Value
OutputValues map[string]cty.Value
Modules map[string]cty.Value
PathAttrs map[string]cty.Value
TerraformAttrs map[string]cty.Value
InputVariables map[string]cty.Value
CheckBlocks map[string]cty.Value
}

var _ Data = &dataForTests{}
Expand Down Expand Up @@ -63,3 +66,11 @@ func (d *dataForTests) GetPathAttr(addr addrs.PathAttr, rng tfdiags.SourceRange)
func (d *dataForTests) GetTerraformAttr(addr addrs.TerraformAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
return d.TerraformAttrs[addr.Name], nil
}

func (d *dataForTests) GetOutput(addr addrs.OutputValue, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
return d.OutputValues[addr.Name], nil
}

func (d *dataForTests) GetCheckBlock(addr addrs.Check, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
return d.CheckBlocks[addr.Name], nil
}
Loading