DSC_WebSite: Change CertificateStoreName value in the schema
Pull Request (PR) description
Change CertificateStoreName value in the schema to match the value returned by DSC This is aligned with the testing defined https://github.com/dsccommunity/WebAdministrationDsc/blob/3db41e47fad389e2f05e6a61709a3c93ef3be450/tests/Unit/DSC_Website.Tests.ps1#L2252
This Pull Request (PR) fixes the following issues
Fixes https://github.com/dsccommunity/WebAdministrationDsc/issues/642
Task list
- [x] 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).
- [x] Resource documentation added/updated in README.md.
- [x] Resource parameter descriptions added/updated in README.md, schema.mof and comment-based help.
- [x] Comment-based help added/updated.
- [x] Localization strings added/updated in all localization files as appropriate.
- [x] Examples appropriately added/updated.
- [x] Unit tests added/updated. See DSC Community Testing Guidelines.
- [x] Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
- [x] New/changed code adheres to DSC Community Style Guidelines.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 88%. Comparing base (
04d3bd3) to head (6c66430). Report is 4 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #641 +/- ##
===================================
Coverage 88% 88%
===================================
Files 15 15
Lines 1952 1952
===================================
Hits 1726 1726
Misses 226 226
| Files with missing lines | Coverage Δ | |
|---|---|---|
| source/DSCResources/DSC_WebSite/DSC_WebSite.psm1 | 97% <100%> (ø) |
Suggest we instead change the value in the code to My instead of MY:
https://github.com/dsccommunity/WebAdministrationDsc/blob/04d3bd3de3977f242c9642263fd169da6abb5c34/source/DSCResources/DSC_WebSite/DSC_WebSite.psm1#L1463
Suggest we instead change the value in the code to
Myinstead ofMY:https://github.com/dsccommunity/WebAdministrationDsc/blob/04d3bd3de3977f242c9642263fd169da6abb5c34/source/DSCResources/DSC_WebSite/DSC_WebSite.psm1#L1463
I've updated my PR accordingly
Hello @johlju, I've double checked but the get is returning the info in upperscale
protocol : https
bindingInformation : *:443:*
sslFlags : 0
isDsMapperEnabled : False
certificateHash : E2CD2BB7A408F069D4C48E4B6ACE5DCEFD9FE94A
certificateStoreName : MY
Attributes : {protocol, bindingInformation, sslFlags, isDsMapperEnabled...}
ChildElements : {}
ElementTagName : binding
Methods : {EnableDsMapper, DisableDsMapper, AddSslCertificate, RemoveSslCertificate...}
Schema : Microsoft.IIs.PowerShell.Framework.ConfigurationElementSchema
So I guess it's why all the code has been done to use 'MY' instead of 'My'
Do you want me to revert the schema defintion to 'MY' or add in the get to transform it to 'My'
Do you want me to revert the schema definition to 'MY' or add in the get to transform it to 'My'
Maybe we need to control what is passes to the *-TargetResource and what is returned. It is more PowerShell-ish to use My, so let us try converting the value to 'My'.
Also, PowerShell allows us to pass 'MY' even if the schema says 'My', so maybe we should also convert the input value to My in all *-TargetResource that uses it?
Also make the tests use -BeExactly to tests case sensetive.
Do you want me to revert the schema definition to 'MY' or add in the get to transform it to 'My'
Maybe we need to control what is passes to the
*-TargetResourceand what is returned. It is more PowerShell-ish to useMy, so let us try converting the value to 'My'.Also, PowerShell allows us to pass 'MY' even if the schema says 'My', so maybe we should also convert the input value to
Myin all*-TargetResourcethat uses it?Also make the tests use
-BeExactlyto tests case sensetive.
PR updated
I resolved conflicts in the hopes that we could have merged it, so you might need to pull in changes too.
Reviewed 8 of 11 files at r3, 2 of 2 files at r4, 1 of 1 files at r5, all commit messages. Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @webalexeu)
source/DSCResources/DSC_WebSite/DSC_WebSite.psm1line 1313 at r5 (raw file):$cimProperties.Add('CertificateThumbprint', [String]$binding.certificateHash) $cimProperties.Add('CertificateStoreName', [String]($binding.certificateStoreName) -replace ('MY','My'))This should only replace 'MY' if the name is exactly 'MY', not 'MYStore' or 'StoreMY'. 🤔
source/DSCResources/DSC_WebSite/DSC_WebSite.psm1line 1470 at r5 (raw file):else { $certificateStoreName = $binding.CertificateStoreName -replace ('MY','My')Same as previous comment.
Indeed, thanks for the review So I made it case sensitive and match only the whole word
Reviewed all commit messages. Reviewable status: 10 of 11 files reviewed, 2 unresolved discussions (waiting on @webalexeu)
source/DSCResources/DSC_WebSite/DSC_WebSite.psm1line 1313 at r5 (raw file):Previously, johlju (Johan Ljunggren) wrote…
This should only replace 'MY' if the name is exactly 'MY', not 'MYStore' or 'StoreMY'. 🤔
The new change will also convert 'MY store' (although spaces might not be allowed). But just to make sure MY is the only word you need to do:
-replace '^MY$','My'
source/DSCResources/DSC_WebSite/DSC_WebSite.psm1line 1470 at r5 (raw file):Previously, webalexeu wrote… See previous comment.
Updated Thank you