Skip to content

Commit

Permalink
use mutex to protect config init
Browse files Browse the repository at this point in the history
  • Loading branch information
isra-fel committed Jun 15, 2022
1 parent cdeb8f0 commit 87d1543
Show file tree
Hide file tree
Showing 18 changed files with 718 additions and 483 deletions.
2 changes: 1 addition & 1 deletion src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ public void OnImport()
try
{
#endif
AzureSessionInitializer.InitializeAzureSession();
AzureSessionInitializer.InitializeAzureSession(WriteInitializationWarnings);
AzureSessionInitializer.MigrateAdalCache(AzureSession.Instance, GetAzureContextContainer, WriteInitializationWarnings);
#if DEBUG
if (!TestMockSupport.RunningMocked)
Expand Down
1 change: 1 addition & 0 deletions src/Accounts/Accounts/ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
-->

## Upcoming Release
* Fixed an issue that Az.Accounts may fail to be imported in parallel PowerShell processes. [#18321]
* Fixed an issue that Az.Accounts failed to be imported if multiple environment variables, which only differ by case, are set. [#18304]

## Version 2.8.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.Commands.Common.Authentication.Config;
using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.PowerShell.Common.Config;
using Microsoft.Rest.ClientRuntime.Azure.TestFramework;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Xunit;
using System.Linq;
using Xunit;

namespace Microsoft.Azure.Authentication.Test.Config
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,21 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.Commands.Common.Authentication.Config;
using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.PowerShell.Common.Config;
using Microsoft.Rest.ClientRuntime.Azure.TestFramework;
using Microsoft.WindowsAzure.Commands.Common;
using System;
using System.Linq;
using Xunit;
using Microsoft.Azure.PowerShell.Common.Config;

namespace Microsoft.Azure.Authentication.Test.Config
{
public class ConfigDefinitionTests : ConfigTestsBase
{
[Fact]
[Trait(TestTraits.AcceptanceType, TestTraits.CheckIn)]
public void CanValidateInput() {
public void CanValidateInput()
{
const string boolKey = "BoolKey";
var boolConfig = new SimpleTypedConfig<bool>(boolKey, "", false);
var rangedIntConfig = new RangedConfig();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,11 @@ protected IConfigManager GetConfigManagerWithInitState(Action<MockDataStore, str

string configPath = Path.GetRandomFileName();
var mockDataStore = new MockDataStore();
mockDataStore.WriteFile(configPath, @"{}");
configFileWriter(mockDataStore, configPath);
var environmentVariables = new MockEnvironmentVariableProvider();
envVarWriter(environmentVariables);
ConfigInitializer ci = new ConfigInitializer(new List<string>() { configPath })
{
DataStore = mockDataStore,
EnvironmentVariableProvider = environmentVariables
};
IConfigManager icm = ci.GetConfigManager();
IConfigManager icm = new DefaultConfigManager(configPath, mockDataStore, environmentVariables);
foreach (var configDefinition in config)
{
icm.RegisterConfig(configDefinition);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.Commands.Common.Authentication.Config;
using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.PowerShell.Common.Config;
using Microsoft.Rest.ClientRuntime.Azure.TestFramework;
using System.Linq;
Expand Down Expand Up @@ -346,7 +346,7 @@ public void ListDefinitionsShouldBeDictOrder()
var config2 = new SimpleTypedConfig<int>(key2, "", 0);
const string key3 = "key3";
var config3 = new SimpleTypedConfig<int>(key3, "", 0);

// register using wrong order
var icm = GetConfigManager(config2, config1, config3);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.Commands.Common.Authentication.Config;
using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.PowerShell.Common.Config;
using Microsoft.Rest.ClientRuntime.Azure.TestFramework;
using System;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.Commands.Common.Authentication.Config;
using Microsoft.Azure.Commands.Common.Exceptions;
using Microsoft.Azure.PowerShell.Common.Config;
using Microsoft.Rest.ClientRuntime.Azure.TestFramework;
using Moq;
Expand Down Expand Up @@ -176,7 +176,7 @@ public void ShouldNotUpdateConfigIfSideEffectThrows()
public void ShouldThrowIfAppliesToIsWrong()
{
var key = "OnlyAppliesToAz";
var config = new SimpleTypedConfig<bool>(key, "", true, null, new AppliesTo[] {AppliesTo.Az});
var config = new SimpleTypedConfig<bool>(key, "", true, null, new AppliesTo[] { AppliesTo.Az });
var icm = GetConfigManager(config);
Assert.Throws<AzPSArgumentException>(() => icm.UpdateConfig(new UpdateConfigOptions(key, true, ConfigScope.CurrentUser) { AppliesTo = "Az.Accounts" }));
}
Expand Down
46 changes: 38 additions & 8 deletions src/Accounts/Authentication/AzureSessionInitializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

using TraceLevel = System.Diagnostics.TraceLevel;
using System.Collections.Generic;
using System.Threading;

namespace Microsoft.Azure.Commands.Common.Authentication
{
Expand All @@ -43,9 +44,9 @@ public static class AzureSessionInitializer
/// <summary>
/// Initialize the azure session if it is not already initialized
/// </summary>
public static void InitializeAzureSession()
public static void InitializeAzureSession(Action<string> writeWarning = null)
{
AzureSession.Initialize(() => CreateInstance());
AzureSession.Initialize(() => CreateInstance(null, writeWarning));
}

/// <summary>
Expand Down Expand Up @@ -137,7 +138,7 @@ public static void MigrateAdalCache(IAzureSession session, Func<IAzureContextCon
new AdalTokenMigrator(adalData, getContextContainer).MigrateFromAdalToMsal();
}
}
catch(Exception e)
catch (Exception e)
{
writeWarning(Resources.FailedToMigrateAdal2Msal.FormatInvariant(e.Message));
}
Expand Down Expand Up @@ -206,7 +207,7 @@ static void InitializeDataCollection(IAzureSession session)
session.RegisterComponent(DataCollectionController.RegistryKey, () => DataCollectionController.Create(session));
}

static IAzureSession CreateInstance(IDataStore dataStore = null)
static IAzureSession CreateInstance(IDataStore dataStore = null, Action<string> writeWarning = null)
{
string profilePath = Path.Combine(
Environment.GetFolderPath(Environment.SpecialFolder.UserProfile),
Expand Down Expand Up @@ -239,23 +240,52 @@ static IAzureSession CreateInstance(IDataStore dataStore = null)
session.TokenCacheDirectory = autoSave.CacheDirectory;
session.TokenCacheFile = autoSave.CacheFile;

InitializeConfigs(session, profilePath);
InitializeConfigs(session, profilePath, writeWarning);
InitializeDataCollection(session);
session.RegisterComponent(HttpClientOperationsFactory.Name, () => HttpClientOperationsFactory.Create());
session.TokenCache = session.TokenCache ?? new AzureTokenCache();
return session;
}

private static void InitializeConfigs(AzureSession session, string profilePath)
private static void InitializeConfigs(AzureSession session, string profilePath, Action<string> writeWarning)
{
if (writeWarning == null) { writeWarning = (string s) => { }; };
var fallbackList = new List<string>()
{
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".Azure", "PSConfig.json"),
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), ".Azure", "PSConfig.json")
};
ConfigInitializer configInitializer = new ConfigInitializer(fallbackList);
configInitializer.MigrateConfigs(profilePath);
configInitializer.InitializeForAzureSession(session);

using (var mutex = new Mutex(false, @"Global\AzurePowerShellConfigInit"))
{
if (mutex.WaitOne(1000))
{
// regular initialization
try
{
configInitializer.MigrateConfigs(profilePath);
configInitializer.Initialize(session);
return; // done, return.
}
catch (Exception ex)
{
writeWarning($"[AzureSessionInitializer] Failed to initialize the configs, reason: {ex.Message}.");
}
finally
{
mutex.ReleaseMutex();
}
}
else
{
writeWarning($"[AzureSessionInitializer] Timed out when initializing the configs.");
}

// initialize in safe mode and do not try to migrate configs
writeWarning($"[AzureSessionInitializer] Config manager will be re-initialized in safe mode. All configs will have only default values.");
configInitializer.SafeInitialize(session);
}
}

public class AdalSession : AzureSession
Expand Down
83 changes: 46 additions & 37 deletions src/Accounts/Authentication/Config/ConfigInitializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
using Microsoft.Azure.Commands.Common.Authentication.Properties;
using Microsoft.Azure.Commands.Shared.Config;
using Microsoft.Azure.PowerShell.Common.Config;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using System;
using System.Collections.Generic;
Expand All @@ -34,11 +33,10 @@ namespace Microsoft.Azure.Commands.Common.Authentication.Config
internal class ConfigInitializer
{
internal IDataStore DataStore { get; set; } = new DiskDataStore();
private static readonly object _fsLock = new object();

internal IEnvironmentVariableProvider EnvironmentVariableProvider { get; set; } = new DefaultEnvironmentVariableProvider();

public string ConfigPath
private string ConfigPath
{
get
{
Expand All @@ -48,6 +46,10 @@ public string ConfigPath
}
return _cachedConfigPath;
}
set
{
_cachedConfigPath = value;
}
}
private string _cachedConfigPath;
private readonly IEnumerable<string> _configPathCandidates;
Expand Down Expand Up @@ -88,16 +90,7 @@ private string GetPathToConfigFile(IEnumerable<string> paths)
continue;
}
}
throw new ApplicationException($"Failed to store the config file. Please make sure any one of the following paths is accessible: {string.Join(", ", paths)}");
}

internal IConfigManager GetConfigManager()
{
lock (_fsLock)
{
ValidateConfigFile();
}
return new ConfigManager(ConfigPath, DataStore, EnvironmentVariableProvider);
throw new ApplicationException($"Failed to create the config file. Please make sure any one of the following paths is accessible: {string.Join(", ", paths)}");
}

private void ValidateConfigFileContent()
Expand Down Expand Up @@ -146,46 +139,62 @@ private void ResetConfigFileToDefault()
}
}

// todo: tests initializes configs in a different way. Maybe there should be an abstraction IConfigInitializer and two concrete classes ConfigInitializer + TestConfigInitializer
internal void InitializeForAzureSession(AzureSession session)
/// <summary>
/// Initializes the config manager and registers it to <see cref="AzureSession"/>.
/// </summary>
/// <param name="session"></param>
public void Initialize(IAzureSession session)
{
ValidateConfigFile();
Initialize(session, new DefaultConfigManager(ConfigPath, DataStore, EnvironmentVariableProvider));
}

private void Initialize(IAzureSession session, IConfigManager configManager)
{
IConfigManager configManager = GetConfigManager();
session.RegisterComponent(nameof(IConfigManager), () => configManager);
session.RegisterComponent(nameof(IConfigManager), () => configManager, true);
RegisterConfigs(configManager);
configManager.BuildConfig();
}

/// <summary>
/// Initializes the config manager with limited capabilities but in a safe manner.
/// Registers it to <see cref="AzureSession"/>.
/// </summary>
/// <param name="session"></param>
public void SafeInitialize(IAzureSession session)
{
ConfigPath = "";
Initialize(session, new SafeConfigManager());
}

/// <summary>
/// Migrates independent configs to the new config framework.
/// </summary>
/// <param name="profilePath">Path of session profile where old config files are stored.</param>
internal void MigrateConfigs(string profilePath)
{
lock (_fsLock)
// Migrate data collection config
string dataCollectionConfigPath = Path.Combine(profilePath, AzurePSDataCollectionProfile.DefaultFileName);
const string legacyConfigKey = "enableAzureDataCollection";
// Migrate only when:
// 1. Old config file exists
// 2. New config file does not exist
if (DataStore.FileExists(dataCollectionConfigPath) && _configPathCandidates.All(path => !DataStore.FileExists(path)))
{
// Migrate data collection config
string dataCollectionConfigPath = Path.Combine(profilePath, AzurePSDataCollectionProfile.DefaultFileName);
const string legacyConfigKey = "enableAzureDataCollection";
// Migrate only when:
// 1. Old config file exists
// 2. New config file does not exist
if (DataStore.FileExists(dataCollectionConfigPath) && _configPathCandidates.All(path => !DataStore.FileExists(path)))
try
{
try
string json = DataStore.ReadFileAsText(dataCollectionConfigPath);
JObject root = JObject.Parse(json);
if (root.TryGetValue(legacyConfigKey, out JToken jToken))
{
string json = DataStore.ReadFileAsText(dataCollectionConfigPath);
JObject root = JObject.Parse(json);
if (root.TryGetValue(legacyConfigKey, out JToken jToken))
{
bool enabled = ((bool)jToken);
new JsonConfigWriter(ConfigPath, DataStore).Update(ConfigPathHelper.GetPathOfConfig(ConfigKeys.EnableDataCollection), enabled);
}
}
catch (Exception)
{
// do not throw for file IO exceptions
bool enabled = ((bool)jToken);
new JsonConfigWriter(ConfigPath, DataStore).Update(ConfigPathHelper.GetPathOfConfig(ConfigKeys.EnableDataCollection), enabled);
}
}
catch (Exception)
{
// do not throw for file IO exceptions
}
}
}

Expand Down
Loading

0 comments on commit 87d1543

Please sign in to comment.