From 3e8c2d22bb7e2038207a25bd3b895def4a44b3cb Mon Sep 17 00:00:00 2001 From: chronolaw Date: Wed, 15 Jan 2025 07:08:39 +0800 Subject: [PATCH 1/6] tests(clustering/rpc): add cases for rpc error handler --- kong/clustering/rpc/socket.lua | 7 +++ .../18-hybrid_rpc/02-error_spec.lua | 63 +++++++++++++++++++ .../kong/plugins/rpc-error-test/handler.lua | 54 ++++++++++++++++ .../kong/plugins/rpc-error-test/schema.lua | 12 ++++ 4 files changed, 136 insertions(+) create mode 100644 spec/02-integration/18-hybrid_rpc/02-error_spec.lua create mode 100644 spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua create mode 100644 spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/schema.lua diff --git a/kong/clustering/rpc/socket.lua b/kong/clustering/rpc/socket.lua index 34d1b7b1aa03..69fc9fe1ca77 100644 --- a/kong/clustering/rpc/socket.lua +++ b/kong/clustering/rpc/socket.lua @@ -202,6 +202,13 @@ function _M:process_rpc_msg(payload, collection) if not interest_cb then ngx_log(ngx_WARN, "[rpc] no interest for RPC response id: ", payload_id, ", dropping it") + -- may be some error message for peer + if payload.error then + ngx_log(ngx.ERR, "[rpc] RPC failed, code: ", + payload.error.code, ", err: ", + payload.error.message) + end + return true end diff --git a/spec/02-integration/18-hybrid_rpc/02-error_spec.lua b/spec/02-integration/18-hybrid_rpc/02-error_spec.lua new file mode 100644 index 000000000000..1caf7115a736 --- /dev/null +++ b/spec/02-integration/18-hybrid_rpc/02-error_spec.lua @@ -0,0 +1,63 @@ +local helpers = require "spec.helpers" + + +-- register a test rpc service in custom plugin rpc-error-test +for _, strategy in helpers.each_strategy() do + describe("Hybrid Mode RPC #" .. strategy, function() + + lazy_setup(function() + helpers.get_db_utils(strategy, { + "clustering_data_planes", + }) -- runs migrations + + assert(helpers.start_kong({ + role = "control_plane", + cluster_cert = "spec/fixtures/kong_clustering.crt", + cluster_cert_key = "spec/fixtures/kong_clustering.key", + database = strategy, + cluster_listen = "127.0.0.1:9005", + nginx_conf = "spec/fixtures/custom_nginx.template", + plugins = "bundled,rpc-error-test", + nginx_worker_processes = 4, -- multiple workers + cluster_rpc = "on", -- enable rpc + cluster_rpc_sync = "off", -- disable rpc sync + })) + + assert(helpers.start_kong({ + role = "data_plane", + database = "off", + prefix = "servroot2", + cluster_cert = "spec/fixtures/kong_clustering.crt", + cluster_cert_key = "spec/fixtures/kong_clustering.key", + cluster_control_plane = "127.0.0.1:9005", + proxy_listen = "0.0.0.0:9002", + nginx_conf = "spec/fixtures/custom_nginx.template", + plugins = "bundled,rpc-error-test", + nginx_worker_processes = 4, -- multiple workers + cluster_rpc = "on", -- enable rpc + cluster_rpc_sync = "off", -- disable rpc sync + })) + end) + + lazy_teardown(function() + helpers.stop_kong("servroot2") + helpers.stop_kong() + end) + + describe("rpc errors", function() + it("in custom plugin", function() + local name = "servroot2/logs/error.log" + + -- dp logs + helpers.pwait_until(function() + assert.logfile(name).has.line( + "test #1 ok", true) + assert.logfile(name).has.no.line( + "assertion failed", true) + return true + end, 10) + + end) + end) + end) +end -- for _, strategy diff --git a/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua new file mode 100644 index 000000000000..838ca7211d0d --- /dev/null +++ b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua @@ -0,0 +1,54 @@ +local cjson = require("cjson") + + +local RpcErrorTestHandler = { + VERSION = "1.0", + PRIORITY = 1000, +} + + +function RpcErrorTestHandler:init_worker() + kong.rpc.callbacks:register("kong.test.exception", function(node_id) + return nil -- no error message, default jsonrpc.SERVER_ERROR + end) + + kong.rpc.callbacks:register("kong.test.error", function(node_id) + return nil, "kong.test.error" + end) + + local worker_events = assert(kong.worker_events) + local node_id = "control_plane" + + -- if rpc is ready we will start to call + worker_events.register(function(capabilities_list) + local res, err = kong.rpc:call(node_id, "kong.test.not_exist") + assert(not res) + assert(err == "Method not found") + + local res, err = kong.rpc:call(node_id, "kong.test.exception") + assert(not res) + assert(err == "Server error") + + local res, err = kong.rpc:call(node_id, "kong.test.error") + assert(not res) + assert(err == "kong.test.error") + + ngx.log(ngx.DEBUG, "test #1 ok") + + end, "clustering:jsonrpc", "connected") + + worker_events.register(function(capabilities_list) + local node_id = "control_plane" + + --local s = next(kong.rpc.clients[node_id]) + + -- send a empty array + --assert(s:push_request({})) + ngx.log(ngx.DEBUG, "test #2 ok") + + end, "clustering:jsonrpc", "connected") + +end + + +return RpcErrorTestHandler diff --git a/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/schema.lua b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/schema.lua new file mode 100644 index 000000000000..ba3266fda136 --- /dev/null +++ b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/schema.lua @@ -0,0 +1,12 @@ +return { + name = "rpc-error-test", + fields = { + { + config = { + type = "record", + fields = { + }, + }, + }, + }, +} From ba883a257cfcd249d6ea447bbd3027c2142e1308 Mon Sep 17 00:00:00 2001 From: chronolaw Date: Wed, 15 Jan 2025 07:25:00 +0800 Subject: [PATCH 2/6] test batch --- kong/clustering/rpc/socket.lua | 18 +++++++++++------- .../18-hybrid_rpc/02-error_spec.lua | 10 ++++++++++ .../kong/plugins/rpc-error-test/handler.lua | 8 ++++---- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/kong/clustering/rpc/socket.lua b/kong/clustering/rpc/socket.lua index 69fc9fe1ca77..9739e576d025 100644 --- a/kong/clustering/rpc/socket.lua +++ b/kong/clustering/rpc/socket.lua @@ -195,6 +195,17 @@ function _M:process_rpc_msg(payload, collection) end else + -- may be some error message for peer + if not payload_id then + if payload.error then + ngx_log(ngx.ERR, "[rpc] RPC failed, code: ", + payload.error.code, ", err: ", + payload.error.message) + end + + return true + end + -- response, don't care about `collection` local interest_cb = self.interest[payload_id] self.interest[payload_id] = nil -- edge trigger only once @@ -202,13 +213,6 @@ function _M:process_rpc_msg(payload, collection) if not interest_cb then ngx_log(ngx_WARN, "[rpc] no interest for RPC response id: ", payload_id, ", dropping it") - -- may be some error message for peer - if payload.error then - ngx_log(ngx.ERR, "[rpc] RPC failed, code: ", - payload.error.code, ", err: ", - payload.error.message) - end - return true end diff --git a/spec/02-integration/18-hybrid_rpc/02-error_spec.lua b/spec/02-integration/18-hybrid_rpc/02-error_spec.lua index 1caf7115a736..43b8f6089f31 100644 --- a/spec/02-integration/18-hybrid_rpc/02-error_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/02-error_spec.lua @@ -57,6 +57,16 @@ for _, strategy in helpers.each_strategy() do return true end, 10) + -- dp logs + helpers.pwait_until(function() + assert.logfile(name).has.line( + "[rpc] RPC failed, code: -32600, err: empty batch array", true) + assert.logfile(name).has.line( + "test #2 ok", true) + assert.logfile(name).has.no.line( + "assertion failed", true) + return true + end, 10) end) end) end) diff --git a/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua index 838ca7211d0d..925292a987f9 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua @@ -38,12 +38,12 @@ function RpcErrorTestHandler:init_worker() end, "clustering:jsonrpc", "connected") worker_events.register(function(capabilities_list) - local node_id = "control_plane" + local s = next(kong.rpc.clients[node_id]) - --local s = next(kong.rpc.clients[node_id]) + -- send an empty array + local msg = setmetatable({}, cjson.array_mt) + assert(s:push_request({})) - -- send a empty array - --assert(s:push_request({})) ngx.log(ngx.DEBUG, "test #2 ok") end, "clustering:jsonrpc", "connected") From 6fa2315381fa09ad8f4c862216d1e62e7a58fcb2 Mon Sep 17 00:00:00 2001 From: chronolaw Date: Wed, 15 Jan 2025 08:08:36 +0800 Subject: [PATCH 3/6] jsonrpc.PARSE_ERROR --- kong/clustering/rpc/socket.lua | 10 ++++++++++ spec/02-integration/18-hybrid_rpc/02-error_spec.lua | 6 ++++++ .../kong/plugins/rpc-error-test/handler.lua | 7 ++++++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/kong/clustering/rpc/socket.lua b/kong/clustering/rpc/socket.lua index 9739e576d025..676864e3c389 100644 --- a/kong/clustering/rpc/socket.lua +++ b/kong/clustering/rpc/socket.lua @@ -282,6 +282,16 @@ function _M:start() local payload = decompress_payload(data) + if not payload then + local res, err = self:push_response( + new_error(nil, jsonrpc.PARSE_ERROR)) + if not res then + return nil, err + end + + goto continue + end + -- single rpc call if not isarray(payload) then local ok, err = self:process_rpc_msg(payload) diff --git a/spec/02-integration/18-hybrid_rpc/02-error_spec.lua b/spec/02-integration/18-hybrid_rpc/02-error_spec.lua index 43b8f6089f31..4a00fd32b38b 100644 --- a/spec/02-integration/18-hybrid_rpc/02-error_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/02-error_spec.lua @@ -61,6 +61,12 @@ for _, strategy in helpers.each_strategy() do helpers.pwait_until(function() assert.logfile(name).has.line( "[rpc] RPC failed, code: -32600, err: empty batch array", true) + + assert.logfile(dp_logfile).has.line( + "[rpc] got batch RPC call: 1", true) + assert.logfile(name).has.line( + "[rpc] RPC failed, code: -32600, err: not a valid object", true) + assert.logfile(name).has.line( "test #2 ok", true) assert.logfile(name).has.no.line( diff --git a/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua index 925292a987f9..58aafafd8404 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua @@ -37,12 +37,17 @@ function RpcErrorTestHandler:init_worker() end, "clustering:jsonrpc", "connected") + -- if rpc is ready we will start to send raw msg worker_events.register(function(capabilities_list) local s = next(kong.rpc.clients[node_id]) -- send an empty array local msg = setmetatable({}, cjson.array_mt) - assert(s:push_request({})) + assert(s:push_request(msg)) + + -- send a invalid msg + local msg = ({"invalid_request"}) + assert(s:push_request(msg)) ngx.log(ngx.DEBUG, "test #2 ok") From 43abe0e93be2ed947a33dcbb0c25e7d782882335 Mon Sep 17 00:00:00 2001 From: chronolaw Date: Wed, 15 Jan 2025 08:17:19 +0800 Subject: [PATCH 4/6] lint fix --- spec/02-integration/18-hybrid_rpc/02-error_spec.lua | 2 -- .../custom_plugins/kong/plugins/rpc-error-test/handler.lua | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/02-integration/18-hybrid_rpc/02-error_spec.lua b/spec/02-integration/18-hybrid_rpc/02-error_spec.lua index 4a00fd32b38b..2773fcc245c7 100644 --- a/spec/02-integration/18-hybrid_rpc/02-error_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/02-error_spec.lua @@ -62,8 +62,6 @@ for _, strategy in helpers.each_strategy() do assert.logfile(name).has.line( "[rpc] RPC failed, code: -32600, err: empty batch array", true) - assert.logfile(dp_logfile).has.line( - "[rpc] got batch RPC call: 1", true) assert.logfile(name).has.line( "[rpc] RPC failed, code: -32600, err: not a valid object", true) diff --git a/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua index 58aafafd8404..c3bd5da9ed3d 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua @@ -45,7 +45,7 @@ function RpcErrorTestHandler:init_worker() local msg = setmetatable({}, cjson.array_mt) assert(s:push_request(msg)) - -- send a invalid msg + -- send an invalid msg local msg = ({"invalid_request"}) assert(s:push_request(msg)) From 736087428f58a73b79f4dd306a6391427b1a0221 Mon Sep 17 00:00:00 2001 From: chronolaw Date: Wed, 15 Jan 2025 10:13:55 +0800 Subject: [PATCH 5/6] clean --- .../custom_plugins/kong/plugins/rpc-error-test/handler.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua index c3bd5da9ed3d..e044784c4de2 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/rpc-error-test/handler.lua @@ -46,7 +46,7 @@ function RpcErrorTestHandler:init_worker() assert(s:push_request(msg)) -- send an invalid msg - local msg = ({"invalid_request"}) + local msg = setmetatable({"invalid_request"}, cjson.array_mt) assert(s:push_request(msg)) ngx.log(ngx.DEBUG, "test #2 ok") From a15193f98d2b9a7f42d390dde3d71c4b18134352 Mon Sep 17 00:00:00 2001 From: chronolaw Date: Thu, 16 Jan 2025 11:51:25 +0800 Subject: [PATCH 6/6] no pwait_until --- .../18-hybrid_rpc/02-error_spec.lua | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/spec/02-integration/18-hybrid_rpc/02-error_spec.lua b/spec/02-integration/18-hybrid_rpc/02-error_spec.lua index 2773fcc245c7..1b7a07ae6f44 100644 --- a/spec/02-integration/18-hybrid_rpc/02-error_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/02-error_spec.lua @@ -49,28 +49,19 @@ for _, strategy in helpers.each_strategy() do local name = "servroot2/logs/error.log" -- dp logs - helpers.pwait_until(function() - assert.logfile(name).has.line( - "test #1 ok", true) - assert.logfile(name).has.no.line( - "assertion failed", true) - return true - end, 10) + assert.logfile(name).has.line( + "test #1 ok", true, 10) -- dp logs - helpers.pwait_until(function() - assert.logfile(name).has.line( - "[rpc] RPC failed, code: -32600, err: empty batch array", true) + assert.logfile(name).has.line( + "[rpc] RPC failed, code: -32600, err: empty batch array", true, 10) + assert.logfile(name).has.line( + "[rpc] RPC failed, code: -32600, err: not a valid object", true, 10) + assert.logfile(name).has.line( + "test #2 ok", true, 10) - assert.logfile(name).has.line( - "[rpc] RPC failed, code: -32600, err: not a valid object", true) - - assert.logfile(name).has.line( - "test #2 ok", true) - assert.logfile(name).has.no.line( - "assertion failed", true) - return true - end, 10) + assert.logfile(name).has.no.line( + "assertion failed", true, 0) end) end) end)