Revert "refactor(shorthand_fields): remove translate_backwards in favor of replaced_with (#13604)"

This reverts commit 11405e5.

Author: nowNick

changelog/unreleased/kong/revert-translate-backwards.yml

@@ -0,0 +1,3 @@
+message: Reverted removal of `translate_backwards` from plugins' metaschema in order to maintain backward compatibility.
+type: bugfix
+scope: Core

kong/db/schema/init.lua

@@ -1746,18 +1746,35 @@ local function validate_deprecation_exclusiveness(data, shorthand_value, shortha
   if shorthand_value == nil or
       shorthand_value == ngx.null or
       shorthand_definition.deprecation == nil or
-      shorthand_definition.deprecation.replaced_with == nil then
+      (shorthand_definition.deprecation.replaced_with == nil and shorthand_definition.translate_backwards == nil) then
     return true
   end
 
-  for _, replaced_with_element in ipairs(shorthand_definition.deprecation.replaced_with) do
-    local new_field_value = replaced_with_element.reverse_mapping_function and replaced_with_element.reverse_mapping_function(data)
-                                                                            or table_path(data, replaced_with_element.path)
+  if shorthand_definition.deprecation.replaced_with then
+    for _, replaced_with_element in ipairs(shorthand_definition.deprecation.replaced_with) do
+      local new_field_value = replaced_with_element.reverse_mapping_function and replaced_with_element.reverse_mapping_function(data)
+                                                                              or table_path(data, replaced_with_element.path)
+
+      if new_field_value and
+        new_field_value ~= ngx.null and
+        not deepcompare(new_field_value, shorthand_value) then
+        local new_field_name = join_string(".", replaced_with_element.path)
+
+        return nil, string.format(
+          "both deprecated and new field are used but their values mismatch: %s = %s vs %s = %s",
+          shorthand_name, tostring(shorthand_value),
+          new_field_name, tostring(new_field_value)
+        )
+      end
+    end
+  elseif shorthand_definition.translate_backwards then
+    local new_field_value = shorthand_definition.translate_backwards_with and shorthand_definition.translate_backwards_with(data)
+                                                                          or table_path(data, shorthand_definition.translate_backwards)
 
     if new_field_value and
-      new_field_value ~= ngx.null and
-      not deepcompare(new_field_value, shorthand_value) then
-      local new_field_name = join_string(".", replaced_with_element.path)
+        new_field_value ~= ngx.null and
+        not deepcompare(new_field_value, shorthand_value) then
+        local new_field_name = join_string(".", shorthand_definition.translate_backwards)
 
       return nil, string.format(
         "both deprecated and new field are used but their values mismatch: %s = %s vs %s = %s",
@@ -1847,10 +1864,16 @@ function Schema:process_auto_fields(data, context, nulls, opts)
                                                     sdata.deprecation.replaced_with[1]
         if replaced_with then
           if replaced_with.reverse_mapping_function then
-          data[sname] = replaced_with.reverse_mapping_function(data)
+            data[sname] = replaced_with.reverse_mapping_function(data)
           else
             data[sname] = table_path(data, replaced_with.path)
           end
+
+        -- Falling back to processing `translate_backwards` for backwards compatibility
+        -- this might be the case when someone is using `rate-limiting`, `acme`, `response-ratelimiting` plugin
+        -- from version 3.6.x or 3.7.x
+        elseif sdata.translate_backwards then
+          data[sname] = table_path(data, sdata.translate_backwards)
         end
       end
     end
@@ -2000,21 +2023,25 @@ function Schema:process_auto_fields(data, context, nulls, opts)
     elseif not ((key == "ttl"   and self.ttl) or
                 (key == "ws_id" and show_ws)) then
 
-      local should_be_in_ouput = false
+      local should_be_in_output = false
 
       if self.shorthand_fields then
         for _, shorthand_field in ipairs(self.shorthand_fields) do
           if shorthand_field[key] then
             local replaced_with = shorthand_field[key].deprecation and shorthand_field[key].deprecation.replaced_with and
                                                                         #shorthand_field[key].deprecation.replaced_with[1]
-            if replaced_with then
-              should_be_in_ouput = is_select
+
+            -- Either using replaced_with or falling back to processing `translate_backwards` for backwards compatibility
+            -- this might be the case when someone is using `rate-limiting`, `acme`, `response-ratelimiting` plugin
+            -- from version 3.6.x or 3.7.x
+            if replaced_with or shorthand_field[key].translate_backwards then
+              should_be_in_output = is_select
             end
           end
         end
       end
 
-      if not should_be_in_ouput then
+      if not should_be_in_output then
         data[key] = nil
       end
     end

kong/db/schema/metaschema.lua

@@ -716,6 +716,7 @@ local function make_shorthand_field_schema()
   shorthand_field_schema[1] = { type = { type = "string", one_of = shorthand_field_types, required = true }, }
 
   insert(shorthand_field_schema, { func = { type = "function", required = true } })
+  insert(shorthand_field_schema, { translate_backwards = { type = "array", elements = { type = "string" }, required = false } })
   return shorthand_field_schema
 end
 

spec/02-integration/03-db/23-shorthand_fields_translate_backwards_spec.lua

@@ -0,0 +1,274 @@
+local helpers = require "spec.helpers"
+local cjson = require "cjson"
+local uuid = require("kong.tools.uuid").uuid
+
+
+describe("an old plugin: translate_backwards", function()
+  local bp, db, route, admin_client
+  local plugin_id = uuid()
+
+  lazy_setup(function()
+    helpers.test_conf.lua_package_path = helpers.test_conf.lua_package_path .. ";./spec-ee/fixtures/custom_plugins/?.lua"
+    bp, db = helpers.get_db_utils(nil, {
+      "plugins",
+    }, { 'translate-backwards-older-plugin' })
+
+    route = assert(bp.routes:insert {
+      hosts = { "redis.test" },
+    })
+
+    assert(helpers.start_kong({
+      plugins = "bundled,translate-backwards-older-plugin",
+      nginx_conf = "spec/fixtures/custom_nginx.template",
+      lua_package_path  = "?./spec-ee/fixtures/custom_plugins/?.lua",
+    }))
+
+    admin_client = assert(helpers.admin_client())
+  end)
+
+  lazy_teardown(function()
+    if admin_client then
+      admin_client:close()
+    end
+
+    helpers.stop_kong()
+  end)
+
+  describe("when creating custom plugin", function()
+    after_each(function()
+      db:truncate("plugins")
+    end)
+
+    describe("when using the new field", function()
+      it("creates the custom plugin and fills in old field in response", function()
+        -- POST
+        local res = assert(admin_client:send {
+          method = "POST",
+          route = {
+            id = route.id
+          },
+          path = "/plugins",
+          headers = { ["Content-Type"] = "application/json" },
+          body = {
+            id = plugin_id,
+            name = "translate-backwards-older-plugin",
+            config = {
+              new_field = "ABC"
+            },
+          },
+        })
+
+        local json = cjson.decode(assert.res_status(201, res))
+        assert.same(json.config.new_field, "ABC")
+        assert.same(json.config.old_field, "ABC")
+
+        -- PATCH
+        res = assert(admin_client:send {
+          method = "PATCH",
+          path = "/plugins/" .. plugin_id,
+          headers = { ["Content-Type"] = "application/json" },
+          body = {
+            name = "translate-backwards-older-plugin",
+            config = {
+              new_field = "XYZ"
+            },
+          },
+        })
+
+        json = cjson.decode(assert.res_status(200, res))
+        assert.same(json.config.new_field, "XYZ")
+        assert.same(json.config.old_field, "XYZ")
+
+        -- GET
+        res = assert(admin_client:send {
+          method = "GET",
+          path = "/plugins/" .. plugin_id
+        })
+
+        json = cjson.decode(assert.res_status(200, res))
+        assert.same(json.config.new_field, "XYZ")
+        assert.same(json.config.old_field, "XYZ")
+      end)
+    end)
+
+    describe("when using the old field", function()
+      it("creates the custom plugin and fills in old field in response", function()
+        -- POST
+        local res = assert(admin_client:send {
+          method = "POST",
+          route = {
+            id = route.id
+          },
+          path = "/plugins",
+          headers = { ["Content-Type"] = "application/json" },
+          body = {
+            id = plugin_id,
+            name = "translate-backwards-older-plugin",
+            config = {
+              old_field = "ABC"
+            },
+          },
+        })
+
+        local json = cjson.decode(assert.res_status(201, res))
+        assert.same(json.config.new_field, "ABC")
+        assert.same(json.config.old_field, "ABC")
+
+        -- PATCH
+        res = assert(admin_client:send {
+          method = "PATCH",
+          path = "/plugins/" .. plugin_id,
+          headers = { ["Content-Type"] = "application/json" },
+          body = {
+            name = "translate-backwards-older-plugin",
+            config = {
+              old_field = "XYZ"
+            },
+          },
+        })
+
+        json = cjson.decode(assert.res_status(200, res))
+        assert.same(json.config.new_field, "XYZ")
+        assert.same(json.config.old_field, "XYZ")
+
+        -- GET
+        res = assert(admin_client:send {
+          method = "GET",
+          path = "/plugins/" .. plugin_id
+        })
+
+        json = cjson.decode(assert.res_status(200, res))
+        assert.same(json.config.new_field, "XYZ")
+        assert.same(json.config.old_field, "XYZ")
+      end)
+    end)
+
+    describe("when using the both new and old fields", function()
+      describe("when their values match", function()
+        it("creates the custom plugin and fills in old field in response", function()
+          -- POST
+          local res = assert(admin_client:send {
+            method = "POST",
+            route = {
+              id = route.id
+            },
+            path = "/plugins",
+            headers = { ["Content-Type"] = "application/json" },
+            body = {
+              id = plugin_id,
+              name = "translate-backwards-older-plugin",
+              config = {
+                new_field = "ABC",
+                old_field = "ABC"
+              },
+            },
+          })
+
+          local json = cjson.decode(assert.res_status(201, res))
+          assert.same(json.config.new_field, "ABC")
+          assert.same(json.config.old_field, "ABC")
+
+          -- PATCH
+          res = assert(admin_client:send {
+            method = "PATCH",
+            path = "/plugins/" .. plugin_id,
+            headers = { ["Content-Type"] = "application/json" },
+            body = {
+              name = "translate-backwards-older-plugin",
+              config = {
+                new_field = "XYZ",
+                old_field = "XYZ"
+              },
+            },
+          })
+
+          json = cjson.decode(assert.res_status(200, res))
+          assert.same(json.config.new_field, "XYZ")
+          assert.same(json.config.old_field, "XYZ")
+
+          -- GET
+          res = assert(admin_client:send {
+            method = "GET",
+            path = "/plugins/" .. plugin_id
+          })
+
+          json = cjson.decode(assert.res_status(200, res))
+          assert.same(json.config.new_field, "XYZ")
+          assert.same(json.config.old_field, "XYZ")
+        end)
+      end)
+
+      describe("when their values mismatch", function()
+        it("rejects such plugin", function()
+          -- POST --- with mismatched values
+          local res = assert(admin_client:send {
+            method = "POST",
+            route = {
+              id = route.id
+            },
+            path = "/plugins",
+            headers = { ["Content-Type"] = "application/json" },
+            body = {
+              id = plugin_id,
+              name = "translate-backwards-older-plugin",
+              config = {
+                new_field = "ABC",
+                old_field = "XYZ"
+              },
+            },
+          })
+
+          assert.res_status(400, res)
+
+          -- POST --- with correct values so that we can send PATCH below
+          res = assert(admin_client:send {
+            method = "POST",
+            route = {
+              id = route.id
+            },
+            path = "/plugins",
+            headers = { ["Content-Type"] = "application/json" },
+            body = {
+              id = plugin_id,
+              name = "translate-backwards-older-plugin",
+              config = {
+                new_field = "ABC",
+                old_field = "ABC"
+              },
+            },
+          })
+
+          local json = cjson.decode(assert.res_status(201, res))
+          assert.same(json.config.new_field, "ABC")
+          assert.same(json.config.old_field, "ABC")
+
+          -- PATCH
+          res = assert(admin_client:send {
+            method = "PATCH",
+            path = "/plugins/" .. plugin_id,
+            headers = { ["Content-Type"] = "application/json" },
+            body = {
+              name = "translate-backwards-older-plugin",
+              config = {
+                new_field = "EFG",
+                old_field = "XYZ"
+              },
+            },
+          })
+
+          assert.res_status(400, res)
+
+          -- GET
+          res = assert(admin_client:send {
+            method = "GET",
+            path = "/plugins/" .. plugin_id
+          })
+
+          json = cjson.decode(assert.res_status(200, res))
+          assert.same(json.config.new_field, "ABC")
+          assert.same(json.config.old_field, "ABC")
+        end)
+      end)
+    end)
+  end)
+end)

spec/fixtures/custom_plugins/kong/plugins/translate-backwards-older-plugin/handler.lua

@@ -0,0 +1,12 @@
+local kong = kong
+
+local TranslateBackwardsOlderPlugin = {
+  PRIORITY = 1000,
+  VERSION = "0.1.0",
+}
+
+function TranslateBackwardsOlderPlugin:access(conf)
+    kong.log("access phase")
+end
+
+return TranslateBackwardsOlderPlugin