Added New parameter ProductCoveredbySA that is introduced in SQL 2022

[sara921-spec]

This Pull Request (PR) fixes the following issues

Pull Request (PR) description

• 'ProductCoveredbySA'
  • Added new parameter ProductCoveredbySA which is introduced in SQL 2022.

Adding parameter ProductCoveredbySA (introduced in sql 2022) into DSC_SqlSetup
Specifies the license coverage for SQL Server. True indicates it's covered under Software Assurance or SQL Server
subscription.
False, or omitting the parameter, indicates it's covered under a SQL Server license.
Default value is False.

This Pull Request (PR) fixes the following issues

• Fixes New parameter productcoveredbysa : Command proposal #2040

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.

---

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94%. Comparing base (3c30929) to head (7de6c99).
Report is 1 commits behind head on main.

Additional details and impacted files

@@         Coverage Diff          @@
##           main   #2044   +/-   ##
====================================
  Coverage    94%     94%           
====================================
  Files        94      94           
  Lines      7922    7930    +8     
====================================
+ Hits       7492    7500    +8     
  Misses      430     430           

Flag	Coverage Δ
unit	94% <100%> (+<1%)	⬆️

Files with missing lines	Coverage Δ
source/DSCResources/DSC_SqlSetup/DSC_SqlSetup.psm1	98% <100%> (+<1%)	⬆️

[johlju]

Thank you for sending this in.

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @sara921-spec)

a discussion (no related file):
Please also update the schema: https://github.com/dsccommunity/SqlServerDsc/blob/main/source/DSCResources/DSC_SqlSetup/DSC_SqlSetup.schema.mof

---

---

source/DSCResources/DSC_SqlSetup/DSC_SqlSetup.psm1 line 1319 at r2 (raw file):

        {
            $setupArguments['ProductCoveredBySA'] = $false
        }

I think we can skip this? Omitting the parameter from the arguments is the same as saying /PRODUCTCOVEREDBYSA=False. It will also simplify the unit test cases needed.

Code quote:

        else
        {
            $setupArguments['ProductCoveredBySA'] = $false
        }

[johlju]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @sara921-spec)

---

CHANGELOG.md line 11 at r2 (raw file):

- 'ProductCoveredbySA'
 - Added new parameter ProductCoveredbySA which in introduced in SQL 2022.

Suggest changing to the below. Use SqlSetup and make sure second entry is indented with 2 spaces for the list to render correctly.

Suggestion:

- SqlSetup
  - Added new parameter ProductCoveredbySA which in introduced in SQL 2022.

[johlju]

Happy to take a look, but might take a few days before I have time. 😊

[johlju]

Reviewed 1 of 4 files at r3, all commit messages.
Reviewable status: 1 of 4 files reviewed, 2 unresolved discussions (waiting on @sara921-spec)

---

source/DSCResources/DSC_SqlSetup/DSC_SqlSetup.psm1 line 325 at r3 (raw file):

        {
           # Grab the value of ProductCoveredBySA from the registry based on the instance
           $getTargetResourceReturnValue.ProductCoveredBySA = Get-ItemProperty "HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\MSSQL$($SqlVersion).$($InstanceName)\Setup\IsProductCoveredBySA"

Instead of Get-ItemProperty, try using the helper function Get-RegistryPropertyValue: https://github.com/dsccommunity/SqlServerDsc/blob/3c3092976409645d74f58707331f66ffe1967127/source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1#L18C10-L18C35

But you probably have to test that it actually can get the value in a real environment using the helper function. 🤔 .

Code quote:

Get-ItemProperty

[johlju]

If the above command (in the review comment) is able to get the value, then it will should resolve the error in the tests. If the command does not work, then you need to handle the error being thrown another way. 🙂

[sara921-spec]

If the above command (in the review comment) is able to get the value, then it will should resolve the error in the tests. If the command does not work, then you need to handle the error being thrown another way. 🙂

@johlju This update to use "Get-RegistryPropertyValue" as suggested worked to pass the HKLM path errors, we have only 2 check failures now.

error - 'Conn' is not a valid value for setting 'FEATURES'.

[johlju]

The Feature CONN and some other features are not available in SQL Server 2022. I see you have enabled SQL Server 2022 (major version '16') for a bunch of tests which is good because that is really needed, but that is why it is failing. You need to make sure the correct values are passed to *-TargetResource and then evaluated the correct values where returned when the major version is 16. For example not calling the *-TargetResource with 'CONN' features parameter.

[sara921-spec]

Hi @johlju What are the available features for 2022 or how to get that? any idea?

#1566

Seems "RS" is not valid too

Error: Context When installing a default instance for major version 16
Context When installing all features
##[error] [-] Should set the system in the desired state when feature is SQLENGINE 59ms (54ms|5ms)
##[error] Expected no exception to be thrown, but an exception "System.InvalidOperationException: 'Rs' is not a valid value for setting 'FEATURES'. Refer to SQL Help for more information." was thrown from D:\a\1\s\output\builtModule\SqlServerDsc\17.0.0\Modules\DscResource.Common\0.17.2\DscResource.Common.psm1:3755 char:9

[dan-hughes]

Just the last few bits left and I can pass it off to someone with merge rights :-).

No use splatting
Code snippet:

$getRegistryPropertyParams = @{
   Path = "HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\MSSQL$($SqlVersion).$($InstanceName)\Setup"
   Name = 'IsProductCoveredBySA'
}

@dan-hughes Does it not needed to be passed like below? I mean for ($getTargetResourceReturnValue?) or that line is not needed? Thought we need that gettargetrespurcereturnvalue

$getTargetResourceReturnValue.ProductCoveredBySA = Get-RegistryPropertyValue @getRegistryPropertyParams

You are correct that is how you pass a splat to a command. The last commit you made looks to be good.

[dan-hughes]

@sara921-spec, run a format on the module file and commit again, it should fix the HQRM test.

[dan-hughes]

@dan-hughes It actually looked for single quotes as mentioned here https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects).,

I believe we are looking good now! do u see any other updates required?

The path string was not the issue, it was the formatting. I've just cloned your repo and opened it in VSCode.

If I put your double quotes back so the string has the correct values in and format it it looks like this.

[sara921-spec]

@dan-hughes does it looks good now?

[dan-hughes]

@sara921-spec, I'll look at this over the weekend.

[dan-hughes]

Unit Tests:
ContextBlock 'When SQL Server version is and the system is not in the desired state for default instance'

• It needs to add the check that the value is false.

[sara921-spec]

Unit Tests: ContextBlock 'When SQL Server version is and the system is not in the desired state for default instance'

• It needs to add the check that the value is false.

@dan-hughes False for the parameter productcoveredbysa? In the past johan had the below thought (or am I not reading your request correctly?)

[dan-hughes]

Unit Tests: ContextBlock 'When SQL Server version is and the system is not in the desired state for default instance'

• It needs to add the check that the value is false.

@dan-hughes False for the parameter productcoveredbysa? In the past johan had the below thought (or am I not reading your request correctly?)

Apologies for not being clear,

It was more of adding the $result.ProductCoveredBySA | Should -BeFalse check here for the as the resource should return the default value for the type.

It looks like you have resolved all the other issues so once this is done and it builds it's good to go.

[dan-hughes]

I'm not sure where I got that Context block title from though as I've just searched and cannot find it!

[dan-hughes]

I'm not in front of a big enough screen to look at the whole test file, but this test looks to be a copy + paste of the test added at line 2484. Is this supposed to be here?

[sara921-spec]

ah, line 2826 is the actual test, I don't know how 2484 got there, I should have misplaced it somehow. let me remove it

[sara921-spec]

@dan-hughes all set I guess -:)

[johlju]

@sara921-spec awesome work on this, thank you! Also big thanks to @dan-hughes for the review! 🙇‍♂️