Add `aws_iam_role_deprecated_policy_attributes` rule #833

alexjfisher · 2025-02-25T09:56:41Z

No description provided.

bendrucker · 2025-02-25T19:18:49Z

rules/aws_iam_role_deprecated_policy_attributes.go

+			}
+		}
+
+		for _, rule := range resource.Body.Blocks {


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

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!)

I understand this as a safety check that asserts expectations.

Just a safety check!

bendrucker · 2025-02-25T19:22:23Z

rules/aws_iam_role_deprecated_policy_attributes.go

+		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 {


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.

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.

Oh, in that case quoting the provider docs is fine

bendrucker · 2025-02-25T19:22:34Z

rules/aws_iam_role_deprecated_policy_attributes.go

+		}
+
+		for _, rule := range resource.Body.Blocks {
+			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 {


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 {

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.

The warning messages have been copied verbatim from the upstream documentation of the `aws_iam_role` resource.

wata727 · 2025-03-06T06:50:38Z

Thanks for working on this. However, it looks like these deprecation warnings have already been addressed by the AWS provider.
https://github.com/hashicorp/terraform-provider-aws/blob/v5.89.0/internal/service/iam/role.go#L94-L98
https://github.com/hashicorp/terraform-provider-aws/blob/v5.89.0/internal/service/iam/role.go#L134-L138

Given this, it seems like there would be no point in introducing the same rule in TFLint, but what use cases do you have?

bendrucker · 2025-03-06T21:32:53Z

terraform validate will print warnings for deprecated provider attributes. Duplicating that in TFLint is costly. It gives you control over disabling the rule or ignoring it for specific files/lines. But it does make me think twice about adding the rule.

alexjfisher · 2025-03-07T09:30:36Z

Thanks for taking another look at this.

I can see the warning during a terraform plan, but AFAICT there isn't a built in way to treat the warnings as an error and exit non-zero? Looks like there was a proposal for this, but it's yet to be implemented??
(Maybe I could parse the output of the plan file?? But that's not very convenient compared to tflint that is integrated into my editor.)

With terraform validate I'm not seeing any warnings. If it did, I think you're right that the lint plugin would be unnecessary duplication.

❯ terraform version
Terraform v1.11.1
on linux_amd64
+ provider registry.terraform.io/hashicorp/aws v5.90.0
❯ terraform validate -json
{
  "format_version": "1.0",
  "valid": true,
  "error_count": 0,
  "warning_count": 0,
  "diagnostics": []
}
❯ cat main.tf
# -SNIP-

resource "aws_iam_role" "this" {
  name_prefix = "Redacted-"
  path        = "/Redacted/"

  assume_role_policy = <<-EOF
  {
    "Version": "2012-10-17",
    "Statement": [
      {
        "Effect": "Allow",
        "Principal": {
          "Service": ["events.amazonaws.com"]
        },
        "Action": "sts:AssumeRole"
      }
    ]
  }
  EOF

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

data "aws_iam_policy_document" "inline_policy" {
  statement {
    actions   = ["events:PutEvents"]
    resources = local.target_arns
  }
}

alexjfisher · 2025-03-07T09:45:05Z

Actually, looks like inline_policy doesn't produce a warning with terraform validate, but managed_policy_arns does... A bug or a limitation of deprecated block attributes??
Also like plan there's no built-in way of treating warnings as errors with validate.

bendrucker · 2025-03-07T18:38:56Z

For this config:

terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
    }
  }
}

resource "aws_iam_role" "example_role" {
  name = "example-role"
  assume_role_policy = jsonencode({})

  managed_policy_arns = [
    "arn:aws:iam::aws:policy/AmazonS3ReadOnlyAccess",
    "arn:aws:iam::aws:policy/CloudWatchLogsFullAccess"
  ]

  inline_policy {
    policy = jsonencode({})
  }
}

I get this terraform validate -json output:

{
  "format_version": "1.0",
  "valid": true,
  "error_count": 0,
  "warning_count": 2,
  "diagnostics": [
    {
      "severity": "warning",
      "summary": "Argument is deprecated",
      "detail": "managed_policy_arns 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.",
      "address": "aws_iam_role.example_role",
      "range": {
        "filename": "main.tf",
        "start": {
          "line": 13,
          "column": 25,
          "byte": 220
        },
        "end": {
          "line": 16,
          "column": 4,
          "byte": 334
        }
      },
      "snippet": {
        "context": "resource \"aws_iam_role\" \"example_role\"",
        "code": "  managed_policy_arns = [\n    \"arn:aws:iam::aws:policy/AmazonS3ReadOnlyAccess\",\n    \"arn:aws:iam::aws:policy/CloudWatchLogsFullAccess\"\n  ]",
        "start_line": 13,
        "highlight_start_offset": 24,
        "highlight_end_offset": 138,
        "values": []
      }
    },
    {
      "severity": "warning",
      "summary": "Argument is deprecated",
      "detail": "inline_policy 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.",
      "address": "aws_iam_role.example_role",
      "range": {
        "filename": "main.tf",
        "start": {
          "line": 9,
          "column": 40,
          "byte": 131
        },
        "end": {
          "line": 9,
          "column": 41,
          "byte": 132
        }
      },
      "snippet": {
        "context": "resource \"aws_iam_role\" \"example_role\"",
        "code": "resource \"aws_iam_role\" \"example_role\" {",
        "start_line": 9,
        "highlight_start_offset": 39,
        "highlight_end_offset": 40,
        "values": []
      }
    }
  ]
}

Which looks correct.

I get that the Terraform CLI doesn't handle the whole workflow but the pieces are there. Initially I was going to say it's a separate tool but maybe we should consider something where TFLint calls terraform validate -json, for the sake of integrating output, allowing control of exit code level, using annotations, etc.

I'd say the idea of TFLint handling plans is pretty dead (terraform-linters/tflint#328) but validate has a much stronger case.

alexjfisher · 2025-03-12T10:48:00Z

Interesting...

It looks like the validate warning isn't generated in the case the inline_policy uses a data source.

terraform {
  required_providers {
    aws = {
      source = "hashicorp/aws"
    }
  }
}

resource "aws_iam_role" "warning_generated" {
  name               = "example-1"
  assume_role_policy = jsonencode({})

  inline_policy {
    policy = jsonencode({})
  }
}

resource "aws_iam_role" "no_warning" {
  name               = "example-2"
  assume_role_policy = jsonencode({})

  inline_policy {
    policy = data.aws_iam_policy_document.inline_policy.json
  }
}

data "aws_iam_policy_document" "inline_policy" {
  statement {
    actions   = ["events:PutEvents"]
    resources = ["*"]
  }
}

{
  "format_version": "1.0",
  "valid": true,
  "error_count": 0,
  "warning_count": 1,
  "diagnostics": [
    {
      "severity": "warning",
      "summary": "Argument is deprecated",
      "detail": "inline_policy 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.",
      "address": "aws_iam_role.warning_generated",
      "range": {
        "filename": "test.tf",
        "start": {
          "line": 9,
          "column": 45,
          "byte": 138
        },
        "end": {
          "line": 9,
          "column": 46,
          "byte": 139
        }
      },
      "snippet": {
        "context": "resource \"aws_iam_role\" \"warning_generated\"",
        "code": "resource \"aws_iam_role\" \"warning_generated\" {",
        "start_line": 9,
        "highlight_start_offset": 44,
        "highlight_end_offset": 45,
        "values": []
      }
    }
  ]
}

Not sure if this is a limitation in terraform itself, or the provider.

It's common to use aws_iam_policy_document so if this can't be fixed in validate, maybe this rule is still useful?

bendrucker · 2025-03-12T18:33:26Z

Not sure if this is a limitation in terraform itself, or the provider.

Terraform. Even though the full shape of the JSON is unknown, Terraform still knows that the block is declared. It just can't see the attribute value during validation with the data source. So there shouldn't be any difference in validate output.

That bug is worth squashing upstream. But given the significant usage of the iam_role resource I'd say a dedicated rule is justified if terraform validate can't reliably produce the same warnings, @wata727?

alexjfisher · 2025-03-28T16:04:48Z

Any closer to a consensus on this? I bumped into this again today with a role with

managed_policy_arns = [
  "arn:${data.aws_partition.current.partition}:iam::aws:policy/SecurityAudit"
]

Had the partition been hard-coded, validate would have picked this up.

wata727 · 2025-04-16T07:22:38Z

Sorry for the late reply. I agree with @bendrucker's suggestion.

Fix rule template typo

38a8cd9

bendrucker reviewed Feb 25, 2025

View reviewed changes

Add aws_iam_role_deprecated_policy_attributes rule

ff01475

The warning messages have been copied verbatim from the upstream documentation of the `aws_iam_role` resource.

alexjfisher force-pushed the aws_iam_role_deprecated_policy_attributes branch from 57cd57a to ff01475 Compare February 26, 2025 14:43

alexjfisher mentioned this pull request Mar 12, 2025

Add --fail-on-warnings option to validate and plan opentofu/opentofu#2585

Closed

bendrucker approved these changes Apr 16, 2025

View reviewed changes

bendrucker merged commit 100cf11 into terraform-linters:master Apr 16, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `aws_iam_role_deprecated_policy_attributes` rule #833

Add `aws_iam_role_deprecated_policy_attributes` rule #833

alexjfisher commented Feb 25, 2025

bendrucker Feb 25, 2025

alexjfisher Feb 25, 2025

wata727 Mar 6, 2025

bendrucker Mar 6, 2025

bendrucker Feb 25, 2025

alexjfisher Feb 25, 2025

bendrucker Mar 6, 2025

bendrucker Feb 25, 2025

alexjfisher Feb 25, 2025

wata727 commented Mar 6, 2025

bendrucker commented Mar 6, 2025

alexjfisher commented Mar 7, 2025

alexjfisher commented Mar 7, 2025

bendrucker commented Mar 7, 2025

alexjfisher commented Mar 12, 2025

bendrucker commented Mar 12, 2025

alexjfisher commented Mar 28, 2025

wata727 commented Apr 16, 2025

	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 {

Add aws_iam_role_deprecated_policy_attributes rule #833

Add aws_iam_role_deprecated_policy_attributes rule #833

Conversation

alexjfisher commented Feb 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wata727 commented Mar 6, 2025

bendrucker commented Mar 6, 2025

alexjfisher commented Mar 7, 2025

alexjfisher commented Mar 7, 2025

bendrucker commented Mar 7, 2025

alexjfisher commented Mar 12, 2025

bendrucker commented Mar 12, 2025

alexjfisher commented Mar 28, 2025

wata727 commented Apr 16, 2025

Add `aws_iam_role_deprecated_policy_attributes` rule #833

Add `aws_iam_role_deprecated_policy_attributes` rule #833