Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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.

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

feat: Generic rule to prevent certain dangerous resources like aws_iam_policy_attachment #35

Closed
nitrocode opened this issue Apr 3, 2020 · 14 comments · May be fixed by #786
Closed

feat: Generic rule to prevent certain dangerous resources like aws_iam_policy_attachment #35

nitrocode opened this issue Apr 3, 2020 · 14 comments · May be fixed by #786
Assignees
Labels
enhancement New feature or request

Comments

@nitrocode
Copy link

There is a large warning for the resource aws_iam_policy_attachment to use aws_iam_role_policy_attachment instead.

I'd like to prevent the first resource from getting into our infrastructure with an appropriate tflint rule. We could make it specific to this resource but I'd prefer a more generic one so if we find a future resource, we can simply add it to a list within the rule.

rule "terraform_denylisted_resources" {
  enabled = true

  resources = [
    "aws_iam_policy_attachment",
    "google_organization_iam_binding",
  ]

  # in case some upstream module refuses to remove xyz resource 
  # and you don't want to manage a fork
  allow_in_modules = true
}

Edit: @mveitas put in PR terraform-linters/tflint#769

@mveitas
Copy link

mveitas commented May 21, 2020

@nitrocode Are you thinking along the lines of a rule named terraform_blacklisted_resources?

@mveitas mveitas self-assigned this May 21, 2020
@nitrocode
Copy link
Author

Yes exactly

@mveitas
Copy link

mveitas commented May 21, 2020

I'll try and get this knocked out. I think this is a great thing to have and am going to make use of it as soon as it is released!

@bendrucker
Copy link
Member

Would be nice for this to take a map of suggested resources and the preferred alternative as opposed to just returning that something was blacklisted. I'm not sure what actual use cases there'd be for entirely blacklisting a resource with no suggested alternative.

@nitrocode
Copy link
Author

nitrocode commented May 21, 2020

Would be nice for this to take a map of suggested resources and the preferred alternative as opposed to just returning that something was blacklisted.

@bendrucker yes or even a custom message could be printed if the code was flagged due to the terraform_blacklisted_resources rule (terraform-linters/tflint#768).

I'm not sure what actual use cases there'd be for entirely blacklisting a resource with no suggested alternative.

@bendrucker, the original post mentions a resource that has been dangerous which was the incentive for this ticket. nvm just caught the tail end "with no suggested alternative" agreed.

@bendrucker
Copy link
Member

bendrucker commented May 21, 2020

Ha, was typing as I saw your edit. In theory I could imagine someone wanting to blacklist a resource with no alternative because it has bugs, but that seems like a pretty narrow/tricky use case.

Suggestions to use b instead of a could cover a lot of providers—the Google provider has resources with similar caveats, e.g.:

https://www.terraform.io/docs/providers/google/r/google_organization_iam_binding.html

@mveitas
Copy link

mveitas commented May 22, 2020

It would be nice to have this within the provider so tools could leverage this information. It sort of already exists as the documentation for a provide has the information.

@mveitas
Copy link

mveitas commented May 22, 2020

@nitrocode I have an initial version of the rule: terraform-linters/tflint#769

@wata727
Copy link
Member

wata727 commented May 22, 2020

In my opinion, it is better to obviously resources that should be avoided (like aws_iam_policy_attachment) are provided as defaults in TFLint, rather than being investigated and configure by users themselves.

On the other hand, I agree that such a rule is effective when you want to avoid some resources for some special reason, but if possible, I want to make it a tool that provides such best practices.

@mveitas
Copy link

mveitas commented May 22, 2020

In my opinion, it is better to obviously resources that should be avoided (like aws_iam_policy_attachment) are provided as defaults in TFLint, rather than being investigated and configure by users themselves.

That starts to add an opinion with regards to best practices and each organization is going to have their own view on what's best for them. IMO TFLint is best served to provide a framework for people to enforce rules. The overhead associated with understanding all of the resources associated with a provider is a large amount of work.

@bendrucker
Copy link
Member

+1 from me on usable defaults, e.g. suggesting alternatives to aws_iam_policy_attachment, rather than requiring users to supply that configuration. It's fine to also have a generic configurable rule so that end users can cover less common providers. But the overall value of good defaults will always be a lot higher than an infinitely configurable option that only a tiny fraction of users do the work to adopt.

@mveitas
Copy link

mveitas commented May 22, 2020

👍

Since these things are very much provide specific, can we say that the terraform_blacklisted_resources is the generic configurable rule and rules to support provider specific defaults should be placed under the rules/awsrules?

@bendrucker
Copy link
Member

Yes. I'd avoid making the tests or examples too AWS centric to help avoid confusion with an eventual AWS-specific rule.

@nitrocode nitrocode changed the title Generic rule to prevent certain dangerous resources like aws_iam_policy_attachment feat: Generic rule to prevent certain dangerous resources like aws_iam_policy_attachment Jun 17, 2020
@wata727 wata727 transferred this issue from terraform-linters/tflint Jan 4, 2021
@wata727 wata727 added the enhancement New feature or request label Jan 4, 2021
@m00lecule
Copy link

@mveitas are there still chances, that this rule will be implemented?

@terraform-linters terraform-linters locked and limited conversation to collaborators Dec 7, 2022
@bendrucker bendrucker converted this issue into discussion #422 Dec 7, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

5 participants