From 713908f565f80ad83a57ac304114252d7e05c3d6 Mon Sep 17 00:00:00 2001 From: Jun Ouyang Date: Thu, 7 Nov 2024 13:27:00 +0800 Subject: [PATCH] chore(patchs):`kong.request.enable_buffering` PDK function can now be used when downstream uses HTTP/2. (#13614) AG-108 FTI-5725 --- Makefile | 2 +- ...-hardcode-limitation-in-ngx-location.patch | 17 ++++++ ...vert-http2-limitation-buffered-request.yml | 4 ++ kong/init.lua | 9 +-- kong/pdk/service/request.lua | 7 --- .../05-proxy/24-buffered_spec.lua | 12 +++- .../02-openai_integration_spec.lua | 12 +++- .../03-anthropic_integration_spec.lua | 12 +++- .../04-cohere_integration_spec.lua | 13 ++++- .../38-ai-proxy/05-azure_integration_spec.lua | 12 +++- .../06-mistral_integration_spec.lua | 12 +++- .../07-llama2_integration_spec.lua | 13 ++++- .../08-encoding_integration_spec.lua | 12 +++- .../09-streaming_integration_spec.lua | 12 +++- spec/internal/client.lua | 56 ++++++++++++++++++- t/01-pdk/06-service-request/00-phase_checks.t | 2 +- 16 files changed, 169 insertions(+), 38 deletions(-) create mode 100644 build/openresty/patches/ngx_lua-0.10.26_07-remove-http2-hardcode-limitation-in-ngx-location.patch create mode 100644 changelog/unreleased/kong/revert-http2-limitation-buffered-request.yml diff --git a/Makefile b/Makefile index 402bdffd0308..42c1f853f4c1 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ OS := $(shell uname | awk '{print tolower($$0)}') MACHINE := $(shell uname -m) -DEV_ROCKS = "busted 2.2.0" "busted-hjtest 0.0.5" "luacheck 1.2.0" "lua-llthreads2 0.1.6" "ldoc 1.5.0" "luacov 0.15.0" +DEV_ROCKS = "busted 2.2.0" "busted-hjtest 0.0.5" "luacheck 1.2.0" "lua-llthreads2 0.1.6" "ldoc 1.5.0" "luacov 0.15.0" "lua-reqwest 0.1.0" WIN_SCRIPTS = "bin/busted" "bin/kong" "bin/kong-health" BUSTED_ARGS ?= -v TEST_CMD ?= bin/busted $(BUSTED_ARGS) diff --git a/build/openresty/patches/ngx_lua-0.10.26_07-remove-http2-hardcode-limitation-in-ngx-location.patch b/build/openresty/patches/ngx_lua-0.10.26_07-remove-http2-hardcode-limitation-in-ngx-location.patch new file mode 100644 index 000000000000..8bcea9fac313 --- /dev/null +++ b/build/openresty/patches/ngx_lua-0.10.26_07-remove-http2-hardcode-limitation-in-ngx-location.patch @@ -0,0 +1,17 @@ +diff --git a/bundle/ngx_lua-0.10.26/src/ngx_http_lua_subrequest.c b/bundle/ngx_lua-0.10.26/src/ngx_http_lua_subrequest.c +index f4db9aa..d887b28 100644 +--- a/bundle/ngx_lua-0.10.26/src/ngx_http_lua_subrequest.c ++++ b/bundle/ngx_lua-0.10.26/src/ngx_http_lua_subrequest.c +@@ -172,12 +172,6 @@ ngx_http_lua_ngx_location_capture_multi(lua_State *L) + return luaL_error(L, "no request object found"); + } + +-#if (NGX_HTTP_V2) +- if (r->main->stream) { +- return luaL_error(L, "http2 requests not supported yet"); +- } +-#endif +- + ctx = ngx_http_get_module_ctx(r, ngx_http_lua_module); + if (ctx == NULL) { + return luaL_error(L, "no ctx found"); diff --git a/changelog/unreleased/kong/revert-http2-limitation-buffered-request.yml b/changelog/unreleased/kong/revert-http2-limitation-buffered-request.yml new file mode 100644 index 000000000000..9f1a19670973 --- /dev/null +++ b/changelog/unreleased/kong/revert-http2-limitation-buffered-request.yml @@ -0,0 +1,4 @@ +message: | + Fixed an issue where the `kong.request.enable_buffering` can not be used when downstream uses HTTP/2. +type: bugfix +scope: Core diff --git a/kong/init.lua b/kong/init.lua index 0313e8aa0855..57441b6e4347 100644 --- a/kong/init.lua +++ b/kong/init.lua @@ -1252,9 +1252,8 @@ function Kong.access() if ctx.buffered_proxying then - local version = ngx.req.http_version() local upgrade = var.upstream_upgrade or "" - if version < 2 and upgrade == "" then + if upgrade == "" then if has_timing then req_dyn_hook_run_hook("timing", "after:access") end @@ -1262,11 +1261,7 @@ function Kong.access() return Kong.response() end - if version >= 2 then - ngx_log(ngx_NOTICE, "response buffering was turned off: incompatible HTTP version (", version, ")") - else - ngx_log(ngx_NOTICE, "response buffering was turned off: connection upgrade (", upgrade, ")") - end + ngx_log(ngx_NOTICE, "response buffering was turned off: connection upgrade (", upgrade, ")") ctx.buffered_proxying = nil end diff --git a/kong/pdk/service/request.lua b/kong/pdk/service/request.lua index 24fd9b15d9d5..f583d390fa14 100644 --- a/kong/pdk/service/request.lua +++ b/kong/pdk/service/request.lua @@ -90,13 +90,6 @@ local function new(self) -- kong.service.request.enable_buffering() request.enable_buffering = function() check_phase(access_rewrite_balancer) - - if ngx.req.http_version() >= 2 then - error("buffered proxying cannot currently be enabled with http/" .. - ngx.req.http_version() .. ", please use http/1.x instead", 2) - end - - ngx.ctx.buffered_proxying = true end diff --git a/spec/02-integration/05-proxy/24-buffered_spec.lua b/spec/02-integration/05-proxy/24-buffered_spec.lua index 15e639c0bb53..011359f5bb3d 100644 --- a/spec/02-integration/05-proxy/24-buffered_spec.lua +++ b/spec/02-integration/05-proxy/24-buffered_spec.lua @@ -6,8 +6,9 @@ local md5 = ngx.md5 local TCP_PORT = helpers.get_available_port() +for _, client_protocol in ipairs({ "http", "https", "http2" }) do for _, strategy in helpers.each_strategy() do - describe("Buffered Proxying [#" .. strategy .. "]", function() + describe("Buffered Proxying [#" .. strategy .. "] [#" .. client_protocol .. "]", function() -- TODO: http2 / grpc does not currently work with -- ngx.location.capture that buffered proxying uses @@ -155,7 +156,13 @@ for _, strategy in helpers.each_strategy() do end) before_each(function() - proxy_client = helpers.proxy_client() + if client_protocol == "http" then + proxy_client = helpers.proxy_client() + elseif client_protocol == "https" then + proxy_client = helpers.proxy_ssl_client() + elseif client_protocol == "http2" then + proxy_client = helpers.proxy_ssl_client(nil, nil, 2) + end proxy_ssl_client = helpers.proxy_ssl_client() end) @@ -272,3 +279,4 @@ for _, strategy in helpers.each_strategy() do end) end) end +end -- for _, client_protocol diff --git a/spec/03-plugins/38-ai-proxy/02-openai_integration_spec.lua b/spec/03-plugins/38-ai-proxy/02-openai_integration_spec.lua index 42a9d7232bac..d7545a856ab0 100644 --- a/spec/03-plugins/38-ai-proxy/02-openai_integration_spec.lua +++ b/spec/03-plugins/38-ai-proxy/02-openai_integration_spec.lua @@ -63,8 +63,9 @@ local _EXPECTED_CHAT_STATS = { }, } +for _, client_protocol in ipairs({ "http", "https", "http2" }) do for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then - describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "]", function() + describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "] [#" .. client_protocol .. "]", function() local client lazy_setup(function() @@ -825,7 +826,13 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) before_each(function() - client = helpers.proxy_client() + if client_protocol == "http" then + client = helpers.proxy_client() + elseif client_protocol == "https" then + client = helpers.proxy_ssl_client() + elseif client_protocol == "http2" then + client = helpers.proxy_ssl_client(nil, nil, 2) + end -- Note: if file is removed instead of trunacted, file-log ends writing to a unlinked file handle truncate_file(FILE_LOG_PATH_STATS_ONLY) truncate_file(FILE_LOG_PATH_NO_LOGS) @@ -1695,3 +1702,4 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) end end +end -- for _, client_protocol diff --git a/spec/03-plugins/38-ai-proxy/03-anthropic_integration_spec.lua b/spec/03-plugins/38-ai-proxy/03-anthropic_integration_spec.lua index 51d7e23ae311..395beb83f841 100644 --- a/spec/03-plugins/38-ai-proxy/03-anthropic_integration_spec.lua +++ b/spec/03-plugins/38-ai-proxy/03-anthropic_integration_spec.lua @@ -6,8 +6,9 @@ local deepcompare = require("pl.tablex").deepcompare local PLUGIN_NAME = "ai-proxy" local MOCK_PORT = helpers.get_available_port() +for _, client_protocol in ipairs({ "http", "https", "http2" }) do for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then - describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "]", function() + describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "] [#" .. client_protocol .. "]", function() local client lazy_setup(function() @@ -515,7 +516,13 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) before_each(function() - client = helpers.proxy_client() + if client_protocol == "http" then + client = helpers.proxy_client() + elseif client_protocol == "https" then + client = helpers.proxy_ssl_client() + elseif client_protocol == "http2" then + client = helpers.proxy_ssl_client(nil, nil, true) + end end) after_each(function() @@ -781,3 +788,4 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) end end +end -- for _, client_protocol diff --git a/spec/03-plugins/38-ai-proxy/04-cohere_integration_spec.lua b/spec/03-plugins/38-ai-proxy/04-cohere_integration_spec.lua index da1a59e5f03d..09f1f5849d83 100644 --- a/spec/03-plugins/38-ai-proxy/04-cohere_integration_spec.lua +++ b/spec/03-plugins/38-ai-proxy/04-cohere_integration_spec.lua @@ -5,9 +5,9 @@ local pl_file = require "pl.file" local PLUGIN_NAME = "ai-proxy" local MOCK_PORT = helpers.get_available_port() +for _, client_protocol in ipairs({ "http", "https", "http2" }) do for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then - describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "]", function() - local client + describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "] [#" .. client_protocol .. "]", function() local client lazy_setup(function() local bp = helpers.get_db_utils(strategy == "off" and "postgres" or strategy, nil, { PLUGIN_NAME }) @@ -390,7 +390,13 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) before_each(function() - client = helpers.proxy_client() + if client_protocol == "http" then + client = helpers.proxy_client() + elseif client_protocol == "https" then + client = helpers.proxy_ssl_client() + elseif client_protocol == "http2" then + client = helpers.proxy_ssl_client(nil, nil, 2) + end end) after_each(function() @@ -624,3 +630,4 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) end end +end -- for _, client_protocol diff --git a/spec/03-plugins/38-ai-proxy/05-azure_integration_spec.lua b/spec/03-plugins/38-ai-proxy/05-azure_integration_spec.lua index 676e9a9a3871..c6d8a9f5a51b 100644 --- a/spec/03-plugins/38-ai-proxy/05-azure_integration_spec.lua +++ b/spec/03-plugins/38-ai-proxy/05-azure_integration_spec.lua @@ -5,8 +5,9 @@ local pl_file = require "pl.file" local PLUGIN_NAME = "ai-proxy" local MOCK_PORT = helpers.get_available_port() +for _, client_protocol in ipairs({ "http", "https", "http2" }) do for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then - describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "]", function() + describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "] [#" .. client_protocol .. "]", function() local client lazy_setup(function() @@ -407,7 +408,13 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) before_each(function() - client = helpers.proxy_client() + if client_protocol == "http" then + client = helpers.proxy_client() + elseif client_protocol == "https" then + client = helpers.proxy_ssl_client() + elseif client_protocol == "http2" then + client = helpers.proxy_ssl_client(nil, nil, 2) + end end) after_each(function() @@ -650,3 +657,4 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) end end +end -- for _, client_protocol diff --git a/spec/03-plugins/38-ai-proxy/06-mistral_integration_spec.lua b/spec/03-plugins/38-ai-proxy/06-mistral_integration_spec.lua index f45d16058ae4..215c4bba764d 100644 --- a/spec/03-plugins/38-ai-proxy/06-mistral_integration_spec.lua +++ b/spec/03-plugins/38-ai-proxy/06-mistral_integration_spec.lua @@ -5,8 +5,9 @@ local pl_file = require "pl.file" local PLUGIN_NAME = "ai-proxy" local MOCK_PORT = helpers.get_available_port() +for _, client_protocol in ipairs({ "http", "https", "http2" }) do for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then - describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "]", function() + describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "] [#" .. client_protocol .. "]", function() local client lazy_setup(function() @@ -343,7 +344,13 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) before_each(function() - client = helpers.proxy_client() + if client_protocol == "http" then + client = helpers.proxy_client() + elseif client_protocol == "https" then + client = helpers.proxy_ssl_client() + elseif client_protocol == "http2" then + client = helpers.proxy_ssl_client(nil, nil, 2) + end end) after_each(function() @@ -512,3 +519,4 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) end end +end -- for _, client_protocol diff --git a/spec/03-plugins/38-ai-proxy/07-llama2_integration_spec.lua b/spec/03-plugins/38-ai-proxy/07-llama2_integration_spec.lua index f1240d81a506..b998e0b987f9 100644 --- a/spec/03-plugins/38-ai-proxy/07-llama2_integration_spec.lua +++ b/spec/03-plugins/38-ai-proxy/07-llama2_integration_spec.lua @@ -5,8 +5,10 @@ local pl_file = require "pl.file" local PLUGIN_NAME = "ai-proxy" local MOCK_PORT = helpers.get_available_port() +for _, client_protocol in ipairs({ "http", "https", "http2" }) do + for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then - describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "]", function() + describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "] [#" .. client_protocol .. "]", function() local client lazy_setup(function() @@ -191,7 +193,13 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) before_each(function() - client = helpers.proxy_client() + if client_protocol == "http" then + client = helpers.proxy_client() + elseif client_protocol == "https" then + client = helpers.proxy_ssl_client() + elseif client_protocol == "http2" then + client = helpers.proxy_ssl_client(nil, nil, 2) + end end) after_each(function() @@ -443,3 +451,4 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) end end +end -- for _, client_protocol diff --git a/spec/03-plugins/38-ai-proxy/08-encoding_integration_spec.lua b/spec/03-plugins/38-ai-proxy/08-encoding_integration_spec.lua index 0e9801a2923e..3e2c2537d2da 100644 --- a/spec/03-plugins/38-ai-proxy/08-encoding_integration_spec.lua +++ b/spec/03-plugins/38-ai-proxy/08-encoding_integration_spec.lua @@ -110,8 +110,9 @@ local plugin_conf = { }, } +for _, client_protocol in ipairs({ "http", "https", "http2" }) do for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then - describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "]", function() + describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "] [#" .. client_protocol .. "]", function() local client lazy_setup(function() @@ -241,7 +242,13 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) before_each(function() - client = helpers.proxy_client() + if client_protocol == "http" then + client = helpers.proxy_client() + elseif client_protocol == "https" then + client = helpers.proxy_ssl_client() + elseif client_protocol == "http2" then + client = helpers.proxy_ssl_client(nil, nil, 2) + end end) after_each(function() @@ -369,3 +376,4 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then ---- end end +end -- for _, client_protocol diff --git a/spec/03-plugins/38-ai-proxy/09-streaming_integration_spec.lua b/spec/03-plugins/38-ai-proxy/09-streaming_integration_spec.lua index 24707b8039eb..841eb36949eb 100644 --- a/spec/03-plugins/38-ai-proxy/09-streaming_integration_spec.lua +++ b/spec/03-plugins/38-ai-proxy/09-streaming_integration_spec.lua @@ -7,8 +7,9 @@ local http = require("resty.http") local PLUGIN_NAME = "ai-proxy" local MOCK_PORT = helpers.get_available_port() +for _, client_protocol in ipairs({ "http", "https", "http2" }) do for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then - describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "]", function() + describe(PLUGIN_NAME .. ": (access) [#" .. strategy .. "] [#" .. client_protocol .. "]", function() local client lazy_setup(function() @@ -500,7 +501,13 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) before_each(function() - client = helpers.proxy_client() + if client_protocol == "http" then + client = helpers.proxy_client() + elseif client_protocol == "https" then + client = helpers.proxy_ssl_client() + elseif client_protocol == "http2" then + client = helpers.proxy_ssl_client(nil, nil, 2) + end end) after_each(function() @@ -812,3 +819,4 @@ for _, strategy in helpers.all_strategies() do if strategy ~= "cassandra" then end) end end +end -- for _, client_protocol diff --git a/spec/internal/client.lua b/spec/internal/client.lua index 58662fe6d32c..8ef6411f44a6 100644 --- a/spec/internal/client.lua +++ b/spec/internal/client.lua @@ -12,7 +12,7 @@ local cjson = require("cjson.safe") local http = require("resty.http") local kong_table = require("kong.tools.table") local uuid = require("kong.tools.uuid").uuid - +local reqwest = require("reqwest") local CONSTANTS = require("spec.internal.constants") local conf = require("spec.internal.conf") @@ -138,6 +138,34 @@ function resty_http_proxy_mt:send(opts, is_reopen) opts.query = nil end + if self.options.http_version and self.options.http_version == 2 then + local url = self.options.scheme .. "://" .. self.options.host .. ":" .. self.options.port .. opts.path + local reqwest_opt = { + version = 2, + method = opts.method, + body = opts.body, + headers = opts.headers, + tls_verify = false, + } + local res, err = reqwest.request(url, reqwest_opt) + if not res then + return nil, err + end + local headers_mt = { + __index = function(t, k) + return rawget(t, string.lower(k)) + end + } + local res_headers = setmetatable(res.headers, headers_mt) + return { + status = res.status, + headers = res_headers, + read_body = function() + return res.body, nil + end + } + end + local res, err = self:request(opts) if res then -- wrap the read_body() so it caches the result and can be called multiple @@ -176,6 +204,10 @@ function resty_http_proxy_mt:_connect() opts.read_timeout = CONSTANTS.TEST_COVERAGE_TIMEOUT * 1000 end + if opts.http_version and opts.http_version == 2 then + return + end + local _, err = self:connect(opts) if err then error("Could not connect to " .. @@ -310,15 +342,25 @@ end -- @param timeout (optional, number) the timeout to use -- @param forced_port (optional, number) if provided will override the port in -- the Kong configuration with this port -local function proxy_client(timeout, forced_port, forced_ip) +-- @param forced_ip (optional, string) if provided will override the ip in +-- the Kong configuration with this ip +-- @param http_version (optional, number) the http version to use +local function proxy_client(timeout, forced_port, forced_ip, http_version) local proxy_ip = get_proxy_ip(false) local proxy_port = get_proxy_port(false) + local opts_http_version + if http_version == 2 then + opts_http_version = 2 + proxy_ip = get_proxy_ip(false, true) + proxy_port = get_proxy_port(false, true) + end assert(proxy_ip, "No http-proxy found in the configuration") return http_client_opts({ scheme = "http", host = forced_ip or proxy_ip, port = forced_port or proxy_port, timeout = timeout or 60000, + http_version = opts_http_version, }) end @@ -327,9 +369,16 @@ end -- @function proxy_ssl_client -- @param timeout (optional, number) the timeout to use -- @param sni (optional, string) the sni to use -local function proxy_ssl_client(timeout, sni) +-- @param http_version (optional, number) the http version to use +local function proxy_ssl_client(timeout, sni, http_version) local proxy_ip = get_proxy_ip(true, true) local proxy_port = get_proxy_port(true, true) + local opts_http_version + if http_version == 2 then + opts_http_version = 2 + proxy_ip = get_proxy_ip(true, true) + proxy_port = get_proxy_port(true, true) + end assert(proxy_ip, "No https-proxy found in the configuration") local client = http_client_opts({ scheme = "https", @@ -338,6 +387,7 @@ local function proxy_ssl_client(timeout, sni) timeout = timeout or 60000, ssl_verify = false, ssl_server_name = sni, + http_version = opts_http_version, }) return client end diff --git a/t/01-pdk/06-service-request/00-phase_checks.t b/t/01-pdk/06-service-request/00-phase_checks.t index 6d87c75b4606..493e4926c0e8 100644 --- a/t/01-pdk/06-service-request/00-phase_checks.t +++ b/t/01-pdk/06-service-request/00-phase_checks.t @@ -33,7 +33,7 @@ qq{ { method = "enable_buffering", args = { "http" }, - init_worker = false, + init_worker = "forced false", certificate = "pending", rewrite = true, access = true,