-
Notifications
You must be signed in to change notification settings - Fork 141
Description
There are a couple of issues with the current Route template, specifically the implementation of https redirects.
helm-charts/charts/open-webui/templates/route.yaml
Lines 54 to 64 in 71222d2
| parentRefs: | |
| {{- range .Values.route.parentRefs }} | |
| - {{ toYaml (omit . "sectionName") | nindent 4 | trim }} | |
| {{- if $.Values.route.httpsRedirect }} | |
| sectionName: https | |
| {{- else if not .sectionName }} | |
| sectionName: http | |
| {{- else }} | |
| sectionName: {{ .sectionName }} | |
| {{- end }} | |
| {{- end }} |
-
If
sectionNameis not specified inparentRefs, the template will forcesectionName: httpto be applied. parentRefs should instead be left untouched unless httpsRedirect is enabled, as it's common to not specify a sectionName and allow the route to bind to all listeners. -
With
httpsRedirect: true, thesectionNamein the main https route is not configurable, and is hardcoded tohttps. Configuration ofsectionNamein what you would assume to be the main route configuration is actually reflected in the new redirect route.
I can see the benefit in having an httpsRedirect option, but right now the current implementation is restrictive. A configuration option should be available to change either sectionName or parentRefs in the redirect route. parentRefs in the main route should be left as is, unless sectionName is omitted, where a default of https can be used.
The better approach for this, which a lot of charts are doing, is to allow individual configuration of separate routes in the route: section, similar to this:
route:
main:
# main route, including parentRefs pointing to https listener
redirect:
# redirect route, including parentRefs pointing to http listener
httpsRedirect: true
Unfortunately this will likely be a breaking change but I would consider making it now before GatewayAPI starts to overtake Ingress.