fix(server): appset list uses argocd's namespace instead of all (argo…

…proj#15429) (argoproj#15432) * fix(server): appset list uses argocd's namespace instead of all Signed-off-by: Jorge Turrado <[email protected]> * use lister to scope the observed namespaces based on which namespaces monitors for apps Signed-off-by: Jorge Turrado <[email protected]> * apply feedback Signed-off-by: Jorge Turrado <[email protected]> * add missing change 🤦 Signed-off-by: Jorge Turrado <[email protected]> * update generated manifests Signed-off-by: Jorge Turrado <[email protected]> --------- Signed-off-by: Jorge Turrado <[email protected]>
AmadeusITGroup · Jun 16, 2024 · a270441 · a270441
1 parent 5dac646
commit a270441
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 31 deletions.
diff --git a/manifests/cluster-rbac/server/argocd-server-clusterrole.yaml b/manifests/cluster-rbac/server/argocd-server-clusterrole.yaml
@@ -32,6 +32,7 @@ rules:
   - "argoproj.io"
   resources:
   - "applications"
+  - "applicationsets"
   verbs:
   - get
   - list

diff --git a/manifests/ha/install.yaml b/manifests/ha/install.yaml
@@ -20609,6 +20609,7 @@ rules:
   - argoproj.io
   resources:
   - applications
+  - applicationsets
   verbs:
   - get
   - list

diff --git a/manifests/install.yaml b/manifests/install.yaml
@@ -20568,6 +20568,7 @@ rules:
   - argoproj.io
   resources:
   - applications
+  - applicationsets
   verbs:
   - get
   - list

