feat: add Gateway API HTTPRoute support - #133
Open
charlychiu wants to merge 2 commits into
Open
Conversation
Adds sourcebot.httpRoute values and a templates/httproute.yaml rendering a gateway.networking.k8s.io/v1 HTTPRoute. When rules is empty a default rule forwarding all traffic to the Sourcebot service is generated. AUTH_URL is now also derived from sourcebot.httpRoute.hostnames when ingress is disabled. Previously it was only set from ingress.hosts, so disabling ingress to use a Gateway made Sourcebot fall back to http://localhost:3000, breaking login callbacks and search result links. Ingress takes precedence over httpRoute for AUTH_URL, so existing installations are unaffected.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Adds a helm-unittest suite for the HTTPRoute template and the AUTH_URL logic, matching the existing tests/basic_test.yaml conventions. Also fixes two gaps found while writing the tests: enabling httpRoute without parentRefs now fails with an explanatory message instead of silently rendering a route attached to no Gateway, and AUTH_URL derivation skips wildcard hostnames, which are common on Gateway listeners and produced an unusable URL. AUTH_URL derivation moved into a sourcebot.authUrl helper. The rendered output for existing ingress configurations is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds Gateway API
HTTPRoutesupport to the chart, as an alternative to the existing Ingress.Two parts:
templates/httproute.yaml— renders agateway.networking.k8s.io/v1HTTPRoute, driven by newsourcebot.httpRoute.*values. Whenrulesis empty, a default rule forwarding all traffic to the Sourcebot service is generated, so the common case is justenabled+parentRefs+hostnames.AUTH_URLalso derived fromhttpRoute.hostnames— this is the part I'd like to highlight, because a template-only change would be incomplete.Why the
AUTH_URLchange is neededToday
AUTH_URLis set only fromingress.hosts[0]:https://github.com/sourcebot-dev/sourcebot-helm-chart/blob/main/charts/sourcebot/templates/deployment.yaml#L63-L66
Anyone who sets
ingress.enabled=falsein order to route traffic through a Gateway losesAUTH_URLentirely, and the entrypoint falls back tohttp://localhost:3000. That silently breaks login callbacks, links in search results, and the MCP OAuth issuer — with no error at deploy time, which makes it fairly unpleasant to debug.We hit exactly this when migrating an internal deployment from Ingress to Gateway API, and worked around it by hand-setting
AUTH_URLviaadditionalEnv. Since every user switching toHTTPRoutewould hit the same thing, it seemed better to fix it in the chart.The derivation now lives in a
sourcebot.authUrlhelper. It prefers the first ingress host, falls back to the first non-wildcardhttpRoutehostname, and returns nothing otherwise. Wildcards are skipped becausehttps://*.example.comis not a usable URL, and wildcard listeners are common with Gateway API.Behaviour
main.AUTH_URL. Existing installations are unaffected, and exactly oneAUTH_URLentry is ever emitted.httpRoutewithoutparentRefsfails the render with an explanatory message, rather than producing a route attached to no Gateway that silently receives no traffic.httpRoute— TLS is terminated at the Gateway listener.Three decisions I'd like your take on
apiVersionis hardcoded togateway.networking.k8s.io/v1rather than detected via.Capabilities. This matches howingress.yamlhardcodesnetworking.k8s.io/v1. Happy to switch to capability detection if you'd prefer.ingressandhttpRouteare not mutually exclusive — enabling both renders both objects rather than failing. I assumed people may want both during a migration, and only madeAUTH_URLpick a winner. If you'd rather this be a hardfail, that's a small change.A wildcard-only
hostnameslist produces noAUTH_URL— and therefore thehttp://localhost:3000fallback this PR is trying to eliminate. I want to flag this explicitly rather than leave it in a values comment, because it is the same failure mode, just narrowed to one configuration.I chose not to
failthere because a wildcard-only HTTPRoute is a legitimate Gateway API configuration, and the operator may be supplyingAUTH_URLthroughenvFrom, whose contents the template cannot inspect — so a hard failure would have false positives. CheckingadditionalEnvfor anAUTH_URLentry would not close that gap either.The options I see are: leave it documented as it is;
failand accept the false positives; or emit ahelm.sh/hook-free warning, which Helm does not really have a mechanism for. Happy to go whichever way you prefer.Also happy to rename the values key (
httpRoutevsgatewayvsgatewayApi) if you have a preference.Testing
helm unittest charts/sourcebot/— 29 passed, including 11 new cases intests/httproute_test.yamlcovering: not rendered by default, rendering when enabled, the generated default rule, customrulesoverriding it, annotations, theparentRefsfailure,AUTH_URLderived from a hostname, wildcard hostnames skipped, noAUTH_URLwhen all hostnames are wildcards, ingress winning when both are enabled, and noAUTH_URLwhen neither is enabled.helm lint charts/sourcebot/ -f charts/sourcebot/values.lint.yaml— passeshelm-docs(v1.14.2, same as CI) run and committed; a second run produces no diffhelm templatewith default values, and with an ingress configuration, produces output identical tomainOne small thing unrelated to this PR:
CONTRIBUTING.mddocuments the test command ashelm test sourcebot, which is the command forhelm testhooks. The chart's tests are helm-unittest specs, run withhelm unittest charts/sourcebot/and needing no cluster. Happy to fix that in a separate PR if useful.