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

[BC Break] Storage IAM #395

Merged
merged 8 commits into from
Mar 27, 2017
Merged

[BC Break] Storage IAM #395

merged 8 commits into from
Mar 27, 2017

Conversation

jdpedrie
Copy link
Contributor

@jdpedrie jdpedrie commented Mar 14, 2017

Closes #386.

Here is the implementation of IAM for GCS. It's a bit more involved than I expected. The storage IAM resources are just different enough to require modifications to the IAM system.

I also modified the Pub/Sub classes which implement IAM slightly, to delay creating the IAM objects until they're actually needed.

NOTE The return value of Iam::testPermissions() has changed. Where it previously returned the API result, now it only returns the value of the permissions key on that result, or an empty array.

todo

  • Full unit tests
  • Snippet tests
  • System tests

@jdpedrie jdpedrie added api: pubsub Issues related to the Pub/Sub API. api: storage Issues related to the Cloud Storage API. api: core iam do not merge Indicates a pull request not ready for merge, due to either quality or timing. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Mar 14, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 14, 2017
@jdpedrie
Copy link
Contributor Author

jdpedrie commented Mar 16, 2017

Blocked by storage whitelist.

@jdpedrie jdpedrie removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 25, 2017
@jdpedrie jdpedrie changed the title Storage IAM [BC Break[ Storage IAM Mar 25, 2017
@jdpedrie jdpedrie changed the title [BC Break[ Storage IAM [BC Break] Storage IAM Mar 25, 2017
Copy link
Contributor

@dwsupplee dwsupplee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you strikeout the note in the description about test permissions returning null since that has changed? The refactor we discussed LGTM. Just a few notes, otherwise consider this signed off. 👍

* @param array $options Configuration Options
* @return array An array of policy data
* @throws BadMethodCallException If the given policy is not an array or PolicyBuilder.

This comment was marked as spam.

}

if (!is_array($policy)) {
throw new \BadMethodCallException('Given policy data must be an array or an instance of PolicyBuilder.');

This comment was marked as spam.

@jdpedrie
Copy link
Contributor Author

@dwsupplee all set!

@jdpedrie jdpedrie merged commit bc1bdc3 into googleapis:master Mar 27, 2017
@jdpedrie jdpedrie mentioned this pull request Mar 27, 2017
@jdpedrie jdpedrie deleted the storage-iam branch June 9, 2017 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: core api: pubsub Issues related to the Pub/Sub API. api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement. iam type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants