bug: cannot disable plugins in apisix/2.15.1

[tokers]

Current Behavior

When using Apache APISIX 2.15.1, users cannot disable a plugin when setting the disable field to true. For instance, a CORS plugin config like the below will still be effective:

{
  "allow_credential": false,
  "allow_headers": "*",
  "allow_origins": "*",
  "expose_headers": "*",
  "allow_methods": "GET",
  "disable": true,
  "max_age": 5
}

This is due to a broken change introduced in APISIX 2.15.1 (see #8162), which causes the original disable field useless, and if you want to disable a plugin, you have to edit a new field _meta.disable.

This is a broken change, and should not be backported to the LTS release directly. The backward compatibility should be kept.

Expected Behavior

The disable field should be in effective in APISIX 2.15.1.

Error Logs

No error logs.

Steps to Reproduce

Create a route with a CORS plugin

curl http://127.0.0.1:9080/apisix/admin/routes -d '{
	"uri": "/get",
	"host": "test.httpbin.org",
	"upstream": {
		"type": "roundrobin",
		"nodes": [
			{"host": "httpbin.org", "port": 80, "weight": 100}
		]
	},
	"plugins": {
		"cors": {
  			"allow_credential": false,
			"allow_headers": "*",
			"allow_origins": "*",
			"expose_headers": "*",
			"allow_methods": "GET",
			"disable": true,
			"max_age": 5
		}
	}
}`

Send a request

curl http://127.0.0.1:9080/get -H 'Host: test.httpbin.org' -H 'Origin: a.com' -v

Response headers will be:

< HTTP/1.1 200 OK
< Content-Type: application/json
< Content-Length: 344
< Connection: keep-alive
< Date: Wed, 04 Jan 2023 03:53:26 GMT
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Credentials: true
< Server: APISIX/2.15.1
< Access-Control-Allow-Methods: GET
< Access-Control-Max-Age: 5
< Access-Control-Expose-Headers: *
< Access-Control-Allow-Headers: *

The CORS plugin was diabled, but still, CORS related headers were shown.

Environment

• APISIX version (run apisix version): `2.15.1
• Operating system (run uname -a):
• OpenResty / Nginx version (run openresty -V or nginx -V):
• etcd version, if relevant (run curl http://127.0.0.1:9090/v1/server_info):
• APISIX Dashboard version, if relevant:
• Plugin runner version, for issues related to plugin runners:
• LuaRocks version, for installation issues (run luarocks --version):

[spacewander]

I check the commit log and it turns out that this break change is introduced when I solved the merge conflict.
Considering apisix/2.15.1 already contains this break change, we can't simply revert it, otherwise people will suffer a new break change in 2.15.2.

So my idea is to add the old disable feature back in release/2.15 branch, so both ways are supported in 2.15.2, which will be the upcoming release.

[tokers]

Agree with this idea. 👍🏻

[lingsamuel]

I am going to close this since the PR has been merged