Merge pull request #765 from akutz/feature/byok-rekey-disks

✨ Rekey disks for encrypted VMs

api/v1alpha3/virtualmachine_storage_types.go

@@ -84,6 +84,22 @@ const (
 	VirtualMachineStorageDiskTypeManaged VirtualMachineVolumeType = "Managed"
 )
 
+type VirtualMachineVolumeCryptoStatus struct {
+	// +optional
+
+	// ProviderID describes the provider ID used to encrypt the volume.
+	// Please note, this field will be empty if the volume is not
+	// encrypted.
+	ProviderID string `json:"providerID,omitempty"`
+
+	// +optional
+
+	// KeyID describes the key ID used to encrypt the volume.
+	// Please note, this field will be empty if the volume is not
+	// encrypted.
+	KeyID string `json:"keyID,omitempty"`
+}
+
 // VirtualMachineVolumeStatus defines the observed state of a
 // VirtualMachineVolume instance.
 type VirtualMachineVolumeStatus struct {
@@ -97,6 +113,11 @@ type VirtualMachineVolumeStatus struct {
 
 	// +optional
 
+	// Crypto describes the volume's encryption status.
+	Crypto *VirtualMachineVolumeCryptoStatus `json:"crypto,omitempty"`
+
+	// +optional
+
 	// Limit describes the storage limit for the volume.
 	Limit *resource.Quantity `json:"limit,omitempty"`
 

api/v1alpha3/virtualmachine_types.go

@@ -745,11 +745,9 @@ type VirtualMachineCryptoStatus struct {
 	//
 	// - Config -- This refers to all of the files related to a VM except any
 	//             virtual disks.
-	// - Disk   -- This refers to all of the VM's virtual disks that are *not*
-	//             PVCs.
-	//
-	// To determine whether or not a PVC is encrypted, please refer to the PVC
-	// resource.
+	// - Disks  -- This refers to at least one of the VM's attached disks. To
+	//             determine the encryption state of the individual disks,
+	//             please refer to status.volumes[].crypto.
 	Encrypted []VirtualMachineEncryptionType `json:"encrypted,omitempty"`
 
 	// +optional

config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml

@@ -4688,11 +4688,9 @@ spec:
 
                       - Config -- This refers to all of the files related to a VM except any
                                   virtual disks.
-                      - Disk   -- This refers to all of the VM's virtual disks that are *not*
-                                  PVCs.
-
-                      To determine whether or not a PVC is encrypted, please refer to the PVC
-                      resource.
+                      - Disks  -- This refers to at least one of the VM's attached disks. To
+                                  determine the encryption state of the individual disks,
+                                  please refer to status.volumes[].crypto.
                     items:
                       type: string
                     type: array
@@ -5403,6 +5401,22 @@ spec:
                         Attached represents whether a volume has been successfully attached to
                         the VirtualMachine or not.
                       type: boolean
+                    crypto:
+                      description: Crypto describes the volume's encryption status.
+                      properties:
+                        keyID:
+                          description: |-
+                            KeyID describes the key ID used to encrypt the volume.
+                            Please note, this field will be empty if the volume is not
+                            encrypted.
+                          type: string
+                        providerID:
+                          description: |-
+                            ProviderID describes the provider ID used to encrypt the volume.
+                            Please note, this field will be empty if the volume is not
+                            encrypted.
+                          type: string
+                      type: object
                     diskUUID:
                       description: |-
                         DiskUUID represents the underlying virtual disk UUID and is present when

controllers/storageclass/storageclass_controller_intg_test.go

@@ -59,7 +59,7 @@ func intgTestsReconcile() {
 		})
 		It("should eventually mark the storage class as encrypted in the context", func() {
 			Eventually(func(g Gomega) {
-				ok, err := kubeutil.IsEncryptedStorageClass(ctx, ctx.Client, obj.Name)
+				ok, _, err := kubeutil.IsEncryptedStorageClass(ctx, ctx.Client, obj.Name)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(ok).To(BeTrue())
 			})
@@ -69,7 +69,7 @@ func intgTestsReconcile() {
 	Context("reconciling a plain storage class", func() {
 		It("should never mark the storage class as encrypted in the context", func() {
 			Consistently(func(g Gomega) {
-				ok, err := kubeutil.IsEncryptedStorageClass(ctx, ctx.Client, obj.Name)
+				ok, _, err := kubeutil.IsEncryptedStorageClass(ctx, ctx.Client, obj.Name)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(ok).To(BeFalse())
 			})

controllers/storageclass/storageclass_controller_unit_test.go

@@ -112,7 +112,11 @@ func unitTestsReconcile() {
 		When("no storage policy ID", func() {
 			It("should return an error", func() {
 				Expect(err).To(HaveOccurred())
-				Expect(err).To(MatchError(`StorageClass "my-storage-class" does not have 'storagePolicyID' parameter`))
+				Expect(err).To(MatchError(
+					kubeutil.ErrMissingParameter{
+						StorageClassName: "my-storage-class",
+						ParameterName:    "storagePolicyID",
+					}))
 			})
 		})
 		When("not encrypted", func() {
@@ -123,7 +127,7 @@ func unitTestsReconcile() {
 			})
 			It("marks item as encrypted and returns success", func() {
 				Expect(err).ToNot(HaveOccurred())
-				ok, err := kubeutil.IsEncryptedStorageClass(ctx, ctx.Client, obj.Name)
+				ok, _, err := kubeutil.IsEncryptedStorageClass(ctx, ctx.Client, obj.Name)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(ok).To(BeFalse())
 			})
@@ -136,7 +140,7 @@ func unitTestsReconcile() {
 			})
 			It("marks item as encrypted and returns success", func() {
 				Expect(err).ToNot(HaveOccurred())
-				ok, err := kubeutil.IsEncryptedStorageClass(ctx, ctx.Client, obj.Name)
+				ok, _, err := kubeutil.IsEncryptedStorageClass(ctx, ctx.Client, obj.Name)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(ok).To(BeTrue())
 			})

docs/concepts/workloads/vm.md

@@ -476,14 +476,14 @@ The field `spec.crypto` may be used in conjunction with a VM's storage class and
 
 #### Encryption type
 
-The type of encryption depends on the storage class and hardware present in the VM as the chart below illustrates:
+The type of encryption used by the VM is reported in the list `status.crypto.encrypted`. The list may contain the values `Config` and/or `Disks`, depending on the storage class and hardware present in the VM as the chart below illustrates:
 
 |                          | Config | Disks |
 |--------------------------|:------:|:-----:|
 | Encryption storage class |    ✓   |   ✓   |
 | vTPM                     |    ✓   |       |
 
-The `Config` type refers to all files related to a VM except for virtual disks. The `Disks` type refers to a VM's virtual disks except for PVCs. To ascertain the encryption status of a PVC, please refer directly to the PVC resource.
+The `Config` type refers to all files related to a VM except for virtual disks. The `Disks` type indicates at least one of a VM's virtual disks is encrypted. To determine which of the VM's disks are encrypted, please refer to `status.volumes[].crypto`. 
 
 #### Default Key Provider
 

go.mod

@@ -33,7 +33,7 @@ require (
 	github.com/vmware-tanzu/vm-operator/external/tanzu-topology v0.0.0-00010101000000-000000000000
 	github.com/vmware-tanzu/vm-operator/pkg/backup/api v0.0.0-00010101000000-000000000000
 	github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels v0.0.0-00010101000000-000000000000
-	github.com/vmware/govmomi v0.31.1-0.20241015181712-cd4331eda367
+	github.com/vmware/govmomi v0.31.1-0.20241024144701-6df52891f229
 	golang.org/x/exp v0.0.0-20230515195305-f3d0a9c9a5cc
 	// * https://github.com/vmware-tanzu/vm-operator/security/dependabot/24
 	golang.org/x/text v0.19.0

go.sum

@@ -459,8 +459,8 @@ github.com/vmware-tanzu/net-operator-api v0.0.0-20240523152550-862e2c4eb0e0 h1:y
 github.com/vmware-tanzu/net-operator-api v0.0.0-20240523152550-862e2c4eb0e0/go.mod h1:w6QJGm3crIA16ZIz1FVQXD2NVeJhOgGXxW05RbVTSTo=
 github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20240902045731-00a14868c72d h1:6pMXrQmTYpu5FipoQ9fT4FJG3VPMMbBoIi6h3KvdQc8=
 github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20240902045731-00a14868c72d/go.mod h1:Q4JzNkNMvjo7pXtlB5/R3oME4Nhah7fAObWgghVmtxk=
-github.com/vmware/govmomi v0.31.1-0.20241015181712-cd4331eda367 h1:4gP1VC+N9onypUJh52dSrAKsnpyhQLHc/g3tH9qd4vM=
-github.com/vmware/govmomi v0.31.1-0.20241015181712-cd4331eda367/go.mod h1:uoLVU9zlXC4p4GmLVG+ZJmBC0Gn3Q7mytOJvi39OhxA=
+github.com/vmware/govmomi v0.31.1-0.20241024144701-6df52891f229 h1:XopiNtNFPXBeamYFQRQBQsv2kYYhwYGNYdhbEuoaHxQ=
+github.com/vmware/govmomi v0.31.1-0.20241024144701-6df52891f229/go.mod h1:uoLVU9zlXC4p4GmLVG+ZJmBC0Gn3Q7mytOJvi39OhxA=
 github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
 github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=

pkg/providers/vsphere/vmlifecycle/update_status.go

@@ -827,21 +827,32 @@ func updateVolumeStatus(vm *vmopv1.VirtualMachine, moVM mo.VirtualMachine) {
 			// existing status with the usage information.
 			di, _ := vmdk.GetVirtualDiskInfoByUUID(ctx, nil, moVM, false, diskUUID)
 			vm.Status.Volumes[diskIndex].Used = BytesToResourceGiB(di.UniqueSize)
+			if di.CryptoKey.ProviderID != "" || di.CryptoKey.KeyID != "" {
+				vm.Status.Volumes[diskIndex].Crypto = &vmopv1.VirtualMachineVolumeCryptoStatus{
+					ProviderID: di.CryptoKey.ProviderID,
+					KeyID:      di.CryptoKey.KeyID,
+				}
+			}
 		} else if !isFCD {
 			// The disk is a classic, non-FCD that must be added to the list of
 			// volume statuses.
 			di, _ := vmdk.GetVirtualDiskInfoByUUID(ctx, nil, moVM, false, diskUUID)
 			dp := diskPath.Path
-			vm.Status.Volumes = append(
-				vm.Status.Volumes,
-				vmopv1.VirtualMachineVolumeStatus{
-					Name:     strings.TrimSuffix(path.Base(dp), path.Ext(dp)),
-					Type:     vmopv1.VirtualMachineStorageDiskTypeClassic,
-					Attached: true,
-					DiskUUID: diskUUID,
-					Limit:    BytesToResourceGiB(di.CapacityInBytes),
-					Used:     BytesToResourceGiB(di.UniqueSize),
-				})
+			volStatus := vmopv1.VirtualMachineVolumeStatus{
+				Name:     strings.TrimSuffix(path.Base(dp), path.Ext(dp)),
+				Type:     vmopv1.VirtualMachineStorageDiskTypeClassic,
+				Attached: true,
+				DiskUUID: diskUUID,
+				Limit:    BytesToResourceGiB(di.CapacityInBytes),
+				Used:     BytesToResourceGiB(di.UniqueSize),
+			}
+			if di.CryptoKey.ProviderID != "" || di.CryptoKey.KeyID != "" {
+				volStatus.Crypto = &vmopv1.VirtualMachineVolumeCryptoStatus{
+					ProviderID: di.CryptoKey.ProviderID,
+					KeyID:      di.CryptoKey.KeyID,
+				}
+			}
+			vm.Status.Volumes = append(vm.Status.Volumes, volStatus)
 		}
 	}
 

pkg/providers/vsphere/vmlifecycle/update_status_test.go

@@ -547,6 +547,12 @@ var _ = Describe("UpdateStatus", func() {
 												FileName: "[datastore] vm/my-disk-100.vmdk",
 											},
 											Uuid: "100",
+											KeyId: &vimtypes.CryptoKeyId{
+												KeyId: "my-key-id",
+												ProviderId: &vimtypes.KeyProviderId{
+													Id: "my-provider-id",
+												},
+											},
 										},
 										Key: 100,
 									},
@@ -610,6 +616,12 @@ var _ = Describe("UpdateStatus", func() {
 												FileName: "[datastore] vm/my-disk-105.vmdk",
 											},
 											Uuid: "105",
+											KeyId: &vimtypes.CryptoKeyId{
+												KeyId: "my-key-id",
+												ProviderId: &vimtypes.KeyProviderId{
+													Id: "my-provider-id",
+												},
+											},
 										},
 										Key: 105,
 									},
@@ -788,6 +800,10 @@ var _ = Describe("UpdateStatus", func() {
 								Name:     "my-disk-100",
 								DiskUUID: "100",
 								Type:     vmopv1.VirtualMachineStorageDiskTypeClassic,
+								Crypto: &vmopv1.VirtualMachineVolumeCryptoStatus{
+									KeyID:      "my-key-id",
+									ProviderID: "my-provider-id",
+								},
 								Attached: true,
 								Limit:    vmlifecycle.BytesToResourceGiB(10 * oneGiBInBytes),
 								Used:     vmlifecycle.BytesToResourceGiB(500 + (1 * oneGiBInBytes)),
@@ -846,6 +862,10 @@ var _ = Describe("UpdateStatus", func() {
 								Name:     "my-disk-100",
 								DiskUUID: "100",
 								Type:     vmopv1.VirtualMachineStorageDiskTypeClassic,
+								Crypto: &vmopv1.VirtualMachineVolumeCryptoStatus{
+									KeyID:      "my-key-id",
+									ProviderID: "my-provider-id",
+								},
 								Attached: true,
 								Limit:    vmlifecycle.BytesToResourceGiB(10 * oneGiBInBytes),
 								Used:     vmlifecycle.BytesToResourceGiB(500 + (1 * oneGiBInBytes)),
@@ -886,6 +906,10 @@ var _ = Describe("UpdateStatus", func() {
 								Name:     "my-disk-105",
 								DiskUUID: "105",
 								Type:     vmopv1.VirtualMachineStorageDiskTypeManaged,
+								Crypto: &vmopv1.VirtualMachineVolumeCryptoStatus{
+									KeyID:      "my-key-id",
+									ProviderID: "my-provider-id",
+								},
 								Attached: false,
 								Limit:    vmlifecycle.BytesToResourceGiB(100 * oneGiBInBytes),
 								Used:     vmlifecycle.BytesToResourceGiB(500 + (50 * oneGiBInBytes)),
@@ -912,6 +936,10 @@ var _ = Describe("UpdateStatus", func() {
 								Name:     "my-disk-100",
 								DiskUUID: "100",
 								Type:     vmopv1.VirtualMachineStorageDiskTypeClassic,
+								Crypto: &vmopv1.VirtualMachineVolumeCryptoStatus{
+									ProviderID: "my-provider-id",
+									KeyID:      "my-key-id",
+								},
 								Attached: true,
 								Limit:    vmlifecycle.BytesToResourceGiB(10 * oneGiBInBytes),
 								Used:     vmlifecycle.BytesToResourceGiB(500 + (1 * oneGiBInBytes)),