From 899d65dc8652eefe31c8ace771c5f5389d58ef45 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Thu, 18 Aug 2022 17:36:52 +0800 Subject: [PATCH] change: move the disable to _meta (#7707) --- apisix/plugin.lua | 43 ++++++++++++++-------------- apisix/schema_def.lua | 6 ++-- docs/en/latest/admin-api.md | 2 +- docs/en/latest/terminology/plugin.md | 15 ++++++++++ docs/zh/latest/admin-api.md | 2 +- docs/zh/latest/terminology/plugin.md | 15 ++++++++++ t/admin/plugins.t | 12 ++++---- t/admin/proto.t | 4 ++- t/admin/routes.t | 4 ++- t/control/schema.t | 12 ++++++-- t/node/merge-route.t | 4 ++- t/plugin/basic-auth.t | 4 +-- t/plugin/hmac-auth2.t | 4 +-- t/plugin/ip-restriction.t | 4 ++- t/plugin/jwt-auth.t | 4 +-- t/plugin/ldap-auth.t | 4 +-- t/plugin/limit-count2.t | 4 ++- t/plugin/opa2.t | 8 ++++-- t/plugin/ua-restriction.t | 4 ++- 19 files changed, 104 insertions(+), 51 deletions(-) diff --git a/apisix/plugin.lua b/apisix/plugin.lua index a624a56961c6..a0fd0719fa49 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -76,6 +76,21 @@ local function custom_sort_plugin(l, r) return l._meta.priority > r._meta.priority end +local function check_disable(plugin_conf) + if not plugin_conf then + return nil + end + + if not plugin_conf._meta then + return nil + end + + if type(plugin_conf._meta) ~= "table" then + return nil + end + + return plugin_conf._meta.disable +end local PLUGIN_TYPE_HTTP = 1 local PLUGIN_TYPE_STREAM = 2 @@ -143,14 +158,6 @@ local function load_plugin(name, plugins_list, plugin_type) local plugin_injected_schema = core.schema.plugin_injected_schema if plugin.schema['$comment'] ~= plugin_injected_schema['$comment'] then - if properties.disable then - core.log.error("invalid plugin [", name, - "]: found forbidden 'disable' field in the schema") - return - end - - properties.disable = plugin_injected_schema.disable - if properties._meta then core.log.error("invalid plugin [", name, "]: found forbidden '_meta' field in the schema") @@ -161,7 +168,6 @@ local function load_plugin(name, plugins_list, plugin_type) -- new injected fields should be added under `_meta` -- 1. so we won't break user's code when adding any new injected fields -- 2. the semantics is clear, especially in the doc and in the caller side - -- TODO: move the `disable` to `_meta` too plugin.schema['$comment'] = plugin_injected_schema['$comment'] end @@ -434,10 +440,12 @@ function _M.filter(ctx, conf, plugins, route_conf, phase) end local matched = meta_filter(ctx, name, plugin_conf) - if not plugin_conf.disable and matched then + local disable = check_disable(plugin_conf) + if not disable and matched then if plugin_obj.run_policy == "prefer_route" and route_plugin_conf ~= nil then local plugin_conf_in_route = route_plugin_conf[name] - if plugin_conf_in_route and not plugin_conf_in_route.disable then + local disable_in_route = check_disable(plugin_conf_in_route) + if plugin_conf_in_route and not disable_in_route then goto continue end end @@ -515,7 +523,8 @@ function _M.stream_filter(user_route, plugins) local name = plugin_obj.name local plugin_conf = user_plugin_conf[name] - if type(plugin_conf) == "table" and not plugin_conf.disable then + local disable = check_disable(plugin_conf) + if type(plugin_conf) == "table" and not disable then core.table.insert(plugins, plugin_obj) core.table.insert(plugins, plugin_conf) end @@ -761,9 +770,6 @@ local function check_single_plugin_schema(name, plugin_conf, schema_type, skip_d end if plugin_obj.check_schema then - local disable = plugin_conf.disable - plugin_conf.disable = nil - local ok, err = plugin_obj.check_schema(plugin_conf, schema_type) if not ok then return false, "failed to check the configuration of plugin " @@ -776,8 +782,6 @@ local function check_single_plugin_schema(name, plugin_conf, schema_type, skip_d return nil, "failed to validate the 'vars' expression: " .. err end end - - plugin_conf.disable = disable end return true @@ -825,16 +829,11 @@ local function stream_check_schema(plugins_conf, schema_type, skip_disabled_plug end if plugin_obj.check_schema then - local disable = plugin_conf.disable - plugin_conf.disable = nil - local ok, err = plugin_obj.check_schema(plugin_conf, schema_type) if not ok then return false, "failed to check the configuration of " .. "stream plugin [" .. name .. "]: " .. err end - - plugin_conf.disable = disable end ::CONTINUE:: diff --git a/apisix/schema_def.lua b/apisix/schema_def.lua index 1fc8dfacb30f..6169cc16ea1b 100644 --- a/apisix/schema_def.lua +++ b/apisix/schema_def.lua @@ -937,12 +937,12 @@ _M.id_schema = id_schema _M.plugin_injected_schema = { ["$comment"] = "this is a mark for our injected plugin schema", - disable = { - type = "boolean", - }, _meta = { type = "object", properties = { + disable = { + type = "boolean", + }, error_response = { oneOf = { { type = "string" }, diff --git a/docs/en/latest/admin-api.md b/docs/en/latest/admin-api.md index 8193f61528f5..9650bfcb3921 100644 --- a/docs/en/latest/admin-api.md +++ b/docs/en/latest/admin-api.md @@ -1001,7 +1001,7 @@ $ curl "http://127.0.0.1:9080/apisix/admin/plugins/list" -H 'X-API-KEY: edd ["zipkin","request-id",...] $ curl "http://127.0.0.1:9080/apisix/admin/plugins/key-auth" -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -{"properties":{"disable":{"type":"boolean"}},"additionalProperties":false,"type":"object"} +{"$comment":"this is a mark for our injected plugin schema","properties":{"header":{"default":"apikey","type":"string"},"hide_credentials":{"default":false,"type":"boolean"},"_meta":{"properties":{"filter":{"type":"array","description":"filter determines whether the plugin needs to be executed at runtime"},"disable":{"type":"boolean"},"error_response":{"oneOf":[{"type":"string"},{"type":"object"}]},"priority":{"type":"integer","description":"priority of plugins by customized order"}},"type":"object"},"query":{"default":"apikey","type":"string"}},"type":"object"} ``` **API**: /apisix/admin/plugins?all=true diff --git a/docs/en/latest/terminology/plugin.md b/docs/en/latest/terminology/plugin.md index cd4576d343d6..bbfdf29edbc3 100644 --- a/docs/en/latest/terminology/plugin.md +++ b/docs/en/latest/terminology/plugin.md @@ -80,10 +80,25 @@ Some common configurations can be applied to plugins through the `_meta` configu | Name | Type | Description | |--------------|------|-------------| +| disable | boolean | Whether to disable the plugin | | error_response | string/object | Custom error response | | priority | integer | Custom plugin priority | | filter | array | Depending on the requested parameters, it is decided at runtime whether the plugin should be executed. Something like this: `{{var, operator, val}, {var, operator, val}, ...}}`. For example: `{"arg_version", "==", "v2"}`, indicating that the current request parameter `version` is `v2`. The variables here are consistent with NGINX internal variables. For details on supported operators, please see [lua-resty-expr](https://github.com/api7/lua-resty-expr#operator-list). | +### Disable the plugin + +Through the `disable` configuration, you can add a new plugin with disabled status and the request will not go through the plugin. + +```json +{ + "proxy-rewrite": { + "_meta": { + "disable": true + } + } +} +``` + ### Custom error response Through the `error_response` configuration, you can configure the error response of any plugin to a fixed value to avoid troubles caused by the built-in error response information of the plugin. diff --git a/docs/zh/latest/admin-api.md b/docs/zh/latest/admin-api.md index a5a2dffaaa36..bdd9734b8028 100644 --- a/docs/zh/latest/admin-api.md +++ b/docs/zh/latest/admin-api.md @@ -1019,7 +1019,7 @@ $ curl "http://127.0.0.1:9080/apisix/admin/plugins/list" -H 'X-API-KEY: edd ["zipkin","request-id",...] $ curl "http://127.0.0.1:9080/apisix/admin/plugins/key-auth" -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -{"properties":{"disable":{"type":"boolean"}},"additionalProperties":false,"type":"object"} +{"$comment":"this is a mark for our injected plugin schema","properties":{"header":{"default":"apikey","type":"string"},"hide_credentials":{"default":false,"type":"boolean"},"_meta":{"properties":{"filter":{"type":"array","description":"filter determines whether the plugin needs to be executed at runtime"},"disable":{"type":"boolean"},"error_response":{"oneOf":[{"type":"string"},{"type":"object"}]},"priority":{"type":"integer","description":"priority of plugins by customized order"}},"type":"object"},"query":{"default":"apikey","type":"string"}},"type":"object"} ``` *地址*：/apisix/admin/plugins?all=true diff --git a/docs/zh/latest/terminology/plugin.md b/docs/zh/latest/terminology/plugin.md index c22ac9243e7c..8aea99c006f0 100644 --- a/docs/zh/latest/terminology/plugin.md +++ b/docs/zh/latest/terminology/plugin.md @@ -72,10 +72,25 @@ local _M = { | 名称 | 类型 | 描述 | |--------------|------|----------------| +| disable | boolean | 是否禁用该插件。 | | error_response | string/object | 自定义错误响应。 | | priority | integer | 自定义插件优先级。 | | filter | array | 根据请求的参数，在运行时控制插件是否执行。此配置由一个或多个 {var, operator, val} 元素组成列表，类似：{{var, operator, val}, {var, operator, val}, ...}}。例如 `{"arg_version", "==", "v2"}`，表示当前请求参数 `version` 是 `v2`。这里的 `var` 与 NGINX 内部自身变量命名是保持一致。操作符的具体用法请看[lua-resty-expr](https://github.com/api7/lua-resty-expr#operator-list) 的 operator-list 部分。| +### 禁用插件 + +通过 `disable` 配置，你可以新增一个处于禁用状态的插件，请求不会经过该插件。 + +```json +{ + "proxy-rewrite": { + "_meta": { + "disable": true + } + } +} +``` + ### 自定义错误响应 通过 `error_response` 配置，可以将任意插件的错误响应配置成一个固定的值，避免因为插件内置的错误响应信息而带来困扰。 diff --git a/t/admin/plugins.t b/t/admin/plugins.t index 1d78b2432989..6c89c2bb0858 100644 --- a/t/admin/plugins.t +++ b/t/admin/plugins.t @@ -150,7 +150,7 @@ GET /apisix/admin/plugins ngx.HTTP_GET, nil, [[ - {"type":"object","required":["rate","burst","key"],"properties":{"rate":{"type":"number","exclusiveMinimum":0},"key_type":{"type":"string","enum":["var","var_combination"],"default":"var"},"burst":{"type":"number","minimum":0},"disable":{"type":"boolean"},"nodelay":{"type":"boolean","default":false},"key":{"type":"string"},"rejected_code":{"type":"integer","minimum":200,"maximum":599,"default":503},"rejected_msg":{"type":"string","minLength":1},"allow_degradation":{"type":"boolean","default":false}}} + {"type":"object","required":["rate","burst","key"],"properties":{"rate":{"type":"number","exclusiveMinimum":0},"key_type":{"type":"string","enum":["var","var_combination"],"default":"var"},"burst":{"type":"number","minimum":0},"nodelay":{"type":"boolean","default":false},"key":{"type":"string"},"rejected_code":{"type":"integer","minimum":200,"maximum":599,"default":503},"rejected_msg":{"type":"string","minLength":1},"allow_degradation":{"type":"boolean","default":false}}} ]] ) @@ -172,7 +172,7 @@ plugins: ngx.HTTP_GET, nil, [[ -{"properties":{"disable":{"type":"boolean"}},"type":"object"} +{"properties":{},"type":"object"} ]] ) @@ -191,7 +191,7 @@ plugins: ngx.HTTP_GET, nil, [[ -{"properties":{"disable":{"type":"boolean"}},"type":"object"} +{"properties":{},"type":"object"} ]] ) @@ -210,7 +210,7 @@ plugins: ngx.HTTP_GET, nil, [[ -{"properties":{"disable":{"type":"boolean"}},"title":"work with route or service object","type":"object"} +{"properties":{},"title":"work with route or service object","type":"object"} ]] ) @@ -265,7 +265,7 @@ plugins: } } --- response_body eval -qr/\{"metadata_schema":\{"properties":\{"ikey":\{"minimum":0,"type":"number"\},"skey":\{"type":"string"\}\},"required":\["ikey","skey"\],"type":"object"\},"priority":0,"schema":\{"\$comment":"this is a mark for our injected plugin schema","properties":\{"_meta":\{"properties":\{"error_response":\{"oneOf":\[\{"type":"string"\},\{"type":"object"\}\]\},"filter":\{"description":"filter determines whether the plugin needs to be executed at runtime","type":"array"\},"priority":\{"description":"priority of plugins by customized order","type":"integer"\}\},"type":"object"\},"disable":\{"type":"boolean"\},"i":\{"minimum":0,"type":"number"\},"ip":\{"type":"string"\},"port":\{"type":"integer"\},"s":\{"type":"string"\},"t":\{"minItems":1,"type":"array"\}\},"required":\["i"\],"type":"object"\},"version":0.1\}/ +qr/\{"metadata_schema":\{"properties":\{"ikey":\{"minimum":0,"type":"number"\},"skey":\{"type":"string"\}\},"required":\["ikey","skey"\],"type":"object"\},"priority":0,"schema":\{"\$comment":"this is a mark for our injected plugin schema","properties":\{"_meta":\{"properties":\{"disable":\{"type":"boolean"\},"error_response":\{"oneOf":\[\{"type":"string"\},\{"type":"object"\}\]\},"filter":\{"description":"filter determines whether the plugin needs to be executed at runtime","type":"array"\},"priority":\{"description":"priority of plugins by customized order","type":"integer"\}\},"type":"object"\},"i":\{"minimum":0,"type":"number"\},"ip":\{"type":"string"\},"port":\{"type":"integer"\},"s":\{"type":"string"\},"t":\{"minItems":1,"type":"array"\}\},"required":\["i"\],"type":"object"\},"version":0.1\}/ @@ -366,7 +366,7 @@ qr/\{"properties":\{"password":\{"type":"string"\},"username":\{"type":"string"\ } } --- response_body -{"priority":1003,"schema":{"$comment":"this is a mark for our injected plugin schema","properties":{"_meta":{"properties":{"error_response":{"oneOf":[{"type":"string"},{"type":"object"}]},"filter":{"description":"filter determines whether the plugin needs to be executed at runtime","type":"array"},"priority":{"description":"priority of plugins by customized order","type":"integer"}},"type":"object"},"burst":{"minimum":0,"type":"integer"},"conn":{"exclusiveMinimum":0,"type":"integer"},"default_conn_delay":{"exclusiveMinimum":0,"type":"number"},"disable":{"type":"boolean"},"key":{"type":"string"},"key_type":{"default":"var","enum":["var","var_combination"],"type":"string"},"only_use_default_delay":{"default":false,"type":"boolean"}},"required":["conn","burst","default_conn_delay","key"],"type":"object"},"version":0.1} +{"priority":1003,"schema":{"$comment":"this is a mark for our injected plugin schema","properties":{"_meta":{"properties":{"disable":{"type":"boolean"},"error_response":{"oneOf":[{"type":"string"},{"type":"object"}]},"filter":{"description":"filter determines whether the plugin needs to be executed at runtime","type":"array"},"priority":{"description":"priority of plugins by customized order","type":"integer"}},"type":"object"},"burst":{"minimum":0,"type":"integer"},"conn":{"exclusiveMinimum":0,"type":"integer"},"default_conn_delay":{"exclusiveMinimum":0,"type":"number"},"key":{"type":"string"},"key_type":{"default":"var","enum":["var","var_combination"],"type":"string"},"only_use_default_delay":{"default":false,"type":"boolean"}},"required":["conn","burst","default_conn_delay","key"],"type":"object"},"version":0.1} diff --git a/t/admin/proto.t b/t/admin/proto.t index 0e9cc7427fef..e560ffffcd20 100644 --- a/t/admin/proto.t +++ b/t/admin/proto.t @@ -141,7 +141,9 @@ __DATA__ "methods": ["GET"], "plugins": { "grpc-transcode": { - "disable": false, + "_meta": { + "disable": false + }, "method": "SayHi", "proto_id": 2, "service": "proto.Hello" diff --git a/t/admin/routes.t b/t/admin/routes.t index febfdc446fc8..8bede46683fc 100644 --- a/t/admin/routes.t +++ b/t/admin/routes.t @@ -654,7 +654,9 @@ GET /t "time_window": 60, "rejected_code": 503, "key": "remote_addr", - "disable": true + "_meta": { + "disable": true + } } }, "uri": "/index.html" diff --git a/t/control/schema.t b/t/control/schema.t index 5b3c7799720c..ae9c676d7591 100644 --- a/t/control/schema.t +++ b/t/control/schema.t @@ -69,7 +69,11 @@ __DATA__ "schema": { "type":"object", "properties": { - "disable": {"type": "boolean"} + "_meta": { + "properties": { + "disable": {"type": "boolean"} + } + } } }, "metadata_schema": {"type":"object"} @@ -84,7 +88,11 @@ __DATA__ "schema": { "type":"object", "properties": { - "disable": {"type": "boolean"} + "_meta": { + "properties": { + "disable": {"type": "boolean"} + } + } } }, "priority": 1000 diff --git a/t/node/merge-route.t b/t/node/merge-route.t index ab0b45963133..2d1f48f4a1ae 100644 --- a/t/node/merge-route.t +++ b/t/node/merge-route.t @@ -180,7 +180,9 @@ qr/1980/ "time_window": 60, "rejected_code": 503, "key": "remote_addr", - "disable": true + "_meta": { + "disable": true + } } }, "uri": "/server_port", diff --git a/t/plugin/basic-auth.t b/t/plugin/basic-auth.t index 5d626edd07af..ca2a82055b90 100644 --- a/t/plugin/basic-auth.t +++ b/t/plugin/basic-auth.t @@ -340,7 +340,7 @@ GET /t ngx.HTTP_GET, nil, [[ -{"properties":{"disable":{"type":"boolean"}},"title":"work with route or service object","type":"object"} +{"properties":{},"title":"work with route or service object","type":"object"} ]] ) ngx.status = code @@ -384,7 +384,7 @@ GET /t ngx.HTTP_GET, nil, [[ -{"properties":{"disable":{"type":"boolean"}},"title":"work with route or service object","type":"object"} +{"properties":{},"title":"work with route or service object","type":"object"} ]] ) ngx.status = code diff --git a/t/plugin/hmac-auth2.t b/t/plugin/hmac-auth2.t index 4358ef0f8cb8..a845f3de95fd 100644 --- a/t/plugin/hmac-auth2.t +++ b/t/plugin/hmac-auth2.t @@ -400,7 +400,7 @@ x-real-ip: 127.0.0.1 ngx.HTTP_GET, nil, [[ -{"properties":{"disable":{"type":"boolean"}},"title":"work with route or service object","type":"object"} +{"properties":{},"title":"work with route or service object","type":"object"} ]] ) ngx.status = code @@ -436,7 +436,7 @@ x-real-ip: 127.0.0.1 ngx.HTTP_GET, nil, [[ -{"properties":{"disable":{"type":"boolean"}},"title":"work with route or service object","type":"object"} +{"properties":{},"title":"work with route or service object","type":"object"} ]] ) ngx.status = code diff --git a/t/plugin/ip-restriction.t b/t/plugin/ip-restriction.t index d2cbc75c0325..d32c8b2e41a4 100644 --- a/t/plugin/ip-restriction.t +++ b/t/plugin/ip-restriction.t @@ -587,7 +587,9 @@ qr/failed to validate item 1: object matches none of the required/ "blacklist": [ "127.0.0.0/24" ], - "disable": true + "_meta": { + "disable": true + } } } }]] diff --git a/t/plugin/jwt-auth.t b/t/plugin/jwt-auth.t index a6ed72833a81..9e841f820722 100644 --- a/t/plugin/jwt-auth.t +++ b/t/plugin/jwt-auth.t @@ -517,7 +517,7 @@ property "key" is required ngx.HTTP_GET, nil, [[ - {"properties":{"disable":{"type":"boolean"}},"type":"object"} + {"properties":{},"type":"object"} ]] ) ngx.status = code @@ -535,7 +535,7 @@ property "key" is required ngx.HTTP_GET, nil, [[ - {"properties":{"disable":{"type":"boolean"}},"type":"object"} + {"properties":{},"type":"object"} ]] ) ngx.status = code diff --git a/t/plugin/ldap-auth.t b/t/plugin/ldap-auth.t index 4cf5fa92b9fe..31b7a643013e 100644 --- a/t/plugin/ldap-auth.t +++ b/t/plugin/ldap-auth.t @@ -304,7 +304,7 @@ find consumer user01 ngx.HTTP_GET, nil, [[ -{"title":"work with route or service object","required":["base_dn","ldap_uri"],"properties":{"base_dn":{"type":"string"},"ldap_uri":{"type":"string"},"use_tls":{"type":"boolean"},"tls_verify":{"type":"boolean"},"disable":{"type":"boolean"},"uid":{"type":"string"}},"type":"object"} +{"title":"work with route or service object","required":["base_dn","ldap_uri"],"properties":{"base_dn":{"type":"string"},"ldap_uri":{"type":"string"},"use_tls":{"type":"boolean"},"tls_verify":{"type":"boolean"},"uid":{"type":"string"}},"type":"object"} ]] ) ngx.status = code @@ -340,7 +340,7 @@ find consumer user01 ngx.HTTP_GET, nil, [[ -{"title":"work with route or service object","required":["base_dn","ldap_uri"],"properties":{"base_dn":{"type":"string"},"ldap_uri":{"type":"string"},"use_tls":{"type":"boolean"},"tls_verify":{"type":"boolean"},"disable":{"type":"boolean"},"uid":{"type":"string"}},"type":"object"} ]] +{"title":"work with route or service object","required":["base_dn","ldap_uri"],"properties":{"base_dn":{"type":"string"},"ldap_uri":{"type":"string"},"use_tls":{"type":"boolean"},"tls_verify":{"type":"boolean"},"uid":{"type":"string"}},"type":"object"} ]] ) ngx.status = code } diff --git a/t/plugin/limit-count2.t b/t/plugin/limit-count2.t index e3025be5d564..0dadaf78e990 100644 --- a/t/plugin/limit-count2.t +++ b/t/plugin/limit-count2.t @@ -781,7 +781,9 @@ limit key: afafafhao2:remote_addr "time_window": 60, "rejected_code": 503, "group": "abcd", - "disable": false + "_meta": { + "disable": false + } } }, "upstream": { diff --git a/t/plugin/opa2.t b/t/plugin/opa2.t index 75d9632ba26a..d14269ce696c 100644 --- a/t/plugin/opa2.t +++ b/t/plugin/opa2.t @@ -56,7 +56,9 @@ __DATA__ "username": "test", "plugins": { "key-auth": { - "disable": false, + "_meta": { + "disable": false + }, "key": "test-key" } } @@ -68,7 +70,9 @@ __DATA__ "name": "s1", "plugins": { "key-auth": { - "disable": false + "_meta": { + "disable": false + } } } }]], diff --git a/t/plugin/ua-restriction.t b/t/plugin/ua-restriction.t index 82e665894655..0e8a9544bd34 100644 --- a/t/plugin/ua-restriction.t +++ b/t/plugin/ua-restriction.t @@ -725,7 +725,9 @@ hello world "denylist": [ "foo" ], - "disable": true + "_meta": { + "disable": true + } } } }]]