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

aws_elasticsearch_domain access_policies keeps reapplying #3634

Closed
lancehudson opened this issue Oct 26, 2015 · 34 comments
Closed

aws_elasticsearch_domain access_policies keeps reapplying #3634

lancehudson opened this issue Oct 26, 2015 · 34 comments

Comments

@lancehudson
Copy link

I have

resource "aws_elasticsearch_domain" "es" {
  domain_name = "test"
  advanced_options {
    "rest.action.multi.allow_explicit_index" = true
  }
  access_policies = <<EOF
{
  "Statement": [
    {
      "Action": "es:*",
      "Condition": {
        "IpAddress": {
          "aws:SourceIp": [
            "${aws_instance.nat_a.public_ip}",
            "${aws_instance.nat_c.public_ip}"
          ]
        }
      },
      "Effect": "Allow",
      "Principal": {
        "AWS": "*"
      },
      "Resource": "arn:aws:es:${var.AWS_REGION}:${var.AWS_ACCOUNT_ID}:domain/test/*",
      "Sid": "ES"
    }
  ],
  "Version": "2012-10-17"
}
EOF
  ebs_options {
    ebs_enabled = true
    volume_type = "gp2"
    volume_size = 35
  }
  cluster_config {
    instance_type = "t2.small.elasticsearch"
    instance_count = 2
    dedicated_master_enabled = false
    zone_awareness_enabled = true
  }
  snapshot_options {
    automated_snapshot_start_hour = 23
  }
}

The access_policies section keeps reapplying on every run. I can verify in AWS that the correct policy is applied. I suspect it is this line master...lancehudson:patch-1

@lancehudson
Copy link
Author

it appears to be changing it from unsorted to sorted over and over, but I tried updating the policy to match the output here, and it still changes it.

