fix(api) parent arg was missing in plugin endpoint handlers (Kong#5174)

Customization of Admin API endpoints for DAO objects return a 4th argument `parent` for the handler function, which contains the original implementation of the endpoint. This argument was missing for endpoints declared by plugins. 

This fix makes it possible for a plugin's endpoint handler in `api.lua` to reuse the original function via `parent` when overriding Admin API functions like we do in some core DAO routes.

kong/api/init.lua

@@ -31,9 +31,54 @@ end
 
 
 do
+  -- This function takes the auto-generated routes and then customizes them
+  -- based on custom_endpoints. It will add one argument to actual function
+  -- call `parent` that the customized function can use to call the original
+  -- auto-generated function.
+  --
+  -- E.g. the `/routes/:routes` API gets autogenerated from `routes` DAO.
+  -- Now if your plugin adds `api.lua` that also defines the same endpoint:
+  -- `/routes/:routes`, it means that the plugin one overrides the original
+  -- function. Original is kept and passed to the customized function as an
+  -- function argument (of course usually plugins want to only customize
+  -- the autogenerated endpoints the plugin's own DAOs introduced).
+  local function customize_routes(routes, custom_endpoints, schema)
+    for route_pattern, verbs in pairs(custom_endpoints) do
+      if routes[route_pattern] ~= nil and type(verbs) == "table" then
+        for verb, handler in pairs(verbs) do
+          local parent = routes[route_pattern]["methods"][verb]
+          if parent ~= nil and type(handler) == "function" then
+            routes[route_pattern]["methods"][verb] = function(self, db, helpers)
+              return handler(self, db, helpers, function(post_process)
+                return parent(self, db, helpers, post_process)
+              end)
+            end
+
+          else
+            routes[route_pattern]["methods"][verb] = handler
+          end
+        end
+
+      else
+        routes[route_pattern] = {
+          schema  = verbs.schema or schema,
+          methods = verbs,
+        }
+      end
+    end
+  end
+
+  local function is_new_db_routes(routes)
+    for _, verbs in pairs(routes) do
+      if type(verbs) == "table" then -- ignore "before" functions
+        return verbs.schema
+      end
+    end
+  end
+
   local routes = {}
 
-  -- Auto Generated Routes
+  -- DAO Routes
   for _, dao in pairs(singletons.db.daos) do
     if dao.schema.generate_admin_api ~= false and not dao.schema.legacy then
       routes = Endpoints.new(dao.schema, routes)
@@ -43,66 +88,32 @@ do
   -- Custom Routes
   for _, dao in pairs(singletons.db.daos) do
     local schema = dao.schema
-
     local ok, custom_endpoints = utils.load_module_if_exists("kong.api.routes." .. schema.name)
     if ok then
-      for route_pattern, verbs in pairs(custom_endpoints) do
-        if routes[route_pattern] ~= nil and type(verbs) == "table" then
-          for verb, handler in pairs(verbs) do
-            local parent = routes[route_pattern]["methods"][verb]
-            if parent ~= nil and type(handler) == "function" then
-              routes[route_pattern]["methods"][verb] = function(self, db, helpers)
-                return handler(self, db, helpers, function(post_process)
-                  return parent(self, db, helpers, post_process)
-                end)
-              end
-
-            else
-              routes[route_pattern]["methods"][verb] = handler
-            end
-          end
-
-        else
-          routes[route_pattern] = {
-            schema  = dao.schema,
-            methods = verbs,
-          }
-        end
-      end
+      customize_routes(routes, custom_endpoints, schema)
     end
   end
 
-  api_helpers.attach_new_db_routes(app, routes)
-end
+  -- Plugin Routes
+  if singletons.configuration and singletons.configuration.loaded_plugins then
+    for k in pairs(singletons.configuration.loaded_plugins) do
+      local loaded, custom_endpoints = utils.load_module_if_exists("kong.plugins." .. k .. ".api")
+      if loaded then
+        ngx.log(ngx.DEBUG, "Loading API endpoints for plugin: ", k)
+        if is_new_db_routes(custom_endpoints) then
+          customize_routes(routes, custom_endpoints, custom_endpoints.schema)
 
+        else
+          api_helpers.attach_routes(app, custom_endpoints)
+        end
 
-local function is_new_db_routes(mod)
-  for _, verbs in pairs(mod) do
-    if type(verbs) == "table" then -- ignore "before" functions
-      return verbs.schema
-    end
-  end
-end
-
-
--- Loading plugins routes
-if singletons.configuration and singletons.configuration.loaded_plugins then
-  for k in pairs(singletons.configuration.loaded_plugins) do
-    local loaded, mod = utils.load_module_if_exists("kong.plugins." .. k .. ".api")
-
-    if loaded then
-      ngx.log(ngx.DEBUG, "Loading API endpoints for plugin: ", k)
-      if is_new_db_routes(mod) then
-        api_helpers.attach_new_db_routes(app, mod)
       else
-        api_helpers.attach_routes(app, mod)
+        ngx.log(ngx.DEBUG, "No API endpoints loaded for plugin: ", k)
       end
-
-    else
-      ngx.log(ngx.DEBUG, "No API endpoints loaded for plugin: ", k)
     end
   end
-end
 
+  api_helpers.attach_new_db_routes(app, routes)
+end
 
 return app

spec/02-integration/04-admin_api/09-routes_routes_spec.lua

@@ -1816,4 +1816,97 @@ for _, strategy in helpers.each_strategy() do
       end)
     end)
   end)
