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

Terraform aws s3 bucket policy are planned for change every time #4948

Closed
aldarund opened this issue Feb 2, 2016 · 29 comments
Closed

Terraform aws s3 bucket policy are planned for change every time #4948

aldarund opened this issue Feb 2, 2016 · 29 comments

Comments

@aldarund
Copy link

aldarund commented Feb 2, 2016

Every time i run plan and apply terraform says bucket policy is changed and needed to be applied. But its same policy all over again.

Path: /terraform/terraform.tfplan

~ aws_s3_bucket.rent_front_s3
    policy: "{\"Statement\":[{\"Action\":\"s3:GetObject\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Resource\":\"arn:aws:s3:::rent-front-static-staging/*\",\"Sid\":\"AddPerm\"}],\"Version\":\"2012-10-17\"}" => "{\"Statement\":[{\"Action\":[\"s3:GetObject\"],\"Effect\":\"Allow\",\"Principal\":\"*\",\"Resource\":[\"arn:aws:s3:::rent-front-static-staging/*\"],\"Sid\":\"AddPerm\"}],\"Version\":\"2012-10-17\"}"

@radeksimko
Copy link
Member

Hi @aldarund
this is a known issue with IAM policies across all IAM/policy-based resources.

Short-term solution is to always use string instead of array for Action and Resource values if these only contain one item.

Long-term solution

In ideal case we could get IAM JSON mappings from the AWS SDK and do the (un)marshalling easily, that's being discussed in aws/aws-sdk-go#127

Another, similar approach was proposed in #3124 where the mappings are part of the codebase.

Also there's #4278 which is following the same principal as the other linked PR.

@brikis98
Copy link
Contributor

brikis98 commented Mar 1, 2016

I ran across the same issue with an aws_s3_bucket resource where it would show up changed every time I ran terraform plan. @radeksimko's fix to make all the Action and Resource values strings instead of arrays was not enough to fix it. I also had to get the Principal value just right.

Here's what the aws_s3_bucket looked like before the fix:

resource "aws_s3_bucket" "my-bucket" {
  bucket = "my-bucket"

  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [{
    "Sid": "",
    "Action": ["s3:PutObject"],
    "Effect": "Allow",
    "Resource": ["my-resource"],
    "Principal": {
      "AWS": "1234567890"
    }
  }]
}
EOF
}

And here is the aws_s3_bucket after the fix:

resource "aws_s3_bucket" "my-bucket" {
  bucket = "my-bucket"

  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [{
    "Sid": "",
    "Action": "s3:PutObject",
    "Effect": "Allow",
    "Resource": "my-resource",
    "Principal": {
      "AWS": "arn:aws:iam::1234567890:root"
    }
  }]
}
EOF
}

Notice that Action and Resource are now strings instead of arrays and that the account id in the Principal entry is now surrounded with arn:aws:iam:: and :root.

@akumria
Copy link

akumria commented May 31, 2016

We also noticed that we had to make each Action separate as well.

@Ehekatl
Copy link

Ehekatl commented Jul 15, 2016

Quick Fix Here: I just found that the policy has to be exactly the same as you see it in web console, so first apply your change and copy the result from web console to terraform file

@mattupstate
Copy link

Specifically, for me, this occurs when wanting to specify multiple principals in the form of:

...
    "Principal": {
        "AWS": [
            ...
        ]
    }
...

@lra
Copy link

lra commented Jul 25, 2016

Yes, I don't see a workaround when we need to specify a list of principals, e.g.:

"Principal": {
    "AWS": [
        "arn:aws:iam::1234567890:root",
        "arn:aws:iam::2345678901:root",
        "arn:aws:iam::3456789012:root"
    ]
}

Any workaround is welcome. For now, we won't track those buckets with terraform.

@cornfeedhobo
Copy link

This is making me a sad panda, but I would like to confirm the workaround by @Ehekatl
My issue, for example, was needing to replace leading spaces with tabs, as AWS does.

@AMeng
Copy link
Contributor

AMeng commented Aug 26, 2016

@lra there is a workaround for that case. You need to completely remove all lists from the policy and instead split them into their own statements. For example:

{
    "Version": "2008-10-17",
    "Statement": [
        {
            "Sid": "",
            "Effect": "Allow",
            "Principal": {
                "AWS": [
                    "arn:aws:iam::XXXXXXXXXXXX:root",
                    "arn:aws:iam::YYYYYYYYYYYY:root"
                ]
            },
            "Action": "s3:*",
            "Resource": "arn:aws:s3:::ZZZZZZZZZ/*"
        }
    ]
}

... should change to this:

{
    "Version": "2008-10-17",
    "Statement": [
        {
            "Sid": "",
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::XXXXXXXXXXXX:root"
            },
            "Action": "s3:*",
            "Resource": "arn:aws:s3:::ZZZZZZZZZ/*"
        },
        {
            "Sid": "",
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::YYYYYYYYYYYY:root"
            },
            "Action": "s3:*",
            "Resource": "arn:aws:s3:::ZZZZZZZZZ/*"
        }
    ]
}

@stack72
Copy link
Contributor

stack72 commented Sep 3, 2016

Hi @aldarund

This has been fixed via #8615 :)

This will be released in 0.7.3. TL;DR - we are now able to test the structure of the AWS policy for similarities. This means that we know if a string equals the item in a single array etc. It will also understand the ordering of the blocks in a policy

Hope this helps

Paul

@stack72 stack72 closed this as completed Sep 3, 2016
@lra
Copy link

lra commented Sep 7, 2016

@stack72 looks like it's not working for me using 0.7.3 =(
I have a diff on each terraform refresh

I'll switch to @AMeng solution and split the statements.

@cread
Copy link

cread commented Sep 8, 2016

Even using the @AMeng solution on 0.7.3 does not work for me.

In my case though I've narrowed it down to using Conditions in the policy. If I remove the Condition then it works as expected.

@alonbecker
Copy link

Using a aws_iam_policy_document data resource with an s3 bucket policy and getting the same error and I am on 0.7.4

@grundic
Copy link

grundic commented Oct 24, 2016

I got the same problem, but advice from @Ehekatl helped: I figured out that "Version" was missing after copy/pasting from console.

@evanstachowiak
Copy link

I'm having the same issue in v0.7.13 and even removing the arrays doesn't seem to be helping.

@cread
Copy link

cread commented Dec 1, 2016

v0.7.13 is working well for me.

One thing I have changed though is to stop using the terrible policy = <<EOF pattern. Using aws_iam_policy_document is better in every way. There are good examples in https://www.terraform.io/docs/providers/aws/d/iam_policy_document.html.

@evanstachowiak
Copy link

I am also using an aws_s3_bucket_policy resource with a template_file

@syed-awais-ali
Copy link

I am importing the buckets into the state file using

terraform import aws_s3_bucket.bucketname bucketname

Then I create resources based on the state-file, when I run the plan I am expecting a empty plan because the resources that I generated from the state-file are same with same configurations, but the policy always seem to change although the policy is also same in the resource file and in the state-file but is the plan it shows that It is going to change the policy.

One more thing it does is It marks the buckets with attached polices to destroy and that is strange. If I have 13 buckets and 6 of them have policies Plans shows that 13 resources would be changed and 6 would be destroyed. Those 6 are also in the changed section and also showed in the mark to destroy section

@cpuspellcaster
Copy link

cpuspellcaster commented Dec 7, 2016

I'm encountering this problem as well with Terraform 0.7.13

data "aws_iam_policy_document" "REDACTED_access_logs" {
  statement {
    actions   = ["s3:PutObject"]
    effect    = "Allow"
    resources = ["arn:aws:s3:::REDACTED/AWSLogs/REDACTED/*"]

    principals {
      type        = "AWS"
      identifiers = ["797873946194"]
    }
  }
}

resource "aws_s3_bucket" "REDACTED_access_logs" {
  bucket = "REDACTED-access-logs"
  acl    = "private"
  policy = "${data.aws_iam_policy_document.REDACTED_access_logs.json}"

  tags {
    Name      = "REDACTED Access Logs"
    Terraform = "true"
  }
}

I'm using the aws_iam_policy_document method that @cread suggested earlier, but I'm still seeing the unnecessary change to the aws_s3_bucket resource during every plan/apply cycle.

Can this issue be re-opened, or should a new issue be created?

@cornfeedhobo
Copy link

cornfeedhobo commented Dec 7, 2016

@onoffleftright I am running 0.8.0-dev and with the aws_iam_policy_document am no longer seeing this issue. It might be best to make another issue and reference this one to make sure the 0.8.x releases at least fix the problem.

@jurajseffer
Copy link

This was happening to me on Terraform v0.8.2 but I've noticed that the policy document saved for the bucket in S3 is not exactly the same as my rendered template. I had

"Principal": {
  "AWS": "156460612806"
}

to enable alb access logs in eu-west-1. AWS API accepts that but turns it into

"Principal": {
  "AWS": "arn:aws:iam::156460612806:root"
}

and that's what's saved against the bucket so terraform will always show a change.

@evanstachowiak
Copy link

If I change my ELB bucket policy to what @jurajseffer suggests, terraform no longer updates the policy on every run.

@rafaelmagu
Copy link

Confirming that using the ARN instead of the account ID for an AWS Principal fixed my issues (I, too, was setting up an S3 policy for ELB logs).

@while1eq1
Copy link

Can confirm that ARN instead of account ID worked for me as well.

@ozbillwang
Copy link

ozbillwang commented May 31, 2017

I faced a similar issue, realized the sample code in below document is the right way to fix my issue.

If you are working with cloudfront, you should read this:

Updating your bucket policy

Note that the AWS API may translate the s3_canonical_user_id CanonicalUser principal into an AWS IAM ARN principal when supplied in an aws_s3_bucket bucket policy, causing spurious diffs in Terraform. If you see this behaviour, use the iam_arn instead:

https://www.terraform.io/docs/providers/aws/r/cloudfront_origin_access_identity.html#updating-your-bucket-policy

data "aws_iam_policy_document" "s3_policy" {
  statement {
    actions   = ["s3:GetObject"]
    resources = ["${module.names.s3_endpoint_arn_base}/*"]

    principals {
      type        = "AWS"
      identifiers = ["${aws_cloudfront_origin_access_identity.origin_access_identity.iam_arn}"]
    }
  }

  statement {
    actions   = ["s3:ListBucket"]
    resources = ["${module.names.s3_endpoint_arn_base}"]

    principals {
      type        = "AWS"
      identifiers = ["${aws_cloudfront_origin_access_identity.origin_access_identity.iam_arn}"]
    }
  }
}

resource "aws_s3_bucket" "bucket" {
  # ...
  policy = "${data.aws_iam_policy_document.s3_policy.json}"
}

@s0rc3r3r01
Copy link

I've had a similar problem with a valid policy in Terraform
"s3:x-amz-server-side-encryption": true
that was not maching with the quoted version:
"s3:x-amz-server-side-encryption": "true"
on AWS side

vandrijevik added a commit to tablexi/terraform_modules that referenced this issue May 16, 2018
Due to a Terraform bug (hashicorp/terraform#4948), if Action is set to a single-element array, the S3 policy always reports that it needs to be modified in place. The workaround is to use a string.
vandrijevik added a commit to tablexi/terraform_modules that referenced this issue May 16, 2018
Due to a Terraform bug (hashicorp/terraform#4948), if Principal is set to an account id, Terraform always reports the resource as if it needs changing (because the AWS API accepts the value, but turns it into an ARN).
vandrijevik added a commit to tablexi/terraform_modules that referenced this issue May 16, 2018
Due to a Terraform bug (hashicorp/terraform#4948), if Principal is set to an account id, Terraform always reports the resource as if it needs changing (because the AWS API accepts the value, but turns it into an ARN).
@jvelonis
Copy link

I was having this problem with Terraform 0.11.13, and none of the suggestions above helped. Turns out my aws_iam_policy_document contained depends_on = ["aws_s3_bucket.foo"] which I think I had added to work around a different problem. Removing this solved the issue with spurious changes.

@ykhli
Copy link

ykhli commented May 30, 2019

@jvelonis having the same problem. Do you know why adding depends_on in aws_iam_policy_document would introduce this problem?

@Wenzil
Copy link

Wenzil commented Jul 10, 2019

Using terraform 0.12 and having the issue with multiple Principals.

Update: After doing terraform apply once, I can't reproduce it

@ghost
Copy link

ghost commented Jul 24, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Jul 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests