fix(*): reject config if both deprecated and new field defined and th…

…eir values mismatch (#13565)

* fix(*): reject config if both deprecated and new field defined and their values mismatch

This commit adds a validation to deprecated fields
that checks if in case both new field and old field were defined -
their values must match.

Note that if one of the fields is null then the validation passes even
if the other is not null.

KAG-5262

* fixup! fix(*): reject config if both deprecated and new field defined and their values mismatch

PR fixes

* fixup! fix(*): reject config if both deprecated and new field defined and their values mismatch

PR fixes 2

changelog/unreleased/kong/reject-config-on-deprecated-fields-mismatch.yml

@@ -0,0 +1,5 @@
+message: |
+  Changed the behaviour of shorthand fields that are used to describe deprecated fields. If
+  both fields are sent in the request and their values mismatch - the request will be rejected.
+type: bugfix
+scope: Core

kong/db/schema/init.lua

@@ -17,6 +17,7 @@ local table_merge = require "kong.tools.table".table_merge
 local null_aware_table_merge = require "kong.tools.table".null_aware_table_merge
 local table_path = require "kong.tools.table".table_path
 local is_array = require "kong.tools.table".is_array
+local join_string = require "kong.tools.string".join
 
 
 local setmetatable = setmetatable
@@ -1704,6 +1705,35 @@ local function collect_field_reference(refs, key, reference)
 end
 
 
+local function validate_deprecation_exclusiveness(data, shorthand_value, shorthand_name, shorthand_definition)
+  if shorthand_value == nil or
+      shorthand_value == ngx.null or
+      shorthand_definition.deprecation == nil or
+      shorthand_definition.deprecation.replaced_with == 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 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
+
+  return true
+end
+
+
 --- Given a table, update its fields whose schema
 -- definition declares them as `auto = true`,
 -- based on its CRUD operation context, and set
@@ -1741,27 +1771,34 @@ function Schema:process_auto_fields(data, context, nulls, opts)
           errs[sname] = err
           has_errs = true
         else
-          data[sname] = nil
-          local new_values = sdata.func(value)
-          if new_values then
-            -- a shorthand field may have a deprecation property, that is used
-            -- to determine whether the shorthand's return value takes precedence
-            -- over the similarly named actual schema fields' value when both
-            -- are present. On deprecated shorthand fields the actual schema
-            -- field value takes the precedence, otherwise the shorthand's
-            -- return value takes the precedence.
-            local deprecation = sdata.deprecation
-            for k, v in pairs(new_values) do
-              if type(v) == "table" then
-                local source = {}
-                if data[k] and data[k] ~= null then
-                  source = data[k]
-                end
-                data[k] = deprecation and null_aware_table_merge(v, source)
-                                       or table_merge(source, v)
+          local _, deprecation_error = validate_deprecation_exclusiveness(data, value, sname, sdata)
 
-              elseif not deprecation or (data[k] == nil or data[k] == null) then
-                data[k] = v
+          if deprecation_error then
+            errs[sname] = deprecation_error
+            has_errs = true
+          else
+            data[sname] = nil
+            local new_values = sdata.func(value)
+            if new_values then
+              -- a shorthand field may have a deprecation property, that is used
+              -- to determine whether the shorthand's return value takes precedence
+              -- over the similarly named actual schema fields' value when both
+              -- are present. On deprecated shorthand fields the actual schema
+              -- field value takes the precedence, otherwise the shorthand's
+              -- return value takes the precedence.
+              local deprecation = sdata.deprecation
+              for k, v in pairs(new_values) do
+                if type(v) == "table" then
+                  local source = {}
+                  if data[k] and data[k] ~= null then
+                    source = data[k]
+                  end
+                  data[k] = deprecation and null_aware_table_merge(v, source)
+                                        or table_merge(source, v)
+
+                elseif not deprecation or (data[k] == nil or data[k] == null) then
+                  data[k] = v
+                end
               end
             end
           end

kong/db/schema/metaschema.lua

@@ -206,6 +206,15 @@ local field_schema = {
       { message = { type = "string", required = true } },
       { removal_in_version = { type = "string", required = true } },
       { old_default = { type = "any", required = false } },
+      { replaced_with = { type = "array", required = false,
+          elements = { type = "record",
+            required = false,
+            fields = {
+              { path = { type = "array", len_min = 1, required = true, elements = { type = "string"}} },
+              { reverse_mapping_function = { type = "function", required = false }}
+            },
+          }
+      } },
     },
   } },
 }

