-
Notifications
You must be signed in to change notification settings - Fork 937
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 "could not assert Secret Manager permissions" Cloud Build error #6904
Fix "could not assert Secret Manager permissions" Cloud Build error #6904
Conversation
4baff20
to
8c94eb9
Compare
@@ -174,7 +173,20 @@ async function getOrCreateOauthConnection( | |||
projectId: string, | |||
location: string, | |||
): Promise<gcb.Connection> { | |||
let conn = await getOrCreateConnection(projectId, location, APPHOSTING_OAUTH_CONN_NAME); |
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 for the fix! I wonder if we can get rid of getOrCreateConnection now by inlining it in the other callsites (which I believe should just be linkGitHubRepository()
)
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.
ya we can but I do like that it helps with readability.
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.
Can we add a test?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6904 +/- ##
==========================================
+ Coverage 54.44% 54.52% +0.07%
==========================================
Files 353 353
Lines 24652 24657 +5
Branches 5094 5095 +1
==========================================
+ Hits 13423 13443 +20
+ Misses 10012 9995 -17
- Partials 1217 1219 +2 ☔ View full report in Codecov by Sentry. |
35339f2
to
71b426b
Compare
Users are often running into this
could not assert Secret Manager permissions
error when onboarding to apphosting.This occurs when the user has no Cloud Build repo connections (including the sentinel Oauth connection) and the secret manager admin role has not been granted to the Cloud Build P4SA. So when the user goes to create an Oauth connection first but Cloud Build has no permissions to save the oath token in secret manager, it throws the above error.
To fix this we just need to ensure that the correct secret manager privileges are set to the Cloud Build P4SA before even creating the initial sentinel Oauth connection.