Google-cloud-python: Plan To Implement: IAM Mixin for resources to provide idempotent policy updates

[elibixby]

Before I go ahead and do a bunch of work I wanted feedback on a proposed implementation

Since IAM is a meta-api, we can be sure that all One Platform resource types will implement a common interface for IAM, there is smoothing that can be done client side for this common interface. Namely, properly executing the flow of getIAMPolicy -> edit policy -> setIAMPolicy with etag is non-trivial, and common between all resources.

To implement the IAM Mixin a resource would simply provide _get_iam_policy and _set_iam_policy methods, that the mixin would use to implement _edit_iam_policy an idempotent policy update, which would then serve as the basis for add_role(self, member, role) and remove_role(self, member or regex, role), the external surface provided by the mixin.

I am using something like this in an internal project and it would hopefully be fast to implement. But wanted to gather as much feedback as possible on this.

Other things that I haven't implemented but could potentially be provided are query_grantable_rolls and test_iam_permissions. Not sure what the use of these would be but they are included in the IAM API. One possibility would be a @requires_rolls decorator for resource functions that would call test_iam_permissions before making API calls to prevent 403s but I'm not sure how useful that would be.

[dhermes]

We create a usage doc before writing any good. You could start by just providing some usage snippets / examples here. (TDD --> example / interface driven dev)

[elibixby]

Sure. I'm using something like this for an app I have which manages 3rd party projects

The distilled usage is something like.

from gcloud import resource_manager
client = resource_manager.Client() // Client uses 1-st party service account credentials

def create_user_callback(user_id, user_email):
    """ Called when a user creates an account """
    project = client.new_project('managed-project-{}'.format(user_id))
    project.add_roll('roles/owner', 'user:{}'.format(user_email))

def delete_user_callback(user_id, user_email):
    """ Called when a user deletes their account """
    project = client.fetch_project('managed-project-{}'.format(user_id))
    project.remove_roll('roles/owner', 'user:{}'.format(user_email) // prevent user from undeleting

Obviously the same use case applies to pretty much any resource, for users that are wrapping our services on behalf of customers, or want to use our built in ACLs for any reason.

This represents more or less what I've already built. Obviously the method signatures are a little rough, and could be cleaned up by:

• wrapping "member" strings as types... so resource.add_roll('roles/owner', 'user:{}'.format(user_email)) becomes resource.add_roll('roles/owner', iam.User(user_email))
• having implementing resources provide enums for the possible roles.
  so resource.add_roll('roles/owner', 'user:{}'.format(user_email)) becomes resource.add_roll(resource.OWNER, 'user:{}'.format(user_email))

Not sure how I feel about both of these, but open to input.

[elibixby]

So, the other way that this works, which I think is less elegant. Is to flip it on it's head like this:

user = iam.User(user_email)
project = client.fetch_project('managed-project-{}'.format(user_id))
user.add_roll(project, project.OWNER)

Then iam.User.add_roll checks for the existence of a _set_iam_policy and _get_iam_policy in project resource, and the heavy lifting of idempotent updates is done in something like iam._BaseIAMMember.

I think this is a little disingenuous as the ACLs are tracked relative to the resources, and not relative to the members, so I prefer the first approach. But wanted to throw this out there as something I had considered.

[theacodes]

add_roll

Why not add_barrel_roll?

[elibixby]

Also, worth discussing: The ability to fetch ACL information.

>>> resource.get_policy()
{'roles/owners': [ 'user:[email protected]', 'group:[email protected]']}

Or with wrapping:

>>> resource.get_policy()
{resource.OWNER: [<iam.User>, <iam.Group>]}

Similarly

>>> resource.get_members(resource.ROLE)
[<iam.User>, <iam.Group>]

etc.

[elibixby]

Thanks to Jon's snarky gif I just realized I should have been typing role instead of roll... so: s/roll/role everywhere I was discussing interface...

[elibixby]

One thing that is slightly weird, is where the line between oauth2client and gcloud-python is. Since e.g. wrapping service_account:appengine-default@<myprojectid>.iam.google.com with a new object seems strange when a GoogleCredentials object exists.

This would be an argument against wrapping the {member_type}:{member email} strings

Maybe we could construct them by providing an enum from iam. So usage would be:

resource.add_role(resource.OWNER, iam.USER, user_email)

and iam would provide enums for the member types (user, service account, group, etc).

This seems like the right solution.

[elibixby]

woops

[elibixby]

Also, something that I ended up wanting in my use case, was the ability to specify a regex for remove_role, so I could remove all members of a certain type (remove_role(resource.OWNER, iam.USER, '*')), remove all the members of a certain domain (remove_role(resource.OWNER, iam.USER, '*@google.com')) etc..

and also, the ability to remove multiple roles from a single user e.g. remove_roles([resource.OWNER, resource.BILLING_ADMINISTRATOR], iam.USER, '[email protected]")

Ultimately there's a lot of ways to slice it. I managed all the slices by having each helper method construct an "update_dict" of the form {role: {'add': members_to_add, 'remove': members_to_remove}, ... }, not sure if exposing that on the mixin and letting resources implement their own helper functions would be a better idea?

[daspecster]

@elibixby, this looks like great work! We usually create a new RST file in the docs or extend one as might be applicable in your case, to outline the implementation. Then we can comment on the lines in the rst file PR to talk about specific topics.

[elibixby]

@daspecster Thanks! I'll do that

So, possible solution to the "Lots of ways to slice it" problem, might look something like this

resource.update_policy(
    resource.role(roles.OWNER)
                  .add(iam.USER, alice_email)
                  .remove(iam.GROUP, group_email),
    resource.role(roles.EDITOR)
                  .add(iam.USER, bob_email))

Is this crazy?
Maybe?

[theacodes]

Is this crazy?

Not crazy, but could perhaps be a bit more pythonic.

Maybe take the simplest case first:

resource.update_iam_policy(role='owner', add=alice_email)

Then support multiple targets

resource.update_iam_policy(role='owner', add=[alice_email, group_email'])

Then multiple operations

resource.update_iam_policy(role='owner', add=alice_email, remove=group_email)

The support the most advanced case of multiple mutations on different roles:

change1 = iam.PolicyChange(role='owner', add=[alice_email, group_email])
change2 = iam.PolicyChange(role='editor', remove=bob_email)
resource.update_iam_policy(change1, change2)

[elibixby]

Actually I like this. Even better would be

resource.update_policy(
    resource.owners().add(iam.USER, alice_email).remove(iam.GROUP, group_email),
    resource.editors().add(iam.USER, bob_email))

and then in e.g. resource_manager we have:

class Project(Resource):
   def __init__(self, ...):
      self.editors = iam.Role('roles/editor')
      self.owners = iam.Role('roles/owners')

[elibixby]

Awww damn. Jon and I double commented and I like his interface better.

However, I think 'owner' needs to be part of a resource specific method or enum, since roles aren't consistent across resources.

[theacodes]

That's fine, food for thought.

[elibixby]

Alright, I have an idea of what I want things to look like now. Gonna write up an RST.

[elibixby]

described in #2031