argoproj-labs / argocd-image-updater

Automatic container image update for Argo CD
https://argocd-image-updater.readthedocs.io/en/stable/
Apache License 2.0
1.17k stars 240 forks source link

Git write-back-target helmvalues does not replace tag and repo in the image section #744

Open zagr0 opened 3 weeks ago

zagr0 commented 3 weeks ago

Describe the bug Image updater not able to find tag and repository within image hierarchy to replace values, instead it adds new entries image.tag and image.repository.

Helm values.yaml file used:

image:
  repository: my.repo.domain/common/app
  tag: 0.0.6-SNAPSHOT_3ced321a

The commited result by image updater in git:

image:
  repository: my.repo.domain/common/app
  tag: 0.0.6-SNAPSHOT_3ced321a
image.repository: my.repo.domain/common/app
image.tag: 0.0.5_b579c34b

To Reproduce Steps to reproduce the behavior: Configure application with annotations to update image tag with helmvales:

  annotations:
    argocd-image-updater.argoproj.io/image-list: main=my.repo.doamin/common/app
    argocd-image-updater.argoproj.io/main.pull-secret: pullsecret:argocd/registry
    argocd-image-updater.argoproj.io/main.helm.image-name: image.repository
    argocd-image-updater.argoproj.io/main.helm.image-tag: image.tag
    argocd-image-updater.argoproj.io/main.update-strategy: newest-build
    argocd-image-updater.argoproj.io/main.allow-tags: regexp:^[0-9]+.[0-9]+.[0-9+]_[0-9a-f]{8}$
    argocd-image-updater.argoproj.io/write-back-method: git:secret:argocd/git-creds-ssh
    argocd-image-updater.argoproj.io/write-back-target: helmvalues:./helm/app/values.yaml
    argocd-image-updater.argoproj.io/git-repository: git@my.repo.domain:common/deploy.git
    argocd-image-updater.argoproj.io/git-branch: master

with the ./helm/app/values.yaml file content from example above.

Expected behavior We expect to have updated tag and repo inside image section in values.yaml like:

image:
  repository: my.repo.domain/common/app
  tag: 0.0.5_b579c34b

Version This is latest build from master, because we need the latest fixes for helmvalues git write-back

askhari commented 2 weeks ago

Thank you @zagr0 for pointing this out.

As you may see in the comments, I opened #748 to fix the issue. I test it in my own environments, but please let me know if you may test it as well.

Let's see if it can be reviewed soon.

Cheers!

zagr0 commented 2 weeks ago

Hi @askhari , thank you very much for your work! I have tested the PRs image and repository/tag replacement works as expected. For reformatting I noticed that it does not reorder keys in the values file anymore, but still empty lines are removed and indentaition for lists are changed.

askhari commented 2 weeks ago

Hi @zagr0,

Thank you for your feedback :).

For the reformatting there isn't a good solution afaik. It comes from how yaml.v2 marshall and unmarshall methods work. In yaml.v3, there is a different way to preserve order, but the removal of empty lines and indentation changes should remain. Usually for lists it changes the format when it's like this:

# original list
my_list:
  - "first item"
  - "second item"

# reformated list
my_list:
- "first item"
- "second item"

Is this the behaviour you see?

Cheers!

zagr0 commented 2 weeks ago

@askhari , yes, exactly for list indentation in addition to empty lines removal I noticed the string values like "string" replaced with string but this is not very critical for us, thank you very much!

askhari commented 2 weeks ago

@zagr0 , thank you again for your feedback. I really appreciate it :).

All of these formatting changes comes from the serialization and deserialization of YAMLs (marshal and unmarshal). And then the content is writting into a file. There is not much we can do to avoid those changes using the default marshal and unmarshal methods. We may built a custom marshal/unmarshal, but I don't really know how to do it right now (a bit rookie in programming languages yet :D).

Cheers!

askhari commented 1 week ago

@zagr0 , now that PR #748 has been merged, I think these issues should be solved. If that's the case, could you please let me know if we may close this bug issue?

askhari commented 2 days ago

@zagr0 , I write this comment just as a reminder. Just in case we may close this issue :) .

zagr0 commented 2 days ago

Hi @askhari , yes, we can close it, thank you very much!