diff --git a/api/v1alpha1/zz_generated.conversion.go b/api/v1alpha1/zz_generated.conversion.go index 24a5b7eac..da7726599 100644 --- a/api/v1alpha1/zz_generated.conversion.go +++ b/api/v1alpha1/zz_generated.conversion.go @@ -2242,6 +2242,7 @@ func autoConvert_v1alpha1_VirtualMachineVolumeStatus_To_v1alpha3_VirtualMachineV func autoConvert_v1alpha3_VirtualMachineVolumeStatus_To_v1alpha1_VirtualMachineVolumeStatus(in *v1alpha3.VirtualMachineVolumeStatus, out *VirtualMachineVolumeStatus, s conversion.Scope) error { out.Name = in.Name // WARNING: in.Type requires manual conversion: does not exist in peer-type + // WARNING: in.Crypto requires manual conversion: does not exist in peer-type // WARNING: in.Limit requires manual conversion: does not exist in peer-type // WARNING: in.Used requires manual conversion: does not exist in peer-type out.Attached = in.Attached diff --git a/api/v1alpha2/zz_generated.conversion.go b/api/v1alpha2/zz_generated.conversion.go index edc9aa452..c1da86e90 100644 --- a/api/v1alpha2/zz_generated.conversion.go +++ b/api/v1alpha2/zz_generated.conversion.go @@ -3387,6 +3387,7 @@ func autoConvert_v1alpha2_VirtualMachineVolumeStatus_To_v1alpha3_VirtualMachineV func autoConvert_v1alpha3_VirtualMachineVolumeStatus_To_v1alpha2_VirtualMachineVolumeStatus(in *v1alpha3.VirtualMachineVolumeStatus, out *VirtualMachineVolumeStatus, s conversion.Scope) error { out.Name = in.Name // WARNING: in.Type requires manual conversion: does not exist in peer-type + // WARNING: in.Crypto requires manual conversion: does not exist in peer-type // WARNING: in.Limit requires manual conversion: does not exist in peer-type // WARNING: in.Used requires manual conversion: does not exist in peer-type out.Attached = in.Attached diff --git a/api/v1alpha3/virtualmachine_storage_types.go b/api/v1alpha3/virtualmachine_storage_types.go index b5e88a5f9..95db6d194 100644 --- a/api/v1alpha3/virtualmachine_storage_types.go +++ b/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"` diff --git a/api/v1alpha3/virtualmachine_types.go b/api/v1alpha3/virtualmachine_types.go index e3dee944b..f3cb45254 100644 --- a/api/v1alpha3/virtualmachine_types.go +++ b/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 diff --git a/api/v1alpha3/zz_generated.deepcopy.go b/api/v1alpha3/zz_generated.deepcopy.go index d99b43b67..f30440350 100644 --- a/api/v1alpha3/zz_generated.deepcopy.go +++ b/api/v1alpha3/zz_generated.deepcopy.go @@ -2280,6 +2280,21 @@ func (in *VirtualMachineVolume) DeepCopy() *VirtualMachineVolume { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VirtualMachineVolumeCryptoStatus) DeepCopyInto(out *VirtualMachineVolumeCryptoStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VirtualMachineVolumeCryptoStatus. +func (in *VirtualMachineVolumeCryptoStatus) DeepCopy() *VirtualMachineVolumeCryptoStatus { + if in == nil { + return nil + } + out := new(VirtualMachineVolumeCryptoStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VirtualMachineVolumeSource) DeepCopyInto(out *VirtualMachineVolumeSource) { *out = *in @@ -2303,6 +2318,11 @@ func (in *VirtualMachineVolumeSource) DeepCopy() *VirtualMachineVolumeSource { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VirtualMachineVolumeStatus) DeepCopyInto(out *VirtualMachineVolumeStatus) { *out = *in + if in.Crypto != nil { + in, out := &in.Crypto, &out.Crypto + *out = new(VirtualMachineVolumeCryptoStatus) + **out = **in + } if in.Limit != nil { in, out := &in.Limit, &out.Limit x := (*in).DeepCopy() diff --git a/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml b/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml index 18e1b75d4..7ffdb43d0 100644 --- a/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml +++ b/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 diff --git a/controllers/storageclass/storageclass_controller_intg_test.go b/controllers/storageclass/storageclass_controller_intg_test.go index 43bbfe207..5007bc55f 100644 --- a/controllers/storageclass/storageclass_controller_intg_test.go +++ b/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()) }) diff --git a/controllers/storageclass/storageclass_controller_unit_test.go b/controllers/storageclass/storageclass_controller_unit_test.go index 5514b795a..5ee7e9e36 100644 --- a/controllers/storageclass/storageclass_controller_unit_test.go +++ b/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()) }) diff --git a/docs/concepts/workloads/vm.md b/docs/concepts/workloads/vm.md index fb032906e..10feb67b1 100644 --- a/docs/concepts/workloads/vm.md +++ b/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 diff --git a/go.mod b/go.mod index fa70635a7..5f0942c51 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 8d8041cfe..9884e5df6 100644 --- a/go.sum +++ b/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= diff --git a/pkg/providers/vsphere/vmlifecycle/update_status.go b/pkg/providers/vsphere/vmlifecycle/update_status.go index 4dbc09e26..73d20688d 100644 --- a/pkg/providers/vsphere/vmlifecycle/update_status.go +++ b/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) } } diff --git a/pkg/providers/vsphere/vmlifecycle/update_status_test.go b/pkg/providers/vsphere/vmlifecycle/update_status_test.go index e1d90dc20..d2954f71e 100644 --- a/pkg/providers/vsphere/vmlifecycle/update_status_test.go +++ b/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)), diff --git a/pkg/providers/vsphere/vmprovider_vm_test.go b/pkg/providers/vsphere/vmprovider_vm_test.go index a274f411f..899f59d3a 100644 --- a/pkg/providers/vsphere/vmprovider_vm_test.go +++ b/pkg/providers/vsphere/vmprovider_vm_test.go @@ -1493,7 +1493,6 @@ func vmTests() { Expect(vm.Status.Crypto.Encrypted).To(HaveExactElements( []vmopv1.VirtualMachineEncryptionType{ vmopv1.VirtualMachineEncryptionTypeConfig, - vmopv1.VirtualMachineEncryptionTypeDisks, })) Expect(vm.Status.Crypto.ProviderID).To(Equal(ctx.NativeKeyProviderID)) Expect(vm.Status.Crypto.KeyID).ToNot(BeEmpty()) @@ -1514,7 +1513,6 @@ func vmTests() { Expect(vm.Status.Crypto.Encrypted).To(HaveExactElements( []vmopv1.VirtualMachineEncryptionType{ vmopv1.VirtualMachineEncryptionTypeConfig, - vmopv1.VirtualMachineEncryptionTypeDisks, })) Expect(vm.Status.Crypto.ProviderID).To(Equal(ctx.EncryptionClass1ProviderID)) Expect(vm.Status.Crypto.KeyID).ToNot(BeEmpty()) @@ -1536,7 +1534,6 @@ func vmTests() { Expect(vm.Status.Crypto.Encrypted).To(HaveExactElements( []vmopv1.VirtualMachineEncryptionType{ vmopv1.VirtualMachineEncryptionTypeConfig, - vmopv1.VirtualMachineEncryptionTypeDisks, })) Expect(vm.Status.Crypto.ProviderID).To(Equal(ctx.EncryptionClass1ProviderID)) Expect(vm.Status.Crypto.KeyID).To(Equal(nsInfo.EncryptionClass1KeyID)) @@ -1579,7 +1576,6 @@ func vmTests() { Expect(vm.Status.Crypto.Encrypted).To(HaveExactElements( []vmopv1.VirtualMachineEncryptionType{ vmopv1.VirtualMachineEncryptionTypeConfig, - vmopv1.VirtualMachineEncryptionTypeDisks, })) Expect(vm.Status.Crypto.ProviderID).To(Equal(ctx.NativeKeyProviderID)) Expect(vm.Status.Crypto.KeyID).ToNot(BeEmpty()) @@ -1605,7 +1601,6 @@ func vmTests() { Expect(vm.Status.Crypto.Encrypted).To(HaveExactElements( []vmopv1.VirtualMachineEncryptionType{ vmopv1.VirtualMachineEncryptionTypeConfig, - vmopv1.VirtualMachineEncryptionTypeDisks, })) Expect(vm.Status.Crypto.ProviderID).To(Equal(ctx.EncryptionClass1ProviderID)) Expect(vm.Status.Crypto.KeyID).ToNot(BeEmpty()) @@ -1632,7 +1627,6 @@ func vmTests() { Expect(vm.Status.Crypto.Encrypted).To(HaveExactElements( []vmopv1.VirtualMachineEncryptionType{ vmopv1.VirtualMachineEncryptionTypeConfig, - vmopv1.VirtualMachineEncryptionTypeDisks, })) Expect(vm.Status.Crypto.ProviderID).To(Equal(ctx.EncryptionClass2ProviderID)) Expect(vm.Status.Crypto.KeyID).To(Equal(nsInfo.EncryptionClass2KeyID)) @@ -1724,7 +1718,6 @@ func vmTests() { Expect(vm.Status.Crypto.Encrypted).To(HaveExactElements( []vmopv1.VirtualMachineEncryptionType{ vmopv1.VirtualMachineEncryptionTypeConfig, - vmopv1.VirtualMachineEncryptionTypeDisks, })) Expect(vm.Status.Crypto.ProviderID).To(Equal(ctx.NativeKeyProviderID)) Expect(vm.Status.Crypto.KeyID).ToNot(BeEmpty()) @@ -1751,7 +1744,6 @@ func vmTests() { Expect(vm.Status.Crypto.Encrypted).To(HaveExactElements( []vmopv1.VirtualMachineEncryptionType{ vmopv1.VirtualMachineEncryptionTypeConfig, - vmopv1.VirtualMachineEncryptionTypeDisks, })) Expect(vm.Status.Crypto.ProviderID).To(Equal(ctx.EncryptionClass2ProviderID)) Expect(vm.Status.Crypto.KeyID).ToNot(BeEmpty()) @@ -1779,7 +1771,6 @@ func vmTests() { Expect(vm.Status.Crypto.Encrypted).To(HaveExactElements( []vmopv1.VirtualMachineEncryptionType{ vmopv1.VirtualMachineEncryptionTypeConfig, - vmopv1.VirtualMachineEncryptionTypeDisks, })) Expect(vm.Status.Crypto.ProviderID).To(Equal(ctx.EncryptionClass2ProviderID)) Expect(vm.Status.Crypto.KeyID).To(Equal(nsInfo.EncryptionClass2KeyID)) diff --git a/pkg/util/kube/storage.go b/pkg/util/kube/storage.go index a380afae5..73095f567 100644 --- a/pkg/util/kube/storage.go +++ b/pkg/util/kube/storage.go @@ -119,19 +119,52 @@ func GetPVCZoneConstraints( return zones, nil } +// ErrMissingParameter is returned from GetStoragePolicyID if the StorageClass +// does not have the storage policy ID parameter. +type ErrMissingParameter struct { + StorageClassName string + ParameterName string +} + +func (e ErrMissingParameter) String() string { + return fmt.Sprintf( + "StorageClass %q does not have %q parameter", + e.StorageClassName, e.ParameterName) +} + +func (e ErrMissingParameter) Error() string { + return e.String() +} + // GetStoragePolicyID returns the storage policy ID for a given StorageClass. // If no ID is found, an error is returned. func GetStoragePolicyID(obj storagev1.StorageClass) (string, error) { policyID, ok := obj.Parameters[internal.StoragePolicyIDParameter] if !ok { - return "", fmt.Errorf( - "StorageClass %q does not have '"+ - internal.StoragePolicyIDParameter+"' parameter", - obj.Name) + return "", ErrMissingParameter{ + StorageClassName: obj.Name, + ParameterName: internal.StoragePolicyIDParameter, + } } return policyID, nil } +// SetStoragePolicyID sets the storage policy ID on the given StorageClass. +// An empty id removes the parameter from the StorageClass. +func SetStoragePolicyID(obj *storagev1.StorageClass, id string) { + if obj == nil { + panic("storageClass is nil") + } + if id == "" { + delete(obj.Parameters, internal.StoragePolicyIDParameter) + return + } + if obj.Parameters == nil { + obj.Parameters = map[string]string{} + } + obj.Parameters[internal.StoragePolicyIDParameter] = id +} + // MarkEncryptedStorageClass records the provided StorageClass as encrypted. func MarkEncryptedStorageClass( ctx context.Context, @@ -211,20 +244,21 @@ func MarkEncryptedStorageClass( return k8sClient.Patch(ctx, &obj, objPatch) } -// IsEncryptedStorageClass returns true if the provided StorageClass was marked -// as encrypted. +// IsEncryptedStorageClass returns true if the provided StorageClass name was +// marked as encrypted. If encryption is supported, the StorageClass's profile +// ID is also returned. func IsEncryptedStorageClass( ctx context.Context, k8sClient ctrlclient.Client, - storageClassName string) (bool, error) { + name string) (bool, string, error) { var obj storagev1.StorageClass if err := k8sClient.Get( ctx, - ctrlclient.ObjectKey{Name: storageClassName}, + ctrlclient.ObjectKey{Name: name}, &obj); err != nil { - return false, ctrlclient.IgnoreNotFound(err) + return false, "", ctrlclient.IgnoreNotFound(err) } return isEncryptedStorageClass(ctx, k8sClient, obj) @@ -244,7 +278,11 @@ func IsEncryptedStorageProfile( for i := range obj.Items { if pid, _ := GetStoragePolicyID(obj.Items[i]); pid == profileID { - return isEncryptedStorageClass(ctx, k8sClient, obj.Items[i]) + ok, _, err := isEncryptedStorageClass( + ctx, + k8sClient, + obj.Items[i]) + return ok, err } } @@ -254,7 +292,7 @@ func IsEncryptedStorageProfile( func isEncryptedStorageClass( ctx context.Context, k8sClient ctrlclient.Client, - storageClass storagev1.StorageClass) (bool, error) { + storageClass storagev1.StorageClass) (bool, string, error) { var ( obj corev1.ConfigMap @@ -266,8 +304,13 @@ func isEncryptedStorageClass( ) if err := k8sClient.Get(ctx, objKey, &obj); err != nil { - return false, ctrlclient.IgnoreNotFound(err) + return false, "", ctrlclient.IgnoreNotFound(err) + } + + if slices.Contains(obj.OwnerReferences, ownerRef) { + profileID, err := GetStoragePolicyID(storageClass) + return true, profileID, err } - return slices.Contains(obj.OwnerReferences, ownerRef), nil + return false, "", nil } diff --git a/pkg/util/kube/storage_test.go b/pkg/util/kube/storage_test.go index 72e7f6f1b..2e7442df5 100644 --- a/pkg/util/kube/storage_test.go +++ b/pkg/util/kube/storage_test.go @@ -60,9 +60,61 @@ var _ = DescribeTable("GetStoragePolicyID", "does not have policy ID", "", fakeString, - `StorageClass "my-storage-class" does not have 'storagePolicyID' parameter`), + kubeutil.ErrMissingParameter{ + StorageClassName: "my-storage-class", + ParameterName: internal.StoragePolicyIDParameter, + }.Error()), ) +var _ = Describe("SetStoragePolicyID", func() { + var ( + obj storagev1.StorageClass + ) + + BeforeEach(func() { + obj = storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: fakeString, + }, + Parameters: map[string]string{ + internal.StoragePolicyIDParameter: fakeString, + }, + } + }) + + When("storageClass is nil", func() { + It("should panic", func() { + Expect(func() { + kubeutil.SetStoragePolicyID(nil, "") + }).To(PanicWith("storageClass is nil")) + }) + }) + When("id is empty", func() { + It("should remove the policy ID from the StorageClass", func() { + id, err := kubeutil.GetStoragePolicyID(obj) + Expect(err).ToNot(HaveOccurred()) + Expect(id).To(Equal(fakeString)) + + kubeutil.SetStoragePolicyID(&obj, "") + + id, err = kubeutil.GetStoragePolicyID(obj) + Expect(err).To(MatchError(kubeutil.ErrMissingParameter{ + StorageClassName: fakeString, + ParameterName: internal.StoragePolicyIDParameter, + })) + Expect(id).To(BeEmpty()) + }) + }) + When("id is non-empty", func() { + It("should set the policy ID on the StorageClass", func() { + kubeutil.SetStoragePolicyID(&obj, fakeString+"1") + id, err := kubeutil.GetStoragePolicyID(obj) + Expect(err).ToNot(HaveOccurred()) + Expect(id).To(Equal(fakeString + "1")) + }) + }) +}) + var _ = Describe("GetPVCZoneConstraints", func() { It("Unmarshal JSON", func() { @@ -316,7 +368,7 @@ var _ = Describe("IsEncryptedStorageClass", func() { WithInterceptorFuncs(funcs). Build() - ok, err = kubeutil.IsEncryptedStorageClass( + ok, _, err = kubeutil.IsEncryptedStorageClass( ctx, client, storageClass.Name) }) @@ -390,11 +442,11 @@ var _ = Describe("IsEncryptedStorageProfile", func() { UID: types.UID(uuid.NewString()), }, Parameters: map[string]string{ - internal.StoragePolicyIDParameter: "fake", + internal.StoragePolicyIDParameter: fakeString, }, } withObjs = []ctrlclient.Object{&storageClass} - profile = "fake" + profile = fakeString }) JustBeforeEach(func() { @@ -446,7 +498,7 @@ var _ = Describe("IsEncryptedStorageProfile", func() { When("there is a StorageClass but does not match the profile ID", func() { BeforeEach(func() { - profile = "fake1" + profile = fakeString + "1" }) It("should return false", func() { Expect(err).ToNot(HaveOccurred()) @@ -480,6 +532,9 @@ var _ = Describe("EncryptedStorageClass", func() { Name: fakeString, UID: types.UID(uuid.NewString()), }, + Parameters: map[string]string{ + internal.StoragePolicyIDParameter: fakeString, + }, } }) @@ -495,18 +550,20 @@ var _ = Describe("EncryptedStorageClass", func() { Expect(kubeutil.MarkEncryptedStorageClass(ctx, client, storageClass, true)).To(Succeed()) }) It("should return true", func() { - ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + ok, pid, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) Expect(err).ToNot(HaveOccurred()) Expect(ok).To(BeTrue()) + Expect(pid).To(Equal(fakeString)) }) When("the storage class is marked as encrypted again", func() { JustBeforeEach(func() { Expect(kubeutil.MarkEncryptedStorageClass(ctx, client, storageClass, true)).To(Succeed()) }) It("should return true", func() { - ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + ok, pid, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) Expect(err).ToNot(HaveOccurred()) Expect(ok).To(BeTrue()) + Expect(pid).To(Equal(fakeString)) }) }) When("the storage class is marked as unencrypted", func() { @@ -514,9 +571,11 @@ var _ = Describe("EncryptedStorageClass", func() { Expect(kubeutil.MarkEncryptedStorageClass(ctx, client, storageClass, false)).To(Succeed()) }) It("should return false", func() { - ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + ok, pid, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) Expect(err).ToNot(HaveOccurred()) Expect(ok).To(BeFalse()) + Expect(pid).To(BeEmpty()) + }) When("the storage class is marked as encrypted again", func() { @@ -524,9 +583,10 @@ var _ = Describe("EncryptedStorageClass", func() { Expect(kubeutil.MarkEncryptedStorageClass(ctx, client, storageClass, true)).To(Succeed()) }) It("should return true", func() { - ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + ok, pid, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) Expect(err).ToNot(HaveOccurred()) Expect(ok).To(BeTrue()) + Expect(pid).To(Equal(fakeString)) }) }) }) @@ -537,9 +597,10 @@ var _ = Describe("EncryptedStorageClass", func() { Expect(kubeutil.MarkEncryptedStorageClass(ctx, client, storageClass, false)).To(Succeed()) }) It("should return false for the second storage class", func() { - ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + ok, pid, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) Expect(err).ToNot(HaveOccurred()) Expect(ok).To(BeFalse()) + Expect(pid).To(BeEmpty()) }) }) }) @@ -549,17 +610,19 @@ var _ = Describe("EncryptedStorageClass", func() { Expect(kubeutil.MarkEncryptedStorageClass(ctx, client, storageClass, false)).To(Succeed()) }) It("should return false", func() { - ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + ok, pid, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) Expect(err).ToNot(HaveOccurred()) Expect(ok).To(BeFalse()) + Expect(pid).To(BeEmpty()) }) }) When("the storage class is not marked at all", func() { It("should return false", func() { - ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + ok, pid, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) Expect(err).ToNot(HaveOccurred()) Expect(ok).To(BeFalse()) + Expect(pid).To(BeEmpty()) }) }) @@ -580,9 +643,10 @@ var _ = Describe("EncryptedStorageClass", func() { } }) It("should return an error for IsEncryptedStorageClass", func() { - ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + ok, pid, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) Expect(err).To(MatchError(apierrors.NewInternalError(errors.New(fakeString)).Error())) Expect(ok).To(BeFalse()) + Expect(pid).To(BeEmpty()) }) It("should return an error for MarkEncryptedStorageClass", func() { err := kubeutil.MarkEncryptedStorageClass(ctx, client, storageClass, false) @@ -624,6 +688,9 @@ var _ = Describe("EncryptedStorageClass", func() { for i := range storageClasses { storageClasses[i].Name = strconv.Itoa(i) storageClasses[i].Provisioner = fakeString + storageClasses[i].Parameters = map[string]string{ + internal.StoragePolicyIDParameter: fakeString, + } Expect(client.Create(ctx, &storageClasses[i])).To(Succeed()) } }) @@ -683,10 +750,11 @@ var _ = Describe("EncryptedStorageClass", func() { Expect(refs).To(ContainElements(expRefs...)) for i := range storageClasses { - ok, err := kubeutil.IsEncryptedStorageClass( + ok, pid, err := kubeutil.IsEncryptedStorageClass( ctx, client, storageClasses[i].Name) Expect(err).ToNot(HaveOccurred()) Expect(ok).To(BeTrue()) + Expect(pid).To(Equal(fakeString)) } }) }) @@ -733,10 +801,11 @@ var _ = Describe("EncryptedStorageClass", func() { Expect(refs).To(BeEmpty()) for i := range storageClasses { - ok, err := kubeutil.IsEncryptedStorageClass( + ok, pid, err := kubeutil.IsEncryptedStorageClass( ctx, client, storageClasses[i].Name) Expect(err).ToNot(HaveOccurred()) Expect(ok).To(BeFalse()) + Expect(pid).To(BeEmpty()) } }) }) @@ -801,13 +870,15 @@ var _ = Describe("EncryptedStorageClass", func() { Expect(refs).To(ContainElements(expRefs...)) for i := range storageClasses { - ok, err := kubeutil.IsEncryptedStorageClass( + ok, pid, err := kubeutil.IsEncryptedStorageClass( ctx, client, storageClasses[i].Name) Expect(err).ToNot(HaveOccurred()) if i%2 == 0 { Expect(ok).To(BeFalse()) + Expect(pid).To(BeEmpty()) } else { Expect(ok).To(BeTrue()) + Expect(pid).To(Equal(fakeString)) } } }) diff --git a/pkg/vmconfig/crypto/crypto_reconciler_pre.go b/pkg/vmconfig/crypto/crypto_reconciler_pre.go index 6e84b8c49..44f6f37e2 100644 --- a/pkg/vmconfig/crypto/crypto_reconciler_pre.go +++ b/pkg/vmconfig/crypto/crypto_reconciler_pre.go @@ -50,14 +50,25 @@ type reconcileArgs struct { } var ( + // ErrMustUseVTPMOrEncryptedStorageClass is returned by the Reconcile + // function if an EncryptionClass is specified without using an encrypted + // StorageClass or vTPM. ErrMustUseVTPMOrEncryptedStorageClass = errors.New( "must use encrypted StorageClass or have vTPM") + // ErrMustNotUseVTPMOrEncryptedStorageClass is returned by the Reconcile + // function is an encrypted StorageClass or vTPM are used without specifying + // an EncryptionClass or if there is no default key provider. ErrMustNotUseVTPMOrEncryptedStorageClass = errors.New( "vTPM and/or encrypted StorageClass require encryption") + // ErrNoDefaultKeyProvider is returned by the Reconcile function if an + // existing VM is encrypted, no EncryptionClass is specified, and there is + // no default key provider. ErrNoDefaultKeyProvider = errors.New("no default key provider") + // ErrInvalidKeyProvider is returned if the key provider specified by an + // EncryptionClass is invalid. ErrInvalidKeyProvider = errors.New("invalid key provider") ) @@ -113,8 +124,8 @@ func (r reconciler) Reconcile( // ID and key ID. args.curKey = getCurCryptoKey(moVM) - // Check whether or not the storage class is encrypted. - args.isEncStorClass, err = kubeutil.IsEncryptedStorageClass( + // Check whether or not the StorageClass supports encryption. + args.isEncStorClass, _, err = kubeutil.IsEncryptedStorageClass( ctx, k8sClient, vm.Spec.StorageClass) @@ -123,8 +134,8 @@ func (r reconciler) Reconcile( } if paused.ByAdmin(moVM) || paused.ByDevOps(vm) { - // Do not proceed further if the VM is paused. - return nil + // If the VM is paused, just update the status. + return updateStatus(ctx, args, false) } // Record whether the VM has, is adding, or is removing a vTPM. @@ -242,7 +253,7 @@ func (r reconciler) reconcileUpdate( // // Update the VM's status. - return updateStatus(ctx, args) + return updateStatus(ctx, args, true) } func (r reconciler) reconcileUpdateEncryptionClass( @@ -381,7 +392,10 @@ func setConditionAndReturnErr(args reconcileArgs, err error, r Reason) error { return err } -func updateStatus(ctx context.Context, args reconcileArgs) error { +func updateStatus( + ctx context.Context, + args reconcileArgs, + doValidation bool) error { if args.curKey.provider == "" { @@ -406,20 +420,49 @@ func updateStatus(ctx context.Context, args reconcileArgs) error { args.vm.Status.Crypto.ProviderID = args.curKey.provider args.vm.Status.Crypto.KeyID = args.curKey.id - if args.isEncStorClass { - args.vm.Status.Crypto.Encrypted = []vmopv1.VirtualMachineEncryptionType{ - vmopv1.VirtualMachineEncryptionTypeConfig, + // Because the VM has an encryption key, we know the VM's config files / + // home dir are/is encrypted. + args.vm.Status.Crypto.Encrypted = []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + } + + // Check to see if the any of the VM's disks are encrypted. + if args.moVM.Config != nil && areAnyDisksEncrypted(args) { + args.vm.Status.Crypto.Encrypted = append( + args.vm.Status.Crypto.Encrypted, vmopv1.VirtualMachineEncryptionTypeDisks, - } - } else if args.hasVTPM && !args.remVTPM { - args.vm.Status.Crypto.Encrypted = []vmopv1.VirtualMachineEncryptionType{ - vmopv1.VirtualMachineEncryptionTypeConfig, - } + ) + } + + if !doValidation { + return nil } return doOp(ctx, args, doUpdateEncrypted) } +func areAnyDisksEncrypted(args reconcileArgs) bool { + for _, baseDev := range args.moVM.Config.Hardware.Device { + if disk, ok := baseDev.(*vimtypes.VirtualDisk); ok { + switch tBack := disk.Backing.(type) { + case *vimtypes.VirtualDiskFlatVer2BackingInfo: + if tBack.KeyId != nil { + return true + } + case *vimtypes.VirtualDiskSeSparseBackingInfo: + if tBack.KeyId != nil { + return true + } + case *vimtypes.VirtualDiskSparseVer2BackingInfo: + if tBack.KeyId != nil { + return true + } + } + } + } + return false +} + func doOp( ctx context.Context, args reconcileArgs, @@ -571,17 +614,107 @@ func onRecrypt( }, } + recryptedDisks := onRecryptDisks(args) + logger.Info( "Recrypt VM", "currentKeyID", args.curKey.id, "currentProviderID", args.curKey.provider, "newKeyID", args.newKey.id, "newProviderID", args.newKey.provider, - "newProviderIsDefault", args.newKey.isDefaultProvider) + "newProviderIsDefault", args.newKey.isDefaultProvider, + "recryptedDisks", recryptedDisks) return 0, nil, nil } +func onRecryptDisks(args reconcileArgs) []string { + var fileNames []string + for _, baseDev := range args.moVM.Config.Hardware.Device { + if disk, ok := baseDev.(*vimtypes.VirtualDisk); ok { + if disk.VDiskId == nil { // Skip FCDs + + switch tBack := disk.Backing.(type) { + case *vimtypes.VirtualDiskFlatVer2BackingInfo: + if tBack.KeyId != nil { + if updateDiskBackingForRecrypt(args, disk) { + fileNames = append(fileNames, tBack.FileName) + } + } + case *vimtypes.VirtualDiskSeSparseBackingInfo: + if tBack.KeyId != nil { + if updateDiskBackingForRecrypt(args, disk) { + fileNames = append(fileNames, tBack.FileName) + } + } + case *vimtypes.VirtualDiskSparseVer2BackingInfo: + if tBack.KeyId != nil { + if updateDiskBackingForRecrypt(args, disk) { + fileNames = append(fileNames, tBack.FileName) + } + } + } + } + } + } + return fileNames +} + +func updateDiskBackingForRecrypt( + args reconcileArgs, + disk *vimtypes.VirtualDisk) bool { + + devSpec := getOrCreateDeviceChangeForDisk(args, disk) + if devSpec == nil { + return false + } + + if devSpec.Backing == nil { + devSpec.Backing = &vimtypes.VirtualDeviceConfigSpecBackingSpec{} + } + + // Set the device change's crypto spec to be the same as the VM's. + devSpec.Backing.Crypto = args.configSpec.Crypto + + return true +} + +// getOrCreateDeviceChangeForDisk returns the device change for the specified +// disk. If there is an existing Edit change, that is returned. If there is an +// existing Add/Remove change, nil is returned. Otherwise a new Edit change is +// returned. +func getOrCreateDeviceChangeForDisk( + args reconcileArgs, + disk *vimtypes.VirtualDisk) *vimtypes.VirtualDeviceConfigSpec { + + for i := range args.configSpec.DeviceChange { + if dc := args.configSpec.DeviceChange[i]; dc != nil { + if ds := dc.GetVirtualDeviceConfigSpec(); ds != nil { + if bd := ds.Device; bd != nil { + if vd := bd.GetVirtualDevice(); vd != nil { + if vd.Key == disk.Key { + if ds.Operation == vimtypes.VirtualDeviceConfigSpecOperationEdit { + return ds + } + return nil + } + } + } + } + } + } + + ds := &vimtypes.VirtualDeviceConfigSpec{ + Device: disk, + Operation: vimtypes.VirtualDeviceConfigSpecOperationEdit, + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{}, + } + + args.configSpec.DeviceChange = append(args.configSpec.DeviceChange, ds) + + return ds +} + func validateEncrypt( ctx context.Context, args reconcileArgs) (Reason, []string, error) { @@ -797,9 +930,14 @@ func isAddEditDeviceSpecEncryptedSansPolicy( if backing := spec.Backing; backing != nil { switch backing.Crypto.(type) { + + case *vimtypes.CryptoSpecShallowRecrypt: + // shallowRecrypt sans policy is supported + case *vimtypes.CryptoSpecEncrypt, - *vimtypes.CryptoSpecDeepRecrypt, - *vimtypes.CryptoSpecShallowRecrypt: + *vimtypes.CryptoSpecDeepRecrypt: + + // encrypt/deepRecrypt require policy for i := range spec.Profile { if ok, err := isEncryptedProfile( @@ -811,11 +949,13 @@ func isAddEditDeviceSpecEncryptedSansPolicy( } else if ok { - return false, nil // is encrypted/recrypted with policy + // encrypt/deepRecrypt with policy + return false, nil } } - return true, nil // is encrypted/recrypted sans policy + // encrypt/deepRecrypt with policy + return true, nil } } } diff --git a/pkg/vmconfig/crypto/crypto_reconciler_pre_test.go b/pkg/vmconfig/crypto/crypto_reconciler_pre_test.go index c4a6a3081..308956ac1 100644 --- a/pkg/vmconfig/crypto/crypto_reconciler_pre_test.go +++ b/pkg/vmconfig/crypto/crypto_reconciler_pre_test.go @@ -274,7 +274,7 @@ var _ = Describe("Reconcile", Label(testlabels.Crypto), func() { When("spec.crypto.encryptionClassName is non-empty", func() { When("the EncryptionClass does not exit", func() { BeforeEach(func() { - vm.Spec.Crypto.EncryptionClassName += "-fake" + vm.Spec.Crypto.EncryptionClassName += fakeString }) It("should return an error", func() { Expect(err).To(HaveOccurred()) @@ -296,25 +296,25 @@ var _ = Describe("Reconcile", Label(testlabels.Crypto), func() { opts ...ctrlclient.GetOption) error { if _, ok := obj.(*byokv1.EncryptionClass); ok { - return errors.New("fake") + return errors.New(fakeString) } return client.Get(ctx, key, obj, opts...) } }) It("should return an error", func() { Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError("fake")) + Expect(err).To(MatchError(fakeString)) c := conditions.Get(vm, vmopv1.VirtualMachineEncryptionSynced) Expect(c).ToNot(BeNil()) Expect(c.Status).To(Equal(metav1.ConditionFalse)) Expect(c.Reason).To(Equal(pkgcrypto.ReasonInternalError.String())) - Expect(c.Message).To(Equal("fake")) + Expect(c.Message).To(Equal(fakeString)) }) }) When("the EncryptionClass specifies an invalid provider", func() { BeforeEach(func() { - encClass.Spec.KeyProvider = "fake" + encClass.Spec.KeyProvider = fakeString }) It("should return an error", func() { Expect(err).To(HaveOccurred()) @@ -652,6 +652,104 @@ var _ = Describe("Reconcile", Label(testlabels.Crypto), func() { Expect(c.Status).To(Equal(metav1.ConditionFalse)) Expect(c.Reason).To(Equal(pkgcrypto.ReasonNoDefaultKeyProvider.String())) }) + + When("vm is paused", func() { + Context("by admin", func() { + BeforeEach(func() { + moVM.Config.Hardware.Device = []vimtypes.BaseVirtualDevice{ + &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Backing: &vimtypes.VirtualDiskFlatVer2BackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + }, + } + moVM.Config.ExtraConfig = []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{ + Key: vmopv1.PauseVMExtraConfigKey, + Value: "true", + }, + } + }) + It("should update the status without returning an error", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + Expect(vm.Status.Crypto).To(Equal(&vmopv1.VirtualMachineCryptoStatus{ + Encrypted: []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + vmopv1.VirtualMachineEncryptionTypeDisks, + }, + ProviderID: provider1ID, + KeyID: provider1Key1ID, + })) + }) + }) + + Context("by devops", func() { + BeforeEach(func() { + moVM.Config.Hardware.Device = []vimtypes.BaseVirtualDevice{ + &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Backing: &vimtypes.VirtualDiskSeSparseBackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + }, + } + vm.Annotations = map[string]string{ + vmopv1.PauseAnnotation: "", + } + }) + It("should update the status without returning an error", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + Expect(vm.Status.Crypto).To(Equal(&vmopv1.VirtualMachineCryptoStatus{ + Encrypted: []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + vmopv1.VirtualMachineEncryptionTypeDisks, + }, + ProviderID: provider1ID, + KeyID: provider1Key1ID, + })) + }) + }) + + Context("by admin and devops", func() { + BeforeEach(func() { + moVM.Config.Hardware.Device = []vimtypes.BaseVirtualDevice{ + &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Backing: &vimtypes.VirtualDiskSparseVer2BackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + }, + } + moVM.Config.ExtraConfig = []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{ + Key: vmopv1.PauseVMExtraConfigKey, + Value: "true", + }, + } + vm.Annotations = map[string]string{ + vmopv1.PauseAnnotation: "", + } + }) + It("should update the status without returning an error", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + Expect(vm.Status.Crypto).To(Equal(&vmopv1.VirtualMachineCryptoStatus{ + Encrypted: []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + vmopv1.VirtualMachineEncryptionTypeDisks, + }, + ProviderID: provider1ID, + KeyID: provider1Key1ID, + })) + }) + }) + }) }) When("the vm is not encrypted", func() { @@ -673,7 +771,7 @@ var _ = Describe("Reconcile", Label(testlabels.Crypto), func() { When("spec.crypto.encryptionClassName is non-empty", func() { When("the EncryptionClass does not exit", func() { BeforeEach(func() { - vm.Spec.Crypto.EncryptionClassName += "-fake" + vm.Spec.Crypto.EncryptionClassName += fakeString }) It("should return an error", func() { Expect(err).To(HaveOccurred()) @@ -741,7 +839,7 @@ var _ = Describe("Reconcile", Label(testlabels.Crypto), func() { Operation: vimtypes.VirtualDeviceConfigSpecOperationAdd, Profile: []vimtypes.BaseVirtualMachineProfileSpec{ &vimtypes.VirtualMachineDefinedProfileSpec{ - ProfileId: "fake", + ProfileId: fakeString, }, }, }, @@ -1053,6 +1151,191 @@ var _ = Describe("Reconcile", Label(testlabels.Crypto), func() { }) }) + When("there are encrypted disks", func() { + BeforeEach(func() { + moVM.Config.Hardware.Device = []vimtypes.BaseVirtualDevice{ + &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 1, + Backing: &vimtypes.VirtualDiskFlatVer2BackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + }, + &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 2, + Backing: &vimtypes.VirtualDiskSeSparseBackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + }, + &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 3, + Backing: &vimtypes.VirtualDiskSparseVer2BackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + VDiskId: &vimtypes.ID{}, // FCD + }, + &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 4, + Backing: &vimtypes.VirtualDiskSparseVer2BackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + }, + &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 5, + Backing: &vimtypes.VirtualDiskSparseVer2BackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + }, + } + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Device: &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 2, + Backing: &vimtypes.VirtualDiskSeSparseBackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + CapacityInBytes: 100, + }, + Operation: vimtypes.VirtualDeviceConfigSpecOperationEdit, + }, + &vimtypes.VirtualDeviceConfigSpec{ + Device: &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 5, + Backing: &vimtypes.VirtualDiskSeSparseBackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + }, + Operation: vimtypes.VirtualDeviceConfigSpecOperationRemove, + }, + } + }) + It("should recrypt the vm and the disks", func() { + Expect(err).ToNot(HaveOccurred()) + c := conditions.Get(vm, vmopv1.VirtualMachineEncryptionSynced) + Expect(c).To(BeNil()) + cryptoSpec, ok := configSpec.Crypto.(*vimtypes.CryptoSpecShallowRecrypt) + Expect(ok).To(BeTrue()) + Expect(cryptoSpec.NewKeyId.KeyId).To(Equal(provider1Key1ID)) + Expect(cryptoSpec.NewKeyId.ProviderId.Id).To(Equal(provider1ID)) + Expect(configSpec.DeviceChange).To(HaveLen(4)) + + Expect(configSpec.DeviceChange).To(ConsistOf( + &vimtypes.VirtualDeviceConfigSpec{ + Device: &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 1, + Backing: &vimtypes.VirtualDiskFlatVer2BackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + }, + Operation: vimtypes.VirtualDeviceConfigSpecOperationEdit, + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: configSpec.Crypto, + }, + }, + &vimtypes.VirtualDeviceConfigSpec{ + Device: &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 2, + Backing: &vimtypes.VirtualDiskSeSparseBackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + CapacityInBytes: 100, + }, + Operation: vimtypes.VirtualDeviceConfigSpecOperationEdit, + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: configSpec.Crypto, + }, + }, + &vimtypes.VirtualDeviceConfigSpec{ + Device: &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 4, + Backing: &vimtypes.VirtualDiskSparseVer2BackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + }, + Operation: vimtypes.VirtualDeviceConfigSpecOperationEdit, + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: configSpec.Crypto, + }, + }, + &vimtypes.VirtualDeviceConfigSpec{ + Device: &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 5, + Backing: &vimtypes.VirtualDiskSeSparseBackingInfo{ + KeyId: &vimtypes.CryptoKeyId{}, + }, + }, + }, + Operation: vimtypes.VirtualDeviceConfigSpecOperationRemove, + }, + )) + }) + }) + + When("shallow recrypting devices sans policy", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: &vimtypes.CryptoSpecShallowRecrypt{}, + }, + Device: &vimtypes.VirtualDisk{}, + Operation: vimtypes.VirtualDeviceConfigSpecOperationEdit, + }, + } + }) + It("should recrypt the vm", func() { + Expect(err).ToNot(HaveOccurred()) + c := conditions.Get(vm, vmopv1.VirtualMachineEncryptionSynced) + Expect(c).To(BeNil()) + cryptoSpec, ok := configSpec.Crypto.(*vimtypes.CryptoSpecShallowRecrypt) + Expect(ok).To(BeTrue()) + Expect(cryptoSpec.NewKeyId.KeyId).To(Equal(provider1Key1ID)) + Expect(cryptoSpec.NewKeyId.ProviderId.Id).To(Equal(provider1ID)) + }) + }) + + When("deep recrypting devices sans policy", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: &vimtypes.CryptoSpecDeepRecrypt{}, + }, + Device: &vimtypes.VirtualDisk{}, + Operation: vimtypes.VirtualDeviceConfigSpecOperationEdit, + }, + } + }) + It("should set EncryptionSynced=false with InvalidChanges", func() { + Expect(err).ToNot(HaveOccurred()) + c := conditions.Get(vm, vmopv1.VirtualMachineEncryptionSynced) + Expect(c).ToNot(BeNil()) + Expect(c.Status).To(Equal(metav1.ConditionFalse)) + Expect(c.Reason).To(Equal(pkgcrypto.ReasonInvalidChanges.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("recrypting", "specify policy when encrypting devices"))) + }) + }) + When("adding encrypted devices sans policy", func() { BeforeEach(func() { configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ diff --git a/pkg/vmconfig/crypto/crypto_reconciler_suite_test.go b/pkg/vmconfig/crypto/crypto_reconciler_suite_test.go index 6ff3e2565..98342b888 100644 --- a/pkg/vmconfig/crypto/crypto_reconciler_suite_test.go +++ b/pkg/vmconfig/crypto/crypto_reconciler_suite_test.go @@ -13,6 +13,8 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" ) +const fakeString = "fake" + func init() { klog.SetOutput(GinkgoWriter) logf.SetLogger(klog.Background()) diff --git a/webhooks/virtualmachine/mutation/virtualmachine_mutator.go b/webhooks/virtualmachine/mutation/virtualmachine_mutator.go index 59329a370..547170d6f 100644 --- a/webhooks/virtualmachine/mutation/virtualmachine_mutator.go +++ b/webhooks/virtualmachine/mutation/virtualmachine_mutator.go @@ -577,7 +577,7 @@ func SetDefaultEncryptionClass( } // Check if the VM's storage class supports encryption. - if ok, err := kubeutil.IsEncryptedStorageClass( + if ok, _, err := kubeutil.IsEncryptedStorageClass( ctx, k8sClient, vm.Spec.StorageClass); err != nil { diff --git a/webhooks/virtualmachine/mutation/virtualmachine_mutator_intg_test.go b/webhooks/virtualmachine/mutation/virtualmachine_mutator_intg_test.go index 57427d19d..2da0fc020 100644 --- a/webhooks/virtualmachine/mutation/virtualmachine_mutator_intg_test.go +++ b/webhooks/virtualmachine/mutation/virtualmachine_mutator_intg_test.go @@ -597,8 +597,9 @@ func intgTestsMutating() { ObjectMeta: metav1.ObjectMeta{ Name: storClassName, }, - Provisioner: "fake", + Provisioner: fakeString, } + kubeutil.SetStoragePolicyID(&storClass, fakeString) ctx.vm.Spec.StorageClass = storClass.Name diff --git a/webhooks/virtualmachine/validation/virtualmachine_validator.go b/webhooks/virtualmachine/validation/virtualmachine_validator.go index 42610e34a..547cf3253 100644 --- a/webhooks/virtualmachine/validation/virtualmachine_validator.go +++ b/webhooks/virtualmachine/validation/virtualmachine_validator.go @@ -517,7 +517,7 @@ func (v validator) validateCrypto( encClassNamePath = cryptoPath.Child("encryptionClassName") ) - if ok, err := kubeutil.IsEncryptedStorageClass( + if ok, _, err := kubeutil.IsEncryptedStorageClass( ctx, v.client, vm.Spec.StorageClass); err != nil {