kong/plugins/acme/schema.lua

@@ -43,6 +43,7 @@ local LEGACY_SCHEMA_TRANSLATIONS = {
     len_min = 0,
     translate_backwards = {'password'},
     deprecation = {
+      replaced_with = { { path = { 'password' } } },
       message = "acme: config.storage_config.redis.auth is deprecated, please use config.storage_config.redis.password instead",
       removal_in_version = "4.0", },
     func = function(value)
@@ -53,6 +54,7 @@ local LEGACY_SCHEMA_TRANSLATIONS = {
     type = "string",
     translate_backwards = {'server_name'},
     deprecation = {
+      replaced_with = { { path = { 'server_name' } } },
       message = "acme: config.storage_config.redis.ssl_server_name is deprecated, please use config.storage_config.redis.server_name instead",
       removal_in_version = "4.0", },
     func = function(value)
@@ -64,6 +66,7 @@ local LEGACY_SCHEMA_TRANSLATIONS = {
     len_min = 0,
     translate_backwards = {'extra_options', 'namespace'},
     deprecation = {
+      replaced_with = { { path = { 'extra_options', 'namespace' } } },
       message = "acme: config.storage_config.redis.namespace is deprecated, please use config.storage_config.redis.extra_options.namespace instead",
       removal_in_version = "4.0", },
     func = function(value)
@@ -74,6 +77,7 @@ local LEGACY_SCHEMA_TRANSLATIONS = {
     type = "integer",
     translate_backwards = {'extra_options', 'scan_count'},
     deprecation = {
+      replaced_with = { { path = { 'extra_options', 'scan_count' } } },
       message = "acme: config.storage_config.redis.scan_count is deprecated, please use config.storage_config.redis.extra_options.scan_count instead",
       removal_in_version = "4.0", },
     func = function(value)

kong/plugins/rate-limiting/schema.lua

@@ -104,6 +104,7 @@ return {
             type = "string",
             translate_backwards = {'redis', 'host'},
             deprecation = {
+              replaced_with = { { path = { 'redis', 'host' } } },
               message = "rate-limiting: config.redis_host is deprecated, please use config.redis.host instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -114,6 +115,7 @@ return {
             type = "integer",
             translate_backwards = {'redis', 'port'},
             deprecation = {
+              replaced_with = { { path = { 'redis', 'port' } } },
               message = "rate-limiting: config.redis_port is deprecated, please use config.redis.port instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -125,6 +127,7 @@ return {
             len_min = 0,
             translate_backwards = {'redis', 'password'},
             deprecation = {
+              replaced_with = { { path = { 'redis', 'password' } } },
               message = "rate-limiting: config.redis_password is deprecated, please use config.redis.password instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -135,6 +138,7 @@ return {
             type = "string",
             translate_backwards = {'redis', 'username'},
             deprecation = {
+              replaced_with = { { path = { 'redis', 'username' } } },
               message = "rate-limiting: config.redis_username is deprecated, please use config.redis.username instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -145,6 +149,7 @@ return {
             type = "boolean",
             translate_backwards = {'redis', 'ssl'},
             deprecation = {
+              replaced_with = { { path = { 'redis', 'ssl' } } },
               message = "rate-limiting: config.redis_ssl is deprecated, please use config.redis.ssl instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -155,6 +160,7 @@ return {
             type = "boolean",
             translate_backwards = {'redis', 'ssl_verify'},
             deprecation = {
+              replaced_with = { { path = { 'redis', 'ssl_verify' } } },
               message = "rate-limiting: config.redis_ssl_verify is deprecated, please use config.redis.ssl_verify instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -165,6 +171,7 @@ return {
             type = "string",
             translate_backwards = {'redis', 'server_name'},
             deprecation = {
+              replaced_with = { { path = { 'redis', 'server_name' } } },
               message = "rate-limiting: config.redis_server_name is deprecated, please use config.redis.server_name instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -175,6 +182,7 @@ return {
             type = "integer",
             translate_backwards = {'redis', 'timeout'},
             deprecation = {
+              replaced_with = { { path = { 'redis', 'timeout' } } },
               message = "rate-limiting: config.redis_timeout is deprecated, please use config.redis.timeout instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -185,6 +193,7 @@ return {
             type = "integer",
             translate_backwards = {'redis', 'database'},
             deprecation = {
+              replaced_with = { { path = { 'redis', 'database' } } },
               message = "rate-limiting: config.redis_database is deprecated, please use config.redis.database instead",
               removal_in_version = "4.0", },
             func = function(value)

kong/plugins/response-ratelimiting/schema.lua

@@ -143,6 +143,7 @@ return {
             type = "string",
             translate_backwards = {'redis', 'host'},
             deprecation = {
+              replaced_with = { { path = { 'redis', 'host' } } },
               message = "response-ratelimiting: config.redis_host is deprecated, please use config.redis.host instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -153,6 +154,7 @@ return {
             type = "integer",
             translate_backwards = {'redis', 'port'},
             deprecation = {
+              replaced_with = { { path = {'redis', 'port'} } },
               message = "response-ratelimiting: config.redis_port is deprecated, please use config.redis.port instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -164,6 +166,7 @@ return {
             len_min = 0,
             translate_backwards = {'redis', 'password'},
             deprecation = {
+              replaced_with = { { path = {'redis', 'password'} } },
               message = "response-ratelimiting: config.redis_password is deprecated, please use config.redis.password instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -174,6 +177,7 @@ return {
             type = "string",
             translate_backwards = {'redis', 'username'},
             deprecation = {
+              replaced_with = { { path = {'redis', 'username'} } },
               message = "response-ratelimiting: config.redis_username is deprecated, please use config.redis.username instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -184,6 +188,7 @@ return {
             type = "boolean",
             translate_backwards = {'redis', 'ssl'},
             deprecation = {
+              replaced_with = { { path = {'redis', 'ssl'} } },
               message = "response-ratelimiting: config.redis_ssl is deprecated, please use config.redis.ssl instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -194,6 +199,7 @@ return {
             type = "boolean",
             translate_backwards = {'redis', 'ssl_verify'},
             deprecation = {
+              replaced_with = { { path = {'redis', 'ssl_verify'} } },
               message = "response-ratelimiting: config.redis_ssl_verify is deprecated, please use config.redis.ssl_verify instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -204,6 +210,7 @@ return {
             type = "string",
             translate_backwards = {'redis', 'server_name'},
             deprecation = {
+              replaced_with = { { path = {'redis', 'server_name'} } },
               message = "response-ratelimiting: config.redis_server_name is deprecated, please use config.redis.server_name instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -214,6 +221,7 @@ return {
             type = "integer",
             translate_backwards = {'redis', 'timeout'},
             deprecation = {
+              replaced_with = { { path = {'redis', 'timeout'} } },
               message = "response-ratelimiting: config.redis_timeout is deprecated, please use config.redis.timeout instead",
               removal_in_version = "4.0", },
             func = function(value)
@@ -224,6 +232,7 @@ return {
             type = "integer",
             translate_backwards = {'redis', 'database'},
             deprecation = {
+              replaced_with = { { path = {'redis', 'database'} } },
               message = "response-ratelimiting: config.redis_database is deprecated, please use config.redis.database instead",
               removal_in_version = "4.0", },
             func = function(value)