Skip to content

Commit

Permalink
feat: add aws_iam_policy_attachment_exclusive_attachment rule (#786)
Browse files Browse the repository at this point in the history
* Add aws_iam_policy_attachment_has_alternatives rule

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

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

* fix:

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?

* rename in docs

* fix check comment

* fix docs

* fix review issues

* restore .gitignore

* fix test

* add updated warning message to docs

* fetch the resource only

---------

Co-authored-by: Liam Clancy (metafeather) <liam.clancy@superawesome.com>
Co-authored-by: Liam Clancy <metafeather@users.noreply.github.com>
  • Loading branch information
3 people authored Dec 30, 2024
1 parent 83ceb86 commit 1538e44
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 0 deletions.
1 change: 1 addition & 0 deletions docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ These rules enforce best practices and naming conventions:
|[aws_elasticache_replication_group_previous_type](aws_elasticache_replication_group_previous_type.md)|Disallow using previous node types||
|[aws_elasticache_replication_group_default_parameter_group](aws_elasticache_replication_group_default_parameter_group.md)|Disallow using default parameter group||
|[aws_instance_previous_type](aws_instance_previous_type.md)|Disallow using previous generation instance types||
|[aws_iam_policy_attachment_exclusive_attachment](aws_iam_policy_attachment_exclusive_attachment.md)|Consider alternative resources to `aws_iam_policy_attachment`||
|[aws_iam_policy_document_gov_friendly_arns](aws_iam_policy_document_gov_friendly_arns.md)|Ensure `iam_policy_document` data sources do not contain `arn:aws:` ARN's||
|[aws_iam_policy_gov_friendly_arns](aws_iam_policy_gov_friendly_arns.md)|Ensure `iam_policy` resources do not contain `arn:aws:` ARN's||
|[aws_iam_role_policy_gov_friendly_arns](aws_iam_role_policy_gov_friendly_arns.md)|Ensure `iam_role_policy` resources do not contain `arn:aws:` ARN's||
Expand Down
1 change: 1 addition & 0 deletions docs/rules/README.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ These rules enforce best practices and naming conventions:
|[aws_elasticache_replication_group_previous_type](aws_elasticache_replication_group_previous_type.md)|Disallow using previous node types|✔|
|[aws_elasticache_replication_group_default_parameter_group](aws_elasticache_replication_group_default_parameter_group.md)|Disallow using default parameter group|✔|
|[aws_instance_previous_type](aws_instance_previous_type.md)|Disallow using previous generation instance types|✔|
|[aws_iam_policy_attachment_exclusive_attachment](aws_iam_policy_attachment_exclusive_attachment.md)|Consider alternative resources to `aws_iam_policy_attachment`||
|[aws_iam_policy_document_gov_friendly_arns](aws_iam_policy_document_gov_friendly_arns.md)|Ensure `iam_policy_document` data sources do not contain `arn:aws:` ARN's||
|[aws_iam_policy_gov_friendly_arns](aws_iam_policy_gov_friendly_arns.md)|Ensure `iam_policy` resources do not contain `arn:aws:` ARN's||
|[aws_iam_role_policy_gov_friendly_arns](aws_iam_role_policy_gov_friendly_arns.md)|Ensure `iam_role_policy` resources do not contain `arn:aws:` ARN's||
Expand Down
37 changes: 37 additions & 0 deletions docs/rules/aws_iam_policy_attachment_exclusive_attachment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# aws_iam_policy_attachment_exclusive_attachment

This rule checks whether the `aws_iam_policy_attachment` resource is used.

The `aws_iam_policy_attachment` resource creates exclusive attachments for IAM policies. Within the entire AWS account, all users, roles, and groups that a single policy is attached to must be specified by a single aws_iam_policy_attachment resource.

## Configuration

```hcl
rule "aws_iam_policy_attachment_exclusive_attachment" {
enabled = true
}
```

## Example

```hcl
resource "aws_iam_policy_attachment" "attachment" {
name = "test_attachment"
}
```

```shell
$ tflint
1 issue(s) found:
Warning: Within the entire AWS account, all users, roles, and groups that a single policy is attached to must be specified by a single aws_iam_policy_attachment resource. Consider aws_iam_role_policy_attachment, aws_iam_user_policy_attachment, or aws_iam_group_policy_attachment instead. (aws_iam_policy_attachment_has_alternatives)
on template.tf line 2:
2: resource "aws_iam_policy_attachment" "attachment" {
```
## Why
The [`aws_iam_policy_attachment`](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy_attachment) resource creates exclusive attachments of IAM policies. Across the entire AWS account, all the users/roles/groups to which a single policy is attached must be declared by a single `aws_iam_policy_attachment` resource. This means that even any users/roles/groups that have the attached policy via any other mechanism (including other Terraform resources) will have that attached policy revoked by this resource.
## How To Fix
Consider using `aws_iam_role_policy_attachment`, `aws_iam_user_policy_attachment`, or `aws_iam_group_policy_attachment` instead. These resources do not enforce exclusive attachment of an IAM policy.
65 changes: 65 additions & 0 deletions rules/aws_iam_policy_attachment_exclusive_attachment.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package rules

import (
"github.com/terraform-linters/tflint-plugin-sdk/hclext"
"github.com/terraform-linters/tflint-plugin-sdk/tflint"
"github.com/terraform-linters/tflint-ruleset-aws/project"
)

// AwsIAMPolicyAttachmentExclusiveAttachmentRule warns that the resource has alternatives recommended
type AwsIAMPolicyAttachmentExclusiveAttachmentRule struct {
tflint.DefaultRule

resourceType string
attributeName string
}

// AwsIAMPolicyAttachmentExclusiveAttachmentRule returns new rule with default attributes
func NewAwsIAMPolicyAttachmentExclusiveAttachmentRule() *AwsIAMPolicyAttachmentExclusiveAttachmentRule {
return &AwsIAMPolicyAttachmentExclusiveAttachmentRule{
resourceType: "aws_iam_policy_attachment",
attributeName: "name",
}
}

// Name returns the rule name
func (r *AwsIAMPolicyAttachmentExclusiveAttachmentRule) Name() string {
return "aws_iam_policy_attachment_exclusive_attachment"
}

// Enabled returns whether the rule is enabled by default
func (r *AwsIAMPolicyAttachmentExclusiveAttachmentRule) Enabled() bool {
return false
}

// Severity returns the rule severity
func (r *AwsIAMPolicyAttachmentExclusiveAttachmentRule) Severity() tflint.Severity {
return tflint.WARNING
}

// Link returns the rule reference link
func (r *AwsIAMPolicyAttachmentExclusiveAttachmentRule) Link() string {
return project.ReferenceLink(r.Name())
}

// Check that the resource is not used
func (r *AwsIAMPolicyAttachmentExclusiveAttachmentRule) Check(runner tflint.Runner) error {
resources, err := runner.GetResourceContent(r.resourceType, &hclext.BodySchema{}, nil)
if err != nil {
return err
}

for _, resource := range resources.Blocks {
runner.EmitIssue(
r,
"Within the entire AWS account, all users, roles, and groups that a single policy is attached to must be specified by a single aws_iam_policy_attachment resource. Consider aws_iam_role_policy_attachment, aws_iam_user_policy_attachment, or aws_iam_group_policy_attachment instead.",
resource.DefRange,
)

if err != nil {
return err
}
}

return nil
}
61 changes: 61 additions & 0 deletions rules/aws_iam_policy_attachment_exclusive_attachment_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package rules

import (
"math/rand"
"testing"
"time"

hcl "github.com/hashicorp/hcl/v2"
"github.com/terraform-linters/tflint-plugin-sdk/helper"
)

func Test_AwsIAMPolicyAttachmentExclusiveAttachmentRule(t *testing.T) {
rand.Seed(time.Now().UnixNano())

cases := []struct {
Name string
Content string
Expected helper.Issues
}{
{
Name: "resource has alternatives",
Content: `
resource "aws_iam_policy_attachment" "attachment" {
name = "test_attachment"
}
`,
Expected: helper.Issues{
{
Rule: NewAwsIAMPolicyAttachmentExclusiveAttachmentRule(),
Message: "Within the entire AWS account, all users, roles, and groups that a single policy is attached to must be specified by a single aws_iam_policy_attachment resource. Consider aws_iam_role_policy_attachment, aws_iam_user_policy_attachment, or aws_iam_group_policy_attachment instead.",
Range: hcl.Range{
Filename: "resource.tf",
Start: hcl.Pos{Line: 2, Column: 1},
End: hcl.Pos{Line: 2, Column: 50},
},
},
},
},
{
Name: "no issues with resource",
Content: `
resource "aws_iam_role_policy_attachment" "attachment" {
role = "test_role"
}
`,
Expected: helper.Issues{},
},
}

rule := NewAwsIAMPolicyAttachmentExclusiveAttachmentRule()

for _, tc := range cases {
runner := helper.TestRunner(t, map[string]string{"resource.tf": tc.Content})

if err := rule.Check(runner); err != nil {
t.Fatalf("Unexpected error occurred: %s", err)
}

helper.AssertIssues(t, tc.Expected, runner.Issues)
}
}
1 change: 1 addition & 0 deletions rules/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var manualRules = []tflint.Rule{
NewAwsElastiCacheReplicationGroupDefaultParameterGroupRule(),
NewAwsElastiCacheReplicationGroupInvalidTypeRule(),
NewAwsElastiCacheReplicationGroupPreviousTypeRule(),
NewAwsIAMPolicyAttachmentExclusiveAttachmentRule(),
NewAwsIAMPolicySidInvalidCharactersRule(),
NewAwsIAMPolicyTooLongPolicyRule(),
NewAwsLambdaFunctionDeprecatedRuntimeRule(),
Expand Down

0 comments on commit 1538e44

Please sign in to comment.