VMHyperV: Added the ability to enable or disable the TPM on a VM #215
Conversation
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
johlju
added
help wanted
trevonmckay
approved these changes
Oct 14, 2025
johlju
approved these changes
Nov 25, 2025
WalkthroughThis pull request adds Trusted Platform Module (TPM) enable/disable support to the VMHyperV DSC resource. Changes include new schema properties, Get-Target/Set-Target/Test-Target resource enhancements, a helper function for TPM state validation, example configuration, and corresponding unit tests. Changes
Sequence DiagramsequenceDiagram
participant Config as DSC Config
participant GetTarget as Get-TargetResource
participant SetTarget as Set-TargetResource
participant TestTarget as Test-TargetResource
participant VM as Hyper-V VM
participant Security as Get-VMSecurity
participant TPM as Enable/Disable-VMTPM
Config->>GetTarget: Invoke (Gen2 VM)
GetTarget->>Security: Query TPM state
Security-->>GetTarget: TpmEnabled status
GetTarget-->>Config: Return TpmEnabled property
Config->>SetTarget: Invoke (EnableTPM param)
alt EnableTPM = true
SetTarget->>TPM: Enable-VMTPM (with key protector if needed)
else EnableTPM = false
SetTarget->>TPM: Disable-VMTPM
end
TPM-->>SetTarget: Complete
SetTarget-->>Config: State applied
Config->>TestTarget: Validate (EnableTPM param)
TestTarget->>Security: Check current TPM state
Security-->>TestTarget: Current TpmEnabled value
TestTarget-->>Config: Compliant (true/false)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
CHANGELOG.md (1)
25-25: Link TPM change to issue #214 in Unreleased sectionTo align with the changelog guidelines (reference issues from entries), consider updating this bullet to include a link to the tracked issue, e.g.
... TPM on a virtual machine. Fixes [issue #214](https://github.com/dsccommunity/HyperVDsc/issues/214).Based on learnings, …
source/Examples/Resources/VMHyperV/7-TPMEnabled.ps1 (1)
1-43: Expand comment-based help to full structureThe example only has a
.DESCRIPTIONblock. To comply with the PowerShell/comment-based help guidelines, consider adding at least.SYNOPSIS,.PARAMETERentries for NodeName, VMName, VhdPath, and a simple.EXAMPLEshowing how to invokeExample. This keeps examples consistent with the rest of the module’s documentation.As per coding guidelines, …
tests/Unit/DSC_VMHyperV.Tests.ps1 (1)
394-398: UseShould -Invokeinstead ofAssert-MockCalledin new testThe new test correctly validates that
Get-VMSecurityis called for generation 2 VMs, but it introduces anotherAssert-MockCalled, which goes against the current Pester guidelines for this repo (preferShould -Invoke).Consider rewriting the assertion like this:
- It 'Calls Get-VMSecurity if a generation 2 VM' { - Mock -CommandName Get-VMSecurity -MockWith { return $true } - $null = Get-TargetResource -Name 'Generation2VM' -VhdPath $stubVhdxDisk.Path - Assert-MockCalled -CommandName Get-VMSecurity -Scope It -Exactly 1 - } + It 'Calls Get-VMSecurity if a generation 2 VM' { + Mock -CommandName Get-VMSecurity + + $null = Get-TargetResource -Name 'Generation2VM' -VhdPath $stubVhdxDisk.Path + + Should -Invoke -CommandName Get-VMSecurity -Exactly -Times 1 -Scope It + }This keeps the behavior while aligning with the “never use Assert-MockCalled” rule.
As per coding guidelines, …
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1(8 hunks)source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.schema.mof(1 hunks)source/Examples/Resources/VMHyperV/7-TPMEnabled.ps1(1 hunks)tests/Unit/DSC_VMHyperV.Tests.ps1(3 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Classes:
source/Classes/{DependencyGroupNumber}.{ClassName}.ps1- Enums:
source/Enum/{DependencyGroupNumber}.{EnumName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/Examples/Resources/VMHyperV/7-TPMEnabled.ps1tests/Unit/DSC_VMHyperV.Tests.ps1CHANGELOG.mdsource/DSCResources/DSC_VMHyperV/DSC_VMHyperV.schema.mofsource/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
source/Examples/Resources/VMHyperV/7-TPMEnabled.ps1tests/Unit/DSC_VMHyperV.Tests.ps1source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
source/**/*.ps1
⚙️ CodeRabbit configuration file
source/**/*.ps1: # Localization GuidelinesRequirements
- Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages
- Use localized string keys, not hardcoded strings
- Assume
$script:localizedDatais availableString Files
- Commands/functions:
source/en-US/{MyModuleName}.strings.psd1- Class resources:
source/en-US/{ResourceClassName}.strings.psd1Key Naming Patterns
- Format:
Verb_FunctionName_Action(underscore separators), e.g.Get_Database_ConnectingToDatabaseString Format
ConvertFrom-StringData @' KeyName = Message with {0} placeholder. (PREFIX0001) '@String IDs
- Format:
(PREFIX####)- PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
- Number: Sequential from 0001
Usage
Write-Verbose -Message ($script:localizedData.KeyName -f $value1)
Files:
source/Examples/Resources/VMHyperV/7-TPMEnabled.ps1
tests/[Uu]nit/**/*.[Tt]ests.ps1
⚙️ CodeRabbit configuration file
tests/[Uu]nit/**/*.[Tt]ests.ps1: # Unit Tests Guidelines
- Test with localized strings: Use
InModuleScope -ScriptBlock { $script:localizedData.Key }- Mock files: Use
$TestDrivevariable (path to the test drive)- All public commands require parameter set validation tests
- After modifying classes, always run tests in new session (for changes to take effect)
Test Setup Requirements
Use this exact setup block before
Describe:[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Mock:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Should:ModuleName'] = $script:moduleName } AfterAll { $PSDefaultParameterValues.Remove('InModuleScope:ModuleName') $PSDefaultParameterValues.Remove('Mock:ModuleNam...
Files:
tests/Unit/DSC_VMHyperV.Tests.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings or creating an object using an internal class)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester ...
Files:
tests/Unit/DSC_VMHyperV.Tests.ps1
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format issue #<issue_number>
- No empty lines between list items in same section
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
source/DSCResources/**/*.psm1
⚙️ CodeRabbit configuration file
source/DSCResources/**/*.psm1: # MOF-based Desired State Configuration (DSC) Resources GuidelinesRequired Functions
- Every DSC resource must define:
Get-TargetResource,Set-TargetResource,Test-TargetResource- Export using
*-TargetResourcepatternFunction Return Types
Get-TargetResource: Must return hashtable with all resource propertiesTest-TargetResource: Must return boolean ($true/$false)Set-TargetResource: Must not return anything (void)Parameter Guidelines
Get-TargetResource: Only include parameters needed to retrieve actual current state valuesGet-TargetResource: Remove non-mandatory parameters that are never usedSet-TargetResourceandTest-TargetResource: Must have identical parametersSet-TargetResourceandTest-TargetResource: Unused mandatory parameters: Add "Not used in <function_name>" to help commentRequired Elements
- Each function must include
Write-Verboseat least once
Get-TargetResource: Use verbose message starting with "Getting the current state of..."Set-TargetResource: Use verbose message starting with "Setting the desired state of..."Test-TargetResource: Use verbose message starting with "Determining the current state of..."- Use localized strings for all messages (Write-Verbose, Write-Error, etc.)
- Import localized strings using
Get-LocalizedDataat module topError Handling
- Do not use
throwfor terminating errors- Use
try/catchblocks to handle exceptions- Throw localized exceptions using the appropriate
New-*Exceptioncmdlet:
New‑InvalidDataExceptionNew-ArgumentExceptionNew-InvalidOperationException- [
New-ObjectNotFoundException](https:/...
Files:
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
🧠 Learnings (32)
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Unit/{Classes,Public,Private}/*.Tests.ps1 : Add unit tests for all commands, functions, and resources
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Add integration tests for all public commands (and resources)
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : All public commands and class-based resources must have integration tests
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-10-12T11:23:30.123Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-10-12T11:23:30.123Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Call commands with -Force where applicable to avoid prompting
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-08-10T15:11:52.897Z
Learnt from: dan-hughes
Repo: dsccommunity/ActiveDirectoryDsc PR: 741
File: azure-pipelines.yml:104-104
Timestamp: 2025-08-10T15:11:52.897Z
Learning: In the ActiveDirectoryDsc project, HQRM (High Quality Resource Module) tests must run in PowerShell 5 (Windows PowerShell) using `pwsh: false` in the Azure Pipelines configuration, while unit tests can run in PowerShell 7 (PowerShell Core) with `pwsh: true`. This differentiation is intentional due to HQRM's specific requirements and dependencies on Windows PowerShell.
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-10-12T11:23:30.123Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-10-12T11:23:30.123Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Cover all scenarios and code paths in integration tests
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : All public commands must include parameter set validation tests
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Set $PSDefaultParameterValues entries for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Prefix variables used for mocks with 'mock'
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-08-18T13:50:53.789Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never use Assert-MockCalled; use Should -Invoke instead
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Test-TargetResource must return a boolean ($true/$false)
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Omit -MockWith when the mock should return $null
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-09-12T13:20:14.850Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-09-12T13:20:14.850Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : Implement [System.Boolean] Test() that calls ([ResourceBase] $this).Test() and returns the result
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
📚 Learning: 2025-09-25T16:38:08.867Z
Learnt from: johlju
Repo: dsccommunity/ActiveDirectoryDsc PR: 741
File: tests/Integration/MSFT_ADReadOnlyDomainControllerAccount.Integration.Tests.ps1:102-104
Timestamp: 2025-09-25T16:38:08.867Z
Learning: The PowerShell guideline "Use -BeTrue/-BeFalse; never use -Be $true/-Be $false" applies only when the actual return value is a boolean. When a cmdlet returns string values like 'True' or 'False', Should -Be 'True' is the appropriate assertion.
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Use the Parameter Properties template to assert a parameter is mandatory via $parameterInfo = (Get-Command -Name 'CommandName').Parameters['ParameterName']; $parameterInfo.Attributes.Mandatory | Should -BeTrue
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never add an empty -MockWith block
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use $PSDefaultParameterValues only for Pester commands (Describe, Context, It, Mock, Should, InModuleScope)
Applied to files:
tests/Unit/DSC_VMHyperV.Tests.ps1
📚 Learning: 2025-09-14T19:17:05.477Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-changelog.instructions.md:0-0
Timestamp: 2025-09-14T19:17:05.477Z
Learning: Applies to CHANGELOG.md : Describe notable changes briefly, with no more than 2 items per change type
Applied to files:
CHANGELOG.md
📚 Learning: 2025-09-23T17:13:51.341Z
Learnt from: Borgquite
Repo: dsccommunity/DnsServerDsc PR: 308
File: source/DSCResources/DSC_DnsServerSetting/DSC_DnsServerSetting.psm1:114-114
Timestamp: 2025-09-23T17:13:51.341Z
Learning: In DSC MOF schema files, [Write] properties include both read and write functionality by default. [Write] means the property can be configured and will be returned by Get-TargetResource. [Read] properties are read-only and cannot be configured but are returned for informational purposes.
Applied to files:
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.schema.mof
📚 Learning: 2025-09-23T17:13:51.341Z
Learnt from: Borgquite
Repo: dsccommunity/DnsServerDsc PR: 308
File: source/DSCResources/DSC_DnsServerSetting/DSC_DnsServerSetting.psm1:114-114
Timestamp: 2025-09-23T17:13:51.341Z
Learning: In DSC MOF schema files, [Write] properties inherently include both read and write functionality. [Write] means the property can be configured and will be returned by Get-TargetResource. [Read] properties are read-only informational properties that cannot be configured. There is no need for explicit [Read, Write] qualifiers because [Write] already includes read capability.
Applied to files:
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.schema.mof
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Set-TargetResource: Write-Verbose message should start with "Setting the desired state of..."
Applied to files:
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Every MOF DSC resource module must define the functions: Get-TargetResource, Set-TargetResource, and Test-TargetResource
Applied to files:
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Export resource functions using the *-TargetResource naming pattern
Applied to files:
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Each of Get-/Set-/Test-TargetResource must include at least one Write-Verbose call
Applied to files:
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Test-TargetResource: Write-Verbose message should start with "Determining the current state of..."
Applied to files:
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Set-TargetResource and Test-TargetResource must have identical parameters
Applied to files:
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Get-TargetResource parameters: include only those needed to retrieve actual current state values
Applied to files:
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Get-TargetResource: remove non-mandatory parameters that are never used
Applied to files:
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: dsccommunity.HyperVDsc (Build Package Module)
🔇 Additional comments (2)
source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1 (2)
618-647: Creation path TPM handling looks consistent withEnableTPMsemanticsIn the new-VM path you only touch TPM when
Generation -eq 2andEnableTPMis explicitly$true, and you prepare a non-default key protector before callingEnable-VMTPM. This matches the intended semantics (“default disabled; enable only on request”) and ensures the VM is off when TPM is enabled (since this is before anySet-VMStatecall).No issues here; just noting that this path is consistent with the fixed existing-VM logic above.
758-762:Test-TargetResource/Test-VMTpmEnabledintegration looks correct once naming is fixedA few quick observations on the test side of the resource:
Test-TargetResourcenow takes[System.Boolean] $EnableTPM = $falseand, for generation 2 VMs, compares it againstTest-VMTpmEnabled -Name $Name, returning$falseon mismatch. That matches the overall pattern used forSecureBoot.- The new helper
Test-VMTpmEnabledsimply doesGet-VMthenGet-VMSecurity.TpmEnabled, which is exactly what you use inGet-TargetResource.Once the schema/
Get-TargetResourcenaming is aligned (returningEnableTPMinstead ofTpmEnabled), this part of the implementation is consistent and doesn’t need structural changes.Also applies to: 941-946, 1071-1081
Comment on lines
49
to
51
| $vmSecureBootState = $false | ||
| $vmTPMState = $false | ||
| if ($vmobj.Generation -eq 2) |
There was a problem hiding this comment.
Return EnableTPM from Get-TargetResource instead of (or in addition to) TpmEnabled
This block correctly queries TPM state via Get-VMSecurity, but the hashtable below exposes it as TpmEnabled:
$vmTPMState = ($vmobj | Get-VMSecurity).TpmEnabled
…
TpmEnabled = $vmTPMStateThe public schema and Set/Test functions use EnableTPM as the configurable property name. To keep the resource contract consistent:
- Either change the key to
EnableTPM = $vmTPMStatesoGet-TargetResourcereturns the configured/writable property; or - If you explicitly want a separate read-only
TpmEnabledvalue, add it as a[Read]property in the schema and also returnEnableTPMhere.
Right now, the configured EnableTPM value will not appear in Get-DscConfiguration, and consumers will instead see an undocumented TpmEnabled property.
Based on learnings, …
Also applies to: 56-57, 82-99
🤖 Prompt for AI Agents
In source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1 around lines 49-51 (also
review 56-57 and 82-99), the TPM state is queried into $vmTPMState but the
Get-TargetResource output exposes it as TpmEnabled, which mismatches the
resource schema that uses EnableTPM; change the returned hashtable so it exposes
the configurable property name EnableTPM = $vmTPMState (you may optionally
retain TpmEnabled as an extra read-only value only if you also add a [Read]
TpmEnabled property to the module schema), and ensure any other returns in lines
56-57 and 82-99 are updated to consistently return EnableTPM so
Get-DscConfiguration shows the correct configurable property.
Comment on lines
+215
to
+218
| # Enable Trusted Platform Module for Generation 2 VMs | ||
| [Parameter()] | ||
| [System.Boolean] | ||
| $EnableTPM = $false, |
There was a problem hiding this comment.
Fix inverted TPM enable/disable logic in Set-TargetResource for existing VMs
The new EnableTPM parameter is added, but in the existing-VM path the logic that calls Enable-VMTPM vs Disable-VMTPM is currently reversed:
$vmTPMEnabled = Test-VMTpmEnabled -Name $Name
if ($EnableTPM -ne $vmTPMEnabled)
{
# ...
if (-not $EnableTPM)
{
# prepare key protector
$setVMPropertyParams = @{
VMCommand = 'Enable-VMTPM'
}
}
else
{
$setVMPropertyParams = @{
VMCommand = 'Disable-VMTPM'
}
}
Set-VMProperty @setVMPropertyParams
}If EnableTPM is $true and TPM is currently disabled, this will end up calling Disable-VMTPM, and vice versa.
You can correct this by making the EnableTPM $true branch handle the enable path (including key protector preparation), and the $false branch handle disable:
- # Retrive the current TPM state
- $vmTPMEnabled = Test-VMTpmEnabled -Name $Name
- if ($EnableTPM -ne $vmTPMEnabled)
- {
- Write-Verbose -Message ($script:localizedData.VMPropertyShouldBe -f 'TPMEnabled', $EnableTPM, $vmTPMEnabled)
-
- # Cannot change the TPM state whilst the VM is powered on.
- if (-not $EnableTPM)
- {
- # The default value for the key protector is 0,0,0,4
- $keyProtectorDefaultValue = @(0,0,0,4)
- # compare the default key protector value and the VM's key protector value
- $isVMKeyProtectorDefault = -not(Compare-Object -ReferenceObject (Get-VMKeyProtector -VMName $Name) -DifferenceObject $keyProtectorDefaultValue)
-
- # If the VM has a default key protector, we need to create a new one before enabling the TPM
- if ($isVMKeyProtectorDefault) {
- Set-VMKeyProtector -VMName $Name -NewLocalKeyProtector
- }
-
- $setVMPropertyParams = @{
- VMName = $Name
- VMCommand = 'Enable-VMTPM'
- RestartIfNeeded = $RestartIfNeeded
- }
- }
- else
- {
- $setVMPropertyParams = @{
- VMName = $Name
- VMCommand = 'Disable-VMTPM'
- RestartIfNeeded = $RestartIfNeeded
- }
- }
+ # Retrive the current TPM state
+ $vmTPMEnabled = Test-VMTpmEnabled -Name $Name
+ if ($EnableTPM -ne $vmTPMEnabled)
+ {
+ Write-Verbose -Message ($script:localizedData.VMPropertyShouldBe -f 'TPMEnabled', $EnableTPM, $vmTPMEnabled)
+
+ # Cannot change the TPM state whilst the VM is powered on.
+ if ($EnableTPM)
+ {
+ # The default value for the key protector is 0,0,0,4
+ $keyProtectorDefaultValue = @(0,0,0,4)
+
+ # Compare the default key protector value and the VM's key protector value.
+ $isVMKeyProtectorDefault = -not (Compare-Object -ReferenceObject (Get-VMKeyProtector -VMName $Name) -DifferenceObject $keyProtectorDefaultValue)
+
+ # If the VM has a default key protector, create a new one before enabling TPM.
+ if ($isVMKeyProtectorDefault)
+ {
+ Set-VMKeyProtector -VMName $Name -NewLocalKeyProtector
+ }
+
+ $setVMPropertyParams = @{
+ VMName = $Name
+ VMCommand = 'Enable-VMTPM'
+ RestartIfNeeded = $RestartIfNeeded
+ }
+ }
+ else
+ {
+ $setVMPropertyParams = @{
+ VMName = $Name
+ VMCommand = 'Disable-VMTPM'
+ RestartIfNeeded = $RestartIfNeeded
+ }
+ }
@@
- Set-VMProperty @setVMPropertyParams
- Write-Verbose -Message ($script:localizedData.VMPropertySet -f 'TPMEnabled', $EnableTPM)
+ Set-VMProperty @setVMPropertyParams
+ Write-Verbose -Message ($script:localizedData.VMPropertySet -f 'TPMEnabled', $EnableTPM)Without this change, any attempt to enable TPM on an existing VM will actually disable it, and vice versa.
Also applies to: 440-476
🤖 Prompt for AI Agents
In source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1 around lines 215-218 (and
also apply same fix to lines ~440-476), the existing-VM branch in
Set-TargetResource inverts EnableTPM logic: when $EnableTPM differs from current
state it selects the wrong VMCommand. Change the conditional so the $EnableTPM
-eq $true branch prepares the key protector and sets VMCommand to
'Enable-VMTPM', and the $EnableTPM -eq $false branch sets VMCommand to
'Disable-VMTPM'; keep the surrounding Test-VMTpmEnabled check and call to
Set-VMProperty unchanged.
| [Write, Description("Specifies if the VM should be Present (created) or Absent (removed). The default value is `Present`."), ValueMap{"Present","Absent"}, Values{"Present","Absent"}] String Ensure; | ||
| [Write, Description("Notes about the VM.")] String Notes; | ||
| [Write, Description("Specifies if Secure Boot should be enabled for Generation 2 virtual machines. **Only supports generation 2 virtual machines**. Default value is `$true`.")] Boolean SecureBoot; | ||
| [Write, Description("Specifies if Trusted Platform Module (TPM) should be enabled for Generation 2 virtual machines. **Only supports generation 2 virtual machines**. Default value is `$false`.")] Boolean EnableTPM; |
There was a problem hiding this comment.
Align EnableTPM schema property with Get-TargetResource output
You’ve added a writable EnableTPM property in the schema, but Get-TargetResource currently returns a key named TpmEnabled rather than EnableTPM. For MOF resources, all [Write] properties should be present in the hashtable returned from Get-TargetResource; otherwise Get-DscConfiguration won’t surface EnableTPM as a configured property.
Consider either:
- Returning
EnableTPM = $vmTPMStatefromGet-TargetResource(replacingTpmEnabled), or - Declaring an additional
[Read]Boolean TpmEnabled;in the schema if you want to expose both desired (EnableTPM) and actual (TpmEnabled) TPM state.
Based on learnings, …
🤖 Prompt for AI Agents
In source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.schema.mof around line 20, the
writable schema property EnableTPM does not match the key returned by
Get-TargetResource (which currently returns TpmEnabled), so Get-DscConfiguration
won't report EnableTPM; fix by either updating Get-TargetResource to return
EnableTPM = $vmTPMState in the hashtable (replacing TpmEnabled) so the writeable
property is present, or alternatively add a Read-only Boolean TpmEnabled;
declaration to the MOF schema to explicitly expose the actual TPM state while
keeping EnableTPM as the desired state.
Comment on lines
481
to
485
| It 'Returns $true when VM .vhdx file is specified with a generation 2 VM' { | ||
| Mock -CommandName Test-VMSecureBoot -MockWith { return $true } | ||
| Mock -CommandName Test-VMTpmEnabled -MockWith { return $false } | ||
| Test-TargetResource -Name 'Generation2VM' -Generation 2 @testParams | Should -Be $true | ||
| } |
There was a problem hiding this comment.
Fix TPM-related tests: wrong parameter name, wrong helper, and inverted expectations
The TPM tests in this context have several concrete issues:
Test-TargetResourcedoes not have a-TpmEnabledparameter; the implementation and schema useEnableTPM. Calling-TpmEnabledwill throw a parameter binding error.- In the second test,
TpmEnabled $falseis missing the-and will be treated as positional arguments, conflicting with the splatted@testParams(duplicateVhdPath) and failing at runtime. - The implementation uses
Test-VMTpmEnabled, but the tests mockTest-VMSecurity, so they don’t exercise the actual helper. - The descriptions and expected results are inverted relative to the code: when
$EnableTPMand the actual TPM state differ,Test-TargetResourcereturns$false, not$true.
A minimal fix that aligns the tests with the implementation and semantics would be:
It 'Returns $true when VM .vhdx file is specified with a generation 2 VM' {
Mock -CommandName Test-VMSecureBoot -MockWith { return $true }
- Mock -CommandName Test-VMTpmEnabled -MockWith { return $false }
+ Mock -CommandName Test-VMTpmEnabled -MockWith { return $false }
Test-TargetResource -Name 'Generation2VM' -Generation 2 @testParams | Should -Be $true
}
@@
- It 'Returns $true when TpmEnabled is disabled and requested "TpmEnabled" = "$true"' {
- Mock -CommandName Test-VMSecurity -MockWith { return $false }
- Test-TargetResource -Name 'Generation2VM' -TpmEnabled $true -Generation 2 @testParams | Should -Be $true
- }
-
- It 'Returns $false when TpmEnabled is disabled and requested "TpmEnabled" = "$false"' {
- Mock -CommandName Test-VMSecurity -MockWith { return $false }
- Test-TargetResource -Name 'Generation2VM' TpmEnabled $false -Generation 2 @testParams | Should -Be $false
- }
+ It 'Returns $true when TPM is enabled and requested "EnableTPM" = "$true"' {
+ Mock -CommandName Test-VMTpmEnabled -MockWith { return $true }
+
+ Test-TargetResource -Name 'Generation2VM' -EnableTPM $true -Generation 2 @testParams |
+ Should -BeTrue
+ }
+
+ It 'Returns $false when TPM is disabled and requested "EnableTPM" = "$true"' {
+ Mock -CommandName Test-VMTpmEnabled -MockWith { return $false }
+
+ Test-TargetResource -Name 'Generation2VM' -EnableTPM $true -Generation 2 @testParams |
+ Should -BeFalse
+ }This ensures:
- The correct parameter (
EnableTPM) is used. - The correct helper (
Test-VMTpmEnabled) is mocked. - The expectations match
Test-TargetResource’s behavior (return$trueonly when desired and actual TPM states match).
As per coding guidelines, …
Also applies to: 522-530
🤖 Prompt for AI Agents
In tests/Unit/DSC_VMHyperV.Tests.ps1 around lines 481-485 (and similarly
522-530), the TPM-related tests use the wrong parameter name and helper and have
inverted expectations; update the tests to call Test-TargetResource with the
correct named parameter -EnableTPM (not -TpmEnabled and ensure the boolean is
prefixed with -), mock the actual helper Test-VMTpmEnabled (not
Test-VMSecurity), avoid passing positional args that collide with @testParams,
and flip the assertions so Test-TargetResource returns $true only when the
desired -EnableTPM value matches the mocked Test-VMTpmEnabled result (i.e., when
both true or both false) and $false when they differ.
Member
|
@jmerriweather look through coderabbitai comments, the first two I read was relevant to resolve, please look at the others as well. |
johlju
added
waiting for code fix
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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
This adds the ability to enable or disable the TPM on a virtual machine
This Pull Request (PR) fixes the following issues
Task list
Entry should say what was changed and how that affects users (if applicable), and
reference the issue being resolved (if applicable).
This change is