+
+  describe("Admin API Override #" .. strategy, function()
+    local bp
+    local db
+    local client
+
+    lazy_setup(function()
+      bp, db = helpers.get_db_utils(strategy, {
+        "routes",
+        "services",
+      }, {
+        "api-override",
+      })
+
+
+      assert(helpers.start_kong({
+        database      = strategy,
+        plugins       = "bundled,api-override",
+        nginx_conf    = "spec/fixtures/custom_nginx.template",
+      }))
+    end)
+
+    lazy_teardown(function()
+      helpers.stop_kong(nil, true)
+    end)
+
+    before_each(function()
+      client = assert(helpers.admin_client())
+    end)
+
+    after_each(function()
+      if client then
+        client:close()
+      end
+    end)
+    describe("/routes", function()
+      describe("POST", function()
+        it_content_types("creates a route", function(content_type)
+          return function()
+            if content_type == "multipart/form-data" then
+              -- the client doesn't play well with this
+              return
+            end
+
+            local res = client:post("/routes", {
+              body = {
+                protocols = { "http" },
+                hosts     = { "my.route.com" },
+                headers   = { location = { "my-location" } },
+                service   = bp.services:insert(),
+              },
+              headers = { ["Content-Type"] = content_type }
+            })
+            local body = assert.res_status(201, res)
+            local json = cjson.decode(body)
+            assert.same({ "my.route.com" }, json.hosts)
+            assert.same({ location = { "my-location" } }, json.headers)
+            assert.is_number(json.created_at)
+            assert.is_number(json.regex_priority)
+            assert.is_string(json.id)
+            assert.equals(cjson.null, json.name)
+            assert.equals(cjson.null, json.paths)
+            assert.False(json.preserve_host)
+            assert.True(json.strip_path)
+
+            assert.equal("ok", res.headers["Kong-Api-Override"])
+          end
+        end)
+      end)
+      describe("GET", function()
+        describe("with data", function()
+          lazy_setup(function()
+            db:truncate("routes")
+            for i = 1, 10 do
+              bp.routes:insert({ paths = { "/route-" .. i } })
+            end
+          end)
+
+          it("retrieves the first page", function()
+            local res = assert(client:send {
+              method = "GET",
+              path   = "/routes"
+            })
+            local body = assert.res_status(200, res)
+            local json = cjson.decode(body)
+            assert.equal(10, #json.data)
+
+            assert.equal("ok", res.headers["Kong-Api-Override"])
+          end)
+        end)
+      end)
+    end)
+  end)
 end

spec/fixtures/custom_plugins/kong/plugins/api-override/api.lua

@@ -0,0 +1,16 @@
+local kong = kong
+
+
+return {
+  ["/routes"] = {
+    schema = kong.db.routes.schema,
+    GET = function(_, _, _, parent)
+      kong.response.set_header("Kong-Api-Override", "ok")
+      return parent()
+    end,
+    POST = function(_, _, _, parent)
+      kong.response.set_header("Kong-Api-Override", "ok")
+      return parent()
+    end,
+  },
+}

spec/fixtures/custom_plugins/kong/plugins/api-override/handler.lua

@@ -0,0 +1 @@
+return {}

spec/fixtures/custom_plugins/kong/plugins/api-override/schema.lua

@@ -0,0 +1,27 @@
+local typedefs = require "kong.db.schema.typedefs"
+
+
+return {
+  name = "api-override",
+  fields = {
+    {
+      protocols = typedefs.protocols {
+        default = {
+          "http",
+          "https",
+          "tcp",
+          "tls",
+          "grpc",
+          "grpcs"
+        },
+      },
+    },
+    {
+      config = {
+        type = "record",
+        fields = {
+        },
+      },
+    },
+  },
+}