Website binding certificate subject compare fails with multiple entries

[wh33ly]

Pull Request (PR) description

Website binding certificate subject compare fails with multiple entries
As described in the issue, this will fix an issue where the binding needs the correct certificate. Search for the certificate failed when a subject contains more then one property and the order differs from the original subject.

This Pull Request (PR) fixes the following issues

• Fixes xWebsite: Certificate subject compare fails with multiple entries  #412

Task list

• Added an entry under the Unreleased section of the change log in the README.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.
• Localization strings added/updated in all localization files as appropriate.
• Comment-based help added/updated.
• 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 

[msftclas]

All CLA requirements met.

[wh33ly]

I also added tests for the different scenario's I encountered

[codecov-io]

Codecov Report

Merging #413 into dev will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##              dev     #413   +/-   ##
=======================================
  Coverage   90.77%   90.77%           
=======================================
  Files          17       17           
  Lines        2438     2438           
=======================================
  Hits         2213     2213           
  Misses        225      225

Impacted Files	Coverage Δ
DSCResources/Helper.psm1	90% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd15c1a...3abb213. Read the comment docs.

[regedit32]

Reviewed 2 of 2 files at r1, 1 of 1 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @wh33ly)

---

README.md, line 319 at r3 (raw file):

* Helper: Fixed subject comparison for multiple entries

Can you also add the name of the function to be more specific?

---

DSCResources/Helper.psm1, line 168 at r3 (raw file):

        $certFilters += @('(@(Compare-Object -ReferenceObject (($_.Subject -split ", ").trim()|sort-object) -DifferenceObject (($subject -split ",").trim()|sort-object)| Where-Object -Property SideIndicator -eq "=>").Count -eq 0)')

There's a lot of logic in this one line and we should avoid extensive piping. Breaking this out will make it more readable and easier for debugging and testing. I see there are other older parts of this helper that are doing it this way, but we don't have to continue that practice.

[regedit32]

@wh33ly Thanks for submitting a fix for this issue. Just small feedback provided and I think it'll be ready.

[wh33ly]

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @regedit32)

---

README.md, line 319 at r3 (raw file):

Previously, regedit32 (Reggie Gibson) wrote…

* Helper: Fixed subject comparison for multiple entries

Can you also add the name of the function to be more specific?

Updated the unreleased infomation

---

DSCResources/Helper.psm1, line 168 at r3 (raw file):

Previously, regedit32 (Reggie Gibson) wrote…

        $certFilters += @('(@(Compare-Object -ReferenceObject (($_.Subject -split ", ").trim()|sort-object) -DifferenceObject (($subject -split ",").trim()|sort-object)| Where-Object -Property SideIndicator -eq "=>").Count -eq 0)')

There's a lot of logic in this one line and we should avoid extensive piping. Breaking this out will make it more readable and easier for debugging and testing. I see there are other older parts of this helper that are doing it this way, but we don't have to continue that practice.

Thanks for your feedback.

Would you prefer something like this ? Or do you have another suggestion how to improve this. Breaking out makes it definitely more readable but there still is a lot of logic in one line.

I also broke out the other lines in the function

[wh33ly]

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @regedit32)

---

README.md, line 319 at r3 (raw file):

Previously, wh33ly wrote…

Updated the unreleased infomation

Done.

---

DSCResources/Helper.psm1, line 168 at r3 (raw file):

Previously, wh33ly wrote…

Thanks for your feedback.

Would you prefer something like this ? Or do you have another suggestion how to improve this. Breaking out makes it definitely more readable but there still is a lot of logic in one line.

I also broke out the other lines in the function

Done.

[wh33ly]

Updated according to comments

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @regedit32)

[wh33ly]

@regedit32 Think I'm done, is there anything from my side I need to do now ?

[stale]

Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again.

[wh33ly]

@regedit32 Bump

[regedit32]

@wh33ly Sorry for the delay, let me take a look today.