From 72156b393b6b17c4a89b757c10ee1969755845ad Mon Sep 17 00:00:00 2001 From: Damiano Donati Date: Thu, 16 Oct 2025 15:43:30 +0200 Subject: [PATCH 1/3] vSphere: add VirtualTPM support --- pkg/controller/vsphere/reconciler.go | 48 ++++++++++++ pkg/controller/vsphere/reconciler_test.go | 96 +++++++++++++++++++++++ 2 files changed, 144 insertions(+) diff --git a/pkg/controller/vsphere/reconciler.go b/pkg/controller/vsphere/reconciler.go index 74240b3b73..ed8da70dfd 100644 --- a/pkg/controller/vsphere/reconciler.go +++ b/pkg/controller/vsphere/reconciler.go @@ -1015,6 +1015,13 @@ func clone(s *machineScope) (string, error) { deviceSpecs = append(deviceSpecs, networkDevices...) + // Add VirtualTPM device if needed (TODO: placeholder for future provider spec field) + tpmDevices, err := getVirtualTPMDevices(s, devices) + if err != nil { + return "", fmt.Errorf("error getting VirtualTPM specs: %w", err) + } + deviceSpecs = append(deviceSpecs, tpmDevices...) + extraConfig := []types.BaseOptionValue{} extraConfig = append(extraConfig, IgnitionConfig(userData)...) @@ -1417,6 +1424,47 @@ func getNetworkDevices(s *machineScope, resourcepool *object.ResourcePool, devic return networkDevices, nil } +// getVirtualTPMDevices creates VirtualTPM device specifications for the VM. +// This function is prepared for future integration with a VirtualTPM field in VSphereMachineProviderSpec. +func getVirtualTPMDevices(s *machineScope, devices object.VirtualDeviceList) ([]types.BaseVirtualDeviceConfigSpec, error) { + var tpmDevices []types.BaseVirtualDeviceConfigSpec + + // TODO: Replace this placeholder with actual provider spec field check + // When VSphereMachineProviderSpec.VirtualTPM field is added, replace the condition below: + // if s.providerSpec.VirtualTPM != nil && s.providerSpec.VirtualTPM.Enabled { + + // For now, this is a placeholder that doesn't add TPM devices + // Set this to true to test VirtualTPM functionality + enableVirtualTPM := true + + if enableVirtualTPM { + // Check if TPM already exists in template + existingTPMs := devices.SelectByType((*types.VirtualTPM)(nil)) + if len(existingTPMs) > 0 { + klog.V(3).Infof("VirtualTPM already exists in template, skipping addition") + return tpmDevices, nil + } + + // Create new VirtualTPM device + tpmDevice := &types.VirtualTPM{ + VirtualDevice: types.VirtualDevice{ + Key: devices.NewKey(), + // Note: VirtualTPM doesn't require backing info or controller key + // as it's a standalone security device + }, + } + + tpmDevices = append(tpmDevices, &types.VirtualDeviceConfigSpec{ + Device: tpmDevice, + Operation: types.VirtualDeviceConfigSpecOperationAdd, + }) + + klog.V(2).Infof("Adding VirtualTPM device to VM %s", s.machine.GetName()) + } + + return tpmDevices, nil +} + func newVMFlagInfo() *types.VirtualMachineFlagInfo { diskUUIDEnabled := true return &types.VirtualMachineFlagInfo{ diff --git a/pkg/controller/vsphere/reconciler_test.go b/pkg/controller/vsphere/reconciler_test.go index b60252f583..76b44f1cbf 100644 --- a/pkg/controller/vsphere/reconciler_test.go +++ b/pkg/controller/vsphere/reconciler_test.go @@ -1419,6 +1419,102 @@ func TestCreateDataDisks(t *testing.T) { } } +func TestGetVirtualTPMDevices(t *testing.T) { + model, session, server := initSimulator(t) + t.Cleanup(model.Remove) + t.Cleanup(server.Close) + vm := simulator.Map.Any("VirtualMachine").(*simulator.VirtualMachine) + machine := object.NewVirtualMachine(session.Client.Client, vm.Reference()) + + deviceList, err := machine.Device(context.TODO()) + if err != nil { + t.Fatalf("Failed to obtain vm devices: %v", err) + } + + getMachineScope := func() *machineScope { + gates, _ := testutils.NewDefaultMutableFeatureGate() + return &machineScope{ + Context: context.TODO(), + machine: &machinev1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-tpm", + Namespace: "test", + Labels: map[string]string{ + machinev1.MachineClusterIDLabel: "CLUSTERID", + }, + }, + }, + providerSpec: &machinev1.VSphereMachineProviderSpec{}, + session: session, + providerStatus: &machinev1.VSphereMachineProviderStatus{}, + featureGates: gates, + } + } + + testCases := []struct { + name string + devices object.VirtualDeviceList + expectedDeviceCount int + expectedError string + }{ + { + name: "No existing TPM devices", + devices: deviceList, + expectedDeviceCount: 0, // Currently disabled by default + }, + { + name: "Empty device list", + devices: object.VirtualDeviceList{}, + expectedDeviceCount: 0, + }, + } + + for _, test := range testCases { + tc := test + t.Run(tc.name, func(t *testing.T) { + scope := getMachineScope() + + // Call the function under test + tpmDevices, err := getVirtualTPMDevices(scope, tc.devices) + + // Check for expected errors + if tc.expectedError != "" { + if err == nil || err.Error() != tc.expectedError { + t.Fatalf("Expected error '%s', got: %v", tc.expectedError, err) + } + return + } + + // Check for unexpected errors + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + // Check device count + if len(tpmDevices) != tc.expectedDeviceCount { + t.Fatalf("Expected %d TPM devices, got %d", tc.expectedDeviceCount, len(tpmDevices)) + } + + // If devices were created, validate their structure + for _, deviceSpec := range tpmDevices { + spec := deviceSpec.(*types.VirtualDeviceConfigSpec) + if spec.Operation != types.VirtualDeviceConfigSpecOperationAdd { + t.Fatalf("Expected operation to be Add, got %v", spec.Operation) + } + + tpmDevice, ok := spec.Device.(*types.VirtualTPM) + if !ok { + t.Fatalf("Expected device to be VirtualTPM, got %T", spec.Device) + } + + if tpmDevice.Key == 0 { + t.Fatalf("Expected TPM device to have a non-zero key") + } + } + }) + } +} + func createAdditionalDisks(devices object.VirtualDeviceList, controller types.BaseVirtualController, numOfDisks int) object.VirtualDeviceList { deviceList := devices disks := devices.SelectByType((*types.VirtualDisk)(nil)) From 755c3e833b73a79b88b43e562a8ba5ce9fa2ca63 Mon Sep 17 00:00:00 2001 From: Damiano Donati Date: Fri, 17 Oct 2025 09:06:29 +0200 Subject: [PATCH 2/3] wip --- pkg/controller/vsphere/reconciler.go | 21 ++++++++++----------- pkg/controller/vsphere/reconciler_test.go | 21 ++++++++------------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/pkg/controller/vsphere/reconciler.go b/pkg/controller/vsphere/reconciler.go index ed8da70dfd..cdb2f9ac84 100644 --- a/pkg/controller/vsphere/reconciler.go +++ b/pkg/controller/vsphere/reconciler.go @@ -1016,11 +1016,11 @@ func clone(s *machineScope) (string, error) { deviceSpecs = append(deviceSpecs, networkDevices...) // Add VirtualTPM device if needed (TODO: placeholder for future provider spec field) - tpmDevices, err := getVirtualTPMDevices(s, devices) + tpmDeviceSpec, err := addVirtualTPMDevice(s, devices) if err != nil { return "", fmt.Errorf("error getting VirtualTPM specs: %w", err) } - deviceSpecs = append(deviceSpecs, tpmDevices...) + deviceSpecs = append(deviceSpecs, tpmDeviceSpec) extraConfig := []types.BaseOptionValue{} @@ -1424,17 +1424,16 @@ func getNetworkDevices(s *machineScope, resourcepool *object.ResourcePool, devic return networkDevices, nil } -// getVirtualTPMDevices creates VirtualTPM device specifications for the VM. +// addVirtualTPMDevice creates VirtualTPM device specifications for the VM. // This function is prepared for future integration with a VirtualTPM field in VSphereMachineProviderSpec. -func getVirtualTPMDevices(s *machineScope, devices object.VirtualDeviceList) ([]types.BaseVirtualDeviceConfigSpec, error) { - var tpmDevices []types.BaseVirtualDeviceConfigSpec +func addVirtualTPMDevice(s *machineScope, devices object.VirtualDeviceList) (types.BaseVirtualDeviceConfigSpec, error) { + var baseVirtualDeviceConfigSpec types.BaseVirtualDeviceConfigSpec // TODO: Replace this placeholder with actual provider spec field check // When VSphereMachineProviderSpec.VirtualTPM field is added, replace the condition below: // if s.providerSpec.VirtualTPM != nil && s.providerSpec.VirtualTPM.Enabled { - // For now, this is a placeholder that doesn't add TPM devices - // Set this to true to test VirtualTPM functionality + // For now, this is a placeholder that hardcodes adding a TPM device. enableVirtualTPM := true if enableVirtualTPM { @@ -1442,7 +1441,7 @@ func getVirtualTPMDevices(s *machineScope, devices object.VirtualDeviceList) ([] existingTPMs := devices.SelectByType((*types.VirtualTPM)(nil)) if len(existingTPMs) > 0 { klog.V(3).Infof("VirtualTPM already exists in template, skipping addition") - return tpmDevices, nil + return baseVirtualDeviceConfigSpec, nil } // Create new VirtualTPM device @@ -1454,15 +1453,15 @@ func getVirtualTPMDevices(s *machineScope, devices object.VirtualDeviceList) ([] }, } - tpmDevices = append(tpmDevices, &types.VirtualDeviceConfigSpec{ + baseVirtualDeviceConfigSpec = &types.VirtualDeviceConfigSpec{ Device: tpmDevice, Operation: types.VirtualDeviceConfigSpecOperationAdd, - }) + } klog.V(2).Infof("Adding VirtualTPM device to VM %s", s.machine.GetName()) } - return tpmDevices, nil + return baseVirtualDeviceConfigSpec, nil } func newVMFlagInfo() *types.VirtualMachineFlagInfo { diff --git a/pkg/controller/vsphere/reconciler_test.go b/pkg/controller/vsphere/reconciler_test.go index 76b44f1cbf..cc5242e049 100644 --- a/pkg/controller/vsphere/reconciler_test.go +++ b/pkg/controller/vsphere/reconciler_test.go @@ -1475,7 +1475,7 @@ func TestGetVirtualTPMDevices(t *testing.T) { scope := getMachineScope() // Call the function under test - tpmDevices, err := getVirtualTPMDevices(scope, tc.devices) + tpmDeviceSpec, err := addVirtualTPMDevice(scope, tc.devices) // Check for expected errors if tc.expectedError != "" { @@ -1491,25 +1491,20 @@ func TestGetVirtualTPMDevices(t *testing.T) { } // Check device count - if len(tpmDevices) != tc.expectedDeviceCount { - t.Fatalf("Expected %d TPM devices, got %d", tc.expectedDeviceCount, len(tpmDevices)) + if tc.expectedDeviceCount != 1 { + t.Fatalf("Expected %d TPM devices, got %d", tc.expectedDeviceCount, 1) } // If devices were created, validate their structure - for _, deviceSpec := range tpmDevices { - spec := deviceSpec.(*types.VirtualDeviceConfigSpec) + if tpmDeviceSpec != nil { + spec := tpmDeviceSpec.(*types.VirtualDeviceConfigSpec) if spec.Operation != types.VirtualDeviceConfigSpecOperationAdd { t.Fatalf("Expected operation to be Add, got %v", spec.Operation) } + } - tpmDevice, ok := spec.Device.(*types.VirtualTPM) - if !ok { - t.Fatalf("Expected device to be VirtualTPM, got %T", spec.Device) - } - - if tpmDevice.Key == 0 { - t.Fatalf("Expected TPM device to have a non-zero key") - } + if tpmDeviceSpec == nil && tc.expectedDeviceCount != 0 { + t.Fatalf("Expected %d TPM devices, got %d", tc.expectedDeviceCount, 0) } }) } From 237a843abb59e8715187bfe76a0d1b2619f3800b Mon Sep 17 00:00:00 2001 From: Damiano Donati Date: Fri, 17 Oct 2025 10:15:08 +0200 Subject: [PATCH 3/3] wip --- pkg/controller/vsphere/reconciler.go | 107 ++++++++++++---------- pkg/controller/vsphere/reconciler_test.go | 28 ++++-- 2 files changed, 80 insertions(+), 55 deletions(-) diff --git a/pkg/controller/vsphere/reconciler.go b/pkg/controller/vsphere/reconciler.go index cdb2f9ac84..3f96b749d8 100644 --- a/pkg/controller/vsphere/reconciler.go +++ b/pkg/controller/vsphere/reconciler.go @@ -1015,12 +1015,22 @@ func clone(s *machineScope) (string, error) { deviceSpecs = append(deviceSpecs, networkDevices...) - // Add VirtualTPM device if needed (TODO: placeholder for future provider spec field) - tpmDeviceSpec, err := addVirtualTPMDevice(s, devices) - if err != nil { - return "", fmt.Errorf("error getting VirtualTPM specs: %w", err) + // TODO: Replace this placeholder with actual provider spec field check + // When VSphereMachineProviderSpec.VirtualTPM field is added, replace the condition below: + // if s.providerSpec.VirtualTPM != nil && s.providerSpec.VirtualTPM.Enabled { + // For now, this is a placeholder that hardcodes adding a TPM device. + enableVirtualTPM := true + + if enableVirtualTPM { + // Add VirtualTPM device if needed (TODO: placeholder for future provider spec field) + tpmDeviceSpec, err := addVirtualTPMDevice(s, devices) + if err != nil { + return "", fmt.Errorf("error getting VirtualTPM specs: %w", err) + } + if tpmDeviceSpec != nil { + deviceSpecs = append(deviceSpecs, tpmDeviceSpec) + } } - deviceSpecs = append(deviceSpecs, tpmDeviceSpec) extraConfig := []types.BaseOptionValue{} @@ -1047,20 +1057,24 @@ func clone(s *machineScope) (string, error) { } } + // Create the VM configuration spec + vmConfigSpec := &types.VirtualMachineConfigSpec{ + Annotation: s.machine.GetName(), + // Assign the clone's InstanceUUID the value of the Kubernetes Machine + // object's UID. This allows lookup of the cloned VM prior to knowing + // the VM's UUID. + InstanceUuid: string(s.machine.UID), + Flags: newVMFlagInfo(), + ExtraConfig: extraConfig, + DeviceChange: deviceSpecs, + NumCPUs: numCPUs, + NumCoresPerSocket: numCoresPerSocket, + MemoryMB: s.providerSpec.MemoryMiB, + Firmware: getVMFirmware(enableVirtualTPM), + } + spec := types.VirtualMachineCloneSpec{ - Config: &types.VirtualMachineConfigSpec{ - Annotation: s.machine.GetName(), - // Assign the clone's InstanceUUID the value of the Kubernetes Machine - // object's UID. This allows lookup of the cloned VM prior to knowing - // the VM's UUID. - InstanceUuid: string(s.machine.UID), - Flags: newVMFlagInfo(), - ExtraConfig: extraConfig, - DeviceChange: deviceSpecs, - NumCPUs: numCPUs, - NumCoresPerSocket: numCoresPerSocket, - MemoryMB: s.providerSpec.MemoryMiB, - }, + Config: vmConfigSpec, Location: types.VirtualMachineRelocateSpec{ Datastore: types.NewReference(datastore.Reference()), Folder: types.NewReference(folder.Reference()), @@ -1428,40 +1442,39 @@ func getNetworkDevices(s *machineScope, resourcepool *object.ResourcePool, devic // This function is prepared for future integration with a VirtualTPM field in VSphereMachineProviderSpec. func addVirtualTPMDevice(s *machineScope, devices object.VirtualDeviceList) (types.BaseVirtualDeviceConfigSpec, error) { var baseVirtualDeviceConfigSpec types.BaseVirtualDeviceConfigSpec + // Check if TPM already exists in template + existingTPMs := devices.SelectByType((*types.VirtualTPM)(nil)) + if len(existingTPMs) > 0 { + klog.V(3).Infof("VirtualTPM already exists in template, skipping addition") + return baseVirtualDeviceConfigSpec, nil + } + + // Create new VirtualTPM device + tpmDevice := &types.VirtualTPM{ + VirtualDevice: types.VirtualDevice{ + Key: devices.NewKey(), + // Note: VirtualTPM doesn't require backing info or controller key + // as it's a standalone security device + }, + } - // TODO: Replace this placeholder with actual provider spec field check - // When VSphereMachineProviderSpec.VirtualTPM field is added, replace the condition below: - // if s.providerSpec.VirtualTPM != nil && s.providerSpec.VirtualTPM.Enabled { - - // For now, this is a placeholder that hardcodes adding a TPM device. - enableVirtualTPM := true - - if enableVirtualTPM { - // Check if TPM already exists in template - existingTPMs := devices.SelectByType((*types.VirtualTPM)(nil)) - if len(existingTPMs) > 0 { - klog.V(3).Infof("VirtualTPM already exists in template, skipping addition") - return baseVirtualDeviceConfigSpec, nil - } + baseVirtualDeviceConfigSpec = &types.VirtualDeviceConfigSpec{ + Device: tpmDevice, + Operation: types.VirtualDeviceConfigSpecOperationAdd, + } - // Create new VirtualTPM device - tpmDevice := &types.VirtualTPM{ - VirtualDevice: types.VirtualDevice{ - Key: devices.NewKey(), - // Note: VirtualTPM doesn't require backing info or controller key - // as it's a standalone security device - }, - } + klog.V(2).Infof("Adding VirtualTPM device to VM %s", s.machine.GetName()) - baseVirtualDeviceConfigSpec = &types.VirtualDeviceConfigSpec{ - Device: tpmDevice, - Operation: types.VirtualDeviceConfigSpecOperationAdd, - } + return baseVirtualDeviceConfigSpec, nil +} - klog.V(2).Infof("Adding VirtualTPM device to VM %s", s.machine.GetName()) +// getVMFirmware returns the VM firmware type based on certain factors. +func getVMFirmware(enableVirtualTPM bool) string { + if enableVirtualTPM { + // VirtualTPM requires EFI firmware. + return string(types.GuestOsDescriptorFirmwareTypeEfi) } - - return baseVirtualDeviceConfigSpec, nil + return string(types.GuestOsDescriptorFirmwareTypeBios) } func newVMFlagInfo() *types.VirtualMachineFlagInfo { diff --git a/pkg/controller/vsphere/reconciler_test.go b/pkg/controller/vsphere/reconciler_test.go index cc5242e049..d70a8db166 100644 --- a/pkg/controller/vsphere/reconciler_test.go +++ b/pkg/controller/vsphere/reconciler_test.go @@ -1419,6 +1419,10 @@ func TestCreateDataDisks(t *testing.T) { } } +func TestGetVMFirmware(t *testing.T) { + // TODO +} + func TestGetVirtualTPMDevices(t *testing.T) { model, session, server := initSimulator(t) t.Cleanup(model.Remove) @@ -1460,12 +1464,12 @@ func TestGetVirtualTPMDevices(t *testing.T) { { name: "No existing TPM devices", devices: deviceList, - expectedDeviceCount: 0, // Currently disabled by default + expectedDeviceCount: 1, // TPM is enabled by default now }, { name: "Empty device list", devices: object.VirtualDeviceList{}, - expectedDeviceCount: 0, + expectedDeviceCount: 1, // TPM is enabled by default now }, } @@ -1490,9 +1494,12 @@ func TestGetVirtualTPMDevices(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } - // Check device count - if tc.expectedDeviceCount != 1 { - t.Fatalf("Expected %d TPM devices, got %d", tc.expectedDeviceCount, 1) + // Check device count - if we expect a device, tpmDeviceSpec should not be nil + if tc.expectedDeviceCount > 0 && tpmDeviceSpec == nil { + t.Fatalf("Expected %d TPM devices, got nil device spec", tc.expectedDeviceCount) + } + if tc.expectedDeviceCount == 0 && tpmDeviceSpec != nil { + t.Fatalf("Expected %d TPM devices, got non-nil device spec", tc.expectedDeviceCount) } // If devices were created, validate their structure @@ -1501,10 +1508,15 @@ func TestGetVirtualTPMDevices(t *testing.T) { if spec.Operation != types.VirtualDeviceConfigSpecOperationAdd { t.Fatalf("Expected operation to be Add, got %v", spec.Operation) } - } - if tpmDeviceSpec == nil && tc.expectedDeviceCount != 0 { - t.Fatalf("Expected %d TPM devices, got %d", tc.expectedDeviceCount, 0) + tpmDevice, ok := spec.Device.(*types.VirtualTPM) + if !ok { + t.Fatalf("Expected device to be VirtualTPM, got %T", spec.Device) + } + + if tpmDevice.Key == 0 { + t.Fatalf("Expected TPM device to have a non-zero key") + } } }) }