terraform-linters / tflint-ruleset-aws

TFLint ruleset for terraform-provider-aws
Mozilla Public License 2.0
337 stars 72 forks source link

`aws_resource_missing_tags` panics on unknown tags #528

Closed fatbasstard closed 1 year ago

fatbasstard commented 1 year ago

Hi,

when enabling the aws_resource_missing_tags rule I get a Failed to check ruleset; error reading from server: EOF error. This happens apparantly because we use variables for tags. Instead of "not checking" the rule breaks. Here's a simple test setup:

Example

variable "tags" {
  type        = map(string)
  description = "A mapping of tags to assign to resource"
}

resource "aws_ssm_parameter" "test_var" {
  name  = "/test"
  type  = "String"
  value = "test"
  tags  = var.tags
}

resource "aws_ssm_parameter" "test_no_tags" {
  name  = "/test"
  type  = "String"
  value = "test"
}

resource "aws_ssm_parameter" "tags" {
  name  = "/test"
  type  = "String"
  value = "test"
  tags = {
    foo = "Bar"
    bar = "Baz"
  }
}

.tflint.hcl:

plugin "terraform" {
  enabled = true
  preset  = "all"
}

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

rule "aws_resource_missing_tags" {
  enabled = true
  tags = ["foo", "bar"]
}

# Test setup
rule "terraform_required_providers" {
  enabled = false
}
rule "terraform_required_version" {
  enabled = false
}

rule "terraform_standard_module_structure" {
  enabled = false
}

Debug output:

15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: 15:28:51 [DEBUG] rules/aws_resource_missing_tags.go:180: Walk `%s` attribute: EXTRA_VALUE_AT_END=aws_ssm_parameter.test_var.tags
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: panic: can't use ElementIterator on unknown value
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: goroutine 53 [running]:
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: github.com/zclconf/go-cty/cty.Value.ElementIterator({{{0x1020eea70?, 0x14000a69230?}}, {0x101cd2ac0?, 0x102f7fe10?}})
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.13.2/cty/value_ops.go:1193 +0xc8
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: github.com/zclconf/go-cty/cty.Value.ForEachElement({{{0x1020eea70?, 0x14000a69230?}}, {0x101cd2ac0?, 0x102f7fe10?}}, 0x14000f0ec10)
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.13.2/cty/value_ops.go:1221 +0x58
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: github.com/terraform-linters/tflint-ruleset-aws/rules.getKeysForValue({{{0x1020eea70?, 0x14000a69230?}}, {0x101cd2ac0?, 0x102f7fe10?}})
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/work/tflint-ruleset-aws/tflint-ruleset-aws/rules/aws_resource_missing_tags.go:386 +0x78
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: github.com/terraform-linters/tflint-ruleset-aws/rules.(*AwsResourceMissingTagsRule).Check.func1({{{0x1020eea70?, 0x14000a69230?}}, {0x101cd2ac0?, 0x102f7fe10?}})
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/work/tflint-ruleset-aws/tflint-ruleset-aws/rules/aws_resource_missing_tags.go:186 +0x7c
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: reflect.Value.call({0x101c08940?, 0x14001062ff0?, 0xd5?}, {0x101659967, 0x4}, {0x14000f0f5e0, 0x1, 0x11?})
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /opt/hostedtoolcache/go/1.20.6/x64/src/reflect/value.go:586 +0x87c
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: reflect.Value.Call({0x101c08940?, 0x14001062ff0?, 0x14001057620?}, {0x14000f0f5e0?, 0x14001041800?, 0x0?})
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /opt/hostedtoolcache/go/1.20.6/x64/src/reflect/value.go:370 +0x90
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: github.com/terraform-linters/tflint-plugin-sdk/plugin/internal/plugin2host.(*GRPCClient).EvaluateExpr(0x14000df5638?, {0x1020ee370, 0x14001057620}, {0x101c08940, 0x14001062ff0}, 0x5?)
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/go/pkg/mod/github.com/terraform-linters/tflint-plugin-sdk@v0.18.0/plugin/internal/plugin2host/client.go:323 +0x334
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: github.com/terraform-linters/tflint-ruleset-aws/rules.(*AwsResourceMissingTagsRule).Check(0x102fb84e0, {0x102111060, 0x140005c6400})
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/work/tflint-ruleset-aws/tflint-ruleset-aws/rules/aws_resource_missing_tags.go:185 +0x78c
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: github.com/terraform-linters/tflint-plugin-sdk/plugin/internal/host2plugin.(*GRPCServer).Check(0x14000bd5ad0, {0x1020ee060, 0x14000698b40}, 0x101b84240?)
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/go/pkg/mod/github.com/terraform-linters/tflint-plugin-sdk@v0.18.0/plugin/internal/host2plugin/server.go:145 +0x39c
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: github.com/terraform-linters/tflint-plugin-sdk/plugin/internal/proto._RuleSet_Check_Handler.func1({0x1020ee060, 0x14000698b40}, {0x101e4c540?, 0x14000698b70})
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/go/pkg/mod/github.com/terraform-linters/tflint-plugin-sdk@v0.18.0/plugin/internal/proto/tflint_grpc.pb.go:354 +0x74
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: github.com/terraform-linters/tflint-plugin-sdk/plugin/internal/interceptor.RequestLogging.func1({0x1020ee060, 0x14000698b40}, {0x101e4c540?, 0x14000698b70?}, 0x140006a8200, 0x14000696420)
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/go/pkg/mod/github.com/terraform-linters/tflint-plugin-sdk@v0.18.0/plugin/internal/interceptor/logging.go:16 +0x22c
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: github.com/terraform-linters/tflint-plugin-sdk/plugin/internal/proto._RuleSet_Check_Handler({0x101fe7e20?, 0x14000bd5ad0}, {0x1020ee060, 0x14000698b40}, 0x140006b4230, 0x1400000c108)
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/go/pkg/mod/github.com/terraform-linters/tflint-plugin-sdk@v0.18.0/plugin/internal/proto/tflint_grpc.pb.go:356 +0x138
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: google.golang.org/grpc.(*Server).processUnaryRPC(0x140002385a0, {0x10210e7c0, 0x14000003d40}, 0x140000b65a0, 0x14000bd5b60, 0x102f6d4a0, 0x0)
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/go/pkg/mod/google.golang.org/grpc@v1.57.0/server.go:1360 +0xcb4
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: google.golang.org/grpc.(*Server).handleStream(0x140002385a0, {0x10210e7c0, 0x14000003d40}, 0x140000b65a0, 0x0)
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/go/pkg/mod/google.golang.org/grpc@v1.57.0/server.go:1737 +0x82c
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: google.golang.org/grpc.(*Server).serveStreams.func1.1()
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/go/pkg/mod/google.golang.org/grpc@v1.57.0/server.go:982 +0x84
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws: created by google.golang.org/grpc.(*Server).serveStreams.func1
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:1046: tflint-ruleset-aws:  /home/runner/go/pkg/mod/google.golang.org/grpc@v1.57.0/server.go:980 +0x16c
15:28:51 [DEBUG] go-plugin@v1.4.10/grpc_stdio.go:142: stdio: received EOF, stopping recv loop: err="rpc error: code = Unavailable desc = error reading from server: EOF"
15:28:51 [ERROR] go-plugin@v1.4.10/client.go:662: plugin process exited: path=/Users/xxxx/.tflint.d/plugins/github.com/terraform-linters/tflint-ruleset-aws/0.25.0/tflint-ruleset-aws pid=33930 error="exit status 2"
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:472: plugin exited
15:28:51 [DEBUG] go-plugin@v1.4.10/grpc_stdio.go:142: stdio: received EOF, stopping recv loop: err="rpc error: code = Unavailable desc = error reading from server: EOF"
15:28:51 [INFO]  go-plugin@v1.4.10/client.go:665: plugin process exited: path=/opt/homebrew/bin/tflint pid=33929
15:28:51 [DEBUG] go-plugin@v1.4.10/client.go:472: plugin exited
Failed to check ruleset; error reading from server: EOF

