Skip to content
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

Namespace the global.image.registry (or update docs to explicitly allow the old keys) #12624

Closed
dominykas opened this issue Jan 3, 2025 · 2 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@dominykas
Copy link

dominykas commented Jan 3, 2025

#12028 moved all the configuration for the image registries under global.image.registry. This is a generic location and would work very nicely for all the charts and subcharts if one is only ever pulling from registry.k8s.io, but more often than not - multiple different registries are involved when combining multiple charts, so this setting will in practice only apply to ingress-nginx.

That's not an issue in itself, although the potential for conflicts if other charts decide to use the same key does exist. A bigger problem is that the key itself does not have any indication that it is ingress-nginx specific - which could be misleading and confusing.

There still is value in using global, however can it be namespaced at global.ingress-nginx.image.registry?

Alternatively, the code still supports controller.image.registry, but it was removed from the README - does that mean it's deprecated and will be removed or can it be kept around?

@dominykas dominykas added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 3, 2025
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Jan 3, 2025
@dominykas dominykas changed the title Namespace the global.image.registry Namespace the global.image.registry (or update docs to explicitly allow the old keys) Jan 3, 2025
@Gacko
Copy link
Member

Gacko commented Jan 4, 2025

This is intended behavior and a best practice among a lot of charts I know. You will probably find a lot of people rather asking for this feature than for still having it namespaced.

The reason is quite simple: Often users tend to copy images they use to their local registries, so they do not rely on external registries to be available all the time. Also this approach reduces the outbound bandwidth usage as you only download images once and then can share them using your local network.

We didn't choose this specific key name just because we wanted to, we chose it because it's the same for other charts. If you still want to use a separate registry, you can override controller.image.registry as we are only using values from the global.image object that are not existing in the component specific image object like controller.image or defaultBackend.image.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

No branches or pull requests

3 participants