Skip to content

Commit

Permalink
Fix missing labels on CVMI resources
Browse files Browse the repository at this point in the history 
This patch fixes an issue where the labels on the CVMI resources
from the CCLI resources were missing. The labels were added via
vmware-tanzu#406 but were
accidentally removed via
vmware-tanzu#851.

When adding them back, there are now also tests to validate the
logic works as there were no tests in the original PR.
  • Loading branch information
@akutz
akutz committed Jan 28, 2025
1 parent 123d5bd commit 4be5413
Show file tree
Hide file tree
Showing 2 changed files with 228 additions and 1 deletion.
39 changes: 39 additions & 0 deletions controllers/contentlibrary/utils/controller_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ func (r *Reconciler) ReconcileNormal(
}()

if err := r.setUpVMIFromCLItem(
ctx,
cliObj,
*cliSpec,
*cliStatus,
Expand Down Expand Up @@ -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,
Expand All @@ -407,6 +409,43 @@ 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] = ""
}
}

if len(vmiLabels) > 0 {
vmiObj.SetLabels(vmiLabels)
}
}

vmiStatus.Name = cliStatus.Name
vmiStatus.ProviderItemID = string(cliSpec.UUID)
vmiStatus.Type = string(cliStatus.Type)
Expand Down
190 changes: 189 additions & 1 deletion controllers/contentlibrary/utils/controller_builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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{
Expand All @@ -543,6 +729,7 @@ func newVMI(
Status: status,
}
ExpectWithOffset(1, ctx.Client.Create(ctx, &o)).To(Succeed())
return &o
}

o := vmopv1.ClusterVirtualMachineImage{
Expand All @@ -553,6 +740,7 @@ func newVMI(
Status: status,
}
ExpectWithOffset(1, ctx.Client.Create(ctx, &o)).To(Succeed())
return &o
}

func assertVMImageFromCLItem(
Expand Down

0 comments on commit 4be5413

Please sign in to comment.