feat(auth): add UniverseDomain to DetectOptions #9536
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
codyoss
approved these changes
Mar 11, 2024
codyoss
reviewed
Mar 15, 2024
quartzmo
changed the title
quartzmo
force-pushed
the
auth-universe-domain-2
branch
from
d387ed7
to
8b5567a
Compare
codyoss
reviewed
Mar 20, 2024
| @@ -93,6 +93,7 @@ func DetectDefault(opts *DetectOptions) (*auth.Credentials, error) { | |||
| ProjectIDProvider: auth.CredentialsPropertyFunc(func(context.Context) (string, error) { | |||
| return metadata.ProjectID() | |||
| }), | |||
| UniverseDomainProvider: &internal.ComputeUniverseDomainProvider{}, | |||
There was a problem hiding this comment.
nit: maybe the struct returned by the computeTokenProvider method could have a method that impl's this and/or return a function to impl it. Then all the compute logic is tied to a single struct.
There was a problem hiding this comment.
after reviewing a bit more maybe this is fine as it is used in more than one spot, I will leave the comment open though for your consideration.
There was a problem hiding this comment.
Agreed, let's leave as-is.
| base: base, | ||
| creds: creds, | ||
| base: base, | ||
| // TODO(quartzmo): Somehow set clientUniverseDomain from impersonate calls. |
There was a problem hiding this comment.
Do we need this to take an optional third arg, or what are your thoughts here?
There was a problem hiding this comment.
I'm not sure how this should be done. If you can suggest the change, let's do it in this PR. Otherwise, we could wait.
| // httpGetMetadataUniverseDomain is a package var for unit test substitution. | ||
| var httpGetMetadataUniverseDomain = func(ctx context.Context) (string, error) { | ||
| client := metadata.NewClient(&http.Client{Timeout: time.Second}) | ||
| // TODO(quartzmo): set ctx on request |
There was a problem hiding this comment.
I would really like if we updated the surface of the metadata package to be context aware in the future.
There was a problem hiding this comment.
Agreed, let's do it soon.
gcf-merge-on-green bot
pushed a commit
that referenced
this pull request
Apr 15, 2024
🤖 I have created a release *beep* *boop* --- ## [0.2.0](https://github.com/googleapis/google-cloud-go/compare/auth/v0.1.1...auth/v0.2.0) (2024-04-15) ### Features * **auth/credentials/externalaccount:** Add default TokenURL ([#9700](https://github.com/googleapis/google-cloud-go/issues/9700)) ([81830e6](https://github.com/googleapis/google-cloud-go/commit/81830e6848ceefd055aa4d08f933d1154455a0f6)) * **auth:** Add downscope.Options.UniverseDomain ([#9634](https://github.com/googleapis/google-cloud-go/issues/9634)) ([52cf7d7](https://github.com/googleapis/google-cloud-go/commit/52cf7d780853594291c4e34302d618299d1f5a1d)) * **auth:** Add universe domain to grpctransport and httptransport ([#9663](https://github.com/googleapis/google-cloud-go/issues/9663)) ([67d353b](https://github.com/googleapis/google-cloud-go/commit/67d353beefe3b607c08c891876fbd95ab89e5fe3)), refs [#9670](https://github.com/googleapis/google-cloud-go/issues/9670) * **auth:** Add UniverseDomain to DetectOptions ([#9536](https://github.com/googleapis/google-cloud-go/issues/9536)) ([3618d3f](https://github.com/googleapis/google-cloud-go/commit/3618d3f7061615c0e189f376c75abc201203b501)) * **auth:** Make package externalaccount public ([#9633](https://github.com/googleapis/google-cloud-go/issues/9633)) ([a0978d8](https://github.com/googleapis/google-cloud-go/commit/a0978d8e96968399940ebd7d092539772bf9caac)) * **auth:** Move credentials to base auth package ([#9590](https://github.com/googleapis/google-cloud-go/issues/9590)) ([1a04baf](https://github.com/googleapis/google-cloud-go/commit/1a04bafa83c27342b9308d785645e1e5423ea10d)) * **auth:** Refactor public sigs to use Credentials ([#9603](https://github.com/googleapis/google-cloud-go/issues/9603)) ([69cb240](https://github.com/googleapis/google-cloud-go/commit/69cb240c530b1f7173a9af2555c19e9a1beb56c5)) ### Bug Fixes * **auth/oauth2adapt:** Update protobuf dep to v1.33.0 ([30b038d](https://github.com/googleapis/google-cloud-go/commit/30b038d8cac0b8cd5dd4761c87f3f298760dd33a)) * **auth:** Fix uint32 conversion ([9221c7f](https://github.com/googleapis/google-cloud-go/commit/9221c7fa12cef9d5fb7ddc92f41f1d6204971c7b)) * **auth:** Port sts expires fix ([#9618](https://github.com/googleapis/google-cloud-go/issues/9618)) ([7bec97b](https://github.com/googleapis/google-cloud-go/commit/7bec97b2f51ed3ac4f9b88bf100d301da3f5d1bd)) * **auth:** Read universe_domain from all credentials files ([#9632](https://github.com/googleapis/google-cloud-go/issues/9632)) ([16efbb5](https://github.com/googleapis/google-cloud-go/commit/16efbb52e39ea4a319e5ee1e95c0e0305b6d9824)) * **auth:** Remove content-type header from idms get requests ([#9508](https://github.com/googleapis/google-cloud-go/issues/9508)) ([8589f41](https://github.com/googleapis/google-cloud-go/commit/8589f41599d265d7c3d46a3d86c9fab2329cbdd9)) * **auth:** Update protobuf dep to v1.33.0 ([30b038d](https://github.com/googleapis/google-cloud-go/commit/30b038d8cac0b8cd5dd4761c87f3f298760dd33a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #9603