From 8e4f8b59b0e0034dd5ff7e213349d5accb8cb722 Mon Sep 17 00:00:00 2001 From: Sagar Muchhal Date: Tue, 12 Dec 2023 18:51:53 -0800 Subject: [PATCH] Fetches secret data for createArgs verification Instead of waiting till doBootstrap to pull the secret data for inline sysprep, this patch adds the secret data to the BootstrapArgs earlier for verification purposes. Signed-off-by: Sagar Muchhal --- .../vsphere2/vmlifecycle/bootstrap_sysprep.go | 14 +- .../vmlifecycle/bootstrap_sysprep_test.go | 10 +- .../providers/vsphere2/vmprovider_vm_utils.go | 60 +++--- .../vsphere2/vmprovider_vm_utils_test.go | 201 +++++++++++++++++- .../validation/virtualmachine_validator.go | 6 +- 5 files changed, 244 insertions(+), 47 deletions(-) diff --git a/pkg/vmprovider/providers/vsphere2/vmlifecycle/bootstrap_sysprep.go b/pkg/vmprovider/providers/vsphere2/vmlifecycle/bootstrap_sysprep.go index 59f601c96..e00a2143d 100644 --- a/pkg/vmprovider/providers/vsphere2/vmlifecycle/bootstrap_sysprep.go +++ b/pkg/vmprovider/providers/vsphere2/vmlifecycle/bootstrap_sysprep.go @@ -54,7 +54,7 @@ func BootstrapSysPrep( Value: data, } } else if sysPrep := sysPrepSpec.Sysprep; sysPrep != nil { - identity = ConvertTo(sysPrep, bsArgs.BootstrapData) + identity = convertTo(sysPrep, bsArgs.BootstrapData) } nicSettingMap, err := network.GuestOSCustomization(bsArgs.NetworkResults) @@ -85,7 +85,7 @@ func BootstrapSysPrep( return configSpec, customSpec, nil } -func ConvertTo(from *vmopv1_prep.Sysprep, bootstrapData BootstrapData) *vimTypes.CustomizationSysprep { +func convertTo(from *vmopv1_prep.Sysprep, bootstrapData BootstrapData) *vimTypes.CustomizationSysprep { sysprepCustomization := &vimTypes.CustomizationSysprep{} if from.GUIUnattended != nil { @@ -102,9 +102,13 @@ func ConvertTo(from *vmopv1_prep.Sysprep, bootstrapData BootstrapData) *vimTypes if from.UserData != nil { sysprepCustomization.UserData = vimTypes.CustomizationUserData{ - FullName: from.UserData.FullName, - OrgName: from.UserData.OrgName, - ProductId: *bootstrapData.SysprepProductID, + FullName: from.UserData.FullName, + OrgName: from.UserData.OrgName, + } + // In the case of a VMI with volume license key, this might not be set. + // Hence add a check to see if the productID is set to empty. + if bootstrapData.SysprepProductID != nil { + sysprepCustomization.UserData.ProductId = *bootstrapData.SysprepProductID } } diff --git a/pkg/vmprovider/providers/vsphere2/vmlifecycle/bootstrap_sysprep_test.go b/pkg/vmprovider/providers/vsphere2/vmlifecycle/bootstrap_sysprep_test.go index cd2542852..5fcc8a48b 100644 --- a/pkg/vmprovider/providers/vsphere2/vmlifecycle/bootstrap_sysprep_test.go +++ b/pkg/vmprovider/providers/vsphere2/vmlifecycle/bootstrap_sysprep_test.go @@ -8,7 +8,6 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/vmware/govmomi/vim25/types" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" @@ -106,18 +105,17 @@ var _ = Describe("SysPrep Bootstrap", func() { GUIUnattended: &sysprep.GUIUnattended{ AutoLogon: true, AutoLogonCount: 2, - Password: corev1.SecretKeySelector{ + Password: &common.SecretKeySelector{ // omitting the name of the secret, since it does not get used // in this function - LocalObjectReference: corev1.LocalObjectReference{}, - Key: "pwd_key", + Key: "pwd_key", }, TimeZone: 4, }, UserData: &sysprep.UserData{ FullName: "foo-bar", OrgName: "foo-org", - ProductID: corev1.SecretKeySelector{Key: "product_id_key"}, + ProductID: &common.SecretKeySelector{Key: "product_id_key"}, }, GUIRunOnce: sysprep.GUIRunOnce{ Commands: []string{"blah", "boom"}, @@ -126,7 +124,7 @@ var _ = Describe("SysPrep Bootstrap", func() { DomainAdmin: "[Foo/Administrator]", JoinDomain: "foo.local", JoinWorkgroup: "foo.local.wg", - DomainAdminPassword: corev1.SecretKeySelector{Key: "admin_pwd_key"}, + DomainAdminPassword: &common.SecretKeySelector{Key: "admin_pwd_key"}, }, LicenseFilePrintData: &sysprep.LicenseFilePrintData{ AutoMode: sysprep.CustomizationLicenseDataModePerServer, diff --git a/pkg/vmprovider/providers/vsphere2/vmprovider_vm_utils.go b/pkg/vmprovider/providers/vsphere2/vmprovider_vm_utils.go index a707b83a3..875fac31b 100644 --- a/pkg/vmprovider/providers/vsphere2/vmprovider_vm_utils.go +++ b/pkg/vmprovider/providers/vsphere2/vmprovider_vm_utils.go @@ -14,7 +14,7 @@ import ( ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" - vmopv1common "github.com/vmware-tanzu/vm-operator/api/v1alpha2/common" + "github.com/vmware-tanzu/vm-operator/api/v1alpha2/sysprep" conditions "github.com/vmware-tanzu/vm-operator/pkg/conditions2" "github.com/vmware-tanzu/vm-operator/pkg/context" "github.com/vmware-tanzu/vm-operator/pkg/util" @@ -163,30 +163,32 @@ func GetVirtualMachineBootstrap( return bootstrapData, nil } - var secretSelector *vmopv1common.SecretKeySelector var data, vAppData map[string]string var vAppExData map[string]map[string]string - if cloudInit := bootstrapSpec.CloudInit; cloudInit != nil { - secretSelector = cloudInit.RawCloudConfig - } else if sysprep := bootstrapSpec.Sysprep; sysprep != nil { - secretSelector = sysprep.RawSysprep - } - - if secretSelector != nil { - var err error - data, err = getSecretData(vmCtx, k8sClient, secretSelector.Name, secretSelector.Key, true) - if err != nil { - reason, msg := errToConditionReasonAndMessage(err) - conditions.MarkFalse(vmCtx.VM, vmopv1.VirtualMachineConditionBootstrapReady, reason, msg) - return bootstrapData, err + if v := bootstrapSpec.CloudInit; v != nil { + if cooked := v.CloudConfig; cooked != nil { + _ = cooked + // TODO + } else if raw := v.RawCloudConfig; raw != nil { + var err error + data, err = getSecretData(vmCtx, k8sClient, raw.Name, raw.Key, true) + if err != nil { + return bootstrapData, err + } + } + } else if v := bootstrapSpec.Sysprep; v != nil { + if cooked := v.Sysprep; cooked != nil { + if err := fetchInlineSysprepSecretData(vmCtx, *cooked, k8sClient, &bootstrapData); err != nil { + return bootstrapData, err + } + } else if raw := v.RawSysprep; raw != nil { + var err error + data, err = getSecretData(vmCtx, k8sClient, raw.Name, raw.Key, true) + if err != nil { + return bootstrapData, err + } } - } - - if err := fetchInlineSysprepSecretData(vmCtx, k8sClient, &bootstrapData); err != nil { - reason, msg := errToConditionReasonAndMessage(err) - conditions.MarkFalse(vmCtx.VM, vmopv1.VirtualMachineConditionBootstrapReady, reason, msg) - return bootstrapData, err } // vApp bootstrap can be used alongside LinuxPrep/Sysprep. @@ -246,17 +248,13 @@ func GetVirtualMachineBootstrap( // secrets specified the Sysprep field. func fetchInlineSysprepSecretData( vmCtx context.VirtualMachineContextA2, + sysprep sysprep.Sysprep, k8sClient ctrlclient.Client, bootstrapData *vmlifecycle.BootstrapData) error { - if vmCtx.VM.Spec.Bootstrap.Sysprep == nil || - vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep == nil { - return nil - } - - sysprep := vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep - if userData := sysprep.UserData; userData != nil { - productIDData, err := getSecretData(vmCtx, &userData.ProductID, false, k8sClient) + if userData := sysprep.UserData; userData != nil && userData.ProductID != nil { + // this is an optional secret key selector even when FullName or OrgName are set. + productIDData, err := getSecretData(vmCtx, k8sClient, userData.ProductID.Name, userData.ProductID.Key, false) if err != nil { return err } @@ -265,7 +263,7 @@ func fetchInlineSysprepSecretData( } if guiUnattended := sysprep.GUIUnattended; guiUnattended != nil && guiUnattended.AutoLogon { - passwordData, err := getSecretData(vmCtx, &guiUnattended.Password, false, k8sClient) + passwordData, err := getSecretData(vmCtx, k8sClient, guiUnattended.Password.Name, guiUnattended.Password.Key, false) if err != nil { return err } @@ -274,7 +272,7 @@ func fetchInlineSysprepSecretData( } if identification := sysprep.Identification; identification != nil && identification.JoinDomain != "" { - domainPwdData, err := getSecretData(vmCtx, &identification.DomainAdminPassword, false, k8sClient) + domainPwdData, err := getSecretData(vmCtx, k8sClient, identification.DomainAdminPassword.Name, identification.DomainAdminPassword.Key, false) if err != nil { return err } diff --git a/pkg/vmprovider/providers/vsphere2/vmprovider_vm_utils_test.go b/pkg/vmprovider/providers/vsphere2/vmprovider_vm_utils_test.go index c32542460..caaab70b3 100644 --- a/pkg/vmprovider/providers/vsphere2/vmprovider_vm_utils_test.go +++ b/pkg/vmprovider/providers/vsphere2/vmprovider_vm_utils_test.go @@ -9,7 +9,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - + "github.com/onsi/gomega/gstruct" "github.com/vmware/govmomi/vim25/types" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -17,6 +17,7 @@ import ( 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" conditions "github.com/vmware-tanzu/vm-operator/pkg/conditions2" "github.com/vmware-tanzu/vm-operator/pkg/context" "github.com/vmware-tanzu/vm-operator/pkg/lib" @@ -321,7 +322,7 @@ func vmUtilTests() { }) - When("Bootstrap via Sysprep", func() { + When("Bootstrap via RawSysprep", func() { BeforeEach(func() { vmCtx.VM.Spec.Bootstrap = &vmopv1.VirtualMachineBootstrapSpec{ Sysprep: &vmopv1.VirtualMachineBootstrapSysprepSpec{}, @@ -365,6 +366,202 @@ func vmUtilTests() { }) }) + Context("Bootstrap via inline Sysprep", func() { + anotherKey := "some_other_key" + + BeforeEach(func() { + vmCtx.VM.Spec.Bootstrap = &vmopv1.VirtualMachineBootstrapSpec{ + Sysprep: &vmopv1.VirtualMachineBootstrapSysprepSpec{}, + } + }) + + Context("for UserData", func() { + productIDSecretName := "product_id_secret" + BeforeEach(func() { + vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep = &sysprep.Sysprep{ + UserData: &sysprep.UserData{ + ProductID: &common.SecretKeySelector{ + Name: productIDSecretName, + Key: "product_id", + }, + }, + } + }) + + When("secret is present", func() { + BeforeEach(func() { + initObjects = append(initObjects, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: productIDSecretName, + Namespace: vmCtx.VM.Namespace, + }, + Data: map[string][]byte{ + "product_id": []byte("foo_product_id"), + }, + }) + }) + + It("returns success", func() { + bsData, err := vsphere.GetVirtualMachineBootstrap(vmCtx, k8sClient) + Expect(err).ToNot(HaveOccurred()) + Expect(bsData.SysprepProductID).To(gstruct.PointTo(Equal("foo_product_id"))) + }) + + When("key from selector is not present", func() { + BeforeEach(func() { + vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep.UserData.ProductID.Key = anotherKey + }) + + It("returns an error", func() { + bsData, err := vsphere.GetVirtualMachineBootstrap(vmCtx, k8sClient) + Expect(err).To(HaveOccurred()) + Expect(conditions.IsFalse(vmCtx.VM, vmopv1.VirtualMachineConditionBootstrapReady)).To(BeTrue()) + Expect(bsData.SysprepProductID).To(BeNil()) + }) + }) + }) + + When("secret is not present", func() { + It("returns an error", func() { + bsData, err := vsphere.GetVirtualMachineBootstrap(vmCtx, k8sClient) + Expect(err).To(HaveOccurred()) + Expect(conditions.IsFalse(vmCtx.VM, vmopv1.VirtualMachineConditionBootstrapReady)).To(BeTrue()) + Expect(bsData.SysprepProductID).To(BeNil()) + }) + }) + + When("secret selector is absent", func() { + + BeforeEach(func() { + vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep.UserData.FullName = "foo" + vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep.UserData.ProductID = nil + }) + + It("does not return an error", func() { + bsData, err := vsphere.GetVirtualMachineBootstrap(vmCtx, k8sClient) + Expect(err).ToNot(HaveOccurred()) + Expect(bsData.SysprepProductID).To(BeNil()) + }) + }) + }) + + Context("for GUIUnattended", func() { + pwdSecretName := "password_secret" + + BeforeEach(func() { + vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep = &sysprep.Sysprep{ + GUIUnattended: &sysprep.GUIUnattended{ + AutoLogon: true, + Password: &common.SecretKeySelector{ + Name: pwdSecretName, + Key: "password", + }, + }, + } + }) + + When("secret is present", func() { + BeforeEach(func() { + initObjects = append(initObjects, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: pwdSecretName, + Namespace: vmCtx.VM.Namespace, + }, + Data: map[string][]byte{ + "password": []byte("foo_bar123"), + }, + }) + }) + + It("returns success", func() { + bsData, err := vsphere.GetVirtualMachineBootstrap(vmCtx, k8sClient) + Expect(err).ToNot(HaveOccurred()) + Expect(bsData.SysprepPassword).To(gstruct.PointTo(Equal("foo_bar123"))) + }) + + When("key from selector is not present", func() { + BeforeEach(func() { + vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep.GUIUnattended.Password.Key = anotherKey + }) + + It("returns an error", func() { + bsData, err := vsphere.GetVirtualMachineBootstrap(vmCtx, k8sClient) + Expect(err).To(HaveOccurred()) + Expect(conditions.IsFalse(vmCtx.VM, vmopv1.VirtualMachineConditionBootstrapReady)).To(BeTrue()) + Expect(bsData.SysprepPassword).To(BeNil()) + }) + }) + }) + + When("secret is not present", func() { + It("returns an error", func() { + bsData, err := vsphere.GetVirtualMachineBootstrap(vmCtx, k8sClient) + Expect(err).To(HaveOccurred()) + Expect(conditions.IsFalse(vmCtx.VM, vmopv1.VirtualMachineConditionBootstrapReady)).To(BeTrue()) + Expect(bsData.SysprepPassword).To(BeNil()) + }) + }) + }) + + Context("for Identification", func() { + pwdSecretName := "domain_password_secret" + + BeforeEach(func() { + vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep = &sysprep.Sysprep{ + Identification: &sysprep.Identification{ + JoinDomain: "foo", + DomainAdminPassword: &common.SecretKeySelector{ + Name: pwdSecretName, + Key: "domain_password", + }, + }, + } + }) + + When("secret is present", func() { + BeforeEach(func() { + initObjects = append(initObjects, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: pwdSecretName, + Namespace: vmCtx.VM.Namespace, + }, + Data: map[string][]byte{ + "domain_password": []byte("foo_bar_fizz123"), + }, + }) + }) + + It("returns success", func() { + bsData, err := vsphere.GetVirtualMachineBootstrap(vmCtx, k8sClient) + Expect(err).ToNot(HaveOccurred()) + Expect(bsData.SysprepDomainPassword).To(gstruct.PointTo(Equal("foo_bar_fizz123"))) + }) + + When("key from selector is not present", func() { + BeforeEach(func() { + vmCtx.VM.Spec.Bootstrap.Sysprep.Sysprep.Identification.DomainAdminPassword.Key = anotherKey + }) + + It("returns an error", func() { + bsData, err := vsphere.GetVirtualMachineBootstrap(vmCtx, k8sClient) + Expect(err).To(HaveOccurred()) + Expect(conditions.IsFalse(vmCtx.VM, vmopv1.VirtualMachineConditionBootstrapReady)).To(BeTrue()) + Expect(bsData.SysprepDomainPassword).To(BeNil()) + }) + }) + }) + + When("secret is not present", func() { + It("returns an error", func() { + bsData, err := vsphere.GetVirtualMachineBootstrap(vmCtx, k8sClient) + Expect(err).To(HaveOccurred()) + Expect(conditions.IsFalse(vmCtx.VM, vmopv1.VirtualMachineConditionBootstrapReady)).To(BeTrue()) + Expect(bsData.SysprepDomainPassword).To(BeNil()) + }) + }) + }) + }) + When("Bootstrap with vAppConfig", func() { BeforeEach(func() { diff --git a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go index a42357ff5..3532ffa7b 100644 --- a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go +++ b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go @@ -241,7 +241,7 @@ func (v validator) validateBootstrap( if sysPrep.Sysprep != nil { s := p.Child("sysprep") if guiUnattended := sysPrep.Sysprep.GUIUnattended; guiUnattended != nil { - if guiUnattended.AutoLogon && guiUnattended.Password.LocalObjectReference.Name == "" { + if guiUnattended.AutoLogon && guiUnattended.Password.Name == "" { allErrs = append(allErrs, field.Invalid(s, "guiUnattended", "autoLogon requires password selector to be set")) } @@ -253,13 +253,13 @@ func (v validator) validateBootstrap( "joinDomain and joinWorkgroup are mutually exclusive")) } - if identification.JoinDomain != "" && identification.DomainAdminPassword.LocalObjectReference.Name == "" { + if identification.JoinDomain != "" && identification.DomainAdminPassword.Name == "" { allErrs = append(allErrs, field.Invalid(s, "identification", "joinDomain requires domainAdminPassword selector to be set")) } if identification.JoinWorkgroup != "" { - if identification.DomainAdmin != "" || identification.DomainAdminPassword.LocalObjectReference.Name != "" { + if identification.DomainAdmin != "" || identification.DomainAdminPassword.Name != "" { allErrs = append(allErrs, field.Invalid(s, "identification", "joinWorkgroup and domainAdmin/domainAdminPassword are mutually exclusive")) }