feat: Add Gateway API support to Helm chart (HTTPRoute)#24979
feat: Add Gateway API support to Helm chart (HTTPRoute)#24979dtherhtun wants to merge 6 commits intoBerriAI:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds optional Kubernetes Gateway API support ( Confidence Score: 5/5Safe to merge — all previous review concerns are resolved, the feature is fully opt-in, and tests cover the key scenarios. All four prior review issues (missing No files require special attention.
|
| Filename | Overview |
|---|---|
| deploy/charts/litellm-helm/templates/httproute.yaml | New HTTPRoute template with required guard on gatewayName, conditional rendering for all optional parentRef fields, and correct use of $ scope in range loop. |
| deploy/charts/litellm-helm/tests/http_route_tests.yaml | Comprehensive helm unit tests covering disabled-by-default, parentRefs, hostnames, annotations, custom matches/backendRefs; all are mock-only (no network calls). |
| deploy/charts/litellm-helm/values.yaml | New httpRoute block added with gatewayName: "" default ensuring the required guard fires, all optional fields empty by default, and enabled: false preserving backward compatibility. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[helm install / upgrade] --> B{httpRoute.enabled?}
B -- false --> C[No HTTPRoute rendered\nIngress used as before]
B -- true --> D{gatewayName set?}
D -- no / empty --> E[Helm required helper\nfails with clear error]
D -- yes --> F[Render HTTPRoute v1]
F --> G[parentRefs: name + optional namespace/group/kind/sectionName]
F --> H{httpRoute.hostnames set?}
H -- yes --> I[spec.hostnames rendered]
H -- no --> J[no hostnames field]
F --> K{httpRoute.rules set?}
K -- yes --> L[Iterate rules\ndefault matches/backendRefs if omitted]
K -- no --> M[Default rule:\nPathPrefix / → litellm service:port]
Reviews (5): Last reviewed commit: "fix: update comment as AI said" | Re-trigger Greptile
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Agentic Code Review✅ LGTM — No significant issues found in PR #24979. |
Agentic Code Review🚨 Needs Changes —
|
|
Tip: Greploop — Automatically fix all review issues by running Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal. |
ShashankFC
left a comment
There was a problem hiding this comment.
Agentic Code Review
🚨 Needs Changes —
deploy/charts/litellm-helm/templates/httproute.yaml
⚠️ [MAJOR] Line 14: Missingrequiredvalidation ongatewayName: whenhttpRoute.enabled=truebutgatewayNameis left at its default empty string, the template silently rendersname: ""inparentRefs, producing an invalid HTTPRoute that Kubernetes will reject. Add{{ required "httpRoute.gatewayName is required when httpRoute.enabled is true" .Values.httpRoute.gatewayName }}to catch this athelm template/helm installtime instead of at apply time.confidence=7 file_read line 14 shows
name: {{ .Values.httpRoute.gatewayName }}with no guard; values.yaml line 142 confirms default isgatewayName: ""; grep of tests/http_route_tests.yaml shows all 7 tests supply gatewayName explicitly and none test the empty case.⚠️ [MAJOR] Line 15: Thenamespacefield inparentRefsis always emitted, falling back to.Release.NamespacewhengatewayNamespaceis unset. Per the Gateway API spec, omittingnamespacemeans "same namespace as the HTTPRoute," but explicitly setting it to the release namespace breaks cross-namespace setups where the Gateway lives in a different namespace (e.g.,infra-system): the route will silently reference a non-existent Gateway in the wrong namespace. The field should be conditional — only rendered whengatewayNamespaceis explicitly provided.confidence=7 file_read line 15:
namespace: {{ default .Release.Namespace .Values.httpRoute.gatewayNamespace }}; values.yaml line 143:gatewayNamespace: ""; the unconditional fallback changes spec-defined default behavior (omit = local namespace) to always emitting a namespace value.
deploy/charts/litellm-helm/values.yaml
⚠️ [MAJOR] Line 142:gatewayNamedefaults to""but the template renders it unconditionally asname: {{ .Values.httpRoute.gatewayName }}(httproute.yaml line 14) — no{{- required }}guard exists anywhere in the template. A user who setshttpRoute.enabled: truewithout supplyinggatewayNamewill silently produce an HTTPRoute withname: "", which is rejected by the Kubernetes API server. Add a{{- required "httpRoute.gatewayName is required when httpRoute.enabled is true" .Values.httpRoute.gatewayName }}guard in the template, or at minimum add a prominent comment here marking this field as required.confidence=7 file_read of deploy/charts/litellm-helm/templates/httproute.yaml line 14 shows
name: {{ .Values.httpRoute.gatewayName }}with no conditional/required guard. bash grep for 'required|fail' across all templates returned 0 results. values.yaml line 142 setsgatewayName: ""as default.
Generated by Agentic Reviewer · model: us.anthropic.claude-sonnet-4-6
ShashankFC
left a comment
There was a problem hiding this comment.
Agentic Code Review
🚨 Needs Changes — 🚨 1 critical
deploy/charts/litellm-helm/templates/httproute.yaml
- 🚨 [CRITICAL] Line 14: Missing
requiredguard ongatewayName: if a user setshttpRoute.enabled: truewithout overridinggatewayName(which defaults to""in values.yaml), the template rendersname:— an empty string — producing an invalid HTTPRoute that Kubernetes will reject. Use{{ required "httpRoute.gatewayName is required when httpRoute.enabled is true" .Values.httpRoute.gatewayName }}to fail fast at render time with a clear error message.confidence=7 file_read line 14 shows
- name: {{ .Values.httpRoute.gatewayName }}with no validation. values.yaml (file_read line 142) shows the default isgatewayName: "". The test suite (http_route_tests.yaml) always sets gatewayName explicitly and has no test covering the empty-default path. ⚠️ [MAJOR] Line 15:namespaceis unconditionally emitted on every parentRef, defaulting to.Release.NamespacewhengatewayNamespaceis not set. The Gateway API spec treats an omittednamespaceas "same namespace as the HTTPRoute", so the default behavior is semantically equivalent for same-namespace gateways — but emitting an explicit namespace value for a cross-namespace Gateway (where the user intentionally omitsgatewayNamespace) will inject the release namespace rather than leaving it absent, potentially causing the HTTPRoute to target the wrong parent. The PR diff's original code used{{- with (default "" .Values.httpRoute.gatewayNamespace) }}to only emitnamespace:when explicitly provided; the committed version changed this to always emit it.confidence=7 file_read line 15 shows
namespace: {{ default .Release.Namespace .Values.httpRoute.gatewayNamespace }}— unconditional. values.yaml line 143 shows defaultgatewayNamespace: "". The diff patch shows the original template used a conditional{{- with (default "" .Values.httpRoute.gatewayNamespace) }}block, meaning the committed file diverged from the diff. No test in http_route_tests.yaml covers the case where gatewayNamespace is unset and verifies what namespace value is (or isn't) rendered.
deploy/charts/litellm-helm/values.yaml
⚠️ [MAJOR] Line 142:gatewayNamedefaults to""but the template renders it unconditionally asname: {{ .Values.httpRoute.gatewayName }}with norequiredorfailguard. If a user setshttpRoute.enabled: truewithout providinggatewayName, Helm will silently render an HTTPRoute withname: "", producing an invalid Kubernetes resource that will be rejected by the API server at apply time. Add a{{ required "httpRoute.gatewayName is required when httpRoute.enabled is true" .Values.httpRoute.gatewayName }}guard in the template, or document this as a required field.confidence=7 file_read of deploy/charts/litellm-helm/templates/httproute.yaml line 14 shows
- name: {{ .Values.httpRoute.gatewayName }}with no required/fail guard. grep confirmed norequiredorfailcalls anywhere in httproute.yaml. values.yaml line 142 defaults gatewayName to empty string.
Generated by Agentic Reviewer · model: us.anthropic.claude-sonnet-4-6
Relevant issues
Fixes #24973
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
deploy/charts/litellm-helm/tests/http_route_tests.yamldirectory, Adding at least 1 test is a hard requirement - see detailshelm unittest -f 'tests/*.yaml' deploy/charts/litellm-helm@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewType
🆕 New Feature
🚄 Infrastructure
Changes
Added optional support for Kubernetes Gateway API in the Helm chart
Introduced templates for:
HTTPRouteAdded new
httpRouteconfiguration block in values.yaml:httpRoute.enabled flag to toggle the feature
Configurable fields for HTTPRoute resources
Ensured Gateway API resources are only created when explicitly enabled
Maintained backward compatibility by keeping Ingress as the default behavior