Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PubSub: Skip two IAM tests for VPCSC #8677

Closed
wants to merge 2 commits into from
Closed

PubSub: Skip two IAM tests for VPCSC #8677

wants to merge 2 commits into from

Conversation

anguillanneuf
Copy link
Contributor

Make test_managing_topic_iam_policy and test_managing_subscription_iam_policy VPCSC-compatible by excluding them when the environment variable GOOGLE_CLOUD_TESTS_IN_VPCSC is set to "true".

Reference: #6215

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 15, 2019
Copy link
Contributor

@plamut plamut left a comment

Choose a reason for hiding this comment

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

The changes look good, consistent with what can be found at other places in the project.

I would just like to know what VPCSC is and how can I verify that these two tests should indeed be excluded in that case?

@anguillanneuf
Copy link
Contributor Author

Thanks Peter for reviewing!

@bmccutchon may be able to elaborate that better. Brian?

@plamut plamut added the api: pubsub Issues related to the Pub/Sub API. label Jul 15, 2019
@plamut plamut changed the title pubsub: skip two iam tests for VPCSC PubSub: Skip two IAM tests for VPCSC Jul 15, 2019
@bmccutchon
Copy link
Contributor

VPC Service Controls lets you create secure perimeters around projects to prevent data exfiltration. To verify that these client libraries are compatible with it, we need to be able to run the tests in a secure VPCSC environment. When run in this environment, tests can't access resources in other projects. So, in short, tests that access resources in projects separate from the one they run on—e.g. public datasets—need to be skipped or modified to be compatible with VPCSC.

That said, these tests don't access resources in other projects, so we probably shouldn't skip them. Instead, we discovered that these tests were actually failing due to another issue that we are able to fix.

@plamut
Copy link
Contributor

plamut commented Jul 16, 2019

@bmccutchon Thanks for the explanation!

@anguillanneuf Since the root cause of the failures can be fixed elsewhere, I presume we do not need to skip the tests, unless it's semi-urgent and we still want to disable them temporarily?

If so, please add a comment to the source and open a GitHub issue so that we don't forget to revert the changes later. Or, if we can live with the current state for a bit longer, we should probably close this PR without merging.

@anguillanneuf
Copy link
Contributor Author

anguillanneuf commented Jul 17, 2019

Thanks @bmccutchon and @plamut! I will close this PR for now but keeping the branch just in case!

@tseaver tseaver deleted the vpcsc branch August 5, 2019 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants