From 21cfae62a130f9206da3223a06e2e3befc78c826 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 23 Mar 2016 12:37:54 -0400 Subject: [PATCH] Add system tests for topic/subscription IAM policy get/set methods. --- docs/pubsub-usage.rst | 12 ++++----- gcloud/pubsub/iam.py | 12 ++++----- gcloud/pubsub/subscription.py | 3 ++- gcloud/pubsub/test_iam.py | 12 ++++----- gcloud/pubsub/test_subscription.py | 20 +++++++------- gcloud/pubsub/test_topic.py | 24 ++++++++--------- gcloud/pubsub/topic.py | 3 ++- system_tests/pubsub.py | 43 +++++++++++++++++++++++++++--- 8 files changed, 84 insertions(+), 45 deletions(-) diff --git a/docs/pubsub-usage.rst b/docs/pubsub-usage.rst index df58f7273ae6..2f74fd584120 100644 --- a/docs/pubsub-usage.rst +++ b/docs/pubsub-usage.rst @@ -99,12 +99,12 @@ Test permissions allowed by the current IAM policy on a topic: .. doctest:: >>> from gcloud import pubsub - >>> from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + >>> from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE >>> client = pubsub.Client() >>> topic = client.topic('topic_name') >>> allowed = topic.check_iam_permissions( - ... [READER_ROLE, WRITER_ROLE, OWNER_ROLE]) # API request - >>> allowed == [READER_ROLE, WRITER_ROLE] + ... [VIEWER_ROLE, EDITOR_ROLE, OWNER_ROLE]) # API request + >>> allowed == [VIEWER_ROLE, EDITOR_ROLE] True @@ -349,11 +349,11 @@ Test permissions allowed by the current IAM policy on a subscription: .. doctest:: >>> from gcloud import pubsub - >>> from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + >>> from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE >>> client = pubsub.Client() >>> topic = client.topic('topic_name') >>> subscription = topic.subscription('subscription_name') >>> allowed = subscription.check_iam_permissions( - ... [READER_ROLE, WRITER_ROLE, OWNER_ROLE]) # API request - >>> allowed == [READER_ROLE, WRITER_ROLE] + ... [VIEWER_ROLE, EDITOR_ROLE, OWNER_ROLE]) # API request + >>> allowed == [VIEWER_ROLE, EDITOR_ROLE] True diff --git a/gcloud/pubsub/iam.py b/gcloud/pubsub/iam.py index 1ead9d05ce92..064acbb48682 100644 --- a/gcloud/pubsub/iam.py +++ b/gcloud/pubsub/iam.py @@ -16,10 +16,10 @@ OWNER_ROLE = 'roles/owner' """IAM permission implying all rights to an object.""" -WRITER_ROLE = 'roles/writer' +EDITOR_ROLE = 'roles/editor' """IAM permission implying rights to modify an object.""" -READER_ROLE = 'roles/reader' +VIEWER_ROLE = 'roles/viewer' """IAM permission implying rights to access an object without modifying it.""" @@ -127,9 +127,9 @@ def from_api_repr(cls, resource): members = set(binding['members']) if role == OWNER_ROLE: policy.owners = members - elif role == WRITER_ROLE: + elif role == EDITOR_ROLE: policy.writers = members - elif role == READER_ROLE: + elif role == VIEWER_ROLE: policy.readers = members else: raise ValueError('Unknown role: %s' % (role,)) @@ -157,11 +157,11 @@ def to_api_repr(self): if self.writers: bindings.append( - {'role': WRITER_ROLE, 'members': sorted(self.writers)}) + {'role': EDITOR_ROLE, 'members': sorted(self.writers)}) if self.readers: bindings.append( - {'role': READER_ROLE, 'members': sorted(self.readers)}) + {'role': VIEWER_ROLE, 'members': sorted(self.readers)}) if bindings: resource['bindings'] = bindings diff --git a/gcloud/pubsub/subscription.py b/gcloud/pubsub/subscription.py index b9e5ef30834b..fc058c948347 100644 --- a/gcloud/pubsub/subscription.py +++ b/gcloud/pubsub/subscription.py @@ -305,8 +305,9 @@ def set_iam_policy(self, policy, client=None): client = self._require_client(client) path = '%s:setIamPolicy' % (self.path,) resource = policy.to_api_repr() + wrapped = {'policy': resource} resp = client.connection.api_request( - method='POST', path=path, data=resource) + method='POST', path=path, data=wrapped) return Policy.from_api_repr(resp) def check_iam_permissions(self, permissions, client=None): diff --git a/gcloud/pubsub/test_iam.py b/gcloud/pubsub/test_iam.py index d6a6e165e715..dfd0087d8ba0 100644 --- a/gcloud/pubsub/test_iam.py +++ b/gcloud/pubsub/test_iam.py @@ -87,7 +87,7 @@ def test_from_api_repr_only_etag(self): self.assertEqual(list(policy.readers), []) def test_from_api_repr_complete(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'user:phred@example.com' OWNER2 = 'group:cloud-logs@google.com' WRITER1 = 'domain:google.com' @@ -99,8 +99,8 @@ def test_from_api_repr_complete(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } klass = self._getTargetClass() @@ -134,7 +134,7 @@ def test_to_api_repr_only_etag(self): self.assertEqual(policy.to_api_repr(), {'etag': 'DEADBEEF'}) def test_to_api_repr_full(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'group:cloud-logs@google.com' OWNER2 = 'user:phred@example.com' WRITER1 = 'domain:google.com' @@ -146,8 +146,8 @@ def test_to_api_repr_full(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } policy = self._makeOne('DEADBEEF', 17) diff --git a/gcloud/pubsub/test_subscription.py b/gcloud/pubsub/test_subscription.py index d4d3b2746e89..de1ac8764e28 100644 --- a/gcloud/pubsub/test_subscription.py +++ b/gcloud/pubsub/test_subscription.py @@ -485,7 +485,7 @@ def test_delete_w_alternate_client(self): self.assertEqual(req['path'], '/%s' % SUB_PATH) def test_get_iam_policy_w_bound_client(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'user:phred@example.com' OWNER2 = 'group:cloud-logs@google.com' WRITER1 = 'domain:google.com' @@ -497,8 +497,8 @@ def test_get_iam_policy_w_bound_client(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } PROJECT = 'PROJECT' @@ -557,7 +557,7 @@ def test_get_iam_policy_w_alternate_client(self): self.assertEqual(req['path'], '/%s' % PATH) def test_set_iam_policy_w_bound_client(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE from gcloud.pubsub.iam import Policy OWNER1 = 'group:cloud-logs@google.com' OWNER2 = 'user:phred@example.com' @@ -570,8 +570,8 @@ def test_set_iam_policy_w_bound_client(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } RESPONSE = POLICY.copy() @@ -607,7 +607,7 @@ def test_set_iam_policy_w_bound_client(self): req = conn._requested[0] self.assertEqual(req['method'], 'POST') self.assertEqual(req['path'], '/%s' % PATH) - self.assertEqual(req['data'], POLICY) + self.assertEqual(req['data'], {'policy': POLICY}) def test_set_iam_policy_w_alternate_client(self): from gcloud.pubsub.iam import Policy @@ -639,7 +639,7 @@ def test_set_iam_policy_w_alternate_client(self): req = conn2._requested[0] self.assertEqual(req['method'], 'POST') self.assertEqual(req['path'], '/%s' % PATH) - self.assertEqual(req['data'], {}) + self.assertEqual(req['data'], {'policy': {}}) def test_check_iam_permissions_w_bound_client(self): PROJECT = 'PROJECT' @@ -647,7 +647,7 @@ def test_check_iam_permissions_w_bound_client(self): SUB_NAME = 'sub_name' PATH = 'projects/%s/subscriptions/%s:testIamPermissions' % ( PROJECT, SUB_NAME) - ROLES = ['roles/reader', 'roles/writer', 'roles/owner'] + ROLES = ['roles/viewer', 'roles/editor', 'roles/owner'] REQUESTED = { 'permissions': ROLES, } @@ -674,7 +674,7 @@ def test_check_iam_permissions_w_alternate_client(self): SUB_NAME = 'sub_name' PATH = 'projects/%s/subscriptions/%s:testIamPermissions' % ( PROJECT, SUB_NAME) - ROLES = ['roles/reader', 'roles/writer', 'roles/owner'] + ROLES = ['roles/viewer', 'roles/editor', 'roles/owner'] REQUESTED = { 'permissions': ROLES, } diff --git a/gcloud/pubsub/test_topic.py b/gcloud/pubsub/test_topic.py index de52cddc1c7f..96830c789bb6 100644 --- a/gcloud/pubsub/test_topic.py +++ b/gcloud/pubsub/test_topic.py @@ -453,7 +453,7 @@ def test_list_subscriptions_missing_key(self): self.assertEqual(req['query_params'], {}) def test_get_iam_policy_w_bound_client(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'user:phred@example.com' OWNER2 = 'group:cloud-logs@google.com' WRITER1 = 'domain:google.com' @@ -465,8 +465,8 @@ def test_get_iam_policy_w_bound_client(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } TOPIC_NAME = 'topic_name' @@ -522,7 +522,7 @@ def test_get_iam_policy_w_alternate_client(self): def test_set_iam_policy_w_bound_client(self): from gcloud.pubsub.iam import Policy - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'group:cloud-logs@google.com' OWNER2 = 'user:phred@example.com' WRITER1 = 'domain:google.com' @@ -534,8 +534,8 @@ def test_set_iam_policy_w_bound_client(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } RESPONSE = POLICY.copy() @@ -569,7 +569,7 @@ def test_set_iam_policy_w_bound_client(self): req = conn._requested[0] self.assertEqual(req['method'], 'POST') self.assertEqual(req['path'], '/%s' % PATH) - self.assertEqual(req['data'], POLICY) + self.assertEqual(req['data'], {'policy': POLICY}) def test_set_iam_policy_w_alternate_client(self): from gcloud.pubsub.iam import Policy @@ -599,15 +599,15 @@ def test_set_iam_policy_w_alternate_client(self): req = conn2._requested[0] self.assertEqual(req['method'], 'POST') self.assertEqual(req['path'], '/%s' % PATH) - self.assertEqual(req['data'], {}) + self.assertEqual(req['data'], {'policy': {}}) def test_check_iam_permissions_w_bound_client(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE TOPIC_NAME = 'topic_name' PROJECT = 'PROJECT' PATH = 'projects/%s/topics/%s:testIamPermissions' % ( PROJECT, TOPIC_NAME) - ROLES = [READER_ROLE, WRITER_ROLE, OWNER_ROLE] + ROLES = [VIEWER_ROLE, EDITOR_ROLE, OWNER_ROLE] REQUESTED = { 'permissions': ROLES, } @@ -628,12 +628,12 @@ def test_check_iam_permissions_w_bound_client(self): self.assertEqual(req['data'], REQUESTED) def test_check_iam_permissions_w_alternate_client(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE TOPIC_NAME = 'topic_name' PROJECT = 'PROJECT' PATH = 'projects/%s/topics/%s:testIamPermissions' % ( PROJECT, TOPIC_NAME) - ROLES = [READER_ROLE, WRITER_ROLE, OWNER_ROLE] + ROLES = [VIEWER_ROLE, EDITOR_ROLE, OWNER_ROLE] REQUESTED = { 'permissions': ROLES, } diff --git a/gcloud/pubsub/topic.py b/gcloud/pubsub/topic.py index 859a84c93d29..5e442afeeb59 100644 --- a/gcloud/pubsub/topic.py +++ b/gcloud/pubsub/topic.py @@ -299,8 +299,9 @@ def set_iam_policy(self, policy, client=None): client = self._require_client(client) path = '%s:setIamPolicy' % (self.path,) resource = policy.to_api_repr() + wrapped = {'policy': resource} resp = client.connection.api_request( - method='POST', path=path, data=resource) + method='POST', path=path, data=wrapped) return Policy.from_api_repr(resp) def check_iam_permissions(self, permissions, client=None): diff --git a/system_tests/pubsub.py b/system_tests/pubsub.py index 956a788c6d36..cce3e0118d04 100644 --- a/system_tests/pubsub.py +++ b/system_tests/pubsub.py @@ -89,7 +89,7 @@ def test_create_subscription_defaults(self): self.assertFalse(topic.exists()) topic.create() self.to_delete.append(topic) - SUBSCRIPTION_NAME = 'subscribing-now' + SUBSCRIPTION_NAME = 'subscribing-now-%d' % (1000 * time.time(),) subscription = topic.subscription(SUBSCRIPTION_NAME) self.assertFalse(subscription.exists()) subscription.create() @@ -103,7 +103,7 @@ def test_create_subscription_w_ack_deadline(self): self.assertFalse(topic.exists()) topic.create() self.to_delete.append(topic) - SUBSCRIPTION_NAME = 'subscribing-now' + SUBSCRIPTION_NAME = 'subscribing-now-%d' % (1000 * time.time(),) subscription = topic.subscription(SUBSCRIPTION_NAME, ack_deadline=120) self.assertFalse(subscription.exists()) subscription.create() @@ -142,7 +142,7 @@ def test_message_pull_mode_e2e(self): self.assertFalse(topic.exists()) topic.create() self.to_delete.append(topic) - SUBSCRIPTION_NAME = 'subscribing-now' + SUBSCRIPTION_NAME = 'subscribing-now-%d' % (1000 * time.time(),) subscription = topic.subscription(SUBSCRIPTION_NAME) self.assertFalse(subscription.exists()) subscription.create() @@ -168,3 +168,40 @@ def _by_timestamp(message): self.assertEqual(message1.attributes['extra'], EXTRA_1) self.assertEqual(message2.data, MESSAGE_2) self.assertEqual(message2.attributes['extra'], EXTRA_2) + + def test_topic_iam_policy(self): + topic_name = 'test-topic-iam-policy-topic-%d' % (1000 * time.time(),) + topic = Config.CLIENT.topic(topic_name) + topic.create() + count = 5 + while count > 0 and not topic.exists(): + time.sleep(1) + count -= 1 + self.to_delete.append(topic) + policy = topic.get_iam_policy() + policy.readers.add(policy.user('jjg@google.com')) + new_policy = topic.set_iam_policy(policy) + self.assertEqual(new_policy.readers, policy.readers) + + def test_subscription_iam_policy(self): + topic_name = 'test-sub-iam-policy-topic-%d' % (1000 * time.time(),) + topic = Config.CLIENT.topic(topic_name) + topic.create() + count = 5 + while count > 0 and not topic.exists(): + time.sleep(1) + count -= 1 + self.assertTrue(topic.exists()) + self.to_delete.append(topic) + SUB_NAME = 'test-sub-iam-policy-sub-%d' % (1000 * time.time(),) + subscription = topic.subscription(SUB_NAME) + subscription.create() + count = 5 + while count > 0 and not subscription.exists(): + time.sleep(1) + count -= 1 + self.to_delete.insert(0, subscription) + policy = subscription.get_iam_policy() + policy.readers.add(policy.user('jjg@google.com')) + new_policy = subscription.set_iam_policy(policy) + self.assertEqual(new_policy.readers, policy.readers)