feat: Extra Helm values from external git repo #5826

[KaiReichart]

Feature: External Helm values from git

CLOSES #5826

This PR allows for external values.yaml from other git repos in a helm installation.

Sample application.yaml

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: external-test
  finalizers:
    - resources-finalizer.argocd.argoproj.io
spec:
  project: default
  source:
    repoURL: https://charts.bitnami.com/bitnami
    targetRevision: 8.5.8
    helm:
      valueFiles:
        - values.yaml
      externalValueFiles:
        - repoURL: https://github.com/KaiReichart/argo-test-values.git
          targetRevision: main
          valueFiles:
            - values.yaml
    chart: mysql
  destination:
    server: 'https://kubernetes.default.svc'
    namespace: default

Current State:

• Created API changes
• correct rendering in repo-server
• UI changes
• documentation

CAVEATS:
Due to the way the caching system works, changes in external repos will only be checked when performing a hard refresh. Will address this in another PR.

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:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).

[codecov]

Codecov Report

Merging #6280 (4a5c754) into master (bcfc112) will decrease coverage by 0.02%.
The diff coverage is 45.00%.

❗ Current head 4a5c754 differs from pull request most recent head c9eeb80. Consider uploading reports for the commit c9eeb80 to get more accurate results

@@            Coverage Diff             @@
##           master    #6280      +/-   ##
==========================================
- Coverage   41.10%   41.08%   -0.03%     
==========================================
  Files         157      157              
  Lines       20967    21053      +86     
==========================================
+ Hits         8619     8650      +31     
- Misses      11122    11170      +48     
- Partials     1226     1233       +7     

Impacted Files	Coverage Δ
controller/appcontroller.go	52.95% <ø> (-0.42%)	⬇️
pkg/apis/application/v1alpha1/types.go	57.87% <ø> (+0.15%)	⬆️
server/application/application.go	32.74% <34.37%> (-0.03%)	⬇️
reposerver/repository/repository.go	59.36% <40.00%> (-1.64%)	⬇️
controller/state.go	66.83% <73.91%> (-0.86%)	⬇️
controller/cache/info.go	63.50% <0.00%> (-1.38%)	⬇️
util/settings/settings.go	47.77% <0.00%> (ø)
cmd/argocd/commands/repocreds.go	0.00% <0.00%> (ø)
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcfc112...c9eeb80. Read the comment docs.

[topikachu]

@KaiReichart This change is very useful. Would you please fix the conflict and work with argocd team?
Thanks

[eytanhanig]

@KaiReichart Can you please update/rebase this so there are no conflicts and sign-off on your contribution to resolve the DCO? Instructions for doing so can be found here.

[rouke-broersma]

@KaiReichart Can you please update/rebase this so there are no conflicts and sign-off on your contribution to resolve the DCO? Instructions for doing so can be found here.

The DCO is caused by @Reuuke and not @KaiReichart

[tyrken]

FWIW I suspect my teams' use case aligns more with @rouke-broersma comment earlier, and was a bit surprised in this PR to see the main ArgoCD Application CRD's source field & targetrevision used to point to the helm chart, rather than the more "local" thing which would be the values file we fine-tune for our deployment.

I did a while back look a bit at the Application CRD's spec.source.chart property, as Helm (the binary) is actually quite flexible as regards the chart "name" you pass in - if it's actually a URL then it would download & unpack that URL to use as the chart data. Unfortunately our chart repo at the time required authentication so I never completed hacking/testing & we went a different way, but that would seem more in keeping a way of allowing the app to get it's config from two places.

Re caching/invalidation - helm charts built for distribution are meant to be immutable (see helm/helm#3084) though the physical spec doesn't absolutely enforce this. Hence I'd like ArgoCD to monitor & update to see changes to the git repo storing the values file (if targetRevision == HEAD), but don't see any need to monitor/invalidate cache if the chart content changes.

[mkilchhofer]

I like this also 👍 👍
Maybe this #5664 could also be respected when defining the extension to the current CRD? :)

[karma-git]

Hi everyone, any updates here?
Our team is waiting for this PR more than 2 months, is there any chance that PR will be merged soon?
Thanks.

[jessesuen]

Hi everyone, any updates here?

You can read an update about this here:
#2789 (comment)

Maintainers agree to have this feature. However, the PR needs to accommodate some additional requirements w.r.t cache invalidation (from when the values in the git repo changes) and check the project restrictions to see if the referenced repo is in the allow list. Finally, because the feature is effectively allowing 2+ git/helm sources per application, we want the approach to be made in such a way that design could eventually support #677.

All of these added requirements may be too much to ask of the original author, @KaiReichart, which is why we are offering to work on this, but in the v2.3 timeframe.

[dracowf]

Hi @jessesuen, maybe we'll mention also this issue #6280 in Milestone v2.3?

[alexmt]

@dracowf, agreed, adding PR and issue to milestone and roadmap.md

[kopaygorodsky]

I would like to help with this PR to speed up the process :)

