From ffa95c407ce5b3d5f149fa304771571f71490408 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 6 Nov 2019 11:51:37 -0500 Subject: [PATCH 1/5] tests(asset): do not set VPCSC env vars via nox. --- asset/noxfile.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/asset/noxfile.py b/asset/noxfile.py index 393f4b64a33c..819d5e0797f6 100644 --- a/asset/noxfile.py +++ b/asset/noxfile.py @@ -118,13 +118,6 @@ def system(session): session.install("-e", "../test_utils/") session.install("-e", ".") - # Additional setup for VPCSC system tests - env = { - "PROJECT_ID": os.environ.get("PROJECT_ID"), - "GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT": "secure-gcp-test-project-4", - "GOOGLE_CLOUD_TESTS_IN_VPCSC": "true", - } - # Run py.test against the system tests. if system_test_exists: session.run("py.test", "--quiet", system_test_path, env=env, *session.posargs) From 7881b503991bfa48a53a5142e430758b9835ecdf Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 6 Nov 2019 11:52:53 -0500 Subject: [PATCH 2/5] tests(asset): all these tests must run inside VPCSC - Move 'skip_unless_inside_vpcsc' to class scope. - Drop never-reached branch for outside in '_do_test'. - Don't swallow unexpected exceptions inside '_is_rejected. --- asset/tests/system/test_vpcsc.py | 50 +++++++++----------------------- 1 file changed, 13 insertions(+), 37 deletions(-) diff --git a/asset/tests/system/test_vpcsc.py b/asset/tests/system/test_vpcsc.py index e3336bfe4f4f..8f883887dd7f 100644 --- a/asset/tests/system/test_vpcsc.py +++ b/asset/tests/system/test_vpcsc.py @@ -22,14 +22,10 @@ from google.api_core import exceptions from google.cloud import asset_v1 from google.cloud.asset_v1 import enums - -PROJECT_INSIDE = os.environ.get("PROJECT_ID", None) -PROJECT_OUTSIDE = os.environ.get( - "GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT", None -) -IS_INSIDE_VPCSC = os.environ.get("GOOGLE_CLOUD_TESTS_IN_VPCSC", "true") +from test_utils.vpcsc_config import vpcsc_config +@vpcsc_config.skip_unless_inside_vpcsc class TestVPCServiceControl(object): @staticmethod def _is_rejected(call): @@ -37,52 +33,32 @@ def _is_rejected(call): responses = call() except exceptions.PermissionDenied as e: return e.message == "Request is prohibited by organization's policy" - except: - pass return False - @staticmethod - def _do_test(delayed_inside, delayed_outside): - if IS_INSIDE_VPCSC.lower() == "true": - assert TestVPCServiceControl._is_rejected(delayed_outside) - assert not (TestVPCServiceControl._is_rejected(delayed_inside)) - else: - assert not (TestVPCServiceControl._is_rejected(delayed_outside)) - assert TestVPCServiceControl._is_rejected(delayed_inside) + def _do_test(self, delayed_inside, delayed_outside): + assert self._is_rejected(delayed_outside) + assert not (self._is_rejected(delayed_inside)) - @pytest.mark.skipif( - PROJECT_INSIDE is None, reason="Missing environment variable: PROJECT_ID" - ) - @pytest.mark.skipif( - PROJECT_OUTSIDE is None, - reason="Missing environment variable: GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT", - ) def test_export_assets(self): + bucket_uri = "gs:{}/g-c-p-export-test".format(vpcsc_config.bucket_outside) client = asset_v1.AssetServiceClient() - output_config = {} - parent_inside = "projects/" + PROJECT_INSIDE + output_config = {"gcsDestination": {"uri": bucket_uri}} + parent_inside = "projects/" + vpcsc_config.project_inside delayed_inside = lambda: client.export_assets(parent_inside, output_config) - parent_outside = "projects/" + PROJECT_OUTSIDE + parent_outside = "projects/" + vpcsc_config.project_outside delayed_outside = lambda: client.export_assets(parent_outside, output_config) - TestVPCServiceControl._do_test(delayed_inside, delayed_outside) + self._do_test(delayed_inside, delayed_outside) - @pytest.mark.skipif( - PROJECT_INSIDE is None, reason="Missing environment variable: PROJECT_ID" - ) - @pytest.mark.skipif( - PROJECT_OUTSIDE is None, - reason="Missing environment variable: GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT", - ) def test_batch_get_assets_history(self): client = asset_v1.AssetServiceClient() content_type = enums.ContentType.CONTENT_TYPE_UNSPECIFIED read_time_window = {} - parent_inside = "projects/" + PROJECT_INSIDE + parent_inside = "projects/" + vpcsc_config.project_inside delayed_inside = lambda: client.batch_get_assets_history( parent_inside, content_type, read_time_window ) - parent_outside = "projects/" + PROJECT_OUTSIDE + parent_outside = "projects/" + vpcsc_config.project_outside delayed_outside = lambda: client.batch_get_assets_history( parent_outside, content_type, read_time_window ) - TestVPCServiceControl._do_test(delayed_inside, delayed_outside) + self._do_test(delayed_inside, delayed_outside) From 681037ac53d2db212ac11723817ef4ba7f120492 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 6 Nov 2019 12:18:09 -0500 Subject: [PATCH 3/5] test: remove unused var. --- asset/noxfile.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/asset/noxfile.py b/asset/noxfile.py index 819d5e0797f6..3d92df19084f 100644 --- a/asset/noxfile.py +++ b/asset/noxfile.py @@ -120,11 +120,9 @@ def system(session): # Run py.test against the system tests. if system_test_exists: - session.run("py.test", "--quiet", system_test_path, env=env, *session.posargs) + session.run("py.test", "--quiet", system_test_path, *session.posargs) if system_test_folder_exists: - session.run( - "py.test", "--quiet", system_test_folder_path, env=env, *session.posargs - ) + session.run("py.test", "--quiet", system_test_folder_path, *session.posargs) @nox.session(python="3.7") From fb232d8d9a2bb56d91007bc61fe6ca64a7704c08 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Nov 2019 15:30:30 -0500 Subject: [PATCH 4/5] refactor: use one func per testcase --- asset/tests/system/test_vpcsc.py | 94 ++++++++++++++++++++------------ 1 file changed, 58 insertions(+), 36 deletions(-) diff --git a/asset/tests/system/test_vpcsc.py b/asset/tests/system/test_vpcsc.py index 8f883887dd7f..3480338693ff 100644 --- a/asset/tests/system/test_vpcsc.py +++ b/asset/tests/system/test_vpcsc.py @@ -24,41 +24,63 @@ from google.cloud.asset_v1 import enums from test_utils.vpcsc_config import vpcsc_config +_VPCSC_PROHIBITED_MESSAGE = "Request is prohibited by organization's policy" + + +@pytest.fixture +def client(): + return asset_v1.AssetServiceClient() + + +@pytest.fixture +def output_config(): + bucket_uri = "gs:{}/g-c-p-export-test".format(vpcsc_config.bucket_outside) + output_config = {"gcsDestination": {"uri": bucket_uri}} + + +@pytest.fixture +def parent_inside(): + return "projects/" + vpcsc_config.project_inside + + +@pytest.fixture +def parent_outside(): + return "projects/" + vpcsc_config.project_outside + + +@vpcsc_config.skip_unless_inside_vpcsc +def test_export_assets_inside(client, output_config, parent_inside): + with pytest.raises(exceptions.InvalidArgument): + client.export_assets(parent_inside, output_config) + + +@vpcsc_config.skip_unless_inside_vpcsc +def test_export_assets_outside(client, output_config, parent_outside): + with pytest.raises(exceptions.PermissionDenied) as exc: + client.export_assets(parent_outside, output_config) + + assert _VPCSC_PROHIBITED_MESSAGE in exc.value.message + @vpcsc_config.skip_unless_inside_vpcsc -class TestVPCServiceControl(object): - @staticmethod - def _is_rejected(call): - try: - responses = call() - except exceptions.PermissionDenied as e: - return e.message == "Request is prohibited by organization's policy" - return False - - def _do_test(self, delayed_inside, delayed_outside): - assert self._is_rejected(delayed_outside) - assert not (self._is_rejected(delayed_inside)) - - def test_export_assets(self): - bucket_uri = "gs:{}/g-c-p-export-test".format(vpcsc_config.bucket_outside) - client = asset_v1.AssetServiceClient() - output_config = {"gcsDestination": {"uri": bucket_uri}} - parent_inside = "projects/" + vpcsc_config.project_inside - delayed_inside = lambda: client.export_assets(parent_inside, output_config) - parent_outside = "projects/" + vpcsc_config.project_outside - delayed_outside = lambda: client.export_assets(parent_outside, output_config) - self._do_test(delayed_inside, delayed_outside) - - def test_batch_get_assets_history(self): - client = asset_v1.AssetServiceClient() - content_type = enums.ContentType.CONTENT_TYPE_UNSPECIFIED - read_time_window = {} - parent_inside = "projects/" + vpcsc_config.project_inside - delayed_inside = lambda: client.batch_get_assets_history( - parent_inside, content_type, read_time_window - ) - parent_outside = "projects/" + vpcsc_config.project_outside - delayed_outside = lambda: client.batch_get_assets_history( - parent_outside, content_type, read_time_window - ) - self._do_test(delayed_inside, delayed_outside) +def test_batch_get_assets_history_inside(client, parent_inside): + read_time_window = {} + client.batch_get_assets_history( + parent_inside, + content_type=enums.ContentType.CONTENT_TYPE_UNSPECIFIED, + read_time_window={}, + ) + + +@vpcsc_config.skip_unless_inside_vpcsc +def test_batch_get_assets_history_outside(client, parent_outside): + content_type = enums.ContentType.CONTENT_TYPE_UNSPECIFIED + read_time_window = {} + with pytest.raises(exceptions.PermissionDenied) as exc: + client.batch_get_assets_history( + parent_outside, + content_type=enums.ContentType.CONTENT_TYPE_UNSPECIFIED, + read_time_window={}, + ) + + assert _VPCSC_PROHIBITED_MESSAGE in exc.value.message From 600d1eb27a9352a8427dd1a703762a883dc19a99 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Nov 2019 17:13:11 -0500 Subject: [PATCH 5/5] fix: blacken --- asset/tests/system/test_vpcsc.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/asset/tests/system/test_vpcsc.py b/asset/tests/system/test_vpcsc.py index 3480338693ff..bfe9f8242e50 100644 --- a/asset/tests/system/test_vpcsc.py +++ b/asset/tests/system/test_vpcsc.py @@ -78,9 +78,9 @@ def test_batch_get_assets_history_outside(client, parent_outside): read_time_window = {} with pytest.raises(exceptions.PermissionDenied) as exc: client.batch_get_assets_history( - parent_outside, - content_type=enums.ContentType.CONTENT_TYPE_UNSPECIFIED, - read_time_window={}, - ) + parent_outside, + content_type=enums.ContentType.CONTENT_TYPE_UNSPECIFIED, + read_time_window={}, + ) assert _VPCSC_PROHIBITED_MESSAGE in exc.value.message