-
Notifications
You must be signed in to change notification settings - Fork 108
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
auth: Support URL type external account source #239
Conversation
@@ -95,16 +96,19 @@ impl TokenSource for ExternalAccountTokenSource { | |||
async fn subject_token_source( | |||
audience: Option<String>, | |||
source: CredentialSource, | |||
) -> Result<impl SubjectTokenSource, Error> { | |||
) -> Result<Box<dyn SubjectTokenSource>, Error> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any particular reason for changing from impl SubjectTokenSource
to Box<dyn SubjectTokenSource>
?
There seems to be no need for the change since the function return is limited to AWSSubjectTokenSource and UrlSubjectTokenSource
.
(I was looking at this PR and noticed that I can remove the async_trait
of the SubjectTokenSource
in rust 1.75. Thank you very much.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yoshidan
impl SubjectTokenSource
can return either AWSSubjectTokenSource
or UrlSubjectTokenSource
. However, if the type returned changes depending on the CredentialSource within the same function, it becomes impossible to specify the type at compile time, so dynamic dispatch may be required
(I might be misunderstanding something 🙏 )
Actually, the following error occurs when I specify impl SubjectTokenSource
at the return position
error[E0308]: mismatched types
--> foundation/auth/src/token_source/external_account_source/mod.rs:109:12
|
109 | Ok(ts)
| -- ^^ expected `AWSSubjectTokenSource`, found `UrlSubjectTokenSource`
| |
| arguments to this enum variant are incorrect
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. you are right.
This PR updates
google-cloud-auth
to support URL type external account source. By this update, Workload Identity Federation for Github Actions can be enabled.From: #233