BREAKING CHANGE: SqlDatabasePermission: Refactored resource (class-based resource) #1769

johlju · 2022-07-24T08:25:55Z

Pull Request (PR) description

SqlServerDsc
- Added a file prefix.ps1 which content is placed first in the built module
  (.psm1). This file imports dependent modules, and imports localized strings
  used by private and public commands.
- The following classes were added to the module:
  - DatabasePermission - complex type for the DSC resource SqlDatabasePermission.
  - Ensure - Enum to be used for the property Ensure in class-based
    resources.
  - Reason - Used by method Get() to return the reason a property is not
    in desired state.
  - ResourceBase - class that can be inherited by class-based resource and
    provides functionality meant simplify the creating of class-based resource.
- The following private functions were added to the module (see comment-based
  help for more information):
  - ConvertFrom-CompareResult
  - ConvertTo-Reason
  - Get-ClassName
  - Get-DscProperty
  - Get-LocalizedDataRecursive
  - Test-ResourceHasProperty
  - Test-ResourcePropertyIsAssigned
- The following public functions were added to the module (see comment-based
  help for more information):
  - Connect-SqlDscDatabaseEngine
  - ConvertFrom-SqlDscDatabasePermission
  - ConvertTo-SqlDscDatabasePermission
  - Get-SqlDscDatabasePermission
  - Set-SqlDscDatabasePermission
  - Test-SqlDscIsDatabasePrincipal
- Exclude Script Analyzer rule TypeNotFound in the file .vscode/analyzersettings.psd1.
- Update CONTRIBUTING.md describing error handling in commands and class-based
  resources.
- The QA tests are now run in Windows PowerShell due to a bug in PowerShell 7
  that makes class-based resource using inheritance to not work.
- The QA test are excluding the rule TypeNotFound because it cannot
  run on the source files (there is a new issue that is tracking so this
  rule is only run on the built module).
- The Pester code coverage has been switched to use the older functionality
  that uses breakpoints to calculate coverage. Newer functionality sometimes
  throw an exception when used in conjunction with class-based resources.¨
- The SMO stubs (used in the unit tests) was updated to remove a bug related
  to the type DatabasePermissionInfo when used with the type DatabasePermissionSet.
  The stubs suggested that the property PermissionType (of type DatabasePermissionSet)
  in DatabasePermissionInfo should have been a array DatabasePermissionSet[].
  This conflicted with real SMO as it does not pass an array, but instead
  a single DatabasePermissionSet. The stubs was modified to mimic the
  real SMO. At the same time some old mock code in the SMO stubs was removed
  as it was no longer in use.
SqlServerDsc.Common
- The parameter SetupCredential of the function Connect-SQL was renamed
  to Credential and the parameter name SetupCredential was made a
  parameter alias.
SqlDatabasePermission
- BREAKING CHANGE: The resource has been refactored. The parameters
  ParameterState and Permissions has been replaced by parameters
  Permission, PermissionToInclude, and PermissionToExclude. These
  permissions parameters are now an instance of the type DatabasePermission.
  The type DatabasePermission contains two properties; State and
  Permission.
- The resource was refactored into a class-based resource.

This Pull Request (PR) fixes the following issues

Fixes SqlDatabasePermission: Possible to both grant and deny same permission for a user #1555

Task list

Added an entry to the change log under the Unreleased section of the
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
Resource documentation updated in the resource's README.md.
Resource parameter descriptions updated in schema.mof.
Comment-based help updated, including parameter descriptions.
Localization strings updated.
Examples updated.
Unit tests updated. See DSC Community Testing Guidelines.
Integration tests updated (where possible). See DSC Community Testing Guidelines.
Code changes adheres to DSC Community Style Guidelines.

This change is

johlju · 2022-07-24T08:26:55Z

I would be happy to get initial review on this while I get the integration tests finished.

codecov · 2022-07-24T09:09:54Z

Codecov Report

Merging #1769 (ce11053) into main (5b0a7e3) will decrease coverage by 1%.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           main   #1769    +/-   ##
=====================================
- Coverage    87%     86%    -2%     
=====================================
  Files        38      37     -1     
  Lines      6349    6261    -88     
=====================================
- Hits       5575    5422   -153     
- Misses      774     839    +65

Flag	Coverage Δ
unit	`86% <100%> (-2%)`	⬇️

Impacted Files	Coverage Δ
...dules/SqlServerDsc.Common/SqlServerDsc.Common.psm1	`98% <100%> (ø)`
...ces/DSC_SqlConfiguration/DSC_SqlConfiguration.psm1	`94% <0%> (-6%)`	⬇️
...DSC_SqlAlwaysOnService/DSC_SqlAlwaysOnService.psm1	`95% <0%> (-5%)`	⬇️
...efaultLocation/DSC_SqlDatabaseDefaultLocation.psm1	`95% <0%> (-5%)`	⬇️
...ces/DSC_SqlAgentFailsafe/DSC_SqlAgentFailsafe.psm1	`95% <0%> (-5%)`	⬇️
...urce/DSCResources/DSC_SqlScript/DSC_SqlScript.psm1	`95% <0%> (-5%)`	⬇️
...sources/DSC_SqlScriptQuery/DSC_SqlScriptQuery.psm1	`95% <0%> (-5%)`	⬇️
...SCResources/DSC_SqlWaitForAG/DSC_SqlWaitForAG.psm1	`95% <0%> (-5%)`	⬇️
...lEndpointPermission/DSC_SqlEndpointPermission.psm1	`95% <0%> (-5%)`	⬇️
...Resources/DSC_SqlPermission/DSC_SqlPermission.psm1	`96% <0%> (-4%)`	⬇️
... and 13 more

johlju

Reviewed 24 of 55 files at r1, 42 of 43 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions

source/Private/ConvertTo-Reason.ps1 line 34 at r4 (raw file):

        [Parameter(Mandatory = $true)]
        [System.String]
        $ResourceName

Missing the parameter in comment-based help, and example is wromg.

Code quote:

$ResourceName

source/Private/Get-ClassName.ps1 line 24 at r4 (raw file):

        [Parameter()]
        [System.Management.Automation.SwitchParameter]
        $Recurse

Missing the parameter in comment-based help, and missing an example.

Code quote:

$Recurse

source/Private/Get-DscProperty.ps1 line 27 at r4 (raw file):

        [Parameter()]
        [System.String[]]
        $Name,

Missing the parameter in comment-based help, and missing an example.

Code quote:

$Name,

source/Private/Get-LocalizedDataRecursive.ps1 line 22 at r4 (raw file):

        Returns a string array with at least one item.
#>
function Get-LocalizedDataRecursive

Missing an example.

Code quote:

Get-LocalizedDataRecursive

source/Private/Test-ResourceHasProperty.ps1 line 22 at r4 (raw file):

        [Boolean]
#>
function Test-ResourceHasProperty

Missing an example

Code quote:

Test-ResourceHasProperty

source/Private/Test-ResourcePropertyIsAssigned.ps1 line 17 at r4 (raw file):

        [Boolean]
#>
function Test-ResourcePropertyIsAssigned

Missing an example.

Code quote:

Test-ResourcePropertyIsAssigned

source/Public/Set-SqlDscDatabasePermission.ps1 line 94 at r4 (raw file):

        [Parameter()]
        [System.Management.Automation.SwitchParameter]
        $Force

Missing the parameter in comment-based help

Code quote:

$Force

johlju

Reviewable status: 46 of 67 files reviewed, 1 unresolved discussion (waiting on @johlju)

source/Private/Get-ClassName.ps1 line 24 at r4 (raw file):