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

Helm parameters defined in Application CRD are overriden by .argocd-source-<app>.yml #6665

Open
3 tasks done
Alan-pad opened this issue Jul 8, 2021 · 6 comments
Open
3 tasks done
Labels
bug Something isn't working

Comments

@Alan-pad
Copy link

Alan-pad commented Jul 8, 2021

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

Helm parameters defined in Application CRD are overriden by .argocd-source-.yml

To Reproduce

Create an Application with Helm parameters and valueFiles defined (I didn't test without valueFiles defined so this might also be linked) :

 project: XXX
source:
  repoURL: 'XXX'
  path: XXX
  targetRevision: master
  helm:
    valueFiles:
      - ../../../values/XXX.common.yaml
      - values.yaml
    parameters:
      - name: 'www.ingress.hosts[0].host'
        value: host
        forceString: true
destination:
  server: 'https://kubernetes.default.svc'
  namespace: XXX
syncPolicy:
  automated:
    prune: true
    selfHeal: true

Add an .argocd-source-.yml in the same path as Application.spec.source.path with other Helm parameters :

helm:
  parameters:
  - name: xxx.image.name
    value: xxx
    forcestring: true
  - name: xxx.image.tag
    value: xxx
    forcestring: true
  • ArgoCD UI will detect the helm parameters in Application CRD and show them as overriden
  • The helm template command won't have any --set option defined for those helm parameters

Expected behavior

I expect helm parameters from Application CRD and helm parameters from .argocd-source-.yml to be somewhat merge using --set option of Helm.

Version

argocd: v2.0.4+0842d44
  BuildDate: 2021-06-23T01:27:53Z
  GitCommit: 0842d448107eb1397b251e63ec4d4bc1b4efdd6e
  GitTreeState: clean
  GoVersion: go1.16
  Compiler: gc
  Platform: linux/amd64```

Tell me if you need any more information or context

@Alan-pad Alan-pad added the bug Something isn't working label Jul 8, 2021
@vanko1987
Copy link

I am running argocd v2.1.7 and have labeled an app for usage with image updater v0.11.0. I am using argocd-image-updater.argoproj.io/write-back-method=git.
What happens is that:

  1. once a parameter is modified for the first time via the ArgoCD UI -its value is stored in the argocd-source-appname.yaml and the application is properly synced/updated
  2. any further changes to that same parameter (via ArgoCD UI) are only reflected in the UI -> App Details -> Manifest , however they are in fact ignored and the argocd-source-appname.yaml is not updated at all.

@Alan-pad
Copy link
Author

@vanko1987 I don't know if it's the same issue, what I observed is that helm parameters you set in Application CRD are not taken into account at all in the helm template command launched even if you those helm parameters are not defined in .argocd-source-.yml

@brightdroid
Copy link

This also happens to me with version v2.5.2+148d8da and a helm chart in git directory (source.path)

The helm.parameters from Application is not merged with .argocd-source-....yml

@grzegdl
Copy link

grzegdl commented Jan 18, 2023

We also hit this bug.

ArgoCD Image Updater persists App params set from Appset + it’s image tag overrides in the .argocd-source-<appName>.yaml

However if later we add additional parameters in ApplicationSet i.e.:

          parameters:
            - name: "global.environment.ENVIRONMENT"
              value: '{{metadata.labels.env}}'

They will be visible in ArgoCD UI but they won’t be applied with helm template until ArgoCD Image Updater has a new image to write in .argocd-source-<appName>.yaml
(which can be a long time for services that are not actively developed anymore)

So despite that we introduce a new parameter i.e. global.environment.ENVIRONMENT which completely doesn’t exist in .argocd-source-<appName>.yaml it’s not taken into account for helm template. As ArgoCD only watches those parameters from the git written file. It doesn’t merge those which it has in the Application params.

My guess is that it would be very hard for ArgoCD Image Updater to track all those App/Appset params changes and commit them to git file. It’s not its job anyway.

So ideal thing would be if ArgoCD would be merging params from the file (higher priority) with missing/additional params from the Application params.

@crenshaw-dev
Copy link
Collaborator

crenshaw-dev commented Apr 3, 2023

I bet it's poor merging logic. Instead of merging the parameters fields, using the name field as a key, we just replace the whole parameters block.

We probably just need to improve that merge logic, but we'll need to make it opt-in so people don't suddenly find that their apps behave differently than they used to.

@crenshaw-dev
Copy link
Collaborator

I believe this is the problem line. We just need to use a better-informed merge tool:

data, err = jsonpatch.MergePatch(data, patch)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants