Test for duplicate keys in a Terraform map literal

[tanelso2]

Hi there,

I ran into this issue yesterday in production and was looking to add some tests to make sure it doesn't happen again.

Would it be possible to implement this check in tflint? I'm willing to do so but unfamiliar with the codebase. Will need some guidance.

[bendrucker]

A good idea! You'll probably have to drop down to the hclsyntax layer:

https://pkg.go.dev/github.com/hashicorp/hcl2/hcl/hclsyntax

At the point where anything is represented as a Go map, either directly or indirectly, duplicates will be overridden. Go shares Terraform's behavior, FWIW, where the later entry in a map literal wins on key collisions.

The lower-level hclsyntax package offers more flexible/low-level data structure, in which objects are represented as lists of key/value pairs, i.e. slices of structs instead of an actual map:

https://pkg.go.dev/github.com/hashicorp/hcl2/hcl/hclsyntax#ObjectConsExpr

Here's a starting point for you, partially GPT-written but edited to get some of the finer points of type handling right:

package main

import (
	"fmt"

	"github.com/hashicorp/hcl/v2"
	"github.com/hashicorp/hcl/v2/hclsyntax"
	"github.com/zclconf/go-cty/cty"
)

func main() {
	hclContent := `
resource "null_resource" "test" {
    triggers = {
        a = "b"
        a = "c"
				dynamic = "d"
				(dynamic) = "e"
				(unknown) = "f"
    }
}
`
	file, diagnostics := hclsyntax.ParseConfig([]byte(hclContent), "main.tf", hcl.InitialPos)
	if diagnostics.HasErrors() {
		panic(diagnostics.Error())
	}

	findDuplicateKeys(file.Body)
}

func findDuplicateKeys(body hcl.Body) {
	content, _, diagnostics := body.PartialContent(&hcl.BodySchema{
		Blocks: []hcl.BlockHeaderSchema{{Type: "resource", LabelNames: []string{"type", "name"}}},
	})
	if diagnostics.HasErrors() {
		panic(diagnostics.Error())
	}

	for _, block := range content.Blocks {
		processBlock(block)
	}
}

func processBlock(block *hcl.Block) {
	content, _, diags := block.Body.PartialContent(&hcl.BodySchema{
		Attributes: []hcl.AttributeSchema{{Name: "triggers"}},
	})
	if diags.HasErrors() {
		panic(diags.Error())
	}

	if triggers, exists := content.Attributes["triggers"]; exists {
		expr := triggers.Expr
		exprMap, ok := expr.(*hclsyntax.ObjectConsExpr)
		if !ok {
			panic("Expected object expression")
		}

		keys := make(map[string]struct{})
		for _, item := range exprMap.Items {
			expr := item.KeyExpr.(*hclsyntax.ObjectConsKeyExpr)

			kv, diags := expr.Value(&hcl.EvalContext{
				Variables: map[string]cty.Value{
					"dynamic": cty.StringVal("dynamic"),
					"unknown": cty.UnknownVal(cty.String),
				},
			})

			if diags.HasErrors() {
				panic(diags.Error())
			}

			if !kv.IsKnown() {
				fmt.Printf("Unknown key, continuing")
				continue
			}

			if _, exists := keys[kv.AsString()]; exists {
				fmt.Printf("Duplicate key: %s\n", kv.AsString())
			} else {
				keys[kv.AsString()] = struct{}{}
			}
		}
	}
}

This prints:

Duplicate key: a
Duplicate key: dynamic
Unknown key, continuing

The hcl.EvalContext you'd pass would have all the Terraform things like var, locals, etc., allowing the rule to evaluate any statically analyzable reference. Values that are not known statically will need to be skipped, since calling AsString() on them will panic.

Hope this helps as a starting point!

[bendrucker]

Since .tfvars files are not part of Terraform modules themselves, TFLint will not load them. To give you an example from another hclsyntax rule, .tfvars files won't/can't come back from GetFiles():

https://github.com/terraform-linters/tflint-ruleset-terraform/blob/main/rules/terraform_comment_syntax.go#L53

While they're still HCL, they are a different type/structure of document. In theory it would be possible to provide a GetVarfiles API that rules that also wanted to inspect any varfile(s) configured with the run can also do that.

[bendrucker]

So if you manage to go ahead and implement this rule for Terraform files, it'll be pretty easy to add handling for varfiles. Which if we now have a real use case for, we can do.

[tanelso2]

The hcl.EvalContext you'd pass would have all the Terraform things like var, locals, etc., allowing the rule to evaluate any statically analyzable reference. Values that are not known statically will need to be skipped, since calling AsString() on them will panic.

How can I get the variables and locals inside of a tflint test?

I'm currently trying to use runner.EvaluateExpr to evaluate the expression, but it returns an error on this simple example:

locals {
  x = "y"
  y = local.x
}

resource "null_resource" "test" {
	map = {
	  (local.x) = 5
	  (local.y) = local.x
	}
}

The error:

module.tf:9,7-12: Unknown variable; There is no variable named "local"

[bendrucker]

The test runner does not ship with complete support for Terraform documents. Only variable blocks are parsed at this time and provided as part of the eval context, not locals. And IIRC functions may also be unsupported.

https://github.com/terraform-linters/tflint-plugin-sdk/blob/63caa056a756aad72f6a4abd322acf74568fa772/helper/runner.go#L375-L400

You can define a variable with a default to provide a known reference, or no default to provide an unknown one.

[tanelso2]

Thanks for the guidance! I have my first draft of this change ready for review: #194