Skip to content

Commit 50dbd11

Browse files
chetan-rnschetan-rns
authored
fix: sync default appproject updates to managed agents (#529)
Signed-off-by: Chetan Banavikalmutt <[email protected]>
1 parent c99c56d commit 50dbd11

File tree

8 files changed

+769
-273
lines changed

8 files changed

+769
-273
lines changed

‎agent/inbound_test.go‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,152 @@ func Test_ProcessIncomingAppProjectWithUIDMismatch(t *testing.T) {
545545
require.Equal(t, expectedCalls, gotCalls)
546546
require.False(t, a.appManager.IsManaged(incomingAppProject.Name))
547547
})
548+
549+
// Missing Source UID Annotation scenarios - when existing AppProject lacks the source UID annotation
550+
t.Run("Create: Existing AppProject without source UID annotation should be deleted and recreated", func(t *testing.T) {
551+
// Setup separate backend for this test to avoid mock conflicts
552+
beMissing := backend_mocks.NewAppProject(t)
553+
var err error
554+
a.projectManager, err = appproject.NewAppProjectManager(beMissing, "argocd", appproject.WithAllowUpsert(true))
555+
require.NoError(t, err)
556+
557+
// Setup for missing source UID scenario
558+
existingAppProject := &v1alpha1.AppProject{
559+
ObjectMeta: v1.ObjectMeta{
560+
Name: "test",
561+
Namespace: "argocd",
562+
UID: ktypes.UID("existing_uid"),
563+
},
564+
}
565+
566+
// Configure separate backend
567+
getMockMissing := beMissing.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(existingAppProject, nil)
568+
createMockMissing := beMissing.On("Create", mock.Anything, mock.Anything).Return(createdAppProject, nil)
569+
deleteMockMissing := beMissing.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
570+
571+
defer func() {
572+
getMockMissing.Unset()
573+
createMockMissing.Unset()
574+
deleteMockMissing.Unset()
575+
}()
576+
577+
a.projectManager.Manage(existingAppProject.Name)
578+
defer a.projectManager.ClearManaged()
579+
580+
ev := event.New(evs.AppProjectEvent(event.Create, incomingAppProject), event.TargetAppProject)
581+
err = a.processIncomingAppProject(ev)
582+
583+
// The process should succeed after deleting the existing AppProject and creating the new one
584+
require.NoError(t, err)
585+
586+
// Verify the sequence: Get (to compare UID), Delete (existing), Create (new)
587+
expectedCalls := []string{"Get", "Delete", "Create"}
588+
gotCalls := []string{}
589+
for _, call := range beMissing.Calls {
590+
gotCalls = append(gotCalls, call.Method)
591+
}
592+
require.Equal(t, expectedCalls, gotCalls)
593+
594+
// Verify the created AppProject has the correct source UID annotation
595+
appInterface := beMissing.Calls[2].ReturnArguments[0]
596+
latestAppProject, ok := appInterface.(*v1alpha1.AppProject)
597+
require.True(t, ok)
598+
require.Equal(t, string(incomingAppProject.UID), latestAppProject.Annotations[manager.SourceUIDAnnotation])
599+
})
600+
601+
t.Run("Update: Existing AppProject without source UID annotation should be deleted and recreated", func(t *testing.T) {
602+
// Setup separate backend for this test to avoid mock conflicts
603+
beMissing := backend_mocks.NewAppProject(t)
604+
var err error
605+
a.projectManager, err = appproject.NewAppProjectManager(beMissing, "argocd", appproject.WithAllowUpsert(true))
606+
require.NoError(t, err)
607+
608+
// Setup for missing source UID scenario
609+
existingAppProject := &v1alpha1.AppProject{
610+
ObjectMeta: v1.ObjectMeta{
611+
Name: "test",
612+
Namespace: "argocd",
613+
UID: ktypes.UID("existing_uid"),
614+
},
615+
}
616+
617+
// Configure separate backend
618+
getMockMissing := beMissing.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(existingAppProject, nil)
619+
createMockMissing := beMissing.On("Create", mock.Anything, mock.Anything).Return(createdAppProject, nil)
620+
deleteMockMissing := beMissing.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
621+
622+
defer func() {
623+
getMockMissing.Unset()
624+
createMockMissing.Unset()
625+
deleteMockMissing.Unset()
626+
}()
627+
628+
a.projectManager.Manage(existingAppProject.Name)
629+
defer a.projectManager.ClearManaged()
630+
631+
ev := event.New(evs.AppProjectEvent(event.SpecUpdate, incomingAppProject), event.TargetAppProject)
632+
err = a.processIncomingAppProject(ev)
633+
634+
// The process should succeed after deleting the existing AppProject and creating the new one
635+
require.NoError(t, err)
636+
637+
// Verify the sequence: Get (to compare UID), Delete (existing), Create (new)
638+
expectedCalls := []string{"Get", "Delete", "Create"}
639+
gotCalls := []string{}
640+
for _, call := range beMissing.Calls {
641+
gotCalls = append(gotCalls, call.Method)
642+
}
643+
require.Equal(t, expectedCalls, gotCalls)
644+
645+
// Verify the created AppProject has the correct source UID annotation
646+
appInterface := beMissing.Calls[2].ReturnArguments[0]
647+
latestAppProject, ok := appInterface.(*v1alpha1.AppProject)
648+
require.True(t, ok)
649+
require.Equal(t, string(incomingAppProject.UID), latestAppProject.Annotations[manager.SourceUIDAnnotation])
650+
})
651+
652+
t.Run("Delete: Existing AppProject without source UID annotation should be deleted", func(t *testing.T) {
653+
// Setup separate backend for this test to avoid mock conflicts
654+
beMissing := backend_mocks.NewAppProject(t)
655+
var err error
656+
a.projectManager, err = appproject.NewAppProjectManager(beMissing, "argocd", appproject.WithAllowUpsert(true))
657+
require.NoError(t, err)
658+
659+
// Setup for missing source UID scenario
660+
existingAppProject := &v1alpha1.AppProject{
661+
ObjectMeta: v1.ObjectMeta{
662+
Name: "test",
663+
Namespace: "argocd",
664+
UID: ktypes.UID("existing_uid"),
665+
},
666+
}
667+
668+
// Configure separate backend
669+
getMockMissing := beMissing.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(existingAppProject, nil)
670+
deleteMockMissing := beMissing.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
671+
672+
defer func() {
673+
getMockMissing.Unset()
674+
deleteMockMissing.Unset()
675+
}()
676+
677+
a.projectManager.Manage(existingAppProject.Name)
678+
defer a.projectManager.ClearManaged()
679+
680+
ev := event.New(evs.AppProjectEvent(event.Delete, incomingAppProject), event.TargetAppProject)
681+
err = a.processIncomingAppProject(ev)
682+
683+
// Delete events should succeed after deleting the existing AppProject
684+
require.NoError(t, err)
685+
686+
// Verify the sequence: Get (to compare UID), Delete (existing)
687+
expectedCalls := []string{"Get", "Delete"}
688+
gotCalls := []string{}
689+
for _, call := range beMissing.Calls {
690+
gotCalls = append(gotCalls, call.Method)
691+
}
692+
require.Equal(t, expectedCalls, gotCalls)
693+
})
548694
}
549695

