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

feat: add aws_iam_policy_attachment_exclusive_attachment rule #786

Merged
merged 15 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
.idea/
Copy link
Member

Choose a reason for hiding this comment

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

Revert this


tools/provider-schema/.terraform/
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
35 changes: 35 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,35 @@
# aws_iam_policy_attachment_exclusive_attachment

Consider alternative resources to `aws_iam_policy_attachment`.
Copy link
Member

Choose a reason for hiding this comment

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

Get a little bit of the why up here. The section below can expand but this intro should give the reader a 1-2 sentence summary of what and why.


## 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: 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: name "test_attachment"
```

## Why

The `aws_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. https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy_attachment
Copy link
Member

Choose a reason for hiding this comment

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

Hyperlink the first use of the identifier (in backticks) instead of appending the URL at the end


## 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.
72 changes: 72 additions & 0 deletions rules/aws_iam_policy_attachment_exclusive_attachment.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
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{
Attributes: []hclext.AttributeSchema{{Name: r.attributeName}},
}, nil)
if err != nil {
return err
}

for _, resource := range resources.Blocks {
attribute, exists := resource.Body.Attributes[r.attributeName]
Copy link
Member

Choose a reason for hiding this comment

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

Why does this logic apply? If there's no name attribute, the resource should be skipped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see this logic on my machine. We don't need it here.

if !exists {
continue
}

runner.EmitIssue(
r,
"Consider aws_iam_role_policy_attachment, aws_iam_user_policy_attachment, or aws_iam_group_policy_attachment instead.",
Copy link
Member

Choose a reason for hiding this comment

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

Include a brief bit of why here (exclusive for whole account)

attribute.Expr.Range(),
)

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: "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: 3, Column: 9},
End: hcl.Pos{Line: 3, Column: 26},
},
},
},
},
{
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
Loading