argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
16.75k stars 5.07k forks source link

Manifest is missing required RBAC rules for ApplicationSetController leader election #14369

Open Manuelraa opened 1 year ago

Manuelraa commented 1 year ago

Checklist:

Describe the bug

Manifest is missing required RBAC rules for functionality. I was unable to find documentation saying those need to be added in order to use the features. v2.7.6 manifests/ha/install.yaml

I had to apply following Kustomize patches to get rid of RBAC permissions errors.

Role argocd-applicationset-controller is missing following permissions required when leader election is enabled. applicationsetcontroller.enable.leader.election=true

  # Add rules for ApplicationSetController
  # Allow create and update configmap for leader election
  # Allow get, create and update leases for leader election
  - target:
      group: rbac.authorization.k8s.io
      version: v1
      kind: Role
      name: argocd-applicationset-controller
    patch: |
      - op: add
        path: /rules/0
        value:
          apiGroups:
            - ''
          resources:
            - configmaps
          verbs:
            - create
            - update
      - op: add
        path: /rules/0
        value:
          apiGroups:
            - 'coordination.k8s.io'
          resources:
            - leases
          verbs:
            - get
            - create
            - update

To Reproduce

  1. Deploy ArgoCD using the manifest
  2. Enable "applicationsetcontroller.enable.leader.election=true"
  3. Errors show up in logs. Leader election does not work.

Expected behavior

Manifest include all required RBAC rules.

Screenshots

Not applicable

Version

argocd: v2.7.6+00c914a
  BuildDate: 2023-06-20T21:18:20Z
  GitCommit: 00c914a948d9e8ad99be8bd82a368fbdeba12f88
  GitTreeState: clean
  GoVersion: go1.19.10
  Compiler: gc
  Platform: linux/amd64
argocd-server: v2.7.6+00c914a.dirty
  BuildDate: 2023-06-20T20:51:13Z
  GitCommit: 00c914a948d9e8ad99be8bd82a368fbdeba12f88
  GitTreeState: dirty
  GoVersion: go1.19.10
  Compiler: gc
  Platform: linux/amd64
  Kustomize Version: v5.0.1 2023-03-14T01:32:48Z
  Helm Version: v3.11.2+g912ebc1
  Kubectl Version: v0.24.2
  Jsonnet Version: v0.19.1

Logs

e.g. from argocd-applicationset-controller (While slowly adding all required permissions one by one)

Failed to update lock: leases.coordination.k8s.io "58ac56fa.applicationsets.argoproj.io" is forbidden: User "system:serviceaccount:argocd:argocd-applicationset-controller" cannot update resource "leases" in API group "coordination.k8s.io" in the namespace "argocd"
crenshaw-dev commented 1 year ago

Good catch. Would you mind opening a PR to add a clusterrole and clusterrolebinding to the cluster-scope Argo CD manifests to give it access to ApplicationSet resources? The PR should probably add a release note that, if the user doesn't want to use appsets-in-any-namespace, they can remove the clusterrole/clusterrolebinding.

crenshaw-dev commented 1 year ago

I completely misunderstood the issue, yours is about leader election, not appset access. I think the fix is to just add the RBAC you specified by default.

crenshaw-dev commented 1 year ago

Can we restrict configmap access to a certain resource name?

Manuelraa commented 1 year ago

Hi @crenshaw-dev I can find time to make that PR.

There is rules.resourceNames but it does not seem to allow a wildcard.

In my case the configmap is called 58ac56fa.applicationsets.argoproj.io and the lease 58ac56fa.applicationsets.argoproj.io. Looks like a hash to me without knowing the functionality. Which would mean limiting the rule to those specific entries would not be possible.

Manuelraa commented 1 year ago

Hi @crenshaw-dev I can find time to make that PR.

There is rules.resourceNames but it does not seem to allow a wildcard.

In my case the configmap is called 58ac56fa.applicationsets.argoproj.io and the lease 58ac56fa.applicationsets.argoproj.io. Looks like a hash to me without knowing the functionality. Which would mean limiting the rule to those specific entries would not be possible.

Correction to my last comment.

I have found that 58ac56fa.applicationsets.argoproj.io is a hardcoded value so we are able to limit access to certain resource names. https://github.com/argoproj/argo-cd/blob/e2e0da7fcc4e6a5cf040117a3af9c15a7d0c267f/cmd/argocd-applicationset-controller/commands/applicationset_controller.go#L118 Reflected in PR.

rumstead commented 9 months ago

Could this be merged as part of the 2.9 RC?

rumstead commented 4 months ago

Anything left here to enable leader election without custom patches for the rbac?