diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 32755718f0c..1a7137a62eb 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -155,11 +155,6 @@ jobs: - 6379:6379 options: --entrypoint redis-server - squid: - image: datadog/squid - ports: - - 3128:3128 - zipkin: image: openzipkin/zipkin:2.19 ports: @@ -283,11 +278,6 @@ jobs: - 6379:6379 options: --entrypoint redis-server - squid: - image: datadog/squid - ports: - - 3128:3128 - zipkin: image: openzipkin/zipkin:2.19 ports: diff --git a/CHANGELOG.md b/CHANGELOG.md index e9d81ae5fa4..49f0ceb04f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -154,6 +154,9 @@ - **External Plugins**: Unwrap `ConsumerSpec` and `AuthenticateArgs`. Thanks, [@raptium](https://github.com/raptium)! [#8280](https://github.com/Kong/kong/pull/8280) +- **AWS-Lambda**: Fixed incorrect behavior when configured to use an http proxy + and deprecated the `proxy_scheme` config attribute for removal in 3.0 + [#8406](https://github.com/Kong/kong/pull/8406) ## [2.7.1] diff --git a/kong/plugins/aws-lambda/CHANGELOG.md b/kong/plugins/aws-lambda/CHANGELOG.md index d752f861b61..6fad6f55582 100644 --- a/kong/plugins/aws-lambda/CHANGELOG.md +++ b/kong/plugins/aws-lambda/CHANGELOG.md @@ -11,9 +11,24 @@ - upload to luarocks; `luarocks upload kong-plugin-aws-lambda-x.y.z-1.rockspec --api-key=abc...` - test rockspec; `luarocks install kong-plugin-aws-lambda` +## aws-lambda 3.6.3 15-Feb-2022 -## unreleased +- tests: update forward-proxy fixture +- tests: remove old http proxy tests +- fix: always use https_proxy +- fix: deprecate proxy_scheme parameter +- fix: ensure proxy_url scheme is http +## aws-lambda 3.6.2 07-Sep-2021 + +- chore: bump lua-resty-http from 0.15 to 0.16.1 [#7797](https://github.com/Kong/kong/pull/7797) + +## aws-lambda 3.6.1 07-Sep-2021 +- refactor: use error function instead of kong.log.err + kong.response.error [#7797](https://github.com/Kong/kong/pull/7797) + +## aws-lambda 3.6.0 30-Aug-2021 + +- feat: add support for detecting AWS region from environment variable [#7765](https://github.com/Kong/kong/pull/7765) - fix: handle multivalueheaders [#59](https://github.com/Kong/kong-plugin-aws-lambda/pull/59) ## aws-lambda 3.5.4 22-Mar-2021 diff --git a/kong/plugins/aws-lambda/handler.lua b/kong/plugins/aws-lambda/handler.lua index 3291fc54ebb..486be33f662 100644 --- a/kong/plugins/aws-lambda/handler.lua +++ b/kong/plugins/aws-lambda/handler.lua @@ -17,6 +17,8 @@ local AWS_REGION do AWS_REGION = os.getenv("AWS_REGION") or os.getenv("AWS_DEFAULT_REGION") end +local _logged_proxy_scheme_warning + local fetch_credentials do local credential_sources = { require "kong.plugins.aws-lambda.iam-ecs-credentials", @@ -44,7 +46,6 @@ local error = error local pairs = pairs local kong = kong local type = type -local find = string.find local fmt = string.format @@ -244,17 +245,17 @@ function AWSLambdaHandler:access(conf) local uri = port and fmt("https://%s:%d", host, port) or fmt("https://%s", host) + if conf.proxy_scheme and not _logged_proxy_scheme_warning then + kong.log.warn("`proxy_scheme` is deprecated and will be removed in Kong 3.0") + _logged_proxy_scheme_warning = true + end + local proxy_opts if conf.proxy_url then - if find(conf.proxy_url, "https", 1, true) == 1 then - proxy_opts = { - https_proxy = conf.proxy_url, - } - else - proxy_opts = { - http_proxy = conf.proxy_url, - } - end + -- lua-resty-http uses the request scheme to determine which of + -- http_proxy/https_proxy it will use, and from this plugin's POV, the + -- request scheme is always https + proxy_opts = { https_proxy = conf.proxy_url } end -- Trigger request @@ -326,6 +327,6 @@ function AWSLambdaHandler:access(conf) end AWSLambdaHandler.PRIORITY = 750 -AWSLambdaHandler.VERSION = "3.6.2" +AWSLambdaHandler.VERSION = "3.6.3" return AWSLambdaHandler diff --git a/kong/plugins/aws-lambda/kong-plugin-aws-lambda-3.5.4-1.rockspec b/kong/plugins/aws-lambda/kong-plugin-aws-lambda-3.6.3-0.rockspec similarity index 88% rename from kong/plugins/aws-lambda/kong-plugin-aws-lambda-3.5.4-1.rockspec rename to kong/plugins/aws-lambda/kong-plugin-aws-lambda-3.6.3-0.rockspec index 030baad00b5..14fd0ceacc0 100644 --- a/kong/plugins/aws-lambda/kong-plugin-aws-lambda-3.5.4-1.rockspec +++ b/kong/plugins/aws-lambda/kong-plugin-aws-lambda-3.6.3-0.rockspec @@ -1,10 +1,10 @@ package = "kong-plugin-aws-lambda" -version = "3.5.4-1" +version = "3.6.3-1" supported_platforms = {"linux", "macosx"} source = { url = "git://github.com/kong/kong-plugin-aws-lambda", - tag = "3.5.4", + tag = "3.6.3", } description = { @@ -26,7 +26,6 @@ build = { ["kong.plugins.aws-lambda.iam-ecs-credentials"] = "kong/plugins/aws-lambda/iam-ecs-credentials.lua", ["kong.plugins.aws-lambda.schema"] = "kong/plugins/aws-lambda/schema.lua", ["kong.plugins.aws-lambda.v4"] = "kong/plugins/aws-lambda/v4.lua", - ["kong.plugins.aws-lambda.http.connect-better"] = "kong/plugins/aws-lambda/http/connect-better.lua", ["kong.plugins.aws-lambda.request-util"] = "kong/plugins/aws-lambda/request-util.lua", } } diff --git a/kong/plugins/aws-lambda/schema.lua b/kong/plugins/aws-lambda/schema.lua index 02a76d111b7..82f8f314ee0 100644 --- a/kong/plugins/aws-lambda/schema.lua +++ b/kong/plugins/aws-lambda/schema.lua @@ -75,6 +75,7 @@ return { type = "boolean", default = false, } }, + -- TODO: remove proxy_scheme in Kong 3.0 { proxy_scheme = { type = "string", one_of = { "http", "https" } @@ -93,7 +94,23 @@ return { } }, entity_checks = { { mutually_required = { "config.aws_key", "config.aws_secret" } }, - { mutually_required = { "config.proxy_scheme", "config.proxy_url" } }, { mutually_exclusive = { "config.aws_region", "config.host" } }, + { custom_entity_check = { + field_sources = { "config.proxy_url" }, + fn = function(entity) + local proxy_url = entity.config and entity.config.proxy_url + + if type(proxy_url) == "string" then + local scheme = proxy_url:match("^([^:]+)://") + + if scheme and scheme ~= "http" then + return nil, "proxy_url scheme must be http" + end + end + + return true + end, + } + }, } } diff --git a/spec/03-plugins/27-aws-lambda/02-schema_spec.lua b/spec/03-plugins/27-aws-lambda/02-schema_spec.lua index fd59d1d0bd9..4f97c1cb7d9 100644 --- a/spec/03-plugins/27-aws-lambda/02-schema_spec.lua +++ b/spec/03-plugins/27-aws-lambda/02-schema_spec.lua @@ -79,15 +79,18 @@ describe("Plugin: AWS Lambda (schema)", function() assert.falsy(ok) end) - it("errors if proxy_scheme is missing while proxy_url is provided", function() - local ok, err = v({ - proxy_url = "http://hello.com/proxy", - aws_region = "us-east-1", - function_name = "my-function" - }, schema_def) - - assert.equal("all or none of these fields must be set: 'config.proxy_scheme', 'config.proxy_url'", err["@entity"][1]) - assert.falsy(ok) + it("errors with a non-http proxy_url", function() + for _, scheme in ipairs({"https", "ftp", "wss"}) do + local ok, err = v({ + proxy_url = scheme .. "://squid:3128", + aws_region = "us-east-1", + function_name = "my-function" + }, schema_def) + + assert.not_nil(err) + assert.falsy(ok) + assert.equals("proxy_url scheme must be http", err["@entity"][1]) + end end) it("accepts a host", function() diff --git a/spec/03-plugins/27-aws-lambda/50-http-proxy_spec.lua b/spec/03-plugins/27-aws-lambda/50-http-proxy_spec.lua deleted file mode 100644 index b732fdcd766..00000000000 --- a/spec/03-plugins/27-aws-lambda/50-http-proxy_spec.lua +++ /dev/null @@ -1,133 +0,0 @@ -require "spec.helpers" -local http = require "resty.http" - -local configs = { - { - name = "plain #http", - scheme = "http", - host = "httpbin.org", - path = "/anything", - },{ - name = "plain #https", - scheme = "https", - host = "httpbin.org", - path = "/anything", - },{ - name = "#http via proxy", - scheme = "http", - host = "mockbin.org", - path = "/request", - proxy_url = "http://127.0.0.1:3128/", - },{ - name = "#https via proxy", - scheme = "https", - host = "mockbin.org", - path = "/request", - proxy_url = "http://127.0.0.1:3128/", - },{ - name = "#http via authenticated proxy", - scheme = "http", - host = "httpbin.org", - path = "/anything", - proxy_url = "http://127.0.0.1:3128/", - authorization = "Basic a29uZzpraW5n", -- base64("kong:king") - },{ - name = "#https via authenticated proxy", - scheme = "https", - host = "httpbin.org", - path = "/anything", - proxy_url = "http://127.0.0.1:3128/", - authorization = "Basic a29uZzpraW5n", -- base64("kong:king") - } -} - -local max_idle_timeout = 3 - -local function make_request(config) - -- create and connect the client - local client = http.new() - local ok, err = client:connect { - scheme = config.scheme, - host = config.host, - port = config.scheme == "https" and 443 or 80, - ssl_verify = config.scheme == "https" and false, - ssl_server_name = config.scheme == "https" and config.host, - proxy_opts = config.proxy_url and { - http_proxy = config.proxy_url, - http_proxy_authorization = config.authorization, - } - } - assert.is_nil(err) - assert.truthy(ok) - - -- make the request - local res, err = client:request { - method = "GET", - path = config.path, - body = nil, - headers = { - Host = config.host, - -- for plain http; proxy-auth must be in the headers - ["Proxy-Authorization"] = (config.scheme == "http" and config.authorization), - } - } - - assert.is_nil(err) - assert.truthy(res) - - -- read the body to finish socket ops - res.body = res:read_body() - local reuse = client.sock:getreusedtimes() - - -- close it - ok, err = client:set_keepalive(max_idle_timeout) --luacheck: ignore - --assert.is_nil(err) -- result can be: 2, with error connection had to be closed - assert.truthy(ok) -- resul 2 also qualifies as truthy. - - -- verify http result - if res.status ~= 200 then assert.equal({}, res) end - assert.equal(200, res.status) - return reuse -end - - - - -describe("#proxy #squid", function() - lazy_teardown(function() - ngx.sleep(max_idle_timeout + 0.5) -- wait for keepalive to expire and all socket pools to become empty again - end) - - for _, config in ipairs(configs) do - it("Make a request " .. config.name, function() - make_request(config) - end) - end - -end) - - - -describe("#keepalive #squid", function() - lazy_teardown(function() - ngx.sleep(max_idle_timeout + 0.5) -- wait for keepalive to expire and all socket pools to become empty again - end) - - for _, config in ipairs(configs) do - it("Repeat a request " .. config.name, function() - local reuse = 0 - local loop_size = 10 - - for i = 1, loop_size do - local conn_count = make_request(config) - reuse = math.max(reuse, conn_count) - end - - --print(reuse) - assert(reuse > 0, "expected socket re-use to be > 0, but got: " .. tostring(reuse)) - assert(reuse < loop_size, "re-use expected to be less than " .. loop_size .. - " but was " .. reuse .. ". So probably the socket-poolname is not unique?") - end) - end - -end) diff --git a/spec/03-plugins/27-aws-lambda/99-access_spec.lua b/spec/03-plugins/27-aws-lambda/99-access_spec.lua index 87f8d3b3ae3..5115e7c2d71 100644 --- a/spec/03-plugins/27-aws-lambda/99-access_spec.lua +++ b/spec/03-plugins/27-aws-lambda/99-access_spec.lua @@ -129,6 +129,13 @@ for _, strategy in helpers.each_strategy() do service = null, } + local route20 = bp.routes:insert { + hosts = { "lambda20.test" }, + protocols = { "http", "https" }, + service = null, + } + + bp.plugins:insert { name = "aws-lambda", route = { id = route1.id }, @@ -389,6 +396,20 @@ for _, strategy in helpers.each_strategy() do } } + bp.plugins:insert { + name = "aws-lambda", + route = { id = route20.id }, + config = { + port = 10001, + aws_key = "mock-key", + aws_secret = "mock-secret", + function_name = "functionEcho", + proxy_url = "http://127.0.0.1:13128", + keepalive = 1, + } + } + + fixtures.dns_mock:A({ name = "lambda18.test", address = helpers.mock_upstream_host, @@ -400,6 +421,10 @@ for _, strategy in helpers.each_strategy() do database = strategy, plugins = "aws-lambda", nginx_conf = "spec/fixtures/custom_nginx.template", + + -- we don't actually use any stream proxy features in this test suite, + -- but this is needed in order to load our forward-proxy stream_mock fixture + stream_listen = helpers.get_proxy_ip(false) .. ":19000", }, nil, nil, fixtures)) end) @@ -1001,6 +1026,20 @@ for _, strategy in helpers.each_strategy() do assert.is_string(res.headers.age) assert.is_array(res.headers["Access-Control-Allow-Origin"]) end) + + it("works with a forward proxy", function() + local res = assert(proxy_client:send({ + method = "GET", + path = "/get?a=1&b=2", + headers = { + ["Host"] = "lambda20.test" + } + })) + + assert.res_status(200, res) + local req = assert.response(res).has.jsonbody() + assert.equals("https", req.vars.scheme) + end) end) end) end diff --git a/spec/fixtures/aws-lambda.lua b/spec/fixtures/aws-lambda.lua index 7798428a640..78972384aef 100644 --- a/spec/fixtures/aws-lambda.lua +++ b/spec/fixtures/aws-lambda.lua @@ -45,6 +45,9 @@ local fixtures = { elseif string.match(ngx.var.uri, "functionWithMultiValueHeadersResponse") then ngx.say("{\"statusCode\": 200, \"headers\": { \"Age\": \"3600\"}, \"multiValueHeaders\": {\"Access-Control-Allow-Origin\": [\"site1.com\", \"site2.com\"]}}") + elseif string.match(ngx.var.uri, "functionEcho") then + require("spec.fixtures.mock_upstream").send_default_json_response() + elseif type(res) == 'string' then ngx.header["Content-Length"] = #res + 1 ngx.say(res) @@ -96,6 +99,17 @@ local fixtures = { }, } +fixtures.stream_mock = { + lambda_proxy = [[ + server { + listen 13128; + + content_by_lua_block { + require("spec.fixtures.forward-proxy-server").connect() + } + } + ]], +} fixtures.dns_mock:A { name = "lambda.us-east-1.amazonaws.com", diff --git a/spec/fixtures/forward-proxy-server.lua b/spec/fixtures/forward-proxy-server.lua new file mode 100644 index 00000000000..61bda1196a9 --- /dev/null +++ b/spec/fixtures/forward-proxy-server.lua @@ -0,0 +1,76 @@ +local _M = {} + +local split = require("kong.tools.utils").split + + +-- This is a very naive forward proxy, which accepts a CONNECT over HTTP, and +-- then starts tunnelling the bytes blind (for end-to-end SSL). +function _M.connect() + + local req_sock = ngx.req.socket(true) + req_sock:settimeouts(1000, 1000, 1000) + + -- receive request line + local req_line = req_sock:receive() + ngx.log(ngx.DEBUG, "request line: ", req_line) + + local method, host_port = unpack(split(req_line, " ")) + if method ~= "CONNECT" then + return ngx.exit(400) + end + + local upstream_host, upstream_port = unpack(split(host_port, ":")) + + -- receive and discard any headers + repeat + local line = req_sock:receive("*l") + ngx.log(ngx.DEBUG, "request header: ", line) + until ngx.re.find(line, "^\\s*$", "jo") + + -- Connect to requested upstream + local upstream_sock = ngx.socket.tcp() + upstream_sock:settimeouts(1000, 1000, 1000) + local ok, err = upstream_sock:connect(upstream_host, upstream_port) + if not ok then + ngx.log(ngx.ERR, "connect to upstream ", upstream_host, ":", upstream_port, + " failed: ", err) + return ngx.exit(504) + end + + -- Tell the client we are good to go + ngx.print("HTTP/1.1 200 OK\n\n") + ngx.flush() + + -- 10Kb in either direction should be plenty + local max_bytes = 10 * 1024 + + repeat + local req_data = req_sock:receiveany(max_bytes) + if req_data then + ngx.log(ngx.DEBUG, "client RCV ", #req_data, " bytes") + + local bytes, err = upstream_sock:send(req_data) + if bytes then + ngx.log(ngx.DEBUG, "upstream SND ", bytes, " bytes") + elseif err then + ngx.log(ngx.ERR, "upstream SND failed: ", err) + end + end + + local res_data = upstream_sock:receiveany(max_bytes) + if res_data then + ngx.log(ngx.DEBUG, "upstream RCV ", #res_data, " bytes") + + local bytes, err = req_sock:send(res_data) + if bytes then + ngx.log(ngx.DEBUG, "client SND: ", bytes, " bytes") + elseif err then + ngx.log(ngx.ERR, "client SND failed: ", err) + end + end + until not req_data and not res_data -- request socket should be closed + + upstream_sock:close() +end + +return _M