From f7d67529ec86dc9e3edcdb620cae2b960906b687 Mon Sep 17 00:00:00 2001 From: akutz Date: Thu, 17 Oct 2024 11:33:21 -0500 Subject: [PATCH] Support BYOK for Instance Storage PVCs This patch adds support for creating Instance Storage PVCs with the EncryptionClass annotation that matches the VM's EncryptionClass. --- .../volume/volume_controller.go | 5 ++ .../volume/volume_controller_intg_test.go | 38 ++++++++++++++ .../volume/volume_controller_unit_test.go | 51 +++++++++++++++++++ pkg/providers/vsphere/constants/constants.go | 5 ++ 4 files changed, 99 insertions(+) diff --git a/controllers/virtualmachine/volume/volume_controller.go b/controllers/virtualmachine/volume/volume_controller.go index ae5feacf7..0db408e34 100644 --- a/controllers/virtualmachine/volume/volume_controller.go +++ b/controllers/virtualmachine/volume/volume_controller.go @@ -525,6 +525,11 @@ func (r *Reconciler) createInstanceStoragePVC( }, } + if c := ctx.VM.Spec.Crypto; c != nil && c.EncryptionClassName != "" { + // Assign the InstanceStorage PVC the same EncryptionClass as the VM. + pvc.Annotations[constants.EncryptionClassNameAnnotation] = c.EncryptionClassName + } + if err := controllerutil.SetControllerReference(ctx.VM, pvc, r.Client.Scheme()); err != nil { // This is an unexpected error. return fmt.Errorf("cannot set controller reference on PersistentVolumeClaim: %w", err) diff --git a/controllers/virtualmachine/volume/volume_controller_intg_test.go b/controllers/virtualmachine/volume/volume_controller_intg_test.go index 9b917efcc..698f5bffe 100644 --- a/controllers/virtualmachine/volume/volume_controller_intg_test.go +++ b/controllers/virtualmachine/volume/volume_controller_intg_test.go @@ -244,7 +244,9 @@ func intgTestsReconcile() { config.Features.InstanceStorage = true config.InstanceStorage.PVPlacementFailedTTL = 0 }) + }) + JustBeforeEach(func() { vm.Spec.Volumes = append(vm.Spec.Volumes, builder.DummyInstanceStorageVirtualMachineVolumes()...) vm.Labels = map[string]string{constants.InstanceStorageLabelKey: "true"} Expect(ctx.Client.Create(ctx, vm)).To(Succeed()) @@ -305,6 +307,42 @@ func intgTestsReconcile() { waitForVirtualMachineInstanceStorage(vmKey, false, false) }) }) + + When("VM has non-empty spec.crypto.encryptionClassName", func() { + BeforeEach(func() { + if vm.Annotations == nil { + vm.Annotations = map[string]string{} + } + vm.Annotations[constants.InstanceStorageSelectedNodeAnnotationKey] = dummySelectedNode + vm.Annotations[constants.InstanceStorageSelectedNodeMOIDAnnotationKey] = dummySelectedNodeMOID + + vm.Spec.Crypto = &vmopv1.VirtualMachineCryptoSpec{ + EncryptionClassName: "my-encryption-class", + } + }) + Specify("PVCs are created with the expected annotation", func() { + Expect(ctx.Client.Get(ctx, vmKey, vm)).To(Succeed()) + + volumes := instancestorage.FilterVolumes(vm) + Expect(volumes).ToNot(BeEmpty()) + + Eventually(func(g Gomega) { + for i := range volumes { + var obj corev1.PersistentVolumeClaim + g.Expect(ctx.Client.Get( + ctx, + client.ObjectKey{ + Namespace: vm.Namespace, + Name: volumes[i].PersistentVolumeClaim.ClaimName, + }, + &obj)).To(Succeed()) + g.Expect(obj.Annotations).To(HaveKeyWithValue( + constants.EncryptionClassNameAnnotation, + vm.Spec.Crypto.EncryptionClassName)) + } + }).Should(Succeed()) + }) + }) }) Context("Reconcile", func() { diff --git a/controllers/virtualmachine/volume/volume_controller_unit_test.go b/controllers/virtualmachine/volume/volume_controller_unit_test.go index caad41abe..72cb4cde4 100644 --- a/controllers/virtualmachine/volume/volume_controller_unit_test.go +++ b/controllers/virtualmachine/volume/volume_controller_unit_test.go @@ -270,6 +270,57 @@ func unitTestsReconcile() { Expect(err.Error()).To(ContainSubstring("insufficient quota")) expectPVCsStatus(volCtx, ctx, true, false, 0) }) + + When("VM has nil spec.crypto", func() { + BeforeEach(func() { + vm.Spec.Crypto = nil + }) + Specify("PVCs are created sans the expected annotation", func() { + Expect(reconciler.ReconcileNormal(volCtx)).To(Succeed()) + pvcList, err := getInstanceStoragePVCs(volCtx, ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(pvcList).ToNot(BeEmpty()) + for i := range pvcList { + Expect(pvcList[i].Annotations).ToNot(HaveKey(constants.EncryptionClassNameAnnotation)) + } + }) + }) + + When("VM has empty spec.crypto.encryptionClassName", func() { + BeforeEach(func() { + vm.Spec.Crypto = &vmopv1.VirtualMachineCryptoSpec{ + EncryptionClassName: "", + } + }) + Specify("PVCs are created sans the expected annotation", func() { + Expect(reconciler.ReconcileNormal(volCtx)).To(Succeed()) + pvcList, err := getInstanceStoragePVCs(volCtx, ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(pvcList).ToNot(BeEmpty()) + for i := range pvcList { + Expect(pvcList[i].Annotations).ToNot(HaveKey(constants.EncryptionClassNameAnnotation)) + } + }) + }) + + When("VM has non-empty spec.crypto.encryptionClassName", func() { + BeforeEach(func() { + vm.Spec.Crypto = &vmopv1.VirtualMachineCryptoSpec{ + EncryptionClassName: "my-encryption-class", + } + }) + Specify("PVCs are created with the expected annotation", func() { + Expect(reconciler.ReconcileNormal(volCtx)).To(Succeed()) + pvcList, err := getInstanceStoragePVCs(volCtx, ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(pvcList).ToNot(BeEmpty()) + for i := range pvcList { + Expect(pvcList[i].Annotations).To(HaveKeyWithValue( + constants.EncryptionClassNameAnnotation, + vm.Spec.Crypto.EncryptionClassName)) + } + }) + }) }) When("VM does not have BiosUUID", func() { diff --git a/pkg/providers/vsphere/constants/constants.go b/pkg/providers/vsphere/constants/constants.go index 27cc2d672..0fdf9ccba 100644 --- a/pkg/providers/vsphere/constants/constants.go +++ b/pkg/providers/vsphere/constants/constants.go @@ -68,6 +68,11 @@ const ( CloudInitGuestInfoUserdata = "guestinfo.userdata" CloudInitGuestInfoUserdataEncoding = "guestinfo.userdata.encoding" + // EncryptionClassNameAnnotation specifies the name of an EncryptionClass + // resource. This is used by APIs that participate in BYOK but cannot modify + // their spec to do so, such as the PersistentVolumeClaim API. + EncryptionClassNameAnnotation = "encryption.vmware.com/encryption-class-name" + // InstanceStoragePVCNamePrefix prefix of auto-generated PVC names. InstanceStoragePVCNamePrefix = "instance-pvc-" // InstanceStorageLabelKey identifies resources related to instance storage.