Skip to content
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

Internal improvements (builds on #77) #78

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

msdrigg
Copy link

@msdrigg msdrigg commented Aug 18, 2023

Mostly improving some optional parameters in the key formats

The last commit makes some internal improvements. Feel free to accept or reject any individual commit.

This PR supports parsing both formats in either the `GOOGLE_APPLICATION_CREDENTIALS` env variable or the `~/.config/gcloud/application_default_credentials.json` file.
This PR adds a new `ServiceAccount` format that takes credentials from `source_credentials: ServiceAccount` and then makes a request to get a service account token using those credentials.

This also adds the ability to parse the token format created by `gcloud auth application-default login --impersonate-service-account <service account>`
- Sort scope vec before using it as a hashmap key. Vecs of scopes aren't dependent on order, so the hashmap key shouldn't be either
- Reduce complexity by moving all calls to `token::from_string` to `token::new`
@msdrigg msdrigg changed the title Internal improvements (depends on #77) Internal improvements (builds on #77) Aug 18, 2023
@msdrigg
Copy link
Author

msdrigg commented Aug 18, 2023

Tested this PR with real world keys

  • Tested with GOOGLE_APPLICATION_CREDENTIALS and ~/.config/gcloud/application_default_credentials.json and verified the new tagged enum is used in both cases
  • Tested with service account key, user creds, and an impersonated service account and verified that I can make API calls

@djc
Copy link
Owner

djc commented May 27, 2024

Hey @msdrigg sorry for taking so long to get back to this. I've done a fairly big refactoring in #108 which I think will help doing some of the things here. Are you still interested in getting some of these changes merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants