From dea5bcc457a5c3097d591721572d12994819d3c2 Mon Sep 17 00:00:00 2001 From: Jorge Reus Date: Wed, 5 Jul 2023 17:14:27 -0600 Subject: [PATCH] fix: Fixed regression with dynamic values in tags - Fixed the error when using dynamic values in tags without the usage of default_tags that showed issues even though the tag Key was present - Added debug logs Signed-off-by: Jorge Reus --- rules/aws_resource_missing_tags.go | 47 +++++++++++--- rules/aws_resource_missing_tags_test.go | 85 +++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 10 deletions(-) diff --git a/rules/aws_resource_missing_tags.go b/rules/aws_resource_missing_tags.go index ec428b4c..ef3934d4 100644 --- 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 index f05709aa..d4364002 100644 --- 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{}, },