Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: GCECredentials - Allow retrieval of ID token#425

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@StupidCodeFactory
Copy link
Contributor

@StupidCodeFactoryStupidCodeFactory commentedApr 2, 2023•
edited
Loading

CONTEXT:

While deploying a couple of services onCloud RUN:

  1. service A running with service account A
  2. service B running with service account B
  3. I addedroles/run.invoker for service account B on service A.

I expected to be able to retrieve an ID token and be able to call service A from service B. After some debugging I realised I was getting anaccess_token rather then anid_token.
I tracked it down to the GCECredentials instantiation only passing the scope to the subclass of theSignet::Oauth2::Client, hence not being able to pass thetarget_audience for retrieve anid_token

I believe this shouldfix:#299

@StupidCodeFactoryStupidCodeFactory requested a review froma team as acode ownerApril 2, 2023 16:04
@google-cla
Copy link

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View thisfailed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@conventional-commit-lint-gcf
Copy link

conventional-commit-lint-gcfbot commentedApr 2, 2023•
edited
Loading

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or useautomerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@StupidCodeFactoryStupidCodeFactoryforce-pushed thepass-options-to-gce-credentials branch 4 times, most recently from1163c39 to97255afCompareApril 2, 2023 16:18
@StupidCodeFactoryStupidCodeFactory changed the titleFix GCECredentials - Allow passing of options down to the OAuth 2 client.GCECredentials - Allow retrieval of ID tokenApr 2, 2023
@StupidCodeFactoryStupidCodeFactory changed the titleGCECredentials - Allow retrieval of ID tokenfix: GCECredentials - Allow retrieval of ID tokenApr 2, 2023
end

it"honors passing options to OAuth 2 client"do
stub=stub_request(:get,"http://169.254.169.254")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Nit: since we're repeating this stub request. Can you put it in a variable?

Copy link
Contributor

@bajajneha27bajajneha27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for raising this PR. Just one tiny comment, overall LGTM

@StupidCodeFactoryStupidCodeFactoryforce-pushed thepass-options-to-gce-credentials branch from97255af to9c49861CompareApril 7, 2023 09:37
expect(creds).to_notbe_nil
describe"when on compute engine"do
beforedo
@compute_metadata_server=stub_request(:get,"http://169.254.169.254")
Copy link
ContributorAuthor

@StupidCodeFactoryStupidCodeFactoryApr 7, 2023•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@bajajneha27 thanks for your feedback.
While one wouldnormally use alet!(:compute_metadata_server) as it's more idiomatic to RSpec test suite, this file seems to only be using@instance_variable so for the sake of consistency I've kept the pattern used in the file.

There seem to be a mixed use oflet andbefore { @instance_varialbe = "foo" } throughout the test suite. It's also unclear what is the preferred way.
As a long term rubyist I'd be inclined to favour thelet {}.
I'd be happy to convert this file from using@instance_variable tolet blocks, in this pull request or another one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This looks fine. Thanks for implementing this.

@StupidCodeFactoryStupidCodeFactoryforce-pushed thepass-options-to-gce-credentials branch from9c49861 to0158267CompareApril 7, 2023 10:44
@bajajneha27
Copy link
Contributor

@StupidCodeFactory , you'd have to update your branch with main.

  Passing of options down to the OAuth 2 client. optional options to GCEcredentials, enabling the creation of ID tokens.
@StupidCodeFactoryStupidCodeFactoryforce-pushed thepass-options-to-gce-credentials branch from0158267 tocc797e3CompareApril 11, 2023 13:29
@bajajneha27bajajneha27 merged commitfd9afc7 intogoogleapis:mainApr 12, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@bajajneha27bajajneha27bajajneha27 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Getting id-token from compute engine default service account not working

2 participants

@StupidCodeFactory@bajajneha27
[8]ページ先頭
©2009-2025 Movatter.jp