Skip to content

feat(argo-workflows): Allow controller to whitelist secrets#1646

Merged
yu-croco merged 4 commits intoargoproj:mainfrom
emmayylu:add-get-any-secrets-rules-to-controller
Nov 23, 2022
Merged

feat(argo-workflows): Allow controller to whitelist secrets#1646
yu-croco merged 4 commits intoargoproj:mainfrom
emmayylu:add-get-any-secrets-rules-to-controller

Conversation

@emmayylu
Copy link
Contributor

@emmayylu emmayylu commented Nov 15, 2022

Allow the controller to get any secrets.

When I was working with argo events, I created the following objects:

  • an argo workflow that uses service account A
  • service account A using mimagePullSecret docker-registry

The argo events sensor get an error when triggering the workflow. The error is secrets "docker-registry" is forbidden: User "system:serviceaccount:argo:my-argo-workflows-workflow-controller" cannot get resource "secrets" in API group "" in the namespace "yolu-ci"'

The error happens because the service account that the controller is using does not have permission to get secrets. This PR allows users to whitelist secrets. After making this change, the error is gone

Signed-off-by: emmayylu 84873428+yolu-kxs@users.noreply.github.com

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • I have bumped the chart version according to versioning
  • I have updated the documentation according to documentation
  • I have updated the chart changelog with all the changes that come with this pull request according to changelog.
  • Any new values are backwards compatible and/or have sensible default.
  • I have signed off all my commits as required by DCO.
  • My build is green (troubleshooting builds).

Changes are automatically published when merged to main. They are not published on branches.

@yu-croco
Copy link
Collaborator

yu-croco commented Nov 15, 2022

[IMO]
Many of organizations/companies want to restrict secret access, so I think we need to keep this resourceNames block. 🤔
Instead of allowing all secret access, maybe we can add new block, like below.
*Same logic is implemented here

# charts/argo-workflows/templates/controller/workflow-controller-cluster-roles.yaml
{{- if .Values.controller.secretWhitelist }}
- apiGroups:
  - ""
  resources:
  - secrets
  verbs:
  - get
  - list
  - watch
  {{- with .Values.controller.secretWhitelist }}
  resourceNames: {{- toYaml . | nindent 4 }}
  {{- end }}
{{- end }}

WDYT? 👀 @stefansedich @paguos @vladlosev @yann-soubeyrand @jmeridth

@vladlosev
Copy link
Collaborator

@yu-croco I think this solution makes sense. The code can be simplified slightly, though:

{{- with .Values.controller.secretWhitelist }}
- apiGroups:
  - ""
  resources:
  - secrets
  verbs:
  - get
  - list
  - watch
  resourceNames: {{- toYaml . | nindent 4 }}
{{- end }}

@yu-croco
Copy link
Collaborator

yu-croco commented Nov 16, 2022

Thank you for checking, @vladlosev .
You're right, that's perfect. 👍

