Rule aws_s3_bucket_name does work on module parameters

stksz commented 2 years ago

I have the following in place:

tflint-config:

plugin "aws" {
  enabled = true
  version = "0.8.0"
  source  = "github.com/terraform-linters/tflint-ruleset-aws"
}

config {
  module = true
}

rule "aws_s3_bucket_name" {
  enabled = true
  regex = "^[a-z\\-]+$"
  prefix = "my-prefix-"
}

Terrafrom "main-module":

module "my_bucket" {
  source = "../modules/s3"
  bucket_name = "my-new-bucket......" # <--- This is on purpose wrong!!!
}

The used Terraform-module:

resource "aws_s3_bucket" "bucket" {
  bucket = var.bucket_name # <-- This variable is defined in a "variables.tf" from within the module-directory
  acl    = "private"

  server_side_encryption_configuration {
    rule {
      apply_server_side_encryption_by_default {
        sse_algorithm = "AES256"
      }
    }
  }
}

When I now run "tflint" within the "main-modules"-directory I don't get any error even though the rule for "aws_s3_bucket_name" should produce an error as the value for "bucket_name" does not match die regex and the prefix of the rule. Changing "bucket" from within the module from "bucket = var.bucket_name" to something like "bucket = "foobar"" results in getting the expected error-message as the bucket-name does not adhere to the rule(s).

Version

$ tflint -v
TFLint version 0.33.1
+ ruleset.aws (0.8.0)
$ terraform -v
Terraform v1.0.10
on darwin_amd64
+ provider registry.terraform.io/hashicorp/aws v3.63.0

bendrucker commented 2 years ago

Hmm, the correct behavior would be the reverse. TFLint's module feature is meant to raise errors caused by the supplied variables in a call, not to check the static content of the module itself.

bendrucker commented 2 years ago

Confirmed this is not an issue with that rule. It evaluates the variable and the logic is correct. This is a bug with TFLint's propagation of issues from modules. Issues should only be propagated when their expression references a module variable:

https://github.com/terraform-linters/tflint/blob/4db35875d3981252938d78c6bfe5672a2a35ae52/tflint/runner.go#L285-L302

This logic is not working correctly for plugins. Will need to spend more time debugging to find a fix.

wata727 commented 2 years ago

This is a bug in the rule. In order to propagate the issue and expression associations to the host, the plugin must use the appropriate API.

The aws_s3_bucket_name rule uses the EmitIssue API and you can't specify the association with the expression: https://github.com/terraform-linters/tflint-ruleset-aws/blob/v0.8.0/rules/aws_s3_bucket_name.go#L73-L77

In this case, the rule should use EmitIssueOnExpr API. https://github.com/terraform-linters/tflint-ruleset-aws/blob/v0.8.0/rules/aws_api_gateway_model_invalid_name.go#L55-L59

terraform-linters / tflint-ruleset-aws

Rule aws_s3_bucket_name does work on module parameters #194

Version