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

provider/aws: aws_iam_role_policy add support for managed IAM policies #2100

Closed
zhukau opened this issue May 27, 2015 · 20 comments
Closed

provider/aws: aws_iam_role_policy add support for managed IAM policies #2100

zhukau opened this issue May 27, 2015 · 20 comments
Labels
enhancement provider/aws waiting-response An issue/pull request is waiting for a response from the community

Comments

@zhukau
Copy link
Contributor

zhukau commented May 27, 2015

Currently aws_iam_role_policy supports only attachment of inline policies to aws_iam_role. Recently Amazon introduced managed IAM policies and recommends using them over inline policies. It would be nice if Terraform supports both types of policies.

@takhyon
Copy link

takhyon commented May 27, 2015

👍

@catsby
Copy link
Contributor

catsby commented May 29, 2015

We do currently have a iam_policy resource. It seems we're missing something like this:

resource "aws_iam_role_policy" "test_policy" {
    name = "test_policy"
    role = "${aws_iam_role.test_role.id}"
    policy_arn = "${aws_iam_policy.policy.arn}"
}

resource "aws_iam_policy" "policy" {
    name = "test_policy"
    path = "/"
    description = "My test policy"
    policy = # omitted
}

resource "aws_iam_role" "test_role" {
    name = "test_role"
    assume_role_policy = # omitted
}

and then invoke AttachRolePolicy(documented here). Am I reading this correctly?

@catsby catsby added the waiting-response An issue/pull request is waiting for a response from the community label May 29, 2015
@failshell
Copy link

https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-roles-for-amazon-ec2.html

this page describes the process when doing it with the awscli toolkit.

@failshell
Copy link

Also, another interesting bit of information I found when debugging this earlier:

For AWS managed policies and customer managed policies, the policy's Version element must be set to 2012-10-17. For inline policies, the policy's Version element can be set to 2012-10-17 or to 2008-10-17. We recommend that you set the Version element to 2012-10-17 for all policies.

Found here: https://docs.aws.amazon.com/IAM/latest/UserGuide/policies_managed-vs-inline.html

Might be helpful to help you test.

@pgray
Copy link
Contributor

pgray commented Jun 8, 2015

I think this could be better achieved using similar logic to #2273

The association of a policy with a user/role/group could be implemented as:

resource "aws_iam_user" "testu" {
    name = "testu"
}
resource "aws_iam_role" "testr" {
    name = "testr"
    assume_role_policy = # omitted
}
resource "aws_iam_group" "testg" {
    name = "testg"
}
resource "aws_iam_policy" "policy" {
    name = "test_policy"
    path = "/"
    description = "My test policy"
    policy = # omitted
}

resource "aws_iam_policy_attach" "test_policy_attach" {
    name = "test_policy_assoc"
    user = "${aws_iam_user.testu.name}"   # Optional
    role = "${aws_iam_role.testr.name}"   # Optional
    group = "${aws_iam_group.testg.name}"   # Optional
    policy_arn = "${aws_iam_policy.policy.arn}"
}

The AWS api describes (user/role/group) as having almost identical functionality:

This would also simplify the syntax to one association per policy instead of requiring a refactor of either aws_iam_policy_(user/role/group). Makes it interesting to think of associations as resources, but this seems to be the best way right now according to #1419 and other issues.

Also, I think the mappings should be 1-to-many so that multiple users/roles/groups can be addressed with a single policy association.

@johnrengelman
Copy link
Contributor

The implementation of aws_iam_policy_attachment doesn't work in mixed-managed environments. It requires a single declaration of the policy_attachment with all the corresponding user, roles, and groups.
You can't declare multiple policy_attachment resources at different times to manage the attachment.

The simple case is that you have created an IAM Policy outside of Terraform and it is attached to roles and users that are not controller by Terraform. If you then use Terraform to create a role and attach the policy to it, it succeeds on the the first apply but subsequent plans show that Terraform will try to detach the unmanaged entities from the policy.