550696
func Test_UpdateApplication(t *testing.T) {

‎internal/manager/appproject/appproject.go‎

Lines changed: 86 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
"context"
1919
"encoding/json"
2020
"fmt"
21+
"net/url"
22+
"strings"
2123
"time"
2224

2325
"github.com/argoproj-labs/argocd-agent/internal/backend"
@@ -355,23 +357,100 @@ func (m *AppProjectManager) CompareSourceUID(ctx context.Context, incoming *v1al
355357
// If there is an existing appProject with the same name/namespace, compare its source UID with the incoming appProject.
356358
sourceUID, ok := existing.Annotations[manager.SourceUIDAnnotation]
357359
if !ok {
358-
return true, false, fmt.Errorf("source UID Annotation is not found for appProject: %s", incoming.Name)
360+
return true, false, nil
359361
}
360362

361363
return true, string(incoming.UID) == sourceUID, nil
362364
}
363365

364-
// DoesAgentMatchWithProject checks if the agent name matches the AppProject's destinations and source namespaces
366+
// DoesAgentMatchWithProject checks if the agent name matches the given AppProject.
367+
// We match the agent to an AppProject if:
368+
// 1. The agent name matches any one of the destination names OR
369+
// 2. The agent name is empty but the agent name is present in the server URL parameter AND
370+
// 3. The agent name is not denied by any of the destination names
371+
// Ref: https://github.com/argoproj/argo-cd/blob/master/pkg/apis/application/v1alpha1/app_project_types.go#L477
365372
func DoesAgentMatchWithProject(agentName string, appProject v1alpha1.AppProject) bool {
366-
matched := false
373+
destinationMatched := false
374+
367375
for _, dst := range appProject.Spec.Destinations {
368-
if glob.Match(dst.Name, agentName) {
369-
matched = true
370-
break
376+
// Return immediately if the agent name is denied by any of the destination names
377+
if dst.Name != "" && isDenyPattern(dst.Name) && glob.Match(dst.Name[1:], agentName) {
378+
return false
379+
}
380+
381+
// Some AppProjects (e.g. default) may not always have a name so we need to check the server URL
382+
if dst.Name == "" && dst.Server != "" {
383+
if dst.Server == "*" {
384+
destinationMatched = true
385+
continue
386+
}
387+
388+
// Server URL will be the resource proxy URL https://<rp-hostname>:<port>?agentName=<agent-name>
389+
server, err := url.Parse(dst.Server)
390+
if err != nil {
391+
continue
392+
}
393+
394+
serverAgentName := server.Query().Get("agentName")
395+
if serverAgentName == "" {
396+
continue
397+
}
398+
399+
if glob.Match(serverAgentName, agentName) {
400+
destinationMatched = true
401+
}
402+
}
403+
404+
// Match the agent name to the destination name and continue looking for deny patterns
405+
if dst.Name != "" && glob.Match(dst.Name, agentName) {
406+
destinationMatched = true
371407
}
372408
}
373409

374-
return matched && glob.MatchStringInList(appProject.Spec.SourceNamespaces, agentName, glob.REGEXP)
410+
// Must match both destination and source namespace requirements
411+
return destinationMatched &&
412+
glob.MatchStringInList(appProject.Spec.SourceNamespaces, agentName, glob.REGEXP)
413+
}
414+
415+
func isDenyPattern(pattern string) bool {
416+
return strings.HasPrefix(pattern, "!")
417+
}
418+
419+
// AgentSpecificAppProject returns an agent specific version of the given AppProject
420+
// We don't have to check for deny patterns because we only construct the agent specific AppProject
421+
// if the agent name matches the AppProject's destinations.
422+
func AgentSpecificAppProject(appProject v1alpha1.AppProject, agent string) v1alpha1.AppProject {
423+
// Only keep the destinations that are relevant to the given agent
424+
filteredDst := []v1alpha1.ApplicationDestination{}
425+
for _, dst := range appProject.Spec.Destinations {
426+
nameMatches := dst.Name != "" && glob.Match(dst.Name, agent)
427+
serverMatches := false
428+
429+
// Handle server-only destinations (like default project)
430+
if dst.Name == "" && dst.Server != "" {
431+
if dst.Server == "*" {
432+
serverMatches = true
433+
} else if server, err := url.Parse(dst.Server); err == nil {
434+
serverAgentName := server.Query().Get("agentName")
435+
serverMatches = serverAgentName != "" && glob.Match(serverAgentName, agent)
436+
}
437+
}
438+
439+
if nameMatches || serverMatches {
440+
dst.Name = "in-cluster"
441+
dst.Server = "https://kubernetes.default.svc"
442+
filteredDst = append(filteredDst, dst)
443+
}
444+
}
445+
appProject.Spec.Destinations = filteredDst
446+
447+
// Only allow Applications to be managed from the agent namespace
448+
appProject.Spec.SourceNamespaces = []string{agent}
449+
450+
// Remove the roles since they are not relevant on the workload cluster
451+
appProject.Spec.Roles = []v1alpha1.ProjectRole{}
452+
453+
return appProject
375454
}
376455

377456
func log() *logrus.Entry {

0 commit comments

Comments
 (0)