Skip to content

Commit

Permalink
fix(db) iterate through all snis
Browse files Browse the repository at this point in the history
Fixes the iteration of SNIs, so that all SNIs of a certificate
are iterated through, and not only the first page.

Includes a regression test.
  • Loading branch information
hishamhm authored and thibaultcha committed Aug 23, 2018
1 parent 8216936 commit be50425
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 32 deletions.
13 changes: 7 additions & 6 deletions kong/db/dao/snis.lua
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,13 @@ end
-- Returns the name list for a given certificate
function _SNIs:list_for_certificate(cert_pk)
local name_list = setmetatable({}, cjson.empty_array_mt)
local rows, err, err_t = self:page_for_certificate(cert_pk)
if err then
return nil, err, err_t
end
for i = 1, #rows do
name_list[i] = rows[i].name

for sni, err, err_t in self:each_for_certificate(cert_pk) do
if err then
return nil, err, err_t
end

table.insert(name_list, sni.name)
end

table.sort(name_list)
Expand Down
16 changes: 6 additions & 10 deletions kong/runloop/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -271,16 +271,12 @@ return {
log(DEBUG, "[events] SSL cert updated, invalidating cached certificates")
local certificate = data.entity

local rows, err = db.snis:page_for_certificate({
id = certificate.id
})
if not rows then
log(ERR, "[events] could not find associated snis for certificate: ",
err)
end

for i = 1, #rows do
local sn = rows[i]
for sn, err in db.snis:each_for_certificate({ id = certificate.id }) do
if err then
log(ERR, "[events] could not find associated snis for certificate: ",
err)
break
end

cache:invalidate("pem_ssl_certificates:" .. sn.name)
cache:invalidate("parsed_ssl_certificates:" .. sn.name)
Expand Down
54 changes: 38 additions & 16 deletions spec/02-integration/04-admin_api/06-certificates_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -65,34 +65,56 @@ describe("Admin API: #" .. strategy, function()
end)

describe("/certificates", function()
before_each(function()
assert(db:truncate("certificates"))
assert(db:truncate("snis"))

local res = client:post("/certificates", {
body = {
cert = ssl_fixtures.cert,
key = ssl_fixtures.key,
snis = { "foo.com", "bar.com" },
},
headers = { ["Content-Type"] = "application/json" },
})
assert.res_status(201, res)
end)

describe("GET", function()
it("retrieves all certificates", function()

it("retrieves all certificates with snis", function()

assert(db:truncate("certificates"))
assert(db:truncate("snis"))

local my_snis = {}
for i = 1, 150 do
table.insert(my_snis, string.format("my-sni-%03d.test", i))
end

local res = client:post("/certificates", {
body = {
cert = ssl_fixtures.cert,
key = ssl_fixtures.key,
snis = my_snis,
},
headers = { ["Content-Type"] = "application/json" },
})
assert.res_status(201, res)

local res = client:get("/certificates")
local body = assert.res_status(200, res)
local json = cjson.decode(body)
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].snis)
assert.same(my_snis, json.data[1].snis)
end)
end)

describe("POST", function()

before_each(function()
assert(db:truncate("certificates"))
assert(db:truncate("snis"))

local res = client:post("/certificates", {
body = {
cert = ssl_fixtures.cert,
key = ssl_fixtures.key,
snis = { "foo.com", "bar.com" },
},
headers = { ["Content-Type"] = "application/json" },
})
assert.res_status(201, res)
end)

it("returns a conflict when duplicated snis are present in the request", function()
local res = client:post("/certificates", {
body = {
Expand Down

0 comments on commit be50425

Please sign in to comment.