Add new rule: terraform_map_duplicate_values

tiulpin · tiulpin · commit e530dc1fca2c · 2024-11-26T13:39:22.000+01:00
diff --git a/docs/rules/terraform_map_duplicate_values.md b/docs/rules/terraform_map_duplicate_values.md
@@ -0,0 +1,35 @@
+# terraform_map_duplicate_values
+
+Disallow duplicate values in a map object.
+
+## Example
+
+```hcl
+locals {
+  map = {
+    foo = 1
+    bar = 1 // duplicate value
+  }
+}
+```
+
+```
+$ tflint
+1 issue(s) found:
+
+Warning: Duplicate key: "bar", first defined at main.tf:4,5-8 (terraform_map_duplicate_values)
+
+  on main.tf line 5:
+   5:     bar = 3 // duplicated value
+
+Reference: https://github.com/terraform-linters/tflint-ruleset-terraform/blob/v0.11.0/docs/rules/terraform_map_duplicate_values.md
+```
+
+## Why
+
+Sometimes, you want to maintain a map that contains only unique values (e.g., do not want to get duplicated SSM parameters values). This rule will catch such mistakes early.
+The map structure is not a set, so it is possible to have duplicate values in a map, so make sure you run this rule only on files where you want to enforce unique values.
+
+## How To Fix
+
+Remove the duplicate values and leave the correct value.
diff --git a/rules/preset.go b/rules/preset.go
@@ -12,6 +12,7 @@ var PresetRules = map[string][]tflint.Rule{
 		NewTerraformDocumentedVariablesRule(),
 		NewTerraformEmptyListEqualityRule(),
 		NewTerraformMapDuplicateKeysRule(),
+		NewTerraformMapDuplicateValuesRule(),
 		NewTerraformModulePinnedSourceRule(),
 		NewTerraformModuleVersionRule(),
 		NewTerraformNamingConventionRule(),
diff --git a/rules/terraform_map_duplicate_values.go b/rules/terraform_map_duplicate_values.go
@@ -0,0 +1,117 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/hcl/v2"
+	"github.com/hashicorp/hcl/v2/hclsyntax"
+	"github.com/terraform-linters/tflint-plugin-sdk/logger"
+	"github.com/terraform-linters/tflint-plugin-sdk/tflint"
+	"github.com/terraform-linters/tflint-ruleset-terraform/project"
+	"github.com/zclconf/go-cty/cty"
+	"github.com/zclconf/go-cty/cty/convert"
+)
+
+// This rule checks for map literals with duplicate values
+type TerraformMapDuplicateValuesRule struct {
+	tflint.DefaultRule
+}
+
+func NewTerraformMapDuplicateValuesRule() *TerraformMapDuplicateValuesRule {
+	return &TerraformMapDuplicateValuesRule{}
+}
+
+func (r *TerraformMapDuplicateValuesRule) Name() string {
+	return "terraform_map_duplicate_values"
+}
+
+func (r *TerraformMapDuplicateValuesRule) Enabled() bool {
+	return true
+}
+
+func (r *TerraformMapDuplicateValuesRule) Severity() tflint.Severity {
+	return tflint.WARNING
+}
+
+func (r *TerraformMapDuplicateValuesRule) Link() string {
+	return project.ReferenceLink(r.Name())
+}
+
+func (r *TerraformMapDuplicateValuesRule) Check(runner tflint.Runner) error {
+	path, err := runner.GetModulePath()
+	if err != nil {
+		return err
+	}
+	if !path.IsRoot() {
+		// This rule does not evaluate child modules
+		return nil
+	}
+
+	diags := runner.WalkExpressions(tflint.ExprWalkFunc(func(e hcl.Expression) hcl.Diagnostics {
+		return r.checkObjectConsExpr(e, runner)
+	}))
+	if diags.HasErrors() {
+		return diags
+	}
+
+	return nil
+}
+
+func (r *TerraformMapDuplicateValuesRule) checkObjectConsExpr(e hcl.Expression, runner tflint.Runner) hcl.Diagnostics {
+	objExpr, ok := e.(*hclsyntax.ObjectConsExpr)
+	if !ok {
+		return nil
+	}
+
+	var diags hcl.Diagnostics
+	values := make(map[string]hcl.Range)
+
+	for _, item := range objExpr.Items {
+		valExpr := item.ValueExpr
+		var val cty.Value
+
+		err := runner.EvaluateExpr(valExpr, &val, nil)
+		if err != nil {
+			logger.Debug("Failed to evaluate value. The value will be ignored", "range", valExpr.Range(), "error", err.Error())
+			continue
+		}
+
+		if !val.IsKnown() || val.IsNull() || val.IsMarked() {
+			logger.Debug("Unprocessable value, continuing", "range", valExpr.Range())
+			continue
+		}
+		// Map values must be strings, but some values ​​can be converted to strings and become valid values,
+		// so try to convert them here.
+		if converted, err := convert.Convert(val, cty.String); err == nil {
+			val = converted
+		}
+
+		// ignore unprocessable values and boolean values
+		if val.Type() != cty.String || val.AsString() == "true" || val.AsString() == "false" {
+			logger.Debug("Unprocessable value, continuing", "range", valExpr.Range())
+			continue
+		}
+
+		if declRange, exists := values[val.AsString()]; exists {
+			if err := runner.EmitIssue(
+				r,
+				fmt.Sprintf("Duplicate value: %q, first defined at %s", val.AsString(), declRange),
+				valExpr.Range(),
+			); err != nil {
+				diags = append(diags, &hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "failed to call EmitIssue()",
+					Detail:   err.Error(),
+				})
+
+				return diags
+			}
+
+			continue
+		}
+
+		values[val.AsString()] = valExpr.Range()
+	}
+
+	return diags
+}
diff --git a/rules/terraform_map_duplicate_values_test.go b/rules/terraform_map_duplicate_values_test.go
@@ -0,0 +1,233 @@
+package rules
+
+import (
+	"testing"
+
+	"github.com/hashicorp/hcl/v2"
+	"github.com/terraform-linters/tflint-plugin-sdk/helper"
+)
+
+func Test_TerraformMapDuplicateValues(t *testing.T) {
+	cases := []struct {
+		Name     string
+		Content  string
+		Expected helper.Issues
+		Fixed    string
+	}{
+		{
+			Name: "No duplicates",
+			Content: `
+resource "null_resource" "test" {
+	test = {
+	  a = 1
+	  b = 2
+	  c = 3
+	}
+}`,
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "duplicate values in map literal",
+			Content: `
+resource "null_resource" "test" {
+    triggers = {
+        a = "b"
+        c = "b"
+    }
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewTerraformMapDuplicateValuesRule(),
+					Message: `Duplicate value: "b", first defined at module.tf:4,13-16`,
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start:    hcl.Pos{Line: 5, Column: 13},
+						End:      hcl.Pos{Line: 5, Column: 16},
+					},
+				},
+			},
+		},
+		{
+			Name: "duplicate values with quoting",
+			Content: `
+resource "null_resource" "test" {
+    triggers = {
+        a = "b"
+        c = "b"
+    }
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewTerraformMapDuplicateValuesRule(),
+					Message: `Duplicate value: "b", first defined at module.tf:4,13-16`,
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start:    hcl.Pos{Line: 5, Column: 13},
+						End:      hcl.Pos{Line: 5, Column: 16},
+					},
+				},
+			},
+		},
+		{
+			Name: "Using variables as values",
+			Content: `
+variable "a" {
+  type    = string
+  default = "b"
+}
+
+resource "null_resource" "test" {
+	map = {
+	  key1 = var.a
+	  key2 = "b"
+	}
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewTerraformMapDuplicateValuesRule(),
+					Message: `Duplicate value: "b", first defined at module.tf:9,11-16`,
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start:    hcl.Pos{Line: 10, Column: 11},
+						End:      hcl.Pos{Line: 10, Column: 14},
+					},
+				},
+			},
+		},
+		{
+			Name: "Using a variable as a value without a default",
+			Content: `
+variable "unknown" {
+  type    = string
+}
+
+resource "null_resource" "test" {
+	map = {
+	  key1 = "x"
+	  key2 = var.unknown
+	}
+}`,
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "Multiple duplicates in same map",
+			Content: `
+resource "null_resource" "test" {
+	map = {
+	  key1 = "a"
+	  key2 = "a"
+	  key3 = "a"
+	}
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewTerraformMapDuplicateValuesRule(),
+					Message: `Duplicate value: "a", first defined at module.tf:4,11-14`,
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start:    hcl.Pos{Line: 5, Column: 11},
+						End:      hcl.Pos{Line: 5, Column: 14},
+					},
+				},
+				{
+					Rule:    NewTerraformMapDuplicateValuesRule(),
+					Message: `Duplicate value: "a", first defined at module.tf:4,11-14`,
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start:    hcl.Pos{Line: 6, Column: 11},
+						End:      hcl.Pos{Line: 6, Column: 14},
+					},
+				},
+			},
+		},
+		{
+			Name: "Using same value in different maps is okay",
+			Content: `
+resource "null_resource" "test" {
+	map1 = {
+	  key1 = "x"
+	}
+	map2 = {
+	  key2 = "x"
+	}
+}`,
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "Using sensitive variable values",
+			Content: `
+variable "sensitive" {
+  default = "secret"
+  sensitive = true
+}
+
+resource "null_resource" "test" {
+  map = {
+    key1 = var.sensitive
+    key2 = "secret"
+  }
+}`,
+			// Do not report sensitive duplicate values to prevent unintentional exposure of sensitive values
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "Using non-string values",
+			Content: `
+resource "null_resource" "test" {
+  map = {
+    key1 = 1
+    key2 = 1
+    key3 = {}
+  }
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewTerraformMapDuplicateValuesRule(),
+					Message: `Duplicate value: "1", first defined at module.tf:4,12-13`,
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start:    hcl.Pos{Line: 5, Column: 12},
+						End:      hcl.Pos{Line: 5, Column: 13},
+					},
+				},
+			},
+		},
+		{
+			Name: "values in for expressions",
+			Content: `
+resource "null_resource" "test" {
+  list = [for a in ["foo", "bar"] : {
+    key1 = "${a}_baz"
+	key2 = "foo_baz"
+  }]
+}`,
+			// The current implementation cannot find duplicate values in for expressions.
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "ignore boolean string values",
+			Content: `
+resource "null_resource" "test" {
+  map = {
+    key1 = true
+    key2 = true
+  }
+}`,
+			Expected: helper.Issues{},
+		},
+	}
+
+	rule := NewTerraformMapDuplicateValuesRule()
+
+	for _, tc := range cases {
+		t.Run(tc.Name, func(t *testing.T) {
+			runner := testRunner(t, map[string]string{"module.tf": tc.Content})
+
+			if err := rule.Check(runner); err != nil {
+				t.Fatalf("Unexpected error occurred: %s", err)
+			}
+
+			helper.AssertIssues(t, tc.Expected, runner.Runner.(*helper.Runner).Issues)
+		})
+	}
+}
