Skip to content

Testing: rationalize / normalize VPCSC environment detection in systests #9580

Closed
Closed
Testing: rationalize / normalize VPCSC environment detection in systests#9580
Assignees
tseaver
Labels
api: automlIssues related to the AutoML API.api: cloudassetIssues related to the Cloud Asset Inventory API.api: cloudtraceIssues related to the Cloud Trace API.api: dlpIssues related to the Sensitive Data Protection API.api: monitoringIssues related to the Cloud Monitoring API.api: storageIssues related to the Cloud Storage API.api: translationIssues related to the Cloud Translation API API.api: videointelligenceIssues related to the Video Intelligence API API.api: visionIssues related to the Cloud Vision API.testingtype: processA process-related concern. May include testing, release, or the like.

Description

@tseaver
tseaver
opened on Oct 31, 2019

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:

Activity

  1. tseaver
    added
    api: storageIssues related to the Cloud Storage API.
    testing
    api: monitoringIssues related to the Cloud Monitoring API.
    api: translationIssues related to the Cloud Translation API API.
    api: videointelligenceIssues related to the Video Intelligence API API.
    type: processA process-related concern. May include testing, release, or the like.
    api: dlpIssues related to the Sensitive Data Protection API.
    api: automlIssues related to the AutoML API.
    api: cloudassetIssues related to the Cloud Asset Inventory API.
    on Oct 31, 2019
  2. tseaver
    self-assigned this
    on Oct 31, 2019
  3. tseaver
    added
    api: cloudtraceIssues related to the Cloud Trace API.
    api: visionIssues related to the Cloud Vision API.
    on Oct 31, 2019

  4. tseaver commented on Oct 31, 2019

    @tseaver
    tseaver
    on Oct 31, 2019
    ContributorAuthor

    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.

  5. busunkim96 commented on Oct 31, 2019

    @busunkim96
    busunkim96
    on Oct 31, 2019
    Contributor

    👋 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.

  6. crwilcox commented on Nov 1, 2019

    @crwilcox
    crwilcox
    on Nov 1, 2019
    Contributor

    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 ?

  7. tseaver commented on Nov 5, 2019

    @tseaver
    tseaver
    on Nov 5, 2019
    ContributorAuthor

    @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.

  8. tseaver
    added a commit that references this issue on Nov 5, 2019

    tests: add centralized VPCSC config detection (#9581)

    41ae858

  9. tseaver commented on Nov 6, 2019

    @tseaver
    tseaver
    on Nov 6, 2019
    ContributorAuthor

    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.

  10. tseaver
    added 5 commits that reference this issue on Nov 7, 2019

    tests(automl): normalize VPCSC configuration in systests (#9607)

    e6ddd3b

    tests(trace): normalize VPCSC configuration in systests (#9618)

    ef309fc

    tests(dlp): normalize VPCSC configuration in systests (#9608)

    3dab9f3

    tests(asset): normalize VPCSC configuration in systests (#9614)

    d08b210

    tests(monitoring): normalize VPCSC configuration in systests (#9615)

    505d553

  11. tseaver commented on Feb 13, 2020

    @tseaver
    tseaver
    on Feb 13, 2020
    ContributorAuthor

    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.

  12. tseaver
    closed this as completedon Feb 13, 2020
  13. parthea
    added a commit that references this issue on Sep 22, 2023

    tests(dlp): normalize VPCSC configuration in systests (#9608)

    7f04e61
  14. parthea
    added 4 commits that reference this issue on Oct 21, 2023

    tests(asset): normalize VPCSC configuration in systests (#9614)

    3a076bb

    tests(monitoring): normalize VPCSC configuration in systests (#9615)

    c684392

    tests(automl): normalize VPCSC configuration in systests (#9607)

    8452888

    tests(trace): normalize VPCSC configuration in systests (#9618)

    7bdc256
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

api: automlIssues related to the AutoML API.api: cloudassetIssues related to the Cloud Asset Inventory API.api: cloudtraceIssues related to the Cloud Trace API.api: dlpIssues related to the Sensitive Data Protection API.api: monitoringIssues related to the Cloud Monitoring API.api: storageIssues related to the Cloud Storage API.api: translationIssues related to the Cloud Translation API API.api: videointelligenceIssues related to the Video Intelligence API API.api: visionIssues related to the Cloud Vision API.testingtype: processA process-related concern. May include testing, release, or the like.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions