Skip to content

Conversation

@steinwaywhw
Copy link
Contributor

This new file is added in order to test client lib compatibility of VPC SC. The tests are auto-generated from the service definition of the monitoring API. The tests can be run inside or outside of VPC service perimeter. The input to the script should be the following environment variables.

  1. PROJECT_ID: a project that is inside the VPC perimeter.
  2. GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT: a project that is outside the VPC perimeter.

@steinwaywhw steinwaywhw requested a review from crwilcox as a code owner April 23, 2019 21:35
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 23, 2019

@staticmethod
def _do_test(delayed_inside, delayed_outside):
if ('GOOGLE_CLOUD_TESTS_IN_VPCSC' in os.environ) and (os.environ['GOOGLE_CLOUD_TESTS_IN_VPCSC'].lower() == 'True'.lower()):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just write os.environ.get('GOOGLE_CLOUD_TESTS_IN_VPCSC', 'false').lower() == 'true'.

@tseaver tseaver added api: monitoring Issues related to the Cloud Monitoring API. testing labels Apr 29, 2019
Copy link
Contributor

@tseaver tseaver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These new tests are all being skipped under CI. We should probably work out how to get them enabled before merging.

@steinwaywhw
Copy link
Contributor Author

Hi @tseaver, the VPC team has their own Kokoro workflow to test this. I believe @bmccutchon knows more. Otherwise, if you'd like to run this test in the pull request workflow, please supply these environment variables: PROJECT_ID=secure-gcp-test-project-4 and GOOGLE_CLOUD_TESTS_VPCSC_OUTSIDE_PERIMETER_PROJECT=some GCP test project that you know of

@tseaver tseaver changed the title Add VPCSC system test. Monitoring: add VPCSC system test. Apr 30, 2019
@tseaver
Copy link
Contributor

tseaver commented Apr 30, 2019

@tseaver It doesn't seem to me to make sense to have the tests maintained in a separate repository from the location where the CI runs. @busunkim96, @crwilcox please chime in.

@steinwaywhw
Copy link
Contributor Author

This is a requirement from the VPC team. Many teams are going to implement such tests. I have merged a similar one here: googleapis/google-cloud-java#5006, and the Spanner team has done it here https://github.com/googleapis/google-cloud-java/blob/master/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITVPCNegativeTest.java.

@bmccutchon
Copy link
Contributor

@tseaver Echoing what @steinwaywhw said, we expect the client library teams to write and maintain these tests. The easiest way to do that is to keep them in this repo. I agree that it would make sense to have the CI workflow also be in this repo or run against affected pull requests, but that isn't feasible quite yet.

@busunkim96
Copy link
Contributor

@bmccutchon Would it make sense for us to run these tests against PRs in this repo, or is there some limitation that would keep us from doing that? Are there requirements beyond setting the environment variables @steinwaywhw mentioned?

We would probably edit the nox session here to set the required environment variables.

@steinwaywhw
Copy link
Contributor Author

Hi @busunkim96, there are essentially two ways to run these tests: 1) from inside VPCSC, 2) from outside VPCSC. For case 1, you need to setup VPCSC environments, which is non-trivial. The VPC team has a setup, and they can run it. For case 2, you do not need a VPCSC setup, and the tests can be run by simply setting those two environment variables, and the test case still provides valuable informations on the effectiveness of VPC service control.

@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Apr 30, 2019
@steinwaywhw
Copy link
Contributor Author

Friendly ping 👋

@busunkim96
Copy link
Contributor

Merging this now. I will create a follow-up PR to set the environment variables to make the tests run.

@busunkim96 busunkim96 merged commit 6f78228 into googleapis:master May 2, 2019
@steinwaywhw
Copy link
Contributor Author

@busunkim96 Thank you very much! If you have further questions, feel free to reach out to me.

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

Labels

api: monitoring Issues related to the Cloud Monitoring API. cla: yes This human has signed the Contributor License Agreement. 🚨 This issue needs some love. testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants