Skip to content

Commit

Permalink
hotfix(db) allow plugins on empty services (#4312)
Browse files Browse the repository at this point in the history
Before this change, assigning a plugin to an empty service (with no routes) would result in an “no compatible routes found in service” error. That is considered a breaking change (for things like automatic setups) and has been altered now: when a plugin is assigned to a service, and the service has no routes, then there will be no error. The error will still be raised when the service has at least one route, but none of its routes are compatible.
  • Loading branch information
kikito authored and hishamhm committed Feb 14, 2019
1 parent dffdd54 commit c7e9b6d
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
9 changes: 7 additions & 2 deletions kong/db/dao/plugins.lua
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,21 @@ local function has_a_common_protocol_with_route(plugin, route)
end


local function has_a_compatible_route_in_service(self, plugin, service_pk)
local function has_common_protocol_with_service(self, plugin, service_pk)
local had_at_least_one_route = false
for route, err, err_t in self.db.routes:each_for_service(service_pk) do
if not route then
return nil, err, err_t
end

had_at_least_one_route = true

if has_a_common_protocol_with_route(plugin, route) then
return true
end
end

return not had_at_least_one_route
end


Expand All @@ -64,7 +69,7 @@ local function check_protocols_match(self, plugin)
end

if type(plugin.service) == "table" then
if not has_a_compatible_route_in_service(self, plugin, plugin.service) then
if not has_common_protocol_with_service(self, plugin, plugin.service) then
local err_t = self.errors:schema_violation({
protocols = "must match the protocols of at least one route " ..
"pointing to this Plugin's service",
Expand Down
17 changes: 14 additions & 3 deletions spec/02-integration/03-db/03-plugins_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ for _, strategy in helpers.each_strategy() do

before_each(function()
service = bp.services:insert()
route = bp.routes:insert({ protocols = { "tcp" },
route = bp.routes:insert({ service = { id = service.id },
protocols = { "tcp" },
sources = { { ip = "127.0.0.1" } },
})
end)
Expand Down Expand Up @@ -76,7 +77,7 @@ for _, strategy in helpers.each_strategy() do
[["},consumer=null}']], err_t.message)
end)

it("returns an error when inserting mismatched plugins", function()
it("does not validate when associated to an incompatible route, or a service with only incompatible routes", function()
local plugin, _, err_t = db.plugins:insert({ name = "key-auth",
protocols = { "http" },
route = { id = route.id },
Expand All @@ -85,13 +86,23 @@ for _, strategy in helpers.each_strategy() do
assert.equals(err_t.fields.protocols, "must match the associated route's protocols")

local plugin, _, err_t = db.plugins:insert({ name = "key-auth",
protocols = { "tcp" },
protocols = { "http" },
service = { id = service.id },
})
assert.is_nil(plugin)
assert.equals(err_t.fields.protocols,
"must match the protocols of at least one route pointing to this Plugin's service")
end)

it("validates when associated to a service with no routes", function()
local service_with_no_routes = bp.services:insert()
local plugin, _, err_t = db.plugins:insert({ name = "key-auth",
protocols = { "http" },
service = { id = service_with_no_routes.id },
})
assert.truthy(plugin)
assert.is_nil(err_t)
end)
end)

describe(":update()", function()
Expand Down

0 comments on commit c7e9b6d

Please sign in to comment.