Skip to content
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

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

Open
jonpayne opened this issue Jun 24, 2022 · 26 comments
Assignees
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved. needs-team-attention This issue needs attention from Azure service team or SDK team

Comments

@jonpayne
Copy link
Member

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

@ghost ghost added the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Jun 24, 2022
@azure-sdk azure-sdk added Azure.Identity Client This issue points to a problem in the data-plane of the library. needs-team-triage This issue needs the team to triage. labels Jun 24, 2022
@ghost ghost removed the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Jun 24, 2022
@jsquire jsquire added needs-team-attention This issue needs attention from Azure service team or SDK team and removed needs-team-triage This issue needs the team to triage. labels Jun 24, 2022
@jsquire
Copy link
Member

jsquire commented Jun 24, 2022

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

@christothes
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

related #24767

@heldersousa-planetpayment

@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 #24767 (comment))

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

C:\Users\helder.sousa> dotnet --version
6.0.400

@Ro3A
Copy link

Ro3A commented Mar 30, 2023

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
Copy link

drdamour commented Apr 7, 2023

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

var options = new DefaultAzureCredentialOptions();

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
Copy link

Kiechlus commented May 11, 2023

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

Steps to reproduce:

  • Login to Azure in VSCode
  • Assign appropriate roles (Service Bus, Storage Account) on user in Azure
  • Make calls that use Azure service

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
Copy link
Member

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
Copy link

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
Copy link
Member

christothes commented May 15, 2023

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
Copy link

drdamour commented May 16, 2023

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
Copy link
Member

is there a __skip_managed_identity option that is not documented?

public bool ExcludeManagedIdentityCredential { get; set; }

@drdamour
Copy link

is there a __skip_managed_identity option that is not documented?

public bool ExcludeManagedIdentityCredential { get; set; }

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
Copy link
Member

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

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

@drdamour
Copy link

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
Copy link
Member

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
Copy link

drdamour commented May 19, 2023

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

var options = new DefaultAzureCredentialOptions();

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

@JerryBlake
Copy link

@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
Copy link
Member

Jaxelr commented Oct 6, 2023

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
Copy link

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
Copy link
Member

Jaxelr commented Oct 10, 2023

@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
Copy link

Cowraider commented Oct 23, 2023

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
Copy link

btull89 commented Nov 3, 2023

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
Copy link

julioct commented Dec 17, 2023

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!

@pallavit pallavit added the feature-request This issue requires a new behavior in the product in order be resolved. label Dec 19, 2023
@onionhammer
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved. needs-team-attention This issue needs attention from Azure service team or SDK team
Projects
Status: Untriaged
Development

No branches or pull requests