access_policies: "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"ES\",
\"Effect\":\"Allow\",\"Principal\":{\"AWS\":\"*\"},\"Action\":\"es:*\",
\"Resource\":\"arn:aws:es:us-east-1:xxxx:domain/test/*\",
\"Condition\":{\"IpAddress\":{\"aws:SourceIp\":[\"127.0.0.1\",\"127.0.0.2\"]}}}]}" => 
"{\"Statement\":[{\"Action\":\"es:*\",\"Condition\":{\"IpAddress\":{\"aws:SourceIp\":
[\"127.0.0.1\",\"127.0.0.2\"]}},\"Effect\":\"Allow\",\"Principal\":{\"AWS\":\"*\"},
\"Resource\":\"arn:aws:es:us-east-1:xxxx:domain/test/*\",\"Sid\":\"ES\"}],
\"Version\":\"2012-10-17\"}"

@filiptepper
Copy link

Having the same issue here, tried the same solution.

@savingschampion
Copy link

Same issue here too

@maxbeatty
Copy link

I'm seeing the same issue. I tried moving the policy to a template_file but that didn't help

@radeksimko
Copy link
Member

This is a general problem with all IAM policies in AWS which keeps appearing in many different places, it's probably best to follow this thread:

#3124

TL;DR The issue is that we don't (de)serialise the JSON before/after sending it to AWS and AWS returns the JSON in a different format - either different ordering or list with 1 member instead of a string or visa versa.

I don't think we can/should be solving this on the resource level, I'm mentioning a potential solution in the linked thread #3124 (comment)

@radeksimko
Copy link
Member

#4245 has fixed some of the cases

@willejs
Copy link

willejs commented Jan 13, 2016

This is fixed now. You need to compare the proposed changes and you will realise amazon reformats the input you give it, just match the output from amazon to the correct format in your policy and its all ok.

@steveh
Copy link
Contributor

steveh commented Jan 13, 2016

@willejs fixed in HEAD or in a release? As of Terraform 0.6.9 it still changes each time, even after reformatting to match AWS output

@willejs
Copy link

willejs commented Jan 13, 2016

@steveh latest release.

@grahamjenson
Copy link

this still exists in Terraform v0.6.15

@szymonrychu
Copy link

Also exists in Terraform v0.6.16.. Please fix this.. :(

@aq1018
Copy link

aq1018 commented Jul 14, 2016

@willejs I think matching amazon output doesn't always work. For example, this policy allows user1, user2, user3 to access some-es-cluster :

{
  "Statement": [{
    "Action": "es:*",
    "Effect": "Allow",
    "Principal": {
      "AWS: [
        "arn:aws:iam::123456789012:user/user1",
        "arn:aws:iam::123456789012:user/user2",
        "arn:aws:iam::123456789012:user/user3"
      ]
    },
    "Resource": "arn:aws:es:us-east-1:123456789012:domain/some-es-cluster/*"
  }],
  "Version": "2012-10-17"
}

However, if you query the api repeatedly, you will notice the ordering of the users array is not deterministic. So, it's possible to get [user1, user2, user3], or [user3, user2, user1], or [user2, user3, user1], etc.

I'm not sure what' the best approach to solve this, but maybe we can parse the returned JSON and sort the array? ( Seems pretty hacky though... )

For now, I'm using:

lifecycle {
  ignore_changes  = ["access_policies"]
}

as a temporary workaround.

@dtolnay
Copy link
Contributor

dtolnay commented Jul 23, 2016

I have a fix in #7785.

@aerickson
Copy link

What's the status of this? I've seen some fixes landed, but terraform built from master still always shows changes when I run plan.

@stack72
Copy link
Contributor

stack72 commented Sep 7, 2016

Hi all

I believe that this has been fixed as of Terraform 0.7.3

We introduced a feature that will allow us to compare the structure of IAM Policy documents

The line:

"access_policies": &schema.Schema{
                Type:             schema.TypeString,
                DiffSuppressFunc: suppressEquivalentAwsPolicyDiffs,
                Optional:         true,
            },

Should be fixing anything that is considered equivalent. This is things like ordering, single array values as strings etc

Please let me know if this is not the case for you

Paul

@stack72 stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Sep 7, 2016
@sstarcher
Copy link

@stack72 this does not fix our

  • aws_sns_topic delivery_policy
  • aws_sqs_queue policy
  • aws_elasticsearch_domain access_policies

Here is our sns delivery_policy

{ "Version": "2008-10-17", "Id": "__default_policy_ID", "Statement": [ { "Sid": "__default_statement_ID", "Effect": "Allow", "Principal": { "AWS": "*" }, "Action": [ "SNS:Publish", "SNS:RemovePermission", "SNS:SetTopicAttributes", "SNS:DeleteTopic", "SNS:ListSubscriptionsByTopic", "SNS:GetTopicAttributes", "SNS:Receive", "SNS:AddPermission", "SNS:Subscribe" ], "Resource": "arn:aws:sns:us-east-1:112321321321:cloudtrail", "Condition": { "StringEquals": { "AWS:SourceOwner": "112321321321" } } }, { "Sid": "AWSCloudTrailSNSPolicy20141006", "Effect": "Allow", "Principal": { "AWS": [ "arn:aws:iam::112321321321:root", "arn:aws:iam::123132321321:root" ] }, "Action": "SNS:Publish", "Resource": "arn:aws:sns:us-east-1:112321321321:cloudtrail" } ] }

@sstarcher
Copy link

Just to verify

terraform --version
Terraform v0.7.3

@aerickson
Copy link

Terraform v0.7.3

aws_elasticsearch_domain access_policies are still being applied on each run.

@aerickson
Copy link

@stack72 can we remove the 'waiting-response' tag and possibly see some work on this bug?

@kwilczynski
Copy link
Contributor

kwilczynski commented Sep 26, 2016

@aerickson hi there! Are you suggesting you are going to spent some time looking into the issue
That is really amazing! I believe it would be much appreciated, very much so, especially since this is an Open Source project, and we welcome Pull Requests with unit tests and acceptance tests with open arms. Fixing a hard to fix issue is also a great way to help others from the community, and it is very rewarding. Thank you in advance!

Alternatively, I urge your to remain polite and vigilant of inappropriate comments, and have some patience. I am sure that this issue is going to be fixed in an upcoming releases, so stay put!

@apparentlymart
Copy link
Contributor

Hi all!

Thanks for the investigation and workarounds here. IAM policies have been an ongoing source of stress for Terraform due to each service normalizing them in a slightly different way. We recently added some mechanisms to be able to "normalize" diffs to take care of this sort of thing, but there hasn't yet been a specific change to address this ElasticSearch-specific weirdness, so this remains a problem as of this writing.

I'm going to close this out in favor of the later ticket #5067, because there's been further discussion and investigation on that one that could aid in resolution and I think it'd be best to consolidate discussion over there.

@apparentlymart apparentlymart removed the waiting-response An issue/pull request is waiting for a response from the community label Sep 30, 2016
@mnothic
Copy link

mnothic commented Jun 1, 2017

as workaround I use ingnore_changes for now:
lifecycle { ignore_changes = ["access_policies"] }

@ProTip
Copy link

ProTip commented Jul 1, 2017

This is closed buy I'll leave this comment here in case somebody runs across it having a sim issue:

If the principal is an account number the ES service will change it to this format:

arn:aws:iam::<account>:root

Change the policy in terraform or it will see it as different every time.

@radeksimko
Copy link
Member

@ProTip you're absolutely right, I did notice a few acc test failures in the past weeks. Oddly they were intermittent, but they don't affect just ES policy, but all policies.
We'll likely need to enhance the library dealing with IAM policy diffs.

The trouble is that we can't translate arn:aws:iam::<account-id>:role/test/tf_acc_test_jikdmyt92q to AROAI4PPNGEZ6AUO7UYBG nor the other way around without making an API call, which means we'd have to call iam:GetUser, iam:GetRole and possibly iam:GetGroup because the principal can be either of those and require this API call to be allowed when provisioning any resource which has a policy, which is really most of them. 😞

@apparentlymart
Copy link
Contributor

@radeksimko I think I'm not following your example there; is the API here really normalizing a specific role into the whole AWS account? That seems problematic, since it would grant broader access than otherwise and thus isn't semantically equivalent.

My read of @ProTip's comment made me think it was something more managable:

    "Principal": {"AWS": "1234567"}

...normalizes to...

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

If that's right then this at least should be possible to normalize without any further API calls. These two are documented as being equivalent in the Principal element reference, under "Specific AWS Accounts". There are some other similar things in this area that the library isn't covering right now AFAICT, such as normalizing "*" to {"AWS": "*"}, though that's likely irrelevant in this case since it'd be strange to make an ElasticSearch domain be open to every AWS account.

@radeksimko
Copy link
Member

@apparentlymart No, but it is flapping between ARN and "unique ID" of a role/user.

I think you just mentioned 3rd format the Principal accepts - AWS account ID and yes - that can be swapped for ARN ending in :root without API call, unlike unique ID.

The trouble is that people don't always grant access to the whole account (nor they should), but to a specific user or a role.

Check out this test failure from yesterday:

=== RUN   TestAccAWSSNSTopic_withIAMRole
--- FAIL: TestAccAWSSNSTopic_withIAMRole (16.68s)
    testing.go:428: Step 0 error: After applying this step, the plan was not empty:
        
        DIFF:
        
        UPDATE: aws_sns_topic.test_topic
          policy: "{\"Id\":\"Policy1445931846145\",\"Statement\":[{\"Action\":\"sns:Publish\",\"Effect\":\"Allow\",\"Principal\":{\"AWS\":\"AROAI4PPNGEZ6AUO7UYBG\"},\"Resource\":\"arn:aws:sns:us-west-2::example\",\"Sid\":\"Stmt1445931846145\"}],\"Version\":\"2012-10-17\"}" => "{\"Id\":\"Policy1445931846145\",\"Statement\":[{\"Action\":\"sns:Publish\",\"Effect\":\"Allow\",\"Principal\":{\"AWS\":\"arn:aws:iam::*******:role/test/tf_acc_test_jikdmyt92q\"},\"Resource\":\"arn:aws:sns:us-west-2::example\",\"Sid\":\"Stmt1445931846145\"}],\"Version\":\"2012-10-17\"}"
        

@apparentlymart
Copy link
Contributor

Ahh, okay... understood. Was missing that context, and indeed didn't even know that roles had ids like that. Fair enough!

I wonder if in this case we could convince AWS to consider this a bug; given that role unique ids and ARNs are so dissimilar, even putting Terraform's diffing needs aside it seems like a UX problem to replace something human-understandable (an ARN with the human-selected role name embedded) with something human-opaque that requires further API queries to understand.

@Puneeth-n
Copy link
Contributor

Puneeth-n commented Jul 4, 2017

@mnothic I don't think ignoring access_policies is a good solution! The work around I found was to see how terraform and aws described the policy during plan phase and copy the one reported by aws and make a note of it.

  1. do not pass any access policy in aws_elasticsearch_domain resource
  2. pass the aws reported policy in aws_elasticsearch_domain_policy resource

main.tf

resource "aws_elasticsearch_domain_policy" "main" {
    domain_name     = "${aws_elasticsearch_domain.bi-es.domain_name}"
    access_policies = "${chomp(data.template_file.es_access_policy.rendered)}"
}

template file

{
    "Version": "2012-10-17",
    "Statement": [{
        "Effect": "Allow",
        "Principal": {
            "AWS": [
                "${ec2_role}",
                "${lambda_role}"
                ]
            },
        "Action": "es:*",
        "Resource": "${es_resource}"
        }]
}

@rjinski
Copy link

rjinski commented Jul 19, 2017

Is this still being managed by #5067 ?

This thread seems like the latest conversation and I am still getting problems with terraform 'flip-flopping' using data.aws_iam_policy_document and aws_elasticsearch_domain with version 0.9.10.

I've used, the not ideal, lifecycle workaround but should this still be happening?

@Puneeth-n
Copy link
Contributor

@rjinski did you try my approach above

@rjinski
Copy link

rjinski commented Jul 20, 2017

@Puneeth-n No, mainly as I've been sticking with using data.aws_iam_policy_document rather than template files.

Happy to stick with lifecycle as a workaround but with the repository split I'm just hoping there is still some visibility/thoughts/plans to continue looking at these errors

@aterreno
Copy link

aterreno commented Oct 4, 2017

FYI Still happening on Terraform v0.10.7, best shot is what @Puneeth-n wrote here

@gdxhsw
Copy link

gdxhsw commented Nov 14, 2017

FYI v0.10.8 still has this issue.

@ghost
Copy link

ghost commented Apr 6, 2020

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 Apr 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.