You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/01/13 14:34:08 UTC
[GitHub] [apisix] zaunist opened a new pull request #6092: feat(plugin config): add dependency for delete plugin config
zaunist opened a new pull request #6092:
URL: https://github.com/apache/apisix/pull/6092
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
fix https://github.com/apache/apisix/issues/5917
When a plugin config is referencing by any route, it should not be deleted.
example:
create plugin config:
```
curl http://127.0.0.1:9180/apisix/admin/plugin_configs/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -i -d '
{
"desc": "吾乃插件配置1",
"plugins": {
"limit-count": {
"count": 2,
"time_window": 60,
"rejected_code": 503
}
}
}'
```
bounding route to plugin config:
```
curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -i -d '
{
"uris": ["/index.html"],
"plugin_config_id": 1,
"upstream": {
"type": "roundrobin",
"nodes": {
"39.97.63.215:80": 1
}
}
}'
```
Then, delete plugin config, apisix will return 400 error.
```
▶ curl -X DELETE http://127.0.0.1:9080/apisix/admin/plugin_configs/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1'
{"error_msg":"can not delete this plugin config, route [1] is still using it now"}
```
### Pre-submission checklist:
<!--
Please follow the PR manners:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. If you need to resolve merge conflicts after the PR is reviewed, please merge master but do not rebase
6. Use "request review" to notify the reviewer once you have resolved the review
7. Only reviewer can click "Resolve conversation" to mark the reviewer's review resolved
-->
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #6092:
URL: https://github.com/apache/apisix/pull/6092
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#discussion_r785607790
##########
File path: t/admin/plugin-configs.t
##########
@@ -430,3 +430,139 @@ passed
--- response_body
{"error_msg":"invalid configuration: property \"labels\" validation failed: wrong type: expected object, got string"}
--- error_code: 400
+
+
+
+=== TEST 10: set plugin-configs(id: 1)
+--- 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/plugin_configs/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ }
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+
+ local res = assert(etcd.get('/plugin_configs/1'))
+ local create_time = res.body.node.value.create_time
+ assert(create_time ~= nil, "create_time is nil")
+ local update_time = res.body.node.value.update_time
+ assert(update_time ~= nil, "update_time is nil")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 11: set route(id: 1)
+--- 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,
+ [[{
+ "plugin_config_id": 1,
+ "uri": "/index.html",
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:8080": 1
+ },
+ "type": "roundrobin"
+ },
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
Review comment:
We can remove the duplicate sections which are set in the beginning.
##########
File path: t/admin/plugin-configs.t
##########
@@ -430,3 +430,139 @@ passed
--- response_body
{"error_msg":"invalid configuration: property \"labels\" validation failed: wrong type: expected object, got string"}
--- error_code: 400
+
+
+
+=== TEST 10: set plugin-configs(id: 1)
+--- 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/plugin_configs/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ }
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+
+ local res = assert(etcd.get('/plugin_configs/1'))
+ local create_time = res.body.node.value.create_time
+ assert(create_time ~= nil, "create_time is nil")
+ local update_time = res.body.node.value.update_time
+ assert(update_time ~= nil, "update_time is nil")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 11: set route(id: 1)
+--- 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,
+ [[{
+ "plugin_config_id": 1,
+ "uri": "/index.html",
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:8080": 1
+ },
+ "type": "roundrobin"
+ },
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
Review comment:
Ditto
##########
File path: apisix/admin/plugin_config.lua
##########
@@ -100,13 +102,33 @@ end
function _M.delete(id)
+ if not id then
+ return 400, {error_msg = "missing plugin config id"}
+ end
+
+ local routes, routes_ver = get_routes()
+ core.log.info("routes: ", core.json.delay_encode(routes, true))
Review comment:
As the size of Nginx log is limited, we can't log all the routes. Let's remove it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zaunist commented on a change in pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
zaunist commented on a change in pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#discussion_r785656254
##########
File path: t/admin/plugin-configs.t
##########
@@ -430,3 +430,124 @@ passed
--- response_body
{"error_msg":"invalid configuration: property \"labels\" validation failed: wrong type: expected object, got string"}
--- error_code: 400
+
+
+
+=== TEST 10: set plugin-configs(id: 1)
+--- 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/plugin_configs/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ }
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+
+ local res = assert(etcd.get('/plugin_configs/1'))
+ local create_time = res.body.node.value.create_time
+ assert(create_time ~= nil, "create_time is nil")
+ local update_time = res.body.node.value.update_time
+ assert(update_time ~= nil, "update_time is nil")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 11: set route(id: 1)
+--- 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,
+ [[{
+ "plugin_config_id": 1,
+ "uri": "/index.html",
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:8080": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 12: delete-plugin configs failed(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ ngx.sleep(0.3)
Review comment:
The current step depends on the previous steps, so set a short time for etcd to sync data.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#discussion_r785434259
##########
File path: apisix/admin/plugin_config.lua
##########
@@ -100,13 +102,33 @@ end
function _M.delete(id)
+ if not id then
+ return 400, {error_msg = "missing plugin config id"}
+ end
+
+ local routes, routes_ver = get_routes()
+ core.log.info("routes: ", core.json.delay_encode(routes, true))
Review comment:
Is it necessary to dump all the routes?
##########
File path: t/admin/delete-plugin-configs.t
##########
@@ -0,0 +1,174 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_root_location();
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: set plugin-configs(id: 1)
+--- 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/plugin_configs/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ }
+ }]],
+ [[{
Review comment:
We don't need to check the response in every test.
##########
File path: t/admin/delete-plugin-configs.t
##########
@@ -0,0 +1,174 @@
+#
Review comment:
Could we add it to `t/admin/plugin-configs.t`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zaunist commented on a change in pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
zaunist commented on a change in pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#discussion_r785583833
##########
File path: apisix/admin/plugin_config.lua
##########
@@ -100,13 +102,33 @@ end
function _M.delete(id)
+ if not id then
+ return 400, {error_msg = "missing plugin config id"}
+ end
+
+ local routes, routes_ver = get_routes()
+ core.log.info("routes: ", core.json.delay_encode(routes, true))
Review comment:
I did it with reference to the processing of upstream. At the same time, I also considered whether to logging all route information. If it is not log here, does it also need to be modified in upstream?
https://github.com/apache/apisix/blob/c38e94f20144b0ba01018970fa954c935a325ac1/apisix/admin/upstreams.lua#L130
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#issuecomment-1014321760
Usually, it is because of a malformed config section. Could you check the `t/servroot/conf/nginx.conf`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zaunist commented on pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
zaunist commented on pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#issuecomment-1014249991
> I found the CI is broken with msg:
>
> ```
> nginx: [emerg] "listen" directive is not allowed here in /home/runner/work/apisix/apisix/t/servroot/conf/nginx.conf:202
> ```
>
> https://github.com/apache/apisix/runs/4836910403?check_suite_focus=true
Thanks for your careful, but I don't know how to slove this problem. Any one can help me to do this ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#discussion_r785440244
##########
File path: t/admin/delete-plugin-configs.t
##########
@@ -0,0 +1,174 @@
+#
Review comment:
We should not add more new `delete-*` files. For example, the new case in https://github.com/apache/apisix/pull/4575 is added in `proto3.t`.
I also submit a PR to remove the stale cases: https://github.com/apache/apisix/pull/6121
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zaunist commented on pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
zaunist commented on pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#issuecomment-1013852558
cc @tzssangglass
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] bisakhmondal commented on a change in pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
bisakhmondal commented on a change in pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#discussion_r785635111
##########
File path: t/admin/plugin-configs.t
##########
@@ -430,3 +430,124 @@ passed
--- response_body
{"error_msg":"invalid configuration: property \"labels\" validation failed: wrong type: expected object, got string"}
--- error_code: 400
+
+
+
+=== TEST 10: set plugin-configs(id: 1)
+--- 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/plugin_configs/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ }
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+
+ local res = assert(etcd.get('/plugin_configs/1'))
+ local create_time = res.body.node.value.create_time
+ assert(create_time ~= nil, "create_time is nil")
+ local update_time = res.body.node.value.update_time
+ assert(update_time ~= nil, "update_time is nil")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 11: set route(id: 1)
+--- 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,
+ [[{
+ "plugin_config_id": 1,
+ "uri": "/index.html",
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:8080": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 12: delete-plugin configs failed(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ ngx.sleep(0.3)
+ local t = require("lib.test_admin").test
+ local code, message = t('/apisix/admin/plugin_configs/1',
+ ngx.HTTP_DELETE,
+ nil,
+ [[{"action": "delete"}]]
+ )
+ ngx.print("[delete] code: ", code, " message: ", message)
+ }
+ }
+--- response_body
+{"error_msg":"can not delete this plugin config, route [1] is still using it now"}
+--- error_code: 400
+
+
+
+=== TEST 13: delete route(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ ngx.sleep(0.3)
+ local t = require("lib.test_admin").test
+ local code, message = t('/apisix/admin/routes/1',
+ ngx.HTTP_DELETE,
+ nil,
+ [[{"action": "delete"}]]
+ )
+ ngx.say("[delete] code: ", code, " message: ", message)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 14: delete plugin-configs(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ ngx.sleep(0.3)
Review comment:
ditto
##########
File path: t/admin/plugin-configs.t
##########
@@ -430,3 +430,124 @@ passed
--- response_body
{"error_msg":"invalid configuration: property \"labels\" validation failed: wrong type: expected object, got string"}
--- error_code: 400
+
+
+
+=== TEST 10: set plugin-configs(id: 1)
+--- 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/plugin_configs/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ }
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+
+ local res = assert(etcd.get('/plugin_configs/1'))
+ local create_time = res.body.node.value.create_time
+ assert(create_time ~= nil, "create_time is nil")
+ local update_time = res.body.node.value.update_time
+ assert(update_time ~= nil, "update_time is nil")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 11: set route(id: 1)
+--- 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,
+ [[{
+ "plugin_config_id": 1,
+ "uri": "/index.html",
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:8080": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 12: delete-plugin configs failed(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ ngx.sleep(0.3)
+ local t = require("lib.test_admin").test
+ local code, message = t('/apisix/admin/plugin_configs/1',
+ ngx.HTTP_DELETE,
+ nil,
+ [[{"action": "delete"}]]
+ )
+ ngx.print("[delete] code: ", code, " message: ", message)
+ }
+ }
+--- response_body
+{"error_msg":"can not delete this plugin config, route [1] is still using it now"}
+--- error_code: 400
+
+
+
+=== TEST 13: delete route(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ ngx.sleep(0.3)
Review comment:
ditto
##########
File path: t/admin/plugin-configs.t
##########
@@ -430,3 +430,124 @@ passed
--- response_body
{"error_msg":"invalid configuration: property \"labels\" validation failed: wrong type: expected object, got string"}
--- error_code: 400
+
+
+
+=== TEST 10: set plugin-configs(id: 1)
+--- 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/plugin_configs/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ }
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+
+ local res = assert(etcd.get('/plugin_configs/1'))
+ local create_time = res.body.node.value.create_time
+ assert(create_time ~= nil, "create_time is nil")
+ local update_time = res.body.node.value.update_time
+ assert(update_time ~= nil, "update_time is nil")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 11: set route(id: 1)
+--- 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,
+ [[{
+ "plugin_config_id": 1,
+ "uri": "/index.html",
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:8080": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 12: delete-plugin configs failed(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ ngx.sleep(0.3)
Review comment:
Hi, is there any reason for which we need to add this 0.3-sec sleep?
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zaunist commented on a change in pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
zaunist commented on a change in pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#discussion_r785583858
##########
File path: t/admin/delete-plugin-configs.t
##########
@@ -0,0 +1,174 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_root_location();
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: set plugin-configs(id: 1)
+--- 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/plugin_configs/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ }
+ }]],
+ [[{
Review comment:
Got it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zaunist commented on a change in pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
zaunist commented on a change in pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#discussion_r785581928
##########
File path: t/admin/delete-plugin-configs.t
##########
@@ -0,0 +1,174 @@
+#
Review comment:
Got it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zaunist commented on pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
zaunist commented on pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#issuecomment-1015090613
> Usually, it is because of a malformed config section. Could you check the `t/servroot/conf/nginx.conf`?
Got it, and CI passed now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #6092: feat(plugin config): add dependency for delete plugin config
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #6092:
URL: https://github.com/apache/apisix/pull/6092#issuecomment-1014215106
I found the CI is broken with msg:
```
nginx: [emerg] "listen" directive is not allowed here in /home/runner/work/apisix/apisix/t/servroot/conf/nginx.conf:202
```
https://github.com/apache/apisix/runs/4836910403?check_suite_focus=true
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org