Failed to initialize a runner when the count is string

[wata727]

Introduction

Terraform implicitly converts the count attribute to a number, so passing it as a string works fine:

resource "aws_instance" "main" {
  count = "1"
}

But this config doesn't work in TFLint v0.41.

Expected Behavior

It works fine without errors.

Actual behavior

Failed to initialize a runner; number value is required.

% tflint
Failed to initialize a runner; number value is required

Step to Reproduce

$ cat << EOS > .tflint.hcl
plugin "aws" {
    enabled = true
    version = "0.17.1"
    source  = "github.com/terraform-linters/tflint-ruleset-aws"
}
EOS
$ tflint --init
$ cat <<EOS > main.tf
resource "aws_instance" "main" {
  count = "1"
}
EOS
$ tflint
Failed to initialize a runner; number value is required

Additional Context

$ tflint -v
TFLint version 0.41.0
+ ruleset.aws (0.17.1)
+ ruleset.terraform (0.1.1-bundled)
$ terraform -v
Terraform v1.3.0

[wata727]

This seems to be a bug when evaluating the count attribute with the wrong type. cty.DynamicPseudoType is used instead of cty.Number in v0.41.
https://github.com/terraform-linters/tflint/blob/v0.41.0/tflint/runner_eval.go#L51
https://github.com/terraform-linters/tflint/blob/v0.40.1/tflint/runner_eval.go#L168

Fortunately, this bug seems to be fixed as a side effect of #1537.

[stevehipwell]

This looks like desirable behaviour to me, although the error message might need improving. Just because Terraform allows you to do something doesn't mean that it's necessarily a good idea.

[gtirloni]

TFLint can complain something violates a certain rule, but it shouldn't crash. String vs number is such a low-level thing that maybe TFLint shouldn't also come up with its own requirements in this area. Yes, people should use a number but TF allows a string so be it... If TFLint considers that a critical failure, people will keep getting puzzled because the behavior is different from TF itself.

[bendrucker]

TFLint's intent is to be able to parse anything that the supported Terraform version(s) can parse.

You can certainly question whether support for this type conversion should have been cut in a past version of Terraform, either 0.12 or 1.0. The reason it's there is for backward compatibility. In 0.11, all attributes were provided as strings:

count = "${var.resource_count}"

We could certainly consider a terraform_count rule in https://github.com/terraform-linters/tflint-ruleset-terraform that would make more specific assertions on the contents of count than the parser. This may be a bit tricky as count is treated as a meta-parameter rather than a resource attribute.

We also can't always know the actual type, as in this example:

count = data.terraform_remote_state.foo.outputs.resource_count

Even if the output resource_count is correctly typed in the remote workspace and won't require type coercion, TFLint must treat the expression as unknown since the data is not available until plan time.

[bendrucker]

Also, minor point, but this wouldn't be considered a crash, just an unexpected error. TFLint is exiting with an error from the parser while initializing a runner. A panic would be considered a crash, meaning that the code encountered a condition it couldn't handle and had to be terminated immediately.