Skip to content

Commit 2b021b5

Browse files
chetan-rnschetan-rns
committed
compare the agent name with the server URL param
Signed-off-by: Chetan Banavikalmutt <chetanrns1997@gmail.com>
1 parent 0271dd7 commit 2b021b5

File tree

2 files changed

+69
-146
lines changed

2 files changed

+69
-146
lines changed

‎principal/callbacks.go

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package principal
1616

1717
import (
18+
"net/url"
1819
"strings"
1920

2021
"github.com/argoproj-labs/argocd-agent/internal/event"
@@ -315,26 +316,51 @@ func (s *Server) mapAppProjectToAgents(appProject v1alpha1.AppProject) map[strin
315316

316317
// doesAgentMatchWithProject checks if the agent name matches the given AppProject.
317318
// We match the agent to an AppProject if:
318-
// 1. The destination name is not empty and matches the agent name OR
319-
// 2. The destination server is a wildcard AND
319+
// 1. The agent name matches any one of the destination names OR
320+
// 2. The agent name is empty but the agent name is present in the server URL parameter AND
320321
// 3. The agent name is not denied by any of the destination names
321322
// Ref: https://github.com/argoproj/argo-cd/blob/master/pkg/apis/application/v1alpha1/app_project_types.go#L477
322323
func doesAgentMatchWithProject(agentName string, appProject v1alpha1.AppProject) bool {
323324
destinationMatched := false
324325

326+
logCtx := log().WithFields(logrus.Fields{
327+
"agent_name": agentName,
328+
"app_project": appProject.Name,
329+
})
330+
325331
for _, dst := range appProject.Spec.Destinations {
326-
if isDenyPattern(dst.Name) {
327-
// Return immediately if the agent name is denied by any of the destination names
328-
if glob.Match(dst.Name[1:], agentName) {
329-
return false
332+
// Return immediately if the agent name is denied by any of the destination names
333+
if dst.Name != "" && isDenyPattern(dst.Name) && glob.Match(dst.Name[1:], agentName) {
334+
return false
335+
}
336+
337+
// Some AppProjects (e.g. default) may not always have a name so we need to check the server URL
338+
if dst.Name == "" && dst.Server != "" {
339+
if dst.Server == "*" {
340+
destinationMatched = true
341+
continue
342+
}
343+
344+
// Server URL will be the resource proxy URL https://<rp-hostname>:<port>?agentName=<agent-name>
345+
server, err := url.Parse(dst.Server)
346+
if err != nil {
347+
logCtx.WithError(err).Errorf("Invalid server URL: %s", dst.Server)
348+
continue
349+
}
350+
351+
serverAgentName := server.Query().Get("agentName")
352+
if serverAgentName == "" {
353+
continue
354+
}
355+
356+
if glob.Match(serverAgentName, agentName) {
357+
destinationMatched = true
330358
}
331-
continue
332359
}
333360

334-
dstNameMatched := dst.Name != "" && glob.Match(dst.Name, agentName)
335-
dstServerMatched := dst.Server == "*"
336-
if dstNameMatched || dstServerMatched {
337-
destinationMatched = true // Continue matching to look for deny patterns
361+
// Match the agent name to the destination name and continue looking for deny patterns
362+
if dst.Name != "" && glob.Match(dst.Name, agentName) {
363+
destinationMatched = true
338364
}
339365
}
340366

@@ -400,12 +426,22 @@ func AgentSpecificAppProject(appProject v1alpha1.AppProject, agent string) v1alp
400426
// Only keep the destinations that are relevant to the given agent
401427
filteredDst := []v1alpha1.ApplicationDestination{}
402428
for _, dst := range appProject.Spec.Destinations {
403-
dstNameMatched := dst.Name != "" && glob.Match(dst.Name, agent)
404-
dstServerMatched := dst.Server == "*"
405-
if dstNameMatched || dstServerMatched {
429+
nameMatches := dst.Name != "" && glob.Match(dst.Name, agent)
430+
serverMatches := false
431+
432+
// Handle server-only destinations (like default project)
433+
if dst.Name == "" && dst.Server != "" {
434+
if dst.Server == "*" {
435+
serverMatches = true
436+
} else if server, err := url.Parse(dst.Server); err == nil {
437+
serverAgentName := server.Query().Get("agentName")
438+
serverMatches = serverAgentName != "" && glob.Match(serverAgentName, agent)
439+
}
440+
}
441+
442+
if nameMatches || serverMatches {
406443
dst.Name = "in-cluster"
407444
dst.Server = "https://kubernetes.default.svc"
408-
409445
filteredDst = append(filteredDst, dst)
410446
}
411447
}

‎principal/callbacks_test.go

Lines changed: 18 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -539,16 +539,16 @@ func TestAgentSpecificAppProject(t *testing.T) {
539539
},
540540
},
541541
{
542-
name: "no agent name but server is wildcard",
542+
name: "agent name matches wildcard destination",
543543
appProject: v1alpha1.AppProject{
544544
ObjectMeta: metav1.ObjectMeta{
545-
Name: "empty-project",
545+
Name: "wildcard-project",
546546
Namespace: "argocd",
547547
},
548548
Spec: v1alpha1.AppProjectSpec{
549549
Destinations: []v1alpha1.ApplicationDestination{
550550
{
551-
Server: "*",
551+
Name: "*",
552552
Namespace: "default",
553553
},
554554
},
@@ -559,7 +559,7 @@ func TestAgentSpecificAppProject(t *testing.T) {
559559
agent: "test-agent",
560560
want: v1alpha1.AppProject{
561561
ObjectMeta: metav1.ObjectMeta{
562-
Name: "empty-project",
562+
Name: "wildcard-project",
563563
Namespace: "argocd",
564564
},
565565
Spec: v1alpha1.AppProjectSpec{
@@ -677,19 +677,7 @@ func TestDoesAgentMatchWithProject(t *testing.T) {
677677
},
678678
want: true,
679679
},
680-
{
681-
name: "wildcard server with a different name",
682-
agentName: "any-agent",
683-
appProject: v1alpha1.AppProject{
684-
Spec: v1alpha1.AppProjectSpec{
685-
Destinations: []v1alpha1.ApplicationDestination{
686-
{Name: "different-name", Server: "*", Namespace: "default"},
687-
},
688-
SourceNamespaces: []string{"*"},
689-
},
690-
},
691-
want: true, // Should match wildcard server
692-
},
680+
693681
{
694682
name: "agent matches destination name with wildcard",
695683
agentName: "agent-staging",
@@ -744,20 +732,7 @@ func TestDoesAgentMatchWithProject(t *testing.T) {
744732
},
745733
want: true,
746734
},
747-
{
748-
name: "order independence - deny first, then allow",
749-
agentName: "prod-agent",
750-
appProject: v1alpha1.AppProject{
751-
Spec: v1alpha1.AppProjectSpec{
752-
Destinations: []v1alpha1.ApplicationDestination{
753-
{Name: "!prod-*", Namespace: "default"}, // Deny prod first
754-
{Name: "*", Namespace: "default"}, // Allow all
755-
},
756-
SourceNamespaces: []string{"*"},
757-
},
758-
},
759-
want: false, // Same result regardless of order
760-
},
735+
761736
{
762737
name: "agent matches destination but not source namespace",
763738
agentName: "agent-test",
@@ -771,61 +746,6 @@ func TestDoesAgentMatchWithProject(t *testing.T) {
771746
},
772747
want: false,
773748
},
774-
{
775-
name: "agent matches neither destination name nor server",
776-
agentName: "agent-nomatch",
777-
appProject: v1alpha1.AppProject{
778-
Spec: v1alpha1.AppProjectSpec{
779-
Destinations: []v1alpha1.ApplicationDestination{
780-
{Name: "different-name", Server: "different-server", Namespace: "default"},
781-
},
782-
SourceNamespaces: []string{"agent-nomatch"},
783-
},
784-
},
785-
want: false,
786-
},
787-
{
788-
name: "empty destination name with non-wildcard server",
789-
agentName: "any-agent",
790-
appProject: v1alpha1.AppProject{
791-
Spec: v1alpha1.AppProjectSpec{
792-
Destinations: []v1alpha1.ApplicationDestination{
793-
{Name: "", Server: "https://kubernetes.default.svc", Namespace: "default"},
794-
},
795-
SourceNamespaces: []string{"any-agent"},
796-
},
797-
},
798-
want: false,
799-
},
800-
{
801-
name: "multiple deny patterns - first match wins",
802-
agentName: "prod-sensitive-agent",
803-
appProject: v1alpha1.AppProject{
804-
Spec: v1alpha1.AppProjectSpec{
805-
Destinations: []v1alpha1.ApplicationDestination{
806-
{Name: "!prod-*", Namespace: "default"}, // This matches first
807-
{Name: "!*-sensitive-*", Namespace: "default"}, // This would also match
808-
{Name: "*", Namespace: "default"}, // Allow all others
809-
},
810-
SourceNamespaces: []string{"*"},
811-
},
812-
},
813-
want: false,
814-
},
815-
{
816-
name: "wildcard with deny pattern override",
817-
agentName: "admin-user",
818-
appProject: v1alpha1.AppProject{
819-
Spec: v1alpha1.AppProjectSpec{
820-
Destinations: []v1alpha1.ApplicationDestination{
821-
{Name: "", Server: "*", Namespace: "default"}, // Wildcard allow
822-
{Name: "!admin-*", Namespace: "default"}, // Deny admin
823-
},
824-
SourceNamespaces: []string{"*"},
825-
},
826-
},
827-
want: false, // Deny overrides wildcard
828-
},
829749
{
830750
name: "only deny patterns - no positive match",
831751
agentName: "dev-agent",
@@ -840,76 +760,43 @@ func TestDoesAgentMatchWithProject(t *testing.T) {
840760
want: false, // No positive pattern to match
841761
},
842762
{
843-
name: "deny pattern doesn't match - should continue to positive patterns",
844-
agentName: "dev-agent",
763+
name: "server URL with exact agent name match",
764+
agentName: "prod-cluster-1",
845765
appProject: v1alpha1.AppProject{
846766
Spec: v1alpha1.AppProjectSpec{
847767
Destinations: []v1alpha1.ApplicationDestination{
848-
{Name: "!prod-*", Namespace: "default"}, // Deny doesn't match
849-
{Name: "dev-*", Namespace: "default"}, // Positive matches
768+
{Name: "", Server: "https://resource-proxy.example.com:8080?agentName=prod-cluster-1", Namespace: "default"},
850769
},
851770
SourceNamespaces: []string{"*"},
852771
},
853772
},
854-
want: true, // Should match positive pattern after deny doesn't match
773+
want: true, // Agent name matches extracted query parameter
855774
},
856775
{
857-
name: "multiple deny patterns none match - positive pattern wins",
858-
agentName: "test-agent",
776+
name: "server URL with agent name pattern",
777+
agentName: "prod-cluster-2",
859778
appProject: v1alpha1.AppProject{
860779
Spec: v1alpha1.AppProjectSpec{
861780
Destinations: []v1alpha1.ApplicationDestination{
862-
{Name: "!prod-*", Namespace: "default"}, // Doesn't match
863-
{Name: "!staging-*", Namespace: "default"}, // Doesn't match
864-
{Name: "*", Namespace: "default"}, // Matches
781+
{Name: "", Server: "https://resource-proxy.example.com:8080?agentName=prod-cluster-*", Namespace: "default"},
865782
},
866783
SourceNamespaces: []string{"*"},
867784
},
868785
},
869-
want: true,
786+
want: true, // Agent name matches extracted pattern
870787
},
871788
{
872-
name: "mixed destinations with deny pattern",
873-
agentName: "dev-agent",
789+
name: "server URL with non-matching agent name",
790+
agentName: "dev-cluster-1",
874791
appProject: v1alpha1.AppProject{
875792
Spec: v1alpha1.AppProjectSpec{
876793
Destinations: []v1alpha1.ApplicationDestination{
877-
{Name: "!admin-*", Namespace: "default"}, // Name deny (doesn't match)
878-
{Name: "", Server: "*", Namespace: "default"}, // Server wildcard
879-
{Name: "dev-*", Namespace: "default"}, // Name allow (matches)
794+
{Name: "", Server: "https://resource-proxy.example.com:8080?agentName=prod-cluster-1", Namespace: "default"},
880795
},
881796
SourceNamespaces: []string{"*"},
882797
},
883798
},
884-
want: true, // Should pass deny check and match positive patterns
885-
},
886-
887-
{
888-
name: "deny pattern in first destination, positive in second",
889-
agentName: "prod-cluster",
890-
appProject: v1alpha1.AppProject{
891-
Spec: v1alpha1.AppProjectSpec{
892-
Destinations: []v1alpha1.ApplicationDestination{
893-
{Name: "!prod-cluster", Namespace: "default"}, // Exact deny match
894-
{Name: "*", Namespace: "default"}, // Would allow all
895-
},
896-
SourceNamespaces: []string{"*"},
897-
},
898-
},
899-
want: false, // Deny pattern matches exactly, should reject immediately
900-
},
901-
{
902-
name: "deny pattern with different namespace requirements",
903-
agentName: "restricted-agent",
904-
appProject: v1alpha1.AppProject{
905-
Spec: v1alpha1.AppProjectSpec{
906-
Destinations: []v1alpha1.ApplicationDestination{
907-
{Name: "!restricted-*", Namespace: "default"}, // Deny pattern matches
908-
},
909-
SourceNamespaces: []string{"restricted-agent"}, // Namespace would match
910-
},
911-
},
912-
want: false, // Should be denied even though namespace matches
799+
want: false, // Agent name doesn't match extracted query parameter
913800
},
914801
}
915802

0 commit comments

Comments
 (0)