fix: plugins repeats the rewrite phase when consumer configs with a plugin

apisix/plugin.lua

@@ -494,22 +494,35 @@ function _M.filter(ctx, conf, plugins, route_conf, phase)
             goto continue
         end
 
-        if not check_disable(plugin_conf) then
-            if plugin_obj.run_policy == "prefer_route" and route_plugin_conf ~= nil then
-                local plugin_conf_in_route = route_plugin_conf[name]
-                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
+        if check_disable(plugin_conf) then
+            goto continue
+        end
 
-            if plugin_conf._meta and plugin_conf._meta.priority then
-                custom_sort = true
+        if plugin_obj.run_policy == "prefer_route" and route_plugin_conf ~= nil then
+            local plugin_conf_in_route = route_plugin_conf[name]
+            local disable_in_route = check_disable(plugin_conf_in_route)
+            if plugin_conf_in_route and not disable_in_route then
+                goto continue
             end
-            core.table.insert(plugins, plugin_obj)
-            core.table.insert(plugins, plugin_conf)
         end
 
+        -- in the rewrite phase, the plugin executes in the following order:
+        -- 1. execute the rewrite phase of the plugins on route(including the auth plugins)
+        -- 2. merge plugins from consumer and route
+        -- 3. execute the rewrite phase of the plugins on consumer(phase: rewrite_in_consumer)
+        -- in this case, we need to skip the plugins that was already executed(step 1)
+        if phase == "rewrite_in_consumer"
+                and (not plugin_conf._from_consumer or plugin_obj.type == "auth") then
+            plugin_conf._skip_rewrite_in_consumer = true
+        end
+
+        if plugin_conf._meta and plugin_conf._meta.priority then
+            custom_sort = true
+        end
+
+        core.table.insert(plugins, plugin_obj)
+        core.table.insert(plugins, plugin_conf)
+
         ::continue::
     end
 
@@ -523,15 +536,6 @@ function _M.filter(ctx, conf, plugins, route_conf, phase)
             local plugin_obj = plugins[i]
             local plugin_conf = plugins[i + 1]
 
-            -- in the rewrite phase, the plugin executes in the following order:
-            -- 1. execute the rewrite phase of the plugins on route(including the auth plugins)
-            -- 2. merge plugins from consumer and route
-            -- 3. execute the rewrite phase of the plugins on consumer(phase: rewrite_in_consumer)
-            -- in this case, we need to skip the plugins that was already executed(step 1)
-            if phase == "rewrite_in_consumer" and not plugin_conf._from_consumer then
-                plugin_conf._skip_rewrite_in_consumer = true
-            end
-
             tmp_plugin_objs[plugin_conf] = plugin_obj
             core.table.insert(tmp_plugin_confs, plugin_conf)
 
@@ -1179,20 +1183,13 @@ function _M.run_plugin(phase, plugins, api_ctx)
         and phase ~= "delayed_body_filter"
     then
         for i = 1, #plugins, 2 do
-            local phase_func
-            if phase == "rewrite_in_consumer" then
-                if plugins[i].type == "auth" then
-                    plugins[i + 1]._skip_rewrite_in_consumer = true
-                end
-                phase_func = plugins[i]["rewrite"]
-            else
-                phase_func = plugins[i][phase]
-            end
 
             if phase == "rewrite_in_consumer" and plugins[i + 1]._skip_rewrite_in_consumer then
                 goto CONTINUE
             end
 
+            local phase_func = phase == "rewrite_in_consumer" and plugins[i]["rewrite"]
+                               or plugins[i][phase]
             if phase_func then
                 local conf = plugins[i + 1]
                 if not meta_filter(api_ctx, plugins[i]["name"], conf)then

t/node/plugin1.t

@@ -0,0 +1,104 @@
+#
+# 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 strict;
+use warnings FATAL => 'all';
+use t::APISIX 'no_plan';
+
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /apisix/admin/routes");
+    }
+
+    if (!$block->no_error_log && !$block->error_log) {
+        $block->set_value("no_error_log", "[error]\n[alert]");
+    }
+});
+run_tests;
+__DATA__
+
+=== TEST 1: set up configuration
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumers/jack',
+                 ngx.HTTP_PUT,
+                [[{
+                    "username": "jack",
+                    "plugins": {
+                        "key-auth": {
+                          "key": "auth-jack"
+                        }
+                    }
+                }]]
+                )
+            if code >= 300 then
+                ngx.say(body)
+                return
+            end
+            local code, body = t('/apisix/admin/routes/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                        "plugins": {
+                            "key-auth": {},
+                            "proxy-rewrite": {
+                               "headers": {
+                                   "add": {
+                                      "xtest": "123"
+                                    }
+                               }
+                            },
+                            "serverless-post-function": {
+                              "functions": [
+                                "return function(conf, ctx) \n ngx.say(ngx.req.get_headers().xtest); \n end"
+                                ]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                }]]
+                )
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- timeout: 15
+--- response_body
+passed
+
+
+
+=== TEST 2: the proxy-rewrite runs at 'rewrite' phase and should get executed only once, hence the response body is expected '123' not '123123'
+--- request
+GET /hello
+--- more_headers
+apikey: auth-jack
+--- timeout: 15
+--- response_body
+123