@pgray
Copy link
Contributor

pgray commented Jul 15, 2015

@johnrengelman have you tried aws_iam_group_membership to associate users with groups? If so, do you see the same problem there?

@johnrengelman
Copy link
Contributor

My use case is attaching a policy to a role.

@pgray
Copy link
Contributor

pgray commented Jul 15, 2015

Understood. I was asking because I referred to the source code of aws_iam_group_membership when I wrote this code. If that acts in the same way and it's determined to be a bug, I was hoping to work on the fix for both, unless the current functionality is determined to be acceptable/desired.

@johnrengelman
Copy link
Contributor

Yeah, I'm not doing an user<-> group control with Terraform, so I don't know.

@catsby
Copy link
Contributor

catsby commented Jul 22, 2015

Hello! Sorry for the delay. We did merge #2395 which claims to implement this behavior, so I'm going to close this issue now. Let me know if you believe that is in error, or have other questions.

Thanks for writing in and discussing!

@catsby catsby closed this as completed Jul 22, 2015
@johnrengelman
Copy link
Contributor

@catsby my comments are with respect to the implement in #2395.
The problem is that the current implementation requires the aws_iam_policy_attachment resource to model ALL the possible user/group/role attachments for that policy. That's not correct to do. It makes the resource useless as you cannot declare attachments for a single policy in different places (you can, but the behavior is not what you expected)

@sstarcher
Copy link

@johnrengelman This seems like a major problem. We had 2 different roles referencing the AWS ReadOnly policy. When TF detached that policy from the role it was managing it also detached the policy from another role not being reference in TF.

@jmccarty3 correct me if I'm wrong on what we saw.

@jmccarty3
Copy link

What we saw from the output was:

aws_iam_policy_attachment.kube-attach: Modifying...
  roles.#:          "2" => "1"
  roles.#omitted: "Non TF Managed Role" => ""
  roles.#omitted: "TF Managed Role" => "TF Managed Role"

This resulted in the behavior @sstarcher mentioned.

The statement that created the above log:

resource "aws_iam_policy_attachment" "kube-attach" {
  name = "KubeAttachment"
  roles = ["${TF Managed Role.name}"]
  policy_arn = "arn:aws:iam::aws:policy/ReadOnly"
}

@mrwilby
Copy link

mrwilby commented Nov 22, 2015

+1 - the current behavior for policy attachments is not very conducive to reuse and modularization in complex projects for the reasons stated above - the attachment has to be handled once and managed by terraform entirely.

For our scenario, we wanted to use terraform to attach ECS Service Role and ECS Instance Role to various bits of infra. We were surprised that some non-terraform-managed infrastructure kept losing these (standard AWS) policy attachments each time we destroyed our terraform-managed infra. Finally we tracked it down to this behavior and then the big warning comment added in the docs triggered our understanding.

@johnrengelman
Copy link
Contributor

There's nothing in the AWS API that's forcing this particular implementation, I highly suggest that an alternative implementation that is closer to aws_iam_role_policy be created. I had thought I'd be able to do it myself, but I don't think I will in a reasonable about of time (read as months)

@pgray
Copy link
Contributor

pgray commented Nov 23, 2015

Hi All. I've tried to get the functionality you're all requesting implemented in #4016

Feel free to pull down that branch and test if you'd like.

@jeffw-wherethebitsroam
Copy link

@johnrengelman let me know that this issue is addressed by my PR for issue #4165 which is also about the issues with aws_iam_policy_attachment in a complex modular environment.

@jonapich
Copy link

Can someone explain (or document) why this resource has a required "name" argument?

The common idiom for attachment resources (based on sns/lambda/dynamo attachments) is to provide source and target(s) and no name, since it doesn't really represent an AWS resource, justa logical link between resources.

@ghost
Copy link

ghost commented Apr 28, 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 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement provider/aws waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

No branches or pull requests