[Storage] Bucket-level IAM Samples by frankyn · Pull Request #2008 · googleapis/google-cloud-java

frankyn · 2017-04-26T18:46:11Z

Hi Garrett,

PR Summary:
This pull request will add the following samples to google-cloud-examples:
view_bucket_iam_members -- list bucket-level iam roles and their members
add_bucket_iam_member -- add a bucket-level iam member
remove_bucket_iam_member -- remove a bucket-level iam member

I copied the existing file BucketSnippets.java to keep consistency, and because ACL and Bucket-level IAM have a conflicting class name com.google.cloud.storage.Acl.Role and com.google.cloud.Role.

What additional work do I need to include in this PR to satisfy this line "This file is referenced in Storage's javadoc. Any change to this file should be reflected in Storage's javadoc.".?

Thank you!

coveralls · 2017-04-26T19:08:06Z

Changes Unknown when pulling 6ec2d3c on frankyn:storage-iam-snippets into ** on GoogleCloudPlatform:master**.

Sign in to view

+  /**
+   * Example of listing the Bucket-Level IAM Roles and Members
+   */
+


Sign in to view

+    Map<Role, Set<Identity>> policyBindings = policy.getBindings();
+    for(Map.Entry<Role, Set<Identity>> entry : policyBindings.entrySet()) {
+        System.out.printf("Role: %s", entry.getKey());
+        System.out.printf(" Identities: %s\n", entry.getValue());


Sign in to view

+    // Update the bucket IAM Policy
+    storage.setIamPolicy(bucketName, updatedPolicy);
+
+    System.out.printf("Added %s with role %s to %s\n", identity, role, bucketName);


Sign in to view

+/*
+ * EDITING INSTRUCTIONS
+ * This file is referenced in Storage's javadoc. Any change to this file should be reflected in
+ * Storage's javadoc.


frankyn · 2017-04-27T16:13:14Z

Thanks @shinfan! I have updated my PR to reflect your comments. PTAL

coveralls · 2017-04-27T16:33:04Z

Changes Unknown when pulling b21740b on frankyn:storage-iam-snippets into ** on GoogleCloudPlatform:master**.

coveralls · 2017-04-27T17:01:26Z

Changes Unknown when pulling 873b8ff on frankyn:storage-iam-snippets into ** on GoogleCloudPlatform:master**.

coveralls · 2017-04-27T18:21:19Z

Changes Unknown when pulling 8b49ff9 on frankyn:storage-iam-snippets into ** on GoogleCloudPlatform:master**.

shinfan

LGTM

frankyn · 2017-04-27T19:07:00Z

@shinfan do I need to wait for Garrett's LGTM as well or can I merge?

shinfan · 2017-04-27T19:07:27Z

@garrettjonesgoogle Do you wanna take another look?

garrettjonesgoogle · 2017-04-27T23:09:44Z

Adding @lesv for final review

lesv

The snippets are good, but you might wish to return updatedPolicy for the last two.

You might run into propagation issues with this, so your tests should be a bit less brittle.

Sign in to view

+      System.out.printf("Added %s with role %s to %s\n", identity, role, bucketName);
+    }
+    // [END add_bucket_iam_member]
+  }


Sign in to view

+      System.out.printf("Removed %s with role %s from %s\n", identity, role, bucketName);
+    }
+    // [END remove_bucket_iam_member]
+  }


Sign in to view

+  @Test
+  public void testAddBucketIamMemeber() {
+    // Test a member is added to Bucket-level IAM
+    Policy policy = storage.getIamPolicy(BUCKET);


Sign in to view

+
+  @Test
+  public void testRemoveBucketIamMember() {
+    // Test a member is removed from Bucket-level IAM


Sign in to view

+  @Test
+  public void testListBucketIamMembers() {
+    // Test an added Bucket-level IAM member is listed 
+    Policy policy = storage.getIamPolicy(BUCKET);


Sign in to view

+    assertNull(policy.getBindings().get(StorageRoles.admin()));
+    storage.setIamPolicy(BUCKET, policy.toBuilder().addIdentity(StorageRoles.admin(),
+            Identity.user(USER_EMAIL)).build());
+    policy = storage.getIamPolicy(BUCKET);


…etIamPolicy

coveralls · 2017-04-28T08:06:57Z

Changes Unknown when pulling 00555f3 on frankyn:storage-iam-snippets into ** on GoogleCloudPlatform:master**.

frankyn · 2017-04-28T08:14:22Z

Thanks @lesv, I have applied changes. PTAL

lesv

Sorry I wasn't thinking expressively yesterday.

What I would do is:

In your create bucket, I would add several policies, so all of them are done.
I would test myAddBucketIAM first
Then I run myRemoveBucketIAMMember final.
I would test myListBucketIAM.

Use lots of email addresses, not just one. Deal with the fact that you might not see yours.

You have a good chance to see the initial add's (setup) by the time you do your list. (but no guarantee).

The key point is Eventual Consistency -- I think it's a part of everything we deal with.

Sign in to view

+        Identity.user(USER_EMAIL)).build());
+    assertTrue(policy.getBindings().get(StorageRoles.admin()).contains(Identity.user(USER_EMAIL)));
+    Policy snippetPolicy = bucketIamSnippets.listBucketIamMembers(BUCKET);
+    assertTrue(snippetPolicy.getBindings().get(StorageRoles.admin()).


Sign in to view

+    Policy policy = storage.getIamPolicy(BUCKET);
+    policy = storage.setIamPolicy(BUCKET,
+        policy.toBuilder().removeRole(StorageRoles.admin()).build());
+    assertNull(policy.getBindings().get(StorageRoles.admin()));


Sign in to view

+    assertNull(policy.getBindings().get(StorageRoles.admin()));
+    policy = storage.setIamPolicy(BUCKET, policy.toBuilder().addIdentity(StorageRoles.admin(),
+        Identity.user(USER_EMAIL)).build());
+    assertTrue(policy.getBindings().get(StorageRoles.admin()).contains(Identity.user(USER_EMAIL)));


Sign in to view

+    Policy policy = storage.getIamPolicy(BUCKET);
+    policy = storage.setIamPolicy(BUCKET,
+        policy.toBuilder().removeRole(StorageRoles.admin()).build());
+    assertNull(policy.getBindings().get(StorageRoles.admin()));


Sign in to view

+    Policy policy = storage.getIamPolicy(BUCKET);
+    policy = storage.setIamPolicy(BUCKET,
+        policy.toBuilder().removeRole(StorageRoles.admin()).build());
+    assertNull(policy.getBindings().get(StorageRoles.admin()));


Sign in to view

+    assertNull(policy.getBindings().get(StorageRoles.admin()));
+    policy = storage.setIamPolicy(BUCKET, policy.toBuilder().addIdentity(StorageRoles.admin(),
+        Identity.user(USER_EMAIL)).build());
+    assertTrue(policy.getBindings().get(StorageRoles.admin()).contains(Identity.user(USER_EMAIL)));


lesv · 2017-04-29T00:10:55Z

I'm going to improve several of the tests by hand, then approve this PR as Frank had something come up and this needs to be published on Monday.

lesv

I think these might be more brittle than I'd like, but for now, it's working. I'll ask Frank to update later.

🤖 I have created a release *beep* *boop* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).

…e to v3.30.7 (#2008) Co-authored-by: Tom Andersen <[email protected]>

Frank Natividad added 2 commits April 26, 2017 01:59

Adding Bucket-level IAM snippets in google-cloud-examples

13973f1

Removing unwanted whitespace (tabs)..

6ec2d3c

frankyn added the android label Apr 26, 2017

frankyn requested a review from garrettjonesgoogle April 26, 2017 18:46

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 26, 2017

frankyn changed the title ~~Storage iam snippets~~ [Storage] Bucket-level IAM Samples Apr 26, 2017

frankyn added api: storage Issues related to the Cloud Storage API. and removed android labels Apr 26, 2017

garrettjonesgoogle requested a review from shinfan April 26, 2017 19:52

shinfan reviewed Apr 26, 2017

View reviewed changes

Removed unnecessary comment and verified post-operation

b21740b

Removing unused class imports and variables

873b8ff

frankyn force-pushed the storage-iam-snippets branch from b3753a3 to 873b8ff Compare April 27, 2017 16:40

Removing empty constructor

8b49ff9

shinfan approved these changes Apr 27, 2017

View reviewed changes

garrettjonesgoogle requested a review from lesv April 27, 2017 23:09

lesv suggested changes Apr 27, 2017

View reviewed changes

lesv reviewed Apr 28, 2017

View reviewed changes

Using snippet policy to verify correctness. Removed extra API calls g…

00555f3

…etIamPolicy

lesv suggested changes Apr 28, 2017

View reviewed changes

lesv approved these changes Apr 29, 2017

View reviewed changes

lesv merged commit f44ac0d into googleapis:master Apr 29, 2017

meltsufin pushed a commit that referenced this pull request May 2, 2026

chore(deps): update dependency com.google.cloud:google-cloud-firestor…

7f664c4

…e to v3.30.7 (#2008) Co-authored-by: Tom Andersen <[email protected]>

Conversation

frankyn commented Apr 26, 2017

Uh oh!

coveralls commented Apr 26, 2017

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

frankyn commented Apr 27, 2017

Uh oh!

coveralls commented Apr 27, 2017

Uh oh!

coveralls commented Apr 27, 2017

Uh oh!

coveralls commented Apr 27, 2017

Uh oh!

shinfan left a comment

Choose a reason for hiding this comment

Uh oh!

frankyn commented Apr 27, 2017

Uh oh!

shinfan commented Apr 27, 2017

Uh oh!

garrettjonesgoogle commented Apr 27, 2017

Uh oh!

lesv left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

coveralls commented Apr 28, 2017

Uh oh!

frankyn commented Apr 28, 2017

Uh oh!

lesv left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

lesv commented Apr 29, 2017

Uh oh!

lesv left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants