From 40d9cd3bc7839e1c3fe78b4bfccf32fd6001c9bd Mon Sep 17 00:00:00 2001 From: Ruben Guerrero Date: Wed, 10 May 2023 11:34:32 -0700 Subject: [PATCH] Allow ${WinGetConfigRoot} variable expansion (#3237) Allow settings to use the `$WinGetConfigRoot` variable. This variable means the directory containing the configuration file and is helpful for units that want reference paths that are relative to the location of the configuration file. For example, our test resource can use it like this: ``` properties: configurationVersion: 0.2 resources: - resource: xSimpleTestResource/SimpleFileResource settings: path: '${WinGetConfigRoot}\..\test.txt' content: 'Ella baila sola' ``` To make it PowerShelly, the variable expansion is done ignoring case. Since this is still in preview, the name of the variable can change when `winget configure` release. --- .github/actions/spelling/expect.txt | 1 + .../Public/AppInstallerErrors.h | 1 + .../Exceptions/ErrorCodes.cs | 5 ++ .../UnitSettingConfigRootException.cs | 39 +++++++++++ .../Helpers/ConfigurationUnitAndResource.cs | 8 +-- .../Helpers/ConfigurationUnitInternal.cs | 70 +++++++++++++++++++ .../Set/ConfigurationSetProcessor.cs | 4 +- .../Unit/ConfigurationUnitProcessor.cs | 6 +- .../Tests/ConfigurationDetailsTests.cs | 2 +- .../Tests/ConfigurationSetProcessorTests.cs | 2 +- .../Tests/ConfigurationUnitInternalTests.cs | 61 +++++++++++++++- .../Tests/ConfigurationUnitProcessorTests.cs | 1 + 12 files changed, 186 insertions(+), 14 deletions(-) create mode 100644 src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index ce8e06d60e..930e9f6146 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -431,6 +431,7 @@ winapifamily windir windowsdeveloper winerror +wingetconfigroot wingetcreate wingetdev wingetutil diff --git a/src/AppInstallerSharedLib/Public/AppInstallerErrors.h b/src/AppInstallerSharedLib/Public/AppInstallerErrors.h index 8e6fa30524..fdd0183654 100644 --- a/src/AppInstallerSharedLib/Public/AppInstallerErrors.h +++ b/src/AppInstallerSharedLib/Public/AppInstallerErrors.h @@ -178,6 +178,7 @@ #define WINGET_CONFIG_ERROR_UNIT_MODULE_CONFLICT ((HRESULT)0x8A15C107) #define WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE ((HRESULT)0x8A15C108) #define WINGET_CONFIG_ERROR_UNIT_INVOKE_INVALID_RESULT ((HRESULT)0x8A15C109) +#define WINGET_CONFIG_ERROR_UNIT_SETTING_CONFIG_ROOT ((HRESULT)0x8A15C110) namespace AppInstaller { diff --git a/src/Microsoft.Management.Configuration.Processor/Exceptions/ErrorCodes.cs b/src/Microsoft.Management.Configuration.Processor/Exceptions/ErrorCodes.cs index 5431a296bf..89b32996c8 100644 --- a/src/Microsoft.Management.Configuration.Processor/Exceptions/ErrorCodes.cs +++ b/src/Microsoft.Management.Configuration.Processor/Exceptions/ErrorCodes.cs @@ -60,5 +60,10 @@ internal static class ErrorCodes /// The unit returned an invalid result. /// internal const int WinGetConfigUnitInvokeInvalidResult = unchecked((int)0x8A15C109); + + /// + /// The unit contains a setting that requires config root. + /// + internal const int WinGetConfigUnitSettingConfigRoot = unchecked((int)0x8A15C110); } } diff --git a/src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs b/src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs new file mode 100644 index 0000000000..54aa4c051f --- /dev/null +++ b/src/Microsoft.Management.Configuration.Processor/Exceptions/UnitSettingConfigRootException.cs @@ -0,0 +1,39 @@ +// ----------------------------------------------------------------------------- +// +// Copyright (c) Microsoft Corporation. Licensed under the MIT License. +// +// ----------------------------------------------------------------------------- + +namespace Microsoft.Management.Configuration.Processor.Exceptions +{ + using System; + + /// + /// A setting uses the config root variable and the Path was not set in the ConfigurationSet. + /// + internal class UnitSettingConfigRootException : Exception + { + /// + /// Initializes a new instance of the class. + /// + /// Unit name. + /// Setting. + public UnitSettingConfigRootException(string unitName, string setting) + : base($"Unit: {unitName} Setting {setting} requires the ConfigurationSet Path") + { + this.HResult = ErrorCodes.WinGetConfigUnitSettingConfigRoot; + this.UnitName = unitName; + this.Setting = setting; + } + + /// + /// Gets the resource name. + /// + public string UnitName { get; } + + /// + /// Gets the setting that reference the config root variable. + /// + public string Setting { get; } + } +} diff --git a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitAndResource.cs b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitAndResource.cs index 5afa2d4fcf..645d298499 100644 --- a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitAndResource.cs +++ b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitAndResource.cs @@ -99,14 +99,12 @@ public string ResourceName } /// - /// TODO: Implement. - /// I am so sad because rs.SessionStateProxy.InvokeCommand.ExpandString doesn't work as I wanted. - /// PowerShell assumes all code passed to ExpandString is trusted and we cannot assume that. + /// Gets the settings of the unit. /// /// ValueSet with settings. - public ValueSet GetExpandedSettings() + public ValueSet GetSettings() { - return this.Unit.Settings; + return this.UnitInternal.GetExpandedSettings(); } } } diff --git a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs index 6a8b1dc654..1d91a4e0b2 100644 --- a/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs +++ b/src/Microsoft.Management.Configuration.Processor/Helpers/ConfigurationUnitInternal.cs @@ -6,9 +6,13 @@ namespace Microsoft.Management.Configuration.Processor.Helpers { + using System; using System.Collections.Generic; + using System.IO; using Microsoft.Management.Configuration.Processor.Constants; + using Microsoft.Management.Configuration.Processor.Exceptions; using Microsoft.PowerShell.Commands; + using Windows.Foundation.Collections; /// /// Wrapper around Configuration units and its directives. Creates a normalized directives map @@ -16,15 +20,20 @@ namespace Microsoft.Management.Configuration.Processor.Helpers /// internal class ConfigurationUnitInternal { + private const string ConfigRootVar = "${WinGetConfigRoot}"; + + private readonly string? configurationFileRootPath = null; private readonly Dictionary normalizedDirectives = new (); /// /// Initializes a new instance of the class. /// /// Configuration unit. + /// The configuration file path. /// Directives overlay. public ConfigurationUnitInternal( ConfigurationUnit unit, + string configurationFilePath, IReadOnlyDictionary? directivesOverlay = null) { this.Unit = unit; @@ -45,6 +54,16 @@ internal class ConfigurationUnitInternal this.GetDirective(DirectiveConstants.MaxVersion), this.GetDirective(DirectiveConstants.ModuleGuid)); } + + if (!string.IsNullOrEmpty(configurationFilePath)) + { + if (!File.Exists(configurationFilePath)) + { + throw new FileNotFoundException(configurationFilePath); + } + + this.configurationFileRootPath = Path.GetDirectoryName(configurationFilePath); + } } /// @@ -150,6 +169,57 @@ public string ToIdentifyingString() return null; } + /// + /// TODO: Implement for more variables. + /// I am so sad because rs.SessionStateProxy.InvokeCommand.ExpandString doesn't work as I wanted. + /// PowerShell assumes all code passed to ExpandString is trusted and we cannot assume that. + /// + /// ValueSet with settings. + public ValueSet GetExpandedSettings() + { + var valueSet = new ValueSet(); + foreach (var value in this.Unit.Settings) + { + if (value.Value is string) + { + // For now, we just expand config root. + valueSet.Add(value.Key, this.ExpandConfigRoot(value.Value as string, value.Key)); + } + else + { + valueSet.Add(value); + } + } + + return valueSet; + } + + private string? ExpandConfigRoot(string? value, string settingName) + { + if (!string.IsNullOrEmpty(value)) + { + // TODO: since we only support one variable, this only finds and replace + // ${WingetConfigRoot} if found in the string when the work of expanding + // string is done it should take into account other operators like the subexpression operator $() + if (value.Contains(ConfigRootVar, StringComparison.OrdinalIgnoreCase)) + { + if (string.IsNullOrEmpty(this.configurationFileRootPath)) + { + throw new UnitSettingConfigRootException(this.Unit.UnitName, settingName); + } + + if (this.configurationFileRootPath == null) + { + throw new ArgumentException(); + } + + return value.Replace(ConfigRootVar, this.configurationFileRootPath, StringComparison.OrdinalIgnoreCase); + } + } + + return value; + } + private void InitializeDirectives() { // Overlay directives have precedence. diff --git a/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs b/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs index 4a0cdc3a51..e11e0a3824 100644 --- a/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs +++ b/src/Microsoft.Management.Configuration.Processor/Set/ConfigurationSetProcessor.cs @@ -57,7 +57,7 @@ public ConfigurationSetProcessor(IProcessorEnvironment processorEnvironment, Con { try { - var configurationUnitInternal = new ConfigurationUnitInternal(unit, directivesOverlay); + var configurationUnitInternal = new ConfigurationUnitInternal(unit, this.configurationSet.Path, directivesOverlay); this.OnDiagnostics(DiagnosticLevel.Verbose, $"Creating unit processor for: {configurationUnitInternal.ToIdentifyingString()}..."); var dscResourceInfo = this.PrepareUnitForProcessing(configurationUnitInternal); @@ -87,7 +87,7 @@ public ConfigurationSetProcessor(IProcessorEnvironment processorEnvironment, Con { try { - var unitInternal = new ConfigurationUnitInternal(unit); + var unitInternal = new ConfigurationUnitInternal(unit, this.configurationSet.Path); this.OnDiagnostics(DiagnosticLevel.Verbose, $"Getting unit details [{detailLevel}] for: {unitInternal.ToIdentifyingString()}"); var dscResourceInfo = this.ProcessorEnvironment.GetDscResource(unitInternal); diff --git a/src/Microsoft.Management.Configuration.Processor/Unit/ConfigurationUnitProcessor.cs b/src/Microsoft.Management.Configuration.Processor/Unit/ConfigurationUnitProcessor.cs index 3fba8c04d8..ddc77a04f4 100644 --- a/src/Microsoft.Management.Configuration.Processor/Unit/ConfigurationUnitProcessor.cs +++ b/src/Microsoft.Management.Configuration.Processor/Unit/ConfigurationUnitProcessor.cs @@ -64,7 +64,7 @@ public GetSettingsResult GetSettings() try { result.Settings = this.processorEnvironment.InvokeGetResource( - this.unitResource.GetExpandedSettings(), + this.unitResource.GetSettings(), this.unitResource.ResourceName, this.unitResource.Module); } @@ -97,7 +97,7 @@ public TestSettingsResult TestSettings() try { bool testResult = this.processorEnvironment.InvokeTestResource( - this.unitResource.GetExpandedSettings(), + this.unitResource.GetSettings(), this.unitResource.ResourceName, this.unitResource.Module); @@ -132,7 +132,7 @@ public ApplySettingsResult ApplySettings() try { result.RebootRequired = this.processorEnvironment.InvokeSetResource( - this.unitResource.GetExpandedSettings(), + this.unitResource.GetSettings(), this.unitResource.ResourceName, this.unitResource.Module); } diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationDetailsTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationDetailsTests.cs index 25b674e2ca..7e8573352f 100644 --- a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationDetailsTests.cs +++ b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationDetailsTests.cs @@ -189,7 +189,7 @@ private ConfigurationUnit CreteConfigurationUnit() // This is easier than trying to mock sealed class from external code... var testEnv = this.fixture.PrepareTestProcessorEnvironment(true); - var dscResourceInfo = testEnv.GetDscResource(new ConfigurationUnitInternal(unit, null)); + var dscResourceInfo = testEnv.GetDscResource(new ConfigurationUnitInternal(unit, string.Empty, null)); var psModuleInfo = testEnv.GetAvailableModule(PowerShellHelpers.CreateModuleSpecification("xSimpleTestResource", "0.0.0.1")); if (dscResourceInfo is null || psModuleInfo is null) diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs index d1d18b7f71..2214f8c59e 100644 --- a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs +++ b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs @@ -694,7 +694,7 @@ private ConfigurationUnit CreteConfigurationUnit() { // This is easier than trying to mock sealed class from external code... var testEnv = this.fixture.PrepareTestProcessorEnvironment(true); - var dscResourceInfo = testEnv.GetDscResource(new ConfigurationUnitInternal(unit, null)); + var dscResourceInfo = testEnv.GetDscResource(new ConfigurationUnitInternal(unit, string.Empty, null)); var psModuleInfo = testEnv.GetAvailableModule(PowerShellHelpers.CreateModuleSpecification("xSimpleTestResource", "0.0.0.1")); if (dscResourceInfo is null || psModuleInfo is null) diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs index 7cf05f480f..e48f80b76b 100644 --- a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs +++ b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitInternalTests.cs @@ -8,10 +8,13 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests { using System; using System.Collections.Generic; + using System.IO; using System.Management.Automation; using Microsoft.Management.Configuration; + using Microsoft.Management.Configuration.Processor.Exceptions; using Microsoft.Management.Configuration.Processor.Helpers; using Microsoft.Management.Configuration.UnitTests.Fixtures; + using Microsoft.Management.Configuration.UnitTests.Helpers; using Xunit; using Xunit.Abstractions; @@ -73,7 +76,7 @@ public void GetDirectivesTest() { anotherDirective, overlayAnother }, }; - var unitInternal = new ConfigurationUnitInternal(unit, overlays); + var unitInternal = new ConfigurationUnitInternal(unit, string.Empty, overlays); var description = unitInternal.GetDirective(descriptionDirective); Assert.Equal(description, overlayDescription); @@ -107,7 +110,61 @@ public void GetVersion_BadVersion() unit.Directives.Add("version", "not a version"); Assert.Throws( - () => new ConfigurationUnitInternal(unit, null)); + () => new ConfigurationUnitInternal(unit, string.Empty, null)); + } + + /// + /// Verifies expansion of ConfigRoot. + /// + [Fact] + public void GetExpandedSettings_ConfigRoot() + { + using var tmpFile = new TempFile("fakeConfigFile.yml", content: "content"); + + var unit = new ConfigurationUnit(); + unit.Settings.Add("var1", @"$WinGetConfigRoot\this\is\a\path.txt"); + unit.Settings.Add("var2", @"${WinGetConfigRoot}\this\is\a\path.txt"); + unit.Settings.Add("var3", @"this\is\a\$WINGETCONFIGROOT\path.txt"); + unit.Settings.Add("var4", @"this\is\a\${WINGETCONFIGROOT}\path.txt"); + unit.Settings.Add("var5", @"this\is\a\path\$wingetconfigroot"); + unit.Settings.Add("var6", @"this\is\a\path\${wingetconfigroot}"); + + string configPath = tmpFile.FullFileName; + string? expectedPath = Path.GetDirectoryName(configPath); + var unitInternal = new ConfigurationUnitInternal(unit, configPath); + + var expandedSettings = unitInternal.GetExpandedSettings(); + + var var1 = expandedSettings["var1"]; + Assert.Equal(@"$WinGetConfigRoot\this\is\a\path.txt", var1 as string); + + var var2 = expandedSettings["var2"]; + Assert.Equal($@"{expectedPath}\this\is\a\path.txt", var2 as string); + + var var3 = expandedSettings["var3"]; + Assert.Equal(@"this\is\a\$WINGETCONFIGROOT\path.txt", var3 as string); + + var var4 = expandedSettings["var4"]; + Assert.Equal($@"this\is\a\{expectedPath}\path.txt", var4 as string); + + var var5 = expandedSettings["var5"]; + Assert.Equal(@"this\is\a\path\$wingetconfigroot", var5 as string); + + var var6 = expandedSettings["var6"]; + Assert.Equal($@"this\is\a\path\{expectedPath}", var6 as string); + } + + /// + /// Verifies throws when config root is not set. + /// + [Fact] + public void GetExpandedSetting_ConfigRoot_Throw() + { + var unit = new ConfigurationUnit(); + unit.Settings.Add("var2", @"${WinGetConfigRoot}\this\is\a\path.txt"); + + var unitInternal = new ConfigurationUnitInternal(unit, null!); + Assert.Throws(() => unitInternal.GetExpandedSettings()); } } } diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitProcessorTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitProcessorTests.cs index 0cdf6d9b87..467fa486a6 100644 --- a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitProcessorTests.cs +++ b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationUnitProcessorTests.cs @@ -362,6 +362,7 @@ private ConfigurationUnitAndResource CreateUnitResource(ConfigurationUnitIntent UnitName = resourceName, Intent = intent, }, + string.Empty, new Dictionary()), new DscResourceInfoInternal(resourceName, null, null)); }