From 5c464c0812a3b422031725b51820b818bd6fd17a Mon Sep 17 00:00:00 2001 From: Blake Pettersson Date: Fri, 18 Apr 2025 15:05:49 +0200 Subject: [PATCH 1/3] fix: do not normalize resource tracking on live crds Fixes #22721. CRDs should not have resource annotations set on them. This normalization causes erroneus diffs when used with ServerSideDiff. Signed-off-by: Blake Pettersson --- util/argo/resource_tracking.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/util/argo/resource_tracking.go b/util/argo/resource_tracking.go index b3bcf8e7045fe..446bdccd2b751 100644 --- a/util/argo/resource_tracking.go +++ b/util/argo/resource_tracking.go @@ -6,6 +6,7 @@ import ( "regexp" "strings" + kubeutil "github.com/argoproj/gitops-engine/pkg/utils/kube" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/argoproj/argo-cd/v3/common" @@ -232,23 +233,25 @@ func (rt *resourceTracking) Normalize(config, live *unstructured.Unstructured, l return nil } - annotation, err := kube.GetAppInstanceAnnotation(config, common.AnnotationKeyAppInstance) - if err != nil { - return err - } - err = kube.SetAppInstanceAnnotation(live, common.AnnotationKeyAppInstance, annotation) - if err != nil { - return err - } + if !kubeutil.IsCRD(live) { + annotation, err := kube.GetAppInstanceAnnotation(config, common.AnnotationKeyAppInstance) + if err != nil { + return err + } + err = kube.SetAppInstanceAnnotation(live, common.AnnotationKeyAppInstance, annotation) + if err != nil { + return err + } - label, err = kube.GetAppInstanceLabel(config, labelKey) - if err != nil { - return fmt.Errorf("failed to get app instance label: %w", err) - } - if label == "" { - err = kube.RemoveLabel(live, labelKey) + label, err = kube.GetAppInstanceLabel(config, labelKey) if err != nil { - return fmt.Errorf("failed to remove app instance label: %w", err) + return fmt.Errorf("failed to get app instance label: %w", err) + } + if label == "" { + err = kube.RemoveLabel(live, labelKey) + if err != nil { + return fmt.Errorf("failed to remove app instance label: %w", err) + } } } From 0a9cda43d3c67775e1185df3c8348ec8914a37d2 Mon Sep 17 00:00:00 2001 From: Blake Pettersson Date: Fri, 18 Apr 2025 22:35:50 +0200 Subject: [PATCH 2/3] chore: add test and minor refactor Signed-off-by: Blake Pettersson --- util/argo/resource_tracking.go | 37 +++++++++++++----------- util/argo/resource_tracking_test.go | 44 +++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 17 deletions(-) diff --git a/util/argo/resource_tracking.go b/util/argo/resource_tracking.go index 446bdccd2b751..ac6c5dab9607c 100644 --- a/util/argo/resource_tracking.go +++ b/util/argo/resource_tracking.go @@ -233,25 +233,28 @@ func (rt *resourceTracking) Normalize(config, live *unstructured.Unstructured, l return nil } - if !kubeutil.IsCRD(live) { - annotation, err := kube.GetAppInstanceAnnotation(config, common.AnnotationKeyAppInstance) - if err != nil { - return err - } - err = kube.SetAppInstanceAnnotation(live, common.AnnotationKeyAppInstance, annotation) - if err != nil { - return err - } + if kubeutil.IsCRD(live) { + // CRDs don't get tracking annotations. + return nil + } - label, err = kube.GetAppInstanceLabel(config, labelKey) + annotation, err := kube.GetAppInstanceAnnotation(config, common.AnnotationKeyAppInstance) + if err != nil { + return err + } + err = kube.SetAppInstanceAnnotation(live, common.AnnotationKeyAppInstance, annotation) + if err != nil { + return err + } + + label, err = kube.GetAppInstanceLabel(config, labelKey) + if err != nil { + return fmt.Errorf("failed to get app instance label: %w", err) + } + if label == "" { + err = kube.RemoveLabel(live, labelKey) if err != nil { - return fmt.Errorf("failed to get app instance label: %w", err) - } - if label == "" { - err = kube.RemoveLabel(live, labelKey) - if err != nil { - return fmt.Errorf("failed to remove app instance label: %w", err) - } + return fmt.Errorf("failed to remove app instance label: %w", err) } } diff --git a/util/argo/resource_tracking_test.go b/util/argo/resource_tracking_test.go index 289734293aad5..b24c119388c0a 100644 --- a/util/argo/resource_tracking_test.go +++ b/util/argo/resource_tracking_test.go @@ -204,6 +204,50 @@ func TestResourceIdNormalizer_Normalize(t *testing.T) { assert.False(t, hasOldLabel) } +func TestResourceIdNormalizer_NormalizeCRD(t *testing.T) { + rt := NewResourceTracking() + + // live object is a CRD resource + liveObj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": map[string]interface{}{ + "name": "crontabs.stable.example.com", + "labels": map[string]interface{}{ + common.LabelKeyAppInstance: "my-app", + }, + }, + "spec": map[string]interface{}{ + "group": "stable.example.com", + "scope": "Namespaced", + }, + }, + } + + // config object is a CRD resource + configObj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": map[string]interface{}{ + "name": "crontabs.stable.example.com", + "labels": map[string]interface{}{ + common.LabelKeyAppInstance: "my-app", + }, + }, + "spec": map[string]interface{}{ + "group": "stable.example.com", + "scope": "Namespaced", + }, + }, + } + + require.NoError(t, rt.Normalize(configObj, liveObj, common.LabelKeyAppInstance, string(TrackingMethodAnnotation))) + // the normalization should not apply any changes to the live object + require.NotContains(t, liveObj.GetAnnotations(), common.AnnotationKeyAppInstance) +} + func TestResourceIdNormalizer_Normalize_ConfigHasOldLabel(t *testing.T) { rt := NewResourceTracking() From 77d60d441870ca956982d845fa95e18b5901ba5d Mon Sep 17 00:00:00 2001 From: Blake Pettersson Date: Fri, 18 Apr 2025 23:07:27 +0200 Subject: [PATCH 3/3] chore: lint Signed-off-by: Blake Pettersson --- util/argo/resource_tracking_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/util/argo/resource_tracking_test.go b/util/argo/resource_tracking_test.go index b24c119388c0a..5c1f813bee889 100644 --- a/util/argo/resource_tracking_test.go +++ b/util/argo/resource_tracking_test.go @@ -209,16 +209,16 @@ func TestResourceIdNormalizer_NormalizeCRD(t *testing.T) { // live object is a CRD resource liveObj := &unstructured.Unstructured{ - Object: map[string]interface{}{ + Object: map[string]any{ "apiVersion": "apiextensions.k8s.io/v1", "kind": "CustomResourceDefinition", - "metadata": map[string]interface{}{ + "metadata": map[string]any{ "name": "crontabs.stable.example.com", - "labels": map[string]interface{}{ + "labels": map[string]any{ common.LabelKeyAppInstance: "my-app", }, }, - "spec": map[string]interface{}{ + "spec": map[string]any{ "group": "stable.example.com", "scope": "Namespaced", }, @@ -227,16 +227,16 @@ func TestResourceIdNormalizer_NormalizeCRD(t *testing.T) { // config object is a CRD resource configObj := &unstructured.Unstructured{ - Object: map[string]interface{}{ + Object: map[string]any{ "apiVersion": "apiextensions.k8s.io/v1", "kind": "CustomResourceDefinition", - "metadata": map[string]interface{}{ + "metadata": map[string]any{ "name": "crontabs.stable.example.com", - "labels": map[string]interface{}{ + "labels": map[string]any{ common.LabelKeyAppInstance: "my-app", }, }, - "spec": map[string]interface{}{ + "spec": map[string]any{ "group": "stable.example.com", "scope": "Namespaced", },