Testing: rationalize / normalize VPCSC environment detection in systests

[tseaver]

We need a clear pattern for how to test whether the appropriate environment variables are set for VPCSC, and ways to skip tests when they are missing. I believe the constraints should be:

• Never set the environment variables in noxfile.py.
• If the inside / outside project variables are missing, skip the affected testcases cleanly.

I'm proposing to create a new module, test_utils/test_utils/vpcsc_config.py, which centralizes all this policy. Usage from systests would look like:

from test_utils.vpcsc_config import vpcsc_config


@vpcsc_config.skip_unless_inside_project
def test_requiring_inside_project():
    do_something_with(vpcsc_config.project_inside)


@vpcsc_config.skip_unless_outside_project
def test_requiring_outside_project():
    do_something_with(vpcsc_config.project_outside)


@vpcsc_config.skip_unless_outside_bucket
def test_requiring_outside_bucket():
    do_something_with(vpcsc_config.bucket_outside)

@vpcsc_config.skip_unless_inside_project
@vpcsc_config.skip_unless_outside_project
def test_requiring_inside_and_outside_projects():
    if vpcsc_config.inside_vpcsc:
        do_something_with(vpcsc_config.project_inside, vpcsc_config.project_outside)

@vpcsc_config.skip_if_inside_vpcsc
def test_incompatible_with_vpcsc():
   ...


@vpcsc_config.skip_unless_inside_vpcsc
def test_requiring_requires_vpcsc():
   ...

Steps:

• Add 'test_utils/test_utils/vpcsc_config.py`, with the indicated 'VPCSCConfig' class and singleton (tests: add centralized VPCSC config detection #9581)
• Apply in asset/tests/system/test_vpcsc.py.(tests(asset): normalize VPCSC configuration in systests #9614)
• Apply in automl/tests/system/gapic/v1beta1/test_system_tables_client_v1.py. (tests(automl): normalize VPCSC configuration in systests #9607)
• Apply in dlp/tests/system/gapic/v2/test_system_dlp_service_v2_vpcsc.py. (tests(dlp): normalize VPCSC configuration in systests #9608)
• Apply in monitoring/tests/system/test_vpcsc_v3.py. (tests(monitoring): normalize VPCSC configuration in systests #9615)
• Apply in storage/tests/system.py. (tests(storage): normalize VPCSC configuration in systests. #9616)
• Apply in trace/tests/system/gapic/v{1,3}/test_system_trace_service_v{1,2}_vpcsc.py. (tests(trace): normalize VPCSC configuration in systests #9618)
• Appy lin translate/tests/system/test_vpcsc.py. (tests(translate): normalize VPCSC configuration in systests #9619)
• Appy lin videointelligence/tests/system.py. (tests(videointelligence): normalize VPCSC configuration in systests #9776)
• Appy lin vision/tests/system.py. (tests(vision): normalize VPCSC configuration in systests #9620)

[tseaver]

Ugh, I've just found out that different test suites differ as to how the GOOGLE_CLOUD_TESTS_IN_VPCSC env var should be handled if not set:

• asset defaults IS_INSIDE_VPCSC to True; it also forcibly sets the envvars in noxfile.py.
• automl defaults RUNNING_IN_VPCSC to False if the env var is missing.
• dlp defaults IS_INSIDE_VPCSC to False if the env var is missing; if False, its noxfile.py also scribbles on the other env vars.
• monitoring defaults IS_INSIDE_VPCSC to False if the env var is missing.; if False, its noxfile.py also scribbles on the other env vars.
• storage defaults RUNNING_IN_VPCSC to False if the env var is missing. If True, skips tests which use public buckets.
• trace doesn't use GOOGLE_CLOUD_TESTS_IN_VPCSC at all: it just tests for PROJECT_ID and GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT. Its noxfile.py forcibly sets both of those variables.
• translate sets IS_INSIDE_VPCSC to True if the GOOGLE_CLOUD_TESTS_IN_VPCSC is present in the environment, without testing its value!
• videointelligence sets IS_INSIDE_VPCSC to None if the env var is missing, and uses it as a boolean value (no test for "true" or "false"). It has separate tests for "inside" and "outside" buckets, which use separate env vars: those env vars aren't set by noxfile.py, also it does set the (seemingly unused) GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_IP and GOOGLE_CLOUD_TESTS_VPCSC_INSIDE_IP variables.
• vision doesn't use the GOOGLE_CLOUD_TESTS_IN_VPCSC env var at all: it uses two custom ones, GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT and GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_BUCKET.

[busunkim96]

👋 Ack, that is definitely a mess.

I wrote up this doc to clarify with VPCSC and backend folks as to what they need.

Tests should run only when the environment variable GOOGLE_CLOUD_TESTS_IN_VPCSC is set. This is set only in the VPC-SC team's test environment.

We can get rid of env var setup in the noxfile and run the tests depending on GOOGLE_CLOUD_TESTS_IN_VPCSC.

import os

IS_INSIDE_VPCSC = "GOOGLE_CLOUD_TESTS_IN_VPCSC" in os.environ

# If IS_INSIDE_VPCSC is set, these environment variables should also be set
if IS_INSIDE_VPCSC:
    PROJECT_INSIDE = os.environ["PROJECT_ID"]
    PROJECT_OUTSIDE = os.environ["GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT"]
    # Bucket, other resources if necessary

   @pytest.mark.skipif(
        not IS_INSIDE_VPCSC,
        reason="This test must be run in VPCSC. To enable this test, set the environment variable GOOGLE_CLOUD_TESTS_IN_VPCSC to True",
    )
    def test_create_alert_policy(self):
        # …
        # … 

GOOGLE_CLOUD_TESTS_IN_VPCSC	True in the VPC-SC Kokoro environment. Not set otherwise.
GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_BUCKET	A bucket outside the VPC-SC perimeter.
GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT	A project outside the VPC-SC perimeter.
PROJECT_ID	A project inside the VPC-SC perimeter.

[crwilcox]

Thanks for working towards this @tseaver. As @busunkim96 said, this is a bit of a mess right now, but I think the approach you are taking makes sense.

Do you have sufficient information to move forward on this work?

Also, in the PRs I see edits to noxfiles. Would it make sense to make those edits, at least eventually, in https://github.com/googleapis/synthtool/blob/master/synthtool/gcp/templates/python_library/noxfile.py.j2 ?

[tseaver]

@crwilcox I intend to add one PR per API which currently has any VPCSC-specific tests, and will remove any VPCSC-specific envvar-setting in those PRs.

Trying to get the noxfiles back to "pristine / as generated" is outside the scope of this issue.

[tseaver]

The VPCSC stuff in the videointelligence systests is a mess: it actually removed a real systest and replaced with bits which use requests to make raw HTTP requests, rather than testing the client.

I've asked to revert PR #9193 and #8607, and work on building in proper VPCSC support.

[tseaver]

The last PR for this issue is now open in the new python-videointelligence repo. Closing here, as bugs against the libraries aren't being managed here any longer.