Skip to content

Commit

Permalink
rename process timeout options (#35472)
Browse files Browse the repository at this point in the history
* rename process timeout options
  • Loading branch information
christothes authored Apr 11, 2023
1 parent 1d95087 commit 321d9ac
Show file tree
Hide file tree
Showing 17 changed files with 42 additions and 37 deletions.
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 @@ -422,8 +422,8 @@ public partial class VisualStudioCredentialOptions : Azure.Identity.TokenCredent
{
public VisualStudioCredentialOptions() { }
public System.Collections.Generic.IList<string> AdditionallyAllowedTenants { get { throw null; } }
public System.TimeSpan? ProcessTimeout { get { throw null; } set { } }
public string TenantId { get { throw null; } set { } }
public System.TimeSpan? VisualStudioProcessTimeout { 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

0 comments on commit 321d9ac

Please sign in to comment.