diff --git a/server/applicationset/applicationset.go b/server/applicationset/applicationset.go
@@ -25,7 +25,6 @@ import (
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	appclientset "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned"
 	applisters "github.com/argoproj/argo-cd/v2/pkg/client/listers/application/v1alpha1"
-	servercache "github.com/argoproj/argo-cd/v2/server/cache"
 	"github.com/argoproj/argo-cd/v2/server/rbacpolicy"
 	"github.com/argoproj/argo-cd/v2/util/argo"
 	"github.com/argoproj/argo-cd/v2/util/collections"
@@ -40,11 +39,9 @@ type Server struct {
 	ns                string
 	db                db.ArgoDB
 	enf               *rbac.Enforcer
-	cache             *servercache.Cache
 	appclientset      appclientset.Interface
-	appLister         applisters.ApplicationLister
 	appsetInformer    cache.SharedIndexInformer
-	appsetLister      applisters.ApplicationSetNamespaceLister
+	appsetLister      applisters.ApplicationSetLister
 	projLister        applisters.AppProjectNamespaceLister
 	auditLogger       *argo.AuditLogger
 	settings          *settings.SettingsManager
@@ -57,11 +54,9 @@ func NewServer(
 	db db.ArgoDB,
 	kubeclientset kubernetes.Interface,
 	enf *rbac.Enforcer,
-	cache *servercache.Cache,
 	appclientset appclientset.Interface,
-	appLister applisters.ApplicationLister,
 	appsetInformer cache.SharedIndexInformer,
-	appsetLister applisters.ApplicationSetNamespaceLister,
+	appsetLister applisters.ApplicationSetLister,
 	projLister applisters.AppProjectNamespaceLister,
 	settings *settings.SettingsManager,
 	namespace string,
@@ -70,11 +65,9 @@ func NewServer(
 ) applicationset.ApplicationSetServiceServer {
 	s := &Server{
 		ns:                namespace,
-		cache:             cache,
 		db:                db,
 		enf:               enf,
 		appclientset:      appclientset,
-		appLister:         appLister,
 		appsetInformer:    appsetInformer,
 		appsetLister:      appsetLister,
 		projLister:        projLister,
@@ -94,7 +87,7 @@ func (s *Server) Get(ctx context.Context, q *applicationset.ApplicationSetGetQue
 		return nil, security.NamespaceNotPermittedError(namespace)
 	}
 
-	a, err := s.appclientset.ArgoprojV1alpha1().ApplicationSets(namespace).Get(ctx, q.Name, metav1.GetOptions{})
+	a, err := s.appsetLister.ApplicationSets(namespace).Get(q.Name)
 
 	if err != nil {
 		return nil, fmt.Errorf("error getting ApplicationSet: %w", err)
@@ -113,14 +106,19 @@ func (s *Server) List(ctx context.Context, q *applicationset.ApplicationSetListQ
 		return nil, fmt.Errorf("error parsing the selector: %w", err)
 	}
 
-	appIf := s.appclientset.ArgoprojV1alpha1().ApplicationSets(q.AppsetNamespace)
-	appsetList, err := appIf.List(ctx, metav1.ListOptions{LabelSelector: selector.String()})
+	var appsets []*v1alpha1.ApplicationSet
+	if q.AppsetNamespace == "" {
+		appsets, err = s.appsetLister.List(selector)
+	} else {
+		appsets, err = s.appsetLister.ApplicationSets(q.AppsetNamespace).List(selector)
+	}
+
 	if err != nil {
 		return nil, fmt.Errorf("error listing ApplicationSets with selectors: %w", err)
 	}
 
 	newItems := make([]v1alpha1.ApplicationSet, 0)
-	for _, a := range appsetList.Items {
+	for _, a := range appsets {
 
 		// Skip any application that is neither in the conrol plane's namespace
 		// nor in the list of enabled namespaces.
@@ -129,7 +127,7 @@ func (s *Server) List(ctx context.Context, q *applicationset.ApplicationSetListQ
 		}
 
 		if s.enf.Enforce(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionGet, a.RBACName(s.ns)) {
-			newItems = append(newItems, a)
+			newItems = append(newItems, *a)
 		}
 	}
 
@@ -140,7 +138,7 @@ func (s *Server) List(ctx context.Context, q *applicationset.ApplicationSetListQ
 		return newItems[i].Name < newItems[j].Name
 	})
 
-	appsetList = &v1alpha1.ApplicationSetList{
+	appsetList := &v1alpha1.ApplicationSetList{
 		ListMeta: metav1.ListMeta{
 			ResourceVersion: s.appsetInformer.LastSyncResourceVersion(),
 		},
@@ -336,7 +334,7 @@ func (s *Server) waitSync(appset *v1alpha1.ApplicationSet) {
 		return
 	}
 	for {
-		if currAppset, err := s.appsetLister.Get(appset.Name); err == nil {
+		if currAppset, err := s.appsetLister.ApplicationSets(appset.Namespace).Get(appset.Name); err == nil {
 			currVersion, err := strconv.Atoi(currAppset.ResourceVersion)
 			if err == nil && currVersion >= minVersion {
 				return

diff --git a/server/applicationset/applicationset_test.go b/server/applicationset/applicationset_test.go
@@ -50,10 +50,21 @@ func newTestAppSetServer(objects ...runtime.Object) *Server {
 		_ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
 		enf.SetDefaultRole("role:admin")
 	}
-	return newTestAppSetServerWithEnforcerConfigure(f, objects...)
+	scopedNamespaces := ""
+	return newTestAppSetServerWithEnforcerConfigure(f, scopedNamespaces, objects...)
 }
 
-func newTestAppSetServerWithEnforcerConfigure(f func(*rbac.Enforcer), objects ...runtime.Object) *Server {
+// return an ApplicationServiceServer which returns fake data
+func newTestNamespacedAppSetServer(objects ...runtime.Object) *Server {
+	f := func(enf *rbac.Enforcer) {
+		_ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
+		enf.SetDefaultRole("role:admin")
+	}
+	scopedNamespaces := "argocd"
+	return newTestAppSetServerWithEnforcerConfigure(f, scopedNamespaces, objects...)
+}
+
+func newTestAppSetServerWithEnforcerConfigure(f func(*rbac.Enforcer), namespace string, objects ...runtime.Object) *Server {
 	kubeclientset := fake.NewSimpleClientset(&v1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: testNamespace,
@@ -97,7 +108,7 @@ func newTestAppSetServerWithEnforcerConfigure(f func(*rbac.Enforcer), objects ..
 	objects = append(objects, defaultProj, myProj)
 
 	fakeAppsClientset := apps.NewSimpleClientset(objects...)
-	factory := appinformer.NewSharedInformerFactoryWithOptions(fakeAppsClientset, 0, appinformer.WithNamespace(""), appinformer.WithTweakListOptions(func(options *metav1.ListOptions) {}))
+	factory := appinformer.NewSharedInformerFactoryWithOptions(fakeAppsClientset, 0, appinformer.WithNamespace(namespace), appinformer.WithTweakListOptions(func(options *metav1.ListOptions) {}))
 	fakeProjLister := factory.Argoproj().V1alpha1().AppProjects().Lister().AppProjects(testNamespace)
 
 	enforcer := rbac.NewEnforcer(kubeclientset, testNamespace, common.ArgoCDRBACConfigMapName, nil)
@@ -114,6 +125,12 @@ func newTestAppSetServerWithEnforcerConfigure(f func(*rbac.Enforcer), objects ..
 	if !k8scache.WaitForCacheSync(ctx.Done(), appInformer.HasSynced) {
 		panic("Timed out waiting for caches to sync")
 	}
+	// populate the appset informer with the fake objects
+	appsetInformer := factory.Argoproj().V1alpha1().ApplicationSets().Informer()
+	go appsetInformer.Run(ctx.Done())
+	if !k8scache.WaitForCacheSync(ctx.Done(), appsetInformer.HasSynced) {
+		panic("Timed out waiting for caches to sync")
+	}
 
 	projInformer := factory.Argoproj().V1alpha1().AppProjects().Informer()
 	go projInformer.Run(ctx.Done())
@@ -125,11 +142,9 @@ func newTestAppSetServerWithEnforcerConfigure(f func(*rbac.Enforcer), objects ..
 		db,
 		kubeclientset,
 		enforcer,
-		nil,
 		fakeAppsClientset,
-		factory.Argoproj().V1alpha1().Applications().Lister(),
 		appInformer,
-		factory.Argoproj().V1alpha1().ApplicationSets().Lister().ApplicationSets(testNamespace),
+		factory.Argoproj().V1alpha1().ApplicationSets().Lister(),
 		fakeProjLister,
 		settingsMgr,
 		testNamespace,
@@ -223,21 +238,22 @@ func testListAppsetsWithLabels(t *testing.T, appsetQuery applicationset.Applicat
 }
 
 func TestListAppSetsInNamespaceWithLabels(t *testing.T) {
+	testNamespace := "test-namespace"
 	appSetServer := newTestAppSetServer(newTestAppSet(func(appset *appsv1.ApplicationSet) {
 		appset.Name = "AppSet1"
-		appset.ObjectMeta.Namespace = "test-namespace"
+		appset.ObjectMeta.Namespace = testNamespace
 		appset.SetLabels(map[string]string{"key1": "value1", "key2": "value1"})
 	}), newTestAppSet(func(appset *appsv1.ApplicationSet) {
 		appset.Name = "AppSet2"
-		appset.ObjectMeta.Namespace = "test-namespace"
+		appset.ObjectMeta.Namespace = testNamespace
 		appset.SetLabels(map[string]string{"key1": "value2"})
 	}), newTestAppSet(func(appset *appsv1.ApplicationSet) {
 		appset.Name = "AppSet3"
-		appset.ObjectMeta.Namespace = "test-namespace"
+		appset.ObjectMeta.Namespace = testNamespace
 		appset.SetLabels(map[string]string{"key1": "value3"})
 	}))
-	appSetServer.ns = "test-namespace"
-	appsetQuery := applicationset.ApplicationSetListQuery{AppsetNamespace: "test-namespace"}
+	appSetServer.enabledNamespaces = []string{testNamespace}
+	appsetQuery := applicationset.ApplicationSetListQuery{AppsetNamespace: testNamespace}
 
 	testListAppsetsWithLabels(t, appsetQuery, appSetServer)
 }
@@ -258,6 +274,32 @@ func TestListAppSetsInDefaultNSWithLabels(t *testing.T) {
 	testListAppsetsWithLabels(t, appsetQuery, appSetServer)
 }
 
+// This test covers https://github.com/argoproj/argo-cd/issues/15429
+// If the namespace isn't provided during listing action, argocd's
+// default namespace must be used and not all the namespaces
+func TestListAppSetsWithoutNamespace(t *testing.T) {
+	testNamespace := "test-namespace"
+	appSetServer := newTestNamespacedAppSetServer(newTestAppSet(func(appset *appsv1.ApplicationSet) {
+		appset.Name = "AppSet1"
+		appset.ObjectMeta.Namespace = testNamespace
+		appset.SetLabels(map[string]string{"key1": "value1", "key2": "value1"})
+	}), newTestAppSet(func(appset *appsv1.ApplicationSet) {
+		appset.Name = "AppSet2"
+		appset.ObjectMeta.Namespace = testNamespace
+		appset.SetLabels(map[string]string{"key1": "value2"})
+	}), newTestAppSet(func(appset *appsv1.ApplicationSet) {
+		appset.Name = "AppSet3"
+		appset.ObjectMeta.Namespace = testNamespace
+		appset.SetLabels(map[string]string{"key1": "value3"})
+	}))
+	appSetServer.enabledNamespaces = []string{testNamespace}
+	appsetQuery := applicationset.ApplicationSetListQuery{}
+
+	res, err := appSetServer.List(context.Background(), &appsetQuery)
+	assert.NoError(t, err)
+	assert.Equal(t, 0, len(res.Items))
+}
+
 func TestCreateAppSet(t *testing.T) {
 	testAppSet := newTestAppSet()
 	appServer := newTestAppSetServer()

diff --git a/server/server.go b/server/server.go
@@ -178,7 +178,7 @@ type ArgoCDServer struct {
 	appInformer    cache.SharedIndexInformer
 	appLister      applisters.ApplicationLister
 	appsetInformer cache.SharedIndexInformer
-	appsetLister   applisters.ApplicationSetNamespaceLister
+	appsetLister   applisters.ApplicationSetLister
 	db             db.ArgoDB
 
 	// stopCh is the channel which when closed, will shutdown the Argo CD server
@@ -264,7 +264,7 @@ func NewServer(ctx context.Context, opts ArgoCDServerOpts) *ArgoCDServer {
 	appLister := appFactory.Argoproj().V1alpha1().Applications().Lister()
 
 	appsetInformer := appFactory.Argoproj().V1alpha1().ApplicationSets().Informer()
-	appsetLister := appFactory.Argoproj().V1alpha1().ApplicationSets().Lister().ApplicationSets(opts.Namespace)
+	appsetLister := appFactory.Argoproj().V1alpha1().ApplicationSets().Lister()
 
 	userStateStorage := util_session.NewUserStateStorage(opts.RedisClient)
 	sessionMgr := util_session.NewSessionManager(settingsMgr, projLister, opts.DexServerAddr, opts.DexTLSConfig, userStateStorage)
@@ -471,6 +471,7 @@ func (a *ArgoCDServer) Listen() (*Listeners, error) {
 func (a *ArgoCDServer) Init(ctx context.Context) {
 	go a.projInformer.Run(ctx.Done())
 	go a.appInformer.Run(ctx.Done())
+	go a.appsetInformer.Run(ctx.Done())
 	go a.configMapInformer.Run(ctx.Done())
 	go a.secretInformer.Run(ctx.Done())
 }
@@ -852,9 +853,7 @@ func newArgoCDServiceSet(a *ArgoCDServer) *ArgoCDServiceSet {
 		a.db,
 		a.KubeClientset,
 		a.enf,
-		a.Cache,
 		a.AppClientset,
-		a.appLister,
 		a.appsetInformer,
 		a.appsetLister,
 		a.projLister,