Skip to content

Commit

Permalink
fix(aws-lambda) proxy correctly with lua-resty-http (#8406)
Browse files Browse the repository at this point in the history
Fix some bugs and warts in the aws-lambda plugin:

* Fix broken proxying by always using `https_proxy` with resty.http
* Deprecate `proxy_scheme` config param

Some minimal test coverage for proxying was added, and some defunct test cases were removed.
  • Loading branch information
flrgh authored and kikito committed Feb 23, 2022
1 parent d02d4b1 commit 90b0e36
Show file tree
Hide file tree
Showing 11 changed files with 192 additions and 168 deletions.
10 changes: 0 additions & 10 deletions .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@
the header `Access-Control-Allow-Origin` is set to `*`.
Thanks, [@jkla-dr](https://github.com/jkla-dr)!
[#8401](https://github.com/Kong/kong/pull/8401)
- **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]
Expand Down
17 changes: 16 additions & 1 deletion kong/plugins/aws-lambda/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 12 additions & 11 deletions kong/plugins/aws-lambda/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -44,7 +46,6 @@ local error = error
local pairs = pairs
local kong = kong
local type = type
local find = string.find
local fmt = string.format


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -326,6 +327,6 @@ function AWSLambdaHandler:access(conf)
end

AWSLambdaHandler.PRIORITY = 750
AWSLambdaHandler.VERSION = "3.6.2"
AWSLambdaHandler.VERSION = "3.6.3"

return AWSLambdaHandler
Original file line number Diff line number Diff line change
@@ -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 = {
Expand All @@ -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",
}
}
19 changes: 18 additions & 1 deletion kong/plugins/aws-lambda/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ return {
type = "boolean",
default = false,
} },
-- TODO: remove proxy_scheme in Kong 3.0
{ proxy_scheme = {
type = "string",
one_of = { "http", "https" }
Expand All @@ -95,7 +96,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,
}
},
}
}
21 changes: 12 additions & 9 deletions spec/03-plugins/27-aws-lambda/02-schema_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
133 changes: 0 additions & 133 deletions spec/03-plugins/27-aws-lambda/50-http-proxy_spec.lua

This file was deleted.

39 changes: 39 additions & 0 deletions spec/03-plugins/27-aws-lambda/99-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down Expand Up @@ -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,
Expand All @@ -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)

Expand Down Expand Up @@ -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
Loading

0 comments on commit 90b0e36

Please sign in to comment.