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

Unrelated changes are made during push #13

Closed
simpsora opened this issue Apr 14, 2016 · 13 comments
Closed

Unrelated changes are made during push #13

simpsora opened this issue Apr 14, 2016 · 13 comments

Comments

@simpsora
Copy link

simpsora commented Apr 14, 2016

I made a change to a role, and in addition to the desired change, iamy also output the following change:

        aws iam update-assume-role-policy --role-name cloudwatch-alarm-deleter-role --policy-document '{
          "Statement": [
            {
              "Action": "sts:AssumeRole",
              "Effect": "Allow",
              "Principal": {
                "Service": [
                  "lambda.amazonaws.com",
                  "sns.amazonaws.com"
                ]
              },
              "Sid": ""
            }
          ],
          "Version": "2012-10-17"
        }'

This change is completely unrelated to the previous change. When allow iamy to apply the change, it applies cleanly. When I do a subsequent pull, I see that the order of the two entries in the Service array above has been reversed:

$ git diff iamy/.../role/cloudwatch-alarm-deleter-role.yaml
--- a/iamy/.../role/cloudwatch-alarm-deleter-role.yaml
+++ b/iamy/./role/cloudwatch-alarm-deleter-role.yaml
@@ -4,8 +4,8 @@ AssumeRolePolicyDocument:
     Effect: Allow
     Principal:
       Service:
-      - lambda.amazonaws.com
       - sns.amazonaws.com
+      - lambda.amazonaws.com
     Sid: ""
   Version: 2012-10-17
 InlinePolicies:

This behavior is repeatable, and although it's not always the above role that gets updated, it is always just a re-order of array entries.

I've tried both v1.0.2, and the dev version that go get gives me; both exhibit the same behavior.

Great tool by the way! Makes IAM management so much easier.

Cheers,
Ross

@simpsora
Copy link
Author

Update: I notice that the above behavior occurs every time I run iamy pull! It reorders the items in that array every single time.

@mtibben
Copy link
Member

mtibben commented Apr 15, 2016

Hey @simpsora, what order do you get when querying the data from aws directly via aws iam get-role-policy?

@simpsora
Copy link
Author

Hi @mtibben,

I get the following, repeatably -- sns is always first, followed by lambda:

$ aws iam get-role --role-name cloudwatch-alarm-deleter-role
{
    "Role": {
        "AssumeRolePolicyDocument": {
            "Version": "2012-10-17",
            "Statement": [
                {
                    "Action": "sts:AssumeRole",
                    "Principal": {
                        "Service": [
                            "sns.amazonaws.com",
                            "lambda.amazonaws.com"
                        ]
                    },
                    "Effect": "Allow",
                    "Sid": ""
                }
            ]
        },
        "RoleId": "...",
        "CreateDate": "2015-09-15T05:17:33Z",
        "RoleName": "cloudwatch-alarm-deleter-role",
        "Path": "/",
        "Arn": "..."
    }
}

@mtibben
Copy link
Member

mtibben commented Apr 15, 2016

Yep - so the issue is with AWS. We give AWS something in a particular order, but they return it in another. I'm not sure how this could be easily worked around

@simpsora
Copy link
Author

@mtibben of course, that makes sense. Terraform (which we used before iamy) solves this problem by running the AWS output through a JSON normalizer before doing anything with it. I'm aware that this is a different project, but that approach may be worth considering.

Here's an example of what I mean, there are certainly more but this was the first one I could find:
hashicorp/terraform#4278
See normalizeJson and normalizePolicyDocument.

@simpsora
Copy link
Author

simpsora commented May 3, 2016

@mtibben Would you be amenable to a PR to fix this (that would just normalize the order of array elements before sending and receiving)?

@mtibben
Copy link
Member

mtibben commented May 10, 2016

@simpsora sure, absolutely

@mtibben
Copy link
Member

mtibben commented Sep 1, 2016

Hey @simpsora I think I've addressed this in #17. Give v2.0.0-beta2 a try, let me know if it's fixed for you

@simpsora
Copy link
Author

simpsora commented Sep 1, 2016

@mtibben wonderful news! Just gave it a quick try, and works as promised. Will try it in anger tomorrow with real account data and let you know for sure.

Thanks for tackling this!

@simpsora
Copy link
Author

simpsora commented Sep 1, 2016

By the bye, one thing that appears to have changed is default region handling. Previously I had AWS_DEFAULT_REGION set, and iamy worked fine. Now, same environment, I get the following error:

$ ~/bin/iamy-darwin-amd64-2.0.0-beta2 pull
Error fetching S3 data: Error listing buckets: Error while calling ListBuckets: MissingRegion: could not find region configuration130 <nil>
exit status 1

If I set AWS_REGION instead, it all works peachily. Unsure if that's due to the use of S3, or an upgraded AWS dependency, or something like that.

Happy to create an issue if you'd like.

@mtibben
Copy link
Member

mtibben commented Sep 1, 2016

I'd say that's due to the AWS Go SDK - actually I believe I submitted this very issue with them last year! aws/aws-sdk-go#384

@mtibben
Copy link
Member

mtibben commented Sep 1, 2016

Also looks like terraform are still dealing with the same issue hashicorp/terraform#8350

@simpsora
Copy link
Author

simpsora commented Sep 2, 2016

@mtibben: We've used the new version on a couple of different accounts and it's looking great! Thanks again for knocking this out.

Regarding the region issue: that ticket suggests that a fix was merged in early August. Do you know if the pre-release iamy build contains their fix and it doesn't work, or if it hasn't made it in yet?

It's not a big deal, although the error message could be a bit clearer ("could not find region configuration130 " is a bit odd).

In any case, thanks again! :)

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

No branches or pull requests

2 participants