argoproj / argo-cd

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

ArgoCD should support `helm template --skip-tests` #19956

Open DerekTBrown opened 1 week ago

DerekTBrown commented 1 week ago

Summary

TL;DR; Add source.helm.skip_tests, which would pass-through the --skip-tests flag to helm template.

Motivation

Proposal

ArgoCD should support passing the --skip-tests flag to helm template as part of the existing helm configuration block. E.g.:

source:
  helm:
   ...
   skip_tests: true

How do you think this should be implemented?

jaehanbyun commented 1 week ago

Can I try this task?

agaudreault commented 1 week ago

It sounds to me like the default behavior of skipTests should be true when not specified.

jaehanbyun commented 3 days ago

I've implemented the changes to add the source.helm.skip_tests option, which passes through the --skip-tests flag to helm template. However, for this feature to fully work, the helm.sh/hook: test hook needs to be added in the GitOps Engine repository file

Since I'm new to contributing to ArgoCD, I might be missing something. If there's a way to address this within the ArgoCD repository, I would appreciate any guidance. Otherwise, I'll proceed by opening an issue and pull request in the GitOps Engine repository to add support for the helm.sh/hook: test hook.

Thank you for your time and assistance!

agaudreault commented 3 days ago

Hey @jaehanbyun, can you explain why the changes are necessary? The issue description does not point at any necessary support 🤔

From the following:

Note: As discussed in https://github.com/argoproj/argo-cd/discussions/15302, ArgoCD currently skips manifests that include hooks not supported by ArgoCD, including helm test hooks. While this feature covers many testing usecases, it is not totally congruent with --skip-tests, which excludes everything in the /tests directory.

It would seem that gitops already exclude the test files.

jaehanbyun commented 3 days ago

Thank you for your feedback, @agaudreault.

I initially misunderstood how the skipTests option interacts with the helm.sh/hook: test annotation. I assumed that setting skipTests to false would ensure that manifests with the helm.sh/hook: test annotation are recognized and deployed. However, upon re-reading the following text, I realize that my understanding was incorrect.