fix: Fixed regression in tags with dynamic values (#511)

terraform-linters · Jul 12, 2023 · 666253c · 666253c
1 parent 619b911
commit 666253c
Show file tree

Hide file tree

Showing 2 changed files with 122 additions and 10 deletions.
diff --git a/rules/aws_resource_missing_tags.go b/rules/aws_resource_missing_tags.go
@@ -28,6 +28,7 @@ type awsResourceTagsRuleConfig struct {
 }
 
 const (
+	defaultTagsBlockName  = "default_tags"
 	tagsAttributeName     = "tags"
 	tagBlockName          = "tag"
 	providerAttributeName = "provider"
@@ -68,7 +69,7 @@ func (r *AwsResourceMissingTagsRule) getProviderLevelTags(runner tflint.Runner)
 		},
 		Blocks: []hclext.BlockSchema{
 			{
-				Type: "default_tags",
+				Type: defaultTagsBlockName,
 				Body: &hclext.BodySchema{Attributes: []hclext.AttributeSchema{{Name: tagsAttributeName}}},
 			},
 		},
@@ -89,8 +90,9 @@ func (r *AwsResourceMissingTagsRule) getProviderLevelTags(runner tflint.Runner)
 			providerAlias = "default"
 		} else {
 			err := runner.EvaluateExpr(providerAttr.Expr, func(alias string) error {
+				logger.Debug("Walk `%s` provider", providerAlias)
 				providerAlias = alias
-        // Init the provider reference even if it doesn't have tags
+				// Init the provider reference even if it doesn't have tags
 				allProviderTags[alias] = nil
 				return nil
 			}, nil)
@@ -106,7 +108,22 @@ func (r *AwsResourceMissingTagsRule) getProviderLevelTags(runner tflint.Runner)
 				continue
 			}
 
-			err := runner.EvaluateExpr(attr.Expr, func(tags map[string]string) error {
+			err := runner.EvaluateExpr(attr.Expr, func(val cty.Value) error {
+				// Skip unknown values
+				if !val.IsKnown() || val.IsNull() {
+					logger.Warn("The missing aws tags rule can only evaluate provided variables, skipping %s.", provider.Labels[0]+"."+providerAlias+"."+defaultTagsBlockName+"."+tagsAttributeName)
+					return nil
+				}
+
+				valMap := val.AsValueMap()
+				var tags map[string]string = make(map[string]string, len(valMap))
+				for k, v := range valMap {
+					tags[k] = ""
+					if !v.IsNull() && v.Type().FriendlyName() == "string" {
+						tags[k] = v.AsString()
+					}
+				}
+				logger.Debug("Walk `%s` provider with tags `%v`", providerAlias, tags)
 				providerTags = tags
 				return nil
 			}, nil)
@@ -188,17 +205,27 @@ func (r *AwsResourceMissingTagsRule) Check(runner tflint.Runner) error {
 					"Walk `%s` attribute",
 					resource.Labels[0]+"."+resource.Labels[1]+"."+tagsAttributeName,
 				)
-				// Since the evlaluateExpr, overrides k/v pairs, we need to re-copy the tags
-				resourceTagsAux := make(map[string]string)
 
-				err := runner.EvaluateExpr(attribute.Expr, func(val map[string]string) error {
-					resourceTagsAux = val
+				err := runner.EvaluateExpr(attribute.Expr, func(val cty.Value) error {
+					// Skip unknown values
+					if !val.IsKnown() || val.IsNull() {
+						logger.Warn("The missing aws tags rule can only evaluate provided variables, skipping %s.", resource.Labels[0]+"."+resource.Labels[1]+"."+tagsAttributeName)
+						return nil
+					}
+					valMap := val.AsValueMap()
+					// Since the evlaluateExpr, overrides k/v pairs, we need to re-copy the tags
+					var evaluatedTags map[string]string = make(map[string]string, len(valMap))
+					for k, v := range valMap {
+						evaluatedTags[k] = ""
+						if !v.IsNull() && v.Type().FriendlyName() == "string" {
+							evaluatedTags[k] = v.AsString()
+						}
+					}
+					maps.Copy(resourceTags, evaluatedTags)
+					r.emitIssue(runner, resourceTags, config, attribute.Expr.Range())
 					return nil
 				}, nil)
 
-				maps.Copy(resourceTags, resourceTagsAux)
-				r.emitIssue(runner, resourceTags, config, attribute.Expr.Range())
-
 				if err != nil {
 					return err
 				}

diff --git a/rules/aws_resource_missing_tags_test.go b/rules/aws_resource_missing_tags_test.go
@@ -426,6 +426,91 @@ resource "aws_ssm_parameter" "param" {
 rule "aws_resource_missing_tags" {
   enabled = true
   tags = ["Foo"]
+}`,
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "Unkown value maps should be silently ignored",
+			Content: `variable "aws_region" {
+  default = "us-east-1"
+  type = string
+}
+
+provider "aws" {
+  region = "us-east-1"
+  alias = "foo"
+  default_tags {
+    tags = {
+      Owner = "Owner"
+    }
+  }
+}
+
+resource "aws_s3_bucket" "a" {
+  provider = aws.foo
+  name = "a"
+}
+
+resource "aws_s3_bucket" "b" {
+  name = "b"
+  tags = var.default_tags
+}
+
+variable "default_tags" {
+  type = map(string)
+}
+
+provider "aws" {
+  region = "us-east-1"
+  alias = "bar"
+  default_tags {
+    tags = var.default_tags
+  }
+}`,
+			Config: `
+rule "aws_resource_missing_tags" {
+  enabled = true
+  tags = ["Owner"]
+}`,
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "Not wholly known tags as value maps should work as long as each key is statically defined",
+			Content: `variable "owner" {
+  type = string
+}
+
+variable "project" {
+  type = string
+}
+
+provider "aws" {
+  region = "us-east-1"
+  alias = "foo"
+  default_tags {
+    tags = {
+      Owner = var.owner
+      Project = var.project
+    }
+  }
+}
+
+resource "aws_s3_bucket" "a" {
+  provider = aws.foo
+  name = "a"
+}
+
+resource "aws_s3_bucket" "b" {
+  name = "b"
+  tags = {
+    Owner = var.owner
+    Project = var.project
+  }
+}`,
+			Config: `
+rule "aws_resource_missing_tags" {
+  enabled = true
+  tags = ["Owner", "Project"]
 }`,
 			Expected: helper.Issues{},
 		},