feat(helm): add optional networking.k8s.io/v1 Ingress for the UI#2238
feat(helm): add optional networking.k8s.io/v1 Ingress for the UI#2238elihai-risk wants to merge 1 commit into
Conversation
The chart exposes the UI via Gateway API HTTPRoute and OpenShift Route, but clusters on classic ingress controllers (AWS ALB, ingress-nginx, Traefik) have no built-in option and must hand-roll an Ingress. Add ui.ingress (disabled by default): className, labels/annotations, hosts/paths with per-path backend override (e.g. to front the UI with the bundled oauth2-proxy), and verbatim tls passthrough. Includes helm-unittest coverage (default off, defaults to the UI Service, backend override, tls/annotations passthrough). Signed-off-by: Elihai Blomberg <elihai.blomberg@riskified.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds an optional networking.k8s.io/v1 Ingress for exposing the UI on clusters that use classic ingress controllers, with per-path backend overrides, annotations/labels, and TLS passthrough.
Changes:
- Add
ui.ingressvalues (disabled by default) to configure a classic Ingress. - Add
ui-ingress.yamlHelm template renderingnetworking.k8s.io/v1Ingress with per-path backend override. - Add helm-unittest coverage for default-off, default backend, override backend, and tls/annotations passthrough.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| helm/kagent/values.yaml | Adds ui.ingress configuration schema and examples for classic Ingress. |
| helm/kagent/templates/ui-ingress.yaml | Implements optional UI Ingress rendering with per-host/path rules and backend override. |
| helm/kagent/tests/ui-ingress_test.yaml | Adds unit tests validating render behavior and key fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: {{ (.backend).name | default (printf "%s-ui" (include "kagent.fullname" $)) }} | ||
| port: | ||
| number: {{ (.backend).port | default $.Values.ui.service.ports.port }} |
| rules: | ||
| {{- range .Values.ui.ingress.hosts }} | ||
| - host: {{ .host | quote }} | ||
| http: | ||
| paths: | ||
| {{- range .paths }} |
| {{- end }} | ||
| spec: | ||
| {{- with .Values.ui.ingress.className }} | ||
| ingressClassName: {{ . }} |
| http: | ||
| paths: | ||
| {{- range .paths }} | ||
| - path: {{ .path }} |
|
Hey there, I really appreciate the PR but frankly I think this is too specific for the core kagent helm chart, this could be a good use case for extraObjects as proposed in #2174 |
|
Addressed the review: fail-fast on |
|
Fair call — |
What
Adds an optional classic
networking.k8s.io/v1Ingress for the UI (ui.ingress, disabled by default) — className, labels/annotations, hosts/paths with per-path backend override, andtlspassthrough.Why
The chart currently exposes the UI via Gateway API
HTTPRouteand OpenShiftRoute, but clusters on classic ingress controllers (AWS ALB, ingress-nginx, Traefik) have no built-in option and must hand-roll an Ingress out-of-band. We hit this deploying kagent 0.9.11 on EKS behind an internal ALB.The per-path
backendoverride covers the common auth pattern of pointing the Ingress at the bundled oauth2-proxy instead of the UI Service:Notes
ui.httpRouteconventions (labels merged with chart labels, annotations passthrough, backend defaults to the UI Service onui.service.ports.port).Related: #2237 (podLabels for controller/UI pods) — together these let the chart run unmodified behind cluster admission policies + classic ingress.
🤖 Generated with Claude Code