From d073aca32aca2313ab9e2ce1598ae1724a891c24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20Garc=C3=ADa=20Cota?= Date: Mon, 9 Apr 2018 15:24:53 +0200 Subject: [PATCH] feat(api) deactivate some /server_names endpoints, add specs --- kong-0.13.0-0.rockspec | 5 +- kong/api/endpoints.lua | 2 - kong/api/routes/certificates.lua | 62 +-- kong/api/routes/snis.lua | 16 + kong/core/certificate.lua | 18 +- kong/core/handler.lua | 6 +- kong/dao/migrations/cassandra.lua | 14 +- kong/dao/migrations/postgres.lua | 53 +-- kong/db/dao/certificates.lua | 100 ++--- kong/db/dao/{server_names.lua => snis.lua} | 61 +-- kong/db/errors.lua | 22 - kong/db/init.lua | 13 +- .../entities/{server_names.lua => snis.lua} | 4 +- .../02-integration/05-proxy/05-ssl_spec.lua | 2 +- .../02-core_entities_invalidations_spec.lua | 20 +- .../01-helpers/01-blueprints_spec.lua | 6 +- .../06-certificates_routes_spec.lua | 388 +++++++++--------- spec/02-integration/05-proxy/05-ssl_spec.lua | 2 +- .../02-core_entities_invalidations_spec.lua | 24 +- spec/fixtures/blueprints.lua | 6 +- 20 files changed, 395 insertions(+), 429 deletions(-) create mode 100644 kong/api/routes/snis.lua rename kong/db/dao/{server_names.lua => snis.lua} (69%) rename kong/db/schema/entities/{server_names.lua => snis.lua} (83%) diff --git a/kong-0.13.0-0.rockspec b/kong-0.13.0-0.rockspec index dce4d6c1036c..0df4a3cf761e 100644 --- a/kong-0.13.0-0.rockspec +++ b/kong-0.13.0-0.rockspec @@ -86,6 +86,7 @@ build = { ["kong.api.routes.cache"] = "kong/api/routes/cache.lua", ["kong.api.routes.upstreams"] = "kong/api/routes/upstreams.lua", ["kong.api.routes.certificates"] = "kong/api/routes/certificates.lua", + ["kong.api.routes.snis"] = "kong/api/routes/snis.lua", ["kong.tools.ip"] = "kong/tools/ip.lua", ["kong.tools.ciphers"] = "kong/tools/ciphers.lua", @@ -127,12 +128,12 @@ build = { ["kong.db.errors"] = "kong/db/errors.lua", ["kong.db.dao"] = "kong/db/dao/init.lua", ["kong.db.dao.certificates"] = "kong/db/dao/certificates.lua", - ["kong.db.dao.server_names"] = "kong/db/dao/server_names.lua", + ["kong.db.dao.snis"] = "kong/db/dao/snis.lua", ["kong.db.schema"] = "kong/db/schema/init.lua", ["kong.db.schema.entities.routes"] = "kong/db/schema/entities/routes.lua", ["kong.db.schema.entities.services"] = "kong/db/schema/entities/services.lua", ["kong.db.schema.entities.certificates"] = "kong/db/schema/entities/certificates.lua", - ["kong.db.schema.entities.server_names"] = "kong/db/schema/entities/server_names.lua", + ["kong.db.schema.entities.snis"] = "kong/db/schema/entities/snis.lua", ["kong.db.schema.entity"] = "kong/db/schema/entity.lua", ["kong.db.schema.metaschema"] = "kong/db/schema/metaschema.lua", ["kong.db.schema.typedefs"] = "kong/db/schema/typedefs.lua", diff --git a/kong/api/endpoints.lua b/kong/api/endpoints.lua index 0f056e79a3b8..080e8cfad1d4 100644 --- a/kong/api/endpoints.lua +++ b/kong/api/endpoints.lua @@ -20,8 +20,6 @@ local ERRORS_HTTP_CODES = { [Errors.codes.NOT_FOUND] = 404, [Errors.codes.INVALID_OFFSET] = 400, [Errors.codes.DATABASE_ERROR] = 500, - [Errors.codes.CONFLICTING_INPUT] = 409, - [Errors.codes.INVALID_INPUT] = 400, } diff --git a/kong/api/routes/certificates.lua b/kong/api/routes/certificates.lua index 08ee34f71193..7f8747dcbd9d 100644 --- a/kong/api/routes/certificates.lua +++ b/kong/api/routes/certificates.lua @@ -1,56 +1,30 @@ local endpoints = require "kong.api.endpoints" local utils = require "kong.tools.utils" +local responses = require "kong.tools.responses" -local function get_cert_by_server_name_or_id(self, db, helpers) - local id = self.params.certificates +local function get_cert_id_from_sni(self, db, helpers) + local id = self.params.certificates if not utils.is_valid_uuid(id) then - local cert, _, err_t = db.certificates:select_by_server_name(id) + local sni, _, err_t = db.snis:select_by_name(id) if err_t then return endpoints.handle_error(err_t) end - self.params.certificates = cert.id + if not sni then + responses.send_HTTP_NOT_FOUND("SNI not found") + end + + self.params.certificates = sni.certificate.id end end return { - ["/certificates"] = { - -- override to include the server_names list when getting all certificates - GET = function(self, db, helpers) - local data, _, err_t, offset = - db.certificates:page_with_name_list(self.args.size, - self.args.offset) - if not data then - return endpoints.handle_error(err_t) - end - - local next_page = offset and string.format("/certificates?offset=%s", - ngx.escape_uri(offset)) or ngx.null - - return helpers.responses.send_HTTP_OK { - data = data, - offset = offset, - next = next_page, - } - end, - - -- override to accept the server_names param when creating a certificate - POST = function(self, db, helpers) - local data, _, err_t = db.certificates:insert_with_name_list(self.args.post) - if err_t then - return endpoints.handle_error(err_t) - end - - return helpers.responses.send_HTTP_CREATED(data) - end, - }, - ["/certificates/:certificates"] = { - before = get_cert_by_server_name_or_id, + before = get_cert_id_from_sni, - -- override to include the server_names list when getting an individual certificate + -- override to include the snis list when getting an individual certificate GET = function(self, db, helpers) local pk = { id = self.params.certificates } @@ -61,20 +35,10 @@ return { return helpers.responses.send_HTTP_OK(cert) end, - - -- override to accept the server_names param when updating a certificate - PATCH = function(self, db, helpers) - local pk = { id = self.params.certificates } - local cert, _, err_t = db.certificates:update_with_name_list(pk, self.args.post) - if err_t then - return endpoints.handle_error(err_t) - end - return helpers.responses.send_HTTP_OK(cert) - end, }, - ["/certificates/:certificates/server_names"] = { - before = get_cert_by_server_name_or_id, + ["/certificates/:certificates/snis"] = { + before = get_cert_id_from_sni, }, } diff --git a/kong/api/routes/snis.lua b/kong/api/routes/snis.lua new file mode 100644 index 000000000000..187b259bb9c6 --- /dev/null +++ b/kong/api/routes/snis.lua @@ -0,0 +1,16 @@ +local function not_found(self, db, helpers) + return helpers.responses.send_HTTP_NOT_FOUND() +end + +return { + -- GET / PATCH / DELETE /snis/sni are the only methods allowed + + ["/snis"] = { + before = not_found, + }, + + ["/snis/:snis/certificate"] = { + before = not_found, + }, + +} diff --git a/kong/core/certificate.lua b/kong/core/certificate.lua index 8bae5af669ea..4515d09ced49 100644 --- a/kong/core/certificate.lua +++ b/kong/core/certificate.lua @@ -15,19 +15,19 @@ end local _M = {} -local function find_certificate(sn) - local row, err = singletons.db.server_names:select_by_name(sn) +local function find_certificate(sni_name) + local row, err = singletons.db.snis:select_by_name(sni_name) if err then return nil, err end if not row then - log(DEBUG, "no server name registered for client-provided name: '", - sn, "'") + log(DEBUG, "no SNI registered for client-provided name: '", + sni_name, "'") return true end - -- fetch SSL certificate for this server name + -- fetch SSL certificate for this sni local certificate, err = singletons.db.certificates:select(row.certificate) if err then @@ -35,7 +35,7 @@ local function find_certificate(sn) end if not certificate then - return nil, "no SSL certificate configured for server name: " .. sn + return nil, "no SSL certificate configured for sni: " .. sni_name end return { @@ -46,16 +46,16 @@ end function _M.execute() - -- retrieve server name or raw server IP + -- retrieve sni or raw server IP local sn, err = ssl.server_name() if err then - log(ERR, "could not retrieve Server Name Indication: ", err) + log(ERR, "could not retrieve SNI: ", err) return ngx.exit(ngx.ERROR) end if not sn then - log(DEBUG, "no Server Name Indication provided by client, serving ", + log(DEBUG, "no SNI provided by client, serving ", "default proxy SSL certificate") -- use fallback certificate return diff --git a/kong/core/handler.lua b/kong/core/handler.lua index d0d7d1f2b7e6..373fcc8bba5a 100644 --- a/kong/core/handler.lua +++ b/kong/core/handler.lua @@ -257,18 +257,18 @@ return { cache:invalidate("pem_ssl_certificates:" .. sn.name) cache:invalidate("parsed_ssl_certificates:" .. sn.name) - end, "crud", "server_names") + end, "crud", "snis") worker_events.register(function(data) log(DEBUG, "[events] SSL cert updated, invalidating cached certificates") local certificate = data.entity - local rows, err = db.server_names:for_certificate({ + local rows, err = db.snis:for_certificate({ id = certificate.id }) if not rows then - log(ERR, "[events] could not find associated server names for certificate: ", + log(ERR, "[events] could not find associated snis for certificate: ", err) end diff --git a/kong/dao/migrations/cassandra.lua b/kong/dao/migrations/cassandra.lua index e5fea85e86a5..56cc85223590 100644 --- a/kong/dao/migrations/cassandra.lua +++ b/kong/dao/migrations/cassandra.lua @@ -644,7 +644,7 @@ return { PRIMARY KEY (partition, id) ); - CREATE TABLE IF NOT EXISTS server_names( + CREATE TABLE IF NOT EXISTS snis( partition text, id uuid, name text, @@ -653,8 +653,8 @@ return { PRIMARY KEY (partition, id) ); - CREATE INDEX IF NOT EXISTS server_names_name_idx ON server_names(name); - CREATE INDEX IF NOT EXISTS server_names_certificate_id_idx ON server_names(certificate_id); + CREATE INDEX IF NOT EXISTS snis_name_idx ON snis(name); + CREATE INDEX IF NOT EXISTS snis_certificate_id_idx ON snis(certificate_id); ]], down = nil }, @@ -707,8 +707,8 @@ return { partition_keys = { "name", "ssl_certificate_id" }, } - local server_names_def = { - name = "server_names", + local snis_def = { + name = "snis", columns = { partition = "text", id = "uuid", @@ -721,8 +721,8 @@ return { local _, err = migration_helpers.cassandra.copy_records(dao, ssl_servers_names_def, - server_names_def, { - partition = function() return cassandra.text("server_names") end, + snis_def, { + partition = function() return cassandra.text("snis") end, id = function() return cassandra.uuid(utils.uuid(3)) end, name = "name", certificate_id = "ssl_certificate_id", diff --git a/kong/dao/migrations/postgres.lua b/kong/dao/migrations/postgres.lua index c697897b0ce8..2ec929ccd2cf 100644 --- a/kong/dao/migrations/postgres.lua +++ b/kong/dao/migrations/postgres.lua @@ -695,20 +695,20 @@ return { down = nil }, { - name = "2018-03-27-123400_prepare_certs_and_server_names", + name = "2018-03-27-123400_prepare_certs_and_snis", up = [[ DO $$ BEGIN ALTER TABLE ssl_certificates RENAME TO certificates; - ALTER TABLE ssl_servers_names RENAME TO server_names; + ALTER TABLE ssl_servers_names RENAME TO snis; EXCEPTION WHEN duplicate_table THEN -- Do nothing, accept existing state END$$; DO $$ BEGIN - ALTER TABLE server_names RENAME COLUMN ssl_certificate_id TO certificate_id; - ALTER TABLE server_names ADD COLUMN id uuid; + ALTER TABLE snis RENAME COLUMN ssl_certificate_id TO certificate_id; + ALTER TABLE snis ADD COLUMN id uuid; EXCEPTION WHEN undefined_column THEN -- Do nothing, accept existing state END$$; @@ -716,48 +716,51 @@ return { down = nil }, { - name = "2018-03-27-125400_fill_in_server_names_ids", + name = "2018-03-27-125400_fill_in_snis_ids", up = function(_, _, dao) + local fmt = string.format + local rows, err = dao.db:query([[ - SELECT * FROM server_names; + SELECT * FROM snis; ]]) if err then return err end + local sql_buffer = { "BEGIN;" } + local len = #rows + for i = 1, len do + sql_buffer[i + 1] = fmt("UPDATE snis SET id = '%s' WHERE name = '%s';", + utils.uuid(), + rows[i].name) + end + sql_buffer[len + 2] = "COMMIT;" - local fmt = string.format - - for _, row in ipairs(rows) do - local sql = fmt("UPDATE server_names SET id = '%s' WHERE name = '%s';", - utils.uuid(), - row.name) - local _, err = dao.db:query(sql) - if err then - return err - end + local _, err = dao.db:query(table.concat(sql_buffer)) + if err then + return err end end, down = nil }, { - name = "2018-03-27-130400_make_ids_primary_keys_in_server_names", + name = "2018-03-27-130400_make_ids_primary_keys_in_snis", up = [[ - ALTER TABLE server_names + ALTER TABLE snis DROP CONSTRAINT IF EXISTS ssl_servers_names_pkey; - ALTER TABLE server_names - DROP CONSTRAINT IF EXISTS ssl_server_names_ssl_certificate_id_fkey; + ALTER TABLE snis + DROP CONSTRAINT IF EXISTS ssl_servers_names_ssl_certificate_id_fkey; DO $$ BEGIN - ALTER TABLE server_names - ADD CONSTRAINT server_names_name_unique UNIQUE(name); + ALTER TABLE snis + ADD CONSTRAINT snis_name_unique UNIQUE(name); - ALTER TABLE server_names + ALTER TABLE snis ADD PRIMARY KEY (id); - ALTER TABLE server_names - ADD CONSTRAINT server_names_certificate_id_fkey + ALTER TABLE snis + ADD CONSTRAINT snis_certificate_id_fkey FOREIGN KEY (certificate_id) REFERENCES certificates; EXCEPTION WHEN duplicate_table THEN diff --git a/kong/db/dao/certificates.lua b/kong/db/dao/certificates.lua index e7ab877304a4..d0c7a34fc086 100644 --- a/kong/db/dao/certificates.lua +++ b/kong/db/dao/certificates.lua @@ -2,29 +2,28 @@ local singletons = require "kong.singletons" local cjson = require "cjson" local utils = require "kong.tools.utils" --- Get an array of server names from either a string (split+sort), +-- Get an array of SNI names from either -- an array(sort) or ngx.null(return {}) -- Returns an error if the list has duplicates -- Returns nil if input is falsy. local function parse_name_list(input, errors) local name_list - if type(input) == "string" then - name_list = utils.split(input, ",") - elseif type(input) == "table" then + if type(input) == "table" then name_list = utils.shallow_copy(input) + elseif input == ngx.null then name_list = {} - end - if not name_list then + else return nil end local found = {} for _, name in ipairs(name_list) do if found[name] then - local msg = "duplicate server name in request: " .. name - local err_t = errors:invalid_input(msg) + local err_t = errors:schema_violation({ + snis = name .. " is duplicated", + }) return nil, tostring(err_t), err_t end found[name] = true @@ -38,32 +37,32 @@ end local _Certificates = {} -- Creates a certificate --- If the provided cert has a field called "server_names" it will be used to generate server +-- If the provided cert has a field called "snis" it will be used to generate server -- names associated to the cert, after being parsed by parse_name_list. --- Returns a certificate with the server_names sorted alphabetically. -function _Certificates:insert_with_name_list(cert) +-- Returns a certificate with the snis sorted alphabetically. +function _Certificates:insert(cert) local db = singletons.db - local name_list, err, err_t = parse_name_list(cert.server_names, self.errors) + local name_list, err, err_t = parse_name_list(cert.snis, self.errors) if err then return nil, err, err_t end if name_list then - local ok, err, err_t = db.server_names:check_list_is_new(name_list) + local ok, err, err_t = db.snis:check_list_is_new(name_list) if not ok then return nil, err, err_t end end - cert.server_names = nil - cert, err, err_t = assert(self:insert(cert)) + cert.snis = nil + cert, err, err_t = self.super.insert(self, cert) if not cert then return nil, err, err_t end - cert.server_names = name_list or cjson.empty_array + cert.snis = name_list or cjson.empty_array if name_list then - local ok, err, err_t = db.server_names:insert_list({id = cert.id}, name_list) + local ok, err, err_t = db.snis:insert_list({id = cert.id}, name_list) if not ok then return nil, err, err_t end @@ -72,24 +71,24 @@ function _Certificates:insert_with_name_list(cert) return cert end --- Updates a certificate --- If the cert has a "server_names" attribute it will be used to update the server names +-- Update override +-- If the cert has a "snis" attribute it will be used to update the SNIs -- associated to the cert. --- * If the cert had any names associated which are not on `server_names`, they will be +-- * If the cert had any names associated which are not on `snis`, they will be -- removed. -- * Any new certificates will be added to the db. -- Returns an error if any of the new certificates where already assigned to a cert different -- from the one identified by cert_pk -function _Certificates:update_with_name_list(cert_pk, cert) +function _Certificates:update(cert_pk, cert) local db = singletons.db - local name_list, err, err_t = parse_name_list(cert.server_names, self.errors) + local name_list, err, err_t = parse_name_list(cert.snis, self.errors) if err then return nil, err, err_t end if name_list then local ok, err, err_t = - db.server_names:check_list_is_new_or_in_cert(cert_pk, name_list) + db.snis:check_list_is_new_or_in_cert(cert_pk, name_list) if not ok then return nil, err, err_t end @@ -97,24 +96,24 @@ function _Certificates:update_with_name_list(cert_pk, cert) -- update certificate if necessary if cert.key or cert.cert then - cert.server_names = nil - cert, err, err_t = self:update(cert_pk, cert) + cert.snis = nil + cert, err, err_t = self.super.update(self, cert_pk, cert) if err then return nil, err, err_t end end if name_list then - cert.server_names = name_list + cert.snis = name_list - local ok, err, err_t = db.server_names:update_list(cert_pk, name_list) + local ok, err, err_t = db.snis:update_list(cert_pk, name_list) if not ok then return nil, err, err_t end else - cert.server_names, err, err_t = db.server_names:list_for_certificate(cert_pk) - if not cert.server_names then + cert.snis, err, err_t = db.snis:list_for_certificate(cert_pk) + if not cert.snis then return nil, err, err_t end end @@ -122,25 +121,10 @@ function _Certificates:update_with_name_list(cert_pk, cert) return cert end --- Returns a single certificate provided one of its server names. Can return nil -function _Certificates:select_by_server_name(name) - local db = singletons.db - - local sn, err, err_t = db.server_names:select_by_name(name) - if err then - return nil, err, err_t - end - if not sn then - local err_t = self.errors:not_found({ name = name }) - return nil, tostring(err_t), err_t - end - - return self:select(sn.certificate) -end -- Returns the certificate identified by cert_pk but adds the --- `server_names` pseudo attribute to it. It is an array of strings --- representing the server names associated to the certificate. +-- `snis` pseudo attribute to it. It is an array of strings +-- representing the SNIs associated to the certificate. function _Certificates:select_with_name_list(cert_pk) local db = singletons.db @@ -154,7 +138,7 @@ function _Certificates:select_with_name_list(cert_pk) return nil, tostring(err_t), err_t end - cert.server_names, err, err_t = db.server_names:list_for_certificate(cert_pk) + cert.snis, err, err_t = db.snis:list_for_certificate(cert_pk) if err_t then return nil, err, err_t end @@ -162,42 +146,42 @@ function _Certificates:select_with_name_list(cert_pk) return cert end --- Returns a page of certificates, each with the `server_names` pseudo-attribute +-- Returns a page of certificates, each with the `snis` pseudo-attribute -- associated to them. This method does N+1 queries, but for now we are limited --- by the DAO's select options (we can't query for "all the server names for this +-- by the DAO's select options (we can't query for "all the SNIs for this -- list of certificate ids" in one go). -function _Certificates:page_with_name_list(size, offset) +function _Certificates:page(size, offset) local db = singletons.db - local certs, err, err_t, offset = self:page(size, offset) + local certs, err, err_t, offset = self.super.page(self, size, offset) if not certs then return nil, err, err_t end for i=1, #certs do local cert = certs[i] - local server_names, err, err_t = - db.server_names:list_for_certificate({ id = cert.id }) - if not server_names then + local snis, err, err_t = + db.snis:list_for_certificate({ id = cert.id }) + if not snis then return nil, err, err_t end - cert.server_names = server_names + cert.snis = snis end return certs, nil, nil, offset end --- Overrides the default delete function by cascading-deleting all the server names +-- Overrides the default delete function by cascading-deleting all the SNIs -- associated to the certificate function _Certificates:delete(cert_pk) local db = singletons.db local name_list, err, err_t = - db.server_names:list_for_certificate(cert_pk) + db.snis:list_for_certificate(cert_pk) if not name_list then return nil, err, err_t end - local ok, err, err_t = db.server_names:delete_list(name_list) + local ok, err, err_t = db.snis:delete_list(name_list) if not ok then return nil, err, err_t end diff --git a/kong/db/dao/server_names.lua b/kong/db/dao/snis.lua similarity index 69% rename from kong/db/dao/server_names.lua rename to kong/db/dao/snis.lua index c5b258ef096b..311354101809 100644 --- a/kong/db/dao/server_names.lua +++ b/kong/db/dao/snis.lua @@ -1,8 +1,8 @@ local singletons = require "kong.singletons" +local cjson = require "cjson" --- A "name_list" is an array of server names like { "example.com", "foo.com" } --- All methods containing "list" on their name act on name lists, not on lists --- of complete ServerName entities +-- A "name_list" is an array of names like { "example.com", "foo.com" } +-- All methods containing "list" on their name act on name lists, not on lists of complete SNI entities -- { "a", "b", "c" } - { "a", "c" } = { "b" } local function list_diff(list1, list2) @@ -24,18 +24,18 @@ local function list_diff(list1, list2) end -local _ServerNames = {} +local _SNIs = {} -- Truthy if all the names on the list don't exist on the db -function _ServerNames:check_list_is_new(name_list) +function _SNIs:check_list_is_new(name_list) -- when creating a new cert (no cert_id provided): - -- dont add the certificate or any names if we have an server name conflict + -- dont add the certificate or any names if we have a name conflict -- its fairly inefficient that we have to loop twice over the datastore -- but no support for OR queries means we gotsta! for i=1, #name_list do local name = name_list[i] - local row, err, err_t = singletons.db.server_names:select_by_name(name) + local row, err, err_t = singletons.db.snis:select_by_name(name) if err then return nil, err, err_t end @@ -44,40 +44,43 @@ function _ServerNames:check_list_is_new(name_list) -- Note: it could be that the name is not associated with any -- certificate, but we don't handle this case. (for PostgreSQL -- only, as C* requires a cert_id for its partition key). - local msg = "Server name already exists: " .. name - local err_t = self.errors:conflicting_input(msg) + local err_t = self.errors:schema_violation({ + snis = name .. " already exists", + }) return nil, tostring(err_t), err_t end end - return 1 + return true end -- Truthy if all the names on the list don't exist on the db or exist but are -- associated to the given certificate -function _ServerNames:check_list_is_new_or_in_cert(cert_pk, name_list) +function _SNIs:check_list_is_new_or_in_cert(cert_pk, name_list) for i=1, #name_list do local row, err, err_t = self:select_by_name(name_list[i]) if err then return nil, err, err_t end if row and row.certificate.id ~= cert_pk.id then - local msg = "Server Name '" .. row.name .. - "' already associated with existing " .. - "certificate (" .. row.certificate.id .. ")" - local err_t = self.errors:conflicting_input(msg) + local msg = row.name .. + " already associated with existing " .. + "certificate '" .. row.certificate.id .. "'" + local err_t = self.errors:schema_violation({ + snis = msg, + }) return nil, tostring(err_t), err_t end end - return 1 + return true end --- Creates one instance of ServerName for each name in name_list +-- Creates one instance of SNI for each name in name_list -- All created instances will be associated to the given certificate -function _ServerNames:insert_list(cert_pk, name_list) +function _SNIs:insert_list(cert_pk, name_list) for _, name in ipairs(name_list) do local _, err, err_t = self:insert({ name = name, @@ -88,12 +91,12 @@ function _ServerNames:insert_list(cert_pk, name_list) end end - return 1 + return true end --- Deletes all server names on the given name list -function _ServerNames:delete_list(name_list) +-- Deletes all SNIs on the given name list +function _SNIs:delete_list(name_list) local err_list = {} local errors_len = 0 local first_err_t = nil @@ -110,13 +113,13 @@ function _ServerNames:delete_list(name_list) return nil, table.concat(err_list, ","), first_err_t end - return 1 + return true end -- Returns the name list for a given certificate -function _ServerNames:list_for_certificate(cert_pk) - local name_list = {} +function _SNIs:list_for_certificate(cert_pk) + local name_list = setmetatable({}, cjson.empty_array_mt) local rows, err, err_t = self:for_certificate(cert_pk) if err then return nil, err, err_t @@ -131,9 +134,9 @@ end -- Replaces the names of a given certificate --- It does not try to insert server names which are already inserted --- It does not try to delete server names which don't exist -function _ServerNames:update_list(cert_pk, new_list) +-- It does not try to insert SNIs which are already inserted +-- It does not try to delete SNIs which don't exist +function _SNIs:update_list(cert_pk, new_list) -- Get the names currently associated to the certificate local current_list, err, err_t = self:list_for_certificate(cert_pk) if not current_list then @@ -152,8 +155,8 @@ function _ServerNames:update_list(cert_pk, new_list) -- returning 4xx here risks invalid states and is confusing to the user self:delete_list(delete_list) - return 1 + return true end -return _ServerNames +return _SNIs diff --git a/kong/db/errors.lua b/kong/db/errors.lua index e9b037c7985f..401ac4806232 100644 --- a/kong/db/errors.lua +++ b/kong/db/errors.lua @@ -32,8 +32,6 @@ local ERRORS = { NOT_FOUND = 6, -- WHERE clause leads nowhere (HTTP 404) INVALID_OFFSET = 7, -- page(size, offset) is invalid DATABASE_ERROR = 8, -- connection refused or DB error (HTTP 500) - CONFLICTING_INPUT = 9, -- user-provided data generated a conflict (HTTP 409) - INVALID_INPUT = 10, -- user-provided data is not valid (HTTP 400) } @@ -49,8 +47,6 @@ local ERRORS_NAMES = { [ERRORS.NOT_FOUND] = "not found", [ERRORS.INVALID_OFFSET] = "invalid offset", [ERRORS.DATABASE_ERROR] = "database error", - [ERRORS.CONFLICTING_INPUT] = "conflicting input", - [ERRORS.INVALID_INPUT] = "invalid input", } @@ -299,24 +295,6 @@ function _M:unique_violation(unique_key) end -function _M:conflicting_input(message) - if type(message) ~= "string" then - error("message must be a string", 2) - end - - return new_err_t(self, ERRORS.CONFLICTING_INPUT, message) -end - - -function _M:invalid_input(message) - if type(message) ~= "string" then - error("message must be a string", 2) - end - - return new_err_t(self, ERRORS.INVALID_INPUT, message) -end - - function _M:invalid_offset(offset, err) if type(offset) ~= "string" then error("offset must be a string", 2) diff --git a/kong/db/init.lua b/kong/db/init.lua index e997dc64a20d..bbd1ed4c2f5b 100644 --- a/kong/db/init.lua +++ b/kong/db/init.lua @@ -3,7 +3,6 @@ local Entity = require "kong.db.schema.entity" local Errors = require "kong.db.errors" local Strategies = require "kong.db.strategies" local MetaSchema = require "kong.db.schema.metaschema" -local pl_pretty = require "pl.pretty" local fmt = string.format @@ -22,7 +21,7 @@ local CORE_ENTITIES = { "routes", "services", "certificates", - "server_names", + "snis", } @@ -41,6 +40,10 @@ function DB.new(kong_config, strategy) error("strategy must be a string", 2) end + -- load errors + + local errors = Errors.new(strategy) + local schemas = {} do @@ -55,17 +58,13 @@ function DB.new(kong_config, strategy) local ok, err_t = MetaSchema:validate(entity_schema) if not ok then return nil, fmt("schema of entity '%s' is invalid: %s", entity_name, - pl_pretty.write(err_t)) + tostring(errors:schema_violation(err_t))) end schemas[entity_name] = Entity.new(entity_schema) end end - -- load errors - - local errors = Errors.new(strategy) - -- load strategy local connector, strategies, err = Strategies.new(kong_config, strategy, diff --git a/kong/db/schema/entities/server_names.lua b/kong/db/schema/entities/snis.lua similarity index 83% rename from kong/db/schema/entities/server_names.lua rename to kong/db/schema/entities/snis.lua index c9fa7a5fdfcb..94a34e634ee2 100644 --- a/kong/db/schema/entities/server_names.lua +++ b/kong/db/schema/entities/snis.lua @@ -1,9 +1,9 @@ local typedefs = require "kong.db.schema.typedefs" return { - name = "server_names", + name = "snis", primary_key = { "id" }, - dao = "kong.db.dao.server_names", + dao = "kong.db.dao.snis", fields = { { id = typedefs.uuid, }, diff --git a/spec-old-api/02-integration/05-proxy/05-ssl_spec.lua b/spec-old-api/02-integration/05-proxy/05-ssl_spec.lua index 417f1d82928b..690dd17bbf98 100644 --- a/spec-old-api/02-integration/05-proxy/05-ssl_spec.lua +++ b/spec-old-api/02-integration/05-proxy/05-ssl_spec.lua @@ -79,7 +79,7 @@ describe("SSL", function() body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "example.com,ssl1.com", + snis = { "example.com", "ssl1.com" }, }, headers = { ["Content-Type"] = "application/json" }, }) diff --git a/spec-old-api/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua b/spec-old-api/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua index d665b6b610f7..c80ad10d05b5 100644 --- a/spec-old-api/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua +++ b/spec-old-api/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua @@ -215,7 +215,7 @@ describe("core entities are invalidated with db: #" .. strategy, function() -- ssl_certificates ------------------- - describe("#o ssl_certificates / server_names", function() + describe("ssl_certificates / snis", function() local function get_cert(port, sni) local pl_utils = require "pl.utils" @@ -244,14 +244,14 @@ describe("core entities are invalidated with db: #" .. strategy, function() assert.matches("CN=localhost", cert_2, nil, true) end) - it("on certificate+Server Name create", function() + it("on certificate+SNI create", function() local admin_res = assert(admin_client_1:send { method = "POST", path = "/certificates", body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "ssl-example.com", + snis = "ssl-example.com", }, headers = { ["Content-Type"] = "application/json", @@ -273,7 +273,7 @@ describe("core entities are invalidated with db: #" .. strategy, function() it("on certificate delete+re-creation", function() -- TODO: PATCH/PUT update are currently not possible - -- with the admin API because server_names have their name as their + -- with the admin API because snis have their name as their -- primary key and the DAO has limited support for such updates. local admin_res = assert(admin_client_1:send { @@ -288,7 +288,7 @@ describe("core entities are invalidated with db: #" .. strategy, function() body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "new-ssl-example.com", + snis = "new-ssl-example.com", }, headers = { ["Content-Type"] = "application/json", @@ -316,7 +316,7 @@ describe("core entities are invalidated with db: #" .. strategy, function() it("on certificate update", function() -- update our certificate *without* updating the - -- attached Server Name + -- attached SNI local admin_res = assert(admin_client_1:send { method = "PATCH", @@ -343,19 +343,19 @@ describe("core entities are invalidated with db: #" .. strategy, function() assert.matches("CN=ssl-alt.com", cert_2, nil, true) end) - pending("on Server Name update", function() - -- Pending: currently, server_names cannot be updated: + pending("on SNI update", function() + -- Pending: currently, snis cannot be updated: -- - A PATCH updating the name property would not work, since -- the URI path expects the current name, and so does the -- query fetchign the row to be updated -- -- -- - -- update our Server Name but leave certificate untouched + -- update our SNI but leave certificate untouched local admin_res = assert(admin_client_1:send { method = "PATCH", - path = "/server_names/new-ssl-example.com", + path = "/snis/new-ssl-example.com", body = { name = "updated-sni.com", }, diff --git a/spec/02-integration/01-helpers/01-blueprints_spec.lua b/spec/02-integration/01-helpers/01-blueprints_spec.lua index 4eefa3e4d02a..1b5880443364 100644 --- a/spec/02-integration/01-helpers/01-blueprints_spec.lua +++ b/spec/02-integration/01-helpers/01-blueprints_spec.lua @@ -113,12 +113,12 @@ dao_helpers.for_each_dao(function(kong_config) assert.equal("string", type(c.key)) end) - it("inserts server names", function() + it("inserts snis", function() local c = bp.certificates:insert() - local s = bp.server_names:insert({ certificate = c }) + local s = bp.snis:insert({ certificate = c }) assert.equal("string", type(s.name)) - local s2 = bp.server_names:insert() + local s2 = bp.snis:insert() assert.equal("string", type(s2.name)) assert.equal("string", type(s2.certificate.id)) end) diff --git a/spec/02-integration/04-admin_api/06-certificates_routes_spec.lua b/spec/02-integration/04-admin_api/06-certificates_routes_spec.lua index cabb583d12f0..0fb71f671d85 100644 --- a/spec/02-integration/04-admin_api/06-certificates_routes_spec.lua +++ b/spec/02-integration/04-admin_api/06-certificates_routes_spec.lua @@ -12,6 +12,27 @@ local function it_content_types(title, fn) end +local function get_snis_lists(certs) + local lists = {} + for i=1, #certs do + lists[i] = certs[i].snis + end + + table.sort(lists, function(a,b) + if not a[1] then + return true + end + if not b[1] then + return false + end + + return a[1] < b[1] + end) + + return lists +end + + for _, strategy in helpers.each_strategy() do describe("Admin API: #" .. strategy, function() @@ -49,9 +70,9 @@ describe("Admin API: #" .. strategy, function() body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "foo.com,bar.com", + snis = { "foo.com", "bar.com" }, }, - headers = { ["Content-Type"] = "application/x-www-form-urlencoded" }, + headers = { ["Content-Type"] = "application/json" }, }) assert.res_status(201, res) end) @@ -64,35 +85,30 @@ describe("Admin API: #" .. strategy, function() assert.equal(1, #json.data) assert.is_string(json.data[1].cert) assert.is_string(json.data[1].key) - assert.same({ "bar.com", "foo.com" }, json.data[1].server_names) + assert.same({ "bar.com", "foo.com" }, json.data[1].snis) end) end) describe("POST", function() - it("returns a conflict when duplicated server_names are present in the request", function() + it("returns a conflict when duplicated snis are present in the request", function() local res = client:post("/certificates", { body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "foobar.com,baz.com,foobar.com", + snis = { "foobar.com", "baz.com", "foobar.com" }, }, - headers = { ["Content-Type"] = "application/x-www-form-urlencoded" }, + headers = { ["Content-Type"] = "application/json" }, }) local body = assert.res_status(400, res) local json = cjson.decode(body) - assert.equals("duplicate server name in request: foobar.com", json.message) + assert.equals("schema violation (snis: foobar.com is duplicated)", json.message) - -- make sure we dont add any server_names - res = client:get("/server_names") - body = assert.res_status(200, res) - json = cjson.decode(body) - assert.equal(2, #json.data) - - -- make sure we didnt add the certificate + -- make sure we didnt add the certificate, or any snis res = client:get("/certificates") body = assert.res_status(200, res) json = cjson.decode(body) assert.equal(1, #json.data) + assert.same({ "bar.com", "foo.com" }, json.data[1].snis) end) it("returns a conflict when a pre-existing sni is detected", function() @@ -100,38 +116,29 @@ describe("Admin API: #" .. strategy, function() body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "foo.com", + snis = { "foo.com" }, }, - headers = { ["Content-Type"] = "application/x-www-form-urlencoded" }, + headers = { ["Content-Type"] = "application/json" }, }) - local body = assert.res_status(409, res) + local body = assert.res_status(400, res) local json = cjson.decode(body) - assert.equals("Server name already exists: foo.com", json.message) - - -- make sure we only have two server_names - res = client:get("/server_names") - body = assert.res_status(200, res) - json = cjson.decode(body) - assert.equal(2, #json.data) - local names = { json.data[1].name, json.data[2].name } - table.sort(names) - assert.same({ "bar.com", "foo.com" }, names) + assert.equals("schema violation (snis: foo.com already exists)", json.message) - -- make sure we only have one certificate + -- make sure we only have one certificate, with two snis res = client:get("/certificates") body = assert.res_status(200, res) json = cjson.decode(body) assert.equal(1, #json.data) - assert.same({ "bar.com", "foo.com" }, json.data[1].server_names) + assert.same({ "bar.com", "foo.com" }, json.data[1].snis) end) - it_content_types("creates a certificate and returns it with the server_names pseudo-property", function(content_type) + it_content_types("creates a certificate and returns it with the snis pseudo-property", function(content_type) return function() local res = client:post("/certificates", { body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "foobar.com,baz.com", + snis = { "foobar.com", "baz.com" }, }, headers = { ["Content-Type"] = content_type }, }) @@ -140,11 +147,11 @@ describe("Admin API: #" .. strategy, function() local json = cjson.decode(body) assert.is_string(json.cert) assert.is_string(json.key) - assert.same({ "baz.com", "foobar.com" }, json.server_names) + assert.same({ "baz.com", "foobar.com" }, json.snis) end end) - it_content_types("returns server_names as [] when none is set", function(content_type) + it_content_types("returns snis as [] when none is set", function(content_type) return function() local res = client:post("/certificates", { body = { @@ -158,29 +165,42 @@ describe("Admin API: #" .. strategy, function() local json = cjson.decode(body) assert.is_string(json.cert) assert.is_string(json.key) - assert.matches('"server_names":[]', body, nil, true) + assert.matches('"snis":[]', body, nil, true) end end) end) end) - describe("/certificates/cert_id_or_server_name", function() + describe("/certificates/cert_id_or_sni", function() + local certificate + before_each(function() assert(db:truncate()) local res = client:post("/certificates", { body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "foo.com,bar.com", + snis = { "foo.com", "bar.com" }, }, - headers = { ["Content-Type"] = "application/x-www-form-urlencoded" }, + headers = { ["Content-Type"] = "application/json" }, }) - assert.res_status(201, res) + local body = assert.res_status(201, res) + certificate = cjson.decode(body) end) describe("GET", function() - it("retrieves a certificate by Server Name", function() + it("retrieves a certificate by id", function() + local res1 = client:get("/certificates/" .. certificate.id) + local body1 = assert.res_status(200, res1) + local json1 = cjson.decode(body1) + + assert.is_string(json1.cert) + assert.is_string(json1.key) + assert.same({ "bar.com", "foo.com" }, json1.snis) + end) + + it("retrieves a certificate by sni", function() local res1 = client:get("/certificates/foo.com") local body1 = assert.res_status(200, res1) local json1 = cjson.decode(body1) @@ -191,7 +211,7 @@ describe("Admin API: #" .. strategy, function() assert.is_string(json1.cert) assert.is_string(json1.key) - assert.same({ "bar.com", "foo.com" }, json1.server_names) + assert.same({ "bar.com", "foo.com" }, json1.snis) assert.same(json1, json2) end) @@ -200,7 +220,7 @@ describe("Admin API: #" .. strategy, function() assert.res_status(404, res) end) - it("returns 404 for a random non-existing Server Name", function() + it("returns 404 for a random non-existing sni", function() local res = client:get("/certificates/doesntexist.com") assert.res_status(404, res) end) @@ -217,9 +237,9 @@ describe("Admin API: #" .. strategy, function() body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "foo.com", + snis = { "foo.com" }, }, - headers = { ["Content-Type"] = "application/x-www-form-urlencoded" }, + headers = { ["Content-Type"] = "application/json" }, }) local body = assert.res_status(201, res) cert_foo = cjson.decode(body) @@ -228,15 +248,32 @@ describe("Admin API: #" .. strategy, function() body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "bar.com", + snis = { "bar.com" }, }, - headers = { ["Content-Type"] = "application/x-www-form-urlencoded" }, + headers = { ["Content-Type"] = "application/json" }, }) local body = assert.res_status(201, res) cert_bar = cjson.decode(body) end) - it_content_types("updates a certificate by Server Name", function(content_type) + it_content_types("updates a certificate by cert id", function(content_type) + return function() + local res = client:patch("/certificates/" .. cert_foo.id, { + body = { + cert = "foo_cert", + key = "foo_key", + }, + headers = { ["Content-Type"] = content_type } + }) + + local body = assert.res_status(200, res) + local json = cjson.decode(body) + + assert.equal("foo_cert", json.cert) + end + end) + + it_content_types("updates a certificate by sni", function(content_type) return function() local res = client:patch("/certificates/foo.com", { body = { @@ -258,55 +295,41 @@ describe("Admin API: #" .. strategy, function() body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "baz.com", + snis = { "baz.com" }, }, - headers = { ["Content-Type"] = "application/x-www-form-urlencoded" }, + headers = { ["Content-Type"] = "application/json" }, }) assert.res_status(404, res) - -- make sure we did not add any sni - res = client:get("/server_names") + -- make sure we did not add any certificate or sni + res = client:get("/certificates") local body = assert.res_status(200, res) local json = cjson.decode(body) assert.equal(2, #json.data) - - -- make sure we did not add any certificate - res = client:get("/certificates") - body = assert.res_status(200, res) - json = cjson.decode(body) - assert.equal(2, #json.data) + assert.same({ { "bar.com" }, { "foo.com" } }, get_snis_lists(json.data)) end) - it("updates server_names associated with a certificate", function() + it("updates snis associated with a certificate", function() local res = client:patch("/certificates/" .. cert_foo.id, { - body = { server_names = "baz.com" }, - headers = { ["Content-Type"] = "application/x-www-form-urlencoded" }, + body = { snis = { "baz.com" }, }, + headers = { ["Content-Type"] = "application/json" }, }) local body = assert.res_status(200, res) local json = cjson.decode(body) + assert.same({ "baz.com" }, json.snis) - assert.same({ "baz.com" }, json.server_names) - - -- make sure number of server_names don't change - -- since we delete foo.com and added baz.com - res = client:get("/server_names") - body = assert.res_status(200, res) - json = cjson.decode(body) - assert.equal(2, #json.data) - local names = { json.data[1].name, json.data[2].name } - table.sort(names) - assert.are.same( { "bar.com", "baz.com" } , names) - - -- make sure we did not add any certificate + -- make sure we did not add any certificate, and that the snis + -- are correct res = client:get("/certificates") body = assert.res_status(200, res) json = cjson.decode(body) assert.equal(2, #json.data) + assert.same({ { "bar.com" }, { "baz.com" } }, get_snis_lists(json.data)) end) - it("updates only the certificate if no server_names are specified", function() + it("updates only the certificate if no snis are specified", function() local res = client:patch( "/certificates/" .. cert_bar.id, { body = { cert = "bar_cert", @@ -319,7 +342,7 @@ describe("Admin API: #" .. strategy, function() local json = cjson.decode(body) -- make sure certificate got updated and sni remains the same - assert.same({ "bar.com" }, json.server_names) + assert.same({ "bar.com" }, json.snis) assert.same("bar_cert", json.cert) assert.same("bar_key", json.key) @@ -330,118 +353,92 @@ describe("Admin API: #" .. strategy, function() assert.equal("bar_cert", json.cert) assert.equal("bar_key", json.key) - -- make sure number of server_names don't change - res = client:get("/server_names") - body = assert.res_status(200, res) - json = cjson.decode(body) - assert.equal(2, #json.data) - - -- make sure we did not add any certificate + -- make sure we did not add any certificate or sni res = client:get("/certificates") body = assert.res_status(200, res) json = cjson.decode(body) - assert.equal(2, #json.data) + assert.same({ { "bar.com" }, { "foo.com" } }, get_snis_lists(json.data)) end) - it("returns a conflict when duplicates server_names are present in the request", function() + it("returns a conflict when duplicated snis are present in the request", function() local res = client:patch("/certificates/" .. cert_bar.id, { body = { - server_names = "baz.com,baz.com", + snis = { "baz.com", "baz.com" }, }, - headers = { ["Content-Type"] = "application/x-www-form-urlencoded" }, + headers = { ["Content-Type"] = "application/json" }, }) local body = assert.res_status(400, res) local json = cjson.decode(body) - assert.equals("duplicate server name in request: baz.com", json.message) - - -- make sure number of server_names don't change - res = client:get("/server_names") - body = assert.res_status(200, res) - json = cjson.decode(body) - assert.equal(2, #json.data) + assert.equals("schema violation (snis: baz.com is duplicated)", json.message) - -- make sure we did not add any certificate + -- make sure we did not change certificates or snis res = client:get("/certificates") body = assert.res_status(200, res) json = cjson.decode(body) assert.equal(2, #json.data) + assert.same({ { "bar.com" }, { "foo.com" } }, get_snis_lists(json.data)) end) - it("returns a conflict when a pre-existing server name present in " .. + it("returns a conflict when a pre-existing sni present in " .. "the request is associated with another certificate", function() local res = client:patch("/certificates/" .. cert_bar.id, { body = { - server_names = "foo.com,baz.com", + snis = { "foo.com", "baz.com" }, }, - headers = { ["Content-Type"] = "application/x-www-form-urlencoded" }, + headers = { ["Content-Type"] = "application/json" }, }) - local body = assert.res_status(409, res) + local body = assert.res_status(400, res) local json = cjson.decode(body) - assert.equals("Server Name 'foo.com' already associated with " .. - "existing certificate (" .. cert_foo.id .. ")", + assert.equals("schema violation (snis: foo.com already associated with " .. + "existing certificate '" .. cert_foo.id .. "')", json.message) - -- make sure number of server_names don't change - res = client:get("/server_names") - body = assert.res_status(200, res) - json = cjson.decode(body) - assert.equal(2, #json.data) - - -- make sure we did not add any certificate + -- make sure we did not add any certificate or sni res = client:get("/certificates") body = assert.res_status(200, res) json = cjson.decode(body) assert.equal(2, #json.data) + assert.same({ { "bar.com" }, { "foo.com" } }, get_snis_lists(json.data)) end) - it("deletes all server_names from a certificate if server_names field is JSON null", function() + it("deletes all snis from a certificate if snis field is JSON null", function() -- Note: we currently do not support unsetting a field with -- form-urlencoded requests. This depends on upcoming work -- to the Admin API. We here follow the road taken by: -- https://github.com/Kong/kong/pull/2700 local res = client:patch("/certificates/" .. cert_bar.id, { body = { - server_names = ngx.null, + snis = ngx.null, }, headers = { ["Content-Type"] = "application/json" }, }) local body = assert.res_status(200, res) local json = cjson.decode(body) - assert.equal(0, #json.server_names) - assert.matches('"server_names":[]', body, nil, true) + assert.equal(0, #json.snis) + assert.matches('"snis":[]', body, nil, true) - -- make sure the sni was deleted - res = client:get("/server_names") - body = assert.res_status(200, res) - json = cjson.decode(body) - assert.equal(1, #json.data) - assert.equal("foo.com", json.data[1].name) - - -- make sure we did not add any certificate + -- make sure we did not add any certificate and the sni was deleted res = client:get("/certificates") body = assert.res_status(200, res) json = cjson.decode(body) assert.equal(2, #json.data) + assert.same({ {}, { "foo.com" } }, get_snis_lists(json.data)) end) end) describe("DELETE", function() - it("deletes a certificate and all related server_names", function() + it("deletes a certificate and all related snis", function() local res = client:delete("/certificates/foo.com") - assert.res_status(204, res) - res = client:get("/server_names/foo.com") - - assert.res_status(404, res) - - res = client:get("/server_names/bar.com") - - assert.res_status(404, res) + res = client:get("/certificates") + local body = assert.res_status(200, res) + local json = cjson.decode(body) + assert.equal(0, #json.data) end) it("deletes a certificate by id", function() @@ -456,14 +453,18 @@ describe("Admin API: #" .. strategy, function() local json = cjson.decode(body) local res = client:delete("/certificates/" .. json.id) - assert.res_status(204, res) + + res = client:get("/certificates") + local body = assert.res_status(200, res) + local json = cjson.decode(body) + assert.equal(1, #json.data) end) end) end) - describe("/server_names", function() + describe("/certificates/:certificate/snis", function() describe("POST", function() local certificate @@ -471,32 +472,48 @@ describe("Admin API: #" .. strategy, function() assert(db:truncate()) certificate = bp.certificates:insert() + bp.snis:insert({ + name = "ttt.com", + certificate = { id = certificate.id } + }) end) describe("errors", function() it("certificate doesn't exist", function() - local res = client:post("/server_names", { - body = { - name = "bar.com", - certificate = { id = "585e4c16-c656-11e6-8db9-5f512d8a12cd" }, + local res = client:post("/certificates/585e4c16-c656-11e6-8db9-5f512d8a12cd/snis", { + body = { + name = "bar.com", }, headers = { ["Content-Type"] = "application/json" }, }) - local body = assert.res_status(400, res) + local body = assert.res_status(404, res) local json = cjson.decode(body) - assert.same("the foreign key '{id=\"585e4c16-c656-11e6-8db9-5f512d8a12cd\"}' " .. - "does not reference an existing 'certificates' entity.", - json.message) + assert.same("Not found", json.message) end) end) - it_content_types("creates a Server Name", function(content_type) + it_content_types("creates a sni using a certificate id", function(content_type) return function() - local res = client:post("/server_names", { - body = { - name = "foo.com", - certificate = { id = certificate.id }, + local res = client:post("/certificates/" .. certificate.id .. "/snis", { + body = { + name = "foo.com", + }, + headers = { ["Content-Type"] = content_type }, + }) + + local body = assert.res_status(201, res) + local json = cjson.decode(body) + assert.equal("foo.com", json.name) + assert.equal(certificate.id, json.certificate.id) + end + end) + + it_content_types("creates a sni using a sni to id the certificate", function(content_type) + return function() + local res = client:post("/certificates/ttt.com/snis", { + body = { + name = "foo.com", }, headers = { ["Content-Type"] = content_type }, }) @@ -508,16 +525,15 @@ describe("Admin API: #" .. strategy, function() end end) - it("returns a conflict when an Server Name already exists", function() - bp.server_names:insert { + it("returns a conflict when an sni already exists", function() + bp.snis:insert { name = "foo.com", certificate = certificate, } - local res = client:post("/server_names", { + local res = client:post("/certificates/" .. certificate.id .. "/snis", { body = { - name = "foo.com", - certificate = { id = certificate.id }, + name = "foo.com", }, headers = { ["Content-Type"] = "application/json" }, }) @@ -529,15 +545,15 @@ describe("Admin API: #" .. strategy, function() end) describe("GET", function() - it("retrieves a list of server names", function() + it("retrieves a list of snis", function() assert(db:truncate()) local certificate = bp.certificates:insert() - bp.server_names:insert { + bp.snis:insert { name = "foo.com", certificate = certificate, } - local res = client:get("/server_names") + local res = client:get("/certificates/" .. certificate.id .. "/snis") local body = assert.res_status(200, res) local json = cjson.decode(body) assert.equal(1, #json.data) @@ -547,21 +563,28 @@ describe("Admin API: #" .. strategy, function() end) end) - describe("/server_names/:name", function() - local certificate + describe("/snis/:name", function() + local certificate, sni before_each(function() assert(db:truncate()) certificate = bp.certificates:insert() - bp.server_names:insert { + sni = bp.snis:insert { name = "foo.com", certificate = certificate, } end) describe("GET", function() - it("retrieves a Server Name", function() - local res = client:get("/server_names/foo.com") + it("retrieves a sni using the name", function() + local res = client:get("/snis/foo.com") + local body = assert.res_status(200, res) + local json = cjson.decode(body) + assert.equal("foo.com", json.name) + assert.equal(certificate.id, json.certificate.id) + end) + it("retrieves a sni using the id", function() + local res = client:get("/snis/" .. sni.id) local body = assert.res_status(200, res) local json = cjson.decode(body) assert.equal("foo.com", json.name) @@ -570,41 +593,38 @@ describe("Admin API: #" .. strategy, function() end) describe("PATCH", function() - do - local test = it - if strategy == "cassandra" then - test = pending - end + it("updates a sni", function() + local certificate_2 = bp.certificates:insert { + cert = "foo", + key = "bar", + } - test("updates a Server Name", function() - -- SKIP: this test fails with Cassandra because the PRIMARY KEY - -- used by the C* table is a composite of (name, - -- certificate_id), and hence, we cannot update the - -- certificate_id field because it is in the `SET` part of the - -- query built by the db, but in C*, one cannot change a value - -- from the clustering key. - local certificate_2 = bp.certificates:insert { - cert = "foo", - key = "bar", - } + local res = client:patch("/snis/foo.com", { + body = { + certificate = { id = certificate_2.id }, + }, + headers = { ["Content-Type"] = "application/json" }, + }) - local res = client:patch("/server_names/foo.com", { - body = { - certificate = { id = certificate_2.id }, - }, - headers = { ["Content-Type"] = "application/json" }, - }) + local body = assert.res_status(200, res) + local json = cjson.decode(body) + assert.equal(certificate_2.id, json.certificate.id) - local body = assert.res_status(200, res) - local json = cjson.decode(body) - assert.equal(certificate_2.id, json.certificate.id) - end) - end + local res = client:get("/certificates/" .. certificate.id) + local body = assert.res_status(200, res) + local json = cjson.decode(body) + assert.same({}, json.snis) + + local res = client:get("/certificates/" .. certificate_2.id) + local body = assert.res_status(200, res) + local json = cjson.decode(body) + assert.same({ "foo.com" }, json.snis) + end) end) describe("DELETE", function() - it("deletes a Server Name", function() - local res = client:delete("/server_names/foo.com") + it("deletes a sni", function() + local res = client:delete("/snis/foo.com") assert.res_status(204, res) end) end) diff --git a/spec/02-integration/05-proxy/05-ssl_spec.lua b/spec/02-integration/05-proxy/05-ssl_spec.lua index 0b75dc491050..0aca03743801 100644 --- a/spec/02-integration/05-proxy/05-ssl_spec.lua +++ b/spec/02-integration/05-proxy/05-ssl_spec.lua @@ -99,7 +99,7 @@ for _, strategy in helpers.each_strategy() do body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "example.com,ssl1.com", + snis = { "example.com", "ssl1.com" }, }, headers = { ["Content-Type"] = "application/json" }, }) diff --git a/spec/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua b/spec/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua index b0d469576fea..ce490dcd1c30 100644 --- a/spec/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua +++ b/spec/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua @@ -373,7 +373,7 @@ for _, strategy in helpers.each_strategy() do -- ssl_certificates ------------------- - describe("ssl_certificates / Server names", function() + describe("ssl_certificates / snis", function() local function get_cert(port, sn) local pl_utils = require "pl.utils" @@ -402,12 +402,12 @@ for _, strategy in helpers.each_strategy() do assert.matches("CN=localhost", cert_2, nil, true) end) - it("on certificate+Server name create", function() + it("on certificate+sni create", function() local admin_res = admin_client_1:post("/certificates", { body = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, - server_names = "ssl-example.com", + snis = { "ssl-example.com" }, }, headers = { ["Content-Type"] = "application/json" } }) @@ -427,7 +427,7 @@ for _, strategy in helpers.each_strategy() do it("on certificate delete+re-creation", function() -- TODO: PATCH update are currently not possible - -- with the admin API because server names have their name as their + -- with the admin API because snis have their name as their -- primary key and the DAO has limited support for such updates. local admin_res = admin_client_1:delete("/certificates/ssl-example.com") @@ -435,9 +435,9 @@ for _, strategy in helpers.each_strategy() do local admin_res = admin_client_1:post("/certificates", { body = { - cert = ssl_fixtures.cert, - key = ssl_fixtures.key, - server_names = "new-ssl-example.com", + cert = ssl_fixtures.cert, + key = ssl_fixtures.key, + snis = { "new-ssl-example.com" }, }, headers = { ["Content-Type"] = "application/json", @@ -465,7 +465,7 @@ for _, strategy in helpers.each_strategy() do it("on certificate update", function() -- update our certificate *without* updating the - -- attached server name + -- attached sni local admin_res = assert(admin_client_1:send { method = "PATCH", @@ -492,19 +492,19 @@ for _, strategy in helpers.each_strategy() do assert.matches("CN=ssl-alt.com", cert_2, nil, true) end) - pending("on server name update", function() - -- Pending: currently, server names cannot be updated: + pending("on sni update", function() + -- Pending: currently, snis cannot be updated: -- - A PATCH updating the name property would not work, since -- the URI path expects the current name, and so does the -- query fetchign the row to be updated -- -- -- - -- update our Server name but leave certificate untouched + -- update our sni but leave certificate untouched local admin_res = assert(admin_client_1:send { method = "PATCH", - path = "/server_names/new-ssl-example.com", + path = "/snis/new-ssl-example.com", body = { name = "updated-sn.com", }, diff --git a/spec/fixtures/blueprints.lua b/spec/fixtures/blueprints.lua index bc058957d2f6..8c07e9dfbef7 100644 --- a/spec/fixtures/blueprints.lua +++ b/spec/fixtures/blueprints.lua @@ -65,10 +65,10 @@ local _M = {} function _M.new(dao, db) local res = {} - local server_name_seq = new_sequence("server-name-%d") - res.server_names = new_blueprint(db.server_names, function(overrides) + local sni_seq = new_sequence("server-name-%d") + res.snis = new_blueprint(db.snis, function(overrides) return { - name = server_name_seq:next(), + name = sni_seq:next(), certificate = overrides.certificate or res.certificates:insert(), } end)