-
Notifications
You must be signed in to change notification settings - Fork 379
CSR Metadata request acts as a probe for ImdsV2 #5359
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
CSR Metadata request acts as a probe for ImdsV2 #5359
Conversation
src/client/Microsoft.Identity.Client/ManagedIdentity/CsrMetadata.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/CsrMetadata.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsV2Tests.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsV2Tests.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsV2Tests.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/PublicApi/net472/PublicAPI.Shipped.txt
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/CsrMetadata.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHttpMessageHandler.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsManagedIdentitySource.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentityApplication.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs
Outdated
Show resolved
Hide resolved
neha-bhargava
left a comment
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.
Need to undo the changes to public API to get the managed identity source
f0f4eb3 to
6aa43d0
Compare
src/client/Microsoft.Identity.Client/ManagedIdentity/CsrMetadata.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/AppServiceTests.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/AppServiceTests.cs
Show resolved
Hide resolved
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public async Task<ManagedIdentitySource> GetManagedIdentitySourceAsync() |
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.
Why not keep this as static as well. The method to get the source was created static so that Azure Identity can detect if managed identity is available before creating the source. Was this discussed with them?
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.
@neha-bhargava, this method passes the Service Bundle through the ManagedIdentityClient's GetManagedIdentitySourceAsync method, and into ImdsV2ManagedIdentitySource, where the csr metadata probe uses things from the Service Bundle: IdType, UserAssignedId, RetryPolicyFactory, HttpManager, Logger, etc.
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.
@gladjohn Can you confirm that this would work with Azure Identity? Did you discuss with them?
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.
thought, we agreed to keep this static @Robbie-Microsoft.
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.
The dev experience won't change for Azure SDK. They are still creating an instance of Managed Identity Application for their tests - they will be able to call this method from the managedAidentityApplication. You, @bgavrilMS and I can discuss over chat.
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsV2Tests.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Http/Retry/HttpRetryCondition.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Show resolved
Hide resolved
38dbae3
into
rginsburg/msiv2_feature_branch
This PR introduces ImdsV2 as a Managed Identity source, and focuses on executing a network request to get metadata to be used in the rest of the flow.