Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.56k stars 4.82k forks source link

[FEATURE REQ] ManagedIdentityCredential should fail fast when running outside of Azure #29471

Closed jonpayne closed 5 months ago

jonpayne commented 2 years ago

Library name

Azure.Identity

Please describe the feature.

When using DefaultAzureCredential outside of Azure, the ManagedIdentityCredential class retries four times before failing. This adds 8 to 10 seconds to token requests. ManagedIdentityCredential should use a heuristic to determine when the code is not running in Azure (e.g., environment variables, network errors, ...), and fail fast.

It is possible to work around this by disabling Managed Identity authentication:

var credential = new DefaultAzureCredential(new DefaultAzureCredentialOptions
{
    ExcludeManagedIdentityCredential = true
});

I do not like this solution as: a) It is hard to discover -- many users may just assume that token requests are slow b) It makes code less portable to Azure c) It is boilerplate code that has to be repeated in each application

jsquire commented 2 years ago

Thank you for your feedback. Tagging and routing to the team member best able to assist.

christothes commented 2 years ago

Hi @jonpayne
We're trying to collect more information about the environments that experience excessive delays due to the ManagedIdentityCredential retry behavior in DefaultAzureCredential. Could you please provide your OS version and version of .NET?

jonpayne commented 2 years ago

I'm using Windows 11 22H2 with .NET 6. I see the following times:

DefaultAzureCredential.GetToken                     11525.10 ms
EnvironmentCredential.GetToken                          1.86 ms
ManagedIdentityCredential.GetToken                   9356.77 ms
VisualStudioCredential.GetToken                      2153.34 ms

This is the code I used for timing:

using Azure.Core;
using Azure.Core.Diagnostics;
using Azure.Identity;
using System.Diagnostics.Tracing;

var tokenTimer = new TokenTimer();

new AzureEventSourceListener(
    (args, message) =>
    {
        if (args.EventName == "GetToken") tokenTimer.StartGetToken((string)(args.Payload![0])!);
        if (args.EventName == "GetTokenSucceeded" || args.EventName == "GetTokenFailed") tokenTimer.EndGetToken((string)(args.Payload![0])!);
    },
    EventLevel.Informational);

var credential = new DefaultAzureCredential();
await credential.GetTokenAsync(new TokenRequestContext(new[] { "https://management.core.windows.net/.default" }, null));
tokenTimer.Print();

class TokenTimer
{
    private readonly Dictionary<string, long> _startTimes = new();
    private readonly Dictionary<string, long> _endTimes = new();
    public void StartGetToken(string method) => _startTimes[method] = DateTime.UtcNow.Ticks;
    public void EndGetToken(string method) => _endTimes[method] = DateTime.UtcNow.Ticks;
    public void Print()
    {
        foreach (var t in _startTimes)
        {
            var ms = (_endTimes[t.Key] - t.Value) / 10000.0;
            Console.WriteLine($"{t.Key,-50}{ms,10:0.00} ms");
        }
    }
}
christothes commented 2 years ago

related #24767

heldersousa-planetpayment commented 2 years ago

@christothes I ran the code provided by @jonpayne and you can see the results below.

ManagedIdentityCredential is the main culprit when one runs this code from Visual Studio 2022, but VisualStudioCredential can also be really slow when developers are not using Visual Studio 2022 and do dotnet run from the command line (similar times if code is executed from VS Code).

Running from Visual Studio 2022

DefaultAzureCredential.GetToken                     10099.17 ms
EnvironmentCredential.GetToken                          5.40 ms
ManagedIdentityCredential.GetToken                   6128.55 ms
VisualStudioCredential.GetToken                      3880.83 ms

Running from the command line while Visual Studio 2022 was opened)

C:\Users\helder.sousa> dotnet run
DefaultAzureCredential.GetToken                      7921.18 ms
EnvironmentCredential.GetToken                          1.36 ms
ManagedIdentityCredential.GetToken                   6162.71 ms
VisualStudioCredential.GetToken                      1743.74 ms

Running from the command line (Visual Studio 2022 was closed)

C:\Users\helder.sousa> dotnet run
DefaultAzureCredential.GetToken                     16002.06 ms
EnvironmentCredential.GetToken                          1.79 ms
ManagedIdentityCredential.GetToken                   5434.74 ms
VisualStudioCredential.GetToken                      9548.20 ms
VisualStudioCodeCredential.GetToken                     3.20 ms
AzureCliCredential.GetToken                           993.89 ms

