-
Notifications
You must be signed in to change notification settings - Fork 950
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
Propogate explicit service account to eventarc #6859
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6859 +/- ##
=======================================
Coverage 54.27% 54.27%
=======================================
Files 352 352
Lines 24534 24536 +2
Branches 5082 5083 +1
=======================================
+ Hits 13315 13317 +2
Misses 10006 10006
Partials 1213 1213 ☔ View full report in Codecov by Sentry. |
FYI: This is a breaking change that does ignore the We would have much preferred an additional parameter to set the service account for the event_arc trigger separately instead of re-using an existing service account and as such forcing the use of one service account for both the event arc trigger and the workload. |
This is unfortunate. I can go through the process of creating an API proposal to have two separate fields. As hard-coding a possibly non-existent service account might have been considered a bug, we may instead roll forward where the trigger service account defaults to the service account used within the container and can be overridden. Can you help me understand your use case a bit better? I assume you were using a lower-privileged service account in your functions to ensure bugs don't have a large blast radius? |
Dear @inlined thank you for your immediate response! 🙇 We would absolutely appreciate the API being amended to have two separate fields. The motivation for multiple service accounts is exactly as you assumed to adhere to best practices and minimise any potential bugs or security incidents have highly limited blast radius (e.g. the service account for the CloudRun service would usually not have permissions to invoke itself). Additionally, it makes it easier to manage/maintain the service accounts permission if the service account is more narrowly scoped thus further reducing the risk of unnecessary permissions that are kept due to uncertainty of them being in use. Our setup Unfortunately, when changing the service account of a Event Arc Trigger for a function it needs to be re-created, which also means that the corresponding Pub/Sub topic and Pub/Sub subscription are re-created. This means that we either would have to re-import the subscription on every deployment or we need to adhere to how firebase-cli manages the services accounts (one service account for both trigger and the service). Thank you for your time and consideration @inlined! 🙇 |
Another part of the fix to #6814. Similar to #6858, this propagates the function's service account through to the invoking service.