proposal: support multiple sources for an Application

[ishitasequeira]

This is a proposal for supporting multiple sources for an Application.

Related issues:

• Helm chart + values files from Git #2789
• Proposal: support multiple sources for an Application #677

Signed-off-by: ishitasequeira isequeir@redhat.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:

• 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.
• Optional. My organization is added to USERS.md.
• 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 #8322 (70431e9) into master (3659b7b) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8322      +/-   ##
==========================================
- Coverage   44.96%   44.92%   -0.05%     
==========================================
  Files         212      212              
  Lines       25264    25264              
==========================================
- Hits        11361    11350      -11     
- Misses      12296    12310      +14     
+ Partials     1607     1604       -3     

Impacted Files	Coverage Δ
applicationset/services/scm_provider/github.go	63.52% <0.00%> (-17.65%)	⬇️
applicationset/services/scm_provider/utils.go	88.50% <0.00%> (+4.59%)	⬆️

---

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 3659b7b...70431e9. Read the comment docs.

[crenshaw-dev]

Overall, looks great! Most comments are just looking for clarification. I think the most interesting part is around the details of what Helm+external values will look like.

[tgruenert]

Just a hint - not sure about this is within the scope of this proposal.
As a user i´d like to see that encryption via sops is already supported with multiple application sources in same kind like it is now. Personal I use sops / helm-plugin to encrypt secrets within helm values.yaml files.

[romuduck]

Stupid questions:

• will this option ignoreMissingValueFiles be available to externalValueFiles?
• UI wise, what will be displayed under last sync result?
• UI wise, in an application tree view for example, will it be visible which resource comes from which source?
• playing with sync waves, it seems we could artificially rollout source per source by setting proper weights on resources globally... is it correct?

thanks

[ishitasequeira]

Stupid questions:

• will this option ignoreMissingValueFiles be available to externalValueFiles?
• UI wise, what will be displayed under last sync result?
• UI wise, in an application tree view for example, will it be visible which resource comes from which source?
• playing with sync waves, it seems we could artificially rollout source per source by setting proper weights on resources globally... is it correct?

thanks

Please find my comments on the respective questions:

• Yes, we would want to have ignoreMissingValueFiles with externalValueFiles.
• We have not included UI in this proposal and would have a separate proposal for UI changes.
• For Sync waves, I guess yes, it should be possible to play around with Sync waves.

[romuduck]

Hello all,
Are we confident it will meet v2.4 as planned? I'm especially interested by the #2789 issue for which 8322 could be an answer...
But given the additional complexity and the fact UI won't be covered i wonder if we LL be able to use it in the next couple of months.
Should not we reconsider #2789 to be a separate topic as it looks much simpler to address.

Thanks

[todaywasawesome]

@romuduck We discussed in the contributor experience meeting today and the team was fairly confident in meeting the 2.4 release for this one. There's some more design work they wanted to do. I marked it for the 2.4 milestone which is slated for May.

[alexmt]

Unless I'm missing something, the proposed design allows using additional sources only with Helm. However I think we have two common use cases here that can be useful for any config management tool:

• Provide additional set of file that are needed to generate manifests ( like values.yaml for helm template )
• Create an application that is combined with several helm charts/kustomizes. Right now users are forced to create several apps or use some config management tool to combine two sources.

First use case is easier. We just let use define multiple sources. In this case Argo CD would generate manifests from all sources and merge them in one list:

spec:
  # application that consists of MongoDB and ElasticSearch resources
  sources:
    - repoURL: https://github.com/helm/charts
      targetRevision: master
      path: incubator/mongodb
    - repoURL: https://github.com/helm/charts
      targetRevision: master
      path: incubator/elasticsearch

To enable the second use case I propose to make path/chart field optional: if it is missing then manifests are not generated for the source. In order to make repo files accessible for other sources user would have to provide a ref that might be referenced in other sources.

spec:
  sources:
    - repoURL: https://github.com/my-org/my-repo # path is missing so no manifests are generated
      targetRevision: master
      ref: myRepo                                 # repo is available via symlink "my-repo"
    - repoURL: https://github.com/helm/charts
      targetRevision: master
      path: incubator/elasticsearch               # path "incubator/elasticsearch" is used to generate manifests
      helm:
        valueFiles:
          - $myRepo/values.yaml                   # values.yaml is located in source with reference name $myRepo

This way we can cover Helm use-case without introducing Helm specific settings and other use cases as well. WDYT?

[ishitasequeira]

Hi @alexmt, the idea above sounds good. I just had a small question.

For the second use case, I was thinking how adding multiple files would work. For example, (expanding on your example above)

spec:
  sources:
    - repoURL: https://github.com/my-org/my-repo # path is missing so no manifests are generated
      targetRevision: master
      ref: myRepo                                 # repo is available via symlink "my-repo"
    - repoURL: https://github.com/helm/charts
      targetRevision: master
      path: incubator/elasticsearch               # path "incubator/elasticsearch" is used to generate manifests
      ref: repoES
      helm:
        valueFiles:
          - $myRepo/values.yaml                   # values.yaml is located in source with reference name $myRepo
          - gen_values.yaml                       # values file present in the current source
          - $repoES/esValues.yaml                 # values file present in the current source

Would we support adding values files from current source using it's ref $repoES (e.g. $repoES/esValues.yaml) and also without it's ref (e.g. gen_values.yaml)?

Also, would the referenced source (for e.g. $myRepo) need to be declared before using them in other sources?

[alexmt]

I'm thinking the Argo CD would just resolve $<refName> to the absolute path where the repo is cloned. To reference values file from current source user would just use a relative path (same as now). The source needs reference only if some other source needs to use file from it.

[KlavsKlavsen]

It would be REALLY nice if value file "pointers" also could have a branch (like the application itself has) - so for development f.ex. - you'd point an application at a 'testing' branc - and the values file then also to an equivalent testing branch in its repo.
Thats how we typicly test applications before we merge them - by adding them - pointing to the devel branch for testing via argocd - but today - our values files HAVE to be in master branch - as we use the path hack from above (where the repo is in /tmp/.../ - because its a repo we already have added).

[alexmt]

@KlavsKlavsen this should be covered by the targetRevision field. The only caveat is that it won't be possible to reference two revisions of the same branch. This limitation will be removed after switching to Config Management Plugins though.

[crenshaw-dev]

Is the connection done via symlink, or via a variable which Argo CD will populate in some set of fields?

Symlink is simple and easy to understand, but I'm not sure whether all relevant Helm/Kustomize features will permit out-of-bound symlinking.

Variables make sense, we just need to make sure our sensitive-path sanitization logic is working correctly.

[alexmt]

I would suggest using variables because we are doing it already. Users can specify the env variable e.g. in helm file params etc and Argo CD performs substitution:

argo-cd/reposerver/repository/repository.go

Line 752 in 0ff1ada

resolvedPath, _, err := pathutil.ResolveFilePath(appPath, repoRoot, env.Envsubst(p.Path), q.GetValuesFileSchemes())

[crenshaw-dev]

That same env variable gets used for --set and --set-string, so I think it would be trivial to leak the randomized repo paths.

We could augment env with the path variable. But that would be a Helm-specific solution. We'd have to write additional features to support Kustomize, and we'd have to ask CMP authors to please use the variables responsibly and avoid leaking the values to users.

[alexmt]

LGTM!

[ishitasequeira]

Hi @crenshaw-dev, are we good to merge the PR?

[crenshaw-dev]

@ishitasequeira I have some concerns that only apply if we plan to use $refVars anywhere besides valueFiles. If we're limiting them to valueFiles (at least for the initial implementation), then I'm good.

[dracowf]

Its sounds so good! We were waiting this for decades! 🤝

[alexmt]

@crenshaw-dev I'm assuming you are concerned about security, right? I agree. We should not allow resolving references in every spec field. Let's limit it to valueFiles and additionally in spec.source.helm.fileParameters (since it is exact same use-case as valueFiles). Sounds good?

[crenshaw-dev]

Yep! With that limitation, I think security should be fine. Will approve/merge.

[romuduck]

Can't wait .. thanks contributors

[chenfli]

is PR-6280 will be part of the new multiple sources for an application?
Or is this going to be postponed to 2.5 ?

[crenshaw-dev]

@chenfli that PR had some shortcomings which this proposal addressed. We do expect that this feature will be in 2.5.

[KlavsKlavsen]

Is there a PR for this feature yet?

[crenshaw-dev]

@KlavsKlavsen not yet. Will post on the main issue thread when that happens!

[romuduck]

Hello @ishitasequeira , is the feature available in v2.4? I saw another PR related to this proposal marked for 2.5. While v2.4 release note mentions it.
++
Thanks

[crenshaw-dev]

@romuduck it will not be available until 2.5. Ishita has a draft PR open for it.

[romuduck]

Thanks @crenshaw-dev and @ishitasequeira ... Enjoy holidays in case :-)

[wmgroot]

PR is here for those following.
#10432

[rnpaiva]

is it released for v2.5.1 ?
I'm trying something like this :

spec:
  project: default
  source:
  - repoURL: 'git@github.com:myrepository/infrastructure.git'
    path: apps/authelia
    targetRevision: main
    ref: myRepo
  - repoURL: 'https://charts.authelia.com'
    chart: authelia
    targetRevision: 0.8.38
    helm:
      valuesFiles:
        - $myRepo/values-prod.yaml

It's failing , I followed an example that I saw above .

[blakepettersson]

It's targeted for 2.6.

[fabhuebner]

is it released for v2.5.1 ? I'm trying something like this :

spec:
  project: default
  source:
  - repoURL: 'git@github.com:myrepository/infrastructure.git'
    path: apps/authelia
    targetRevision: main
    ref: myRepo
  - repoURL: 'https://charts.authelia.com'
    chart: authelia
    targetRevision: 0.8.38
    helm:
      valuesFiles:
        - $myRepo/values-prod.yaml

It's failing , I followed an example that I saw above .

Note the singular/plural spelling of "source/sources" (also maps vs. array of maps):

For single source apps:

spec:
  project: default
  source:
    repoURL: 'git@github.com:myrepository/infrastructure.git'
    ...

For multi source apps:

spec:
  project: default
  sources:
    - repoURL: 'git@github.com:myrepository/infrastructure.git'
      ...
    - repoURL: 'git@github.com:myrepository/infrastructure_01.git'
      ...