From 4c92f1b7dd8b4fad442a8e2b1dc2b4bbfc061b4d Mon Sep 17 00:00:00 2001 From: Chetan Banavikalmutt Date: Tue, 3 Dec 2024 10:06:50 +0530 Subject: [PATCH] feat: handle UID mismatch between incoming and existing resources Signed-off-by: Chetan Banavikalmutt --- agent/inbound.go | 59 +++- agent/inbound_test.go | 266 ++++++++++++++++++ internal/manager/application/application.go | 38 +++ .../manager/application/application_test.go | 63 ++++- internal/manager/appproject/appproject.go | 38 +++ .../manager/appproject/appproject_test.go | 55 ++++ internal/manager/manager.go | 6 + 7 files changed, 522 insertions(+), 3 deletions(-) diff --git a/agent/inbound.go b/agent/inbound.go index a59cca9..97db70e 100644 --- a/agent/inbound.go +++ b/agent/inbound.go @@ -55,13 +55,36 @@ func (a *Agent) processIncomingApplication(ev *event.Event) error { return err } + sourceUIDMatch, err := a.appManager.CompareSourceUID(a.context, incomingApp) + if err != nil { + return fmt.Errorf("failed to compare the source UID of app: %w", err) + } + switch ev.Type() { case event.Create: + if !sourceUIDMatch { + logCtx.Debug("An app already exists with a different source UID. Deleting the existing app") + if err := a.deleteApplication(incomingApp); err != nil { + return err + } + } + _, err = a.createApplication(incomingApp) if err != nil { logCtx.Errorf("Error creating application: %v", err) } case event.SpecUpdate: + if !sourceUIDMatch { + logCtx.Debug("Source UID mismatch between the incoming app and existing app. Deleting the existing app") + if err := a.deleteApplication(incomingApp); err != nil { + return err + } + + logCtx.Debug("Creating the incoming app after deleting the existing app") + _, err := a.createApplication(incomingApp) + return err + } + _, err = a.updateApplication(incomingApp) if err != nil { logCtx.Errorf("Error updating application: %v", err) @@ -87,13 +110,36 @@ func (a *Agent) processIncomingAppProject(ev *event.Event) error { return err } + sourceUIDMatch, err := a.projectManager.CompareSourceUID(a.context, incomingAppProject) + if err != nil { + return fmt.Errorf("failed to validate source UID of appProject: %w", err) + } + switch ev.Type() { case event.Create: + if !sourceUIDMatch { + logCtx.Debug("An appProject already exists with a different source UID. Deleting the existing appProject") + if err := a.deleteAppProject(incomingAppProject); err != nil { + return err + } + } + _, err = a.createAppProject(incomingAppProject) if err != nil { logCtx.Errorf("Error creating appproject: %v", err) } case event.SpecUpdate: + if !sourceUIDMatch { + logCtx.Debug("Source UID mismatch between the incoming and existing appProject. Deleting the existing appProject") + if err := a.deleteAppProject(incomingAppProject); err != nil { + return err + } + + logCtx.Debug("Creating the incoming appProject after deleting the existing appProject") + _, err := a.createAppProject(incomingAppProject) + return err + } + _, err = a.updateAppProject(incomingAppProject) if err != nil { logCtx.Errorf("Error updating appproject: %v", err) @@ -153,13 +199,17 @@ func (a *Agent) createApplication(incoming *v1alpha1.Application) (*v1alpha1.App } func (a *Agent) updateApplication(incoming *v1alpha1.Application) (*v1alpha1.Application, error) { - // incoming.ObjectMeta.SetNamespace(a.namespace) logCtx := log().WithFields(logrus.Fields{ "method": "UpdateApplication", "app": incoming.QualifiedName(), "resourceVersion": incoming.ResourceVersion, }) + + if !a.appManager.IsManaged(incoming.QualifiedName()) { + return nil, fmt.Errorf("application %s is not managed", incoming.QualifiedName()) + } + if a.appManager.IsChangeIgnored(incoming.QualifiedName(), incoming.ResourceVersion) { logCtx.Tracef("Discarding this event, because agent has seen this version %s already", incoming.ResourceVersion) return nil, event.NewEventDiscardedErr("the version %s has already been seen by this agent", incoming.ResourceVersion) @@ -262,6 +312,11 @@ func (a *Agent) updateAppProject(incoming *v1alpha1.AppProject) (*v1alpha1.AppPr "app": incoming.Name, "resourceVersion": incoming.ResourceVersion, }) + + if !a.projectManager.IsManaged(incoming.Name) { + return nil, fmt.Errorf("appProject %s is not managed", incoming.Name) + } + if a.appManager.IsChangeIgnored(incoming.Name, incoming.ResourceVersion) { logCtx.Tracef("Discarding this event, because agent has seen this version %s already", incoming.ResourceVersion) return nil, event.NewEventDiscardedErr("the version %s has already been seen by this agent", incoming.ResourceVersion) @@ -287,7 +342,7 @@ func (a *Agent) deleteAppProject(project *v1alpha1.AppProject) error { // means that we're out-of-sync from the control plane. // // TODO(jannfis): Handle this situation properly instead of throwing an error. - if !a.appManager.IsManaged(project.Name) { + if !a.projectManager.IsManaged(project.Name) { return fmt.Errorf("appProject %s is not managed", project.Name) } diff --git a/agent/inbound_test.go b/agent/inbound_test.go index 5974050..1c5387b 100644 --- a/agent/inbound_test.go +++ b/agent/inbound_test.go @@ -24,10 +24,12 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" backend_mocks "github.com/argoproj-labs/argocd-agent/internal/backend/mocks" + "github.com/argoproj-labs/argocd-agent/internal/event" "github.com/argoproj-labs/argocd-agent/internal/manager" "github.com/argoproj-labs/argocd-agent/internal/manager/application" "github.com/argoproj-labs/argocd-agent/internal/manager/appproject" "github.com/argoproj-labs/argocd-agent/pkg/types" + ktypes "k8s.io/apimachinery/pkg/types" ) func Test_CreateApplication(t *testing.T) { @@ -68,6 +70,270 @@ func Test_CreateApplication(t *testing.T) { } +func Test_ProcessIncomingAppWithUIDMismatch(t *testing.T) { + a := newAgent(t) + a.mode = types.AgentModeManaged + evs := event.NewEventSource("test") + var be *backend_mocks.Application + var getMock, createMock, deleteMock *mock.Call + + // incomingApp is the app sent by the principal + incomingApp := &v1alpha1.Application{ObjectMeta: v1.ObjectMeta{ + Name: "test", + Namespace: "argocd", + }} + + // oldApp is the app that is already present on the agent + oldApp := incomingApp.DeepCopy() + oldApp.Annotations = map[string]string{ + manager.SourceUIDAnnotation: "old_uid", + } + a.appManager.Manage(oldApp.QualifiedName()) + defer a.appManager.ClearManaged() + + incomingApp.UID = ktypes.UID("new_uid") + + // createdApp is the app that is finally created on the cluster. + createdApp := incomingApp.DeepCopy() + createdApp.UID = ktypes.UID("random_uid") + createdApp.Annotations = map[string]string{ + // the final version of the app should have the source UID from the incomingApp + manager.SourceUIDAnnotation: string(incomingApp.UID), + } + + configureManager := func(t *testing.T) { + t.Helper() + be = backend_mocks.NewApplication(t) + var err error + a.appManager, err = application.NewApplicationManager(be, "argocd", application.WithAllowUpsert(true)) + require.NoError(t, err) + require.NotNil(t, a) + + // Get is used to retrieve the oldApp and compare its UID annotation. + getMock = be.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(oldApp, nil) + + // Create is used to create the latest version of the app on the agent. + createMock = be.On("Create", mock.Anything, mock.Anything).Return(createdApp, nil) + + // Delete is used to delete the oldApp from the agent. + deleteMock = be.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(nil) + } + + unsetMocks := func(t *testing.T) { + t.Helper() + getMock.Unset() + createMock.Unset() + deleteMock.Unset() + } + + t.Run("Create: Old app with diff UID must be deleted before creating the incoming app", func(t *testing.T) { + configureManager(t) + defer unsetMocks(t) + a.appManager.Manage(oldApp.QualifiedName()) + defer a.appManager.ClearManaged() + + ev := event.New(evs.ApplicationEvent(event.Create, incomingApp), event.TargetApplication) + err := a.processIncomingApplication(ev) + require.Nil(t, err) + + // Check if the API calls were made in the same order: + // compare the UID, delete old app, and create a new app. + expectedCalls := []string{"Get", "Delete", "Create"} + gotCalls := []string{} + for _, call := range be.Calls { + gotCalls = append(gotCalls, call.Method) + } + require.Equal(t, expectedCalls, gotCalls) + + // Check if the new app has the updated source UID annotation. + appInterface := be.Calls[2].ReturnArguments[0] + latestApp, ok := appInterface.(*v1alpha1.Application) + require.True(t, ok) + require.Equal(t, string(incomingApp.UID), latestApp.Annotations[manager.SourceUIDAnnotation]) + }) + + t.Run("Update: Old app with diff UID must be deleted and a new app must be created", func(t *testing.T) { + configureManager(t) + defer unsetMocks(t) + a.appManager.Manage(oldApp.QualifiedName()) + defer a.appManager.ClearManaged() + + // Create an Update event for the new app + ev := event.New(evs.ApplicationEvent(event.SpecUpdate, incomingApp), event.TargetApplication) + err := a.processIncomingApplication(ev) + require.Nil(t, err) + + // Check if the API calls were made in the same order: + // compare the UID, delete old app, and create a new app. + expectedCalls := []string{"Get", "Delete", "Create"} + gotCalls := []string{} + for _, call := range be.Calls { + gotCalls = append(gotCalls, call.Method) + } + require.Equal(t, expectedCalls, gotCalls) + + // Check if the new app has the updated source UID annotation. + appInterface := be.Calls[2].ReturnArguments[0] + + latestApp, ok := appInterface.(*v1alpha1.Application) + require.True(t, ok) + require.Equal(t, string(incomingApp.UID), latestApp.Annotations[manager.SourceUIDAnnotation]) + }) + + t.Run("Delete: Old app with diff UID must be deleted", func(t *testing.T) { + configureManager(t) + defer unsetMocks(t) + a.appManager.Manage(oldApp.QualifiedName()) + defer a.appManager.ClearManaged() + + // Create a delete event for the new app + ev := event.New(evs.ApplicationEvent(event.Delete, incomingApp), event.TargetApplication) + err := a.processIncomingApplication(ev) + require.Nil(t, err) + + // Check if the API calls were made in the same order: + // compare the UID and delete old app + expectedCalls := []string{"Get", "Delete"} + gotCalls := []string{} + for _, call := range be.Calls { + gotCalls = append(gotCalls, call.Method) + } + require.Equal(t, expectedCalls, gotCalls) + require.False(t, a.appManager.IsManaged(incomingApp.QualifiedName())) + }) +} + +func Test_ProcessIncomingAppProjectWithUIDMismatch(t *testing.T) { + a := newAgent(t) + a.mode = types.AgentModeManaged + evs := event.NewEventSource("test") + var be *backend_mocks.AppProject + var getMock, createMock, deleteMock *mock.Call + + // incomingAppProject is the appProject sent by the principal + incomingAppProject := &v1alpha1.AppProject{ObjectMeta: v1.ObjectMeta{ + Name: "test", + Namespace: "argocd", + }} + + // oldAppProject is the appProject that is already present on the agent + oldAppProject := incomingAppProject.DeepCopy() + oldAppProject.Annotations = map[string]string{ + manager.SourceUIDAnnotation: "old_uid", + } + + incomingAppProject.UID = ktypes.UID("new_uid") + + // createdAppProject is the final version of appProject created on the cluster. + createdAppProject := incomingAppProject.DeepCopy() + createdAppProject.UID = ktypes.UID("random_uid") + createdAppProject.Annotations = map[string]string{ + manager.SourceUIDAnnotation: string(incomingAppProject.UID), + } + + configureManager := func(t *testing.T) { + t.Helper() + be = backend_mocks.NewAppProject(t) + var err error + a.projectManager, err = appproject.NewAppProjectManager(be, "argocd", appproject.WithAllowUpsert(true)) + require.NoError(t, err) + require.NotNil(t, a) + + // Get is used to retrieve the oldAppProject and compare its UID annotation. + getMock = be.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(oldAppProject, nil) + + // Create is used to create the latest version of the appProject on the agent. + createMock = be.On("Create", mock.Anything, mock.Anything).Return(createdAppProject, nil) + + // Delete is used to delete the oldAppProject from the agent. + deleteMock = be.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(nil) + } + + unsetMocks := func(t *testing.T) { + t.Helper() + getMock.Unset() + createMock.Unset() + deleteMock.Unset() + } + + t.Run("Create: Old appProject with diff UID must be deleted before creating the incoming appProject", func(t *testing.T) { + configureManager(t) + defer unsetMocks(t) + a.projectManager.Manage(oldAppProject.Name) + defer a.projectManager.ClearManaged() + ev := event.New(evs.AppProjectEvent(event.Create, incomingAppProject), event.TargetAppProject) + err := a.processIncomingAppProject(ev) + require.Nil(t, err) + + // Check if the API calls were made in the same order: + // compare the UID, delete old appProject, and create a new appProject. + expectedCalls := []string{"Get", "Delete", "Create"} + gotCalls := []string{} + for _, call := range be.Calls { + gotCalls = append(gotCalls, call.Method) + } + require.Equal(t, expectedCalls, gotCalls) + + // Check if the new app has the updated source UID annotation. + appInterface := be.Calls[2].ReturnArguments[0] + latestAppProject, ok := appInterface.(*v1alpha1.AppProject) + require.True(t, ok) + require.Equal(t, string(incomingAppProject.UID), latestAppProject.Annotations[manager.SourceUIDAnnotation]) + }) + + t.Run("Update: Old appProject with diff UID must be deleted and a new appProject must be created", func(t *testing.T) { + configureManager(t) + defer unsetMocks(t) + a.projectManager.Manage(oldAppProject.Name) + defer a.projectManager.ClearManaged() + + // Create an Update event for the new appProject + ev := event.New(evs.AppProjectEvent(event.SpecUpdate, incomingAppProject), event.TargetAppProject) + err := a.processIncomingAppProject(ev) + require.Nil(t, err) + + // Check if the API calls were made in the same order: + // compare the UID, delete old appProject, and create a new appProject. + expectedCalls := []string{"Get", "Delete", "Create"} + gotCalls := []string{} + for _, call := range be.Calls { + gotCalls = append(gotCalls, call.Method) + } + require.Equal(t, expectedCalls, gotCalls) + + // Check if the new appProject has the updated source UID annotation. + appInterface := be.Calls[2].ReturnArguments[0] + + latestAppProject, ok := appInterface.(*v1alpha1.AppProject) + require.True(t, ok) + require.Equal(t, string(incomingAppProject.UID), latestAppProject.Annotations[manager.SourceUIDAnnotation]) + }) + + t.Run("Delete: Old appProject with the same UID must be deleted", func(t *testing.T) { + configureManager(t) + defer unsetMocks(t) + a.projectManager.Manage(oldAppProject.Name) + defer a.projectManager.ClearManaged() + + // Create a delete event for the new appProject + ev := event.New(evs.AppProjectEvent(event.Delete, incomingAppProject), event.TargetAppProject) + err := a.processIncomingAppProject(ev) + require.Nil(t, err) + + // Check if the API calls were made in the same order: + // compare the UID and delete old appProject + expectedCalls := []string{"Get", "Delete"} + gotCalls := []string{} + for _, call := range be.Calls { + gotCalls = append(gotCalls, call.Method) + } + require.Equal(t, expectedCalls, gotCalls) + require.False(t, a.appManager.IsManaged(incomingAppProject.Name)) + }) +} + func Test_UpdateApplication(t *testing.T) { a := newAgent(t) be := backend_mocks.NewApplication(t) diff --git a/internal/manager/application/application.go b/internal/manager/application/application.go index bfdccb2..843c2f4 100644 --- a/internal/manager/application/application.go +++ b/internal/manager/application/application.go @@ -147,6 +147,11 @@ func (m *ApplicationManager) Create(ctx context.Context, app *v1alpha1.Applicati stampLastUpdated(app) } + if app.Annotations == nil { + app.Annotations = make(map[string]string) + } + app.Annotations[manager.SourceUIDAnnotation] = string(app.UID) + created, err := m.applicationBackend.Create(ctx, app) if err == nil { if err := m.Manage(created.QualifiedName()); err != nil { @@ -191,6 +196,12 @@ func (m *ApplicationManager) UpdateManagedApp(ctx context.Context, incoming *v1a } updated, err = m.update(ctx, m.allowUpsert, incoming, func(existing, incoming *v1alpha1.Application) { + if v, ok := existing.Annotations[manager.SourceUIDAnnotation]; ok { + if incoming.Annotations == nil { + incoming.Annotations = make(map[string]string) + } + incoming.Annotations[manager.SourceUIDAnnotation] = v + } existing.ObjectMeta.Annotations = incoming.ObjectMeta.Annotations existing.ObjectMeta.Labels = incoming.ObjectMeta.Labels existing.ObjectMeta.Finalizers = incoming.ObjectMeta.Finalizers @@ -205,6 +216,14 @@ func (m *ApplicationManager) UpdateManagedApp(ctx context.Context, incoming *v1a } incoming.Annotations["argocd.argoproj.io/refresh"] = v } + + if v, ok := existing.Annotations[manager.SourceUIDAnnotation]; ok { + if incoming.Annotations == nil { + incoming.Annotations = make(map[string]string) + } + incoming.Annotations[manager.SourceUIDAnnotation] = v + } + target := &v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ Annotations: incoming.Annotations, @@ -248,6 +267,25 @@ func (m *ApplicationManager) UpdateManagedApp(ctx context.Context, incoming *v1a return updated, err } +// CompareSourceUID checks for an existing app with the same name/namespace and compare its source UID with the incoming app. +func (m *ApplicationManager) CompareSourceUID(ctx context.Context, incoming *v1alpha1.Application) (bool, error) { + existing, err := m.applicationBackend.Get(ctx, incoming.Name, incoming.Namespace) + if err != nil { + if errors.IsNotFound(err) { + return true, nil + } + return false, err + } + + // If there is an existing app with the same name/namespace, compare its source UID with the incoming app. + sourceUID, exists := existing.Annotations[manager.SourceUIDAnnotation] + if !exists { + return false, fmt.Errorf("source UID Annotation is not found for app: %s", incoming.Name) + } + + return string(incoming.UID) == sourceUID, nil +} + // UpdateAutonomousApp updates the Application resource on the control plane side // when the agent is in autonomous mode. It will update changes to .spec and // .status fields along with syncing labels and annotations. diff --git a/internal/manager/application/application_test.go b/internal/manager/application/application_test.go index f1ee744..a6004ea 100644 --- a/internal/manager/application/application_test.go +++ b/internal/manager/application/application_test.go @@ -39,6 +39,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + ktypes "k8s.io/apimachinery/pkg/types" ) var appExistsError = errors.NewAlreadyExists(schema.GroupResource{Group: "argoproj.io", Resource: "application"}, "existing") @@ -114,6 +115,7 @@ func Test_ManagerCreate(t *testing.T) { ObjectMeta: v1.ObjectMeta{ Name: "test", Namespace: "default", + UID: ktypes.UID("sample"), }, } mockedBackend := appmock.NewApplication(t) @@ -123,6 +125,7 @@ func Test_ManagerCreate(t *testing.T) { rapp, err := m.Create(context.TODO(), app) assert.NoError(t, err) assert.Equal(t, "test", rapp.Name) + assert.Equal(t, string(app.UID), rapp.Annotations[manager.SourceUIDAnnotation]) }) } @@ -144,7 +147,8 @@ func Test_ManagerUpdateManaged(t *testing.T) { "foo": "bar", }, Annotations: map[string]string{ - "bar": "foo", + "bar": "foo", + manager.SourceUIDAnnotation: "random_uid", }, }, Spec: v1alpha1.ApplicationSpec{ @@ -178,6 +182,7 @@ func Test_ManagerUpdateManaged(t *testing.T) { Annotations: map[string]string{ "bar": "bar", "argocd.argoproj.io/refresh": "normal", + manager.SourceUIDAnnotation: "old_uid", }, }, Spec: v1alpha1.ApplicationSpec{ @@ -219,6 +224,10 @@ func Test_ManagerUpdateManaged(t *testing.T) { // Refresh annotation should only be overwritten by the controller, not // by the incoming app require.Contains(t, updated.Annotations, "argocd.argoproj.io/refresh") + + // Source UID annotation should not be overwritten by the incoming app + require.Equal(t, existing.Annotations[manager.SourceUIDAnnotation], updated.Annotations[manager.SourceUIDAnnotation]) + // Labels and annotations must be in sync with incoming require.Equal(t, incoming.Labels, updated.Labels) // Non-refresh annotations must be in sync with incoming @@ -626,6 +635,58 @@ func Test_stampLastUpdated(t *testing.T) { }) } +func Test_CompareSourceUIDForApp(t *testing.T) { + oldApp := &v1alpha1.Application{ + ObjectMeta: v1.ObjectMeta{ + Name: "test", + Namespace: "argocd", + Annotations: map[string]string{ + manager.SourceUIDAnnotation: "old_uid", + }, + }, + } + + mockedBackend := appmock.NewApplication(t) + mockedBackend.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(oldApp, nil) + m, err := NewApplicationManager(mockedBackend, "") + require.Nil(t, err) + ctx := context.Background() + + t.Run("should return true if the UID matches", func(t *testing.T) { + incoming := oldApp.DeepCopy() + incoming.UID = ktypes.UID("old_uid") + + uidMatch, err := m.CompareSourceUID(ctx, incoming) + require.Nil(t, err) + require.True(t, uidMatch) + }) + + t.Run("should return false if the UID doesn't match", func(t *testing.T) { + incoming := oldApp.DeepCopy() + incoming.UID = ktypes.UID("new_uid") + + uidMatch, err := m.CompareSourceUID(ctx, incoming) + require.Nil(t, err) + require.False(t, uidMatch) + }) + + t.Run("should return an error if there is no UID annotation", func(t *testing.T) { + oldApp.Annotations = map[string]string{} + mockedBackend.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(oldApp, nil) + m, err := NewApplicationManager(mockedBackend, "") + require.Nil(t, err) + ctx := context.Background() + + incoming := oldApp.DeepCopy() + incoming.UID = ktypes.UID("new_uid") + + uidMatch, err := m.CompareSourceUID(ctx, incoming) + require.NotNil(t, err) + require.EqualError(t, err, "source UID Annotation is not found for app: test") + require.False(t, uidMatch) + }) +} + func init() { logrus.SetLevel(logrus.TraceLevel) } diff --git a/internal/manager/appproject/appproject.go b/internal/manager/appproject/appproject.go index d23b680..97ab0cd 100644 --- a/internal/manager/appproject/appproject.go +++ b/internal/manager/appproject/appproject.go @@ -138,6 +138,11 @@ func (m *AppProjectManager) Create(ctx context.Context, project *v1alpha1.AppPro stampLastUpdated(project) } + if project.Annotations == nil { + project.Annotations = make(map[string]string) + } + project.Annotations[manager.SourceUIDAnnotation] = string(project.UID) + // If the AppProject has a source namespace, we only allow the agent to manage the AppProject // if the source namespace matches the agent's namespace if m.role == manager.ManagerRoleAgent { @@ -202,12 +207,26 @@ func (m *AppProjectManager) UpdateAppProject(ctx context.Context, incoming *v1al incoming.SetNamespace(m.namespace) updated, err = m.update(ctx, m.allowUpsert, incoming, func(existing, incoming *v1alpha1.AppProject) { + if v, ok := existing.Annotations[manager.SourceUIDAnnotation]; ok { + if incoming.Annotations == nil { + incoming.Annotations = make(map[string]string) + } + incoming.Annotations[manager.SourceUIDAnnotation] = v + } + existing.ObjectMeta.Annotations = incoming.ObjectMeta.Annotations existing.ObjectMeta.Labels = incoming.ObjectMeta.Labels existing.ObjectMeta.Finalizers = incoming.ObjectMeta.Finalizers existing.Spec = *incoming.Spec.DeepCopy() existing.Status = *incoming.Status.DeepCopy() }, func(existing, incoming *v1alpha1.AppProject) (jsondiff.Patch, error) { + if v, ok := existing.Annotations[manager.SourceUIDAnnotation]; ok { + if incoming.Annotations == nil { + incoming.Annotations = make(map[string]string) + } + incoming.Annotations[manager.SourceUIDAnnotation] = v + } + target := &v1alpha1.AppProject{ ObjectMeta: v1.ObjectMeta{ Annotations: incoming.Annotations, @@ -358,6 +377,25 @@ func (m *AppProjectManager) EnsureSynced(duration time.Duration) error { return m.appprojectBackend.EnsureSynced(duration) } +// CompareSourceUID checks for an existing appProject with the same name/namespace and compare its source UID with the incoming appProject. +func (m *AppProjectManager) CompareSourceUID(ctx context.Context, incoming *v1alpha1.AppProject) (bool, error) { + existing, err := m.appprojectBackend.Get(ctx, incoming.Name, incoming.Namespace) + if err != nil { + if errors.IsNotFound(err) { + return true, nil + } + return false, err + } + + // If there is an existing appProject with the same name/namespace, compare its source UID with the incoming appProject. + sourceUID, ok := existing.Annotations[manager.SourceUIDAnnotation] + if !ok { + return false, fmt.Errorf("source UID Annotation is not found for appProject: %s", incoming.Name) + } + + return string(incoming.UID) == sourceUID, nil +} + func log() *logrus.Entry { return logrus.WithField("component", "AppManager") } diff --git a/internal/manager/appproject/appproject_test.go b/internal/manager/appproject/appproject_test.go index c01c023..160ea2a 100644 --- a/internal/manager/appproject/appproject_test.go +++ b/internal/manager/appproject/appproject_test.go @@ -27,6 +27,7 @@ import ( "github.com/sirupsen/logrus" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ktypes "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" @@ -256,6 +257,7 @@ func TestCreateAppProject(t *testing.T) { ObjectMeta: v1.ObjectMeta{ Name: "test", Namespace: "default", + UID: "new_uid", }, } mockedBackend := appmock.NewAppProject(t) @@ -265,6 +267,59 @@ func TestCreateAppProject(t *testing.T) { rapp, err := m.Create(context.TODO(), app) assert.NoError(t, err) assert.Equal(t, "test", rapp.Name) + assert.Equal(t, string(app.UID), rapp.Annotations[manager.SourceUIDAnnotation]) + }) +} + +func Test_CompareSourceUIDForAppProject(t *testing.T) { + oldAppProject := &v1alpha1.AppProject{ + ObjectMeta: v1.ObjectMeta{ + Name: "test", + Namespace: "argocd", + Annotations: map[string]string{ + manager.SourceUIDAnnotation: "old_uid", + }, + }, + } + + mockedBackend := appmock.NewAppProject(t) + mockedBackend.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(oldAppProject, nil) + m, err := NewAppProjectManager(mockedBackend, "") + require.Nil(t, err) + ctx := context.Background() + + t.Run("should return true if the UID matches", func(t *testing.T) { + incoming := oldAppProject.DeepCopy() + incoming.UID = ktypes.UID("old_uid") + + uidMatch, err := m.CompareSourceUID(ctx, incoming) + require.Nil(t, err) + require.True(t, uidMatch) + }) + + t.Run("should return false if the UID doesn't match", func(t *testing.T) { + incoming := oldAppProject.DeepCopy() + incoming.UID = ktypes.UID("new_uid") + + uidMatch, err := m.CompareSourceUID(ctx, incoming) + require.Nil(t, err) + require.False(t, uidMatch) + }) + + t.Run("should return an error if there is no UID annotation", func(t *testing.T) { + oldAppProject.Annotations = map[string]string{} + mockedBackend.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(oldAppProject, nil) + m, err := NewAppProjectManager(mockedBackend, "") + require.Nil(t, err) + ctx := context.Background() + + incoming := oldAppProject.DeepCopy() + incoming.UID = ktypes.UID("new_uid") + + uidMatch, err := m.CompareSourceUID(ctx, incoming) + require.NotNil(t, err) + require.EqualError(t, err, "source UID Annotation is not found for appProject: test") + require.False(t, uidMatch) }) } diff --git a/internal/manager/manager.go b/internal/manager/manager.go index cca2953..8272053 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -29,6 +29,12 @@ const ( ManagerModeManaged ) +const ( + // SourceUIDAnnotation is an annotation that represents the UID of the source resource. + // It is added to the resources managed on the target. + SourceUIDAnnotation = "argocd.argoproj.io/source-uid" +) + type Manager interface { SetRole(role ManagerRole) SetMode(role ManagerRole)