Enable support for Fully Qualified Repository url without combining registry and repository

Description

The helper methods responsible for parsing the image specified in the values files requires the seperation of registry and repository. This applies to every _helper.tpl method with the name ".image" (example shown here with azure-metrics-exporter: charts/azure-metrics-exporter/templates/_helpers.tpl#L69) Code in question: _helpers.tpl

...
{{/*
The image to use
*/}}
{{- define "azure-metrics-exporter.image" -}}
{{- if .Values.image.sha -}}
{{- printf "%s/%s:%s@%s" .Values.image.registry .Values.image.repository (default (printf "%s" .Chart.AppVersion) .Values.image.tag) .Values.image.sha }}
{{- else -}}
{{- printf "%s/%s:%s" .Values.image.registry .Values.image.repository (default (printf "%s" .Chart.AppVersion) .Values.image.tag) }}
{{- end }}
{{- end }}
...

deployment.yaml

...
      containers:
        #######################
        # Kube pool manager
        #######################
        - name: azure-metrics-exporter
          image: {{ include "azure-metrics-exporter.image" . | quote }}
          imagePullPolicy: {{ .Values.image.pullPolicy | quote }}
...

Thus when specifying the repository as a Fully Qualified url, the Helm templating ends up failing to invalid image url: values.yaml

image:
  registry: ""
  repository: "myorg.myregistry.io/azure-metrics-exporter" 
  tag: "22.9.0-2023-09-25-16-27"

Expected behaviour deployment.yaml

...
      containers:
        #######################
        # Kube pool manager
        #######################
        - name: azure-metrics-exporter
          image: "myorg.myregistry.io/azure-metrics-exporter:22.9.0-2023-09-25-16-27"
...

Actual behaviour deployment.yaml

...
      containers:
        #######################
        # Kube pool manager
        #######################
        - name: azure-metrics-exporter
          image: "/myorg.myregistry.io/azure-metrics-exporter:22.9.0-2023-09-25-16-27"
...

Suggestion

I want to submit a Pull Request that can solve this issue without changing the default behaviour. I have two proposals:

Check for empty registry. In this case, the _helpers.tpl definition of ".image" (fx {{- define "azure-metrics-exporter.image" -}}) would allow the registry to be empty, like so:

{{- define "azure-metrics-exporter.image" -}}
{{- if .Values.image.registry -}}
{{- $imageQualifier := (printf "%s/%s" .Values.image.registry .Values.image.repository) -}}
{{- else -}}
{{- $imageQualifier := .Values.image.repository -}}
{{- end }}
{{- if .Values.image.sha -}}
{{- printf "%s:%s@%s" $imageQualifier (default (printf "%s" .Chart.AppVersion) .Values.image.tag) .Values.image.sha }}
{{- else -}}
{{- printf "%s:%s" $imageQualifier (default (printf "%s" .Chart.AppVersion) .Values.image.tag) }}
{{- end }}
{{- end }}

Allow an overwriting repository qualification In this case, _helpers.tpl definition of ".image" (fx {{- define "azure-metrics-exporter.image" -}}) would check for a variable in values.yaml that will override the returning image, like so:

{{/*
The image to use
*/}}
{{- define "azure-metrics-exporter.image" -}}
{{- if .Values.image.repositoryOverride -}}
{{- .Values.image.registryOverride }}
{{- else if .Values.image.sha -}}
{{- printf "%s/%s:%s@%s" .Values.image.registry .Values.image.repository (default (printf "%s" .Chart.AppVersion) .Values.image.tag) .Values.image.sha }}
{{- else -}}
{{- printf "%s/%s:%s" .Values.image.registry .Values.image.repository (default (printf "%s" .Chart.AppVersion) .Values.image.tag) }}
{{- end }}
{{- end }}

Concerns

I have seen that this implementation of image resolution is very similar to the implementation in prometheus-community/helm-charts/charts/kube-state-metrics/templates/_helpers.tpl#L123. This was introduced in the kube-state-metrics by this issue: [kube-state-metrics] Image breaks when passing global.imageRegistry #3075 . And the PR that fixed it mentioned that it was done due to conform to other Helm charts they manage:

We need to split registry and repository values in a proper way as usual in another charts.

I am wondering if the WebDevOps team would like to keep it consistent with such implementations? If that is the case, then I will perhaps suggest it to the prometheus-community, as they have conflicting implementations.

Sidenote: Why don't I just use the values separately?

I sadly cannot specify these registry and repository values individually. For my usecase, I do a cached build of every public image, incase I need to patch any vulnerability, and thus I have my own image hosted on a seperate Container Registry. This process is automatic, and results in a central values.yaml file, which contains a list of every image cached in the following format:

global:
  images:
    azure-metrics-exporter:
      repository: "myorg.myregistry.io/azure-metrics-exporter"
      tag: "22.9.0-2023-09-25-16-27"

When I then want to use this Helm Chart, I then install it as an Argo CD application pointing to this Chart, and specify Helm values to override:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: azure-metrics-exporter
  ...
spec:
  source:
    ...
    helm:
      values: |
        image:
          repository: {{ .Values.global.images.azure_metrics_exporter.repository | quote }}
          tag: {{ .Values.global.images.azure_metrics_exporter.tag | quote }}

However in this case I can't just input my Fully Qualified Repository url.

I hope that this is okay for me to suggest. I will try and create a Pull Request, with the teams permission. I am leaning more towards my first suggestion, as it would allow specifying both a Fully Qualified Repository url and a tag.

webdevops / helm-charts