terraform-linters / tflint

A Pluggable Terraform Linter
Mozilla Public License 2.0
4.86k stars 354 forks source link

Duplicate issues emitted for tags attributes inside of aws_instance modules #1063

Closed stahs closed 3 years ago

stahs commented 3 years ago

Hi! I have run into a problem I'm hoping you can help me solve. My situation is a little complex, so bear with me please :)

I have built a tflint plugin, which in an extension of the missing-tags rule. My custom "required-tags" rule allows me to specify allowed sets of tag values for certain tag keys, which fulfills a business need of ensuring that certain tags are populated and are correctly set.

For example, tag Zone must be set and the value must be in ["zone1", "zone2"].

My Terraform code uses a custom module, which provides aws_server resources.

When I run tflint in a directory containing an instance of this aws_server module with a single non-compliant tag, tflint emits 4 issues instead of 1.

Example instance of the terraform module as it appears in my terraform reposiotry

module "chef-useast1a-01" {
  security  = var.security["control-allow-all"]
  source    = "git@github.com:privaterepository"
  subnet    = var.subnet["control-1a-private"]
  type      = "t3a.medium"
  base_role = "role[base]"

  aws_amis = {
    ubuntu1804 = "ami-ax244312434fdfdsdfg"
  }

  tags = {
    Name        = "chef-useast1a-01"
    Zone        = "zone1"
    Environment = "bad_env_name"
    Service = "test"
  }
}

tflint output shows 4 issues found instead of expected 1 issue

tflint -c ../../.tflint-required-tags.hcl --loglevel trace
19:39:29 config.go:96: [INFO] Load config: ../../.tflint-required-tags.hcl
19:39:29 config.go:311: [DEBUG] Config loaded
19:39:29 config.go:312: [DEBUG]   Module: true
19:39:29 config.go:313: [DEBUG]   Force: false
19:39:29 config.go:314: [DEBUG]   IgnoreModules: map[string]bool{}
19:39:29 config.go:315: [DEBUG]   Varfiles: []string{}
19:39:29 config.go:316: [DEBUG]   Variables: []string{}
19:39:29 config.go:317: [DEBUG]   DisabledByDefault: false
19:39:29 config.go:318: [DEBUG]   Rules: map[string]*tflint.RuleConfig{"aws_resource_required_tags":(*tflint.RuleConfig)(0xc000303620), "terraform_deprecated_interpolation":(*tflint.RuleConfig)(0xc000302f60), "terraform_module_pinned_source":(*tflint.RuleConfig)(0xc0003032c0), "terraform_workspace_remote":(*tflint.RuleConfig)(0xc000303410)}
19:39:29 config.go:319: [DEBUG]   Plugins: map[string]*tflint.PluginConfig{"aws":(*tflint.PluginConfig)(0xc0003038c0), "required-tags":(*tflint.PluginConfig)(0xc000303b60)}
19:39:29 option.go:48: [DEBUG] CLI Options
19:39:29 option.go:49: [DEBUG]   Module: false
19:39:29 option.go:50: [DEBUG]   Force: false
19:39:29 option.go:51: [DEBUG]   IgnoreModules: map[string]bool{}
19:39:29 option.go:52: [DEBUG]   EnableRules: []string(nil)
19:39:29 option.go:53: [DEBUG]   DisableRules: []string(nil)
19:39:29 option.go:54: [DEBUG]   Only: []string(nil)
19:39:29 option.go:55: [DEBUG]   Varfiles: []string{}
19:39:29 option.go:56: [DEBUG]   Variables: []string{}
19:39:29 loader.go:57: [INFO] Initialize new loader
19:39:29 loader.go:68: [INFO] Module manifest file found. Initializing...
19:39:29 loader.go:291: [DEBUG] Parsing the module manifest file: {"Modules":[{"Key":"","Source":"","Dir":"."},{"Key":"chef-useast1a-01","Source":"git@github.com:privaterepo","Dir":".terraform/modules/chef-useast1a-01"}]}
19:39:29 loader.go:82: [INFO] Load configurations under .
19:39:29 loader.go:97: [INFO] Module inspection is enabled. Building a root module with children...
19:39:29 loader.go:273: [DEBUG] Trying to load the module: key=module.chef-useast1a-01, version=, dir=.terraform/modules/chef-useast1a-01
19:39:29 loader.go:170: [INFO] Load values files
19:39:29 runner.go:50: [INFO] Initialize new runner for root
19:39:29 runner.go:50: [INFO] Initialize new runner for module.chef-useast1a-01
19:39:29 discovery.go:54: [INFO] Plugin `aws` is not installed, but bundled plugins are available.
19:39:29 discovery.go:86: [INFO] Plugin `aws` found, but the plugin is disabled
19:39:29 discovery.go:68: [INFO] Plugin `required-tags` found
19:39:29 provider.go:62: [INFO] Prepare rules
19:39:29 provider.go:76: [DEBUG] `terraform_deprecated_interpolation` is disabled
19:39:29 provider.go:76: [DEBUG] `terraform_module_pinned_source` is disabled
19:39:29 provider.go:76: [DEBUG] `terraform_workspace_remote` is disabled
19:39:29 provider.go:90: [INFO]   0 rules enabled

