refactor(db/schema): improve expression validation

.requirements

@@ -9,5 +9,5 @@ LUA_KONG_NGINX_MODULE=4d19e8d19c6dbc07eba5cf6f5ebacad95266f928 # 0.6.0
 LUA_RESTY_LMDB=222546680e9c31a9b97733b5db595688a521cd1a # 1.1.0
 LUA_RESTY_EVENTS=2f6fa23eb3d0b76a3b35fd915711200e90bc6732 # 0.1.6
 LUA_RESTY_WEBSOCKET=60eafc3d7153bceb16e6327074e0afc3d94b1316 # 0.4.0
-ATC_ROUTER=fb82fdd3a4c111e24e2b0044ee151e50fb37499f # 1.0.5
+ATC_ROUTER=72cc8fddeac024c54c9c1fa5a25c28a72d79080e # 1.1.0
 

CHANGELOG.md

@@ -145,6 +145,8 @@
   [#11140](https://github.com/Kong/kong/pull/11140)
 - Bumped pgmoon from 1.16.0 to 1.16.1 (Kong's fork)
   [#11181](https://github.com/Kong/kong/pull/11181)
+- Bumped atc-router from 1.0.5 to 1.1.0
+  [#10100](https://github.com/Kong/kong/pull/10100)
 
 ## 3.3.0
 

build/BUILD.bazel

@@ -29,9 +29,11 @@ lualib_deps = [
 install_lualib_deps_cmd = "\n".join([
     """
     DEP=$(pwd)/external/%s
-    # TODO: fix atc_router makefile so that we can choose to only install lualib
-    sed -i -e '/libatc_router.so/d' ${DEP}/Makefile
-    INSTALL=/usr/bin/install make --silent -C ${DEP} LUA_LIB_DIR=${BUILD_DESTDIR}/openresty/lualib install
+    if [[ ${DEP} == */atc_router ]]; then
+        INSTALL=/usr/bin/install make --silent -C ${DEP} LUA_LIB_DIR=${BUILD_DESTDIR}/openresty/lualib install-lualib
+    else
+        INSTALL=/usr/bin/install make --silent -C ${DEP} LUA_LIB_DIR=${BUILD_DESTDIR}/openresty/lualib install
+    fi
 """ % dep.lstrip("@").split("/")[0]
     for dep in lualib_deps
 ])

kong/db/schema/entities/routes.lua

@@ -2,18 +2,31 @@ local typedefs = require("kong.db.schema.typedefs")
 local router = require("resty.router.router")
 local deprecation = require("kong.deprecation")
 
-local CACHED_SCHEMA = require("kong.router.atc").schema
-local get_expression = require("kong.router.compat").get_expression
+local validate_route
+local has_paths
+do
+  local isempty        = require("table.isempty")
+  local CACHED_SCHEMA  = require("kong.router.atc").schema
+  local get_expression = require("kong.router.compat").get_expression
 
-local function validate_expression(id, exp)
-  local r = router.new(CACHED_SCHEMA)
+  local type = type
 
-  local res, err = r:add_matcher(0, id, exp)
-  if not res then
-    return nil, "Router Expression failed validation: " .. err
+  -- works with both `traditional_compatiable` and `expressions` routes`
+  validate_route = function(entity)
+    local exp = entity.expression or get_expression(entity)
+
+    local ok, err = router.validate(CACHED_SCHEMA, exp)
+    if not ok then
+      return nil, "Router Expression failed validation: " .. err
+    end
+
+    return true
   end
 
-  return true
+  has_paths = function(entity)
+    local paths = entity.paths
+    return type(paths) == "table" and not isempty(paths)
+  end
 end
 
 local kong_router_flavor = kong and kong.configuration and kong.configuration.router_flavor
@@ -62,7 +75,7 @@ if kong_router_flavor == "expressions" then
       { custom_entity_check = {
         field_sources = { "expression", "id", },
         fn = function(entity)
-          local ok, err = validate_expression(entity.id, entity.expression)
+          local ok, err = validate_route(entity)
           if not ok then
             return nil, err
           end
@@ -75,6 +88,59 @@ if kong_router_flavor == "expressions" then
 
 -- router_flavor in ('traditional_compatible', 'traditional')
 else
+  local PATH_V1_DEPRECATION_MSG
+
+  if kong_router_flavor == "traditional" then
+    PATH_V1_DEPRECATION_MSG =
+      "path_handling='v1' is deprecated and " ..
+      "will be removed in future version, " ..
+      "please use path_handling='v0' instead"
+
+  elseif kong_router_flavor == "traditional_compatible" then
+    PATH_V1_DEPRECATION_MSG =
+      "path_handling='v1' is deprecated and " ..
+      "will not work under 'traditional_compatible' router_flavor, " ..
+      "please use path_handling='v0' instead"
+  end
+
+  local entity_checks = {
+    { conditional = { if_field = "protocols",
+                      if_match = { elements = { type = "string", not_one_of = { "grpcs", "https", "tls", "tls_passthrough" }}},
+                      then_field = "snis",
+                      then_match = { len_eq = 0 },
+                      then_err = "'snis' can only be set when 'protocols' is 'grpcs', 'https', 'tls' or 'tls_passthrough'",
+                    }},
+    { custom_entity_check = {
+      field_sources = { "path_handling" },
+      fn = function(entity)
+        if entity.path_handling == "v1" then
+          deprecation(PATH_V1_DEPRECATION_MSG, { after = "3.0", })
+        end
+
+        return true
+      end,
+    }},
+  }
+
+  if kong_router_flavor == "traditional_compatible" then
+    table.insert(entity_checks,
+      { custom_entity_check = {
+        run_with_missing_fields = true,
+        field_sources = { "id", "paths", },
+        fn = function(entity)
+          if has_paths(entity) then
+            local ok, err = validate_route(entity)
+            if not ok then
+              return nil, err
+            end
+          end
+
+          return true
+        end,
+      }}
+    )
+  end
+
   return {
     name         = "routes",
     primary_key  = { "id" },
@@ -134,46 +200,6 @@ else
       type = "foreign", reference = "services" }, },
     },
 
-    entity_checks = {
-      { conditional = { if_field = "protocols",
-                        if_match = { elements = { type = "string", not_one_of = { "grpcs", "https", "tls", "tls_passthrough" }}},
-                        then_field = "snis",
-                        then_match = { len_eq = 0 },
-                        then_err = "'snis' can only be set when 'protocols' is 'grpcs', 'https', 'tls' or 'tls_passthrough'",
-                      }},
-      { custom_entity_check = {
-        field_sources = { "path_handling" },
-        fn = function(entity)
-          if entity.path_handling == "v1" then
-            if kong_router_flavor == "traditional" then
-              deprecation("path_handling='v1' is deprecated and will be removed in future version, " ..
-                          "please use path_handling='v0' instead", { after = "3.0", })
-
-            elseif kong_router_flavor == "traditional_compatible" then
-              deprecation("path_handling='v1' is deprecated and will not work under traditional_compatible " ..
-                          "router_flavor, please use path_handling='v0' instead", { after = "3.0", })
-            end
-          end
-
-          return true
-        end,
-      }},
-      { custom_entity_check = {
-        run_with_missing_fields = true,
-        field_sources = { "id", "paths", },
-        fn = function(entity)
-          if kong_router_flavor == "traditional_compatible" and
-             type(entity.paths) == "table" and #entity.paths > 0 then
-            local exp = get_expression(entity)
-            local ok, err = validate_expression(entity.id, exp)
-            if not ok then
-              return nil, err
-            end
-          end
-
-          return true
-        end,
-      }},
-    },
+    entity_checks = entity_checks,
   }
 end

spec/01-unit/01-db/01-schema/06-routes_spec.lua

@@ -1356,14 +1356,31 @@ describe("routes schema (flavor = traditional_compatible)", function()
       id             = a_valid_uuid,
       name           = "my_route",
       protocols      = { "http" },
-      paths          = { "~/\\/*/user$" },
+      paths          = { "~/[abc/*/user$" },
       service        = { id = another_uuid },
     }
     route = Routes:process_auto_fields(route, "insert")
     local ok, errs = Routes:validate_insert(route)
     assert.falsy(ok)
-    assert.truthy(errs["@entity"])
-    assert.matches("Router Expression failed validation", errs["@entity"][1],
+    assert.truthy(errs["paths"])
+    assert.matches("invalid regex:", errs["paths"][1],
                    nil, true)
+
+    -- verified by `schema/typedefs.lua`
+    assert.falsy(errs["@entity"])
+  end)
+
+  it("won't fail when rust.regex update to 1.8", function()
+    local route = {
+      id             = a_valid_uuid,
+      name           = "my_route",
+      protocols      = { "http" },
+      paths          = { "~/\\/*/user$" },
+      service        = { id = another_uuid },
+    }
+    route = Routes:process_auto_fields(route, "insert")
+    local ok, errs = Routes:validate_insert(route)
+    assert.truthy(ok)
+    assert.is_nil(errs)
   end)
 end)

spec/02-integration/05-proxy/02-router_spec.lua

@@ -2343,22 +2343,17 @@ for _, strategy in helpers.each_strategy() do
         assert.equal("no Service found with those values", json.message)
       end)
 
-      it("#db rebuilds router correctly after passing invalid route", function()
+      it("#db rebuilds router correctly after passing route with special escape", function()
         local admin_client = helpers.admin_client()
 
         local res = assert(admin_client:post("/routes", {
           headers = { ["Content-Type"] = "application/json" },
           body = {
-            -- this is a invalid regex path
+            -- this is a valid regex path in Rust.regex 1.8
             paths = { "~/delay/(?<delay>[^\\/]+)$", },
           },
         }))
-        if flavor == "traditional" then
-          assert.res_status(201, res)
-
-        else
-          assert.res_status(400, res)
-        end
+        assert.res_status(201, res)
 
         helpers.wait_for_all_config_update()