Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ For older change log history see the [historic changelog](HISTORIC_CHANGELOG.md)
- Fix multiple DNS IP adresses does not work #190
- NetworkSetting parameter is now optional and no default actions are taken if not specified
- Switch to use VM image `windows-latest` to build phase.
- Added support to enable or disable the TPM on a virtual machine

## [3.18.0] - 2022-06-04

Expand Down
92 changes: 92 additions & 0 deletions source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,17 @@ function Get-TargetResource
}

$vmSecureBootState = $false
$vmTPMState = $false
if ($vmobj.Generation -eq 2)
Comment on lines 49 to 51
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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 = $vmTPMState

The 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 = $vmTPMState so Get-TargetResource returns the configured/writable property; or
  • If you explicitly want a separate read-only TpmEnabled value, add it as a [Read] property in the schema and also return EnableTPM here.

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.

{
# Retrieve secure boot status (can only be enabled on Generation 2 VMs) and convert to a boolean.
$vmSecureBootState = ($vmobj | Get-VMFirmware).SecureBoot -eq 'On'

# Retrieve TPM status (can only be enabled on Generation 2 VMs) and return boolean.
$vmTPMState = ($vmobj | Get-VMSecurity).TpmEnabled
}


$guestServiceId = 'Microsoft:{0}\6C09BB55-D683-4DA0-8931-C9BF705F6480' -f $vmObj.Id

$macAddress = @()
Expand Down Expand Up @@ -90,6 +95,7 @@ function Get-TargetResource
Path = $vmobj.Path
Generation = $vmobj.Generation
SecureBoot = $vmSecureBootState
TpmEnabled = $vmTPMState
StartupMemory = $vmobj.MemoryStartup
MinimumMemory = $vmobj.MemoryMinimum
MaximumMemory = $vmobj.MemoryMaximum
Expand Down Expand Up @@ -206,6 +212,11 @@ function Set-TargetResource
[System.Boolean]
$SecureBoot = $true,

# Enable Trusted Platform Module for Generation 2 VMs
[Parameter()]
[System.Boolean]
$EnableTPM = $false,
Comment on lines +215 to +218
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.


# Enable Guest Services
[Parameter()]
[System.Boolean]
Expand Down Expand Up @@ -425,6 +436,44 @@ function Set-TargetResource
Set-VMProperty @setVMPropertyParams
Write-Verbose -Message ($script:localizedData.VMPropertySet -f 'SecureBoot', $SecureBoot)
}

# 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
}
}

Set-VMProperty @setVMPropertyParams
Write-Verbose -Message ($script:localizedData.VMPropertySet -f 'TPMEnabled', $EnableTPM)
}
}

if ($Notes -ne $null)
Expand Down Expand Up @@ -576,6 +625,25 @@ function Set-TargetResource
{
Set-VMFirmware -VMName $Name -EnableSecureBoot Off
}

<#
TPM is only applicable to Generation 2 VMs and it defaults to disabled.
Therefore, we only need to explicitly set it to enabled if specified.
#>
if ($EnableTPM -eq $true)
{
# 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
}

Enable-VMTPM -VMName $Name
}
}

if ($EnableGuestService)
Expand Down Expand Up @@ -687,6 +755,11 @@ function Test-TargetResource
[System.Boolean]
$SecureBoot = $true,

# Enable Trusted Platform Module for Generation 2 VMs
[Parameter()]
[System.Boolean]
$EnableTPM = $false,

[Parameter()]
[System.Boolean]
$EnableGuestService = $false,
Expand Down Expand Up @@ -864,6 +937,13 @@ function Test-TargetResource
Write-Verbose -Message ($script:localizedData.VMPropertyShouldBe -f 'SecureBoot', $SecureBoot, $vmSecureBoot)
return $false
}

$vmTPMEnabled = Test-VMTpmEnabled -Name $Name
if ($EnableTPM -ne $vmTPMEnabled)
{
Write-Verbose -Message ($script:localizedData.VMPropertyShouldBe -f 'TPMEnabled', $EnableTPM, $vmTPMEnabled)
return $false
}
}

$guestServiceId = 'Microsoft:{0}\6C09BB55-D683-4DA0-8931-C9BF705F6480' -f $vmObj.Id
Expand Down Expand Up @@ -988,6 +1068,18 @@ function Test-VMSecureBoot
return (Get-VMFirmware -VM $vm).SecureBoot -eq 'On'
}

function Test-VMTpmEnabled
{
param
(
[Parameter(Mandatory = $true)]
[System.String]
$Name
)
$vm = Get-VM -Name $Name
return (Get-VMSecurity -VM $vm).TpmEnabled
}

#endregion

Export-ModuleMember -Function *-TargetResource
1 change: 1 addition & 0 deletions source/DSCResources/DSC_VMHyperV/DSC_VMHyperV.schema.mof
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class DSC_VMHyperV : OMI_BaseResource
[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;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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 = $vmTPMState from Get-TargetResource (replacing TpmEnabled), 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.

[Write, Description("Enable Guest Service Interface for the VM. The default value is `$false`.")] Boolean EnableGuestService;
[Write, Description("Enable AutomaticCheckpoints for the VM.")] Boolean AutomaticCheckpointsEnabled;
[Read, Description("Returns the unique ID for the VM.")] String ID;
Expand Down
43 changes: 43 additions & 0 deletions source/Examples/Resources/VMHyperV/7-TPMEnabled.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<#
.DESCRIPTION
Create a new VM.
#>
configuration Example
{
param
(
[System.String[]]
$NodeName = 'localhost',

[Parameter(Mandatory = $true)]
[System.String]
$VMName,

[Parameter(Mandatory = $true)]
[System.String]
$VhdPath
)

Import-DscResource -ModuleName 'HyperVDsc'

Node $NodeName
{
# Install HyperV feature, if not installed - Server SKU only
WindowsFeature HyperV
{
Ensure = 'Present'
Name = 'Hyper-V'
}

# Ensures a VM with default settings
VMHyperV NewVM
{
Ensure = 'Present'
Name = $VMName
VhdPath = $VhdPath
Generation = 2
EnableTPM = $true
DependsOn = '[WindowsFeature]HyperV'
}
}
}
17 changes: 17 additions & 0 deletions tests/Unit/DSC_VMHyperV.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,12 @@ try
Assert-MockCalled -CommandName Get-VMFirmware -Scope It -Exactly 1
}

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 'Hash table contains key EnableGuestService' {
$targetResource = Get-TargetResource -Name 'RunningVM' -VhdPath $stubVhdxDisk.Path
$targetResource.ContainsKey('EnableGuestService') | Should -Be $true
Expand Down Expand Up @@ -474,6 +480,7 @@ try

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
}
Comment on lines 481 to 485
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix TPM-related tests: wrong parameter name, wrong helper, and inverted expectations

The TPM tests in this context have several concrete issues:

  • Test-TargetResource does not have a -TpmEnabled parameter; the implementation and schema use EnableTPM. Calling -TpmEnabled will throw a parameter binding error.
  • In the second test, TpmEnabled $false is missing the - and will be treated as positional arguments, conflicting with the splatted @testParams (duplicate VhdPath) and failing at runtime.
  • The implementation uses Test-VMTpmEnabled, but the tests mock Test-VMSecurity, so they don’t exercise the actual helper.
  • The descriptions and expected results are inverted relative to the code: when $EnableTPM and the actual TPM state differ, Test-TargetResource returns $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 $true only 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.


Expand Down Expand Up @@ -512,6 +519,16 @@ try
Test-TargetResource -Name 'Generation2VM' -SecureBoot $false -Generation 2 @testParams | Should -Be $false
}

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 VM has snapshot chain' {
Mock -CommandName Get-VhdHierarchy -MockWith {
return @($studVhdxDiskSnapshot, $stubVhdxDisk)
Expand Down