SqlServerEndpoint : Add parameter "Owner"

[mdaniou]

Pull Request (PR) description

Adding the parameter "Owner" to allow to specify it at creation
It seems like there is no integration test for this resource.

This Pull Request (PR) fixes the following issues

• Fixes SqlServerEndpoint : Should let us choose the owner #1251

Task list

• Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
  Entry should say what was changed, and how that affects users (if applicable).
• Resource documentation added/updated in README.md.
• Resource parameter descriptions added/updated in README.md, schema.mof
  and comment-based help.
• Comment-based help added/updated.
• Localization strings added/updated in all localization files as appropriate.
• Examples appropriately added/updated.
• Unit tests added/updated. See DSC Resource Testing Guidelines.
• Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
• New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

---

This change is 

[codecov-io]

Codecov Report

Merging #1271 into dev will increase coverage by <1%.
The diff coverage is 100%.

@@          Coverage Diff          @@
##            dev   #1271    +/-   ##
=====================================
+ Coverage    97%     97%   +<1%     
=====================================
  Files        34      34            
  Lines      4197    4208    +11     
=====================================
+ Hits       4106    4117    +11     
  Misses       91      91

[johlju]

Great work on this! 🙂 Just a couple of comments, one a logical error I think in the Test-TargetResource function.

Reviewed 1 of 6 files at r1, 5 of 5 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mdaniou)

---

DSCResources/MSFT_SqlServerEndpoint/MSFT_SqlServerEndpoint.psm1, line 302 at r2 (raw file):

elseif ($getTargetResourceResult.Ensure -eq 'Present' -and $Owner)

Instead of this, shouldn't we check this like Port and IpAddress are checked in the if-block above? 🤔

        if ($getTargetResourceResult.Ensure -eq 'Present' `
                -and (
                        $getTargetResourceResult.Port -ne $Port `
                    -or $getTargetResourceResult.IpAddress -ne $IpAddress
                    -or $getTargetResourceResult.Owner-ne $Owner
                )
        )

---

Tests/Unit/MSFT_SqlServerEndpoint.Tests.ps1, line 333 at r2 (raw file):

 listener IP address

should me 'owner' here in the comment?

[johlju]

@mdaniou There hasn't been a integration test written for this resource yet, so I will no enforce it. There is another issue open to get an integration tests for this resource. 🙂

[mdaniou]

@johlju I am actually working on the integration test for this resource.

[johlju]

Oh that is awesome! 😄 Is your plan to add it to this PR or send in a new PR with the integration test? Either way works for me.

[mdaniou]

Reviewable status: 4 of 6 files reviewed, 2 unresolved discussions (waiting on @johlju)

---

DSCResources/MSFT_SqlServerEndpoint/MSFT_SqlServerEndpoint.psm1, line 302 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

elseif ($getTargetResourceResult.Ensure -eq 'Present' -and $Owner)

Instead of this, shouldn't we check this like Port and IpAddress are checked in the if-block above? 🤔

        if ($getTargetResourceResult.Ensure -eq 'Present' `
                -and (
                        $getTargetResourceResult.Port -ne $Port `
                    -or $getTargetResourceResult.IpAddress -ne $IpAddress
                    -or $getTargetResourceResult.Owner-ne $Owner
                )
        )

Well I did that at first then I realized that if $Owner parameter is null then it will fail as the owner is set at creation and is the login used to actually create it :) . Hence the comparison won't work. I need to test only if the parameter is filled.
However I now figure that my code is incomplete. It should be :

elseif ($getTargetResourceResult.Ensure -eq 'Present' -and $Owner `
		-and $getTargetResourceResult.Owner -ne $Owner
)

---

Tests/Unit/MSFT_SqlServerEndpoint.Tests.ps1, line 333 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

 listener IP address

should me 'owner' here in the comment?

Indeed

[mdaniou]

Reviewable status: 4 of 6 files reviewed, 2 unresolved discussions (waiting on @johlju)

---

DSCResources/MSFT_SqlServerEndpoint/MSFT_SqlServerEndpoint.psm1, line 302 at r2 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

Well I did that at first then I realized that if $Owner parameter is null then it will fail as the owner is set at creation and is the login used to actually create it :) . Hence the comparison won't work. I need to test only if the parameter is filled.
However I now figure that my code is incomplete. It should be :

elseif ($getTargetResourceResult.Ensure -eq 'Present' -and $Owner `
		-and $getTargetResourceResult.Owner -ne $Owner
)

Done.

---

Tests/Unit/MSFT_SqlServerEndpoint.Tests.ps1, line 333 at r2 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

Indeed

Done.

[mdaniou]

I will keep them separate as I started working on it after I created this PR. No need to mix up everything :)

[johlju]

Reviewed 2 of 2 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved