From a361e0a61f8ad411a9e1cef64965ade4555bf69c Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Tue, 6 Oct 2020 12:00:47 +0200 Subject: [PATCH 01/36] Major overhaul of resource. added tests, and fix for issue #550 --- .../DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 271 +++--- tests/Unit/DSC_SqlRole.Tests.ps1 | 833 +++++++++--------- 2 files changed, 599 insertions(+), 505 deletions(-) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index c20944e11..d0a52be90 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -10,23 +10,17 @@ $script:localizedData = Get-LocalizedData -DefaultUICulture 'en-US' .SYNOPSIS This function gets the sql server role properties. - .PARAMETER Members - The members the server role should have. - - .PARAMETER MembersToInclude - The members the server role should include. + .PARAMETER ServerName + The host name of the SQL Server to be configured. Default value is $env:COMPUTERNAME. - .PARAMETER MembersToExclude - The members the server role should exclude. + .PARAMETER InstanceName + The name of the SQL instance to be configured. .PARAMETER ServerRoleName The name of server role to be created or dropped. - .PARAMETER ServerName - The host name of the SQL Server to be configured. Default value is $env:COMPUTERNAME. + #returns $null if role does not exist. Else an array of all role members. This Array can be empty. - .PARAMETER InstanceName - The name of the SQL instance to be configured. #> function Get-TargetResource { @@ -35,35 +29,24 @@ function Get-TargetResource param ( [Parameter()] - [System.String[]] - $Members, - - [Parameter()] - [System.String[]] - $MembersToInclude, - - [Parameter()] - [System.String[]] - $MembersToExclude, - - [Parameter(Mandatory = $true)] [ValidateNotNullOrEmpty()] [System.String] - $ServerRoleName, + $ServerName = $env:COMPUTERNAME, - [Parameter()] + [Parameter(Mandatory = $true)] [ValidateNotNullOrEmpty()] [System.String] - $ServerName = $env:COMPUTERNAME, + $InstanceName, [Parameter(Mandatory = $true)] [ValidateNotNullOrEmpty()] [System.String] - $InstanceName + $ServerRoleName ) $sqlServerObject = Connect-SQL -ServerName $ServerName -InstanceName $InstanceName - $ensure = 'Present' + $ensure = 'Absent' + $membersInRole = $null if ($sqlServerObject) { @@ -77,6 +60,7 @@ function Get-TargetResource try { [System.String[]] $membersInRole = $sqlServerRoleObject.EnumMemberNames() + $ensure = 'Present' } catch { @@ -85,76 +69,16 @@ function Get-TargetResource New-InvalidOperationException -Message $errorMessage -ErrorRecord $_ } - - if ($Members) - { - if ($MembersToInclude -or $MembersToExclude) - { - $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - New-InvalidOperationException -Message $errorMessage - } - - if ( $null -ne (Compare-Object -ReferenceObject $membersInRole -DifferenceObject $Members)) - { - Write-Verbose -Message ( - $script:localizedData.DesiredMembersNotPresent ` - -f $ServerRoleName - ) - - $ensure = 'Absent' - } - } - else - { - if ($MembersToInclude) - { - foreach ($memberToInclude in $MembersToInclude) - { - if ($membersInRole -notcontains $memberToInclude) - { - Write-Verbose -Message ( - $script:localizedData.MemberNotPresent ` - -f $ServerRoleName, $memberToInclude - ) - - $ensure = 'Absent' - } - } - } - - if ($MembersToExclude) - { - foreach ($memberToExclude in $MembersToExclude) - { - if ($membersInRole -contains $memberToExclude) - { - Write-Verbose -Message ( - $script:localizedData.MemberPresent ` - -f $ServerRoleName, $memberToExclude - ) - - $ensure = 'Absent' - } - } - } - } - } - else - { - $ensure = 'Absent' } } - $returnValue = @{ - Ensure = $ensure - Members = $membersInRole - MembersToInclude = $MembersToInclude - MembersToExclude = $MembersToExclude - ServerRoleName = $ServerRoleName - ServerName = $ServerName - InstanceName = $InstanceName + return @{ + ServerRoleName = $ServerRoleName + Ensure = $ensure + ServerName = $ServerName + InstanceName = $InstanceName + membersInRole = $membersInRole } - $returnValue } <# @@ -283,19 +207,22 @@ function Set-TargetResource } } + $saneParameters = @{ + ServerRoleName = $ServerRoleName + Members = $Members + MembersToInclude = $MembersToInclude + MembersToExclude = $MembersToExclude + } + + $saneInputParameters = Sanitize-InputObjects @saneParameters + if ($Members) { - if ($MembersToInclude -or $MembersToExclude) - { - $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - New-InvalidOperationException -Message $errorMessage - } - $memberNamesInRoleObject = $sqlServerObject.Roles[$ServerRoleName].EnumMemberNames() foreach ($memberName in $memberNamesInRoleObject) { - if ($Members -notcontains $memberName) + if ($saneInputParameters.Members -notcontains $memberName) { Remove-SqlDscServerRoleMember -SqlServerObject $sqlServerObject ` -SecurityPrincipal $memberName ` @@ -303,7 +230,7 @@ function Set-TargetResource } } - foreach ($memberToAdd in $Members) + foreach ($memberToAdd in $saneInputParameters.Members) { if ($memberNamesInRoleObject -notcontains $memberToAdd) { @@ -319,7 +246,7 @@ function Set-TargetResource { $memberNamesInRoleObject = $sqlServerObject.Roles[$ServerRoleName].EnumMemberNames() - foreach ($memberToInclude in $MembersToInclude) + foreach ($memberToInclude in $saneInputParameters.MembersToInclude) { if ($memberNamesInRoleObject -notcontains $memberToInclude) { @@ -334,7 +261,7 @@ function Set-TargetResource { $memberNamesInRoleObject = $sqlServerObject.Roles[$ServerRoleName].EnumMemberNames() - foreach ($memberToExclude in $MembersToExclude) + foreach ($memberToExclude in $saneInputParameters.MembersToExclude) { if ($memberNamesInRoleObject -contains $memberToExclude) { @@ -420,13 +347,19 @@ function Test-TargetResource -f $ServerRoleName ) + $saneParameters = @{ + ServerRoleName = $ServerRoleName + Members = $Members + MembersToInclude = $MembersToInclude + MembersToExclude = $MembersToExclude + } + + $saneInputParameters = Sanitize-InputObjects @saneParameters + $getTargetResourceParameters = @{ - InstanceName = $PSBoundParameters.InstanceName + InstanceName = $InstanceName ServerName = $ServerName - ServerRoleName = $PSBoundParameters.ServerRoleName - Members = $PSBoundParameters.Members - MembersToInclude = $PSBoundParameters.MembersToInclude - MembersToExclude = $PSBoundParameters.MembersToExclude + ServerRoleName = $ServerRoleName } $getTargetResourceResult = Get-TargetResource @getTargetResourceParameters @@ -458,6 +391,53 @@ function Test-TargetResource $isServerRoleInDesiredState = $false } + + if ($Members) + { + if ( $null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.membersInRole -DifferenceObject $saneInputParameters.Members)) + { + Write-Verbose -Message ( + $script:localizedData.DesiredMembersNotPresent ` + -f $ServerRoleName + ) + + $isServerRoleInDesiredState = $false + } + } + else + { + if ($MembersToInclude) + { + foreach ($memberToInclude in $saneInputParameters.MembersToInclude) + { + if ($getTargetResourceResult.membersInRole -notcontains $memberToInclude) + { + Write-Verbose -Message ( + $script:localizedData.MemberNotPresent ` + -f $ServerRoleName, $memberToInclude + ) + + $isServerRoleInDesiredState = $false + } + } + } + + if ($MembersToExclude) + { + foreach ($memberToExclude in $saneInputParameters.MembersToExclude) + { + if ($getTargetResourceResult.membersInRole -contains $memberToExclude) + { + Write-Verbose -Message ( + $script:localizedData.MemberPresent ` + -f $ServerRoleName, $memberToExclude + ) + + $isServerRoleInDesiredState = $false + } + } + } + } } } @@ -617,4 +597,77 @@ function Test-SqlSecurityPrincipal return $true } +<# + .SYNOPSIS + This function sanitizes the parameters + If Members is filled, MembersToInclude and MembersToExclude should be empty. + If ServerRoleName is sysadmin, make sure we dont try to alter SA. + + .PARAMETER Members + The members the server role should have. + + .PARAMETER MembersToInclude + The members the server role should include. + + .PARAMETER MembersToExclude + The members the server role should exclude. + + .PARAMETER ServerRoleName + The name of server role to be created or dropped. +#> +function Sanitize-InputObjects +{ + param + ( + [Parameter()] + [System.String[]] + $Members, + + [Parameter()] + [System.String[]] + $MembersToInclude, + + [Parameter()] + [System.String[]] + $MembersToExclude, + + [ValidateNotNullOrEmpty()] + [System.String] + $ServerRoleName + ) + + if ($Members) + { + if ($MembersToInclude -or $MembersToExclude) + { + $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull + New-InvalidOperationException -Message $errorMessage + } + + if ($ServerRoleName -eq 'sysadmin') + { + if ($Members -notcontains 'SA') + { + $Members += 'SA' + } + } + } + else + { + if ($ServerRoleName -eq 'sysadmin') + { + if ($MembersToExclude -contains 'SA') + { + $MembersToExclude = $MembersToExclude -ne 'SA' + } + } + } + + return @{ + Members = $Members + MembersToInclude = $MembersToInclude + MembersToExclude = $MembersToExclude + } +} + Export-ModuleMember -Function *-TargetResource diff --git a/tests/Unit/DSC_SqlRole.Tests.ps1 b/tests/Unit/DSC_SqlRole.Tests.ps1 index 3749dc8d7..58a8ce293 100644 --- a/tests/Unit/DSC_SqlRole.Tests.ps1 +++ b/tests/Unit/DSC_SqlRole.Tests.ps1 @@ -50,10 +50,17 @@ try $mockSqlServerLoginTwo = 'CONTOSO\Kelly' $mockSqlServerLoginThree = 'CONTOSO\Lucy' $mockSqlServerLoginFour = 'CONTOSO\Steve' + $mockSqlServerRoleSysAdmin = 'sysadmin' + $mockSqlServerSA = 'SA' $mockEnumMemberNames = @( $mockSqlServerLoginOne, $mockSqlServerLoginTwo ) + $mockEnumMemberNamesSysAdmin = @( + $mockSqlServerLoginOne, + $mockSqlServerLoginTwo, + $mockSqlServerSA + ) $mockSecurityPrincipals = @( $mockSqlServerLoginOne $mockSqlServerLoginTwo @@ -61,6 +68,14 @@ try $mockSqlServerLoginFour $mockSqlServerChildRole ) + $mockSecurityPrincipalsSysAdmin = @( + $mockSqlServerLoginOne + $mockSqlServerLoginTwo + $mockSqlServerLoginThree + $mockSqlServerLoginFour + $mockSqlServerChildRole + $mockSqlServerSA + ) $mockSqlServerLoginType = 'WindowsUser' $mockExpectedServerRoleToDrop = 'ServerRoleToDrop' $mockPrincipalsAsArrays = $false @@ -233,27 +248,34 @@ try ServerRoleName = 'UnknownRoleName' } + $result = Get-TargetResource @testParameters + It 'Should return the state as absent when the role does not exist' { - $result = Get-TargetResource @testParameters $result.Ensure | Should -Be 'Absent' - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It } - It 'Should return the members as null' { - $result = Get-TargetResource @testParameters - $result.Members | Should -Be $null +# It 'Should be executed once' { +# Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context +# } - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should return the members as null' { +# $result = Get-TargetResource @testParameters + $result.membersInRole | Should -Be $null } +# It 'Should be executed once' { +# Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context +# } + It 'Should return the same values as passed as parameters' { - $result = Get-TargetResource @testParameters +# $result = Get-TargetResource @testParameters $result.ServerName | Should -Be $testParameters.ServerName $result.InstanceName | Should -Be $testParameters.InstanceName $result.ServerRoleName | Should -Be $testParameters.ServerRoleName + } - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should be executed once' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } @@ -263,35 +285,44 @@ try ServerRoleName = $mockSqlServerRole } + $result = Get-TargetResource @testParameters + It 'Should not return the state as absent when the role exist' { - $result = Get-TargetResource @testParameters $result.Ensure | Should -Not -Be 'Absent' - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It } - It 'Should return the members as not null' { - $result = Get-TargetResource @testParameters - $result.Members | Should -Not -Be $null +# It 'Should be executed once' { +# Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context +# } - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should return the members as not null' { +# $result = Get-TargetResource @testParameters + $result.membersInRole | Should -Not -Be $null } +# It 'Should be executed once' { +# Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context +# } + # Regression test for issue #790 It 'Should return the members as string array' { - $result = Get-TargetResource @testParameters - ($result.Members -is [System.String[]]) | Should -Be $true - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It +# $result = Get-TargetResource @testParameters + ($result.membersInRole -is [System.String[]]) | Should -BeTrue } +# It 'Should be executed once' { +# Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context +# } + It 'Should return the same values as passed as parameters' { - $result = Get-TargetResource @testParameters +# $result = Get-TargetResource @testParameters $result.ServerName | Should -Be $testParameters.ServerName $result.InstanceName | Should -Be $testParameters.InstanceName $result.ServerRoleName | Should -Be $testParameters.ServerRoleName + } - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should be executed once' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } @@ -314,145 +345,62 @@ try } } - Context 'When the system is in the desired state, parameter Members is assigned a value and ensure is set to Present' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - ServerRoleName = $mockSqlServerRole - Members = $mockEnumMemberNames - } - - It 'Should return the state as present when the members are correct' { - $result = Get-TargetResource @testParameters - $result.Ensure | Should -Be 'Present' - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } - - It 'Should return the members as not null' { - $result = Get-TargetResource @testParameters - $result.Members | Should -Be $testParameters.Members - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } - - # Regression test for issue #790 - It 'Should return the members as string array' { - $result = Get-TargetResource @testParameters - ($result.Members -is [System.String[]]) | Should -Be $true - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } - - It 'Should return the same values as passed as parameters' { - $result = Get-TargetResource @testParameters - $result.ServerName | Should -Be $testParameters.ServerName - $result.InstanceName | Should -Be $testParameters.InstanceName - $result.ServerRoleName | Should -Be $testParameters.ServerRoleName + Assert-VerifiableMock + } - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } + Describe "DSC_SqlRole\Test-TargetResource" -Tag 'Test' { + BeforeAll { + Mock -CommandName Connect-SQL -MockWith $mockConnectSQL -Verifiable } - Context 'When the system is in the desired state, parameter MembersToInclude is assigned a value and ensure is set to Present' { + Context 'When the system is not in the desired state and ensure is set to Absent' { $testParameters = $mockDefaultParameters $testParameters += @{ - ServerRoleName = $mockSqlServerRole - MembersToInclude = $mockSqlServerLoginTwo - } - - It 'Should return the state as present when the correct members exist' { - $result = Get-TargetResource @testParameters - $result.Ensure | Should -Be 'Present' - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } - - It 'Should return the members as not null' { - $result = Get-TargetResource @testParameters - $result.Members | Should -Not -Be $null - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + Ensure = 'Absent' + ServerRoleName = $mockSqlServerRole } - # Regression test for issue #790 - It 'Should return the members as string array' { - $result = Get-TargetResource @testParameters - ($result.Members -is [System.String[]]) | Should -Be $true + $result = Test-TargetResource @testParameters - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should return false when desired server role exist' { + $result | Should -BeFalse } - It 'Should return the same values as passed as parameters' { - $result = Get-TargetResource @testParameters - $result.ServerName | Should -Be $testParameters.ServerName - $result.InstanceName | Should -Be $testParameters.InstanceName - $result.ServerRoleName | Should -Be $testParameters.ServerRoleName - $result.MembersToInclude | Should -Be $testParameters.MembersToInclude - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should be executed once' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } - Context 'When the system is in the desired state, parameter MembersToExclude is assigned a value and ensure is set to Present' { + Context 'When the system is in the desired state and ensure is set to Absent' { $testParameters = $mockDefaultParameters $testParameters += @{ - ServerRoleName = $mockSqlServerRole - MembersToExclude = $mockSqlServerLoginThree + Ensure = 'Absent' + ServerRoleName = 'newServerRole' } - - It 'Should return the state as present when the members does not exist' { - $result = Get-TargetResource @testParameters - $result.Ensure | Should -Be 'Present' - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + $result = Test-TargetResource @testParameters + It 'Should return true when desired server role does not exist' { + $result | Should -BeTrue } - It 'Should return the same values as passed as parameters' { - $result = Get-TargetResource @testParameters - $result.ServerName | Should -Be $testParameters.ServerName - $result.InstanceName | Should -Be $testParameters.InstanceName - $result.ServerRoleName | Should -Be $testParameters.ServerRoleName - $result.MembersToExclude | Should -Be $testParameters.MembersToExclude - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should be executed once' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } - Context 'When the system is not in the desired state, parameter MembersToInclude is assigned a value, parameter Members is assigned a value, and ensure is set to Present' { - It 'Should throw the correct error' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - ServerRoleName = $mockSqlServerRole - Members = $mockEnumMemberNames - MembersToInclude = $mockSqlServerLoginThree - } - - $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - - { Get-TargetResource @testParameters } | Should -Throw $errorMessage - } - - It 'Should call the mock function Connect-SQL' { - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context + Context 'When the system is in the desired state and ensure is set to Present' { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole } - } - - Context 'When the system is not in the desired state, parameter MembersToExclude is assigned a value, parameter Members is assigned a value, and ensure is set to Present' { - It 'Should throw the correct error' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - ServerRoleName = $mockSqlServerRole - Members = $mockEnumMemberNames - MembersToExclude = $mockSqlServerLoginThree - } + $result = Test-TargetResource @testParameters - $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - - { Get-TargetResource @testParameters } | Should -Throw $errorMessage + It 'Should return true when desired server role exist' { + $result | Should -BeTrue } - It 'Should call the mock function Connect-SQL' { + It 'Should be executed once' { Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } @@ -460,303 +408,115 @@ try Context 'When the system is not in the desired state and ensure is set to Present' { $testParameters = $mockDefaultParameters $testParameters += @{ - ServerRoleName = 'UnknownRoleName' - } - - It 'Should return the state as absent when the role does not exist' { - $result = Get-TargetResource @testParameters - $result.Ensure | Should -Be 'Absent' - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + Ensure = 'Present' + ServerRoleName = 'newServerRole' } - It 'Should return the members as null' { - $result = Get-TargetResource @testParameters - $result.Members | Should -Be $null + $result = Test-TargetResource @testParameters - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should return false when desired server role does not exist' { + $result | Should -BeFalse } - It 'Should return the same values as passed as parameters' { - $result = Get-TargetResource @testParameters - $result.ServerName | Should -Be $testParameters.ServerName - $result.InstanceName | Should -Be $testParameters.InstanceName - $result.ServerRoleName | Should -Be $testParameters.ServerRoleName - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should be executed once' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } - Context 'When the system is not in the desired state, parameter Members is assigned a value and ensure is set to Present' { + Context 'When the system is not in the desired state and ensure parameter is set to Present' { $testParameters = $mockDefaultParameters $testParameters += @{ + Ensure = 'Present' ServerRoleName = $mockSqlServerRole - Members = @($mockSqlServerLoginOne, $mockSqlServerLoginThree) - } - - It 'Should return the state as absent when the members in the role are wrong' { - $result = Get-TargetResource @testParameters - $result.Ensure | Should -Be 'Absent' - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } - - It 'Should return the members as not null' { - $result = Get-TargetResource @testParameters - $result.Members | Should -Not -Be $null - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + Members = @($mockSqlServerLoginThree, $mockSqlServerLoginFour) } - # Regression test for issue #790 - It 'Should return the members as string array' { - $result = Get-TargetResource @testParameters - ($result.Members -is [System.String[]]) | Should -Be $true + $result = Test-TargetResource @testParameters - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should return false when desired members are not in desired server role' { + $result | Should -BeFalse } - It 'Should return the same values as passed as parameters' { - $result = Get-TargetResource @testParameters - $result.ServerName | Should -Be $testParameters.ServerName - $result.InstanceName | Should -Be $testParameters.InstanceName - $result.ServerRoleName | Should -Be $testParameters.ServerRoleName - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should be executed once' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } - Context 'When the system is not in the desired state, parameter MembersToInclude is assigned a value and ensure is set to Present' { + Context 'When both the parameters Members and MembersToInclude are assigned a value and ensure is set to Present' { $testParameters = $mockDefaultParameters $testParameters += @{ + Ensure = 'Present' ServerRoleName = $mockSqlServerRole + Members = $mockEnumMemberNames MembersToInclude = $mockSqlServerLoginThree } - It 'Should return the state as absent when the members in the role are missing' { - $result = Get-TargetResource @testParameters - $result.Ensure | Should -Be 'Absent' - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } - - It 'Should return the members as not null' { - $result = Get-TargetResource @testParameters - $result.Members | Should -Not -Be $null + $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should throw the correct error' { + { Test-TargetResource @testParameters } | Should -Throw $errorMessage } - # Regression test for issue #790 - It 'Should return the members as string array' { - $result = Get-TargetResource @testParameters - ($result.Members -is [System.String[]]) | Should -Be $true - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } - - It 'Should return the same values as passed as parameters' { - $result = Get-TargetResource @testParameters - $result.ServerName | Should -Be $testParameters.ServerName - $result.InstanceName | Should -Be $testParameters.InstanceName - $result.ServerRoleName | Should -Be $testParameters.ServerRoleName - $result.MembersToInclude | Should -Be $testParameters.MembersToInclude - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should not be executed' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 0 -Scope Context } } - Context 'When the system is not in the desired state, parameter MembersToExclude is assigned a value and ensure is set to Present' { + Context 'When parameter MembersToInclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { $testParameters = $mockDefaultParameters $testParameters += @{ + Ensure = 'Present' ServerRoleName = $mockSqlServerRole - MembersToExclude = $mockSqlServerLoginTwo + MembersToInclude = $mockSqlServerLoginTwo } - It 'Should return the state as absent when the members in the role are present' { - $result = Get-TargetResource @testParameters - $result.Ensure | Should -Be 'Absent' + $result = Test-TargetResource @testParameters - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should return true when desired server role exist' { + $result | Should -BeTrue } - It 'Should return the same values as passed as parameters' { - $result = Get-TargetResource @testParameters - $result.ServerName | Should -Be $testParameters.ServerName - $result.InstanceName | Should -Be $testParameters.InstanceName - $result.ServerRoleName | Should -Be $testParameters.ServerRoleName - $result.MembersToExclude | Should -Be $testParameters.MembersToExclude - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should be executed once' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } - Context 'When evaluating role membership, case sensitivity should not be used. (Issue #1153)' { - It 'Should return Present when the MemberToInclude is a member of the role.' { - $testParameters = $mockDefaultParameters.Clone() - $testParameters += @{ - ServerRoleName = $mockSqlServerRole - MembersToInclude = $mockSqlServerLoginOne.ToUpper() - } - - $result = Get-TargetResource @testParameters - - $result.Ensure | Should -Be 'Present' - $result.Members | Should -Contain $mockSqlServerLoginOne - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } - - It 'Should return Absent when the MembersToExclude is a member of the role.' { - $testParameters = $mockDefaultParameters.Clone() - $testParameters += @{ - ServerRoleName = $mockSqlServerRole - MembersToExclude = $mockSqlServerLoginOne.ToUpper() - } - - $result = Get-TargetResource @testParameters - - $result.Ensure | Should -Be 'Absent' - $result.Members | Should -Contain $mockSqlServerLoginOne - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + Context 'When parameter MembersToInclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + MembersToInclude = $mockSqlServerLoginThree } - } - - Assert-VerifiableMock - } - Describe "DSC_SqlRole\Test-TargetResource" -Tag 'Test' { - BeforeAll { - Mock -CommandName Connect-SQL -MockWith $mockConnectSQL -Verifiable - } + $result = Test-TargetResource @testParameters - Context 'When the system is not in the desired state and ensure is set to Absent' { - It 'Should return false when desired server role exist' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Absent' - ServerRoleName = $mockSqlServerRole - } - - $result = Test-TargetResource @testParameters - $result | Should -Be $false - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should return false when desired server role does not exist' { + $result | Should -BeFalse } - } - Context 'When the system is in the desired state and ensure is set to Absent' { - It 'Should return true when desired server role does not exist' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Absent' - ServerRoleName = 'newServerRole' - } - - $result = Test-TargetResource @testParameters - $result | Should -Be $true - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should be executed once' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } - Context 'When the system is in the desired state and ensure is set to Present' { - It 'Should return true when desired server role exist' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - } - - $result = Test-TargetResource @testParameters - $result | Should -Be $true - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + Context 'When both the parameters Members and MembersToExclude are assigned a value and ensure is set to Present' { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + Members = $mockEnumMemberNames + MembersToExclude = $mockSqlServerLoginTwo } - } - - Context 'When the system is not in the desired state and ensure parameter is set to Present' { - It 'Should return false when desired members are not in desired server role' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - Members = @($mockSqlServerLoginThree, $mockSqlServerLoginFour) - } - - $result = Test-TargetResource @testParameters - $result | Should -Be $false - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } - } + $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - Context 'When both the parameters Members and MembersToInclude are assigned a value and ensure is set to Present' { It 'Should throw the correct error' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - Members = $mockEnumMemberNames - MembersToInclude = $mockSqlServerLoginThree - } - - $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - - { Test-TargetResource @testParameters } | Should -Throw $errorMessage - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + {Test-TargetResource @testParameters} | Should -Throw $errorMessage } - } - - Context 'When parameter MembersToInclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { - It 'Should return true when desired server role exist' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - MembersToInclude = $mockSqlServerLoginTwo - } - - $result = Test-TargetResource @testParameters - $result | Should -Be $true - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } - } - - Context 'When parameter MembersToInclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { - It 'Should return false when desired server role does not exist' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - MembersToInclude = $mockSqlServerLoginThree - } - - $result = Test-TargetResource @testParameters - $result | Should -Be $false - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It - } - } - - Context 'When both the parameters Members and MembersToExclude are assigned a value and ensure is set to Present' { - It 'Should throw the correct error' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - Members = $mockEnumMemberNames - MembersToExclude = $mockSqlServerLoginTwo - } - - $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - - { Test-TargetResource @testParameters } | Should -Throw $errorMessage - - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should not be executed' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 0 -Scope Context } } @@ -768,11 +528,11 @@ try ServerRoleName = $mockSqlServerRole MembersToExclude = $mockSqlServerLoginThree } - + $mockEnumMemberNames $result = Test-TargetResource @testParameters - $result | Should -Be $true + $result | Should -BeTrue - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } @@ -786,9 +546,9 @@ try } $result = Test-TargetResource @testParameters - $result | Should -Be $false + $result | Should -BeFalse - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } @@ -807,18 +567,20 @@ try } Context 'When the system is not in the desired state and ensure is set to Absent' { - It 'Should not throw when calling the drop method' { - $mockSqlServerRole = 'ServerRoleToDrop' - $mockExpectedServerRoleToDrop = 'ServerRoleToDrop' - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Absent' - ServerRoleName = $mockSqlServerRole - } + $mockSqlServerRole = 'ServerRoleToDrop' + $mockExpectedServerRoleToDrop = 'ServerRoleToDrop' + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Absent' + ServerRoleName = $mockSqlServerRole + } + It 'Should not throw when calling the drop method' { { Set-TargetResource @testParameters } | Should -Not -Throw + } - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should be executed once' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } @@ -1104,7 +866,7 @@ try } Context 'When specifying a list of security principals to include in the Role.' { - It 'Should not throw when a member to inculde is a Role.' { + It 'Should not throw when a member to include is a Role.' { $mockExpectedMemberToAdd = $mockSqlServerChildRole $testParameters = $mockDefaultParameters.Clone() @@ -1240,6 +1002,12 @@ try SecurityPrincipal = $testSecurityPrincipal } + <# + @person doing the code review: This test should fail. the only reason $result is False, is because it is not filled. + eg powershell evolves an empty variable to false. i vote to remove this test, but do not know the history of this test + and why it is in here. + #> + <# Pester will still see the error on the stack regardless of the value used for ErrorAction Wrap this call in a try/catch to swallow the exception and capture the return value. @@ -1253,7 +1021,7 @@ try continue; } - $result | Should -Be $false + $result | Should -BeFalse } } @@ -1265,7 +1033,7 @@ try SecurityPrincipal = $mockSqlServerLoginOne } - Test-SqlSecurityPrincipal @testParameters | Should -Be $true + Test-SqlSecurityPrincipal @testParameters | Should -BeTrue } It 'Should return true when the principal is a Login and case does not match.' { @@ -1274,7 +1042,7 @@ try SecurityPrincipal = $mockSqlServerLoginOne.ToUpper() } - Test-SqlSecurityPrincipal @testParameters | Should -Be $true + Test-SqlSecurityPrincipal @testParameters | Should -BeTrue } It 'Should return true when the principal is a Role.' { @@ -1283,7 +1051,7 @@ try SecurityPrincipal = $mockSqlServerRole } - Test-SqlSecurityPrincipal @testParameters | Should -Be $true + Test-SqlSecurityPrincipal @testParameters | Should -BeTrue } It 'Should return true when the principal is a Role and case does not match.' { @@ -1292,7 +1060,280 @@ try SecurityPrincipal = $mockSqlServerRole.ToUpper() } - Test-SqlSecurityPrincipal @testParameters | Should -Be $true + Test-SqlSecurityPrincipal @testParameters | Should -BeTrue + } + } + } + + Describe 'DSC_SqlRole\Sanitize-InputObjects' -Tag 'Helper' { + Context 'When parameter Members is assigned a value, parameter MembersToInclude is assigned a value.' { + It 'Should throw the correct exception' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + Members = $mockEnumMemberNames + MembersToInclude = $mockSqlServerLoginOne + } + + $testErrorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull + + { Sanitize-InputObjects @testParameters } | Should -Throw -ExpectedMessage $testErrorMessage + } + + } + + Context 'When parameter Members is assigned a value, parameter MembersToExclude is assigned a value.' { + It 'Should throw the correct exception' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + Members = $mockEnumMemberNames + MembersToExclude = $mockSqlServerLoginOne + } + + $testErrorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull + + { Sanitize-InputObjects @testParameters } | Should -Throw -ExpectedMessage $testErrorMessage + } + } + + Context 'When parameter Members is assigned a value and the role is not sysadmin, the output should be the same' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + Members = $mockEnumMemberNames + } + + $result = Sanitize-InputObjects @testParameters + + It 'Should return an array with 2 elements' { + $result.Members | Should -HaveCount 2 + } + + It 'Should return the same elements' { + $result.Members | Should -Be $mockEnumMemberNames + } + + It 'Should not return extra values' { + $result.MembersToInclude | Should -BeNullOrEmpty + $result.MembersToExclude | Should -BeNullOrEmpty + } + } + + Context 'When parameter Members is assigned a value and the role is sysadmin, if SA is in Members, the output should be the same' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + Members = $mockEnumMemberNamesSysAdmin + } + + $result = Sanitize-InputObjects @testParameters + + It 'Should return an array with 3 elements' { + $result.Members | Should -HaveCount 3 + } + + It 'Should return the same elements' { + $result.Members | Should -Be $mockEnumMemberNamesSysAdmin + } + + It 'Should not return extra values' { + $result.MembersToInclude | Should -BeNullOrEmpty + $result.MembersToExclude | Should -BeNullOrEmpty + } + } + + Context 'When parameter Members is assigned a value and the role is sysadmin, if SA is not in Members, SA should be added to the output' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + Members = $mockEnumMemberNames + } + + $result = Sanitize-InputObjects @testParameters + + It 'Should return an array with 3 elements' { + $result.Members | Should -HaveCount 3 + } + + It 'Should have SA in Members' { + $result.Members | Should -Contain $mockSqlServerSA + } + + It 'Should not return extra values' { + $result.MembersToInclude | Should -BeNullOrEmpty + $result.MembersToExclude | Should -BeNullOrEmpty + } + } + + Context 'When parameter MembersToInclude is assigned a value and the role is not sysadmin, if SA is in MembersToInclude, the output should be the same' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + MembersToInclude = $mockEnumMemberNamesSysAdmin + } + + $result = Sanitize-InputObjects @testParameters + + It 'Should return an array with 3 elements' { + $result.MembersToInclude | Should -HaveCount 3 + } + + It 'Should return the elements from Members' { + $result.MembersToInclude | Should -Be $mockEnumMemberNamesSysAdmin + } + + It 'Should not return extra values' { + $result.Members | Should -BeNullOrEmpty + $result.MembersToExclude | Should -BeNullOrEmpty + } + } + + Context 'When parameter MembersToInclude is assigned a value and the role is not sysadmin, if SA is not in MembersToInclude, the output should be the same' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + MembersToInclude = $mockEnumMemberNames + } + + $result = Sanitize-InputObjects @testParameters + + It 'Should return an array with 2 elements' { + $result.MembersToInclude | Should -HaveCount 2 + } + + It 'Should return the elements from Members' { + $result.MembersToInclude | Should -Be $mockEnumMemberNames + } + + It 'Should not return extra values' { + $result.Members | Should -BeNullOrEmpty + $result.MembersToExclude | Should -BeNullOrEmpty + } + } + + Context 'When parameter MembersToInclude is assigned a value and the role is sysadmin, if SA is not in MembersToInclude, the output should be the same' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + MembersToInclude = $mockEnumMemberNames + } + + $result = Sanitize-InputObjects @testParameters + + It 'Should return an array with 2 elements' { + $result.MembersToInclude | Should -HaveCount 2 + } + + It 'Should return the elements from Members' { + $result.MembersToInclude | Should -Be $mockEnumMemberNames + } + + It 'Should not return extra values' { + $result.Members | Should -BeNullOrEmpty + $result.MembersToExclude | Should -BeNullOrEmpty + } + } + + Context 'When parameter MembersToInclude is assigned a value and the role is sysadmin, if SA is in MembersToInclude, the output should be the same' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + MembersToInclude = $mockEnumMemberNamesSysAdmin + } + + $result = Sanitize-InputObjects @testParameters + + It 'Should return an array with 3 elements' { + $result.MembersToInclude | Should -HaveCount 3 + } + + It 'Should return the elements from Members' { + $result.MembersToInclude | Should -Be $mockEnumMemberNamesSysAdmin + } + + It 'Should not return extra values' { + $result.Members | Should -BeNullOrEmpty + $result.MembersToExclude | Should -BeNullOrEmpty + } + } + + Context 'When parameter MembersToExclude is assigned a value and the role is not sysadmin, if SA is in MembersToExclude, the output should be the same' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + MembersToExclude = $mockEnumMemberNamesSysAdmin + } + + $result = Sanitize-InputObjects @testParameters + + It 'Should return an array with 3 elements' { + $result.MembersToExclude | Should -HaveCount 3 + } + + It 'Should return the elements from Members' { + $result.MembersToExclude | Should -Be $mockEnumMemberNamesSysAdmin + } + + It 'Should not return extra values' { + $result.Members | Should -BeNullOrEmpty + $result.MembersToInclude | Should -BeNullOrEmpty + } + } + + Context 'When parameter MembersToExclude is assigned a value and the role is not sysadmin, if SA is not in MembersToExclude, the output should be the same' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + MembersToExclude = $mockEnumMemberNames + } + + $result = Sanitize-InputObjects @testParameters + + It 'Should return an array with 2 elements' { + $result.MembersToExclude | Should -HaveCount 2 + } + + It 'Should return the elements from Members' { + $result.MembersToExclude | Should -Be $mockEnumMemberNames + } + + It 'Should not return extra values' { + $result.Members | Should -BeNullOrEmpty + $result.MembersToInclude | Should -BeNullOrEmpty + } + } + + Context 'When parameter MembersToExclude is assigned a value and the role is sysadmin, if SA is not in MembersToExclude, the output should be the same' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + MembersToExclude = $mockEnumMemberNames + } + + $result = Sanitize-InputObjects @testParameters + + It 'Should return an array with 2 elements' { + $result.MembersToExclude | Should -HaveCount 2 + } + + It 'Should return the elements from Members' { + $result.MembersToExclude | Should -Be $mockEnumMemberNames + } + + It 'Should not return extra values' { + $result.Members | Should -BeNullOrEmpty + $result.MembersToInclude | Should -BeNullOrEmpty + } + } + + Context 'When parameter MembersToExclude is assigned a value and the role is sysadmin, if SA is in MembersToExclude, SA should be removed' { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + MembersToExclude = $mockEnumMemberNamesSysAdmin + } + + $result = Sanitize-InputObjects @testParameters + + It 'Should return an array with 2 elements' { + $result.MembersToExclude | Should -HaveCount 2 + } + + It 'Should return the elements from Members' { + $result.MembersToExclude | Should -Not -Contain $mockSqlServerSA + } + + It 'Should not return extra values' { + $result.Members | Should -BeNullOrEmpty + $result.MembersToInclude | Should -BeNullOrEmpty } } } From bb949fc322d4433c3ed95fbb0e930161fbb74c53 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Tue, 6 Oct 2020 12:10:05 +0200 Subject: [PATCH 02/36] major overhaul fix #550 lots of extra tests --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 685877b4a..9c3b432d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +-SqlRole + - Major overhaul of resource, added lots of tests. + - Added sanitize function for issue #550 + ### Added - SqlEndpoint From 14e1f0466f761309463641e4808103e2e638fb0d Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Tue, 6 Oct 2020 13:34:46 +0200 Subject: [PATCH 03/36] parameterblock added --- source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 1 + 1 file changed, 1 insertion(+) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index d0a52be90..8b992f412 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -631,6 +631,7 @@ function Sanitize-InputObjects [System.String[]] $MembersToExclude, + [Parameter()] [ValidateNotNullOrEmpty()] [System.String] $ServerRoleName From 174e4e7985b6d04dae25936706c7aebc98a98641 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Tue, 6 Oct 2020 15:33:23 +0200 Subject: [PATCH 04/36] fix for changed parameter return in test --- .../DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 10 ++++--- tests/Unit/DSC_SqlRole.Tests.ps1 | 28 ++----------------- 2 files changed, 8 insertions(+), 30 deletions(-) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index 8b992f412..fc3eb925d 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -77,7 +77,9 @@ function Get-TargetResource Ensure = $ensure ServerName = $ServerName InstanceName = $InstanceName - membersInRole = $membersInRole + members = $membersInRole + membersToInclude = $null + membersToExclude = $null } } @@ -394,7 +396,7 @@ function Test-TargetResource if ($Members) { - if ( $null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.membersInRole -DifferenceObject $saneInputParameters.Members)) + if ( $null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.members -DifferenceObject $saneInputParameters.Members)) { Write-Verbose -Message ( $script:localizedData.DesiredMembersNotPresent ` @@ -410,7 +412,7 @@ function Test-TargetResource { foreach ($memberToInclude in $saneInputParameters.MembersToInclude) { - if ($getTargetResourceResult.membersInRole -notcontains $memberToInclude) + if ($getTargetResourceResult.members -notcontains $memberToInclude) { Write-Verbose -Message ( $script:localizedData.MemberNotPresent ` @@ -426,7 +428,7 @@ function Test-TargetResource { foreach ($memberToExclude in $saneInputParameters.MembersToExclude) { - if ($getTargetResourceResult.membersInRole -contains $memberToExclude) + if ($getTargetResourceResult.members -contains $memberToExclude) { Write-Verbose -Message ( $script:localizedData.MemberPresent ` diff --git a/tests/Unit/DSC_SqlRole.Tests.ps1 b/tests/Unit/DSC_SqlRole.Tests.ps1 index 58a8ce293..f192934e0 100644 --- a/tests/Unit/DSC_SqlRole.Tests.ps1 +++ b/tests/Unit/DSC_SqlRole.Tests.ps1 @@ -254,21 +254,11 @@ try $result.Ensure | Should -Be 'Absent' } -# It 'Should be executed once' { -# Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context -# } - It 'Should return the members as null' { -# $result = Get-TargetResource @testParameters $result.membersInRole | Should -Be $null } -# It 'Should be executed once' { -# Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context -# } - It 'Should return the same values as passed as parameters' { -# $result = Get-TargetResource @testParameters $result.ServerName | Should -Be $testParameters.ServerName $result.InstanceName | Should -Be $testParameters.InstanceName $result.ServerRoleName | Should -Be $testParameters.ServerRoleName @@ -291,29 +281,15 @@ try $result.Ensure | Should -Not -Be 'Absent' } -# It 'Should be executed once' { -# Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context -# } - It 'Should return the members as not null' { -# $result = Get-TargetResource @testParameters - $result.membersInRole | Should -Not -Be $null + $result.members | Should -Not -Be $null } -# It 'Should be executed once' { -# Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context -# } - # Regression test for issue #790 It 'Should return the members as string array' { -# $result = Get-TargetResource @testParameters - ($result.membersInRole -is [System.String[]]) | Should -BeTrue + ($result.members -is [System.String[]]) | Should -BeTrue } -# It 'Should be executed once' { -# Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context -# } - It 'Should return the same values as passed as parameters' { # $result = Get-TargetResource @testParameters $result.ServerName | Should -Be $testParameters.ServerName From c16cd25aa1aee6001c14df0f606dfef06cdd4183 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Tue, 6 Oct 2020 17:25:09 +0200 Subject: [PATCH 05/36] Forgot integration tests after rehaul. with new code get-DscConfiguration does not return MembersToInclude and MembersToExclude. --- .../DSC_SqlRole.Integration.Tests.ps1 | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 045dddb1d..de116b5a6 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -76,7 +76,8 @@ try $resourceCurrentState.Ensure | Should -Be 'Present' $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role1Name $resourceCurrentState.Members | Should -Be $ConfigurationData.AllNodes.User4Name - $resourceCurrentState.MembersToInclude | Should -Be $ConfigurationData.AllNodes.User4Name + $resourceCurrentState.MembersToInclude | Should -BeNullOrEmpty + #$resourceCurrentState.MembersToInclude | Should -Be $ConfigurationData.AllNodes.User4Name $resourceCurrentState.MembersToExclude | Should -BeNullOrEmpty } @@ -295,6 +296,7 @@ try $ConfigurationData.AllNodes.User2Name $ConfigurationData.AllNodes.User4Name ) + $resourceCurrentState.MembersToInclude | Should -BeNullOrEmpty $resourceCurrentState.MembersToExclude | Should -BeNullOrEmpty } @@ -346,10 +348,11 @@ try $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role2Name $resourceCurrentState.Members | Should -Be $ConfigurationData.AllNodes.User4Name $resourceCurrentState.MembersToInclude | Should -BeNullOrEmpty - $resourceCurrentState.MembersToExclude | Should -Be @( - $ConfigurationData.AllNodes.User1Name - $ConfigurationData.AllNodes.User2Name - ) + $resourceCurrentState.MembersToExclude | Should -BeNullOrEmpty + #$resourceCurrentState.MembersToExclude | Should -Be @( + # $ConfigurationData.AllNodes.User1Name + # $ConfigurationData.AllNodes.User2Name + #) } It 'Should return $true when Test-DscConfiguration is run' { @@ -512,7 +515,8 @@ try $currentState.Ensure | Should -Be 'Present' $currentstate.Members | Should -BeNullOrEmpty $currentState.MembersToInclude | Should -BeNullOrEmpty - $currentState.MembersToExclude | Should -Be $testMemberName + $currentState.MembersToExclude | Should -BeNullOrEmpty + #$currentState.MembersToExclude | Should -Be $testMemberName } } } From fdd7ac47c3bf9b0cde679fa745a362b02bf5b11d Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Mon, 12 Oct 2020 21:42:12 +0200 Subject: [PATCH 06/36] With dubug code --- CHANGELOG.md | 2 +- .../DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 72 ++++++----- .../DSC_SqlRole.Integration.Tests.ps1 | 4 + tests/Unit/DSC_SqlRole.Tests.ps1 | 118 +++++++----------- 4 files changed, 97 insertions(+), 99 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48ca2b8a8..038861dce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] --SqlRole +- SqlRole - Major overhaul of resource, added lots of tests. - Added sanitize function for issue #550 diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index fc3eb925d..746081212 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -19,8 +19,6 @@ $script:localizedData = Get-LocalizedData -DefaultUICulture 'en-US' .PARAMETER ServerRoleName The name of server role to be created or dropped. - #returns $null if role does not exist. Else an array of all role members. This Array can be empty. - #> function Get-TargetResource { @@ -77,9 +75,9 @@ function Get-TargetResource Ensure = $ensure ServerName = $ServerName InstanceName = $InstanceName - members = $membersInRole - membersToInclude = $null - membersToExclude = $null + Members = $membersInRole + MembersToInclude = $null + MembersToExclude = $null } } @@ -147,6 +145,18 @@ function Set-TargetResource $InstanceName ) + $assertBoundParameterParameters = @{ + BoundParameterList = $PSBoundParameters + MutuallyExclusiveList1 = @( + 'Members' + ) + MutuallyExclusiveList2 = @( + 'MembersToExclude', 'MembersToInclude' + ) + } + + Assert-BoundParameter @assertBoundParameterParameters + $sqlServerObject = Connect-SQL -ServerName $ServerName -InstanceName $InstanceName if ($sqlServerObject) @@ -209,14 +219,14 @@ function Set-TargetResource } } - $saneParameters = @{ + $originalParameters = @{ ServerRoleName = $ServerRoleName Members = $Members MembersToInclude = $MembersToInclude MembersToExclude = $MembersToExclude } - $saneInputParameters = Sanitize-InputObjects @saneParameters + $correctedParameters = Get-CorrectedMemberParameters @originalParameters if ($Members) { @@ -224,7 +234,7 @@ function Set-TargetResource foreach ($memberName in $memberNamesInRoleObject) { - if ($saneInputParameters.Members -notcontains $memberName) + if ($correctedParameters.Members -notcontains $memberName) { Remove-SqlDscServerRoleMember -SqlServerObject $sqlServerObject ` -SecurityPrincipal $memberName ` @@ -232,7 +242,7 @@ function Set-TargetResource } } - foreach ($memberToAdd in $saneInputParameters.Members) + foreach ($memberToAdd in $correctedParameters.Members) { if ($memberNamesInRoleObject -notcontains $memberToAdd) { @@ -248,7 +258,7 @@ function Set-TargetResource { $memberNamesInRoleObject = $sqlServerObject.Roles[$ServerRoleName].EnumMemberNames() - foreach ($memberToInclude in $saneInputParameters.MembersToInclude) + foreach ($memberToInclude in $correctedParameters.MembersToInclude) { if ($memberNamesInRoleObject -notcontains $memberToInclude) { @@ -263,7 +273,7 @@ function Set-TargetResource { $memberNamesInRoleObject = $sqlServerObject.Roles[$ServerRoleName].EnumMemberNames() - foreach ($memberToExclude in $saneInputParameters.MembersToExclude) + foreach ($memberToExclude in $correctedParameters.MembersToExclude) { if ($memberNamesInRoleObject -contains $memberToExclude) { @@ -349,14 +359,26 @@ function Test-TargetResource -f $ServerRoleName ) - $saneParameters = @{ + $assertBoundParameterParameters = @{ + BoundParameterList = $PSBoundParameters + MutuallyExclusiveList1 = @( + 'Members' + ) + MutuallyExclusiveList2 = @( + 'MembersToExclude', 'MembersToInclude' + ) + } + + Assert-BoundParameter @assertBoundParameterParameters + + $originalParameters = @{ ServerRoleName = $ServerRoleName Members = $Members MembersToInclude = $MembersToInclude MembersToExclude = $MembersToExclude } - $saneInputParameters = Sanitize-InputObjects @saneParameters + $correctedParameters = Get-CorrectedMemberParameters @originalParameters $getTargetResourceParameters = @{ InstanceName = $InstanceName @@ -396,7 +418,7 @@ function Test-TargetResource if ($Members) { - if ( $null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.members -DifferenceObject $saneInputParameters.Members)) + if ( $null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.Members -DifferenceObject $correctedParameters.Members)) { Write-Verbose -Message ( $script:localizedData.DesiredMembersNotPresent ` @@ -410,9 +432,9 @@ function Test-TargetResource { if ($MembersToInclude) { - foreach ($memberToInclude in $saneInputParameters.MembersToInclude) + foreach ($memberToInclude in $correctedParameters.MembersToInclude) { - if ($getTargetResourceResult.members -notcontains $memberToInclude) + if ($getTargetResourceResult.Members -notcontains $memberToInclude) { Write-Verbose -Message ( $script:localizedData.MemberNotPresent ` @@ -426,9 +448,9 @@ function Test-TargetResource if ($MembersToExclude) { - foreach ($memberToExclude in $saneInputParameters.MembersToExclude) + foreach ($memberToExclude in $correctedParameters.MembersToExclude) { - if ($getTargetResourceResult.members -contains $memberToExclude) + if ($getTargetResourceResult.Members -contains $memberToExclude) { Write-Verbose -Message ( $script:localizedData.MemberPresent ` @@ -617,7 +639,7 @@ function Test-SqlSecurityPrincipal .PARAMETER ServerRoleName The name of server role to be created or dropped. #> -function Sanitize-InputObjects +function Get-CorrectedMemberParameters # Sanitize-InputObjects { param ( @@ -641,12 +663,6 @@ function Sanitize-InputObjects if ($Members) { - if ($MembersToInclude -or $MembersToExclude) - { - $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - New-InvalidOperationException -Message $errorMessage - } - if ($ServerRoleName -eq 'sysadmin') { if ($Members -notcontains 'SA') @@ -667,9 +683,9 @@ function Sanitize-InputObjects } return @{ - Members = $Members - MembersToInclude = $MembersToInclude - MembersToExclude = $MembersToExclude + Members = [System.String[]]$Members + MembersToInclude = [System.String[]]$MembersToInclude + MembersToExclude = [System.String[]]$MembersToExclude } } diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index de116b5a6..e2c1d8f26 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -148,6 +148,8 @@ try ConfigurationData = $ConfigurationData } + Write-verbose "Config parameters $configurationParameters" + & $configurationName @configurationParameters $startDscConfigurationParameters = @{ @@ -159,6 +161,8 @@ try ErrorAction = 'Stop' } + Write-Verbose "startDscConfigurationParameters $startDscConfigurationParameters" + Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw } diff --git a/tests/Unit/DSC_SqlRole.Tests.ps1 b/tests/Unit/DSC_SqlRole.Tests.ps1 index f192934e0..ea09b8626 100644 --- a/tests/Unit/DSC_SqlRole.Tests.ps1 +++ b/tests/Unit/DSC_SqlRole.Tests.ps1 @@ -282,16 +282,15 @@ try } It 'Should return the members as not null' { - $result.members | Should -Not -Be $null + $result.Members | Should -Not -Be $null } # Regression test for issue #790 It 'Should return the members as string array' { - ($result.members -is [System.String[]]) | Should -BeTrue + ($result.Members -is [System.String[]]) | Should -BeTrue } It 'Should return the same values as passed as parameters' { -# $result = Get-TargetResource @testParameters $result.ServerName | Should -Be $testParameters.ServerName $result.InstanceName | Should -Be $testParameters.InstanceName $result.ServerRoleName | Should -Be $testParameters.ServerRoleName @@ -399,7 +398,7 @@ try } } - Context 'When the system is not in the desired state and ensure parameter is set to Present' { + Context 'When the system is not in the desired state and ensure is set to Present' { $testParameters = $mockDefaultParameters $testParameters += @{ Ensure = 'Present' @@ -407,7 +406,7 @@ try Members = @($mockSqlServerLoginThree, $mockSqlServerLoginFour) } - $result = Test-TargetResource @testParameters + $result = Test-TargetResource @testParameters -verbose It 'Should return false when desired members are not in desired server role' { $result | Should -BeFalse @@ -430,7 +429,7 @@ try $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull It 'Should throw the correct error' { - { Test-TargetResource @testParameters } | Should -Throw $errorMessage + { Test-TargetResource @testParameters } | Should -Throw '(DRC0010)' } It 'Should not be executed' { @@ -461,7 +460,7 @@ try $testParameters = $mockDefaultParameters $testParameters += @{ Ensure = 'Present' - ServerRoleName = $mockSqlServerRole + ServerRoleName = 'RoleNotExist' # $mockSqlServerRole MembersToInclude = $mockSqlServerLoginThree } @@ -488,7 +487,7 @@ try $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull It 'Should throw the correct error' { - {Test-TargetResource @testParameters} | Should -Throw $errorMessage + {Test-TargetResource @testParameters} | Should -Throw '(DRC0010)' } It 'Should not be executed' { @@ -497,33 +496,39 @@ try } Context 'When parameter MembersToExclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + MembersToExclude = $mockSqlServerLoginThree + } + $mockEnumMemberNames + $result = Test-TargetResource @testParameters + It 'Should return true when desired server role does not exist' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - MembersToExclude = $mockSqlServerLoginThree - } - $mockEnumMemberNames - $result = Test-TargetResource @testParameters $result | Should -BeTrue + } + It 'Should be executed once' { Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } Context 'When parameter MembersToExclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { - It 'Should return false when desired server role exist' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - MembersToExclude = $mockSqlServerLoginTwo - } + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + MembersToExclude = $mockSqlServerLoginTwo + } + + $result = Test-TargetResource @testParameters - $result = Test-TargetResource @testParameters + It 'Should return false when desired server role exist' { $result | Should -BeFalse + } + It 'Should be executed once' { Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope Context } } @@ -636,11 +641,13 @@ try MembersToInclude = $mockSqlServerLoginThree } - $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull + #$errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - { Set-TargetResource @testParameters } | Should -Throw $errorMessage + { Set-TargetResource @testParameters } | Should -Throw '(DRC0010)' + } - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + It 'Should should not call Connect-SQL' { + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 0 -Scope Context } } @@ -656,9 +663,9 @@ try $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - { Set-TargetResource @testParameters } | Should -Throw $errorMessage + { Set-TargetResource @testParameters } | Should -Throw '(DRC0010)' - Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 0 -Scope It } } @@ -1041,43 +1048,14 @@ try } } - Describe 'DSC_SqlRole\Sanitize-InputObjects' -Tag 'Helper' { - Context 'When parameter Members is assigned a value, parameter MembersToInclude is assigned a value.' { - It 'Should throw the correct exception' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRole - Members = $mockEnumMemberNames - MembersToInclude = $mockSqlServerLoginOne - } - - $testErrorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - - { Sanitize-InputObjects @testParameters } | Should -Throw -ExpectedMessage $testErrorMessage - } - - } - - Context 'When parameter Members is assigned a value, parameter MembersToExclude is assigned a value.' { - It 'Should throw the correct exception' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRole - Members = $mockEnumMemberNames - MembersToExclude = $mockSqlServerLoginOne - } - - $testErrorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - - { Sanitize-InputObjects @testParameters } | Should -Throw -ExpectedMessage $testErrorMessage - } - } - + Describe 'DSC_SqlRole\Get-CorrectedMemberParameters' -Tag 'Helper' { Context 'When parameter Members is assigned a value and the role is not sysadmin, the output should be the same' { $testParameters = @{ ServerRoleName = $mockSqlServerRole Members = $mockEnumMemberNames } - $result = Sanitize-InputObjects @testParameters + $result = Get-CorrectedMemberParameters @testParameters It 'Should return an array with 2 elements' { $result.Members | Should -HaveCount 2 @@ -1099,7 +1077,7 @@ try Members = $mockEnumMemberNamesSysAdmin } - $result = Sanitize-InputObjects @testParameters + $result = Get-CorrectedMemberParameters @testParameters It 'Should return an array with 3 elements' { $result.Members | Should -HaveCount 3 @@ -1121,7 +1099,7 @@ try Members = $mockEnumMemberNames } - $result = Sanitize-InputObjects @testParameters + $result = Get-CorrectedMemberParameters @testParameters It 'Should return an array with 3 elements' { $result.Members | Should -HaveCount 3 @@ -1143,7 +1121,7 @@ try MembersToInclude = $mockEnumMemberNamesSysAdmin } - $result = Sanitize-InputObjects @testParameters + $result = Get-CorrectedMemberParameters @testParameters It 'Should return an array with 3 elements' { $result.MembersToInclude | Should -HaveCount 3 @@ -1165,7 +1143,7 @@ try MembersToInclude = $mockEnumMemberNames } - $result = Sanitize-InputObjects @testParameters + $result = Get-CorrectedMemberParameters @testParameters It 'Should return an array with 2 elements' { $result.MembersToInclude | Should -HaveCount 2 @@ -1187,7 +1165,7 @@ try MembersToInclude = $mockEnumMemberNames } - $result = Sanitize-InputObjects @testParameters + $result = Get-CorrectedMemberParameters @testParameters It 'Should return an array with 2 elements' { $result.MembersToInclude | Should -HaveCount 2 @@ -1209,7 +1187,7 @@ try MembersToInclude = $mockEnumMemberNamesSysAdmin } - $result = Sanitize-InputObjects @testParameters + $result = Get-CorrectedMemberParameters @testParameters It 'Should return an array with 3 elements' { $result.MembersToInclude | Should -HaveCount 3 @@ -1231,7 +1209,7 @@ try MembersToExclude = $mockEnumMemberNamesSysAdmin } - $result = Sanitize-InputObjects @testParameters + $result = Get-CorrectedMemberParameters @testParameters It 'Should return an array with 3 elements' { $result.MembersToExclude | Should -HaveCount 3 @@ -1253,7 +1231,7 @@ try MembersToExclude = $mockEnumMemberNames } - $result = Sanitize-InputObjects @testParameters + $result = Get-CorrectedMemberParameters @testParameters It 'Should return an array with 2 elements' { $result.MembersToExclude | Should -HaveCount 2 @@ -1275,7 +1253,7 @@ try MembersToExclude = $mockEnumMemberNames } - $result = Sanitize-InputObjects @testParameters + $result = Get-CorrectedMemberParameters @testParameters It 'Should return an array with 2 elements' { $result.MembersToExclude | Should -HaveCount 2 @@ -1297,7 +1275,7 @@ try MembersToExclude = $mockEnumMemberNamesSysAdmin } - $result = Sanitize-InputObjects @testParameters + $result = Get-CorrectedMemberParameters @testParameters It 'Should return an array with 2 elements' { $result.MembersToExclude | Should -HaveCount 2 From ddb2b4582395eeaeaeea3d972241c8a45dd1a0a3 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Mon, 12 Oct 2020 23:01:34 +0200 Subject: [PATCH 07/36] debug pipeline --- tests/Integration/DSC_SqlRole.Integration.Tests.ps1 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index e2c1d8f26..6bfde81f3 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -142,6 +142,10 @@ try Context ('When using configuration {0}' -f $configurationName) { It 'Should compile and apply the MOF without throwing' { { + Write-verbose "ConfigurationName $configurationName" + Write-verbose "TestDrive $TestDrive" + Write-verbose "ConfigurationData $ConfigurationData" + $configurationParameters = @{ OutputPath = $TestDrive # The variable $ConfigurationData was dot-sourced above. From a833614f0b665c5accc1fac5498d8243af02dafe Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Mon, 12 Oct 2020 23:02:52 +0200 Subject: [PATCH 08/36] debuf pipeline --- source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 b/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 index ada6aa051..c65183a85 100644 --- a/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 +++ b/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 @@ -1,11 +1,11 @@ # Localized resources for SqlRole +#MembersToIncludeAndExcludeParamMustBeNull = The parameter MembersToInclude and/or MembersToExclude must not be set, or be set to $null, when parameter Members are used. ConvertFrom-StringData @' GetProperties = Getting properties of the SQL Server role '{0}'. SetProperties = Setting properties of the SQL Server role '{0}'. TestProperties = Testing properties of the SQL Server role '{0}'. EnumMemberNamesServerRoleGetError = Failed to enumerate members of the server role named '{2}' on '{0}\\{1}'. - MembersToIncludeAndExcludeParamMustBeNull = The parameter MembersToInclude and/or MembersToExclude must not be set, or be set to $null, when parameter Members are used. DropServerRoleSetError = Failed to drop the server role named '{2}' on '{0}\\{1}'. CreateServerRoleSetError = Failed to create the server role named '{2}' on '{0}\\{1}'. AddMemberServerRoleSetError = Failed to add member '{3}' to the server role named '{2}' on '{0}\\{1}'. From aeff59c47cb4e6fad858e9ff266b1afac429ba26 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Mon, 12 Oct 2020 23:08:28 +0200 Subject: [PATCH 09/36] new branch --- source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 | 1 + 1 file changed, 1 insertion(+) diff --git a/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 b/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 index c65183a85..8fd5f3bfb 100644 --- a/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 +++ b/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 @@ -1,6 +1,7 @@ # Localized resources for SqlRole #MembersToIncludeAndExcludeParamMustBeNull = The parameter MembersToInclude and/or MembersToExclude must not be set, or be set to $null, when parameter Members are used. + ConvertFrom-StringData @' GetProperties = Getting properties of the SQL Server role '{0}'. SetProperties = Setting properties of the SQL Server role '{0}'. From c772ac178f093b0ced95ec71d4c1bebd8d18d028 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Tue, 13 Oct 2020 08:29:36 +0200 Subject: [PATCH 10/36] with verbose --- tests/Integration/DSC_SqlRole.Integration.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 6bfde81f3..bc995869f 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -168,7 +168,7 @@ try Write-Verbose "startDscConfigurationParameters $startDscConfigurationParameters" Start-DscConfiguration @startDscConfigurationParameters - } | Should -Not -Throw + } | Should -Not -Throw -Verbose } It 'Should be able to call Get-DscConfiguration without throwing' { From e4c3ecf6e15bf4eb5a8a25d2f60cc2662adfcadd Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Tue, 13 Oct 2020 08:51:46 +0200 Subject: [PATCH 11/36] just do it --- tests/Integration/DSC_SqlRole.Integration.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index bc995869f..058dd8979 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -174,7 +174,7 @@ try It 'Should be able to call Get-DscConfiguration without throwing' { { $script:currentConfiguration = Get-DscConfiguration -Verbose -ErrorAction Stop - } | Should -Not -Throw + } | Should -Not -Throw -Verbose } It 'Should have set the resource and all the parameters should match' { From e92d1f3bf9c775d10d172c1e2f0f7403cb2036e6 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Tue, 13 Oct 2020 17:46:55 +0200 Subject: [PATCH 12/36] azure cicd working? --- source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 1 + 1 file changed, 1 insertion(+) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index 746081212..87b62ce71 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -446,6 +446,7 @@ function Test-TargetResource } } + if ($MembersToExclude) { foreach ($memberToExclude in $correctedParameters.MembersToExclude) From 683456bc702d41368a2f2aaceae7e72c0f13a76c Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Tue, 13 Oct 2020 23:13:29 +0200 Subject: [PATCH 13/36] change for cicd --- source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 1 - 1 file changed, 1 deletion(-) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index 87b62ce71..746081212 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -446,7 +446,6 @@ function Test-TargetResource } } - if ($MembersToExclude) { foreach ($memberToExclude in $correctedParameters.MembersToExclude) From 1563d5d1e658badd957f103a25bdb0fc02b05d91 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Tue, 13 Oct 2020 23:22:10 +0200 Subject: [PATCH 14/36] test cicd --- source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 1 + 1 file changed, 1 insertion(+) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index 746081212..d59b4fdab 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -444,6 +444,7 @@ function Test-TargetResource $isServerRoleInDesiredState = $false } } + } if ($MembersToExclude) From 91374bec2ef54748871dd2aa9afa839747b278ad Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Wed, 14 Oct 2020 00:26:07 +0200 Subject: [PATCH 15/36] test change --- .../DSC_SqlRole.Integration.Tests.ps1 | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 058dd8979..f68eb7384 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -139,34 +139,34 @@ try $configurationName = "$($script:dscResourceName)_AddRole3_Config" - Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing' { - { - Write-verbose "ConfigurationName $configurationName" - Write-verbose "TestDrive $TestDrive" - Write-verbose "ConfigurationData $ConfigurationData" + Context ('When using configuration {0}' -f $configurationName) + Write-verbose "ConfigurationName $configurationName" + Write-verbose "TestDrive $TestDrive" + Write-verbose "ConfigurationData $ConfigurationData" - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - Write-verbose "Config parameters $configurationParameters" + Write-verbose "Config parameters $configurationParameters" - & $configurationName @configurationParameters + & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } - Write-Verbose "startDscConfigurationParameters $startDscConfigurationParameters" + Write-Verbose "startDscConfigurationParameters $startDscConfigurationParameters" + It 'Should compile and apply the MOF without throwing' { + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw -Verbose } From 34f8b50bc0c57484c22c4029e0f97a975445d030 Mon Sep 17 00:00:00 2001 From: Fiander <51764122+Fiander@users.noreply.github.com> Date: Wed, 14 Oct 2020 09:14:42 +0200 Subject: [PATCH 16/36] Set up CI with Azure Pipelines [skip ci] --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 664b96bfd..9f600caf9 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -1,7 +1,7 @@ trigger: branches: include: - - master + - SQLRole paths: include: - source/* From e3114f5b530b754efa3f7e8d092c78b68ff2aaa8 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Wed, 14 Oct 2020 10:53:45 +0200 Subject: [PATCH 17/36] fix with { --- tests/Integration/DSC_SqlRole.Integration.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index f68eb7384..642fa7113 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -139,7 +139,7 @@ try $configurationName = "$($script:dscResourceName)_AddRole3_Config" - Context ('When using configuration {0}' -f $configurationName) + Context ('When using configuration {0}' -f $configurationName){ Write-verbose "ConfigurationName $configurationName" Write-verbose "TestDrive $TestDrive" Write-verbose "ConfigurationData $ConfigurationData" From 4d00273dbb28b043e570dbbdeec218ec29d80fd9 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Wed, 14 Oct 2020 14:03:47 +0200 Subject: [PATCH 18/36] yet another try --- tests/Integration/DSC_SqlRole.Integration.Tests.ps1 | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 642fa7113..a042b2f1e 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -140,9 +140,6 @@ try $configurationName = "$($script:dscResourceName)_AddRole3_Config" Context ('When using configuration {0}' -f $configurationName){ - Write-verbose "ConfigurationName $configurationName" - Write-verbose "TestDrive $TestDrive" - Write-verbose "ConfigurationData $ConfigurationData" $configurationParameters = @{ OutputPath = $TestDrive @@ -150,8 +147,6 @@ try ConfigurationData = $ConfigurationData } - Write-verbose "Config parameters $configurationParameters" - & $configurationName @configurationParameters $startDscConfigurationParameters = @{ @@ -163,10 +158,13 @@ try ErrorAction = 'Stop' } - Write-Verbose "startDscConfigurationParameters $startDscConfigurationParameters" - It 'Should compile and apply the MOF without throwing' { { + Write-verbose "ConfigurationName $configurationName" + Write-verbose "TestDrive $TestDrive" + Write-verbose "ConfigurationData $ConfigurationData" + Write-verbose "Config parameters $configurationParameters" + Write-Verbose "startDscConfigurationParameters $startDscConfigurationParameters" Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw -Verbose } From 55d7f95297d358aafe9f2e817a194b20b85ad3ae Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Wed, 14 Oct 2020 17:50:16 +0200 Subject: [PATCH 19/36] jippie --- tests/Integration/DSC_SqlRole.Integration.Tests.ps1 | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index a042b2f1e..56949f0e3 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -165,14 +165,20 @@ try Write-verbose "ConfigurationData $ConfigurationData" Write-verbose "Config parameters $configurationParameters" Write-Verbose "startDscConfigurationParameters $startDscConfigurationParameters" + return 1 + } | Should -be 1 -Verbose + } + + It 'Should compile and apply the MOF without throwing' { + { Start-DscConfiguration @startDscConfigurationParameters - } | Should -Not -Throw -Verbose + } | Should -Not -Throw } It 'Should be able to call Get-DscConfiguration without throwing' { { $script:currentConfiguration = Get-DscConfiguration -Verbose -ErrorAction Stop - } | Should -Not -Throw -Verbose + } | Should -Not -Throw } It 'Should have set the resource and all the parameters should match' { From d6fbb14e043c16e330cad29fbd8b6bbc876e0d17 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Wed, 14 Oct 2020 19:00:20 +0200 Subject: [PATCH 20/36] sjugt --- .../DSC_SqlRole.Integration.Tests.ps1 | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 56949f0e3..1d593773d 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -158,13 +158,20 @@ try ErrorAction = 'Stop' } + + Write-Host "ConfigurationName $configurationName" + Write-Host "TestDrive $TestDrive" + Write-Host "ConfigurationData $ConfigurationData" + Write-Host "Config parameters $configurationParameters" + Write-Host "startDscConfigurationParameters $startDscConfigurationParameters" + It 'Should compile and apply the MOF without throwing' { + Write-verbose "ConfigurationName $configurationName" + Write-verbose "TestDrive $TestDrive" + Write-verbose "ConfigurationData $ConfigurationData" + Write-verbose "Config parameters $configurationParameters" + Write-Verbose "startDscConfigurationParameters $startDscConfigurationParameters" { - Write-verbose "ConfigurationName $configurationName" - Write-verbose "TestDrive $TestDrive" - Write-verbose "ConfigurationData $ConfigurationData" - Write-verbose "Config parameters $configurationParameters" - Write-Verbose "startDscConfigurationParameters $startDscConfigurationParameters" return 1 } | Should -be 1 -Verbose } From e2715cf4ed7adc63799c4f054b46c8c4104f80e7 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Wed, 14 Oct 2020 23:38:47 +0200 Subject: [PATCH 21/36] damm --- .../DSC_SqlRole.Integration.Tests.ps1 | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 1d593773d..053a752d9 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -161,20 +161,13 @@ try Write-Host "ConfigurationName $configurationName" Write-Host "TestDrive $TestDrive" - Write-Host "ConfigurationData $ConfigurationData" - Write-Host "Config parameters $configurationParameters" - Write-Host "startDscConfigurationParameters $startDscConfigurationParameters" - - It 'Should compile and apply the MOF without throwing' { - Write-verbose "ConfigurationName $configurationName" - Write-verbose "TestDrive $TestDrive" - Write-verbose "ConfigurationData $ConfigurationData" - Write-verbose "Config parameters $configurationParameters" - Write-Verbose "startDscConfigurationParameters $startDscConfigurationParameters" - { - return 1 - } | Should -be 1 -Verbose - } +# Write-Host "ConfigurationData $ConfigurationData" +# Write-Host "Config parameters $configurationParameters" +# Write-Host "startDscConfigurationParameters $startDscConfigurationParameters" + $ConfigurationData + $ConfigurationData.AllNodes + $configurationParameters + $startDscConfigurationParameters It 'Should compile and apply the MOF without throwing' { { From 208476bae6597d5064669066606075afd6f1f7c8 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Thu, 15 Oct 2020 08:59:33 +0200 Subject: [PATCH 22/36] 23 --- source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 3 ++- .../Integration/DSC_SqlRole.Integration.Tests.ps1 | 15 +++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index d59b4fdab..1643ca220 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -226,7 +226,8 @@ function Set-TargetResource MembersToExclude = $MembersToExclude } - $correctedParameters = Get-CorrectedMemberParameters @originalParameters + #$correctedParameters = Get-CorrectedMemberParameters @originalParameters + $correctedParameters = $originalParameters.Clone() if ($Members) { diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 053a752d9..6e412a4f3 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -164,10 +164,17 @@ try # Write-Host "ConfigurationData $ConfigurationData" # Write-Host "Config parameters $configurationParameters" # Write-Host "startDscConfigurationParameters $startDscConfigurationParameters" - $ConfigurationData - $ConfigurationData.AllNodes - $configurationParameters - $startDscConfigurationParameters + $cd = $ConfigurationData | out-string + Write-Host "ConfigurationData $cd" + + $cda = $ConfigurationData.AllNodes | out-string + Write-Host "ConfigurationData.AllNodes $cda" + + $cp = $configurationParameters | out-string + Write-Host "configurationParameters $cp" + + $sdcp = $startDscConfigurationParameters | out-string + Write-Host "startDscConfigurationParameters $sdcp" It 'Should compile and apply the MOF without throwing' { { From 2b18e460687e18428ffe326dcb2e3dc79da816d9 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Thu, 15 Oct 2020 10:36:08 +0200 Subject: [PATCH 23/36] 24 --- source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index 1643ca220..b92aabe4e 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -231,22 +231,34 @@ function Set-TargetResource if ($Members) { + Write-Verbose -Message "00" + $cpm = $correctedParameters.Members | Out-String + Write-Verbose -Message "01 $cpm" + $memberNamesInRoleObject = $sqlServerObject.Roles[$ServerRoleName].EnumMemberNames() + $mniro = $memberNamesInRoleObject | Out-String + Write-Verbose -Message "02 $mniro" + foreach ($memberName in $memberNamesInRoleObject) { + Write-Verbose -Message "03 $memberName" if ($correctedParameters.Members -notcontains $memberName) { + Write-Verbose -Message "04" Remove-SqlDscServerRoleMember -SqlServerObject $sqlServerObject ` -SecurityPrincipal $memberName ` -ServerRoleName $ServerRoleName } } + Write-Verbose -Message "05" foreach ($memberToAdd in $correctedParameters.Members) { + Write-Verbose -Message "06 $memberToAdd" if ($memberNamesInRoleObject -notcontains $memberToAdd) { + Write-Verbose -Message "07" Add-SqlDscServerRoleMember -SqlServerObject $sqlServerObject ` -SecurityPrincipal $memberToAdd ` -ServerRoleName $ServerRoleName From 67c97a735eac2e89ed488b4bfdc44b93a087b149 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Thu, 15 Oct 2020 11:44:24 +0200 Subject: [PATCH 24/36] 26 --- .../DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 62 ++++++++++--------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index b92aabe4e..42a2e151c 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -428,50 +428,52 @@ function Test-TargetResource $isServerRoleInDesiredState = $false } - - if ($Members) + else { - if ( $null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.Members -DifferenceObject $correctedParameters.Members)) + if ($Members) { - Write-Verbose -Message ( - $script:localizedData.DesiredMembersNotPresent ` - -f $ServerRoleName - ) + if ( $null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.Members -DifferenceObject $correctedParameters.Members)) + { + Write-Verbose -Message ( + $script:localizedData.DesiredMembersNotPresent ` + -f $ServerRoleName + ) - $isServerRoleInDesiredState = $false + $isServerRoleInDesiredState = $false + } } - } - else - { - if ($MembersToInclude) + else { - foreach ($memberToInclude in $correctedParameters.MembersToInclude) + if ($MembersToInclude) { - if ($getTargetResourceResult.Members -notcontains $memberToInclude) + foreach ($memberToInclude in $correctedParameters.MembersToInclude) { - Write-Verbose -Message ( - $script:localizedData.MemberNotPresent ` - -f $ServerRoleName, $memberToInclude - ) + if ($getTargetResourceResult.Members -notcontains $memberToInclude) + { + Write-Verbose -Message ( + $script:localizedData.MemberNotPresent ` + -f $ServerRoleName, $memberToInclude + ) - $isServerRoleInDesiredState = $false + $isServerRoleInDesiredState = $false + } } - } - } + } - if ($MembersToExclude) - { - foreach ($memberToExclude in $correctedParameters.MembersToExclude) + if ($MembersToExclude) { - if ($getTargetResourceResult.Members -contains $memberToExclude) + foreach ($memberToExclude in $correctedParameters.MembersToExclude) { - Write-Verbose -Message ( - $script:localizedData.MemberPresent ` - -f $ServerRoleName, $memberToExclude - ) + if ($getTargetResourceResult.Members -contains $memberToExclude) + { + Write-Verbose -Message ( + $script:localizedData.MemberPresent ` + -f $ServerRoleName, $memberToExclude + ) - $isServerRoleInDesiredState = $false + $isServerRoleInDesiredState = $false + } } } } From d257c731905f03134af621363fafd120003f134d Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Thu, 15 Oct 2020 13:34:15 +0200 Subject: [PATCH 25/36] last check --- tests/Integration/DSC_SqlRole.Integration.Tests.ps1 | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 6e412a4f3..4e1439573 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -302,7 +302,6 @@ try -and $_.ResourceId -eq $resourceId } - $resourceCurrentState.Ensure | Should -Be 'Present' $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role2Name $resourceCurrentState.Members | Should -Be @( @@ -310,11 +309,6 @@ try $ConfigurationData.AllNodes.User2Name $ConfigurationData.AllNodes.User4Name ) - $resourceCurrentState.MembersToInclude | Should -Be @( - $ConfigurationData.AllNodes.User1Name - $ConfigurationData.AllNodes.User2Name - $ConfigurationData.AllNodes.User4Name - ) $resourceCurrentState.MembersToInclude | Should -BeNullOrEmpty $resourceCurrentState.MembersToExclude | Should -BeNullOrEmpty } @@ -362,16 +356,11 @@ try -and $_.ResourceId -eq $resourceId } - $resourceCurrentState.Ensure | Should -Be 'Present' $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role2Name $resourceCurrentState.Members | Should -Be $ConfigurationData.AllNodes.User4Name $resourceCurrentState.MembersToInclude | Should -BeNullOrEmpty $resourceCurrentState.MembersToExclude | Should -BeNullOrEmpty - #$resourceCurrentState.MembersToExclude | Should -Be @( - # $ConfigurationData.AllNodes.User1Name - # $ConfigurationData.AllNodes.User2Name - #) } It 'Should return $true when Test-DscConfiguration is run' { @@ -417,7 +406,6 @@ try -and $_.ResourceId -eq $resourceId } - $resourceCurrentState.Ensure | Should -Be 'Absent' $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role3Name $resourceCurrentState.Members | Should -BeNullOrEmpty From 02e4cd98e2c09f29e000476047bb8369fb0cd967 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Thu, 15 Oct 2020 14:49:14 +0200 Subject: [PATCH 26/36] last one? --- tests/Integration/DSC_SqlRole.Integration.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 4e1439573..fa5b7b41a 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -475,7 +475,7 @@ try $currentState.Ensure | Should -Be 'Present' $currentState.Members | Should -Be @($testMemberName) - $currentState.MembersToInclude | Should -Be @($testMemberName) + $currentState.MembersToInclude | Should -BeNullOrEmpty $currentState.MembersToExclude | Should -BeNullOrEmpty } } From 87ff1483f4b245e0d0d3ae29767d7639b4c6dab8 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Fri, 16 Oct 2020 00:11:09 +0200 Subject: [PATCH 27/36] last cleanup --- CHANGELOG.md | 9 +- .../DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 22 ++--- source/DSCResources/DSC_SqlRole/README.md | 2 + .../en-US/DSC_SqlRole.strings.psd1 | 3 - .../DSC_SqlRole.Integration.Tests.ps1 | 86 +++++++------------ tests/Unit/DSC_SqlRole.Tests.ps1 | 21 +++-- 6 files changed, 61 insertions(+), 82 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 038861dce..dc02a9746 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - SqlRole - - Major overhaul of resource, added lots of tests. - - Added sanitize function for issue #550 + - Major overhaul of resource + - Removed decision making from get-TargetResource; this prevented a simple solution for + issue #550. it now just tels if a role exists or not. And what members are in that + role. MembersToInclude and MembersToExclude now always return $null + - Added sanitize function (Get-CorrectedMemberParameters) to make it so for the + sysadmin role SA does not get altered. ([issue #550](https://github.com/dsccommunity/SqlServerDsc/issues/550)) + - added lots of tests. ### Added diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index 42a2e151c..0f81699fa 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -226,39 +226,26 @@ function Set-TargetResource MembersToExclude = $MembersToExclude } - #$correctedParameters = Get-CorrectedMemberParameters @originalParameters - $correctedParameters = $originalParameters.Clone() + $correctedParameters = Get-CorrectedMemberParameters @originalParameters if ($Members) { - Write-Verbose -Message "00" - $cpm = $correctedParameters.Members | Out-String - Write-Verbose -Message "01 $cpm" - $memberNamesInRoleObject = $sqlServerObject.Roles[$ServerRoleName].EnumMemberNames() - $mniro = $memberNamesInRoleObject | Out-String - Write-Verbose -Message "02 $mniro" - foreach ($memberName in $memberNamesInRoleObject) { - Write-Verbose -Message "03 $memberName" if ($correctedParameters.Members -notcontains $memberName) { - Write-Verbose -Message "04" Remove-SqlDscServerRoleMember -SqlServerObject $sqlServerObject ` -SecurityPrincipal $memberName ` -ServerRoleName $ServerRoleName } } - Write-Verbose -Message "05" foreach ($memberToAdd in $correctedParameters.Members) { - Write-Verbose -Message "06 $memberToAdd" if ($memberNamesInRoleObject -notcontains $memberToAdd) { - Write-Verbose -Message "07" Add-SqlDscServerRoleMember -SqlServerObject $sqlServerObject ` -SecurityPrincipal $memberToAdd ` -ServerRoleName $ServerRoleName @@ -630,7 +617,8 @@ function Test-SqlSecurityPrincipal # Principal is neither a Login nor a Server role, raise exception New-ObjectNotFoundException -Message $errorMessage - return $false + #this is never reached. Commented out; This should be removed togeter with row 979 till 1017 in DSC_SqlRole.Tests.ps1, but not my decision + #return $false } } @@ -641,7 +629,7 @@ function Test-SqlSecurityPrincipal .SYNOPSIS This function sanitizes the parameters If Members is filled, MembersToInclude and MembersToExclude should be empty. - If ServerRoleName is sysadmin, make sure we dont try to alter SA. + If ServerRoleName is sysadmin, make sure we dont try to delete SA from it. .PARAMETER Members The members the server role should have. @@ -655,7 +643,7 @@ function Test-SqlSecurityPrincipal .PARAMETER ServerRoleName The name of server role to be created or dropped. #> -function Get-CorrectedMemberParameters # Sanitize-InputObjects +function Get-CorrectedMemberParameters { param ( diff --git a/source/DSCResources/DSC_SqlRole/README.md b/source/DSCResources/DSC_SqlRole/README.md index 2c2401fab..dec1b3d56 100644 --- a/source/DSCResources/DSC_SqlRole/README.md +++ b/source/DSCResources/DSC_SqlRole/README.md @@ -5,6 +5,8 @@ The `SqlRole` DSC resource is used to create a server role, when is set to `'Absent'`. The resource also manages members in both built-in and user created server roles. +When the target role is sysadmin, SA wil not be removed from this role. + For more information about server roles, please read the below articles. * [Create a Server Role](https://msdn.microsoft.com/en-us/library/ee677627.aspx) diff --git a/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 b/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 index 8fd5f3bfb..cfb6dfa40 100644 --- a/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 +++ b/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 @@ -1,7 +1,4 @@ # Localized resources for SqlRole -#MembersToIncludeAndExcludeParamMustBeNull = The parameter MembersToInclude and/or MembersToExclude must not be set, or be set to $null, when parameter Members are used. - - ConvertFrom-StringData @' GetProperties = Getting properties of the SQL Server role '{0}'. SetProperties = Setting properties of the SQL Server role '{0}'. diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index fa5b7b41a..0e6e314c6 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -77,7 +77,6 @@ try $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role1Name $resourceCurrentState.Members | Should -Be $ConfigurationData.AllNodes.User4Name $resourceCurrentState.MembersToInclude | Should -BeNullOrEmpty - #$resourceCurrentState.MembersToInclude | Should -Be $ConfigurationData.AllNodes.User4Name $resourceCurrentState.MembersToExclude | Should -BeNullOrEmpty } @@ -89,25 +88,25 @@ try $configurationName = "$($script:dscResourceName)_AddRole2_Config" Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing' { - { - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - & $configurationName @configurationParameters + & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + It 'Should compile and apply the MOF without throwing' { + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw } @@ -124,7 +123,6 @@ try -and $_.ResourceId -eq $resourceId } - $resourceCurrentState.Ensure | Should -Be 'Present' $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role2Name $resourceCurrentState.Members | Should -BeNullOrEmpty @@ -140,7 +138,6 @@ try $configurationName = "$($script:dscResourceName)_AddRole3_Config" Context ('When using configuration {0}' -f $configurationName){ - $configurationParameters = @{ OutputPath = $TestDrive # The variable $ConfigurationData was dot-sourced above. @@ -158,24 +155,6 @@ try ErrorAction = 'Stop' } - - Write-Host "ConfigurationName $configurationName" - Write-Host "TestDrive $TestDrive" -# Write-Host "ConfigurationData $ConfigurationData" -# Write-Host "Config parameters $configurationParameters" -# Write-Host "startDscConfigurationParameters $startDscConfigurationParameters" - $cd = $ConfigurationData | out-string - Write-Host "ConfigurationData $cd" - - $cda = $ConfigurationData.AllNodes | out-string - Write-Host "ConfigurationData.AllNodes $cda" - - $cp = $configurationParameters | out-string - Write-Host "configurationParameters $cp" - - $sdcp = $startDscConfigurationParameters | out-string - Write-Host "startDscConfigurationParameters $sdcp" - It 'Should compile and apply the MOF without throwing' { { Start-DscConfiguration @startDscConfigurationParameters @@ -213,25 +192,25 @@ try $configurationName = "$($script:dscResourceName)_Role1_ChangeMembers_Config" Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing' { - { - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - & $configurationName @configurationParameters + & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + It 'Should compile and apply the MOF without throwing' { + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw } @@ -523,7 +502,6 @@ try $currentstate.Members | Should -BeNullOrEmpty $currentState.MembersToInclude | Should -BeNullOrEmpty $currentState.MembersToExclude | Should -BeNullOrEmpty - #$currentState.MembersToExclude | Should -Be $testMemberName } } } diff --git a/tests/Unit/DSC_SqlRole.Tests.ps1 b/tests/Unit/DSC_SqlRole.Tests.ps1 index ea09b8626..a44727b5c 100644 --- a/tests/Unit/DSC_SqlRole.Tests.ps1 +++ b/tests/Unit/DSC_SqlRole.Tests.ps1 @@ -977,6 +977,15 @@ try { Test-SqlSecurityPrincipal @testParameters } | Should -Throw -ExpectedMessage $testErrorMessage } + + + + + <# + @person doing the code review: This test should fail. the only reason $result is False, is because it is not filled. + eg powershell evolves an empty variable to false. i vote to remove this test, but do not know the history of this test + and why it is in here. + #> It 'Should return false when ErrorAction is set to SilentlyContinue' { $testSecurityPrincipal = 'Nabrond' @@ -985,12 +994,6 @@ try SecurityPrincipal = $testSecurityPrincipal } - <# - @person doing the code review: This test should fail. the only reason $result is False, is because it is not filled. - eg powershell evolves an empty variable to false. i vote to remove this test, but do not know the history of this test - and why it is in here. - #> - <# Pester will still see the error on the stack regardless of the value used for ErrorAction Wrap this call in a try/catch to swallow the exception and capture the return value. @@ -1006,6 +1009,12 @@ try $result | Should -BeFalse } + + + + + + } Context 'When the security principal exists.' { From 3742d0f0a5fec245ce2b5bd06d84e3aba160660c Mon Sep 17 00:00:00 2001 From: Fiander <51764122+Fiander@users.noreply.github.com> Date: Fri, 16 Oct 2020 08:57:21 +0200 Subject: [PATCH 28/36] Sqlrole (#2) Small fix with new role with members. And lots of cleanup --- CHANGELOG.md | 9 +- azure-pipelines.yml | 2 +- .../DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 68 +++++----- source/DSCResources/DSC_SqlRole/README.md | 2 + .../en-US/DSC_SqlRole.strings.psd1 | 2 - .../DSC_SqlRole.Integration.Tests.ps1 | 119 ++++++++---------- tests/Unit/DSC_SqlRole.Tests.ps1 | 21 +++- 7 files changed, 111 insertions(+), 112 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 038861dce..dc02a9746 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - SqlRole - - Major overhaul of resource, added lots of tests. - - Added sanitize function for issue #550 + - Major overhaul of resource + - Removed decision making from get-TargetResource; this prevented a simple solution for + issue #550. it now just tels if a role exists or not. And what members are in that + role. MembersToInclude and MembersToExclude now always return $null + - Added sanitize function (Get-CorrectedMemberParameters) to make it so for the + sysadmin role SA does not get altered. ([issue #550](https://github.com/dsccommunity/SqlServerDsc/issues/550)) + - added lots of tests. ### Added diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 664b96bfd..9f600caf9 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -1,7 +1,7 @@ trigger: branches: include: - - master + - SQLRole paths: include: - source/* diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index 746081212..0f81699fa 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -415,49 +415,52 @@ function Test-TargetResource $isServerRoleInDesiredState = $false } - - if ($Members) + else { - if ( $null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.Members -DifferenceObject $correctedParameters.Members)) + if ($Members) { - Write-Verbose -Message ( - $script:localizedData.DesiredMembersNotPresent ` - -f $ServerRoleName - ) + if ( $null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.Members -DifferenceObject $correctedParameters.Members)) + { + Write-Verbose -Message ( + $script:localizedData.DesiredMembersNotPresent ` + -f $ServerRoleName + ) - $isServerRoleInDesiredState = $false + $isServerRoleInDesiredState = $false + } } - } - else - { - if ($MembersToInclude) + else { - foreach ($memberToInclude in $correctedParameters.MembersToInclude) + if ($MembersToInclude) { - if ($getTargetResourceResult.Members -notcontains $memberToInclude) + foreach ($memberToInclude in $correctedParameters.MembersToInclude) { - Write-Verbose -Message ( - $script:localizedData.MemberNotPresent ` - -f $ServerRoleName, $memberToInclude - ) + if ($getTargetResourceResult.Members -notcontains $memberToInclude) + { + Write-Verbose -Message ( + $script:localizedData.MemberNotPresent ` + -f $ServerRoleName, $memberToInclude + ) - $isServerRoleInDesiredState = $false + $isServerRoleInDesiredState = $false + } } + } - } - if ($MembersToExclude) - { - foreach ($memberToExclude in $correctedParameters.MembersToExclude) + if ($MembersToExclude) { - if ($getTargetResourceResult.Members -contains $memberToExclude) + foreach ($memberToExclude in $correctedParameters.MembersToExclude) { - Write-Verbose -Message ( - $script:localizedData.MemberPresent ` - -f $ServerRoleName, $memberToExclude - ) + if ($getTargetResourceResult.Members -contains $memberToExclude) + { + Write-Verbose -Message ( + $script:localizedData.MemberPresent ` + -f $ServerRoleName, $memberToExclude + ) - $isServerRoleInDesiredState = $false + $isServerRoleInDesiredState = $false + } } } } @@ -614,7 +617,8 @@ function Test-SqlSecurityPrincipal # Principal is neither a Login nor a Server role, raise exception New-ObjectNotFoundException -Message $errorMessage - return $false + #this is never reached. Commented out; This should be removed togeter with row 979 till 1017 in DSC_SqlRole.Tests.ps1, but not my decision + #return $false } } @@ -625,7 +629,7 @@ function Test-SqlSecurityPrincipal .SYNOPSIS This function sanitizes the parameters If Members is filled, MembersToInclude and MembersToExclude should be empty. - If ServerRoleName is sysadmin, make sure we dont try to alter SA. + If ServerRoleName is sysadmin, make sure we dont try to delete SA from it. .PARAMETER Members The members the server role should have. @@ -639,7 +643,7 @@ function Test-SqlSecurityPrincipal .PARAMETER ServerRoleName The name of server role to be created or dropped. #> -function Get-CorrectedMemberParameters # Sanitize-InputObjects +function Get-CorrectedMemberParameters { param ( diff --git a/source/DSCResources/DSC_SqlRole/README.md b/source/DSCResources/DSC_SqlRole/README.md index 2c2401fab..dec1b3d56 100644 --- a/source/DSCResources/DSC_SqlRole/README.md +++ b/source/DSCResources/DSC_SqlRole/README.md @@ -5,6 +5,8 @@ The `SqlRole` DSC resource is used to create a server role, when is set to `'Absent'`. The resource also manages members in both built-in and user created server roles. +When the target role is sysadmin, SA wil not be removed from this role. + For more information about server roles, please read the below articles. * [Create a Server Role](https://msdn.microsoft.com/en-us/library/ee677627.aspx) diff --git a/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 b/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 index ada6aa051..cfb6dfa40 100644 --- a/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 +++ b/source/DSCResources/DSC_SqlRole/en-US/DSC_SqlRole.strings.psd1 @@ -1,11 +1,9 @@ # Localized resources for SqlRole - ConvertFrom-StringData @' GetProperties = Getting properties of the SQL Server role '{0}'. SetProperties = Setting properties of the SQL Server role '{0}'. TestProperties = Testing properties of the SQL Server role '{0}'. EnumMemberNamesServerRoleGetError = Failed to enumerate members of the server role named '{2}' on '{0}\\{1}'. - MembersToIncludeAndExcludeParamMustBeNull = The parameter MembersToInclude and/or MembersToExclude must not be set, or be set to $null, when parameter Members are used. DropServerRoleSetError = Failed to drop the server role named '{2}' on '{0}\\{1}'. CreateServerRoleSetError = Failed to create the server role named '{2}' on '{0}\\{1}'. AddMemberServerRoleSetError = Failed to add member '{3}' to the server role named '{2}' on '{0}\\{1}'. diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index e2c1d8f26..0e6e314c6 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -77,7 +77,6 @@ try $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role1Name $resourceCurrentState.Members | Should -Be $ConfigurationData.AllNodes.User4Name $resourceCurrentState.MembersToInclude | Should -BeNullOrEmpty - #$resourceCurrentState.MembersToInclude | Should -Be $ConfigurationData.AllNodes.User4Name $resourceCurrentState.MembersToExclude | Should -BeNullOrEmpty } @@ -89,25 +88,25 @@ try $configurationName = "$($script:dscResourceName)_AddRole2_Config" Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing' { - { - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - & $configurationName @configurationParameters + & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + It 'Should compile and apply the MOF without throwing' { + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw } @@ -124,7 +123,6 @@ try -and $_.ResourceId -eq $resourceId } - $resourceCurrentState.Ensure | Should -Be 'Present' $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role2Name $resourceCurrentState.Members | Should -BeNullOrEmpty @@ -139,30 +137,26 @@ try $configurationName = "$($script:dscResourceName)_AddRole3_Config" - Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing' { - { - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } - - Write-verbose "Config parameters $configurationParameters" - - & $configurationName @configurationParameters + Context ('When using configuration {0}' -f $configurationName){ + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } + & $configurationName @configurationParameters - Write-Verbose "startDscConfigurationParameters $startDscConfigurationParameters" + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + It 'Should compile and apply the MOF without throwing' { + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw } @@ -198,25 +192,25 @@ try $configurationName = "$($script:dscResourceName)_Role1_ChangeMembers_Config" Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing' { - { - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - & $configurationName @configurationParameters + & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + It 'Should compile and apply the MOF without throwing' { + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw } @@ -287,7 +281,6 @@ try -and $_.ResourceId -eq $resourceId } - $resourceCurrentState.Ensure | Should -Be 'Present' $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role2Name $resourceCurrentState.Members | Should -Be @( @@ -295,11 +288,6 @@ try $ConfigurationData.AllNodes.User2Name $ConfigurationData.AllNodes.User4Name ) - $resourceCurrentState.MembersToInclude | Should -Be @( - $ConfigurationData.AllNodes.User1Name - $ConfigurationData.AllNodes.User2Name - $ConfigurationData.AllNodes.User4Name - ) $resourceCurrentState.MembersToInclude | Should -BeNullOrEmpty $resourceCurrentState.MembersToExclude | Should -BeNullOrEmpty } @@ -347,16 +335,11 @@ try -and $_.ResourceId -eq $resourceId } - $resourceCurrentState.Ensure | Should -Be 'Present' $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role2Name $resourceCurrentState.Members | Should -Be $ConfigurationData.AllNodes.User4Name $resourceCurrentState.MembersToInclude | Should -BeNullOrEmpty $resourceCurrentState.MembersToExclude | Should -BeNullOrEmpty - #$resourceCurrentState.MembersToExclude | Should -Be @( - # $ConfigurationData.AllNodes.User1Name - # $ConfigurationData.AllNodes.User2Name - #) } It 'Should return $true when Test-DscConfiguration is run' { @@ -402,7 +385,6 @@ try -and $_.ResourceId -eq $resourceId } - $resourceCurrentState.Ensure | Should -Be 'Absent' $resourceCurrentState.ServerRoleName | Should -Be $ConfigurationData.AllNodes.Role3Name $resourceCurrentState.Members | Should -BeNullOrEmpty @@ -472,7 +454,7 @@ try $currentState.Ensure | Should -Be 'Present' $currentState.Members | Should -Be @($testMemberName) - $currentState.MembersToInclude | Should -Be @($testMemberName) + $currentState.MembersToInclude | Should -BeNullOrEmpty $currentState.MembersToExclude | Should -BeNullOrEmpty } } @@ -520,7 +502,6 @@ try $currentstate.Members | Should -BeNullOrEmpty $currentState.MembersToInclude | Should -BeNullOrEmpty $currentState.MembersToExclude | Should -BeNullOrEmpty - #$currentState.MembersToExclude | Should -Be $testMemberName } } } diff --git a/tests/Unit/DSC_SqlRole.Tests.ps1 b/tests/Unit/DSC_SqlRole.Tests.ps1 index ea09b8626..a44727b5c 100644 --- a/tests/Unit/DSC_SqlRole.Tests.ps1 +++ b/tests/Unit/DSC_SqlRole.Tests.ps1 @@ -977,6 +977,15 @@ try { Test-SqlSecurityPrincipal @testParameters } | Should -Throw -ExpectedMessage $testErrorMessage } + + + + + <# + @person doing the code review: This test should fail. the only reason $result is False, is because it is not filled. + eg powershell evolves an empty variable to false. i vote to remove this test, but do not know the history of this test + and why it is in here. + #> It 'Should return false when ErrorAction is set to SilentlyContinue' { $testSecurityPrincipal = 'Nabrond' @@ -985,12 +994,6 @@ try SecurityPrincipal = $testSecurityPrincipal } - <# - @person doing the code review: This test should fail. the only reason $result is False, is because it is not filled. - eg powershell evolves an empty variable to false. i vote to remove this test, but do not know the history of this test - and why it is in here. - #> - <# Pester will still see the error on the stack regardless of the value used for ErrorAction Wrap this call in a try/catch to swallow the exception and capture the return value. @@ -1006,6 +1009,12 @@ try $result | Should -BeFalse } + + + + + + } Context 'When the security principal exists.' { From 2cb8255eea5cb1fc1f84731eaf7a589470f61c8d Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Fri, 16 Oct 2020 09:43:29 +0200 Subject: [PATCH 29/36] axidential updated azure-pipelines with my own version --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 9f600caf9..02d01c1b8 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -1,7 +1,7 @@ trigger: branches: include: - - SQLRole + - Master paths: include: - source/* From dc92c33ec41c7dfd9a3c84144ce554fceb9ab43a Mon Sep 17 00:00:00 2001 From: Fiander <51764122+Fiander@users.noreply.github.com> Date: Fri, 16 Oct 2020 19:39:50 +0200 Subject: [PATCH 30/36] Update azure-pipelines.yml --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 9f600caf9..664b96bfd 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -1,7 +1,7 @@ trigger: branches: include: - - SQLRole + - master paths: include: - source/* From 423f6f5f02056699687a354d347e6f94b5837fb1 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Sat, 31 Oct 2020 20:57:21 +0100 Subject: [PATCH 31/36] some changes as requested --- .../DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 3 - source/DSCResources/DSC_SqlRole/README.md | 4 +- .../DSC_SqlRole.Integration.Tests.ps1 | 187 +++++---- tests/Unit/DSC_SqlRole.Tests.ps1 | 388 +++++++++--------- 4 files changed, 303 insertions(+), 279 deletions(-) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index 0f81699fa..4c91bdd03 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -616,9 +616,6 @@ function Test-SqlSecurityPrincipal # Principal is neither a Login nor a Server role, raise exception New-ObjectNotFoundException -Message $errorMessage - - #this is never reached. Commented out; This should be removed togeter with row 979 till 1017 in DSC_SqlRole.Tests.ps1, but not my decision - #return $false } } diff --git a/source/DSCResources/DSC_SqlRole/README.md b/source/DSCResources/DSC_SqlRole/README.md index dec1b3d56..1c312b26b 100644 --- a/source/DSCResources/DSC_SqlRole/README.md +++ b/source/DSCResources/DSC_SqlRole/README.md @@ -5,7 +5,9 @@ The `SqlRole` DSC resource is used to create a server role, when is set to `'Absent'`. The resource also manages members in both built-in and user created server roles. -When the target role is sysadmin, SA wil not be removed from this role. +When the target role is sysadmin the DSC resource will prevent the user +'sa' from being removed. This is done to keep the DSC resource from +throwing an error since SQL Server does not allow this user to be removed. For more information about server roles, please read the below articles. diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 0e6e314c6..77d1a9b03 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -88,21 +88,23 @@ try $configurationName = "$($script:dscResourceName)_AddRole2_Config" Context ('When using configuration {0}' -f $configurationName) { - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } + BeforeAll { + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - & $configurationName @configurationParameters + & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } } It 'Should compile and apply the MOF without throwing' { @@ -138,21 +140,23 @@ try $configurationName = "$($script:dscResourceName)_AddRole3_Config" Context ('When using configuration {0}' -f $configurationName){ - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } + BeforeAll { + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - & $configurationName @configurationParameters + & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } } It 'Should compile and apply the MOF without throwing' { @@ -192,21 +196,23 @@ try $configurationName = "$($script:dscResourceName)_Role1_ChangeMembers_Config" Context ('When using configuration {0}' -f $configurationName) { - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } + BeforeAll { + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - & $configurationName @configurationParameters + & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } } It 'Should compile and apply the MOF without throwing' { @@ -246,25 +252,27 @@ try $configurationName = "$($script:dscResourceName)_Role2_AddMembers_Config" Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing' { - { + BeforeAll { $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - & $configurationName @configurationParameters + & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + } + It 'Should compile and apply the MOF without throwing' { + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw } @@ -300,25 +308,27 @@ try $configurationName = "$($script:dscResourceName)_Role2_RemoveMembers_Config" Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing' { - { + BeforeAll { $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - & $configurationName @configurationParameters + & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + } + It 'Should compile and apply the MOF without throwing' { + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw } @@ -350,25 +360,27 @@ try $configurationName = "$($script:dscResourceName)_RemoveRole3_Config" Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing' { - { - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } + BeforeAll { + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } - & $configurationName @configurationParameters + & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + } + It 'Should compile and apply the MOF without throwing' { + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw } @@ -485,8 +497,9 @@ try } It 'Should be able to call Get-DscConfiguration without throwing an exception' { - { $script:currentConfiguration = Get-DscConfiguration -Verbose -ErrorAction Stop } | - Should -Not -Throw + { + $script:currentConfiguration = Get-DscConfiguration -Verbose -ErrorAction Stop + } | Should -Not -Throw } It "Should have set the resource and all values should match for $($ConfigurationData.AllNodes.Role5Name)." { diff --git a/tests/Unit/DSC_SqlRole.Tests.ps1 b/tests/Unit/DSC_SqlRole.Tests.ps1 index a44727b5c..911da3628 100644 --- a/tests/Unit/DSC_SqlRole.Tests.ps1 +++ b/tests/Unit/DSC_SqlRole.Tests.ps1 @@ -243,12 +243,14 @@ try } Context 'When the system is in the desired state and ensure is set to Absent' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - ServerRoleName = 'UnknownRoleName' - } + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + ServerRoleName = 'UnknownRoleName' + } - $result = Get-TargetResource @testParameters + $result = Get-TargetResource @testParameters + } It 'Should return the state as absent when the role does not exist' { $result.Ensure | Should -Be 'Absent' @@ -270,12 +272,14 @@ try } Context 'When the system is not in the desired state and ensure is set to Absent' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - ServerRoleName = $mockSqlServerRole - } + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + ServerRoleName = $mockSqlServerRole + } - $result = Get-TargetResource @testParameters + $result = Get-TargetResource @testParameters + } It 'Should not return the state as absent when the role exist' { $result.Ensure | Should -Not -Be 'Absent' @@ -302,7 +306,7 @@ try } Context 'When passing values to parameters and throwing with EnumMemberNames method' { - It 'Should throw the correct error' { + BeforeAll { $mockInvalidOperationForEnumMethod = $true $testParameters = $mockDefaultParameters $testParameters += @{ @@ -311,7 +315,9 @@ try $errorMessage = $script:localizedData.EnumMemberNamesServerRoleGetError ` -f $mockServerName, $mockInstanceName, $mockSqlServerRole + } + It 'Should throw the correct error' { { Get-TargetResource @testParameters } | Should -Throw $errorMessage } @@ -329,13 +335,15 @@ try } Context 'When the system is not in the desired state and ensure is set to Absent' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Absent' - ServerRoleName = $mockSqlServerRole - } + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Absent' + ServerRoleName = $mockSqlServerRole + } - $result = Test-TargetResource @testParameters + $result = Test-TargetResource @testParameters + } It 'Should return false when desired server role exist' { $result | Should -BeFalse @@ -347,12 +355,16 @@ try } Context 'When the system is in the desired state and ensure is set to Absent' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Absent' - ServerRoleName = 'newServerRole' + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Absent' + ServerRoleName = 'newServerRole' + } + + $result = Test-TargetResource @testParameters } - $result = Test-TargetResource @testParameters + It 'Should return true when desired server role does not exist' { $result | Should -BeTrue } @@ -363,13 +375,15 @@ try } Context 'When the system is in the desired state and ensure is set to Present' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - } + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + } - $result = Test-TargetResource @testParameters + $result = Test-TargetResource @testParameters + } It 'Should return true when desired server role exist' { $result | Should -BeTrue @@ -381,13 +395,15 @@ try } Context 'When the system is not in the desired state and ensure is set to Present' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = 'newServerRole' - } + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = 'newServerRole' + } - $result = Test-TargetResource @testParameters + $result = Test-TargetResource @testParameters + } It 'Should return false when desired server role does not exist' { $result | Should -BeFalse @@ -399,14 +415,16 @@ try } Context 'When the system is not in the desired state and ensure is set to Present' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - Members = @($mockSqlServerLoginThree, $mockSqlServerLoginFour) - } + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + Members = @($mockSqlServerLoginThree, $mockSqlServerLoginFour) + } - $result = Test-TargetResource @testParameters -verbose + $result = Test-TargetResource @testParameters + } It 'Should return false when desired members are not in desired server role' { $result | Should -BeFalse @@ -418,15 +436,17 @@ try } Context 'When both the parameters Members and MembersToInclude are assigned a value and ensure is set to Present' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - Members = $mockEnumMemberNames - MembersToInclude = $mockSqlServerLoginThree - } + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + Members = $mockEnumMemberNames + MembersToInclude = $mockSqlServerLoginThree + } - $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull + $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull + } It 'Should throw the correct error' { { Test-TargetResource @testParameters } | Should -Throw '(DRC0010)' @@ -438,14 +458,16 @@ try } Context 'When parameter MembersToInclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - MembersToInclude = $mockSqlServerLoginTwo - } + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + MembersToInclude = $mockSqlServerLoginTwo + } - $result = Test-TargetResource @testParameters + $result = Test-TargetResource @testParameters + } It 'Should return true when desired server role exist' { $result | Should -BeTrue @@ -457,14 +479,16 @@ try } Context 'When parameter MembersToInclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = 'RoleNotExist' # $mockSqlServerRole - MembersToInclude = $mockSqlServerLoginThree - } + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = 'RoleNotExist' # $mockSqlServerRole + MembersToInclude = $mockSqlServerLoginThree + } - $result = Test-TargetResource @testParameters + $result = Test-TargetResource @testParameters + } It 'Should return false when desired server role does not exist' { $result | Should -BeFalse @@ -476,15 +500,17 @@ try } Context 'When both the parameters Members and MembersToExclude are assigned a value and ensure is set to Present' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - Members = $mockEnumMemberNames - MembersToExclude = $mockSqlServerLoginTwo - } + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + Members = $mockEnumMemberNames + MembersToExclude = $mockSqlServerLoginTwo + } - $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull + $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull + } It 'Should throw the correct error' { {Test-TargetResource @testParameters} | Should -Throw '(DRC0010)' @@ -496,14 +522,16 @@ try } Context 'When parameter MembersToExclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - MembersToExclude = $mockSqlServerLoginThree + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + MembersToExclude = $mockSqlServerLoginThree + } + + $result = Test-TargetResource @testParameters } - $mockEnumMemberNames - $result = Test-TargetResource @testParameters It 'Should return true when desired server role does not exist' { $result | Should -BeTrue @@ -515,14 +543,16 @@ try } Context 'When parameter MembersToExclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Present' - ServerRoleName = $mockSqlServerRole - MembersToExclude = $mockSqlServerLoginTwo - } + BeforeAll { + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + MembersToExclude = $mockSqlServerLoginTwo + } - $result = Test-TargetResource @testParameters + $result = Test-TargetResource @testParameters + } It 'Should return false when desired server role exist' { $result | Should -BeFalse @@ -548,12 +578,14 @@ try } Context 'When the system is not in the desired state and ensure is set to Absent' { - $mockSqlServerRole = 'ServerRoleToDrop' - $mockExpectedServerRoleToDrop = 'ServerRoleToDrop' - $testParameters = $mockDefaultParameters - $testParameters += @{ - Ensure = 'Absent' - ServerRoleName = $mockSqlServerRole + BeforeAll { + $mockSqlServerRole = 'ServerRoleToDrop' + $mockExpectedServerRoleToDrop = 'ServerRoleToDrop' + $testParameters = $mockDefaultParameters + $testParameters += @{ + Ensure = 'Absent' + ServerRoleName = $mockSqlServerRole + } } It 'Should not throw when calling the drop method' { @@ -641,8 +673,6 @@ try MembersToInclude = $mockSqlServerLoginThree } - #$errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull - { Set-TargetResource @testParameters } | Should -Throw '(DRC0010)' } @@ -976,50 +1006,10 @@ try { Test-SqlSecurityPrincipal @testParameters } | Should -Throw -ExpectedMessage $testErrorMessage } - - - - - - <# - @person doing the code review: This test should fail. the only reason $result is False, is because it is not filled. - eg powershell evolves an empty variable to false. i vote to remove this test, but do not know the history of this test - and why it is in here. - #> - It 'Should return false when ErrorAction is set to SilentlyContinue' { - $testSecurityPrincipal = 'Nabrond' - - $testParameters = @{ - SqlServerObject = $testSqlServerObject - SecurityPrincipal = $testSecurityPrincipal - } - - <# - Pester will still see the error on the stack regardless of the value used for ErrorAction - Wrap this call in a try/catch to swallow the exception and capture the return value. - #> - try - { - $result = Test-SqlSecurityPrincipal @testParameters -ErrorAction SilentlyContinue - } - catch - { - continue; - } - - $result | Should -BeFalse - } - - - - - - } Context 'When the security principal exists.' { It 'Should return true when the principal is a Login.' { - $testParameters = @{ SqlServerObject = $testSqlServerObject SecurityPrincipal = $mockSqlServerLoginOne @@ -1059,12 +1049,14 @@ try Describe 'DSC_SqlRole\Get-CorrectedMemberParameters' -Tag 'Helper' { Context 'When parameter Members is assigned a value and the role is not sysadmin, the output should be the same' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRole - Members = $mockEnumMemberNames - } + BeforeAll { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + Members = $mockEnumMemberNames + } - $result = Get-CorrectedMemberParameters @testParameters + $result = Get-CorrectedMemberParameters @testParameters + } It 'Should return an array with 2 elements' { $result.Members | Should -HaveCount 2 @@ -1081,12 +1073,14 @@ try } Context 'When parameter Members is assigned a value and the role is sysadmin, if SA is in Members, the output should be the same' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRoleSysAdmin - Members = $mockEnumMemberNamesSysAdmin - } + BeforeAll { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + Members = $mockEnumMemberNamesSysAdmin + } - $result = Get-CorrectedMemberParameters @testParameters + $result = Get-CorrectedMemberParameters @testParameters + } It 'Should return an array with 3 elements' { $result.Members | Should -HaveCount 3 @@ -1103,12 +1097,14 @@ try } Context 'When parameter Members is assigned a value and the role is sysadmin, if SA is not in Members, SA should be added to the output' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRoleSysAdmin - Members = $mockEnumMemberNames - } + BeforeAll { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + Members = $mockEnumMemberNames + } - $result = Get-CorrectedMemberParameters @testParameters + $result = Get-CorrectedMemberParameters @testParameters + } It 'Should return an array with 3 elements' { $result.Members | Should -HaveCount 3 @@ -1125,12 +1121,14 @@ try } Context 'When parameter MembersToInclude is assigned a value and the role is not sysadmin, if SA is in MembersToInclude, the output should be the same' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRole - MembersToInclude = $mockEnumMemberNamesSysAdmin - } + BeforeAll { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + MembersToInclude = $mockEnumMemberNamesSysAdmin + } - $result = Get-CorrectedMemberParameters @testParameters + $result = Get-CorrectedMemberParameters @testParameters + } It 'Should return an array with 3 elements' { $result.MembersToInclude | Should -HaveCount 3 @@ -1147,12 +1145,14 @@ try } Context 'When parameter MembersToInclude is assigned a value and the role is not sysadmin, if SA is not in MembersToInclude, the output should be the same' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRole - MembersToInclude = $mockEnumMemberNames - } + BeforeAll { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + MembersToInclude = $mockEnumMemberNames + } - $result = Get-CorrectedMemberParameters @testParameters + $result = Get-CorrectedMemberParameters @testParameters + } It 'Should return an array with 2 elements' { $result.MembersToInclude | Should -HaveCount 2 @@ -1169,12 +1169,14 @@ try } Context 'When parameter MembersToInclude is assigned a value and the role is sysadmin, if SA is not in MembersToInclude, the output should be the same' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRoleSysAdmin - MembersToInclude = $mockEnumMemberNames - } + BeforeAll { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + MembersToInclude = $mockEnumMemberNames + } - $result = Get-CorrectedMemberParameters @testParameters + $result = Get-CorrectedMemberParameters @testParameters + } It 'Should return an array with 2 elements' { $result.MembersToInclude | Should -HaveCount 2 @@ -1191,12 +1193,14 @@ try } Context 'When parameter MembersToInclude is assigned a value and the role is sysadmin, if SA is in MembersToInclude, the output should be the same' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRoleSysAdmin - MembersToInclude = $mockEnumMemberNamesSysAdmin - } + BeforeAll { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + MembersToInclude = $mockEnumMemberNamesSysAdmin + } - $result = Get-CorrectedMemberParameters @testParameters + $result = Get-CorrectedMemberParameters @testParameters + } It 'Should return an array with 3 elements' { $result.MembersToInclude | Should -HaveCount 3 @@ -1213,12 +1217,14 @@ try } Context 'When parameter MembersToExclude is assigned a value and the role is not sysadmin, if SA is in MembersToExclude, the output should be the same' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRole - MembersToExclude = $mockEnumMemberNamesSysAdmin - } + BeforeAll { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + MembersToExclude = $mockEnumMemberNamesSysAdmin + } - $result = Get-CorrectedMemberParameters @testParameters + $result = Get-CorrectedMemberParameters @testParameters + } It 'Should return an array with 3 elements' { $result.MembersToExclude | Should -HaveCount 3 @@ -1235,12 +1241,14 @@ try } Context 'When parameter MembersToExclude is assigned a value and the role is not sysadmin, if SA is not in MembersToExclude, the output should be the same' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRole - MembersToExclude = $mockEnumMemberNames - } + BeforeAll { + $testParameters = @{ + ServerRoleName = $mockSqlServerRole + MembersToExclude = $mockEnumMemberNames + } - $result = Get-CorrectedMemberParameters @testParameters + $result = Get-CorrectedMemberParameters @testParameters + } It 'Should return an array with 2 elements' { $result.MembersToExclude | Should -HaveCount 2 @@ -1257,12 +1265,14 @@ try } Context 'When parameter MembersToExclude is assigned a value and the role is sysadmin, if SA is not in MembersToExclude, the output should be the same' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRoleSysAdmin - MembersToExclude = $mockEnumMemberNames - } + BeforeAll { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + MembersToExclude = $mockEnumMemberNames + } - $result = Get-CorrectedMemberParameters @testParameters + $result = Get-CorrectedMemberParameters @testParameters + } It 'Should return an array with 2 elements' { $result.MembersToExclude | Should -HaveCount 2 @@ -1279,12 +1289,14 @@ try } Context 'When parameter MembersToExclude is assigned a value and the role is sysadmin, if SA is in MembersToExclude, SA should be removed' { - $testParameters = @{ - ServerRoleName = $mockSqlServerRoleSysAdmin - MembersToExclude = $mockEnumMemberNamesSysAdmin - } + BeforeAll { + $testParameters = @{ + ServerRoleName = $mockSqlServerRoleSysAdmin + MembersToExclude = $mockEnumMemberNamesSysAdmin + } - $result = Get-CorrectedMemberParameters @testParameters + $result = Get-CorrectedMemberParameters @testParameters + } It 'Should return an array with 2 elements' { $result.MembersToExclude | Should -HaveCount 2 From 7735fed51700bcf60b257bc0a05bca288f8952a6 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Sun, 1 Nov 2020 13:29:42 +0100 Subject: [PATCH 32/36] -Be $null removed --- tests/Unit/DSC_SqlRole.Tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Unit/DSC_SqlRole.Tests.ps1 b/tests/Unit/DSC_SqlRole.Tests.ps1 index 911da3628..b84bdcb32 100644 --- a/tests/Unit/DSC_SqlRole.Tests.ps1 +++ b/tests/Unit/DSC_SqlRole.Tests.ps1 @@ -257,7 +257,7 @@ try } It 'Should return the members as null' { - $result.membersInRole | Should -Be $null + $result.membersInRole | Should -BeNullOrEmpty } It 'Should return the same values as passed as parameters' { @@ -286,7 +286,7 @@ try } It 'Should return the members as not null' { - $result.Members | Should -Not -Be $null + $result.Members | Should -Not -BeNullOrEmpty } # Regression test for issue #790 From 11f926ca2f18275f2e91301144beffb9b9e12316 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Mon, 2 Nov 2020 11:52:52 +0100 Subject: [PATCH 33/36] fixed integrationTests for exeption --- .../DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 7 ++ .../DSC_SqlRole.Integration.Tests.ps1 | 102 +++++++++++------- 2 files changed, 70 insertions(+), 39 deletions(-) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index 4c91bdd03..68a1c9a9c 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -19,6 +19,13 @@ $script:localizedData = Get-LocalizedData -DefaultUICulture 'en-US' .PARAMETER ServerRoleName The name of server role to be created or dropped. + .Notes + Because the return of this function will always be the actual members in the role, + and not the desired members in a role, there is no point in returning $MembersToInclude and exclude. + You now get "present" if the role exists, and when it exists, you get the current members in that role. + + The way it was: if a role does not exists, you still get $membersToInclude And $membersToExclude. + To me that seems like strange behavior. #> function Get-TargetResource { diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 77d1a9b03..69a4a16f6 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -37,25 +37,29 @@ try $configurationName = "$($script:dscResourceName)_AddRole1_Config" Context ('When using configuration {0}' -f $configurationName) { + BeforeAll { + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } + + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + } + It 'Should compile and apply the MOF without throwing' { { - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } - & $configurationName @configurationParameters + } | Should -Not -Throw - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } - + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw } @@ -95,8 +99,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -108,6 +110,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -147,8 +153,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -160,6 +164,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -203,8 +211,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -216,6 +222,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -259,8 +269,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -272,6 +280,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -315,8 +327,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -328,6 +338,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -367,8 +381,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -380,6 +392,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -412,16 +428,12 @@ try $configurationName = "$($script:dscResourceName)_AddNestedRole_Config" Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing an exception' { + BeforeAll { $configurationParameters = @{ OutputPath = $TestDrive ConfigurationData = $ConfigurationData } - { & $configurationName @configurationParameters } | Should -Not -Throw - } - - It 'Should apply the configuration successfully' { $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -430,8 +442,16 @@ try Force = $true ErrorAction = 'Stop' } + } + + It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw - { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw + { + Start-DscConfiguration @startDscConfigurationParameters + } | Should -Not -Throw } It 'Should be able to call Get-DscConfiguration without throwing an exception' { @@ -474,16 +494,12 @@ try $configurationName = "$($script:dscResourceName)_RemoveNestedRole_Config" Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing an exception' { + BeforeAll { $configurationParameters = @{ OutputPath = $TestDrive ConfigurationData = $ConfigurationData } - { & $configurationName @configurationParameters } | Should -Not -Throw - } - - It 'Should apply the configuration successfully' { $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -492,8 +508,16 @@ try Force = $true ErrorAction = 'Stop' } + } - { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw + It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + + { + Start-DscConfiguration @startDscConfigurationParameters + } | Should -Not -Throw } It 'Should be able to call Get-DscConfiguration without throwing an exception' { From 5d51aa51648da5a7180bccdcc904aa26f24160f7 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Mon, 2 Nov 2020 11:52:52 +0100 Subject: [PATCH 34/36] fixed integrationTests for exeption --- .../DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | 7 ++ .../DSC_SqlRole.Integration.Tests.ps1 | 102 +++++++++++------- 2 files changed, 70 insertions(+), 39 deletions(-) diff --git a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 index 4c91bdd03..68a1c9a9c 100644 --- a/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 +++ b/source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 @@ -19,6 +19,13 @@ $script:localizedData = Get-LocalizedData -DefaultUICulture 'en-US' .PARAMETER ServerRoleName The name of server role to be created or dropped. + .Notes + Because the return of this function will always be the actual members in the role, + and not the desired members in a role, there is no point in returning $MembersToInclude and exclude. + You now get "present" if the role exists, and when it exists, you get the current members in that role. + + The way it was: if a role does not exists, you still get $membersToInclude And $membersToExclude. + To me that seems like strange behavior. #> function Get-TargetResource { diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 77d1a9b03..69a4a16f6 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -37,25 +37,29 @@ try $configurationName = "$($script:dscResourceName)_AddRole1_Config" Context ('When using configuration {0}' -f $configurationName) { + BeforeAll { + $configurationParameters = @{ + OutputPath = $TestDrive + # The variable $ConfigurationData was dot-sourced above. + ConfigurationData = $ConfigurationData + } + + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + } + It 'Should compile and apply the MOF without throwing' { { - $configurationParameters = @{ - OutputPath = $TestDrive - # The variable $ConfigurationData was dot-sourced above. - ConfigurationData = $ConfigurationData - } - & $configurationName @configurationParameters + } | Should -Not -Throw - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } - + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw } @@ -95,8 +99,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -108,6 +110,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -147,8 +153,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -160,6 +164,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -203,8 +211,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -216,6 +222,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -259,8 +269,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -272,6 +280,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -315,8 +327,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -328,6 +338,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -367,8 +381,6 @@ try ConfigurationData = $ConfigurationData } - & $configurationName @configurationParameters - $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -380,6 +392,10 @@ try } It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw @@ -412,16 +428,12 @@ try $configurationName = "$($script:dscResourceName)_AddNestedRole_Config" Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing an exception' { + BeforeAll { $configurationParameters = @{ OutputPath = $TestDrive ConfigurationData = $ConfigurationData } - { & $configurationName @configurationParameters } | Should -Not -Throw - } - - It 'Should apply the configuration successfully' { $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -430,8 +442,16 @@ try Force = $true ErrorAction = 'Stop' } + } + + It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw - { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw + { + Start-DscConfiguration @startDscConfigurationParameters + } | Should -Not -Throw } It 'Should be able to call Get-DscConfiguration without throwing an exception' { @@ -474,16 +494,12 @@ try $configurationName = "$($script:dscResourceName)_RemoveNestedRole_Config" Context ('When using configuration {0}' -f $configurationName) { - It 'Should compile and apply the MOF without throwing an exception' { + BeforeAll { $configurationParameters = @{ OutputPath = $TestDrive ConfigurationData = $ConfigurationData } - { & $configurationName @configurationParameters } | Should -Not -Throw - } - - It 'Should apply the configuration successfully' { $startDscConfigurationParameters = @{ Path = $TestDrive ComputerName = 'localhost' @@ -492,8 +508,16 @@ try Force = $true ErrorAction = 'Stop' } + } - { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw + It 'Should compile and apply the MOF without throwing' { + { + & $configurationName @configurationParameters + } | Should -Not -Throw + + { + Start-DscConfiguration @startDscConfigurationParameters + } | Should -Not -Throw } It 'Should be able to call Get-DscConfiguration without throwing an exception' { From 56db008fd952606fabe509c6e8ac8212f8c3e3ed Mon Sep 17 00:00:00 2001 From: Jess Pomfret Date: Thu, 29 Oct 2020 15:10:27 +0000 Subject: [PATCH 35/36] SqlDatabaseRole: Add a test to check error message on helper function (#1626) - SqlDatabaseRole - Added test to ensure Add-SqlDscDatabaseRoleMember throws the expected error (issue #1620). --- CHANGELOG.md | 3 +++ tests/Unit/DSC_SqlDatabaseRole.Tests.ps1 | 26 +++++++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc02a9746..c22853999 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - SqlEndpoint - Added support for the Service Broker Endpoint ([issue #498](https://github.com/dsccommunity/SqlServerDsc/issues/498)). +- SqlDatabaseRole + - Added test to ensure Add-SqlDscDatabaseRoleMember throws the expected error + ([issue #1620](https://github.com/dsccommunity/SqlServerDsc/issues/1620)). ### Changed diff --git a/tests/Unit/DSC_SqlDatabaseRole.Tests.ps1 b/tests/Unit/DSC_SqlDatabaseRole.Tests.ps1 index 87db1a9d4..5d4995de8 100644 --- a/tests/Unit/DSC_SqlDatabaseRole.Tests.ps1 +++ b/tests/Unit/DSC_SqlDatabaseRole.Tests.ps1 @@ -228,7 +228,7 @@ try } -PassThru )) } - }-PassThru -Force + } -PassThru -Force )) } } -PassThru -Force | @@ -1108,6 +1108,30 @@ try Assert-VerifiableMock } + + Describe 'Add-SqlDscDatabaseRoleMember' -Tag 'Helper' { + BeforeAll { + $mockSqlDatabaseObject = @{ + Name = $mockSqlDatabaseName + Roles = @{ + $mockSqlDatabaseRole1 = 'Role' + } + Users = @{ + $mockSqlServerLogin1 = 'User' + } + } + $mockName = 'MissingRole' + $mockMemberName = 'MissingUser' + + } + Context 'When calling with a role that does not exist' { + It 'Should throw the correct error' { + { + Add-SqlDscDatabaseRoleMember -SqlDatabaseObject $mockSqlDatabaseObject -Name $mockName -MemberName $mockMemberName + } | Should -Throw ($script:localizedData.DatabaseRoleOrUserNotFound -f $mockName, $mockMemberName, $mockSqlDatabaseName) + } + } + } } } finally From 7888735e066c6febc14729904c7dbe2aa6fa2c68 Mon Sep 17 00:00:00 2001 From: Paul Woudwijk Date: Wed, 4 Nov 2020 18:49:13 +0100 Subject: [PATCH 36/36] IntegrationTest updates --- .../DSC_SqlRole.Integration.Tests.ps1 | 124 ++++++++---------- 1 file changed, 57 insertions(+), 67 deletions(-) diff --git a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 index 69a4a16f6..570b8af5f 100644 --- a/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 +++ b/tests/Integration/DSC_SqlRole.Integration.Tests.ps1 @@ -32,10 +32,9 @@ try Describe "$($script:dscResourceName)_Integration" { BeforeAll { $resourceId = "[$($script:dscResourceFriendlyName)]Integration_Test" + $configurationName = "$($script:dscResourceName)_AddRole1_Config" } - $configurationName = "$($script:dscResourceName)_AddRole1_Config" - Context ('When using configuration {0}' -f $configurationName) { BeforeAll { $configurationParameters = @{ @@ -85,7 +84,7 @@ try } It 'Should return $true when Test-DscConfiguration is run' { - Test-DscConfiguration -Verbose | Should -Be 'True' + Test-DscConfiguration -Verbose | Should -BeTrue } } @@ -97,15 +96,15 @@ try OutputPath = $TestDrive # The variable $ConfigurationData was dot-sourced above. ConfigurationData = $ConfigurationData - } - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } } } @@ -139,7 +138,7 @@ try } It 'Should return $true when Test-DscConfiguration is run' { - Test-DscConfiguration -Verbose | Should -Be 'True' + Test-DscConfiguration -Verbose | Should -BeTrue } } @@ -151,15 +150,15 @@ try OutputPath = $TestDrive # The variable $ConfigurationData was dot-sourced above. ConfigurationData = $ConfigurationData - } - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } } } @@ -197,7 +196,7 @@ try } It 'Should return $true when Test-DscConfiguration is run' { - Test-DscConfiguration -Verbose | Should -Be 'True' + Test-DscConfiguration -Verbose | Should -BeTrue } } @@ -209,15 +208,15 @@ try OutputPath = $TestDrive # The variable $ConfigurationData was dot-sourced above. ConfigurationData = $ConfigurationData - } - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } } } @@ -255,7 +254,7 @@ try } It 'Should return $true when Test-DscConfiguration is run' { - Test-DscConfiguration -Verbose | Should -Be 'True' + Test-DscConfiguration -Verbose | Should -BeTrue } } @@ -263,19 +262,19 @@ try Context ('When using configuration {0}' -f $configurationName) { BeforeAll { - $configurationParameters = @{ + $configurationParameters = @{ OutputPath = $TestDrive # The variable $ConfigurationData was dot-sourced above. ConfigurationData = $ConfigurationData - } - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } } } @@ -313,7 +312,7 @@ try } It 'Should return $true when Test-DscConfiguration is run' { - Test-DscConfiguration -Verbose | Should -Be 'True' + Test-DscConfiguration -Verbose | Should -BeTrue } } @@ -326,15 +325,6 @@ try # The variable $ConfigurationData was dot-sourced above. ConfigurationData = $ConfigurationData } - - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' - } } It 'Should compile and apply the MOF without throwing' { @@ -367,7 +357,7 @@ try } It 'Should return $true when Test-DscConfiguration is run' { - Test-DscConfiguration -Verbose | Should -Be 'True' + Test-DscConfiguration -Verbose | Should -BeTrue } } @@ -379,15 +369,15 @@ try OutputPath = $TestDrive # The variable $ConfigurationData was dot-sourced above. ConfigurationData = $ConfigurationData - } - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } } } @@ -421,7 +411,7 @@ try } It 'Should return $true when Test-DscConfiguration is run' { - Test-DscConfiguration -Verbose | Should -Be 'True' + Test-DscConfiguration -Verbose | Should -BeTrue } } @@ -432,15 +422,15 @@ try $configurationParameters = @{ OutputPath = $TestDrive ConfigurationData = $ConfigurationData - } - $startDscConfigurationParameters = @{ - Path = $TestDrive - ComputerName = 'localhost' - Wait = $true - Verbose = $true - Force = $true - ErrorAction = 'Stop' + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } } }