diff --git a/controller/state.go b/controller/state.go index 9b9a3ab225cd8..929b2f5b8128b 100644 --- a/controller/state.go +++ b/controller/state.go @@ -494,6 +494,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *ap } gvk := obj.GroupVersionKind() + isSelfReferencedObj := m.isSelfReferencedObj(liveObj, appLabelKey, trackingMethod) + resState := v1alpha1.ResourceStatus{ Namespace: obj.GetNamespace(), Name: obj.GetName(), @@ -501,7 +503,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *ap Version: gvk.Version, Group: gvk.Group, Hook: hookutil.IsHook(obj), - RequiresPruning: targetObj == nil && liveObj != nil, + RequiresPruning: targetObj == nil && liveObj != nil && isSelfReferencedObj, } var diffResult diff.DiffResult @@ -510,8 +512,11 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *ap } else { diffResult = diff.DiffResult{Modified: false, NormalizedLive: []byte("{}"), PredictedLive: []byte("{}")} } - if resState.Hook || ignore.Ignore(obj) || (targetObj != nil && hookutil.Skip(targetObj)) { - // For resource hooks or skipped resources, don't store sync status, and do not affect overall sync status + if resState.Hook || ignore.Ignore(obj) || (targetObj != nil && hookutil.Skip(targetObj)) || !isSelfReferencedObj { + // For resource hooks, skipped resources or objects that may have + // been created by another controller with annotations copied from + // the source object, don't store sync status, and do not affect + // overall sync status } else if diffResult.Modified || targetObj == nil || liveObj == nil { // Set resource state to OutOfSync since one of the following is true: // * target and live resource are different @@ -667,3 +672,33 @@ func NewAppStateManager( resourceTracking: resourceTracking, } } + +// isSelfReferencedObj returns whether the given obj is managed by the application +// according to the values in the tracking annotation. It returns true when all +// of the properties in the annotation (name, namespace, group and kind) match +// the properties of the inspected object, or if the tracking method used does +// not provide the required properties for matching. +func (m *appStateManager) isSelfReferencedObj(obj *unstructured.Unstructured, appLabelKey string, trackingMethod v1alpha1.TrackingMethod) bool { + if obj == nil { + return true + } + + // If tracking method doesn't contain required metadata for this check, + // we are not able to determine and just assume the object to be managed. + if trackingMethod == argo.TrackingMethodLabel { + return true + } + + // In order for us to assume obj to be managed by this application, the + // values from the annotation have to match the properties from the live + // object. + appInstance := m.resourceTracking.GetAppInstance(obj, appLabelKey, trackingMethod) + if appInstance != nil { + return obj.GetNamespace() == appInstance.Namespace && + obj.GetName() == appInstance.Name && + obj.GetObjectKind().GroupVersionKind().Group == appInstance.Group && + obj.GetObjectKind().GroupVersionKind().Kind == appInstance.Kind + } + + return true +} diff --git a/controller/state_test.go b/controller/state_test.go index 0dba8b0c3a159..f5428e7b81da4 100644 --- a/controller/state_test.go +++ b/controller/state_test.go @@ -13,6 +13,7 @@ import ( . "github.com/argoproj/gitops-engine/pkg/utils/testing" "github.com/stretchr/testify/assert" v1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -21,6 +22,7 @@ import ( argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v2/reposerver/apiclient" "github.com/argoproj/argo-cd/v2/test" + "github.com/argoproj/argo-cd/v2/util/argo" ) // TestCompareAppStateEmpty tests comparison when both git and live have no objects @@ -770,3 +772,128 @@ func TestComparisonResult_GetSyncStatus(t *testing.T) { assert.Equal(t, status, res.GetSyncStatus()) } + +func TestIsLiveResourceManaged(t *testing.T) { + managedObj := kube.MustToUnstructured(&corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "configmap1", + Namespace: "default", + Annotations: map[string]string{ + common.AnnotationKeyAppInstance: "guestbook:/ConfigMap:default/configmap1", + }, + }, + }) + managedObjWithLabel := kube.MustToUnstructured(&corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "configmap1", + Namespace: "default", + Labels: map[string]string{ + common.LabelKeyAppInstance: "guestbook", + }, + }, + }) + unmanagedObjWrongName := kube.MustToUnstructured(&corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "configmap2", + Namespace: "default", + Annotations: map[string]string{ + common.AnnotationKeyAppInstance: "guestbook:/ConfigMap:default/configmap1", + }, + }, + }) + unmanagedObjWrongKind := kube.MustToUnstructured(&corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "configmap2", + Namespace: "default", + Annotations: map[string]string{ + common.AnnotationKeyAppInstance: "guestbook:/Service:default/configmap2", + }, + }, + }) + unmanagedObjWrongGroup := kube.MustToUnstructured(&corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "configmap2", + Namespace: "default", + Annotations: map[string]string{ + common.AnnotationKeyAppInstance: "guestbook:apps/ConfigMap:default/configmap2", + }, + }, + }) + unmanagedObjWrongNamespace := kube.MustToUnstructured(&corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "configmap2", + Namespace: "default", + Annotations: map[string]string{ + common.AnnotationKeyAppInstance: "guestbook:/ConfigMap:fakens/configmap2", + }, + }, + }) + ctrl := newFakeController(&fakeData{ + apps: []runtime.Object{app, &defaultProj}, + manifestResponse: &apiclient.ManifestResponse{ + Manifests: []string{}, + Namespace: test.FakeDestNamespace, + Server: test.FakeClusterURL, + Revision: "abc123", + }, + managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{ + kube.GetResourceKey(managedObj): managedObj, + kube.GetResourceKey(unmanagedObjWrongName): unmanagedObjWrongName, + kube.GetResourceKey(unmanagedObjWrongKind): unmanagedObjWrongKind, + kube.GetResourceKey(unmanagedObjWrongGroup): unmanagedObjWrongGroup, + kube.GetResourceKey(unmanagedObjWrongNamespace): unmanagedObjWrongNamespace, + }, + }) + + manager := ctrl.appStateManager.(*appStateManager) + + // Managed resource w/ annotations + assert.True(t, manager.isSelfReferencedObj(managedObj, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel)) + assert.True(t, manager.isSelfReferencedObj(managedObj, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotation)) + + // Managed resource w/ label + assert.True(t, manager.isSelfReferencedObj(managedObjWithLabel, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel)) + + // Wrong resource name + assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongName, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel)) + assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongName, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotation)) + + // Wrong resource group + assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongGroup, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel)) + assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongGroup, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotation)) + + // Wrong resource kind + assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongKind, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel)) + assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongKind, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotation)) + + // Wrong resource namespace + assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongNamespace, common.AnnotationKeyAppInstance, argo.TrackingMethodLabel)) + assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongNamespace, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotationAndLabel)) + + // Nil resource + assert.True(t, manager.isSelfReferencedObj(nil, common.AnnotationKeyAppInstance, argo.TrackingMethodAnnotation)) +} diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go index 815f486573619..b511d168cea34 100644 --- a/test/e2e/app_management_test.go +++ b/test/e2e/app_management_test.go @@ -2250,3 +2250,55 @@ func TestSwitchTrackingLabel(t *testing.T) { Expect(SyncStatusIs(SyncStatusCodeSynced)). Expect(HealthIs(health.HealthStatusHealthy)) } + +func TestAnnotationTrackingExtraResources(t *testing.T) { + ctx := Given(t) + + SetTrackingMethod(string(argo.TrackingMethodAnnotation)) + ctx. + Path("deployment"). + When(). + CreateApp(). + Sync(). + Refresh(RefreshTypeNormal). + Then(). + Expect(OperationPhaseIs(OperationSucceeded)). + Expect(SyncStatusIs(SyncStatusCodeSynced)). + Expect(HealthIs(health.HealthStatusHealthy)). + When(). + And(func() { + // Add a resource with an annotation that is not referencing the + // resource. + FailOnErr(KubeClientset.CoreV1().ConfigMaps(DeploymentNamespace()).Create(context.Background(), &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "extra-configmap", + Annotations: map[string]string{ + common.AnnotationKeyAppInstance: fmt.Sprintf("%s:apps/Deployment:%s/guestbook-cm", Name(), DeploymentNamespace()), + }, + }, + }, metav1.CreateOptions{})) + }). + Refresh(RefreshTypeNormal). + Then(). + Expect(OperationPhaseIs(OperationSucceeded)). + Expect(SyncStatusIs(SyncStatusCodeSynced)). + Expect(HealthIs(health.HealthStatusHealthy)). + When(). + And(func() { + // Add a resource with an annotation that is self-referencing the + // resource. + FailOnErr(KubeClientset.CoreV1().ConfigMaps(DeploymentNamespace()).Create(context.Background(), &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-configmap", + Annotations: map[string]string{ + common.AnnotationKeyAppInstance: fmt.Sprintf("%s:/ConfigMap:%s/other-configmap", Name(), DeploymentNamespace()), + }, + }, + }, metav1.CreateOptions{})) + }). + Refresh(RefreshTypeNormal). + Then(). + Expect(OperationPhaseIs(OperationSucceeded)). + Expect(SyncStatusIs(SyncStatusCodeOutOfSync)). + Expect(HealthIs(health.HealthStatusHealthy)) +} diff --git a/util/argo/resource_tracking.go b/util/argo/resource_tracking.go index fce9a45102e27..1741bf413d05d 100644 --- a/util/argo/resource_tracking.go +++ b/util/argo/resource_tracking.go @@ -29,6 +29,7 @@ var LabelMaxLength = 63 // ResourceTracking defines methods which allow setup and retrieve tracking information to resource type ResourceTracking interface { GetAppName(un *unstructured.Unstructured, key string, trackingMethod v1alpha1.TrackingMethod) string + GetAppInstance(un *unstructured.Unstructured, key string, trackingMethod v1alpha1.TrackingMethod) *AppInstanceValue SetAppInstance(un *unstructured.Unstructured, key, val, namespace string, trackingMethod v1alpha1.TrackingMethod) error BuildAppInstanceValue(value AppInstanceValue) string ParseAppInstanceValue(value string) (*AppInstanceValue, error) @@ -64,15 +65,23 @@ func IsOldTrackingMethod(trackingMethod string) bool { return trackingMethod == "" || trackingMethod == string(TrackingMethodLabel) } +func (rt *resourceTracking) getAppInstanceValue(un *unstructured.Unstructured, key string, trackingMethod v1alpha1.TrackingMethod) *AppInstanceValue { + appInstanceAnnotation := argokube.GetAppInstanceAnnotation(un, common.AnnotationKeyAppInstance) + value, err := rt.ParseAppInstanceValue(appInstanceAnnotation) + if err != nil { + return nil + } + return value +} + // GetAppName retrieve application name base on tracking method func (rt *resourceTracking) GetAppName(un *unstructured.Unstructured, key string, trackingMethod v1alpha1.TrackingMethod) string { retrieveAppInstanceValue := func() string { - appInstanceAnnotation := argokube.GetAppInstanceAnnotation(un, common.AnnotationKeyAppInstance) - value, err := rt.ParseAppInstanceValue(appInstanceAnnotation) - if err != nil { - return "" + value := rt.getAppInstanceValue(un, key, trackingMethod) + if value != nil { + return value.ApplicationName } - return value.ApplicationName + return "" } switch trackingMethod { case TrackingMethodLabel: @@ -86,6 +95,18 @@ func (rt *resourceTracking) GetAppName(un *unstructured.Unstructured, key string } } +// GetAppInstance returns the representation of the app instance annotation. +// If the tracking method does not support metadata, or the annotation could +// not be parsed, it returns nil. +func (rt *resourceTracking) GetAppInstance(un *unstructured.Unstructured, key string, trackingMethod v1alpha1.TrackingMethod) *AppInstanceValue { + switch trackingMethod { + case TrackingMethodAnnotation, TrackingMethodAnnotationAndLabel: + return rt.getAppInstanceValue(un, key, trackingMethod) + default: + return nil + } +} + // SetAppInstance set label/annotation base on tracking method func (rt *resourceTracking) SetAppInstance(un *unstructured.Unstructured, key, val, namespace string, trackingMethod v1alpha1.TrackingMethod) error { setAppInstanceAnnotation := func() error {