Skip to content

Commit

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

This was reported with #5401. When I added DAO transformations,
for some reason I ended up writing stupid code that just broke
the whole schema validation on declarative. This commit is an
attempt to fix that. Sorry for all the pain caused.

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
bungle committed Jan 20, 2020
1 parent c0d9cf2 commit f6148e9
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 f6148e9

Please sign in to comment.