4 issue(s) found:

Error: The resource is missing required Environment tag, must be one of: [production stage test] (aws_resource_required_tags)

  on ec2.tf line 12:
  12:   tags = {
  13:     Name        = "chef-useast1a-01"
  14:     Zone        = "zone1"
  15:     Environment = "bad_env_name"
  16:   }

Callers:
   ec2.tf:12,10-16,4
   .terraform/modules/chef-useast1a-01/instance.tf:13,10-18,4

Error: The resource is missing required Environment tag, must be one of: [production stage test] (aws_resource_required_tags)

  on ec2.tf line 12:
  12:   tags = {
  13:     Name        = "chef-useast1a-01"
  14:     Zone        = "zone1"
  15:     Environment = "bad_env_name"
  16:   }

Callers:
   ec2.tf:12,10-16,4
   .terraform/modules/chef-useast1a-01/instance.tf:13,10-18,4

Error: The resource is missing required Environment tag, must be one of: [production stage test] (aws_resource_required_tags)

  on ec2.tf line 12:
  12:   tags = {
  13:     Name        = "chef-useast1a-01"
  14:     Zone        = "Infrastructure"
  15:     Environment = "bad_env_name"
  16:   }

Callers:
   ec2.tf:12,10-16,4
   .terraform/modules/chef-useast1a-01/instance.tf:13,10-18,4

Error: The resource is missing required Environment tag, must be one of: [production stage test] (aws_resource_required_tags)

  on ec2.tf line 12:
  12:   tags = {
  13:     Name        = "chef-useast1a-01"
  14:     Zone        = "Infrastructure"
  15:     Environment = "bad_env_name"
  16:   }

Callers:
   ec2.tf:12,10-16,4
   .terraform/modules/chef-useast1a-01/instance.tf:13,10-18,4

The number of emitted issues is equivalent to the number of variables evaluated in the tags attribute inside of the terraform module. I am getting 4 issues because my tags attribute contains 4 vars (Name, Zone, Environment, Service)

My environment

Version

tflint -v
TFLint version 0.24.1

required tags plugin main.go (built using SDK v0.8.1)

package rules

import (
    "encoding/gob"
    "fmt"

    hcl "github.com/hashicorp/hcl/v2"
    "github.com/stahs/tflint-ruleset-required-tags/project"
    "github.com/terraform-linters/tflint-plugin-sdk/terraform/configs"
    "github.com/terraform-linters/tflint-plugin-sdk/tflint"
)

const (
    envMessage  = "The resource is missing required Environment tag, must be one of:"
    roleMessage = "The resource is missing required Role tag, must be one of:"
    zoneMessage = "The resource is missing required Zone tag, must be one of:"
)

type awsResourceRequiredTagsRuleConfig struct {
    ZoneTags  []string `hcl:"zones"`
    EnvTags   []string `hcl:"environments"`
    RoleTags  []string `hcl:"roles"`
    Resources []string `hcl:"resources,optional"`
    Enabled   bool     `hcl:"enabled,optional"`
}

// AwsResourceRequiredTagsRule checks whether ...
type AwsResourceRequiredTagsRule struct{}

// NewAwsResourceRequiredTagsRule returns a new rule
func NewAwsResourceRequiredTagsRule() *AwsResourceRequiredTagsRule {
    return &AwsResourceRequiredTagsRule{}
}

// Name returns the rule name
func (r *AwsResourceRequiredTagsRule) Name() string {
    return "aws_resource_required_tags"
}

// Enabled returns whether the rule is enabled by default
func (r *AwsResourceRequiredTagsRule) Enabled() bool {
    return true
}

// Severity returns the rule severity
func (r *AwsResourceRequiredTagsRule) Severity() string {
    return tflint.ERROR
}

// Link returns the rule reference link
func (r *AwsResourceRequiredTagsRule) Link() string {
    return project.ReferenceLink(r.Name())
}

