commit a9a02eddfe56fd1f81c17fea5e799c2831d69902 parent 9f8fb4c9bc8dd30f83e6368bdcd17d90db27c45c Author: Ruben Guerrero <rubengu@microsoft.com> Date: Tue, 4 Apr 2023 20:35:23 -0700 Bug fixes (#3127) Set 2.* as MaximumVersion for PsDesiredStateConfiguration to explicitly not support DSC v3 DscModuleV2.cs Set ErrorAction to Stop for Invoke-DscResource calls. If the cmdlet write to the error stream, we will now throw an exception and add the error to the exception message. Support nested maps in settings. Diffstat:
14 files changed, 336 insertions(+), 33 deletions(-)
diff --git a/src/Microsoft.Management.Configuration.Processor/Constants/PowerShellConstants.cs b/src/Microsoft.Management.Configuration.Processor/Constants/PowerShellConstants.cs @@ -26,6 +26,7 @@ namespace Microsoft.Management.Configuration.Processor.Constants public const string PSDesiredStateConfigurationMinVersion = "2.0.6"; public const string PowerShellGet = "PowerShellGet"; public const string PowerShellGetMinVersion = "2.2.5"; + public const string PSDesiredStateConfigurationMaxVersion = "2.*"; } internal static class Commands diff --git a/src/Microsoft.Management.Configuration.Processor/DscModules/DscModuleV2.cs b/src/Microsoft.Management.Configuration.Processor/DscModules/DscModuleV2.cs @@ -11,8 +11,10 @@ namespace Microsoft.Management.Configuration.Processor.DscModule using System.Collections.ObjectModel; using System.Linq; using System.Management.Automation; + using System.Text; using Microsoft.Management.Configuration.Processor.DscResourcesInfo; using Microsoft.Management.Configuration.Processor.Exceptions; + using Microsoft.Management.Configuration.Processor.Extensions; using Microsoft.Management.Configuration.Processor.Helpers; using Microsoft.PowerShell.Commands; using Windows.Foundation.Collections; @@ -39,7 +41,8 @@ namespace Microsoft.Management.Configuration.Processor.DscModule { this.ModuleSpecification = PowerShellHelpers.CreateModuleSpecification( Modules.PSDesiredStateConfiguration, - minVersion: Modules.PSDesiredStateConfigurationMinVersion); + minVersion: Modules.PSDesiredStateConfigurationMinVersion, + maxVersion: Modules.PSDesiredStateConfigurationMaxVersion); } /// <inheritdoc/> @@ -95,9 +98,15 @@ namespace Microsoft.Management.Configuration.Processor.DscModule var getResult = pwsh.AddCommand(this.InvokeDscResourceCmd) .AddParameters(PrepareInvokeParameters(name, settings, moduleSpecification)) .AddParameter(Parameters.Method, DscMethods.Get) - .Invoke() + .InvokeAndStopOnError() .FirstOrDefault(); + string? errorMessage = pwsh.GetErrorMessage(); + if (errorMessage is not null) + { + throw new InvokeDscResourceGetException(name, moduleSpecification, errorMessage); + } + if (getResult is null) { throw new InvokeDscResourceGetException(name, moduleSpecification); @@ -132,9 +141,15 @@ namespace Microsoft.Management.Configuration.Processor.DscModule dynamic? testResult = pwsh.AddCommand(this.InvokeDscResourceCmd) .AddParameters(PrepareInvokeParameters(name, settings, moduleSpecification)) .AddParameter(Parameters.Method, DscMethods.Test) - .Invoke() + .InvokeAndStopOnError() .FirstOrDefault(); + string? errorMessage = pwsh.GetErrorMessage(); + if (errorMessage is not null) + { + throw new InvokeDscResourceTestException(name, moduleSpecification, errorMessage); + } + if (testResult is null || !TypeHelpers.PropertyWithTypeExists<bool>(testResult, InDesiredState)) { @@ -156,9 +171,15 @@ namespace Microsoft.Management.Configuration.Processor.DscModule dynamic? setResult = pwsh.AddCommand(this.InvokeDscResourceCmd) .AddParameters(PrepareInvokeParameters(name, settings, moduleSpecification)) .AddParameter(Parameters.Method, DscMethods.Set) - .Invoke() + .InvokeAndStopOnError() .FirstOrDefault(); + string? errorMessage = pwsh.GetErrorMessage(); + if (errorMessage is not null) + { + throw new InvokeDscResourceSetException(name, moduleSpecification, errorMessage); + } + if (setResult is null || !TypeHelpers.PropertyWithTypeExists<bool>(setResult, RebootRequired)) { @@ -173,11 +194,7 @@ namespace Microsoft.Management.Configuration.Processor.DscModule ValueSet settings, ModuleSpecification? moduleSpecification) { - var properties = new Hashtable(); - foreach (var setting in settings) - { - properties.Add(setting.Key, setting.Value); - } + Hashtable properties = settings.ToHashtable(); var parameters = new Dictionary<string, object>() { diff --git a/src/Microsoft.Management.Configuration.Processor/Exceptions/ImportModuleException.cs b/src/Microsoft.Management.Configuration.Processor/Exceptions/ImportModuleException.cs @@ -1,5 +1,5 @@ // ----------------------------------------------------------------------------- -// <copyright file="BlockedFileException.cs" company="Microsoft Corporation"> +// <copyright file="ImportModuleException.cs" company="Microsoft Corporation"> // Copyright (c) Microsoft Corporation. Licensed under the MIT License. // </copyright> // ----------------------------------------------------------------------------- diff --git a/src/Microsoft.Management.Configuration.Processor/Exceptions/InvokeDscResourceGetException.cs b/src/Microsoft.Management.Configuration.Processor/Exceptions/InvokeDscResourceGetException.cs @@ -20,7 +20,21 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions /// <param name="resourceName">Resource name.</param> /// <param name="module">Optional module.</param> public InvokeDscResourceGetException(string resourceName, ModuleSpecification? module) - : base($"Failed when calling `Get` for resource: {resourceName} [{module?.ToString() ?? "<no module>"}]") + : base(CreateMessage(resourceName, module, null)) + { + this.HResult = ErrorCodes.WinGetConfigUnitInvokeGet; + this.ResourceName = resourceName; + this.Module = module; + } + + /// <summary> + /// Initializes a new instance of the <see cref="InvokeDscResourceGetException"/> class. + /// </summary> + /// <param name="resourceName">Resource name.</param> + /// <param name="module">Optional module.</param> + /// <param name="message">Message.</param> + public InvokeDscResourceGetException(string resourceName, ModuleSpecification? module, string message) + : base(CreateMessage(resourceName, module, message)) { this.HResult = ErrorCodes.WinGetConfigUnitInvokeGet; this.ResourceName = resourceName; @@ -36,5 +50,16 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions /// Gets the module, if any. /// </summary> public ModuleSpecification? Module { get; } + + private static string CreateMessage(string resourceName, ModuleSpecification? module, string? message) + { + string result = $"Failed when calling `Get` for resource: {resourceName} [{module?.ToString() ?? "<no module>"}]"; + if (message != null) + { + result += $" Message: '{message}'"; + } + + return result; + } } } diff --git a/src/Microsoft.Management.Configuration.Processor/Exceptions/InvokeDscResourceSetException.cs b/src/Microsoft.Management.Configuration.Processor/Exceptions/InvokeDscResourceSetException.cs @@ -20,7 +20,21 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions /// <param name="resourceName">Resource name.</param> /// <param name="module">Optional module.</param> public InvokeDscResourceSetException(string resourceName, ModuleSpecification? module) - : base($"Failed when calling `Set` for resource: {resourceName} [{module?.ToString() ?? "<no module>"}]") + : base(CreateMessage(resourceName, module, null)) + { + this.HResult = ErrorCodes.WinGetConfigUnitInvokeSet; + this.ResourceName = resourceName; + this.Module = module; + } + + /// <summary> + /// Initializes a new instance of the <see cref="InvokeDscResourceSetException"/> class. + /// </summary> + /// <param name="resourceName">Resource name.</param> + /// <param name="module">Optional module.</param> + /// <param name="message">Message.</param> + public InvokeDscResourceSetException(string resourceName, ModuleSpecification? module, string message) + : base(CreateMessage(resourceName, module, message)) { this.HResult = ErrorCodes.WinGetConfigUnitInvokeSet; this.ResourceName = resourceName; @@ -36,5 +50,16 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions /// Gets the module, if any. /// </summary> public ModuleSpecification? Module { get; } + + private static string CreateMessage(string resourceName, ModuleSpecification? module, string? message) + { + string result = $"Failed when calling `Set` for resource: {resourceName} [{module?.ToString() ?? "<no module>"}]"; + if (message != null) + { + result += $" Message: '{message}'"; + } + + return result; + } } } diff --git a/src/Microsoft.Management.Configuration.Processor/Exceptions/InvokeDscResourceTestException.cs b/src/Microsoft.Management.Configuration.Processor/Exceptions/InvokeDscResourceTestException.cs @@ -20,7 +20,21 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions /// <param name="resourceName">Resource name.</param> /// <param name="module">Optional module.</param> public InvokeDscResourceTestException(string resourceName, ModuleSpecification? module) - : base($"Failed when calling `Test` for resource: {resourceName} [{module?.ToString() ?? "<no module>"}]") + : base(CreateMessage(resourceName, module, null)) + { + this.HResult = ErrorCodes.WinGetConfigUnitInvokeTest; + this.ResourceName = resourceName; + this.Module = module; + } + + /// <summary> + /// Initializes a new instance of the <see cref="InvokeDscResourceTestException"/> class. + /// </summary> + /// <param name="resourceName">Resource name.</param> + /// <param name="module">Optional module.</param> + /// <param name="message">Message.</param> + public InvokeDscResourceTestException(string resourceName, ModuleSpecification? module, string message) + : base(CreateMessage(resourceName, module, message)) { this.HResult = ErrorCodes.WinGetConfigUnitInvokeTest; this.ResourceName = resourceName; @@ -36,5 +50,16 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions /// Gets the module, if any. /// </summary> public ModuleSpecification? Module { get; } + + private static string CreateMessage(string resourceName, ModuleSpecification? module, string? message) + { + string result = $"Failed when calling `Test` for resource: {resourceName} [{module?.ToString() ?? "<no module>"}]"; + if (message != null) + { + result += $" Message: '{message}'"; + } + + return result; + } } } diff --git a/src/Microsoft.Management.Configuration.Processor/Extensions/PowerShellExtensions.cs b/src/Microsoft.Management.Configuration.Processor/Extensions/PowerShellExtensions.cs @@ -9,6 +9,8 @@ namespace Microsoft.Management.Configuration.Processor.Extensions using System.Collections.ObjectModel; using System.Management.Automation; using System.Text; + using Microsoft.Management.Configuration.Processor.Exceptions; + using System.Xml.Linq; using Microsoft.PowerShell.Commands; /// <summary> @@ -28,11 +30,7 @@ namespace Microsoft.Management.Configuration.Processor.Extensions ErrorActionPreference = ActionPreference.Stop, }; - var result = pwsh.Invoke(null, settings); - - pwsh.ValidateErrorStream(); - - return result; + return pwsh.Invoke(null, settings); } /// <summary> @@ -48,18 +46,15 @@ namespace Microsoft.Management.Configuration.Processor.Extensions ErrorActionPreference = ActionPreference.Stop, }; - var result = pwsh.Invoke<T>(null, settings); - - pwsh.ValidateErrorStream(); - - return result; + return pwsh.Invoke<T>(null, settings); } /// <summary> - /// Throws <see cref="WriteErrorException"/> if there are streams in the stream error. + /// Gets the error stream message if any. /// </summary> /// <param name="pwsh">PowerShell.</param> - public static void ValidateErrorStream(this PowerShell pwsh) + /// <returns>Error message. Null if none.</returns> + public static string? GetErrorMessage(this PowerShell pwsh) { if (pwsh.HadErrors) { @@ -69,8 +64,10 @@ namespace Microsoft.Management.Configuration.Processor.Extensions psStreamBuilder.AppendLine(line.ToString()); } - throw new WriteErrorException(psStreamBuilder.ToString()); + return psStreamBuilder.ToString(); } + + return null; } } } diff --git a/src/Microsoft.Management.Configuration.Processor/Extensions/ValueSetExtensions.cs b/src/Microsoft.Management.Configuration.Processor/Extensions/ValueSetExtensions.cs @@ -0,0 +1,42 @@ +// ----------------------------------------------------------------------------- +// <copyright file="ValueSetExtensions.cs" company="Microsoft Corporation"> +// Copyright (c) Microsoft Corporation. Licensed under the MIT License. +// </copyright> +// ----------------------------------------------------------------------------- + +namespace Microsoft.Management.Configuration.Processor.Extensions +{ + using System.Collections; + using Windows.Foundation.Collections; + + /// <summary> + /// Extensions for ValueSet. + /// </summary> + internal static class ValueSetExtensions + { + /// <summary> + /// Extension method to transform a ValueSet to a Hashtable. + /// </summary> + /// <param name="valueSet">Value set.</param> + /// <returns>A hashtable.</returns> + public static Hashtable ToHashtable(this ValueSet valueSet) + { + var hashtable = new Hashtable(); + + foreach (var keyValuePair in valueSet) + { + if (keyValuePair.Value is ValueSet) + { + ValueSet innerValueSet = (ValueSet)keyValuePair.Value; + hashtable.Add(keyValuePair.Key, innerValueSet.ToHashtable()); + } + else + { + hashtable.Add(keyValuePair.Key, keyValuePair.Value); + } + } + + return hashtable; + } + } +} diff --git a/src/Microsoft.Management.Configuration.Processor/Helpers/PowerShellHelpers.cs b/src/Microsoft.Management.Configuration.Processor/Helpers/PowerShellHelpers.cs @@ -15,6 +15,8 @@ namespace Microsoft.Management.Configuration.Processor.Helpers /// </summary> internal static class PowerShellHelpers { + private const string MaxRange = "999999999"; + /// <summary> /// Creates a module specification object. /// </summary> @@ -56,7 +58,14 @@ namespace Microsoft.Management.Configuration.Processor.Helpers if (!string.IsNullOrEmpty(maxVersion)) { - moduleInfo.Add(Parameters.MaximumVersion, maxVersion); + // For some reason, the constructor of ModuleSpecification that takes + // a hashtable calls ModuleCmdletBase.GetMaximumVersion. This method will + // validate the max version and replace * for 999999999 only if its the last + // char in the string. But then the returned value is not assigned to the + // ModuleSpecification's MaximumVersion property. If we want to set a + // MaximumVersion with a wildcard and pass this to Install-Module it will + // fail with "Cannot convert value 'x.*' to type 'System.Version'." + moduleInfo.Add(Parameters.MaximumVersion, maxVersion.Replace("*", MaxRange)); } if (!string.IsNullOrEmpty(guid)) diff --git a/src/Microsoft.Management.Configuration.UnitTests/TestCollateral/PowerShellModules/xSimpleTestResource/xSimpleTestResource.psd1 b/src/Microsoft.Management.Configuration.UnitTests/TestCollateral/PowerShellModules/xSimpleTestResource/xSimpleTestResource.psd1 @@ -24,6 +24,7 @@ DscResourcesToExport = @( 'SimpleTestResource' 'SimpleTestResourceThrows' 'SimpleTestResourceError' + 'SimpleTestResourceTypes' ) HelpInfoURI = 'https://www.contoso.com/help' diff --git a/src/Microsoft.Management.Configuration.UnitTests/TestCollateral/PowerShellModules/xSimpleTestResource/xSimpleTestResource.psm1 b/src/Microsoft.Management.Configuration.UnitTests/TestCollateral/PowerShellModules/xSimpleTestResource/xSimpleTestResource.psm1 @@ -168,4 +168,95 @@ class SimpleTestResourceError { Write-Error "Error in Set" } -}- \ No newline at end of file +} + +[DscResource()] +class SimpleTestResourceTypes +{ + [DscProperty(Key)] + [string] $key + + [DscProperty()] + [boolean] $boolProperty + + [DscProperty()] + [int] $intProperty; + + [DscProperty()] + [double] $doubleProperty; + + [DscProperty()] + [char] $charProperty; + + [DscProperty()] + [Hashtable] $hashtableProperty; + + [SimpleTestResourceTypes] Get() + { + $result = @{ + key = "SimpleTestResourceTypesKey" + boolProperty = $false + intProperty = 0 + doubleProperty = 0.0 + charProperty = 'z' + hashtableProperty = @{} + } + return $result + } + + [bool] Test() + { + # Because we can't get the error stream from a class based resource, I throw so is easier to know if + # there's something wrong. + if ($this.boolProperty -ne $true) + { + throw "Failed boolProperty" + } + + if ($this.intProperty -ne 3) + { + throw "Failed intProperty. Got $($this.intProperty)" + } + + if ($this.doubleProperty -ne -9.876) + { + throw "Failed doubleProperty Got $($this.doubleProperty)" + } + + if ($this.charProperty -ne 'f') + { + throw "Failed charProperty Got $($this.charProperty)" + } + + if ($this.hashtableProperty.ContainsKey("secretStringKey")) + { + if ($this.hashtableProperty["secretStringKey"] -ne "secretCode") + { + throw "Failed comparing value of `$hashtableProperty.secretStringKey Got $($this.hashtableProperty["secretStringKey"])" + } + } + else + { + throw "Failed finding secretStringKey in hashtableProperty" + } + + if ($this.hashtableProperty.ContainsKey("secretIntKey")) + { + if ($this.hashtableProperty["secretIntKey"] -ne 123456) + { + throw "Failed comparing value of `$hashtableProperty.secretIntKey Got $($this.hashtableProperty["secretIntKey"])" + } + } + else + { + throw "Failed finding secretIntKey in hashtableProperty" + } + + return $true + } + + [void] Set() + { + # no-op + } +} diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs @@ -19,6 +19,7 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests using Microsoft.Management.Configuration.UnitTests.Helpers; using Microsoft.PowerShell.Commands; using Moq; + using Windows.Foundation.Collections; using Windows.Security.Cryptography.Certificates; using Xunit; using Xunit.Abstractions; @@ -642,6 +643,43 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests Times.Exactly(2)); } + /// <summary> + /// This tests uses SimpleTestResourceTypes Test to validate the resource got the correct types + /// from the processor. + /// </summary> + [Fact] + public void CreateUnitProcessor_TestTypes() + { + var processorEnv = this.fixture.PrepareTestProcessorEnvironment(); + + var setProcessor = new ConfigurationSetProcessor(processorEnv, new ConfigurationSet()); + + var unit = new ConfigurationUnit + { + UnitName = "SimpleTestResourceTypes", + Intent = ConfigurationUnitIntent.Assert, + }; + + unit.Directives.Add("module", "xSimpleTestResource"); + unit.Directives.Add("version", "0.0.0.1"); + + var hashtableProperty = new ValueSet + { + { "secretStringKey", "secretCode" }, + { "secretIntKey", "123456" }, + }; + + unit.Settings.Add("boolProperty", true); + unit.Settings.Add("intProperty", 3); + unit.Settings.Add("doubleProperty", -9.876); + unit.Settings.Add("charProperty", 'f'); + unit.Settings.Add("hashtableProperty", hashtableProperty); + + var unitProcessor = setProcessor.CreateUnitProcessor(unit, null); + + unitProcessor.TestSettings(); + } + private ConfigurationUnit CreteConfigurationUnit() { var unit = new ConfigurationUnit(); diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/DscModuleV2Tests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/DscModuleV2Tests.cs @@ -61,7 +61,7 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests /// <param name="module">Module.</param> /// <param name="expectedResources">Expected DSC resources.</param> [Theory] - [InlineData(TestModule.SimpleTestResourceModuleName, 4)] + [InlineData(TestModule.SimpleTestResourceModuleName, 5)] [InlineData("MyReallyFakeModule", 0)] public void GetDscResourcesInModule_Test(string module, int expectedResources) { @@ -111,7 +111,7 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests PowerShellHelpers.CreateModuleSpecification( TestModule.SimpleTestResourceModuleName, version: TestModule.SimpleTestResourceVersion)); - Assert.Equal(4, ogResources.Count); + Assert.Equal(5, ogResources.Count); } { @@ -121,7 +121,7 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests PowerShellHelpers.CreateModuleSpecification( TestModule.SimpleTestResourceModuleName, version: newVersion)); - Assert.Equal(4, newVersionResources.Count); + Assert.Equal(5, newVersionResources.Count); } { @@ -556,5 +556,33 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests version: newVersion)); } } + + /// <summary> + /// Calls Invoke-DscResource invalid arguments. + /// </summary> + /// <param name="value">Setting value.</param> + /// <param name="rebootRequired">Expected reboot required.</param> + [Fact] + public void InvokeSetResource_InvalidArguments() + { + var testEnvironment = this.fixture.PrepareTestProcessorEnvironment(); + + var dscModule = new DscModuleV2(); + + var settings = new ValueSet() + { + { "Fake", "please dont add it" }, + }; + + using PowerShell pwsh = PowerShell.Create(testEnvironment.Runspace); + var e = Assert.Throws<InvokeDscResourceSetException>(() => dscModule.InvokeSetResource( + pwsh, + settings, + TestModule.SimpleTestResourceName, + PowerShellHelpers.CreateModuleSpecification( + TestModule.SimpleTestResourceModuleName))); + + Assert.Contains("The property 'Fake' cannot be found on this object.", e.Message); + } } } diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ProcessorEnvironmentTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ProcessorEnvironmentTests.cs @@ -6,9 +6,14 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests { + using System; using System.Collections.Generic; + using System.Linq; + using System.Management.Automation; + using Microsoft.Management.Configuration.Processor.ProcessorEnvironments; using Microsoft.Management.Configuration.UnitTests.Fixtures; using Moq; + using Windows.Security.Authentication.OnlineId; using Xunit; using Xunit.Abstractions; using static Microsoft.Management.Configuration.Processor.Constants.PowerShellConstants; @@ -209,7 +214,7 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests psModulePathExpected = "CleanupPSModulePathPath2;CleanupPSModulePathPath3"; psModulePath = processorEnv.GetVariable<string>(Variables.PSModulePath); - Assert.Equal(psModulePath, psModulePath); + Assert.Equal(psModulePathExpected, psModulePath); } } }