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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to inline_policy blocks on aws_iam_role always recreates policy #22336

Open
danopia opened this issue Dec 23, 2021 · 17 comments 路 May be fixed by #35634
Open

Changes to inline_policy blocks on aws_iam_role always recreates policy #22336

danopia opened this issue Dec 23, 2021 · 17 comments 路 May be fixed by #35634
Labels
bug Addresses a defect in current functionality. service/iam Issues and PRs that pertain to the iam service.

Comments

@danopia
Copy link

danopia commented Dec 23, 2021

Community Note

  • Please vote on this issue by adding a 馃憤 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform CLI and Terraform AWS Provider Version

Terraform v1.1.2
on darwin_amd64
+ provider registry.terraform.io/hashicorp/aws v3.70.0

Affected Resource(s)

  • resource.aws_iam_role
  • I'm also using data.aws_iam_policy_document in the reproduction

Terraform Configuration Files

resource "aws_iam_role" "main" {
  assume_role_policy = data.aws_iam_policy_document.trust.json
  name = "terraform-issue-reproduction"

  inline_policy {
    name = "InlinePolicy"
    policy = data.aws_iam_policy_document.inline.json
  }
}

# Inline policy document that we'll be changing
data "aws_iam_policy_document" "inline" {
  statement {
    sid = "S3"
    actions = ["s3:ListBucket"]
    resources = [
      "arn:aws:s3:::some-bucket-a",
      # After the first apply, uncomment this next line
      # "arn:aws:s3:::some-bucket-b",
    ]
  }
}

# Minimal, arbitrary trust statement to make the role valid
data "aws_iam_policy_document" "trust" {
  statement {
    actions = ["sts:AssumeRole"]
    principals {
      type        = "Service"
      identifiers = ["apigateway.amazonaws.com"]
    }
  }
}

Debug Output

Redacted debug-level log: https://gist.github.com/danopia/0941f0d5f487432770c670603b221ea1

Expected Behavior

Because the inline_policy blocks have the same name value, Terraform should view this change as an update to the existing inline_policy:

  1. The diff should point out the actual changes to the policy.
  2. When applying, Terraform shouldn't delete the old policy. Creating the new policy will cause IAM to perform an overwrite operation internally.

I'd expect the diff to look like this:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the
following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # aws_iam_role.main will be updated in-place
  ~ resource "aws_iam_role" "main" {
        id                    = "terraform-issue-reproduction"
        name                  = "terraform-issue-reproduction"
        tags                  = {}
        # (9 unchanged attributes hidden)

      ~ inline_policy {
            name   = "InlinePolicy"
          ~ policy = jsonencode(
              ~ {
                  ~ Statement = [
                      ~ {
                            Action   = "s3:ListBucket"
                            Effect   = "Allow"
                          ~ Resource = [
                                "arn:aws:s3:::some-bucket-a",
                              + "arn:aws:s3:::some-bucket-b",
                            ]
                            Sid      = "S3"
                        },
                    ]
                    Version   = "2012-10-17"
                }
            )
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Actual Behavior

Terraform prints a diff showing an inline_policy removal and then also an inline_policy creation:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the
following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # aws_iam_role.main will be updated in-place
  ~ resource "aws_iam_role" "main" {
        id                    = "terraform-issue-reproduction"
        name                  = "terraform-issue-reproduction"
        tags                  = {}
        # (9 unchanged attributes hidden)

      + inline_policy {
          + name   = "InlinePolicy"
          + policy = jsonencode(
                {
                  + Statement = [
                      + {
                          + Action   = "s3:ListBucket"
                          + Effect   = "Allow"
                          + Resource = [
                              + "arn:aws:s3:::some-bucket-b",
                              + "arn:aws:s3:::some-bucket-a",
                            ]
                          + Sid      = "S3"
                        },
                    ]
                  + Version   = "2012-10-17"
                }
            )
        }
      - inline_policy {
          - name   = "InlinePolicy" -> null
          - policy = jsonencode(
                {
                  - Statement = [
                      - {
                          - Action   = "s3:ListBucket"
                          - Effect   = "Allow"
                          - Resource = "arn:aws:s3:::some-bucket-a"
                          - Sid      = "S3"
                        },
                    ]
                  - Version   = "2012-10-17"
                }
            ) -> null
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Also, from the attached debug log, we can see that Terraform is actually doing a delete-then-create sequence:

Action=DeleteRolePolicy&PolicyName=InlinePolicy&RoleName=terraform-issue-reproduction&Version=2010-05-08
...later...
Action=PutRolePolicy&PolicyDocument=%7B%0A++%22Version%22%3A+%222012-10-17%22%2C%0A++%22Statement%22%3A+%5B%0A++++%7B%0A++++++%22Sid%22%3A+%22S3%22%2C%0A++++++%22Effect%22%3A+%22Allow%22%2C%0A++++++%22Action%22%3A+%22s3%3AListBucket%22%2C%0A++++++%22Resource%22%3A+%5B%0A++++++++%22arn%3Aaws%3As3%3A%3A%3Asome-bucket-b%22%2C%0A++++++++%22arn%3Aaws%3As3%3A%3A%3Asome-bucket-a%22%0A++++++%5D%0A++++%7D%0A++%5D%0A%7D&PolicyName=InlinePolicy&RoleName=terraform-issue-reproduction&Version=2010-05-08

Steps to Reproduce

  1. terraform apply
  2. Uncomment the line mentioning some-bucket-b
  3. terraform apply

References

I've found this issue mentioned within other inline_policy issues:

@github-actions github-actions bot added needs-triage Waiting for first response or review from a maintainer. service/iam Issues and PRs that pertain to the iam service. labels Dec 23, 2021
@danopia danopia changed the title Changes to inline_policy blocks on aws_iam_role always show as recreations Changes to inline_policy blocks on aws_iam_role always recreates policy Jan 3, 2022
@justinretzolk justinretzolk added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Jan 14, 2022
@ChaseFreeman17
Copy link

Adding some extra information as I think this is also related but not 100% sure.

When using a blank inline policy (which enforces that any added inline policy is removed). See example: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role#example-of-removing-inline-policies

As of version 3.70 the inline block will always show a change (even if there is no inline policy to remove):
+ inline_policy {}

3.69.0 works as expect.

@danopia
Copy link
Author

danopia commented Feb 16, 2022

As of version 3.70 the inline block will always show a change (even if there is no inline policy to remove):
+ inline_policy {}

3.69.0 works as expect.

I also have this issue, but I don't think it's the same cause, so it should probably be split out into a different ticket. I believe the inline policy update issue is not a regression.

@tmccombs
Copy link
Contributor

tmccombs commented Jan 4, 2023

This makes it difficult to see what the actual changes to the policy are.

@tmccombs
Copy link
Contributor

tmccombs commented Jan 4, 2023

I think part of the problem here is that the schema of inline_policy is TypeSet, but it really should be a dictionary, since any change in a Set marks shows up as the whole object being different. However, changing that is probably backwards incompatible. Maybe if terraform core added support for specifying a "key" for a set, so that it does internal diffs for items within the set based on the key?

Alternatively, maybe it would be possible to create a custom diff for inline_policy, and change the update to detect if a single policy is changed and do an update (with PutRolePolicy) instead of a delete and add?

@tmccombs
Copy link
Contributor

tmccombs commented Jan 5, 2023

This is actually a bit of a theme in a lot of issues with this provider: TypeSet is used for complex objects in the schema, and then if there is a change in one of those objects, it is done as a destroy and create, instead of an in-place update, even when AWS has an API to do an in-place update.

@teddylear
Copy link
Contributor

@tmccombs Hit the same issue. I'll take a look at updating the custom diff for the policies to see if that could be updated to resolve this as that would break backwards compatibility.

@teddylear
Copy link
Contributor

teddylear commented Jun 10, 2023

I guess another alternative would be to have some sort of flag for not using inline_policy directly on the iam role and use aws_iam_role_policy instead. I believe s3 has moved to do this with removing it. But having a flag on the terraform resource that is optional to not have any inline policies managed by state directly in aws_iam_role resource would avoid the issue inline policies on the resource breaking things. Alternatively removing inline_policy all together (like s3 did) would resolve this issue. I'm fine with either approach, I know the flag would be a little hacky but temporarily resolve issues for customers now before a new major and/or minor release to remove completely.

@justinretzolk Would it be possible to have one of the maintainers give input in here before I start development? More than willing to help contribute this, just need to know which direction I should go before I start development.

@tmccombs
Copy link
Contributor

One advantage of inline_policy over aws_iam_role_policy, is that it is better for detecting drift, since it will show a diff if there is a policy that isn't managed as part of the role. There is a similar issue with security group rules.

@teddylear
Copy link
Contributor

@tmccombs Ahhh good point. Let me make an upstream issue on terraform then because as you said, this seems like a common issue with TypeSetand handling diffs for those types of resources. Hopefully maintainers there can give more insight on how this type of situation should be handled.

@teddylear
Copy link
Contributor

So from this thread from discuss Hashicorp about TypeSet, it looks like TypeSet is expected to act this way. However, the provider can control how things are updated in the backend and it could possibly be moved to something like 'TypeList`. I think first I'll try to update the backend to do a update then delete instead of the delete then add it's doing now so users have a better experience regarding this. Then make follow up PRs with playing around with other types for this.

@tmccombs
Copy link
Contributor

Are there any plans to migrate this provider to the terraform-plugin-framework?

@teddylear
Copy link
Contributor

I'm going to attempt to upgrade iam_role resource to the terraform-plugin-framework unless anyone objections or is already working on.

@teddylear
Copy link
Contributor

I'm going to try to break this updates into parts as much as possible as it will be a pretty big changeset. This first PR is just add arn valid functionality to internal terraform validators for the plugin framework

@teddylear
Copy link
Contributor

@justinretzolk Not sure the right channel to ask this but wanted to check before working more on my PR to upgrade the IAM resource to the plugin framework from sdk v2. I was planning on taking the inline_policy attribute and turn it into inline_policies as a plugin map attribute where the name of the iam role would be the key and the value would be the policies json. This way plans would be much easier to generate as each policy name has to be unique and users wouldn't run into issues of weird plans. However as this is a pretty big change, I wanted to clarify before continuing the upgrade work I was doing with maintainers of the project. If there's a better way to resolve this above issue, please let me know and I'm more than happy to work on it. Also if there's a better channel for me to communicate this please let me know.

@atheiman
Copy link
Contributor

@teddylear were you able to get any direction on this? cc @justinretzolk

@teddylear
Copy link
Contributor

@atheiman I haven't so far. I was waiting on a response, but for now I'll continue to work on this with using the plugin map attribute as I listed above as I believe that most accurately represents how the inline policies work. Then if folks want it changed, it can be updated in the PR.

@teddylear
Copy link
Contributor

Was able to make above open PR that should resolve issue. inline_policy was moved to inline_policies as a map so it could make plans like below from current updated terraform.

resource "aws_iam_role" "main" {
  assume_role_policy = data.aws_iam_policy_document.trust.json
  name               = "terraform-issue-reproduction"

  inline_policies = {
    "InlinePolicy" = data.aws_iam_policy_document.inline.json
  }
}

# Inline policy document that we'll be changing
data "aws_iam_policy_document" "inline" {
  statement {
    sid     = "S3"
    actions = ["s3:ListBucket"]
    resources = [
      "arn:aws:s3:::some-bucket-a",
      # After the first apply, uncomment this next line
      # "arn:aws:s3:::some-bucket-b",
    ]
  }
}

# Minimal, arbitrary trust statement to make the role valid
data "aws_iam_policy_document" "trust" {
  statement {
    actions = ["sts:AssumeRole"]
    principals {
      type        = "Service"
      identifiers = ["apigateway.amazonaws.com"]
    }
  }
}

And the plan to add second bucket in inline policy produces this plan which is close to original one in ticket:

  # aws_iam_role.main will be updated in-place
  ~ resource "aws_iam_role" "main" {
        id                    = "terraform-issue-reproduction"
      ~ inline_policies       = {
          ~ "InlinePolicy" = jsonencode(
              ~ {
                  ~ Statement = [
                      ~ {
                          ~ Resource = "arn:aws:s3:::some-bucket-a" -> [
                              + "arn:aws:s3:::some-bucket-b",
                              + "arn:aws:s3:::some-bucket-a",
                            ]
                            # (3 unchanged attributes hidden)
                        },
                    ]
                    # (1 unchanged attribute hidden)
                }
            )
        }
        name                  = "terraform-issue-reproduction"
        # (8 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Adding a third bucket produces this plan which is even more user friendly:

  # aws_iam_role.main will be updated in-place
  ~ resource "aws_iam_role" "main" {
        id                    = "terraform-issue-reproduction"
      ~ inline_policies       = {
          ~ "InlinePolicy" = jsonencode(
              ~ {
                  ~ Statement = [
                      ~ {
                          ~ Resource = [
                              + "arn:aws:s3:::some-bucket-c",
                                "arn:aws:s3:::some-bucket-b",
                                # (1 unchanged element hidden)
                            ]
                            # (3 unchanged attributes hidden)
                        },
                    ]
                    # (1 unchanged attribute hidden)
                }
            )
        }
        name                  = "terraform-issue-reproduction"
        # (8 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

If there is anything I should change to update about this please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Addresses a defect in current functionality. service/iam Issues and PRs that pertain to the iam service.
Projects
None yet
6 participants