Listing ApplicationSets requires global permissions even if argocd is namespaced

[JorTurFer]

Checklist:

• I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
• I've included steps to reproduce the bug.
• I've pasted the output of argocd version.

Describe the bug

I'm using ArgoCD v2.8.2 and when I execute argocd appset list I get RBAC errors because ArgoCD is trying to list the appsets in the wrong namespace:

argocd appset list
FATA[0000] rpc error: code = PermissionDenied desc = error listing ApplicationSets with selectors: applicationsets.argoproj.io is forbidden: User "system:serviceaccount:argocd:argocd-server" cannot list resource "applicationsets" in API group "argoproj.io" at the cluster scope

To Reproduce

Install ArgoCD with appsets enabled and ArgoCD using a single namespace, then execute argocd appset list

Expected behavior

ApplicationSets are correctly listed without any RBAC error

[JorTurFer]

I'm preparing a PR with the fix, just needing to think about how to test this properly

[jannfis]

You can use the -N <namespace> parameter to the appset list command as a workaround. Have you tried this?

I don't think the right fix is to revert the list command to be namespace-scoped, as it will not let you list application sets across all namespaces.

Potentially, but this would be a breaking change for the CLI UX, we could mimic behaviour of kubectl, i.e.

• No namespace parameter given: List appsets in Argo CD's control plane namespace
• Namespace parameter given: List appsets in given namespace
• --all-namespaces: List appsets across all namespaces

It is important though that app and appset subcommands behave consistent in this regard.

As mentioned, this would be a breaking change (as is your fix), so it needs some thought.

[JorTurFer]

Hi,

You can use the -N parameter to the appset list command as a workaround. Have you tried this?

Yeah, I know it, but this has some problems:

• We need to expose extra information to our development teams because they need to know extra info about the installation (like ArgoCD namespace)
• All the scripts need to be updated

but this would be a breaking change for the CLI UX

I have to disagree because actually, this breaking change has already happened, in v2.7 appset list works and using v2.8 it fails due to RBAC. Any automation around this command is broken atm.

It is important though that app and appset subcommands behave consistent in this regard.

I get your point totally, and I agree with you. Given the current ArgoCD scenario, this makes sense totally, but in that case, the RBAC has to be updated accordingly (applications are allowed by ClusterRole but applicationset aren't). If appset list requests all the namespaces as default, the RBAC has to allow it as default too.

If you want, I can update my PR adding the option for --all-namespaces in the command, something like you proposed (and updating the RBAC for that).

As a quick fix, I have updated our cluster role to allow this permission and that's totally enough, but I'd like to solve it here instead of patching the release within our clusters.

[jannfis]

I have to disagree because actually, this breaking change has already happened, in v2.7 appset list works and using v2.8 it fails due to RBAC. Any automation around this command is broken atm.

Yeah, that's true. Thinking, both apps-in-any-namespace and appsets-in-any-namespace are beta features, so I think it leaves us a little room for actually making some breaking changes to un-break a change introduced by that particular feature.

If you want, I can update my PR adding the option for --all-namespaces in the command, something like you proposed (and updating the RBAC for that).

That'd be great!

Then all that's left is to align the app list command to this new syntax and behaviour, too. For applications, the change is in a little longer already, and somehow, none complained yet. I believe this is due to the fact that the API serving Application makes use of a lister, which is already initialized being either namespace scoped or cluster scoped, depending on the setting for --allowed-namespaces.

The ApplicationSet API in contrast uses a common Kube clientset instead. So, I'm wondering whether it would make sense to use a lister for ApplicationSet, too. It's got some performance advantages, too, to use a lister.

[JorTurFer]

The ApplicationSet API in contrast uses a common Kube clientset instead. So, I'm wondering whether it would make sense to use a lister for ApplicationSet, too. It's got some performance advantages, too, to use a lister.

I think that lister is the way to go for self managed resources when they are used intensively, but the server doesn't use them in general (maybe because there isn't any appset ui). I mean, the price of the cache could be higher than the performance improvement if the resources aren't consumed frequently. For the applicationset-controller I don't have doubts about using listers, but for the repo server I'm not totally sure, but I'm willing to add it if you think that it's the way to go. It's true that having a lister/informer the eventual appset UI will have a better performance.

I believe this is due to the fact that the API serving Application makes use of a lister, which is already initialized being either namespace scoped or cluster scoped, depending on the setting for --allowed-namespaces.

I don't think so, the reason is that applications are cluster scoped (ClusterRole). Actually, that's the only required change to allow this, the RBAC change (other are for improving things, but the RBAC is the only mandatory change)

[jannfis]

I don't think so, the reason is that applications are cluster scoped (ClusterRole). Actually, that's the only required change to allow this, the RBAC change (other are for improving things, but the RBAC is the only mandatory change)

But the argocd app list command works even when running Argo CD without any cluster role at all, i.e. in namespace scoped mode, as long as no --allowed-namespaces are set for the API server (apps-in-any-namespaces requires a cluster-scoped installation). The informer is set up accordingly, either in namespace-scope or cluster-scope.

But thinking a little further, I think you could even check for whether apps-in-any-namespace is enabled or not in the ApplicationSet's List function, for example (pseudo-code):

if len(s.ApplicationNamespaces) > 0 {
  listNs = q.AppSetNamespace // Now it shouldn't matter if the empty string (cluster scope) or not
} else {
  listNs = s.ns
}
appIf := s.appclientset.ArgoprojV1alpha1().ApplicationSets(listNs)

The above should work regardless of the mode the API server is running in, and wouldn't require the use of an informer.

[JorTurFer]

We have been talking about this by slack to speed up the process, and this is the summary (please correct me if I said somethign wrong @jannfis ):

• We will implement a lister/informer for appsets instead of kubernetes clientset
  • As default, the lister will be scoped to ArgoCD namespace
  • The flag --allow-namespaces will enable the lister for reading from more namespaces
• As starting point, the RBAC for appsets will be exactly the same as current RBAC for applications