From cfb6a92554b242a99bd33ddb2d88fa2a60813895 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Fri, 10 Jan 2025 14:20:36 +0800 Subject: [PATCH 01/42] feat(sync): validate deltas --- kong-3.10.0-0.rockspec | 1 + kong/clustering/services/sync/rpc.lua | 9 +- kong/clustering/services/sync/validate.lua | 67 ++++++++++++++ kong/db/schema/others/declarative_config.lua | 94 ++++++++++++++++++-- 4 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 kong/clustering/services/sync/validate.lua diff --git a/kong-3.10.0-0.rockspec b/kong-3.10.0-0.rockspec index 31f68741f8c..b189153bd6d 100644 --- a/kong-3.10.0-0.rockspec +++ b/kong-3.10.0-0.rockspec @@ -101,6 +101,7 @@ build = { ["kong.clustering.services.sync"] = "kong/clustering/services/sync/init.lua", ["kong.clustering.services.sync.rpc"] = "kong/clustering/services/sync/rpc.lua", ["kong.clustering.services.sync.hooks"] = "kong/clustering/services/sync/hooks.lua", + ["kong.clustering.services.sync.validate"] = "kong/clustering/services/sync/validate.lua", ["kong.clustering.services.sync.strategies.postgres"] = "kong/clustering/services/sync/strategies/postgres.lua", ["kong.cluster_events"] = "kong/cluster_events/init.lua", diff --git a/kong/clustering/services/sync/rpc.lua b/kong/clustering/services/sync/rpc.lua index 56e2a9f1d27..5a6d297b420 100644 --- a/kong/clustering/services/sync/rpc.lua +++ b/kong/clustering/services/sync/rpc.lua @@ -10,6 +10,7 @@ local isempty = require("table.isempty") local events = require("kong.runloop.events") +local validate_deltas = require("kong.clustering.services.sync.validate").validate_deltas local insert_entity_for_txn = declarative.insert_entity_for_txn local delete_entity_for_txn = declarative.delete_entity_for_txn local DECLARATIVE_HASH_KEY = constants.DECLARATIVE_HASH_KEY @@ -194,6 +195,7 @@ local function do_sync() return nil, "default namespace does not exist inside params" end + local wipe = ns_delta.wipe local deltas = ns_delta.deltas if not deltas then @@ -219,9 +221,14 @@ local function do_sync() end assert(type(kong.default_workspace) == "string") + -- validate deltas + local ok, err = validate_deltas(deltas, wipe) + if not ok then + return nil, err + end + local t = txn.begin(512) - local wipe = ns_delta.wipe if wipe then t:db_drop(false) end diff --git a/kong/clustering/services/sync/validate.lua b/kong/clustering/services/sync/validate.lua new file mode 100644 index 00000000000..c982b3efa65 --- /dev/null +++ b/kong/clustering/services/sync/validate.lua @@ -0,0 +1,67 @@ +local errors = require("kong.db.errors") +local declarative_config = require("kong.db.schema.others.declarative_config") + +local validate = declarative_config.validate +local pk_string = declarative_config.pk_string +local validate_references_full = declarative_config.validate_references_full +local validate_references_sync = declarative_config.validate_references_sync + + +local function validate_deltas(deltas, is_full_sync) + + -- genearte deltas table mapping primary key string to entity item + local deltas_map = {} + + -- generate declarative config table + local dc_table = { _format_version = "3.0", } + + for _, delta in ipairs(deltas) do + local delta_type = delta.type + local delta_entity = delta.entity + + if delta_entity ~= nil and delta_entity ~= ngx.null then + dc_table[delta_type] = dc_table[delta_type] or {} + + table.insert(dc_table[delta_type], delta_entity) + + -- table: primary key string -> entity + if not is_full_sync then + local schema = kong.db[delta_type].schema + local pk = schema:extract_pk_values(delta_entity) + local pks = pk_string(schema, pk) + + deltas_map[pks] = delta_entity + end + end + end + + -- validate schema + local dc_schema = kong.db.declarative_config.schema + + local ok, err_t = validate(dc_schema, dc_table) + if not ok then + return nil, errors:schema_violation(err_t) + end + + -- validate references for full sync + if is_full_sync then + local ok, err_t = validate_references_full(dc_schema, dc_table) + if not ok then + return nil, errors:schema_violation(err_t) + end + return true + end + + -- validate references for non full sync + local ok, err_t = validate_references_sync(deltas, deltas_map) + if not ok then + return nil, errors:schema_violation(err_t) + end + + return true +end + + +return { + validate_deltas = validate_deltas, +} diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 5e29ffda318..ff9f23571c3 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -14,6 +14,7 @@ local null = ngx.null local type = type local next = next local pairs = pairs +local fmt = string.format local yield = require("kong.tools.yield").yield local ipairs = ipairs local insert = table.insert @@ -506,6 +507,71 @@ local function validate_references(self, input) end +function DeclarativeConfig.validate_references_full(self, input) + return validate_references(self, input) +end + + +function DeclarativeConfig.validate_references_sync(deltas, deltas_map) + local errs = {} + + for _, delta in ipairs(deltas) do + local item_type = delta.type + local item = delta.entity + local ws_id = delta.ws_id or kong.default_workspace + + local foreign_refs = foreign_references[item_type] + + if not item or item == ngx.null or not foreign_refs then + goto continue + end + + for k, v in pairs(item) do + + local foreign_entity = foreign_refs[k] + + if foreign_entity and v ~= null then -- k is foreign key + + local db = kong.db[foreign_entity] + + -- try to find it in deltas + local pks = DeclarativeConfig.pk_string(db.schema, v) + local fvalue = deltas_map[pks] + if fvalue then + goto found + end + + -- try to find it in DB (LMDB) + fvalue, _ = db:select(v, { workspace = ws_id }) + + -- could not find its foreign reference + if not fvalue then + errs[item_type] = errs[item_type] or {} + errs[item_type][foreign_entity] = errs[item_type][foreign_entity] or {} + + local msg = fmt("could not find %s's foreign refrences %s (%s)", + item_type, foreign_entity, + (type(v) == "string" and v or cjson_encode(v))) + + insert(errs[item_type][foreign_entity], msg) + end + end + + ::found:: + end + + ::continue:: + end + + + if next(errs) then + return nil, errs + end + + return true +end + + -- This is a best-effort generation of a cache-key-like identifier -- to feed the hash when generating deterministic UUIDs. -- We do not use the actual `cache_key` function from the DAO because @@ -781,15 +847,7 @@ local function get_unique_key(schema, entity, field, value) end -local function flatten(self, input) - -- manually set transform here - -- we can't do this in the schema with a `default` because validate - -- needs to happen before process_auto_fields, which - -- is the one in charge of filling out default values - if input._transform == nil then - input._transform = true - end - +function DeclarativeConfig.validate(self, input) local ok, err = self:validate(input) if not ok then yield() @@ -812,6 +870,24 @@ local function flatten(self, input) yield() end + return true +end + + +local function flatten(self, input) + -- manually set transform here + -- we can't do this in the schema with a `default` because validate + -- needs to happen before process_auto_fields, which + -- is the one in charge of filling out default values + if input._transform == nil then + input._transform = true + end + + local ok, err = DeclarativeConfig.validate(self, input) + if not ok then + return nil, err + end + generate_ids(input, self.known_entities) yield() From f190970b30624e6e22a31e5c268303334564652f Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 13 Jan 2025 15:43:39 +0800 Subject: [PATCH 02/42] add test case --- bin/busted | 4 + kong/clustering/services/sync/validate.lua | 9 +- kong/db/declarative/init.lua | 3 + .../18-hybrid_rpc/06-validate_deltas_spec.lua | 190 ++++++++++++++++++ spec/fixtures/dc_blueprints.lua | 8 +- spec/internal/db.lua | 4 +- 6 files changed, 208 insertions(+), 10 deletions(-) create mode 100644 spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua diff --git a/bin/busted b/bin/busted index e59760322fa..5be3493b056 100755 --- a/bin/busted +++ b/bin/busted @@ -64,6 +64,10 @@ if not os.getenv("KONG_BUSTED_RESPAWNED") then -- create shared dict resty_flags = resty_flags .. require("spec.fixtures.shared_dict") + -- create lmdb environment + local lmdb_env = os.tmpname() + resty_flags = resty_flags .. string.format(' --main-conf "lmdb_environment_path %s;" ', lmdb_env) + if resty_flags then table.insert(cmd, cmd_prefix_count+1, resty_flags) end diff --git a/kong/clustering/services/sync/validate.lua b/kong/clustering/services/sync/validate.lua index c982b3efa65..312f1301525 100644 --- a/kong/clustering/services/sync/validate.lua +++ b/kong/clustering/services/sync/validate.lua @@ -1,10 +1,11 @@ -local errors = require("kong.db.errors") +local declarative = require("kong.db.declarative") local declarative_config = require("kong.db.schema.others.declarative_config") local validate = declarative_config.validate local pk_string = declarative_config.pk_string local validate_references_full = declarative_config.validate_references_full local validate_references_sync = declarative_config.validate_references_sync +local pretty_print_error = declarative.pretty_print_error local function validate_deltas(deltas, is_full_sync) @@ -40,14 +41,14 @@ local function validate_deltas(deltas, is_full_sync) local ok, err_t = validate(dc_schema, dc_table) if not ok then - return nil, errors:schema_violation(err_t) + return nil, pretty_print_error(err_t) end -- validate references for full sync if is_full_sync then local ok, err_t = validate_references_full(dc_schema, dc_table) if not ok then - return nil, errors:schema_violation(err_t) + return nil, pretty_print_error(err_t) end return true end @@ -55,7 +56,7 @@ local function validate_deltas(deltas, is_full_sync) -- validate references for non full sync local ok, err_t = validate_references_sync(deltas, deltas_map) if not ok then - return nil, errors:schema_violation(err_t) + return nil, pretty_print_error(err_t) end return true diff --git a/kong/db/declarative/init.lua b/kong/db/declarative/init.lua index 1f209657f0e..c58f40eb088 100644 --- a/kong/db/declarative/init.lua +++ b/kong/db/declarative/init.lua @@ -263,5 +263,8 @@ _M.delete_entity_for_txn = declarative_import.delete_entity_for_txn _M.workspace_id = declarative_import.workspace_id _M.GLOBAL_WORKSPACE_TAG = declarative_import.GLOBAL_WORKSPACE_TAG +-- helpful function +_M.pretty_print_error = pretty_print_error + return _M diff --git a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua new file mode 100644 index 00000000000..8529c75c9b5 --- /dev/null +++ b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua @@ -0,0 +1,190 @@ +local helpers = require "spec.helpers" +local pl_file = require "pl.file" +local txn = require "resty.lmdb.transaction" +local declarative = require "kong.db.declarative" + +local insert_entity_for_txn = declarative.insert_entity_for_txn +local validate_deltas = require("kong.clustering.services.sync.validate").validate_deltas + + +local function lmdb_drop() + local t = txn.begin(512) + t:db_drop(false) + t:commit() +end + + +local function lmdb_insert(name, entity) + local t = txn.begin(512) + local res, err = insert_entity_for_txn(t, name, entity, nil) + if not res then + error("lmdb insert failed: " .. err) + end + + local ok, err = t:commit() + if not ok then + error("lmdb t:commit() failed: " .. err) + end +end + + +-- insert into LMDB +local function db_insert(bp, name, entity) + -- insert into dc blueprints + entity = bp[name]:insert(entity) + + -- insert into LMDB + lmdb_insert(name, entity) + + assert(kong.db[name]:select({id = entity.id})) + + return entity +end + + +local function setup_bp() + -- reset lmdb + lmdb_drop() + + -- init bp / db ( true for expand_foreigns) + bp, db = helpers.get_db_utils("off", nil, nil, nil, nil, true) + + -- init workspaces + local workspaces = require "kong.workspaces" + workspaces.upsert_default(db) + + -- init declarative config + local dc, err = declarative.new_config(kong.configuration) + assert(dc, err) + kong.db.declarative_config = dc + + return bp, db +end + + +describe("[delta validations]",function() + + it("workspace id", function() + local bp = setup_bp() + + -- add entities + local ws = db_insert(bp, "workspaces", { name = "ws-001" }) + local service = db_insert(bp, "services", { name = "service-001", }) + local route = db_insert(bp, "routes", { + name = "route-001", + paths = { "/mock" }, + service = { id = service.id }, + }) + + local deltas = declarative.export_config_sync() + + for _, delta in ipairs(deltas) do + local delta_entity = delta.entity + local delta_type = delta.type + local ws_id = delta.ws_id + assert(ws_id) + end + end) + + it("route has foreign service", function() + local bp = setup_bp() + + -- add entities + local ws = db_insert(bp, "workspaces", { name = "ws-001" }) + local service = db_insert(bp, "services", { name = "service-001", }) + local route = db_insert(bp, "routes", { + name = "route-001", + paths = { "/mock" }, + service = { id = service.id }, + }) + + local deltas = declarative.export_config_sync() + + local ok, err = validate_deltas(deltas) + assert(ok, "validate should not fail: " .. tostring(err)) + end) + + it("route has unmatched foreign service", function() + local bp = setup_bp() + + -- add entities + local ws = db_insert(bp, "workspaces", { name = "ws-001" }) + local service = db_insert(bp, "services", { name = "service-001", }) + local route = db_insert(bp, "routes", { + name = "route-001", + paths = { "/mock" }, + -- unmatched service + service = { id = "00000000-0000-0000-0000-000000000000" }, + }) + + local deltas = declarative.export_config_sync() + end) + + it("100 routes -> 1 services: matched foreign keys", function() + local bp = setup_bp() + + -- add entities + local ws = db_insert(bp, "workspaces", { name = "ws-001" }) + local service = db_insert(bp, "services", { name = "service-001", }) + + for i = 1, 100 do + local route = db_insert(bp, "routes", { + name = "route-001", + paths = { "/mock" }, + -- unmatched service + service = { id = service.id }, + }) + end + + local deltas = declarative.export_config_sync() + local ok, err = validate_deltas(deltas, false) + assert(ok, "validate should not fail: " .. tostring(err)) + end) + + it("100 routes -> 100 services: matched foreign keys", function() + local bp = setup_bp() + + -- add entities + local ws = db_insert(bp, "workspaces", { name = "ws-001" }) + + for i = 1, 100 do + local service = db_insert(bp, "services", { name = "service-001", }) + + local route = db_insert(bp, "routes", { + name = "route-001", + paths = { "/mock" }, + -- unmatched service + service = { id = service.id }, + }) + end + + local deltas = declarative.export_config_sync() + local ok, err = validate_deltas(deltas, false) + assert(ok, "validate should not fail: " .. tostring(err)) + end) + + it("100 routes: unmatched foreign service", function() + local bp = setup_bp() + + -- add entities + local ws = db_insert(bp, "workspaces", { name = "ws-001" }) + + for i = 1, 100 do + local route = db_insert(bp, "routes", { + name = "route-001", + paths = { "/mock" }, + -- unmatched service + service = { id = "00000000-0000-0000-0000-000000000000" }, + }) + end + + local deltas = declarative.export_config_sync() + local ok, err = validate_deltas(deltas, false) + for i = 1, 100 do + assert.matches( + "entry " .. i .. " of 'services': " .. + "could not find routes's foreign refrences services", + err) + end + end) +end) diff --git a/spec/fixtures/dc_blueprints.lua b/spec/fixtures/dc_blueprints.lua index 139bd9c5141..0e617e47017 100644 --- a/spec/fixtures/dc_blueprints.lua +++ b/spec/fixtures/dc_blueprints.lua @@ -28,7 +28,7 @@ local function remove_nulls(tbl) end -local function wrap_db(db) +local function wrap_db(db, expand_foreigns) local dc_as_db = {} local config = new_config() @@ -43,7 +43,7 @@ local function wrap_db(db) local schema = db.daos[name].schema tbl = schema:process_auto_fields(tbl, "insert") for fname, field in schema:each_field() do - if field.type == "foreign" then + if not expand_foreigns and field.type == "foreign" then tbl[fname] = type(tbl[fname]) == "table" and tbl[fname].id or nil @@ -110,8 +110,8 @@ local function wrap_db(db) end -function dc_blueprints.new(db) - local dc_as_db = wrap_db(db) +function dc_blueprints.new(db, expand_foreigns) + local dc_as_db = wrap_db(db, expand_foreigns) local save_dc = new_config() diff --git a/spec/internal/db.lua b/spec/internal/db.lua index 5659cdf72ef..6403835092f 100644 --- a/spec/internal/db.lua +++ b/spec/internal/db.lua @@ -277,7 +277,7 @@ end -- route = { id = route1.id }, -- config = {}, -- } -local function get_db_utils(strategy, tables, plugins, vaults, skip_migrations) +local function get_db_utils(strategy, tables, plugins, vaults, skip_migrations, expand_foreigns) strategy = strategy or conf.database conf.database = strategy -- overwrite kong.configuration.database @@ -343,7 +343,7 @@ local function get_db_utils(strategy, tables, plugins, vaults, skip_migrations) bp = assert(Blueprints.new(db)) dcbp = nil else - bp = assert(dc_blueprints.new(db)) + bp = assert(dc_blueprints.new(db, expand_foreigns)) dcbp = bp end From 7aafdd9e5c223dcabc50cfc579af17a52aeffd50 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 13 Jan 2025 15:45:14 +0800 Subject: [PATCH 03/42] localize table.insert --- kong/clustering/services/sync/validate.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kong/clustering/services/sync/validate.lua b/kong/clustering/services/sync/validate.lua index 312f1301525..1f11df97c2d 100644 --- a/kong/clustering/services/sync/validate.lua +++ b/kong/clustering/services/sync/validate.lua @@ -1,6 +1,7 @@ local declarative = require("kong.db.declarative") local declarative_config = require("kong.db.schema.others.declarative_config") +local tb_insert = table.insert local validate = declarative_config.validate local pk_string = declarative_config.pk_string local validate_references_full = declarative_config.validate_references_full @@ -23,7 +24,7 @@ local function validate_deltas(deltas, is_full_sync) if delta_entity ~= nil and delta_entity ~= ngx.null then dc_table[delta_type] = dc_table[delta_type] or {} - table.insert(dc_table[delta_type], delta_entity) + tb_insert(dc_table[delta_type], delta_entity) -- table: primary key string -> entity if not is_full_sync then From 5a4aa84bee708f9a445d38b6653857b7bb1f7f4f Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 13 Jan 2025 15:46:19 +0800 Subject: [PATCH 04/42] localize kong.db --- kong/clustering/services/sync/validate.lua | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kong/clustering/services/sync/validate.lua b/kong/clustering/services/sync/validate.lua index 1f11df97c2d..18a6bcfcc1d 100644 --- a/kong/clustering/services/sync/validate.lua +++ b/kong/clustering/services/sync/validate.lua @@ -17,6 +17,8 @@ local function validate_deltas(deltas, is_full_sync) -- generate declarative config table local dc_table = { _format_version = "3.0", } + local db = kong.db + for _, delta in ipairs(deltas) do local delta_type = delta.type local delta_entity = delta.entity @@ -28,7 +30,7 @@ local function validate_deltas(deltas, is_full_sync) -- table: primary key string -> entity if not is_full_sync then - local schema = kong.db[delta_type].schema + local schema = db[delta_type].schema local pk = schema:extract_pk_values(delta_entity) local pks = pk_string(schema, pk) @@ -38,7 +40,7 @@ local function validate_deltas(deltas, is_full_sync) end -- validate schema - local dc_schema = kong.db.declarative_config.schema + local dc_schema = db.declarative_config.schema local ok, err_t = validate(dc_schema, dc_table) if not ok then From 1b5011b6f1688d52e7933a6afe977aa179df75ca Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 13 Jan 2025 15:48:03 +0800 Subject: [PATCH 05/42] ngx.null -> null --- kong/db/schema/others/declarative_config.lua | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index ff9f23571c3..b09ca025210 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -332,7 +332,7 @@ end local function ws_id_for(item) - if item.ws_id == nil or item.ws_id == ngx.null then + if item.ws_id == nil or item.ws_id == null then return "*" end return item.ws_id @@ -414,7 +414,7 @@ local function populate_references(input, known_entities, by_id, by_key, expecte local key = use_key and item[endpoint_key] local failed = false - if key and key ~= ngx.null then + if key and key ~= null then local ok = add_to_by_key(by_key, entity_schema, item, entity, key) if not ok then add_error(errs, parent_entity, parent_idx, entity, i, @@ -522,7 +522,7 @@ function DeclarativeConfig.validate_references_sync(deltas, deltas_map) local foreign_refs = foreign_references[item_type] - if not item or item == ngx.null or not foreign_refs then + if not item or item == null or not foreign_refs then goto continue end @@ -936,7 +936,7 @@ local function flatten(self, input) if field.unique then local flat_value = flat_entry[name] - if flat_value and flat_value ~= ngx.null then + if flat_value and flat_value ~= null then local unique_key = get_unique_key(schema, entry, field, flat_value) uniques[name] = uniques[name] or {} if uniques[name][unique_key] then From 1a63d10fea25fc139dde1fd3af2e4890ed9dbe7e Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 13 Jan 2025 15:58:09 +0800 Subject: [PATCH 06/42] fix lint error of test case --- .../18-hybrid_rpc/06-validate_deltas_spec.lua | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua index 8529c75c9b5..21c28fd42c3 100644 --- a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua @@ -1,5 +1,4 @@ local helpers = require "spec.helpers" -local pl_file = require "pl.file" local txn = require "resty.lmdb.transaction" local declarative = require "kong.db.declarative" @@ -47,7 +46,7 @@ local function setup_bp() lmdb_drop() -- init bp / db ( true for expand_foreigns) - bp, db = helpers.get_db_utils("off", nil, nil, nil, nil, true) + local bp, db = helpers.get_db_utils("off", nil, nil, nil, nil, true) -- init workspaces local workspaces = require "kong.workspaces" @@ -68,9 +67,9 @@ describe("[delta validations]",function() local bp = setup_bp() -- add entities - local ws = db_insert(bp, "workspaces", { name = "ws-001" }) + db_insert(bp, "workspaces", { name = "ws-001" }) local service = db_insert(bp, "services", { name = "service-001", }) - local route = db_insert(bp, "routes", { + db_insert(bp, "routes", { name = "route-001", paths = { "/mock" }, service = { id = service.id }, @@ -79,8 +78,6 @@ describe("[delta validations]",function() local deltas = declarative.export_config_sync() for _, delta in ipairs(deltas) do - local delta_entity = delta.entity - local delta_type = delta.type local ws_id = delta.ws_id assert(ws_id) end @@ -90,9 +87,9 @@ describe("[delta validations]",function() local bp = setup_bp() -- add entities - local ws = db_insert(bp, "workspaces", { name = "ws-001" }) + db_insert(bp, "workspaces", { name = "ws-001" }) local service = db_insert(bp, "services", { name = "service-001", }) - local route = db_insert(bp, "routes", { + db_insert(bp, "routes", { name = "route-001", paths = { "/mock" }, service = { id = service.id }, @@ -108,9 +105,8 @@ describe("[delta validations]",function() local bp = setup_bp() -- add entities - local ws = db_insert(bp, "workspaces", { name = "ws-001" }) - local service = db_insert(bp, "services", { name = "service-001", }) - local route = db_insert(bp, "routes", { + db_insert(bp, "workspaces", { name = "ws-001" }) + db_insert(bp, "routes", { name = "route-001", paths = { "/mock" }, -- unmatched service @@ -118,17 +114,21 @@ describe("[delta validations]",function() }) local deltas = declarative.export_config_sync() + local _, err = validate_deltas(deltas, false) + assert.matches( + "entry 1 of 'services': could not find routes's foreign refrences services", + err) end) it("100 routes -> 1 services: matched foreign keys", function() local bp = setup_bp() -- add entities - local ws = db_insert(bp, "workspaces", { name = "ws-001" }) + db_insert(bp, "workspaces", { name = "ws-001" }) local service = db_insert(bp, "services", { name = "service-001", }) for i = 1, 100 do - local route = db_insert(bp, "routes", { + db_insert(bp, "routes", { name = "route-001", paths = { "/mock" }, -- unmatched service @@ -145,12 +145,12 @@ describe("[delta validations]",function() local bp = setup_bp() -- add entities - local ws = db_insert(bp, "workspaces", { name = "ws-001" }) + db_insert(bp, "workspaces", { name = "ws-001" }) for i = 1, 100 do local service = db_insert(bp, "services", { name = "service-001", }) - local route = db_insert(bp, "routes", { + db_insert(bp, "routes", { name = "route-001", paths = { "/mock" }, -- unmatched service @@ -167,10 +167,10 @@ describe("[delta validations]",function() local bp = setup_bp() -- add entities - local ws = db_insert(bp, "workspaces", { name = "ws-001" }) + db_insert(bp, "workspaces", { name = "ws-001" }) for i = 1, 100 do - local route = db_insert(bp, "routes", { + db_insert(bp, "routes", { name = "route-001", paths = { "/mock" }, -- unmatched service @@ -179,7 +179,7 @@ describe("[delta validations]",function() end local deltas = declarative.export_config_sync() - local ok, err = validate_deltas(deltas, false) + local _, err = validate_deltas(deltas, false) for i = 1, 100 do assert.matches( "entry " .. i .. " of 'services': " .. From d31a2b69feaee21fbc92112393c2cc51a8902a38 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 13 Jan 2025 16:04:30 +0800 Subject: [PATCH 07/42] fix typo: generate --- kong/clustering/services/sync/validate.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/clustering/services/sync/validate.lua b/kong/clustering/services/sync/validate.lua index 18a6bcfcc1d..59c2f628e3e 100644 --- a/kong/clustering/services/sync/validate.lua +++ b/kong/clustering/services/sync/validate.lua @@ -11,7 +11,7 @@ local pretty_print_error = declarative.pretty_print_error local function validate_deltas(deltas, is_full_sync) - -- genearte deltas table mapping primary key string to entity item + -- generate deltas table mapping primary key string to entity item local deltas_map = {} -- generate declarative config table From a19f5bcabc64ff09a4f9dcd80e96c942f7edf7dc Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 13 Jan 2025 16:34:48 +0800 Subject: [PATCH 08/42] coding style --- kong/clustering/services/sync/validate.lua | 3 ++- spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/kong/clustering/services/sync/validate.lua b/kong/clustering/services/sync/validate.lua index 59c2f628e3e..7a4cb21b305 100644 --- a/kong/clustering/services/sync/validate.lua +++ b/kong/clustering/services/sync/validate.lua @@ -1,6 +1,7 @@ local declarative = require("kong.db.declarative") local declarative_config = require("kong.db.schema.others.declarative_config") +local null = ngx.null local tb_insert = table.insert local validate = declarative_config.validate local pk_string = declarative_config.pk_string @@ -23,7 +24,7 @@ local function validate_deltas(deltas, is_full_sync) local delta_type = delta.type local delta_entity = delta.entity - if delta_entity ~= nil and delta_entity ~= ngx.null then + if delta_entity ~= nil and delta_entity ~= null then dc_table[delta_type] = dc_table[delta_type] or {} tb_insert(dc_table[delta_type], delta_entity) diff --git a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua index 21c28fd42c3..e3db266076b 100644 --- a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua @@ -2,6 +2,7 @@ local helpers = require "spec.helpers" local txn = require "resty.lmdb.transaction" local declarative = require "kong.db.declarative" + local insert_entity_for_txn = declarative.insert_entity_for_txn local validate_deltas = require("kong.clustering.services.sync.validate").validate_deltas From bf13811c327012f3a6a7aec6874f07fe8174d4e9 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 13 Jan 2025 17:16:12 +0800 Subject: [PATCH 09/42] add comment --- kong/db/schema/others/declarative_config.lua | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index b09ca025210..c853e11d044 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -528,6 +528,10 @@ function DeclarativeConfig.validate_references_sync(deltas, deltas_map) for k, v in pairs(item) do + -- Try to check if item's some foreign key exists in the deltas or LMDB. + -- For example, `item[k]` could be `["service"]`, we need + -- to find the referenced foreign service entity for this router entity. + local foreign_entity = foreign_refs[k] if foreign_entity and v ~= null then -- k is foreign key From 980029c303009a91a63c7faa52d553d56bf7df53 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 13 Jan 2025 17:45:19 +0800 Subject: [PATCH 10/42] add comment for expand_foreigns --- spec/internal/db.lua | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/internal/db.lua b/spec/internal/db.lua index 6403835092f..4dff46aa8d6 100644 --- a/spec/internal/db.lua +++ b/spec/internal/db.lua @@ -261,6 +261,10 @@ end -- custom plugins as loaded. -- @param vaults (optional) vault configuration to use. -- @param skip_migrations (optional) if true, migrations will not be run. +-- @param expand_foreigns (optional) If true, it will prevent converting foreign +-- keys from primary key value pairs to strings. For example, it will keep the +-- foreign key of the router entity as `service = { id = "" }` instead of +-- converting it to `service = ""`. -- @return BluePrint, DB -- @usage -- local PLUGIN_NAME = "my_fancy_plugin" From 089d82ed3a6af1a813c9a96fe2bbbb4a1ac91948 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 10:10:10 +0800 Subject: [PATCH 11/42] Update kong/db/schema/others/declarative_config.lua Co-authored-by: Chrono --- kong/db/schema/others/declarative_config.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index c853e11d044..df322b20f28 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -565,7 +565,7 @@ function DeclarativeConfig.validate_references_sync(deltas, deltas_map) end ::continue:: - end + end -- for _, delta in ipairs(deltas) if next(errs) then From 4526fd9f9629872120864915ec29833d101d61ae Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 10:11:20 +0800 Subject: [PATCH 12/42] Update kong/db/schema/others/declarative_config.lua Co-authored-by: Chrono --- kong/db/schema/others/declarative_config.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index df322b20f28..33d04cc23fb 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -562,7 +562,7 @@ function DeclarativeConfig.validate_references_sync(deltas, deltas_map) end ::found:: - end + end -- for k, v in pairs(item) ::continue:: end -- for _, delta in ipairs(deltas) From 8f8de422bf15f414b2edd734835f798c49834587 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 10:13:55 +0800 Subject: [PATCH 13/42] fix comments --- spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua index e3db266076b..d7558742d46 100644 --- a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua @@ -80,7 +80,7 @@ describe("[delta validations]",function() for _, delta in ipairs(deltas) do local ws_id = delta.ws_id - assert(ws_id) + assert(ws_id and ws_id ~= ngx.null) end end) @@ -99,7 +99,7 @@ describe("[delta validations]",function() local deltas = declarative.export_config_sync() local ok, err = validate_deltas(deltas) - assert(ok, "validate should not fail: " .. tostring(err)) + assert.is_true(ok, "validate should not fail: " .. tostring(err)) end) it("route has unmatched foreign service", function() From bdb40b8f388fb585b6ad0f0b63d60f218b11a3c6 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 10:14:28 +0800 Subject: [PATCH 14/42] select return fix --- kong/db/schema/others/declarative_config.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 33d04cc23fb..a983250a56f 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -546,7 +546,7 @@ function DeclarativeConfig.validate_references_sync(deltas, deltas_map) end -- try to find it in DB (LMDB) - fvalue, _ = db:select(v, { workspace = ws_id }) + fvalue = db:select(v, { workspace = ws_id }) -- could not find its foreign reference if not fvalue then From 36e470154a031970f2d4a518e072387f14be66f3 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 10:23:21 +0800 Subject: [PATCH 15/42] fixed tests: assert -> assert.is_true --- spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua index d7558742d46..6ca15fac062 100644 --- a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua @@ -139,7 +139,7 @@ describe("[delta validations]",function() local deltas = declarative.export_config_sync() local ok, err = validate_deltas(deltas, false) - assert(ok, "validate should not fail: " .. tostring(err)) + assert.is_true(ok, "validate should not fail: " .. tostring(err)) end) it("100 routes -> 100 services: matched foreign keys", function() @@ -161,7 +161,7 @@ describe("[delta validations]",function() local deltas = declarative.export_config_sync() local ok, err = validate_deltas(deltas, false) - assert(ok, "validate should not fail: " .. tostring(err)) + assert.is_true(ok, "validate should not fail: " .. tostring(err)) end) it("100 routes: unmatched foreign service", function() From ab513d033733d6d136784733e7f725a97bf3e584 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 14:20:15 +0800 Subject: [PATCH 16/42] improve loop logic --- kong/db/schema/others/declarative_config.lua | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index a983250a56f..b86b93ea1a7 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -536,19 +536,18 @@ function DeclarativeConfig.validate_references_sync(deltas, deltas_map) if foreign_entity and v ~= null then -- k is foreign key - local db = kong.db[foreign_entity] + local dao = kong.db[foreign_entity] -- try to find it in deltas - local pks = DeclarativeConfig.pk_string(db.schema, v) + local pks = DeclarativeConfig.pk_string(dao.schema, v) local fvalue = deltas_map[pks] - if fvalue then - goto found - end -- try to find it in DB (LMDB) - fvalue = db:select(v, { workspace = ws_id }) + if not fvalue then + fvalue = dao:select(v, { workspace = ws_id }) + end - -- could not find its foreign reference + -- record an error if not finding its foreign reference if not fvalue then errs[item_type] = errs[item_type] or {} errs[item_type][foreign_entity] = errs[item_type][foreign_entity] or {} @@ -561,8 +560,7 @@ function DeclarativeConfig.validate_references_sync(deltas, deltas_map) end end - ::found:: - end -- for k, v in pairs(item) + end -- for k, v in pairs(item) ::continue:: end -- for _, delta in ipairs(deltas) From e0db8108ae6fa8acf3289eef13720e16a98a0eb0 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 15:11:35 +0800 Subject: [PATCH 17/42] improve perf of test cases --- .../18-hybrid_rpc/06-validate_deltas_spec.lua | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua index 6ca15fac062..437b697cced 100644 --- a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua @@ -42,6 +42,11 @@ local function db_insert(bp, name, entity) end +-- Cache the declarative_config to avoid the overhead of repeatedly executing +-- the time-consuming chain: +-- declarative.new_config -> declarative_config.load -> load_plugin_subschemas +local cached_dc + local function setup_bp() -- reset lmdb lmdb_drop() @@ -54,9 +59,13 @@ local function setup_bp() workspaces.upsert_default(db) -- init declarative config - local dc, err = declarative.new_config(kong.configuration) - assert(dc, err) - kong.db.declarative_config = dc + if not cached_dc then + local err + cached_dc, err = declarative.new_config(kong.configuration) + assert(cached_dc, err) + end + + kong.db.declarative_config = cached_dc return bp, db end From e3494c02c0bf25af7c91cb4fc0b25ad20cd78c19 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 16:08:19 +0800 Subject: [PATCH 18/42] generaete full schema of declarative config for sync --- kong/db/declarative/init.lua | 5 +++-- kong/db/schema/others/declarative_config.lua | 4 ++++ kong/init.lua | 5 ++++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/kong/db/declarative/init.lua b/kong/db/declarative/init.lua index c58f40eb088..1a8dc0694ad 100644 --- a/kong/db/declarative/init.lua +++ b/kong/db/declarative/init.lua @@ -33,8 +33,8 @@ local _MT = { __index = _M, } -- @tparam boolean partial Input is not a full representation -- of the database (e.g. for db_import) -- @treturn table A Config schema adjusted for this configuration -function _M.new_config(kong_config, partial) - local schema, err = declarative_config.load(kong_config.loaded_plugins, kong_config.loaded_vaults) +function _M.new_config(kong_config, partial, include_foreign) + local schema, err = declarative_config.load(kong_config.loaded_plugins, kong_config.loaded_vaults, include_foreign) if not schema then return nil, err end @@ -42,6 +42,7 @@ function _M.new_config(kong_config, partial) local self = { schema = schema, partial = partial, + include_foreign = include_foreign, } setmetatable(self, _MT) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index b86b93ea1a7..53d5e13ece6 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -854,6 +854,10 @@ function DeclarativeConfig.validate(self, input) if not ok then yield() + if self.include_foreign then + return nil, err + end + -- the error may be due entity validation that depends on foreign entity, -- and that is the reason why we try to validate the input again with the -- filled foreign keys diff --git a/kong/init.lua b/kong/init.lua index 8c4d4514221..4f49edb3215 100644 --- a/kong/init.lua +++ b/kong/init.lua @@ -712,7 +712,10 @@ function Kong.init() end if is_dbless(config) then - local dc, err = declarative.new_config(config) + -- If sync is enabled, we need to genearte full schema (including complete + -- foreign entity) for declarative config. Therefore, sync deltas validation + -- could work as expected to check full schema entities. + local dc, err = declarative.new_config(config, nil, not not kong.sync) if not dc then error(err) end From 1bca8c86acbb4dcb99efb0d93c590db32a3a46f9 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 16:12:03 +0800 Subject: [PATCH 19/42] add blank to fix coding style --- kong/clustering/services/sync/validate.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/kong/clustering/services/sync/validate.lua b/kong/clustering/services/sync/validate.lua index 7a4cb21b305..5a26be9f6aa 100644 --- a/kong/clustering/services/sync/validate.lua +++ b/kong/clustering/services/sync/validate.lua @@ -1,6 +1,7 @@ local declarative = require("kong.db.declarative") local declarative_config = require("kong.db.schema.others.declarative_config") + local null = ngx.null local tb_insert = table.insert local validate = declarative_config.validate From f77ca9b24124d7a5e8b6b7e562184e6383948361 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 17:13:51 +0800 Subject: [PATCH 20/42] simplify logic: we dont validate full sync, all run vallidate sync --- kong/clustering/services/sync/validate.lua | 24 ++++++---------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/kong/clustering/services/sync/validate.lua b/kong/clustering/services/sync/validate.lua index 5a26be9f6aa..29b2d96206f 100644 --- a/kong/clustering/services/sync/validate.lua +++ b/kong/clustering/services/sync/validate.lua @@ -6,7 +6,6 @@ local null = ngx.null local tb_insert = table.insert local validate = declarative_config.validate local pk_string = declarative_config.pk_string -local validate_references_full = declarative_config.validate_references_full local validate_references_sync = declarative_config.validate_references_sync local pretty_print_error = declarative.pretty_print_error @@ -31,13 +30,11 @@ local function validate_deltas(deltas, is_full_sync) tb_insert(dc_table[delta_type], delta_entity) -- table: primary key string -> entity - if not is_full_sync then - local schema = db[delta_type].schema - local pk = schema:extract_pk_values(delta_entity) - local pks = pk_string(schema, pk) + local schema = db[delta_type].schema + local pk = schema:extract_pk_values(delta_entity) + local pks = pk_string(schema, pk) - deltas_map[pks] = delta_entity - end + deltas_map[pks] = delta_entity end end @@ -49,17 +46,8 @@ local function validate_deltas(deltas, is_full_sync) return nil, pretty_print_error(err_t) end - -- validate references for full sync - if is_full_sync then - local ok, err_t = validate_references_full(dc_schema, dc_table) - if not ok then - return nil, pretty_print_error(err_t) - end - return true - end - - -- validate references for non full sync - local ok, err_t = validate_references_sync(deltas, deltas_map) + -- validate references + local ok, err_t = validate_references_sync(deltas, deltas_map, is_full_sync) if not ok then return nil, pretty_print_error(err_t) end From 9fec3e64d7342e468297ba74bc5fb276b80e2ebc Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 17:15:11 +0800 Subject: [PATCH 21/42] Revert "generaete full schema of declarative config for sync" This reverts commit 432931405704d2afe000b66c2eae8270056ef411. --- kong/db/declarative/init.lua | 5 ++--- kong/db/schema/others/declarative_config.lua | 4 ---- kong/init.lua | 5 +---- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/kong/db/declarative/init.lua b/kong/db/declarative/init.lua index 1a8dc0694ad..c58f40eb088 100644 --- a/kong/db/declarative/init.lua +++ b/kong/db/declarative/init.lua @@ -33,8 +33,8 @@ local _MT = { __index = _M, } -- @tparam boolean partial Input is not a full representation -- of the database (e.g. for db_import) -- @treturn table A Config schema adjusted for this configuration -function _M.new_config(kong_config, partial, include_foreign) - local schema, err = declarative_config.load(kong_config.loaded_plugins, kong_config.loaded_vaults, include_foreign) +function _M.new_config(kong_config, partial) + local schema, err = declarative_config.load(kong_config.loaded_plugins, kong_config.loaded_vaults) if not schema then return nil, err end @@ -42,7 +42,6 @@ function _M.new_config(kong_config, partial, include_foreign) local self = { schema = schema, partial = partial, - include_foreign = include_foreign, } setmetatable(self, _MT) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 53d5e13ece6..b86b93ea1a7 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -854,10 +854,6 @@ function DeclarativeConfig.validate(self, input) if not ok then yield() - if self.include_foreign then - return nil, err - end - -- the error may be due entity validation that depends on foreign entity, -- and that is the reason why we try to validate the input again with the -- filled foreign keys diff --git a/kong/init.lua b/kong/init.lua index 4f49edb3215..8c4d4514221 100644 --- a/kong/init.lua +++ b/kong/init.lua @@ -712,10 +712,7 @@ function Kong.init() end if is_dbless(config) then - -- If sync is enabled, we need to genearte full schema (including complete - -- foreign entity) for declarative config. Therefore, sync deltas validation - -- could work as expected to check full schema entities. - local dc, err = declarative.new_config(config, nil, not not kong.sync) + local dc, err = declarative.new_config(config) if not dc then error(err) end From 134e2c6aa5fa7617b12a29e3995236c10ae5b751 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 17:18:35 +0800 Subject: [PATCH 22/42] 1 --- kong/db/schema/others/declarative_config.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index b86b93ea1a7..4327aa72e4e 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -558,7 +558,7 @@ function DeclarativeConfig.validate_references_sync(deltas, deltas_map) insert(errs[item_type][foreign_entity], msg) end - end + end -- if foreign_entity and v ~= null end -- for k, v in pairs(item) From 88992ffda7b380e0f2b08713f018b0a22c5ffec6 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 17:34:29 +0800 Subject: [PATCH 23/42] fix 09-node-id-persistence_spec.lua --- .../09-hybrid_mode/09-node-id-persistence_spec.lua | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua b/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua index 7b358f62920..14859f74103 100644 --- a/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua +++ b/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua @@ -20,6 +20,12 @@ local write_node_id = [[ local function get_http_node_id() local client = helpers.proxy_client(nil, 9002) finally(function() client:close() end) + + -- wait until 15s: Its original value is 5s. Since we added delta validation + -- logic, the foreign entity in the deltas items is complete, like + -- { id = "uuid" }. If it uses kong.db.declarative_config to validate + -- deltas, it will fail. Then, declarative_config will load a full schema to + -- re-validate, which consumes a large amount of extra time. helpers.wait_until(function() local res = client:get("/request", { headers = { host = "http.node-id.test" }, @@ -29,7 +35,7 @@ local function get_http_node_id() res:read_body() end return res and res.status == 200 - end, 5, 0.5) + end, 15, 0.5) helpers.wait_for_file("file", PREFIX .. "/kong.id.http") return helpers.file.read(PREFIX .. "/kong.id.http") From bf38ec11fcdecc1bbcc1c70b0ec56b640cb3ac06 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 17:41:05 +0800 Subject: [PATCH 24/42] rename: declarative_config.validate -> declarative_config.validate_schema --- kong/clustering/services/sync/validate.lua | 6 +++--- kong/db/schema/others/declarative_config.lua | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/kong/clustering/services/sync/validate.lua b/kong/clustering/services/sync/validate.lua index 29b2d96206f..1bdc57528b6 100644 --- a/kong/clustering/services/sync/validate.lua +++ b/kong/clustering/services/sync/validate.lua @@ -4,7 +4,7 @@ local declarative_config = require("kong.db.schema.others.declarative_config") local null = ngx.null local tb_insert = table.insert -local validate = declarative_config.validate +local validate_schema = declarative_config.validate_schema local pk_string = declarative_config.pk_string local validate_references_sync = declarative_config.validate_references_sync local pretty_print_error = declarative.pretty_print_error @@ -38,10 +38,10 @@ local function validate_deltas(deltas, is_full_sync) end end - -- validate schema + -- validate schema (same logic as the sync v1 full-sync schema validation) local dc_schema = db.declarative_config.schema - local ok, err_t = validate(dc_schema, dc_table) + local ok, err_t = validate_schema(dc_schema, dc_table) if not ok then return nil, pretty_print_error(err_t) end diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 4327aa72e4e..a58d1c27abe 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -849,7 +849,7 @@ local function get_unique_key(schema, entity, field, value) end -function DeclarativeConfig.validate(self, input) +function DeclarativeConfig.validate_schema(self, input) local ok, err = self:validate(input) if not ok then yield() @@ -885,7 +885,7 @@ local function flatten(self, input) input._transform = true end - local ok, err = DeclarativeConfig.validate(self, input) + local ok, err = DeclarativeConfig.validate_schema(self, input) if not ok then return nil, err end From c2c58d527f9a8ad22f41921a514dd1befd2bdd2d Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 17:42:56 +0800 Subject: [PATCH 25/42] remove DeclarativeConfig.validate_references_full --- kong/db/schema/others/declarative_config.lua | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index a58d1c27abe..953e38dfb82 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -507,12 +507,7 @@ local function validate_references(self, input) end -function DeclarativeConfig.validate_references_full(self, input) - return validate_references(self, input) -end - - -function DeclarativeConfig.validate_references_sync(deltas, deltas_map) +function DeclarativeConfig.validate_references_sync(deltas, deltas_map, is_full_sync) local errs = {} for _, delta in ipairs(deltas) do @@ -543,7 +538,7 @@ function DeclarativeConfig.validate_references_sync(deltas, deltas_map) local fvalue = deltas_map[pks] -- try to find it in DB (LMDB) - if not fvalue then + if not fvalue and not is_full_sync then fvalue = dao:select(v, { workspace = ws_id }) end From ec2e2414a650f2f13609648a0886c7b44f5ece20 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 14 Jan 2025 17:44:31 +0800 Subject: [PATCH 26/42] fix title of 09-node-id-persistence_spec.lua --- .../09-hybrid_mode/09-node-id-persistence_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua b/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua index 14859f74103..1bdd522ab52 100644 --- a/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua +++ b/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua @@ -93,7 +93,7 @@ for _, v in ipairs({ {"off", "off"}, {"on", "off"}, {"on", "on"}, }) do local rpc, rpc_sync = v[1], v[2] for _, strategy in helpers.each_strategy() do - describe("node id persistence " .. " rpc_sync=" .. rpc_sync, function() + describe("node id persistence rpc_sync = " .. rpc_sync, function() local control_plane_config = { role = "control_plane", From 61594644eda7ee70c798010f8cfdfb3a18c4cfd3 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Wed, 15 Jan 2025 17:25:52 +0800 Subject: [PATCH 27/42] fix test case 30 --- .../09-hybrid_mode/09-node-id-persistence_spec.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua b/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua index 1bdd522ab52..5cde45a7cff 100644 --- a/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua +++ b/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua @@ -21,7 +21,7 @@ local function get_http_node_id() local client = helpers.proxy_client(nil, 9002) finally(function() client:close() end) - -- wait until 15s: Its original value is 5s. Since we added delta validation + -- wait until 30s: Its original value is 5s. Since we added delta validation -- logic, the foreign entity in the deltas items is complete, like -- { id = "uuid" }. If it uses kong.db.declarative_config to validate -- deltas, it will fail. Then, declarative_config will load a full schema to @@ -35,7 +35,7 @@ local function get_http_node_id() res:read_body() end return res and res.status == 200 - end, 15, 0.5) + end, 30, 0.5) helpers.wait_for_file("file", PREFIX .. "/kong.id.http") return helpers.file.read(PREFIX .. "/kong.id.http") From f9de4ebf760a80c5259a7ea7b927f3722b4bc5d0 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Wed, 15 Jan 2025 19:20:36 +0800 Subject: [PATCH 28/42] add wait time --- .../09-hybrid_mode/09-node-id-persistence_spec.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua b/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua index 5cde45a7cff..fa345bda089 100644 --- a/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua +++ b/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua @@ -21,7 +21,7 @@ local function get_http_node_id() local client = helpers.proxy_client(nil, 9002) finally(function() client:close() end) - -- wait until 30s: Its original value is 5s. Since we added delta validation + -- wait until 45s: Its original value is 5s. Since we added delta validation -- logic, the foreign entity in the deltas items is complete, like -- { id = "uuid" }. If it uses kong.db.declarative_config to validate -- deltas, it will fail. Then, declarative_config will load a full schema to @@ -35,7 +35,7 @@ local function get_http_node_id() res:read_body() end return res and res.status == 200 - end, 30, 0.5) + end, 45, 0.5) helpers.wait_for_file("file", PREFIX .. "/kong.id.http") return helpers.file.read(PREFIX .. "/kong.id.http") From f05309d55c22673eee6b99eff6e4054d79e068a0 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Thu, 16 Jan 2025 14:54:35 +0800 Subject: [PATCH 29/42] delcarative_config.load() supports sync.v2 --- kong/db/declarative/init.lua | 8 ++++-- kong/db/schema/others/declarative_config.lua | 28 +++++++++++++++++-- kong/init.lua | 2 +- .../09-node-id-persistence_spec.lua | 7 +---- .../18-hybrid_rpc/06-validate_deltas_spec.lua | 2 +- 5 files changed, 35 insertions(+), 12 deletions(-) diff --git a/kong/db/declarative/init.lua b/kong/db/declarative/init.lua index c58f40eb088..09202266140 100644 --- a/kong/db/declarative/init.lua +++ b/kong/db/declarative/init.lua @@ -32,9 +32,13 @@ local _MT = { __index = _M, } -- @tparam table kong_config The Kong configuration table -- @tparam boolean partial Input is not a full representation -- of the database (e.g. for db_import) +-- @tparam is_sync It generates full schema and foreign references to validate +-- schema and references for sync.v2 -- @treturn table A Config schema adjusted for this configuration -function _M.new_config(kong_config, partial) - local schema, err = declarative_config.load(kong_config.loaded_plugins, kong_config.loaded_vaults) +function _M.new_config(kong_config, partial, include_foreign, is_sync) + local schema, err = declarative_config.load(kong_config.loaded_plugins, + kong_config.loaded_vaults, + include_foreign, is_sync) if not schema then return nil, err end diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 953e38dfb82..5c26d256cf6 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -225,6 +225,25 @@ local function nest_foreign_relationships(known_entities, records, include_forei end +-- It generates foreign_references to be used by sync v2's reference validation +-- and does not clear fdata's reference in records. +local function reference_foreign_by_name_sync(known_entities, records) + for i = #known_entities, 1, -1 do + local entity = known_entities[i] + local record = records[entity] + for _, f in ipairs(record.fields) do + local fname, fdata = next(f) + if fdata.type == "foreign" then + if not foreign_references[entity] then + foreign_references[entity] = {} + end + foreign_references[entity][fname] = fdata.reference + end + end + end +end + + local function reference_foreign_by_name(known_entities, records) for i = #known_entities, 1, -1 do local entity = known_entities[i] @@ -847,6 +866,10 @@ end function DeclarativeConfig.validate_schema(self, input) local ok, err = self:validate(input) if not ok then + if self.is_sync then + return nil, err + end + yield() -- the error may be due entity validation that depends on foreign entity, @@ -980,7 +1003,7 @@ local function load_entity_subschemas(entity_name, entity) end -function DeclarativeConfig.load(plugin_set, vault_set, include_foreign) +function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, is_sync) all_schemas = {} local schemas_array = {} for _, entity in ipairs(constants.CORE_ENTITIES) do @@ -1015,7 +1038,7 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign) known_entities[i] = schema.name end - local fields, records = build_fields(known_entities, include_foreign) + local fields, records = build_fields(known_entities, include_foreign or is_sync) -- assert(no_foreign(fields)) local ok, err = load_plugin_subschemas(fields, plugin_set) @@ -1048,6 +1071,7 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign) schema.insert_default_workspace_if_not_given = insert_default_workspace_if_not_given schema.plugin_set = plugin_set schema.vault_set = vault_set + schema.is_sync = is_sync return schema, nil, def end diff --git a/kong/init.lua b/kong/init.lua index 8c4d4514221..34dd9b3a435 100644 --- a/kong/init.lua +++ b/kong/init.lua @@ -712,7 +712,7 @@ function Kong.init() end if is_dbless(config) then - local dc, err = declarative.new_config(config) + local dc, err = declarative.new_config(config, nil, nil, not not kong.sync) if not dc then error(err) end diff --git a/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua b/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua index fa345bda089..5ef1f552d8c 100644 --- a/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua +++ b/spec/02-integration/09-hybrid_mode/09-node-id-persistence_spec.lua @@ -21,11 +21,6 @@ local function get_http_node_id() local client = helpers.proxy_client(nil, 9002) finally(function() client:close() end) - -- wait until 45s: Its original value is 5s. Since we added delta validation - -- logic, the foreign entity in the deltas items is complete, like - -- { id = "uuid" }. If it uses kong.db.declarative_config to validate - -- deltas, it will fail. Then, declarative_config will load a full schema to - -- re-validate, which consumes a large amount of extra time. helpers.wait_until(function() local res = client:get("/request", { headers = { host = "http.node-id.test" }, @@ -35,7 +30,7 @@ local function get_http_node_id() res:read_body() end return res and res.status == 200 - end, 45, 0.5) + end, 5, 0.5) helpers.wait_for_file("file", PREFIX .. "/kong.id.http") return helpers.file.read(PREFIX .. "/kong.id.http") diff --git a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua index 437b697cced..587c20a675b 100644 --- a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua @@ -61,7 +61,7 @@ local function setup_bp() -- init declarative config if not cached_dc then local err - cached_dc, err = declarative.new_config(kong.configuration) + cached_dc, err = declarative.new_config(kong.configuration, nil, nil, true) assert(cached_dc, err) end From 4d76b7c43cf36a1b03b0a54ce36320c1e90a095c Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Thu, 16 Jan 2025 15:24:04 +0800 Subject: [PATCH 30/42] delcarative_config.load() supports sync.v2 (part 2) --- kong/db/schema/others/declarative_config.lua | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 5c26d256cf6..ef17812dce9 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -1051,10 +1051,19 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, is_sync) return nil, err end - -- we replace the "foreign"-type fields at the top-level - -- with "string"-type fields only after the subschemas have been loaded, - -- otherwise they will detect the mismatch. - if not include_foreign then + -- If sync.v2 enabled, it generates foreign_references and does not replace + -- replace the "foreign"-type fields with "string"-type fields. + -- + -- For sync.v2, + -- * The "foreign"-type fields are used to validate schema. + -- * The foreign_references are used to validate references. + if is_sync then + reference_foreign_by_name_sync(known_entities, records) + + elseif not include_foreign then + -- we replace the "foreign"-type fields at the top-level + -- with "string"-type fields only after the subschemas have been loaded, + -- otherwise they will detect the mismatch. reference_foreign_by_name(known_entities, records) end From d3c619bb54eca92e1f044e66da0f946fba497b30 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Thu, 16 Jan 2025 17:05:44 +0800 Subject: [PATCH 31/42] delcarative_config.load() supports sync.v2 (full schema preload) --- kong/db/schema/others/declarative_config.lua | 55 ++++++++------------ 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index ef17812dce9..216841362ee 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -225,25 +225,6 @@ local function nest_foreign_relationships(known_entities, records, include_forei end --- It generates foreign_references to be used by sync v2's reference validation --- and does not clear fdata's reference in records. -local function reference_foreign_by_name_sync(known_entities, records) - for i = #known_entities, 1, -1 do - local entity = known_entities[i] - local record = records[entity] - for _, f in ipairs(record.fields) do - local fname, fdata = next(f) - if fdata.type == "foreign" then - if not foreign_references[entity] then - foreign_references[entity] = {} - end - foreign_references[entity][fname] = fdata.reference - end - end - end -end - - local function reference_foreign_by_name(known_entities, records) for i = #known_entities, 1, -1 do local entity = known_entities[i] @@ -866,10 +847,6 @@ end function DeclarativeConfig.validate_schema(self, input) local ok, err = self:validate(input) if not ok then - if self.is_sync then - return nil, err - end - yield() -- the error may be due entity validation that depends on foreign entity, @@ -1051,16 +1028,27 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, is_sync) return nil, err end - -- If sync.v2 enabled, it generates foreign_references and does not replace - -- replace the "foreign"-type fields with "string"-type fields. - -- - -- For sync.v2, - -- * The "foreign"-type fields are used to validate schema. - -- * The foreign_references are used to validate references. + -- Pre-load full schema to validate schema for sync.v2: + local full_schema + if is_sync then - reference_foreign_by_name_sync(known_entities, records) + local def = { + name = "declarative_config", + primary_key = {}, + -- copy fields to avoid its "foreign"-type fields from being cleared by + -- reference_foreign_by_name() + fields = kong_table.cycle_aware_deep_copy(fields, true), + } + full_schema = Schema.new(def) + + full_schema.known_entities = known_entities + full_schema.flatten = flatten + full_schema.insert_default_workspace_if_not_given = insert_default_workspace_if_not_given + full_schema.plugin_set = plugin_set + full_schema.vault_set = vault_set + end - elseif not include_foreign then + if not include_foreign then -- we replace the "foreign"-type fields at the top-level -- with "string"-type fields only after the subschemas have been loaded, -- otherwise they will detect the mismatch. @@ -1080,7 +1068,10 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, is_sync) schema.insert_default_workspace_if_not_given = insert_default_workspace_if_not_given schema.plugin_set = plugin_set schema.vault_set = vault_set - schema.is_sync = is_sync + + if is_sync then + schema.full_schema = full_schema + end return schema, nil, def end From 5d56bb0e372af3be8fad1c00380d74999f93d9ee Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Thu, 16 Jan 2025 17:25:45 +0800 Subject: [PATCH 32/42] fix comment --- kong/db/schema/others/declarative_config.lua | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 216841362ee..5560d42cdef 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -1028,7 +1028,9 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, is_sync) return nil, err end - -- Pre-load full schema to validate schema for sync.v2: + -- Pre-load the full schema to validate the schema for sync.v2. Lazy loading + -- the full schema with DeclarativeConfig.load() will consume a lot of time + -- due to load_plugin_subschemas(). local full_schema if is_sync then From 8d3ed7dd146e394b3ddc9304b002139975b9c566 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Fri, 17 Jan 2025 10:30:36 +0800 Subject: [PATCH 33/42] add comment for wrap_db() expand_foreigns param --- spec/fixtures/dc_blueprints.lua | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/fixtures/dc_blueprints.lua b/spec/fixtures/dc_blueprints.lua index 0e617e47017..26301d8c0e0 100644 --- a/spec/fixtures/dc_blueprints.lua +++ b/spec/fixtures/dc_blueprints.lua @@ -28,6 +28,8 @@ local function remove_nulls(tbl) end +-- tparam boolean expand_foreigns expand the complete "foreign"-type fields, not +-- replacing it with "string"-type fields local function wrap_db(db, expand_foreigns) local dc_as_db = {} From d71d10ca81882f53f40137d2b12eed3e388e7b3b Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Fri, 17 Jan 2025 10:31:25 +0800 Subject: [PATCH 34/42] Update kong/db/schema/others/declarative_config.lua Co-authored-by: Chrono --- kong/db/schema/others/declarative_config.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 5560d42cdef..e984bf7e106 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -549,7 +549,7 @@ function DeclarativeConfig.validate_references_sync(deltas, deltas_map, is_full_ local msg = fmt("could not find %s's foreign refrences %s (%s)", item_type, foreign_entity, - (type(v) == "string" and v or cjson_encode(v))) + type(v) == "string" and v or cjson_encode(v)) insert(errs[item_type][foreign_entity], msg) end From ed677e2d832b3d602c4b57582bec78dae0b571dc Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Fri, 17 Jan 2025 10:39:13 +0800 Subject: [PATCH 35/42] comment --- kong/db/schema/others/declarative_config.lua | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index e984bf7e106..7b128017124 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -980,6 +980,8 @@ local function load_entity_subschemas(entity_name, entity) end +-- @tparam is_sync It generates full schema and foreign references to validate +-- schema and references for sync.v2 function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, is_sync) all_schemas = {} local schemas_array = {} From 64015e8f31fd9b777733a277af249c18a2dbe8a2 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Fri, 17 Jan 2025 10:40:40 +0800 Subject: [PATCH 36/42] coding style: not not sync -> sync ~= nil --- kong/init.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/init.lua b/kong/init.lua index 34dd9b3a435..09a054f4d9d 100644 --- a/kong/init.lua +++ b/kong/init.lua @@ -712,7 +712,7 @@ function Kong.init() end if is_dbless(config) then - local dc, err = declarative.new_config(config, nil, nil, not not kong.sync) + local dc, err = declarative.new_config(config, nil, nil, kong.sync ~= nil) if not dc then error(err) end From de9f3a546120874937b13532b88bbabe170b7b4f Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Fri, 17 Jan 2025 11:31:42 +0800 Subject: [PATCH 37/42] coding style --- kong/db/declarative/init.lua | 6 ++---- kong/db/schema/others/declarative_config.lua | 13 +++++++------ kong/init.lua | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/kong/db/declarative/init.lua b/kong/db/declarative/init.lua index 09202266140..293b2829f0a 100644 --- a/kong/db/declarative/init.lua +++ b/kong/db/declarative/init.lua @@ -32,13 +32,11 @@ local _MT = { __index = _M, } -- @tparam table kong_config The Kong configuration table -- @tparam boolean partial Input is not a full representation -- of the database (e.g. for db_import) --- @tparam is_sync It generates full schema and foreign references to validate --- schema and references for sync.v2 -- @treturn table A Config schema adjusted for this configuration -function _M.new_config(kong_config, partial, include_foreign, is_sync) +function _M.new_config(kong_config, partial, include_foreign) local schema, err = declarative_config.load(kong_config.loaded_plugins, kong_config.loaded_vaults, - include_foreign, is_sync) + include_foreign, kong.sync ~= nil) if not schema then return nil, err end diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 7b128017124..2f67a03bdda 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -980,9 +980,9 @@ local function load_entity_subschemas(entity_name, entity) end --- @tparam is_sync It generates full schema and foreign references to validate --- schema and references for sync.v2 -function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, is_sync) +-- @tparam sync_v2_enabled It generates full schema and foreign references to +-- validate schema and references for sync.v2 +function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, sync_v2_enabled) all_schemas = {} local schemas_array = {} for _, entity in ipairs(constants.CORE_ENTITIES) do @@ -1017,7 +1017,7 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, is_sync) known_entities[i] = schema.name end - local fields, records = build_fields(known_entities, include_foreign or is_sync) + local fields, records = build_fields(known_entities, include_foreign or sync_v2_enabled) -- assert(no_foreign(fields)) local ok, err = load_plugin_subschemas(fields, plugin_set) @@ -1035,7 +1035,7 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, is_sync) -- due to load_plugin_subschemas(). local full_schema - if is_sync then + if sync_v2_enabled then local def = { name = "declarative_config", primary_key = {}, @@ -1043,6 +1043,7 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, is_sync) -- reference_foreign_by_name() fields = kong_table.cycle_aware_deep_copy(fields, true), } + full_schema = Schema.new(def) full_schema.known_entities = known_entities @@ -1073,7 +1074,7 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, is_sync) schema.plugin_set = plugin_set schema.vault_set = vault_set - if is_sync then + if sync_v2_enabled then schema.full_schema = full_schema end diff --git a/kong/init.lua b/kong/init.lua index 09a054f4d9d..8c4d4514221 100644 --- a/kong/init.lua +++ b/kong/init.lua @@ -712,7 +712,7 @@ function Kong.init() end if is_dbless(config) then - local dc, err = declarative.new_config(config, nil, nil, kong.sync ~= nil) + local dc, err = declarative.new_config(config) if not dc then error(err) end From bfc35c343835b84f7bed41cbfaa2cd6ac9ccfebf Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Fri, 17 Jan 2025 14:22:58 +0800 Subject: [PATCH 38/42] fix 06-validate_deltas_spec.lua --- spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua index 587c20a675b..0653a775cf1 100644 --- a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua @@ -60,8 +60,9 @@ local function setup_bp() -- init declarative config if not cached_dc then + kong.sync = "fake sync to generate dc with sync_v2_enabled" local err - cached_dc, err = declarative.new_config(kong.configuration, nil, nil, true) + cached_dc, err = declarative.new_config(kong.configuration) assert(cached_dc, err) end From d20ebeb7dfda8378d383c013a14739b6f0b9f79d Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 20 Jan 2025 17:31:14 +0800 Subject: [PATCH 39/42] validate schema with kong.db.schema.validate() --- kong/clustering/services/sync/validate.lua | 34 +++++++++------- kong/db/declarative/init.lua | 4 +- kong/db/schema/others/declarative_config.lua | 39 +++---------------- .../18-hybrid_rpc/06-validate_deltas_spec.lua | 28 +++++++++++++ 4 files changed, 53 insertions(+), 52 deletions(-) diff --git a/kong/clustering/services/sync/validate.lua b/kong/clustering/services/sync/validate.lua index 1bdc57528b6..203509e9d90 100644 --- a/kong/clustering/services/sync/validate.lua +++ b/kong/clustering/services/sync/validate.lua @@ -3,8 +3,6 @@ local declarative_config = require("kong.db.schema.others.declarative_config") local null = ngx.null -local tb_insert = table.insert -local validate_schema = declarative_config.validate_schema local pk_string = declarative_config.pk_string local validate_references_sync = declarative_config.validate_references_sync local pretty_print_error = declarative.pretty_print_error @@ -12,12 +10,11 @@ local pretty_print_error = declarative.pretty_print_error local function validate_deltas(deltas, is_full_sync) + local errs = {} + -- generate deltas table mapping primary key string to entity item local deltas_map = {} - -- generate declarative config table - local dc_table = { _format_version = "3.0", } - local db = kong.db for _, delta in ipairs(deltas) do @@ -25,25 +22,32 @@ local function validate_deltas(deltas, is_full_sync) local delta_entity = delta.entity if delta_entity ~= nil and delta_entity ~= null then - dc_table[delta_type] = dc_table[delta_type] or {} - - tb_insert(dc_table[delta_type], delta_entity) - -- table: primary key string -> entity local schema = db[delta_type].schema local pk = schema:extract_pk_values(delta_entity) local pks = pk_string(schema, pk) deltas_map[pks] = delta_entity + + -- validate entity + local dao = kong.db[delta_type] + if dao then + local ws_id = delta_entity.ws_id -- bypass ws_id field for validation + delta_entity.ws_id = nil + + local ok, err_t = dao.schema:validate(delta_entity) + + delta_entity.ws_id = ws_id + + if not ok then + errs[#errs + 1] = { [delta_type] = err_t } + end + end end end - -- validate schema (same logic as the sync v1 full-sync schema validation) - local dc_schema = db.declarative_config.schema - - local ok, err_t = validate_schema(dc_schema, dc_table) - if not ok then - return nil, pretty_print_error(err_t) + if next(errs) then + return nil, pretty_print_error(errs, "deltas") end -- validate references diff --git a/kong/db/declarative/init.lua b/kong/db/declarative/init.lua index 293b2829f0a..1fb04ed2b1b 100644 --- a/kong/db/declarative/init.lua +++ b/kong/db/declarative/init.lua @@ -34,9 +34,7 @@ local _MT = { __index = _M, } -- of the database (e.g. for db_import) -- @treturn table A Config schema adjusted for this configuration function _M.new_config(kong_config, partial, include_foreign) - local schema, err = declarative_config.load(kong_config.loaded_plugins, - kong_config.loaded_vaults, - include_foreign, kong.sync ~= nil) + local schema, err = declarative_config.load(kong_config.loaded_plugins, kong_config.loaded_vaults, include_foreign) if not schema then return nil, err end diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 2f67a03bdda..31024b9264d 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -980,9 +980,7 @@ local function load_entity_subschemas(entity_name, entity) end --- @tparam sync_v2_enabled It generates full schema and foreign references to --- validate schema and references for sync.v2 -function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, sync_v2_enabled) +function DeclarativeConfig.load(plugin_set, vault_set, include_foreign) all_schemas = {} local schemas_array = {} for _, entity in ipairs(constants.CORE_ENTITIES) do @@ -1017,7 +1015,7 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, sync_v2_ known_entities[i] = schema.name end - local fields, records = build_fields(known_entities, include_foreign or sync_v2_enabled) + local fields, records = build_fields(known_entities, include_foreign) -- assert(no_foreign(fields)) local ok, err = load_plugin_subschemas(fields, plugin_set) @@ -1030,33 +1028,10 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, sync_v2_ return nil, err end - -- Pre-load the full schema to validate the schema for sync.v2. Lazy loading - -- the full schema with DeclarativeConfig.load() will consume a lot of time - -- due to load_plugin_subschemas(). - local full_schema - - if sync_v2_enabled then - local def = { - name = "declarative_config", - primary_key = {}, - -- copy fields to avoid its "foreign"-type fields from being cleared by - -- reference_foreign_by_name() - fields = kong_table.cycle_aware_deep_copy(fields, true), - } - - full_schema = Schema.new(def) - - full_schema.known_entities = known_entities - full_schema.flatten = flatten - full_schema.insert_default_workspace_if_not_given = insert_default_workspace_if_not_given - full_schema.plugin_set = plugin_set - full_schema.vault_set = vault_set - end - + -- we replace the "foreign"-type fields at the top-level + -- with "string"-type fields only after the subschemas have been loaded, + -- otherwise they will detect the mismatch. if not include_foreign then - -- we replace the "foreign"-type fields at the top-level - -- with "string"-type fields only after the subschemas have been loaded, - -- otherwise they will detect the mismatch. reference_foreign_by_name(known_entities, records) end @@ -1074,10 +1049,6 @@ function DeclarativeConfig.load(plugin_set, vault_set, include_foreign, sync_v2_ schema.plugin_set = plugin_set schema.vault_set = vault_set - if sync_v2_enabled then - schema.full_schema = full_schema - end - return schema, nil, def end diff --git a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua index 0653a775cf1..96e050989f4 100644 --- a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua @@ -94,6 +94,34 @@ describe("[delta validations]",function() end end) + it("route has unknown field", function() + local bp = setup_bp() + + -- add entities + db_insert(bp, "workspaces", { name = "ws-001" }) + local service = db_insert(bp, "services", { name = "service-001", }) + db_insert(bp, "routes", { + name = "route-001", + paths = { "/mock" }, + service = { id = service.id }, + }) + + local deltas = declarative.export_config_sync() + + for _, delta in ipairs(deltas) do + if delta.type == "routes" then + delta.entity.foo = "invalid_field_value" + break + end + end + + local _, err = validate_deltas(deltas) + assert.matches([[- in entry 1 of 'deltas': + in 'routes': + in 'foo': unknown field]], + err) + end) + it("route has foreign service", function() local bp = setup_bp() From ab9f6ddbec61455bfa9a5495badee307e31946ec Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 20 Jan 2025 18:17:40 +0800 Subject: [PATCH 40/42] remove unncessary logic --- kong/db/declarative/init.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kong/db/declarative/init.lua b/kong/db/declarative/init.lua index 1fb04ed2b1b..c58f40eb088 100644 --- a/kong/db/declarative/init.lua +++ b/kong/db/declarative/init.lua @@ -33,8 +33,8 @@ local _MT = { __index = _M, } -- @tparam boolean partial Input is not a full representation -- of the database (e.g. for db_import) -- @treturn table A Config schema adjusted for this configuration -function _M.new_config(kong_config, partial, include_foreign) - local schema, err = declarative_config.load(kong_config.loaded_plugins, kong_config.loaded_vaults, include_foreign) +function _M.new_config(kong_config, partial) + local schema, err = declarative_config.load(kong_config.loaded_plugins, kong_config.loaded_vaults) if not schema then return nil, err end From d51e1a018221225230d4907f2a91453197b22ec6 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 20 Jan 2025 18:20:11 +0800 Subject: [PATCH 41/42] remove unncessary logic --- kong/db/schema/others/declarative_config.lua | 28 +++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 31024b9264d..4ab298acfb8 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -844,7 +844,15 @@ local function get_unique_key(schema, entity, field, value) end -function DeclarativeConfig.validate_schema(self, input) +local function flatten(self, input) + -- manually set transform here + -- we can't do this in the schema with a `default` because validate + -- needs to happen before process_auto_fields, which + -- is the one in charge of filling out default values + if input._transform == nil then + input._transform = true + end + local ok, err = self:validate(input) if not ok then yield() @@ -867,24 +875,6 @@ function DeclarativeConfig.validate_schema(self, input) yield() end - return true -end - - -local function flatten(self, input) - -- manually set transform here - -- we can't do this in the schema with a `default` because validate - -- needs to happen before process_auto_fields, which - -- is the one in charge of filling out default values - if input._transform == nil then - input._transform = true - end - - local ok, err = DeclarativeConfig.validate_schema(self, input) - if not ok then - return nil, err - end - generate_ids(input, self.known_entities) yield() From d619ef15e1897cb66d6671422090b4365776222a Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 21 Jan 2025 10:47:07 +0800 Subject: [PATCH 42/42] add comment and remove unnecessary fake sync in test case --- kong/clustering/services/sync/validate.lua | 10 +++++++--- kong/db/schema/others/declarative_config.lua | 3 +++ .../18-hybrid_rpc/06-validate_deltas_spec.lua | 1 - 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/kong/clustering/services/sync/validate.lua b/kong/clustering/services/sync/validate.lua index 203509e9d90..8b5aa91ccf9 100644 --- a/kong/clustering/services/sync/validate.lua +++ b/kong/clustering/services/sync/validate.lua @@ -32,12 +32,16 @@ local function validate_deltas(deltas, is_full_sync) -- validate entity local dao = kong.db[delta_type] if dao then - local ws_id = delta_entity.ws_id -- bypass ws_id field for validation - delta_entity.ws_id = nil + -- CP will insert ws_id into the entity, which will be validated as an + -- unknown field. + -- TODO: On the CP side, remove ws_id from the entity and set it only + -- in the delta. + local ws_id = delta_entity.ws_id + delta_entity.ws_id = nil -- clear ws_id local ok, err_t = dao.schema:validate(delta_entity) - delta_entity.ws_id = ws_id + delta_entity.ws_id = ws_id -- restore ws_id if not ok then errs[#errs + 1] = { [delta_type] = err_t } diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index 4ab298acfb8..e564104beca 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -507,6 +507,9 @@ local function validate_references(self, input) end +-- TODO: Completely implement validate_references_sync without associating it +-- to declarative config. Currently, we will use the dc-generated +-- foreign_references table to accelerate iterating over entity foreign keys. function DeclarativeConfig.validate_references_sync(deltas, deltas_map, is_full_sync) local errs = {} diff --git a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua index 96e050989f4..9d8327692c0 100644 --- a/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua +++ b/spec/02-integration/18-hybrid_rpc/06-validate_deltas_spec.lua @@ -60,7 +60,6 @@ local function setup_bp() -- init declarative config if not cached_dc then - kong.sync = "fake sync to generate dc with sync_v2_enabled" local err cached_dc, err = declarative.new_config(kong.configuration) assert(cached_dc, err)