When removing/commenting thattest_var I get a clean and normal message:

terraform/main.tf:1:1: Warning - variable "tags" is declared but not used (terraform_unused_declarations)
terraform/main.tf:13:1: Notice - The resource is missing the following tags: "bar", "foo". (aws_resource_missing_tags)

I'd expect/hope that the rule can at least check IF the tags are configured, even without checking the specific tags. I've tried enforcing this by removing the tags to validate (assuming it would just do a "missing tags" check):

rule "aws_resource_missing_tags" {
  enabled = true
}

But this throws an error as well: Failed to check ruleset; failed to check "aws_resource_missing_tags" rule: .tflint.hcl:12,34-34: Missing required argument; The argument "tags" is required, but no definition was found.

Changing to tags = [] also doesn't work, just doesn't check anything

Conclusion Multiple ways to fix/support this I guess:

bendrucker commented 1 year ago

I'd expect/hope that the rule can at least check IF the tags are configured, even without checking the specific tags.

This is not what the rule is intended to do and doesn't seem to be especially useful.

The panic is a legit bug though, assuming it's reproducible in the latest version, and should be easily fixable. The proper handling here is to ignore expressions that are unknown, which includes variables with no set value or default.

wata727 commented 1 year ago

Hmm, CanIterateElements seems to cause panic in unknown and NULL cases. https://github.com/terraform-linters/tflint-ruleset-aws/blob/47711edd982426cc670aed89db48baf95067478f/rules/aws_resource_missing_tags.go#L382 https://github.com/zclconf/go-cty/blob/v1.13.2/cty/value_ops.go#L1190-L1197

I'm surprised because this is different behavior from the documentation...

CanIterateElements returns true if the receiver can support the ElementIterator method (and by extension, ForEachElement) without panic.

https://pkg.go.dev/github.com/zclconf/go-cty@v1.13.2/cty#Value.CanIterateElements

teddy-wahle commented 1 year ago

Having this same error in every version after 24.1. 24.1 is fine but 24.2+ has the bug.