BREAKING: DSC Protocol Class-Based#72
Open
dan-hughes wants to merge 82 commits intodsccommunity:mainfrom
Open
Conversation
Contributor
WalkthroughReplaces the function-based Changes
Sequence DiagramsequenceDiagram
participant DSC as DSC Engine
participant Resource as SChannelProtocolClient/<br/>SChannelProtocolServer
participant Base as SChannelProtocolBase
participant Helper as Get-/Enable-/Disable-/Reset-TlsProtocol
participant Enum as Get-EnumFlags
participant Registry as Registry Layer
DSC->>Resource: Get()
Resource->>Base: delegate Get()
Base->>Helper: Get-TlsProtocol(ClientSide)
Helper->>Registry: Read protocol registry
Registry-->>Helper: Current states
Helper-->>Base: Enabled/Disabled/Default lists
Base-->>Resource: Current state
Resource-->>DSC: Return state
DSC->>Resource: Test()
Resource->>Base: delegate Test()
Base->>Base: Compare current vs desired
Base-->>Resource: In-sync? true/false
Resource-->>DSC: Return result
DSC->>Resource: Set()
Resource->>Base: delegate Set()/Modify()
Base->>Enum: Get-EnumFlags(ProtocolsEnabled)
Enum-->>Base: Individual flags
Base->>Helper: Enable-TlsProtocol(each flag)
Helper->>Registry: Write enabled
Base->>Enum: Get-EnumFlags(ProtocolsDisabled)
Enum-->>Base: Individual flags
Base->>Helper: Disable-TlsProtocol(each flag)
Helper->>Registry: Write disabled
Base-->>Resource: Modification complete
Resource-->>DSC: Enforcement done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
johlju
reviewed
Feb 2, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #72 +/- ##
====================================
Coverage ? 98%
====================================
Files ? 21
Lines ? 583
Branches ? 0
====================================
Hits ? 575
Misses ? 8
Partials ? 0
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
source/Examples/Resources/SChannelProtocolServer/2-DisableProtocol.ps1 (1)
48-54: Opening brace should be on a new line afternode localhost.As per coding guidelines, braces should be on a new line (except for variable assignments). This applies to all the new example files in this PR.
Proposed fix
- node localhost { - SChannelProtocolServer DisableSSLv3 + node localhost + { + SChannelProtocolServer 'DisableSSLv3' { IsSingleInstance = 'Yes' ProtocolsDisabled = 'Ssl3' } }Also note: the resource instance name
DisableSSLv3should be quoted as a string literal ('DisableSSLv3') for consistency with DSC Community conventions (e.g., line 51 ofSChannelSettings/3-ConfigureWinHTTPProtocols.ps1uses'ConfigureWinHTTPProtocols').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/Examples/Resources/SChannelProtocolServer/2-DisableProtocol.ps1` around lines 48 - 54, Move the opening brace for the node block onto its own line so the block reads with the brace on a new line after the `node localhost` declaration; adjust the snippet containing `SChannelProtocolServer DisableSSLv3` accordingly to follow the brace style, and change the resource instance name `DisableSSLv3` to a quoted string literal (`'DisableSSLv3'`) to match DSC Community conventions and other examples such as `SChannelSettings/3-ConfigureWinHTTPProtocols.ps1`.source/Private/Get-EnumFlags.ps1 (2)
17-37: Minor style issues: param block formatting and abbreviated type name.Per coding guidelines,
paramopening parenthesis should be on its own line, and full type names should be preferred (e.g.,[System.Enum]instead of[Enum]on line 30).Proposed fix
function Get-EnumFlags { [CmdletBinding()] - param ( + param + ( [Parameter(Mandatory = $true, ValueFromPipeline = $true, Position = 0)] [System.Enum] $Value ) process { $enumType = $Value.GetType() - foreach ($flag in [Enum]::GetValues($enumType)) + foreach ($flag in [System.Enum]::GetValues($enumType)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/Private/Get-EnumFlags.ps1` around lines 17 - 37, The Get-EnumFlags function has minor style issues: move the param block's opening parenthesis to its own line and replace the abbreviated type name [Enum] with the full type [System.Enum]; specifically, in the Get-EnumFlags declaration adjust the param block so the "(" is on its own line and update the foreach line that currently calls [Enum]::GetValues($enumType) to use [System.Enum]::GetValues($enumType) so full type names are used consistently.
1-16: Comment-based help is missing.INPUTSand.NOTESsections.Since the function accepts pipeline input (
ValueFromPipeline), the.INPUTSsection should documentSystem.Enum. Also,[OutputType([System.Enum])]should be added to the function declaration. As per coding guidelines,.INPUTSmust list each pipeline-accepted type, and[OutputType()]is required for functions with output.Proposed fix for comment-based help and OutputType
.OUTPUTS System.Enum + The individual flags that are set in the input value. + + .INPUTS + System.Enum + + The enum value to expand. + .EXAMPLE Get-EnumFlags -Value ([SChannelSslProtocols]::Tls12 -bor [SChannelSslProtocols]::Tls13) #> function Get-EnumFlags { [CmdletBinding()] + [OutputType([System.Enum])] param (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/Private/Get-EnumFlags.ps1` around lines 1 - 16, Add missing comment-based help and OutputType: update the comment block for the Get-EnumFlags function to include an .INPUTS section documenting that the function accepts pipeline input of type System.Enum (since the Value parameter uses ValueFromPipeline), add a .NOTES section (brief note about behavior or author/version as per project conventions), and add an [OutputType([System.Enum])] attribute to the Get-EnumFlags function declaration so the function signature advertises it emits System.Enum values.tests/Unit/Classes/SChannelProtocolClient.Tests.ps1 (2)
32-40:Import-Modulemissing-ErrorAction 'Stop'.The required unit test setup template specifies
-ErrorAction 'Stop'on theImport-Modulecall to surface import failures immediately. As per coding guidelines, the exact setup block template should be followed.Proposed fix
- Import-Module -Name $script:dscModuleName + Import-Module -Name $script:dscModuleName -ErrorAction 'Stop'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/Classes/SChannelProtocolClient.Tests.ps1` around lines 32 - 40, The BeforeAll block uses Import-Module without forcing failures to surface; update the Import-Module invocation in the BeforeAll setup (the Import-Module call) to include -ErrorAction 'Stop' so module import errors are thrown immediately and the test setup fails fast.
53-58:Should -Not -Throwon constructor — consider invoking directly.Per test guidelines, prefer invoking commands directly and letting Pester catch exceptions naturally. The subsequent test on lines 61-68 already validates that
::new()succeeds and returns a non-null instance, making this block somewhat redundant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/Classes/SChannelProtocolClient.Tests.ps1` around lines 53 - 58, The test uses "{ [SChannelProtocolClient]::new() } | Should -Not -Throw" which is redundant and against guidelines; either remove this block or change it to invoke the constructor directly (e.g., call [SChannelProtocolClient]::new() inside the InModuleScope so Pester will naturally surface exceptions), noting that the subsequent test already asserts the constructor returns a non-null instance; update the test containing SChannelProtocolClient::new accordingly.tests/Integration/Resources/DSC_SChannelProtocolServer.Integration.Tests.ps1 (1)
115-124: Backtick line continuation on lines 117-118.Same as client test — consider restructuring to avoid backticks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Integration/Resources/DSC_SChannelProtocolServer.Integration.Tests.ps1` around lines 115 - 124, The Where-Object filter uses backtick line continuation (in the scriptblock passed to Where-Object -FilterScript) which is fragile; change the $resourceCurrentState assignment to avoid backticks by writing the scriptblock without backticks (e.g., place the whole scriptblock on one line or wrap the conditions in parentheses and break lines naturally) so the comparisons using $_.ConfigurationName and $_.ResourceId are combined with -and inside the FilterScript cleanly; mirror the same non-backtick style used in the client test for consistency and update the $resourceCurrentState variable assignment accordingly.tests/Unit/Classes/SChannelProtocolServer.Tests.ps1 (1)
32-40:Import-Modulemissing-ErrorAction 'Stop'— same as client test.Proposed fix
- Import-Module -Name $script:dscModuleName + Import-Module -Name $script:dscModuleName -ErrorAction 'Stop'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/Classes/SChannelProtocolServer.Tests.ps1` around lines 32 - 40, The BeforeAll block uses Import-Module to load $script:dscModuleName but omits -ErrorAction 'Stop'; update the Import-Module call in the BeforeAll block (where $script:dscModuleName is set) to include -ErrorAction 'Stop' so module load failures throw and fail the test setup instead of continuing silently.tests/Integration/Resources/DSC_SChannelProtocolClient.Integration.Tests.ps1 (1)
115-124: Backtick line continuation on lines 117-118.Consider restructuring to avoid backticks, e.g., by splatting or enclosing the filter in parentheses. As per coding guidelines, backtick line continuation should be avoided.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Integration/Resources/DSC_SChannelProtocolClient.Integration.Tests.ps1` around lines 115 - 124, The Where-Object filter uses PowerShell backtick line continuation between the $_.ConfigurationName and $_.ResourceId conditions; remove the backticks by rewriting the filter as a single expression (e.g., wrap the predicate in parentheses or a scriptblock on one line) so the block in Where-Object -FilterScript { $_.ConfigurationName -eq $configurationName -and $_.ResourceId -eq $resourceId } has no trailing backticks; update the reference to $script:currentConfiguration | Where-Object -FilterScript accordingly to keep the same variables ($configurationName, $resourceId) and logic.CHANGELOG.md (2)
20-24: Inconsistent issue reference format.Other entries in this file wrap issue references in parentheses, e.g.,
([issue#69](…))on line 15. Line 20 omits the outer parentheses. Also, the DTLS entry on line 24 follows the same unwrapped pattern.Proposed fix
-- BREAKING: Converted Protocol to class-based resource [issue `#50`](https://github.com/dsccommunity/SChannelDsc/issues/50). +- BREAKING: Converted Protocol to class-based resource ([issue `#50`](https://github.com/dsccommunity/SChannelDsc/issues/50)). - This now consists of two resources - SChannelProtocolClient - SChannelProtocolServer - - Add DTLS support [issue `#37`](https://github.com/dsccommunity/SChannelDsc/issues/37). + - Add DTLS support ([issue `#37`](https://github.com/dsccommunity/SChannelDsc/issues/37)).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 20 - 24, Change the two issue references in the BREAKING entry so they match the file's established format by wrapping each issue link in an outer pair of parentheses; specifically update the text fragments "BREAKING: Converted Protocol to class-based resource [issue `#50`](...)" and "Add DTLS support [issue `#37`](...)" to use "([issue `#50`](...))" and "([issue `#37`](...))" respectively so the parentheses style is consistent with other entries like the one containing "([issue `#69`](...))".
50-50: Missing issue reference for Protocol removal.This entry is part of the same conversion effort. Consider referencing issue
#50for traceability, consistent with the Added entry.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` at line 50, Update the CHANGELOG entry "Protocol Resource - Replaced by class resources" to include the issue reference `#50` for traceability (match the format used in the Added entry); locate that line and append " (see `#50`)" or "— see `#50`" so the Protocol removal entry explicitly references issue `#50`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@source/Examples/Resources/SChannelSettings/1-ConfigureSChannel.ps1`:
- Line 6: The script contains a placeholder sequential GUID assigned to the
.GUID token (".GUID a1b2c3d4-5e6f-7081-92a3-bcdef0123456"); replace this with a
real unique GUID generated via [guid]::NewGuid() (either generate once and paste
the resulting GUID literal, or change the assignment to evaluate
[guid]::NewGuid() at runtime) so the .GUID value is unique and avoids
collisions.
---
Duplicate comments:
In `@source/Examples/Resources/SChannelProtocolClient/1-EnableProtocol.ps1`:
- Line 48: The opening brace for the node block in "node localhost {" uses
brace-on-same-line formatting; update the block in 1-EnableProtocol.ps1 so the
brace is placed on its own line (e.g., change "node localhost {" to "node
localhost" followed by a new line containing "{") to match the server example's
style and keep consistent formatting for the node block.
In `@source/Examples/Resources/SChannelProtocolClient/2-DisableProtocol.ps1`:
- Line 48: The opening brace for the node block is on the same line as the
header ("node localhost {"), violating the project's brace style; change
occurrences of "node localhost {" to place the brace on the next line (i.e.,
"node localhost" then a new line with "{") to match the server example style,
and ensure the corresponding closing brace remains correctly paired; search for
other similar blocks in this file and make the same adjustment.
In `@source/Examples/Resources/SChannelProtocolServer/3-DefaultProtocol.ps1`:
- Line 48: The opening brace is on the same line as the node declaration ("node
localhost {"); move the brace to its own following line so the block starts with
"node localhost" on one line and "{" on the next, matching the style used in the
server disable example and ensuring consistent brace placement for the node
block.
In
`@tests/Integration/Resources/DSC_SChannelProtocolClient.Integration.Tests.ps1`:
- Around line 39-43: Remove the call to Initialize-TestEnvironment (it's
MOF-specific) for the class-based resource SChannelProtocolClient (which
inherits from ResourceBase via SChannelProtocolBase); instead simply
Import-Module the module containing the class (the file under
source/Classes/020.SChannelProtocolClient.ps1) in the test setup and delete the
corresponding Restore-TestEnvironment call in the AfterAll cleanup; also update
the failing error message referenced near line 22 to say "Tasks build" and
instruct users to run ".\build.ps1 -ResolveDependency -Tasks build" instead of
mentioning "Tasks noop".
In
`@tests/Integration/Resources/DSC_SChannelProtocolServer.Integration.Tests.ps1`:
- Around line 39-43: Replace the class-resource-specific test setup: remove the
Initialize-TestEnvironment invocation that uses -ResourceType 'Mof' and instead
simply Import-Module the compiled DSC module (use the same module identifier
used elsewhere in the test, e.g. $script:moduleName or the compiled module path)
so the class-based resource (SChannelProtocolServer/SChannelProtocolBase) is
available; then remove the Restore-TestEnvironment call from the AfterAll
cleanup since no environment was created; finally update the error message
referenced on line 22 to read "Tasks build" so it reports the correct context.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 20-24: Change the two issue references in the BREAKING entry so
they match the file's established format by wrapping each issue link in an outer
pair of parentheses; specifically update the text fragments "BREAKING: Converted
Protocol to class-based resource [issue `#50`](...)" and "Add DTLS support [issue
`#37`](...)" to use "([issue `#50`](...))" and "([issue `#37`](...))" respectively so
the parentheses style is consistent with other entries like the one containing
"([issue `#69`](...))".
- Line 50: Update the CHANGELOG entry "Protocol Resource - Replaced by class
resources" to include the issue reference `#50` for traceability (match the format
used in the Added entry); locate that line and append " (see `#50`)" or "— see
`#50`" so the Protocol removal entry explicitly references issue `#50`.
In `@source/Examples/Resources/SChannelProtocolServer/2-DisableProtocol.ps1`:
- Around line 48-54: Move the opening brace for the node block onto its own line
so the block reads with the brace on a new line after the `node localhost`
declaration; adjust the snippet containing `SChannelProtocolServer DisableSSLv3`
accordingly to follow the brace style, and change the resource instance name
`DisableSSLv3` to a quoted string literal (`'DisableSSLv3'`) to match DSC
Community conventions and other examples such as
`SChannelSettings/3-ConfigureWinHTTPProtocols.ps1`.
In `@source/Private/Get-EnumFlags.ps1`:
- Around line 17-37: The Get-EnumFlags function has minor style issues: move the
param block's opening parenthesis to its own line and replace the abbreviated
type name [Enum] with the full type [System.Enum]; specifically, in the
Get-EnumFlags declaration adjust the param block so the "(" is on its own line
and update the foreach line that currently calls [Enum]::GetValues($enumType) to
use [System.Enum]::GetValues($enumType) so full type names are used
consistently.
- Around line 1-16: Add missing comment-based help and OutputType: update the
comment block for the Get-EnumFlags function to include an .INPUTS section
documenting that the function accepts pipeline input of type System.Enum (since
the Value parameter uses ValueFromPipeline), add a .NOTES section (brief note
about behavior or author/version as per project conventions), and add an
[OutputType([System.Enum])] attribute to the Get-EnumFlags function declaration
so the function signature advertises it emits System.Enum values.
In
`@tests/Integration/Resources/DSC_SChannelProtocolClient.Integration.Tests.ps1`:
- Around line 115-124: The Where-Object filter uses PowerShell backtick line
continuation between the $_.ConfigurationName and $_.ResourceId conditions;
remove the backticks by rewriting the filter as a single expression (e.g., wrap
the predicate in parentheses or a scriptblock on one line) so the block in
Where-Object -FilterScript { $_.ConfigurationName -eq $configurationName -and
$_.ResourceId -eq $resourceId } has no trailing backticks; update the reference
to $script:currentConfiguration | Where-Object -FilterScript accordingly to keep
the same variables ($configurationName, $resourceId) and logic.
In
`@tests/Integration/Resources/DSC_SChannelProtocolServer.Integration.Tests.ps1`:
- Around line 115-124: The Where-Object filter uses backtick line continuation
(in the scriptblock passed to Where-Object -FilterScript) which is fragile;
change the $resourceCurrentState assignment to avoid backticks by writing the
scriptblock without backticks (e.g., place the whole scriptblock on one line or
wrap the conditions in parentheses and break lines naturally) so the comparisons
using $_.ConfigurationName and $_.ResourceId are combined with -and inside the
FilterScript cleanly; mirror the same non-backtick style used in the client test
for consistency and update the $resourceCurrentState variable assignment
accordingly.
In `@tests/Unit/Classes/SChannelProtocolClient.Tests.ps1`:
- Around line 32-40: The BeforeAll block uses Import-Module without forcing
failures to surface; update the Import-Module invocation in the BeforeAll setup
(the Import-Module call) to include -ErrorAction 'Stop' so module import errors
are thrown immediately and the test setup fails fast.
- Around line 53-58: The test uses "{ [SChannelProtocolClient]::new() } | Should
-Not -Throw" which is redundant and against guidelines; either remove this block
or change it to invoke the constructor directly (e.g., call
[SChannelProtocolClient]::new() inside the InModuleScope so Pester will
naturally surface exceptions), noting that the subsequent test already asserts
the constructor returns a non-null instance; update the test containing
SChannelProtocolClient::new accordingly.
In `@tests/Unit/Classes/SChannelProtocolServer.Tests.ps1`:
- Around line 32-40: The BeforeAll block uses Import-Module to load
$script:dscModuleName but omits -ErrorAction 'Stop'; update the Import-Module
call in the BeforeAll block (where $script:dscModuleName is set) to include
-ErrorAction 'Stop' so module load failures throw and fail the test setup
instead of continuing silently.
source/Examples/Resources/SChannelSettings/1-ConfigureSChannel.ps1
Outdated
Show resolved
Hide resolved
johlju
approved these changes
Feb 17, 2026
Member
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
Ready to merge we you feel it is ready (and tests passes)
@johlju partially reviewed 63 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on dan-hughes).
GitVersion.yml line 2 at r3 (raw file):
mode: ContinuousDelivery next-version: 2.0.0
This will likely result in a 2.1.0 preview release (see title in the pipeline run). Maybe 1.9.0 might make next release 2.0.0? 🤔
source/Classes/010.SChannelProtocolBase.ps1 line 5 at r3 (raw file):
.PARAMETER Reasons Returns the reason a property is not in desired state. #>
Should we add the other parameters as well?
Code quote:
<#
.PARAMETER Reasons
Returns the reason a property is not in desired state.
#>source/Private/Get-EnumFlags.ps1 line 20 at r3 (raw file):
{ [CmdletBinding()] param (
Move parentheses to next row.
Code quote:
param (
Member
|
FYI using "breaking" in pr title should bump major version to next major version without changing gitversion.yml |
Contributor
Author
|
That's what I expected to happen, but pipelines shows 1.6.0. |
Member
|
Ah, correction: a PR needs to have "breaking" in a commit message. When you squash merge a PR the PR title becomes the commit message on main branch so it needs "breaking" in it, then it will bump major version on main branch. |
Member
|
These control how versions are bumped based on text in commit messages: Lines 3 to 6 in cecce02 But any commit message will always bump patch version regardless of the above regex in |
This reverts commit 4a5fd64.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
source/Private/Get-EnumFlags.ps1 (3)
1-25: Add missing.INPUTSsection and[OutputType()]attribute.Two coding guideline violations:
- The comment-based help is missing a
.INPUTSsection, required because$Valueis a pipeline parameter (ValueFromPipeline = $true).- There is no
[OutputType()]attribute, which is required for any function that produces output.♻️ Proposed fix
<# .SYNOPSIS Expands a [Flags] enum value into its individual flags. .DESCRIPTION Accepts an enum value and returns the individual enum members that are set. .PARAMETER Value The enum value (or numeric value) to expand. Can be a pipeline input. + .INPUTS + System.Enum + + The [Flags] enum value to expand into its individual set flags. + .OUTPUTS System.Enum .EXAMPLE Get-EnumFlags -Value ([SChannelSslProtocols]::Tls12 -bor [SChannelSslProtocols]::Tls13) #> function Get-EnumFlags { [CmdletBinding()] + [OutputType([System.Enum])] paramAs per coding guidelines: "INPUTS: List each pipeline‑accepted type as inline code with a 1‑line description." and "
[OutputType({return type})]for functions with output".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/Private/Get-EnumFlags.ps1` around lines 1 - 25, Add a .INPUTS comment-based help section that documents the pipeline-accepted type for Get-EnumFlags (e.g., `System.Enum` — "Enum value to expand") and add an [OutputType([System.Enum])] attribute immediately above the Get-EnumFlags function declaration to declare the function's output type (reference symbols: Get-EnumFlags, parameter Value).
31-33: Use[System.Enum]full type name and prefer$flag.value__over[System.Int32]cast.Two issues in this block:
- Line 31:
[Enum]is a type alias; the coding guidelines require using the full type name[System.Enum].- Line 33:
[System.Int32] $flag -ne 0assumes the enum's underlying type fits inInt32. Since the function signature accepts any[System.Enum], enums backed byInt64orUInt64could overflow. Thevalue__auto-property returns the underlying value as the correct native type and avoids any cast.♻️ Proposed fix
- foreach ($flag in [Enum]::GetValues($enumType)) + foreach ($flag in [System.Enum]::GetValues($enumType)) { - if ([System.Int32] $flag -ne 0 -and $Value.HasFlag($flag)) + if ($flag.value__ -ne 0 -and $Value.HasFlag($flag)) {As per coding guidelines: "Use full type name when type casting."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/Private/Get-EnumFlags.ps1` around lines 31 - 33, Replace the type alias and unsafe cast in Get-EnumFlags: use the full type name [System.Enum] when calling GetValues ($enumType) and replace the cast ([System.Int32] $flag -ne 0) with checking the enum member's underlying value via $flag.value__ (e.g., $flag.value__ -ne 0) so the check works for any underlying enum type; keep the HasFlag($flag) usage and the $enumType, $flag and $Value identifiers as-is.
17-17: Rename toGet-EnumFlag(singular noun).The
PSUseSingularNounsrule flags plural nouns in function names; PowerShell conventions require singular nouns. Update call sites at:
source/Public/Test-TlsProtocol.ps1:79source/Public/Reset-TlsProtocol.ps1:98source/Private/Set-TlsProtocolRegistryValue.ps1:100Also update the example at
source/Private/Get-EnumFlags.ps1:15.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/Private/Get-EnumFlags.ps1` at line 17, Rename the function Get-EnumFlags to Get-EnumFlag (singular) to satisfy PSUseSingularNouns; update all internal references and the example inside the same module (the example near the top of the file) and replace invocations in the other scripts that call it (Test-TlsProtocol, Reset-TlsProtocol, and Set-TlsProtocolRegistryValue) to call Get-EnumFlag; ensure exported/used function name, any help/comments, and any splatting or module manifest exports are updated consistently.tests/Integration/Resources/DSC_SChannelProtocolServer.config.ps1 (1)
20-21:ProtocolsDisabledandProtocolsDefaultuse inconsistent array syntax.
ProtocolsEnabledis an explicit@(...)array whileProtocolsDisabledis a bare string andProtocolsDefaultrelies on the implicit comma. Use consistent@(...)syntax for all three.♻️ Proposed fix
- ProtocolsDisabled = 'Tls11' - ProtocolsDefault = 'Tls11', 'Tls13' + ProtocolsDisabled = @( + 'Tls11' + ) + ProtocolsDefault = @( + 'Tls11' + 'Tls13' + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Integration/Resources/DSC_SChannelProtocolServer.config.ps1` around lines 20 - 21, ProtocolsDisabled and ProtocolsDefault use inconsistent array syntax compared to ProtocolsEnabled; update ProtocolsDisabled to use an explicit array (e.g., @('Tls11')) and ProtocolsDefault to use an explicit array (e.g., @('Tls11','Tls13')) so all three properties use the same @(...) array syntax, leaving the values unchanged and preserving commas between elements.tests/Unit/Classes/SChannelProtocolClient.Tests.ps1 (2)
51-79: Multiple top-levelDescribeblocks — each method should be aContextinstead.The guidelines require one
Describeblock per file matching the tested entity name, with each class method in a separateContextblock. The file currently has four separateDescribeblocks (SChannelProtocolClient,SChannelProtocolClient\Get(),SChannelProtocolClient\Set(),SChannelProtocolClient\Test()).♻️ Suggested restructure (outline)
-Describe 'SChannelProtocolClient' { - Context 'When class is instantiated' { ... } -} - -Describe 'SChannelProtocolClient\Get()' -Tag 'Get' { - ... -} - -Describe 'SChannelProtocolClient\Set()' -Tag 'Set' { - ... -} - -Describe 'SChannelProtocolClient\Test()' -Tag 'Test' { - ... -} +Describe 'SChannelProtocolClient' { + Context 'When class is instantiated' { ... } + + Context 'When Get() is called' -Tag 'Get' { + # existing Get() contexts nested here + } + + Context 'When Set() is called' -Tag 'Set' { + # existing Set() contexts nested here + } + + Context 'When Test() is called' -Tag 'Test' { + # existing Test() contexts nested here + } +}As per coding guidelines: "One
Describeblock per file matching the tested entity name" and "Each class method = separateContextblock".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/Classes/SChannelProtocolClient.Tests.ps1` around lines 51 - 79, Consolidate the multiple top-level Describe blocks into a single Describe 'SChannelProtocolClient' block and change each existing method-level Describe to a Context block (e.g., Context 'Get()', Context 'Set()', Context 'Test()') so each class method is tested inside the one Describe for SChannelProtocolClient; update the existing test blocks that reference [SChannelProtocolClient]::new(), GetType(), and the method-specific assertions to live under their respective Contexts and remove the extra Describe declarations to follow the one-Describe-per-file guideline.
32-40:$script:dscModuleNamedeviates from the required unit test template.The exact setup block in the guidelines uses
$script:moduleName.$script:dscModuleName(the MOF-resource pattern) is defined inconsistently with the class-based resource template.♻️ Proposed refactor
BeforeAll { - $script:dscModuleName = 'SChannelDsc' + $script:moduleName = 'SChannelDsc' - Import-Module -Name $script:dscModuleName -ErrorAction 'Stop' + Import-Module -Name $script:moduleName -ErrorAction 'Stop' - $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:dscModuleName - $PSDefaultParameterValues['Mock:ModuleName'] = $script:dscModuleName - $PSDefaultParameterValues['Should:ModuleName'] = $script:dscModuleName + $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:moduleName + $PSDefaultParameterValues['Mock:ModuleName'] = $script:moduleName + $PSDefaultParameterValues['Should:ModuleName'] = $script:moduleName } AfterAll { ... - Get-Module -Name $script:dscModuleName -All | Remove-Module -Force + Get-Module -Name $script:moduleName -All | Remove-Module -Force }As per coding guidelines, the exact unit test setup block uses
$script:moduleNamefor class-based resources.$script:dscModuleNameis the pattern for MOF-based resources.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/Classes/SChannelProtocolClient.Tests.ps1` around lines 32 - 40, The BeforeAll setup uses the MOF-style variable $script:dscModuleName instead of the class-based test template variable $script:moduleName; change the variable name to $script:moduleName and update all references in the BeforeAll block (Import-Module and the three $PSDefaultParameterValues entries) so the setup matches the required class-based unit test template and uses $script:moduleName consistently.tests/Integration/Resources/DSC_SChannelProtocolClient.config.ps1 (1)
20-21: Same array-syntax inconsistency as the server config —ProtocolsDisabledis a bare string.♻️ Proposed fix
- ProtocolsDisabled = 'Tls11' - ProtocolsDefault = 'Tls11', 'Tls13' + ProtocolsDisabled = @( + 'Tls11' + ) + ProtocolsDefault = @( + 'Tls11' + 'Tls13' + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Integration/Resources/DSC_SChannelProtocolClient.config.ps1` around lines 20 - 21, The ProtocolsDisabled entry is using a bare string while ProtocolsDefault uses an array; update ProtocolsDisabled to use the same array syntax as ProtocolsDefault (i.e., make ProtocolsDisabled an array containing 'Tls11') so both entries are consistent; locate the ProtocolsDisabled and ProtocolsDefault keys in the DSC_SChannelProtocolClient.config.ps1 and change ProtocolsDisabled to the array form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@RequiredModules.psd1`:
- Line 23: Remove the trailing spaces on the otherwise-empty line in
RequiredModules.psd1 so the blank line contains no whitespace; edit the file (or
configure your editor/formatter) to trim end-of-line whitespace for that blank
line so it becomes a true empty line.
In `@tests/Integration/Resources/DSC_SChannelProtocolClient.config.ps1`:
- Line 8: The DSC ConfigurationData loading uses ConvertFrom-Json which returns
a PSCustomObject but DSC expects a [Hashtable]; update the assignment for
$ConfigurationData (the Get-Content | ConvertFrom-Json pipeline) to produce a
Hashtable instead (e.g., use ConvertFrom-Json -AsHashtable or cast/wrap the
result into a [Hashtable]) so the DSC ConfigurationData variable is a proper
hashtable at runtime.
In `@tests/Integration/Resources/DSC_SChannelProtocolServer.config.ps1`:
- Line 8: The code assigns $ConfigurationData from ConvertFrom-Json which
returns a PSCustomObject but DSC expects a [Hashtable]; update the assignment so
$ConfigurationData is a hashtable (use ConvertFrom-Json -AsHashtable when
running on PowerShell 7.3+ or otherwise pipe/convert the PSCustomObject into a
recursive helper that transforms objects/arrays into hashtables/arrays) and
ensure the variable name $ConfigurationData is replaced with the converted
hashtable before it’s passed to the DSC Configuration block.
In
`@tests/Integration/Resources/DSC_SChannelProtocolServer.Integration.Tests.ps1`:
- Line 101: Replace the hardcoded ComputerName = 'localhost' with the CI-safe
helper by calling Get-ComputerName from DscResource.Common; update the test to
import/ensure DscResource.Common is available if not already and set the
ComputerName property using Get-ComputerName() (reference the ComputerName
assignment in the DSC_SChannelProtocolServer.Integration.Tests.ps1 test where
'localhost' is used).
- Line 22: The error string thrown in the setup of the integration test
currently instructs users to run ".\build.ps1 -ResolveDependency -Tasks noop";
update that thrown message to instruct ".\build.ps1 -ResolveDependency -Tasks
build" so the guidance matches the required template—locate the throw
'DscResource.Test module dependency not found...' statement and replace the
"noop" task name with "build" in the message.
---
Duplicate comments:
In `@source/Classes/020.SChannelProtocolClient.ps1`:
- Around line 1-12: The .DESCRIPTION block in the SChannelProtocolClient class
is missing the required "## Requirements" and "## Known issues" subsections;
update the .DESCRIPTION in source/Classes/020.SChannelProtocolClient.ps1 (the
class SChannelProtocolClient) by adding a "## Requirements" section that lists
the minimum OS/PowerShell/DSC module prerequisites and any version constraints,
and a "## Known issues" section summarizing critical or expected limitations and
the established pattern for reporting/workarounds (e.g., known
incompatibilities, reboot behavior, audit limitations); ensure these headings
are formatted exactly as "## Requirements" and "## Known issues" and placed
inside the existing .DESCRIPTION comment block.
- Line 14: The DSC resource attribute on the class is missing the
RunAsCredential parameter; update the class-level attribute from [DscResource()]
to include RunAsCredential (either 'Optional' or 'Mandatory' per requirements),
e.g. change [DscResource()] to [DscResource(RunAsCredential = 'Optional')] (or
'Mandatory') so the class (the DSC resource in 020.SChannelProtocolClient.ps1)
complies with the coding guideline.
In `@source/Classes/020.SChannelProtocolServer.ps1`:
- Line 14: The DSC resource attribute on the class is missing the
RunAsCredential parameter; update the [DscResource()] decorator on the class
declaration to include RunAsCredential (e.g., [DscResource(RunAsCredential =
'Optional')] or 'Mandatory' as appropriate for this resource's needs) so the
class-based resource complies with the coding guideline; modify the attribute
where [DscResource()] appears in the file (the class declaration for the
s-channel protocol server resource) to include the chosen RunAsCredential value.
- Around line 1-12: The .DESCRIPTION block in the class-based DSC resource (top
of 020.SChannelProtocolServer.ps1) is missing the required "## Requirements" and
"## Known issues" subsections; update the .DESCRIPTION for the
SChannelProtocolServer class by adding a "## Requirements" section that lists
minimum runtime/OS/module prerequisites (e.g., minimum PowerShell/Windows/DSC
module versions used by this resource) and a "## Known issues" section that
enumerates any critical or expected limitations and patterns (reboot behavior,
unsupported protocols, auditing caveats) so the SChannelProtocolServer DSC
resource documentation is complete and follows the class-based DSC resource
guidelines.
In
`@tests/Integration/Resources/DSC_SChannelProtocolServer.Integration.Tests.ps1`:
- Around line 39-43: The test initializes the environment for a class-based DSC
resource using Initialize-TestEnvironment with -ResourceType 'Mof'; update the
test to use -ResourceType 'Class' instead (i.e., change the -ResourceType
argument passed to Initialize-TestEnvironment) or remove the
Initialize-TestEnvironment call and convert setup to the class-based test
pattern using BeforeDiscovery/BeforeAll hooks; target the call that sets
$script:testEnvironment and the Initialize-TestEnvironment invocation so
SChannelProtocolServer (class inheriting ResourceBase) is initialized correctly.
---
Nitpick comments:
In `@source/Private/Get-EnumFlags.ps1`:
- Around line 1-25: Add a .INPUTS comment-based help section that documents the
pipeline-accepted type for Get-EnumFlags (e.g., `System.Enum` — "Enum value to
expand") and add an [OutputType([System.Enum])] attribute immediately above the
Get-EnumFlags function declaration to declare the function's output type
(reference symbols: Get-EnumFlags, parameter Value).
- Around line 31-33: Replace the type alias and unsafe cast in Get-EnumFlags:
use the full type name [System.Enum] when calling GetValues ($enumType) and
replace the cast ([System.Int32] $flag -ne 0) with checking the enum member's
underlying value via $flag.value__ (e.g., $flag.value__ -ne 0) so the check
works for any underlying enum type; keep the HasFlag($flag) usage and the
$enumType, $flag and $Value identifiers as-is.
- Line 17: Rename the function Get-EnumFlags to Get-EnumFlag (singular) to
satisfy PSUseSingularNouns; update all internal references and the example
inside the same module (the example near the top of the file) and replace
invocations in the other scripts that call it (Test-TlsProtocol,
Reset-TlsProtocol, and Set-TlsProtocolRegistryValue) to call Get-EnumFlag;
ensure exported/used function name, any help/comments, and any splatting or
module manifest exports are updated consistently.
In `@tests/Integration/Resources/DSC_SChannelProtocolClient.config.ps1`:
- Around line 20-21: The ProtocolsDisabled entry is using a bare string while
ProtocolsDefault uses an array; update ProtocolsDisabled to use the same array
syntax as ProtocolsDefault (i.e., make ProtocolsDisabled an array containing
'Tls11') so both entries are consistent; locate the ProtocolsDisabled and
ProtocolsDefault keys in the DSC_SChannelProtocolClient.config.ps1 and change
ProtocolsDisabled to the array form.
In `@tests/Integration/Resources/DSC_SChannelProtocolServer.config.ps1`:
- Around line 20-21: ProtocolsDisabled and ProtocolsDefault use inconsistent
array syntax compared to ProtocolsEnabled; update ProtocolsDisabled to use an
explicit array (e.g., @('Tls11')) and ProtocolsDefault to use an explicit array
(e.g., @('Tls11','Tls13')) so all three properties use the same @(...) array
syntax, leaving the values unchanged and preserving commas between elements.
In `@tests/Unit/Classes/SChannelProtocolClient.Tests.ps1`:
- Around line 51-79: Consolidate the multiple top-level Describe blocks into a
single Describe 'SChannelProtocolClient' block and change each existing
method-level Describe to a Context block (e.g., Context 'Get()', Context
'Set()', Context 'Test()') so each class method is tested inside the one
Describe for SChannelProtocolClient; update the existing test blocks that
reference [SChannelProtocolClient]::new(), GetType(), and the method-specific
assertions to live under their respective Contexts and remove the extra Describe
declarations to follow the one-Describe-per-file guideline.
- Around line 32-40: The BeforeAll setup uses the MOF-style variable
$script:dscModuleName instead of the class-based test template variable
$script:moduleName; change the variable name to $script:moduleName and update
all references in the BeforeAll block (Import-Module and the three
$PSDefaultParameterValues entries) so the setup matches the required class-based
unit test template and uses $script:moduleName consistently.
| AllowPrerelease = $true | ||
| } | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Trailing whitespace on blank line.
Line 23 is an otherwise-empty line containing only spaces. As per coding guidelines, "No line shall have trailing whitespace."
🔧 Proposed fix
-
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@RequiredModules.psd1` at line 23, Remove the trailing spaces on the
otherwise-empty line in RequiredModules.psd1 so the blank line contains no
whitespace; edit the file (or configure your editor/formatter) to trim
end-of-line whitespace for that blank line so it becomes a true empty line.
| Allows reading the configuration data from a JSON file, | ||
| for real testing scenarios outside of the CI. | ||
| #> | ||
| $ConfigurationData = Get-Content -Path $configFile | ConvertFrom-Json |
Contributor
There was a problem hiding this comment.
Same ConvertFrom-Json → PSCustomObject issue as in the server config.
ConvertFrom-Json without -AsHashtable returns a PSCustomObject; DSC ConfigurationData requires a [Hashtable]. The JSON loading path will fail at runtime when a developer provides a companion JSON file.
🛠️ Proposed fix
- $ConfigurationData = Get-Content -Path $configFile | ConvertFrom-Json
+ $ConfigurationData = Get-Content -Path $configFile | ConvertFrom-Json -AsHashtable🧰 Tools
🪛 PSScriptAnalyzer (1.24.0)
[warning] 8-8: The variable 'ConfigurationData' is assigned but never used.
(PSUseDeclaredVarsMoreThanAssignments)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Integration/Resources/DSC_SChannelProtocolClient.config.ps1` at line 8,
The DSC ConfigurationData loading uses ConvertFrom-Json which returns a
PSCustomObject but DSC expects a [Hashtable]; update the assignment for
$ConfigurationData (the Get-Content | ConvertFrom-Json pipeline) to produce a
Hashtable instead (e.g., use ConvertFrom-Json -AsHashtable or cast/wrap the
result into a [Hashtable]) so the DSC ConfigurationData variable is a proper
hashtable at runtime.
| Allows reading the configuration data from a JSON file, | ||
| for real testing scenarios outside of the CI. | ||
| #> | ||
| $ConfigurationData = Get-Content -Path $configFile | ConvertFrom-Json |
Contributor
There was a problem hiding this comment.
ConvertFrom-Json returns PSCustomObject, but DSC ConfigurationData requires a [Hashtable].
Without -AsHashtable, ConvertFrom-Json produces a PSCustomObject. Passing that to a DSC Configuration block's -ConfigurationData parameter will cause a type validation error at runtime for any developer who creates a companion JSON file.
🛠️ Proposed fix (PowerShell 7.3+)
- $ConfigurationData = Get-Content -Path $configFile | ConvertFrom-Json
+ $ConfigurationData = Get-Content -Path $configFile | ConvertFrom-Json -AsHashtableFor broader PowerShell version compatibility, a recursive conversion helper may be needed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $ConfigurationData = Get-Content -Path $configFile | ConvertFrom-Json | |
| $ConfigurationData = Get-Content -Path $configFile | ConvertFrom-Json -AsHashtable |
🧰 Tools
🪛 PSScriptAnalyzer (1.24.0)
[warning] 8-8: The variable 'ConfigurationData' is assigned but never used.
(PSUseDeclaredVarsMoreThanAssignments)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Integration/Resources/DSC_SChannelProtocolServer.config.ps1` at line 8,
The code assigns $ConfigurationData from ConvertFrom-Json which returns a
PSCustomObject but DSC expects a [Hashtable]; update the assignment so
$ConfigurationData is a hashtable (use ConvertFrom-Json -AsHashtable when
running on PowerShell 7.3+ or otherwise pipe/convert the PSCustomObject into a
recursive helper that transforms objects/arrays into hashtables/arrays) and
ensure the variable name $ConfigurationData is replaced with the converted
hashtable before it’s passed to the DSC Configuration block.
| } | ||
| catch [System.IO.FileNotFoundException] | ||
| { | ||
| throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' |
Contributor
There was a problem hiding this comment.
Error message instructs noop task — should be build.
The required template (both unit and integration) instructs users to run .\build.ps1 -ResolveDependency -Tasks build, not noop.
🛠️ Proposed fix
- throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.'
+ throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.'As per coding guidelines, the integration test required setup block must tell users to run ".\build.ps1 -ResolveDependency -Tasks build".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Integration/Resources/DSC_SChannelProtocolServer.Integration.Tests.ps1`
at line 22, The error string thrown in the setup of the integration test
currently instructs users to run ".\build.ps1 -ResolveDependency -Tasks noop";
update that thrown message to instruct ".\build.ps1 -ResolveDependency -Tasks
build" so the guidance matches the required template—locate the throw
'DscResource.Test module dependency not found...' statement and replace the
"noop" task name with "build" in the message.
|
|
||
| $startDscConfigurationParameters = @{ | ||
| Path = ('{0}\{1}' -f $TestDrive, $configurationName) | ||
| ComputerName = 'localhost' |
Contributor
There was a problem hiding this comment.
Hardcoded 'localhost' — use Get-ComputerName instead.
Integration test guidelines require Get-ComputerName (from DscResource.Common) for all computer names in CI to ensure cross-environment compatibility.
🛠️ Proposed fix
- ComputerName = 'localhost'
+ ComputerName = 'localhost' # or: (Get-ComputerName)If Get-ComputerName is available from DscResource.Common:
- ComputerName = 'localhost'
+ ComputerName = Get-ComputerNameAs per coding guidelines, integration tests must use Get-ComputerName for computer names in CI environments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Integration/Resources/DSC_SChannelProtocolServer.Integration.Tests.ps1`
at line 101, Replace the hardcoded ComputerName = 'localhost' with the CI-safe
helper by calling Get-ComputerName from DscResource.Common; update the test to
import/ensure DscResource.Common is available if not already and set the
ComputerName property using Get-ComputerName() (reference the ComputerName
assignment in the DSC_SChannelProtocolServer.Integration.Tests.ps1 test where
'localhost' is used).
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
source/Classes/010.SChannelProtocolBase.ps1 (1)
70-70:GetCurrentStateshould behiddenfor consistency withModifyandAssertProperties.Both
Modify(line 110) andAssertProperties(line 172) are declaredhidden.GetCurrentStateis called internally byResourceBase.Get(), not by consumers, and should match.♻️ Proposed fix
- [System.Collections.Hashtable] GetCurrentState([System.Collections.Hashtable] $properties) + hidden [System.Collections.Hashtable] GetCurrentState([System.Collections.Hashtable] $properties)As per coding guidelines, the required method pattern specifies
hidden [System.Collections.Hashtable] GetCurrentState(...).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/Classes/010.SChannelProtocolBase.ps1` at line 70, The GetCurrentState method is missing the hidden visibility attribute; update its declaration to match the pattern used by Modify and AssertProperties by adding the hidden modifier to the method signature for GetCurrentState (i.e., change the declaration of GetCurrentState([System.Collections.Hashtable] $properties) to the hidden variant) so it is not exposed to consumers and remains consistent with ResourceBase.Get() usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@source/Classes/010.SChannelProtocolBase.ps1`:
- Around line 150-205: The current drift comes from ProtocolsDefault lacking an
inverse and AssertProperties not enforcing full coverage; choose one of two
fixes: (A) Enforce full coverage in AssertProperties by computing the union of
values from ProtocolsEnabled, ProtocolsDisabled and ProtocolsDefault, compare
that union against a canonical list of known protocols
(Ssl2,Ssl3,Tls,Tls11,Tls12,Tls13,Dtls1,Dtls12) and throw New-ArgumentException
if any are missing or duplicated (reference AssertProperties and
$protocolProperties), or (B) change comparison semantics in
GetCurrentState/Test/Modify so GetCurrentState returns only the subset of
protocols the user supplied (i.e., filter current-state results to the keys
present in the incoming $properties) and adjust Modify flows (including
Reset-TlsProtocol/ProtocolsDefault handling) to operate only on those specified
protocols, thereby avoiding the need for full coverage; implement one approach
and update associated logic (Reset-TlsProtocol, ProtocolsDefault handling, and
Test/Modify callers) accordingly.
---
Nitpick comments:
In `@source/Classes/010.SChannelProtocolBase.ps1`:
- Line 70: The GetCurrentState method is missing the hidden visibility
attribute; update its declaration to match the pattern used by Modify and
AssertProperties by adding the hidden modifier to the method signature for
GetCurrentState (i.e., change the declaration of
GetCurrentState([System.Collections.Hashtable] $properties) to the hidden
variant) so it is not exposed to consumers and remains consistent with
ResourceBase.Get() usage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request (PR) description
Convert Protocol DSC resource to two class based resources. One each for Server and Client settings.
This Pull Request (PR) fixes the following issues
Protocol: Convert to class-based resource #50Task list
Entry should say what was changed and how that affects users (if applicable), and
reference the issue being resolved (if applicable).
help.
This change is