diff --git a/.github/workflows/trivy.yml b/.github/workflows/trivy.yml index ab9140f53..765d3dbfb 100644 --- a/.github/workflows/trivy.yml +++ b/.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: diff --git a/pkg/controllers/work/apply_controller_helper_test.go b/pkg/controllers/work/apply_controller_helper_test.go index e4aaf29e1..e0cd3f935 100644 --- a/pkg/controllers/work/apply_controller_helper_test.go +++ b/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()) diff --git a/pkg/controllers/work/apply_controller_integration_test.go b/pkg/controllers/work/apply_controller_integration_test.go index eb8c167c9..195d10f28 100644 --- a/pkg/controllers/work/apply_controller_integration_test.go +++ b/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 diff --git a/pkg/controllers/work/patch_util.go b/pkg/controllers/work/patch_util.go index 10d433b1d..6964d731a 100644 --- a/pkg/controllers/work/patch_util.go +++ b/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 func threeWayMergePatch 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 }