Skip to content

Fix helm chart image version#546

Closed
mjudeikis wants to merge 1 commit into
kbind-dev:mainfrom
mjudeikis:fix.chart.install
Closed

Fix helm chart image version#546
mjudeikis wants to merge 1 commit into
kbind-dev:mainfrom
mjudeikis:fix.chart.install

Conversation

@mjudeikis

Copy link
Copy Markdown
Contributor

Summary

What Type of PR Is This?

Make the default image tag tolerate both v-prefixed and unprefixed Chart.appVersion by stripping any leading v and always prepending one. Fixes installs failing with ImagePullBackOff when appVersion is published without the v prefix (e.g. 0.8.1 (how helm charts does it) while images are tagged v0.8.1.

Related Issue(s)

Fixes #

Release Notes

NONE

@mjudeikis mjudeikis requested a review from a team as a code owner May 18, 2026 11:57
cnvergence
cnvergence previously approved these changes May 18, 2026
@xrstf

xrstf commented May 19, 2026

Copy link
Copy Markdown
Contributor

when appVersion is published without the v prefix (e.g. 0.8.1 (how helm charts does it) while images are tagged v0.8.1.

Why? We control the appVersion, no? Why do we make the chart resilient against ourselves here? I am so confused... Why would the chart ever be published with a broken appVersion field?

@cnvergence

Copy link
Copy Markdown
Member

thought it was for the dev setup, now I see that latest tagged helm chart has a wrong appVersion, we should fix it

@mjudeikis

mjudeikis commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

It might be now knowing helm again. But helm charts (OCI charts) dont accepts v prefix so helm is always 0.8.1 while our image tags are v0.8.1. And when helm is published I think its stomps appVersion withou v prefix.

Maybe its issue with helm publishing?

now I see that latest tagged helm chart has a wrong appVersion

helm complains if OCI helm image is with v :/

@cnvergence

Copy link
Copy Markdown
Member

well, I ran into that when running development image and helm chart

@mjudeikis

Copy link
Copy Markdown
Contributor Author

So either this or we need to make sure helm charts stays without v and appversion is with v prefix (if possible)

@xrstf

xrstf commented May 19, 2026

Copy link
Copy Markdown
Contributor

Helm chart's version fields must be a valid semver, without the leading v prefix. But appVersion can be anything and depends on the wrapped application. For Minio you'd have appVersion: "RELEASE-2026....", for example.

Comment thread hack/helm-build.sh
Comment on lines 35 to +36
sed -i.tmp "s/^version:.*/version: $CHART_VERSION/" "${chart_dir}Chart.yaml"
sed -i.tmp "s/^appVersion:.*/appVersion: $CHART_VERSION/" "${chart_dir}Chart.yaml"
sed -i.tmp "s/^appVersion:.*/appVersion: v$CHART_VERSION/" "${chart_dir}Chart.yaml"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I vote for a cleanup:

  1. Rename CHART_VERSION in the image workflow to APP_VERSION. Also do not strip the leading v off it anymore (i.e. instead of running this script with CHART_VERSION=0.8.1, it should be run with APP_VERSION=v.0.8.1).
  2. Strip the leading v here in this script to form a valid version for the Helm chart.
  3. Replace version and appVersion in the Chart.yaml with 0.0.0 to indicate that their values are entirely irrelevant, should never be touched and would be overwritten with every kube-bind release anyway. Ideally a small comment # updated by helm-build.sh during the build is placed somewhere in there, too.

My reasoning is that my brain finds the thought

the chart version is used as the app version

more confusing than

the app version is also used as the chart version

^^ Especially since we're not actually maintining a dedicated version just for the chart, we derive it from the appVersion (the Git tag).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do this, wdyt @mjudeikis?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to contribute. I dont see this complex, so if you have ideas how to make it better - take a stab.

@cnvergence

Copy link
Copy Markdown
Member

closing in favor of #554

@cnvergence cnvergence closed this Jun 16, 2026
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