fix(cli): propagate url_prefix to FastAPI root_path - #7071
Open
luissilva-mollie wants to merge 2 commits into
Open
fix(cli): propagate url_prefix to FastAPI root_path#7071luissilva-mollie wants to merge 2 commits into
luissilva-mollie wants to merge 2 commits into
Conversation
/docs and /openapi.json ignore --url_prefix because the FastAPI app is constructed without root_path. url_prefix is currently only used to set the dev-ui's backendUrl and the /dev-ui/ redirect, so when the app is served behind a reverse proxy that strips the prefix, Swagger UI's embedded reference to /openapi.json resolves against the bare domain root instead of the proxied path, breaking /docs. Derive root_path from url_prefix (accepting both a bare path like '/adk' and a full absolute URL like 'https://host/adk') and pass it to FastAPI so generated URLs are correctly prefixed.
Parametrized over: no prefix, bare path (with/without trailing slash), and full absolute URL (with/without trailing slash), asserting both app.root_path and that the /docs page's embedded openapi.json reference is correctly prefixed.
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.
Fixes #7070
Problem
--url_prefixis documented as making generated URLs work correctly whenadk webis mounted behind a reverse proxy under a path prefix, but it's currently only used to set the dev-ui'sbackendUrland the/dev-ui/redirect target. It's never passed to theFastAPIapp asroot_path, so/docsand/openapi.json(and any other Starlette-generated URL) are always generated relative to/, breaking them when the app is proxied under a prefix that gets stripped before reaching the app.Fix
Derive
root_pathfromurl_prefixviaurllib.parse.urlparse(url_prefix).pathand pass it intoFastAPI(...). This handles both formsurl_prefixcan take today: a bare path (e.g./adk, per the CLI help text) and a full absolute URL (e.g.https://host/adk, which some deployments pass since the same value is reused for the frontend's absolutebackendUrl).Testing
Verified locally with a minimal FastAPI app that setting
root_pathcauses/docsto reference/<prefix>/openapi.jsoninstead of/openapi.json:No existing tests cover
url_prefix/root_pathbehavior intests/unittests/cli/test_fast_api.py; happy to add coverage if maintainers can point me at the preferred test harness pattern forApiServer.get_fast_api_app/run_server.