From ace88894487b490d9453ffdb9cf27565c014ec0a Mon Sep 17 00:00:00 2001 From: akutz Date: Tue, 28 Jan 2025 15:34:06 -0600 Subject: [PATCH] Fix missing labels on CVMI resources This patch fixes an issue where the labels on the CVMI resources from the CCLI resources were missing. The labels were added via https://github.com/vmware-tanzu/vm-operator/pull/406 but were accidentally removed via https://github.com/vmware-tanzu/vm-operator/pull/851. When adding them back, there are now also tests to validate the logic works as there were no tests in the original PR. --- .../utils/controller_builder.go | 37 ++++ .../utils/controller_builder_test.go | 190 +++++++++++++++++- 2 files changed, 226 insertions(+), 1 deletion(-) diff --git a/controllers/contentlibrary/utils/controller_builder.go b/controllers/contentlibrary/utils/controller_builder.go index c0329c765..e228c2249 100644 --- a/controllers/contentlibrary/utils/controller_builder.go +++ b/controllers/contentlibrary/utils/controller_builder.go @@ -280,6 +280,7 @@ func (r *Reconciler) ReconcileNormal( }() if err := r.setUpVMIFromCLItem( + ctx, cliObj, *cliSpec, *cliStatus, @@ -384,6 +385,7 @@ func (r *Reconciler) ReconcileNormal( // setUpVMIFromCLItem sets up the VirtualMachineImage fields that // are retrievable from the given ContentLibraryItem resource. func (r *Reconciler) setUpVMIFromCLItem( + ctx context.Context, cliObj client.Object, cliSpec imgregv1a1.ContentLibraryItemSpec, cliStatus imgregv1a1.ContentLibraryItemStatus, @@ -407,6 +409,41 @@ func (r *Reconciler) setUpVMIFromCLItem( Name: cliObj.GetName(), } + if cliObj.GetNamespace() == "" { + var ( + cliLabels = cliObj.GetLabels() + vmiLabels = vmiObj.GetLabels() + labelKeyPrefix = TKGServiceTypeLabelKeyPrefix + ) + + if pkgcfg.FromContext(ctx).Features.TKGMultipleCL { + labelKeyPrefix = MultipleCLServiceTypeLabelKeyPrefix + + // Remove any labels on the VMI object that have a matching + // prefix do not also exist on the CLI resource. + for k := range vmiLabels { + if strings.HasPrefix(k, labelKeyPrefix) { + if _, ok := cliLabels[k]; !ok { + delete(vmiLabels, k) + } + } + } + } + + // Copy the labels from the CLI object that match the given prefix + // to the VMI resource. + for k := range cliLabels { + if strings.HasPrefix(k, labelKeyPrefix) { + if vmiLabels == nil { + vmiLabels = map[string]string{} + } + vmiLabels[k] = "" + } + } + + vmiObj.SetLabels(vmiLabels) + } + vmiStatus.Name = cliStatus.Name vmiStatus.ProviderItemID = string(cliSpec.UUID) vmiStatus.Type = string(cliStatus.Type) diff --git a/controllers/contentlibrary/utils/controller_builder_test.go b/controllers/contentlibrary/utils/controller_builder_test.go index 521215e5a..c38ea07b1 100644 --- a/controllers/contentlibrary/utils/controller_builder_test.go +++ b/controllers/contentlibrary/utils/controller_builder_test.go @@ -204,6 +204,87 @@ var _ = Describe("Reconcile", }) }) + When("Library item has TKG labels", func() { + var cliLabels map[string]string + + BeforeEach(func() { + cliLabels = cliObj.GetLabels() + if cliLabels == nil { + cliLabels = map[string]string{} + } + }) + + When("Multiple Content Library feature is disabled", func() { + BeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.TKGMultipleCL = false + }) + + cliLabels[utils.TKGServiceTypeLabelKeyPrefix+"1"] = "" + cliLabels[utils.TKGServiceTypeLabelKeyPrefix+"2"] = "" + cliLabels[utils.MultipleCLServiceTypeLabelKeyPrefix+"1"] = "" + cliLabels[utils.MultipleCLServiceTypeLabelKeyPrefix+"2"] = "" + + cliObj.SetLabels(cliLabels) + }) + + It("should not copy the feature or non-feature labels to the vmi", func() { + _, err := reconciler.Reconcile(context.Background(), req) + Expect(err).ToNot(HaveOccurred()) + + vmiObj, _, _ := getVMI(ctx, req.Namespace, vmiName) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.TKGServiceTypeLabelKeyPrefix + "1")) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.TKGServiceTypeLabelKeyPrefix + "2")) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.MultipleCLServiceTypeLabelKeyPrefix + "1")) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.MultipleCLServiceTypeLabelKeyPrefix + "2")) + }) + }) + When("Multiple Content Library feature is enabled", func() { + BeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.TKGMultipleCL = true + }) + + cliLabels[utils.TKGServiceTypeLabelKeyPrefix+"1"] = "" + cliLabels[utils.TKGServiceTypeLabelKeyPrefix+"2"] = "" + cliLabels[utils.MultipleCLServiceTypeLabelKeyPrefix+"1"] = "" + cliLabels[utils.MultipleCLServiceTypeLabelKeyPrefix+"2"] = "" + + cliObj.SetLabels(cliLabels) + }) + + JustBeforeEach(func() { + vmiObj := newVMI( + ctx, + req.Namespace, + vmiName, + vmopv1.VirtualMachineImageStatus{ + ProviderContentVersion: "stale", + Firmware: "should-be-updated", + }) + vmiLabels := vmiObj.GetLabels() + if vmiLabels == nil { + vmiLabels = map[string]string{} + } + vmiLabels[utils.TKGServiceTypeLabelKeyPrefix+"3"] = "" + vmiLabels[utils.MultipleCLServiceTypeLabelKeyPrefix+"3"] = "" + vmiObj.SetLabels(vmiLabels) + Expect(ctx.Client.Update(ctx, vmiObj)).To(Succeed()) + }) + + It("should not copy the feature or non-feature labels to the vmi", func() { + _, err := reconciler.Reconcile(context.Background(), req) + Expect(err).ToNot(HaveOccurred()) + + vmiObj, _, _ := getVMI(ctx, req.Namespace, vmiName) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.TKGServiceTypeLabelKeyPrefix + "1")) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.TKGServiceTypeLabelKeyPrefix + "2")) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.MultipleCLServiceTypeLabelKeyPrefix + "1")) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.MultipleCLServiceTypeLabelKeyPrefix + "2")) + }) + }) + }) + When("Library item resource is not security compliant", func() { BeforeEach(func() { @@ -402,6 +483,111 @@ var _ = Describe("Reconcile", }) Context("ReconcileNormal", func() { + + When("Library item has TKG labels", func() { + var cliLabels map[string]string + + BeforeEach(func() { + cliLabels = cliObj.GetLabels() + if cliLabels == nil { + cliLabels = map[string]string{} + } + }) + + When("Multiple Content Library feature is disabled", func() { + BeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.TKGMultipleCL = false + }) + + cliLabels[utils.TKGServiceTypeLabelKeyPrefix+"1"] = "" + cliLabels[utils.TKGServiceTypeLabelKeyPrefix+"2"] = "" + cliLabels[utils.MultipleCLServiceTypeLabelKeyPrefix+"1"] = "" + cliLabels[utils.MultipleCLServiceTypeLabelKeyPrefix+"2"] = "" + + cliObj.SetLabels(cliLabels) + }) + + JustBeforeEach(func() { + vmiObj := newVMI( + ctx, + req.Namespace, + vmiName, + vmopv1.VirtualMachineImageStatus{ + ProviderContentVersion: "stale", + Firmware: "should-be-updated", + }) + vmiLabels := vmiObj.GetLabels() + if vmiLabels == nil { + vmiLabels = map[string]string{} + } + vmiLabels[utils.TKGServiceTypeLabelKeyPrefix+"3"] = "" + vmiLabels[utils.MultipleCLServiceTypeLabelKeyPrefix+"3"] = "" + vmiObj.SetLabels(vmiLabels) + Expect(ctx.Client.Update(ctx, vmiObj)).To(Succeed()) + }) + + It("should copy the non-feature labels to the vmi", func() { + _, err := reconciler.Reconcile(context.Background(), req) + Expect(err).ToNot(HaveOccurred()) + + vmiObj, _, _ := getVMI(ctx, req.Namespace, vmiName) + Expect(vmiObj.GetLabels()).To(HaveKey(utils.TKGServiceTypeLabelKeyPrefix + "1")) + Expect(vmiObj.GetLabels()).To(HaveKey(utils.TKGServiceTypeLabelKeyPrefix + "2")) + Expect(vmiObj.GetLabels()).To(HaveKey(utils.TKGServiceTypeLabelKeyPrefix + "3")) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.MultipleCLServiceTypeLabelKeyPrefix + "1")) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.MultipleCLServiceTypeLabelKeyPrefix + "2")) + Expect(vmiObj.GetLabels()).To(HaveKey(utils.MultipleCLServiceTypeLabelKeyPrefix + "3")) + }) + }) + When("Multiple Content Library feature is enabled", func() { + BeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.TKGMultipleCL = true + }) + + cliLabels[utils.TKGServiceTypeLabelKeyPrefix+"1"] = "" + cliLabels[utils.TKGServiceTypeLabelKeyPrefix+"2"] = "" + cliLabels[utils.MultipleCLServiceTypeLabelKeyPrefix+"1"] = "" + cliLabels[utils.MultipleCLServiceTypeLabelKeyPrefix+"2"] = "" + + cliObj.SetLabels(cliLabels) + }) + + JustBeforeEach(func() { + vmiObj := newVMI( + ctx, + req.Namespace, + vmiName, + vmopv1.VirtualMachineImageStatus{ + ProviderContentVersion: "stale", + Firmware: "should-be-updated", + }) + vmiLabels := vmiObj.GetLabels() + if vmiLabels == nil { + vmiLabels = map[string]string{} + } + vmiLabels[utils.TKGServiceTypeLabelKeyPrefix+"3"] = "" + vmiLabels[utils.MultipleCLServiceTypeLabelKeyPrefix+"3"] = "" + vmiObj.SetLabels(vmiLabels) + Expect(ctx.Client.Update(ctx, vmiObj)).To(Succeed()) + }) + + It("should copy the feature labels to the vmi", func() { + _, err := reconciler.Reconcile(context.Background(), req) + Expect(err).ToNot(HaveOccurred()) + + vmiObj, _, _ := getVMI(ctx, req.Namespace, vmiName) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.TKGServiceTypeLabelKeyPrefix + "1")) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.TKGServiceTypeLabelKeyPrefix + "2")) + Expect(vmiObj.GetLabels()).To(HaveKey(utils.TKGServiceTypeLabelKeyPrefix + "3")) + Expect(vmiObj.GetLabels()).To(HaveKey(utils.MultipleCLServiceTypeLabelKeyPrefix + "1")) + Expect(vmiObj.GetLabels()).To(HaveKey(utils.MultipleCLServiceTypeLabelKeyPrefix + "2")) + Expect(vmiObj.GetLabels()).ToNot(HaveKey(utils.MultipleCLServiceTypeLabelKeyPrefix + "3")) + }) + }) + }) + When("Library item resource is ready and security complaint", func() { JustBeforeEach(func() { // The dummy library item should meet these requirements. @@ -525,7 +711,7 @@ func getVMI( func newVMI( ctx *builder.UnitTestContextForController, namespace, name string, - status vmopv1.VirtualMachineImageStatus) { + status vmopv1.VirtualMachineImageStatus) client.Object { spec := vmopv1.VirtualMachineImageSpec{ ProviderRef: &common.LocalObjectRef{ @@ -543,6 +729,7 @@ func newVMI( Status: status, } ExpectWithOffset(1, ctx.Client.Create(ctx, &o)).To(Succeed()) + return &o } o := vmopv1.ClusterVirtualMachineImage{ @@ -553,6 +740,7 @@ func newVMI( Status: status, } ExpectWithOffset(1, ctx.Client.Create(ctx, &o)).To(Succeed()) + return &o } func assertVMImageFromCLItem(