fix: support export auth export with envvar for backwards compat #426
Conversation
src/setup-gcloud.ts
Outdated
| process.env.GOOGLE_GHA_CREDS_PATH, | ||
| 'utf8', | ||
| ); | ||
| serviceAccountKeyObj = parseServiceAccountKey(cPath); |
There was a problem hiding this comment.
I don't think this will work, because the export from auth could be a WIF cred file, which isn't a "service account key"
There was a problem hiding this comment.
I refactored the logic a bit based on what we discussed yesterday and added some test cases for more scenarios. I also added a warning to help users migrating that export_creds is not necessary when using setup-gcloud in conjunction with auth.
| credsPath = process.env.GOOGLE_GHA_CREDS_PATH; | ||
| // User is likely using google-github-actions/auth for auth followed by setup-gcloud with export_default_credentials. | ||
| // This is unnecessary as auth already exports credentials. | ||
| core.warning( |
There was a problem hiding this comment.
setup-gcloud also exports GOOGLE_GHA_CREDS_PATH (maybe we should remove that?), so I think you'd hit this block with back-to-back export_default_credentials for two setup-gcloud uses.
We should add an integration test to make sure back-to-back setup-gcloud works and uses the correct auth:
- uses: setup-gcloud
with:
credentials: cred1
- run: gcloud auth list | gregp 'cred1email'
- uses: setup-gcloud
with:
credentials: cred2
- run: gcloud auth list | gregp 'cred2email'There was a problem hiding this comment.
I think it'll be fine because it'll fall into the first case of this conditional, but it gets weird if the user does:
- uses: setup-gcloud
with:
credentials: cred1
- run: gcloud auth list | gregp 'cred1email'
# I think this will throw the warning
- uses: setup-gcloudThere was a problem hiding this comment.
Yeah the first case would be fine
For
- uses: setup-gcloud
with:
credentials: cred1
- uses: setup-gcloudsince user is not specifying explicit creds, it is no op.
Using something else other than GOOGLE_GHA_CREDS_PATH for cleanup in setup-gcloud is also feasible.
src/setup-gcloud.ts
Outdated
| credsPath, | ||
| JSON.stringify(serviceAccountKeyObj, null, 2), // Print to file as string w/ indents | ||
| ); | ||
| core.exportVariable('GCLOUD_PROJECT', serviceAccountKeyObj.project_id); |
There was a problem hiding this comment.
I think we should only set the project ID here if the project_id input wasn't given above. If the user explicitly set a project_id, we should prefer that over the SA's project. Probably be good to compare the project IDs and throw a warning if they are different too
There was a problem hiding this comment.
Done and added a warning. In the earlier flow we were setting GCLOUD_PROJECT to serviceAccountKeyObj.project_id and then re exporting GCLOUD_PROJECT as projectID if set. Hopefully the newer flow is clearer.
|
Hello, |
|
Hi @kaominghsu - if you're having an issue, please open a new GitHub issue and complete the issue template so we have the necessary information to debug. |
fixes google-github-actions/auth#73