[odelreym]

Hi there and good job

Wanted to ask a question regarding the PR. Then the following ApplicationSet should work when merged, right?

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: services
spec:
  generators:
  - list:
      elements:
      - appName: 'app1'
        repo: '[email protected]:repo1/path.git'
        version: branch
      - appName: 'app2'
        repo: '[email protected]:repo2/path.git'
        version: branch
  template:
    metadata:
      name: '{{appName}}'
    spec:
      project: default
      source:
        repoURL: '{{repo}}'
        targetRevision: '{{version}}'
        path: deployment
        helm:
          valueFiles:
           - values.yaml
        externalValueFiles:
        - repoURL: '[email protected]:repo3/path.git'
          targetRevision: master
          valueFiles:
            - values.yaml
      destination:
        namespace: default
        server: https://kubernetes.default.svc

[ishitasequeira]

Hi @KaiReichart , are you still working on this PR? Is it still in your list?

[loeken]

will this be merged? where are we stuck?

[ArshiAAkhavan]

@tyrken approach seems more reasonable to me
is it taken into consideration?

[ashish1099]

any idea when is this going to get merged ?

[KlavsKlavsen]

Soo looking forward to this feature.. We use a central repo - that we sync to every location - which has helm charts etc. and common value files.. and then for each team - they have their own cluster they manage and store values files "to override defaults" - and decide which applications they run - in their own repo.
This way we can be strict about what we merge (and review properly) in the central repo that ALL teams use - which also means we have coordination of versions across all clusters.. But at the same time, each team has their own repo - that applies to ONLY their cluster, where they can manage exactly which applications they run and have override valuesfile in there.
Works great - without having it be "wild west" where anyone can merge in the repo that affects every cluster :)

[begemotik]

@alexmt, Is it officially out of scope for the release v2.3 now? The community seems to be craving for that feature

[ltmleo]

I also use helm chart as a central repo, and each microservice has its own value, this feature will work great for me.

[lucaslobolp]

I also use helm chart as a central repo, and each microservice has its own value, this feature will work great for me.

[ArshiAAkhavan]

could any one give us an update about this pr?
perhaps if the pr is not well implemented, the community can provide you with another one

or if its not towards argocds main goals, let us know so we can make our own forks

as mentioned above, this feature is highly useful in our production environment

[ishitasequeira]

@ArshiAAkhavan , we have an ongoing proposal doc on this PR that has been started and is being discussed in the community. Reviews on it would be helpful.

[YperXristis]

@kostis-codefresh @romulus-ai
Here is the solution I came up with which works for me, please share it.

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  annotations:
    argocd.argoproj.io/sync-wave: "6"
  name: prometheus-stack
  namespace: argocd
  finalizers:
  - resources-finalizer.argocd.argoproj.io
spec:
  destination:
    namespace: monitoring
    server: https://kubernetes.default.svc
  project: ska
  source:
    chart: kube-prometheus-stack
    helm:
      version: v3
      values: |
{{ .Files.Get "prometheus-stack-vaules.yaml" | indent 8 }}
    repoURL: https://prometheus-community.github.io/helm-charts
    targetRevision: 34.9.0
  syncPolicy:
    automated:
      selfHeal: true
      prune: false
    syncOptions:
    - CreateNamespace=true
   

[crenshaw-dev]

Closing in favor of multi-source repos, which is currently expected in 2.5.

Sorry for the wait folks, we really want to get this right. 😃

[zakkg3]

ppl reading this is looking for: https://argo-cd.readthedocs.io/en/stable/user-guide/multiple_sources/#helm-value-files-from-external-git-repository