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

[sophora-dashboard] introduce global value for "registry" #84

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

muffl0n
Copy link
Contributor

@muffl0n muffl0n commented Mar 25, 2024

Makes it much easier to override the default registry. Otherwise one would have to override each and every declaration of repository. Far worse, new entries would go undetected if not checked externally with e.g. Kyverno.

This change should be applied to all charts.

kube-prometheus-stack has a similar approach leveraging global.imageRegistry: https://github.com/prometheus-community/helm-charts/blob/main/charts/kube-prometheus-stack/values.yaml#L223

Makes it much easier to override the default registry. Otherwise one would have to override each and every declaration of "repository".
Far worse, new repository entries would go undetected if not checked externally with e.g. Kyverno.

This change should be applied to all charts.
@muffl0n
Copy link
Contributor Author

muffl0n commented Apr 2, 2024

On a second thought: We should probably add something like this

registries: 
  subshell: docker.subshell.com

instead of simply

registry: docker.subshell.com

so that we can add third-party registries more easily:

registries: 
  subshell: docker.subshell.com
  quay: quay.io

@philmtd
Copy link
Member

philmtd commented Apr 8, 2024

I did not see this anywhere else, and I doubt that there is much benefit. I understand all this intends to do is to make it easier to replace a registry with a proxy, which now would require two lines instead of one, right? I'd rather vote to close this and leave it as it is. kube-prometheus-stack has way more than two images it pulls, so I think it's more justified there.

@muffl0n
Copy link
Contributor Author

muffl0n commented Apr 8, 2024

Importer is pulling three images, server is pulling four.
But I get your point and would be okay with closing this PR.

@muffl0n
Copy link
Contributor Author

muffl0n commented Apr 8, 2024

@philmtd philmtd self-requested a review April 8, 2024 16:09
@philmtd
Copy link
Member

philmtd commented Apr 9, 2024

Okay. I think we could do a section like this:

global:
  imageRegistry: ""

This is what Bitnami and KPS do and what seems to be the most standard way of doing it.

@muffl0n
Copy link
Contributor Author

muffl0n commented Apr 15, 2024

Changed the PR to reflect this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants