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

refactor(aws-lambda): refactor aws-lambda plugin code with lua-resty-aws #11350

Merged
merged 14 commits into from
Aug 15, 2023
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
1 change: 1 addition & 0 deletions .requirements
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ OPENRESTY=1.21.4.1
LUAROCKS=3.9.2
OPENSSL=3.1.2
PCRE=8.45
LIBEXPAT=2.5.0

LUA_KONG_NGINX_MODULE=4d19e8d19c6dbc07eba5cf6f5ebacad95266f928 # 0.6.0
LUA_RESTY_LMDB=951926f20b674a0622236a0e331b359df1c02d9b # 1.3.0
Expand Down
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ ifeq ($(OS), darwin)
OPENSSL_DIR ?= $(shell brew --prefix)/opt/openssl
GRPCURL_OS ?= osx
YAML_DIR ?= $(shell brew --prefix)/opt/libyaml
EXPAT_DIR ?= $(HOMEBREW_DIR)/opt/expat
else
OPENSSL_DIR ?= /usr
GRPCURL_OS ?= $(OS)
YAML_DIR ?= /usr
EXPAT_DIR ?= $(LIBRARY_PREFIX)
endif

ifeq ($(MACHINE), aarch64)
Expand Down
1 change: 1 addition & 0 deletions build/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ exports_files([

lib_deps = [
"@openssl", #TODO: select over fips (but select doesn't work in list comprehension)
"@libexpat",
]

install_lib_deps_cmd = "\n".join([
Expand Down
16 changes: 16 additions & 0 deletions build/libexpat/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
load("@bazel_skylib//rules:build_test.bzl", "build_test")

exports_files(
[
"BUILD.libexpat.bazel",
],
visibility = ["//visibility:public"],
)

build_test(
name = "build",
targets = [
"@libexpat//:libexpat",
],
visibility = ["//:__pkg__"],
)
58 changes: 58 additions & 0 deletions build/libexpat/BUILD.libexpat.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
load("@rules_foreign_cc//foreign_cc:defs.bzl", "configure_make")
load("@kong_bindings//:variables.bzl", "KONG_VAR")

filegroup(
name = "all_srcs",
srcs = glob(
include = ["**"],
exclude = ["*.bazel"],
),
)

configure_make(
name = "libexpat",
configure_command = "configure",
configure_in_place = True,
configure_options = [
# configure a miminal feature set at first so that we don't
# end up depend to a lot of dependencies; do not when turning
# on any of the feature below, we need to add it o kong package's
# dependencies, and compile it (under build/cross_deps) for
# cross build platforms
"--enable-static=no",
"--without-xmlwf",
"--without-examples",
"--without-docbook",
] + select({
"@kong//:aarch64-linux-anylibc-cross": [
"--host=aarch64-linux",
],
"@kong//:x86_64-linux-musl-cross": [
"--host=x86_64-linux-musl",
],
"//conditions:default": [],
}),
env = select({
"@platforms//os:macos": {
# don't use rule_foreign_cc's libtool as archiver as it seems to be a bug
# see https://github.com/bazelbuild/rules_foreign_cc/issues/947
"AR": "/usr/bin/ar",
},
"//conditions:default": {},
}),
lib_source = ":all_srcs",
# out_lib_dir = "lib",
out_shared_libs = select({
"@platforms//os:macos": [
"libexpat.1.dylib",
],
"//conditions:default": [
"libexpat.so.1",
],
}),
targets = [
"-j" + KONG_VAR["NPROC"],
"install -j" + KONG_VAR["NPROC"],
],
visibility = ["//visibility:public"],
)
20 changes: 20 additions & 0 deletions build/libexpat/repositories.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
"""A module defining the third party dependency OpenResty"""

load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
load("@kong_bindings//:variables.bzl", "KONG_VAR")

def libexpat_repositories():
"""Defines the libexpat repository"""

version = KONG_VAR["LIBEXPAT"]
tag = "R_" + version.replace(".", "_")

maybe(
http_archive,
name = "libexpat",
url = "https://github.com/libexpat/libexpat/releases/download/" + tag + "/expat-" + version + ".tar.gz",
sha256 = "6b902ab103843592be5e99504f846ec109c1abb692e85347587f237a4ffa1033",
strip_prefix = "expat-" + version,
build_file = "//build/libexpat:BUILD.libexpat.bazel",
)
3 changes: 3 additions & 0 deletions build/luarocks/BUILD.luarocks.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ genrule(
name = "luarocks_exec",
srcs = [
"@openssl",
"@libexpat",
] + select({
"@kong//:any-cross": ["@cross_deps_libyaml//:libyaml"],
"//conditions:default": [
Expand All @@ -65,6 +66,7 @@ touch "$$ROCKS_DIR/../luarocks_config.lua"
ROCKS_CONFIG=$$(readlink -f "$$ROCKS_DIR/../luarocks_config.lua")

OPENSSL_DIR=$$WORKSPACE_PATH/$$(echo '$(locations @openssl)' | awk '{print $$1}')
EXPAT_DIR=$$WORKSPACE_PATH/$$(echo '$(locations @libexpat)' | awk '{print $$1}')

# we use system libyaml on macos
if [[ "$$OSTYPE" == "darwin"* ]]; then
Expand Down Expand Up @@ -120,6 +122,7 @@ export EXT_BUILD_ROOT=$$WORKSPACE_PATH # for musl
$$host_luajit $$WORKSPACE_PATH/$$LUAROCKS_HOST/bin/luarocks \\$$@ \\
OPENSSL_DIR=$$OPENSSL_DIR \\
CRYPTO_DIR=$$OPENSSL_DIR \\
EXPAT_DIR=$$EXPAT_DIR \\
YAML_DIR=$$YAML_DIR
EOF
""",
Expand Down
2 changes: 2 additions & 0 deletions build/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
load("@bazel_tools//tools/build_defs/repo:git.bzl", "new_git_repository")
load("//build/luarocks:luarocks_repositories.bzl", "luarocks_repositories")
load("//build/cross_deps:repositories.bzl", "cross_deps_repositories")
load("//build/libexpat:repositories.bzl", "libexpat_repositories")
load("@kong_bindings//:variables.bzl", "KONG_VAR")

_SRCS_BUILD_FILE_CONTENT = """
Expand Down Expand Up @@ -139,6 +140,7 @@ def kong_resty_websocket_repositories():
)

def build_repositories():
libexpat_repositories()
luarocks_repositories()

kong_resty_websocket_repositories()
Expand Down
6 changes: 1 addition & 5 deletions kong-3.5.0-0.rockspec
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ dependencies = {
"lua-protobuf == 0.5.0",
"lua-resty-healthcheck == 1.6.2",
"lua-messagepack == 0.5.2",
"lua-resty-aws == 1.3.0",
"lua-resty-openssl == 0.8.23",
"lua-resty-counter == 0.2.1",
"lua-resty-ipmatcher == 0.6.1",
Expand Down Expand Up @@ -437,13 +438,8 @@ build = {
["kong.plugins.request-termination.handler"] = "kong/plugins/request-termination/handler.lua",
["kong.plugins.request-termination.schema"] = "kong/plugins/request-termination/schema.lua",

["kong.plugins.aws-lambda.aws-serializer"] = "kong/plugins/aws-lambda/aws-serializer.lua",
["kong.plugins.aws-lambda.handler"] = "kong/plugins/aws-lambda/handler.lua",
["kong.plugins.aws-lambda.iam-ec2-credentials"] = "kong/plugins/aws-lambda/iam-ec2-credentials.lua",
["kong.plugins.aws-lambda.iam-ecs-credentials"] = "kong/plugins/aws-lambda/iam-ecs-credentials.lua",
["kong.plugins.aws-lambda.iam-sts-credentials"] = "kong/plugins/aws-lambda/iam-sts-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.request-util"] = "kong/plugins/aws-lambda/request-util.lua",

["kong.plugins.grpc-gateway.deco"] = "kong/plugins/grpc-gateway/deco.lua",
Expand Down
24 changes: 24 additions & 0 deletions kong/db/dao/plugins.lua
Original file line number Diff line number Diff line change
Expand Up @@ -355,5 +355,29 @@ function Plugins:get_handlers()
return list
end

function Plugins:execute_plugin_init()
Copy link
Member Author

Choose a reason for hiding this comment

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

@bungle I'm trying to add support for plugins executing their own code in the init phase, so that plugins can do things that may contain yield which cannot be done inside a require. I'm adding an execute_plugin_init function to achieve that. How do you feel about this? Is it a proper way or any advice?

Copy link
Member

Choose a reason for hiding this comment

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

my 2cts: in the "global patches", replace require with a pure-Lua version. Since this has been a recurring theme for a long time. That would solve a bunch of initialization problems due to "yield across C-boundary" issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

#11177 (review)
Leaving a note here: this is the discussion on the same topic. Apparently needs some works to be done to achieve that. If we can achieve that it'll be a painkiller for the restriction of not able to do any yield-able thing in module level

local handlers, err = self:get_handlers()
if not handlers then
return nil, err
end

local errors

for _, handler in ipairs(handlers) do
if implements(handler.handler, "init") then
local ok, err = pcall(handler.handler.init, handler.handler)
if not ok then
errors = errors or {}
errors[#errors + 1] = "on plugin '" .. handler.name .. "': " .. tostring(err)
end
end
end

if errors then
return nil, "error executing plugin init: " .. table.concat(errors, "; ")
end

return true
end

return Plugins
2 changes: 2 additions & 0 deletions kong/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,8 @@ function Kong.init()
-- Load plugins as late as possible so that everything is set up
assert(db.plugins:load_plugin_schemas(config.loaded_plugins))

assert(db.plugins:execute_plugin_init())

if is_stream_module then
stream_api.load_handlers()
end
Expand Down
138 changes: 0 additions & 138 deletions kong/plugins/aws-lambda/aws-serializer.lua

This file was deleted.

Loading