Hi @emmayylu @yolu-kxs , thank you for your contribution.
Could you please add new block ( #1646 (comment) ), instead of removing existing resourceNames ? 🙋
*I can take over it, so please feel free to ask me if you want to.

@emmayylu emmayylu force-pushed the add-get-any-secrets-rules-to-controller branch from 9185229 to c5f112f Compare November 22, 2022 00:03
@github-actions github-actions bot added size/S and removed size/XS labels Nov 22, 2022
@emmayylu emmayylu changed the title feat(argo-workflows): Allow controller to get any secrets feat(argo-workflows): Allow controller to whitelist secrets Nov 22, 2022
Signed-off-by: emmayylu <84873428+yolu-kxs@users.noreply.github.com>
@emmayylu emmayylu force-pushed the add-get-any-secrets-rules-to-controller branch from c5f112f to 251477b Compare November 22, 2022 00:06
@emmayylu
Copy link
Contributor Author

Thank you for checking, @vladlosev . You're right, that's perfect. 👍

Hi @emmayylu @yolu-kxs , thank you for your contribution. Could you please add new block ( #1646 (comment) ), instead of removing existing resourceNames ? 🙋 *I can take over it, so please feel free to ask me if you want to.

Thank you! I've updated the PR.

Signed-off-by: emmayylu <44856279+emmayylu@users.noreply.github.com>
Signed-off-by: emmayylu <44856279+emmayylu@users.noreply.github.com>
Signed-off-by: emmayylu <44856279+emmayylu@users.noreply.github.com>
Copy link
Collaborator

@yu-croco yu-croco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! LGTM

@yu-croco yu-croco merged commit 16cf7af into argoproj:main Nov 23, 2022
ilia-medvedev added a commit to codefresh-io/argo-helm that referenced this pull request Feb 2, 2023
* feat(argo-cd): Upgrade Argo CD to 2.5.0 (argoproj#1568)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* chore(github): Bump GitHub actions versions (argoproj#1575)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* fix(argo-cd): Chart NOTES nil references (argoproj#1582)

Signed-off-by: Filipe Santos <filipe@not.sh>

* docs(argo-cd): Improve documentation (argoproj#1584)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* fix(argo-workflows): serviceaccount rbac when sso is enabled (argoproj#1586)

Signed-off-by: Nick Fisher <nxf5025@gmail.com>

Signed-off-by: Nick Fisher <nxf5025@gmail.com>

* Fix incorrect applicationSet property in README (argoproj#1590)

Based on [here](https://github.com/argoproj/argo-helm/blob/55b8b34d20ebaf38fa05e1113daf30220d11e725/charts/argo-cd/templates/argocd-applicationset/deployment.yaml#L9), I think `replicas` should be `replicaCount` (though `replicas` would be more consistent).

Signed-off-by: Ashlin Eldridge <ashlin.eldridge@gmail.com>

Signed-off-by: Ashlin Eldridge <ashlin.eldridge@gmail.com>

* fix(argo-cd): Remove AWS volume from server (argoproj#1591)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* chore(argo-cd): Cleanup Redis manifest (argoproj#1577)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* fix(argo-cd): Fix migration path for server configs (argoproj#1585)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* fix(argo-cd): Type conversion for ConfigMaps values (argoproj#1594)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* feat(argo-cd): Add probes for ApplicationSet controller (argoproj#1532)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* chore(argo-cd): Remove liveness probe from application controller (argoproj#1581)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* chore(github): Add dependabot.yml (argoproj#1595)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* feat(argo-cd): Set container security contexts (argoproj#1579)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* feat(argo-cd): Support custom TLS certificates for Dex (argoproj#1477)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* feat(argo-cd): Support manually managed TLS certificate for Server (argoproj#1534)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* fix(argo-cd): Don't install CRDs for disabled components (argoproj#1596)

Signed-off-by: Marco Kilchhofer <mkilchhofer@users.noreply.github.com>

* fix(argo-cd): update network policy port name (argoproj#1603)

Signed-off-by: Eric Cimino <ecimino@vailsys.com>

* chore(argo-workflows): Update ArgoWorkflows to v3.4.3 (argoproj#1610)

Signed-off-by: yu-croco <yu.croco@gmail.com>

* fix(argo-cd): Replace coalesce with merge for old config values (argoproj#1612)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* feat(argo-cd): Add revisionHistoryLimit (argoproj#1599)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* Upgrade Argo Image to the latest (argoproj#1614)

Signed-off-by: Dong Wang <wd@wdicc.com>

Signed-off-by: Dong Wang <wd@wdicc.com>

* chore(argo-cd): Update redis-ha (argoproj#1617)

Signed-off-by: yu-croco <yu.croco@gmail.com>

* fix(argo-cd): Add /tmp voulmeMount to extensions container (argoproj#1620)

* Fixes argoproj#1619 - Add /tmp voulmeMount to extensions container

Signed-off-by: Tim Van de Walle <tvandewalle@trek10.com>

* Bump version, add change notes

Signed-off-by: Tim Van de Walle <tvandewalle@trek10.com>

Signed-off-by: Tim Van de Walle <tvandewalle@trek10.com>

* fix(argo-cd): Add missing ClusterRole permissions to argo-cd-server to manage Application in all namespaces (argoproj#1621)

Signed-off-by: Elad Dolev <dolevelad@gmail.com>

* fix(argo-cd): Use Dex non-distroless image (argoproj#1626)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* chore(argo-cd): Upgrade Argo CD to 2.5.2 (argoproj#1628)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* Allow to add custom artifact repository (argoproj#1453)

Signed-off-by: Max Kochubey <20810306+maxkochubey@users.noreply.github.com>

Signed-off-by: Max Kochubey <20810306+maxkochubey@users.noreply.github.com>

* fix(argo-cd): Use raw json for cluster credentials for Vault compatibility (argoproj#1634)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>
Co-authored-by: Aikawa <yu.croco@gmail.com>

* fix(argo-cd): Cluster credentials config should be a string (argoproj#1636)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* fix(argo-workflows): Added missing attribute for sso (argoproj#1641)

Signed-off-by: yu-croco <yu.croco@gmail.com>

* docs(argo-cd): Improve changelog information (argoproj#1652)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* chore(argo-cd): Consolidated GnuPG configuration (argoproj#1609)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* fix(argo-cd): Invalid argocd-gpg-keys-cm template (argoproj#1656)

The template removed a little too much whitespace resulting in an invalid ConfigMap.

Error:
```
Error: YAML parse error on argocd/charts/argo-cd/templates/argocd-configs/argocd-gpg-keys-cm.yaml: error converting YAML to JSON: yaml: line 10: mapping values are not allowed in this context
```

Signed-off-by: Allex <allexveldman+github@gmail.com>

Signed-off-by: Allex <allexveldman+github@gmail.com>

* feat(argo-workflows): Allow controller to whitelist secrets (argoproj#1646)

* allow users to whitelist secrets

Signed-off-by: emmayylu <84873428+yolu-kxs@users.noreply.github.com>

* remove unnecessary if-statement

Signed-off-by: emmayylu <44856279+emmayylu@users.noreply.github.com>

* use square bracket for array

Signed-off-by: emmayylu <44856279+emmayylu@users.noreply.github.com>

* fix typo and update readme

Signed-off-by: emmayylu <44856279+emmayylu@users.noreply.github.com>

Signed-off-by: emmayylu <84873428+yolu-kxs@users.noreply.github.com>
Signed-off-by: emmayylu <44856279+emmayylu@users.noreply.github.com>
Co-authored-by: emmayylu <84873428+yolu-kxs@users.noreply.github.com>

* feat(argo-workflows): Add labels for ServiceAccounts (argoproj#1665)

* Add labels for ServiceAccounts

Signed-off-by: Eugene Lugovtsov <lug.zhenia@gmail.com>

* fix workflow serviceaccount labels

Signed-off-by: Eugene Lugovtsov <lug.zhenia@gmail.com>

* fix docs

Signed-off-by: Eugene Lugovtsov <lug.zhenia@gmail.com>

Signed-off-by: Eugene Lugovtsov <lug.zhenia@gmail.com>

* fix(argo-cd): deprecate server.extraArgs."--insecure" (argoproj#1669)

Signed-off-by: GitHub <noreply@github.com>

Signed-off-by: GitHub <noreply@github.com>

* chore(argo-workflows): Support workflow retention (argoproj#1668)

Signed-off-by: yu-croco <yu.croco@gmail.com>

* feat(argo-cd): Upgrade argocd to v2.5.3 (argoproj#1671)

Signed-off-by: smcavallo <smcavallo@hotmail.com>

* fix helm install md (argoproj#1672)

Signed-off-by: fsl <1171313930@qq.com>

Signed-off-by: fsl <1171313930@qq.com>

* feat(argo-cd): Add Repo Server strict TLS cert support (argoproj#1673)

Signed-off-by: Karl Parry <karl.parry@imbursepayments.com>

* chore(argo-workflows): Update Argo Workflows to v3.4.4 (argoproj#1674)

Signed-off-by: yu-croco <yu.croco@gmail.com>

* fix(argo-cd): Rename tls secret to include the -secret suffix (argoproj#1676)

- "[Fixed]: TLS secret name so Dex correctly generates the checksum for argocd-dex-server-tls."
- "[Fixed]: Standardise the naming convention of the TLS secret manifests."
- "[Added]: Add checksum to Repo-Server for the argocd-repo-server-tls secret."

Signed-off-by: Karl Parry <karl.parry@imbursepayments.com>

* chore(argo-cd): Remove duplicate ApplicationSet features (argoproj#1598)

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>

* feat(argo-cd): Add ability to annotate Deployments and StatefulSets (argoproj#1608)

* feat(argo-cd): Add ability to annotate Deployments and StatefulSets

Signed-off-by: John Stewart <jstewart@rentpath.com>

* fix: Controller and AppSet controller was mixed

Signed-off-by: Marco Kilchhofer <mkilchhofer@users.noreply.github.com>

Signed-off-by: John Stewart <jstewart@rentpath.com>
Signed-off-by: Marco Kilchhofer <mkilchhofer@users.noreply.github.com>
Co-authored-by: Marco Kilchhofer <mkilchhofer@users.noreply.github.com>

* chart update WIP

* backport applicationset

* backport applicationset

* argocd 2.5.5

---------

Signed-off-by: Petr Drastil <petr.drastil@gmail.com>
Signed-off-by: Filipe Santos <filipe@not.sh>
Signed-off-by: Nick Fisher <nxf5025@gmail.com>
Signed-off-by: Ashlin Eldridge <ashlin.eldridge@gmail.com>
Signed-off-by: Marco Kilchhofer <mkilchhofer@users.noreply.github.com>
Signed-off-by: Eric Cimino <ecimino@vailsys.com>
Signed-off-by: yu-croco <yu.croco@gmail.com>
Signed-off-by: Dong Wang <wd@wdicc.com>
Signed-off-by: Tim Van de Walle <tvandewalle@trek10.com>
Signed-off-by: Elad Dolev <dolevelad@gmail.com>
Signed-off-by: Max Kochubey <20810306+maxkochubey@users.noreply.github.com>
Signed-off-by: Allex <allexveldman+github@gmail.com>
Signed-off-by: emmayylu <84873428+yolu-kxs@users.noreply.github.com>
Signed-off-by: emmayylu <44856279+emmayylu@users.noreply.github.com>
Signed-off-by: Eugene Lugovtsov <lug.zhenia@gmail.com>
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: smcavallo <smcavallo@hotmail.com>
Signed-off-by: fsl <1171313930@qq.com>
Signed-off-by: Karl Parry <karl.parry@imbursepayments.com>
Signed-off-by: John Stewart <jstewart@rentpath.com>
Co-authored-by: Petr Drastil <petr.drastil@gmail.com>
Co-authored-by: Filipe <filipe@not.sh>
Co-authored-by: Nick Fisher <nxf5025@gmail.com>
Co-authored-by: Ashlin Eldridge <ashlin.eldridge@gmail.com>
Co-authored-by: Marco Kilchhofer <mkilchhofer@users.noreply.github.com>
Co-authored-by: Eric Cimino <58572548+cimin0@users.noreply.github.com>
Co-authored-by: Aikawa <yu.croco@gmail.com>
Co-authored-by: Dong Wang <wd@wdicc.com>
Co-authored-by: tvandewalle <1022306+tvandewalle@users.noreply.github.com>
Co-authored-by: Elad Dolev <dolevelad@gmail.com>
Co-authored-by: Max Kochubey <20810306+maxkochubey@users.noreply.github.com>
Co-authored-by: Allex <a.veldman@chain-stock.com>
Co-authored-by: emmayylu <44856279+emmayylu@users.noreply.github.com>
Co-authored-by: emmayylu <84873428+yolu-kxs@users.noreply.github.com>
Co-authored-by: Eugene Lugovtsov <34510252+EugeneLugovtsov@users.noreply.github.com>
Co-authored-by: Zadkiel Aharonian <zadkiel.aharonian@gmail.com>
Co-authored-by: smcavallo <smcavallo@users.noreply.github.com>
Co-authored-by: fsl <1171313930@qq.com>
Co-authored-by: Karl Parry <88431088+karlparry@users.noreply.github.com>
Co-authored-by: John Stewart <32647598+jstewart612@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants