Add MSI eMSI support #5771
Add MSI eMSI support #5771
Conversation
| @@ -93,22 +108,97 @@ public void AuthorizeRequest(Action<string, string> authTokenSetter) | |||
|
|
|||
| void GetOrRenewAuthentication() | |||
| { | |||
| if (_expiration - DateTime.UtcNow < TimeSpan.FromMinutes(5)) | |||
| if (_expiration - DateTime.UtcNow < TimeSpan.FromMinutes(4)) | |||
There was a problem hiding this comment.
@markcowl A comment here about the significance of the 4 minute to expiration time check would be great...
also what drove the reduction of the one minute here 5 to 4?
|
|
||
| public bool IsExpired() | ||
| { | ||
| return DateTime.UtcNow - CreationDate > (TimeSpan.FromSeconds(ExpiresIn) - TimeSpan.FromMinutes(5)); |
There was a problem hiding this comment.
Is the 5 minutes supposed to be a buffer time? So that we never end up with an expired token in our cache? Also is this related to the check for 4 mins in the code above?
If so, should we look at introducing a constant that is set to 5 and use ("said constant" -1) in the code above?
| @@ -26,7 +26,9 @@ namespace Microsoft.Azure.Commands.Common.Authentication.Factories | |||
| { | |||
| public class AuthenticationFactory : IAuthenticationFactory | |||
| { | |||
| public const string CommonAdTenant = "Common", DefaultMSILoginUri = "http://localhost:50342/oauth2/token"; | |||
| public const string CommonAdTenant = "Common", | |||
| DefaultMSILoginUri = "http://169.254.169.254/metadata/identity/oauth2/token", | |||
There was a problem hiding this comment.
Why are we using this specific IP and is it never going to change or should we use a DNS name?
There was a problem hiding this comment.
Because it is the defined ip of the metadata service on every VM host.
| public const string CommonAdTenant = "Common", DefaultMSILoginUri = "http://localhost:50342/oauth2/token"; | ||
| public const string CommonAdTenant = "Common", | ||
| DefaultMSILoginUri = "http://169.254.169.254/metadata/identity/oauth2/token", | ||
| DefaultBackupMSILoginUri = "http://localhost:50342/oauth2/token"; |
There was a problem hiding this comment.
Is this for testing purposes? How would we get back a valid token from the local host?
There was a problem hiding this comment.
No, these are the real endpoints. In the old implementation, there is a loopback endpoint that serves tokens, in the new one, the metadata token service (which has a reserved identical ip on each vm host) is used.
| public TimeSpan GetRetryInterval() | ||
| { | ||
| var result = _currentInterval; | ||
| _currentInterval = TimeSpan.FromTicks(_currentInterval.Ticks * _multiplier); |
There was a problem hiding this comment.
Can there be a case where we have multiple threads/processes retrying for the same endpoint at the same time? If so, we should look at adding a small amount of jitter to each retry... jitter helps relieve the case where multiple threads can end up retrying in sync.
|
(optimistic) on demand: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/644/ |
Description
Checklist
CONTRIBUTING.mdplatyPSmodule