From dc2ad55a9c2bb68e3fdfc9d0e2dea6ad3ac69423 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Wed, 20 Nov 2019 15:01:03 +0300 Subject: [PATCH 1/8] feat(bigtable): add table level IAM policy controls --- bigtable/docs/snippets_table.py | 55 +++++++++++++ bigtable/google/cloud/bigtable/table.py | 68 +++++++++++++++ bigtable/tests/unit/test_table.py | 105 ++++++++++++++++++++++++ 3 files changed, 228 insertions(+) diff --git a/bigtable/docs/snippets_table.py b/bigtable/docs/snippets_table.py index 0fbb16bf74ad..a19977b5f00d 100644 --- a/bigtable/docs/snippets_table.py +++ b/bigtable/docs/snippets_table.py @@ -356,6 +356,61 @@ def test_bigtable_get_cluster_states(): assert CLUSTER_ID in get_cluster_states +def test_bigtable_table_test_iam_permissions(): + table_policy = Config.INSTANCE.table("table_id_iam_policy") + table_policy.create() + assert table_policy.exists + + # [START bigtable_table_test_iam_permissions] + from google.cloud.bigtable import Client + + client = Client(admin=True) + table_admin_client = client.table_admin_client + + client = Client(admin=True) + instance = client.instance(INSTANCE_ID) + table = instance.table("table_id_iam_policy") + + permissions = ["bigtable.tables.mutateRows", "bigtable.tables.readRows"] + permissions_allowed = table.test_iam_permissions(permissions) + # [END bigtable_table_test_iam_permissions] + assert permissions_allowed == permissions + + +def test_bigtable_table_set_iam_policy_then_get_iam_policy(): + table_policy = Config.INSTANCE.table("table_id_iam_policy") + assert table_policy.exists + service_account_email = Config.CLIENT._credentials.service_account_email + + # [START bigtable_table_set_iam_policy] + from google.cloud.bigtable import Client + from google.cloud.bigtable.policy import Policy + from google.cloud.bigtable.policy import BIGTABLE_ADMIN_ROLE + + client = Client(admin=True) + instance = client.instance(INSTANCE_ID) + table = instance.table("table_id_iam_policy") + new_policy = Policy() + new_policy[BIGTABLE_ADMIN_ROLE] = [Policy.service_account(service_account_email)] + + policy_latest = table.set_iam_policy(new_policy) + # [END bigtable_table_set_iam_policy] + assert len(policy_latest.bigtable_admins) > 0 + + # [START bigtable_table_get_iam_policy] + from google.cloud.bigtable import Client + + client = Client(admin=True) + table_admin_client = client.table_admin_client + + client = Client(admin=True) + instance = client.instance(INSTANCE_ID) + table = instance.table("table_id_iam_policy") + policy = table.get_iam_policy() + # [END bigtable_table_get_iam_policy] + assert len(policy.bigtable_admins) > 0 + + def test_bigtable_table_exists(): # [START bigtable_check_table_exists] from google.cloud.bigtable import Client diff --git a/bigtable/google/cloud/bigtable/table.py b/bigtable/google/cloud/bigtable/table.py index 4ced9fbde0c2..c24dcb92b4a5 100644 --- a/bigtable/google/cloud/bigtable/table.py +++ b/bigtable/google/cloud/bigtable/table.py @@ -28,6 +28,7 @@ from google.cloud.bigtable.column_family import ColumnFamily from google.cloud.bigtable.batcher import MutationsBatcher from google.cloud.bigtable.batcher import FLUSH_COUNT, MAX_ROW_BYTES +from google.cloud.bigtable.policy import Policy from google.cloud.bigtable.row import AppendRow from google.cloud.bigtable.row import ConditionalRow from google.cloud.bigtable.row import DirectRow @@ -138,6 +139,73 @@ def name(self): project=project, instance=instance_id, table=self.table_id ) + def get_iam_policy(self): + """Gets the access control policy for a table. + + For example: + + .. literalinclude:: snippets_table.py + :start-after: [START bigtable_table_get_iam_policy] + :end-before: [END bigtable_table_get_iam_policy] + + :rtype: :class:`google.cloud.bigtable.policy.Policy` + :returns: The current IAM policy of this table. + """ + table_client = self._instance._client.table_admin_client + resp = table_client.get_iam_policy(resource=self.name) + return Policy.from_pb(resp) + + def set_iam_policy(self, policy): + """Sets the access control policy on a table. Replaces any + existing policy. + + For more information about policy, please see documentation of + class `google.cloud.bigtable.policy.Policy` + + For example: + + .. literalinclude:: snippets_table.py + :start-after: [START bigtable_table_set_iam_policy] + :end-before: [END bigtable_table_set_iam_policy] + + :type policy: :class:`google.cloud.bigtable.policy.Policy` + :param policy: A new IAM policy to replace the current IAM policy + of this table. + + :rtype: :class:`google.cloud.bigtable.policy.Policy` + :returns: The current IAM policy of this table. + """ + table_client = self._instance._client.table_admin_client + resp = table_client.set_iam_policy(resource=self.name, policy=policy.to_pb()) + return Policy.from_pb(resp) + + def test_iam_permissions(self, permissions): + """Returns permissions that the caller has on the specified table. + + For example: + + .. literalinclude:: snippets_table.py + :start-after: [START bigtable_table_test_iam_permissions] + :end-before: [END bigtable_table_test_iam_permissions] + + :type permissions: list + :param permissions: The set of permissions to check for + the ``resource``. Permissions with wildcards (such as '*' + or 'storage.*') are not allowed. For more information see + `IAM Overview + `_. + `Bigtable Permissions + `_. + + :rtype: list + :returns: A List(string) of permissions allowed on the table. + """ + table_client = self._instance._client.table_admin_client + resp = table_client.test_iam_permissions( + resource=self.name, permissions=permissions + ) + return list(resp.permissions) + def column_family(self, column_family_id, gc_rule=None): """Factory to create a column family associated with this table. diff --git a/bigtable/tests/unit/test_table.py b/bigtable/tests/unit/test_table.py index 495d8660d1f7..d4bb621c28c0 100644 --- a/bigtable/tests/unit/test_table.py +++ b/bigtable/tests/unit/test_table.py @@ -1048,6 +1048,111 @@ def test_mutations_batcher_factory(self): self.assertEqual(mutation_batcher.flush_count, flush_count) self.assertEqual(mutation_batcher.max_row_bytes, max_row_bytes) + def test_get_iam_policy(self): + from google.cloud.bigtable_admin_v2.gapic import bigtable_table_admin_client + from google.iam.v1 import policy_pb2 + from google.cloud.bigtable.policy import BIGTABLE_ADMIN_ROLE + + credentials = _make_credentials() + client = self._make_client( + project="project-id", credentials=credentials, admin=True + ) + instance = client.instance(instance_id=self.INSTANCE_ID) + table = self._make_one(self.TABLE_ID, instance) + + version = 1 + etag = b"etag_v1" + members = ["serviceAccount:service_acc1@test.com", "user:user1@test.com"] + bindings = [{"role": BIGTABLE_ADMIN_ROLE, "members": members}] + iam_policy = policy_pb2.Policy(version=version, etag=etag, bindings=bindings) + + table_api = mock.create_autospec( + bigtable_table_admin_client.BigtableTableAdminClient + ) + client._table_admin_client = table_api + table_api.get_iam_policy.return_value = iam_policy + + result = table.get_iam_policy() + + table_api.get_iam_policy.assert_called_once_with(resource=table.name) + self.assertEqual(result.version, version) + self.assertEqual(result.etag, etag) + admins = result.bigtable_admins + self.assertEqual(len(admins), len(members)) + for found, expected in zip(sorted(admins), sorted(members)): + self.assertEqual(found, expected) + + def test_set_iam_policy(self): + from google.cloud.bigtable_admin_v2.gapic import bigtable_table_admin_client + from google.iam.v1 import policy_pb2 + from google.cloud.bigtable.policy import Policy + from google.cloud.bigtable.policy import BIGTABLE_ADMIN_ROLE + + credentials = _make_credentials() + client = self._make_client( + project="project-id", credentials=credentials, admin=True + ) + instance = client.instance(instance_id=self.INSTANCE_ID) + table = self._make_one(self.TABLE_ID, instance) + + version = 1 + etag = b"etag_v1" + members = ["serviceAccount:service_acc1@test.com", "user:user1@test.com"] + bindings = [{"role": BIGTABLE_ADMIN_ROLE, "members": sorted(members)}] + iam_policy_pb = policy_pb2.Policy(version=version, etag=etag, bindings=bindings) + + table_api = mock.create_autospec( + bigtable_table_admin_client.BigtableTableAdminClient + ) + client._table_admin_client = table_api + table_api.set_iam_policy.return_value = iam_policy_pb + + iam_policy = Policy(etag=etag, version=version) + iam_policy[BIGTABLE_ADMIN_ROLE] = [ + Policy.user("user1@test.com"), + Policy.service_account("service_acc1@test.com"), + ] + + result = table.set_iam_policy(iam_policy) + + table_api.set_iam_policy.assert_called_once_with( + resource=table.name, policy=iam_policy_pb + ) + self.assertEqual(result.version, version) + self.assertEqual(result.etag, etag) + admins = result.bigtable_admins + self.assertEqual(len(admins), len(members)) + for found, expected in zip(sorted(admins), sorted(members)): + self.assertEqual(found, expected) + + def test_test_iam_permissions(self): + from google.cloud.bigtable_admin_v2.gapic import bigtable_table_admin_client + from google.iam.v1 import iam_policy_pb2 + + credentials = _make_credentials() + client = self._make_client( + project="project-id", credentials=credentials, admin=True + ) + instance = client.instance(instance_id=self.INSTANCE_ID) + table = self._make_one(self.TABLE_ID, instance) + + permissions = ["bigtable.tables.mutateRows", "bigtable.tables.readRows"] + + response = iam_policy_pb2.TestIamPermissionsResponse(permissions=permissions) + + table_api = mock.create_autospec( + bigtable_table_admin_client.BigtableTableAdminClient + ) + table_api.test_iam_permissions.return_value = response + client._table_admin_client = table_api + + result = table.test_iam_permissions(permissions) + + self.assertEqual(result, permissions) + table_api.test_iam_permissions.assert_called_once_with( + resource=table.name, permissions=permissions + ) + class Test__RetryableMutateRowsWorker(unittest.TestCase): from grpc import StatusCode From bf07a3c1216256172b1a1fef24425e44805f82d5 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Thu, 21 Nov 2019 11:46:51 +0300 Subject: [PATCH 2/8] remove extra lines --- bigtable/docs/snippets_table.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/bigtable/docs/snippets_table.py b/bigtable/docs/snippets_table.py index a19977b5f00d..702cf31b1447 100644 --- a/bigtable/docs/snippets_table.py +++ b/bigtable/docs/snippets_table.py @@ -364,9 +364,6 @@ def test_bigtable_table_test_iam_permissions(): # [START bigtable_table_test_iam_permissions] from google.cloud.bigtable import Client - client = Client(admin=True) - table_admin_client = client.table_admin_client - client = Client(admin=True) instance = client.instance(INSTANCE_ID) table = instance.table("table_id_iam_policy") @@ -400,9 +397,6 @@ def test_bigtable_table_set_iam_policy_then_get_iam_policy(): # [START bigtable_table_get_iam_policy] from google.cloud.bigtable import Client - client = Client(admin=True) - table_admin_client = client.table_admin_client - client = Client(admin=True) instance = client.instance(INSTANCE_ID) table = instance.table("table_id_iam_policy") From 6242d8c213a698b9f1e3bd78c4f59a0befed2765 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Thu, 21 Nov 2019 13:35:15 +0300 Subject: [PATCH 3/8] system test --- bigtable/tests/system.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/bigtable/tests/system.py b/bigtable/tests/system.py index ae43bb10ecdf..e083bed8d153 100644 --- a/bigtable/tests/system.py +++ b/bigtable/tests/system.py @@ -29,6 +29,8 @@ from google.cloud._helpers import UTC from google.cloud.bigtable.client import Client from google.cloud.bigtable.column_family import MaxVersionsGCRule +from google.cloud.bigtable.policy import Policy +from google.cloud.bigtable.policy import BIGTABLE_ADMIN_ROLE from google.cloud.bigtable.row_filters import ApplyLabelFilter from google.cloud.bigtable.row_filters import ColumnQualifierRegexFilter from google.cloud.bigtable.row_filters import RowFilterChain @@ -688,6 +690,31 @@ def test_create_table(self): sorted_tables = sorted(tables, key=name_attr) self.assertEqual(sorted_tables, expected_tables) + def test_test_iam_permissions(self): + temp_table_id = "test-test-iam-policy-table" + temp_table = Config.INSTANCE_DATA.table(temp_table_id) + temp_table.create() + self.tables_to_delete.append(temp_table) + + permissions = ["bigtable.tables.mutateRows", "bigtable.tables.readRows"] + permissions_allowed = temp_table.test_iam_permissions(permissions) + self.assertEqual(permissions, permissions_allowed) + + def test_set_iam_policy_then_get_iam_policy(self): + temp_table_id = "test-iam-policy-table" + temp_table = Config.INSTANCE_DATA.table(temp_table_id) + temp_table.create() + self.tables_to_delete.append(temp_table) + + new_policy = Policy() + new_policy[BIGTABLE_ADMIN_ROLE] = [Policy.service_account("test-email@gmail.com")] + + policy_latest = temp_table.set_iam_policy(new_policy) + self.assertTrue(len(policy_latest.bigtable_admins) > 0) + + policy = temp_table.get_iam_policy() + self.assertTrue(len(policy.bigtable_admins) > 0) + def test_create_table_with_families(self): temp_table_id = "test-create-table-with-failies" temp_table = Config.INSTANCE_DATA.table(temp_table_id) From 850fcef730c7096486bee5200924d00f53460ea8 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Thu, 21 Nov 2019 13:38:44 +0300 Subject: [PATCH 4/8] black --- bigtable/tests/system.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bigtable/tests/system.py b/bigtable/tests/system.py index e083bed8d153..47892e826b2b 100644 --- a/bigtable/tests/system.py +++ b/bigtable/tests/system.py @@ -707,7 +707,9 @@ def test_set_iam_policy_then_get_iam_policy(self): self.tables_to_delete.append(temp_table) new_policy = Policy() - new_policy[BIGTABLE_ADMIN_ROLE] = [Policy.service_account("test-email@gmail.com")] + new_policy[BIGTABLE_ADMIN_ROLE] = [ + Policy.service_account("test-email@gmail.com") + ] policy_latest = temp_table.set_iam_policy(new_policy) self.assertTrue(len(policy_latest.bigtable_admins) > 0) From c75e16cf60fa27337495f718fcd93b7c3f08f10f Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Thu, 21 Nov 2019 15:18:35 +0300 Subject: [PATCH 5/8] fix system test --- bigtable/tests/system.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bigtable/tests/system.py b/bigtable/tests/system.py index 47892e826b2b..81aae5e7377e 100644 --- a/bigtable/tests/system.py +++ b/bigtable/tests/system.py @@ -707,8 +707,9 @@ def test_set_iam_policy_then_get_iam_policy(self): self.tables_to_delete.append(temp_table) new_policy = Policy() + service_account_email = Config.CLIENT._credentials.service_account_email new_policy[BIGTABLE_ADMIN_ROLE] = [ - Policy.service_account("test-email@gmail.com") + Policy.service_account(service_account_email) ] policy_latest = temp_table.set_iam_policy(new_policy) From 7e47443592dd6cd141940500a5acc08035133081 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Thu, 21 Nov 2019 18:31:40 +0300 Subject: [PATCH 6/8] update system-tests --- bigtable/tests/system.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/bigtable/tests/system.py b/bigtable/tests/system.py index 81aae5e7377e..e9e3ab79179e 100644 --- a/bigtable/tests/system.py +++ b/bigtable/tests/system.py @@ -700,8 +700,18 @@ def test_test_iam_permissions(self): permissions_allowed = temp_table.test_iam_permissions(permissions) self.assertEqual(permissions, permissions_allowed) - def test_set_iam_policy_then_get_iam_policy(self): - temp_table_id = "test-iam-policy-table" + def test_get_iam_policy(self): + temp_table_id = "test-get-iam-policy-table" + temp_table = Config.INSTANCE_DATA.table(temp_table_id) + temp_table.create() + self.tables_to_delete.append(temp_table) + + policy = temp_table.get_iam_policy().to_api_repr() + self.assertEqual(policy["etag"], "ACAB") + self.assertEqual(policy["version"], 0) + + def test_set_iam_policy(self): + temp_table_id = "test-set-iam-policy-table" temp_table = Config.INSTANCE_DATA.table(temp_table_id) temp_table.create() self.tables_to_delete.append(temp_table) @@ -711,12 +721,10 @@ def test_set_iam_policy_then_get_iam_policy(self): new_policy[BIGTABLE_ADMIN_ROLE] = [ Policy.service_account(service_account_email) ] + policy_latest = temp_table.set_iam_policy(new_policy).to_api_repr() - policy_latest = temp_table.set_iam_policy(new_policy) - self.assertTrue(len(policy_latest.bigtable_admins) > 0) - - policy = temp_table.get_iam_policy() - self.assertTrue(len(policy.bigtable_admins) > 0) + self.assertEqual(policy_latest["bindings"][0]["role"], "roles/bigtable.admin") + self.assertIn(service_account_email, policy_latest["bindings"][0]["members"][0]) def test_create_table_with_families(self): temp_table_id = "test-create-table-with-failies" From d0baeadf40d694d20258da49d2a25ea1dd632f5c Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Fri, 22 Nov 2019 13:04:52 +0300 Subject: [PATCH 7/8] chg comment lines --- bigtable/google/cloud/bigtable/table.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bigtable/google/cloud/bigtable/table.py b/bigtable/google/cloud/bigtable/table.py index c24dcb92b4a5..9ea585fb1b3a 100644 --- a/bigtable/google/cloud/bigtable/table.py +++ b/bigtable/google/cloud/bigtable/table.py @@ -140,7 +140,7 @@ def name(self): ) def get_iam_policy(self): - """Gets the access control policy for a table. + """Gets the IAM access control policy for this table. For example: @@ -156,7 +156,7 @@ def get_iam_policy(self): return Policy.from_pb(resp) def set_iam_policy(self, policy): - """Sets the access control policy on a table. Replaces any + """Sets the IAM access control policy for this table. Replaces any existing policy. For more information about policy, please see documentation of @@ -180,7 +180,8 @@ class `google.cloud.bigtable.policy.Policy` return Policy.from_pb(resp) def test_iam_permissions(self, permissions): - """Returns permissions that the caller has on the specified table. + """Tests whether the caller has the given permissions for the specified table. + Returns the permissions that the caller has. For example: From 3ad4dfb7fefaf29077e5eaf2142755140e9d9597 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Tue, 26 Nov 2019 09:57:21 +0300 Subject: [PATCH 8/8] comment correction --- bigtable/google/cloud/bigtable/table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bigtable/google/cloud/bigtable/table.py b/bigtable/google/cloud/bigtable/table.py index 9ea585fb1b3a..69379b21d57e 100644 --- a/bigtable/google/cloud/bigtable/table.py +++ b/bigtable/google/cloud/bigtable/table.py @@ -180,7 +180,7 @@ class `google.cloud.bigtable.policy.Policy` return Policy.from_pb(resp) def test_iam_permissions(self, permissions): - """Tests whether the caller has the given permissions for the specified table. + """Tests whether the caller has the given permissions for this table. Returns the permissions that the caller has. For example: