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

Multi-source doesn't work with CMPs #12476

Open
3 tasks done
gczuczy opened this issue Feb 15, 2023 · 16 comments
Open
3 tasks done

Multi-source doesn't work with CMPs #12476

gczuczy opened this issue Feb 15, 2023 · 16 comments
Labels
bug/in-triage This issue needs further triage to be correctly classified component:cmp Config Management Plugin related issues enhancement New feature or request multi-source-apps Bugs or enhancements related to multi-source Applications. type:bug

Comments

@gczuczy
Copy link
Contributor

gczuczy commented Feb 15, 2023

Checklist:

  • I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
  • I've included steps to reproduce the bug.
  • I've pasted the output of argocd version.

Describe the bug

The new multi-source feature does not work with CMPs

To Reproduce

  1. Define multiple sources in an application
  2. Mix a plugin which is expected to work on multiple sources at once

Expected behavior

Multi-sources can work with CMPs

Version

argocd: v2.6.1-inhouse+stuff
  BuildDate: 2023-02-15T08:15:44Z
  GitCommit:
  GitTreeState: clean
  GoVersion: go1.20
  Compiler: gc
  Platform: linux/amd64

Logs

Tried multiple ways.
When trying to "chain" the avp in the sources:

  - ref: valuesrepo
    repoURL: https://github/valuesrepo.git
    targetRevision: master
  - chart: ourchart
    helm:
      valueFiles:
      - $valuesrepo/dev/values.yaml
    ref: chart
    repoURL: https://inhouse/helmrepo
    targetRevision: 0.42.69
  - plugin:
      name: argocd-vault-plugin
    repoURL: $chart

The error is:

spec.source.repoURL and either source.path, source.chart, or source.ref are required for source {$chart nil nil nil &ApplicationSourcePlugin{Name:argocd-vault-plugin,Env:[]*EnvEntry{},Parameters:[]ApplicationSourcePluginParameter{},} }```

When we're trying to handle the helm chart with a plugin, and passing the values simply through en envvar:

  - ref: valuesrepo
    repoURL: https://github/valuesrepo.git
    targetRevision: master
  - chart: ourchart
    repoURL: https://inhouse/helmrepo
    targetRevision: 0.42.69
    plugin:
      name: "avp-helm-values"|
      env:
        - name: VALUES
          value:  $valuesrepo/dev/values.yaml

The error is quite "funny". Helm template is not able to find the file, and the null values in the template's values are not getting replaced by <placeholders>, and the template generation is being terminated by require templating directive, which argocd treats as a successful run, and we're getting nothing but orphaned resources.

So, it doesn't really matter with what way, but somehow plugins are needed to do placeholder replacements for secrets management. This is kind of a blocker for us, with the cleanup work that the sources offers (separating deployment logic into charts, and environment specifics into values stored separately).

@gczuczy gczuczy added the bug Something isn't working label Feb 15, 2023
@gczuczy
Copy link
Contributor Author

gczuczy commented Feb 15, 2023

Related to #10432

@crenshaw-dev crenshaw-dev added enhancement New feature or request and removed bug Something isn't working labels Feb 15, 2023
@crenshaw-dev
Copy link
Member

The only field which currently supports referencing another source is valueFiles. And it only references files stored in another source's git state. No transformations are applied before the referenced file is passed to Helm.

@crenshaw-dev
Copy link
Member

Maybe of interest: #12471

@gczuczy
Copy link
Contributor Author

gczuczy commented Feb 15, 2023

@crenshaw-dev Yes, that's what we're experienced, and it's not exactly the expected behaviour. And unfortunately, despite AVP is only replacing placeholders, it's refusing to work on non-manifests yaml files, which doesn't have a kind: specification. So, currently it's not possible to pre-render <placeholders> in valuefiles with AVP then feed it into the builtin helm mechanism.

Somehow either chaining in a plugin at the end of renderring the input, or making it possible to resolve the $refs in plugin env/parameters would also do the trick. If we could simply pass a $ref/dir/values.yaml into a plugin, then that would be a sufficient workaround until the multisources feature gets properly extended to cover CMPs.

@gczuczy
Copy link
Contributor Author

gczuczy commented Feb 15, 2023

@crenshaw-dev Also, if you could please give me a few pointers to the source how $ref lookup is being done, and where would be the point of injection of refparsing into the app.sources.*.plugin.env fields, I could probably do a small patch for a workaround.

@crenshaw-dev
Copy link
Member

This is where we pull references sources for Helm:

for _, valueFile := range q.ApplicationSource.Helm.ValueFiles {

I don't think I'd want to replicate that kind of code. I think I'd want to do something like what's proposed in #12471.

Basically, copy a while external ref into the plugin's sources before running the plugin.

@gczuczy
Copy link
Contributor Author

gczuczy commented Feb 15, 2023

@crenshaw-dev That makes sense. Providing a src/dst pair at something like the suggested additionalFiles section, the src could be ref-resolved, then simply symlinked at the dst. That way it could also be a file or a directory, or anything else. And with a symlink, there's no need to do any kind of heavy duty operations, and the files are accessable the same way. And this logic has to run, before the helm/kustomize/plugin execution is being done.

@crenshaw-dev
Copy link
Member

I'd recommend a copy-based implementation rather than symlink-based. It's more filesystem-heavy, but it avoids concurrency issues (you'd have to lock the referenced repository against reads, because you don't know what the referencing repository is gonna do). It also simplifies path-traversal mitigations (just copy everything and then run a symlink scan over the whole source directory.)

@amorozkin
Copy link

So, it doesn't really matter with what way, but somehow plugins are needed to do placeholder replacements for secrets management. This is kind of a blocker for us, with the cleanup work that the sources offers (separating deployment logic into charts, and environment specifics into values stored separately).

Not knowing of that work is in progress - I had to make kind of AVP wrapper which is able to pull HELM chart from one (helm) repo and use values files from another (git) one (as well as simple vault paths permissions checks ...)
If #12471 is implemented - that warpper can become a bit slimer :)
But anyway as far as i understand - even then all template genaration stuff must be handled by wrapper itself, we can't chain built-in argocd Helm tool with AVP with current argocd CMP implementaion.

@evgfitil
Copy link

So, it doesn't really matter with what way, but somehow plugins are needed to do placeholder replacements for secrets management. This is kind of a blocker for us, with the cleanup work that the sources offers (separating deployment logic into charts, and environment specifics into values stored separately).

Not knowing of that work is in progress - I had to make kind of AVP wrapper which is able to pull HELM chart from one (helm) repo and use values files from another (git) one (as well as simple vault paths permissions checks ...) If #12471 is implemented - that warpper can become a bit slimer :) But anyway as far as i understand - even then all template genaration stuff must be handled by wrapper itself, we can't chain built-in argocd Helm tool with AVP with current argocd CMP implementaion.

Can you show me what the ArgoCD Application looks like when using your wrapper, please?

@Thiryn
Copy link

Thiryn commented Aug 23, 2023

wondering about @amorozkin's Application too, in my case the CMP doesn't trigger for sources of type helm and when it triggers using the second source with the remote valuefile, I don't have all the helm env vars, do you set the env var manually in a plugin entry on the valuefile source?

@Thiryn
Copy link

Thiryn commented Aug 24, 2023

Modified @amorozkin's script to be a little bit more generic and added some usage example in this gist

@ghost
Copy link

ghost commented Jun 8, 2024

I have a slightly different use case for this. Each of our helm charts has a templated values file that needs to get populated with inputs from a different repository prior to running helm template. We currently work around this using helmfile but this has a couple shortcomings including:

  • the manifest generation is slow due to remote fetching and packaging of helm charts using helm-git
  • ArgoCD is only aware of the "inputs" repository so a manual, hard-refresh is required when changes are made to the helm charts

If this feature were implemented we could instead write a config management plugin and avoid a lot of headache and workarounds.

@NikOverflow
Copy link

How is this going? I need that feature for secret and configmap replacing. I did my own replacer that uses kubernetes secrets and configmaps and realize that argocd is the limitation.

@ilyavaiser
Copy link

Mid-2024 and here we are - one of the important features does not work.

@alexmt alexmt added bug/in-triage This issue needs further triage to be correctly classified component:cmp Config Management Plugin related issues type:bug labels Jun 25, 2024
@JavierPorron
Copy link

Same issue here. End of 2024. The feature doesn't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/in-triage This issue needs further triage to be correctly classified component:cmp Config Management Plugin related issues enhancement New feature or request multi-source-apps Bugs or enhancements related to multi-source Applications. type:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants