argoproj / argo-cd

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

Multi-source doesn't work with CMPs #12476

Open gczuczy opened 1 year ago

gczuczy commented 1 year ago

Checklist:

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 commented 1 year ago

Related to #10432

crenshaw-dev commented 1 year ago

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 commented 1 year ago

Maybe of interest: https://github.com/argoproj/argo-cd/issues/12471

gczuczy commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

This is where we pull references sources for Helm: https://github.com/argoproj/argo-cd/blob/b6cfe676f3ff9f73c71a0d6256c1bc14cd9c12ed/reposerver/repository/repository.go#L679

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 https://github.com/argoproj/argo-cd/issues/12471.

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

gczuczy commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/argoproj/argo-cd/issues/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 commented 1 year ago

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 commented 10 months ago

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 commented 10 months ago

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

breynolds-parachute commented 4 weeks ago

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:

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

NikOverflow commented 3 weeks ago

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 commented 1 week ago

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