From e353d5afdf853dd77dbb1e59b9dc028da0e27c1b Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 6 Nov 2019 12:24:24 -0500 Subject: [PATCH 1/4] tests(trace): do not set VPCSC env vars via nox --- trace/noxfile.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/trace/noxfile.py b/trace/noxfile.py index b6423e7bf20f..0f528b7f3902 100644 --- a/trace/noxfile.py +++ b/trace/noxfile.py @@ -121,19 +121,11 @@ 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", - } - # 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 fd8f85c21e219ff2b82665f92add9136c2c03858 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 6 Nov 2019 13:07:53 -0500 Subject: [PATCH 2/4] tests(trace): normalize VPCSC configuration in systests - Skip based on 'vpcsc_config.skip_unless_inside_vpcsc'. - Refactor VPCSC tests into one function per case, w/ expected exceptions. - Invoke client methods directly using 'vpcsc_config' to decode env vars. --- .../v1/test_system_trace_service_v1_vpcsc.py | 112 ++++++++---------- .../v2/test_system_trace_service_v2_vpcsc.py | 59 ++++----- 2 files changed, 71 insertions(+), 100 deletions(-) diff --git a/trace/tests/system/gapic/v1/test_system_trace_service_v1_vpcsc.py b/trace/tests/system/gapic/v1/test_system_trace_service_v1_vpcsc.py index d7dabcb24887..7a3436cccc5d 100644 --- a/trace/tests/system/gapic/v1/test_system_trace_service_v1_vpcsc.py +++ b/trace/tests/system/gapic/v1/test_system_trace_service_v1_vpcsc.py @@ -21,68 +21,52 @@ from google.api_core import exceptions from google.cloud import trace_v1 +from test_utils.vpcsc_config import vpcsc_config -PROJECT_INSIDE = os.environ.get("PROJECT_ID", None) -PROJECT_OUTSIDE = os.environ.get( - "GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT", None -) - - -class TestVPCServiceControlV1(object): - @staticmethod - def _is_rejected(call): - try: - responses = call() - except exceptions.PermissionDenied as e: - return e.message == "Request is prohibited by organization's policy" - except: - return False - return False - - @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_list_traces(self): - client = trace_v1.TraceServiceClient() - - list_inside = lambda: list(client.list_traces(PROJECT_INSIDE)) - list_outside = lambda: list(client.list_traces(PROJECT_OUTSIDE)) - - assert not TestVPCServiceControlV1._is_rejected(list_inside) - assert TestVPCServiceControlV1._is_rejected(list_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_get_trace(self): - client = trace_v1.TraceServiceClient() - - get_inside = lambda: client.get_trace(PROJECT_INSIDE, "") - get_outside = lambda: client.get_trace(PROJECT_OUTSIDE, "") - - assert not TestVPCServiceControlV1._is_rejected(get_inside) - assert TestVPCServiceControlV1._is_rejected(get_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_patch_traces(self): - client = trace_v1.TraceServiceClient() - - patch_inside = lambda: client.patch_traces(PROJECT_INSIDE, {}) - patch_outside = lambda: client.patch_traces(PROJECT_OUTSIDE, {}) - - assert not TestVPCServiceControlV1._is_rejected(patch_inside) - assert TestVPCServiceControlV1._is_rejected(patch_outside) +_VPCSC_PROHIBITED_MESSAGE = "Request is prohibited by organization's policy." + + +@pytest.fixture +def client(): + return trace_v1.TraceServiceClient() + + +@vpcsc_config.skip_unless_inside_vpcsc +def test_list_traces_w_inside(client): + list(client.list_traces(vpcsc_config.project_inside)) # no perms issue + + +@vpcsc_config.skip_unless_inside_vpcsc +def test_list_traces_w_outside(client): + with pytest.raises(exceptions.PermissionDenied) as exc: + list(client.list_traces(vpcsc_config.project_outside)) + + assert exc.value.message == _VPCSC_PROHIBITED_MESSAGE + + +@vpcsc_config.skip_unless_inside_vpcsc +def test_get_trace_w_inside(client): + with pytest.raises(exceptions.InvalidArgument): + client.get_trace(vpcsc_config.project_inside, "") # no perms issue + + +@vpcsc_config.skip_unless_inside_vpcsc +def test_get_trace_w_outside(client): + with pytest.raises(exceptions.PermissionDenied) as exc: + client.get_trace(vpcsc_config.project_outside, "") + + assert exc.value.message == _VPCSC_PROHIBITED_MESSAGE + + +@vpcsc_config.skip_unless_inside_vpcsc +def test_patch_traces_w_inside(client): + with pytest.raises(exceptions.InvalidArgument): + client.patch_traces(vpcsc_config.project_inside, {}) # no perms issue + + +@vpcsc_config.skip_unless_inside_vpcsc +def test_patch_traces_w_ouside(client): + with pytest.raises(exceptions.PermissionDenied) as exc: + client.patch_traces(vpcsc_config.project_outside, {}) + + assert exc.value.message == _VPCSC_PROHIBITED_MESSAGE diff --git a/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py b/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py index 2f34849c5929..205defd4a0a5 100644 --- a/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py +++ b/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py @@ -21,40 +21,27 @@ from google.api_core import exceptions from google.cloud import trace_v2 +from test_utils.vpcsc_config import vpcsc_config -PROJECT_INSIDE = os.environ.get("PROJECT_ID", None) -PROJECT_OUTSIDE = os.environ.get( - "GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT", None -) - - -class TestVPCServiceControlV2(object): - @staticmethod - def _is_rejected(call): - try: - responses = call() - except exceptions.PermissionDenied as e: - return e.message == "Request is prohibited by organization's policy" - except: - pass - return False - - @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_write_spans(self): - client = trace_v2.TraceServiceClient() - - proejct_inside = client.project_path(PROJECT_INSIDE) - proejct_outside = client.project_path(PROJECT_OUTSIDE) - spans = [] - - write_inside = lambda: client.batch_write_spans(proejct_inside, spans) - write_outside = lambda: client.batch_write_spans(proejct_outside, spans) - - assert not TestVPCServiceControlV2._is_rejected(write_inside) - assert TestVPCServiceControlV2._is_rejected(write_outside) +_VPCSC_PROHIBITED_MESSAGE = "Request is prohibited by organization's policy." + + +@pytest.fixture +def client(): + return trace_v2.TraceServiceClient() + + +@vpcsc_config.skip_unless_inside_vpcsc +def test_batch_write_spans_w_inside(client): + proejct_inside = client.project_path(vpcsc_config.project_inside) + client.batch_write_spans(proejct_inside, []) # no raise + + +@vpcsc_config.skip_unless_inside_vpcsc +def test_batch_write_spans_w_outside(client): + project_outside = client.project_path(vpcsc_config.project_outside) + + with pytest.raises(exceptions.PermissionDenied) as exc: + client.batch_write_spans(project_outside, []) + + assert exc.value.message == _VPCSC_PROHIBITED_MESSAGE From fc23b4c83d664fba40730c9ba2cea10388797cf0 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Nov 2019 13:14:04 -0500 Subject: [PATCH 3/4] fix: typo --- .../system/gapic/v2/test_system_trace_service_v2_vpcsc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py b/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py index 205defd4a0a5..a0f46a20b657 100644 --- a/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py +++ b/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py @@ -33,8 +33,8 @@ def client(): @vpcsc_config.skip_unless_inside_vpcsc def test_batch_write_spans_w_inside(client): - proejct_inside = client.project_path(vpcsc_config.project_inside) - client.batch_write_spans(proejct_inside, []) # no raise + project_inside = client.project_path(vpcsc_config.project_inside) + client.batch_write_spans(project_inside, []) # no raise @vpcsc_config.skip_unless_inside_vpcsc From e47f6657a099036339c3aa82bf385a275995cfc2 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Nov 2019 14:13:40 -0500 Subject: [PATCH 4/4] fix: review feedback --- .../system/gapic/v1/test_system_trace_service_v1_vpcsc.py | 6 +++--- .../system/gapic/v2/test_system_trace_service_v2_vpcsc.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/trace/tests/system/gapic/v1/test_system_trace_service_v1_vpcsc.py b/trace/tests/system/gapic/v1/test_system_trace_service_v1_vpcsc.py index 7a3436cccc5d..69aeaaf857f1 100644 --- a/trace/tests/system/gapic/v1/test_system_trace_service_v1_vpcsc.py +++ b/trace/tests/system/gapic/v1/test_system_trace_service_v1_vpcsc.py @@ -41,7 +41,7 @@ def test_list_traces_w_outside(client): with pytest.raises(exceptions.PermissionDenied) as exc: list(client.list_traces(vpcsc_config.project_outside)) - assert exc.value.message == _VPCSC_PROHIBITED_MESSAGE + assert _VPCSC_PROHIBITED_MESSAGE in exc.value.message @vpcsc_config.skip_unless_inside_vpcsc @@ -55,7 +55,7 @@ def test_get_trace_w_outside(client): with pytest.raises(exceptions.PermissionDenied) as exc: client.get_trace(vpcsc_config.project_outside, "") - assert exc.value.message == _VPCSC_PROHIBITED_MESSAGE + assert _VPCSC_PROHIBITED_MESSAGE in exc.value.message @vpcsc_config.skip_unless_inside_vpcsc @@ -69,4 +69,4 @@ def test_patch_traces_w_ouside(client): with pytest.raises(exceptions.PermissionDenied) as exc: client.patch_traces(vpcsc_config.project_outside, {}) - assert exc.value.message == _VPCSC_PROHIBITED_MESSAGE + assert _VPCSC_PROHIBITED_MESSAGE in exc.value.message diff --git a/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py b/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py index a0f46a20b657..19add1888a14 100644 --- a/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py +++ b/trace/tests/system/gapic/v2/test_system_trace_service_v2_vpcsc.py @@ -44,4 +44,4 @@ def test_batch_write_spans_w_outside(client): with pytest.raises(exceptions.PermissionDenied) as exc: client.batch_write_spans(project_outside, []) - assert exc.value.message == _VPCSC_PROHIBITED_MESSAGE + assert _VPCSC_PROHIBITED_MESSAGE in exc.value.message