Openresty Update #1272

eloycoto · 2021-05-18T09:59:49Z

Hi,

I bumped the Openresty Version to 1.9.3, the latest stable, prior we had almost this version, but with a few commits on top of that.

The most important change is related to how Luajit iterate over metatables, so the order is no longer the creation one; it's arbitrary, so the ngx.decode, cjon.decode, pairs can a different result each time. Saying that almost all test failed because we tested the string.

Example:

bash-4.4$ cat eloy/encode_args.lua
local oo = {
  a = 1,
  b= 1,
  c = 1,
  d= 1,
}
for i=1,5 do
  print(ngx.encode_args(oo))
end
bash-4.4$ resty eloy/encode_args.lua
a=1&b=1&c=1&d=1
a=1&b=1&c=1&d=1
a=1&b=1&c=1&d=1
a=1&b=1&c=1&d=1
a=1&b=1&c=1&d=1
bash-4.4$ resty eloy/encode_args.lua
c=1&a=1&d=1&b=1
c=1&a=1&d=1&b=1
c=1&a=1&d=1&b=1
c=1&a=1&d=1&b=1
c=1&a=1&d=1&b=1
bash-4.4$

Regarding other changes:

APICAst Module: Is removed, no longer needed, and we don't want to keep in there. Doc team you should remove from docs.
Oath2 support: Also removed, Doc team you should remove it.
Small FFI calls need to be fixed.
Some Integration and unit tests need to be fixed
Now the context is shared on ssl_service, so the PATH routing when SSL was failing, because the path cannot be retrieved, we should return instead of setting the service.
Rate limit with Redis was failing because it used semaphores when it was not allowed; fixed that in the best way possible.

Long PR, each commit is self-documented, so should be easy to get the context.

Fix that updates base image to Openresty 1.19.3 version. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

dfennessy · 2021-05-18T10:24:11Z

doc/parameters.md

-**Deprecated:** Use [policies](./policies.md) instead.
-
-Specifies the name of the main Lua module that implements the API gateway logic. Custom modules can override the functionality of the default `apicast.lua` module. See [an example](../examples/custom-module) of how to use modules.
-


@eloycoto Will there be further text to review? If there is only the removal of text, then it LGTM 😄

My job is to make you happy, so no changes in doc :-)

davidor · 2021-05-19T09:02:36Z

gateway/src/resty/ctx.lua

@@ -37,7 +37,7 @@ function _M.ref()

  local _ = ngx.ctx -- load context

-  local ctx_ref = C.ngx_http_lua_ffi_get_ctx_ref(r)
+  local ctx_ref = C.ngx_http_lua_ffi_get_ctx_ref(r, ffi.new("int[1]"), ffi.new("int[1]"))


What does this do?

The C call definition changed, and now it needs to pass a integer.

Why 1? Does it matter? Or we can just send anything?
Would be good to clarify with a comment.

gateway/src/apicast/management.lua

spec/policy/find_service/find_service_spec.lua

gateway/src/resty/http_ng/backend/test.lua

spec/configuration_store_spec.lua

davidor · 2021-05-19T10:08:56Z

gateway/src/resty/http_ng/headers.lua

@@ -54,7 +56,7 @@ local header_mt = {
  __tostring = function(t)
    local tmp = {}

-    for k,v in pairs(t) do
+    for k,v in tablex_sort(t) do


Why do headers need to be sorted? Is it to pass some tests or is there some part of the code that expects them sorted?

Just to pass the test, is the only way to have consistent data.

davidor · 2021-05-19T10:22:06Z

spec/backend_client_spec.lua

+      prefix = ""
+      first = false
+    end
+    result = string.format("%s%s%s=%s",result, prefix, k, v)


Isn't it better to use concat to avoid passing the tmp string to format on every iteration?

concat has issues with sort.

t/apicast-async-reporting.t

Load configuration module and APICAST_MODULE will be disabled due to the deprecation. From now on, policy_loader should be used. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

When using ssl_certificate phase, before the context was not populated to other phases, so the ctx.service was set by access phase, and path routing was not working at all. return on path routing enabled, and setting that on the access() phase. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Semaphores are not enabled on log_by_lua phases, but we need to run this action here. Thread is not enabled, so the timer is the only way to execute this function. FYI: Timer is a local request, this is why a TCP socket can be open. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

On ngx.encode_args, the encoding is not longer sorted because Luajit changed. This way always set the encode on the sorted way to make test easier to validate. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

This adds a new helper to validate that the response json body is correct but it's not the same at all. This validate that a key can be equal or match a regular expression. Example body: ``` {"a": 1, "b": 2} ``` Example validator: ``` {"key": "a", "value": 1} {"key": "b", "value": "\d+", "op", "regexp"} ``` Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

t/apicast-async-reporting.t

Because of the Luajit update, and the sort issues, some test need to be changed to support dynamic information and not be flaky. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

APICAST_MODULE option is no longer supported, due to the new update, this feature is now deprecated and no longer supported. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Oauth2 is no longer required in the gateway, JWT and Openidconnect is the prefered way and users are aware of this deprecation since APICast 2.5 Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Because of the Luajit update, and the sort issues, some test need to be changed to support dynamic information and not be flaky. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

When setting headers, the order is important in case of duplication, using tablex that keep the order as it was set. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Bump Openresty version to 1.19.3

d27264f

Fix that updates base image to Openresty 1.19.3 version. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

eloycoto requested review from a team as code owners May 18, 2021 09:59

dfennessy reviewed May 18, 2021

View reviewed changes

eloycoto force-pushed the Openresty1.19.3 branch 3 times, most recently from a005dcb to 9c5a062 Compare May 18, 2021 15:47

eloycoto changed the title ~~WIP: Openresty Update~~ Openresty Update May 18, 2021

davidor reviewed May 19, 2021

View reviewed changes

eloycoto added 5 commits May 19, 2021 15:21

Openresty Update: Fix management API.

a5d0419

Load configuration module and APICAST_MODULE will be disabled due to the deprecation. From now on, policy_loader should be used. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Openresty Update: Fix random ngx.encode_args

afd09ef

On ngx.encode_args, the encoding is not longer sorted because Luajit changed. This way always set the encode on the sorted way to make test easier to validate. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

eloycoto force-pushed the Openresty1.19.3 branch from 9c5a062 to c6a2b58 Compare May 19, 2021 13:43

davidor reviewed May 19, 2021

View reviewed changes

t/apicast-async-reporting.t Outdated Show resolved Hide resolved

eloycoto added 6 commits May 19, 2021 16:02

Openresty Update: fix some integration tests

ff6ca74

Because of the Luajit update, and the sort issues, some test need to be changed to support dynamic information and not be flaky. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Openresty Update: Remove APICAST_MODULE OPTION

63d51f5

APICAST_MODULE option is no longer supported, due to the new update, this feature is now deprecated and no longer supported. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Openresty Update: Remove Oauth2

197452a

Oauth2 is no longer required in the gateway, JWT and Openidconnect is the prefered way and users are aware of this deprecation since APICast 2.5 Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Openresty Update: fix some unit tests

f13f987

Because of the Luajit update, and the sort issues, some test need to be changed to support dynamic information and not be flaky. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Openresty Update: Fix headers ordering issues

d19ec3a

When setting headers, the order is important in case of duplication, using tablex that keep the order as it was set. Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Openresty Update: Add openresty to path

c213176

Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

eloycoto force-pushed the Openresty1.19.3 branch from c6a2b58 to c213176 Compare May 19, 2021 14:02

davidor approved these changes May 19, 2021

View reviewed changes

Openresty Update: updating changelog

13d6802

Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

eloycoto merged commit 4aa6b91 into 3scale:master May 24, 2021

davidor mentioned this pull request May 25, 2021

Bump rack from 1.6.5 to 1.6.13 in /examples/oauth2/auth-server #1244

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Openresty Update #1272

Openresty Update #1272

eloycoto commented May 18, 2021 •

edited

Loading

dfennessy May 18, 2021

eloycoto May 18, 2021

davidor May 19, 2021

eloycoto May 19, 2021 •

edited

Loading

davidor May 19, 2021

davidor May 19, 2021

eloycoto May 19, 2021

davidor May 19, 2021

eloycoto May 19, 2021

		Deprecated: Use [policies](./policies.md) instead.

		Specifies the name of the main Lua module that implements the API gateway logic. Custom modules can override the functionality of the default `apicast.lua` module. See [an example](../examples/custom-module) of how to use modules.

Openresty Update #1272

Openresty Update #1272

Conversation

eloycoto commented May 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eloycoto May 19, 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

eloycoto commented May 18, 2021 •

edited

Loading

eloycoto May 19, 2021 •

edited

Loading