[MSI V2 Feature] Adding IMDSV2 probe logic #5135
Conversation
| /// <param name="cancellationToken"></param> | ||
| /// <returns></returns> | ||
| /// <exception cref="ArgumentNullException"></exception> | ||
| public static async Task<ManagedIdentitySource> GetManagedIdentitySourceAsync( |
There was a problem hiding this comment.
nit: be consistent with visibility modifiers. Use internal everywhere
| /// <exception cref="ArgumentNullException"></exception> | ||
| public static async Task<ManagedIdentitySource> GetManagedIdentitySourceAsync( | ||
| IServiceBundle serviceBundle, | ||
| CancellationToken cancellationToken = default) |
There was a problem hiding this comment.
nit; don't use default params in internal code. it leads other developers to use the simpler versions without thinking too much.
| @@ -45,6 +46,7 @@ public static void ResetInternalStaticCaches() | |||
| OidcRetrieverWithCache.ResetCacheForTest(); | |||
| AuthorityManager.ClearValidationCache(); | |||
| SingletonThrottlingManager.GetInstance().ResetCache(); | |||
| ManagedIdentityClient.ResetManagedIdentitySourceCache(); | |||
There was a problem hiding this comment.
Doesn't the method ResetInternalStaticCaches need to be exposed in public API for Azure SDK ? Probably experimental extensiblity PR. Can be as a separate PR, just making sure it's on your radar.
There was a problem hiding this comment.
I will track it as part of #5136
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.Info($"[Credential Probe] Exception during probe: {ex.Message}"); |
| { | ||
| if (response == null) | ||
| { | ||
| _logger.Info("[Credential Probe] No response received from the server."); |
There was a problem hiding this comment.
Should this also be logger.error like on line 80?
| } | ||
| } | ||
|
|
||
| private void LogResponseDetails(HttpResponse response) |
There was a problem hiding this comment.
Actually, it looks like this method can be merged into the EvaluateProbeResponse method. they are perfroming the same check on the response and printing the same messages
| internal class ImdsV2ManagedIdentitySource : AbstractManagedIdentity | ||
| { | ||
| /// <summary> | ||
| /// Factory method to create an instance of `CredentialManagedIdentitySource`. |
There was a problem hiding this comment.
This factory method doesnt seem to have much logic in it besides the log message. Can the constructor be used to instantiate this instead? it returns a AbstractManagedIdentity but it looks like the type will always be ImdsV2ManagedIdentitySource anyway?
Fixes #5115
Please refer to the above task for more details. This is PR is part of a feature branch.
This pull request introduces several changes to the
Microsoft.Identity.Clientlibrary, focusing on enhancing managed identity support, particularly with the addition of a newImdsV2managed identity source. The changes include creating new classes, updating existing methods, and adding new methods to support asynchronous operations.Key changes include:
Enhancements to Managed Identity Support:
ImdsV2ManagedIdentitySourceclass to handle the newImdsV2managed identity source. (src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs)ImdsCredentialProbeManagerclass to probe the IMDS credential endpoint and determine its support. (src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsCredentialProbeManager.cs)ManagedIdentitySourceenum to include the newImdsV2value. (src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentitySource.cs)Asynchronous Method Updates:
ManagedIdentityClientto use asynchronous methods for creating instances and selecting managed identity sources. (src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs) [1] [2]SendTokenRequestForManagedIdentityAsyncmethod to use the new asynchronousCreateAsyncmethod forManagedIdentityClient. (src/client/Microsoft.Identity.Client/Internal/Requests/ManagedIdentityAuthRequest.cs)Public API Updates:
GetManagedIdentitySourcemethod and introduced an asynchronousGetManagedIdentitySourceAsyncmethod inManagedIdentityApplication. (src/client/Microsoft.Identity.Client/ManagedIdentityApplication.cs)ImdsV2managed identity source and the asynchronous method in variousPublicAPI.Unshipped.txtfiles. (src/client/Microsoft.Identity.Client/PublicApi/net462/PublicAPI.Unshipped.txt,src/client/Microsoft.Identity.Client/PublicApi/net472/PublicAPI.Unshipped.txt,src/client/Microsoft.Identity.Client/PublicApi/net8.0-android/PublicAPI.Unshipped.txt)