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

Proposal: IAM Meta Interface #2031

Closed
wants to merge 2 commits into from
Closed

Conversation

elibixby
Copy link

Definition of an IAM meta interface to be used for resources which have implemented the IAM meta api

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 27, 2016
@elibixby
Copy link
Author

Note: I left the contributor usage section blank because I want to (try to) avoid thinking about it until the user interface is solid.

@dhermes
Copy link
Contributor

dhermes commented Jul 28, 2016

Nit: rename as iam-usage.rst

Using IAM For Users
===================

This document does not explain how Google Cloud IAM works, for that, please check out `the docs <https://cloud.google.com/iam/docs/>`_.

This comment was marked as spam.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Jul 28, 2016
@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Jul 28, 2016
Policy Editing Methods
----------------------

>>> resource.add_roles(iam.user('alice@example.com'), iam.roles.OWNER.name, iam.roles.EDITOR.name)

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@daspecster
Copy link
Contributor

This is going to be really awesome! Thanks for your work @elibixby!

@elibixby
Copy link
Author

I can push a skeleton implementation if someone gives this surface the thumbs up...

@elibixby
Copy link
Author

@daspecster Ping on this? Does this seem like a reasonable surface?


``set_policy`` takes a policy dictionary, as well as optional ``version`` and ``etag`` paramters. If updates are made to your policy during this change, they will be overwritten with exactly what is in your policy, or, if an etag is specified they will fail with a ``iam.ConcurrentModificationError``. ``iam.PolicyChange`` performs this "read-modify-write" cycle automatically for the user.

>>> policy['roles/owner'].add('user:charles@example.com')

This comment was marked as spam.

This comment was marked as spam.

~~~~~~~
An IAM member is one of the following:

- ``iam.user(email)`` an individual Google account

This comment was marked as spam.

@elibixby
Copy link
Author

@dhermes On talking with @jonparrott I think I'm going to scale back the initial pull request to just get_policy and set_policy (as well as the ancillary member and Role stuff). Then we can discuss additional sugar in future pull requests.

I think get_policy and set_policy returning the policy, version, etag , and policy being a dict of sets is enough of a usability improvement that it's worth rolling out to the supporting APIs first.

The interface for contributors depends on the implementation details, which I will (try to) avoid thinking about until after the user interface has solidified

fixed all caps

Alternate usage of PolicyChange

Typo and note fix

Yet another typo

iam.rst -> iam-usage.rst

Parentheses are annoying

PolicyChange and *_members same si

Roles in iam instead of Resource

1. This is a bit of future proofing. Right now there is a Role resource
https://cloud.google.com/iam/reference/rest/v1/roles/queryGrantableRoles#Role
That defines a role, but can only be listed as roles are only
curated right now. In the future this will be a fully fledged
resource with users able to define custom roles, and I anticipate them
needing a resource class of their own. Hence wrapping the enumeration
of curated roles in this resource (and needing a .name)

2. The same role can be used across multiple resources, and because
of custom roles there's no sense in adding any "sanity check"
code to the mapping between resources and roles. We'll let the IAM
API take care of this for us, and so just move IAM Curated Roles
into the central iam module.

Add misc methods

remove_fn as seperate function

Added clarification of failure Exception

Also note about retry kwarg

Add version kwarg

Also, make clear that PolicyChange stores state

Sequences instead of *args

misc methods on resource not in iam

Reworked into "Resources" and "Methods" sections

Formatting errors

Remove sugar

Replace gcloud with python
e.g:
>>> from gcloud import pubsub
>>> client = pubsub.Client()
>>> resource = client.Topic('my-topic')

This comment was marked as spam.

@theacodes
Copy link
Contributor

@elibixby the smaller surface is looking way easier to swallow.

``set_policy`` takes one positional, and three keyword arguments:

- The first argument is a policy dictionary, described above.
- ``version`` is an optional integer. If ommited the version of the policy will be set to 0

This comment was marked as spam.

This comment was marked as spam.

@ncouture
Copy link

ncouture commented Jan 1, 2017

@elibixby can I help you with this PR?

Last update was in August and looking at the discussion it seems like there are only small fixes in the documentation that remains to be fixed.

Can someone confirm the state of this PR, or of general IAM functionality for this project?

@elibixby
Copy link
Author

@ncouture This PR was ready for review and merge... It was only a proposal and I wanted confirmation on the surface before I went ahead an implemented since I have had really poor review experiences on this repo before. However it looks like it was neglected for long enough they ignored it and did their own thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants