feat: add ImpersonatedServiceAccountCredentials #421
Conversation
PsyonixMonroe
force-pushed
the
feature/credentials_loader_extension
branch
from
de40edb
to
120bcda
Compare
There was a problem hiding this comment.
Thanks for the PR @PsyonixMonroe :).
It turns out we have the ability to sign a blob with IAM in this library as is, so we should be able to utilize that, it can be found here: https://github.com/googleapis/google-auth-library-php/blob/main/src/Iam.php#L65
See GCECredentials as an example of how the implementation is used.
Would you be open to migrating ImpersonatedServiceAccountCredentials into the credentials directory in this repo so it can be used more widely?
PsyonixMonroe
force-pushed
the
feature/credentials_loader_extension
branch
from
120bcda
to
1f24c4c
Compare
PsyonixMonroe
changed the title
|
@dwsupplee Thanks for the feedback! I didn't know that IAM signing was already supported at this level. I have update the PR to add I also migrated the The nice thing is that with this change there won't need to be a second one in cloud-core. Please let me know if there is any other feedback. |
There was a problem hiding this comment.
This is greatly appreciated, thank you @PsyonixMonroe. Just a few notes from me, otherwise looks really great. I think this will help close out #387 for us as well.
tests/Credentials/ImpersonatedServiceAccountCredentialsTest.php
Outdated
Show resolved
Hide resolved
PsyonixMonroe
force-pushed
the
feature/credentials_loader_extension
branch
from
1f24c4c
to
f715ca7
Compare
PsyonixMonroe
requested review from
dwsupplee and
bshaffer
and removed request for
dwsupplee and
bshaffer
There was a problem hiding this comment.
Just a small nit on the licenses. It looks like we may also need you to sign the CLA, please see here for more: https://cla.developers.google.com/
tests/Credentials/ImpersonatedServiceAccountCredentialsTest.php
Outdated
Show resolved
Hide resolved
|
It looks like there are a few style / static analysis issues as well: https://github.com/googleapis/google-auth-library-php/actions/runs/3340256021/jobs/5619317746 |
|
I'm currently working with the owner of the Group that controls the CLA list for Epic. Its just taking me a bit of time track it down. |
This will refactor the current GCECredentials method for calling IAM to perform request signing into a trait that can be shared with other CredentialLoaders that need to call IAM. Adds ImpersonatedServiceAccountCredentials which uses the new trait to perform blob signing through IAM, but does so when impersonating a service account with `gcloud auth application-default login --impersonate-service-account=<account name>`
PsyonixMonroe
force-pushed
the
feature/credentials_loader_extension
branch
3 times, most recently
from
21cd618
to
8353e6b
Compare
|
It appears that this most recent PHPStan failure is for a file not modified by this PR. |
|
@PsyonixMonroe good call out. It looks like we can safely ignore that for this PR. Once we get the CLA signed and @bshaffer's sign off on his requested changes we should be good to go here. Thanks again for the great PR. |
|
@PsyonixMonroe have you had any luck with the CLA progress? Please let us know if there's anything we can do to help. |
|
@dwsupplee I've been getting the occasional check in from my Epic Point of Contact on the CLA, it appears that at some point in the last year our CLA group was removed(?deleted?) and they are trying to get it recreated. Not sure if there is anything from your side that can be done about getting this set back up. It seems like we are currently working on getting a new CLA in place and the group recreated. |
|
@PsyonixMonroe I don't know anything about the CLA group you're referring to, but the bot simply checks that the email used to make the github commit (matt.monroe@psyonix.com) has signed the CLA, which can be done here: https://cla.developers.google.com/ So I suggest you simply go to the above URL and sign the CLA yourself, and that should do the trick. See the failing check for more information. |
|
@bshaffer Since this is being done as part of my work for Psyonix (owned by Epic Games), I need Epic to go through the Company CLA process, not the individual. |
Sounds good. If there is any "approved GitHub email" with your company which has signed it, alternatively they could commit the changes and submit a new PR and we could merge that. |
|
@bshaffer Looks like the CLA check is passing now 🥳 |
bshaffer
changed the title
In order to add support for IAM Blob Signing via API request in google-cloud-php-iam-credentials, there needs to be a way to extend the different types of credential loaders that are supported by this lower level library. Support cannot be added here directly due to the introduction of a circular dependancy.
This change will allow higher level libraries to "register" custom Credentials Loader Factory Method objects and have them considered when processing application_default_credentials.json
(impersonated_service_account as an example).