Skip to content

Commit

Permalink
Fix incorrect circular reference detection
Browse files Browse the repository at this point in the history
  • Loading branch information
wata727 committed Oct 24, 2022
1 parent f978a90 commit ced535c
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 22 deletions.
45 changes: 25 additions & 20 deletions terraform/evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@ type ContextMeta struct {
OriginalWorkingDir string
}

type CallGraph struct {
edges map[string]addrs.Reference
type CallStack struct {
addrs map[string]addrs.Reference
stack []string
}

func NewCallGraph() *CallGraph {
return &CallGraph{
edges: make(map[string]addrs.Reference),
func NewCallStack() *CallStack {
return &CallStack{
addrs: make(map[string]addrs.Reference),
stack: make([]string, 0),
}
}

func (g *CallGraph) Add(addr addrs.Reference) hcl.Diagnostics {
func (g *CallStack) Push(addr addrs.Reference) hcl.Diagnostics {
g.stack = append(g.stack, addr.Subject.String())

if _, exists := g.edges[addr.Subject.String()]; exists {
if _, exists := g.addrs[addr.Subject.String()]; exists {
return hcl.Diagnostics{
{
Severity: hcl.DiagError,
Expand All @@ -45,20 +45,30 @@ func (g *CallGraph) Add(addr addrs.Reference) hcl.Diagnostics {
},
}
}
g.edges[addr.Subject.String()] = addr
g.addrs[addr.Subject.String()] = addr
return hcl.Diagnostics{}
}

func (g *CallGraph) String() string {
func (g *CallStack) Pop() {
if g.Empty() {
panic("cannot pop from empty call graph")
}

addr := g.stack[len(g.stack)-1]
g.stack = g.stack[:len(g.stack)-1]
delete(g.addrs, addr)
}

func (g *CallStack) String() string {
return strings.Join(g.stack, " -> ")
}

func (g *CallGraph) Empty() bool {
func (g *CallStack) Empty() bool {
return len(g.stack) == 0
}

func (g *CallGraph) Clear() {
g.edges = make(map[string]addrs.Reference)
func (g *CallStack) Clear() {
g.addrs = make(map[string]addrs.Reference)
g.stack = make([]string, 0)
}

Expand All @@ -67,7 +77,7 @@ type Evaluator struct {
ModulePath addrs.ModuleInstance
Config *Config
VariableValues map[string]map[string]cty.Value
CallGraph *CallGraph
CallStack *CallStack
}

func (e *Evaluator) EvaluateExpr(expr hcl.Expression, wantType cty.Type, keyData InstanceKeyEvalData) (cty.Value, hcl.Diagnostics) {
Expand Down Expand Up @@ -298,21 +308,16 @@ func (d *evaluationData) GetLocalValue(addr addrs.LocalValue, rng hcl.Range) (ct
return cty.DynamicVal, diags
}

entry := d.Evaluator.CallGraph.Empty()
// Build a callgraph for circular reference detection only when getting a local value.
if diags := d.Evaluator.CallGraph.Add(addrs.Reference{Subject: addr, SourceRange: rng}); diags.HasErrors() {
if diags := d.Evaluator.CallStack.Push(addrs.Reference{Subject: addr, SourceRange: rng}); diags.HasErrors() {
return cty.UnknownVal(cty.DynamicPseudoType), diags
}

// Always use EvalDataForNoInstanceKey because local values cannot use expressions
// that depend on instance keys, such as `count.*` and `each.*`.
val, diags := d.Evaluator.EvaluateExpr(config.Expr, cty.DynamicPseudoType, EvalDataForNoInstanceKey)

// Since we build a callgraph for each local value,
// we clear the callgraph when the local value is finally resolved.
if entry {
d.Evaluator.CallGraph.Clear()
}
d.Evaluator.CallStack.Pop()
return val, diags
}

Expand Down
14 changes: 13 additions & 1 deletion terraform/evaluator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,18 @@ locals {
return diags.Error() != `main.tf:4,9-18: circular reference found; local.foo -> local.bar -> local.foo`
},
},
{
name: "nested multiple local values",
config: `
locals {
foo = "foo"
bar = [local.foo, local.foo]
}`,
expr: expr(`local.bar`),
ty: cty.List(cty.String),
want: `cty.ListVal([]cty.Value{cty.StringVal("foo"), cty.StringVal("foo")})`,
errCheck: neverHappend,
},
{
name: "count.index in non-counted context",
expr: expr(`count.index`),
Expand Down Expand Up @@ -666,7 +678,7 @@ locals {
ModulePath: config.Path.UnkeyedInstanceShim(),
Config: config,
VariableValues: variableValues,
CallGraph: NewCallGraph(),
CallStack: NewCallStack(),
}

got, diags := evaluator.EvaluateExpr(test.expr, test.ty, test.keyData)
Expand Down
2 changes: 1 addition & 1 deletion tflint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func NewRunner(c *Config, ants map[string]Annotations, cfg *terraform.Config, va
ModulePath: cfg.Path.UnkeyedInstanceShim(),
Config: cfg.Root,
VariableValues: variableValues,
CallGraph: terraform.NewCallGraph(),
CallStack: terraform.NewCallStack(),
}

runner := &Runner{
Expand Down

0 comments on commit ced535c

Please sign in to comment.