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

[IAM-Policy] will be updated in-place on every re-run #348

Open
Moep90 opened this issue Oct 10, 2022 · 5 comments
Open

[IAM-Policy] will be updated in-place on every re-run #348

Moep90 opened this issue Oct 10, 2022 · 5 comments
Labels
question Further information is requested

Comments

@Moep90
Copy link

Moep90 commented Oct 10, 2022

Prerequisites

  • Be sure that theres no open issue already.
    • Did not found a matching one.

Description

When applying IAM policies, I always get a change with every run.

Steps to Reproduce

  1. terraform apply
  2. terraform apply

Expected behavior

That the policies are applied and that a re-run of terraform does not cause a "update-in-place" every time.

Actual behavior

On every re-run of the same policy, terraform tells me that its changed and needs to be "updated-in-place"

Reproduces how often

100%

Versions

terraform --versions
Terraform v1.3.2
on linux_amd64
+ provider registry.terraform.io/aminueza/minio v1.6.0
+ provider registry.terraform.io/hashicorp/random v3.4.3
+ provider registry.terraform.io/hashicorp/time v0.8.0
+ provider registry.terraform.io/hashicorp/vault v3.9.1

Additional Information

# module call
module "loki" {
  source = "./modules/terraform-module-minio-s3"

  bucket = "my-bucket"

  users = {
    "loki" = {
      s3_path = "/*"
      s3_actions = [
        "s3:GetObject",
        "s3:List*",
        "s3:PutObject",
        "s3:DeleteObject"
      ]
    },
  }
}

# inside the module
resource "minio_iam_policy" "this" {

  for_each = var.create ? var.users : tomap({})
  
  name = each.key

  policy = <<-EOT
  {
      "Version": "2012-10-17",
      "Statement":  ${jsonencode([for user, user_value in var.users : {
          "Effect" = "Allow",
          "Action" = "${user_value.s3_actions}",
          "Resource" = [
            "arn:aws:s3:::${minio_s3_bucket.this.id}"
          ]
}])}
  }
EOT
depends_on = [
  minio_s3_bucket.this
]
}
@BuJo
Copy link
Collaborator

BuJo commented Oct 25, 2022

@Moep90 Could you try updating to the latest 1.8.0 or at least 1.7.1?
In principle, this should have been resolved by #180 in 1.5.0... I'm sure it's still that by using jsonencode the ordering of the fields change and thus lead to in place updates.

I couldn't reproduce it with a simpler example not using modules:

  • Terraform version: 1.3.3
  • Provider version: 1.8.0
resource "minio_iam_user" "test6" {
  name   = "test-user-jo6"
}
resource "minio_s3_bucket" "bucket6" {
  bucket = "state-terraform-s3"
  acl    = "public"
}
resource "minio_iam_policy" "bucket6" {
  name   = "test-user-jo6"
  policy = <<-EOT
  {
      "Version": "2012-10-17",
      "Statement":  ${jsonencode([{
          "Effect" = "Allow",
          "Action" = [
            "s3:GetObject",
            "s3:List*",
            "s3:PutObject",
            "s3:DeleteObject"
          ],
          "Resource" = [
            "arn:aws:s3:::${minio_s3_bucket.bucket6.id}"
          ]
        }]
    )}
  }
EOT
  depends_on = [
    minio_s3_bucket.bucket6
  ]
}

@BuJo BuJo added the question Further information is requested label Oct 25, 2022
@onedr0p
Copy link

onedr0p commented Nov 26, 2022

I am seeing the same issue, using version 1.9.1 of this module on terraform 1.3.5.

resource "minio_s3_bucket" "outline_bucket" {
  bucket = "outlinetest"
  acl    = "private"
}

resource "minio_iam_user" "outline_user" {
  name          = var.outline_bucket_user
  force_destroy = true
}

data "minio_iam_policy_document" "outline_user_policy_document" {
  statement {
    sid = ""
    actions = [
      "s3:ListBucket",
      "s3:PutObject",
      "s3:GetObject",
      "s3:DeleteObject"
    ]
    effect = "Allow"
    resources = [
      "arn:aws:s3:::${minio_s3_bucket.outline_bucket.bucket}/*",
      "arn:aws:s3:::${minio_s3_bucket.outline_bucket.bucket}"
    ]
  }
}

resource "minio_iam_policy" "outline_iam_policy" {
  name      = minio_s3_bucket.outline_bucket.bucket
  policy    = data.minio_iam_policy_document.outline_user_policy_document.json
}

resource "minio_iam_user_policy_attachment" "outline_user_policy_attachment" {
  user_name   = minio_iam_user.outline_user.id
  policy_name = minio_iam_policy.outline_iam_policy.id
}

data "minio_iam_policy_document" "outline_bucket_policy_document" {
  statement {
    sid = ""
    actions = ["s3:GetBucketLocation"]
    effect = "Allow"
    principal = "*"
    resources = ["arn:aws:s3:::${minio_s3_bucket.outline_bucket.bucket}"]
  }

  statement {
    sid = ""
    actions = ["s3:ListBucket"]
    condition {
      test     = "StringEquals"
      variable = "s3:prefix"
      values = [
        "avatars",
        "public",
      ]
    }
    effect = "Allow"
    principal = "*"
    resources = ["arn:aws:s3:::${minio_s3_bucket.outline_bucket.bucket}"]
  }

  statement {
    sid = ""
    actions = ["s3:GetObject"]
    effect = "Allow"
    principal = "*"
    resources = [
      "arn:aws:s3:::${minio_s3_bucket.outline_bucket.bucket}/public*",
      "arn:aws:s3:::${minio_s3_bucket.outline_bucket.bucket}/avatars*"
    ]
  }
}

resource "minio_s3_bucket_policy" "outline_bucket_policy" {
  bucket = minio_s3_bucket.outline_bucket.bucket
  policy = data.minio_iam_policy_document.outline_bucket_policy_document.json
}

No matter how much I apply this with no changes, the plan output is always seeing changes:

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:

  # module.minio.minio_s3_bucket_policy.outline_bucket_policy will be updated in-place
  ~ resource "minio_s3_bucket_policy" "outline_bucket_policy" {
        id     = "outlinetest"
      ~ policy = jsonencode(
          ~ {
              ~ Statement = [
                  ~ {
                      ~ Action    = [
                          - "s3:GetBucketLocation",
                        ] -> "s3:GetBucketLocation"
                      ~ Principal = {
                          - AWS = [
                              - "*",
                            ]
                        } -> "*"
                      ~ Resource  = [
                          - "arn:aws:s3:::outlinetest",
                        ] -> "arn:aws:s3:::outlinetest"
                      + Sid       = ""
                        # (1 unchanged element hidden)
                    },
                  ~ {
                      ~ Action    = [
                          - "s3:ListBucket",
                        ] -> "s3:ListBucket"
                      ~ Principal = {
                          - AWS = [
                              - "*",
                            ]
                        } -> "*"
                      ~ Resource  = [
                          - "arn:aws:s3:::outlinetest",
                        ] -> "arn:aws:s3:::outlinetest"
                      + Sid       = ""
                        # (2 unchanged elements hidden)
                    },
                  ~ {
                      ~ Action    = [
                          - "s3:GetObject",
                        ] -> "s3:GetObject"
                      ~ Principal = {
                          - AWS = [
                              - "*",
                            ]
                        } -> "*"
                      ~ Resource  = [
                          - "arn:aws:s3:::outlinetest/avatars*",
                            "arn:aws:s3:::outlinetest/public*",
                          + "arn:aws:s3:::outlinetest/avatars*",
                        ]
                      + Sid       = ""
                        # (1 unchanged element hidden)
                    },
                ]
                # (1 unchanged element hidden)
            }
        )
        # (1 unchanged attribute hidden)
    }

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

@pjsier
Copy link
Collaborator

pjsier commented Dec 3, 2022

From taking a quick look at this, I'm able to reproduce the issue, and it looks like it might have something to do with how we're handling principal in minio_iam_policy_document. Supplying * as the principal causes the Minio server to set the principal to {"AWS": ["*"]}, which then fails the awspolicyequivalence check added in #180 because it's structurally different.

If anything fails in the policy equivalence check, the policy is set to the JSON document returned from the server which includes the reordered fields, which is why we're seeing that pop back up.

policy, err := secondJSONUnlessEquivalent(d.Get("policy").(string), actualPolicyText)

It looks like the real problem is the principal failing in minio_iam_policy_document because the test case in #180 using jsonencode passes. There might be other issues with that but I wasn't able to reproduce them. We could change this ticket to something about handling principals in policy documents potentially, what do you think @BuJo?

@Skaronator
Copy link

Skaronator commented Mar 12, 2024

I just had the same issue with my policy. I just change the order in my terraform code to avoid having a diff on every apply. It would be good if the provider just sorted this alphabetically internally so it doesn't matter which order the user / API provides it.

data "minio_iam_policy_document" "this" {
  dynamic "statement" {
    for_each = var.buckets
    content {
      sid = statement.value
      actions = [
        "s3:PutObject",   # <- changed order in this list
        "s3:ListBucket",
        "s3:GetObject",
        "s3:DeleteObject",
      ]
      resources = [
        "arn:aws:s3:::${statement.value}/*",  # <- swapped 
        "arn:aws:s3:::${statement.value}",    # <- these two
      ]
    }
  }
}

I briefly remember that I had a similar issue with the AWS provider a couple of years ago. I think it was the aws_ecr_repository_policy resource. They also just sorted it alphabetically to fix it.

@jbertozzi
Copy link

I am also facing this issue. I was not able to workaround by swapping the order my policy definition, no matter the order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants