[argo-workflows] workflow-controller-cluster-role is missing delete configmap permission

[Paritosh-Anand]

Describe the bug

about the issue

argo-workflows has a feature of CACHE_GC_PERIOD which helps in clean up of expired cache keys from ConfigMap and clean up of the ConfigMap which is having no data.

https://github.com/argoproj/argo-workflows/blob/main/docs/environment-variables.md

In our setup we have enabled CACHE_GC_PERIOD and we get exceptions like below from workflows-controller when it tries to do the necessary cleanup

failed to delete ConfigMap <name-of-cache-configmap>: configmaps 
"<name-of-cache-configmap>" is forbidden: User "system:serviceaccount:argo-workflows:argo-workflows-workflow-controller" 
cannot delete resource "configmaps" in API group "" in the namespace "argo-workflows"

solution

I think the solution is a one-line change in charts/argo-workflows/templates/controller/workflow-controller-cluster-roles.yaml

adding delete permission should solve the issue. I have tried setting it manually in the ClusterRole on our cluster and it works fine.

  {{- if .Values.controller.rbac.writeConfigMaps }}
  - create
  - update
+ - delete
  {{- end}}

I am looking for suggestion/recommendation on the proposed solution.

If the solution looks alright, then I can go ahead and create a PR to make this change.

Related helm chart

argo-workflows

Helm chart version

0.41.2

To Reproduce

set cache GC configs from values.yaml

# -- Extra environment variables to provide to the controller container
extraEnv:
  - name: CACHE_GC_PERIOD
    value: '30m'
  - name: CACHE_GC_AFTER_NOT_HIT_DURATION
    value: '48h'

and generated manifest and deploy argo-workflows to K8s.

Expected behavior

workflow-controller should have sufficient permissions to delete the configmaps which have no data if CACHE_GC_PERIOD is enabled.

Screenshots

No response

Additional context

No response

[yu-croco]

Hi @Paritosh-Anand , thank you for your investigation.
Since we follow upstream's manifest, can you please make PR to upstream at first?
When upstream is fixed and new version tag is released, we also follow the manifest.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.