[kube-prometheus-stack]: faulty logic when choosing imageRegistry

[dminca]

What this PR does / why we need it

Choose which imageRegistry to use based on whichever is defined.

Currently if .Values.global.imageRegistry is set to quay.io for example, this image will fail to be pulled because it's located in a different registry.

Therefore, the .Values.prometheusOperator.admissionWebhooks.patch.image.registry must take precedence over the former since this image is located in a different registry.

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

• fixes #

Special notes for your reviewer

Checklist

• DCO signed
• Chart Version bumped
• Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

[jkroepke]

The current logic works as indented. The Values.global.imageRegistry is primary designed for registry mirrors. If you set Values.global.imageRegistry to an mirror which does not contain the patch image, you have to manually override .Values.prometheusOperator.admissionWebhooks.patch.image.registry.

It would be a faulty logic if every image follow the same pattern except one.

[dminca]

@jkroepke I agree, that's how it should work. However the situation is different.

Let's say you are mirroring quay.io since that's where majority of the images are so you set .Values.global.imageRegistry to quay-io-mirror.

But in reality the image ingress-nginx/kube-webhook-certgen is pulled from registry.k8s.io so then you decide to overwrite .Values.prometheusOperator.admissionWebhooks.patch.image.registry and point to the k8s-io-mirror.

The problem's .Values.prometheusOperator.admissionWebhooks.patch.image.registry is not overwritten, instead .Values.global.imageRegistry is used.

Hope I managed to explain it properly so that you understand the issue here 😅

[jkroepke]

Okay, I understand that Values.global.imageRegistry always wins and I agree this should not be the case.

But its wrong everywhere, right?

https://github.com/search?q=repo%3Aprometheus-community%2Fhelm-charts+Values.global.imageRegistry+path%3A%2F%5Echarts%5C%2Fkube-prometheus-stack%5C%2F%2F&type=code

[dminca]

@jkroepke it's not wrong everywhere. Only in the places where the image is located in a different container registry from quay.io. If you set .Values.global.imageRegistry do you really want to overwrite also Grafana and anything else unrelated?

That's the problem with the logic here. If all images used by this helm chart would reside in the same container registry then this wouldn't even be required. But here we've got a mixture of registries which users have to be able to overwrite since they're exceptions

[jkroepke]

In general, I for an constant behavior. An specific registry definition should always prefer over global.imageRegistry and as I understand, this is not the case, nowhere. Right?

[dminca]

hmm, I guess so. I'm also for consistency, shall I go ahead and apply this everywhere?

[jkroepke]

Yes please