SMM is not enabled when TPM is enabled in VMware source client

pkg/source/helper.go

@@ -0,0 +1,28 @@
+package source
+
+import (
+	"k8s.io/utils/pointer"
+	kubevirt "kubevirt.io/api/core/v1"
+)
+
+func VMSpecSetupUEFISettings(vmSpec *kubevirt.VirtualMachineSpec, secureBoot, tpm bool) {
+	firmware := &kubevirt.Firmware{
+		Bootloader: &kubevirt.Bootloader{
+			EFI: &kubevirt.EFI{
+				SecureBoot: pointer.Bool(false),
+			},
+		},
+	}
+	if secureBoot {
+		firmware.Bootloader.EFI.SecureBoot = pointer.Bool(true)
+	}
+	vmSpec.Template.Spec.Domain.Firmware = firmware
+	if tpm {
+		vmSpec.Template.Spec.Domain.Devices.TPM = &kubevirt.TPMDevice{}
+	}
+	if secureBoot || tpm {
+		vmSpec.Template.Spec.Domain.Features.SMM = &kubevirt.FeatureState{
+			Enabled: pointer.Bool(true),
+		}
+	}
+}

pkg/source/helper_test.go

@@ -0,0 +1,58 @@
+package source
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	kubevirt "kubevirt.io/api/core/v1"
+)
+
+func Test_vmSpecSetupUefiSettings(t *testing.T) {
+	assert := require.New(t)
+	testCases := []struct {
+		desc       string
+		secureBoot bool
+		tpm        bool
+	}{
+		{
+			desc:       "SecureBoot enabled, TPM disabled",
+			secureBoot: true,
+			tpm:        false,
+		}, {
+			desc:       "SecureBoot disabled, TPM enabled",
+			secureBoot: false,
+			tpm:        true,
+		}, {
+			desc:       "SecureBoot enabled, TPM enabled",
+			secureBoot: true,
+			tpm:        true,
+		}, {
+			desc:       "SecureBoot disabled, TPM disabled",
+			secureBoot: false,
+			tpm:        false,
+		},
+	}
+
+	for _, tc := range testCases {
+		vmSpec := kubevirt.VirtualMachineSpec{
+			Template: &kubevirt.VirtualMachineInstanceTemplateSpec{
+				Spec: kubevirt.VirtualMachineInstanceSpec{
+					Domain: kubevirt.DomainSpec{
+						Features: &kubevirt.Features{},
+					},
+				},
+			},
+		}
+		VMSpecSetupUEFISettings(&vmSpec, tc.secureBoot, tc.tpm)
+		if tc.secureBoot {
+			assert.True(*vmSpec.Template.Spec.Domain.Firmware.Bootloader.EFI.SecureBoot, "expected SecureBoot to be enabled")
+		} else {
+			assert.False(*vmSpec.Template.Spec.Domain.Firmware.Bootloader.EFI.SecureBoot, "expected SecureBoot to be disabled")
+		}
+		if tc.secureBoot || tc.tpm {
+			assert.True(*vmSpec.Template.Spec.Domain.Features.SMM.Enabled, "expected SMM to be enabled")
+		} else {
+			assert.Nil(vmSpec.Template.Spec.Domain.Features.SMM, "expected SMM to be nil")
+		}
+	}
+}

pkg/source/openstack/client.go

@@ -27,10 +27,12 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/pointer"
 	kubevirt "kubevirt.io/api/core/v1"
 
 	migration "github.com/harvester/vm-import-controller/pkg/apis/migration.harvesterhci.io/v1beta1"
 	"github.com/harvester/vm-import-controller/pkg/server"
+	"github.com/harvester/vm-import-controller/pkg/source"
 	"github.com/harvester/vm-import-controller/pkg/util"
 )
 
@@ -410,8 +412,6 @@ func (c *Client) IsPoweredOff(vm *migration.VirtualMachineImport) (bool, error)
 }
 
 func (c *Client) GenerateVirtualMachine(vm *migration.VirtualMachineImport) (*kubevirt.VirtualMachine, error) {
-	var boolFalse = false
-	var boolTrue = true
 	vmObj, err := c.findVM(vm.Spec.VirtualMachineName)
 	if err != nil {
 		return nil, fmt.Errorf("error finding VM in GenerateVirtualMachine: %v", err)
@@ -430,7 +430,7 @@ func (c *Client) GenerateVirtualMachine(vm *migration.VirtualMachineImport) (*ku
 		return nil, fmt.Errorf("error looking up flavor: %v", err)
 	}
 
-	uefi, tpm, secureboot, err := c.ImageFirmwareSettings(&vmObj.Server)
+	uefi, tpm, secureBoot, err := c.ImageFirmwareSettings(&vmObj.Server)
 	if err != nil {
 		return nil, fmt.Errorf("error getting firware settings: %v", err)
 	}
@@ -480,7 +480,7 @@ func (c *Client) GenerateVirtualMachine(vm *migration.VirtualMachineImport) (*ku
 					},
 					Features: &kubevirt.Features{
 						ACPI: kubevirt.FeatureState{
-							Enabled: &boolTrue,
+							Enabled: pointer.Bool(true),
 						},
 					},
 				},
@@ -529,32 +529,15 @@ func (c *Client) GenerateVirtualMachine(vm *migration.VirtualMachineImport) (*ku
 		})
 	}
 
+	// Setup BIOS/EFI, SecureBoot and TPM settings.
 	if uefi {
-		firmware := &kubevirt.Firmware{
-			Bootloader: &kubevirt.Bootloader{
-				EFI: &kubevirt.EFI{
-					SecureBoot: &boolFalse,
-				},
-			},
-		}
-		if secureboot {
-			firmware.Bootloader.EFI.SecureBoot = &boolTrue
-			vmSpec.Template.Spec.Domain.Features.SMM = &kubevirt.FeatureState{
-				Enabled: &boolTrue,
-			}
-		}
-		vmSpec.Template.Spec.Domain.Firmware = firmware
-		if tpm {
-			vmSpec.Template.Spec.Domain.Features.SMM = &kubevirt.FeatureState{
-				Enabled: &boolTrue,
-			}
-			vmSpec.Template.Spec.Domain.Devices.TPM = &kubevirt.TPMDevice{}
-		}
+		source.VMSpecSetupUEFISettings(&vmSpec, secureBoot, tpm)
 	}
 
 	vmSpec.Template.Spec.Networks = networkConfig
 	vmSpec.Template.Spec.Domain.Devices.Interfaces = interfaces
 	newVM.Spec = vmSpec
+
 	// disk attachment needs query by core controller for storage classes, so will be added by the migration controller
 	return newVM, nil
 }

pkg/source/vmware/client.go

@@ -21,11 +21,13 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/pointer"
 	kubevirt "kubevirt.io/api/core/v1"
 
 	migration "github.com/harvester/vm-import-controller/pkg/apis/migration.harvesterhci.io/v1beta1"
 	"github.com/harvester/vm-import-controller/pkg/qemu"
 	"github.com/harvester/vm-import-controller/pkg/server"
+	"github.com/harvester/vm-import-controller/pkg/source"
 	"github.com/harvester/vm-import-controller/pkg/util"
 )
 
@@ -294,12 +296,9 @@ func (c *Client) IsPoweredOff(vm *migration.VirtualMachineImport) (bool, error)
 }
 
 func (c *Client) GenerateVirtualMachine(vm *migration.VirtualMachineImport) (*kubevirt.VirtualMachine, error) {
-	var boolFalse = false
-	var boolTrue = true
-
 	vmObj, err := c.findVM(vm.Spec.Folder, vm.Spec.VirtualMachineName)
 	if err != nil {
-		return nil, fmt.Errorf("error quering vm in GenerateVirtualMachine: %v", err)
+		return nil, fmt.Errorf("error querying vm in GenerateVirtualMachine: %w", err)
 	}
 
 	newVM := &kubevirt.VirtualMachine{
@@ -353,7 +352,7 @@ func (c *Client) GenerateVirtualMachine(vm *migration.VirtualMachineImport) (*ku
 					},
 					Features: &kubevirt.Features{
 						ACPI: kubevirt.FeatureState{
-							Enabled: &boolTrue,
+							Enabled: pointer.Bool(true),
 						},
 					},
 				},
@@ -402,31 +401,21 @@ func (c *Client) GenerateVirtualMachine(vm *migration.VirtualMachineImport) (*ku
 		})
 	}
 
-	// setup bios/efi, secureboot and tpm settings
-
-	if o.Config.Firmware == "efi" {
-		firmware := &kubevirt.Firmware{
-			Bootloader: &kubevirt.Bootloader{
-				EFI: &kubevirt.EFI{
-					SecureBoot: &boolFalse,
-				},
-			},
-		}
-		if *o.Config.BootOptions.EfiSecureBootEnabled {
-			firmware.Bootloader.EFI.SecureBoot = &boolTrue
-			vmSpec.Template.Spec.Domain.Features.SMM = &kubevirt.FeatureState{
-				Enabled: &boolTrue,
-			}
-		}
-		vmSpec.Template.Spec.Domain.Firmware = firmware
-		if *o.Summary.Config.TpmPresent {
-
-			vmSpec.Template.Spec.Domain.Devices.TPM = &kubevirt.TPMDevice{}
-		}
+	// Setup BIOS/EFI, SecureBoot and TPM settings.
+	uefi := strings.ToLower(o.Config.Firmware) == string(types.GuestOsDescriptorFirmwareTypeEfi)
+	secureBoot := false
+	if o.Config.BootOptions != nil {
+		secureBoot = pointer.BoolDeref(o.Config.BootOptions.EfiSecureBootEnabled, false)
 	}
+	tpm := pointer.BoolDeref(o.Summary.Config.TpmPresent, false)
+	if uefi {
+		source.VMSpecSetupUEFISettings(&vmSpec, secureBoot, tpm)
+	}
+
 	vmSpec.Template.Spec.Networks = networkConfig
 	vmSpec.Template.Spec.Domain.Devices.Interfaces = interfaces
 	newVM.Spec = vmSpec
+
 	// disk attachment needs query by core controller for storage classes, so will be added by the migration controller
 	return newVM, nil
 }

pkg/source/vmware/client_test.go

@@ -10,9 +10,12 @@ import (
 
 	"github.com/ory/dockertest/v3"
 	"github.com/stretchr/testify/require"
+	"github.com/vmware/govmomi/find"
 	"github.com/vmware/govmomi/vim25/mo"
+	"github.com/vmware/govmomi/vim25/types"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/pointer"
 
 	migration "github.com/harvester/vm-import-controller/pkg/apis/migration.harvesterhci.io/v1beta1"
 	"github.com/harvester/vm-import-controller/pkg/server"
@@ -27,10 +30,11 @@ func TestMain(t *testing.M) {
 		log.Fatalf("error connecting to dockerd: %v", err)
 	}
 
+	// https://hub.docker.com/r/vmware/vcsim
 	runOpts := &dockertest.RunOptions{
 		Name:       "vcsim",
 		Repository: "vmware/vcsim",
-		Tag:        "v0.29.0",
+		Tag:        "v0.49.0",
 	}
 
 	vcsimMock, err := pool.RunWithOptions(runOpts)
@@ -202,7 +206,7 @@ func Test_ExportVirtualMachine(t *testing.T) {
 	}
 
 	err = c.ExportVirtualMachine(vm)
-	assert.NoError(err, "expected no error during vm export")
+	assert.NoError(err, "expected no error during VM export")
 	t.Log(vm.Status)
 }
 
@@ -247,14 +251,91 @@ func Test_GenerateVirtualMachine(t *testing.T) {
 	}
 
 	newVM, err := c.GenerateVirtualMachine(vm)
-	assert.NoError(err, "expected no error during vm export")
+	assert.NoError(err, "expected no error during VM CR generation")
 	assert.Len(newVM.Spec.Template.Spec.Networks, 1, "should have found the default pod network")
 	assert.Len(newVM.Spec.Template.Spec.Domain.Devices.Interfaces, 1, "should have found a network map")
 	assert.Equal(newVM.Spec.Template.Spec.Domain.Memory.Guest.String(), "32M", "expected VM to have 32M memory")
 	assert.NotEmpty(newVM.Spec.Template.Spec.Domain.Resources.Limits, "expect to find resource requests to be present")
 
 }
 
+func Test_GenerateVirtualMachine_secureboot(t *testing.T) {
+	assert := require.New(t)
+	ctx := context.TODO()
+	endpoint := fmt.Sprintf("https://localhost:%s/sdk", vcsimPort)
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "default",
+		},
+		Data: map[string][]byte{
+			"username": []byte("user"),
+			"password": []byte("pass"),
+		},
+	}
+	vm := &migration.VirtualMachineImport{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "demo",
+			Namespace: "default",
+		},
+		Spec: migration.VirtualMachineImportSpec{
+			SourceCluster:      corev1.ObjectReference{},
+			VirtualMachineName: "test01",
+			Mapping: []migration.NetworkMapping{
+				{
+					SourceNetwork:      "DVSwitch: fea97929-4b2d-5972-b146-930c6d0b4014",
+					DestinationNetwork: "default/vlan",
+				},
+			},
+		},
+	}
+
+	c, err := NewClient(ctx, endpoint, "DC0", secret)
+	assert.NoError(err, "expected no error during creation of client")
+	err = c.Verify()
+	assert.NoError(err, "expected no error during verification of client")
+
+	// https://github.com/vmware/govmomi/blob/main/vcsim/README.md#default-vcenter-inventory
+	f := find.NewFinder(c.Client.Client, true)
+
+	dc, err := f.Datacenter(ctx, c.dc)
+	assert.NoError(err, "expected no error during datacenter lookup")
+
+	f.SetDatacenter(dc)
+
+	ds, err := f.DefaultDatastore(ctx)
+	assert.NoError(err, "expected no error during datastore lookup")
+
+	pool, err := f.ResourcePool(ctx, "DC0_H0/Resources")
+	assert.NoError(err, "expected no error during resource pool lookup")
+
+	folder, err := dc.Folders(ctx)
+	assert.NoError(err, "expected no error during folder lookup")
+
+	vmConfigSpec := types.VirtualMachineConfigSpec{
+		Name:     vm.Spec.VirtualMachineName,
+		GuestId:  string(types.VirtualMachineGuestOsIdentifierOtherGuest64),
+		Firmware: string(types.GuestOsDescriptorFirmwareTypeEfi),
+		BootOptions: &types.VirtualMachineBootOptions{
+			EfiSecureBootEnabled: pointer.Bool(true),
+		},
+		Files: &types.VirtualMachineFileInfo{
+			VmPathName: fmt.Sprintf("[%s] %s", ds.Name(), vm.Spec.VirtualMachineName),
+		},
+	}
+
+	task, err := folder.VmFolder.CreateVM(ctx, vmConfigSpec, pool, nil)
+	assert.NoError(err, "expected no error when creating VM")
+
+	_, err = task.WaitForResult(ctx, nil)
+	assert.NoError(err, "expected no error when waiting for task to complete")
+
+	newVM, err := c.GenerateVirtualMachine(vm)
+	assert.NoError(err, "expected no error during VM CR generation")
+	assert.True(*newVM.Spec.Template.Spec.Domain.Firmware.Bootloader.EFI.SecureBoot, "expected VM to have secure boot enabled")
+	assert.True(*newVM.Spec.Template.Spec.Domain.Features.SMM.Enabled, "expected VM to have SMM enabled")
+}
+
 func Test_identifyNetworkCards(t *testing.T) {
 	ctx := context.TODO()
 	endpoint := fmt.Sprintf("https://localhost:%s/sdk", vcsimPort)
@@ -277,7 +358,7 @@ func Test_identifyNetworkCards(t *testing.T) {
 	assert.NoError(err, "expected no error during verification of client")
 
 	vmObj, err := c.findVM("", "DC0_H0_VM0")
-	assert.NoError(err, "expected no error during vm lookup")
+	assert.NoError(err, "expected no error during VM lookup")
 
 	var o mo.VirtualMachine