From 89c6ed5083d15bcd252bac486704c5507609fa87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20G=C3=B3mez=20Reus?= Date: Tue, 13 Jun 2023 08:39:50 -0600 Subject: [PATCH] feat: Added default tags functionality (#489) * feat: Added default tags functionality * feat: Removed deprecated funcions - Removed deprecated runner.EnsureNoError calls - Removed explicit pass of types to EvaluateExpr calls * fix: Changed runner.EvaluateExpr to gohcl.DecodeExpression * fix: Removed uneccesary runner.EnsureNoError when validating tags * fix: Chaged providerAlias var to "provider" in aws.DecodeProviderConfigRef * fix: Added RaiseErr field in tests to allow checking for errors - Added the RaiseErr field in the test table struct, this is due to having the ability of checking errors apart of helper issues, also checked if the provider existed using the mentioned mechanism - Erased unecessary error checking when decoding provider config --- aws/decode.go | 2 +- aws/runner.go | 2 +- go.mod | 9 +- go.sum | 3 + rules/aws_resource_missing_tags.go | 137 ++++++++++++++++++-- rules/aws_resource_missing_tags_test.go | 158 +++++++++++++++++++++++- 6 files changed, 295 insertions(+), 16 deletions(-) diff --git a/aws/decode.go b/aws/decode.go index 0e6301b2..cdc1dc40 100644 --- a/aws/decode.go +++ b/aws/decode.go @@ -25,7 +25,7 @@ type ProviderConfigRef struct { } // original code: https://github.com/hashicorp/terraform/blob/3fbedf25430ead97eb42575d344427db3c32d524/internal/configs/resource.go#L498-L569 -func decodeProviderConfigRef(expr hcl.Expression, argName string) (*ProviderConfigRef, hcl.Diagnostics) { +func DecodeProviderConfigRef(expr hcl.Expression, argName string) (*ProviderConfigRef, hcl.Diagnostics) { var diags hcl.Diagnostics var shimDiags hcl.Diagnostics diff --git a/aws/runner.go b/aws/runner.go index f37d4ca7..b10560dd 100644 --- a/aws/runner.go +++ b/aws/runner.go @@ -48,7 +48,7 @@ func NewRunner(runner tflint.Runner, config *Config) (*Runner, error) { func (r *Runner) AwsClient(attributes hclext.Attributes) (*Client, error) { provider := "aws" if attr, exists := attributes["provider"]; exists { - providerConfigRef, diags := decodeProviderConfigRef(attr.Expr, "provider") + providerConfigRef, diags := DecodeProviderConfigRef(attr.Expr, "provider") if diags.HasErrors() { logger.Error("parse resource provider attribute: %s", diags) return nil, diags diff --git a/go.mod b/go.mod index 27e6aaf6..8b33df0c 100644 --- a/go.mod +++ b/go.mod @@ -28,10 +28,15 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect ) -require golang.org/x/net v0.10.0 +require ( + github.com/stretchr/testify v1.7.2 + golang.org/x/exp v0.0.0-20230425010034-47ecfdc1ba53 + golang.org/x/net v0.10.0 +) require ( github.com/apparentlymart/go-textseg/v13 v13.0.0 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/golang/protobuf v1.5.2 // indirect github.com/hashicorp/errwrap v1.0.0 // indirect github.com/hashicorp/go-hclog v1.5.0 // indirect @@ -41,10 +46,12 @@ require ( github.com/kr/pretty v0.2.1 // indirect github.com/mattn/go-isatty v0.0.14 // indirect github.com/oklog/run v1.0.0 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/vmihailenco/msgpack/v5 v5.3.5 // indirect github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect golang.org/x/mod v0.10.0 // indirect golang.org/x/sys v0.8.0 // indirect golang.org/x/text v0.9.0 // indirect golang.org/x/tools v0.8.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 298ab45f..6ca86e56 100644 --- a/go.sum +++ b/go.sum @@ -94,6 +94,8 @@ github.com/zclconf/go-cty v1.13.2/go.mod h1:YKQzy/7pZ7iq2jNFzy5go57xdxdWoLLpaEp4 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= +golang.org/x/exp v0.0.0-20230425010034-47ecfdc1ba53 h1:5llv2sWeaMSnA3w2kS57ouQQ4pudlXrR0dCgw51QK9o= +golang.org/x/exp v0.0.0-20230425010034-47ecfdc1ba53/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.10.0 h1:lFO9qtOdlre5W1jxS3r/4szv2/6iXxScdzjoBMXNhYk= @@ -154,6 +156,7 @@ google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp0 google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.30.0 h1:kPPoIgf3TsEvrm0PFe15JQ+570QVxYzEvvHqChK+cng= google.golang.org/protobuf v1.30.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/rules/aws_resource_missing_tags.go b/rules/aws_resource_missing_tags.go index 62e740d7..f8de36a2 100644 --- a/rules/aws_resource_missing_tags.go +++ b/rules/aws_resource_missing_tags.go @@ -1,6 +1,7 @@ package rules import ( + "errors" "fmt" "sort" "strings" @@ -9,9 +10,11 @@ import ( "github.com/terraform-linters/tflint-plugin-sdk/hclext" "github.com/terraform-linters/tflint-plugin-sdk/logger" "github.com/terraform-linters/tflint-plugin-sdk/tflint" + "github.com/terraform-linters/tflint-ruleset-aws/aws" "github.com/terraform-linters/tflint-ruleset-aws/project" "github.com/terraform-linters/tflint-ruleset-aws/rules/tags" "github.com/zclconf/go-cty/cty" + "golang.org/x/exp/maps" ) // AwsResourceMissingTagsRule checks whether resources are tagged correctly @@ -25,8 +28,9 @@ type awsResourceTagsRuleConfig struct { } const ( - tagsAttributeName = "tags" - tagBlockName = "tag" + tagsAttributeName = "tags" + tagBlockName = "tag" + providerAttributeName = "provider" ) // NewAwsResourceMissingTagsRule returns new rules for all resources that support tags @@ -54,6 +58,68 @@ func (r *AwsResourceMissingTagsRule) Link() string { return project.ReferenceLink(r.Name()) } +func (r *AwsResourceMissingTagsRule) getProviderLevelTags(runner tflint.Runner) (map[string]map[string]string, error) { + providerSchema := &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{ + { + Name: "alias", + Required: false, + }, + }, + Blocks: []hclext.BlockSchema{ + { + Type: "default_tags", + Body: &hclext.BodySchema{Attributes: []hclext.AttributeSchema{{Name: tagsAttributeName}}}, + }, + }, + } + + providerBody, err := runner.GetProviderContent("aws", providerSchema, nil) + if err != nil { + return nil, err + } + + // Get provider default tags + allProviderTags := make(map[string]map[string]string) + var providerAlias string + for _, provider := range providerBody.Blocks.OfType(providerAttributeName) { + providerTags := make(map[string]string) + for _, block := range provider.Body.Blocks { + attr, ok := block.Body.Attributes[tagsAttributeName] + if !ok { + continue + } + + err := runner.EvaluateExpr(attr.Expr, func(tags map[string]string) error { + providerTags = tags + return nil + }, nil) + + if err != nil { + return nil, err + } + + // Get the alias attribute, in terraform when there is a single aws provider its called "default" + providerAttr, ok := provider.Body.Attributes["alias"] + if !ok { + providerAlias = "default" + allProviderTags[providerAlias] = providerTags + } else { + err := runner.EvaluateExpr(providerAttr.Expr, func(alias string) error { + providerAlias = alias + return nil + }, nil) + // Assign default provider + allProviderTags[providerAlias] = providerTags + if err != nil { + return nil, err + } + } + } + } + return allProviderTags, nil +} + // Check checks resources for missing tags func (r *AwsResourceMissingTagsRule) Check(runner tflint.Runner) error { config := awsResourceTagsRuleConfig{} @@ -61,6 +127,12 @@ func (r *AwsResourceMissingTagsRule) Check(runner tflint.Runner) error { return err } + providerTagsMap, err := r.getProviderLevelTags(runner) + + if err != nil { + return err + } + for _, resourceType := range tags.Resources { // Skip this resource if its type is excluded in configuration if stringInSlice(resourceType, config.Exclude) { @@ -77,29 +149,74 @@ func (r *AwsResourceMissingTagsRule) Check(runner tflint.Runner) error { } resources, err := runner.GetResourceContent(resourceType, &hclext.BodySchema{ - Attributes: []hclext.AttributeSchema{{Name: tagsAttributeName}}, + Attributes: []hclext.AttributeSchema{ + {Name: tagsAttributeName}, + {Name: providerAttributeName}, + }, }, nil) if err != nil { return err } + if resources.IsEmpty() { + continue + } + for _, resource := range resources.Blocks { - if attribute, ok := resource.Body.Attributes[tagsAttributeName]; ok { - logger.Debug("Walk `%s` attribute", resource.Labels[0]+"."+resource.Labels[1]+"."+tagsAttributeName) - wantType := cty.Map(cty.String) - err := runner.EvaluateExpr(attribute.Expr, func(resourceTags map[string]string) error { - r.emitIssue(runner, resourceTags, config, attribute.Expr.Range()) + providerAlias := "default" + // Override the provider alias if defined + if val, ok := resource.Body.Attributes[providerAttributeName]; ok { + provider, diagnostics := aws.DecodeProviderConfigRef(val.Expr, "provider") + providerAlias = provider.Alias + + if _, hasProvider := providerTagsMap[providerAlias]; !hasProvider { + errString := fmt.Sprintf( + "The aws provider with alias \"%s\" doesn't exist.", + providerAlias, + ) + logger.Error("Error querying provider tags: %s", errString) + return errors.New(errString) + } + + if diagnostics.HasErrors() { + logger.Error("error decoding provider: %w", diagnostics) + return diagnostics + } + } + + resourceTags := make(map[string]string) + + // The provider tags are to be overriden + // https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags + maps.Copy(resourceTags, providerTagsMap[providerAlias]) + + // If the resource has a tags attribute + if attribute, okResource := resource.Body.Attributes[tagsAttributeName]; okResource { + logger.Debug( + "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 return nil - }, &tflint.EvaluateExprOption{WantType: &wantType}) + }, nil) + + maps.Copy(resourceTags, resourceTagsAux) + r.emitIssue(runner, resourceTags, config, attribute.Expr.Range()) + if err != nil { return err } } else { logger.Debug("Walk `%s` resource", resource.Labels[0]+"."+resource.Labels[1]) - r.emitIssue(runner, map[string]string{}, config, resource.DefRange) + r.emitIssue(runner, resourceTags, config, resource.DefRange) } } } + return nil } diff --git a/rules/aws_resource_missing_tags_test.go b/rules/aws_resource_missing_tags_test.go index 9f624a2b..14ebfae0 100644 --- a/rules/aws_resource_missing_tags_test.go +++ b/rules/aws_resource_missing_tags_test.go @@ -1,9 +1,11 @@ package rules import ( + "errors" "testing" hcl "github.com/hashicorp/hcl/v2" + "github.com/stretchr/testify/assert" "github.com/terraform-linters/tflint-plugin-sdk/helper" ) @@ -13,6 +15,7 @@ func Test_AwsResourceMissingTags(t *testing.T) { Content string Config string Expected helper.Issues + RaiseErr error }{ { Name: "Wanted tags: Bar,Foo, found: bar,foo", @@ -259,6 +262,151 @@ rule "aws_resource_missing_tags" { }, }, }, + { + Name: "Default tags multiple providers", + Content: ` +provider "aws" { + default_tags { + tags = { + "Fooz": "Barz" + "Bazz": "Quxz" + } + } +} + +provider "aws" { + alias = "foo" + default_tags { + tags = { + "Bazz": "Quxz" + "Fooz": "Barz" + } + } +} + +resource "aws_instance" "ec2_instance" { + instance_type = "t2.micro" +} + +resource "aws_instance" "ec2_instance_alias" { + provider = aws.foo + instance_type = "t2.micro" +}`, + Config: ` +rule "aws_resource_missing_tags" { + enabled = true + tags = ["Bazz", "Fooz"] +}`, + Expected: helper.Issues{}, + }, + { + Name: "Default Tags Are to Be overriden by resource specific tags", + Content: ` +provider "aws" { + default_tags { + tags = { + "Foo": "Bar" + } + } +} + +resource "aws_instance" "ec2_instance" { + instance_type = "t2.micro" + tags = { + "Foo": "Bazz" + } +}`, + Config: ` +rule "aws_resource_missing_tags" { + enabled = true + tags = ["Foo"] +}`, + Expected: helper.Issues{}, + }, + { + Name: "Resource specific tags are not needed if default tags are placed", + Content: ` +provider "aws" { + default_tags { + tags = { + "Foo": "Bar" + } + } +} + +resource "aws_instance" "ec2_instance" { + instance_type = "t2.micro" +}`, + Config: ` +rule "aws_resource_missing_tags" { + enabled = true + tags = ["Foo"] +}`, + Expected: helper.Issues{}, + }, + { + Name: "Resource tags in combination with provider level tags", + Content: ` +provider "aws" { + default_tags { + tags = { + "Foo": "Bar" + } + } +} + +resource "aws_instance" "ec2_instance_fail" { + instance_type = "t2.micro" +} + + +resource "aws_instance" "ec2_instance" { + instance_type = "t2.micro" + tags = { + "Bazz": "Quazz" + } +}`, + Config: ` +rule "aws_resource_missing_tags" { + enabled = true + tags = ["Foo", "Bazz"] +}`, + Expected: helper.Issues{ + { + Rule: NewAwsResourceMissingTagsRule(), + Message: "The resource is missing the following tags: \"Bazz\".", + Range: hcl.Range{ + Filename: "module.tf", + Start: hcl.Pos{Line: 10, Column: 1}, + End: hcl.Pos{Line: 10, Column: 44}, + }, + }, + }, + }, + { + Name: "Provider reference no existent", + Content: `provider "aws" { + alias = "zoom" + default_tags { + tags = { + "Foo": "Bar" + } + } +} + +resource "aws_instance" "ec2_instance" { + provider = aws.west + instance_type = "t2.micro" +}`, + Config: ` +rule "aws_resource_missing_tags" { + enabled = true + tags = ["Foo"] +}`, + Expected: helper.Issues{ + }, + RaiseErr: errors.New("The aws provider with alias \"west\" doesn't exist."), + }, } rule := NewAwsResourceMissingTagsRule() @@ -266,9 +414,13 @@ rule "aws_resource_missing_tags" { for _, tc := range cases { runner := helper.TestRunner(t, map[string]string{"module.tf": tc.Content, ".tflint.hcl": tc.Config}) - if err := rule.Check(runner); err != nil { - t.Fatalf("Unexpected error occurred: %s", err) - } + err := rule.Check(runner) + + if tc.RaiseErr == nil && err != nil { + t.Fatalf("Unexpected error occurred in test \"%s\": %s", tc.Name, err) + } + + assert.Equal(t, tc.RaiseErr, err) helper.AssertIssues(t, tc.Expected, runner.Issues) }