feat(plugins): aws lambda serverless #5594

bisakhmondal · 2021-11-23T19:13:27Z

What this PR does / why we need it:

create a route

With API key-based authorization

{
    "plugins": {
        "aws-lambda": {
            "function_uri": "https://x9w6z07gb9.execute-api.us-east-1.amazonaws.com/default/test-apisix",
            "authorization": {
                "apikey": "<apikey>"
            },
            "ssl_verify":false
        }
    },
    "uri": "/aws"
}

With IAM v4 request signing

{
    "plugins": {
        "aws-lambda": {
            "function_uri": "https://x9w6z07gb9.execute-api.us-east-1.amazonaws.com/default/test-apisix",
            "authorization": {
                "iam": {
                     "accesskey": <accesskey>,
                     "secretkey": <secretkey>,
                     "aws_region": "us-east-1",
                     "service": "execute-api"
                 }
            },
            "ssl_verify":false
        }
    },
    "uri": "/aws"
}

curl -i -XGET localhost:9080/aws\?name=bisakh
HTTP/1.1 200 OK
Content-Type: application/json
Connection: keep-alive
X-Amzn-Trace-Id: Root=1-619d3cef-5a2070dc05d8cf4f37eb9877;Sampled=0
x-amzn-RequestId: d74198ab-1cfd-4eb3-b4e5-a81095ddb6ba
Date: Tue, 23 Nov 2021 19:11:43 GMT
Content-Length: 16
x-amz-apigw-id: JRZ1aHaHoAMFZmw=
Server: APISIX/2.10.2

"Hello, bisakh!"

Pre-submission checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

bisakhmondal · 2021-11-25T22:24:54Z

depends on: #5616
Need to be rebased once #5616 merges into master
cc @spacewander for review

membphis · 2021-11-28T00:36:48Z

apisix/plugins/azure-functions.lua


-local schema = {
+local azure_authz_schema = {


we are developing a new feature "aws lambda serverless", so we should not update azure-functions.lua, it is a different thing.

one for one thing.

I agree. This plugin depends on the refactoring proposed in #5616. For sake of fast development, I developed the feature on top of the branch (ref). As the changes in 5616 has been merged into master, I have rebased it. Thanks : )

membphis · 2021-11-28T00:38:12Z

apisix/plugins/aws-lambda.lua

+local resty_sha256 = require("resty.sha256")
+
+
+local plugin_name, plugin_version, priority = "aws-lambda", 0.1, -1899


bad style

good style:

local plugin_name = "aws-lambda" local plugin_version = 0.1 local priority = -1899

Thanks, updated

membphis · 2021-11-28T00:50:34Z

apisix/plugins/aws-lambda.lua

+    return kSigning
+end
+
+local aws_authz_schema = {


I think we use schema is fine, the file name is aws-lamda

This is not the complete schema, rather it's the part of the schema (handling the plugin specific authorization details).

see:

apisix/apisix/plugins/serverless/generic-upstream.lua

Line 36 in 8f0b066

authorization = authz_schema,

membphis · 2021-11-28T00:51:21Z

apisix/plugins/aws-lambda.lua

+                service = {
+                    type = "string",
+                    default = "execute-api",
+                    description = "The service that is receiving the request"


The service or the service?

we should use the same style with others.

membphis · 2021-11-28T00:52:22Z

apisix/plugins/aws-lambda.lua

+
+local ngx = ngx
+local pairs = pairs
+local concat = table.concat


bad name, I think we should add a prefix tab_

membphis · 2021-11-28T00:52:29Z

apisix/plugins/aws-lambda.lua

+local ngx = ngx
+local pairs = pairs
+local concat = table.concat
+local sort = table.sort


membphis · 2021-11-28T00:56:05Z

apisix/plugins/aws-lambda.lua

+end
+
+
+return require("apisix.plugins.serverless.generic-upstream")(plugin_name,


it is not easy to read.

another style:

local serverless_obj = require("apisix.plugins.serverless.generic-upstream") local aws_obj = serverless_obj.new({ name = plugin_name, version = plugin_version, priority = priority, ... ... })

Nice. Updated accordingly

spacewander · 2021-11-29T09:21:10Z

t/admin/plugins.t

@@ -40,7 +40,7 @@ __DATA__
 --- request
 GET /apisix/admin/plugins/list
 --- response_body_like eval
-qr/\["real-ip","client-control","ext-plugin-pre-req","zipkin","request-id","fault-injection","serverless-pre-function","batch-requests","cors","ip-restriction","ua-restriction","referer-restriction","uri-blocker","request-validation","openid-connect","authz-casbin","wolf-rbac","ldap-auth","hmac-auth","basic-auth","jwt-auth","key-auth","consumer-restriction","authz-keycloak","proxy-mirror","proxy-cache","proxy-rewrite","api-breaker","limit-conn","limit-count","limit-req","gzip","server-info","traffic-split","redirect","response-rewrite","grpc-transcode","prometheus","datadog","echo","http-logger","skywalking-logger","google-cloud-logging","sls-logger","tcp-logger","kafka-logger","syslog","udp-logger","example-plugin","azure-functions","openwhisk","serverless-post-function","ext-plugin-post-req"\]/
+qr/\["real-ip","client-control","ext-plugin-pre-req","zipkin","request-id","fault-injection","serverless-pre-function","batch-requests","cors","ip-restriction","ua-restriction","referer-restriction","uri-blocker","request-validation","openid-connect","authz-casbin","wolf-rbac","ldap-auth","hmac-auth","basic-auth","jwt-auth","key-auth","consumer-restriction","authz-keycloak","proxy-mirror","proxy-cache","proxy-rewrite","api-breaker","limit-conn","limit-count","limit-req","gzip","server-info","traffic-split","redirect","response-rewrite","grpc-transcode","prometheus","datadog","echo","http-logger","skywalking-logger","google-cloud-logging","sls-logger","tcp-logger","kafka-logger","syslog","udp-logger","example-plugin","aws-lambda","azure-functions","openwhisk","serverless-post-function","ext-plugin-post-req"\]/


Can we split this into multiple lines? We can do it in the next PR.

Yes, It's time to modify this to another easy-to-read format.

Guys, here it is: #5642

spacewander · 2021-11-29T09:30:17Z

apisix/plugins/aws-lambda.lua

+        if k ~= "connection" then
+            signed_headers[#signed_headers+1] = k
+            -- strip starting and trailing spaces including strip multiple spaces into single space
+            canonical_headers[k] =  v:gsub("^%s+", ""):gsub("%s+$", "")


We can use https://refined-github-html-preview.kidonng.workers.dev/lunarmodules/Penlight/raw/master/docs/libraries/pl.stringx.html#strip?

spacewander · 2021-11-29T09:30:22Z

apisix/plugins/aws-lambda.lua

+    headers["X-Amz-Date"] = amzdate
+
+    -- computing canonical uri
+    local canonical_uri = params.path:gsub("//", "/")


Can we use pl.path to do part of the normalized work?
https://refined-github-html-preview.kidonng.workers.dev/lunarmodules/Penlight/raw/master/docs/libraries/pl.path.html#normpath

spacewander · 2021-11-29T09:31:17Z

apisix/plugins/aws-lambda.lua

+local serverless_obj = require("apisix.plugins.serverless.generic-upstream")
+
+return serverless_obj(plugin_name,
+                    plugin_version,


spacewander · 2021-11-29T09:32:26Z

apisix/plugins/aws-lambda.lua

+local tab_sort = table.sort
+local os = os
+local hmac = require("resty.hmac")
+local hexencode = require("resty.string").to_hex


Can we use underscore style for function name?

spacewander · 2021-11-29T09:32:34Z

apisix/plugins/aws-lambda.lua

+    return hexencode(digest)
+end
+
+local function getSignatureKey(key, datestamp, region, service)


spacewander · 2021-11-29T09:36:03Z

docs/en/latest/plugins/aws-lambda.md

+"Hello, apisix!"
+```
+
+For requests where the mode of communication between the client and the Apache APISIX gateway is HTTP/2, the example looks like ( make sure you are running APISIX agent with `enable_http2: true` for a port in conf.yaml or uncomment port 9081 of `node_listen` field inside [config-default.yaml](../../../../conf/config-default.yaml) ) :


Better to use config-default.yaml directly, since people may read the doc on the doc's website. Please fix the azure-functions' doc too.

spacewander · 2021-11-29T09:37:54Z

docs/en/latest/plugins/aws-lambda.md

+
+## How To Enable
+
+The following is an example of how to enable the aws-lambda faas plugin for a specific route URI. Calling the apisix route uri will make an invocation to the lambda function uri (the new upstream).  We are assuming your cloud function is already up and running.


Please use APISIX route instead of apisix route. Need to fix other similar places.

We should use APISIX always

Okay. Got it. Thanks for letting me know : )

membphis · 2021-11-29T13:05:09Z

@shuaijinchao @tzssangglass pls review when you have time

membphis · 2021-11-29T13:07:23Z

docs/en/latest/plugins/aws-lambda.md

+
+## How To Enable
+
+The following is an example of how to enable the aws-lambda faas plugin for a specific route URI. Calling the apisix route uri will make an invocation to the lambda function uri (the new upstream).  We are assuming your cloud function is already up and running.


We should use APISIX always

membphis · 2021-11-29T13:09:48Z

t/admin/plugins.t

@@ -40,7 +40,7 @@ __DATA__
 --- request
 GET /apisix/admin/plugins/list
 --- response_body_like eval
-qr/\["real-ip","client-control","ext-plugin-pre-req","zipkin","request-id","fault-injection","serverless-pre-function","batch-requests","cors","ip-restriction","ua-restriction","referer-restriction","uri-blocker","request-validation","openid-connect","authz-casbin","wolf-rbac","ldap-auth","hmac-auth","basic-auth","jwt-auth","key-auth","consumer-restriction","authz-keycloak","proxy-mirror","proxy-cache","proxy-rewrite","api-breaker","limit-conn","limit-count","limit-req","gzip","server-info","traffic-split","redirect","response-rewrite","grpc-transcode","prometheus","datadog","echo","http-logger","skywalking-logger","google-cloud-logging","sls-logger","tcp-logger","kafka-logger","syslog","udp-logger","example-plugin","azure-functions","openwhisk","serverless-post-function","ext-plugin-post-req"\]/
+qr/\["real-ip","client-control","ext-plugin-pre-req","zipkin","request-id","fault-injection","serverless-pre-function","batch-requests","cors","ip-restriction","ua-restriction","referer-restriction","uri-blocker","request-validation","openid-connect","authz-casbin","wolf-rbac","ldap-auth","hmac-auth","basic-auth","jwt-auth","key-auth","consumer-restriction","authz-keycloak","proxy-mirror","proxy-cache","proxy-rewrite","api-breaker","limit-conn","limit-count","limit-req","gzip","server-info","traffic-split","redirect","response-rewrite","grpc-transcode","prometheus","datadog","echo","http-logger","skywalking-logger","google-cloud-logging","sls-logger","tcp-logger","kafka-logger","syslog","udp-logger","example-plugin","aws-lambda","azure-functions","openwhisk","serverless-post-function","ext-plugin-post-req"\]/


Yes, It's time to modify this to another easy-to-read format.

membphis · 2021-11-29T13:13:58Z

docs/en/latest/plugins/aws-lambda.md

+| authorization.apikey | string | optional    |             |     | Field inside _authorization_. The generate API Key to authorize requests to that endpoint of the aws gateway. |                         |
+| authorization.iam | object | optional    |             |     | Field inside _authorization_. AWS IAM role based authorization, performed via aws v4 request signing. See schema details below ([here](#iam-authorization-schema)). |                         |
+| timeout  | integer | optional    | 3000           | [100,...]     | Proxy request timeout in milliseconds.   |
+| ssl_verify  | boolean | optional    | true           | true/false     | If enabled performs SSL verification of the server.                     |


if the ssl_verify is true, do we need to specify a certificate?

If it is generated from any trusted public CA, we don't have to specify the certificate of the CA as along the hierarchy chain eventually it will point to the root CA. But that's not the case with self signed certs. In that case, simply ssl_verify:false will do or there is an option to specify the CA certificate file. see ref below

The optional ssl_verify argument takes a Lua boolean value to control whether to perform SSL verification. When set to true, the server certificate will be verified according to the CA certificates specified by the lua_ssl_trusted_certificate directive. You may also need to adjust the lua_ssl_verify_depth directive to control how deep we should follow along the certificate chain. Also, when the ssl_verify argument is true and the server_name argument is also specified, the latter will be used to validate the server name in the server certificate.

ref2: https://github.com/openresty/lua-nginx-module#lua_ssl_trusted_certificate

docs/en/latest/plugins/aws-lambda.md

t/plugin/aws-lambda.t

tzssangglass · 2021-11-30T02:00:00Z

t/plugin/aws-lambda.t

+--- response_headers
+Content-Length: 19


why check the Content-Length

Hi, thanks for the review.
It just checks if the headers that are being received from upstream lambda are being forwarded by the plugin. Not just Content-Length specifically but all headers from the mock HTTP endpoint

docs/en/latest/plugins/aws-lambda.md

tzssangglass · 2021-11-30T02:25:01Z

apisix/plugins/aws-lambda.lua

+        if k ~= "connection" then
+            signed_headers[#signed_headers+1] = k


I don't understand here, do we just need to skip connection?

First of all, I would like to specify, we are not tampering connection header for the request that is being sent to the lambda.
This section performs aws v4 request signing with aws iam secret key. As per the aws docs, to have the authenticity of the request (and to validate that the request has not been modified by any third party over the wire) it suggests generating hash digest from as many headers as we can (however host and x-amz-date is mandatory). In real-world testing what i have found that aws gateway doesn't include the connection header value (keep-alive, close) for hash calculation. That's why I simply omitted it as it never was any hard requirement.

bisakhmondal added 2 commits November 23, 2021 16:36

v1 refactor

2c91976

url path forwarding

727cc0a

bisakhmondal changed the title ~~feat(plugins): aws lambda serverless with generic upstream refactoring~~ feat(plugins): aws lambda serverless Nov 25, 2021

bisakhmondal added 9 commits November 25, 2021 21:52

lint fix

73c932f

update request processor

ca539fd

v1 refactor

ce71bd1

use function closure

16915a3

brand new aws-lambda

70c5523

fix lint

6ab2832

Merge branch 'faas-refactoring' into aws-lambda-with-refactoring

a4cbd0f

minor fix

67a446f

Merge branch 'faas-refactoring' into aws-lambda-with-refactoring

a8e6fd8

bisakhmondal force-pushed the aws-lambda-with-refactoring branch from 5331ab3 to a8e6fd8 Compare November 25, 2021 22:21

bisakhmondal added 2 commits November 26, 2021 03:52

iam v4 request signing

bb4ef19

tests

5c74c62

bisakhmondal marked this pull request as ready for review November 25, 2021 22:23

bisakhmondal added 3 commits November 26, 2021 03:56

lint import fix

da42c42

documentation

2d91770

lint fix

c1abe81

membphis reviewed Nov 28, 2021

View reviewed changes

bisakhmondal added 3 commits November 29, 2021 13:40

Merge branch 'master' into aws-lambda-with-refactoring

9b5e7ba

branch rebased with master and suggestions applied from @membphis

70a32e8

minor update

d7376da

spacewander reviewed Nov 29, 2021

View reviewed changes

membphis requested review from tzssangglass and shuaijinchao November 29, 2021 13:04

membphis reviewed Nov 29, 2021

View reviewed changes

updates

d9594e6

This was referenced Nov 29, 2021

docs: minor update in azure faas http2 documentation #5641

Merged

refactor: plugins list test representation #5642

Merged

spacewander reviewed Nov 30, 2021

View reviewed changes

docs/en/latest/plugins/aws-lambda.md Outdated Show resolved Hide resolved

docs/en/latest/plugins/aws-lambda.md Outdated Show resolved Hide resolved

juzhiyuan reviewed Nov 30, 2021

View reviewed changes

docs/en/latest/plugins/aws-lambda.md Outdated Show resolved Hide resolved

docs/en/latest/plugins/aws-lambda.md Outdated Show resolved Hide resolved

tzssangglass reviewed Nov 30, 2021

View reviewed changes

bisakhmondal added 2 commits November 30, 2021 09:12

suggestions

f3fe69c

Merge master and update changes with plugins list

87c7952

spacewander approved these changes Nov 30, 2021

View reviewed changes

moonming mentioned this pull request Nov 30, 2021

lambda and API Gateway aws/serverless-application-model#2254

Closed

juzhiyuan requested review from tzssangglass and membphis December 1, 2021 01:46

shuaijinchao approved these changes Dec 1, 2021

View reviewed changes

spacewander merged commit e90e3b7 into apache:master Dec 1, 2021

		local resty_sha256 = require("resty.sha256")


		local plugin_name, plugin_version, priority = "aws-lambda", 0.1, -1899

		end


		return require("apisix.plugins.serverless.generic-upstream")(plugin_name,


		## How To Enable

		The following is an example of how to enable the aws-lambda faas plugin for a specific route URI. Calling the apisix route uri will make an invocation to the lambda function uri (the new upstream). We are assuming your cloud function is already up and running.

		if k ~= "connection" then
		signed_headers[#signed_headers+1] = k

feat(plugins): aws lambda serverless #5594

feat(plugins): aws lambda serverless #5594

Conversation

bisakhmondal commented Nov 23, 2021 • edited Loading

What this PR does / why we need it:

With API key-based authorization

With IAM v4 request signing

Pre-submission checklist:

bisakhmondal commented Nov 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bisakhmondal Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bisakhmondal Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

membphis commented Nov 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bisakhmondal commented Nov 23, 2021 •

edited

Loading

bisakhmondal Nov 29, 2021 •

edited

Loading

bisakhmondal Nov 29, 2021 •

edited

Loading