fix: relax the case for last applied annotation is removed

.github/workflows/trivy.yml

@@ -3,11 +3,9 @@ on:
   push:
     branches:
       - main
-  pull_request:
-    branches:
-      - main
-      - release-*
-    paths-ignore: [docs/**, "**.md", "**.mdx", "**.png", "**.jpg"]
+  create:
+    # Publish semver tags as releases.
+    tags: [ 'v*.*.*' ]
 
 permissions:
   contents: read
@@ -32,7 +30,7 @@ jobs:
           # registry must be in lowercase
           # store the images under dev
           # TODO: need to cleanup dev images periodically 
-          echo "::set-output name=registry::$(echo "${{ env.REGISTRY }}/${{ github.repository }}/dev" | tr [:upper:] [:lower:])"
+          echo "::set-output name=registry::$(echo "${{ env.REGISTRY }}/${{ github.repository }}" | tr [:upper:] [:lower:])"
   scan-images:
     needs: export-registry
     env:

pkg/controllers/work/apply_controller_helper_test.go

@@ -51,8 +51,8 @@ func verifyAppliedConfigMap(cm *corev1.ConfigMap) *corev1.ConfigMap {
 	for key := range cm.Annotations {
 		Expect(appliedCM.Annotations[key]).Should(Equal(cm.Annotations[key]))
 	}
-	Expect(appliedCM.Annotations[manifestHashAnnotation]).ShouldNot(BeNil())
-	Expect(appliedCM.Annotations[lastAppliedConfigAnnotation]).ShouldNot(BeNil())
+	Expect(appliedCM.Annotations[manifestHashAnnotation]).ShouldNot(BeEmpty())
+	Expect(appliedCM.Annotations[lastAppliedConfigAnnotation]).ShouldNot(BeEmpty())
 
 	By("Check the config map data")
 	Expect(cmp.Diff(appliedCM.Data, cm.Data)).Should(BeEmpty())

pkg/controllers/work/apply_controller_integration_test.go

@@ -474,6 +474,72 @@ var _ = Describe("Work Controller", func() {
 			Expect(appliedCM.OwnerReferences[1].Name).Should(SatisfyAny(Equal(work.GetName()), Equal(work2.GetName())))
 		})
 
+		It("Check that the apply still works if the last applied annotation does not exist", func() {
+			ctx = context.Background()
+			cmName := "test-merge-without-lastapply"
+			cmNamespace := defaultNS
+			cm = &corev1.ConfigMap{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "ConfigMap",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      cmName,
+					Namespace: cmNamespace,
+					Labels: map[string]string{
+						"labelKey1": "value1",
+						"labelKey2": "value2",
+						"labelKey3": "value3",
+					},
+				},
+				Data: map[string]string{
+					"data1": "test1",
+				},
+			}
+
+			By("create the work")
+			work = createWorkWithManifest(workNamespace, cm)
+			err := k8sClient.Create(ctx, work)
+			Expect(err).Should(Succeed())
+
+			By("wait for the work to be applied")
+			waitForWorkToApply(work.GetName(), work.GetNamespace())
+
+			By("Check applied configMap")
+			appliedCM := verifyAppliedConfigMap(cm)
+
+			By("Delete the last applied annotation from the current resource")
+			delete(appliedCM.Annotations, lastAppliedConfigAnnotation)
+			Expect(k8sClient.Update(ctx, appliedCM)).Should(Succeed())
+
+			By("Get the last applied config map and verify it does not have the last applied annotation")
+			var modifiedCM corev1.ConfigMap
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cm.GetName(), Namespace: cm.GetNamespace()}, &modifiedCM)).Should(Succeed())
+			Expect(modifiedCM.Annotations[lastAppliedConfigAnnotation]).Should(BeEmpty())
+
+			By("Modify the manifest")
+			// modify one data
+			cm.Data["data1"] = "modifiedValue"
+			// add a conflict data
+			cm.Data["data2"] = "added by manifest"
+			// change label key3 with a new value
+			cm.Labels["labelKey3"] = "added-back-by-manifest"
+
+			By("update the work")
+			resultWork := waitForWorkToApply(work.GetName(), work.GetNamespace())
+			rawCM, err := json.Marshal(cm)
+			Expect(err).Should(Succeed())
+			resultWork.Spec.Workload.Manifests[0].Raw = rawCM
+			Expect(k8sClient.Update(ctx, resultWork)).Should(Succeed())
+
+			By("wait for the change of the work to be applied")
+			waitForWorkToApply(work.GetName(), work.GetNamespace())
+
+			By("Check applied configMap is modified even without the last applied annotation")
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cmName, Namespace: cmNamespace}, appliedCM)).Should(Succeed())
+			verifyAppliedConfigMap(cm)
+		})
+
 		It("Check that failed to apply manifest has the proper identification", func() {
 			broadcastName := "testfail"
 			namespace := defaultNS

pkg/controllers/work/patch_util.go

@@ -2,7 +2,6 @@ package controllers
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -12,6 +11,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/mergepatch"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -114,14 +114,18 @@ func setModifiedConfigurationAnnotation(obj runtime.Object) error {
 func getOriginalConfiguration(obj runtime.Object) ([]byte, error) {
 	annots, err := metadataAccessor.Annotations(obj)
 	if err != nil {
-		return nil, fmt.Errorf("cannot access metadata.annotations: %w", err)
+		klog.ErrorS(err, "cannot access metadata.annotations", "gvk", obj.GetObjectKind().GroupVersionKind())
+		return nil, err
 	}
+	// the threeWayMergePatch lib can handle the case that the original is empty
 	if annots == nil {
-		return nil, errors.New("object does not have lastAppliedConfigAnnotation")
+		klog.Warning("object does not have annotation", "obj", obj)
+		return nil, nil
 	}
 	original, ok := annots[lastAppliedConfigAnnotation]
 	if !ok {
-		return nil, errors.New("object does not have lastAppliedConfigAnnotation")
+		klog.Warning("object does not have lastAppliedConfigAnnotation", "obj", obj)
+		return nil, nil
 	}
 	return []byte(original), nil
 }