diff --git a/kong/db/errors.lua b/kong/db/errors.lua index bf4b065fdfa..d3911e9cfa0 100644 --- a/kong/db/errors.lua +++ b/kong/db/errors.lua @@ -767,41 +767,15 @@ do end - --- Add foreign key references to child entities. - --- - ---@param entity table - ---@param field_name string - ---@param foreign_field_name string - local function add_foreign_keys(entity, field_name, foreign_field_name) - local foreign_id = validate_id(entity.id) - if not foreign_id then - return - end - - local values = entity[field_name] - if type(values) ~= "table" then - return - end - - local fk = { id = foreign_id } - for i = 1, #values do - values[i][foreign_field_name] = values[i][foreign_field_name] or fk - end - end - - - ---@param entity table - ---@param field_name string - ---@return any - local function replace_with_foreign_key(entity, field_name) - local value = entity[field_name] - entity[field_name] = nil - - if type(value) == "table" and value.id then - entity[field_name] = { id = value.id } + -- given an entity table with an .id attribute that is a valid UUID, + -- yield a primary key table + -- + ---@param entity table + ---@return table? + local function make_pk(entity) + if validate_id(entity.id) then + return { id = entity.id } end - - return value end @@ -812,45 +786,192 @@ do local function add_entity_errors(entity_type, entity, err_t, flattened) if type(err_t) ~= "table" or nkeys(err_t) == 0 then return - end - -- instead of a single entity, we have a collection - if is_array(entity) then - for i, err_t_i in drain(err_t) do - add_entity_errors(entity_type, entity[i], err_t_i, flattened) - end + -- this *should* be unreachable, but it's relatively cheap to guard against + -- compared to everything else we're doing in this code path + elseif type(entity) ~= "table" then + log(WARN, "could not parse ", entity_type, " errors for non-table ", + "input: '", tostring(entity), "'") return end + local entity_pk = make_pk(entity) + -- promote errors for foreign key relationships up to the top level -- array of errors and recursively flatten any of their validation -- errors for ref in each_foreign_field(entity_type) do - local field_name - local field_value - local field_entity_type - - -- owned one-to-one relationship (e.g. service->client_certificate) + -- owned one-to-one relationship + -- + -- In this path, we extract the foreign entity and replace it with a + -- primary key table (if one exists). + -- + -- Example: + -- + -- entity_type => "services" + -- + -- entity => { + -- name = "my-invalid-service", + -- url = "https://localhost:1234" + -- client_certificate = { + -- id = "d2e33f63-1424-408f-be55-d9d16cd2a382", + -- cert = "bad cert data", + -- key = "bad cert key data", + -- } + -- } + -- + -- ref => { + -- entity = "services", + -- field = "client_certificate", + -- reference = "certificates" + -- } + -- + -- field_name => "client_certificate" + -- + -- field_entity_type => "certificates" + -- + -- field_value => { + -- id = "d2e33f63-1424-408f-be55-d9d16cd2a382", + -- cert = "bad cert data", + -- key = "bad cert key data", + -- } + -- + -- replacement => { + -- id = "d2e33f63-1424-408f-be55-d9d16cd2a382" + -- } + -- + -- *after* handling the client_certificate errors, our entity looks like this: + -- + -- entity => { + -- name = "my-invalid-service", + -- url = "https://localhost:1234" + -- client_certificate = { + -- id = "d2e33f63-1424-408f-be55-d9d16cd2a382", + -- } + -- } + -- if ref.entity == entity_type then - field_name = ref.field - field_entity_type = ref.reference - field_value = replace_with_foreign_key(entity, field_name) + local field_name = ref.field + local field_value = entity[field_name] + local field_entity_type = ref.reference + local field_err_t = err_t[field_name] - -- foreign one-to-many relationship (e.g. service->routes) - else - field_name = ref.entity - field_entity_type = field_name - field_value = entity[field_name] + -- if the foreign value is _not_ a table, attempting to treat it like + -- an entity or array of entities will only yield confusion. + -- + -- instead, it's better to leave the error intact so that it will be + -- categorized as a field error on the current entity + if type(field_value) == "table" then + entity[field_name] = make_pk(field_value) + err_t[field_name] = nil - add_foreign_keys(entity, field_name, ref.field) - entity[field_name] = nil - end + add_entity_errors(field_entity_type, field_value, field_err_t, flattened) + end - local field_err_t = err_t[field_name] - err_t[field_name] = nil - if field_value and field_err_t then - add_entity_errors(field_entity_type, field_value, field_err_t, flattened) + -- foreign one-to-many relationship + -- + -- Example: + -- + -- entity_type => "routes" + -- + -- entity => { + -- name = "my-invalid-route", + -- id = "d2e33f63-1424-408f-be55-d9d16cd2a382", + -- paths = { 123 }, + -- plugins = { + -- { + -- name = "http-log", + -- config = { + -- invalid_param = 456, + -- }, + -- }, + -- { + -- name = "file-log", + -- config = { + -- invalid_param = 456, + -- }, + -- }, + -- }, + -- } + -- + -- ref => { + -- entity = "plugins", + -- field = "route", + -- reference = "routes" + -- } + -- + -- field_name => "plugins" + -- + -- field_entity_type => "plugins" + -- + -- field_value => { + -- { + -- name = "http-log", + -- config = { + -- invalid_param = 456, + -- }, + -- }, + -- { + -- name = "file-log", + -- config = { + -- invalid_param = 456, + -- }, + -- }, + -- } + -- + -- before recursing on each plugin in `entity.plugins` to handle their + -- respective validation errors, we add our route's primary key to them, + -- yielding: + -- + -- { + -- { + -- name = "http-log", + -- config = { + -- invalid_param = 456, + -- }, + -- route = { + -- id = "d2e33f63-1424-408f-be55-d9d16cd2a382", + -- }, + -- }, + -- { + -- name = "file-log", + -- config = { + -- invalid_param = 456, + -- }, + -- route = { + -- id = "d2e33f63-1424-408f-be55-d9d16cd2a382", + -- }, + -- }, + -- } + -- + else + local field_name = ref.entity + local field_value = entity[field_name] + local field_entity_type = field_name + local field_err_t = err_t[field_name] + local field_fk = ref.field + + -- same as the one-to-one case: if the field's value is not a table, + -- we will let any errors related to it be categorized as a field-level + -- error instead + if type(field_value) == "table" then + entity[field_name] = nil + err_t[field_name] = nil + + if field_err_t then + for i = 1, #field_value do + local item = field_value[i] + + -- add our entity's primary key to each child item + if item[field_fk] == nil then + item[field_fk] = entity_pk + end + + add_entity_errors(field_entity_type, item, field_err_t[i], flattened) + end + end + end end end 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 d0db966693f..8985166a87c 100644 --- a/spec/02-integration/04-admin_api/15-off_spec.lua +++ b/spec/02-integration/04-admin_api/15-off_spec.lua @@ -1094,6 +1094,13 @@ describe("Admin API #off /config [flattened errors]", function() if debug then helpers.intercept(body) end + + assert.logfile().has.no.line("[emerg]", true, 0.01) + assert.logfile().has.no.line("[crit]", true, 0.01) + assert.logfile().has.no.line("[alert]", true, 0.01) + assert.logfile().has.no.line("[error]", true, 0.01) + assert.logfile().has.no.line("[warn]", true, 0.01) + return errors end @@ -2272,6 +2279,248 @@ R6InCcH2Wh8wSeY5AuDXvu2tv9g/PW9wIJmPuKSHMA== assert.equals(4, #flattened.flattened_errors, "unexpected number of flattened errors") end) + + it("does not throw for invalid input - (#10767)", function() + local username = "774f8446-6427-43f9-9962-ce7ab8097fe4" + local consumer_id = "68d5de9f-2211-5ed8-b827-22f57a492d0f" + local service_name = "default.nginx-sample-1.nginx-sample-1.80" + local upstream_name = "nginx-sample-1.default.80.svc" + + local plugin = { + name = "rate-limiting", + consumer = username, + config = { + error_code = 429, + error_message = "API rate limit exceeded", + fault_tolerant = true, + hide_client_headers = false, + limit_by = "consumer", + policy = "local", + redis_database = 0, + redis_port = 6379, + redis_ssl = false, + redis_ssl_verify = false, + redis_timeout = 2000, + second = 2000, + }, + enabled = true, + protocols = { + "grpc", + "grpcs", + "http", + "https", + }, + tags = { + "k8s-name:nginx-sample-1-rate", + "k8s-namespace:default", + "k8s-kind:KongPlugin", + "k8s-uid:5163972c-543d-48ae-b0f6-21701c43c1ff", + "k8s-group:configuration.konghq.com", + "k8s-version:v1", + }, + } + + local input = { + _format_version = "3.0", + consumers = { + { + acls = { + { + group = "app", + tags = { + "k8s-name:app-acl", + "k8s-namespace:default", + "k8s-kind:Secret", + "k8s-uid:f1c5661c-a087-4c4b-b545-2d8b3870d661", + "k8s-version:v1", + }, + }, + }, + + basicauth_credentials = { + { + password = "6ef728de-ba68-4e59-acb9-6e502c28ae0b", + tags = { + "k8s-name:app-cred", + "k8s-namespace:default", + "k8s-kind:Secret", + "k8s-uid:aadd4598-2969-49ea-82ac-6ab5159e2f2e", + "k8s-version:v1", + }, + username = username, + }, + }, + + id = consumer_id, + tags = { + "k8s-name:app", + "k8s-namespace:default", + "k8s-kind:KongConsumer", + "k8s-uid:7ee19bea-72d5-402b-bf0f-f57bf81032bf", + "k8s-group:configuration.konghq.com", + "k8s-version:v1", + }, + username = username, + }, + }, + + plugins = { + plugin, + + { + config = { + error_code = 429, + error_message = "API rate limit exceeded", + fault_tolerant = true, + hide_client_headers = false, + limit_by = "consumer", + policy = "local", + redis_database = 0, + redis_port = 6379, + redis_ssl = false, + redis_ssl_verify = false, + redis_timeout = 2000, + second = 2000, + }, + consumer = username, + enabled = true, + name = "rate-limiting", + protocols = { + "grpc", + "grpcs", + "http", + "https", + }, + tags = { + "k8s-name:nginx-sample-2-rate", + "k8s-namespace:default", + "k8s-kind:KongPlugin", + "k8s-uid:89fa1cd1-78da-4c3e-8c3b-32be1811535a", + "k8s-group:configuration.konghq.com", + "k8s-version:v1", + }, + }, + + { + config = { + allow = { + "nginx-sample-1", + "app", + }, + hide_groups_header = false, + }, + enabled = true, + name = "acl", + protocols = { + "grpc", + "grpcs", + "http", + "https", + }, + service = service_name, + tags = { + "k8s-name:nginx-sample-1", + "k8s-namespace:default", + "k8s-kind:KongPlugin", + "k8s-uid:b9373482-32e1-4ac3-bd2a-8926ab728700", + "k8s-group:configuration.konghq.com", + "k8s-version:v1", + }, + }, + }, + + services = { + { + connect_timeout = 60000, + host = upstream_name, + id = "8c17ab3e-b6bd-51b2-b5ec-878b4d608b9d", + name = service_name, + path = "/", + port = 80, + protocol = "http", + read_timeout = 60000, + retries = 5, + + routes = { + { + https_redirect_status_code = 426, + id = "84d45463-1faa-55cf-8ef6-4285007b715e", + methods = { + "GET", + }, + name = "default.nginx-sample-1.nginx-sample-1..80", + path_handling = "v0", + paths = { + "/sample/1", + }, + preserve_host = true, + protocols = { + "http", + "https", + }, + regex_priority = 0, + request_buffering = true, + response_buffering = true, + strip_path = false, + tags = { + "k8s-name:nginx-sample-1", + "k8s-namespace:default", + "k8s-kind:Ingress", + "k8s-uid:916a6e5a-eebe-4527-a78d-81963eb3e043", + "k8s-group:networking.k8s.io", + "k8s-version:v1", + }, + }, + }, + tags = { + "k8s-name:nginx-sample-1", + "k8s-namespace:default", + "k8s-kind:Service", + "k8s-uid:f7cc87f4-d5f7-41f8-b4e3-70608017e588", + "k8s-version:v1", + }, + write_timeout = 60000, + }, + }, + + upstreams = { + { + algorithm = "round-robin", + name = upstream_name, + tags = { + "k8s-name:nginx-sample-1", + "k8s-namespace:default", + "k8s-kind:Service", + "k8s-uid:f7cc87f4-d5f7-41f8-b4e3-70608017e588", + "k8s-version:v1", + }, + targets = { + { + target = "nginx-sample-1.default.svc:80", + }, + }, + }, + }, + } + + local flattened = post_config(input) + validate({ + { + entity_type = "plugin", + entity_name = plugin.name, + entity_tags = plugin.tags, + entity = plugin, + + errors = { + { + field = "consumer.id", + message = "missing primary key", + type = "field", + } + }, + }, + }, flattened) + end) end)