From 56dca18a90afccc6c08eaa43dc144d3a69acb6f3 Mon Sep 17 00:00:00 2001 From: Thibault Charbonnier Date: Mon, 1 Jun 2015 17:16:40 +0200 Subject: [PATCH] feat(resolver) APIs schema updates. - APIs now expect either a `public_dns` or a `path`. - some schemas improvements on consumers and apis - rename the newest migration to 0.3.0, the new kong version Former-commit-id: 63b4c5cb4dfd5929f33329129b681d61942274ad --- ...s_path.lua => 2015-05-22-235608_0.3.0.lua} | 2 +- kong-0.3.0-1.rockspec | 1 - kong/dao/schemas/apis.lua | 33 ++++++- kong/dao/schemas/consumers.lua | 10 +-- kong/resolver/access.lua | 33 +++---- kong/resolver/certificate.lua | 25 +++++- kong/resolver/resolver_util.lua | 26 ------ kong/tools/utils.lua | 2 +- spec/integration/admin_api/admin_api_spec.lua | 2 +- .../admin_api/apis_routes_spec.lua | 4 +- .../admin_api/consumers_routes_spec.lua | 2 +- spec/unit/dao/entities_schemas_spec.lua | 85 ++++++++++++++++--- 12 files changed, 156 insertions(+), 69 deletions(-) rename database/migrations/cassandra/{2015-05-22-235608_apis_path.lua => 2015-05-22-235608_0.3.0.lua} (93%) delete mode 100644 kong/resolver/resolver_util.lua diff --git a/database/migrations/cassandra/2015-05-22-235608_apis_path.lua b/database/migrations/cassandra/2015-05-22-235608_0.3.0.lua similarity index 93% rename from database/migrations/cassandra/2015-05-22-235608_apis_path.lua rename to database/migrations/cassandra/2015-05-22-235608_0.3.0.lua index acf147c2409b..706aa1e120f4 100644 --- a/database/migrations/cassandra/2015-05-22-235608_apis_path.lua +++ b/database/migrations/cassandra/2015-05-22-235608_0.3.0.lua @@ -1,5 +1,5 @@ local Migration = { - name = "2015-05-22-235608_apis_path", + name = "2015-05-22-235608_0.3.0", up = function(options) return [[ diff --git a/kong-0.3.0-1.rockspec b/kong-0.3.0-1.rockspec index 3dd0296bd58e..471e2ded02b4 100644 --- a/kong-0.3.0-1.rockspec +++ b/kong-0.3.0-1.rockspec @@ -70,7 +70,6 @@ build = { ["kong.resolver.access"] = "kong/resolver/access.lua", ["kong.resolver.header_filter"] = "kong/resolver/header_filter.lua", ["kong.resolver.certificate"] = "kong/resolver/certificate.lua", - ["kong.resolver.resolver_util"] = "kong/resolver/resolver_util.lua", ["kong.dao.error"] = "kong/dao/error.lua", ["kong.dao.schemas_validation"] = "kong/dao/schemas_validation.lua", diff --git a/kong/dao/schemas/apis.lua b/kong/dao/schemas/apis.lua index 524aa629985e..695dbcc297a7 100644 --- a/kong/dao/schemas/apis.lua +++ b/kong/dao/schemas/apis.lua @@ -1,4 +1,5 @@ local url = require "socket.url" +local stringy = require "stringy" local constants = require "kong.constants" local function validate_target_url(value) @@ -16,12 +17,38 @@ local function validate_target_url(value) return false, "Invalid target URL" end +local function check_public_dns_and_path(value, api_t) + local public_dns = type(api_t.public_dns) == "string" and stringy.strip(api_t.public_dns) or "" + local path = type(api_t.path) == "string" and stringy.strip(api_t.path) or "" + + if path == "" and public_dns == "" then + return false, "At least a 'public_dns' or a 'path' must be specified" + end + + return true +end + +local function check_path(path, api_t) + local valid, err = check_public_dns_and_path(path, api_t) + if not valid then + return false, err + end + + -- Prefix with `/` for the sake of consistency + if path and string.sub(path, 0, 1) ~= "/" then + api_t.path = "/"..path + end + + return true +end + return { id = { type = constants.DATABASE_TYPES.ID }, name = { type = "string", unique = true, queryable = true, default = function(api_t) return api_t.public_dns end }, - public_dns = { type = "string", required = true, unique = true, queryable = true, - regex = "([a-zA-Z0-9-]+(\\.[a-zA-Z0-9-]+)*)" }, - path = { type = "string", queryable = true, unique = true }, + public_dns = { type = "string", unique = true, queryable = true, + func = check_public_dns_and_path, + regex = "([a-zA-Z0-9-]+(\\.[a-zA-Z0-9-]+)*)" }, + path = { type = "string", queryable = true, unique = true, func = check_path }, target_url = { type = "string", required = true, func = validate_target_url }, created_at = { type = constants.DATABASE_TYPES.TIMESTAMP } } diff --git a/kong/dao/schemas/consumers.lua b/kong/dao/schemas/consumers.lua index 6ba7d25d5ec4..69fe5183a8f7 100644 --- a/kong/dao/schemas/consumers.lua +++ b/kong/dao/schemas/consumers.lua @@ -2,13 +2,13 @@ local stringy = require "stringy" local constants = require "kong.constants" local function check_custom_id_and_username(value, consumer_t) - local custom_id = consumer_t.custom_id - local username = consumer_t.username + local username = type(consumer_t.username) == "string" and stringy.strip(consumer_t.username) or "" + local custom_id = type(consumer_t.custom_id) == "string" and stringy.strip(consumer_t.custom_id) or "" - if (custom_id == nil or type(custom_id) == "string" and stringy.strip(custom_id) == "") - and (username == nil or type(username) == "string" and stringy.strip(username) == "") then - return false, "At least a 'custom_id' or a 'username' must be specified" + if custom_id == "" and username == "" then + return false, "At least a 'custom_id' or a 'username' must be specified" end + return true end diff --git a/kong/resolver/access.lua b/kong/resolver/access.lua index bc6e4b25dd8a..086ee8693607 100644 --- a/kong/resolver/access.lua +++ b/kong/resolver/access.lua @@ -47,18 +47,23 @@ local function find_api() local retrieved_api -- retrieve all APIs - local apis_by_public_dns, err = cache.get_or_set("APIS_BY_PUBLIC_DNS", function() + local apis_dics, err = cache.get_or_set("APIS_BY_PUBLIC_DNS", function() local apis, err = dao.apis:find_all() if err then return nil, err end - -- build a dictionnary of public_dns:api for efficient retrieval by Host. - local dic = {} + -- build dictionnaries of public_dns:api and path:apis for efficient lookup. + local dns_dic, path_dic = {}, {} for _, api in ipairs(apis) do - dic[api.public_dns] = api + if api.public_dns then + dns_dic[api.public_dns] = api + end + if api.path then + path_dic[api.path] = api + end end - return dic + return {dns = dns_dic, path = path_dic} end, 60) -- 60 seconds cache if err then @@ -76,8 +81,8 @@ local function find_api() -- for all values of this header, try to find an API using the apis_by_dns dictionnary for _, host in ipairs(hosts) do table.insert(all_hosts, host) - if apis_by_public_dns[host] then - retrieved_api = apis_by_public_dns[host] + if apis_dics.dns[host] then + retrieved_api = apis_dics.dns[host] break end end @@ -91,14 +96,12 @@ local function find_api() -- Otherwise, we look for it by path. We have to loop over all APIs and compare the requested URI. local request_uri = ngx.var.request_uri - for _, api in pairs(apis_by_public_dns) do - if api.path then - local m, err = ngx.re.match(request_uri, api.path) - if err then - ngx.log(ngx.ERR, "[resolver] error matching requested path: "..err) - elseif m then - retrieved_api = api - end + for path, api in pairs(apis_dics.path) do + local m, err = ngx.re.match(request_uri, path) + if err then + ngx.log(ngx.ERR, "[resolver] error matching requested path: "..err) + elseif m then + retrieved_api = api end end diff --git a/kong/resolver/certificate.lua b/kong/resolver/certificate.lua index 909c9d28f593..47756a78f984 100644 --- a/kong/resolver/certificate.lua +++ b/kong/resolver/certificate.lua @@ -1,12 +1,33 @@ -local resolver_util = require("kong.resolver.resolver_util") +local cache = require "kong.tools.database_cache" +local stringy = require "stringy" local _M = {} +local function find_api(hosts) + local retrieved_api, err + for _, host in ipairs(hosts) do + local sanitized_host = stringy.split(host, ":")[1] + + retrieved_api, err = cache.get_or_set(cache.api_key(sanitized_host), function() + local apis, err = dao.apis:find_by_keys { public_dns = sanitized_host } + if err then + return nil, err + elseif apis and #apis == 1 then + return apis[1] + end + end) + + if err or retrieved_api then + return retrieved_api, err + end + end +end + function _M.execute(conf) local ssl = require "ngx.ssl" local server_name = ssl.server_name() if server_name then -- Only support SNI requests - local api, err = resolver_util.find_api({server_name}) + local api, err = find_api({server_name}) if not err and api then ngx.ctx.api = api end diff --git a/kong/resolver/resolver_util.lua b/kong/resolver/resolver_util.lua deleted file mode 100644 index 872a831f81e6..000000000000 --- a/kong/resolver/resolver_util.lua +++ /dev/null @@ -1,26 +0,0 @@ -local cache = require "kong.tools.database_cache" -local stringy = require "stringy" - -local _M = {} - -function _M.find_api(hosts) - local retrieved_api, err - for _, host in ipairs(hosts) do - local sanitized_host = stringy.split(host, ":")[1] - - retrieved_api, err = cache.get_or_set(cache.api_key(sanitized_host), function() - local apis, err = dao.apis:find_by_keys { public_dns = sanitized_host } - if err then - return nil, err - elseif apis and #apis == 1 then - return apis[1] - end - end) - - if err or retrieved_api then - return retrieved_api, err - end - end -end - -return _M diff --git a/kong/tools/utils.lua b/kong/tools/utils.lua index 05e2d3a1dfe3..33b77ffed35a 100644 --- a/kong/tools/utils.lua +++ b/kong/tools/utils.lua @@ -2,7 +2,7 @@ local _M = {} function _M.table_size(t) local res = 0 - for _,_ in pairs(t) do + for _ in pairs(t) do res = res + 1 end return res diff --git a/spec/integration/admin_api/admin_api_spec.lua b/spec/integration/admin_api/admin_api_spec.lua index 06611339cff3..cd83a8e24aa9 100644 --- a/spec/integration/admin_api/admin_api_spec.lua +++ b/spec/integration/admin_api/admin_api_spec.lua @@ -14,7 +14,7 @@ local ENDPOINTS = { } }, update_fields = { public_dns = "newapi.mockbin.com" }, - error_message = '{"public_dns":"public_dns is required","target_url":"target_url is required"}\n' + error_message = '{"public_dns":"At least a \'public_dns\' or a \'path\' must be specified","path":"At least a \'public_dns\' or a \'path\' must be specified","target_url":"target_url is required"}\n' }, { collection = "consumers", diff --git a/spec/integration/admin_api/apis_routes_spec.lua b/spec/integration/admin_api/apis_routes_spec.lua index 97feb988fd3d..2570387d3123 100644 --- a/spec/integration/admin_api/apis_routes_spec.lua +++ b/spec/integration/admin_api/apis_routes_spec.lua @@ -36,7 +36,7 @@ describe("Admin API", function() it("[FAILURE] should return proper errors", function() send_content_types(BASE_URL, "POST", {}, 400, - '{"public_dns":"public_dns is required","target_url":"target_url is required"}') + '{"public_dns":"At least a \'public_dns\' or a \'path\' must be specified","path":"At least a \'public_dns\' or a \'path\' must be specified","target_url":"target_url is required"}') send_content_types(BASE_URL, "POST", {public_dns="api.mockbin.com"}, 400, '{"target_url":"target_url is required"}') @@ -74,7 +74,7 @@ describe("Admin API", function() it("[FAILURE] should return proper errors", function() send_content_types(BASE_URL, "PUT", {}, 400, - '{"public_dns":"public_dns is required","target_url":"target_url is required"}') + '{"public_dns":"At least a \'public_dns\' or a \'path\' must be specified","path":"At least a \'public_dns\' or a \'path\' must be specified","target_url":"target_url is required"}') send_content_types(BASE_URL, "PUT", {public_dns="api.mockbin.com"}, 400, '{"target_url":"target_url is required"}') diff --git a/spec/integration/admin_api/consumers_routes_spec.lua b/spec/integration/admin_api/consumers_routes_spec.lua index ff0c8a092a61..0ecb7b82ebf6 100644 --- a/spec/integration/admin_api/consumers_routes_spec.lua +++ b/spec/integration/admin_api/consumers_routes_spec.lua @@ -159,7 +159,7 @@ describe("Admin API", function() local response, status = http_client.patch(BASE_URL..consumer.id, {username=""}) assert.equal(400, status) - assert.equal('{"username":"username is not a string"}\n', response) + assert.equal('{"custom_id":"At least a \'custom_id\' or a \'username\' must be specified","username":"username is not a string"}\n', response) end) end) diff --git a/spec/unit/dao/entities_schemas_spec.lua b/spec/unit/dao/entities_schemas_spec.lua index 6348035465bf..1cfff4ecd3f3 100644 --- a/spec/unit/dao/entities_schemas_spec.lua +++ b/spec/unit/dao/entities_schemas_spec.lua @@ -1,5 +1,6 @@ local validate = require("kong.dao.schemas_validation").validate local api_schema = require "kong.dao.schemas.apis" +local consumer_schema = require "kong.dao.schemas.consumers" require "kong.tools.ngx_stub" @@ -8,38 +9,100 @@ describe("Entities Schemas", function() it("should return error with wrong target_url", function() local valid, errors = validate({ - public_dns = "hello.com", + public_dns = "mockbin.com", target_url = "asdasd" }, api_schema) assert.False(valid) - assert.same("Invalid target URL", errors.target_url) + assert.equal("Invalid target URL", errors.target_url) end) it("should return error with wrong target_url protocol", function() local valid, errors = validate({ - public_dns = "hello.com", - target_url = "wot://hello.com/" + public_dns = "mockbin.com", + target_url = "wot://mockbin.com/" }, api_schema) assert.False(valid) - assert.same("Supported protocols are HTTP and HTTPS", errors.target_url) + assert.equal("Supported protocols are HTTP and HTTPS", errors.target_url) end) - it("should work without a path", function() + it("should validate without a path", function() local valid, errors = validate({ - public_dns = "hello.com", - target_url = "http://hello.com" + public_dns = "mockbin.com", + target_url = "http://mockbin.com" }, api_schema) + assert.falsy(errors) assert.True(valid) + end) + + it("should validate with upper case protocol", function() + local valid, errors = validate({ + public_dns = "mockbin.com", + target_url = "HTTP://mockbin.com/world" + }, api_schema) assert.falsy(errors) + assert.True(valid) end) - it("should work without upper case protocol", function() + it("should complain if missing `public_dns` and `path`", function() + local valid, errors = validate({ + name = "mockbin" + }, api_schema) + assert.False(valid) + assert.equal("At least a 'public_dns' or a 'path' must be specified", errors.path) + assert.equal("At least a 'public_dns' or a 'path' must be specified", errors.public_dns) + local valid, errors = validate({ - public_dns = "hello2.com", - target_url = "HTTP://hello.com/world" + name = "mockbin", + path = true }, api_schema) + assert.False(valid) + assert.equal("path is not a string", errors.path) + assert.equal("At least a 'public_dns' or a 'path' must be specified", errors.public_dns) + end) + + it("should prefix a `path` with a slash", function() + local api_t = { + name = "mockbin", + path = "status", + target_url = "http://mockbin.com" + } + + local valid, errors = validate(api_t, api_schema) + assert.falsy(errors) assert.True(valid) + assert.equal("/status", api_t.path) + + api_t = { + name = "mockbin", + path = "/status", + target_url = "http://mockbin.com" + } + + local valid, errors = validate(api_t, api_schema) assert.falsy(errors) + assert.True(valid) + assert.equal("/status", api_t.path) + end) + + end) + + describe("Consumers", function() + + it("should require a `custom_id` or `username`", function() + local valid, errors = validate({}, consumer_schema) + assert.False(valid) + assert.equal("At least a 'custom_id' or a 'username' must be specified", errors.username) + assert.equal("At least a 'custom_id' or a 'username' must be specified", errors.custom_id) + + valid, errors = validate({ username = "" }, consumer_schema) + assert.False(valid) + assert.equal("At least a 'custom_id' or a 'username' must be specified", errors.username) + assert.equal("At least a 'custom_id' or a 'username' must be specified", errors.custom_id) + + valid, errors = validate({ username = true }, consumer_schema) + assert.False(valid) + assert.equal("username is not a string", errors.username) + assert.equal("At least a 'custom_id' or a 'username' must be specified", errors.custom_id) end) end)