Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(aws-lambda) proxy correctly with lua-resty-http #8406

Merged
merged 8 commits into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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]
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 @@ -75,6 +75,7 @@ return {
type = "boolean",
default = false,
} },
-- TODO: remove proxy_scheme in Kong 3.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bungle We should explore a "deprecated = true" property on the schema to manage these programmatically.

{ proxy_scheme = {
type = "string",
one_of = { "http", "https" }
Expand All @@ -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,
}
},
}
}
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