From 8c35ce7f7209783d3c750fb79c14d9475c571513 Mon Sep 17 00:00:00 2001 From: Bill Hurt Date: Sun, 16 Feb 2020 22:50:41 -0800 Subject: [PATCH] (#221) Inconsistent Logic in xWebsite The Test-TargetResource function in the xWebSite resource used inconsistent logic in the way that it would test a property for compliance and then report non-compliance. Some tests would use the `$PSBoundParameters` variable to check for parameters to be tested, others would use `[String]::isNullOrEmpty()` Some tests would set the `$inDesiredState` variable, write a verbose message and move on, and others would write the verbose message and use the `return` keyward to immediately exit the function and stop testing additional properties. The desired style should use `$PSBoundParameters` and the `$inDesiredState` variable consistently, so that all properties are tested for troubleshooting purposes, and to promote conciseness and reasability Resolves #221 --- CHANGELOG.md | 2 ++ azure-pipelines.yml | 22 +++++++++++++++++-- .../MSFT_xWebSite/MSFT_xWebSite.psm1 | 22 +++++++++---------- tests/TestHelper/CommonTestHelper.psm1 | 2 +- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee12d4e7e..a60f62ec2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ For older change log history see the [historic changelog](HISTORIC_CHANGELOG.md) does not contain 'dsccommunity'. - Changed the VS Code project settings to trim trailing whitespace for markdown files too. + - Ensure that Test-TargetResourse in xWebSite tests all properties before + returning true or false, and that it uses a consistent style. ([issue #221](https://github.com/PowerShell/xWebAdministration/issues/550)) ### Fixed diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 61a27352e..531f75ce2 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -134,7 +134,16 @@ stages: script: 'winrm quickconfig -quiet' pwsh: false - powershell: | - Install-WindowsFeature -IncludeAllSubFeature -IncludeManagementTools -Name 'Web-Server' -Verbose + $features = @( + 'Web-Common-Http','Web-Health','Web-Performance', + 'Web-Security','Web-AppInit', + 'Web-CGI','Web-ISAPI-Ext','Web-ISAPI-Filter', + 'Web-Includes','Web-WebSockets', 'Web-Scripting-Tools', 'Web-Mgmt-Service' + ) + foreach($feature in $features) { + Write-Host $feature + Install-WindowsFeature -IncludeAllSubFeature -Name $feature -Verbose + } name: InstallWebServerFeature - task: PowerShell@2 name: test @@ -251,7 +260,16 @@ stages: - powershell: | Set-Service -Name wuauserv -StartupType Manual -Verbose Start-Service -name wuauserv -Verbose - Install-WindowsFeature -IncludeAllSubFeature -Name 'Web-Server' -Verbose + $features = @( + 'Web-Common-Http','Web-Health','Web-Performance', + 'Web-Security','Web-AppInit', + 'Web-CGI','Web-ISAPI-Ext','Web-ISAPI-Filter', + 'Web-Includes','Web-WebSockets', 'Web-Scripting-Tools', 'Web-Mgmt-Service' + ) + foreach($feature in $features) { + Write-Host $feature + Install-WindowsFeature -IncludeAllSubFeature -Name $feature -Verbose + } name: InstallWebServerFeature - task: PowerShell@2 name: test diff --git a/source/DSCResources/MSFT_xWebSite/MSFT_xWebSite.psm1 b/source/DSCResources/MSFT_xWebSite/MSFT_xWebSite.psm1 index c57e85de1..ea3e5b285 100644 --- a/source/DSCResources/MSFT_xWebSite/MSFT_xWebSite.psm1 +++ b/source/DSCResources/MSFT_xWebSite/MSFT_xWebSite.psm1 @@ -818,7 +818,7 @@ function Test-TargetResource } # Check Physical Path property - if ([String]::IsNullOrEmpty($PhysicalPath) -eq $false -and ` + if ($PSBoundParameters.ContainsKey('PhysicalPath') -and ` $website.PhysicalPath -ne $PhysicalPath) { $inDesiredState = $false @@ -827,7 +827,8 @@ function Test-TargetResource } # Check State - if ($PSBoundParameters.ContainsKey('State') -and $website.State -ne $State) + if ($PSBoundParameters.ContainsKey('State') -and ` + $website.State -ne $State) { $inDesiredState = $false Write-Verbose -Message ($script:localizedData.VerboseTestTargetFalseState ` @@ -956,9 +957,9 @@ function Test-TargetResource # Check Log Format if ($LogFormat -ne $website.logfile.LogFormat) { + $inDesiredState = $false Write-Verbose -Message ($script:localizedData.VerboseTestTargetFalseLogFormat ` -f $Name) - return $false } } @@ -966,17 +967,17 @@ function Test-TargetResource if ($PSBoundParameters.ContainsKey('LogFlags') -and ` (-not (Compare-LogFlags -Name $Name -LogFlags $LogFlags))) { + $inDesiredState = $false Write-Verbose -Message ($script:localizedData.VerboseTestTargetFalseLogFlags) - return $false } # Check LogPath if ($PSBoundParameters.ContainsKey('LogPath') -and ` ($LogPath -ne $website.logfile.directory)) { + $inDesiredState = $false Write-Verbose -Message ($script:localizedData.VerboseTestTargetFalseLogPath ` -f $Name) - return $false } # Check LogPeriod @@ -988,19 +989,18 @@ function Test-TargetResource Write-Verbose -Message ($script:localizedData.WarningLogPeriod ` -f $Name) } - + $inDesiredState = $false Write-Verbose -Message ($script:localizedData.VerboseTestTargetFalseLogPeriod ` -f $Name) - return $false } # Check LogTruncateSize if ($PSBoundParameters.ContainsKey('LogTruncateSize') -and ` ($LogTruncateSize -ne $website.logfile.LogTruncateSize)) { + $inDesiredState = $false Write-Verbose -Message ($script:localizedData.VerboseTestTargetFalseLogTruncateSize ` -f $Name) - return $false } # Check LoglocalTimeRollover @@ -1008,27 +1008,27 @@ function Test-TargetResource ($LoglocalTimeRollover -ne ` ([System.Convert]::ToBoolean($website.logfile.LocalTimeRollover)))) { + $inDesiredState = $false Write-Verbose -Message ($script:localizedData.VerboseTestTargetFalseLoglocalTimeRollover ` -f $Name) - return $false } # Check LogTargetW3C if ($PSBoundParameters.ContainsKey('LogTargetW3C') -and ` ($LogTargetW3C -ne $website.logfile.LogTargetW3C)) { + $inDesiredState = $false Write-Verbose -Message ($script:localizedData.VerboseTestTargetFalseLogTargetW3C ` -f $Name) - return $false } # Check LogCustomFields if needed if ($PSBoundParameters.ContainsKey('LogCustomFields') -and ` (-not (Test-LogCustomField -Site $Name -LogCustomField $LogCustomFields))) { + $inDesiredState = $false Write-Verbose -Message ($script:localizedData.VerboseTestTargetUpdateLogCustomFields ` -f $Name) - return $false } } diff --git a/tests/TestHelper/CommonTestHelper.psm1 b/tests/TestHelper/CommonTestHelper.psm1 index c0c48f1d3..7da33996c 100644 --- a/tests/TestHelper/CommonTestHelper.psm1 +++ b/tests/TestHelper/CommonTestHelper.psm1 @@ -41,7 +41,7 @@ function Install-NewSelfSignedCertificateExScript Remove-Item -Path $newSelfSignedCertZipPath -Force } - Invoke-WebRequest -Uri $newSelfSignedCertURL -OutFile $newSelfSignedCertZipPath + Invoke-WebRequest -Uri $newSelfSignedCertURL -OutFile $newSelfSignedCertZipPath -SslProtocol Tls12 Add-Type -AssemblyName System.IO.Compression.FileSystem