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

set an explicit ingress class name #484

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aogier
Copy link
Contributor

@aogier aogier commented Jan 7, 2025

This patch allows for specifying an explicit ingress class name, which is often needed in real-world prod clusters.

Thank you, regards

@@ -94,6 +94,8 @@ ingress:
# - secretName: chart-example-tls
# hosts:
# - zulip.example.com
# set an explicit class name
className:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to have a default value here, like "zulip"? I feel like probably that would be a reasonable default name for most deployments of this, but maybe I'm missing something about why someone would want something different, or potential breakage it could cause.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, in this context the class name is basically a label I attach to a specific ingress controller instance, if eg. my k8s cluster is on multiple networks and I need to place an ingress on a specific network. In simpler (and very common) setups, it is common to have a single ingress controller and an empty classname would select this one if you mark it as default see manual:

❯ k get ingressclasses.networking.k8s.io nginx -o yaml
apiVersion: networking.k8s.io/v1
kind: IngressClass
metadata:
  annotations:
    ingressclass.kubernetes.io/is-default-class: "true"
  name: nginx
spec:
  controller: k8s.io/ingress-nginx

so a sane default could be to have it as an empty string, which produces the same design as the current chart, but one can configure it should a specific class is needed.
Take a random chart as an example: https://github.com/bitnami/charts/blob/main/bitnami/sonarqube/values.yaml#L611 (comment there is slightly misleading though)

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

Successfully merging this pull request may close these issues.

2 participants