From 073a43f2448a142631723a4ed9d55f9563af29ec Mon Sep 17 00:00:00 2001 From: Sagar Muchhal Date: Thu, 14 Dec 2023 17:38:52 -0800 Subject: [PATCH] Move key defaulting to CRDs This patch moves the defaulting logic of the secret selector keys to the CRDs instead of relying on a mutating webhook Signed-off-by: Sagar Muchhal --- api/v1alpha2/sysprep/sysprep.go | 40 ++++++++-- api/v1alpha2/sysprep/zz_generated.deepcopy.go | 55 ++++++++++++-- ...vmoperator.vmware.com_virtualmachines.yaml | 3 + .../mutation/virtualmachine_mutator.go | 36 --------- .../virtualmachine_mutator_unit_test.go | 74 ------------------- 5 files changed, 85 insertions(+), 123 deletions(-) diff --git a/api/v1alpha2/sysprep/sysprep.go b/api/v1alpha2/sysprep/sysprep.go index 45535b920..3094af55f 100644 --- a/api/v1alpha2/sysprep/sysprep.go +++ b/api/v1alpha2/sysprep/sysprep.go @@ -5,10 +5,6 @@ package sysprep -import ( - "github.com/vmware-tanzu/vm-operator/api/v1alpha2/common" -) - // Sysprep describes the object representation of a Windows sysprep.xml answer // file. // @@ -94,7 +90,7 @@ type GUIUnattended struct { // `password`. // // +optional - Password *common.SecretKeySelector `json:"password,omitempty"` + Password *PasswordSecretKeySelector `json:"password,omitempty"` // TimeZone is the time zone index for the virtual machine. // @@ -105,6 +101,16 @@ type GUIUnattended struct { TimeZone int32 `json:"timeZone,omitempty"` } +// PasswordSecretKeySelector references the password value from a Secret resource +type PasswordSecretKeySelector struct { + // Name is the name of the secret. + Name string `json:"name"` + + // Key is the key in the secret that specifies the requested data. + // +kubebuilder:default=password + Key string `json:"key"` +} + // Identification maps to the Identification key in the sysprep.xml answer file // and provides information needed to join a workgroup or domain. type Identification struct { @@ -124,7 +130,7 @@ type Identification struct { // `domain_admin_password`. // // +optional - DomainAdminPassword *common.SecretKeySelector `json:"domainAdminPassword,omitempty"` + DomainAdminPassword *DomainPasswordSecretKeySelector `json:"domainAdminPassword,omitempty"` // JoinDomain is the domain that the virtual machine should join. If this // value is supplied, then DomainAdmin and DomainAdminPassword must also be @@ -141,6 +147,16 @@ type Identification struct { JoinWorkgroup string `json:"joinWorkgroup,omitempty"` } +// DomainPasswordSecretKeySelector references the password value from a Secret resource +type DomainPasswordSecretKeySelector struct { + // Name is the name of the secret. + Name string `json:"name"` + + // Key is the key in the secret that specifies the requested data. + // +kubebuilder:default=domain_admin_password + Key string `json:"key"` +} + // CustomizationLicenseDataMode is an enumeration of the different license // modes. // @@ -198,5 +214,15 @@ type UserData struct { // `domain_admin_password`. // // +optional - ProductID *common.SecretKeySelector `json:"productID,omitempty"` + ProductID *ProductIDSecretKeySelector `json:"productID,omitempty"` +} + +// ProductIDSecretKeySelector references the ProductID value from a Secret resource +type ProductIDSecretKeySelector struct { + // Name is the name of the secret. + Name string `json:"name"` + + // Key is the key in the secret that specifies the requested data. + // +kubebuilder:default=product_id + Key string `json:"key"` } diff --git a/api/v1alpha2/sysprep/zz_generated.deepcopy.go b/api/v1alpha2/sysprep/zz_generated.deepcopy.go index 24b692090..d116eb8c1 100644 --- a/api/v1alpha2/sysprep/zz_generated.deepcopy.go +++ b/api/v1alpha2/sysprep/zz_generated.deepcopy.go @@ -8,9 +8,22 @@ package sysprep -import ( - "github.com/vmware-tanzu/vm-operator/api/v1alpha2/common" -) +import () + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DomainPasswordSecretKeySelector) DeepCopyInto(out *DomainPasswordSecretKeySelector) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DomainPasswordSecretKeySelector. +func (in *DomainPasswordSecretKeySelector) DeepCopy() *DomainPasswordSecretKeySelector { + if in == nil { + return nil + } + out := new(DomainPasswordSecretKeySelector) + in.DeepCopyInto(out) + return out +} // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GUIRunOnce) DeepCopyInto(out *GUIRunOnce) { @@ -37,7 +50,7 @@ func (in *GUIUnattended) DeepCopyInto(out *GUIUnattended) { *out = *in if in.Password != nil { in, out := &in.Password, &out.Password - *out = new(common.SecretKeySelector) + *out = new(PasswordSecretKeySelector) **out = **in } } @@ -57,7 +70,7 @@ func (in *Identification) DeepCopyInto(out *Identification) { *out = *in if in.DomainAdminPassword != nil { in, out := &in.DomainAdminPassword, &out.DomainAdminPassword - *out = new(common.SecretKeySelector) + *out = new(DomainPasswordSecretKeySelector) **out = **in } } @@ -92,6 +105,36 @@ func (in *LicenseFilePrintData) DeepCopy() *LicenseFilePrintData { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PasswordSecretKeySelector) DeepCopyInto(out *PasswordSecretKeySelector) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PasswordSecretKeySelector. +func (in *PasswordSecretKeySelector) DeepCopy() *PasswordSecretKeySelector { + if in == nil { + return nil + } + out := new(PasswordSecretKeySelector) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ProductIDSecretKeySelector) DeepCopyInto(out *ProductIDSecretKeySelector) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProductIDSecretKeySelector. +func (in *ProductIDSecretKeySelector) DeepCopy() *ProductIDSecretKeySelector { + if in == nil { + return nil + } + out := new(ProductIDSecretKeySelector) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Sysprep) DeepCopyInto(out *Sysprep) { *out = *in @@ -133,7 +176,7 @@ func (in *UserData) DeepCopyInto(out *UserData) { *out = *in if in.ProductID != nil { in, out := &in.ProductID, &out.ProductID - *out = new(common.SecretKeySelector) + *out = new(ProductIDSecretKeySelector) **out = **in } } diff --git a/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml b/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml index d534c9b9a..b25cc76c9 100644 --- a/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml +++ b/config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml @@ -1118,6 +1118,7 @@ spec: to `password`." properties: key: + default: password description: Key is the key in the secret that specifies the requested data. type: string @@ -1155,6 +1156,7 @@ spec: selector defaults to `domain_admin_password`." properties: key: + default: domain_admin_password description: Key is the key in the secret that specifies the requested data. type: string @@ -1221,6 +1223,7 @@ spec: selector defaults to `domain_admin_password`." properties: key: + default: product_id description: Key is the key in the secret that specifies the requested data. type: string diff --git a/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator.go b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator.go index efaab2b66..b655523e6 100644 --- a/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator.go +++ b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator.go @@ -119,9 +119,6 @@ func (m mutator) Mutate(ctx *context.WebhookRequestContext) admission.Response { } else if mutated { wasMutated = true } - if modified.Spec.Bootstrap != nil && SetDefaultSysprepKeys(modified) { - wasMutated = true - } case admissionv1.Update: oldVM, err := m.vmFromUnstructured(ctx.OldObj) if err != nil { @@ -330,36 +327,3 @@ func ResolveImageName( vm.Spec.ImageName = determinedImageName return true, nil } - -func SetDefaultSysprepKeys(vm *vmopv1.VirtualMachine) bool { - if vm.Spec.Bootstrap.Sysprep == nil || vm.Spec.Bootstrap.Sysprep.Sysprep == nil { - return false - } - - wasMutated := false - inlineSysprep := vm.Spec.Bootstrap.Sysprep.Sysprep - if inlineSysprep.GUIUnattended != nil { - if inlineSysprep.GUIUnattended.Password != nil && inlineSysprep.GUIUnattended.Password.Key == "" { - inlineSysprep.GUIUnattended.Password.Key = "password" - wasMutated = true - } - } - - if inlineSysprep.Identification != nil { - if inlineSysprep.Identification.JoinDomain != "" && - inlineSysprep.Identification.DomainAdminPassword != nil && - inlineSysprep.Identification.DomainAdminPassword.Key == "" { - inlineSysprep.Identification.DomainAdminPassword.Key = "domain_admin_password" - wasMutated = true - } - } - - if inlineSysprep.UserData != nil && inlineSysprep.UserData.ProductID != nil { - if inlineSysprep.UserData.ProductID.Key == "" { - inlineSysprep.UserData.ProductID.Key = "product_id" - wasMutated = true - } - } - - return wasMutated -} diff --git a/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_unit_test.go b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_unit_test.go index 42b5e06e5..ed3fc6820 100644 --- a/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_unit_test.go +++ b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_unit_test.go @@ -19,8 +19,6 @@ import ( "github.com/vmware-tanzu/vm-operator/api/v1alpha1" vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" - "github.com/vmware-tanzu/vm-operator/api/v1alpha2/common" - "github.com/vmware-tanzu/vm-operator/api/v1alpha2/sysprep" "github.com/vmware-tanzu/vm-operator/pkg/lib" "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere2/config" "github.com/vmware-tanzu/vm-operator/test/builder" @@ -500,76 +498,4 @@ func unitTestsMutating() { ) }) }) - - Describe("Sysprep", func() { - Context("When GUIUnattended is set", func() { - BeforeEach(func() { - ctx.vm.Spec.Bootstrap = &vmopv1.VirtualMachineBootstrapSpec{ - Sysprep: &vmopv1.VirtualMachineBootstrapSysprepSpec{ - Sysprep: &sysprep.Sysprep{ - GUIUnattended: &sysprep.GUIUnattended{ - AutoLogon: true, - Password: &common.SecretKeySelector{Name: "pwd_secret"}, - }, - }, - }, - } - }) - - It("should mutate Password selector key", func() { - Expect(mutation.SetDefaultSysprepKeys(ctx.vm)).To(BeTrue()) - Expect(ctx.vm.Spec.Bootstrap.Sysprep.Sysprep.GUIUnattended.Password.Key).To(Equal("password")) - }) - }) - - Context("When Identification is set", func() { - BeforeEach(func() { - ctx.vm.Spec.Bootstrap = &vmopv1.VirtualMachineBootstrapSpec{ - Sysprep: &vmopv1.VirtualMachineBootstrapSysprepSpec{ - Sysprep: &sysprep.Sysprep{ - Identification: &sysprep.Identification{ - JoinDomain: "foo-domain", - DomainAdminPassword: &common.SecretKeySelector{Name: "pwd_secret"}, - }, - }, - }, - } - }) - - It("should mutate DomainAdminPassword selector key", func() { - Expect(mutation.SetDefaultSysprepKeys(ctx.vm)).To(BeTrue()) - Expect(ctx.vm.Spec.Bootstrap.Sysprep.Sysprep.Identification.DomainAdminPassword.Key).To(Equal("domain_admin_password")) - }) - }) - - Context("When UserData is set", func() { - BeforeEach(func() { - ctx.vm.Spec.Bootstrap = &vmopv1.VirtualMachineBootstrapSpec{ - Sysprep: &vmopv1.VirtualMachineBootstrapSysprepSpec{ - Sysprep: &sysprep.Sysprep{ - UserData: &sysprep.UserData{ - OrgName: "foo-org", - ProductID: &common.SecretKeySelector{Name: "product_id_secret"}, - }, - }, - }, - } - }) - - It("should mutate ProductID selector key", func() { - Expect(mutation.SetDefaultSysprepKeys(ctx.vm)).To(BeTrue()) - Expect(ctx.vm.Spec.Bootstrap.Sysprep.Sysprep.UserData.ProductID.Key).To(Equal("product_id")) - }) - - Context("When ProductID selector is not set", func() { - BeforeEach(func() { - ctx.vm.Spec.Bootstrap.Sysprep.Sysprep.UserData.ProductID = nil - }) - - It("should not mutate ProductID selector key", func() { - Expect(mutation.SetDefaultSysprepKeys(ctx.vm)).To(BeFalse()) - }) - }) - }) - }) }