From ed99ad4aeb30ff89e4a1a91c486bd6831dca3e14 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 5 Nov 2018 19:59:27 +0000 Subject: [PATCH 1/2] Take module version into account for SaveDscDependency and remove old dead code --- Engine/Generic/ModuleDependencyHandler.cs | 142 ++++++++++------------ Engine/ScriptAnalyzer.cs | 4 +- 2 files changed, 64 insertions(+), 82 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 0d0833aaf..61d755b04 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -212,21 +212,6 @@ private string GetTempModulePath(string symLinkPath) return line; } - private void SaveModule(PSObject module) - { - ThrowIfNull(module, "module"); - - // TODO validate module - using (var ps = System.Management.Automation.PowerShell.Create()) - { - ps.Runspace = runspace; - ps.AddCommand("Save-Module") - .AddParameter("Path", tempModulePath) - .AddParameter("InputObject", module); - ps.Invoke(); - } - } - private void SetupPSModulePath() { oldPSModulePath = Environment.GetEnvironmentVariable("PSModulePath"); @@ -295,51 +280,17 @@ public ModuleDependencyHandler( } - /// - /// Encapsulates Find-Module - /// - /// Name of the module - /// A PSObject if it finds the modules otherwise returns null - public PSObject FindModule(string moduleName) - { - ThrowIfNull(moduleName, "moduleName"); - moduleName = moduleName.ToLower(); - if (modulesFound.ContainsKey(moduleName)) - { - return modulesFound[moduleName]; - } - Collection modules = null; - using (var ps = System.Management.Automation.PowerShell.Create()) - { - ps.Runspace = runspace; - ps.AddCommand("Find-Module", true) - .AddParameter("Name", moduleName) - .AddParameter("Repository", moduleRepository); - modules = ps.Invoke(); - } - if (modules == null) - { - return null; - } - var module = modules.FirstOrDefault(); - if (module == null ) - { - return null; - } - modulesFound.Add(moduleName, module); - return module; - } - /// /// SaveModule version that doesn't throw /// /// Name of the module + /// (Optional) version of the module /// True if it can save a module otherwise false. - public bool TrySaveModule(string moduleName) + public bool TrySaveModule(string moduleName, Version moduleVersion) { try { - SaveModule(moduleName); + SaveModule(moduleName, moduleVersion); return true; } catch @@ -353,7 +304,8 @@ public bool TrySaveModule(string moduleName) /// Encapsulates Save-Module cmdlet /// /// Name of the module - public void SaveModule(string moduleName) + /// (Optional) version of the module + public void SaveModule(string moduleName, Version moduleVersion) { ThrowIfNull(moduleName, "moduleName"); if (IsModulePresentInTempModulePath(moduleName)) @@ -368,6 +320,10 @@ public void SaveModule(string moduleName) .AddParameter("Name", moduleName) .AddParameter("Repository", moduleRepository) .AddParameter("Force"); + if (moduleVersion != null) + { + ps.AddParameter("RequiredVersion", moduleVersion); + } ps.Invoke(); } } @@ -376,18 +332,25 @@ public void SaveModule(string moduleName) /// Encapsulates Get-Module to check the availability of the module on the system /// /// + /// /// True indicating the presence of the module, otherwise false - public bool IsModuleAvailable(string moduleName) + public bool IsModuleAvailable(string moduleName, Version moduleVersion) { ThrowIfNull(moduleName, "moduleName"); IEnumerable availableModules; using (var ps = System.Management.Automation.PowerShell.Create()) { ps.Runspace = runspace; - availableModules = ps.AddCommand("Get-Module") + ps.AddCommand("Get-Module") .AddParameter("Name", moduleName) - .AddParameter("ListAvailable") - .Invoke(); + .AddParameter("ListAvailable"); + if (moduleVersion != null) + { + ps.AddCommand("Where-Object") + .AddParameter("Filterscript", ScriptBlock.Create($"$_.Version -eq '{moduleVersion}'")); + } + availableModules = ps.Invoke(); + } return availableModules != null ? availableModules.Any() : false; } @@ -405,12 +368,13 @@ public bool IsModuleAvailable(string moduleName) /// /// /// + /// /// An enumeration over the module names that are not available - public IEnumerable GetUnavailableModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast) + public IEnumerable GetUnavailableModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast, out Version moduleVersion) { ThrowIfNull(error, "error"); ThrowIfNull(ast, "ast"); - var moduleNames = ModuleDependencyHandler.GetModuleNameFromErrorExtent(error, ast); + var moduleNames = ModuleDependencyHandler.GetModuleNameFromErrorExtent(error, ast, out moduleVersion); if (moduleNames == null) { return null; @@ -418,12 +382,12 @@ public IEnumerable GetUnavailableModuleNameFromErrorExtent(ParseError er var unavailableModules = new List(); foreach (var moduleName in moduleNames) { - if (!IsModuleAvailable(moduleName)) + if (!IsModuleAvailable(moduleName, moduleVersion)) { unavailableModules.Add(moduleName); } } - //return moduleNames.Where(x => !IsModuleAvailable(x)); + return unavailableModules; } @@ -438,9 +402,11 @@ public IEnumerable GetUnavailableModuleNameFromErrorExtent(ParseError er /// /// Parse error /// AST of the script that contians the parse error + /// Specifc version of the required module /// The name of the module that caused the parser to throw the error. Returns null if it cannot extract the module name. - public static IEnumerable GetModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast) + public static IEnumerable GetModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast, out Version moduleVersion) { + moduleVersion = null; ThrowIfNull(error, "error"); ThrowIfNull(ast, "ast"); var statement = ast.Find(x => x.Extent.Equals(error.Extent), true); @@ -452,12 +418,8 @@ public static IEnumerable GetModuleNameFromErrorExtent(ParseError error, // check if the command name is import-dscmodule // right now we handle only the following forms // 1. Import-DSCResourceModule -ModuleName somemodule - // 2. Import-DSCResourceModule -ModuleName somemodule1,somemodule2 - if (dynamicKywdAst.CommandElements.Count < 3) - { - return null; - } - + // 2. Import-DSCResourceModule -ModuleName somemodule1 -ModuleVersion major.minor.patch.build + // 3. Import-DSCResourceModule -ModuleName somemodule1,somemodule2 var dscKeywordAst = dynamicKywdAst.CommandElements[0] as StringConstantExpressionAst; if (dscKeywordAst == null || !dscKeywordAst.Value.Equals("Import-DscResource", StringComparison.OrdinalIgnoreCase)) { @@ -465,33 +427,52 @@ public static IEnumerable GetModuleNameFromErrorExtent(ParseError error, } // find a parameter named modulename - int k; - for (k = 1; k < dynamicKywdAst.CommandElements.Count; k++) + int positionOfModuleNameParamter = 0; + int positionOfModuleVersionParameter = 0; + for (int i = 1; i < dynamicKywdAst.CommandElements.Count; i++) { - var paramAst = dynamicKywdAst.CommandElements[k] as CommandParameterAst; + var paramAst = dynamicKywdAst.CommandElements[i] as CommandParameterAst; // TODO match the initial letters only - if (paramAst == null || !paramAst.ParameterName.Equals("ModuleName", StringComparison.OrdinalIgnoreCase)) + if (paramAst != null && paramAst.ParameterName.Equals("ModuleName", StringComparison.OrdinalIgnoreCase)) + { + if (i == dynamicKywdAst.CommandElements.Count) + { + // command was Save-DscDependency ... -ModuleName -> module name missing + return null; + } + positionOfModuleNameParamter = i + 1; + continue; + } + + if (paramAst != null && paramAst.ParameterName.Equals("ModuleVersion", StringComparison.OrdinalIgnoreCase)) { + if (i == dynamicKywdAst.CommandElements.Count) + { + // command was Save-DscDependency ... -ModuleVersion -> module version missing + return null; + } + positionOfModuleVersionParameter = i + 1; continue; } - break; } - if (k == dynamicKywdAst.CommandElements.Count) - { - // cannot find modulename - return null; - } var modules = new List(); - // k < count - 1, because only -ModuleName throws parse error and hence not possible - var paramValAst = dynamicKywdAst.CommandElements[++k]; + var paramValAst = dynamicKywdAst.CommandElements[positionOfModuleNameParamter]; // import-dscresource -ModuleName module1 var paramValStrConstExprAst = paramValAst as StringConstantExpressionAst; if (paramValStrConstExprAst != null) { modules.Add(paramValStrConstExprAst.Value); + + // import-dscresource -ModuleName module1 -ModuleVersion major.minor.patch.build + var versionParameterAst = dynamicKywdAst.CommandElements[positionOfModuleVersionParameter] as StringConstantExpressionAst; + if (versionParameterAst != null) + { + Version.TryParse(versionParameterAst.Value, out Version version); // ignore return value since a module version of null means no version + moduleVersion = version; + } return modules; } @@ -513,6 +494,7 @@ public static IEnumerable GetModuleNameFromErrorExtent(ParseError error, } return modules; } + return null; } diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index ae6e22255..01b4ef3cd 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1804,7 +1804,7 @@ private bool TrySaveModules(ParseError[] errors, ScriptBlockAst scriptAst) } foreach (var error in errors.Where(IsModuleNotFoundError)) { - var moduleNames = moduleHandler.GetUnavailableModuleNameFromErrorExtent(error, scriptAst); + var moduleNames = moduleHandler.GetUnavailableModuleNameFromErrorExtent(error, scriptAst, out Version moduleVersion); if (moduleNames == null) { continue; @@ -1815,7 +1815,7 @@ private bool TrySaveModules(ParseError[] errors, ScriptBlockAst scriptAst) String.Format( "Saving module {0} from PSGallery", moduleName)); - var moduleSaved = moduleHandler.TrySaveModule(moduleName); + var moduleSaved = moduleHandler.TrySaveModule(moduleName, moduleVersion); if (!moduleSaved) { this.outputWriter.WriteVerbose( From ebb22db350ba6d5d8d2486b9b90a27999f96fa2b Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 5 Nov 2018 20:48:46 +0000 Subject: [PATCH 2/2] fix tests and add test case --- .../Engine/ModuleDependencyHandler.tests.ps1 | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/Tests/Engine/ModuleDependencyHandler.tests.ps1 b/Tests/Engine/ModuleDependencyHandler.tests.ps1 index 2eb9f219e..0b00c9147 100644 --- a/Tests/Engine/ModuleDependencyHandler.tests.ps1 +++ b/Tests/Engine/ModuleDependencyHandler.tests.ps1 @@ -85,10 +85,26 @@ Describe "Resolve DSC Resource Dependency" { $tokens = $null $parseError = $null $ast = [System.Management.Automation.Language.Parser]::ParseInput($sb, [ref]$tokens, [ref]$parseError) - $resultModuleNames = $moduleHandlerType::GetModuleNameFromErrorExtent($parseError[0], $ast).ToArray() + $resultModuleNames = $moduleHandlerType::GetModuleNameFromErrorExtent($parseError[0], $ast, [ref]$null).ToArray() $resultModuleNames[0] | Should -Be 'SomeDscModule1' } + It "Extracts 1 module name with version" -skip:$skipTest { + $sb = @" +{Configuration SomeConfiguration +{ + Import-DscResource -ModuleName SomeDscModule1 -ModuleVersion 1.2.3.4 +}} +"@ + $tokens = $null + $parseError = $null + $ast = [System.Management.Automation.Language.Parser]::ParseInput($sb, [ref]$tokens, [ref]$parseError) + $moduleVersion = $null + $resultModuleNames = $moduleHandlerType::GetModuleNameFromErrorExtent($parseError[0], $ast, [ref]$moduleVersion).ToArray() + $resultModuleNames[0] | Should -Be 'SomeDscModule1' + $moduleVersion | Should -Be ([version]'1.2.3.4') + } + It "Extracts more than 1 module names" -skip:$skipTest { $sb = @" {Configuration SomeConfiguration @@ -99,7 +115,7 @@ Describe "Resolve DSC Resource Dependency" { $tokens = $null $parseError = $null $ast = [System.Management.Automation.Language.Parser]::ParseInput($sb, [ref]$tokens, [ref]$parseError) - $resultModuleNames = $moduleHandlerType::GetModuleNameFromErrorExtent($parseError[0], $ast).ToArray() + $resultModuleNames = $moduleHandlerType::GetModuleNameFromErrorExtent($parseError[0], $ast, [ref]$null).ToArray() $resultModuleNames[0] | Should -Be 'SomeDscModule1' $resultModuleNames[1] | Should -Be 'SomeDscModule2' $resultModuleNames[2] | Should -Be 'SomeDscModule3' @@ -116,7 +132,7 @@ Describe "Resolve DSC Resource Dependency" { $tokens = $null $parseError = $null $ast = [System.Management.Automation.Language.Parser]::ParseInput($sb, [ref]$tokens, [ref]$parseError) - $resultModuleNames = $moduleHandlerType::GetModuleNameFromErrorExtent($parseError[0], $ast).ToArray() + $resultModuleNames = $moduleHandlerType::GetModuleNameFromErrorExtent($parseError[0], $ast, [ref]$null).ToArray() $resultModuleNames[0] | Should -Be 'SomeDscModule1' } }