Skip to content

Commit 666253c

Browse files
authored
fix: Fixed regression in tags with dynamic values (#511)
1 parent 619b911 commit 666253c

File tree

2 files changed

+122
-10
lines changed

2 files changed

+122
-10
lines changed

rules/aws_resource_missing_tags.go

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ type awsResourceTagsRuleConfig struct {
2828
}
2929

3030
const (
31+
defaultTagsBlockName = "default_tags"
3132
tagsAttributeName = "tags"
3233
tagBlockName = "tag"
3334
providerAttributeName = "provider"
@@ -68,7 +69,7 @@ func (r *AwsResourceMissingTagsRule) getProviderLevelTags(runner tflint.Runner)
6869
},
6970
Blocks: []hclext.BlockSchema{
7071
{
71-
Type: "default_tags",
72+
Type: defaultTagsBlockName,
7273
Body: &hclext.BodySchema{Attributes: []hclext.AttributeSchema{{Name: tagsAttributeName}}},
7374
},
7475
},
@@ -89,8 +90,9 @@ func (r *AwsResourceMissingTagsRule) getProviderLevelTags(runner tflint.Runner)
8990
providerAlias = "default"
9091
} else {
9192
err := runner.EvaluateExpr(providerAttr.Expr, func(alias string) error {
93+
logger.Debug("Walk `%s` provider", providerAlias)
9294
providerAlias = alias
93-
// Init the provider reference even if it doesn't have tags
95+
// Init the provider reference even if it doesn't have tags
9496
allProviderTags[alias] = nil
9597
return nil
9698
}, nil)
@@ -106,7 +108,22 @@ func (r *AwsResourceMissingTagsRule) getProviderLevelTags(runner tflint.Runner)
106108
continue
107109
}
108110

109-
err := runner.EvaluateExpr(attr.Expr, func(tags map[string]string) error {
111+
err := runner.EvaluateExpr(attr.Expr, func(val cty.Value) error {
112+
// Skip unknown values
113+
if !val.IsKnown() || val.IsNull() {
114+
logger.Warn("The missing aws tags rule can only evaluate provided variables, skipping %s.", provider.Labels[0]+"."+providerAlias+"."+defaultTagsBlockName+"."+tagsAttributeName)
115+
return nil
116+
}
117+
118+
valMap := val.AsValueMap()
119+
var tags map[string]string = make(map[string]string, len(valMap))
120+
for k, v := range valMap {
121+
tags[k] = ""
122+
if !v.IsNull() && v.Type().FriendlyName() == "string" {
123+
tags[k] = v.AsString()
124+
}
125+
}
126+
logger.Debug("Walk `%s` provider with tags `%v`", providerAlias, tags)
110127
providerTags = tags
111128
return nil
112129
}, nil)
@@ -188,17 +205,27 @@ func (r *AwsResourceMissingTagsRule) Check(runner tflint.Runner) error {
188205
"Walk `%s` attribute",
189206
resource.Labels[0]+"."+resource.Labels[1]+"."+tagsAttributeName,
190207
)
191-
// Since the evlaluateExpr, overrides k/v pairs, we need to re-copy the tags
192-
resourceTagsAux := make(map[string]string)
193208

194-
err := runner.EvaluateExpr(attribute.Expr, func(val map[string]string) error {
195-
resourceTagsAux = val
209+
err := runner.EvaluateExpr(attribute.Expr, func(val cty.Value) error {
210+
// Skip unknown values
211+
if !val.IsKnown() || val.IsNull() {
212+
logger.Warn("The missing aws tags rule can only evaluate provided variables, skipping %s.", resource.Labels[0]+"."+resource.Labels[1]+"."+tagsAttributeName)
213+
return nil
214+
}
215+
valMap := val.AsValueMap()
216+
// Since the evlaluateExpr, overrides k/v pairs, we need to re-copy the tags
217+
var evaluatedTags map[string]string = make(map[string]string, len(valMap))
218+
for k, v := range valMap {
219+
evaluatedTags[k] = ""
220+
if !v.IsNull() && v.Type().FriendlyName() == "string" {
221+
evaluatedTags[k] = v.AsString()
222+
}
223+
}
224+
maps.Copy(resourceTags, evaluatedTags)
225+
r.emitIssue(runner, resourceTags, config, attribute.Expr.Range())
196226
return nil
197227
}, nil)
198228

199-
maps.Copy(resourceTags, resourceTagsAux)
200-
r.emitIssue(runner, resourceTags, config, attribute.Expr.Range())
201-
202229
if err != nil {
203230
return err
204231
}

rules/aws_resource_missing_tags_test.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,91 @@ resource "aws_ssm_parameter" "param" {
426426
rule "aws_resource_missing_tags" {
427427
enabled = true
428428
tags = ["Foo"]
429+
}`,
430+
Expected: helper.Issues{},
431+
},
432+
{
433+
Name: "Unkown value maps should be silently ignored",
434+
Content: `variable "aws_region" {
435+
default = "us-east-1"
436+
type = string
437+
}
438+
439+
provider "aws" {
440+
region = "us-east-1"
441+
alias = "foo"
442+
default_tags {
443+
tags = {
444+
Owner = "Owner"
445+
}
446+
}
447+
}
448+
449+
resource "aws_s3_bucket" "a" {
450+
provider = aws.foo
451+
name = "a"
452+
}
453+
454+
resource "aws_s3_bucket" "b" {
455+
name = "b"
456+
tags = var.default_tags
457+
}
458+
459+
variable "default_tags" {
460+
type = map(string)
461+
}
462+
463+
provider "aws" {
464+
region = "us-east-1"
465+
alias = "bar"
466+
default_tags {
467+
tags = var.default_tags
468+
}
469+
}`,
470+
Config: `
471+
rule "aws_resource_missing_tags" {
472+
enabled = true
473+
tags = ["Owner"]
474+
}`,
475+
Expected: helper.Issues{},
476+
},
477+
{
478+
Name: "Not wholly known tags as value maps should work as long as each key is statically defined",
479+
Content: `variable "owner" {
480+
type = string
481+
}
482+
483+
variable "project" {
484+
type = string
485+
}
486+
487+
provider "aws" {
488+
region = "us-east-1"
489+
alias = "foo"
490+
default_tags {
491+
tags = {
492+
Owner = var.owner
493+
Project = var.project
494+
}
495+
}
496+
}
497+
498+
resource "aws_s3_bucket" "a" {
499+
provider = aws.foo
500+
name = "a"
501+
}
502+
503+
resource "aws_s3_bucket" "b" {
504+
name = "b"
505+
tags = {
506+
Owner = var.owner
507+
Project = var.project
508+
}
509+
}`,
510+
Config: `
511+
rule "aws_resource_missing_tags" {
512+
enabled = true
513+
tags = ["Owner", "Project"]
429514
}`,
430515
Expected: helper.Issues{},
431516
},

0 commit comments

Comments
 (0)