fix(ai): (FTI-6403) Fixed error with Azure path override formatting #14051
+208
−5
Conversation
This file contains 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
9c0c579 to
bedbcc7
Compare
oowl
reviewed
Dec 30, 2024
kong/llm/drivers/azure.lua
Outdated
| ai_shared.override_upstream_url(parsed_url, conf) | ||
|
|
||
| -- if the path is read from a URL capture, 3re that it is valid | ||
| parsed_url.path = string_gsub(parsed_url.path, "^/*", "/") | ||
| kong.log.warn("JJ: ", parsed_url.path) |
| @@ -118,10 +118,18 @@ function _M.configure_request(conf) | |||
| parsed_url = socket_url.parse(url) | |||
| end | |||
|
|
|||
| -- if the path is read from a URL capture, ensure that it is valid | |||
| parsed_url.path = string_gsub(parsed_url.path, "^/*", "/") | |||
There was a problem hiding this comment.
for what case? it seems not possible to get a dirty URL path.
There was a problem hiding this comment.
This was added because it was reported that it broke when you set like this:
config:
model:
options:
upstream_url: http://localhost:8080for example.
I don't want to remove this, it was to fix a very specific issue for someone.
| -- Azure-specific - prepend the deployment instance path to the | ||
| -- user-override path. | ||
| if conf.model.options and conf.model.options.upstream_path then | ||
| conf.model.options.upstream_path = parsed_url.path |
There was a problem hiding this comment.
The dynamic change conf in runtime is wired for me. we need to rebuilt route in here I think
if conf.model.options.upstream_url then
parsed_url = socket_url.parse(conf.model.options.upstream_url)
else
-- azure has non-standard URL format
local url = fmt(
"%s%s",
ai_shared.upstream_url_format[DRIVER_NAME]:format(conf.model.options.azure_instance, conf.model.options.azure_deployment_id),
conf.model.options
and conf.model.options.upstream_path
or ai_shared.operation_map[DRIVER_NAME][conf.route_type]
and ai_shared.operation_map[DRIVER_NAME][conf.route_type].path
or "/"
)
parsed_url = socket_url.parse(url)
end
There was a problem hiding this comment.
@oowl I don't understand the proposal :( can you just push the change in and run the CI?
My tests absolutely 100% covers the client's issue now.
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
There was a bug found, where the Azure provider stopped parsing 'custom upstream path' overrides since Kong 3.9.0.
Every request would 404 due to incorrect parsing.
I've added a parser / config options test to prevent this from happening again!
Checklist
changelog/unreleased/kongorskip-changeloglabel added on PR if changelog is unnecessary. README.mdIssue reference
FTI-6403
AG-191