fix(db/migration): do migration before validation (#10348)

Co-authored-by: Chrono <[email protected]>
Co-authored-by: Aapo Talvensaari <[email protected]>
Co-authored-by: Enrique García Cota <[email protected]>
(cherry picked from commit 2f82ced)

Author: 4 people

CHANGELOG.md

@@ -82,6 +82,8 @@
   [#10346](https://github.com/Kong/kong/pull/10346)
 - Fix an issue where control plane does not rename fields correctly for `session` for older version of data planes.
   [#10352](https://github.com/Kong/kong/pull/10352)
+- Fix an issue where validation to regex routes may be skipped when the old-fashioned config is used for DB-less Kong.
+  [#10348](https://github.com/Kong/kong/pull/10348)
 
 ## 3.2.0
 

kong/db/declarative/init.lua

@@ -219,13 +219,13 @@ function _M:parse_table(dc_table, hash)
     error("expected a table as input", 2)
   end
 
+  on_the_fly_migration(dc_table)
+
   local entities, err_t, meta = self.schema:flatten(dc_table)
   if err_t then
     return nil, pretty_print_error(err_t), err_t
   end
 
-  on_the_fly_migration(entities, dc_table._format_version)
-
   yield()
 
   if not self.partial then

kong/db/declarative/migrations/route_path.lua

@@ -1,32 +1,47 @@
 local migrate_path = require "kong.db.migrations.migrate_path_280_300"
+local lyaml_null = require("lyaml").null
+local cjson_null = require("cjson").null
 
+local ngx_null = ngx.null
 local pairs  = pairs
 local ipairs = ipairs
-local null   = ngx.null
 
-return function(tbl, version)
-  if not tbl or not (version == "1.1" or version == "2.1") then
-    return
-  end
-
-  local routes = tbl.routes
+local EMPTY = {}
 
-  if not routes then
-    -- no need to migrate
-    return
+local function ensure_table(val)
+  if val == nil or val == ngx_null or val == lyaml_null or val == cjson_null or type(val) ~= "table" then
+    return EMPTY
   end
+  return val
+end
 
+local function migrate_routes(routes)
   for _, route in pairs(routes) do
-    local paths = route.paths
-    if not paths or paths == null then
-      -- no need to migrate
-      goto continue
-    end
+    local paths = ensure_table(route.paths)
 
     for idx, path in ipairs(paths) do
       paths[idx] = migrate_path(path)
     end
+  end
+end
+
+return function(tbl)
+  local version = tbl._format_version
+  if not tbl or not (version == "1.1" or version == "2.1") then
+    return
+  end
+
+  -- migrate top-level routes
+  local routes = ensure_table(tbl.routes)
+  migrate_routes(routes)
 
-    ::continue::
+  -- migrate routes nested in top-level services
+  local services = ensure_table(tbl.services)
+  for _, service in ipairs(services) do
+    local nested_routes = ensure_table(service.routes)
+
+    migrate_routes(nested_routes)
   end
+
+  tbl._format_version = "3.0"
 end

spec/01-unit/01-db/01-schema/11-declarative_config/04-on-the-fly-migration_spec.lua

@@ -154,3 +154,35 @@ describe("declarative config: on the fly migration", function()
     end)
   end
 end)
+
+it("validation should happens after migration", function ()
+  local dc = assert(declarative.new_config(conf_loader()))
+  local config =
+    [[
+      _format_version: "2.1"
+      services:
+      - name: foo
+        host: example.com
+        protocol: https
+        enabled: false
+        _comment: my comment
+      - name: bar
+        host: example.test
+        port: 3000
+        _comment: my comment
+        routes:
+        - name: foo
+          path_handling: v0
+          protocols: ["https"]
+          paths: ["/regex.+(", "/prefix" ]
+          snis:
+          - "example.com"
+    ]]
+
+    local config_tbl, err = dc:parse_string(config)
+
+    assert.falsy(config_tbl)
+    assert.matches("invalid regex:", err, nil, true)
+    assert.matches("/regex.+(", err, nil, true)
+    assert.matches("missing )", err, nil, true)
+end)

spec/02-integration/02-cmd/11-config_spec.lua

@@ -109,8 +109,7 @@ describe("kong config", function()
     local _, res = assert(thread:join())
     assert.matches("signal=config-db-import", res, nil, true)
     -- it will be updated on-the-fly
-    -- but the version should still be 1.1
-    assert.matches("decl_fmt_version=1.1", res, nil, true)
+    assert.matches("decl_fmt_version=3.0", res, nil, true)
     assert.matches("file_ext=.yml", res, nil, true)
 
     local client = helpers.admin_client()