Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

aws_resource_missing_tags panics on unknown tags #528

Closed
fatbasstard opened this issue Aug 3, 2023 · 3 comments · Fixed by #530
Closed

aws_resource_missing_tags panics on unknown tags #528

fatbasstard opened this issue Aug 3, 2023 · 3 comments · Fixed by #530
Labels
bug Something isn't working

Comments

@fatbasstard
Copy link

fatbasstard commented Aug 3, 2023 •
edited

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:

  • Support just testing if tags are configured without a list of keys (by removing the tags from the rule configuration).
  • Handle the usage of variables for tags
  • Sort of both of the above?
@bendrucker bendrucker changed the title Failing aws_resource_missing_tags when using variable for tags: Failed to check ruleset; error reading from server: EOF aws_resource_missing_tags panics on unknown tags Aug 3, 2023
@bendrucker
Copy link
Member

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.

@bendrucker bendrucker added the bug Something isn't working label Aug 3, 2023
@wata727
Copy link
Member

wata727 commented Aug 7, 2023 •
edited

Hmm, CanIterateElements seems to cause panic in unknown and NULL cases.

tflint-ruleset-aws/rules/aws_resource_missing_tags.go

Line 382 in 47711ed

if !value.CanIterateElements() {

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

@wata727 wata727 mentioned this issue Aug 7, 2023
Open
@teddy-wahle
Copy link

teddy-wahle commented Aug 10, 2023 •
edited

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

@wata727 wata727 mentioned this issue Aug 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

resource_missing_tags: Fix panic for unknown and null values terraform-linters/tflint-ruleset-aws
4 participants
@bendrucker @wata727 @fatbasstard @teddy-wahle