-
Notifications
You must be signed in to change notification settings - Fork 778
fix: fix propagation issues in bootstrap on retry #1437
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: fix propagation issues in bootstrap on retry #1437
Conversation
Error and trigger of a retry
Build was sucessful after the retry
|
…by VPC Service Controls
@apeabody Could you PTAL? |
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @daniel-cit!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request addresses two separate issues: it adds explicit depends_on
attributes to several Terraform resources to mitigate propagation delays during retries, and it removes an unsupported service from the VPC Service Controls configuration. The changes are generally good and address the stated goals. I have one suggestion to make one of the new dependencies more specific, which should improve the robustness of the fix.
role = "roles/artifactregistry.reader" | ||
member = "serviceAccount:${google_service_account.terraform-env-sa[each.key].email}" | ||
|
||
depends_on = [module.tf_source] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better dependency clarity and robustness, it's recommended to make this dependency more specific. This resource grants permissions on an Artifact Registry repository, which is created by module.tf_cloud_builder
. While module.tf_cloud_builder
depends on module.tf_source
, explicitly depending on module.tf_cloud_builder
ensures that the repository is fully created before attempting to modify its IAM policy. This makes the configuration more resilient to potential race conditions related to the repository's creation.
depends_on = [module.tf_cloud_builder]
This PR adds a set of depends to the module that creates the CI/CD project to prevent errors when granting roles to resources in that project during a retry of execution.
Additionally this PR also fixes an error related to the service 'alpha-documentai.googleapis.com' not being supported any more as a restricted service in VPC-SC