From 7ef63917993740de97ed9aefd1a3e76115646b8b Mon Sep 17 00:00:00 2001 From: akutz Date: Thu, 26 Sep 2024 10:32:56 -0500 Subject: [PATCH] Support Bring Your Own (Encryption) Key (BYOK) This patch adds support for bringing your own encryption key used to encrypt/recrypt VMs. --- .gitignore | 3 + api/v1alpha1/virtualmachine_conversion.go | 5 + .../virtualmachine_conversion_test.go | 65 + api/v1alpha1/zz_generated.conversion.go | 2 + api/v1alpha2/virtualmachine_conversion.go | 5 + .../virtualmachine_conversion_test.go | 65 + api/v1alpha2/zz_generated.conversion.go | 2 + api/v1alpha3/virtualmachine_types.go | 103 ++ api/v1alpha3/zz_generated.deepcopy.go | 46 + ....vmware.com_virtualmachinereplicasets.yaml | 58 + ...vmoperator.vmware.com_virtualmachines.yaml | 83 ++ ...cryption.vmware.com_encryptionclasses.yaml | 6 +- controllers/controllers.go | 7 + .../storageclass/storageclass_controller.go | 110 ++ .../storageclass_controller_intg_test.go | 78 ++ .../storageclass_controller_suite_test.go | 52 + .../storageclass_controller_unit_test.go | 145 ++ .../virtualmachine_controller.go | 64 + docs/concepts/workloads/vm.md | 115 ++ .../api/v1alpha1/encryptionclass_types.go | 5 +- go.mod | 4 +- go.sum | 8 +- main.go | 16 +- pkg/bitmask/bitmask.go | 73 + pkg/bitmask/bitmask_suite_test.go | 16 + pkg/bitmask/bitmask_test.go | 205 +++ pkg/constants/testlabels/test_labels.go | 3 + pkg/providers/fake/fake_vm_provider.go | 15 + pkg/providers/vm_provider_interface.go | 5 + pkg/providers/vsphere/client/client_test.go | 1 + .../vsphere/session/session_vm_update.go | 139 +- .../vsphere/session/session_vm_update_test.go | 3 +- pkg/providers/vsphere/storage/storageclass.go | 17 +- .../vsphere/virtualmachine/delete.go | 3 +- .../vsphere/virtualmachine/extraconfig.go | 26 - .../virtualmachine/extraconfig_test.go | 88 -- .../virtualmachine_suite_test.go | 1 - .../vsphere/vmlifecycle/update_status.go | 32 +- .../vsphere/vmlifecycle/update_status_test.go | 16 +- pkg/providers/vsphere/vmprovider.go | 13 + pkg/providers/vsphere/vmprovider_vm.go | 23 +- pkg/providers/vsphere/vmprovider_vm_test.go | 390 +++++- pkg/util/configspec.go | 34 +- pkg/util/configspec_test.go | 2 +- .../kube/internal/internal_kube_constants.go | 30 + .../kube/internal/internal_kube_storage.go | 63 + .../internal/internal_kube_storage_test.go | 159 +++ .../kube/internal/internal_kube_suite_test.go | 24 + pkg/util/kube/kube_suite_test.go | 2 + pkg/util/kube/storage.go | 154 +++ pkg/util/kube/storage_test.go | 497 +++++++ pkg/util/paused/paused.go | 24 + pkg/util/paused/paused_suite_test.go | 24 + pkg/util/paused/paused_test.go | 110 ++ pkg/util/vsphere/client/client.go | 12 + pkg/util/vsphere/client/client_test.go | 1 + pkg/vmconfig/crypto/crypto_reconciler.go | 109 ++ pkg/vmconfig/crypto/crypto_reconciler_post.go | 192 +++ .../crypto/crypto_reconciler_post_test.go | 676 ++++++++++ pkg/vmconfig/crypto/crypto_reconciler_pre.go | 690 ++++++++++ .../crypto/crypto_reconciler_pre_test.go | 1176 +++++++++++++++++ .../crypto/crypto_reconciler_suite_test.go | 24 + pkg/vmconfig/crypto/crypto_reconciler_test.go | 24 + .../internal/crypto_reconciler_context.go | 48 + pkg/vmconfig/vmconfig_reconciler.go | 51 + pkg/vmconfig/vmconfig_reconciler_context.go | 86 ++ .../vmconfig_reconciler_context_test.go | 233 ++++ .../vmconfig_reconciler_suite_test.go | 24 + test/builder/dummies.go | 12 +- test/builder/unit_test_context.go | 1 + test/builder/vcsim_test_context.go | 149 ++- test/testutil/util.go | 29 + .../validation/virtualmachine_validator.go | 62 +- .../virtualmachine_validator_unit_test.go | 187 +++ 74 files changed, 6745 insertions(+), 280 deletions(-) create mode 100644 controllers/storageclass/storageclass_controller.go create mode 100644 controllers/storageclass/storageclass_controller_intg_test.go create mode 100644 controllers/storageclass/storageclass_controller_suite_test.go create mode 100644 controllers/storageclass/storageclass_controller_unit_test.go create mode 100644 pkg/bitmask/bitmask.go create mode 100644 pkg/bitmask/bitmask_suite_test.go create mode 100644 pkg/bitmask/bitmask_test.go delete mode 100644 pkg/providers/vsphere/virtualmachine/extraconfig.go delete mode 100644 pkg/providers/vsphere/virtualmachine/extraconfig_test.go create mode 100644 pkg/util/kube/internal/internal_kube_constants.go create mode 100644 pkg/util/kube/internal/internal_kube_storage.go create mode 100644 pkg/util/kube/internal/internal_kube_storage_test.go create mode 100644 pkg/util/kube/internal/internal_kube_suite_test.go create mode 100644 pkg/util/kube/storage.go create mode 100644 pkg/util/kube/storage_test.go create mode 100644 pkg/util/paused/paused.go create mode 100644 pkg/util/paused/paused_suite_test.go create mode 100644 pkg/util/paused/paused_test.go create mode 100644 pkg/vmconfig/crypto/crypto_reconciler.go create mode 100644 pkg/vmconfig/crypto/crypto_reconciler_post.go create mode 100644 pkg/vmconfig/crypto/crypto_reconciler_post_test.go create mode 100644 pkg/vmconfig/crypto/crypto_reconciler_pre.go create mode 100644 pkg/vmconfig/crypto/crypto_reconciler_pre_test.go create mode 100644 pkg/vmconfig/crypto/crypto_reconciler_suite_test.go create mode 100644 pkg/vmconfig/crypto/crypto_reconciler_test.go create mode 100644 pkg/vmconfig/crypto/internal/crypto_reconciler_context.go create mode 100644 pkg/vmconfig/vmconfig_reconciler.go create mode 100644 pkg/vmconfig/vmconfig_reconciler_context.go create mode 100644 pkg/vmconfig/vmconfig_reconciler_context_test.go create mode 100644 pkg/vmconfig/vmconfig_reconciler_suite_test.go diff --git a/.gitignore b/.gitignore index 9b7a12a36..9ce7dc2bb 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,9 @@ config/crd/external-crds/cns.vmware.com_* !config/crd/external-crds/cns.vmware.com_storagepolicyquotas.yaml !config/crd/external-crds/cns.vmware.com_storagepolicyusages.yaml +# Created by the VSCode ginkgo plug-in +ginkgo.report + .DS_Store .cache diff --git a/api/v1alpha1/virtualmachine_conversion.go b/api/v1alpha1/virtualmachine_conversion.go index ef2a6e9e0..4e23a2022 100644 --- a/api/v1alpha1/virtualmachine_conversion.go +++ b/api/v1alpha1/virtualmachine_conversion.go @@ -822,6 +822,10 @@ func Convert_v1alpha3_VirtualMachineStatus_To_v1alpha1_VirtualMachineStatus( return nil } +func restore_v1alpha3_VirtualMachineCryptoSpec(dst, src *vmopv1.VirtualMachine) { + dst.Spec.Crypto = src.Spec.Crypto +} + func restore_v1alpha3_VirtualMachineImage(dst, src *vmopv1.VirtualMachine) { dst.Spec.Image = src.Spec.Image dst.Spec.ImageName = src.Spec.ImageName @@ -1239,6 +1243,7 @@ func (src *VirtualMachine) ConvertTo(dstRaw ctrlconversion.Hub) error { restore_v1alpha3_VirtualMachineInstanceUUID(dst, restored) restore_v1alpha3_VirtualMachineGuestID(dst, restored) restore_v1alpha3_VirtualMachineCdrom(dst, restored) + restore_v1alpha3_VirtualMachineCryptoSpec(dst, restored) // END RESTORE diff --git a/api/v1alpha1/virtualmachine_conversion_test.go b/api/v1alpha1/virtualmachine_conversion_test.go index fa7b997b8..a0e949c60 100644 --- a/api/v1alpha1/virtualmachine_conversion_test.go +++ b/api/v1alpha1/virtualmachine_conversion_test.go @@ -1702,4 +1702,69 @@ func TestVirtualMachineConversion(t *testing.T) { g.Expect(apiequality.Semantic.DeepEqual(hub, expectedHub)).To(BeTrue(), cmp.Diff(hub, expectedHub)) }) }) + + t.Run("VirtualMachine and spec.crypto", func(t *testing.T) { + + t.Run("hub-spoke-hub", func(t *testing.T) { + + t.Run("spec.crypto is empty", func(t *testing.T) { + g := NewWithT(t) + hub := vmopv1.VirtualMachine{ + Spec: vmopv1.VirtualMachineSpec{ + Crypto: vmopv1.VirtualMachineCryptoSpec{}, + }, + } + hubSpokeHub(g, &hub, &vmopv1a1.VirtualMachine{}) + }) + + t.Run("spec.crypto.className is non-empty", func(t *testing.T) { + g := NewWithT(t) + hub := vmopv1.VirtualMachine{ + Spec: vmopv1.VirtualMachineSpec{ + Crypto: vmopv1.VirtualMachineCryptoSpec{ + EncryptionClassName: "fake", + }, + }, + } + hubSpokeHub(g, &hub, &vmopv1a1.VirtualMachine{}) + }) + + t.Run("spec.crypto.useDefaultKeyProvider is &true", func(t *testing.T) { + g := NewWithT(t) + hub := vmopv1.VirtualMachine{ + Spec: vmopv1.VirtualMachineSpec{ + Crypto: vmopv1.VirtualMachineCryptoSpec{ + UseDefaultKeyProvider: &[]bool{true}[0], + }, + }, + } + hubSpokeHub(g, &hub, &vmopv1a1.VirtualMachine{}) + }) + + t.Run("spec.crypto.useDefaultKeyProvider is &false", func(t *testing.T) { + g := NewWithT(t) + hub := vmopv1.VirtualMachine{ + Spec: vmopv1.VirtualMachineSpec{ + Crypto: vmopv1.VirtualMachineCryptoSpec{ + UseDefaultKeyProvider: &[]bool{false}[0], + }, + }, + } + hubSpokeHub(g, &hub, &vmopv1a1.VirtualMachine{}) + }) + + t.Run("spec.crypto is completely filled out", func(t *testing.T) { + g := NewWithT(t) + hub := vmopv1.VirtualMachine{ + Spec: vmopv1.VirtualMachineSpec{ + Crypto: vmopv1.VirtualMachineCryptoSpec{ + EncryptionClassName: "fake", + UseDefaultKeyProvider: &[]bool{false}[0], + }, + }, + } + hubSpokeHub(g, &hub, &vmopv1a1.VirtualMachine{}) + }) + }) + }) } diff --git a/api/v1alpha1/zz_generated.conversion.go b/api/v1alpha1/zz_generated.conversion.go index 704cb6ad1..ea38fa558 100644 --- a/api/v1alpha1/zz_generated.conversion.go +++ b/api/v1alpha1/zz_generated.conversion.go @@ -2052,6 +2052,7 @@ func autoConvert_v1alpha3_VirtualMachineSpec_To_v1alpha1_VirtualMachineSpec(in * // WARNING: in.Image requires manual conversion: does not exist in peer-type out.ImageName = in.ImageName out.ClassName = in.ClassName + // WARNING: in.Crypto requires manual conversion: does not exist in peer-type out.StorageClass = in.StorageClass // WARNING: in.Bootstrap requires manual conversion: does not exist in peer-type // WARNING: in.Network requires manual conversion: does not exist in peer-type @@ -2142,6 +2143,7 @@ func autoConvert_v1alpha3_VirtualMachineStatus_To_v1alpha1_VirtualMachineStatus( } else { out.Conditions = nil } + // WARNING: in.Crypto requires manual conversion: does not exist in peer-type // WARNING: in.Network requires manual conversion: does not exist in peer-type out.UniqueID = in.UniqueID out.BiosUUID = in.BiosUUID diff --git a/api/v1alpha2/virtualmachine_conversion.go b/api/v1alpha2/virtualmachine_conversion.go index f2c099557..2e74ba427 100644 --- a/api/v1alpha2/virtualmachine_conversion.go +++ b/api/v1alpha2/virtualmachine_conversion.go @@ -142,6 +142,10 @@ func Convert_v1alpha3_VirtualMachine_To_v1alpha2_VirtualMachine( return nil } +func restore_v1alpha3_VirtualMachineCryptoSpec(dst, src *vmopv1.VirtualMachine) { + dst.Spec.Crypto = src.Spec.Crypto +} + func restore_v1alpha3_VirtualMachineImage(dst, src *vmopv1.VirtualMachine) { dst.Spec.Image = src.Spec.Image dst.Spec.ImageName = src.Spec.ImageName @@ -293,6 +297,7 @@ func (src *VirtualMachine) ConvertTo(dstRaw ctrlconversion.Hub) error { restore_v1alpha3_VirtualMachineSpecNetworkDomainName(dst, restored) restore_v1alpha3_VirtualMachineGuestID(dst, restored) restore_v1alpha3_VirtualMachineCdrom(dst, restored) + restore_v1alpha3_VirtualMachineCryptoSpec(dst, restored) // END RESTORE diff --git a/api/v1alpha2/virtualmachine_conversion_test.go b/api/v1alpha2/virtualmachine_conversion_test.go index d3cae17f7..b5c667b89 100644 --- a/api/v1alpha2/virtualmachine_conversion_test.go +++ b/api/v1alpha2/virtualmachine_conversion_test.go @@ -607,4 +607,69 @@ func TestVirtualMachineConversion(t *testing.T) { hubSpokeHub(g, &hub, &vmopv1.VirtualMachine{}, &vmopv1a2.VirtualMachine{}) }) }) + + t.Run("VirtualMachine and spec.crypto", func(t *testing.T) { + + t.Run("hub-spoke-hub", func(t *testing.T) { + + t.Run("spec.crypto is empty", func(t *testing.T) { + g := NewWithT(t) + hub := vmopv1.VirtualMachine{ + Spec: vmopv1.VirtualMachineSpec{ + Crypto: vmopv1.VirtualMachineCryptoSpec{}, + }, + } + hubSpokeHub(g, &hub, &vmopv1.VirtualMachine{}, &vmopv1a2.VirtualMachine{}) + }) + + t.Run("spec.crypto.className is non-empty", func(t *testing.T) { + g := NewWithT(t) + hub := vmopv1.VirtualMachine{ + Spec: vmopv1.VirtualMachineSpec{ + Crypto: vmopv1.VirtualMachineCryptoSpec{ + EncryptionClassName: "fake", + }, + }, + } + hubSpokeHub(g, &hub, &vmopv1.VirtualMachine{}, &vmopv1a2.VirtualMachine{}) + }) + + t.Run("spec.crypto.useDefaultKeyProvider is &true", func(t *testing.T) { + g := NewWithT(t) + hub := vmopv1.VirtualMachine{ + Spec: vmopv1.VirtualMachineSpec{ + Crypto: vmopv1.VirtualMachineCryptoSpec{ + UseDefaultKeyProvider: &[]bool{true}[0], + }, + }, + } + hubSpokeHub(g, &hub, &vmopv1.VirtualMachine{}, &vmopv1a2.VirtualMachine{}) + }) + + t.Run("spec.crypto.useDefaultKeyProvider is &false", func(t *testing.T) { + g := NewWithT(t) + hub := vmopv1.VirtualMachine{ + Spec: vmopv1.VirtualMachineSpec{ + Crypto: vmopv1.VirtualMachineCryptoSpec{ + UseDefaultKeyProvider: &[]bool{false}[0], + }, + }, + } + hubSpokeHub(g, &hub, &vmopv1.VirtualMachine{}, &vmopv1a2.VirtualMachine{}) + }) + + t.Run("spec.crypto is completely filled out", func(t *testing.T) { + g := NewWithT(t) + hub := vmopv1.VirtualMachine{ + Spec: vmopv1.VirtualMachineSpec{ + Crypto: vmopv1.VirtualMachineCryptoSpec{ + EncryptionClassName: "fake", + UseDefaultKeyProvider: &[]bool{false}[0], + }, + }, + } + hubSpokeHub(g, &hub, &vmopv1.VirtualMachine{}, &vmopv1a2.VirtualMachine{}) + }) + }) + }) } diff --git a/api/v1alpha2/zz_generated.conversion.go b/api/v1alpha2/zz_generated.conversion.go index 91c61df5e..37a357e6e 100644 --- a/api/v1alpha2/zz_generated.conversion.go +++ b/api/v1alpha2/zz_generated.conversion.go @@ -3175,6 +3175,7 @@ func autoConvert_v1alpha3_VirtualMachineSpec_To_v1alpha2_VirtualMachineSpec(in * // WARNING: in.Image requires manual conversion: does not exist in peer-type out.ImageName = in.ImageName out.ClassName = in.ClassName + // WARNING: in.Crypto requires manual conversion: does not exist in peer-type out.StorageClass = in.StorageClass if in.Bootstrap != nil { in, out := &in.Bootstrap, &out.Bootstrap @@ -3251,6 +3252,7 @@ func autoConvert_v1alpha3_VirtualMachineStatus_To_v1alpha2_VirtualMachineStatus( out.Host = in.Host out.PowerState = VirtualMachinePowerState(in.PowerState) out.Conditions = *(*[]v1.Condition)(unsafe.Pointer(&in.Conditions)) + // WARNING: in.Crypto requires manual conversion: does not exist in peer-type if in.Network != nil { in, out := &in.Network, &out.Network *out = new(VirtualMachineNetworkStatus) diff --git a/api/v1alpha3/virtualmachine_types.go b/api/v1alpha3/virtualmachine_types.go index 1c8ca0c31..3b5004098 100644 --- a/api/v1alpha3/virtualmachine_types.go +++ b/api/v1alpha3/virtualmachine_types.go @@ -41,6 +41,10 @@ const ( // VirtualMachineConditionPlacementReady indicates that the placement decision for the VM is ready. VirtualMachineConditionPlacementReady = "VirtualMachineConditionPlacementReady" + // VirtualMachineEncryptionSynced indicates that the VirtualMachine's + // encryption state is synced to the desired encryption state. + VirtualMachineEncryptionSynced = "VirtualMachineEncryptionSynced" + // VirtualMachineConditionCreated indicates that the VM has been created. VirtualMachineConditionCreated = "VirtualMachineCreated" @@ -357,6 +361,65 @@ type VirtualMachineCdromSpec struct { AllowGuestControl *bool `json:"allowGuestControl,omitempty"` } +// VirtualMachineCryptoSpec defines the desired state of a VirtualMachine's +// encryption state. +type VirtualMachineCryptoSpec struct { + // +optional + + // EncryptionClassName describes the name of the EncryptionClass resource + // used to encrypt this VM. + // + // Please note, this field is not required to encrypt the VM. If the + // underlying platform has a default key provider, the VM may still be fully + // or partially encrypted depending on the specified storage and VM classes. + // + // If there is a default key provider and an encryption storage class is + // selected, the VM's home files and non-PVC disks will be encrypted. + // + // If there is a default key provider and a and a VM Class with a virtual, + // trusted platform module (vTPM) is selected, the VM's home files will be + // encrypted. + // + // If the underlying vSphere platform does not have a default key provider, + // then this field is required when specifying an encryption storage class + // and/or a VM Class with a vTPM. + EncryptionClassName string `json:"encryptionClassName,omitempty"` + + // +optional + // +kubebuilder:default=true + + // UseDefaultKeyProvider describes the desired behavior for when an explicit + // EncryptionClass is not provided. + // + // When an explicit EncryptionClass is not provided and this value is true: + // + // - Deploying a VirtualMachine with an encryption storage policy or vTPM + // will be encrypted using the default key provider. + // + // - If a VirtualMachine is not encrypted, uses an encryption storage + // policy or has a virtual, trusted platform module (vTPM), there is a + // default key provider, the VM will be encrypted using the default key + // provider. + // + // - If a VirtualMachine is encrypted with a provider other than the default + // key provider, the VM will be rekeyed using the default key provider. + // + // When an explicit EncryptionClass is not provided and this value is false: + // + // - Deploying a VirtualMachine with an encryption storage policy or vTPM + // will fail. + // + // - If a VirtualMachine is encrypted with a provider other than the default + // key provider, the VM will be not be rekeyed. + // + // Please note, this could result in a VirtualMachine that cannot be + // powered on since it is encrypted using a provider that may have been + // removed. + // + // Defaults to true if omitted. + UseDefaultKeyProvider *bool `json:"useDefaultKeyProvider,omitempty"` +} + // VirtualMachineSpec defines the desired state of a VirtualMachine. type VirtualMachineSpec struct { // +optional @@ -439,6 +502,11 @@ type VirtualMachineSpec struct { // +optional + // Crypto describes the desired encryption state of the VirtualMachine. + Crypto VirtualMachineCryptoSpec `json:"crypto,omitempty"` + + // +optional + // StorageClass describes the name of a Kubernetes StorageClass resource // used to configure this VM's storage-related attributes. // @@ -694,6 +762,35 @@ type VirtualMachineAdvancedSpec struct { ChangeBlockTracking *bool `json:"changeBlockTracking,omitempty"` } +type VirtualMachineEncryptionType string + +const ( + VirtualMachineEncryptionTypeConfig VirtualMachineEncryptionType = "Config" + VirtualMachineEncryptionTypeDisks VirtualMachineEncryptionType = "Disks" +) + +type VirtualMachineCryptoStatus struct { + // +optional + + // Encrypted describes the observed state of the VirtualMachine's + // encryption. + Encrypted []VirtualMachineEncryptionType `json:"encrypted,omitempty"` + + // +optional + + // ProviderID describes the provider ID used to encrypt the VirtualMachine. + // Please note, this field will be empty if the VirtualMachine is not + // encrypted. + ProviderID string `json:"providerID,omitempty"` + + // +optional + + // KeyID describes the key ID used to encrypt the VirtualMachine. + // Please note, this field will be empty if the VirtualMachine is not + // encrypted. + KeyID string `json:"keyID,omitempty"` +} + // VirtualMachineStatus defines the observed state of a VirtualMachine instance. type VirtualMachineStatus struct { // +optional @@ -720,6 +817,12 @@ type VirtualMachineStatus struct { // +optional + // Crypto describes the observed state of the VirtualMachine's encryption + // configuration. + Crypto *VirtualMachineCryptoStatus `json:"crypto,omitempty"` + + // +optional + // Network describes the observed state of the VM's network configuration. // Please note much of the network status information is only available if // the guest has VM Tools installed. diff --git a/api/v1alpha3/zz_generated.deepcopy.go b/api/v1alpha3/zz_generated.deepcopy.go index e759a1b55..c0e914f27 100644 --- a/api/v1alpha3/zz_generated.deepcopy.go +++ b/api/v1alpha3/zz_generated.deepcopy.go @@ -742,6 +742,46 @@ func (in *VirtualMachineClassStatus) DeepCopy() *VirtualMachineClassStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VirtualMachineCryptoSpec) DeepCopyInto(out *VirtualMachineCryptoSpec) { + *out = *in + if in.UseDefaultKeyProvider != nil { + in, out := &in.UseDefaultKeyProvider, &out.UseDefaultKeyProvider + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VirtualMachineCryptoSpec. +func (in *VirtualMachineCryptoSpec) DeepCopy() *VirtualMachineCryptoSpec { + if in == nil { + return nil + } + out := new(VirtualMachineCryptoSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VirtualMachineCryptoStatus) DeepCopyInto(out *VirtualMachineCryptoStatus) { + *out = *in + if in.Encrypted != nil { + in, out := &in.Encrypted, &out.Encrypted + *out = make([]VirtualMachineEncryptionType, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VirtualMachineCryptoStatus. +func (in *VirtualMachineCryptoStatus) DeepCopy() *VirtualMachineCryptoStatus { + if in == nil { + return nil + } + out := new(VirtualMachineCryptoStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VirtualMachineImage) DeepCopyInto(out *VirtualMachineImage) { *out = *in @@ -2049,6 +2089,7 @@ func (in *VirtualMachineSpec) DeepCopyInto(out *VirtualMachineSpec) { *out = new(VirtualMachineImageRef) **out = **in } + in.Crypto.DeepCopyInto(&out.Crypto) if in.Bootstrap != nil { in, out := &in.Bootstrap, &out.Bootstrap *out = new(VirtualMachineBootstrapSpec) @@ -2108,6 +2149,11 @@ func (in *VirtualMachineStatus) DeepCopyInto(out *VirtualMachineStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Crypto != nil { + in, out := &in.Crypto, &out.Crypto + *out = new(VirtualMachineCryptoStatus) + (*in).DeepCopyInto(*out) + } if in.Network != nil { in, out := &in.Network, &out.Network *out = new(VirtualMachineNetworkStatus) diff --git a/config/crd/bases/vmoperator.vmware.com_virtualmachinereplicasets.yaml b/config/crd/bases/vmoperator.vmware.com_virtualmachinereplicasets.yaml index dcdba9cf7..bde82db7c 100644 --- a/config/crd/bases/vmoperator.vmware.com_virtualmachinereplicasets.yaml +++ b/config/crd/bases/vmoperator.vmware.com_virtualmachinereplicasets.yaml @@ -1007,6 +1007,64 @@ spec: an existing VM on the underlying platform that was not deployed from a VM class. type: string + crypto: + description: Crypto describes the desired encryption state + of the VirtualMachine. + properties: + encryptionClassName: + description: |- + EncryptionClassName describes the name of the EncryptionClass resource + used to encrypt this VM. + + Please note, this field is not required to encrypt the VM. If the + underlying platform has a default key provider, the VM may still be fully + or partially encrypted depending on the specified storage and VM classes. + + If there is a default key provider and an encryption storage class is + selected, the VM's home files and non-PVC disks will be encrypted. + + If there is a default key provider and a and a VM Class with a virtual, + trusted platform module (vTPM) is selected, the VM's home files will be + encrypted. + + If the underlying vSphere platform does not have a default key provider, + then this field is required when specifying an encryption storage class + and/or a VM Class with a vTPM. + type: string + useDefaultKeyProvider: + default: true + description: |- + UseDefaultKeyProvider describes the desired behavior for when an explicit + EncryptionClass is not provided. + + When an explicit EncryptionClass is not provided and this value is true: + + - Deploying a VirtualMachine with an encryption storage policy or vTPM + will be encrypted using the default key provider. + + - If a VirtualMachine is not encrypted, uses an encryption storage + policy or has a virtual, trusted platform module (vTPM), there is a + default key provider, the VM will be encrypted using the default key + provider. + + - If a VirtualMachine is encrypted with a provider other than the default + key provider, the VM will be rekeyed using the default key provider. + + When an explicit EncryptionClass is not provided and this value is false: + + - Deploying a VirtualMachine with an encryption storage policy or vTPM + will fail. + + - If a VirtualMachine is encrypted with a provider other than the default + key provider, the VM will be not be rekeyed. + + Please note, this could result in a VirtualMachine that cannot be + powered on since it is encrypted using a provider that may have been + removed. + + Defaults to true if omitted. + type: boolean + type: object guestID: description: |- GuestID describes the desired guest operating system identifier for a VM. diff --git a/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml b/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml index 232dd3631..94ffa745f 100644 --- a/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml +++ b/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml @@ -3791,6 +3791,64 @@ spec: an existing VM on the underlying platform that was not deployed from a VM class. type: string + crypto: + description: Crypto describes the desired encryption state of the + VirtualMachine. + properties: + encryptionClassName: + description: |- + EncryptionClassName describes the name of the EncryptionClass resource + used to encrypt this VM. + + Please note, this field is not required to encrypt the VM. If the + underlying platform has a default key provider, the VM may still be fully + or partially encrypted depending on the specified storage and VM classes. + + If there is a default key provider and an encryption storage class is + selected, the VM's home files and non-PVC disks will be encrypted. + + If there is a default key provider and a and a VM Class with a virtual, + trusted platform module (vTPM) is selected, the VM's home files will be + encrypted. + + If the underlying vSphere platform does not have a default key provider, + then this field is required when specifying an encryption storage class + and/or a VM Class with a vTPM. + type: string + useDefaultKeyProvider: + default: true + description: |- + UseDefaultKeyProvider describes the desired behavior for when an explicit + EncryptionClass is not provided. + + When an explicit EncryptionClass is not provided and this value is true: + + - Deploying a VirtualMachine with an encryption storage policy or vTPM + will be encrypted using the default key provider. + + - If a VirtualMachine is not encrypted, uses an encryption storage + policy or has a virtual, trusted platform module (vTPM), there is a + default key provider, the VM will be encrypted using the default key + provider. + + - If a VirtualMachine is encrypted with a provider other than the default + key provider, the VM will be rekeyed using the default key provider. + + When an explicit EncryptionClass is not provided and this value is false: + + - Deploying a VirtualMachine with an encryption storage policy or vTPM + will fail. + + - If a VirtualMachine is encrypted with a provider other than the default + key provider, the VM will be not be rekeyed. + + Please note, this could result in a VirtualMachine that cannot be + powered on since it is encrypted using a provider that may have been + removed. + + Defaults to true if omitted. + type: boolean + type: object guestID: description: |- GuestID describes the desired guest operating system identifier for a VM. @@ -4610,6 +4668,31 @@ spec: - type type: object type: array + crypto: + description: |- + Crypto describes the observed state of the VirtualMachine's encryption + configuration. + properties: + encrypted: + description: |- + Encrypted describes the observed state of the VirtualMachine's + encryption. + items: + type: string + type: array + keyID: + description: |- + KeyID describes the key ID used to encrypt the VirtualMachine. + Please note, this field will be empty if the VirtualMachine is not + encrypted. + type: string + providerID: + description: |- + ProviderID describes the provider ID used to encrypt the VirtualMachine. + Please note, this field will be empty if the VirtualMachine is not + encrypted. + type: string + type: object hardwareVersion: description: |- HardwareVersion describes the VirtualMachine resource's observed diff --git a/config/crd/external-crds/vmencryption.vmware.com_encryptionclasses.yaml b/config/crd/external-crds/vmencryption.vmware.com_encryptionclasses.yaml index 3dc53bea2..5e85684e4 100644 --- a/config/crd/external-crds/vmencryption.vmware.com_encryptionclasses.yaml +++ b/config/crd/external-crds/vmencryption.vmware.com_encryptionclasses.yaml @@ -49,8 +49,9 @@ spec: description: EncryptionClassSpec defines the desired state of EncryptionClass. properties: keyID: - description: KeyID describes the key used to encrypt/recrypt/decrypt - resources. + description: |- + KeyID describes the key used to encrypt/recrypt/decrypt resources. + When omitted, a key will be generated from the specified provider. type: string keyProvider: description: |- @@ -58,7 +59,6 @@ spec: resources. type: string required: - - keyID - keyProvider type: object status: diff --git a/controllers/controllers.go b/controllers/controllers.go index f1279617f..014b6fa2b 100644 --- a/controllers/controllers.go +++ b/controllers/controllers.go @@ -10,6 +10,7 @@ import ( "github.com/vmware-tanzu/vm-operator/controllers/contentlibrary" "github.com/vmware-tanzu/vm-operator/controllers/infra" + "github.com/vmware-tanzu/vm-operator/controllers/storageclass" spq "github.com/vmware-tanzu/vm-operator/controllers/storagepolicyquota" "github.com/vmware-tanzu/vm-operator/controllers/virtualmachine" "github.com/vmware-tanzu/vm-operator/controllers/virtualmachineclass" @@ -60,5 +61,11 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err } } + if pkgcfg.FromContext(ctx).Features.BringYourOwnEncryptionKey { + if err := storageclass.AddToManager(ctx, mgr); err != nil { + return fmt.Errorf("failed to initialize StorageClass controller: %w", err) + } + } + return nil } diff --git a/controllers/storageclass/storageclass_controller.go b/controllers/storageclass/storageclass_controller.go new file mode 100644 index 000000000..13f51acdc --- /dev/null +++ b/controllers/storageclass/storageclass_controller.go @@ -0,0 +1,110 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package storageclass + +import ( + "context" + "fmt" + "reflect" + "strings" + + "github.com/go-logr/logr" + storagev1 "k8s.io/api/storage/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" + + pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" + pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" + "github.com/vmware-tanzu/vm-operator/pkg/providers" + "github.com/vmware-tanzu/vm-operator/pkg/record" + kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" +) + +// AddToManager adds this package's controller to the provided manager. +func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) error { + var ( + controlledType = &storagev1.StorageClass{} + controlledTypeName = reflect.TypeOf(controlledType).Elem().Name() + + controllerNameShort = fmt.Sprintf("%s-controller", strings.ToLower(controlledTypeName)) + controllerNameLong = fmt.Sprintf("%s/%s/%s", ctx.Namespace, ctx.Name, controllerNameShort) + ) + + r := NewReconciler( + ctx, + mgr.GetClient(), + ctrl.Log.WithName("controllers").WithName(controlledTypeName), + record.New(mgr.GetEventRecorderFor(controllerNameLong)), + ctx.VMProvider) + + return ctrl.NewControllerManagedBy(mgr). + For(controlledType). + Complete(r) +} + +func NewReconciler( + ctx context.Context, + client client.Client, + logger logr.Logger, + recorder record.Recorder, + vmProvider providers.VirtualMachineProviderInterface) *Reconciler { + + return &Reconciler{ + Context: ctx, + Client: client, + Logger: logger, + Recorder: recorder, + vmProvider: vmProvider, + } +} + +// Reconciler reconciles a VirtualMachineClass object. +type Reconciler struct { + client.Client + Context context.Context + Logger logr.Logger + Recorder record.Recorder + vmProvider providers.VirtualMachineProviderInterface +} + +// +kubebuilder:rbac:groups=storage.k8s.io,resources=storageclasses,verbs=get;list;watch + +func (r *Reconciler) Reconcile( + ctx context.Context, + req ctrl.Request) (_ ctrl.Result, reterr error) { + + ctx = pkgcfg.JoinContext(ctx, r.Context) + + var obj storagev1.StorageClass + if err := r.Get(ctx, req.NamespacedName, &obj); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + logger := ctrl.Log.WithName("StorageClass").WithValues("name", req.Name) + ctx = logr.NewContext(ctx, logger) + + if !obj.DeletionTimestamp.IsZero() { + return ctrl.Result{}, nil + } + + return ctrl.Result{}, r.ReconcileNormal(ctx, &obj) +} + +func (r *Reconciler) ReconcileNormal( + ctx context.Context, + obj *storagev1.StorageClass) error { + + policyID, err := kubeutil.GetStoragePolicyID(*obj) + if err != nil { + return err + } + + ok, err := r.vmProvider.DoesProfileSupportEncryption(ctx, policyID) + if err != nil { + return err + } + + return kubeutil.MarkEncryptedStorageClass(ctx, r.Client, *obj, ok) +} diff --git a/controllers/storageclass/storageclass_controller_intg_test.go b/controllers/storageclass/storageclass_controller_intg_test.go new file mode 100644 index 000000000..b921eea70 --- /dev/null +++ b/controllers/storageclass/storageclass_controller_intg_test.go @@ -0,0 +1,78 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package storageclass_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + storagev1 "k8s.io/api/storage/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels" + kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func intgTests() { + Describe( + "Reconcile", + Label( + testlabels.Controller, + testlabels.EnvTest, + ), + intgTestsReconcile, + ) +} + +func intgTestsReconcile() { + var ( + ctx *builder.IntegrationTestContext + obj storagev1.StorageClass + ) + + BeforeEach(func() { + ctx = suite.NewIntegrationTestContext() + obj = storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "my-storage-class-", + }, + Provisioner: "fake", + Parameters: map[string]string{ + "storagePolicyID": "my-storage-policy", + }, + } + }) + + JustBeforeEach(func() { + Expect(ctx.Client.Create(ctx, &obj)).To(Succeed()) + }) + + AfterEach(func() { + ctx.AfterEach() + }) + + Context("reconciling an encrypted storage class", func() { + BeforeEach(func() { + obj.Parameters["storagePolicyID"] = myEncryptedStoragePolicy + }) + 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) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + }) + }) + }) + + 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) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeFalse()) + }) + }) + }) +} diff --git a/controllers/storageclass/storageclass_controller_suite_test.go b/controllers/storageclass/storageclass_controller_suite_test.go new file mode 100644 index 000000000..a4e536f7d --- /dev/null +++ b/controllers/storageclass/storageclass_controller_suite_test.go @@ -0,0 +1,52 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package storageclass_test + +import ( + "context" + "testing" + + . "github.com/onsi/ginkgo/v2" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/vmware-tanzu/vm-operator/controllers/storageclass" + pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" + pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" + providerfake "github.com/vmware-tanzu/vm-operator/pkg/providers/fake" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +const myEncryptedStoragePolicy = "my-encrypted-storage-policy" + +var intgFakeVMProvider = providerfake.NewVMProvider() + +func init() { + intgFakeVMProvider.DoesProfileSupportEncryptionFn = func( + ctx context.Context, + profileID string) (bool, error) { + + return profileID == "my-encrypted-storage-policy", nil + } +} + +var suite = builder.NewTestSuiteForControllerWithContext( + pkgcfg.UpdateContext( + pkgcfg.NewContext(), + func(config *pkgcfg.Config) { + config.Features.BringYourOwnEncryptionKey = true + }, + ), + storageclass.AddToManager, + func(ctx *pkgctx.ControllerManagerContext, _ ctrlmgr.Manager) error { + ctx.VMProvider = intgFakeVMProvider + return nil + }) + +func TestStorageClassController(t *testing.T) { + suite.Register(t, "StorageClass controller suite", intgTests, unitTests) +} + +var _ = BeforeSuite(suite.BeforeSuite) + +var _ = AfterSuite(suite.AfterSuite) diff --git a/controllers/storageclass/storageclass_controller_unit_test.go b/controllers/storageclass/storageclass_controller_unit_test.go new file mode 100644 index 000000000..ff2e478b0 --- /dev/null +++ b/controllers/storageclass/storageclass_controller_unit_test.go @@ -0,0 +1,145 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package storageclass_test + +import ( + "context" + "time" + + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + storagev1 "k8s.io/api/storage/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/vmware-tanzu/vm-operator/controllers/storageclass" + "github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels" + providerfake "github.com/vmware-tanzu/vm-operator/pkg/providers/fake" + kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func unitTests() { + Describe( + "Reconcile", + Label( + testlabels.Controller, + testlabels.V1Alpha3, + ), + unitTestsReconcile, + ) +} + +func unitTestsReconcile() { + var ( + initObjects []ctrlclient.Object + ctx *builder.UnitTestContextForController + + reconciler *storageclass.Reconciler + obj *storagev1.StorageClass + fakeVMProvider *providerfake.VMProvider + + err error + name string + ) + + BeforeEach(func() { + err = nil + initObjects = nil + obj = &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-storage-class", + UID: types.UID(uuid.NewString()), + }, + } + name = obj.Name + }) + + JustBeforeEach(func() { + initObjects = append(initObjects, obj) + + ctx = suite.NewUnitTestContextForController(initObjects...) + + fakeVMProvider = ctx.VMProvider.(*providerfake.VMProvider) + fakeVMProvider.DoesProfileSupportEncryptionFn = func( + ctx context.Context, + profileID string) (bool, error) { + + return profileID == myEncryptedStoragePolicy, nil + } + + reconciler = storageclass.NewReconciler( + ctx, + ctx.Client, + ctx.Logger, + ctx.Recorder, + fakeVMProvider) + + _, err = reconciler.Reconcile( + context.Background(), + reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + }, + }) + }) + + When("NotFound", func() { + BeforeEach(func() { + name = "invalid" + }) + It("ignores the error", func() { + Expect(err).ToNot(HaveOccurred()) + }) + }) + + When("Deleted", func() { + BeforeEach(func() { + obj.DeletionTimestamp = &metav1.Time{Time: time.Now()} + obj.Finalizers = append(obj.Finalizers, "fake.com/finalizer") + }) + It("returns success", func() { + Expect(err).ToNot(HaveOccurred()) + }) + }) + + When("Normal", func() { + 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`)) + }) + }) + When("not encrypted", func() { + BeforeEach(func() { + obj.Parameters = map[string]string{ + "storagePolicyID": "my-storage-policy", + } + }) + It("marks item in context as not encrypted and returns success", func() { + Expect(err).ToNot(HaveOccurred()) + ok, err := kubeutil.IsEncryptedStorageClass(ctx, ctx.Client, obj.Name) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeFalse()) + }) + }) + When("encrypted", func() { + BeforeEach(func() { + obj.Parameters = map[string]string{ + "storagePolicyID": myEncryptedStoragePolicy, + } + }) + It("marks item in context as encrypted and returns success", func() { + Expect(err).ToNot(HaveOccurred()) + ok, err := kubeutil.IsEncryptedStorageClass(ctx, ctx.Client, obj.Name) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + }) + }) + }) +} diff --git a/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go b/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go index 69d0d90e5..213436bf5 100644 --- a/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go +++ b/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go @@ -24,6 +24,7 @@ import ( vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" + byokv1 "github.com/vmware-tanzu/vm-operator/external/byok/api/v1alpha1" "github.com/vmware-tanzu/vm-operator/pkg/conditions" pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" @@ -39,6 +40,8 @@ import ( kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" "github.com/vmware-tanzu/vm-operator/pkg/util/kube/cource" vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto" ) const ( @@ -104,6 +107,14 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err builder = builder.Watches(&vmopv1.VirtualMachineClass{}, handler.EnqueueRequestsFromMapFunc(classToVMMapperFn(ctx, r.Client, isDefaultVMClassController))) + if pkgcfg.FromContext(ctx).Features.BringYourOwnEncryptionKey { + builder = builder.Watches( + &byokv1.EncryptionClass{}, + handler.EnqueueRequestsFromMapFunc( + encryptionClassToVMMapperFn(ctx, r.Client), + )) + } + return builder.Complete(r) } @@ -167,6 +178,54 @@ func classToVMMapperFn( } } +// encryptionClassToVMMapperFn returns a mapper function that can be used to +// enqueue reconcile requests for VMs in response to an event on the +// EncryptionClass resource. +func encryptionClassToVMMapperFn( + ctx *pkgctx.ControllerManagerContext, + c client.Client) func(_ context.Context, o client.Object) []reconcile.Request { + + // For a given EncryptionClass, return reconcile requests for VMs that + // specify the same EncryptionClass. + return func(_ context.Context, o client.Object) []reconcile.Request { + obj := o.(*byokv1.EncryptionClass) + logger := ctx.Logger.WithValues("name", obj.Name, "namespace", obj.Namespace) + + logger.V(4).Info("Reconciling all VMs referencing an EncryptionClass") + + // Find all VM resources that reference this EncryptionClass. + vmList := &vmopv1.VirtualMachineList{} + if err := c.List(ctx, vmList, client.InNamespace(obj.Namespace)); err != nil { + logger.Error( + err, + "Failed to list VirtualMachines for reconciliation due to EncryptionClass watch") + return nil + } + + // Populate reconcile requests for VMs that reference this + // EncryptionClass. + var requests []reconcile.Request + for _, vm := range vmList.Items { + if vm.Spec.Crypto.EncryptionClassName == obj.Name { + requests = append( + requests, + reconcile.Request{ + NamespacedName: client.ObjectKey{ + Namespace: vm.Namespace, + Name: vm.Name, + }, + }) + } + } + + logger.Info( + "Returning VM reconcile requests due to EncryptionClass watch", + "requests", requests) + + return requests + } +} + func upgradeSchema(ctx *pkgctx.VirtualMachineContext) { // If empty, this VM was created before v1alpha3 added the spec.instanceUUID field. if ctx.VM.Spec.InstanceUUID == "" && ctx.VM.Status.InstanceUUID != "" { @@ -243,6 +302,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re ctx = cource.JoinContext(ctx, r.Context) } + if pkgcfg.FromContext(ctx).Features.BringYourOwnEncryptionKey { + ctx = vmconfig.WithContext(ctx) + ctx = vmconfig.Register(ctx, crypto.New()) + } + ctx = ctxop.WithContext(ctx) vm := &vmopv1.VirtualMachine{} diff --git a/docs/concepts/workloads/vm.md b/docs/concepts/workloads/vm.md index d4ff1aa15..e97c7a36c 100644 --- a/docs/concepts/workloads/vm.md +++ b/docs/concepts/workloads/vm.md @@ -467,3 +467,118 @@ rockylinux_64Guest Rocky Linux (64-bit) windows2022srvNext_64Guest Microsoft Windows Server 2025 (64-bit) ... ``` + +## Crypto + +The field `spec.crypto` may be used in conjunction with a VM's storage class and/or virtual trusted platform module (vTPM) to control a VM's encryption level. + +### Encrypting a VM + +#### Encryption type + +The type of encryption depends on the storage class and hardware present in the VM as the chart below illustrates: + +| | Config | Disks | +|--------------------------|:------:|:-----:| +| Encryption storage class | ✓ | ✓ | +| vTPM | ✓ | | + +#### Default Key Provider + +By default, `spec.crypto.useDefaultKeyProvider` is true, meaning that if there is a default key provider and the VM has an encryption storage class and/or vTPM, the VM will be subject to some level of encryption. For example, if there was a default key provider present, it would be used to encrypt the following VM: + +```yaml +apiVersion: vmoperator.vmware.com/v1alpha3 +kind: VirtualMachine +metadata: + name: my-vm + namespace: my-namespace +spec: + className: my-vm-class + imageName: vmi-0a0044d7c690bcbea + storageClass: my-encrypted-storage-class +``` + +#### Encryption Class + +It is also possible to set `spec.crypto.encryptionClassName` to the name of an `EncryptionClass` resource in the same namespace as the VM. This resource specifies the provider and key ID used to encrypt workloads. For example, the following VM would be deployed and encrypted using the `EncryptionClass` named `my-encryption-class`: + +```yaml +apiVersion: vmoperator.vmware.com/v1alpha3 +kind: VirtualMachine +metadata: + name: my-vm + namespace: my-namespace +spec: + className: my-vm-class + imageName: vmi-0a0044d7c690bcbea + storageClass: my-encrypted-storage-class + crypto: + encryptionClassName: my-encryption-class +``` + +### Rekeying a VM + +Users can rekey VMs by switching to a different `EncryptionClass` or setting a currently non-empty `spec.crypto.encryptionClassName` to an empty value. This will result in the VM being rekeyed using the default key provider if it exists. If it does not, then the VM could be left in a state where it cannot be powered on. + +### Decrypting a VM + +It is not possible to decrypt an encrypted VM using VM Operator. + +### Status + +#### Observed type, provider, & key + +The following fields may be used to determine the encryption status of a VM: + +| Name | Description | +|------|-------------| +| `status.crypto.encrypted` | The observed state of the VM's encryption. May be `config`, `disks`, or both. | +| `status.crypto.providerID` | The provider ID used to encrypt the VM. | +| `status.crypto.keyID` | The key ID used to encrypt the VM. | + +For example, the following is an example of the status of a VM encrypted with an encrypted storage class: + +```yaml +status: + crypto: + encrypted: + - config + - disks + providerID: my-key-provider-id + keyID: my-key-id +``` + +#### EncryptionSynced Condition + +The condition `VirtualMachineEncryptionSynced` is also used to report whether or not the VM's desired encryption state was synchronized. For example, a VM that should be encrypted but is powered on may have the following condition: + +```yaml +status: + conditions: + - type: VirtualMachineEncryptionSynced + status: False + reason: InvalidState + message: Must be powered off to encrypt VM. +``` + +If the VM requested encryption and it was successful, then then condition will be: + +```yaml +status: + conditions: + - type: VirtualMachineEncryptionSynced + status: True +``` + +When `VirtualMachineEncryptionSynced` has `status: False`, the `reason` field may be set to one of or combination of the following values: + +| Reason | Description | +|--------|-------------| +| `EncryptionClassNotFound` | The resource specified with `spec.crypto.encryptionClassName` cannot be found. | +| `EncryptionClassInvalid` | The resource specified with `spec.crypto.encryptionClassName` contains an invalid provider and/or key ID. | +| `InvalidState` | The VM cannot be encrypted, rekeyed, or updated in its current state. | +| `InvalidChanges` | The VM has pending changes that would prohibit an encrypt, rekey, or update operation. | +| `ReconfigureError` | An encrypt, rekey, or update operation was attempted but failed due to a reason related to encryption. | + +If the condition is ever false, please refer first to the condition's `reason` field and then `message` for more information. \ No newline at end of file diff --git a/external/byok/api/v1alpha1/encryptionclass_types.go b/external/byok/api/v1alpha1/encryptionclass_types.go index eff1c3841..67273cd09 100644 --- a/external/byok/api/v1alpha1/encryptionclass_types.go +++ b/external/byok/api/v1alpha1/encryptionclass_types.go @@ -13,8 +13,11 @@ type EncryptionClassSpec struct { // resources. KeyProvider string `json:"keyProvider"` + // +optional + // KeyID describes the key used to encrypt/recrypt/decrypt resources. - KeyID string `json:"keyID"` + // When omitted, a key will be generated from the specified provider. + KeyID string `json:"keyID,omitempty"` } // EncryptionClassStatus defines the observed state of EncryptionClass. diff --git a/go.mod b/go.mod index f06a04e28..2197f4a63 100644 --- a/go.mod +++ b/go.mod @@ -30,9 +30,9 @@ require ( github.com/vmware-tanzu/vm-operator/external/storage-policy-quota v0.0.0-00010101000000-000000000000 github.com/vmware-tanzu/vm-operator/external/tanzu-topology 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.20240730173452-49b88eb9917f + github.com/vmware/govmomi v0.31.1-0.20240920193643-44d4fb3e244a // * https://github.com/vmware-tanzu/vm-operator/security/dependabot/24 - golang.org/x/text v0.16.0 + golang.org/x/text v0.18.0 golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d k8s.io/api v0.31.0 k8s.io/apiextensions-apiserver v0.31.0 diff --git a/go.sum b/go.sum index 09ec928eb..4f954e8e7 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.20240730173452-49b88eb9917f h1:lKzA28fIcNkcZgQdgP8YXaGZQcDMLNrwE9CyMyRKQNg= -github.com/vmware/govmomi v0.31.1-0.20240730173452-49b88eb9917f/go.mod h1:oHzAQ1r6152zYDGcUqeK+EO8LhKo5wjtvWZBGHws2Hc= +github.com/vmware/govmomi v0.31.1-0.20240920193643-44d4fb3e244a h1:Jic9T6J204z/7Ic5LW9VEJoptVgnCMZ7VYzejvawi5A= +github.com/vmware/govmomi v0.31.1-0.20240920193643-44d4fb3e244a/go.mod h1:IOv5nTXCPqH9qVJAlRuAGffogaLsNs8aF+e7vLgsHJU= 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= @@ -641,8 +641,8 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= -golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= +golang.org/x/text v0.18.0 h1:XvMDiNzPAl0jr17s6W9lcaIhGUfUORdGCNsuLmPG224= +golang.org/x/text v0.18.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= diff --git a/main.go b/main.go index 5693d3391..36e3d0940 100644 --- a/main.go +++ b/main.go @@ -17,6 +17,14 @@ import ( "k8s.io/klog/v2" "k8s.io/klog/v2/textlogger" + _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrllog "sigs.k8s.io/controller-runtime/pkg/log" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + ctrlsig "sigs.k8s.io/controller-runtime/pkg/manager/signals" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "github.com/vmware-tanzu/vm-operator/controllers" "github.com/vmware-tanzu/vm-operator/pkg" pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" @@ -26,14 +34,6 @@ import ( pkgmgrinit "github.com/vmware-tanzu/vm-operator/pkg/manager/init" "github.com/vmware-tanzu/vm-operator/pkg/util/kube/cource" "github.com/vmware-tanzu/vm-operator/webhooks" - - _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - ctrllog "sigs.k8s.io/controller-runtime/pkg/log" - ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" - ctrlsig "sigs.k8s.io/controller-runtime/pkg/manager/signals" - "sigs.k8s.io/controller-runtime/pkg/webhook" // +kubebuilder:scaffold:imports ) diff --git a/pkg/bitmask/bitmask.go b/pkg/bitmask/bitmask.go new file mode 100644 index 000000000..7bcb22ba4 --- /dev/null +++ b/pkg/bitmask/bitmask.go @@ -0,0 +1,73 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package bitmask + +import ( + "bytes" +) + +// Bit is a type constraint that includes all unsigned integers. +type Bit interface { + ~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 + + // StringValue returns the string-ified value for the bit. + StringValue() string +} + +// Bitmask is a type constraint that can be used as a bitmask. +type Bitmask[T Bit] interface { + Bit + + // MaxValue returns the maximum, valid bit that may be set in the mask. + MaxValue() T +} + +// Has returns true if (a & b) > 0. +func Has[B Bit, M Bitmask[B]](a, b M) bool { + return (a & b) > 0 +} + +// Set returns a new mask of a | b. +func Set[B Bit, M Bitmask[B]](a, b M) M { + return a | b +} + +// Unset returns a new mask of a &^ b. +func Unset[B Bit, M Bitmask[B]](a, b M) M { + return a &^ b +} + +// String returns the stringified version of the mask. +func String[B Bit, M Bitmask[B]](a M) string { + var ( + b B + w bytes.Buffer + m = a.MaxValue() + writeFn = write[B, M] + ) + + if f, ok := ((any)(a)).(interface{ Write(*bytes.Buffer, M) }); ok { + // If the bitmask implements Write(*bytes.Buffer, M, M), then use it. + // Otherwise use write(*bytes.Buffer, M, M). + writeFn = f.Write + } + + for i := 0; ; i++ { + if b = 1 << i; b > m { + break + } + if Has(a, M(b)) { + writeFn(&w, M(b)) + } + } + + return w.String() +} + +func write[B Bit, M Bitmask[B]](w *bytes.Buffer, b M) { + if w.Len() > 0 { + w.WriteString("And") + } + w.WriteString(b.StringValue()) +} diff --git a/pkg/bitmask/bitmask_suite_test.go b/pkg/bitmask/bitmask_suite_test.go new file mode 100644 index 000000000..55bb34fa5 --- /dev/null +++ b/pkg/bitmask/bitmask_suite_test.go @@ -0,0 +1,16 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package bitmask_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestSuite(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Bitmask Test Suite") +} diff --git a/pkg/bitmask/bitmask_test.go b/pkg/bitmask/bitmask_test.go new file mode 100644 index 000000000..eb9a3215b --- /dev/null +++ b/pkg/bitmask/bitmask_test.go @@ -0,0 +1,205 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package bitmask_test + +import ( + "bytes" + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/vmware-tanzu/vm-operator/pkg/bitmask" +) + +type mask8 uint8 + +const ( + mask8Aay mask8 = 1 << iota + mask8Bee + mask8Cee + + _mask8ValMax = 1 << (iota - 1) +) + +func (m mask8) MaxValue() mask8 { + return _mask8ValMax +} + +func (m mask8) StringValue() string { + switch m { + case mask8Aay: + return "Aay" + case mask8Bee: + return "Bee" + case mask8Cee: + return "Cee" + } + return "" +} + +type mask32 uint64 + +const ( + mask32Aay mask32 = 1 << iota + mask32Bee + mask32Cee + + _mask32ValMax = 1 << (iota - 1) +) + +func (m mask32) MaxValue() mask32 { + return _mask32ValMax +} + +func (m mask32) StringValue() string { + switch m { + case mask32Aay: + return "1" + case mask32Bee: + return "2" + case mask32Cee: + return "4" + } + return "" +} + +func (m mask32) Write(w *bytes.Buffer, b mask32) { + if w.Len() > 0 { + w.WriteString("|") + } + fmt.Fprintf(w, "%d", b) +} + +type bit64 uint64 +type mask64 bit64 + +const ( + bit64Aay bit64 = 1 << iota + bit64Bee + bit64Cee + + _mask64ValMax = 1 << (iota - 1) +) + +func (m mask64) MaxValue() mask64 { + return _mask64ValMax +} + +func (b bit64) String() string { + switch b { + case bit64Aay: + return "1" + case bit64Bee: + return "2" + case bit64Cee: + return "4" + } + return "" +} + +func (m mask64) StringValue() string { + return bit64(m).String() +} + +var _ = Context("Uint8", func() { + DescribeTable("Has", getHasTable(mask8(0))...) + DescribeTable("Set", getSetTable(mask8(0))...) + DescribeTable("Unset", getUnsetTable(mask8(0))...) + DescribeTable("String", getStringTable(mask8(0))...) +}) + +var _ = Describe("Uint32", func() { + DescribeTable("Has", getHasTable(mask32(0))...) + DescribeTable("Set", getSetTable(mask32(0))...) + DescribeTable("Unset", getUnsetTable(mask32(0))...) + DescribeTable( + "String", + func(a int, s string) { + Expect(bitmask.String(mask32(a))).To(Equal(s)) + }, + Entry("0 = \"\"", 0, ""), + Entry("a = \"1\"", 1, "1"), + Entry("b = \"2\"", 2, "2"), + Entry("c = \"4\"", 4, "4"), + Entry("8 = \"\"", 8, ""), + Entry("16 = \"\"", 16, ""), + Entry("32 = \"\"", 32, ""), + Entry("a|b = \"1|2\"", 1|2, "1|2"), + Entry("a|b|c = \"1|2|4\"", 1|2|4, "1|2|4"), + Entry("a|c = \"1|4\"", 1|4, "1|4"), + Entry("b|c = \"2|4\"", 2|4, "2|4"), + Entry("c|b = \"2|4\"", 4|2, "2|4"), + ) +}) + +var _ = Context("Uint64", func() { + DescribeTable("Has", getHasTable(mask64(0))...) + DescribeTable("Set", getSetTable(mask64(0))...) + DescribeTable("Unset", getUnsetTable(mask64(0))...) + DescribeTable("String", getStringTable(mask64(0))...) +}) + +func getHasTable[U bitmask.Bit, M bitmask.Bitmask[U]](_ M) []any { + return []any{ + func(a, b int, expected bool) { + Expect(bitmask.Has(M(a), M(b))).To(Equal(expected)) + }, + Entry("0,0 == false", 0, 0, false), + Entry("a|b,a == true", 1|2, 1, true), + Entry("a|b,b == true", 1|2, 2, true), + Entry("a|b,c == false", 1|2, 4, false), + Entry("a|b|c,a == true", 1|2|4, 1, true), + Entry("a|b|c,b == true", 1|2|4, 2, true), + Entry("a|b|c,c == true", 1|2|4, 4, true), + Entry("a|b|c,a|b == true", 1|2|4, 1|2, true), + } +} + +func getSetTable[U bitmask.Bit, M bitmask.Bitmask[U]](_ M) []any { + return []any{ + func(a, b, c int) { + Expect(bitmask.Set(M(a), M(b))).To(Equal(M(c))) + }, + Entry("0,0 = 0", 0, 0, 0), + Entry("a,a = a", 1, 1, 1), + Entry("a,b = a|b", 1, 2, 1|2), + Entry("a|b,c = a|b|c", 1|2, 4, 1|2|4), + Entry("a|b,b|c = a|b|c", 1|2, 2|4, 1|2|4), + } +} + +func getUnsetTable[U bitmask.Bit, M bitmask.Bitmask[U]](_ M) []any { + return []any{ + func(a, b, c int) { + Expect(bitmask.Unset(M(a), M(b))).To(Equal(M(c))) + }, + Entry("0,0 = 0", 0, 0, 0), + Entry("a,a = 0", 1, 1, 0), + Entry("a,b = a", 1, 2, 1), + Entry("a|b|c,b = a|b", 1|2|4, 2, 1|4), + Entry("a|b|c,0 = a|b|c", 1|2|4, 0, 1|2|4), + Entry("a|b|c,b|c = a", 1|2|4, 2|4, 1), + } +} + +func getStringTable[U bitmask.Bit, M bitmask.Bitmask[U]](_ M) []any { + return []any{ + func(a int, s string) { + Expect(bitmask.String(mask8(a))).To(Equal(s)) + }, + Entry("0 = \"\"", 0, ""), + Entry("a = \"Aay\"", 1, "Aay"), + Entry("b = \"Bee\"", 2, "Bee"), + Entry("c = \"Cee\"", 4, "Cee"), + Entry("8 = \"\"", 8, ""), + Entry("16 = \"\"", 16, ""), + Entry("32 = \"\"", 32, ""), + Entry("a|b = \"AayAndBee\"", 1|2, "AayAndBee"), + Entry("a|b|c = \"AayAndBeeAndCee\"", 1|2|4, "AayAndBeeAndCee"), + Entry("a|c = \"AayAndCee\"", 1|4, "AayAndCee"), + Entry("b|c = \"BeeAndCee\"", 2|4, "BeeAndCee"), + Entry("c|b = \"BeeAndCee\"", 4|2, "BeeAndCee"), + } +} diff --git a/pkg/constants/testlabels/test_labels.go b/pkg/constants/testlabels/test_labels.go index 6e322cff8..1d234547b 100644 --- a/pkg/constants/testlabels/test_labels.go +++ b/pkg/constants/testlabels/test_labels.go @@ -13,6 +13,9 @@ const ( // Create describes a test related to create logic. Create = "create" + // Crypto describes a test related to encryption. + Crypto = "crypto" + // Delete describes a test related to delete logic. Delete = "delete" diff --git a/pkg/providers/fake/fake_vm_provider.go b/pkg/providers/fake/fake_vm_provider.go index d9055dd82..8f5101c34 100644 --- a/pkg/providers/fake/fake_vm_provider.go +++ b/pkg/providers/fake/fake_vm_provider.go @@ -53,6 +53,8 @@ type funcs struct { ComputeCPUMinFrequencyFn func(ctx context.Context) error GetTasksByActIDFn func(ctx context.Context, actID string) (tasksInfo []vimtypes.TaskInfo, retErr error) + + DoesProfileSupportEncryptionFn func(ctx context.Context, profileID string) (bool, error) } type VMProvider struct { @@ -359,6 +361,19 @@ func (s *VMProvider) IsPublishVMCalled() bool { return s.isPublishVMCalled } +func (s *VMProvider) DoesProfileSupportEncryption( + ctx context.Context, + profileID string) (bool, error) { + + s.Lock() + defer s.Unlock() + + if fn := s.DoesProfileSupportEncryptionFn; fn != nil { + return fn(ctx, profileID) + } + return false, nil +} + func NewVMProvider() *VMProvider { provider := VMProvider{ vmMap: map[client.ObjectKey]*vmopv1.VirtualMachine{}, diff --git a/pkg/providers/vm_provider_interface.go b/pkg/providers/vm_provider_interface.go index 2ade5892c..0465aff81 100644 --- a/pkg/providers/vm_provider_interface.go +++ b/pkg/providers/vm_provider_interface.go @@ -40,4 +40,9 @@ type VirtualMachineProviderInterface interface { SyncVirtualMachineImage(ctx context.Context, cli, vmi client.Object) error GetTasksByActID(ctx context.Context, actID string) (tasksInfo []vimtypes.TaskInfo, retErr error) + + // DoesProfileSupportEncryption returns true if the specified profile + // supports encryption by checking whether or not the underlying policy + // contains any IOFILTERs. + DoesProfileSupportEncryption(ctx context.Context, profileID string) (bool, error) } diff --git a/pkg/providers/vsphere/client/client_test.go b/pkg/providers/vsphere/client/client_test.go index 5ffad212f..6da3608fe 100644 --- a/pkg/providers/vsphere/client/client_test.go +++ b/pkg/providers/vsphere/client/client_test.go @@ -14,6 +14,7 @@ import ( . "github.com/onsi/gomega" "github.com/go-logr/logr" + _ "github.com/vmware/govmomi/pbm/simulator" // load PBM simulator "github.com/vmware/govmomi/session" "github.com/vmware/govmomi/session/keepalive" "github.com/vmware/govmomi/simulator" diff --git a/pkg/providers/vsphere/session/session_vm_update.go b/pkg/providers/vsphere/session/session_vm_update.go index 966390269..a25911b2d 100644 --- a/pkg/providers/vsphere/session/session_vm_update.go +++ b/pkg/providers/vsphere/session/session_vm_update.go @@ -16,6 +16,7 @@ import ( vimtypes "github.com/vmware/govmomi/vim25/types" apiEquality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" vmopv1common "github.com/vmware-tanzu/vm-operator/api/v1alpha3/common" @@ -30,10 +31,11 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/virtualmachine" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/vmlifecycle" pkgutil "github.com/vmware-tanzu/vm-operator/pkg/util" - "github.com/vmware-tanzu/vm-operator/pkg/util/annotations" + "github.com/vmware-tanzu/vm-operator/pkg/util/paused" "github.com/vmware-tanzu/vm-operator/pkg/util/resize" vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1" vmutil "github.com/vmware-tanzu/vm-operator/pkg/util/vsphere/vm" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig" ) // VMUpdateArgs contains the arguments needed to update a VM on VC. @@ -566,8 +568,10 @@ func (s *Session) prePowerOnVMReconfigure( vmCtx, vmCtx.Logger.WithName("prePowerOnVMReconfigure"), ), + s.K8sClient, vmCtx.VM, resVM.VcVM(), + vmCtx.MoVM, *configSpec); err != nil { return err @@ -808,8 +812,10 @@ func (s *Session) poweredOnVMReconfigure( vmCtx, vmCtx.Logger.WithName("poweredOnVMReconfigure"), ), + s.K8sClient, vmCtx.VM, resVM.VcVM(), + vmCtx.MoVM, *configSpec) if err != nil { @@ -900,8 +906,10 @@ func (s *Session) resizeVMWhenPoweredStateOff( vmCtx, vmCtx.Logger.WithName("resizeVMWhenPoweredStateOff"), ), + s.K8sClient, vmCtx.VM, vcVM, + vmCtx.MoVM, configSpec) if err != nil { @@ -1004,7 +1012,7 @@ func (s *Session) updateVMDesiredPowerStateOff( refetchProps = true } } else { - refetch, err := defaultReconfigure(vmCtx, vcVM, *vmCtx.MoVM.Config) + refetch, err := defaultReconfigure(vmCtx, s.K8sClient, vcVM) if err != nil { return refetchProps, err } @@ -1037,7 +1045,7 @@ func (s *Session) updateVMDesiredPowerStateSuspended( refetchProps = true } - refetch, err := defaultReconfigure(vmCtx, vcVM, *vmCtx.MoVM.Config) + refetch, err := defaultReconfigure(vmCtx, s.K8sClient, vcVM) if err != nil { return refetchProps, err } @@ -1172,8 +1180,10 @@ func (s *Session) UpdateVirtualMachine( getUpdateArgsFn func() (*VMUpdateArgs, error), getResizeArgsFn func() (*VMResizeArgs, error)) error { - var refetchProps bool - var err error + var ( + refetchProps bool + updateErr error + ) // Only update VM's power state when VM is not paused. if !isVMPaused(vmCtx) { @@ -1190,20 +1200,20 @@ func (s *Session) UpdateVirtualMachine( switch vmCtx.VM.Spec.PowerState { case vmopv1.VirtualMachinePowerStateOff: - refetchProps, err = s.updateVMDesiredPowerStateOff( + refetchProps, updateErr = s.updateVMDesiredPowerStateOff( vmCtx, vcVM, getResizeArgsFn, existingPowerState) case vmopv1.VirtualMachinePowerStateSuspended: - refetchProps, err = s.updateVMDesiredPowerStateSuspended( + refetchProps, updateErr = s.updateVMDesiredPowerStateSuspended( vmCtx, vcVM, existingPowerState) case vmopv1.VirtualMachinePowerStateOn: - refetchProps, err = s.updateVMDesiredPowerStateOn( + refetchProps, updateErr = s.updateVMDesiredPowerStateOn( vmCtx, vcVM, getUpdateArgsFn, @@ -1211,64 +1221,99 @@ func (s *Session) UpdateVirtualMachine( } } else { vmCtx.Logger.Info("VirtualMachine is paused. PowerState is not updated.") - refetchProps, err = defaultReconfigure(vmCtx, vcVM, *vmCtx.MoVM.Config) + refetchProps, updateErr = defaultReconfigure(vmCtx, s.K8sClient, vcVM) + } + + if updateErr != nil { + updateErr = fmt.Errorf("updating state failed with %w", updateErr) } if refetchProps { + vmCtx.Logger.V(8).Info( + "Refetching properties", + "refetchProps", refetchProps, + "powerState", vmCtx.VM.Spec.PowerState) + vmCtx.MoVM = mo.VirtualMachine{} + + if err := vcVM.Properties( + vmCtx, + vcVM.Reference(), + vmlifecycle.VMStatusPropertiesSelector, + &vmCtx.MoVM); err != nil { + + err = fmt.Errorf("refetching props failed with %w", err) + if updateErr == nil { + updateErr = err + } else { + updateErr = fmt.Errorf("%w, %w", updateErr, err) + } + } } - vmCtx.Logger.V(8).Info( - "UpdateVM", - "refetchProps", refetchProps, - "powerState", vmCtx.VM.Spec.PowerState) + if pkgcfg.FromContext(vmCtx).Features.BringYourOwnEncryptionKey { + for _, r := range vmconfig.FromContext(vmCtx) { + if err := r.OnResult( + vmCtx, + vmCtx.VM, + vmCtx.MoVM, + updateErr); err != nil { + + err = fmt.Errorf("%s.OnResult failed with %w", r.Name(), err) + if updateErr == nil { + updateErr = err + } else { + updateErr = fmt.Errorf("%w, %w", updateErr, err) + } + } + } + } - updateErr := vmlifecycle.UpdateStatus(vmCtx, s.K8sClient, vcVM, refetchProps) - if updateErr != nil { - vmCtx.Logger.Error(updateErr, "Updating VM status failed") - if err == nil { - err = updateErr + if err := vmlifecycle.UpdateStatus(vmCtx, s.K8sClient, vcVM); err != nil { + err = fmt.Errorf("updating status failed with %w", err) + if updateErr == nil { + updateErr = err + } else { + updateErr = fmt.Errorf("%w, %w", updateErr, err) } } - return err + return updateErr } // Source of truth is EC and Annotation. func isVMPaused(vmCtx pkgctx.VirtualMachineContext) bool { - vm := vmCtx.VM - - adminPaused := virtualmachine.IsPausedByAdmin(vmCtx.MoVM) - devopsPaused := annotations.HasPaused(vm) + byAdmin := paused.ByAdmin(vmCtx.MoVM) + byDevOps := paused.ByDevOps(vmCtx.VM) - if adminPaused || devopsPaused { - if vm.Labels == nil { - vm.Labels = make(map[string]string) + if byAdmin || byDevOps { + if vmCtx.VM.Labels == nil { + vmCtx.VM.Labels = make(map[string]string) } switch { - case adminPaused && devopsPaused: - vm.Labels[vmopv1.PausedVMLabelKey] = "both" - case adminPaused: - vm.Labels[vmopv1.PausedVMLabelKey] = "admin" - case devopsPaused: - vm.Labels[vmopv1.PausedVMLabelKey] = "devops" + case byAdmin && byDevOps: + vmCtx.VM.Labels[vmopv1.PausedVMLabelKey] = "both" + case byAdmin: + vmCtx.VM.Labels[vmopv1.PausedVMLabelKey] = "admin" + case byDevOps: + vmCtx.VM.Labels[vmopv1.PausedVMLabelKey] = "devops" } return true } - delete(vm.Labels, vmopv1.PausedVMLabelKey) + delete(vmCtx.VM.Labels, vmopv1.PausedVMLabelKey) return false } func defaultReconfigure( vmCtx pkgctx.VirtualMachineContext, - vcVM *object.VirtualMachine, - configInfo vimtypes.VirtualMachineConfigInfo) (bool, error) { + k8sClient ctrlclient.Client, + vcVM *object.VirtualMachine) (bool, error) { var configSpec vimtypes.VirtualMachineConfigSpec if err := vmopv1util.OverwriteAlwaysResizeConfigSpec( vmCtx, *vmCtx.VM, - configInfo, + *vmCtx.MoVM.Config, &configSpec); err != nil { return false, err @@ -1279,17 +1324,39 @@ func defaultReconfigure( vmCtx, vmCtx.Logger.WithName("defaultReconfigure"), ), + k8sClient, vmCtx.VM, vcVM, + vmCtx.MoVM, configSpec) } func doReconfigure( ctx context.Context, + k8sClient ctrlclient.Client, vm *vmopv1.VirtualMachine, vcVM *object.VirtualMachine, + moVM mo.VirtualMachine, configSpec vimtypes.VirtualMachineConfigSpec) (bool, error) { + logger := logr.FromContextOrDiscard(ctx) + if pkgcfg.FromContext(ctx).Features.BringYourOwnEncryptionKey { + for _, r := range vmconfig.FromContext(ctx) { + logger.Info("Reconciling vmconfig", "reconciler", r.Name()) + + if err := r.Reconcile( + ctx, + k8sClient, + vcVM.Client(), + vm, + moVM, + &configSpec); err != nil { + + return false, err + } + } + } + var defaultConfigSpec vimtypes.VirtualMachineConfigSpec if apiEquality.Semantic.DeepEqual(configSpec, defaultConfigSpec) { return false, nil diff --git a/pkg/providers/vsphere/session/session_vm_update_test.go b/pkg/providers/vsphere/session/session_vm_update_test.go index 09dc25c8d..cbe3c8695 100644 --- a/pkg/providers/vsphere/session/session_vm_update_test.go +++ b/pkg/providers/vsphere/session/session_vm_update_test.go @@ -31,6 +31,7 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/util/ptr" pkgclient "github.com/vmware-tanzu/vm-operator/pkg/util/vsphere/client" "github.com/vmware-tanzu/vm-operator/test/builder" + "github.com/vmware-tanzu/vm-operator/test/testutil" ) var _ = Describe("Update ConfigSpec", func() { @@ -1286,7 +1287,7 @@ var _ = Describe("UpdateVirtualMachine", func() { vm.Spec.RestartMode = vmopv1.VirtualMachinePowerOpModeSoft }) It("should return an error about lacking tools", func() { - Expect(sess.UpdateVirtualMachine(vmCtx, vcVM, nil, nil)).To(MatchError("failed to soft restart vm ServerFaultCode: ToolsUnavailable")) + Expect(testutil.ContainsError(sess.UpdateVirtualMachine(vmCtx, vcVM, nil, nil), "failed to soft restart vm ServerFaultCode: ToolsUnavailable")).To(BeTrue()) Expect(vcVM.Properties(ctx, vcVM.Reference(), vmProps, &vmCtx.MoVM)).To(Succeed()) newLastRestartTime := getLastRestartTime(vmCtx.MoVM) Expect(newLastRestartTime).To(Equal(oldLastRestartTime)) diff --git a/pkg/providers/vsphere/storage/storageclass.go b/pkg/providers/vsphere/storage/storageclass.go index 6b4a6a585..607c5701b 100644 --- a/pkg/providers/vsphere/storage/storageclass.go +++ b/pkg/providers/vsphere/storage/storageclass.go @@ -1,16 +1,15 @@ -// Copyright (c) 2022 VMware, Inc. All Rights Reserved. +// Copyright (c) 2022-2024 VMware, Inc. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package storage import ( - "fmt" - storagev1 "k8s.io/api/storage/v1" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" + kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" ) // getStoragePolicyID returns Storage Policy ID from Storage Class Name. @@ -19,18 +18,12 @@ func getStoragePolicyID( client ctrlclient.Client, storageClassName string) (string, error) { - sc := &storagev1.StorageClass{} - if err := client.Get(vmCtx, ctrlclient.ObjectKey{Name: storageClassName}, sc); err != nil { + var sc storagev1.StorageClass + if err := client.Get(vmCtx, ctrlclient.ObjectKey{Name: storageClassName}, &sc); err != nil { vmCtx.Logger.Error(err, "Failed to get StorageClass", "storageClass", storageClassName) return "", err } - - policyID, ok := sc.Parameters["storagePolicyID"] - if !ok { - return "", fmt.Errorf("StorageClass %s does not have 'storagePolicyID' parameter", storageClassName) - } - - return policyID, nil + return kubeutil.GetStoragePolicyID(sc) } // GetVMStoragePoliciesIDs returns a map of storage class names to their storage policy IDs. diff --git a/pkg/providers/vsphere/virtualmachine/delete.go b/pkg/providers/vsphere/virtualmachine/delete.go index 6716d114e..9caaa1b6a 100644 --- a/pkg/providers/vsphere/virtualmachine/delete.go +++ b/pkg/providers/vsphere/virtualmachine/delete.go @@ -14,6 +14,7 @@ import ( vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" + "github.com/vmware-tanzu/vm-operator/pkg/util/paused" vmutil "github.com/vmware-tanzu/vm-operator/pkg/util/vsphere/vm" ) @@ -38,7 +39,7 @@ func DeleteVirtualMachine( return err } // Throw an error to distinguish from successful deletion. - if paused := IsPausedByAdmin(vmCtx.MoVM); paused { + if paused := paused.ByAdmin(vmCtx.MoVM); paused { if vmCtx.VM.Labels == nil { vmCtx.VM.Labels = make(map[string]string) } diff --git a/pkg/providers/vsphere/virtualmachine/extraconfig.go b/pkg/providers/vsphere/virtualmachine/extraconfig.go deleted file mode 100644 index 31bd0c2ac..000000000 --- a/pkg/providers/vsphere/virtualmachine/extraconfig.go +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) 2024 VMware, Inc. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package virtualmachine - -import ( - "strings" - - "github.com/vmware/govmomi/vim25/mo" - - vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" - "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" -) - -func IsPausedByAdmin(moVM mo.VirtualMachine) bool { - for i := range moVM.Config.ExtraConfig { - if o := moVM.Config.ExtraConfig[i].GetOptionValue(); o != nil { - if o.Key == vmopv1.PauseVMExtraConfigKey { - if value, ok := o.Value.(string); ok { - return strings.ToUpper(value) == constants.ExtraConfigTrue - } - } - } - } - return false -} diff --git a/pkg/providers/vsphere/virtualmachine/extraconfig_test.go b/pkg/providers/vsphere/virtualmachine/extraconfig_test.go deleted file mode 100644 index 88c98aa9f..000000000 --- a/pkg/providers/vsphere/virtualmachine/extraconfig_test.go +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright (c) 2024 VMware, Inc. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package virtualmachine_test - -import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/vmware/govmomi/vim25/mo" - vimtypes "github.com/vmware/govmomi/vim25/types" - - vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" - "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" - "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/virtualmachine" - vmutil "github.com/vmware-tanzu/vm-operator/pkg/util/vsphere/vm" -) - -func extraConfigTests() { - getVMMoRef := func() vimtypes.ManagedObjectReference { - return vimtypes.ManagedObjectReference{ - Type: "VirtualMachine", - Value: "vm-44", - } - } - Context("IsPausedByAdmin", func() { - var ( - mgdObj mo.VirtualMachine - ) - - BeforeEach(func() { - moRef := getVMMoRef() - mgdObj = vmutil.ManagedObjectFromMoRef(moRef) - mgdObj.Config = &vimtypes.VirtualMachineConfigInfo{ - ExtraConfig: []vimtypes.BaseOptionValue{}, - } - }) - - It("should return false when PauseVMExtraConfigKey is not set", func() { - Expect(virtualmachine.IsPausedByAdmin(mgdObj)).To(BeFalse()) - }) - - It("should return false when PauseVMExtraConfigKey is set to False", func() { - mgdObj.Config.ExtraConfig = append(mgdObj.Config.ExtraConfig, &vimtypes.OptionValue{ - Key: vmopv1.PauseVMExtraConfigKey, - Value: constants.ExtraConfigFalse, - }) - - Expect(virtualmachine.IsPausedByAdmin(mgdObj)).To(BeFalse()) - }) - - It("should return false when PauseVMExtraConfigKey is set to 1", func() { - mgdObj.Config.ExtraConfig = append(mgdObj.Config.ExtraConfig, &vimtypes.OptionValue{ - Key: vmopv1.PauseVMExtraConfigKey, - Value: 1, - }) - - Expect(virtualmachine.IsPausedByAdmin(mgdObj)).To(BeFalse()) - }) - - It("should return true when PauseVMExtraConfigKey is set to 'true'", func() { - mgdObj.Config.ExtraConfig = append(mgdObj.Config.ExtraConfig, &vimtypes.OptionValue{ - Key: vmopv1.PauseVMExtraConfigKey, - Value: "true", - }) - - Expect(virtualmachine.IsPausedByAdmin(mgdObj)).To(BeTrue()) - }) - - It("should return true when PauseVMExtraConfigKey is set to 'True'", func() { - mgdObj.Config.ExtraConfig = append(mgdObj.Config.ExtraConfig, &vimtypes.OptionValue{ - Key: vmopv1.PauseVMExtraConfigKey, - Value: "True", - }) - - Expect(virtualmachine.IsPausedByAdmin(mgdObj)).To(BeTrue()) - }) - - It("should return true when PauseVMExtraConfigKey is set to 'TRUE'", func() { - mgdObj.Config.ExtraConfig = append(mgdObj.Config.ExtraConfig, &vimtypes.OptionValue{ - Key: vmopv1.PauseVMExtraConfigKey, - Value: constants.ExtraConfigTrue, - }) - - Expect(virtualmachine.IsPausedByAdmin(mgdObj)).To(BeTrue()) - }) - }) -} diff --git a/pkg/providers/vsphere/virtualmachine/virtualmachine_suite_test.go b/pkg/providers/vsphere/virtualmachine/virtualmachine_suite_test.go index 50547c944..2ef796a79 100644 --- a/pkg/providers/vsphere/virtualmachine/virtualmachine_suite_test.go +++ b/pkg/providers/vsphere/virtualmachine/virtualmachine_suite_test.go @@ -19,7 +19,6 @@ func vcSimTests() { Describe("Backup", Label(testlabels.VCSim), backupTests) Describe("GuestInfo", Label(testlabels.VCSim), guestInfoTests) Describe("CD-ROM", Label(testlabels.VCSim), cdromTests) - Describe("ExtraConfig", Label(testlabels.VCSim), extraConfigTests) } var suite = builder.NewTestSuite() diff --git a/pkg/providers/vsphere/vmlifecycle/update_status.go b/pkg/providers/vsphere/vmlifecycle/update_status.go index 8c6d1fff0..1ea031405 100644 --- a/pkg/providers/vsphere/vmlifecycle/update_status.go +++ b/pkg/providers/vsphere/vmlifecycle/update_status.go @@ -36,16 +36,18 @@ import ( ) var ( - // VMStatusPropertiesSelector is the minimum properties needed to be retrieved in order to populate - // the Status. Callers may provide a MO with more. This often saves us a second round trip in the - // common steady state. + // VMStatusPropertiesSelector is the minimum properties needed to be + // retrieved in order to populate the Status. Callers may provide a MO with + // more. This often saves us a second round trip in the common steady state. VMStatusPropertiesSelector = []string{ "config.changeTrackingEnabled", "config.extraConfig", "config.hardware.device", - "resourcePool", + "config.keyId", "layoutEx", "guest", + "resourcePool", + "runtime", "summary", } ) @@ -53,8 +55,7 @@ var ( func UpdateStatus( vmCtx pkgctx.VirtualMachineContext, k8sClient ctrlclient.Client, - vcVM *object.VirtualMachine, - refetchProperties bool) error { + vcVM *object.VirtualMachine) error { vm := vmCtx.VM @@ -79,25 +80,6 @@ func UpdateStatus( vm.Status.Class = nil } - if refetchProperties { - // In the common case, our caller will have already gotten the MO - // properties in order to determine if it had any reconciliation to do, - // and there was nothing to do since the VM is in the steady state so - // that MO is still entirely valid here. - // - // NOTE: The properties must have been retrieved with at least - // VMStatusPropertiesSelector. - if err := vcVM.Properties( - vmCtx, - vcVM.Reference(), - VMStatusPropertiesSelector, - &vmCtx.MoVM); err != nil { - - // Leave the current Status unchanged for now. - return fmt.Errorf("failed to get VM properties for status update: %w", err) - } - } - var ( err error errs []error diff --git a/pkg/providers/vsphere/vmlifecycle/update_status_test.go b/pkg/providers/vsphere/vmlifecycle/update_status_test.go index 8ff508222..e1d90dc20 100644 --- a/pkg/providers/vsphere/vmlifecycle/update_status_test.go +++ b/pkg/providers/vsphere/vmlifecycle/update_status_test.go @@ -47,10 +47,9 @@ var _ = Describe("UpdateVM selected MO Properties", func() { var _ = Describe("UpdateStatus", func() { var ( - ctx *builder.TestContextForVCSim - vmCtx pkgctx.VirtualMachineContext - vcVM *object.VirtualMachine - refetchProps bool + ctx *builder.TestContextForVCSim + vmCtx pkgctx.VirtualMachineContext + vcVM *object.VirtualMachine ) BeforeEach(func() { @@ -75,7 +74,6 @@ var _ = Describe("UpdateStatus", func() { vcVM.Reference(), vmlifecycle.VMStatusPropertiesSelector, &vmCtx.MoVM)).To(Succeed()) - refetchProps = false }) AfterEach(func() { @@ -84,14 +82,18 @@ var _ = Describe("UpdateStatus", func() { }) JustBeforeEach(func() { - err := vmlifecycle.UpdateStatus(vmCtx, ctx.Client, vcVM, refetchProps) + err := vmlifecycle.UpdateStatus(vmCtx, ctx.Client, vcVM) Expect(err).ToNot(HaveOccurred()) }) When("properties are refetched", func() { BeforeEach(func() { vmCtx.MoVM = mo.VirtualMachine{} - refetchProps = true + Expect(vcVM.Properties( + ctx, + vcVM.Reference(), + vmlifecycle.VMStatusPropertiesSelector, + &vmCtx.MoVM)).To(Succeed()) }) Specify("the status is created from the properties fetched from vsphere", func() { moVM := mo.VirtualMachine{} diff --git a/pkg/providers/vsphere/vmprovider.go b/pkg/providers/vsphere/vmprovider.go index 35aa2552d..1bd81ccc8 100644 --- a/pkg/providers/vsphere/vmprovider.go +++ b/pkg/providers/vsphere/vmprovider.go @@ -149,6 +149,7 @@ func (vs *vSphereVMProvider) getVcClient(ctx context.Context) (*vcclient.Client, } vs.vcClient = vcClient + return vcClient, nil } @@ -427,3 +428,15 @@ func (vs *vSphereVMProvider) GetTasksByActID(ctx context.Context, actID string) log.V(5).Info("found tasks", "actID", actID, "tasks", taskList) return taskList, nil } + +func (vs *vSphereVMProvider) DoesProfileSupportEncryption( + ctx context.Context, + profileID string) (bool, error) { + + c, err := vs.getVcClient(ctx) + if err != nil { + return false, err + } + + return c.PbmClient().SupportsEncryption(ctx, profileID) +} diff --git a/pkg/providers/vsphere/vmprovider_vm.go b/pkg/providers/vsphere/vmprovider_vm.go index dcd3689ee..5b3225572 100644 --- a/pkg/providers/vsphere/vmprovider_vm.go +++ b/pkg/providers/vsphere/vmprovider_vm.go @@ -45,6 +45,7 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/util/annotations" kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig" ) // VMCreateArgs contains the arguments needed to create a VM on VC. @@ -468,9 +469,10 @@ func (vs *vSphereVMProvider) createdVirtualMachineFallthroughUpdate( // second fetch of the VM properties. var VMUpdatePropertiesSelector = []string{ "config", + "guest", "layoutEx", "resourcePool", - "guest", + "runtime", "summary", } @@ -515,7 +517,8 @@ func (vs *vSphereVMProvider) updateVirtualMachine( } getUpdateArgsFn := func() (*vmUpdateArgs, error) { - // TODO: Use createArgs if we already got them + // TODO: Use createArgs if we already got them, except for: + // - createArgs.ConfigSpec.Crypto _ = createArgs return vs.vmUpdateGetArgs(vmCtx) } @@ -1076,6 +1079,22 @@ func (vs *vSphereVMProvider) vmCreateGenConfigSpec( createArgs.ImageStatus, minCPUFreq) + // Get the encryption class details for the VM. + if pkgcfg.FromContext(vmCtx).Features.BringYourOwnEncryptionKey { + for _, r := range vmconfig.FromContext(vmCtx) { + if err := r.Reconcile( + vmCtx, + vs.k8sClient, + vs.vcClient.VimClient(), + vmCtx.VM, + vmCtx.MoVM, + &configSpec); err != nil { + + return err + } + } + } + err := vs.vmCreateGenConfigSpecExtraConfig(vmCtx, createArgs) if err != nil { return err diff --git a/pkg/providers/vsphere/vmprovider_vm_test.go b/pkg/providers/vsphere/vmprovider_vm_test.go index ab71d0e2f..85a9afd4e 100644 --- a/pkg/providers/vsphere/vmprovider_vm_test.go +++ b/pkg/providers/vsphere/vmprovider_vm_test.go @@ -15,10 +15,12 @@ import ( . "github.com/onsi/gomega/gstruct" corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + vimcrypto "github.com/vmware/govmomi/crypto" "github.com/vmware/govmomi/object" "github.com/vmware/govmomi/vapi/cluster" "github.com/vmware/govmomi/vim25/mo" @@ -29,6 +31,7 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/conditions" pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" pkgconst "github.com/vmware-tanzu/vm-operator/pkg/constants" + "github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels" "github.com/vmware-tanzu/vm-operator/pkg/providers" vsphere "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" @@ -38,6 +41,8 @@ import ( pkgutil "github.com/vmware-tanzu/vm-operator/pkg/util" kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" "github.com/vmware-tanzu/vm-operator/pkg/util/ptr" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto" "github.com/vmware-tanzu/vm-operator/test/builder" ) @@ -46,6 +51,7 @@ const ( vcsimCPUFreq = 2294 ) +//nolint:gocyclo // allowed is 30, this function is 32 func vmTests() { const ( @@ -104,6 +110,12 @@ func vmTests() { }) AfterEach(func() { + if vm != nil && !pkgcfg.FromContext(ctx).Features.BringYourOwnEncryptionKey { + By("Assert vm.Status.Crypto is nil when BYOK is disabled", func() { + Expect(vm.Status.Crypto).To(BeNil()) + }) + } + vmClass = nil vm = nil skipCreateOrUpdateVM = false @@ -253,10 +265,9 @@ func vmTests() { assertClassNotFound := func(className string) { var err error vcVM, err = createOrUpdateAndGetVcVM(ctx, vm) - ExpectWithOffset(1, err).To(MatchError( - fmt.Sprintf( - "virtualmachineclasses.vmoperator.vmware.com %q not found", - className))) + ExpectWithOffset(1, err).ToNot(BeNil()) + ExpectWithOffset(1, err.Error()).To(ContainSubstring( + fmt.Sprintf("virtualmachineclasses.vmoperator.vmware.com %q not found", className))) ExpectWithOffset(1, vcVM).To(BeNil()) } @@ -1359,6 +1370,374 @@ func vmTests() { }) }) + Context("Crypto", Label(testlabels.Crypto), func() { + BeforeEach(func() { + vm.Spec.Crypto = vmopv1.VirtualMachineCryptoSpec{} + }) + JustBeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.BringYourOwnEncryptionKey = true + }) + ctx.Context = vmconfig.WithContext(ctx.Context) + ctx.Context = vmconfig.Register(ctx.Context, crypto.New()) + + var storageClass storagev1.StorageClass + Expect(ctx.Client.Get( + ctx, + client.ObjectKey{Name: ctx.EncryptedStorageClassName}, + &storageClass)).To(Succeed()) + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, + ctx.Client, + storageClass, + true)).To(Succeed()) + }) + + useExistingVM := func( + cryptoSpec vimtypes.BaseCryptoSpec, vTPM bool) { + + vmList, err := ctx.Finder.VirtualMachineList(ctx, "*") + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + ExpectWithOffset(1, vmList).ToNot(BeEmpty()) + + vcVM := vmList[0] + vm.Spec.BiosUUID = vcVM.UUID(ctx) + + powerState, err := vcVM.PowerState(ctx) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + if powerState == vimtypes.VirtualMachinePowerStatePoweredOn { + tsk, err := vcVM.PowerOff(ctx) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + ExpectWithOffset(1, tsk.Wait(ctx)).To(Succeed()) + } + + if cryptoSpec != nil || vTPM { + configSpec := vimtypes.VirtualMachineConfigSpec{ + Crypto: cryptoSpec, + } + if vTPM { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Device: &vimtypes.VirtualTPM{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: -1000, + ControllerKey: 100, + }, + }, + Operation: vimtypes.VirtualDeviceConfigSpecOperationAdd, + }, + } + } + tsk, err := vcVM.Reconfigure(ctx, configSpec) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + ExpectWithOffset(1, tsk.Wait(ctx)).To(Succeed()) + } + } + + When("deploying an encrypted vm", func() { + JustBeforeEach(func() { + vm.Spec.StorageClass = ctx.EncryptedStorageClassName + }) + + When("using a default provider", func() { + + When("default provider is native key provider", func() { + JustBeforeEach(func() { + m := vimcrypto.NewManagerKmip(ctx.VCClient.Client) + Expect(m.MarkDefault(ctx, ctx.NativeKeyProviderID)).To(Succeed()) + }) + + It("should succeed", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + 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()) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + + When("default provider is not native key provider", func() { + JustBeforeEach(func() { + m := vimcrypto.NewManagerKmip(ctx.VCClient.Client) + Expect(m.MarkDefault(ctx, ctx.EncryptionClass1ProviderID)).To(Succeed()) + }) + + It("should succeed", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + 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()) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + }) + + Context("using an encryption class", func() { + + JustBeforeEach(func() { + vm.Spec.Crypto.EncryptionClassName = ctx.EncryptionClass1Name + }) + + It("should succeed", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + 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)) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + }) + + When("encrypting an existing vm", func() { + var ( + hasVTPM bool + ) + + BeforeEach(func() { + hasVTPM = false + }) + + JustBeforeEach(func() { + useExistingVM(nil, hasVTPM) + vm.Spec.StorageClass = ctx.EncryptedStorageClassName + }) + + When("using a default provider", func() { + + When("default provider is native key provider", func() { + JustBeforeEach(func() { + m := vimcrypto.NewManagerKmip(ctx.VCClient.Client) + Expect(m.MarkDefault(ctx, ctx.NativeKeyProviderID)).To(Succeed()) + }) + + It("should succeed", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + 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()) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + + When("default provider is not native key provider", func() { + JustBeforeEach(func() { + m := vimcrypto.NewManagerKmip(ctx.VCClient.Client) + Expect(m.MarkDefault(ctx, ctx.EncryptionClass1ProviderID)).To(Succeed()) + }) + + It("should succeed", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + 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()) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + }) + + Context("using an encryption class", func() { + + JustBeforeEach(func() { + vm.Spec.Crypto.EncryptionClassName = ctx.EncryptionClass2Name + }) + + It("should succeed", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + 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)) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + + When("using a non-encrypted storage class", func() { + JustBeforeEach(func() { + vm.Spec.StorageClass = ctx.StorageClassName + }) + + When("there is no vTPM", func() { + It("should not error, but have condition", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).To(BeNil()) + c := conditions.Get(vm, vmopv1.VirtualMachineEncryptionSynced) + Expect(c).ToNot(BeNil()) + Expect(c.Status).To(Equal(metav1.ConditionFalse)) + Expect(c.Reason).To(Equal("InvalidState")) + Expect(c.Message).To(Equal("Must use encrypted storage class or have vTPM when encrypting vm")) + }) + }) + + When("there is a vTPM", func() { + BeforeEach(func() { + hasVTPM = true + }) + It("should succeed", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + Expect(vm.Status.Crypto.Encrypted).To(HaveExactElements( + []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + })) + Expect(vm.Status.Crypto.ProviderID).To(Equal(ctx.EncryptionClass2ProviderID)) + Expect(vm.Status.Crypto.KeyID).To(Equal(nsInfo.EncryptionClass2KeyID)) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + }) + }) + }) + + When("recrypting a vm", func() { + var ( + hasVTPM bool + ) + + BeforeEach(func() { + hasVTPM = false + }) + + JustBeforeEach(func() { + useExistingVM(&vimtypes.CryptoSpecEncrypt{ + CryptoKeyId: vimtypes.CryptoKeyId{ + KeyId: nsInfo.EncryptionClass1KeyID, + ProviderId: &vimtypes.KeyProviderId{ + Id: ctx.EncryptionClass1ProviderID, + }, + }, + }, hasVTPM) + vm.Spec.StorageClass = ctx.EncryptedStorageClassName + }) + + When("using a default provider", func() { + + When("default provider is native key provider", func() { + JustBeforeEach(func() { + m := vimcrypto.NewManagerKmip(ctx.VCClient.Client) + Expect(m.MarkDefault(ctx, ctx.NativeKeyProviderID)).To(Succeed()) + }) + + It("should succeed", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + 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()) + Expect(vm.Status.Crypto.KeyID).ToNot(Equal(nsInfo.EncryptionClass1KeyID)) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + + When("default provider is not native key provider", func() { + JustBeforeEach(func() { + m := vimcrypto.NewManagerKmip(ctx.VCClient.Client) + Expect(m.MarkDefault(ctx, ctx.EncryptionClass2ProviderID)).To(Succeed()) + }) + + It("should succeed", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + 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()) + Expect(vm.Status.Crypto.KeyID).ToNot(Equal(nsInfo.EncryptionClass1KeyID)) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + }) + + Context("using an encryption class", func() { + + JustBeforeEach(func() { + vm.Spec.Crypto.EncryptionClassName = ctx.EncryptionClass2Name + }) + + It("should succeed", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + 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)) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + + When("using a non-encrypted storage class with a vTPM", func() { + BeforeEach(func() { + hasVTPM = true + }) + + JustBeforeEach(func() { + vm.Spec.StorageClass = ctx.StorageClassName + }) + + It("should succeed", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) + Expect(vm.Status.Crypto).ToNot(BeNil()) + Expect(vm.Status.Crypto.Encrypted).To(HaveExactElements( + []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + })) + Expect(vm.Status.Crypto.ProviderID).To(Equal(ctx.EncryptionClass2ProviderID)) + Expect(vm.Status.Crypto.KeyID).To(Equal(nsInfo.EncryptionClass2KeyID)) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + }) + }) + }) + Context("VM Class with PCI passthrough devices", func() { BeforeEach(func() { vmClass.Spec.Hardware.Devices = vmopv1.VirtualDevices{ @@ -2026,7 +2405,8 @@ func vmTests() { It("Returns error with non-existence cluster module", func() { vm.Annotations["vsphere-cluster-module-group"] = "bogusClusterMod" err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) - Expect(err).To(MatchError("ClusterModule bogusClusterMod not found")) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("ClusterModule bogusClusterMod not found")) }) }) diff --git a/pkg/util/configspec.go b/pkg/util/configspec.go index 3a3c53dcc..056a4d80d 100644 --- a/pkg/util/configspec.go +++ b/pkg/util/configspec.go @@ -6,8 +6,6 @@ package util import ( "bytes" "context" - "encoding/json" - "fmt" "reflect" "github.com/vmware/govmomi/vim25" @@ -203,35 +201,5 @@ func EnsureMinHardwareVersionInConfigSpec( func SafeConfigSpecToString( in *vimtypes.VirtualMachineConfigSpec) (s string) { - if in == nil { - return "null" - } - - marshalWithStdlibJSONEncoder := func() string { - data, err := json.Marshal(in) - if err != nil { - return fmt.Sprintf("%v", in) - } - return string(data) - } - - defer func() { - if err := recover(); err != nil { - s = marshalWithStdlibJSONEncoder() - } - }() - - var w bytes.Buffer - enc := vimtypes.NewJSONEncoder(&w) - if err := enc.Encode(in); err != nil { - return marshalWithStdlibJSONEncoder() - } - - s = w.String() - if len(s) == 0 { - return s - } - - // Do not include the newline character added by the vimtype JSON encoder. - return s[:len(s)-1] + return vimtypes.ToString(in) } diff --git a/pkg/util/configspec_test.go b/pkg/util/configspec_test.go index 32bee3097..c00cecfb0 100644 --- a/pkg/util/configspec_test.go +++ b/pkg/util/configspec_test.go @@ -900,6 +900,6 @@ var _ = DescribeTable( &vimtypes.VirtualMachineConfigSpec{ VAppConfig: (*vimtypes.VmConfigSpec)(nil), }, - `{"vAppConfig":null}`, + `{"_typeName":"VirtualMachineConfigSpec","vAppConfig":null}`, ), ) diff --git a/pkg/util/kube/internal/internal_kube_constants.go b/pkg/util/kube/internal/internal_kube_constants.go new file mode 100644 index 000000000..c99c3f559 --- /dev/null +++ b/pkg/util/kube/internal/internal_kube_constants.go @@ -0,0 +1,30 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package internal + +const ( + // StoragePolicyIDParameter is the name of the parameter set on a + // StorageClass that indicates the identifier of the underlying + // storage policy/profile. + StoragePolicyIDParameter = "storagePolicyID" + + // StorageClassKind is the kind for a StorageClass resource. + StorageClassKind = "StorageClass" + + // StorageClassGroup is the API group to which a StorageClass resource + // belongs. + StorageClassGroup = "storage.k8s.io" + + // StorageClassResource is the API resource for a StorageClass. + StorageClassResource = "storageclasses" + + // StorageClassGroupVersion is the API group and version version for a + // StorageClass resource. + StorageClassGroupVersion = StorageClassGroup + "/v1" + + // EncryptedStorageClassNamesConfigMapName is the name of the ConfigMap in + // the VM Operator pod's namespace that indicates which StorageClasses + // support encryption by virtue of the OwnerRefs set on the ConfigMap. + EncryptedStorageClassNamesConfigMapName = "encrypted-storage-class-names" +) diff --git a/pkg/util/kube/internal/internal_kube_storage.go b/pkg/util/kube/internal/internal_kube_storage.go new file mode 100644 index 000000000..592f5cdf7 --- /dev/null +++ b/pkg/util/kube/internal/internal_kube_storage.go @@ -0,0 +1,63 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package internal + +import ( + "context" + + corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" +) + +// GetOwnerRefForStorageClass returns an OwnerRef for the provided StorageClass. +func GetOwnerRefForStorageClass( + storageClass storagev1.StorageClass) metav1.OwnerReference { + + return metav1.OwnerReference{ + APIVersion: StorageClassGroupVersion, + Kind: StorageClassKind, + Name: storageClass.Name, + UID: storageClass.UID, + } +} + +// GetEncryptedStorageClassRefs returns a list of the OwnerRef objects for +// StorageClasses marked as encrypted. +func GetEncryptedStorageClassRefs( + ctx context.Context, + k8sClient ctrlclient.Client) ([]metav1.OwnerReference, error) { + + var obj corev1.ConfigMap + if err := k8sClient.Get( + ctx, + ctrlclient.ObjectKey{ + Namespace: pkgcfg.FromContext(ctx).PodNamespace, + Name: EncryptedStorageClassNamesConfigMapName, + }, + &obj); err != nil { + + if apierrors.IsNotFound(err) { + return nil, nil + } + + return nil, err + } + + var refs []metav1.OwnerReference + for i := range obj.OwnerReferences { + ref := obj.OwnerReferences[i] + if ref.Kind == StorageClassKind && + ref.APIVersion == StorageClassGroupVersion { + + refs = append(refs, ref) + } + } + + return refs, nil +} diff --git a/pkg/util/kube/internal/internal_kube_storage_test.go b/pkg/util/kube/internal/internal_kube_storage_test.go new file mode 100644 index 000000000..b7688db26 --- /dev/null +++ b/pkg/util/kube/internal/internal_kube_storage_test.go @@ -0,0 +1,159 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package internal_test + +import ( + "context" + "errors" + + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" + "github.com/vmware-tanzu/vm-operator/pkg/util/kube/internal" +) + +var _ = Describe("GetEncryptedStorageClassRefs", func() { + + const fakeString = "fake" + + var ( + ctx context.Context + client ctrlclient.Client + obj corev1.ConfigMap + err error + funcs interceptor.Funcs + refs []metav1.OwnerReference + ) + + BeforeEach(func() { + ctx = pkgcfg.WithConfig(pkgcfg.Config{ + PodNamespace: fakeString, + }) + funcs = interceptor.Funcs{} + obj = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: pkgcfg.FromContext(ctx).PodNamespace, + Name: internal.EncryptedStorageClassNamesConfigMapName, + }, + } + }) + + JustBeforeEach(func() { + client = fake.NewClientBuilder(). + WithInterceptorFuncs(funcs). + WithObjects(&obj). + Build() + refs, err = internal.GetEncryptedStorageClassRefs(ctx, client) + }) + + When("there is a 404 error getting the ConfigMap", func() { + BeforeEach(func() { + funcs.Get = func( + ctx context.Context, + client ctrlclient.WithWatch, + key ctrlclient.ObjectKey, + obj ctrlclient.Object, + opts ...ctrlclient.GetOption) error { + + return apierrors.NewNotFound( + schema.GroupResource{ + Group: internal.StorageClassGroup, + Resource: internal.StorageClassResource, + }, + obj.GetName()) + } + }) + It("should return an empty slice", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(refs).To(BeEmpty()) + }) + }) + + When("there is a non-404 error getting the ConfigMap", func() { + BeforeEach(func() { + funcs.Get = func( + ctx context.Context, + client ctrlclient.WithWatch, + key ctrlclient.ObjectKey, + obj ctrlclient.Object, + opts ...ctrlclient.GetOption) error { + + return apierrors.NewInternalError(errors.New(fakeString)) + } + }) + It("should return an empty slice", func() { + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError( + apierrors.NewInternalError(errors.New(fakeString)).Error())) + Expect(refs).To(BeEmpty()) + }) + }) + + When("there are no owner refs", func() { + It("should return an empty slice", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(refs).To(BeEmpty()) + }) + }) + + When("there are owner refs", func() { + + var storageClasses []storagev1.StorageClass + + BeforeEach(func() { + storageClasses = []storagev1.StorageClass{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + UID: types.UID(uuid.NewString()), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + UID: types.UID(uuid.NewString()), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + UID: types.UID(uuid.NewString()), + }, + }, + } + + obj.OwnerReferences = make( + []metav1.OwnerReference, len(storageClasses)) + + for i := range storageClasses { + obj.OwnerReferences[i] = internal.GetOwnerRefForStorageClass( + storageClasses[i]) + } + }) + It("should return the owner refs", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(refs).To(HaveLen(len(storageClasses))) + + expRefs := make([]any, len(storageClasses)) + for i := range storageClasses { + expRefs[i] = internal.GetOwnerRefForStorageClass( + storageClasses[i]) + } + Expect(refs).To(HaveLen(len(expRefs))) + Expect(refs).To(ContainElements(expRefs...)) + }) + }) +}) diff --git a/pkg/util/kube/internal/internal_kube_suite_test.go b/pkg/util/kube/internal/internal_kube_suite_test.go new file mode 100644 index 000000000..f0517fb41 --- /dev/null +++ b/pkg/util/kube/internal/internal_kube_suite_test.go @@ -0,0 +1,24 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package internal_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "k8s.io/klog/v2" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +func init() { + klog.SetOutput(GinkgoWriter) + logf.SetLogger(klog.Background()) +} + +func TestKube(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Kube Util Internal Test Suite") +} diff --git a/pkg/util/kube/kube_suite_test.go b/pkg/util/kube/kube_suite_test.go index 1046e4759..57be78d19 100644 --- a/pkg/util/kube/kube_suite_test.go +++ b/pkg/util/kube/kube_suite_test.go @@ -15,6 +15,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/pkg/util/kube/storage.go b/pkg/util/kube/storage.go new file mode 100644 index 000000000..2890a0d7f --- /dev/null +++ b/pkg/util/kube/storage.go @@ -0,0 +1,154 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package kube + +import ( + "context" + "fmt" + "slices" + + corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" + "github.com/vmware-tanzu/vm-operator/pkg/util/kube/internal" +) + +// 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 policyID, nil +} + +// MarkEncryptedStorageClass records the provided StorageClass as encrypted. +func MarkEncryptedStorageClass( + ctx context.Context, + k8sClient ctrlclient.Client, + storageClass storagev1.StorageClass, + encrypted bool) error { + + var ( + obj = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: pkgcfg.FromContext(ctx).PodNamespace, + Name: internal.EncryptedStorageClassNamesConfigMapName, + }, + } + ownerRef = internal.GetOwnerRefForStorageClass(storageClass) + ) + + // Get the ConfigMap. + err := k8sClient.Get(ctx, ctrlclient.ObjectKeyFromObject(&obj), &obj) + + if err != nil { + // Return any error other than 404. + if !apierrors.IsNotFound(err) { + return err + } + + // + // The ConfigMap was not found. + // + + if !encrypted { + // If the goal is to mark the StorageClass ss not encrypted, then we + // do not need to actually create the underlying ConfigMap if it does + // not exist. + return nil + } + + // The ConfigMap does not already exist and the goal is to mark the + // StorageClass as encrypted, so go ahead and create the ConfigMap and + // return early. + obj.OwnerReferences = []metav1.OwnerReference{ownerRef} + return k8sClient.Create(ctx, &obj) + } + + // + // The ConfigMap already exists, so check if it needs to be updated. + // + + storageClassIsOwner := slices.Contains(obj.OwnerReferences, ownerRef) + + switch { + case encrypted && storageClassIsOwner: + // The StorageClass should be marked encrypted, which means it should + // be in the ConfigMap. Since the StorageClass is currently set in the + // ConfigMap, we can return early. + return nil + case !encrypted && !storageClassIsOwner: + // The StorageClass should be marked as not encrypted, which means it + // should not be in the ConfigMap. Since the StorageClass is not + // currently in the ConfigMap, we can return return early. + return nil + } + + // Create the patch used to update the ConfigMap. + objPatch := ctrlclient.StrategicMergeFrom(obj.DeepCopyObject().(ctrlclient.Object)) + if encrypted { + // Add the StorageClass as an owner of the ConfigMap. + obj.OwnerReferences = append(obj.OwnerReferences, ownerRef) + } else { + // Remove the StorageClass as an owner of the ConfigMap. + obj.OwnerReferences = slices.DeleteFunc( + obj.OwnerReferences, + func(o metav1.OwnerReference) bool { return o == ownerRef }) + } + + // Patch the ConfigMap with the change. + return k8sClient.Patch(ctx, &obj, objPatch) +} + +// IsEncryptedStorageClass returns true if the provided StorageClass was marked +// as encrypted. +func IsEncryptedStorageClass( + ctx context.Context, + k8sClient ctrlclient.Client, + storageClassName string) (bool, error) { + + var storageClass storagev1.StorageClass + if err := k8sClient.Get( + ctx, + ctrlclient.ObjectKey{Name: storageClassName}, + &storageClass); err != nil { + + if apierrors.IsNotFound(err) { + return false, nil + } + + return false, err + } + + var ( + obj corev1.ConfigMap + ownerRef = internal.GetOwnerRefForStorageClass(storageClass) + ) + + if err := k8sClient.Get( + ctx, + ctrlclient.ObjectKey{ + Namespace: pkgcfg.FromContext(ctx).PodNamespace, + Name: internal.EncryptedStorageClassNamesConfigMapName, + }, + &obj); err != nil { + + if apierrors.IsNotFound(err) { + return false, nil + } + + return false, err + } + + return slices.Contains(obj.OwnerReferences, ownerRef), nil +} diff --git a/pkg/util/kube/storage_test.go b/pkg/util/kube/storage_test.go new file mode 100644 index 000000000..fd747be49 --- /dev/null +++ b/pkg/util/kube/storage_test.go @@ -0,0 +1,497 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package kube_test + +import ( + "context" + "errors" + "strconv" + "sync" + + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/envtest" + + pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" + kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" + "github.com/vmware-tanzu/vm-operator/pkg/util/kube/internal" +) + +var _ = DescribeTable("GetStoragePolicyID", + func(inPolicyID, expPolicyID, expErr string) { + obj := storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-storage-class", + }, + } + if inPolicyID != "" { + obj.Parameters = map[string]string{"storagePolicyID": inPolicyID} + } + policyID, err := kubeutil.GetStoragePolicyID(obj) + if expErr != "" { + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(expErr)) + } else { + Expect(policyID).To(Equal(expPolicyID)) + } + }, + Entry( + "has policy ID", + fakeString, + fakeString, + ""), + + Entry( + "does not have policy ID", + "", + fakeString, + `StorageClass "my-storage-class" does not have 'storagePolicyID' parameter`), +) + +var _ = Describe("IsEncryptedStorageClass", func() { + var ( + ok bool + ctx context.Context + err error + client ctrlclient.Client + funcs interceptor.Funcs + storageClass storagev1.StorageClass + ) + + BeforeEach(func() { + ctx = pkgcfg.WithConfig(pkgcfg.Config{ + PodNamespace: fakeString, + }) + funcs = interceptor.Funcs{} + storageClass = storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: fakeString, + UID: types.UID(uuid.NewString()), + }, + } + }) + + JustBeforeEach(func() { + client = fake.NewClientBuilder(). + WithObjects(&storageClass). + WithInterceptorFuncs(funcs). + Build() + + ok, err = kubeutil.IsEncryptedStorageClass( + ctx, client, storageClass.Name) + }) + + When("getting the StorageClass returns a 404 error", func() { + BeforeEach(func() { + funcs.Get = func( + ctx context.Context, + client ctrlclient.WithWatch, + key ctrlclient.ObjectKey, + obj ctrlclient.Object, + opts ...ctrlclient.GetOption) error { + + if _, ok := obj.(*storagev1.StorageClass); ok { + return apierrors.NewNotFound( + schema.GroupResource{ + Group: internal.StorageClassGroup, + Resource: internal.StorageClassResource, + }, + obj.GetName()) + } + + return client.Get(ctx, key, obj, opts...) + } + }) + It("should not return an error", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeFalse()) + }) + }) + + When("getting the StorageClass returns a non-404 error", func() { + BeforeEach(func() { + funcs.Get = func( + ctx context.Context, + client ctrlclient.WithWatch, + key ctrlclient.ObjectKey, + obj ctrlclient.Object, + opts ...ctrlclient.GetOption) error { + + if _, ok := obj.(*storagev1.StorageClass); ok { + return apierrors.NewInternalError(errors.New(fakeString)) + } + + return client.Get(ctx, key, obj, opts...) + } + }) + It("should not return the error", func() { + Expect(err).To(MatchError(apierrors.NewInternalError(errors.New(fakeString)).Error())) + Expect(ok).To(BeFalse()) + }) + }) +}) + +var _ = Describe("EncryptedStorageClass", func() { + var ( + ctx context.Context + client ctrlclient.Client + funcs interceptor.Funcs + storageClass storagev1.StorageClass + ) + + BeforeEach(func() { + ctx = pkgcfg.WithConfig(pkgcfg.Config{ + PodNamespace: fakeString, + }) + funcs = interceptor.Funcs{} + storageClass = storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: fakeString, + UID: types.UID(uuid.NewString()), + }, + } + }) + + JustBeforeEach(func() { + client = fake.NewClientBuilder(). + WithObjects(&storageClass). + WithInterceptorFuncs(funcs). + Build() + }) + + When("the storage class is marked as encrypted", 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) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + }) + 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) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + }) + }) + When("the storage class is marked as unencrypted", func() { + JustBeforeEach(func() { + Expect(kubeutil.MarkEncryptedStorageClass(ctx, client, storageClass, false)).To(Succeed()) + }) + It("should return false", func() { + ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeFalse()) + }) + + 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) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + }) + }) + }) + + When("a second storage class is marked as not encrypted", func() { + JustBeforeEach(func() { + storageClass.Name += "1" + 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) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeFalse()) + }) + }) + }) + + When("the storage class is not marked as not encrypted", func() { + JustBeforeEach(func() { + Expect(kubeutil.MarkEncryptedStorageClass(ctx, client, storageClass, false)).To(Succeed()) + }) + It("should return false", func() { + ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeFalse()) + }) + }) + + When("the storage class is not marked at all", func() { + It("should return false", func() { + ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeFalse()) + }) + }) + + When("getting the underlying ConfigMap produces a non-404 error", func() { + BeforeEach(func() { + funcs.Get = func( + ctx context.Context, + client ctrlclient.WithWatch, + key ctrlclient.ObjectKey, + obj ctrlclient.Object, + opts ...ctrlclient.GetOption) error { + + if _, ok := obj.(*corev1.ConfigMap); ok { + return apierrors.NewInternalError(errors.New(fakeString)) + } + + return client.Get(ctx, key, obj, opts...) + } + }) + It("should return an error for IsEncryptedStorageClass", func() { + ok, err := kubeutil.IsEncryptedStorageClass(ctx, client, storageClass.Name) + Expect(err).To(MatchError(apierrors.NewInternalError(errors.New(fakeString)).Error())) + Expect(ok).To(BeFalse()) + }) + It("should return an error for MarkEncryptedStorageClass", func() { + err := kubeutil.MarkEncryptedStorageClass(ctx, client, storageClass, false) + Expect(err).To(MatchError(apierrors.NewInternalError(errors.New(fakeString)).Error())) + }) + }) + + // Please note, this test requires the use of envtest due to a bug in the + // fake client that does not support concurrent patch attempts against the + // same ConfigMap resource. + When("there are concurrent attempts to update the ConfigMap", func() { + + var ( + env envtest.Environment + numAttempts int + storageClasses []storagev1.StorageClass + ) + + BeforeEach(func() { + numAttempts = 10 + + ctx = pkgcfg.WithConfig(pkgcfg.Config{ + PodNamespace: "default", + }) + }) + + JustBeforeEach(func() { + env = envtest.Environment{} + restConfig, err := env.Start() + Expect(err).ToNot(HaveOccurred()) + Expect(restConfig).ToNot(BeNil()) + + c, err := ctrlclient.New(restConfig, ctrlclient.Options{}) + Expect(err).ToNot(HaveOccurred()) + Expect(c).ToNot(BeNil()) + client = c + + storageClasses = make([]storagev1.StorageClass, numAttempts) + for i := range storageClasses { + storageClasses[i].Name = strconv.Itoa(i) + storageClasses[i].Provisioner = fakeString + Expect(client.Create(ctx, &storageClasses[i])).To(Succeed()) + } + }) + + AfterEach(func() { + Expect(env.Stop()).To(Succeed()) + }) + + When("there are concurrent additions", func() { + JustBeforeEach(func() { + // Ensure the ConfigMap exists so all of the additions use the + // Patch operation. + Expect(client.Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: internal.EncryptedStorageClassNamesConfigMapName, + Namespace: pkgcfg.FromContext(ctx).PodNamespace, + }, + })).To(Succeed()) + + var ( + ready sync.WaitGroup + marked sync.WaitGroup + start = make(chan struct{}) + ) + + marked.Add(len(storageClasses)) + ready.Add(len(storageClasses)) + + for i := range storageClasses { + go func(obj storagev1.StorageClass) { + defer GinkgoRecover() + defer func() { + marked.Done() + }() + ready.Done() + <-start + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, client, obj, true)).To(Succeed(), + "StorageClass.name="+obj.Name) + }(storageClasses[i]) + } + + ready.Wait() + close(start) + marked.Wait() + }) + + Specify("all concurrent attempts should succeed", func() { + refs, err := internal.GetEncryptedStorageClassRefs(ctx, client) + Expect(err).ToNot(HaveOccurred()) + + expRefs := make([]any, len(storageClasses)) + for i := range storageClasses { + expRefs[i] = internal.GetOwnerRefForStorageClass(storageClasses[i]) + } + Expect(refs).To(HaveLen(len(expRefs))) + Expect(refs).To(ContainElements(expRefs...)) + + for i := range storageClasses { + ok, err := kubeutil.IsEncryptedStorageClass( + ctx, client, storageClasses[i].Name) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + } + }) + }) + + When("there are concurrent removals", func() { + JustBeforeEach(func() { + // Mark all the StorageClasses as encrypted. + for i := range storageClasses { + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, client, storageClasses[i], true)).To(Succeed()) + } + + var ( + ready sync.WaitGroup + marked sync.WaitGroup + start = make(chan struct{}) + ) + + marked.Add(len(storageClasses)) + ready.Add(len(storageClasses)) + + for i := range storageClasses { + go func(obj storagev1.StorageClass) { + defer GinkgoRecover() + defer func() { + marked.Done() + }() + ready.Done() + <-start + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, client, obj, false)).To(Succeed(), + "StorageClass.name="+obj.Name) + }(storageClasses[i]) + } + + ready.Wait() + close(start) + marked.Wait() + }) + + Specify("all concurrent attempts should succeed", func() { + refs, err := internal.GetEncryptedStorageClassRefs(ctx, client) + Expect(err).ToNot(HaveOccurred()) + Expect(refs).To(BeEmpty()) + + for i := range storageClasses { + ok, err := kubeutil.IsEncryptedStorageClass( + ctx, client, storageClasses[i].Name) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeFalse()) + } + }) + }) + + When("there are concurrent additions and removals", func() { + BeforeEach(func() { + // Set the number of attempts to odd to ensure the count at the + // end is not a false-positive due to equal halves. + numAttempts = 13 + }) + + JustBeforeEach(func() { + // Mark half of the StorageClasses as encrypted. + for i := range storageClasses { + if i%2 == 0 { + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, client, storageClasses[i], true)).To(Succeed()) + } + } + + var ( + ready sync.WaitGroup + marked sync.WaitGroup + start = make(chan struct{}) + ) + + marked.Add(len(storageClasses)) + ready.Add(len(storageClasses)) + + for i := range storageClasses { + go func(i int, obj storagev1.StorageClass) { + defer GinkgoRecover() + defer func() { + marked.Done() + }() + ready.Done() + <-start + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, client, obj, i%2 != 0)).To(Succeed(), + "StorageClass.name="+obj.Name) + }(i, storageClasses[i]) + } + + ready.Wait() + close(start) + marked.Wait() + }) + + Specify("all concurrent attempts should succeed", func() { + refs, err := internal.GetEncryptedStorageClassRefs(ctx, client) + Expect(err).ToNot(HaveOccurred()) + + var expRefs []any + for i := range storageClasses { + if i%2 != 0 { + expRefs = append( + expRefs, + internal.GetOwnerRefForStorageClass(storageClasses[i])) + } + } + Expect(refs).To(HaveLen(len(expRefs))) + Expect(refs).To(ContainElements(expRefs...)) + + for i := range storageClasses { + ok, err := kubeutil.IsEncryptedStorageClass( + ctx, client, storageClasses[i].Name) + Expect(err).ToNot(HaveOccurred()) + if i%2 == 0 { + Expect(ok).To(BeFalse()) + } else { + Expect(ok).To(BeTrue()) + } + } + }) + }) + }) +}) diff --git a/pkg/util/paused/paused.go b/pkg/util/paused/paused.go new file mode 100644 index 000000000..a492da7b7 --- /dev/null +++ b/pkg/util/paused/paused.go @@ -0,0 +1,24 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package paused + +import ( + "github.com/vmware/govmomi/object" + "github.com/vmware/govmomi/vim25/mo" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" + "github.com/vmware-tanzu/vm-operator/pkg/util/annotations" +) + +func ByDevOps(vm *vmopv1.VirtualMachine) bool { + return annotations.HasPaused(vm) +} + +func ByAdmin(moVM mo.VirtualMachine) bool { + if moVM.Config == nil { + return false + } + return object.OptionValueList(moVM.Config.ExtraConfig). + IsTrue(vmopv1.PauseVMExtraConfigKey) +} diff --git a/pkg/util/paused/paused_suite_test.go b/pkg/util/paused/paused_suite_test.go new file mode 100644 index 000000000..c0ea4dd3c --- /dev/null +++ b/pkg/util/paused/paused_suite_test.go @@ -0,0 +1,24 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package paused_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "k8s.io/klog/v2" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +func init() { + klog.SetOutput(GinkgoWriter) + logf.SetLogger(klog.Background()) +} + +func TestPtr(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Paused Util Test Suite") +} diff --git a/pkg/util/paused/paused_test.go b/pkg/util/paused/paused_test.go new file mode 100644 index 000000000..2e331dc94 --- /dev/null +++ b/pkg/util/paused/paused_test.go @@ -0,0 +1,110 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package paused_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/vmware/govmomi/vim25/mo" + vimtypes "github.com/vmware/govmomi/vim25/types" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" + "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" + "github.com/vmware-tanzu/vm-operator/pkg/util/paused" +) + +var _ = Describe("ByAdmin", func() { + var mgdObj mo.VirtualMachine + + BeforeEach(func() { + mgdObj = mo.VirtualMachine{ + ManagedEntity: mo.ManagedEntity{ + ExtensibleManagedObject: mo.ExtensibleManagedObject{ + Self: vimtypes.ManagedObjectReference{ + Type: "VirtualMachine", + Value: "vm-44", + }, + }, + }, + Config: &vimtypes.VirtualMachineConfigInfo{ + ExtraConfig: []vimtypes.BaseOptionValue{}, + }, + } + }) + + It("should return false when moVM.Config is nil", func() { + mgdObj.Config = nil + Expect(paused.ByAdmin(mgdObj)).To(BeFalse()) + }) + + It("should return false when PauseVMExtraConfigKey is not set", func() { + Expect(paused.ByAdmin(mgdObj)).To(BeFalse()) + }) + + It("should return false when PauseVMExtraConfigKey is set to False", func() { + mgdObj.Config.ExtraConfig = append(mgdObj.Config.ExtraConfig, &vimtypes.OptionValue{ + Key: vmopv1.PauseVMExtraConfigKey, + Value: constants.ExtraConfigFalse, + }) + + Expect(paused.ByAdmin(mgdObj)).To(BeFalse()) + }) + + It("should return true when PauseVMExtraConfigKey is set to 1", func() { + mgdObj.Config.ExtraConfig = append(mgdObj.Config.ExtraConfig, &vimtypes.OptionValue{ + Key: vmopv1.PauseVMExtraConfigKey, + Value: 1, + }) + + Expect(paused.ByAdmin(mgdObj)).To(BeTrue()) + }) + + It("should return true when PauseVMExtraConfigKey is set to 'true'", func() { + mgdObj.Config.ExtraConfig = append(mgdObj.Config.ExtraConfig, &vimtypes.OptionValue{ + Key: vmopv1.PauseVMExtraConfigKey, + Value: "true", + }) + + Expect(paused.ByAdmin(mgdObj)).To(BeTrue()) + }) + + It("should return true when PauseVMExtraConfigKey is set to 'True'", func() { + mgdObj.Config.ExtraConfig = append(mgdObj.Config.ExtraConfig, &vimtypes.OptionValue{ + Key: vmopv1.PauseVMExtraConfigKey, + Value: "True", + }) + + Expect(paused.ByAdmin(mgdObj)).To(BeTrue()) + }) + + It("should return true when PauseVMExtraConfigKey is set to 'TRUE'", func() { + mgdObj.Config.ExtraConfig = append(mgdObj.Config.ExtraConfig, &vimtypes.OptionValue{ + Key: vmopv1.PauseVMExtraConfigKey, + Value: constants.ExtraConfigTrue, + }) + + Expect(paused.ByAdmin(mgdObj)).To(BeTrue()) + }) +}) + +var _ = Describe("ByDevOps", func() { + When("paused", func() { + It("should return true", func() { + Expect(paused.ByDevOps(&vmopv1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + vmopv1.PauseAnnotation: "true", + }, + }, + })).To(BeTrue()) + }) + }) + When("not paused", func() { + It("should return false", func() { + Expect(paused.ByDevOps(&vmopv1.VirtualMachine{})).To(BeFalse()) + }) + }) +}) diff --git a/pkg/util/vsphere/client/client.go b/pkg/util/vsphere/client/client.go index 48a41a388..f7a9e2645 100644 --- a/pkg/util/vsphere/client/client.go +++ b/pkg/util/vsphere/client/client.go @@ -12,6 +12,7 @@ import ( "github.com/vmware/govmomi/find" "github.com/vmware/govmomi/object" + "github.com/vmware/govmomi/pbm" "github.com/vmware/govmomi/session" "github.com/vmware/govmomi/session/keepalive" "github.com/vmware/govmomi/vapi/rest" @@ -36,6 +37,7 @@ type Config struct { type Client struct { vimClient *vim25.Client restClient *rest.Client + pbmClient *pbm.Client sessionManager *session.Manager config Config @@ -60,11 +62,17 @@ func NewClient(ctx context.Context, config Config) (*Client, error) { return nil, err } + pbmClient, err := pbm.NewClient(ctx, vimClient) + if err != nil { + return nil, err + } + return &Client{ vimClient: vimClient, finder: finder, datacenter: datacenter, restClient: restClient, + pbmClient: pbmClient, sessionManager: sm, config: config, }, nil @@ -272,6 +280,10 @@ func (c *Client) Datacenter() *object.Datacenter { return c.datacenter } +func (c *Client) PbmClient() *pbm.Client { + return c.pbmClient +} + func (c *Client) RestClient() *rest.Client { return c.restClient } diff --git a/pkg/util/vsphere/client/client_test.go b/pkg/util/vsphere/client/client_test.go index 936e9132d..b86d38f35 100644 --- a/pkg/util/vsphere/client/client_test.go +++ b/pkg/util/vsphere/client/client_test.go @@ -14,6 +14,7 @@ import ( . "github.com/onsi/gomega" "github.com/go-logr/logr" + _ "github.com/vmware/govmomi/pbm/simulator" // load PBM simulator "github.com/vmware/govmomi/session" "github.com/vmware/govmomi/session/keepalive" "github.com/vmware/govmomi/simulator" diff --git a/pkg/vmconfig/crypto/crypto_reconciler.go b/pkg/vmconfig/crypto/crypto_reconciler.go new file mode 100644 index 000000000..26eecfebb --- /dev/null +++ b/pkg/vmconfig/crypto/crypto_reconciler.go @@ -0,0 +1,109 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package crypto + +import ( + "context" + "fmt" + "strings" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" + "github.com/vmware-tanzu/vm-operator/pkg/bitmask" + "github.com/vmware-tanzu/vm-operator/pkg/conditions" + ctxgen "github.com/vmware-tanzu/vm-operator/pkg/context/generic" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto/internal" +) + +type reconciler struct{} + +var _ vmconfig.Reconciler = reconciler{} + +// New returns a new Reconciler for a VM's crypto state. +func New() vmconfig.ReconcilerWithContext { + return reconciler{} +} + +// Name returns the unique name used to identify the reconciler. +func (r reconciler) Name() string { + return "crypto" +} + +func (r reconciler) WithContext(parent context.Context) context.Context { + return ctxgen.WithContext( + parent, + internal.ContextKeyValue, + func() internal.State { return internal.State{} }) +} + +// SprintfStateNotSynced formats and returns the message for when the encryption +// state cannot be synced. +func SprintfStateNotSynced(op string, msgs ...string) string { + if len(msgs) == 0 { + return "" + } + var msg string + switch len(msgs) { + case 1: + msg = msgs[0] + case 2: + msg = fmt.Sprintf("%s and %s", msgs[0], msgs[1]) + default: + msg = fmt.Sprintf( + "%s, and %s", + strings.Join(msgs[:len(msgs)-1], ", "), + msgs[len(msgs)-1]) + } + return fmt.Sprintf("Must %s when %s vm", msg, op) +} + +// Reason is the type used by reasons given to the condition +// vmopv1.VirtualMachineEncryptionSynced. +type Reason uint8 + +const ( + ReasonEncryptionClassNotFound Reason = 1 << iota + ReasonEncryptionClassInvalid + ReasonInvalidState + ReasonInvalidChanges + ReasonReconfigureError + _reasonMax = 1 << (iota - 1) +) + +func (r Reason) MaxValue() Reason { + return _reasonMax +} + +func (r Reason) StringValue() string { + switch r { + case ReasonEncryptionClassNotFound: + return "EncryptionClassNotFound" + case ReasonEncryptionClassInvalid: + return "EncryptionClassInvalid" + case ReasonInvalidState: + return "InvalidState" + case ReasonInvalidChanges: + return "InvalidChanges" + case ReasonReconfigureError: + return "ReconfigureError" + } + return "" +} + +func (r Reason) String() string { + return bitmask.String(r) +} + +func markEncryptionStateNotSynced( + vm *vmopv1.VirtualMachine, + op string, + reason Reason, + msgs ...string) { + + conditions.MarkFalse( + vm, + vmopv1.VirtualMachineEncryptionSynced, + reason.String(), + SprintfStateNotSynced(op, msgs...)) +} diff --git a/pkg/vmconfig/crypto/crypto_reconciler_post.go b/pkg/vmconfig/crypto/crypto_reconciler_post.go new file mode 100644 index 000000000..49fb1f21d --- /dev/null +++ b/pkg/vmconfig/crypto/crypto_reconciler_post.go @@ -0,0 +1,192 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package crypto + +import ( + "context" + + "github.com/vmware/govmomi/fault" + "github.com/vmware/govmomi/vim25/mo" + vimtypes "github.com/vmware/govmomi/vim25/types" + + "github.com/vmware-tanzu/vm-operator/pkg/conditions" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto/internal" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" +) + +//nolint:gocyclo +func (r reconciler) OnResult( + ctx context.Context, + vm *vmopv1.VirtualMachine, + moVM mo.VirtualMachine, + resultErr error) error { + + if ctx == nil { + panic("context is nil") + } + if moVM.Config == nil { + panic("moVM.config is nil") + } + if vm == nil { + panic("vm is nil") + } + + state := internal.FromContext(ctx) + + // Update the VM's crypto status. + if key := getCurCryptoKey(moVM); key.provider == "" && key.id == "" { + vm.Status.Crypto = nil + } else { + if vm.Status.Crypto == nil { + vm.Status.Crypto = &vmopv1.VirtualMachineCryptoStatus{} + } + vm.Status.Crypto.ProviderID = key.provider + vm.Status.Crypto.KeyID = key.id + + if state.IsEncStorClass { + vm.Status.Crypto.Encrypted = []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + vmopv1.VirtualMachineEncryptionTypeDisks, + } + } else if ok, _ := hasVTPM(moVM, nil); ok { + vm.Status.Crypto.Encrypted = []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + } + } + } + + if resultErr == nil { + + // If no reconfigure error occurred then we need to check if there was + // a crypto update as part of the reconfigure. + + if state.Operation == "encrypting" || state.Operation == "recrypting" { + + // A crypto update was successful, so indicate that the encryption + // state of this VM is synced. + conditions.MarkTrue(vm, vmopv1.VirtualMachineEncryptionSynced) + } + + return nil + } + + // Determine the message to put on the condition. + var msgs []string + + // + // At this point we know that a reconfigure error occurred *and* there was + // a crypto update in the ConfigSpec. It is time to parse the reconfigErr + // to determine if it was related to the crypto update. + // + + fault.In( + resultErr, + func( + fault vimtypes.BaseMethodFault, + localizedMessage string, + localizableMessages []vimtypes.LocalizableMessage) bool { + + switch tErr := fault.(type) { + case *vimtypes.GenericVmConfigFault: + for i := range localizableMessages { + switch localizableMessages[i].Key { + case "msg.vigor.enc.keyNotFound": + msgs = append(msgs, "specify a valid key") + case "msg.keysafe.locator": + msgs = append(msgs, "specify a key that can be located") + case "msg.vtpm.add.notEncrypted": + msgs = append(msgs, "add vTPM") + case "msg.vigor.enc.required.vtpm": + msgs = append(msgs, "have vTPM") + } + } + case *vimtypes.SystemError: + switch localizedMessage { + case "Error creating disk Key locator": + msgs = append(msgs, "specify a valid key") + case "Key locator error": + msgs = append(msgs, "specify a key that can be located") + case "Key required for encryption.bundle.": + msgs = append(msgs, "not specify encryption bundle") + } + case *vimtypes.NotSupported: + for i := range localizableMessages { + //nolint:gocritic + switch localizableMessages[i].Key { + case "msg.disk.policyChangeFailure": + msgs = append(msgs, "not have encryption IO filter") + } + } + case *vimtypes.InvalidArgument: + for i := range localizableMessages { + //nolint:gocritic + switch localizableMessages[i].Key { + case "config.extraConfig[\"dataFileKey\"]": + msgs = append(msgs, "not set secret key") + } + } + case *vimtypes.InvalidDeviceOperation: + for i := range localizableMessages { + switch localizableMessages[i].Key { + case "msg.hostd.deviceSpec.enc.encrypted": + msgs = append(msgs, "not specify encrypted disk") + case "msg.hostd.deviceSpec.enc.notEncrypted": + msgs = append(msgs, "not specify decrypted disk") + default: + msgs = append(msgs, "not add/remove device sans crypto spec") + } + } + + case *vimtypes.InvalidDeviceSpec: + for i := range localizableMessages { + switch localizableMessages[i].Key { + case "msg.hostd.deviceSpec.enc.badPolicy": + msgs = append(msgs, "have encryption IO filter") + case "msg.hostd.deviceSpec.enc.notDisk": + msgs = append(msgs, "not apply only to disk") + case "msg.hostd.deviceSpec.enc.sharedBacking": + msgs = append(msgs, "not have disk with shared backing") + case "msg.hostd.deviceSpec.enc.notFile": + msgs = append(msgs, "not have raw disk mapping") + case "msg.hostd.configSpec.enc.mismatch": + msgs = append(msgs, "not add encrypted disk") + case "msg.hostd.deviceSpec.add.noencrypt": + msgs = append(msgs, "not add plain disk") + } + } + case *vimtypes.InvalidPowerState: + if tErr.ExistingState != vimtypes.VirtualMachinePowerStatePoweredOff { + msgs = append(msgs, "be powered off") + } + case *vimtypes.InvalidVmConfig: + for i := range localizableMessages { + switch localizableMessages[i].Key { + case "msg.hostd.configSpec.enc.snapshots": + msgs = append(msgs, "not have snapshots") + case "msg.hostd.deviceSpec.enc.diskChain": + msgs = append(msgs, "not have only disk snapshots") + case "msg.hostd.configSpec.enc.notEncrypted": + msgs = append(msgs, "not be encrypted") + case "msg.hostd.configSpec.enc.encrypted": + msgs = append(msgs, "be encrypted") + case "msg.hostd.configSpec.enc.mismatch": + msgs = append(msgs, "have vm and disks with different encryption states") + } + } + } + return false + }, + ) + + if len(msgs) > 0 { + markEncryptionStateNotSynced( + vm, + state.Operation, + ReasonReconfigureError, + msgs...) + } + + return nil +} diff --git a/pkg/vmconfig/crypto/crypto_reconciler_post_test.go b/pkg/vmconfig/crypto/crypto_reconciler_post_test.go new file mode 100644 index 000000000..2b0ed97c6 --- /dev/null +++ b/pkg/vmconfig/crypto/crypto_reconciler_post_test.go @@ -0,0 +1,676 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package crypto_test + +import ( + "bytes" + "context" + "fmt" + "reflect" + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/vmware/govmomi/task" + "github.com/vmware/govmomi/vim25/mo" + "github.com/vmware/govmomi/vim25/soap" + vimtypes "github.com/vmware/govmomi/vim25/types" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" + "github.com/vmware-tanzu/vm-operator/pkg/conditions" + "github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto/internal" +) + +var _ = Describe("OnResult", Label(testlabels.Crypto), func() { + var ( + r vmconfig.ReconcilerWithContext + ctx context.Context + moVM mo.VirtualMachine + vm *vmopv1.VirtualMachine + reconfigErr error + ) + + BeforeEach(func() { + r = crypto.New() + ctx = r.WithContext(context.Background()) + moVM.Config = &vimtypes.VirtualMachineConfigInfo{} + vm = &vmopv1.VirtualMachine{} + reconfigErr = nil + }) + + assertStateNotSynced := func( + vm *vmopv1.VirtualMachine, + expectedMessage string) { + + c := conditions.Get(vm, vmopv1.VirtualMachineEncryptionSynced) + ExpectWithOffset(1, c).ToNot(BeNil()) + ExpectWithOffset(1, c.Status).To(Equal(metav1.ConditionFalse)) + ExpectWithOffset(1, c.Reason).To(Equal(crypto.ReasonReconfigureError.String())) + ExpectWithOffset(1, c.Message).To(Equal(expectedMessage)) + } + + Context("panic", func() { + When("ctx is nil", func() { + BeforeEach(func() { + ctx = nil + }) + It("should panic", func() { + fn := func() { + _ = r.OnResult(ctx, vm, moVM, reconfigErr) + } + Expect(fn).To(PanicWith("context is nil")) + }) + }) + + When("moVM.config is nil", func() { + BeforeEach(func() { + moVM.Config = nil + }) + It("should panic", func() { + fn := func() { + _ = r.OnResult(ctx, vm, moVM, reconfigErr) + } + Expect(fn).To(PanicWith("moVM.config is nil")) + }) + }) + + When("vm is nil", func() { + BeforeEach(func() { + vm = nil + }) + It("should panic", func() { + fn := func() { + _ = r.OnResult(ctx, vm, moVM, reconfigErr) + } + Expect(fn).To(PanicWith("vm is nil")) + }) + }) + }) + + Context("no panic", func() { + var ( + err error + ) + JustBeforeEach(func() { + err = r.OnResult(ctx, vm, moVM, reconfigErr) + }) + + Context("succeeded", func() { + When("crypto was not involved", func() { + BeforeEach(func() { + internal.SetOperation(ctx, "updating unencrypted") + }) + When("vm does not already have crypto synced condition", func() { + BeforeEach(func() { + conditions.Delete(vm, vmopv1.VirtualMachineEncryptionSynced) + }) + It("should leave the condition alone", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.Has(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeFalse()) + }) + }) + When("vm does already have crypto synced condition", func() { + When("existing condition is true", func() { + BeforeEach(func() { + conditions.MarkTrue(vm, vmopv1.VirtualMachineEncryptionSynced) + }) + It("should leave the condition alone", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + When("existing condition is false", func() { + BeforeEach(func() { + conditions.MarkFalse(vm, vmopv1.VirtualMachineEncryptionSynced, "fake", "fake") + }) + It("should leave the condition alone", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.IsFalse(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + }) + + When("status.crypto is not nil", func() { + BeforeEach(func() { + vm.Status.Crypto = &vmopv1.VirtualMachineCryptoStatus{ + Encrypted: []vmopv1.VirtualMachineEncryptionType{ + "invalid1", + "invalid2", + }, + ProviderID: "invalid-provider-id", + KeyID: "invalid-key-id", + } + }) + When("vm is not encrypted", func() { + It("should clear status.crypto", func() { + Expect(vm.Status.Crypto).To(BeNil()) + }) + }) + When("vm was already encrypted using storage class or vTPM", func() { + BeforeEach(func() { + moVM.Config.KeyId = &vimtypes.CryptoKeyId{ + KeyId: "123", + ProviderId: &vimtypes.KeyProviderId{ + Id: "abc", + }, + } + }) + + When("vm is encrypted using storage class", func() { + BeforeEach(func() { + internal.MarkEncryptedStorageClass(ctx) + }) + It("should update status.crypto", func() { + Expect(vm.Status.Crypto).ToNot(BeNil()) + Expect(vm.Status.Crypto).To(Equal(&vmopv1.VirtualMachineCryptoStatus{ + Encrypted: []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + vmopv1.VirtualMachineEncryptionTypeDisks, + }, + ProviderID: "abc", + KeyID: "123", + })) + }) + }) + When("vm is encrypted using vTPM", func() { + BeforeEach(func() { + moVM.Config.Hardware.Device = []vimtypes.BaseVirtualDevice{ + &vimtypes.VirtualTPM{}, + } + }) + It("should update status.crypto", func() { + Expect(vm.Status.Crypto).ToNot(BeNil()) + Expect(vm.Status.Crypto).To(Equal(&vmopv1.VirtualMachineCryptoStatus{ + Encrypted: []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + }, + ProviderID: "abc", + KeyID: "123", + })) + }) + }) + }) + }) + + }) + When("crypto was involved", func() { + BeforeEach(func() { + internal.SetOperation(ctx, "encrypting") + }) + When("vm does not already have crypto synced condition", func() { + BeforeEach(func() { + conditions.Delete(vm, vmopv1.VirtualMachineEncryptionSynced) + }) + It("should set the condition to true", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + When("vm does already have crypto synced condition", func() { + When("existing condition is true", func() { + BeforeEach(func() { + conditions.MarkTrue(vm, vmopv1.VirtualMachineEncryptionSynced) + }) + It("should set the condition to true", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + When("existing condition is false", func() { + BeforeEach(func() { + conditions.MarkFalse(vm, vmopv1.VirtualMachineEncryptionSynced, "fake", "fake") + }) + It("should set the condition to true", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeTrue()) + }) + }) + }) + When("vm was encrypted using storage class or vTPM", func() { + BeforeEach(func() { + moVM.Config.KeyId = &vimtypes.CryptoKeyId{ + KeyId: "123", + ProviderId: &vimtypes.KeyProviderId{ + Id: "abc", + }, + } + }) + + When("vm was encrypted using storage class", func() { + BeforeEach(func() { + internal.MarkEncryptedStorageClass(ctx) + }) + It("should set status.crypto", func() { + Expect(vm.Status.Crypto).ToNot(BeNil()) + Expect(vm.Status.Crypto).To(Equal(&vmopv1.VirtualMachineCryptoStatus{ + Encrypted: []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + vmopv1.VirtualMachineEncryptionTypeDisks, + }, + ProviderID: "abc", + KeyID: "123", + })) + }) + }) + When("vm was encrypted using vTPM", func() { + BeforeEach(func() { + moVM.Config.Hardware.Device = []vimtypes.BaseVirtualDevice{ + &vimtypes.VirtualTPM{}, + } + }) + It("should set status.crypto", func() { + Expect(vm.Status.Crypto).ToNot(BeNil()) + Expect(vm.Status.Crypto).To(Equal(&vmopv1.VirtualMachineCryptoStatus{ + Encrypted: []vmopv1.VirtualMachineEncryptionType{ + vmopv1.VirtualMachineEncryptionTypeConfig, + }, + ProviderID: "abc", + KeyID: "123", + })) + }) + }) + }) + }) + }) + + When("failed", func() { + Context("with a soap error", func() { + BeforeEach(func() { + reconfigErr = soap.WrapSoapFault(&soap.Fault{ + Detail: struct { + Fault vimtypes.AnyType "xml:\",any,typeattr\"" + }{ + Fault: nil, + }, + }) + }) + It("should not return an error or set any conditions", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.Has(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeFalse()) + }) + }) + + Context("with a task error", func() { + Context("and is a nil LocalizedMethodFault", func() { + BeforeEach(func() { + reconfigErr = task.Error{} + }) + It("should not return an error or set any conditions", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.Has(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeFalse()) + }) + }) + + Context("and is an unknown fault", func() { + BeforeEach(func() { + reconfigErr = task.Error{ + LocalizedMethodFault: &vimtypes.LocalizedMethodFault{}, + } + }) + It("should not return an error or set any conditions", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.Has(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeFalse()) + }) + }) + + DescribeTable("and is a single, known fault", + func( + currentCryptoState *vimtypes.CryptoKeyId, + operation string, + fault vimtypes.BaseMethodFault, + expectedConditionMessage, + localizedMessage string, + msgKeys []string) { + + vm := &vmopv1.VirtualMachine{} + + mf := fault.GetMethodFault() + for i := range msgKeys { + mf.FaultMessage = append( + mf.FaultMessage, + vimtypes.LocalizableMessage{ + Key: msgKeys[i], + }) + } + + internal.SetOperation(ctx, operation) + + Expect(r.OnResult( + ctx, + vm, + mo.VirtualMachine{ + Config: &vimtypes.VirtualMachineConfigInfo{ + KeyId: currentCryptoState, + }, + }, + task.Error{ + LocalizedMethodFault: &vimtypes.LocalizedMethodFault{ + Fault: fault, + LocalizedMessage: localizedMessage, + }, + })).To(Succeed()) + + assertStateNotSynced(vm, expectedConditionMessage) + }, + + joinTableEntries( + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.GenericVmConfigFault{} }, + "specify a valid key", + "", + "msg.vigor.enc.keyNotFound", + ), + + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.GenericVmConfigFault{} }, + "specify a key that can be located", + "", + "msg.keysafe.locator", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.GenericVmConfigFault{} }, + "add vTPM", + "", + "msg.vtpm.add.notEncrypted", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.GenericVmConfigFault{} }, + "have vTPM", + "", + "msg.vigor.enc.required.vtpm", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.GenericVmConfigFault{} }, + "specify a valid key and specify a key that can be located", + "", + "msg.vigor.enc.keyNotFound", + "msg.keysafe.locator", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.GenericVmConfigFault{} }, + "specify a valid key, specify a key that can be located, and add vTPM", + "", + "msg.vigor.enc.keyNotFound", + "msg.keysafe.locator", + "msg.vtpm.add.notEncrypted", + ), + + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.SystemError{} }, + "specify a valid key", + "Error creating disk Key locator", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.SystemError{} }, + "specify a key that can be located", + "Key locator error", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.SystemError{} }, + "not specify encryption bundle", + "Key required for encryption.bundle.", + ), + + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.NotSupported{} }, + "not have encryption IO filter", + "", + "msg.disk.policyChangeFailure", + ), + + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidDeviceOperation{} }, + "not specify encrypted disk", + "", + "msg.hostd.deviceSpec.enc.encrypted", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidDeviceOperation{} }, + "not specify decrypted disk", + "", + "msg.hostd.deviceSpec.enc.notEncrypted", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidDeviceOperation{} }, + "not add/remove device sans crypto spec", + "", + "fake.msg.id", + ), + + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidArgument{} }, + "not set secret key", + "", + "config.extraConfig[\"dataFileKey\"]", + ), + + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidDeviceSpec{} }, + "have encryption IO filter", + "", + "msg.hostd.deviceSpec.enc.badPolicy", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidDeviceSpec{} }, + "not apply only to disk", + "", + "msg.hostd.deviceSpec.enc.notDisk", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidDeviceSpec{} }, + "not have disk with shared backing", + "", + "msg.hostd.deviceSpec.enc.sharedBacking", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidDeviceSpec{} }, + "not have raw disk mapping", + "", + "msg.hostd.deviceSpec.enc.notFile", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidDeviceSpec{} }, + "not add encrypted disk", + "", + "msg.hostd.configSpec.enc.mismatch", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidDeviceSpec{} }, + "not add plain disk", + "", + "msg.hostd.deviceSpec.add.noencrypt", + ), + + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidVmConfig{} }, + "not have snapshots", + "", + "msg.hostd.configSpec.enc.snapshots", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidVmConfig{} }, + "not have only disk snapshots", + "", + "msg.hostd.deviceSpec.enc.diskChain", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidVmConfig{} }, + "not be encrypted", + "", + "msg.hostd.configSpec.enc.notEncrypted", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidVmConfig{} }, + "be encrypted", + "", + "msg.hostd.configSpec.enc.encrypted", + ), + getMethodFaultTableEntries( + func() vimtypes.BaseMethodFault { return &vimtypes.InvalidVmConfig{} }, + "have vm and disks with different encryption states", + "", + "msg.hostd.configSpec.enc.mismatch", + ), + ), + ) + + Context("and is multiple, known faults", func() { + BeforeEach(func() { + internal.SetOperation(ctx, "updating unencrypted") + }) + When("there are two faults", func() { + BeforeEach(func() { + reconfigErr = task.Error{ + LocalizedMethodFault: &vimtypes.LocalizedMethodFault{ + LocalizedMessage: "Key locator error", + Fault: &vimtypes.SystemError{ + RuntimeFault: vimtypes.RuntimeFault{ + MethodFault: vimtypes.MethodFault{ + FaultCause: &vimtypes.LocalizedMethodFault{ + Fault: &vimtypes.NotSupported{ + RuntimeFault: vimtypes.RuntimeFault{ + MethodFault: vimtypes.MethodFault{ + FaultMessage: []vimtypes.LocalizableMessage{ + { + Key: "msg.disk.policyChangeFailure", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + }) + It("should return nil error and set a condition with the expected message", func() { + Expect(err).ToNot(HaveOccurred()) + assertStateNotSynced(vm, "Must specify a key that can be located and not have encryption IO filter when updating unencrypted vm") + }) + }) + When("there are three faults", func() { + BeforeEach(func() { + reconfigErr = task.Error{ + LocalizedMethodFault: &vimtypes.LocalizedMethodFault{ + LocalizedMessage: "Key locator error", + Fault: &vimtypes.SystemError{ + RuntimeFault: vimtypes.RuntimeFault{ + MethodFault: vimtypes.MethodFault{ + FaultCause: &vimtypes.LocalizedMethodFault{ + Fault: &vimtypes.NotSupported{ + RuntimeFault: vimtypes.RuntimeFault{ + MethodFault: vimtypes.MethodFault{ + FaultMessage: []vimtypes.LocalizableMessage{ + { + Key: "msg.disk.policyChangeFailure", + }, + }, + FaultCause: &vimtypes.LocalizedMethodFault{ + Fault: &vimtypes.InvalidPowerState{}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + }) + It("should return nil error and set a condition with the expected message", func() { + Expect(err).ToNot(HaveOccurred()) + assertStateNotSynced(vm, "Must specify a key that can be located, not have encryption IO filter, and be powered off when updating unencrypted vm") + }) + }) + }) + }) + }) + }) +}) + +func joinTableEntries(entries ...[]TableEntry) []TableEntry { + var list []TableEntry + for i := range entries { + list = append(list, entries[i]...) + } + return list +} + +func getMethodFaultTableEntries( + newFault func() vimtypes.BaseMethodFault, + conditionMessage, + localizedMessage string, + msgKeys ...string) []TableEntry { + + faultName := reflect.ValueOf(newFault()).Elem().Type().Name() + + var w bytes.Buffer + if localizedMessage != "" { + fmt.Fprintf(&w, "localizedMessage=%s", localizedMessage) + } + + if len(msgKeys) > 0 { + if w.Len() > 0 { + w.WriteString(", ") + } + fmt.Fprintf(&w, "messageKeys=%s", strings.Join(msgKeys, ";")) + } + + titleSuffix := w.String() + + return []TableEntry{ + Entry( + fmt.Sprintf("%s, decrypting vm, %s", faultName, titleSuffix), + &vimtypes.CryptoKeyId{}, + "decrypting", + newFault(), + crypto.SprintfStateNotSynced("decrypting", conditionMessage), + localizedMessage, + msgKeys, + ), + Entry( + fmt.Sprintf("%s, encrypting vm, %s", faultName, titleSuffix), + nil, + "encrypting", + newFault(), + crypto.SprintfStateNotSynced("encrypting", conditionMessage), + localizedMessage, + msgKeys, + ), + Entry( + fmt.Sprintf("%s, recrypting vm, %s", faultName, titleSuffix), + &vimtypes.CryptoKeyId{}, + "recrypting", + newFault(), + crypto.SprintfStateNotSynced("recrypting", conditionMessage), + localizedMessage, + msgKeys, + ), + Entry( + fmt.Sprintf("%s, updating encrypted vm, %s", faultName, titleSuffix), + &vimtypes.CryptoKeyId{}, + "updating encrypted", + newFault(), + crypto.SprintfStateNotSynced("updating encrypted", conditionMessage), + localizedMessage, + msgKeys, + ), + Entry( + fmt.Sprintf("%s, updating unencrypted vm, %s", faultName, titleSuffix), + nil, + "updating unencrypted", + newFault(), + crypto.SprintfStateNotSynced("updating unencrypted", conditionMessage), + localizedMessage, + msgKeys, + ), + } +} diff --git a/pkg/vmconfig/crypto/crypto_reconciler_pre.go b/pkg/vmconfig/crypto/crypto_reconciler_pre.go new file mode 100644 index 000000000..c13caa29d --- /dev/null +++ b/pkg/vmconfig/crypto/crypto_reconciler_pre.go @@ -0,0 +1,690 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package crypto + +import ( + "context" + "errors" + "regexp" + "strings" + + "github.com/go-logr/logr" + "github.com/vmware/govmomi/crypto" + "github.com/vmware/govmomi/vim25" + "github.com/vmware/govmomi/vim25/mo" + vimtypes "github.com/vmware/govmomi/vim25/types" + apierrors "k8s.io/apimachinery/pkg/api/errors" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + byokv1 "github.com/vmware-tanzu/vm-operator/external/byok/api/v1alpha1" + kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" + "github.com/vmware-tanzu/vm-operator/pkg/util/paused" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto/internal" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" +) + +type ptrCfgSpec = *vimtypes.VirtualMachineConfigSpec +type ptrDevCfgSpec = *vimtypes.VirtualDeviceConfigSpec + +type cryptoKey struct { + id string + provider string + isDefaultProvider bool +} + +type reconcileArgs struct { + k8sClient ctrlclient.Client + vimClient *vim25.Client + vm *vmopv1.VirtualMachine + moVM mo.VirtualMachine + configSpec ptrCfgSpec + curKey cryptoKey + newKey cryptoKey + isEncStorClass bool +} + +//nolint:gocyclo // The allowed complexity is 30, this is 31. +func (r reconciler) Reconcile( + ctx context.Context, + k8sClient ctrlclient.Client, + vimClient *vim25.Client, + vm *vmopv1.VirtualMachine, + moVM mo.VirtualMachine, + configSpec ptrCfgSpec) error { + + if ctx == nil { + panic("context is nil") + } + if k8sClient == nil { + panic("k8sClient is nil") + } + if vimClient == nil { + panic("vimClient is nil") + } + if vm == nil { + panic("vm is nil") + } + if configSpec == nil { + panic("configSpec is nil") + } + + var ( + err error + msgs []string + reason Reason + args = reconcileArgs{ + k8sClient: k8sClient, + vimClient: vimClient, + vm: vm, + moVM: moVM, + configSpec: configSpec, + } + ) + + // Check to if the VM is currently encrypted and record the current provider + // ID and key ID. + args.curKey = getCurCryptoKey(moVM) + + // Check whether or not the storage class is encrypted. + args.isEncStorClass, err = kubeutil.IsEncryptedStorageClass( + ctx, + k8sClient, + vm.Spec.StorageClass) + if err != nil { + return err + } + if args.isEncStorClass { + internal.MarkEncryptedStorageClass(ctx) + } + + // Do not proceed further if the VM is paused. + if paused.ByAdmin(moVM) || paused.ByDevOps(vm) { + return nil + } + + // Get the new provider ID and key ID. + if args.newKey, reason, msgs, err = getNewCryptoKey(ctx, args); err != nil { + return err + } + + var op string + switch { + case args.curKey.provider == "" && args.newKey.provider != "": + op = "encrypting" + if reason == 0 && len(msgs) == 0 { + r, m, err := onEncrypt(ctx, args) + if err != nil { + return err + } + reason |= r + msgs = append(msgs, m...) + } + case args.curKey.provider != "" && args.newKey.provider != "" && + ((args.curKey.provider != args.newKey.provider) || + (args.curKey.provider == args.newKey.provider && args.curKey.id != args.newKey.id)): + + op = "recrypting" + if reason == 0 && len(msgs) == 0 { + r, m, err := onRecrypt(ctx, args) + if err != nil { + return err + } + reason |= r + msgs = append(msgs, m...) + } + case args.curKey.provider != "": + op = "updating encrypted" + if reason == 0 && len(msgs) == 0 { + r, m, err := validateUpdateEncrypted(args) + if err != nil { + return err + } + reason |= r + msgs = append(msgs, m...) + } + + case args.curKey.provider == "": + op = "updating unencrypted" + if reason == 0 && len(msgs) == 0 { + r, m, err := validateUpdateUnencrypted(args) + if err != nil { + return err + } + reason |= r + msgs = append(msgs, m...) + } + } + + if reason == 0 && len(msgs) == 0 { + internal.SetOperation(ctx, op) + } else { + markEncryptionStateNotSynced(vm, op, reason, msgs...) + } + + return nil +} + +func getCurCryptoKey(moVM mo.VirtualMachine) cryptoKey { + var curKey cryptoKey + if moVM.Config == nil { + return curKey + } + if kid := moVM.Config.KeyId; kid != nil { + curKey.id = kid.KeyId + if pid := kid.ProviderId; pid != nil { + curKey.provider = pid.Id + } + } + return curKey +} + +func getNewCryptoKey( + ctx context.Context, + args reconcileArgs) (cryptoKey, Reason, []string, error) { + + key, reason, msgs, err := getNewCryptoKeyFromEncryptionClass(ctx, args) + if err != nil || reason > 0 || len(msgs) > 0 { + // If there was an error getting the key from the encryption class or + // any reason/messages were set, return that information early. + return cryptoKey{}, reason, msgs, err + } + + // The field spec.crypto.UseDefaultProvider defaults to true, so let's + // assume it is true if nil. + useDefaultProvider := true + if args.vm.Spec.Crypto.UseDefaultKeyProvider != nil { + useDefaultProvider = *args.vm.Spec.Crypto.UseDefaultKeyProvider + } + + // The provider was not found via the encryption class and the user has + // specified not to use the default provider, so go ahead and return. + if key.provider == "" && !useDefaultProvider { + return cryptoKey{}, 0, nil, nil + } + + // Create a new VIM crypto manager that points to the CryptoManagerKmip + // singleton on vSphere. + m := crypto.NewManagerKmip(args.vimClient) + + if key.provider != "" { + + // The encryption class specified a provider, so we need to verify it is + // valid. Then, if a key was specified, we need to verify that also. + + ok, err := m.IsValidProvider(ctx, key.provider) + if err != nil { + return cryptoKey{}, 0, nil, err + } + if !ok { + reason |= ReasonEncryptionClassInvalid + msgs = append(msgs, "specify encryption class with a valid provider") + } + if key.id != "" { + ok, err := m.IsValidKey(ctx, key.id) + if err != nil { + return cryptoKey{}, 0, nil, err + } + if !ok { + reason |= ReasonEncryptionClassInvalid + msgs = append(msgs, "specify encryption class with a valid key") + } + } + if reason > 0 || len(msgs) > 0 { + return cryptoKey{}, reason, msgs, nil + } + return key, 0, nil, nil + } + + // + // At this point we know the provider was not discovered via the encryption + // class and the VM wants to try and use the default provider, so we need to + // figure out if one exists. + // + + if key.provider, _ = m.GetDefaultKmsClusterID( + ctx, nil, true); key.provider != "" { + + // Ensure key.id="" so vSphere generates a key using the default + // provider. + key.id = "" + + // Indicate this is the default provider for use later. + key.isDefaultProvider = true + + return key, 0, nil, nil + } + + // There is no default provider. This is not an error, so just return an + // empty provider ID indicating no provider is available. + return cryptoKey{}, 0, nil, nil +} + +func getNewCryptoKeyFromEncryptionClass( + ctx context.Context, + args reconcileArgs) (cryptoKey, Reason, []string, error) { + + objName := args.vm.Spec.Crypto.EncryptionClassName + if objName == "" { + // When no encryption class is specified, there is nothing else to do. + return cryptoKey{}, 0, nil, nil + } + + // When an encryption class is specified, get the provider ID and key ID + // from the encryption class. + + var ( + key cryptoKey + reason Reason + msgs []string + obj byokv1.EncryptionClass + objKey = ctrlclient.ObjectKey{ + Namespace: args.vm.Namespace, + Name: objName, + } + ) + + if err := args.k8sClient.Get(ctx, objKey, &obj); err != nil { + if !apierrors.IsNotFound(err) { + return cryptoKey{}, 0, nil, err + } + + reason |= ReasonEncryptionClassNotFound + msgs = append(msgs, "specify encryption class that exists") + + // Allow the other reconciliation logic to continue. + // The VM's encryption state can be synced once the encryption + // class is ready. + return cryptoKey{}, reason, msgs, nil + + } + + key.provider, key.id = obj.Spec.KeyProvider, obj.Spec.KeyID + if key.provider != "" { + // Return the key from the encryption class. + return key, 0, nil, nil + } + + // Allow the other reconciliation logic to continue. + // The VM's encryption state can be synced once the encryption + // class is ready. + reason |= ReasonEncryptionClassInvalid + msgs = append(msgs, "specify encryption class with a non-empty provider") + return cryptoKey{}, reason, msgs, nil + +} + +func onEncrypt( + ctx context.Context, + args reconcileArgs) (Reason, []string, error) { + + logger := logr.FromContextOrDiscard(ctx) + + reason, msgs, err := validateEncrypt(args) + if reason > 0 || len(msgs) > 0 || err != nil { + return reason, msgs, err + } + + args.configSpec.Crypto = &vimtypes.CryptoSpecEncrypt{ + CryptoKeyId: vimtypes.CryptoKeyId{ + ProviderId: &vimtypes.KeyProviderId{ + Id: args.newKey.provider, + }, + KeyId: args.newKey.id, + }, + } + + logger.Info( + "Encrypt VM", + "newKeyID", args.newKey.id, + "newProviderID", args.newKey.provider, + "newProviderIsDefault", args.newKey.isDefaultProvider) + + return 0, nil, nil +} + +func onRecrypt( + ctx context.Context, + args reconcileArgs) (Reason, []string, error) { + + logger := logr.FromContextOrDiscard(ctx) + + reason, msgs, err := validateRecrypt(args) + if reason > 0 || len(msgs) > 0 || err != nil { + return reason, msgs, err + } + + args.configSpec.Crypto = &vimtypes.CryptoSpecShallowRecrypt{ + NewKeyId: vimtypes.CryptoKeyId{ + ProviderId: &vimtypes.KeyProviderId{ + Id: args.newKey.provider, + }, + KeyId: args.newKey.id, + }, + } + + logger.Info( + "Recrypt VM", + "currentKeyID", args.curKey.id, + "currentProviderID", args.curKey.provider, + "newKeyID", args.newKey.id, + "newProviderID", args.newKey.provider, + "newProviderIsDefault", args.newKey.isDefaultProvider) + + return 0, nil, nil +} + +func validateEncrypt(args reconcileArgs) (Reason, []string, error) { + var ( + msgs []string + reason Reason + ) + if has, add := hasVTPM(args.moVM, args.configSpec); !has && !add && !args.isEncStorClass { + if args.newKey.isDefaultProvider { + return 0, nil, errors.New( + "encrypting vm requires compatible storage class or vTPM") + } + reason |= ReasonInvalidState + msgs = append(msgs, "use encrypted storage class or have vTPM") + } + if r, m := validatePoweredOffNoSnapshots(args.moVM); len(m) > 0 { + reason |= r + msgs = append(msgs, m...) + } + if r, m := validateDeviceChanges(args.configSpec); len(m) > 0 { + reason |= r + msgs = append(msgs, m...) + } + return reason, msgs, nil +} + +func validateRecrypt(args reconcileArgs) (Reason, []string, error) { + var ( + msgs []string + reason Reason + ) + if has, add := hasVTPM(args.moVM, args.configSpec); !has && !add && !args.isEncStorClass { + if args.newKey.isDefaultProvider { + return 0, nil, errors.New( + "recrypting vm requires compatible storage class or vTPM") + } + reason |= ReasonInvalidState + msgs = append(msgs, "use encrypted storage class or have vTPM") + } + if hasSnapshotTree(args.moVM) { + msgs = append(msgs, "not have snapshot tree") + reason |= ReasonInvalidState + } + if r, m := validateDeviceChanges(args.configSpec); len(m) > 0 { + reason |= r + msgs = append(msgs, m...) + } + return reason, msgs, nil +} + +func validateUpdateEncrypted(args reconcileArgs) (Reason, []string, error) { + var ( + msgs []string + reason Reason + ) + if has, add := hasVTPM(args.moVM, args.configSpec); !has && !add && !args.isEncStorClass { + if args.newKey.isDefaultProvider { + return 0, nil, errors.New( + "updating encrypted vm requires compatible storage class or vTPM") + } + reason |= ReasonInvalidState + msgs = append(msgs, "use encrypted storage class or have vTPM") + } + if isChangingSecretKey(args.configSpec) { + msgs = append(msgs, "not add/remove/modify secret key") + reason |= ReasonInvalidChanges + } + if r, m := validateDeviceChanges(args.configSpec); len(m) > 0 { + reason |= r + msgs = append(msgs, m...) + } + return reason, msgs, nil +} + +//nolint:unparam +func validateUpdateUnencrypted(args reconcileArgs) (Reason, []string, error) { + var ( + msgs []string + reason Reason + ) + + if has, add := hasVTPM(args.moVM, args.configSpec); has || add || args.isEncStorClass { + if args.isEncStorClass { + reason |= ReasonInvalidState + msgs = append(msgs, "not use encrypted storage class") + } + if has { + reason |= ReasonInvalidState + msgs = append(msgs, "not have vTPM") + } + if add { + reason |= ReasonInvalidChanges + msgs = append(msgs, "not add vTPM") + } + } + return reason, msgs, nil +} + +func hasVTPM(moVM mo.VirtualMachine, configSpec ptrCfgSpec) (bool, bool) { + var ( + has bool + add bool + ) + if c := moVM.Config; c != nil { + for i := range c.Hardware.Device { + if _, ok := c.Hardware.Device[i].(*vimtypes.VirtualTPM); ok { + has = true + break + } + } + } + if configSpec == nil { + return has, add + } + for i := range configSpec.DeviceChange { + if devChange := configSpec.DeviceChange[i]; devChange != nil { + if devSpec := devChange.GetVirtualDeviceConfigSpec(); devSpec != nil { + if _, ok := devSpec.Device.(*vimtypes.VirtualTPM); ok { + if devSpec.Operation == vimtypes.VirtualDeviceConfigSpecOperationAdd { + add = true + } else if devSpec.Operation == vimtypes.VirtualDeviceConfigSpecOperationRemove { + has = false + } + break + } + } + } + } + return has, add +} + +func validatePoweredOffNoSnapshots(moVM mo.VirtualMachine) (Reason, []string) { + var ( + msgs []string + reason Reason + ) + if moVM.Summary.Runtime.PowerState != "" && + moVM.Summary.Runtime.PowerState != vimtypes.VirtualMachinePowerStatePoweredOff { + + msgs = append(msgs, "be powered off") + reason |= ReasonInvalidState + } + if moVM.Snapshot != nil && moVM.Snapshot.CurrentSnapshot != nil { + msgs = append(msgs, "not have snapshots") + reason |= ReasonInvalidState + } + return reason, msgs +} + +func validateDeviceChanges(configSpec ptrCfgSpec) (Reason, []string) { + + var ( + msgs []string + reason Reason + ) + + for i := range configSpec.DeviceChange { + if devChange := configSpec.DeviceChange[i]; devChange != nil { + devSpec := devChange.GetVirtualDeviceConfigSpec() + if isAddEditDeviceSpecEncryptedSansPolicy(devSpec) { + msgs = append(msgs, "specify policy when encrypting devices") + reason |= ReasonInvalidChanges + } + if isEncryptedRawDiskMapping(devSpec) { + msgs = append(msgs, "not encrypt raw disks") + reason |= ReasonInvalidChanges + } + if isEncryptedDeviceNonDisk(devSpec) { + msgs = append(msgs, "not encrypt non-disk devices") + reason |= ReasonInvalidChanges + } + if isEncryptedDeviceWithMultipleBackings(devSpec) { + msgs = append(msgs, "not encrypt devices with multiple backings") + reason |= ReasonInvalidChanges + } + } + } + + return reason, msgs +} + +var secretKeys = map[string]struct{}{ + "ancestordatafilekeys": {}, + "cryptostate": {}, + "datafilekey": {}, + "encryption.required": {}, + "encryption.required.vtpm": {}, + "encryption.unspecified.default": {}, +} + +func isChangingSecretKey(configSpec ptrCfgSpec) bool { + for i := range configSpec.ExtraConfig { + if bov := configSpec.ExtraConfig[i]; bov != nil { + if ov := bov.GetOptionValue(); ov != nil { + if _, ok := secretKeys[ov.Key]; ok { + return true + } + } + } + } + return false +} + +func isAddEditDeviceSpecEncryptedSansPolicy(spec ptrDevCfgSpec) bool { + if spec != nil { + switch spec.Operation { + case vimtypes.VirtualDeviceConfigSpecOperationAdd, + vimtypes.VirtualDeviceConfigSpecOperationEdit: + + if backing := spec.Backing; backing != nil { + switch backing.Crypto.(type) { + case *vimtypes.CryptoSpecEncrypt, + *vimtypes.CryptoSpecDeepRecrypt, + *vimtypes.CryptoSpecShallowRecrypt: + + for i := range spec.Profile { + if doesProfileHaveIOFilters(spec.Profile[i]) { + return false + } + } + return true // is encrypted/recrypted sans policy + } + } + } + } + return false +} + +var whiteSpaceRegex = regexp.MustCompile(`[\s\t\n\r]`) + +func doesProfileHaveIOFilters(spec vimtypes.BaseVirtualMachineProfileSpec) bool { + if profile, ok := spec.(*vimtypes.VirtualMachineDefinedProfileSpec); ok { + if data := profile.ProfileData; data != nil { + if data.ExtensionKey == "com.vmware.vim.sips" { + return strings.Contains( + whiteSpaceRegex.ReplaceAllString(data.ObjectData, ""), + "IOFILTERS") + } + } + } + return false +} + +func isEncryptedDeviceNonDisk(spec ptrDevCfgSpec) bool { + if spec != nil { + if backing := spec.Backing; backing != nil { + switch backing.Crypto.(type) { + case *vimtypes.CryptoSpecEncrypt, + *vimtypes.CryptoSpecDeepRecrypt, + *vimtypes.CryptoSpecShallowRecrypt: + + _, isDisk := spec.Device.(*vimtypes.VirtualDisk) + if !isDisk { + return true + } + } + } + } + return false +} + +func isEncryptedDeviceWithMultipleBackings(spec ptrDevCfgSpec) bool { + if spec != nil { + if backing := spec.Backing; backing != nil { + switch backing.Crypto.(type) { + case *vimtypes.CryptoSpecEncrypt, + *vimtypes.CryptoSpecDeepRecrypt, + *vimtypes.CryptoSpecShallowRecrypt: + + return spec.Backing.Parent != nil + } + } + } + return false +} + +func isEncryptedRawDiskMapping(spec ptrDevCfgSpec) bool { + if spec != nil { + if backing := spec.Backing; backing != nil { + switch backing.Crypto.(type) { + case *vimtypes.CryptoSpecEncrypt, + *vimtypes.CryptoSpecDeepRecrypt, + *vimtypes.CryptoSpecShallowRecrypt: + + if disk, ok := spec.Device.(*vimtypes.VirtualDisk); ok { + //nolint:gocritic + switch disk.Backing.(type) { + case *vimtypes.VirtualDiskRawDiskVer2BackingInfo: + return true + } + } + } + } + } + return false +} + +func hasSnapshotTree(moVM mo.VirtualMachine) bool { + var snapshotTree []vimtypes.VirtualMachineSnapshotTree + if si := moVM.Snapshot; si != nil { + snapshotTree = si.RootSnapshotList + } + return hasSnapshotTreeInner(snapshotTree) +} + +func hasSnapshotTreeInner(nodes []vimtypes.VirtualMachineSnapshotTree) bool { + switch len(nodes) { + case 0: + return false + case 1: + return hasSnapshotTreeInner(nodes[0].ChildSnapshotList) + default: + return true + } +} diff --git a/pkg/vmconfig/crypto/crypto_reconciler_pre_test.go b/pkg/vmconfig/crypto/crypto_reconciler_pre_test.go new file mode 100644 index 000000000..154115f55 --- /dev/null +++ b/pkg/vmconfig/crypto/crypto_reconciler_pre_test.go @@ -0,0 +1,1176 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package crypto_test + +import ( + "context" + "errors" + "fmt" + + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/vmware/govmomi/crypto" + "github.com/vmware/govmomi/vim25" + "github.com/vmware/govmomi/vim25/mo" + vimtypes "github.com/vmware/govmomi/vim25/types" + storagev1 "k8s.io/api/storage/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" + byokv1 "github.com/vmware-tanzu/vm-operator/external/byok/api/v1alpha1" + "github.com/vmware-tanzu/vm-operator/pkg/conditions" + "github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels" + kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" + "github.com/vmware-tanzu/vm-operator/pkg/util/ptr" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig" + pkgcrypto "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +var _ = Describe("Reconcile", Label(testlabels.Crypto), func() { + var ( + r vmconfig.ReconcilerWithContext + ctx context.Context + k8sClient ctrlclient.Client + vimClient *vim25.Client + cryptoManager *crypto.ManagerKmip + moVM mo.VirtualMachine + vm *vmopv1.VirtualMachine + encClass *byokv1.EncryptionClass + withObjs []ctrlclient.Object + withFuncs interceptor.Funcs + configSpec *vimtypes.VirtualMachineConfigSpec + + provider1ID string + provider1Key1ID string + provider1Key2ID string + + provider2ID string + provider2Key1ID string + + provider3ID string + + storageClass1 *storagev1.StorageClass + storageClass2 *storagev1.StorageClass + ) + + BeforeEach(func() { + r = pkgcrypto.New() + + vcsimCtx := builder.NewTestContextForVCSim(builder.VCSimTestConfig{}) + ctx = vcsimCtx + ctx = r.WithContext(ctx) + ctx = vmconfig.WithContext(ctx) + + vimClient = vcsimCtx.VCClient.Client + cryptoManager = crypto.NewManagerKmip(vimClient) + + provider1ID = uuid.NewString() + Expect(cryptoManager.RegisterKmsCluster( + ctx, + provider1ID, + vimtypes.KmipClusterInfoKmsManagementTypeTrustAuthority, + )).To(Succeed()) + { + var err error + provider1Key1ID, err = cryptoManager.GenerateKey( + ctx, + provider1ID) + Expect(err).ToNot(HaveOccurred()) + Expect(provider1Key1ID).ToNot(BeEmpty()) + } + { + var err error + provider1Key2ID, err = cryptoManager.GenerateKey( + ctx, + provider1ID) + Expect(err).ToNot(HaveOccurred()) + Expect(provider1Key2ID).ToNot(BeEmpty()) + } + + provider2ID = uuid.NewString() + Expect(cryptoManager.RegisterKmsCluster( + ctx, + provider2ID, + vimtypes.KmipClusterInfoKmsManagementTypeTrustAuthority, + )).To(Succeed()) + { + var err error + provider2Key1ID, err = cryptoManager.GenerateKey( + ctx, + provider2ID) + Expect(err).ToNot(HaveOccurred()) + Expect(provider2Key1ID).ToNot(BeEmpty()) + } + + provider3ID = uuid.NewString() + Expect(cryptoManager.RegisterKmsCluster( + ctx, + provider3ID, + vimtypes.KmipClusterInfoKmsManagementTypeNativeProvider, + )).To(Succeed()) + + moVM = mo.VirtualMachine{ + Config: &vimtypes.VirtualMachineConfigInfo{ + KeyId: &vimtypes.CryptoKeyId{ + ProviderId: &vimtypes.KeyProviderId{ + Id: provider1ID, + }, + KeyId: provider1Key1ID, + }, + }, + Summary: vimtypes.VirtualMachineSummary{ + Runtime: vimtypes.VirtualMachineRuntimeInfo{ + PowerState: vimtypes.VirtualMachinePowerStatePoweredOff, + }, + }, + } + + configSpec = &vimtypes.VirtualMachineConfigSpec{} + + vm = &vmopv1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "my-namespace", + Name: "my-vm", + }, + Spec: vmopv1.VirtualMachineSpec{ + StorageClass: "my-storage-class-2", + Crypto: vmopv1.VirtualMachineCryptoSpec{ + EncryptionClassName: "my-encryption-class", + UseDefaultKeyProvider: ptr.To(true), + }, + }, + } + + encClass = &byokv1.EncryptionClass{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "my-namespace", + Name: "my-encryption-class", + }, + Spec: byokv1.EncryptionClassSpec{ + KeyProvider: provider1ID, + KeyID: provider1Key1ID, + }, + } + + storageClass1 = &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-storage-class-1", + UID: types.UID(uuid.NewString()), + }, + } + storageClass2 = &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-storage-class-2", + UID: types.UID(uuid.NewString()), + }, + } + + withFuncs = interceptor.Funcs{} + withObjs = []ctrlclient.Object{encClass, storageClass1, storageClass2, vm} + }) + JustBeforeEach(func() { + k8sClient = builder.NewFakeClientWithInterceptors(withFuncs, withObjs...) + + if ctx != nil && k8sClient != nil { + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, + k8sClient, + *storageClass2, + true)).To(Succeed()) + } + }) + + When("ctx is nil", func() { + BeforeEach(func() { + ctx = nil + }) + It("should panic", func() { + fn := func() { + _ = r.Reconcile(ctx, k8sClient, vimClient, vm, moVM, configSpec) + } + Expect(fn).To(PanicWith("context is nil")) + }) + }) + + When("k8sClient is nil", func() { + JustBeforeEach(func() { + k8sClient = nil + }) + It("should panic", func() { + fn := func() { + _ = r.Reconcile(ctx, k8sClient, vimClient, vm, moVM, configSpec) + } + Expect(fn).To(PanicWith("k8sClient is nil")) + }) + }) + + When("vimClient is nil", func() { + JustBeforeEach(func() { + vimClient = nil + }) + It("should panic", func() { + fn := func() { + _ = r.Reconcile(ctx, k8sClient, vimClient, vm, moVM, configSpec) + } + Expect(fn).To(PanicWith("vimClient is nil")) + }) + }) + + When("moVM.config is nil", func() { + BeforeEach(func() { + moVM.Config = nil + }) + It("should not panic", func() { + fn := func() { + _ = r.Reconcile(ctx, k8sClient, vimClient, vm, moVM, configSpec) + } + Expect(fn).ToNot(Panic()) + }) + }) + + When("vm is nil", func() { + BeforeEach(func() { + vm = nil + }) + It("should panic", func() { + fn := func() { + _ = r.Reconcile(ctx, k8sClient, vimClient, vm, moVM, configSpec) + } + Expect(fn).To(PanicWith("vm is nil")) + }) + }) + + When("configSpec is nil", func() { + BeforeEach(func() { + configSpec = nil + }) + It("should panic", func() { + fn := func() { + _ = r.Reconcile(ctx, k8sClient, vimClient, vm, moVM, configSpec) + } + Expect(fn).To(PanicWith("configSpec is nil")) + }) + }) + + When("encryptionClassName is empty", func() { + + BeforeEach(func() { + vm.Spec.Crypto.EncryptionClassName = "" + }) + + When("there is not default key provider", func() { + var ( + err error + ) + + JustBeforeEach(func() { + err = r.Reconcile(ctx, k8sClient, vimClient, vm, moVM, configSpec) + }) + + Context("vm is not encrypted", func() { + BeforeEach(func() { + moVM.Config.KeyId = nil + vm.Spec.StorageClass = storageClass1.Name + }) + + When("using encrypted storage class", func() { + BeforeEach(func() { + vm.Spec.StorageClass = storageClass2.Name + }) + It(shouldSetEncryptionSyncedWithInvalidState, 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.ReasonInvalidState).String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("updating unencrypted", "not use encrypted storage class"))) + }) + + When("have vtpm", func() { + BeforeEach(func() { + moVM.Config.Hardware.Device = []vimtypes.BaseVirtualDevice{ + &vimtypes.VirtualTPM{}, + } + }) + It(shouldSetEncryptionSyncedWithInvalidState, 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.ReasonInvalidState).String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("updating unencrypted", "not use encrypted storage class and not have vTPM"))) + }) + }) + + When("adding vtpm", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Operation: vimtypes.VirtualDeviceConfigSpecOperationAdd, + Device: &vimtypes.VirtualTPM{}, + }, + } + }) + It(shouldSetEncryptionSyncedWithInvalidChanges, 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 | pkgcrypto.ReasonInvalidState).String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("updating unencrypted", "not use encrypted storage class and not add vTPM"))) + }) + }) + }) + + When("have vtpm", func() { + BeforeEach(func() { + moVM.Config.Hardware.Device = []vimtypes.BaseVirtualDevice{ + &vimtypes.VirtualTPM{}, + } + }) + It(shouldSetEncryptionSyncedWithInvalidChanges, 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.ReasonInvalidState).String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("updating unencrypted", "not have vTPM"))) + }) + }) + + When("adding vtpm", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Operation: vimtypes.VirtualDeviceConfigSpecOperationAdd, + Device: &vimtypes.VirtualTPM{}, + }, + } + }) + It(shouldSetEncryptionSyncedWithInvalidChanges, 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("updating unencrypted", "not add vTPM"))) + }) + }) + + When("spec.crypto.UseDefaultKeyProvider=nil", func() { + BeforeEach(func() { + vm.Spec.Crypto.UseDefaultKeyProvider = nil + }) + It("should not return an error, set a condition, or update the config spec", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.Has(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeFalse()) + Expect(configSpec).To(Equal(&vimtypes.VirtualMachineConfigSpec{})) + }) + }) + + When("spec.crypto.UseDefaultKeyProvider=false", func() { + BeforeEach(func() { + *vm.Spec.Crypto.UseDefaultKeyProvider = false + }) + It("should not return an error, set a condition, or update the config spec", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.Has(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeFalse()) + Expect(configSpec).To(Equal(&vimtypes.VirtualMachineConfigSpec{})) + }) + }) + + }) + + Context("encrypted vm", func() { + When("modifying secret key", func() { + DescribeTable( + shouldSetEncryptionSyncedWithInvalidChanges, + func(key string) { + + Expect(r.Reconcile( + ctx, + k8sClient, + vimClient, + vm, + moVM, + &vimtypes.VirtualMachineConfigSpec{ + ExtraConfig: []vimtypes.BaseOptionValue{ + &vimtypes.OptionValue{ + Key: key, + Value: "", + }, + }, + }), + ).To(Succeed()) + + 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("updating encrypted", "not add/remove/modify secret key"))) + }, + func(key string) string { + return fmt.Sprintf("when key=%s", key) + }, + Entry(nil, "ancestordatafilekeys"), + Entry(nil, "cryptostate"), + Entry(nil, "datafilekey"), + Entry(nil, "encryption.required"), + Entry(nil, "encryption.required.vtpm"), + Entry(nil, "encryption.unspecified.default"), + ) + }) + }) + }) + + When("there is default key provider", func() { + var ( + err error + ) + + BeforeEach(func() { + Expect(cryptoManager.MarkDefault(ctx, provider3ID)).To(Succeed()) + }) + + JustBeforeEach(func() { + err = r.Reconcile(ctx, k8sClient, vimClient, vm, moVM, configSpec) + }) + + When("vm is not encrypted", func() { + BeforeEach(func() { + moVM.Config.KeyId = nil + }) + + When("default provider is not native", func() { + BeforeEach(func() { + Expect(cryptoManager.MarkDefault(ctx, provider2ID)).To(Succeed()) + }) + It("should encrypt the VM", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.Has(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeFalse()) + Expect(configSpec.Crypto).ToNot(BeNil()) + cryptoSpec, ok := configSpec.Crypto.(*vimtypes.CryptoSpecEncrypt) + Expect(ok).To(BeTrue()) + Expect(cryptoSpec.CryptoKeyId.KeyId).To(BeEmpty()) + Expect(cryptoSpec.CryptoKeyId.ProviderId.Id).To(Equal(provider2ID)) + }) + }) + + It("should encrypt the VM", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.Has(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeFalse()) + Expect(configSpec.Crypto).ToNot(BeNil()) + cryptoSpec, ok := configSpec.Crypto.(*vimtypes.CryptoSpecEncrypt) + Expect(ok).To(BeTrue()) + Expect(cryptoSpec.CryptoKeyId.KeyId).To(BeEmpty()) + Expect(cryptoSpec.CryptoKeyId.ProviderId.Id).To(Equal(provider3ID)) + }) + + When("storage class is not encrypted and there is no vTPM", func() { + BeforeEach(func() { + vm.Spec.StorageClass = storageClass1.Name + }) + It("should return an error", func() { + Expect(err).To(MatchError("encrypting vm requires compatible storage class or vTPM")) + }) + }) + }) + + When("vm is encrypted", func() { + When("recrypting vm", func() { + BeforeEach(func() { + moVM.Config.KeyId = &vimtypes.CryptoKeyId{ + KeyId: "123", + ProviderId: &vimtypes.KeyProviderId{ + Id: provider2ID, + }, + } + }) + + When("storage class is not encrypted and there is no vTPM", func() { + BeforeEach(func() { + vm.Spec.StorageClass = storageClass1.Name + }) + It("should return an error", func() { + Expect(err).To(MatchError("recrypting vm requires compatible storage class or vTPM")) + }) + }) + }) + When("updating encrypted vm", func() { + BeforeEach(func() { + moVM.Config.KeyId = &vimtypes.CryptoKeyId{ + ProviderId: &vimtypes.KeyProviderId{ + Id: provider3ID, + }, + } + }) + When("storage class is not encrypted and there is no vTPM", func() { + BeforeEach(func() { + vm.Spec.StorageClass = storageClass1.Name + }) + It("should return an error", func() { + Expect(err).To(MatchError("updating encrypted vm requires compatible storage class or vTPM")) + }) + }) + }) + }) + }) + + }) + + When("encryptionClassName is not empty", func() { + + var ( + err error + ) + + JustBeforeEach(func() { + err = r.Reconcile(ctx, k8sClient, vimClient, vm, moVM, configSpec) + }) + + When("encryption class does not exist", func() { + BeforeEach(func() { + withObjs = []ctrlclient.Object{ + storageClass1, + storageClass2, + vm, + } + }) + It(shouldSetEncryptionSyncedToFalse, 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.ReasonEncryptionClassNotFound.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("updating encrypted", "specify encryption class that exists"))) + }) + }) + + When("encryption class does exist", func() { + + When("there is a non-404 error getting the encryption class", func() { + BeforeEach(func() { + withFuncs.Get = func( + ctx context.Context, + client ctrlclient.WithWatch, + key ctrlclient.ObjectKey, + obj ctrlclient.Object, + opts ...ctrlclient.GetOption) error { + + if _, ok := obj.(*byokv1.EncryptionClass); ok { + return errors.New("fake") + } + return client.Get(ctx, key, obj, opts...) + } + }) + It("should return an error", func() { + Expect(err).To(MatchError("fake")) + }) + }) + + When("encryption class is not ready", func() { + When("providerID is empty", func() { + BeforeEach(func() { + encClass.Spec.KeyProvider = "" + }) + It(shouldSetEncryptionSyncedToFalse, 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.ReasonEncryptionClassInvalid.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("updating encrypted", "specify encryption class with a non-empty provider"))) + }) + }) + When("providerID is invalid", func() { + BeforeEach(func() { + encClass.Spec.KeyProvider = "invalid-provider-id" + }) + It(shouldSetEncryptionSyncedToFalse, 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.ReasonEncryptionClassInvalid.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("updating encrypted", "specify encryption class with a valid provider"))) + }) + }) + + When("keyID is invalid", func() { + BeforeEach(func() { + encClass.Spec.KeyID = "invalid-key-id" + }) + It(shouldSetEncryptionSyncedToFalse, 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.ReasonEncryptionClassInvalid.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("updating encrypted", "specify encryption class with a valid key"))) + }) + }) + + When("providerID and keyID are invalid", func() { + BeforeEach(func() { + encClass.Spec.KeyProvider = "invalid-provider-id" + encClass.Spec.KeyID = "invalid-key-id" + }) + It(shouldSetEncryptionSyncedToFalse, 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.ReasonEncryptionClassInvalid.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("updating encrypted", "specify encryption class with a valid provider and specify encryption class with a valid key"))) + }) + }) + }) + + When("encryption class is ready", func() { + + When("updating encrypted", func() { + BeforeEach(func() { + moVM.Config.KeyId = &vimtypes.CryptoKeyId{ + KeyId: encClass.Spec.KeyID, + ProviderId: &vimtypes.KeyProviderId{ + Id: encClass.Spec.KeyProvider, + }, + } + }) + + When("storage class is not encrypted and no vTPM", func() { + BeforeEach(func() { + vm.Spec.StorageClass = storageClass1.Name + }) + When("storage class is not encrypted and no vTPM", func() { + It(shouldSetEncryptionSyncedToFalse, 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.ReasonInvalidState.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("updating encrypted", "use encrypted storage class or have vTPM"))) + }) + When("has vTPM being removed", func() { + BeforeEach(func() { + moVM.Config.Hardware.Device = []vimtypes.BaseVirtualDevice{ + &vimtypes.VirtualTPM{}, + } + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Device: &vimtypes.VirtualTPM{}, + Operation: vimtypes.VirtualDeviceConfigSpecOperationRemove, + }, + } + }) + It("should return an error", 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.ReasonInvalidState.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("updating encrypted", "use encrypted storage class or have vTPM"))) + }) + }) + }) + }) + + When("no changes", func() { + It("should not return an error or set condition", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.Has(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeFalse()) + }) + }) + + When("adding encrypted disk with policy", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: &vimtypes.CryptoSpecEncrypt{}, + }, + Device: &vimtypes.VirtualDisk{}, + Operation: vimtypes.VirtualDeviceConfigSpecOperationAdd, + Profile: []vimtypes.BaseVirtualMachineProfileSpec{ + &vimtypes.VirtualMachineDefinedProfileSpec{ + ProfileData: &vimtypes.VirtualMachineProfileRawData{ + ExtensionKey: "com.vmware.vim.sips", + ObjectData: profileWithIOFilters, + }, + }, + }, + }, + } + }) + It("should not return an error or set condition", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(conditions.Has(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeFalse()) + }) + }) + + When("adding encrypted devices sans policy", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: &vimtypes.CryptoSpecEncrypt{}, + }, + Device: &vimtypes.VirtualDisk{}, + Operation: vimtypes.VirtualDeviceConfigSpecOperationAdd, + }, + } + }) + It(shouldSetEncryptionSyncedWithInvalidChanges, 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("updating encrypted", "specify policy when encrypting devices"))) + }) + }) + + When("editing encrypted devices sans policy", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: &vimtypes.CryptoSpecEncrypt{}, + }, + Device: &vimtypes.VirtualDisk{}, + Operation: vimtypes.VirtualDeviceConfigSpecOperationEdit, + }, + } + }) + It(shouldSetEncryptionSyncedWithInvalidChanges, 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("updating encrypted", "specify policy when encrypting devices"))) + }) + }) + + When("encrypting raw disks", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: &vimtypes.CryptoSpecEncrypt{}, + }, + Device: &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Backing: &vimtypes.VirtualDiskRawDiskVer2BackingInfo{}, + }, + }, + Operation: vimtypes.VirtualDeviceConfigSpecOperationAdd, + Profile: []vimtypes.BaseVirtualMachineProfileSpec{ + &vimtypes.VirtualMachineDefinedProfileSpec{ + ProfileData: &vimtypes.VirtualMachineProfileRawData{ + ExtensionKey: "com.vmware.vim.sips", + ObjectData: profileWithIOFilters, + }, + }, + }, + }, + } + }) + It(shouldSetEncryptionSyncedWithInvalidChanges, 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("updating encrypted", "not encrypt raw disks"))) + }) + }) + + When("encrypting non-disk devices", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: &vimtypes.CryptoSpecEncrypt{}, + }, + Device: &vimtypes.VirtualAHCIController{}, + Operation: vimtypes.VirtualDeviceConfigSpecOperationAdd, + Profile: []vimtypes.BaseVirtualMachineProfileSpec{ + &vimtypes.VirtualMachineDefinedProfileSpec{ + ProfileData: &vimtypes.VirtualMachineProfileRawData{ + ExtensionKey: "com.vmware.vim.sips", + ObjectData: profileWithIOFilters, + }, + }, + }, + }, + } + }) + It(shouldSetEncryptionSyncedWithInvalidChanges, 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("updating encrypted", "not encrypt non-disk devices"))) + }) + }) + + When("adding encrypted disk with policy and multiple backings", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: &vimtypes.CryptoSpecEncrypt{}, + Parent: &vimtypes.VirtualDeviceConfigSpecBackingSpec{}, + }, + Device: &vimtypes.VirtualDisk{}, + Operation: vimtypes.VirtualDeviceConfigSpecOperationAdd, + Profile: []vimtypes.BaseVirtualMachineProfileSpec{ + &vimtypes.VirtualMachineDefinedProfileSpec{ + ProfileData: &vimtypes.VirtualMachineProfileRawData{ + ExtensionKey: "com.vmware.vim.sips", + ObjectData: profileWithIOFilters, + }, + }, + }, + }, + } + }) + It(shouldSetEncryptionSyncedWithInvalidChanges, 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("updating encrypted", "not encrypt devices with multiple backings"))) + }) + }) + }) + + When("encrypting", func() { + + assertIsEncrypt := func() { + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + ExpectWithOffset(1, conditions.Get(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeNil()) + ExpectWithOffset(1, configSpec.Crypto).ToNot(BeNil()) + ExpectWithOffset(1, configSpec.Crypto).To(Equal(&vimtypes.CryptoSpecEncrypt{ + CryptoKeyId: vimtypes.CryptoKeyId{ + KeyId: encClass.Spec.KeyID, + ProviderId: &vimtypes.KeyProviderId{ + Id: encClass.Spec.KeyProvider, + }, + }, + })) + } + + BeforeEach(func() { + moVM.Config.KeyId = nil + }) + + When("storage class is not encrypted and no vTPM", func() { + BeforeEach(func() { + vm.Spec.StorageClass = storageClass1.Name + }) + When("storage class is not encrypted and no vTPM", func() { + It(shouldSetEncryptionSyncedToFalse, 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.ReasonInvalidState.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("encrypting", "use encrypted storage class or have vTPM"))) + }) + }) + }) + + When("adding encrypted disk sans policy", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: &vimtypes.CryptoSpecEncrypt{}, + }, + Device: &vimtypes.VirtualDisk{}, + Operation: vimtypes.VirtualDeviceConfigSpecOperationAdd, + }, + } + }) + It(shouldSetEncryptionSyncedWithInvalidChanges, 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("encrypting", "specify policy when encrypting devices"))) + }) + }) + + When("vm is powered on", func() { + BeforeEach(func() { + moVM.Summary.Runtime.PowerState = vimtypes.VirtualMachinePowerStatePoweredOn + }) + It(shouldSetEncryptionSyncedWithInvalidState, 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.ReasonInvalidState.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("encrypting", "be powered off"))) + }) + }) + When("vm has snapshots", func() { + BeforeEach(func() { + moVM.Snapshot = getSnapshotInfoWithLinearChain() + }) + It(shouldSetEncryptionSyncedWithInvalidState, 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.ReasonInvalidState.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("encrypting", "not have snapshots"))) + }) + }) + When("vm is powered off with no snapshots", func() { + It("should encrypt the vm", func() { + assertIsEncrypt() + }) + }) + }) + When("recrypting", func() { + + When("new providerID", func() { + BeforeEach(func() { + encClass.Spec.KeyProvider = provider2ID + }) + When("vm has snapshot tree", func() { + BeforeEach(func() { + moVM.Snapshot = getSnapshotInfoWithTree() + }) + It(shouldSetEncryptionSyncedWithInvalidState, 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.ReasonInvalidState.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("recrypting", "not have snapshot tree"))) + }) + }) + }) + When("new keyID", func() { + BeforeEach(func() { + encClass.Spec.KeyID = provider1Key2ID + }) + When("vm has snapshot tree", func() { + BeforeEach(func() { + moVM.Snapshot = getSnapshotInfoWithTree() + }) + It(shouldSetEncryptionSyncedWithInvalidState, 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.ReasonInvalidState.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("recrypting", "not have snapshot tree"))) + }) + }) + }) + When("new providerID and new keyID", func() { + BeforeEach(func() { + encClass.Spec.KeyProvider = provider2ID + encClass.Spec.KeyID = provider2Key1ID + }) + When("vm has snapshot tree", func() { + BeforeEach(func() { + moVM.Snapshot = getSnapshotInfoWithTree() + }) + It(shouldSetEncryptionSyncedWithInvalidState, 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.ReasonInvalidState.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("recrypting", "not have snapshot tree"))) + }) + }) + + assertIsRecrypt := func() { + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + ExpectWithOffset(1, conditions.Get(vm, vmopv1.VirtualMachineEncryptionSynced)).To(BeNil()) + ExpectWithOffset(1, configSpec.Crypto).ToNot(BeNil()) + ExpectWithOffset(1, configSpec.Crypto).To(Equal(&vimtypes.CryptoSpecShallowRecrypt{ + NewKeyId: vimtypes.CryptoKeyId{ + KeyId: encClass.Spec.KeyID, + ProviderId: &vimtypes.KeyProviderId{ + Id: encClass.Spec.KeyProvider, + }, + }, + })) + } + + When("storage class is not encrypted and no vTPM", func() { + BeforeEach(func() { + vm.Spec.StorageClass = storageClass1.Name + }) + When("storage class is not encrypted and no vTPM", func() { + It(shouldSetEncryptionSyncedToFalse, 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.ReasonInvalidState.String())) + Expect(c.Message).To(Equal(pkgcrypto.SprintfStateNotSynced("recrypting", "use encrypted storage class or have vTPM"))) + }) + }) + }) + + When("powered off", func() { + BeforeEach(func() { + moVM.Summary.Runtime.PowerState = vimtypes.VirtualMachinePowerStatePoweredOff + }) + It("should recrypt the vm", func() { + assertIsRecrypt() + }) + When("has linear snapshot chain", func() { + BeforeEach(func() { + moVM.Snapshot = getSnapshotInfoWithLinearChain() + }) + It("should recrypt the vm", func() { + assertIsRecrypt() + }) + }) + }) + When("powered on", func() { + BeforeEach(func() { + moVM.Summary.Runtime.PowerState = vimtypes.VirtualMachinePowerStatePoweredOn + }) + It("should recrypt the vm", func() { + assertIsRecrypt() + }) + When("has linear snapshot chain", func() { + BeforeEach(func() { + moVM.Snapshot = getSnapshotInfoWithLinearChain() + }) + It("should recrypt the vm", func() { + assertIsRecrypt() + }) + }) + }) + When("suspended", func() { + BeforeEach(func() { + moVM.Summary.Runtime.PowerState = vimtypes.VirtualMachinePowerStateSuspended + }) + It("should recrypt the vm", func() { + assertIsRecrypt() + }) + When("has linear snapshot chain", func() { + BeforeEach(func() { + moVM.Snapshot = getSnapshotInfoWithLinearChain() + }) + It("should recrypt the vm", func() { + assertIsRecrypt() + }) + }) + }) + + When("adding encrypted disk sans policy", func() { + BeforeEach(func() { + configSpec.DeviceChange = []vimtypes.BaseVirtualDeviceConfigSpec{ + &vimtypes.VirtualDeviceConfigSpec{ + Backing: &vimtypes.VirtualDeviceConfigSpecBackingSpec{ + Crypto: &vimtypes.CryptoSpecEncrypt{}, + }, + Device: &vimtypes.VirtualDisk{}, + Operation: vimtypes.VirtualDeviceConfigSpecOperationAdd, + }, + } + }) + It(shouldSetEncryptionSyncedWithInvalidChanges, 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"))) + }) + }) + }) + + }) + }) + }) + }) +}) + +const ( + shouldSetEncryptionSyncedToFalse = "should set EncryptionSynced to false" + shouldSetEncryptionSyncedWithInvalidState = "should set EncryptionSynced to false w InvalidState" + shouldSetEncryptionSyncedWithInvalidChanges = "should set EncryptionSynced to false w InvalidChanges" +) + +func getSnapshotInfoWithTree() *vimtypes.VirtualMachineSnapshotInfo { + return &vimtypes.VirtualMachineSnapshotInfo{ + CurrentSnapshot: &vimtypes.ManagedObjectReference{}, + RootSnapshotList: []vimtypes.VirtualMachineSnapshotTree{ + { + Name: "1", + ChildSnapshotList: []vimtypes.VirtualMachineSnapshotTree{ + { + Name: "1a", + }, + { + Name: "1b", + }, + }, + }, + { + Name: "2", + }, + }, + } +} + +func getSnapshotInfoWithLinearChain() *vimtypes.VirtualMachineSnapshotInfo { + return &vimtypes.VirtualMachineSnapshotInfo{ + CurrentSnapshot: &vimtypes.ManagedObjectReference{}, + RootSnapshotList: []vimtypes.VirtualMachineSnapshotTree{ + { + Name: "1", + ChildSnapshotList: []vimtypes.VirtualMachineSnapshotTree{ + { + Name: "1a", + }, + }, + }, + }, + } +} + +const profileWithIOFilters = ` + + + + + + vmwarevmcrypt@encryption + IOFILTERS + + + + Rule-Set 1: IOFILTERS + + + None + 1970-01-01T00:00:00Z + 1970-01-01T00:00:00Z + 1 + None + Phony Profile ID +` diff --git a/pkg/vmconfig/crypto/crypto_reconciler_suite_test.go b/pkg/vmconfig/crypto/crypto_reconciler_suite_test.go new file mode 100644 index 000000000..6ff3e2565 --- /dev/null +++ b/pkg/vmconfig/crypto/crypto_reconciler_suite_test.go @@ -0,0 +1,24 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package crypto_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "k8s.io/klog/v2" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +func init() { + klog.SetOutput(GinkgoWriter) + logf.SetLogger(klog.Background()) +} + +func TestSuite(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Crypto Reconciler Test Suite") +} diff --git a/pkg/vmconfig/crypto/crypto_reconciler_test.go b/pkg/vmconfig/crypto/crypto_reconciler_test.go new file mode 100644 index 000000000..19d52558f --- /dev/null +++ b/pkg/vmconfig/crypto/crypto_reconciler_test.go @@ -0,0 +1,24 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package crypto_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto" +) + +var _ = Describe("New", Label(testlabels.Crypto), func() { + It("should return a reconciler", func() { + Expect(crypto.New()).ToNot(BeNil()) + }) +}) + +var _ = Describe("Name", Label(testlabels.Crypto), func() { + It("should return crypto", func() { + Expect(crypto.New().Name()).To(Equal("crypto")) + }) +}) diff --git a/pkg/vmconfig/crypto/internal/crypto_reconciler_context.go b/pkg/vmconfig/crypto/internal/crypto_reconciler_context.go new file mode 100644 index 000000000..0290c6065 --- /dev/null +++ b/pkg/vmconfig/crypto/internal/crypto_reconciler_context.go @@ -0,0 +1,48 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package internal + +import ( + "context" + + ctxgen "github.com/vmware-tanzu/vm-operator/pkg/context/generic" +) + +type ContextKeyType uint8 + +const ContextKeyValue ContextKeyType = 0 + +type State struct { + Operation string + IsEncStorClass bool +} + +func FromContext(ctx context.Context) State { + return ctxgen.FromContext( + ctx, + ContextKeyValue, + func(val State) State { + return val + }) +} + +func SetOperation(ctx context.Context, op string) { + ctxgen.SetContext( + ctx, + ContextKeyValue, + func(val State) State { + val.Operation = op + return val + }) +} + +func MarkEncryptedStorageClass(ctx context.Context) { + ctxgen.SetContext( + ctx, + ContextKeyValue, + func(val State) State { + val.IsEncStorClass = true + return val + }) +} diff --git a/pkg/vmconfig/vmconfig_reconciler.go b/pkg/vmconfig/vmconfig_reconciler.go new file mode 100644 index 000000000..56d686f67 --- /dev/null +++ b/pkg/vmconfig/vmconfig_reconciler.go @@ -0,0 +1,51 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package vmconfig + +import ( + "context" + + "github.com/vmware/govmomi/vim25" + "github.com/vmware/govmomi/vim25/mo" + vimtypes "github.com/vmware/govmomi/vim25/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" +) + +// Reconciler is a type that modifies a VirtualMachine's configuration before +// and/or after creating or updating the underlying, platform resource. +type Reconciler interface { + + // Name returns the unique name used to identify the reconciler. + Name() string + + // Reconcile should be called prior to creating a VirtualMachine or updating + // its configuration. + Reconcile( + ctx context.Context, + k8sClient ctrlclient.Client, + vimClient *vim25.Client, + vm *vmopv1.VirtualMachine, + moVM mo.VirtualMachine, + configSpec *vimtypes.VirtualMachineConfigSpec) error + + // OnResult should be called after creating a VirtualMachine or updating its + // configuration. + OnResult( + ctx context.Context, + vm *vmopv1.VirtualMachine, + moVM mo.VirtualMachine, + resultErr error) error +} + +// ReconcilerWithContext is a Reconciler that has state to share between its +// Reconcile and OnResult methods and does so via the context. +type ReconcilerWithContext interface { + Reconciler + + // WithContext returns a context used to share state between the Reconcile + // and OnResult methods. + WithContext(context.Context) context.Context +} diff --git a/pkg/vmconfig/vmconfig_reconciler_context.go b/pkg/vmconfig/vmconfig_reconciler_context.go new file mode 100644 index 000000000..2c3a05861 --- /dev/null +++ b/pkg/vmconfig/vmconfig_reconciler_context.go @@ -0,0 +1,86 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package vmconfig + +import ( + "context" + "maps" + + ctxgen "github.com/vmware-tanzu/vm-operator/pkg/context/generic" +) + +type contextKeyType uint8 + +const contextKeyValue contextKeyType = 0 + +type contextValueType map[string]Reconciler + +// Register registers the reconciler in the provided context. +// Please note, if two reconcilers have the same name, the last one registered +// takes precedence. +func Register(ctx context.Context, r Reconciler) context.Context { + ctxgen.SetContext( + ctx, + contextKeyValue, + func(curVal contextValueType) contextValueType { + curVal[r.Name()] = r + return curVal + }) + if rwc, ok := r.(ReconcilerWithContext); ok { + return rwc.WithContext(ctx) + } + return ctx +} + +// FromContext returns the list of registered reconcilers. +func FromContext(ctx context.Context) []Reconciler { + return ctxgen.FromContext( + ctx, + contextKeyValue, + func(val contextValueType) []Reconciler { + var list []Reconciler + for _, r := range val { + list = append(list, r) + } + return list + }) +} + +// WithContext returns a new context with a new reconcilers map, with the +// provided context as the parent. +func WithContext(parent context.Context) context.Context { + return ctxgen.WithContext( + parent, + contextKeyValue, + func() contextValueType { return contextValueType{} }) +} + +// NewContext returns a new context with a new reconcilers map. +func NewContext() context.Context { + return ctxgen.NewContext( + contextKeyValue, + func() contextValueType { return contextValueType{} }) +} + +// ValidateContext returns true if the provided context contains the reconcilers +// map. +func ValidateContext(ctx context.Context) bool { + return ctxgen.ValidateContext[contextValueType](ctx, contextKeyValue) +} + +// JoinContext returns a new context that contains a reference to the +// reconcilers map from the specified context. +// This function panics if the provided context does not contain a reconcilers +// map. +// This function is thread-safe. +func JoinContext(left, right context.Context) context.Context { + return ctxgen.JoinContext( + left, + right, + contextKeyValue, + func(dst, src contextValueType) contextValueType { + maps.Copy(dst, src) + return dst + }) +} diff --git a/pkg/vmconfig/vmconfig_reconciler_context_test.go b/pkg/vmconfig/vmconfig_reconciler_context_test.go new file mode 100644 index 000000000..69ca280d5 --- /dev/null +++ b/pkg/vmconfig/vmconfig_reconciler_context_test.go @@ -0,0 +1,233 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package vmconfig_test + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/vmware/govmomi/vim25" + "github.com/vmware/govmomi/vim25/mo" + vimtypes "github.com/vmware/govmomi/vim25/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" + "github.com/vmware-tanzu/vm-operator/pkg/vmconfig" +) + +var _ = Describe("FromContext", func() { + var ( + ctx context.Context + ) + BeforeEach(func() { + ctx = context.Background() + }) + When("ctx is nil", func() { + BeforeEach(func() { + ctx = nil + }) + It("should panic", func() { + fn := func() { + _ = vmconfig.FromContext(ctx) + } + Expect(fn).To(PanicWith("context is nil")) + }) + }) + When("value is missing from context", func() { + It("should panic", func() { + fn := func() { + _ = vmconfig.FromContext(ctx) + } + Expect(fn).To(PanicWith("value is missing from context")) + }) + }) + When("map is present in context", func() { + BeforeEach(func() { + ctx = vmconfig.NewContext() + }) + It("should return the value", func() { + Expect(vmconfig.FromContext(ctx)).To(Equal([]vmconfig.Reconciler(nil))) + }) + }) +}) + +var _ = Describe("ValidateContext", func() { + var ( + ctx context.Context + ) + BeforeEach(func() { + ctx = context.Background() + }) + When("ctx is nil", func() { + BeforeEach(func() { + ctx = nil + }) + It("should panic", func() { + fn := func() { + _ = vmconfig.ValidateContext(ctx) + } + Expect(fn).To(PanicWith("context is nil")) + }) + }) + When("value is missing from context", func() { + It("should return false", func() { + Expect(vmconfig.ValidateContext(ctx)).To(BeFalse()) + }) + }) + When("value is present in context", func() { + BeforeEach(func() { + ctx = vmconfig.NewContext() + }) + It("should return true", func() { + Expect(vmconfig.ValidateContext(ctx)).To(BeTrue()) + }) + }) +}) + +type reconciler struct { + name string +} + +func (r reconciler) Name() string { + return r.name +} + +func (r reconciler) Reconcile( + ctx context.Context, + k8sClient ctrlclient.Client, + vimClient *vim25.Client, + vm *vmopv1.VirtualMachine, + moVM mo.VirtualMachine, + configSpec *vimtypes.VirtualMachineConfigSpec) error { + + return nil +} + +func (r reconciler) OnResult( + ctx context.Context, + vm *vmopv1.VirtualMachine, + moVM mo.VirtualMachine, + resultErr error) error { + + return nil +} + +var _ = Describe("JoinContext", func() { + var ( + left context.Context + right context.Context + ) + BeforeEach(func() { + left = context.Background() + right = context.Background() + }) + When("left context is nil", func() { + BeforeEach(func() { + left = nil + }) + It("should panic", func() { + fn := func() { + _ = vmconfig.JoinContext(left, right) + } + Expect(fn).To(PanicWith("left context is nil")) + }) + }) + When("right context is nil", func() { + BeforeEach(func() { + right = nil + }) + It("should panic", func() { + fn := func() { + _ = vmconfig.JoinContext(left, right) + } + Expect(fn).To(PanicWith("right context is nil")) + }) + }) + When("value is missing from context", func() { + It("should panic", func() { + fn := func() { + _ = vmconfig.JoinContext(left, right) + } + Expect(fn).To(PanicWith("value is missing from context")) + }) + }) + When("the left context has the map", func() { + BeforeEach(func() { + left = vmconfig.NewContext() + }) + It("should return the left context", func() { + ctx := vmconfig.JoinContext(left, right) + Expect(ctx).ToNot(BeNil()) + Expect(vmconfig.ValidateContext(ctx)).To(BeTrue()) + Expect(ctx).To(Equal(left)) + }) + }) + When("the right context has the map", func() { + BeforeEach(func() { + right = vmconfig.NewContext() + }) + It("should return a new context", func() { + ctx := vmconfig.JoinContext(left, right) + Expect(ctx).ToNot(BeNil()) + Expect(vmconfig.ValidateContext(ctx)).To(BeTrue()) + }) + }) + When("both contexts have the map", func() { + var ( + v0 vmconfig.Reconciler + v1 vmconfig.Reconciler + v2a vmconfig.Reconciler + v2b vmconfig.Reconciler + ) + BeforeEach(func() { + v0 = &reconciler{name: "r0"} + v1 = &reconciler{name: "r1"} + v2a = &reconciler{name: "r2"} + v2b = &reconciler{name: "r2"} + + left = vmconfig.NewContext() + right = vmconfig.NewContext() + + vmconfig.Register(left, v0) + vmconfig.Register(right, v1) + vmconfig.Register(left, v2a) + vmconfig.Register(right, v2b) + + Expect(vmconfig.FromContext(left)).To(ContainElements(v0, v2a)) + Expect(vmconfig.FromContext(right)).To(ContainElements(v1, v2b)) + }) + It("should return the left context with key/value pairs from the right", func() { + ctx := vmconfig.JoinContext(left, right) + Expect(ctx).ToNot(BeNil()) + Expect(vmconfig.ValidateContext(ctx)).To(BeTrue()) + Expect(ctx).To(Equal(left)) + Expect(vmconfig.FromContext(ctx)).To(ContainElements(v0, v1, v2b)) + }) + }) +}) + +var _ = Describe("NewContext", func() { + It("should return a valid context", func() { + Expect(vmconfig.ValidateContext(vmconfig.NewContext())).To(BeTrue()) + }) +}) + +var _ = Describe("WithContext", func() { + When("parent is nil", func() { + It("should panic", func() { + fn := func() { + //nolint:staticcheck + _ = vmconfig.WithContext(nil) + } + Expect(fn).To(PanicWith("parent context is nil")) + }) + }) + When("parent is not nil", func() { + It("should return a context", func() { + ctx := vmconfig.WithContext(context.Background()) + Expect(ctx).ToNot(BeNil()) + }) + }) +}) diff --git a/pkg/vmconfig/vmconfig_reconciler_suite_test.go b/pkg/vmconfig/vmconfig_reconciler_suite_test.go new file mode 100644 index 000000000..9770e9ef5 --- /dev/null +++ b/pkg/vmconfig/vmconfig_reconciler_suite_test.go @@ -0,0 +1,24 @@ +// Copyright (c) 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package vmconfig_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "k8s.io/klog/v2" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +func init() { + klog.SetOutput(GinkgoWriter) + logf.SetLogger(klog.Background()) +} + +func TestSuite(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Reconfig Reconciler Test Suite") +} diff --git a/test/builder/dummies.go b/test/builder/dummies.go index 4863bc754..08985db59 100644 --- a/test/builder/dummies.go +++ b/test/builder/dummies.go @@ -60,18 +60,20 @@ func DummyStorageClass() *storagev1.StorageClass { } } -func DummyResourceQuota(namespace, rlName string) *corev1.ResourceQuota { - return &corev1.ResourceQuota{ +func DummyResourceQuota(namespace string, rlNames ...string) *corev1.ResourceQuota { + obj := &corev1.ResourceQuota{ ObjectMeta: metav1.ObjectMeta{ Name: DummyResourceQuotaName, Namespace: namespace, }, Spec: corev1.ResourceQuotaSpec{ - Hard: corev1.ResourceList{ - corev1.ResourceName(rlName): resource.MustParse("1"), - }, + Hard: corev1.ResourceList{}, }, } + for i := range rlNames { + obj.Spec.Hard[corev1.ResourceName(rlNames[i])] = resource.MustParse("1") + } + return obj } func DummyAvailabilityZone() *topologyv1.AvailabilityZone { diff --git a/test/builder/unit_test_context.go b/test/builder/unit_test_context.go index 0056a6090..e62204d36 100644 --- a/test/builder/unit_test_context.go +++ b/test/builder/unit_test_context.go @@ -29,6 +29,7 @@ type UnitTestContext struct { // NewUnitTestContext returns a new UnitTestContext. func NewUnitTestContext(initObjects ...client.Object) *UnitTestContext { fakeClient := NewFakeClient(initObjects...) + return &UnitTestContext{ Context: ctxop.WithContext(pkgcfg.NewContext()), Client: fakeClient, diff --git a/test/builder/vcsim_test_context.go b/test/builder/vcsim_test_context.go index 73d3a336d..cae2e0ef6 100644 --- a/test/builder/vcsim_test_context.go +++ b/test/builder/vcsim_test_context.go @@ -22,10 +22,13 @@ import ( "sort" "time" + "github.com/google/uuid" . "github.com/onsi/gomega" "github.com/vmware/govmomi" + vimcrypto "github.com/vmware/govmomi/crypto" "github.com/vmware/govmomi/find" "github.com/vmware/govmomi/object" + pbmsim "github.com/vmware/govmomi/pbm/simulator" "github.com/vmware/govmomi/simulator" "github.com/vmware/govmomi/vapi/library" "github.com/vmware/govmomi/vapi/rest" @@ -39,10 +42,17 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" // Blank import to make govmomi client aware of these bindings. - _ "github.com/vmware/govmomi/pbm/simulator" _ "github.com/vmware/govmomi/vapi/cluster/simulator" _ "github.com/vmware/govmomi/vapi/simulator" + // Already imported as pbmsim, but leaving the next import, even though + // commented out, as a reminder this package *must* be imported to register + // its bindings with vC Sim. + + //nolint:godot + // _ "github.com/vmware/govmomi/pbm/simulator" + + byokv1 "github.com/vmware-tanzu/vm-operator/external/byok/api/v1alpha1" topologyv1 "github.com/vmware-tanzu/vm-operator/external/tanzu-topology/api/v1alpha1" vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" @@ -108,6 +118,14 @@ type VCSimTestConfig struct { // WithISOSupport enables the FSS_WCP_VMSERVICE_ISO_SUPPORT FSS. WithISOSupport bool + + // WithoutEncryptionClass disables the creation of the EncryptionClass + // resource in each workload namespace. + WithoutEncryptionClass bool + + // WithoutNativeKeyProvider disables the creation of the native key provider + // in vcsim. + WithoutNativeKeyProvider bool } type TestContextForVCSim struct { @@ -141,8 +159,19 @@ type TestContextForVCSim struct { ContentLibraryIsoItemID string // When WithoutStorageClass is false: - StorageClassName string - StorageProfileID string + StorageClassName string + StorageProfileID string + EncryptedStorageClassName string + EncryptedStorageProfileID string + + // When WithoutEncryptionClass is false: + EncryptionClass1Name string + EncryptionClass1ProviderID string + EncryptionClass2Name string + EncryptionClass2ProviderID string + + // When WithoutNativeKeyProvider is false: + NativeKeyProviderID string networkEnv NetworkEnv NetworkRef object.NetworkReference @@ -158,8 +187,10 @@ type TestContextForVCSim struct { } type WorkloadNamespaceInfo struct { - Namespace string - Folder *object.Folder + Namespace string + Folder *object.Folder + EncryptionClass1KeyID string + EncryptionClass2KeyID string } const ( @@ -169,7 +200,7 @@ const ( clustersPerZone = 1 ) -func (s *TestSuite) NewTestContextForVCSim( +func NewTestContextForVCSim( config VCSimTestConfig, initObjects ...client.Object) *TestContextForVCSim { @@ -184,6 +215,13 @@ func (s *TestSuite) NewTestContextForVCSim( return ctx } +func (s *TestSuite) NewTestContextForVCSim( + config VCSimTestConfig, + initObjects ...client.Object) *TestContextForVCSim { + + return NewTestContextForVCSim(config, initObjects...) +} + func newTestContextForVCSim( config VCSimTestConfig, initObjects []client.Object) *TestContextForVCSim { @@ -295,12 +333,55 @@ func (c *TestContextForVCSim) CreateWorkloadNamespace() WorkloadNamespaceInfo { }, Spec: corev1.ResourceQuotaSpec{ Hard: corev1.ResourceList{ - corev1.ResourceName(c.StorageClassName + ".storageclass.storage.k8s.io/persistentvolumeclaims"): resource.MustParse("1"), + corev1.ResourceName(c.StorageClassName + ".storageclass.storage.k8s.io/persistentvolumeclaims"): resource.MustParse("1"), + corev1.ResourceName(c.EncryptedStorageProfileID + ".storageclass.storage.k8s.io/persistentvolumeclaims"): resource.MustParse("1"), }, }, } Expect(c.Client.Create(c, resourceQuota)).To(Succeed()) + var ( + encryptionClass1KeyID string + encryptionClass2KeyID string + ) + + if c.EncryptionClass1Name != "" || c.EncryptionClass2Name != "" { + m := vimcrypto.NewManagerKmip(c.VCClient.Client) + + createEncClass := func( + className, + providerID string, + keyID *string) { + + var err error + *keyID, err = m.GenerateKey(c, providerID) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + ExpectWithOffset(1, *keyID).ToNot(BeEmpty()) + + ExpectWithOffset(1, c.Client.Create(c, + &byokv1.EncryptionClass{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: className, + }, + Spec: byokv1.EncryptionClassSpec{ + KeyProvider: providerID, + KeyID: *keyID, + }, + })).To(Succeed()) + } + + createEncClass( + c.EncryptionClass1Name, + c.EncryptionClass1ProviderID, + &encryptionClass1KeyID) + + createEncClass( + c.EncryptionClass2Name, + c.EncryptionClass2ProviderID, + &encryptionClass2KeyID) + } + // Make trip through the Finder to populate InventoryPath. objRef, err := c.Finder.ObjectReference(c, nsFolder.Reference()) Expect(err).ToNot(HaveOccurred()) @@ -309,8 +390,10 @@ func (c *TestContextForVCSim) CreateWorkloadNamespace() WorkloadNamespaceInfo { Expect(nsFolder.InventoryPath).ToNot(BeEmpty()) return WorkloadNamespaceInfo{ - Namespace: ns.Name, - Folder: nsFolder, + Namespace: ns.Name, + Folder: nsFolder, + EncryptionClass1KeyID: encryptionClass1KeyID, + EncryptionClass2KeyID: encryptionClass2KeyID, } } @@ -648,18 +731,56 @@ func (c *TestContextForVCSim) setupK8sConfig(config VCSimTestConfig) { data["StorageClassRequired"] = "true" c.StorageClassName = "vcsim-default-storageclass" - // Use the hardcoded vcsim profile ID. - c.StorageProfileID = "aa6d5a82-1c88-45da-85d3-3d74b91a5bad" + c.StorageProfileID = "aa6d5a82-1c88-45da-85d3-3d74b91a5bad" // from vcsim - storageClass := &storagev1.StorageClass{ + Expect(c.Client.Create(c, &storagev1.StorageClass{ ObjectMeta: metav1.ObjectMeta{ Name: c.StorageClassName, }, Parameters: map[string]string{ "storagePolicyID": c.StorageProfileID, }, - } - Expect(c.Client.Create(c, storageClass)).To(Succeed()) + })).To(Succeed()) + + c.EncryptedStorageClassName = "vm-encryption-policy" + c.EncryptedStorageProfileID = pbmsim.DefaultEncryptionProfileID + + Expect(c.Client.Create(c, &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: c.EncryptedStorageClassName, + }, + Parameters: map[string]string{ + "storagePolicyID": c.EncryptedStorageProfileID, + }, + })).To(Succeed()) + } + + if !config.WithoutNativeKeyProvider { + m := vimcrypto.NewManagerKmip(c.VCClient.Client) + + c.NativeKeyProviderID = uuid.NewString() + Expect(m.RegisterKmsCluster( + c, + c.NativeKeyProviderID, + vimtypes.KmipClusterInfoKmsManagementTypeNativeProvider)).To(Succeed()) + } + + if !config.WithoutEncryptionClass { + m := vimcrypto.NewManagerKmip(c.VCClient.Client) + + c.EncryptionClass1Name = uuid.NewString() + c.EncryptionClass1ProviderID = uuid.NewString() + Expect(m.RegisterKmsCluster( + c, + c.EncryptionClass1ProviderID, + vimtypes.KmipClusterInfoKmsManagementTypeTrustAuthority)).To(Succeed()) + + c.EncryptionClass2Name = uuid.NewString() + c.EncryptionClass2ProviderID = uuid.NewString() + Expect(m.RegisterKmsCluster( + c, + c.EncryptionClass2ProviderID, + vimtypes.KmipClusterInfoKmsManagementTypeTrustAuthority)).To(Succeed()) } if !config.WithContentLibrary { diff --git a/test/testutil/util.go b/test/testutil/util.go index 412da06a7..3568d4683 100644 --- a/test/testutil/util.go +++ b/test/testutil/util.go @@ -81,3 +81,32 @@ func WriteKubeConfig(config *rest.Config) ([]byte, error) { CurrentContext: config.ServerName, }) } + +func ContainsError(err error, message string) bool { + for { + if err == nil { + return false + } + if err.Error() == message { + return true + } + switch tErr := err.(type) { + case unwrappableError: + err = tErr.Unwrap() + case unwrappableErrorSlice: + for _, uErr := range tErr.Unwrap() { + if ContainsError(uErr, message) { + return true + } + } + } + } +} + +type unwrappableError interface { + Unwrap() error +} + +type unwrappableErrorSlice interface { + Unwrap() []error +} diff --git a/webhooks/virtualmachine/validation/virtualmachine_validator.go b/webhooks/virtualmachine/validation/virtualmachine_validator.go index 075ed4394..666522b15 100644 --- a/webhooks/virtualmachine/validation/virtualmachine_validator.go +++ b/webhooks/virtualmachine/validation/virtualmachine_validator.go @@ -42,6 +42,7 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/topology" "github.com/vmware-tanzu/vm-operator/pkg/util" cloudinitvalidate "github.com/vmware-tanzu/vm-operator/pkg/util/cloudinit/validate" + kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1" "github.com/vmware-tanzu/vm-operator/webhooks/common" ) @@ -77,6 +78,7 @@ const ( invalidImageKind = "supported: " + vmiKind + "; " + cvmiKind invalidZone = "cannot use zone that is being deleted" restrictedToPrivUsers = "restricted to privileged users" + invalidPVCBYOKFmt = "cannot attach volume to vm with spec.crypto.className=%q" ) // +kubebuilder:webhook:verbs=create;update,path=/default-validate-vmoperator-vmware-com-v1alpha3-virtualmachine,mutating=false,failurePolicy=fail,groups=vmoperator.vmware.com,resources=virtualmachines,versions=v1alpha3,name=default.validating.virtualmachine.v1alpha3.vmoperator.vmware.com,sideEffects=None,admissionReviewVersions=v1;v1beta1 @@ -124,6 +126,7 @@ func (v validator) ValidateCreate(ctx *pkgctx.WebhookRequestContext) admission.R fieldErrs = append(fieldErrs, v.validateImageOnCreate(ctx, vm)...) fieldErrs = append(fieldErrs, v.validateClassOnCreate(ctx, vm)...) fieldErrs = append(fieldErrs, v.validateStorageClass(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateCrypto(ctx, vm)...) fieldErrs = append(fieldErrs, v.validateBootstrap(ctx, vm)...) fieldErrs = append(fieldErrs, v.validateNetwork(ctx, vm)...) fieldErrs = append(fieldErrs, v.validateVolumes(ctx, vm)...) @@ -184,6 +187,7 @@ func (v validator) ValidateUpdate(ctx *pkgctx.WebhookRequestContext) admission.R // Validations for allowed updates. Return validation responses here for conditional updates regardless // of whether the update is allowed or not. + fieldErrs = append(fieldErrs, v.validateCrypto(ctx, vm)...) fieldErrs = append(fieldErrs, v.validateAvailabilityZone(ctx, vm, oldVM)...) fieldErrs = append(fieldErrs, v.validateBootstrap(ctx, vm)...) fieldErrs = append(fieldErrs, v.validateNetwork(ctx, vm)...) @@ -472,6 +476,42 @@ func (v validator) validateStorageClass(ctx *pkgctx.WebhookRequestContext, vm *v return append(allErrs, field.Invalid(scPath, scName, fmt.Sprintf(storageClassNotAssignedFmt, vm.Namespace))) } +func (v validator) validateCrypto( + ctx *pkgctx.WebhookRequestContext, + vm *vmopv1.VirtualMachine) field.ErrorList { + + var ( + allErrs field.ErrorList + path = field.NewPath("spec", "crypto", "className") + encClassName = vm.Spec.Crypto.EncryptionClassName + ) + + if encClassName == "" { + return nil + } + + if !pkgcfg.FromContext(ctx).Features.BringYourOwnEncryptionKey { + + allErrs = append(allErrs, field.Invalid( + path, encClassName, "feature not enabled")) + + } else if ok, err := kubeutil.IsEncryptedStorageClass( + ctx, + v.client, + vm.Spec.StorageClass); err != nil { + + allErrs = append(allErrs, field.InternalError(path, err)) + + } else if !ok { + allErrs = append(allErrs, field.Invalid( + path, + vm.Spec.Crypto.EncryptionClassName, + "requires spec.storageClass specify an encrypted storage class")) + } + + return allErrs +} + func (v validator) validateNetwork(ctx *pkgctx.WebhookRequestContext, vm *vmopv1.VirtualMachine) field.ErrorList { var allErrs field.ErrorList @@ -773,7 +813,7 @@ func (v validator) validateVolumes(ctx *pkgctx.WebhookRequestContext, vm *vmopv1 if vol.PersistentVolumeClaim == nil { allErrs = append(allErrs, field.Required(volPath.Child("persistentVolumeClaim"), "")) } else { - allErrs = append(allErrs, v.validateVolumeWithPVC(ctx, vol, volPath)...) + allErrs = append(allErrs, v.validateVolumeWithPVC(ctx, vm, vol, volPath)...) } } @@ -782,14 +822,26 @@ func (v validator) validateVolumes(ctx *pkgctx.WebhookRequestContext, vm *vmopv1 func (v validator) validateVolumeWithPVC( ctx *pkgctx.WebhookRequestContext, + vm *vmopv1.VirtualMachine, vol vmopv1.VirtualMachineVolume, volPath *field.Path) field.ErrorList { - var allErrs field.ErrorList - pvcPath := volPath.Child("persistentVolumeClaim") + var ( + allErrs field.ErrorList + pvcPath = volPath.Child("persistentVolumeClaim") + claimName = vol.PersistentVolumeClaim.ClaimName + encClassName = vm.Spec.Crypto.EncryptionClassName + ) - if vol.PersistentVolumeClaim.ClaimName == "" { - allErrs = append(allErrs, field.Required(pvcPath.Child("claimName"), "")) + if claimName == "" { + allErrs = append( + allErrs, + field.Required(pvcPath.Child("claimName"), "")) + } else if encClassName != "" && pkgcfg.FromContext(ctx).Features.BringYourOwnEncryptionKey { + allErrs = append(allErrs, field.Invalid( + pvcPath.Child("claimName"), + claimName, + fmt.Sprintf(invalidPVCBYOKFmt, encClassName))) } if vol.PersistentVolumeClaim.ReadOnly { allErrs = append(allErrs, field.NotSupported(pvcPath.Child("readOnly"), true, []string{"false"})) diff --git a/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go b/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go index 86ed59251..2aa287746 100644 --- a/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go +++ b/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go @@ -12,6 +12,7 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/intstr" @@ -31,6 +32,7 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/config" "github.com/vmware-tanzu/vm-operator/pkg/topology" + kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube" "github.com/vmware-tanzu/vm-operator/pkg/util/ptr" vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1" @@ -38,6 +40,7 @@ import ( ) const ( + fake = "fake" updateSuffix = "-updated" dummyInstanceIDVal = "dummy-instance-id" dummyFirstBootDoneVal = "dummy-first-boot-done" @@ -763,6 +766,190 @@ func unitTestsValidateCreate() { ), }, ), + + // + // FSS_WCP_VMSERVICE_BYOK is disabled + // + Entry("disallow spec.crypto.className when FSS_WCP_VMSERVICE_BYOK is disabled", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + ctx.vm.Spec.Crypto.EncryptionClassName = fake + + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.BringYourOwnEncryptionKey = false + }) + }, + validate: doValidateWithMsg( + `spec.crypto.className: Invalid value: "fake": feature not enabled`), + }, + ), + + // + // FSS_WCP_VMSERVICE_BYOK is enabled + // + Entry("allow spec.crypto.className when FSS_WCP_VMSERVICE_BYOK is enabled", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + storageClass1 := builder.DummyStorageClass() + Expect(ctx.Client.Create(ctx, storageClass1)).To(Succeed()) + + rlName := storageClass1.Name + ".storageclass.storage.k8s.io/persistentvolumeclaims" + resourceQuota := builder.DummyResourceQuota(ctx.vm.Namespace, rlName) + Expect(ctx.Client.Create(ctx, resourceQuota)).To(Succeed()) + + ctx.vm.Spec.StorageClass = storageClass1.Name + ctx.vm.Spec.Crypto.EncryptionClassName = fake + ctx.vm.Spec.Volumes = nil + + var storageClass storagev1.StorageClass + Expect(ctx.Client.Get( + ctx, + client.ObjectKey{Name: ctx.vm.Spec.StorageClass}, + &storageClass)).To(Succeed()) + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, + ctx.Client, + storageClass, + true)).To(Succeed()) + + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.BringYourOwnEncryptionKey = true + }) + }, + expectAllowed: true, + }, + ), + Entry("disallow spec.crypto.className for non-encrypted storage class when FSS_WCP_VMSERVICE_BYOK is enabled", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + storageClass1 := builder.DummyStorageClass() + Expect(ctx.Client.Create(ctx, storageClass1)).To(Succeed()) + + rlName := storageClass1.Name + ".storageclass.storage.k8s.io/persistentvolumeclaims" + resourceQuota := builder.DummyResourceQuota(ctx.vm.Namespace, rlName) + Expect(ctx.Client.Create(ctx, resourceQuota)).To(Succeed()) + + ctx.vm.Spec.StorageClass = storageClass1.Name + ctx.vm.Spec.Crypto.EncryptionClassName = fake + ctx.vm.Spec.Volumes = nil + + var storageClass storagev1.StorageClass + Expect(ctx.Client.Get( + ctx, + client.ObjectKey{Name: ctx.vm.Spec.StorageClass}, + &storageClass)).To(Succeed()) + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, + ctx.Client, + storageClass, + false)).To(Succeed()) + + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.BringYourOwnEncryptionKey = true + }) + }, + validate: doValidateWithMsg( + `spec.crypto.className: Invalid value: "fake": requires spec.storageClass specify an encrypted storage class`), + }, + ), + Entry("disallow volume when spec.crypto.className is non-empty when FSS_WCP_VMSERVICE_BYOK is enabled", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + storageClass1 := builder.DummyStorageClass() + Expect(ctx.Client.Create(ctx, storageClass1)).To(Succeed()) + + storageClass2 := builder.DummyStorageClass() + storageClass2.Name += "2" + Expect(ctx.Client.Create(ctx, storageClass2)).To(Succeed()) + + resourceQuota := builder.DummyResourceQuota( + ctx.vm.Namespace, + storageClass1.Name+".storageclass.storage.k8s.io/persistentvolumeclaims", + storageClass2.Name+".storageclass.storage.k8s.io/persistentvolumeclaims") + Expect(ctx.Client.Create(ctx, resourceQuota)).To(Succeed()) + + pvc := builder.DummyPersistentVolumeClaim() + pvc.Name = builder.DummyPVCName + pvc.Namespace = ctx.vm.Namespace + pvc.Spec.StorageClassName = ptr.To(storageClass2.Name) + Expect(ctx.Client.Create(ctx, pvc)).To(Succeed()) + + ctx.vm.Spec.StorageClass = storageClass1.Name + ctx.vm.Spec.Crypto.EncryptionClassName = fake + + var storageClass storagev1.StorageClass + Expect(ctx.Client.Get( + ctx, + client.ObjectKey{Name: ctx.vm.Spec.StorageClass}, + &storageClass)).To(Succeed()) + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, + ctx.Client, + storageClass, + true)).To(Succeed()) + + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.BringYourOwnEncryptionKey = true + }) + }, + validate: doValidateWithMsg( + `spec.volumes[0].persistentVolumeClaim.claimName: Invalid value: "dummyPVCName": cannot attach volume to vm with spec.crypto.className="fake"`), + }, + ), + Entry("allow volume when spec.crypto.className is empty when FSS_WCP_VMSERVICE_BYOK is enabled", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + storageClass1 := builder.DummyStorageClass() + Expect(ctx.Client.Create(ctx, storageClass1)).To(Succeed()) + + storageClass2 := builder.DummyStorageClass() + storageClass2.Name += "2" + Expect(ctx.Client.Create(ctx, storageClass2)).To(Succeed()) + + resourceQuota := builder.DummyResourceQuota( + ctx.vm.Namespace, + storageClass1.Name+".storageclass.storage.k8s.io/persistentvolumeclaims", + storageClass2.Name+".storageclass.storage.k8s.io/persistentvolumeclaims") + Expect(ctx.Client.Create(ctx, resourceQuota)).To(Succeed()) + + pvc := builder.DummyPersistentVolumeClaim() + pvc.Name = builder.DummyPVCName + pvc.Namespace = ctx.vm.Namespace + pvc.Spec.StorageClassName = ptr.To(storageClass2.Name) + Expect(ctx.Client.Create(ctx, pvc)).To(Succeed()) + + ctx.vm.Spec.StorageClass = storageClass1.Name + ctx.vm.Spec.Crypto.EncryptionClassName = "" + + var vmStorageClass storagev1.StorageClass + Expect(ctx.Client.Get( + ctx, + client.ObjectKey{Name: ctx.vm.Spec.StorageClass}, + &vmStorageClass)).To(Succeed()) + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, + ctx.Client, + vmStorageClass, + false)).To(Succeed()) + + var pvcStorageClass storagev1.StorageClass + Expect(ctx.Client.Get( + ctx, + client.ObjectKey{Name: *pvc.Spec.StorageClassName}, + &pvcStorageClass)).To(Succeed()) + Expect(kubeutil.MarkEncryptedStorageClass( + ctx, + ctx.Client, + pvcStorageClass, + false)).To(Succeed()) + + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.BringYourOwnEncryptionKey = true + }) + }, + expectAllowed: true, + }, + ), ) Context("Annotations", func() {