Local environment Windows 10 Enterprise (More system specs in this comment https://github.com/Azure/azure-sdk-for-net/issues/24767#issuecomment-1283153172)

<PackageReference Include="Azure.Identity" Version="1.7.0" />

C:\Users\helder.sousa> dotnet --version
6.0.400
Ro3A commented 1 year ago

What would be the most ideal solution to this issue with regards to using Authentication=Active Directory Default in a Azure SQL connection string?

Using this eliminates the need for an interceptor, so we do not currently have a place to set the DefaultAzureCredential properties to disable the various Identities that we don't need.

I can confirm that the DefaultAzureCredential.GetToken dependency call is taking 10 seconds most of the time, yet still intermittently. Certainly seems that caching is not working as well.

drdamour commented 1 year ago

this really sucks in azure functions because there's no way to override the DefaultAzureCredentialOptions() that AzureComponentFactory leverages to create credentals for a whole slew of various extensions (eventhubs, blobs) so every time it takes liek 10-20 seconds to auth to these services when it should take momennts.

cf https://github.com/Azure/azure-sdk-for-net/blob/af08a035a05b1e3d204cae4939536966dc00d313/sdk/extensions/Microsoft.Extensions.Azure/src/Internal/ClientFactory.cs#L185

I miss the previous libraries ability to put VS credential ahead of managed identity credential.

most devs on my teams are really confused by this delay and do really odd cache wrapper code that ends up being worthless, or they specify environment variables with clientid secret to get client credentials flow ahead of their local identities which is NOT what i want them to be doing.

if we can't fix this at the DefaultAzureCredential level can we at least fix it at the AzureComponentFactory level which has a handle to IServiceProvider so that it could look for a DefaultAzureCredentialsOptions outta the container?

Kiechlus commented 1 year ago

I have a similar problem. Using DefaultAzureCredentialOptions() locally is very slow.

Steps to reproduce:

The calls take five minutes for uploading to two different Service Bus and Storage accounts. If it runs on our AKS, it is fast. Using connection string for local development is not an option for us. There is also no caching or something like this, the next call takes again five minutes.

christothes commented 1 year ago

this really sucks in azure functions because there's no way to override the DefaultAzureCredentialOptions() that AzureComponentFactory leverages to create credentals for a whole slew of various extensions (eventhubs, blobs)

You should be able to customize the client construction via the ConfigureServices method.

drdamour commented 1 year ago

this really sucks in azure functions because there's no way to override the DefaultAzureCredentialOptions() that AzureComponentFactory leverages to create credentals for a whole slew of various extensions (eventhubs, blobs)

You should be able to customize the client construction via the ConfigureServices method.

@christothes not sure if you're suggesting HOW it should work, or suggesting it's already possible. If it's the latter, look at the code i linked, nothing is coming from DI, the library is just new()ing up the DefaultAzureCredentialOptions in it's execution stack thus no chance to work with injection on it. also this is being leveraged inside function app, not mvc.

christothes commented 1 year ago

This is how it works today. Sorry, here is a better link that describes the functions integration: https://learn.microsoft.com/en-us/azure/azure-functions/functions-reference?tabs=blob#configure-an-identity-based-connection . Essentially you should be able to configure any of the credential options via the functions config.

drdamour commented 1 year ago

This is how it works today. Sorry, here is a better link that describes the functions integration: https://learn.microsoft.com/en-us/azure/azure-functions/functions-reference?tabs=blob#configure-an-identity-based-connection . Essentially you should be able to configure any of the credential options via the functions config.

Im not seeing a way to opt out of the managed identity credential without having to specify the client id/secret to use client credentials flow..which is NOT what i want.

is there a __skip_managed_identity option that is not documented?

christothes commented 1 year ago

is there a __skip_managed_identity option that is not documented?

https://github.com/Azure/azure-sdk-for-net/blob/4b8a3621000968bc2f1a2d0a89c8ddc7dfb73efd/sdk/identity/Azure.Identity/src/Credentials/DefaultAzureCredentialOptions.cs#L224

drdamour commented 1 year ago

is there a __skip_managed_identity option that is not documented?

https://github.com/Azure/azure-sdk-for-net/blob/4b8a3621000968bc2f1a2d0a89c8ddc7dfb73efd/sdk/identity/Azure.Identity/src/Credentials/DefaultAzureCredentialOptions.cs#L224

yes for sure, but look at my linked code, it new's that up in the method...the question is if there's a config / appsetting host.json setting that assigns to that property to false?

correct me if i'm wrong, but the linked code does NOT retrieve the DefaultAzureCredentialOptions from a DI container, so i have no opportunity to customize it.

christothes commented 1 year ago

You should be able to do this via the UseCredential method:

services.AddAzureClients(builder => builder.UseCredential(new DefaultAzureCredential(options))
drdamour commented 1 year ago

You should be able to do this via the UseCredential method:

services.AddAzureClients(builder => builder.UseCredential(new DefaultAzureCredential(options))

have you actually tried this and achieved success? you keep pointing me to DI, but the code clearly does NOT get anything from a DI container.

christothes commented 1 year ago

You should be able to do this via the UseCredential method:

services.AddAzureClients(builder => builder.UseCredential(new DefaultAzureCredential(options))

have you actually tried this and achieved success? you keep pointing me to DI, but the code clearly does NOT get anything from a DI container.

I'm not sure which code you are referring to - but yes, your functions code would need to use DI to do this.

https://learn.microsoft.com/en-us/azure/azure-functions/functions-dotnet-dependency-injection#register-services

drdamour commented 1 year ago

You should be able to do this via the UseCredential method:

services.AddAzureClients(builder => builder.UseCredential(new DefaultAzureCredential(options))

have you actually tried this and achieved success? you keep pointing me to DI, but the code clearly does NOT get anything from a DI container.

I'm not sure which code you are referring to - but yes, your functions code would need to use DI to do this.

https://learn.microsoft.com/en-us/azure/azure-functions/functions-dotnet-dependency-injection#register-services

The code i linked that u originally replied to. Its not MY code its the code used in azure functions https://github.com/Azure/azure-sdk-for-net/blob/af08a035a05b1e3d204cae4939536966dc00d313/sdk/extensions/Microsoft.Extensions.Azure/src/Internal/ClientFactory.cs#L185

I had a feeling i was being jerked around here.. :(

JerryBlake commented 1 year ago

@christothes @drdamour,

Hi guys,

I have also hit this issue when using webjobs locally. What drdamour pointed out does seem like an issue, as webjobs extensions does use a DefaultAzureCredentialOptoins when if you don't set the web job specific configuration in your appsettings.json (or environment vars). This seems like a miss to me as they already are checking with their settings if a customer wants to use a managed identity but fail to remove those from the default check later.

What Chirstothes is saying does work around this issue. If you add the azure client directly with the DI builder (outside of the webjobs builder), web jobs will honor that client instead of creating a new one with the DefaultAzureCredentialsOptions. I think that is the disconnect here in this conversation.

Jaxelr commented 1 year ago

hey guys,

Also ran into this issue with my customers recently. These customers use our tools outside of Azure and we are soon planning to upgrade to Public Preview, it could be a bit cumbersome to ask our onboarding customers outside of their azure environments to remember to configure this prior of usage, especially because most of our customers will be using transient build machines to run our service. Is there any way of knowing when a permanent fix will be implemented on the library?

Cheers,

bacobart commented 1 year ago

I've added the below snippet to our services, which makes the ManagedIdentityCredential fail much faster. Obviously you'll lose the retry functionality. We haven't had any issues because of that (yet? ;) )

    DefaultAzureCredentialOptions.Default.Retry.MaxRetries = 0;
    DefaultAzureCredentialOptions.Default.Retry.Delay = TimeSpan.Zero;

The VisualStudio provider is also very slow, to disable that completely you can remove the TokenProvider in %LOCALAPPDATA%\.IdentityService\AzureServiceAuth\tokenprovider.json -- you will then need to use az login to be able to get a token. Please note that visual studio will add the entry again after a restart...

Jaxelr commented 1 year ago

@bacobart thanks, in my scenario that doesn't work because I want retries for our clients. For now, I'm using an exclusion of the MI validation for the customers who dont credentialize using MI via the DefaultAzureCredentialOptions prop, but like I mentioned before, its cumbersome to explain to hundreds of customers these specific types of scenarios.

Cowraider commented 1 year ago

I wasn't even able to use the workaround. We configured something like this:

services.AddAzureClients(builder => { builder.UseCredential(new DefaultAzureCredential( new DefaultAzureCredentialOptions { ExcludeEnvironmentCredential = true, ExcludeWorkloadIdentityCredential = true, ExcludeManagedIdentityCredential = true, ExcludeSharedTokenCacheCredential = true, ExcludeVisualStudioCredential = false, ExcludeVisualStudioCodeCredential = true, ExcludeAzureCliCredential = false, ExcludeAzurePowerShellCredential = true, ExcludeAzureDeveloperCliCredential = true, ExcludeInteractiveBrowserCredential = true, } )); builder.AddBlobServiceClient(new Uri(storageUri)); });

and it would still try to use Managed Identities and takes ~10 seconds (VS 2022, Windows 11),

Nevermind, I did not use "AzureWebJobsStorage": "UseDevelopmentStorage=true" for the Azure Function which is why it looked like it would ignore the Credential settings for the Blob storage.

btull89 commented 1 year ago

In my scenario it took 30 seconds for the DefaultAzureCredential to connect to KeyVault for local development. I moved to AzureCliCredential for local development and now it only takes 2 seconds. This is a considerable feedback loop optimization.

julioct commented 11 months ago

This issue was already impacting my team about a year ago and shockingly I found it is still an issue today as I tried DefaultAzureCredential in a new project.

To unblock my local VS Code dev experience I'm doing something like this:

public static IServiceCollection AddBlobClient(
    this IServiceCollection services,
    IConfiguration configuration,
    IHostEnvironment environment)
{
    var blobServiceUrl = configuration["GameStore:BlobServiceUrl"] ?? throw new Exception("Blob storage connection string is not set");

    TokenCredential credential = environment.IsDevelopment() ? new AzureCliCredential() : new ManagedIdentityCredential();

    services.AddSingleton(new BlobServiceClient(new Uri(blobServiceUrl), credential));

    return services;
}

Hope a proper fix is coming soon!

onionhammer commented 8 months ago

What about the speed of /msi/token inside of Azure? It takes over a second, oftentimes, to return a successful result

christothes commented 5 months ago

This should be significantly improved as of 1.11.3 and later.