Skip to content

Feature/allow empty helm.image-name annotation #1211

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

will4j
Copy link
Contributor

@will4j will4j commented Aug 9, 2025

In case when only image tag is required to update, like:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
...
  annotations:
    argocd-image-updater.argoproj.io/image-list: app=harbor.example.com/nginx
    argocd-image-updater.argoproj.io/app.helm.image-tag: image.tag
    argocd-image-updater.argoproj.io/app.update-strategy: newest-build
    argocd-image-updater.argoproj.io/app.allow-tags: regexp:^sha-[0-9a-f]{7}$
    argocd-image-updater.argoproj.io/write-back-target: helmvalues

It make no sense to let argocd-image-updater.argoproj.io/app.helm.image-name annotation mandatorily, either I have to fill a fake image name value, still it produces meaningless configs in target .argocd-source or helm value files.

By this pr, I Supposed to get values.yaml file like

image:
  registry: harbor.example.com
  repository: nginx
  tag: "sha-xxxx"

instead of

image:
  registry: harbor.example.com
  name: harbor.example.com/nginx # which actually not used
  repository: nginx
  tag: "sha-xxxx"

@will4j will4j force-pushed the feature/allow-empty-image-name branch 2 times, most recently from b16084f to 34703ee Compare August 9, 2025 03:12
@codecov-commenter
Copy link

codecov-commenter commented Aug 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.53%. Comparing base (cc9f71b) to head (334f575).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1211      +/-   ##
==========================================
+ Coverage   63.52%   63.53%   +0.01%     
==========================================
  Files          23       23              
  Lines        3191     3192       +1     
==========================================
+ Hits         2027     2028       +1     
  Misses       1054     1054              
  Partials      110      110              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chengfang
Copy link
Collaborator

when image-name annotation is absent, it defaults to image.name. That's why there is an extra image.name fields generated in your values file. But if you specify it with the following, the expected fields will be updated.

argocd-image-updater.argoproj.io/app.helm.image-name: image.repository

The above tells image updater to use image.repository field for image name.

See https://argocd-image-updater.readthedocs.io/en/stable/configuration/images/#specifying-helm-parameter-names

@chengfang
Copy link
Collaborator

see similar discussion: #1189

@will4j
Copy link
Contributor Author

will4j commented Aug 12, 2025

see similar discussion: #1189

The point is app.helm.image-name annotation is not required in my situation, even if I make it with argocd-image-updater.argoproj.io/app.helm.image-name: image.repository, it will not work as expected, it will generate values file like:

image:
  registry: harbor.example.com
  repository: harbor.example.com/nginx
  tag: "sha-xxxx"

exactly the same problem as mentioned in discussion: #1189

@chengfang
Copy link
Collaborator

Can you update the relevant sections of docs to reflect the new behavior?

Can you also add tests to verify the content of helm values when missing image-name annotation?

@will4j will4j force-pushed the feature/allow-empty-image-name branch from 5e5e8d1 to 27e42a7 Compare August 15, 2025 09:03
@chengfang chengfang force-pushed the feature/allow-empty-image-name branch from 27e42a7 to 334f575 Compare August 15, 2025 15:14
@chengfang
Copy link
Collaborator

Can you also update the docs file: docs/configuration/images.md, e.g., around line 359, to describe the new behavior, that if the <image_alias>.helm.image-tag annotation is set but <image_alias>.helm.image-name annotation is not set, image updater will update the image tag value in the target helm values file, without updating the image name value.

@will4j
Copy link
Contributor Author

will4j commented Aug 18, 2025

@chengfang I'm reconsidering the situation that image.name is the default value of anntotation helm.image-name, which used in both UpdateApplication flow and marshalParamsOverride flow. currently, this pr only handle marshalParamsOverride flow with helmvalues but marshalParamsOverride with default target (which is .argocd-source-*.yaml file) keeps unchanged. I'm not sure if this inconsistent will cause new problem, in addition, I have to confirm that this pr will not affect the UpdateApplication flow. After solving these uncertainty, I will update the doc files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants