diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a2c5a2a4c3..31582e7906 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -10,6 +10,9 @@ PowerShell commands that should be public should always have its separate script file and the command name as the file name with the .ps1 extension, these files shall always be placed in the folder source/Public. +All public command names must have the noun prefixed with 'SqlDsc', e.g. +{Verb}-SqlDsc{Noun}. + Public commands may use private functions to move out logic that can be reused by other public commands, so move out any logic that can be deemed reusable. @@ -196,6 +199,10 @@ case (`It`-block) as possible. Never test, mock or use `Should -Invoke` for `Write-Verbose` and `Write-Debug` regardless of other instructions. +Never use `Should -Not -Throw` to prepare for Pester v6 where it has been +removed. By default the `It` block will handle any unexpected exception. +Instead of `{ Command } | Should -Not -Throw`, use `Command` directly. + Unit tests should be added for all public commands, private functions and class-based resources. The unit tests for class-based resources should be placed in the folder tests/Unit/Classes. The unit tests for public command @@ -206,6 +213,83 @@ they are testing, but should have the suffix .Tests.ps1. The unit tests should be written to cover all possible scenarios and code paths, ensuring that both edge cases and common use cases are tested. +All public commands should always have a test to validate parameter sets +using this template. For commands with a single parameter set: + +```powershell +It 'Should have the correct parameters in parameter set ' -ForEach @( + @{ + MockParameterSetName = '__AllParameterSets' + MockExpectedParameters = '[-Parameter1] [-Parameter2] []' + } +) { + $result = (Get-Command -Name 'CommandName').ParameterSets | + Where-Object -FilterScript { + $_.Name -eq $mockParameterSetName + } | + Select-Object -Property @( + @{ + Name = 'ParameterSetName' + Expression = { $_.Name } + }, + @{ + Name = 'ParameterListAsString' + Expression = { $_.ToString() } + } + ) + + $result.ParameterSetName | Should -Be $MockParameterSetName + $result.ParameterListAsString | Should -Be $MockExpectedParameters +} +``` + +For commands with multiple parameter sets, use this pattern: + +```powershell +It 'Should have the correct parameters in parameter set ' -ForEach @( + @{ + MockParameterSetName = 'ParameterSet1' + MockExpectedParameters = '-ServerObject -Name -Parameter1 []' + } + @{ + MockParameterSetName = 'ParameterSet2' + MockExpectedParameters = '-ServerObject -Name -Parameter2 []' + } +) { + $result = (Get-Command -Name 'CommandName').ParameterSets | + Where-Object -FilterScript { + $_.Name -eq $mockParameterSetName + } | + Select-Object -Property @( + @{ + Name = 'ParameterSetName' + Expression = { $_.Name } + }, + @{ + Name = 'ParameterListAsString' + Expression = { $_.ToString() } + } + ) + + $result.ParameterSetName | Should -Be $MockParameterSetName + $result.ParameterListAsString | Should -Be $MockExpectedParameters +} +``` + +All public commands should also include tests to validate parameter properties: + +```powershell +It 'Should have ParameterName as a mandatory parameter' { + $parameterInfo = (Get-Command -Name 'CommandName').Parameters['ParameterName'] + $parameterInfo.Attributes.Mandatory | Should -Contain $true +} + +It 'Should accept ParameterName from pipeline' { + $parameterInfo = (Get-Command -Name 'CommandName').Parameters['ParameterName'] + $parameterInfo.Attributes.ValueFromPipeline | Should -Contain $true +} +``` + The `BeforeAll` block should be used to set up any necessary test data or mocking Use localized strings in the tests only when necessary. You can assign the @@ -302,6 +386,19 @@ edge cases and common use cases are tested. The integration tests should also be written to test the command in a real environment, using real resources and dependencies. +Integration test script files for public commands must be added to a group +within the 'Integration_Test_Commands_SqlServer' stage in ./azure-pipelines.yml. +Choose the appropriate group number based on the dependencies of the command +being tested (e.g., commands that require Database Engine should be in Group 2 +or later, after the Database Engine installation tests). + +When integration tests need the computer name in CI environments, always use +the Get-ComputerName command, which is available in the build pipeline. + +For integration testing commands use the information in the +tests/Integration/Commands/README.md, which describes the testing environment +including available instances, users, credentials, and other configuration details. + All integration tests must use the below code block prior to the first `Describe`-block. The following code will set up the integration test environment and it will make sure the module being tested is available @@ -343,6 +440,24 @@ The module DscResource.Test is used by the pipeline and its commands are normally not used when testing public functions, private functions or class-based resources. +## SQL Server + +### SQL Server Management Objects (SMO) + +When developing commands, private functions, class-based resources, or making +modifications to existing functionality, always prefer using SQL Server +Management Objects (SMO) as the primary method for interacting with SQL Server. +Only use T-SQL when it is not possible to achieve the desired functionality +with SMO. + +## Change log + +The Unreleased section in CHANGELOG.md should always be updated when making +changes to the codebase. Use the keepachangelog format and provide concrete +release notes that describe the main changes made. This includes new commands, +private functions, class-based resources, or significant modifications to +existing functionality. + ## Style guidelines This project use the style guidelines from the DSC Community: https://dsccommunity.org/styleguidelines @@ -356,6 +471,7 @@ This project use the style guidelines from the DSC Community: https://dsccommuni ### PowerShell files - All files should use UTF8 without BOM. +- All files must end with a new line. ### PowerShell code diff --git a/.vscode/settings.json b/.vscode/settings.json index 4b3e241da3..707029e21f 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -77,7 +77,8 @@ "analyzersettings", "sqlcmd", "PBIRS", - "SSRS" + "SSRS", + "DSCSQLTEST" ], "cSpell.ignorePaths": [ ".git" diff --git a/CHANGELOG.md b/CHANGELOG.md index 4312dd191e..e971bd874b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,12 +10,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added setup workflow for GitHub Copilot. - Switch the workflow to use Linux. - Attempt to unshallow the Copilot branch +- `Assert-SqlDscLogin` + - Added new public command to validate that a specified SQL Server principal + exists as a login, throwing a terminating error if it doesn't exist. + - Supports pipeline input and provides detailed error messages with localization. + - Uses `Test-SqlDscIsLogin` command for login validation following module patterns. ### Changed - `azure-pipelines.yml` - Remove `windows-2019` images fixes [#2106](https://github.com/dsccommunity/SqlServerDsc/issues/2106). - Move individual tasks to `windows-latest`. + - Added integration tests for `Assert-SqlDscLogin` command in Group 2. ## [17.1.0] - 2025-05-22 diff --git a/azure-pipelines.yml b/azure-pipelines.yml index aef4e60715..65f4864543 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -247,6 +247,8 @@ stages: # Group 1 'tests/Integration/Commands/Install-SqlDscServer.Integration.Tests.ps1' 'tests/Integration/Commands/Connect-SqlDscDatabaseEngine.Integration.Tests.ps1' + # Group 2 + 'tests/Integration/Commands/Assert-SqlDscLogin.Integration.Tests.ps1' # Group 9 'tests/Integration/Commands/Uninstall-SqlDscServer.Integration.Tests.ps1' ) diff --git a/source/Public/Assert-SqlDscLogin.ps1 b/source/Public/Assert-SqlDscLogin.ps1 new file mode 100644 index 0000000000..5eb53794fd --- /dev/null +++ b/source/Public/Assert-SqlDscLogin.ps1 @@ -0,0 +1,67 @@ +<# + .SYNOPSIS + Assert that the specified SQL Server principal exists as a login. + + .DESCRIPTION + This command asserts that the specified SQL Server principal exists as a + login. If the principal does not exist as a login, a terminating error + is thrown. + + .PARAMETER ServerObject + Specifies current server connection object. + + .PARAMETER Name + Specifies the name of the principal that needs to exist as a login. + + .EXAMPLE + $serverObject = Connect-SqlDscDatabaseEngine -InstanceName 'MyInstance' + $serverObject | Assert-SqlDscLogin -Name 'MyLogin' + + Asserts that the principal 'MyLogin' exists as a login. + + .EXAMPLE + $serverObject = Connect-SqlDscDatabaseEngine -InstanceName 'MyInstance' + Assert-SqlDscLogin -ServerObject $serverObject -Name 'MyLogin' + + Asserts that the principal 'MyLogin' exists as a login. + + .NOTES + This command throws a terminating error if the specified SQL Server + principal does not exist as a SQL server login. +#> +function Assert-SqlDscLogin +{ + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('UseSyntacticallyCorrectExamples', '', Justification = 'Because the rule does not yet support parsing the code when a parameter type is not available. The ScriptAnalyzer rule UseSyntacticallyCorrectExamples will always error in the editor due to https://github.com/indented-automation/Indented.ScriptAnalyzerRules/issues/8.')] + [CmdletBinding()] + param + ( + [Parameter(Mandatory = $true, ValueFromPipeline = $true)] + [Microsoft.SqlServer.Management.Smo.Server] + $ServerObject, + + [Parameter(Mandatory = $true)] + [System.String] + $Name + ) + + process + { + Write-Verbose -Message ($script:localizedData.Assert_Login_CheckingLogin -f $Name, $ServerObject.InstanceName) + + if (-not (Test-SqlDscIsLogin -ServerObject $ServerObject -Name $Name)) + { + $missingLoginMessage = $script:localizedData.Assert_Login_LoginMissing -f $Name, $ServerObject.InstanceName + + $PSCmdlet.ThrowTerminatingError( + [System.Management.Automation.ErrorRecord]::new( + $missingLoginMessage, + 'ASDL0001', # cspell: disable-line + [System.Management.Automation.ErrorCategory]::ObjectNotFound, + $Name + ) + ) + } + + Write-Debug -Message ($script:localizedData.Assert_Login_LoginExists -f $Name) + } +} diff --git a/source/en-US/SqlServerDsc.strings.psd1 b/source/en-US/SqlServerDsc.strings.psd1 index 6e0839c7e7..cce7d90aad 100644 --- a/source/en-US/SqlServerDsc.strings.psd1 +++ b/source/en-US/SqlServerDsc.strings.psd1 @@ -222,4 +222,9 @@ ConvertFrom-StringData @' ## ConvertTo-SqlDscEditionName ConvertTo_EditionName_ConvertingEditionId = Converting EditionId '{0}' to Edition name. ConvertTo_EditionName_UnknownEditionId = The EditionId '{0}' is unknown and could not be converted. + + ## Assert-SqlDscLogin + Assert_Login_CheckingLogin = Checking if the principal '{0}' exists as a login on the instance '{1}'. + Assert_Login_LoginMissing = The principal '{0}' does not exist as a login on the instance '{1}'. + Assert_Login_LoginExists = The principal '{0}' exists as a login. '@ diff --git a/tests/Integration/Commands/Assert-SqlDscLogin.Integration.Tests.ps1 b/tests/Integration/Commands/Assert-SqlDscLogin.Integration.Tests.ps1 new file mode 100644 index 0000000000..e5da6a23c2 --- /dev/null +++ b/tests/Integration/Commands/Assert-SqlDscLogin.Integration.Tests.ps1 @@ -0,0 +1,91 @@ +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] +param () + +BeforeDiscovery { + try + { + if (-not (Get-Module -Name 'DscResource.Test')) + { + # Assumes dependencies has been resolved, so if this module is not available, run 'noop' task. + if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) + { + # Redirect all streams to $null, except the error stream (stream 2) + & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null + } + + # If the dependencies has not been resolved, this will throw an error. + Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' + } + } + catch [System.IO.FileNotFoundException] + { + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + } +} + +BeforeAll { + $script:dscModuleName = 'SqlServerDsc' + + Import-Module -Name $script:dscModuleName +} + +Describe 'Assert-SqlDscLogin' -Tag @('Integration_SQL2016', 'Integration_SQL2017', 'Integration_SQL2019', 'Integration_SQL2022') { + BeforeAll { + # Starting the named instance SQL Server service prior to running tests. + Start-Service -Name 'MSSQL$DSCSQLTEST' -Verbose -ErrorAction 'Stop' + + $script:instanceName = 'DSCSQLTEST' + $script:computerName = Get-ComputerName + } + + AfterAll { + # Stop the named instance SQL Server service to save memory on the build worker. + Stop-Service -Name 'MSSQL$DSCSQLTEST' -Verbose -ErrorAction 'Stop' + } + + Context 'When connecting to SQL Server instance' { + BeforeAll { + $sqlAdministratorUserName = 'SqlAdmin' # Using computer name as NetBIOS name throw exception. + $sqlAdministratorPassword = ConvertTo-SecureString -String 'P@ssw0rd1' -AsPlainText -Force + + $script:sqlAdminCredential = [System.Management.Automation.PSCredential]::new($sqlAdministratorUserName, $sqlAdministratorPassword) + + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:instanceName -Credential $script:sqlAdminCredential + } + + Context 'When a login exists' { + It 'Should not throw an error for sa login' { + { Assert-SqlDscLogin -ServerObject $script:serverObject -Name 'sa' } | Should -Not -Throw + } + + It 'Should not throw an error when using pipeline' { + { $script:serverObject | Assert-SqlDscLogin -Name 'sa' } | Should -Not -Throw + } + + It 'Should not throw an error for NT AUTHORITY\SYSTEM login' { + { Assert-SqlDscLogin -ServerObject $script:serverObject -Name 'NT AUTHORITY\SYSTEM' } | Should -Not -Throw + } + + It 'Should not throw an error for SqlAdmin login' { + { Assert-SqlDscLogin -ServerObject $script:serverObject -Name ('{0}\SqlAdmin' -f $script:computerName) } | Should -Not -Throw + } + } + + Context 'When a login does not exist' { + It 'Should throw a terminating error for non-existent login' { + { Assert-SqlDscLogin -ServerObject $script:serverObject -Name 'NonExistentLogin123' } | Should -Throw -ExpectedMessage "*does not exist as a login*" + } + + It 'Should throw an error with ObjectNotFound category' { + try + { + Assert-SqlDscLogin -ServerObject $script:serverObject -Name 'NonExistentLogin123' + } + catch + { + $_.CategoryInfo.Category | Should -Be 'ObjectNotFound' + } + } + } + } +} diff --git a/tests/Integration/Commands/Uninstall-SqlDscServer.Integration.Tests.ps1 b/tests/Integration/Commands/Uninstall-SqlDscServer.Integration.Tests.ps1 index 0fc6ab3d94..2e211856eb 100644 --- a/tests/Integration/Commands/Uninstall-SqlDscServer.Integration.Tests.ps1 +++ b/tests/Integration/Commands/Uninstall-SqlDscServer.Integration.Tests.ps1 @@ -24,7 +24,7 @@ BeforeDiscovery { } # cSpell: ignore DSCSQLTEST -Describe 'Install-SqlDscServer' -Tag @('Integration_SQL2016', 'Integration_SQL2017', 'Integration_SQL2019', 'Integration_SQL2022') { +Describe 'Uninstall-SqlDscServer' -Tag @('Integration_SQL2016', 'Integration_SQL2017', 'Integration_SQL2019', 'Integration_SQL2022') { BeforeAll { Write-Verbose -Message ('Running integration test as user ''{0}''.' -f $env:UserName) -Verbose diff --git a/tests/Unit/Public/Assert-SqlDscLogin.Tests.ps1 b/tests/Unit/Public/Assert-SqlDscLogin.Tests.ps1 new file mode 100644 index 0000000000..91198303ef --- /dev/null +++ b/tests/Unit/Public/Assert-SqlDscLogin.Tests.ps1 @@ -0,0 +1,166 @@ +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')] +param () + +BeforeDiscovery { + try + { + if (-not (Get-Module -Name 'DscResource.Test')) + { + # Assumes dependencies has been resolved, so if this module is not available, run 'noop' task. + if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) + { + # Redirect all streams to $null, except the error stream (stream 2) + & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null + } + + # If the dependencies has not been resolved, this will throw an error. + Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' + } + } + catch [System.IO.FileNotFoundException] + { + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + } +} + +BeforeAll { + $script:dscModuleName = 'SqlServerDsc' + + $env:SqlServerDscCI = $true + + Import-Module -Name $script:dscModuleName + + # Loading mocked classes + Add-Type -Path (Join-Path -Path (Join-Path -Path $PSScriptRoot -ChildPath '../Stubs') -ChildPath 'SMO.cs') + + $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:dscModuleName + $PSDefaultParameterValues['Mock:ModuleName'] = $script:dscModuleName + $PSDefaultParameterValues['Should:ModuleName'] = $script:dscModuleName +} + +AfterAll { + $PSDefaultParameterValues.Remove('InModuleScope:ModuleName') + $PSDefaultParameterValues.Remove('Mock:ModuleName') + $PSDefaultParameterValues.Remove('Should:ModuleName') + + # Unload the module being tested so that it doesn't impact any other tests. + Get-Module -Name $script:dscModuleName -All | Remove-Module -Force + + Remove-Item -Path 'env:SqlServerDscCI' +} + +Describe 'Assert-SqlDscLogin' -Tag 'Public' { + Context 'When a login exists' { + BeforeAll { + $mockServerObject = New-Object -TypeName 'Microsoft.SqlServer.Management.Smo.Server' + $mockServerObject | Add-Member -MemberType 'NoteProperty' -Name 'InstanceName' -Value 'TestInstance' -Force + + Mock -CommandName 'Test-SqlDscIsLogin' -MockWith { return $true } + } + + It 'Should not throw an error when the login exists' { + Assert-SqlDscLogin -ServerObject $mockServerObject -Name 'TestLogin' + } + + It 'Should call Test-SqlDscIsLogin with correct parameters' { + Assert-SqlDscLogin -ServerObject $mockServerObject -Name 'TestLogin' + + Should -Invoke -CommandName 'Test-SqlDscIsLogin' -ParameterFilter { + $ServerObject.InstanceName -eq 'TestInstance' -and + $Name -eq 'TestLogin' + } -Exactly -Times 1 + } + + It 'Should accept ServerObject from pipeline' { + $mockServerObject | Assert-SqlDscLogin -Name 'TestLogin' + } + } + + Context 'When a login does not exist' { + BeforeAll { + $mockServerObject = New-Object -TypeName 'Microsoft.SqlServer.Management.Smo.Server' + $mockServerObject | Add-Member -MemberType 'NoteProperty' -Name 'InstanceName' -Value 'TestInstance' -Force + + Mock -CommandName 'Test-SqlDscIsLogin' -MockWith { return $false } + } + + It 'Should throw a terminating error when the login does not exist' { + { Assert-SqlDscLogin -ServerObject $mockServerObject -Name 'NonExistentLogin' } | Should -Throw -ExpectedMessage "*does not exist as a login*" + } + + It 'Should throw an error with the correct error category' { + try + { + Assert-SqlDscLogin -ServerObject $mockServerObject -Name 'NonExistentLogin' + } + catch + { + $_.CategoryInfo.Category | Should -Be 'ObjectNotFound' + $_.FullyQualifiedErrorId | Should -Be 'ASDL0001,Assert-SqlDscLogin' + } + } + + It 'Should include the principal name in the error message' { + { Assert-SqlDscLogin -ServerObject $mockServerObject -Name 'NonExistentLogin' } | Should -Throw -ExpectedMessage "*NonExistentLogin*" + } + + It 'Should include the instance name in the error message' { + { Assert-SqlDscLogin -ServerObject $mockServerObject -Name 'NonExistentLogin' } | Should -Throw -ExpectedMessage "*TestInstance*" + } + + It 'Should call Test-SqlDscIsLogin with correct parameters' { + { Assert-SqlDscLogin -ServerObject $mockServerObject -Name 'NonExistentLogin' } | Should -Throw + + Should -Invoke -CommandName 'Test-SqlDscIsLogin' -ParameterFilter { + $ServerObject.InstanceName -eq 'TestInstance' -and + $Name -eq 'NonExistentLogin' + } -Exactly -Times 1 + } + } + + Context 'When validating parameters' { + BeforeAll { + $mockServerObject = New-Object -TypeName 'Microsoft.SqlServer.Management.Smo.Server' + } + + It 'Should have ServerObject as a mandatory parameter' { + $parameterInfo = (Get-Command -Name 'Assert-SqlDscLogin').Parameters['ServerObject'] + $parameterInfo.Attributes.Mandatory | Should -Contain $true + } + + It 'Should have Name as a mandatory parameter' { + $parameterInfo = (Get-Command -Name 'Assert-SqlDscLogin').Parameters['Name'] + $parameterInfo.Attributes.Mandatory | Should -Contain $true + } + + It 'Should accept ServerObject from pipeline' { + $parameterInfo = (Get-Command -Name 'Assert-SqlDscLogin').Parameters['ServerObject'] + $parameterInfo.Attributes.ValueFromPipeline | Should -Contain $true + } + + It 'Should have the correct parameters in parameter set ' -ForEach @( + @{ + MockParameterSetName = '__AllParameterSets' + MockExpectedParameters = '[-ServerObject] [-Name] []' + } + ) { + $result = (Get-Command -Name 'Assert-SqlDscLogin').ParameterSets | + Where-Object -FilterScript { + $_.Name -eq $mockParameterSetName + } | + Select-Object -Property @( + @{ + Name = 'ParameterSetName' + Expression = { $_.Name } + }, + @{ + Name = 'ParameterListAsString' + Expression = { $_.ToString() } + } + ) + + $result.ParameterSetName | Should -Be $MockParameterSetName + $result.ParameterListAsString | Should -Be $MockExpectedParameters + } + } +}