Skip to content

Commit

Permalink
fix(declarative) schema validation in db-less mode (#5464)
Browse files Browse the repository at this point in the history 
### Summary

This was reported with #5401. The changes for adding DAO transformations ended up breaking schema validation on declarative mode. This commit fixes that.

While fixing this, I also found out that original validation didn't work correctly when you had entity validations that relied on values of foreign keys. This is now also corrected.

### Issues Resolved

Fix #5401
  • Loading branch information
@bungle @hishamhm
bungle authored and hishamhm committed Jan 20, 2020
1 parent c0d9cf2 commit 09105c1
Show file tree
Hide file tree
Showing 2 changed files with 425 additions and 71 deletions.
168 changes: 97 additions & 71 deletions kong/db/schema/others/declarative_config.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,12 @@ local function add_top_level_entities(fields, entities)
end


local function copy_without_foreign(record)
local function copy_record(record, include_foreign)
local copy = utils.deep_copy(record, false)
if include_foreign then
return copy
end

for i = #copy.fields, 1, -1 do
local f = copy.fields[i]
local _, fdata = next(f)
Expand All @@ -144,18 +148,18 @@ end
-- (e.g. `service` as a string key in the `routes` entry).
-- @tparam map<string,table> records A map of top-level record definitions,
-- indexable by entity name. These records are modified in-place.
local function nest_foreign_relationships(records)
local function nest_foreign_relationships(records, include_foreign)
for entity, record in pairs(records) do
for _, f in ipairs(record.fields) do
local fname, fdata = next(f)
local _, fdata = next(f)
if fdata.type == "foreign" then
local ref = fdata.reference
-- allow nested entities
-- (e.g. `routes` inside `services`)
table.insert(records[ref].fields, {
[entity] = {
type = "array",
elements = copy_without_foreign(record, fname),
elements = copy_record(record, include_foreign),
},
})
end
Expand Down Expand Up @@ -187,7 +191,7 @@ local function reference_foreign_by_name(records)
end


local function build_fields(entities)
local function build_fields(entities, include_foreign)
local fields = {
{ _format_version = { type = "string", required = true, eq = "1.1" } },
}
Expand All @@ -197,7 +201,7 @@ local function build_fields(entities)
})

local records = add_top_level_entities(fields, entities)
nest_foreign_relationships(records)
nest_foreign_relationships(records, include_foreign)

return fields, records
end
Expand Down Expand Up @@ -482,41 +486,98 @@ local function generate_ids(input, known_entities, parent_entity)
end


local function populate_ids(input, known_entities, parent_entity, by_id, by_key)
local by_id = by_id or {}
local by_key = by_key or {}
for _, entity in ipairs(known_entities) do
if type(input[entity]) ~= "table" then
goto continue
end

local parent_fk
local child_key
if parent_entity then
local parent_schema = all_schemas[parent_entity]
if parent_schema.fields[entity] then
goto continue
end
parent_fk = parent_schema:extract_pk_values(input)
child_key = foreign_children[parent_entity][entity]
end

local schema = all_schemas[entity]
for _, item in ipairs(input[entity]) do
local pk_name, key = get_key_for_uuid_gen(entity, item, schema,
parent_fk, child_key)
if key and not item[pk_name] then
item[pk_name] = generate_uuid(schema.name, key)
end

populate_ids(item, known_entities, entity, by_id, by_key)

local item_id = DeclarativeConfig.pk_string(schema, item)
by_id[entity] = by_id[entity] or {}
by_id[entity][item_id] = item

local key
if schema.endpoint_key then
key = item[schema.endpoint_key]
if key then
by_key[entity] = by_key[entity] or {}
by_key[entity][key] = item
end
end

if parent_fk and not item[child_key] then
item[child_key] = utils.deep_copy(parent_fk, false)
end
end

::continue::
end

if not parent_entity then
for entity, entries in pairs(by_id) do
local schema = all_schemas[entity]
for _, entry in pairs(entries) do
for name, field in schema:each_field(entry) do
if field.type == "foreign" and type(entry[name]) == "string" then
local found = find_entity(entry[name], field.reference, by_key, by_id)
if found then
entry[name] = all_schemas[field.reference]:extract_pk_values(found)
end
end
end
end
end
end
end


local function flatten(self, input)
local output = {}

-- This may give invalid errors as it validates input that does not have
-- foreign keys filled (e.g. basicauth_credentials fail on this with this
-- config:
--
-- consumers:
-- username: consumer
-- basicauth_credentials:
-- - username: username
-- password: password
--
-- because it tries to validate the credential that has no consumer.id at
-- this point filled in, and transformation implicitly adds mutually_required
-- check on that entity.
--
-- As a short-term solution, we don't error right away here.
local ok, err = self:validate(input)
if not ok then
-- 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
local input_copy = utils.deep_copy(input, false)
populate_ids(input_copy, self.known_entities)
local schema = DeclarativeConfig.load(self.plugin_set, true)
if not schema:validate(input_copy) then
return nil, err
end
end

generate_ids(input, self.known_entities)

local processed = self:process_auto_fields(input, "insert")

local by_id, by_key_or_err = validate_references(self, processed)
local by_id, by_key = validate_references(self, processed)
if not by_id then
-- let's return original error in case it is defined, as the reference
-- validation could have failed because of that.
if not ok then
return nil, err
end

return nil, by_key_or_err
return nil, by_key
end
local by_key = by_key_or_err

for entity, entries in pairs(by_id) do
local schema = all_schemas[entity]
Expand All @@ -535,44 +596,6 @@ local function flatten(self, input)
end
end

-- TODO: is the plugin not enabled a sign of something wrong somewhere?
local ok2, err2 = schema:validate(flat_entry)
if not ok2
and err2
and err2.name ~= string.format("plugin '%s' not enabled; add it to " ..
"the 'plugins' configuration property",
flat_entry.name)
then

if err then
if err2["@entity"] then
for _, errmsg2 in ipairs(err2["@entity"]) do
local found
if err["@entity"] then
for _, errmsg in ipairs(err["@entity"]) do
if errmsg2 == errmsg then
found = true
break
end
end
end

if not found then
if not err["@entity"] then
err["@entity"] = {}
end

table.insert(err["@entity"], errmsg2)
end
end
end

return nil, err
end

return nil, err2
end

output[entity][id] = flat_entry
end
end
Expand All @@ -596,7 +619,7 @@ local function load_entity_subschemas(entity_name, entity)
end


function DeclarativeConfig.load(plugin_set)
function DeclarativeConfig.load(plugin_set, include_foreign)
if not core_entities then
-- a copy of constants.CORE_ENTITIES without "tags"
core_entities = {}
Expand Down Expand Up @@ -633,7 +656,7 @@ function DeclarativeConfig.load(plugin_set)
end
end

local fields, records = build_fields(known_entities)
local fields, records = build_fields(known_entities, include_foreign)
-- assert(no_foreign(fields))

local ok, err = load_plugin_subschemas(fields, plugin_set)
Expand All @@ -644,7 +667,9 @@ function DeclarativeConfig.load(plugin_set)
-- 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(records)
if not include_foreign then
reference_foreign_by_name(records)
end

local def = {
name = "declarative_config",
Expand All @@ -656,6 +681,7 @@ function DeclarativeConfig.load(plugin_set)

schema.known_entities = known_entities
schema.flatten = flatten
schema.plugin_set = plugin_set

return schema, nil, def
end
Expand Down
Loading

0 comments on commit 09105c1

Please sign in to comment.