// Check checks resources for missing tags
func (r *AwsResourceRequiredTagsRule) Check(runner tflint.Runner) error {
    config := awsResourceRequiredTagsRuleConfig{}
    gob.Register(map[string]string{})
    if err := runner.DecodeRuleConfig(r.Name(), &config); err != nil {
        return err
    }

    for _, resourceType := range config.Resources {
        err := runner.WalkResources(resourceType, func(resource *configs.Resource) error {
            body, _, diags := resource.Config.PartialContent(&hcl.BodySchema{
                Attributes: []hcl.AttributeSchema{
                    {
                        Name: "tags",
                    },
                },
            })
            if diags.HasErrors() {
                return diags
            }
            if tagAttribute, ok := body.Attributes["tags"]; ok {
                resourceTags := make(map[string]string)
                err := runner.EvaluateExpr(tagAttribute.Expr, &resourceTags, nil)

                // Zone
                // if `Zone` key is not found among tags, or when it is found, but its value is not correct.
                if zoneName, ok := resourceTags["Zone"]; !ok || !foundIn(zoneName, config.ZoneTags) {
                    runner.EmitIssueOnExpr(r, fmt.Sprintf("%s %s", zoneMessage, config.ZoneTags), tagAttribute.Expr)
                }

                // Environment
                if environmentName, ok := resourceTags["Environment"]; !ok || !foundIn(environmentName, config.EnvTags) {
                    runner.EmitIssueOnExpr(r, fmt.Sprintf("%s %s", envMessage, config.EnvTags), tagAttribute.Expr)
                }

                // Role
                if resourceType == "rename to aws_instance" {
                    if role, ok := resourceTags["Role"]; !ok || !foundIn(role, config.RoleTags) {
                        runner.EmitIssueOnExpr(r, fmt.Sprintf("%s %s", roleMessage, config.RoleTags), tagAttribute.Expr)
                    }
                }

                if err != nil {
                    return err
                }
                return nil
            } else {
                // return runner.EmitIssueOnExpr(r, "Missing attribute: tags", attribute.Expr)
                return runner.EmitIssue(r, "Missing attribute: tags", body.MissingItemRange)
            }
        })

        if err != nil {
            return err
        }
        return nil
    }
    return nil
}

func foundIn(a string, list []string) bool {
    for _, b := range list {
        if b == a {
            return true
        }
    }
    return false
}

.tflint.hcl

config {
  module = true
  force = false
  disabled_by_default = false
}

plugin "aws" {
  enabled = false
}

plugin "required-tags" {
  enabled = true
}

rule "terraform_deprecated_interpolation" {
  enabled = false
}

rule "terraform_module_pinned_source" {
  enabled = false
}

rule "terraform_workspace_remote" {
  enabled = false
}

rule "aws_resource_required_tags" {
  enabled = true
  environments = [
    "production",
    "stage",
    "test"
  ]
  zones = [
    "zone1 ",
    "zone2"
  ]
  resources = [
    "aws_instance",
    "aws_s3_bucket",
    "aws_alb",
    "aws_lb",
    "aws_db_instance",
    "aws_redshift_cluster",
    "aws_kinesis_firehose_delivery_stream",
    "aws_dynamodb_table",
    "aws_lambda_function",
  ]
  roles = [
    "database",
    "nondatabase",
    "other",
  ]
}

terraform module

variable "tags" {
  type = map(string)

  default = {}
}

..

resource "aws_instance" "ss" {
  ami                         = var.aws_amis[var.default_os]
  associate_public_ip_address = var.public
  disable_api_termination     = var.disable_api_termination
  iam_instance_profile        = var.iam_instance_profile
  instance_type               = var.type
  key_name                    = var.ssh_key_name
  private_ip                  = var.private_ip
  source_dest_check           = var.source_dest_check
  subnet_id                   = var.subnet
  vpc_security_group_ids      = split(",", var.security)

  tags = {
    Name        = var.tags["Name"]
    Zone        = var.tags["Zone"]
    Environment = var.tags["Environment"]
    Service = var.tags["Service"]
  }
..

I have observed that if this module is edited to populate only 3 tags like so

resource "aws_instance" "ss" {
..

  tags = {
    Name        = var.tags["Name"]
    Zone        = var.tags["Zone"]
    Environment = var.tags["Environment"]
  }
..

then only 3 issues are emitted (instead of expected 1)

This appears to be related to the fact that EmitIssue emits one issue per output produced by listModuleVars.

Perhaps this is not a bug, but I am simply using the SDK incorrectly? Any help or tips will be very much appreciated! Thank you!!

wata727 commented 3 years ago

Good question.

As you say, this seems to be a problem with the listModuleVars. In this case, var.tags["Name"], var.tags["Zone"], var.tags["Environment"], and var.tags["Service"] are recognized as different variables, so EmitIssue emits issues for each variable. However, it is right to think of this as a single variable.

listVarRefs simply treat all parsed references as variables, but there may be a smarter way to determine it.

stahs commented 3 years ago

Hi and thank you for responding.

Writing all this up has helped me to understand I was overcomplicating things inside of the terraform module. I have verified that when I change my module to pass over the entire tags variable like so:

resource "aws_instance" "ss" {
  ami                         = var.aws_amis[var.default_os]
  associate_public_ip_address = var.public
  disable_api_termination     = var.disable_api_termination
  iam_instance_profile        = var.iam_instance_profile
  instance_type               = var.type
  key_name                    = var.ssh_key_name
  private_ip                  = var.private_ip
  source_dest_check           = var.source_dest_check
  subnet_id                   = var.subnet
  vpc_security_group_ids      = split(",", var.security)

  tags = var.tags

then tflint output is what I expect to see. I would like to be able to use the individual variables inside of the tags attribute, combined with try function, but this works OK for now.

Thanks for looking into this!