bjw-s / helm-charts

A collection of Helm charts
https://bjw-s.github.io/helm-charts/
Apache License 2.0
535 stars 100 forks source link

Image tag doesn't default to Chart's AppVersion #232

Closed BapRx closed 7 months ago

BapRx commented 7 months ago

Details

What steps did you take and what happened:

I'm trying to build a template using the common chart without providing an image tag in order to use the appVersion defined in Chart.yaml, I think that the behavior I'm expecting is defined here.

This default expression is never used because of the tag validation which prevents me from providing an empty value or omitting the tag altogether. Am I missing anything here?

What did you expect to happen:

I'm expecting the rendered image to be something like image.repository:.Chart.AppVersion, instead here's what I get:

❯ rg appVersion:
Chart.yaml
6:appVersion: 0.1.0

❯ yq e -o p values.yaml | grep tag
controllers.main.containers.main.image.tag =

❯ helm template . | rg image:
Error: execution error at (jellyfin/templates/common.yaml:2:3): No image tag specified for container. (controller main, container main)

Use --debug flag to render out invalid YAML
❯ yq e -o p values.yaml | grep tag
controllers.main.containers.main.image.tag = null

❯ helm template . | rg image:
Error: execution error at (jellyfin/templates/common.yaml:2:3): No image tag specified for container. (controller main, container main)

Use --debug flag to render out invalid YAML
❯ yq e -o p values.yaml | grep tag
controllers.main.containers.main.image.tag = test

❯ helm template . | rg image:
          image: linuxserver/jellyfin:test

Additional Information:

common chart v2.3.0

bjw-s commented 7 months ago

Hi, thanks for creating the elaborate issue! I can see how you could think that the validation is the cause of this, but in reality the "bug" is that I still have a default $rootContext.Chart.AppVersion $containerObject.image.tag statement in the image tag definition.

Since the move to the multi-controller/multi-container setup I felt it does not really make sense to have all image tags default to the chart's appVersion field.

If you want to replicate the old-style behavior you can do something like this: https://github.com/bjw-s/helm-charts/blob/main/charts/apps/k8s-ycl/templates/common.yaml#L12

Additionally in an upcoming version I will make it so that you can do this in your values.yaml, since that looks a lot cleaner:

image:
  repository: jellyfin/jellyfin
  tag: "{{.Chart.AppVersion}}"
BapRx commented 7 months ago

Hi, thank you for you quick and great answer, I understand now what's going on and I applied the "hardcoded" method which works great.

Thanks again for this chart and have a nice weekend!