[WIP] - Adding new terraform_blacklisted_resources rule

[mveitas]

This rule adds the ability to prevent a resource of a certain type from being used. Each type specified has a custom message that will be displayed to the user that can be used to provide guidance on an alternative.

Fixes https://github.com/terraform-linters/tflint/issues/700

[mveitas]

Wanted to get some eyes on this approach before I go further.

I was going to take a look at the additional attribute allow_in_modules that was specified in #700 in the example to ensure that this won't break when "deep" module check is enabled

[bendrucker]

This seems like something we should address in AssertIssues rather than requiring a rule to sort like this

[mveitas]

I agree, I just could not see a way to make this happen easily. The right thing to do is to resolve the issue in AssertIssues first instead of polluting the rule.

Terraform stores the resources as a map, so there is no guaranteed order in the case of evaluation of resources.

type Issue struct {
	Rule    Rule
	Message string
	Range   hcl.Range
	Callers []hcl.Range
}

Thoughts?

[bendrucker]

What about a map[string][]string, where the key is the forbidden resource and the values are the suggested alternatives? I think it's best that we avoid putting literal messages in rule configuration, unless it's a meta-parameter that all rules can provide (#768).

[mveitas]

Thoughts on naming this terraform_alternative_resources?

[mveitas]

Going back to the idea of include a message as part of the rule, this is something that would be valuable to be able to provide some context for each type specified. The message would be something that is "optional"

What are you thoughts about a rule structure as such:

rule "terraform_alternative_resources" {
  enabled = true
  types = {
    myprovider_identity = {
      alternatives = ["myprovider_user"],
      message      = "This resource will be deprecated on Sept 1, 2020. See https://myprovider.com/docs/terraform_provider for more information"
    }
  }
}

[bendrucker]

Yeah, I think suggested or alternative or something similar sets the right tone.

And you're right re: the message, since it does need to be on a per-resource basis I think that makes sense.

What does types represent here? Was expecting something more like Terraform's structure, i.e. map[string]map[string]*Resource, a map of provider -> resources, where data. prefixes data sources.

[nitrocode]

Thanks @mveitas for the PR. I think terraform_alternative_resources is a much better name for this rule.

Are the only changes left the rename ?

What does types represent here? Was expecting something more like Terraform's structure, i.e. map[string]map[string]*Resource, a map of provider -> resources, where data. prefixes data sources.

Yes, I was imagining something more like this

rule "terraform_alternative_resources" {
  enabled = true
  aws_identity = {
    alternatives = ["aws_iam_policy_attachment"],
    message      = ["This resource is deprecated. Please use aws_iam_role_policy_attachment instead"]
  }
}

or maybe

rule "terraform_alternative_resources" {
  enabled = true
  resources = [
    # bad aws iam policy attachment
    {
      # string or array ?
      resources = ["aws.aws_iam_policy_attachment"]
      alternatives = [
        "aws.aws_iam_role_policy_attachment",
        "aws.aws_iam_user_policy_attachment",
        "aws.aws_iam_group_policy_attachment",
      ]
      message = "This resource is deprecated. Please use one of these instead: ${join(",", this.alternatives)}"
      # allow_in_modules = true
      allow_in_modules = ["https://github.com/claranet/terraform-aws-lambda"]
    }
    # additional resources that we don't want used for some reason
    # ...
  ]
}

Also what if modules use the flagged resource. Would it be possible to allow this resource in upstream modules ?

[bendrucker]

See #878 for discussion on linting child modules and a proposed behavior change. Otherwise, this rule would behave in unexpected ways with child modules, and only trigger errors when the disallowed resource included a var.* reference.

[nitrocode]

@bendrucker wait how come this was closed ? Is there a way to do this now using a different rule ?

[bendrucker]

This is not merge-able or complete. If someone else wants to take it up in a rule set plugin that's fine.

There's still an open issue too. I'll transfer the terraform rule set to its own repo/plugin soon and will make some decisions on whether to implement any related rule requests at that time.