Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(argo-cd): Upgrade Argo CD to 2.5.0 #1568

Merged
merged 1 commit into from
Oct 25, 2022

Conversation

pdrastil
Copy link
Member

@pdrastil pdrastil commented Oct 22, 2022

Resolves:

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.

@pdrastil pdrastil added the awaiting-upstream Is waiting for a change upstream to be completed before it can be merged. label Oct 22, 2022
{{/*
Create Dex server endpoint
*/}}
{{- define "argo-cd.dex.server" -}}
Copy link
Member Author

Choose a reason for hiding this comment

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

Support for new HTTP / HTTPS for dex server. Tested via OIDC provider config on RC3

ports:
- name: repo-server
containerPort: {{ .Values.repoServer.containerPort }}
protocol: TCP
{{ if .Values.repoServer.metrics.enabled }}
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed all these guards. Metrics ports are not related to configured metrics Service and can be used via other means like Prometheus service discovery. They are also used by some controllers for liveness / readiness probes.

name: gpg-keys
- mountPath: /app/config/gpg/keys
name: gpg-keyring
{{- if .Values.configs.knownHosts }}
Copy link
Member Author

Choose a reason for hiding this comment

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

Not an optional volume. All optional volumes are marked as such via optional: true

@pdrastil
Copy link
Member Author

Synced and tested on RC3. Still waiting for confirmation if new cmd-params for applicationset will be cherry picked to 2.5.0

charts/argo-cd/Chart.yaml Outdated Show resolved Hide resolved
@karlschriek
Copy link

Hey, thanks a lot for getting this up so early. We are pretty anxious to start using 2.5.0 because of the ability to roll out Applications to namespaces other than argocd. I am giving this chart a spin today to see if I can get it to work correctly.

I did notice however that the CRD rollouts in the chart are still Namespaced. Shouldn't they have a Cluster scope for the above feature to work correctly?

@pdrastil
Copy link
Member Author

@karlschriek Hi - you are welcome. CRDs are always global resource in the cluster. As far as I know inside CRD schema can be namespaced (to allow creation of resources inside each namespace) or global (which means that they behave similar like ClusterRoles). This chart just installs the CRDs as part of it installation, namespaced schema ensures that it can be used anywhere.

@karlschriek
Copy link

karlschriek commented Oct 24, 2022 via email

@pdrastil pdrastil force-pushed the feature/upgrade branch 6 times, most recently from 2d3049f to ce8f4e1 Compare October 25, 2022 17:00
@pdrastil pdrastil marked this pull request as ready for review October 25, 2022 17:00
Signed-off-by: Petr Drastil <petr.drastil@gmail.com>
@pdrastil pdrastil removed the awaiting-upstream Is waiting for a change upstream to be completed before it can be merged. label Oct 25, 2022
Copy link
Member

@jmeridth jmeridth left a comment

Choose a reason for hiding this comment

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

This LGTM but would like another approval before merge

Copy link
Contributor

@hairmare hairmare left a comment

Choose a reason for hiding this comment

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

I tried this on a recent kind v1.24.0 and didn't run into any issues using the out-of-the-box values.

This is just just a drive-by LGTM since i'm not a maintainer though.

Copy link
Member

@mkilchhofer mkilchhofer left a comment

Choose a reason for hiding this comment

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

Additional to the comment, we should cleanup the default securityContexts as upstream already has default (hardened) security context defined.
And release 2.5 now adds

          seccompProfile:
            type: RuntimeDefault

Also the TLS stuff for dex is missing. But I assume you want to do this outside the 2.5 PR and test it again via:

But I did not find a blocker. So I am okay releasing this one and fix the rest afterwards.

Comment on lines 235 to 242
httpGet:
path: /healthz
port: {{ .Values.controller.containerPort }}
port: metrics
initialDelaySeconds: {{ .Values.controller.livenessProbe.initialDelaySeconds }}
periodSeconds: {{ .Values.controller.livenessProbe.periodSeconds }}
timeoutSeconds: {{ .Values.controller.livenessProbe.timeoutSeconds }}
successThreshold: {{ .Values.controller.livenessProbe.successThreshold }}
failureThreshold: {{ .Values.controller.livenessProbe.failureThreshold }}
Copy link
Member

Choose a reason for hiding this comment

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

argocd-application-controller livenessprobe is removed in 2.5. Maybe we should treat it as optional via values?

Ref: argoproj/argo-cd#9557

@pdrastil
Copy link
Member Author

@mkilchhofer - See following

Ad security contexts - already have local branch with this because it needs more work on all components.
Dex TLS - The protocol now uses HTTPS by default, certs are self-signed. Custom certs will follow here #1477
Probes - I'll check in next iteration - they were still present in RC3

@bobertrublik
Copy link

Thank you for your work! The argocd-server serviceaccount still needs more permissions when apps are deployed to a different namespace (as per argoproj/argo-cd#9755 (comment)).
I'm however not sure what the best practices are here. Can we grant the permissions to the ClusterRole?

@pdrastil
Copy link
Member Author

I believe it should be ClusterRole as you are using different namespace. I will take a look what needs to be patched and add it as another fix.

ilia-medvedev-codefresh 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.

ApplicationSet CRD update for Gitlab Pull request generator Use https for dex server
6 participants