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_role_deprecated_policy_attributes rule #833

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ These rules enforce best practices and naming conventions:
|[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_deprecated_policy_attributes](aws_iam_role_deprecated_policy_attributes.md)|Disallow using deprecated policy attributes of `aws_iam_role`||
|[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||
|[aws_lambda_function_deprecated_runtime](aws_lambda_function_deprecated_runtime.md)|Disallow deprecated runtimes for Lambda Function|✔|
|[aws_resource_missing_tags](aws_resource_missing_tags.md)|Require specific tags for all AWS resource types that support them||
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 @@ -71,6 +71,7 @@ These rules enforce best practices and naming conventions:
|[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_deprecated_policy_attributes](aws_iam_role_deprecated_policy_attributes.md)|Disallow using deprecated policy attributes of `aws_iam_role`||
|[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||
|[aws_lambda_function_deprecated_runtime](aws_lambda_function_deprecated_runtime.md)|Disallow deprecated runtimes for Lambda Function|✔|
|[aws_resource_missing_tags](aws_resource_missing_tags.md)|Require specific tags for all AWS resource types that support them||
Expand Down
64 changes: 64 additions & 0 deletions docs/rules/aws_iam_role_deprecated_policy_attributes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# aws_iam_role_deprecated_policy_attributes

Disallow `inline_policy` configuration blocks and `managed_policy_arns` argument of the `aws_iam_role` resource.

## Example

```hcl
resource "aws_iam_role" "example" {
name = "example"
assume_role_policy = data.aws_iam_policy_document.instance_assume_role_policy.json # (not shown)

inline_policy {
name = "my_inline_policy"

policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
Action = ["ec2:Describe*"]
Effect = "Allow"
Resource = "*"
},
]
})
}

inline_policy {
name = "policy-8675309"
policy = data.aws_iam_policy_document.inline_policy.json # (not shown)
}

managed_policy_arns = []
}
```

```
$ tflint

3 issue(s) found:

Warning: The inline_policy argument is deprecated. Use the aws_iam_role_policy resource instead. If Terraform should exclusively manage all inline policy associations (the current behavior of this argument), use the aws_iam_role_policies_exclusive resource as well. (aws_iam_role_deprecated_policy_attributes)

on test.tf line 5:
5: inline_policy {

Warning: The inline_policy argument is deprecated. Use the aws_iam_role_policy resource instead. If Terraform should exclusively manage all inline policy associations (the current behavior of this argument), use the aws_iam_role_policies_exclusive resource as well. (aws_iam_role_deprecated_policy_attributes)

on test.tf line 20:
20: inline_policy {

Warning: The managed_policy_arns argument is deprecated. Use the aws_iam_role_policy_attachment resource instead. If Terraform should exclusively manage all managed policy attachments (the current behavior of this argument), use the aws_iam_role_policy_attachments_exclusive resource as well. (aws_iam_role_deprecated_policy_attributes)

on test.tf line 25:
25: managed_policy_arns = []
```

## Why

The `inline_policy` and `managed_policy_arns` arguments are both deprecated since late 2024.

## How To Fix

For managing a role's inline policy, use the [aws_iam_role_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy) resource instead.
For attaching managed policies to a role, use the [aws_iam_role_policy_attachment](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) resource.
84 changes: 84 additions & 0 deletions rules/aws_iam_role_deprecated_policy_attributes.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
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"
)

// AwsIAMRoleDeprecatedPolicyAttributesRule checks that neither `inline_policy` or `managed_policy_arns` are used in an `aws_iam_role`
type AwsIAMRoleDeprecatedPolicyAttributesRule struct {
tflint.DefaultRule

resourceType string
}

// NewAwsIAMRoleDeprecatedPolicyAttributesRule returns new rule with default attributes
func NewAwsIAMRoleDeprecatedPolicyAttributesRule() *AwsIAMRoleDeprecatedPolicyAttributesRule {
return &AwsIAMRoleDeprecatedPolicyAttributesRule{
resourceType: "aws_iam_role",
}
}

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

// Enabled returns whether the rule is enabled by default
func (r *AwsIAMRoleDeprecatedPolicyAttributesRule) Enabled() bool {
// `inline_policy` and `managed_policy_arns` were deprecated in 5.68.0 and 5.72.0 respectively
// At time of writing, this is quite recent, so don't automatically enable the rule.
return false
}

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

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

// Check checks that aws_iam_role resources don't have any `inline_policy` blocks or the `managed_policy_arns` attribute
func (r *AwsIAMRoleDeprecatedPolicyAttributesRule) Check(runner tflint.Runner) error {
resources, err := runner.GetResourceContent(r.resourceType, &hclext.BodySchema{
Blocks: []hclext.BlockSchema{
{
Type: "inline_policy",
},
},
Attributes: []hclext.AttributeSchema{
{
Name: "managed_policy_arns",
},
},
}, nil)
if err != nil {
return err
}

for _, resource := range resources.Blocks {
attribute, exists := resource.Body.Attributes["managed_policy_arns"]

if exists {
if err := runner.EmitIssue(r, "The managed_policy_arns argument is deprecated. Use the aws_iam_role_policy_attachment resource instead. If Terraform should exclusively manage all managed policy attachments (the current behavior of this argument), use the aws_iam_role_policy_attachments_exclusive resource as well.", attribute.Range); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

I would flip these. Users should prefer the exclusive resources by default and fall back on the non-authoritative versions if they want to attach policies separate from the role definition.

Copy link
Author

Choose a reason for hiding this comment

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

The warning messages were taken verbatim from the iam_role documentation page. https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role

If you want exclusive behaviour, you have to use both aws_iam_role_policy_attachments_exclusive and aws_iam_role_policy_attachment resources.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, in that case quoting the provider docs is fine

return err
}
}

for _, rule := range resource.Body.Blocks {
Copy link
Member

Choose a reason for hiding this comment

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

Avoid assuming that inline_policies is the only block type and explicitly check that in this loop and skip other blocks.

Copy link
Author

Choose a reason for hiding this comment

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

I'll look into this. Is this just a safety check, or a really bad assumption I've made with how the schema passed to GetResourceContent works?? I think I was assuming the only block type I'd ever see is for inline_policy. But this is my first attempt at writing a plugin, (and I barely know any go!)

Copy link
Member

Choose a reason for hiding this comment

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

I understand this as a safety check that asserts expectations.

Copy link
Member

Choose a reason for hiding this comment

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

Just a safety check!

if rule.Type != "inline_policy" {
continue
}

if err := runner.EmitIssue(r, "The inline_policy argument is deprecated. Use the aws_iam_role_policy resource instead. If Terraform should exclusively manage all inline policy associations (the current behavior of this argument), use the aws_iam_role_policies_exclusive resource as well.", rule.DefRange); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if err := runner.EmitIssue(r, "The inline_policy argument is deprecated. Use the aws_iam_role_policy resource instead. If Terraform should exclusively manage all inline policy associations (the current behavior of this argument), use the aws_iam_role_policies_exclusive resource as well.", rule.DefRange); err != nil {
if err := runner.EmitIssue(r, "The inline_policy block is deprecated. Use the aws_iam_role_policy resource instead. If Terraform should exclusively manage all inline policy associations (the current behavior of this argument), use the aws_iam_role_policies_exclusive resource as well.", rule.DefRange); err != nil {

Copy link
Author

Choose a reason for hiding this comment

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

Again, taken verbatim from upstream resource docs, but I can't disagree, block is more accurate, so I can still change it if you prefer.

return err
}
}
}

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

import (
"testing"

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

func Test_AwsIAMRoleDeprecatedPolicyAttributes(t *testing.T) {
cases := []struct {
Name string
Content string
Expected helper.Issues
}{
{
Name: "inline_policies",
Content: `
resource "aws_iam_role" "example" {
name = "yak_role"
assume_role_policy = data.aws_iam_policy_document.instance_assume_role_policy.json # (not shown)

inline_policy {
name = "my_inline_policy"

policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
Action = ["ec2:Describe*"]
Effect = "Allow"
Resource = "*"
},
]
})
}

inline_policy {
name = "policy-8675309"
policy = data.aws_iam_policy_document.inline_policy.json
}
}

data "aws_iam_policy_document" "inline_policy" {
statement {
actions = ["ec2:DescribeAccountAttributes"]
resources = ["*"]
}
}
`,
Expected: helper.Issues{
{
Rule: NewAwsIAMRoleDeprecatedPolicyAttributesRule(),
Message: "The inline_policy argument is deprecated. Use the aws_iam_role_policy resource instead. If Terraform should exclusively manage all inline policy associations (the current behavior of this argument), use the aws_iam_role_policies_exclusive resource as well.",
Range: hcl.Range{
Filename: "resource.tf",
Start: hcl.Pos{Line: 6, Column: 3},
End: hcl.Pos{Line: 6, Column: 16},
},
},
{
Rule: NewAwsIAMRoleDeprecatedPolicyAttributesRule(),
Message: "The inline_policy argument is deprecated. Use the aws_iam_role_policy resource instead. If Terraform should exclusively manage all inline policy associations (the current behavior of this argument), use the aws_iam_role_policies_exclusive resource as well.",
Range: hcl.Range{
Filename: "resource.tf",
Start: hcl.Pos{Line: 21, Column: 3},
End: hcl.Pos{Line: 21, Column: 16},
},
},
},
},
{
Name: "managed_policy_arns",
Content: `
resource "aws_iam_role" "example" {
name = "yak_role"
assume_role_policy = data.aws_iam_policy_document.instance_assume_role_policy.json # (not shown)
managed_policy_arns = []
}
`,
Expected: helper.Issues{
{
Rule: NewAwsIAMRoleDeprecatedPolicyAttributesRule(),
Message: "The managed_policy_arns argument is deprecated. Use the aws_iam_role_policy_attachment resource instead. If Terraform should exclusively manage all managed policy attachments (the current behavior of this argument), use the aws_iam_role_policy_attachments_exclusive resource as well.",
Range: hcl.Range{
Filename: "resource.tf",
Start: hcl.Pos{Line: 5, Column: 3},
End: hcl.Pos{Line: 5, Column: 27},
},
},
},
},
}

rule := NewAwsIAMRoleDeprecatedPolicyAttributesRule()

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 @@ -43,6 +43,7 @@ var manualRules = []tflint.Rule{
NewAwsProviderMissingDefaultTagsRule(),
NewAwsSecurityGroupInlineRulesRule(),
NewAwsSecurityGroupRuleDeprecatedRule(),
NewAwsIAMRoleDeprecatedPolicyAttributesRule(),
}

// Rules is a list of all rules
Expand Down
2 changes: 1 addition & 1 deletion rules/rule.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (r *{{ .RuleNameCC }}Rule) Enabled() bool {

// Severity returns the rule severity
func (r *{{ .RuleNameCC }}Rule) Severity() tflint.Severity {
// TODO: Determine the rule's severiry
// TODO: Determine the rule's severity
return tflint.ERROR
}

Expand Down
Loading