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

rename process timeout options #35472

Merged
merged 3 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions sdk/identity/Azure.Identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
### Features Added

### Breaking Changes
- Renamed the developer credential options timeout settings as follows:
- `AzureCliCredential` to `AzureCliCredentialOptions.ProcessTimeout`
- `AzurePowerShellCredential` to `AzurePowerShellCredentialOptions.ProcessTimeout`
- `VisualStudioCredential` to `VisualStudioCredentialOptions.ProcessTimeout`
- `AzureDeveloperCliCredential` to `AzureDeveloperCliCredentialOptions.ProcessTimeout`

### Bugs Fixed
- Setting `DefaultAzureCredentialOptions.ExcludeWorkloadIdentityCredential` to `true` also excludes `TokenExchangeManagedIdentitySource` when using `DefaultAzureCredential` selects the `ManagedIdentityCredential`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public partial class AzureCliCredentialOptions : Azure.Identity.TokenCredentialO
{
public AzureCliCredentialOptions() { }
public System.Collections.Generic.IList<string> AdditionallyAllowedTenants { get { throw null; } }
public System.TimeSpan? CliProcessTimeout { get { throw null; } set { } }
public System.TimeSpan? ProcessTimeout { get { throw null; } set { } }
public string TenantId { get { throw null; } set { } }
}
public partial class AzureDeveloperCliCredential : Azure.Core.TokenCredential
Expand All @@ -75,7 +75,7 @@ public partial class AzureDeveloperCliCredentialOptions : Azure.Identity.TokenCr
{
public AzureDeveloperCliCredentialOptions() { }
public System.Collections.Generic.IList<string> AdditionallyAllowedTenants { get { throw null; } }
public System.TimeSpan? AzdCliProcessTimeout { get { throw null; } set { } }
public System.TimeSpan? ProcessTimeout { get { throw null; } set { } }
public string TenantId { get { throw null; } set { } }
}
public partial class AzurePowerShellCredential : Azure.Core.TokenCredential
Expand All @@ -89,7 +89,7 @@ public partial class AzurePowerShellCredentialOptions : Azure.Identity.TokenCred
{
public AzurePowerShellCredentialOptions() { }
public System.Collections.Generic.IList<string> AdditionallyAllowedTenants { get { throw null; } }
public System.TimeSpan? PowerShellProcessTimeout { get { throw null; } set { } }
public System.TimeSpan? ProcessTimeout { get { throw null; } set { } }
public string TenantId { get { throw null; } set { } }
}
public partial class ChainedTokenCredential : Azure.Core.TokenCredential
Expand Down Expand Up @@ -423,7 +423,7 @@ public partial class VisualStudioCredentialOptions : Azure.Identity.TokenCredent
public VisualStudioCredentialOptions() { }
public System.Collections.Generic.IList<string> AdditionallyAllowedTenants { get { throw null; } }
public string TenantId { get { throw null; } set { } }
public System.TimeSpan? VisualStudioProcessTimeout { get { throw null; } set { } }
public System.TimeSpan? ProcessTimeout { get { throw null; } set { } }
}
public partial class WorkloadIdentityCredential : Azure.Core.TokenCredential
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class AzureCliCredential : TokenCredential
internal const string Troubleshoot = "See the troubleshooting guide for more information. https://aka.ms/azsdk/net/identity/azclicredential/troubleshoot";
internal const string InteractiveLoginRequired = "Azure CLI could not login. Interactive login is required.";
internal const string CLIInternalError = "CLIInternalError: The command failed with an unexpected error. Here is the traceback:";
internal TimeSpan CliProcessTimeout { get; private set; }
internal TimeSpan ProcessTimeout { get; private set; }

// The default install paths are used to find Azure CLI if no path is specified. This is to prevent executing out of the current working directory.
private static readonly string DefaultPathWindows = $"{EnvironmentVariables.ProgramFilesX86}\\Microsoft SDKs\\Azure\\CLI2\\wbin;{EnvironmentVariables.ProgramFiles}\\Microsoft SDKs\\Azure\\CLI2\\wbin";
Expand Down Expand Up @@ -74,7 +74,7 @@ internal AzureCliCredential(CredentialPipeline pipeline, IProcessService process
_processService = processService ?? ProcessService.Default;
TenantId = options?.TenantId;
AdditionallyAllowedTenantIds = TenantIdResolver.ResolveAddionallyAllowedTenantIds((options as ISupportsAdditionallyAllowedTenants)?.AdditionallyAllowedTenants);
CliProcessTimeout = options?.CliProcessTimeout ?? TimeSpan.FromSeconds(13);
ProcessTimeout = options?.ProcessTimeout ?? TimeSpan.FromSeconds(13);
}

/// <summary>
Expand Down Expand Up @@ -123,7 +123,7 @@ private async ValueTask<AccessToken> RequestCliAccessTokenAsync(bool async, Toke

GetFileNameAndArguments(resource, tenantId, out string fileName, out string argument);
ProcessStartInfo processStartInfo = GetAzureCliProcessStartInfo(fileName, argument);
using var processRunner = new ProcessRunner(_processService.Create(processStartInfo), CliProcessTimeout, _logPII, cancellationToken);
using var processRunner = new ProcessRunner(_processService.Create(processStartInfo), ProcessTimeout, _logPII, cancellationToken);

string output;
try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ public class AzureCliCredentialOptions : TokenCredentialOptions, ISupportsAdditi
/// <summary>
/// The Cli process timeout.
/// </summary>
public TimeSpan? CliProcessTimeout { get; set; }
public TimeSpan? ProcessTimeout { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class AzureDeveloperCliCredential : TokenCredential
internal const string Troubleshoot = "Please visit https://aka.ms/azure-dev for installation instructions and then, once installed, authenticate to your Azure account using 'azd login'.";
internal const string InteractiveLoginRequired = "Azure Developer CLI could not login. Interactive login is required.";
internal const string AzdCLIInternalError = "AzdCLIInternalError: The command failed with an unexpected error. Here is the traceback:";
internal TimeSpan AzdCliProcessTimeout { get; private set; }
internal TimeSpan ProcessTimeout { get; private set; }

private static readonly string DefaultWorkingDirWindows = Environment.GetFolderPath(Environment.SpecialFolder.System);
private const string DefaultWorkingDirNonWindows = "/bin/";
Expand Down Expand Up @@ -68,7 +68,7 @@ internal AzureDeveloperCliCredential(CredentialPipeline pipeline, IProcessServic
_processService = processService ?? ProcessService.Default;
TenantId = options?.TenantId;
AdditionallyAllowedTenantIds = TenantIdResolver.ResolveAddionallyAllowedTenantIds((options as ISupportsAdditionallyAllowedTenants)?.AdditionallyAllowedTenants);
AzdCliProcessTimeout = options?.AzdCliProcessTimeout ?? TimeSpan.FromSeconds(13);
ProcessTimeout = options?.ProcessTimeout ?? TimeSpan.FromSeconds(13);
}

/// <summary>
Expand Down Expand Up @@ -114,7 +114,7 @@ private async ValueTask<AccessToken> RequestCliAccessTokenAsync(bool async, Toke

GetFileNameAndArguments(context.Scopes, tenantId, out string fileName, out string argument);
ProcessStartInfo processStartInfo = GetAzureDeveloperCliProcessStartInfo(fileName, argument);
using var processRunner = new ProcessRunner(_processService.Create(processStartInfo), AzdCliProcessTimeout, _logPII, cancellationToken);
using var processRunner = new ProcessRunner(_processService.Create(processStartInfo), ProcessTimeout, _logPII, cancellationToken);

string output;
try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ public class AzureDeveloperCliCredentialOptions : TokenCredentialOptions, ISuppo
/// <summary>
/// The CLI process timeout.
/// </summary>
public TimeSpan? AzdCliProcessTimeout { get; set; }
public TimeSpan? ProcessTimeout { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class AzurePowerShellCredential : TokenCredential
{
private readonly CredentialPipeline _pipeline;
private readonly IProcessService _processService;
internal TimeSpan PowerShellProcessTimeout { get; private set; }
internal TimeSpan ProcessTimeout { get; private set; }
internal bool UseLegacyPowerShell { get; set; }

private const string Troubleshooting = "See the troubleshooting guide for more information. https://aka.ms/azsdk/net/identity/powershellcredential/troubleshoot";
Expand Down Expand Up @@ -66,7 +66,7 @@ internal AzurePowerShellCredential(AzurePowerShellCredentialOptions options, Cre
_pipeline = pipeline ?? CredentialPipeline.GetInstance(options);
_processService = processService ?? ProcessService.Default;
AdditionallyAllowedTenantIds = TenantIdResolver.ResolveAddionallyAllowedTenantIds((options as ISupportsAdditionallyAllowedTenants)?.AdditionallyAllowedTenants);
PowerShellProcessTimeout = options?.PowerShellProcessTimeout ?? TimeSpan.FromSeconds(10);
ProcessTimeout = options?.ProcessTimeout ?? TimeSpan.FromSeconds(10);
}

/// <summary>
Expand Down Expand Up @@ -144,7 +144,7 @@ private async ValueTask<AccessToken> RequestAzurePowerShellAccessTokenAsync(bool
ProcessStartInfo processStartInfo = GetAzurePowerShellProcessStartInfo(fileName, argument);
using var processRunner = new ProcessRunner(
_processService.Create(processStartInfo),
PowerShellProcessTimeout,
ProcessTimeout,
_logPII,
cancellationToken);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ public class AzurePowerShellCredentialOptions : TokenCredentialOptions, ISupport
/// <summary>
/// The Powershell process timeout.
/// </summary>
public TimeSpan? PowerShellProcessTimeout { get; set; } = TimeSpan.FromSeconds(10);
public TimeSpan? ProcessTimeout { get; set; } = TimeSpan.FromSeconds(10);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class VisualStudioCredential : TokenCredential
private readonly bool _logPII;
private readonly bool _logAccountDetails;

internal TimeSpan VisualStudioProcessTimeout { get; private set; }
internal TimeSpan ProcessTimeout { get; private set; }

/// <summary>
/// Creates a new instance of the <see cref="VisualStudioCredential"/>.
Expand All @@ -59,7 +59,7 @@ internal VisualStudioCredential(string tenantId, CredentialPipeline pipeline, IF
_fileSystem = fileSystem ?? FileSystemService.Default;
_processService = processService ?? ProcessService.Default;
AdditionallyAllowedTenantIds = TenantIdResolver.ResolveAddionallyAllowedTenantIds((options as ISupportsAdditionallyAllowedTenants)?.AdditionallyAllowedTenants);
VisualStudioProcessTimeout = options?.VisualStudioProcessTimeout ?? TimeSpan.FromSeconds(30);
ProcessTimeout = options?.ProcessTimeout ?? TimeSpan.FromSeconds(30);
}

/// <inheritdoc />
Expand Down Expand Up @@ -140,7 +140,7 @@ private async Task<AccessToken> RunProcessesAsync(List<ProcessStartInfo> process
string output = string.Empty;
try
{
using var processRunner = new ProcessRunner(_processService.Create(processStartInfo), VisualStudioProcessTimeout, _logPII, cancellationToken);
using var processRunner = new ProcessRunner(_processService.Create(processStartInfo), ProcessTimeout, _logPII, cancellationToken);
output = async
? await processRunner.RunAsync().ConfigureAwait(false)
: processRunner.Run();
Expand All @@ -152,7 +152,7 @@ private async Task<AccessToken> RunProcessesAsync(List<ProcessStartInfo> process
}
catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested)
{
exceptions.Add(new CredentialUnavailableException($"Process \"{processStartInfo.FileName}\" has failed to get access token in {VisualStudioProcessTimeout.TotalSeconds} seconds."));
exceptions.Add(new CredentialUnavailableException($"Process \"{processStartInfo.FileName}\" has failed to get access token in {ProcessTimeout.TotalSeconds} seconds."));
}
catch (JsonException exception)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ public string TenantId
/// <summary>
/// The VisualStudio process timeout.
/// </summary>
public TimeSpan? VisualStudioProcessTimeout { get; set; }
public TimeSpan? ProcessTimeout { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public virtual TokenCredential CreateAzureDeveloperCliCredential()

options.TenantId = Options.TenantId;

options.AzdCliProcessTimeout = Options.DeveloperCredentialTimeout;
options.ProcessTimeout = Options.DeveloperCredentialTimeout;

return new AzureDeveloperCliCredential(Pipeline, default, options);
}
Expand All @@ -173,7 +173,7 @@ public virtual TokenCredential CreateAzureCliCredential()

options.TenantId = Options.TenantId;

options.CliProcessTimeout = Options.DeveloperCredentialTimeout;
options.ProcessTimeout = Options.DeveloperCredentialTimeout;

return new AzureCliCredential(Pipeline, default, options);
}
Expand All @@ -184,7 +184,7 @@ public virtual TokenCredential CreateVisualStudioCredential()

options.TenantId = Options.VisualStudioTenantId;

options.VisualStudioProcessTimeout = Options.DeveloperCredentialTimeout;
options.ProcessTimeout = Options.DeveloperCredentialTimeout;

return new VisualStudioCredential(Options.VisualStudioTenantId, Pipeline, default, default, options);
}
Expand All @@ -204,7 +204,7 @@ public virtual TokenCredential CreateAzurePowerShellCredential()

options.TenantId = Options.TenantId;

options.PowerShellProcessTimeout = Options.DeveloperCredentialTimeout;
options.ProcessTimeout = Options.DeveloperCredentialTimeout;

return new AzurePowerShellCredential(options, Pipeline, default);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public void ConfigureCliProcessTimeout_ProcessTimeout()
AzureCliCredential credential = InstrumentClient(
new AzureCliCredential(CredentialPipeline.GetInstance(null),
new TestProcessService(testProcess),
new AzureCliCredentialOptions() { CliProcessTimeout = TimeSpan.Zero }));
new AzureCliCredentialOptions() { ProcessTimeout = TimeSpan.Zero }));
var ex = Assert.ThrowsAsync<AuthenticationFailedException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
Assert.AreEqual(AzureCliCredential.AzureCliTimeoutError, ex.Message);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public void ConfigureCliProcessTimeout_ProcessTimeout()
AzureDeveloperCliCredential credential = InstrumentClient(
new AzureDeveloperCliCredential(CredentialPipeline.GetInstance(null),
new TestProcessService(testProcess),
new AzureDeveloperCliCredentialOptions() { AzdCliProcessTimeout = TimeSpan.Zero }));
new AzureDeveloperCliCredentialOptions() { ProcessTimeout = TimeSpan.Zero }));
var ex = Assert.ThrowsAsync<AuthenticationFailedException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
Assert.AreEqual(AzureDeveloperCliCredential.AzdCliTimeoutError, ex.Message);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public void ConfigurePowershellProcessTimeout_ProcessTimeout()
var testProcess = new TestProcess { Timeout = 10000 };
AzurePowerShellCredential credential = InstrumentClient(
new AzurePowerShellCredential(
new AzurePowerShellCredentialOptions() { PowerShellProcessTimeout = TimeSpan.Zero },
new AzurePowerShellCredentialOptions() { ProcessTimeout = TimeSpan.Zero },
CredentialPipeline.GetInstance(null),
new TestProcessService(testProcess)));
var ex = Assert.ThrowsAsync<AuthenticationFailedException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public void ValidateVisualStudioOptionsHonored([Values] bool setTenantId, [Value

VisualStudioCredential cred = (VisualStudioCredential)factory.CreateVisualStudioCredential();

Assert.AreEqual(expTimeout ?? TimeSpan.FromSeconds(30), cred.VisualStudioProcessTimeout);
Assert.AreEqual(expTimeout ?? TimeSpan.FromSeconds(30), cred.ProcessTimeout);
Assert.AreEqual(expVisualStudioTenantId ?? expTenantId, cred.TenantId);
CollectionAssert.AreEquivalent(expAdditionallyAllowedTenants, cred.AdditionallyAllowedTenantIds);
}
Expand Down Expand Up @@ -218,13 +218,13 @@ public void ValidateCliOptionsHonored([Values] bool setTenantId, [Values] bool s

AzureCliCredential cred = (AzureCliCredential)factory.CreateAzureCliCredential();

Assert.AreEqual(expTimeout ?? TimeSpan.FromSeconds(13), cred.CliProcessTimeout);
Assert.AreEqual(expTimeout ?? TimeSpan.FromSeconds(13), cred.ProcessTimeout);
Assert.AreEqual(expTenantId, cred.TenantId);
CollectionAssert.AreEquivalent(expAdditionallyAllowedTenants, cred.AdditionallyAllowedTenantIds);

AzureDeveloperCliCredential credAzd = (AzureDeveloperCliCredential)factory.CreateAzureDeveloperCliCredential();

Assert.AreEqual(expTimeout ?? TimeSpan.FromSeconds(13), credAzd.AzdCliProcessTimeout);
Assert.AreEqual(expTimeout ?? TimeSpan.FromSeconds(13), credAzd.ProcessTimeout);
Assert.AreEqual(expTenantId, credAzd.TenantId);
CollectionAssert.AreEquivalent(expAdditionallyAllowedTenants, credAzd.AdditionallyAllowedTenantIds);
}
Expand Down Expand Up @@ -260,7 +260,7 @@ public void ValidatePowerShellOptionsHonored([Values] bool setTenantId, [Values]

AzurePowerShellCredential cred = (AzurePowerShellCredential)factory.CreateAzurePowerShellCredential();

Assert.AreEqual(expTimeout ?? TimeSpan.FromSeconds(10), cred.PowerShellProcessTimeout);
Assert.AreEqual(expTimeout ?? TimeSpan.FromSeconds(10), cred.ProcessTimeout);
Assert.AreEqual(expTenantId, cred.TenantId);
CollectionAssert.AreEquivalent(expAdditionallyAllowedTenants, cred.AdditionallyAllowedTenantIds);
}
Expand Down
Loading