From ab78fc277dded213f857e133e0b51902e15458d5 Mon Sep 17 00:00:00 2001 From: Kong Team Gateway Bot <98048765+team-gateway-bot@users.noreply.github.com> Date: Wed, 18 Dec 2024 04:12:02 +0800 Subject: [PATCH] fix(dbless): nest unique field errors for child entities (#14017) (#10999) * fix(dbless): nest unique field errors for child entities This applies the same fix as b8891eb969a00469afb2a70d8a4a605497c1fc40 but to a different code path that checks fields with a unique constraint. * improve docstring (cherry picked from commit 9dded314d5cdad3500d7ac0d5b865b0b34a89a0e) Co-authored-by: Michael Martin --- .../fix-dbless-consumer-credential-error.yml | 3 + kong/db/schema/others/declarative_config.lua | 38 +++-- .../04-admin_api/15-off_spec.lua | 156 ++++++++++++++++++ 3 files changed, 179 insertions(+), 18 deletions(-) create mode 100644 changelog/unreleased/kong/fix-dbless-consumer-credential-error.yml diff --git a/changelog/unreleased/kong/fix-dbless-consumer-credential-error.yml b/changelog/unreleased/kong/fix-dbless-consumer-credential-error.yml new file mode 100644 index 000000000000..b459e8fab9b4 --- /dev/null +++ b/changelog/unreleased/kong/fix-dbless-consumer-credential-error.yml @@ -0,0 +1,3 @@ +message: "Fixed an issue where `POST /config?flatten_errors=1` could not return a proper response if the input contained duplicate consumer credentials." +type: bugfix +scope: Core diff --git a/kong/db/schema/others/declarative_config.lua b/kong/db/schema/others/declarative_config.lua index d0c1a7df2fb9..62ff4dcd14a7 100644 --- a/kong/db/schema/others/declarative_config.lua +++ b/kong/db/schema/others/declarative_config.lua @@ -458,6 +458,22 @@ local function uniqueness_error_msg(entity, key, value) "with " .. key .. " set to '" .. value .. "' already declared" end +local function add_error(errs, parent_entity, parent_idx, entity, entity_idx, err) + if parent_entity and parent_idx then + errs[parent_entity] = errs[parent_entity] or {} + errs[parent_entity][parent_idx] = errs[parent_entity][parent_idx] or {} + errs[parent_entity][parent_idx][entity] = errs[parent_entity][parent_idx][entity] or {} + + -- e.g. errs["upstreams"][5]["targets"][2] + errs[parent_entity][parent_idx][entity][entity_idx] = err + + else + errs[entity] = errs[entity] or {} + + -- e.g. errs["consumers"][3] + errs[entity][entity_idx] = err + end +end local function populate_references(input, known_entities, by_id, by_key, expected, errs, parent_entity, parent_idx) for _, entity in ipairs(known_entities) do @@ -497,8 +513,8 @@ local function populate_references(input, known_entities, by_id, by_key, expecte if key and key ~= ngx.null then local ok = add_to_by_key(by_key, entity_schema, item, entity, key) if not ok then - errs[entity] = errs[entity] or {} - errs[entity][i] = uniqueness_error_msg(entity, endpoint_key, key) + add_error(errs, parent_entity, parent_idx, entity, i, + uniqueness_error_msg(entity, endpoint_key, key)) failed = true end end @@ -506,22 +522,8 @@ local function populate_references(input, known_entities, by_id, by_key, expecte if item_id then by_id[entity] = by_id[entity] or {} if (not failed) and by_id[entity][item_id] then - local err_t - - if parent_entity and parent_idx then - errs[parent_entity] = errs[parent_entity] or {} - errs[parent_entity][parent_idx] = errs[parent_entity][parent_idx] or {} - errs[parent_entity][parent_idx][entity] = errs[parent_entity][parent_idx][entity] or {} - - -- e.g. errs["upstreams"][5]["targets"] - err_t = errs[parent_entity][parent_idx][entity] - - else - errs[entity] = errs[entity] or {} - err_t = errs[entity] - end - - err_t[i] = uniqueness_error_msg(entity, "primary key", item_id) + add_error(errs, parent_entity, parent_idx, entity, i, + uniqueness_error_msg(entity, "primary key", item_id)) else by_id[entity][item_id] = item diff --git a/spec/02-integration/04-admin_api/15-off_spec.lua b/spec/02-integration/04-admin_api/15-off_spec.lua index d2e74291c54d..fdb35b5aab77 100644 --- a/spec/02-integration/04-admin_api/15-off_spec.lua +++ b/spec/02-integration/04-admin_api/15-off_spec.lua @@ -2372,6 +2372,162 @@ R6InCcH2Wh8wSeY5AuDXvu2tv9g/PW9wIJmPuKSHMA== }, post_config(input)) end) + it("correctly handles nested entity errors", function() + local consumers = { + { + id = "cdac30ee-cd7e-465c-99b6-84f3e4e17015", + username = "consumer-01", + tags = { "consumer-01" }, + basicauth_credentials = { + { + id = "089091f4-cb8b-48f5-8463-8319097be716", + username = "user-01", password = "pwd", + tags = { "consumer-01-credential-01" }, + }, + { + id = "b1443d61-ccd9-4359-b82a-f37515442295", + username = "user-11", password = "pwd", + tags = { "consumer-01-credential-02" }, + }, + { + id = "2603d010-edbe-4713-94ef-145e281cbf4c", + username = "user-02", password = "pwd", + tags = { "consumer-01-credential-03" }, + }, + { + id = "760cf441-613c-48a2-b377-36aebc9f9ed0", + -- unique violation! + username = "user-11", password = "pwd", + tags = { "consumer-01-credential-04" }, + } + }, + }, + { + id = "c0c021f5-dae1-4031-bcf6-42e3c4d9ced9", + username = "consumer-02", + tags = { "consumer-02" }, + basicauth_credentials = { + { + id = "d0cd1919-bb07-4c85-b407-f33feb74f6e2", + username = "user-99", password = "pwd", + tags = { "consumer-02-credential-01" }, + } + }, + }, + { + id = "9acb0270-73aa-4968-b9e4-a4924e4aced5", + username = "consumer-03", + tags = { "consumer-03" }, + basicauth_credentials = { + { + id = "7e8bcd10-cdcd-41f1-8c4d-9790d34aa67d", + -- unique violation! + username = "user-01", password = "pwd", + tags = { "consumer-03-credential-01" }, + }, + { + id = "7fe186bd-44e5-4b97-854d-85a24929889d", + username = "user-33", password = "pwd", + tags = { "consumer-03-credential-02" }, + }, + { + id = "6547c325-5332-41fc-a954-d4972926cdb5", + -- unique violation! + username = "user-02", password = "pwd", + tags = { "consumer-03-credential-03" }, + }, + { + id = "e091a955-9ee1-4403-8d0a-a7f1f8bafaca", + -- unique violation! + username = "user-33", password = "pwd", + tags = { "consumer-03-credential-04" }, + } + }, + } + } + + local input = { + _format_version = "3.0", + consumers = consumers, + } + + validate({ + -- consumer 1 / credential 4 + { + entity = { + consumer = { id = consumers[1].id }, + id = consumers[1].basicauth_credentials[4].id, + tags = consumers[1].basicauth_credentials[4].tags, + password = "pwd", + username = "user-11" + }, + entity_id = consumers[1].basicauth_credentials[4].id, + entity_tags = consumers[1].basicauth_credentials[4].tags, + entity_type = "basicauth_credential", + errors = { { + message = "uniqueness violation: 'basicauth_credentials' entity with username set to 'user-11' already declared", + type = "entity" + } } + }, + + -- consumer 3 / credential 1 + { + entity = { + consumer = { id = consumers[3].id }, + id = consumers[3].basicauth_credentials[1].id, + tags = consumers[3].basicauth_credentials[1].tags, + password = "pwd", + username = "user-01" + }, + entity_id = consumers[3].basicauth_credentials[1].id, + entity_tags = consumers[3].basicauth_credentials[1].tags, + entity_type = "basicauth_credential", + errors = { { + message = "uniqueness violation: 'basicauth_credentials' entity with username set to 'user-01' already declared", + type = "entity" + } } + }, + + -- consumer 3 / credential 3 + { + entity = { + consumer = { id = consumers[3].id }, + id = consumers[3].basicauth_credentials[3].id, + tags = consumers[3].basicauth_credentials[3].tags, + password = "pwd", + username = "user-02" + }, + entity_id = consumers[3].basicauth_credentials[3].id, + entity_tags = consumers[3].basicauth_credentials[3].tags, + entity_type = "basicauth_credential", + errors = { { + message = "uniqueness violation: 'basicauth_credentials' entity with username set to 'user-02' already declared", + type = "entity" + } } + }, + + -- consumer 3 / credential 4 + { + entity = { + consumer = { id = consumers[3].id }, + id = consumers[3].basicauth_credentials[4].id, + tags = consumers[3].basicauth_credentials[4].tags, + password = "pwd", + username = "user-33" + }, + entity_id = consumers[3].basicauth_credentials[4].id, + entity_tags = consumers[3].basicauth_credentials[4].tags, + entity_type = "basicauth_credential", + errors = { { + message = "uniqueness violation: 'basicauth_credentials' entity with username set to 'user-33' already declared", + type = "entity" + } } + }, + + + }, post_config(input)) + end) + it("preserves IDs from the input", function() local id = "0175e0e8-3de9-56b4-96f1-b12dcb4b6691" local service = {