-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Fix tests to use dedicated bootstrapped service accounts instead of one shared account #10418
Conversation
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: Click here to see the affected service packagesall service packages are affected Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataSourceGoogleServiceAccountAccessToken_basic|TestAccDataSourceGoogleServiceAccountIdToken_impersonation|TestAccSqlUser_postgresIAM |
|
/gcbrun |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: Click here to see the affected service packagesall service packages are affected Action takenFound 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataSourceGoogleServiceAccountAccessToken_basic|TestAccDataSourceGoogleServiceAccountIdToken_impersonation|TestAccHealthcareDatasetIamPolicy|TestAccSqlUser_postgresIAM |
|
…ne shared account
04780b4
to
2612ae5
Compare
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: Click here to see the affected service packagesall service packages are affected Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataSourceGoogleServiceAccountAccessToken_basic|TestAccSqlUser_postgresIAM |
|
Fixes hashicorp/terraform-provider-google#16247
Previously, one shared service account was bootstrapped and reused for at least 2 of these tests. During the bootstrap, which is called on every VCR run, the policy on the service account was designed to be overwritten (to control for drift). However, overwriting the policy is a read-modify-write operation that cannot be done concurrently. This creates a race condition when the bootstrap functions are run at the same time, which seems to be more common in the VCR case.
The solution chosen here is to create a separate service account per-test. Service accounts are cheap, there are only 5 such uses for them so far, and this allows us to enforce the policy on every run the way we do now. Since the accounts are isolated per-test, they can still be bootstrapped ahead of time, while not running into concurrency issues.
Release Note Template for Downstream PRs (will be copied)