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(proto): avoid sharing state #6199

Merged
merged 1 commit into from
Jan 26, 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
42 changes: 23 additions & 19 deletions apisix/plugins/grpc-transcode/proto.lua
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
--
local core = require("apisix.core")
local config_util = require("apisix.core.config_util")
local pb = require("pb")
local protoc = require("protoc")
local pcall = pcall
local ipairs = ipairs
Expand All @@ -29,6 +30,10 @@ local lrucache_proto = core.lrucache.new({
local proto_fake_file = "filename for loaded"

local function compile_proto(content)
-- clear pb state
pb.state(nil)
Comment on lines +33 to +34
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it redundant? I think Line 54 is already retrieving the current pb state and setting it to nil.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a default pb state that we need to clear.

Copy link
Member

Choose a reason for hiding this comment

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

Got it.


protoc.reload()
local _p = protoc.new()
-- the loaded proto won't appears in _p.loaded without a file name after lua-protobuf=0.3.2,
-- which means _p.loaded after _p:load(content) is always empty, so we can pass a fake file
Expand All @@ -44,7 +49,23 @@ local function compile_proto(content)
return nil, "failed to load proto content"
end

return _p.loaded
local compiled = _p.loaded
-- fetch pb state
compiled.pb_state = pb.state(nil)

local index = {}
for _, s in ipairs(compiled[proto_fake_file].service or {}) do
local method_index = {}
for _, m in ipairs(s.method) do
method_index[m.name] = m
end

index[compiled[proto_fake_file].package .. '.' .. s.name] = method_index
end

compiled[proto_fake_file].index = index

return compiled
end


Expand All @@ -71,24 +92,7 @@ local function create_proto_obj(proto_id)
return nil, "failed to find proto by id: " .. proto_id
end

local compiled, err = compile_proto(content)

if not compiled then
return nil, err
end

local index = {}
for _, s in ipairs(compiled[proto_fake_file].service or {}) do
local method_index = {}
for _, m in ipairs(s.method) do
method_index[m.name] = m
end

index[compiled[proto_fake_file].package .. '.' .. s.name] = method_index
end

compiled[proto_fake_file].index = index
return compiled
return compile_proto(content)
end


Expand Down
9 changes: 8 additions & 1 deletion apisix/plugins/grpc-transcode/util.lua
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,14 @@ function _M.find_method(protos, service, method)
return nil
end

return loaded.index[service][method]
local res = loaded.index[service][method]
if not res then
return nil
end

-- restore pb state
pb.state(protos.pb_state)
return res
end


Expand Down
102 changes: 102 additions & 0 deletions t/plugin/grpc-transcode2.t
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,105 @@ POST /grpctest
Content-Type: application/json
--- response_body chomp
{"message":"Hello world, name: Joe"}



=== TEST 4: set rule to check if each proto is separate
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local etcd = require("apisix.core.etcd")
local code, body = t('/apisix/admin/proto/2',
ngx.HTTP_PUT,
[[{
"content" : "syntax = \"proto3\";
package helloworld;
service Greeter {
rpc SayHello (HelloRequest) returns (HelloReply) {}
}
// same message, different fields. use to pollute the type info
message HelloRequest {
string name = 1;
string person = 2;
}
message HelloReply {
string message = 1;
}"
}]]
)

if code >= 300 then
ngx.status = code
ngx.say(body)
return
end

local code, body = t('/apisix/admin/routes/2',
ngx.HTTP_PUT,
[[{
"uri": "/fail",
"plugins": {
"grpc-transcode": {
"proto_id": "2",
"service": "helloworld.Greeter",
"method": "SayHello"
}
},
"upstream": {
"scheme": "grpc",
"type": "roundrobin",
"nodes": {
"127.0.0.1:50051": 1
}
}
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- response_body
passed



=== TEST 5: hit route
--- config
location /t {
content_by_lua_block {
local http = require "resty.http"
local uri = "http://127.0.0.1:" .. ngx.var.server_port
local body = [[{"name":"world","person":{"name":"John"}}]]
local opt = {method = "POST", body = body, headers = {["Content-Type"] = "application/json"}}

local function access(path)
local httpc = http.new()
local res, err = httpc:request_uri(uri .. path, opt)
if not res then
ngx.say(err)
return
end
if res.status > 300 then
ngx.say(res.status)
else
ngx.say(res.body)
end
end

access("/fail")
access("/grpctest")
access("/fail")
access("/grpctest")
}
}
--- response_body
400
{"message":"Hello world, name: John"}
400
{"message":"Hello world, name: John"}
--- error_log
failed to encode request data to protobuf