backport 1107 (#69)

fix(bk-rate-limit): update the key to use new limits if the config changed (#62)
fix(patch/opentelemetry.lua): span kind from server to client, for to… (#63)
fix(bk-auth/legacy): patch for invalid params of old gateway (#67)
fix(bin/apisix-start.sh): change the order of apisix start command (#68)

Dockerfile

@@ -35,5 +35,6 @@ RUN ls /usr/local/apisix/patches | sort | xargs -L1 -I __patch_file__ sh -c 'cat
 
 RUN chmod 755 /data/bkgateway/bin/* && chmod 777 /usr/local/apisix/logs
 
-
 CMD ["sh", "-c", "/usr/bin/apisix init && /usr/bin/apisix init_etcd && /usr/local/openresty/bin/openresty -p /usr/local/apisix -g 'daemon off;'"]
+
+STOPSIGNAL SIGQUIT

src/apisix/plugins/README.md

@@ -18,11 +18,12 @@
 
 上下文注入，优先级：18000 ~ 19000
 
+- bk-legacy-invalid-params                  # priority: 18880  # 用于兼容老版本 go1.16 使用 `;` 作为 query string 分隔符
 - bk-opentelemetry                          # priority: 18870  # 这个插件用于 opentelemetry, 需要尽量精准统计全局的耗时，同时需要注入 trace_id/span_id 作为后面所有插件自定义 opentelemetry 上报的 trace_id 即 parent span_id
 - bk-not-found-handler                      # priority: 18860  # 该插件仅适用于由 operator 创建的默认根路由，用以规范化 404 消息。该插件以较高优先级结束请求返回 404 错误信息
 - bk-request-id                             # priority: 18850
 - bk-stage-context                          # priority: 18840
-- bk-service-context                        # priority: 18830
+- bk-service-context                        # priority: 18830 (abandonned)
 - bk-resource-context                       # priority: 18820
 - bk-status-rewrite                         # priority: 18815
 - bk-verified-user-exempted-apps            # priority: 18810 (will be deprecated)
@@ -31,8 +32,8 @@
 
 认证：
 
-- bk-workflow-parameters                    # priority: 18750
-- bk-auth-parameters                        # priority: 18740
+- bk-workflow-parameters                    # priority: 18750 (abandonned)
+- bk-auth-parameters                        # priority: 18740 (abandonned)
 - bk-auth-verify                            # priority: 18730
 
 执行 - 响应：优先级：17500 ~ 18000

src/apisix/plugins/bk-legacy-invalid-params.lua

@@ -0,0 +1,66 @@
+--
+-- TencentBlueKing is pleased to support the open source community by making
+-- 蓝鲸智云 - API 网关(BlueKing - APIGateway) available.
+-- Copyright (C) 2017 THL A29 Limited, a Tencent company. All rights reserved.
+-- Licensed under the MIT License (the "License"); you may not use this file except
+-- in compliance with the License. You may obtain a copy of the License at
+--
+--     http://opensource.org/licenses/MIT
+--
+-- Unless required by applicable law or agreed to in writing, software distributed under
+-- the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+-- either express or implied. See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+-- We undertake not to change the open source license (MIT license) applicable
+-- to the current version of the project delivered to anyone in the future.
+--
+
+-- # bk-legacy-invalid-params
+--
+-- For old gateway calling, because go 1.16 support both `&` and `;` as query string separator, but lua only support `&`
+-- and in some case, the caller html escaped the `&` to `&`(it's ok for go 1.16 gateway)
+-- so we need to adapat the old gateway calling.
+-- e.g.
+-- ?app_code=appC&app_secret=appC
+-- ?app_code=appC&app_secret=appC
+-- ?app_code=appC;app_secret=appC
+-- ?a=1;a=2
+
+local string_replace = require("pl.stringx").replace
+local string_find = string.find
+local core = require("apisix.core")
+
+local schema = {}
+
+local _M = {
+    version = 0.1,
+    priority = 18880,
+    name = "bk-legacy-invalid-params",
+    schema = schema,
+}
+
+function _M.check_schema(conf)
+    return core.schema.check(schema, conf)
+end
+
+function _M.rewrite(conf, ctx)
+    -- FIXME: 未来新的接口使用`;`也不生效, 怎么控制范围?
+
+    -- FIX 1
+    -- in golang 1.16: strings.IndexAny(key, "&;")
+    -- so here we just need to replace `;` to `&`, then reset the uri_args
+    -- args will be decoded like golang version
+
+    -- core.log.error(ctx.var.args)
+    -- only query string contains `;` should be processed
+    if ctx.var.args ~= nil and string_find(ctx.var.args, ";") then
+        local new_args = string_replace(ctx.var.args, ";", "&")
+        -- core.log.error("replace ; to &: ", new_args)
+        core.request.set_uri_args(ctx, new_args)
+    end
+    -- local args = core.request.get_uri_args()
+    -- core.log.error(core.json.delay_encode(args))
+end
+
+return _M

src/apisix/plugins/bk-rate-limit/init.lua

@@ -35,6 +35,7 @@
 --     redis_database: 0
 --     redis_timeout: 1001
 --
+local apisix_plugin = require("apisix.plugin")
 local core = require("apisix.core")
 local rate_limit_redis = require("apisix.plugins.bk-rate-limit.rate-limit-redis")
 local lrucache = core.lrucache.new(
@@ -77,6 +78,14 @@ local _M = {
     },
 }
 
+local function gen_limit_key(conf, key)
+    -- Here we use plugin-level conf version to prevent the counter from being resetting
+    -- because of the change elsewhere.
+    -- e.g. conf_version = 1969078430
+    local new_key = key .. ':' .. apisix_plugin.conf_version(conf)
+    return new_key
+end
+
 ---Create rate-limit-redis object
 ---@param plugin_name string @apisix plugin name
 ---@return table @rate-limit-redis object
@@ -108,6 +117,7 @@ function _M.rate_limit(conf, ctx, plugin_name, key, count, time_window)
         return 500
     end
 
+    key = gen_limit_key(conf, key)
     core.log.info("limit key: ", key)
 
     local delay, remaining, reset = lim:incoming(key, count, time_window)
@@ -145,4 +155,8 @@ function _M.rate_limit(conf, ctx, plugin_name, key, count, time_window)
     end
 end
 
+if _TEST then
+    _M.gen_limit_key = gen_limit_key
+end
+
 return _M

src/apisix/plugins/bk-resource-rate-limit.lua

@@ -62,6 +62,8 @@ function _M.access(conf, ctx)
     end
 
     -- TODO: make it lazy, share the key with other plugins
+    -- FIXME: should change the bk_reosurce_name to bk_resource_id if you need more accurate rate limit
+    --        while the developer may change the bk_resource_name from the frontend page.
     local key = table_concat(
         {
             bk_app_code,

src/apisix/plugins/bk-stage-rate-limit.lua

@@ -85,7 +85,6 @@ function _M.access(conf, ctx)
     else
         for i, rate in ipairs(rates) do
             -- here we should add the rate index into key, otherwise the rate limit will be shared(will be wrong)
-            -- FIXME: if the rate changes, will wait for the period to effect
             local limit_key = key .. ":" .. tostring(i)
             local code = ratelimit.rate_limit(conf, ctx, plugin_name, limit_key, rate.tokens, rate.period)
             if code then

src/apisix/t/bk-legacy-invalid-params.t

@@ -0,0 +1,116 @@
+#
+# TencentBlueKing is pleased to support the open source community by making
+# 蓝鲸智云 - API 网关(BlueKing - APIGateway) available.
+# Copyright (C) 2017 THL A29 Limited, a Tencent company. All rights reserved.
+# Licensed under the MIT License (the "License"); you may not use this file except
+# in compliance with the License. You may obtain a copy of the License at
+#
+#     http://opensource.org/licenses/MIT
+#
+# Unless required by applicable law or agreed to in writing, software distributed under
+# the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+# either express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# We undertake not to change the open source license (MIT license) applicable
+# to the current version of the project delivered to anyone in the future.
+#
+
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }
+});
+
+run_tests;
+
+__DATA__
+=== TEST 1: sanity
+--- config
+    location /t {
+        content_by_lua_block {
+            local plugin = require("apisix.plugins.bk-legacy-invalid-params")
+            local ok, err = plugin.check_schema({})
+            if not ok then
+                ngx.say(err)
+            end
+            ngx.say("done")
+        }
+    }
+--- response_body
+done
+=== TEST 2: add plugin
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                        "plugins": {
+                            "bk-legacy-invalid-params": {
+                            },
+                            "mocking": {
+                                "content_type": "text/plain",
+                                "response_status": 200,
+                                "response_example": "args:$args\n"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1982": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                }]]
+                )
+            if code >= 300 then
+                ngx.status = code
+            end
+            -- code is 201, body is passed
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+=== TEST 3: call no args
+--- request
+GET /hello
+--- response_body
+args:
+
+=== TEST 4: call with normal args
+--- request
+GET /hello?a=1&b=2
+--- response_body
+args:a=1&b=2
+
+=== TEST 5: call with `;`
+--- request
+GET /hello?a=1;b=2
+--- response_body
+args:a=1&b=2
+
+=== TEST 6: call with `&`
+--- request
+GET /hello?a=1&b=2
+--- response_body
+args:a=1&amp&b=2
+
+=== TEST 7: call with `&`
+--- request
+GET /hello?a=1&b=2
+--- response_body
+args:a=1&amp&amp&b=2
+
+

src/apisix/tests/bk-rate-limit/test-init.lua

@@ -48,6 +48,25 @@ describe(
             end
         )
 
+        context(
+            "gen_limit_key", function()
+                it("ok", function()
+                    local conf = {}
+                    local key = ratelimit.gen_limit_key(conf, "key")
+                    assert.equal(key, "key:223132457")
+                    assert.equal(conf._version, "223132457")
+
+                    -- mock the conf change
+                    conf._version = nil
+                    conf["hello"] = "world"
+                    key = ratelimit.gen_limit_key(conf, "key")
+                    assert.equal(key, "key:3624485329")
+                end
+            )
+            end
+        )
+
+
         context(
             "rate_limit", function()
                 local conf

src/apisix/tests/test-bk-legacy-invalid-params.lua

@@ -0,0 +1,94 @@
+--
+-- TencentBlueKing is pleased to support the open source community by making
+-- 蓝鲸智云 - API 网关(BlueKing - APIGateway) available.
+-- Copyright (C) 2017 THL A29 Limited, a Tencent company. All rights reserved.
+-- Licensed under the MIT License (the "License"); you may not use this file except
+-- in compliance with the License. You may obtain a copy of the License at
+--
+--     http://opensource.org/licenses/MIT
+--
+-- Unless required by applicable law or agreed to in writing, software distributed under
+-- the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+-- either express or implied. See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+-- We undertake not to change the open source license (MIT license) applicable
+-- to the current version of the project delivered to anyone in the future.
+--
+
+local core = require("apisix.core")
+
+local plugin = require("apisix.plugins.bk-legacy-invalid-params")
+
+describe(
+    "bk-legacy-invalid-params",
+    function()
+        context(
+            "rewrite",
+            function()
+                local ctx
+                before_each(
+                    function()
+                        ctx = {
+                            var = {}
+                        }
+                        stub(core.request, "set_uri_args")
+                    end
+                )
+
+                after_each(
+                    function()
+                        -- ngx.req.clear_header:revert()
+                        core.request.set_uri_args:revert()
+                    end
+                )
+
+                it(
+                    "no args",
+                    function()
+                        plugin.rewrite({}, ctx)
+
+                        assert.stub(core.request.set_uri_args).was_not_called()
+                    end
+                )
+                it(
+                    "normal args with &",
+                    function()
+                        ctx.var.args = "a=1&b=2"
+                        plugin.rewrite({}, ctx)
+
+                        assert.stub(core.request.set_uri_args).was_not_called()
+                    end
+                )
+                it(
+                    "args with ;",
+                    function()
+                        ctx.var.args = "a=1;b=2"
+                        plugin.rewrite({}, ctx)
+
+                        assert.stub(core.request.set_uri_args).was_called_with(ctx, "a=1&b=2")
+                    end
+                )
+                it(
+                    "args with &",
+                    function()
+                        ctx.var.args = "a=1&b=2"
+                        plugin.rewrite({}, ctx)
+
+                        assert.stub(core.request.set_uri_args).was_called_with(ctx, "a=1&amp&b=2")
+                    end
+                )
+                it(
+                    "args with &",
+                    function()
+                        ctx.var.args = "a=1&b=2"
+                        plugin.rewrite({}, ctx)
+
+                        assert.stub(core.request.set_uri_args).was_called_with(ctx, "a=1&amp&amp&b=2")
+                    end
+                )
+
+            end
+        )
+    end
+)