fix: principal errors due to forbidden - in bindings subject updated the principal ServiceAccount namespace to argocd (from default)

[jblsk]

What does this PR do / why we need it: This PR fixes the CrashLoopBackOff and forbidden errors when argocd-agent-principal is deployed due to ineffective Role and ClusterRole ment to be bound to argocd-agent-principal ServiceAccount.

Which issue(s) this PR fixes: #403

Fixes #?

How to test changes / Special notes to the reviewer:
The argocd-agent-principal ServiceAccount which is the subject of the ClusterRoleBinding and RoleBinding should be deployed to argocd namespace (according to documentation here) and then non-existent in the default namespace. This PR fixes the false namespace value in argocd-agent-principal ClusterRoleBinding and RoleBinding subject's namespace key

Checklist

• No documentation update is required.

[mikeshng]

This was done intentionally in #428 (comment)

I don't have a strong opinion on whether it's good or bad. Just providing an additional reference.

[jannfis]

I think that one is supposed to run kustomize edit set namespace argocd before running kustomize build.

If we're considering argocd to be the default target namespace for the installation, it might make sense to incorporate the changes from this PR. Maybe a proper fix is to have namespace: argocd in the kustomization.yaml. But I agree, using the default namespace as default doesn't make much sense.

[jannfis]

OK, so after a bit of testing, it seems that kustomize will only replace the referred SA's namespace in ClusterRoleBindings and RoleBindings when it is set to default. If set to another value, kustomize won't bother about it.

So, we basically have two choices now:

1. Require the user to install the argocd-agent components into the argocd namespace (i.e. set the SA binding namespace to argocd, instead of default in the manifests).
2. Require the user to either set the namespace field in the kustomization.yaml for each component, or run kustomize edit set namespace argocd (or another namespace) before running kustomize build (or, kubectl apply -k).

My gut tells me it would be good to follow what Argo CD does, and this is option 1. Users can still install to a different namespace, but need a kustomization patch to do so. But it will be usable out of the box, no changes needed, when going with the standard kustomization base and installing to the argocd namespace. Option 2 would require the user to make a change, regardless of which namespace to install to.

So if we're going with option 1, we should also make the same change to the ClusterRoleBinding of the agent's manifests.

WDYT?

[mikeshng]

My vote is option 1 as well.

Setting the namespace to argocd will help us simplify installs. IE. we can remove the first troubleshooting in this doc regarding namespace: https://argocd-agent.readthedocs.io/latest/getting-started/openshift/#troubleshooting

[jblsk]

I see the original intent now. At least we could hint it in the GettingStartedGuide and provide a patch for the case argocd namespace is used and kustomize doesn't patch the namespace in binding's subjects. For some reason kubectl apply -k to my vcluster doesn't (but with --dry-run=client or kustomize build will patch it), see below:

$ git remote show origin
warning: ignoring http.savecookies for empty http.cookiefile
* remote origin
  Fetch URL: https://github.com/argoproj-labs/argocd-agent.git
  Push  URL: https://github.com/argoproj-labs/argocd-agent.git
  HEAD branch: main
  Remote branches:
    dependabot/go_modules/github.com/argoproj/argo-cd/v3-3.1.1              tracked
    dependabot/go_modules/github.com/stretchr/testify-1.11.1                tracked
    feat/api-proxy                                                          tracked
    fix/remap-cluster                                                       tracked
    main                                                                    tracked
    redis-proxy                                                             tracked
    refs/remotes/origin/dependabot/go_modules/github.com/spf13/cobra-1.10.1 stale (use 'git remote prune' to remove)
    release-0.1                                                             tracked
    release-0.2                                                             tracked
    release-0.3                                                             tracked
  Local branch configured for 'git pull':
    main merges with remote main
  Local ref configured for 'git push':
    main pushes to main (local out of date)
$ git status
On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean
$ kubectl apply -n argocd \
  -k 'https://github.com/argoproj-labs/argocd-agent/install/kubernetes/principal?ref=main' \
  --context "$CONTROL_PLANE_CTX" -oyaml --dry-run=client | grep 'subjects:' -A3
  subjects:
  - kind: ServiceAccount
    name: argocd-agent-principal
    namespace: argocd
--
  subjects:
  - kind: ServiceAccount
    name: argocd-agent-principal
    namespace: argocd
$ kubectl apply -n argocd \
  -k 'https://github.com/argoproj-labs/argocd-agent/install/kubernetes/principal?ref=main' \
  --context "$CONTROL_PLANE_CTX" -oyaml --dry-run=server | grep 'subjects:' -A3
  subjects:
  - kind: ServiceAccount
    name: argocd-agent-principal
    namespace: default
--
  subjects:
  - kind: ServiceAccount
    name: argocd-agent-principal
    namespace: default

[jblsk]

ChatGPT explanation of the inconsistency in behavior https://chatgpt.com/s/t_68ba90803b048191a3ba4e8c366f06d7