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.

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

Add aws_iam_policy_attachment_has_alternatives rule #384

Conversation

metafeather
Copy link

@metafeather metafeather commented Oct 10, 2022

Add aws_iam_policy_attachment_has_alternatives rule.

This rule warns when there are recommended alternatives to the resource, and is disabled by default

This recommendation is documented by HashiCorp in the AWS Provider documentation due to common misuse: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy_attachment

In this particular case the blast radius of misuse can be severe across an entire organisation account and cause production outages due to the unexpected removal of permissions.

After reviewing past issues (#35) and PR's (tflint:#769) I think that adding individual rules informed by HashiCorp recommendations is the most practical approach for community maintenance and this PR could be used as an approachable template for further rules without deep knowledge of the project (I used #355)

Thankyou for your consideration.

}

var name string
err := runner.EvaluateExpr(attribute.Expr, &name, nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the name is not used, it seems better to simply EmitIssue if the resource exists.

},
},
},
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a fixed case with no warnings.


// Name returns the rule name
func (r *AwsIAMPolicyAttachmentHasAlternativesRule) Name() string {
return "aws_iam_policy_attachment_has_alternatives"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a name like aws_iam_policy_attachment_deprecated?

Copy link
Member

@wata727 wata727 Oct 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or aws_iam_policy_attachment_exclusive_attachment may be better.

I prefer names that are descriptive of what issue we are warning about. Additionally, the prefix should preferably match the resource name. What do you think?

@bendrucker
Copy link
Member

Are you interested in finishing this PR?

@wata727
Copy link
Member

wata727 commented Feb 2, 2023

Closing this PR due to no recent activity. Feel free to reopen if you are interested in continuing this work. Thanks!

@wata727 wata727 closed this Feb 2, 2023
@kayman-mk
Copy link

kayman-mk commented Dec 11, 2024

@bendrucker Was this implemented in the meantime?

EDIT: I didn't find anything and opened #786

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants