SqlPermission: Added integration tests for server role permissions#2362
SqlPermission: Added integration tests for server role permissions#2362johlju merged 5 commits intodsccommunity:mainfrom
Conversation
WalkthroughAdds server-role support to the SqlPermission DSC resource by changing principal-resolution logic to prefer logins, introducing role-targeted test configurations, and expanding integration tests covering grant/deny/remove and include/exclude permission scenarios for role principals. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
source/Classes/020.SqlPermission.ps1 (1)
2-8: Consider updating synopsis/description to mention server roles as principalsThe header comments currently describe managing “server permissions for a login”, but the implementation (and tests in this PR) now also handle server roles as principals. Consider adjusting the wording (e.g., “for a login or server role”) so the documentation matches actual behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)source/Classes/020.SqlPermission.ps1(1 hunks)tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1(1 hunks)tests/Integration/Resources/DSC_SqlPermission.config.ps1(2 hunks)
🧰 Additional context used
📓 Path-based instructions (17)
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
**/*.ps1: Format public commands as{Verb}-SqlDsc{Noun}following PowerShell naming conventions
Format private functions as{Verb}-{Noun}following PowerShell naming conventions
Files:
source/Classes/020.SqlPermission.ps1tests/Integration/Resources/DSC_SqlPermission.config.ps1tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
source/[cC]lasses/020.*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-class-resource.instructions.md)
DSC class-based resources must use the file naming pattern
source/Classes/020.{ResourceName}.ps1
Files:
source/Classes/020.SqlPermission.ps1
source/[cC]lasses/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-class-resource.instructions.md)
source/[cC]lasses/**/*.ps1: DSC class-based resources must be decorated with[DscResource(RunAsCredential = 'Optional')]or[DscResource(RunAsCredential = 'Mandatory')]based on requirements
DSC class-based resources must inherit fromResourceBaseclass from the DscResource.Base module
DSC class-based resources must use[Nullable[{FullTypeName}]]syntax for value-type properties (e.g.,[Nullable[System.Int32]])
DSC class-based resources must implement a required constructor with signatureResourceName () : base ($PSScriptRoot)and populate$this.ExcludeDscPropertiesarray
DSC class-based resources must implement required methods:Get(),Test(),Set(),GetCurrentState(), andModify()following the specified pattern
DSC class-based resources may optionally implement methods:AssertProperties()andNormalizeProperties()following the specified pattern
DSC class-based resources must include comment-based help with .DESCRIPTION section containing Requirements and Known issues sections with a link to all open issues on GitHub
DSC class-based resources must usetry/catchblocks for exception handling in classes
DSC class-based resources must useNew-*Exceptioncommands (such asNew-InvalidDataException,New-ArgumentException,New-InvalidOperationException,New-ObjectNotFoundException,New-InvalidResultException,New-NotImplementedException) instead ofthrowfor terminating errors in classes
Files:
source/Classes/020.SqlPermission.ps1
⚙️ CodeRabbit configuration file
source/[cC]lasses/**/*.ps1: # DSC Class-Based Resource GuidelinesApplies to: Classes with
[DscResource(...)]decoration only.Requirements
- File:
source/Classes/020.{ResourceName}.ps1- Decoration:
[DscResource(RunAsCredential = 'Optional')](replace with'Mandatory'if required)- Inheritance: Must inherit
ResourceBase(part of module DscResource.Base)$this.localizedDatahashtable auto-populated byResourceBasefrom localization file- value-type properties: Use
[Nullable[{FullTypeName}]](e.g.,[Nullable[System.Int32]])Required constructor
MyResourceName () : base ($PSScriptRoot) { # Property names where state cannot be enforced, e.g. IsSingleInstance, Force $this.ExcludeDscProperties = @() }Required Method Pattern
[MyResourceName] Get() { # Call base implementation to get current state $currentState = ([ResourceBase] $this).Get() # If needed, post-processing on current state that can not be handled by GetCurrentState() return $currentState } [System.Boolean] Test() { # Call base implementation to test current state $inDesiredState = ([ResourceBase] $this).Test() # If needed, post-processing on test result that can not be handled by base Test() return $inDesiredState } [void] Set() { # Call base implementation to set desired state ([ResourceBase] $this).Set() # If needed, additional state changes that can not be handled by Modify() } hidden [System.Collections.Hashtable] GetCurrentState([System.Collections.Hashtable] $properties) { # Always return current state as hashtable, $properties contains key properties } hidden [void] Modify([System.Collections.Hashtable] $properties) { # Always set desired state, $properties contain those that must change state }Optional Method Pattern
hidden [void] AssertProperties([System.Collections.Hashtable] $properties) { # Validate user-provided properties, $p...
Files:
source/Classes/020.SqlPermission.ps1
source/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-localization.instructions.md)
source/**/*.ps1: Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages in PowerShell scripts
Use localized string keys from $script:localizedData, not hardcoded strings, in diagnostic messages
Reference localized strings using the syntax: Write-Verbose -Message ($script:localizedData.KeyName -f $value1)Localize all strings using string keys; remove any orphaned string keys
Files:
source/Classes/020.SqlPermission.ps1
⚙️ CodeRabbit configuration file
source/**/*.ps1: # Localization GuidelinesRequirements
- Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages
- Use localized string keys, not hardcoded strings
- Assume
$script:localizedDatais availableString Files
- Commands/functions:
source/en-US/{MyModuleName}.strings.psd1- Class resources:
source/en-US/{ResourceClassName}.strings.psd1Key Naming Patterns
- Format:
Verb_FunctionName_Action(underscore separators), e.g.Get_Database_ConnectingToDatabaseString Format
ConvertFrom-StringData @' KeyName = Message with {0} placeholder. (PREFIX0001) '@String IDs
- Format:
(PREFIX####)- PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
- Number: Sequential from 0001
Usage
Write-Verbose -Message ($script:localizedData.KeyName -f $value1)
Files:
source/Classes/020.SqlPermission.ps1
source/Classes/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
source/Classes/*.ps1: Classes should be located insource/Classes/{DependencyGroupNumber}.{ClassName}.ps1
DSC resources should always be created as class-based resources
Files:
source/Classes/020.SqlPermission.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Use descriptive names with 3+ characters and no abbreviations for functions, parameters, variables, keywords, and classes
Use PascalCase with Verb-Noun format for function names, using approved PowerShell verbs
Use PascalCase for parameter names
Use camelCase for variable names
Use lower-case for keywords
Use PascalCase for class names
Include scope for script/global/environment variables using$script:,$global:, or$env:prefixes
Use 4 spaces for indentation, not tabs
Use one space around operators (e.g.,$a = 1 + 2)
Use one space between type and variable (e.g.,[String] $name)
Use one space between keyword and parenthesis (e.g.,if ($condition))
Avoid spaces on empty lines
Limit lines to 120 characters
Place opening brace on a new line, except for variable assignments
Add one newline after opening brace
Add two newlines after closing brace (one if followed by another brace or continuation)
Use single quotes unless variable expansion is needed (e.g.,'text'vs"text $variable")
Use single-line array syntax for simple arrays:@('one', 'two', 'three')
Use multi-line array syntax with each element on a separate line with proper indentation
Do not use the unary comma operator (,) in return statements to force an array
Use empty hashtable syntax:@{}
Place each hashtable property on a separate line with proper indentation
Use PascalCase for hashtable properties
Use single-line comment format# Comment(capitalized, on own line)
Use multi-line comment format<# Comment #>with opening and closing brackets on own lines and indented text
Never include commented-out code
Always add comment-based help to all functions and scripts with SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class declaration
Use comment-based help indentation with keywords at 4 spaces and text at 8 spaces
Include examples in comment-based help for all parameter sets and combinations
In comment-based help INPUTS sec...
Files:
source/Classes/020.SqlPermission.ps1tests/Integration/Resources/DSC_SqlPermission.config.ps1tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
**/[0-9][0-9][0-9].*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
Name class files using
###.ClassName.ps1format (e.g.,001.SqlReason.ps1,004.StartupParameters.ps1)
Files:
source/Classes/020.SqlPermission.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
Use
$PSCmdlet.ThrowTerminatingError()for terminating errors (except in classes), use relevant error category, and in try-catch include exception with localized message
Files:
source/Classes/020.SqlPermission.ps1tests/Integration/Resources/DSC_SqlPermission.config.ps1tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Classes:
source/Classes/{DependencyGroupNumber}.{ClassName}.ps1- Enums:
source/Enum/{DependencyGroupNumber}.{EnumName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/Classes/020.SqlPermission.ps1tests/Integration/Resources/DSC_SqlPermission.config.ps1tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1CHANGELOG.md
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type as inline code with a 1‑line description...
Files:
source/Classes/020.SqlPermission.ps1tests/Integration/Resources/DSC_SqlPermission.config.ps1tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
**/tests/Integration/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
Integration tests: use
Connect-SqlDscDatabaseEnginefor SQL Server DB session with correct CI credentials, and always follow withDisconnect-SqlDscDatabaseEngine
Files:
tests/Integration/Resources/DSC_SqlPermission.config.ps1tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
tests/Integration/Resources/*.Integration.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md)
Location for Resources integration tests:
tests/Integration/Resources/{ResourceName}.Integration.Tests.ps1
Files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
tests/Integration/**/*.Integration.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md)
tests/Integration/**/*.Integration.Tests.ps1: Integration tests must not use mocking - use real environment only
Integration tests must cover all scenarios and code paths
UseGet-ComputerNamefor computer names in CI environments
AvoidExpectedMessageparameter forShould -Throwassertions in integration tests
Call commands with-Forceparameter where applicable to avoid prompting
Use-ErrorAction 'Stop'on commands so failures surface immediately
Integration tests must include the required setup block with BeforeDiscovery that imports DscResource.Test module and BeforeAll that imports the module being tested
Files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
**/*.[Tt]ests.ps1: All public commands, private functions and classes must have unit tests
All public commands and class-based resources must have integration tests
Use Pester v5 syntax only
Test code only insideDescribeblocks
Assertions only inItblocks
Never test verbose messages, debug messages or parameter binding behavior
Pass all mandatory parameters to avoid prompts
InsideItblocks, assign unused return objects to$null(unless part of pipeline)
Tested entity must be called from within theItblocks
Keep results and assertions in sameItblock
Avoid try-catch-finally for cleanup, useAfterAllorAfterEach
Avoid unnecessary remove/recreate cycles
OneDescribeblock per file matching the tested entity name
Contextdescriptions start with 'When'
Itdescriptions start with 'Should', must not contain 'when'
Mock variables prefix: 'mock'
Public commands: Never useInModuleScope(unless retrieving localized strings or creating an object using an internal class)
Private functions/class resources: Always useInModuleScope
Each class method = separateContextblock
Each scenario = separateContextblock
Use nestedContextblocks for complex scenarios
Mocking inBeforeAll(BeforeEachonly when required)
Setup/teardown inBeforeAll,BeforeEach/AfterAll,AfterEachclose to usage
Spacing between blocks, arrange, act, and assert for readability
PascalCase:Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach
Use-BeTrue/-BeFalsenever-Be $true/-Be $false
Never useAssert-MockCalled, useShould -Invokeinstead
NoShould -Not -Throw- invoke commands directly
Never add an empty-MockWithblock
Omit-MockWithwhen returning$null
Set$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName
Omit-ModuleNameparameter on Pester commands
Never useMockinsideInModuleScope-block
Never useparam()inside-MockWithscriptblock...
Files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings or creating an object using an internal class)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usage- Spacing between blocks, arrange, act, and assert for readability
Syntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName, `...
Files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1
⚙️ CodeRabbit configuration file
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: # Integration Tests GuidelinesRequirements
- Location Commands:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1- Location Resources:
tests/Integration/Resources/{ResourceName}.Integration.Tests.ps1- No mocking - real environment only
- Cover all scenarios and code paths
- Use
Get-ComputerNamefor computer names in CI- Avoid
ExpectedMessageforShould -Throwassertions- Only run integration tests in CI unless explicitly instructed.
- Call commands with
-Forceparameter where applicable (avoids prompting).- Use
-ErrorAction 'Stop'on commands so failures surface immediatelyRequired Setup Block
[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 have 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 have 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 noop" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' }
Files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
CHANGELOG.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-changelog.instructions.md)
CHANGELOG.md: Always update the Unreleased section in CHANGELOG.md
Use Keep a Changelog format in CHANGELOG.md
Describe notable changes briefly in CHANGELOG.md, ≤2 items per change type
Reference issues using format issue #<issue_number> in CHANGELOG.md
No empty lines between list items in same section of CHANGELOG.md
Skip adding entry if same change already exists in Unreleased section of CHANGELOG.md
No duplicate sections or items in Unreleased section of CHANGELOG.mdAlways update CHANGELOG.md Unreleased section
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format issue #<issue_number>
- No empty lines between list items in same section
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
**/*.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-markdown.instructions.md)
**/*.md: Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Use 2 spaces for indentation in Markdown files
Use '1.' for all items in ordered lists (1/1/1 numbering style)
DisableMD013rule by adding a comment for tables/code blocks exceeding 80 characters
Empty lines required before/after code blocks and headings (except before line 1)
Escape backslashes in file paths only (not in code blocks)
Code blocks must specify language identifiers
Format parameters using bold in Markdown documentation
Format values and literals usinginline codein Markdown documentation
Format resource/module/product names using italic in Markdown documentation
Format commands/files/paths usinginline codein Markdown documentation
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/resources/**/*.ps1 : Database Engine resources must inherit from `SqlResourceBase`
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : `Test-TargetResource` function must return a boolean value ($true or $false)
Applied to files:
source/Classes/020.SqlPermission.ps1tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`
Applied to files:
source/Classes/020.SqlPermission.ps1tests/Integration/Resources/DSC_SqlPermission.config.ps1tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/resources/**/*.ps1 : Add `InstanceName`, `ServerName`, and `Credential` to `$this.ExcludeDscProperties` in Database Engine resources
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.config.ps1CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : Add `$env:SqlServerDscCI = $true` in `BeforeAll` block and remove in `AfterAll` block of unit tests
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.config.ps1tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1CHANGELOG.md
📚 Learning: 2025-10-19T10:33:22.772Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2309
File: tests/Integration/Commands/Initialize-SqlDscRebuildDatabase.Integration.Tests.ps1:132-148
Timestamp: 2025-10-19T10:33:22.772Z
Learning: In SqlServerDsc integration tests, when connecting via Connect-SqlDscDatabaseEngine using a local Windows account (e.g., 'SqlAdmin'), use the plain username format without computer/domain prefix. This is Windows Authentication with a local account. The NetBIOS format (e.g., 'COMPUTER\SqlAdmin') is only used for SqlSysAdminAccounts during SQL Server setup/rebuild and will throw an exception if used with Connect-SqlDscDatabaseEngine for local Windows accounts.
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.config.ps1
📚 Learning: 2025-08-17T10:13:30.079Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.config.ps1
📚 Learning: 2025-11-27T17:58:31.910Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-11-27T17:58:31.910Z
Learning: Applies to tests/Integration/**/*.Integration.Tests.ps1 : Use `Get-ComputerName` for computer names in CI environments
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.config.ps1
📚 Learning: 2025-11-27T17:58:31.910Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-11-27T17:58:31.910Z
Learning: Applies to tests/Integration/**/*.Integration.Tests.ps1 : Integration tests must cover all scenarios and code paths
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : All public commands and class-based resources must have integration tests
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T17:58:31.910Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-11-27T17:58:31.910Z
Learning: Applies to tests/Integration/**/*.Integration.Tests.ps1 : Integration tests must include the required setup block with BeforeDiscovery that imports DscResource.Test module and BeforeAll that imports the module being tested
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use nested `Context` blocks for complex scenarios
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T17:58:31.910Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-11-27T17:58:31.910Z
Learning: Applies to tests/Integration/Resources/*.Integration.Tests.ps1 : Location for Resources integration tests: `tests/Integration/Resources/{ResourceName}.Integration.Tests.ps1`
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T18:00:20.934Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-11-27T18:00:20.934Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Use the exact setup block with BeforeDiscovery, BeforeAll, and AfterAll blocks including DscResource.Test module import and PSDefaultParameterValues configuration
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Each scenario = separate `Context` block
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T17:58:31.910Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-11-27T17:58:31.910Z
Learning: Applies to tests/Integration/**/*.Integration.Tests.ps1 : Call commands with `-Force` parameter where applicable to avoid prompting
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T18:56:46.759Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 1622
File: tests/Integration/DSC_SqlDatabase.Integration.Tests.ps1:1-30
Timestamp: 2025-11-27T18:56:46.759Z
Learning: Integration tests for MOF-based DSC resources (located in source/DSCResources/**) should use the Initialize-TestEnvironment pattern with -ResourceType 'Mof', not the BeforeDiscovery/BeforeAll pattern. The BeforeDiscovery/BeforeAll pattern is for class-based resources only.
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-10-10T14:01:42.703Z
Learnt from: dan-hughes
Repo: dsccommunity/UpdateServicesDsc PR: 86
File: tests/Unit/MSFT_UpdateServicesServer.Tests.ps1:6-45
Timestamp: 2025-10-10T14:01:42.703Z
Learning: For script-based (MOF) DSC resources in UpdateServicesDsc, unit tests should use Initialize-TestEnvironment with variables $script:dscModuleName and $script:dscResourceName, and set -ResourceType 'Mof'. This differs from class-based resource tests which use the simpler $script:moduleName template without Initialize-TestEnvironment.
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Every DSC resource must define the three required functions: `Get-TargetResource`, `Set-TargetResource`, and `Test-TargetResource`
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : `Set-TargetResource` and `Test-TargetResource` must have identical parameters
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must implement required methods: `Get()`, `Test()`, `Set()`, `GetCurrentState()`, and `Modify()` following the specified pattern
Applied to files:
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (11)
CHANGELOG.md (1)
10-12: Changelog entry forSqlPermissionlooks consistent and well-scopedThe new “Added” bullet correctly describes the integration-test change, follows the existing formatting and Keep a Changelog conventions, and fits under the Unreleased/Added section without duplication.
source/Classes/020.SqlPermission.ps1 (1)
363-369: Principal resolution change is correct and slightly more efficientDeferring
Test-SqlDscIsRoleuntil after confirming that the name is not a login preserves existing behavior while avoiding an unnecessary role lookup when a login exists. The subsequent checks (-not $isLogin -and -not $isRole, login vs role principal object resolution, and@principalParameterconstruction) remain valid for both logins and server roles.tests/Integration/Resources/DSC_SqlPermission.config.ps1 (5)
41-43: LGTM! Role principal configuration added correctly.The
Role1_Nameproperty follows the established pattern forUser1_Nameand correctly references the server role created by SqlRole integration tests.
207-248: Role grant configuration looks good.The configuration correctly mirrors the existing user grant configuration structure with appropriate role-specific permissions (
ViewServerState,AlterAnyEndpoint).
250-283: RemoveGrant configuration follows established pattern.Correctly sets empty Grant permissions to test permission removal for the role, consistent with the user-based
DSC_SqlPermission_RemoveGrant_Config.
285-321: Deny configuration correctly structured.Properly configures deny permissions for
ViewServerStateandAlterAnyEndpointwhile clearing Grant state, mirroring the user deny configuration pattern.
323-356: RemoveDeny configuration completes the test coverage.Correctly clears all permission states (Grant, GrantWithGrant, Deny) to test deny permission removal for the role principal.
tests/Integration/Resources/DSC_SqlPermission.Integration.Tests.ps1 (4)
322-380: Role grant integration test looks good.The test correctly validates:
- MOF compilation and application
- Resource state retrieval via
Get-DscConfiguration- Role1_Name assignment and permission verification (ViewServerState, AlterAnyEndpoint)
- Desired state confirmation via
Test-DscConfigurationStructure and assertions follow the established patterns from existing user-based tests.
382-438: Role RemoveGrant test properly validates permission removal.Correctly verifies that Grant permissions are empty (
Should -BeNullOrEmpty) after applying the RemoveGrant configuration for the role principal.
440-503: Role Deny test validates deny permission assignment.Properly tests:
- Grant state is empty after deny configuration
- Deny state contains expected permissions (ViewServerState, AlterAnyEndpoint)
- Both permission counts (2) are verified
505-566: Role RemoveDeny test completes the role permission test coverage.Correctly validates that both Grant and Deny permission arrays are empty after applying the RemoveDeny configuration, ensuring clean state for subsequent test runs.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
source/Classes/020.SqlPermission.ps1 (1)
365-370: Principal resolution and splatting for login vs server role look correctThe new
$isRolehandling, the conditional call toTest-SqlDscIsRoleonly when the principal is not a login, and theprincipalParametersplatting (switching betweenLoginandServerRole) align with the updated server-permission commands and avoid redundant checks. The error path for a missing principal remains clear and localized. One behavioral note: if a name could exist as both a login and a server role, the login will be preferred, which seems reasonable but is something to be aware of for ambiguous naming schemes.Also applies to: 394-406
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(2 hunks)source/Classes/020.SqlPermission.ps1(3 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
**/*.ps1: Format public commands as{Verb}-SqlDsc{Noun}following PowerShell naming conventions
Format private functions as{Verb}-{Noun}following PowerShell naming conventions
Files:
source/Classes/020.SqlPermission.ps1
source/[cC]lasses/020.*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-class-resource.instructions.md)
DSC class-based resources must use the file naming pattern
source/Classes/020.{ResourceName}.ps1
Files:
source/Classes/020.SqlPermission.ps1
source/[cC]lasses/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-class-resource.instructions.md)
source/[cC]lasses/**/*.ps1: DSC class-based resources must be decorated with[DscResource(RunAsCredential = 'Optional')]or[DscResource(RunAsCredential = 'Mandatory')]based on requirements
DSC class-based resources must inherit fromResourceBaseclass from the DscResource.Base module
DSC class-based resources must use[Nullable[{FullTypeName}]]syntax for value-type properties (e.g.,[Nullable[System.Int32]])
DSC class-based resources must implement a required constructor with signatureResourceName () : base ($PSScriptRoot)and populate$this.ExcludeDscPropertiesarray
DSC class-based resources must implement required methods:Get(),Test(),Set(),GetCurrentState(), andModify()following the specified pattern
DSC class-based resources may optionally implement methods:AssertProperties()andNormalizeProperties()following the specified pattern
DSC class-based resources must include comment-based help with .DESCRIPTION section containing Requirements and Known issues sections with a link to all open issues on GitHub
DSC class-based resources must usetry/catchblocks for exception handling in classes
DSC class-based resources must useNew-*Exceptioncommands (such asNew-InvalidDataException,New-ArgumentException,New-InvalidOperationException,New-ObjectNotFoundException,New-InvalidResultException,New-NotImplementedException) instead ofthrowfor terminating errors in classes
Files:
source/Classes/020.SqlPermission.ps1
⚙️ CodeRabbit configuration file
source/[cC]lasses/**/*.ps1: # DSC Class-Based Resource GuidelinesApplies to: Classes with
[DscResource(...)]decoration only.Requirements
- File:
source/Classes/020.{ResourceName}.ps1- Decoration:
[DscResource(RunAsCredential = 'Optional')](replace with'Mandatory'if required)- Inheritance: Must inherit
ResourceBase(part of module DscResource.Base)$this.localizedDatahashtable auto-populated byResourceBasefrom localization file- value-type properties: Use
[Nullable[{FullTypeName}]](e.g.,[Nullable[System.Int32]])Required constructor
MyResourceName () : base ($PSScriptRoot) { # Property names where state cannot be enforced, e.g. IsSingleInstance, Force $this.ExcludeDscProperties = @() }Required Method Pattern
[MyResourceName] Get() { # Call base implementation to get current state $currentState = ([ResourceBase] $this).Get() # If needed, post-processing on current state that can not be handled by GetCurrentState() return $currentState } [System.Boolean] Test() { # Call base implementation to test current state $inDesiredState = ([ResourceBase] $this).Test() # If needed, post-processing on test result that can not be handled by base Test() return $inDesiredState } [void] Set() { # Call base implementation to set desired state ([ResourceBase] $this).Set() # If needed, additional state changes that can not be handled by Modify() } hidden [System.Collections.Hashtable] GetCurrentState([System.Collections.Hashtable] $properties) { # Always return current state as hashtable, $properties contains key properties } hidden [void] Modify([System.Collections.Hashtable] $properties) { # Always set desired state, $properties contain those that must change state }Optional Method Pattern
hidden [void] AssertProperties([System.Collections.Hashtable] $properties) { # Validate user-provided properties, $p...
Files:
source/Classes/020.SqlPermission.ps1
source/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-localization.instructions.md)
source/**/*.ps1: Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages in PowerShell scripts
Use localized string keys from $script:localizedData, not hardcoded strings, in diagnostic messages
Reference localized strings using the syntax: Write-Verbose -Message ($script:localizedData.KeyName -f $value1)Localize all strings using string keys; remove any orphaned string keys
Files:
source/Classes/020.SqlPermission.ps1
⚙️ CodeRabbit configuration file
source/**/*.ps1: # Localization GuidelinesRequirements
- Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages
- Use localized string keys, not hardcoded strings
- Assume
$script:localizedDatais availableString Files
- Commands/functions:
source/en-US/{MyModuleName}.strings.psd1- Class resources:
source/en-US/{ResourceClassName}.strings.psd1Key Naming Patterns
- Format:
Verb_FunctionName_Action(underscore separators), e.g.Get_Database_ConnectingToDatabaseString Format
ConvertFrom-StringData @' KeyName = Message with {0} placeholder. (PREFIX0001) '@String IDs
- Format:
(PREFIX####)- PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
- Number: Sequential from 0001
Usage
Write-Verbose -Message ($script:localizedData.KeyName -f $value1)
Files:
source/Classes/020.SqlPermission.ps1
source/Classes/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
source/Classes/*.ps1: Classes should be located insource/Classes/{DependencyGroupNumber}.{ClassName}.ps1
DSC resources should always be created as class-based resources
Files:
source/Classes/020.SqlPermission.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Use descriptive names with 3+ characters and no abbreviations for functions, parameters, variables, keywords, and classes
Use PascalCase with Verb-Noun format for function names, using approved PowerShell verbs
Use PascalCase for parameter names
Use camelCase for variable names
Use lower-case for keywords
Use PascalCase for class names
Include scope for script/global/environment variables using$script:,$global:, or$env:prefixes
Use 4 spaces for indentation, not tabs
Use one space around operators (e.g.,$a = 1 + 2)
Use one space between type and variable (e.g.,[String] $name)
Use one space between keyword and parenthesis (e.g.,if ($condition))
Avoid spaces on empty lines
Limit lines to 120 characters
Place opening brace on a new line, except for variable assignments
Add one newline after opening brace
Add two newlines after closing brace (one if followed by another brace or continuation)
Use single quotes unless variable expansion is needed (e.g.,'text'vs"text $variable")
Use single-line array syntax for simple arrays:@('one', 'two', 'three')
Use multi-line array syntax with each element on a separate line with proper indentation
Do not use the unary comma operator (,) in return statements to force an array
Use empty hashtable syntax:@{}
Place each hashtable property on a separate line with proper indentation
Use PascalCase for hashtable properties
Use single-line comment format# Comment(capitalized, on own line)
Use multi-line comment format<# Comment #>with opening and closing brackets on own lines and indented text
Never include commented-out code
Always add comment-based help to all functions and scripts with SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class declaration
Use comment-based help indentation with keywords at 4 spaces and text at 8 spaces
Include examples in comment-based help for all parameter sets and combinations
In comment-based help INPUTS sec...
Files:
source/Classes/020.SqlPermission.ps1
**/[0-9][0-9][0-9].*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
Name class files using
###.ClassName.ps1format (e.g.,001.SqlReason.ps1,004.StartupParameters.ps1)
Files:
source/Classes/020.SqlPermission.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
Use
$PSCmdlet.ThrowTerminatingError()for terminating errors (except in classes), use relevant error category, and in try-catch include exception with localized message
Files:
source/Classes/020.SqlPermission.ps1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Classes:
source/Classes/{DependencyGroupNumber}.{ClassName}.ps1- Enums:
source/Enum/{DependencyGroupNumber}.{EnumName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/Classes/020.SqlPermission.ps1CHANGELOG.md
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type as inline code with a 1‑line description...
Files:
source/Classes/020.SqlPermission.ps1
CHANGELOG.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-changelog.instructions.md)
CHANGELOG.md: Always update the Unreleased section in CHANGELOG.md
Use Keep a Changelog format in CHANGELOG.md
Describe notable changes briefly in CHANGELOG.md, ≤2 items per change type
Reference issues using format issue #<issue_number> in CHANGELOG.md
No empty lines between list items in same section of CHANGELOG.md
Skip adding entry if same change already exists in Unreleased section of CHANGELOG.md
No duplicate sections or items in Unreleased section of CHANGELOG.mdAlways update CHANGELOG.md Unreleased section
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format issue #<issue_number>
- No empty lines between list items in same section
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
**/*.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-markdown.instructions.md)
**/*.md: Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Use 2 spaces for indentation in Markdown files
Use '1.' for all items in ordered lists (1/1/1 numbering style)
DisableMD013rule by adding a comment for tables/code blocks exceeding 80 characters
Empty lines required before/after code blocks and headings (except before line 1)
Escape backslashes in file paths only (not in code blocks)
Code blocks must specify language identifiers
Format parameters using bold in Markdown documentation
Format values and literals usinginline codein Markdown documentation
Format resource/module/product names using italic in Markdown documentation
Format commands/files/paths usinginline codein Markdown documentation
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/resources/**/*.ps1 : Add `InstanceName`, `ServerName`, and `Credential` to `$this.ExcludeDscProperties` in Database Engine resources
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : `Test-TargetResource` function must return a boolean value ($true or $false)
Applied to files:
source/Classes/020.SqlPermission.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`
Applied to files:
source/Classes/020.SqlPermission.ps1CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/resources/**/*.ps1 : Add `InstanceName`, `ServerName`, and `Credential` to `$this.ExcludeDscProperties` in Database Engine resources
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : Add `$env:SqlServerDscCI = $true` in `BeforeAll` block and remove in `AfterAll` block of unit tests
Applied to files:
CHANGELOG.md
📚 Learning: 2025-10-03T15:27:24.417Z
Learnt from: Borgquite
Repo: dsccommunity/UpdateServicesDsc PR: 78
File: source/DSCResources/MSFT_UpdateServicesComputerTargetGroup/MSFT_UpdateServicesComputerTargetGroup.psm1:9-22
Timestamp: 2025-10-03T15:27:24.417Z
Learning: In the UpdateServicesDsc repository, MOF-based DSC resources follow a minimal comment-based help convention that includes only .SYNOPSIS and .PARAMETER sections. The .DESCRIPTION, .INPUTS, and .OUTPUTS sections are intentionally omitted, even though functions have [OutputType()] attributes. This is consistent across all existing DSC resources: MSFT_UpdateServicesServer, MSFT_UpdateServicesCleanup, and MSFT_UpdateServicesApprovalRule.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-22T17:36:09.703Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2343
File: source/Classes/004.DatabaseFileGroupSpec.ps1:5-34
Timestamp: 2025-11-22T17:36:09.703Z
Learning: The comment-based help requirements for ## Requirements and ## Known issues sections only apply to class-based DSC resources (classes decorated with [DscResource(...)]) in source/Classes/**/*.ps1, not to regular helper classes, DTO classes, or specification classes in the same directory.
Applied to files:
CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (2)
CHANGELOG.md (1)
10-12: SqlPermission changelog entries are consistent and clearThe new Unreleased
SqlPermissionentries correctly describe the added integration tests and the broadened principal support (logins and server roles), and they align with the existing changelog structure and formatting. No issues from a style or correctness standpoint.Also applies to: 33-34
source/Classes/020.SqlPermission.ps1 (1)
2-9: Doc updates correctly reflect login and server role supportThe synopsis, description, and Name parameter text now accurately describe that the resource targets either a login or a server role as the principal, which matches the new behavior and remains consistent with the rest of the help.
Also applies to: 63-66
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
10-12: Reword to avoid implying a new resource; add issue reference if available.This top-level bullet reads like a newly added resource. Prefer a single bullet that clearly states tests were added and, if possible, reference the tracking issue.
Apply:
-- `SqlPermission` - - Added integration tests for server role permissions to complement the - existing login permission tests. +- Added integration tests for `SqlPermission` server role permissions to + complement the existing login-permission tests. + <!-- optionally append issue reference if one exists --> + <!-- ([issue #<id>](https://github.com/dsccommunity/SqlServerDsc/issues/<id>)) -->Do you have an issue to reference here? If yes, please append it using the standard format.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(2 hunks)source/Classes/020.SqlPermission.ps1(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- source/Classes/020.SqlPermission.ps1
🧰 Additional context used
📓 Path-based instructions (3)
CHANGELOG.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-changelog.instructions.md)
CHANGELOG.md: Always update the Unreleased section in CHANGELOG.md
Use Keep a Changelog format in CHANGELOG.md
Describe notable changes briefly in CHANGELOG.md, ≤2 items per change type
Reference issues using format issue #<issue_number> in CHANGELOG.md
No empty lines between list items in same section of CHANGELOG.md
Skip adding entry if same change already exists in Unreleased section of CHANGELOG.md
No duplicate sections or items in Unreleased section of CHANGELOG.mdAlways update CHANGELOG.md Unreleased section
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format issue #<issue_number>
- No empty lines between list items in same section
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
**/*.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-markdown.instructions.md)
**/*.md: Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Use 2 spaces for indentation in Markdown files
Use '1.' for all items in ordered lists (1/1/1 numbering style)
DisableMD013rule by adding a comment for tables/code blocks exceeding 80 characters
Empty lines required before/after code blocks and headings (except before line 1)
Escape backslashes in file paths only (not in code blocks)
Code blocks must specify language identifiers
Format parameters using bold in Markdown documentation
Format values and literals usinginline codein Markdown documentation
Format resource/module/product names using italic in Markdown documentation
Format commands/files/paths usinginline codein Markdown documentation
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Classes:
source/Classes/{DependencyGroupNumber}.{ClassName}.ps1- Enums:
source/Enum/{DependencyGroupNumber}.{EnumName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
CHANGELOG.md
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/resources/**/*.ps1 : Add `InstanceName`, `ServerName`, and `Credential` to `$this.ExcludeDscProperties` in Database Engine resources
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : Add `$env:SqlServerDscCI = $true` in `BeforeAll` block and remove in `AfterAll` block of unit tests
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/resources/**/*.ps1 : Add `InstanceName`, `ServerName`, and `Credential` to `$this.ExcludeDscProperties` in Database Engine resources
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : Add `$env:SqlServerDscCI = $true` in `BeforeAll` block and remove in `AfterAll` block of unit tests
Applied to files:
CHANGELOG.md
📚 Learning: 2025-10-03T15:27:24.417Z
Learnt from: Borgquite
Repo: dsccommunity/UpdateServicesDsc PR: 78
File: source/DSCResources/MSFT_UpdateServicesComputerTargetGroup/MSFT_UpdateServicesComputerTargetGroup.psm1:9-22
Timestamp: 2025-10-03T15:27:24.417Z
Learning: In the UpdateServicesDsc repository, MOF-based DSC resources follow a minimal comment-based help convention that includes only .SYNOPSIS and .PARAMETER sections. The .DESCRIPTION, .INPUTS, and .OUTPUTS sections are intentionally omitted, even though functions have [OutputType()] attributes. This is consistent across all existing DSC resources: MSFT_UpdateServicesServer, MSFT_UpdateServicesCleanup, and MSFT_UpdateServicesApprovalRule.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-22T17:36:09.703Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2343
File: source/Classes/004.DatabaseFileGroupSpec.ps1:5-34
Timestamp: 2025-11-22T17:36:09.703Z
Learning: The comment-based help requirements for ## Requirements and ## Known issues sections only apply to class-based DSC resources (classes decorated with [DscResource(...)]) in source/Classes/**/*.ps1, not to regular helper classes, DTO classes, or specification classes in the same directory.
Applied to files:
CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (1)
CHANGELOG.md (1)
33-36: Verify documentation update status before accepting changelog entries.The CHANGELOG.md lines at 33-36 claim documentation updates. If the PR checklist shows documentation (README, schema, help, localization, examples) was not updated, remove these lines from the Unreleased section. If documentation updates are planned for this PR, keep these lines and ensure the docs are updated accordingly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2362 +/- ##
====================================
Coverage 94% 94%
====================================
Files 166 166
Lines 9778 9780 +2
====================================
+ Hits 9219 9221 +2
Misses 559 559
🚀 New features to boost your workflow:
|
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
@johlju reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @johlju)
1 participant
Pull Request (PR) description
SqlPermissionexisting login permission tests.
This Pull Request (PR) fixes the following issues
None.
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
This change is