Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(admin) change behaviour of GET target requests #3049

Merged
merged 1 commit into from
Dec 15, 2017
Merged

feat(admin) change behaviour of GET target requests #3049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions kong/api/routes/upstreams.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,6 @@ return {
self.params.upstream_id = self.upstream.id
end,

GET = function(self, dao_factory)
crud.paginated_set(self, dao_factory.targets)
end,

POST = function(self, dao_factory, helpers)
clean_history(self.params.upstream_id, dao_factory)

crud.post(self.params, dao_factory.targets)
end,
},

["/upstreams/:upstream_name_or_id/targets/active"] = {
before = function(self, dao_factory, helpers)
crud.find_upstream_by_name_or_id(self, dao_factory, helpers)
self.params.upstream_id = self.upstream.id
end,

GET = function(self, dao_factory)
self.params.active = nil

Expand Down Expand Up @@ -171,6 +154,23 @@ return {
total = active_n,
data = active,
}
end,

POST = function(self, dao_factory, helpers)
clean_history(self.params.upstream_id, dao_factory)

crud.post(self.params, dao_factory.targets)
end,
},

["/upstreams/:upstream_name_or_id/targets/all"] = {
before = function(self, dao_factory, helpers)
crud.find_upstream_by_name_or_id(self, dao_factory, helpers)
self.params.upstream_id = self.upstream.id
end,

GET = function(self, dao_factory)
crud.paginated_set(self, dao_factory.targets)
end
},

Expand Down
136 changes: 68 additions & 68 deletions spec/02-integration/04-admin_api/08-targets_routes_spec.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,65 @@ describe("Admin API", function()
end)
end)

describe("GET", function()
local upstream_name3 = "example.com"
local apis = {}

before_each(function()
local upstream3 = assert(helpers.dao.upstreams:insert {
name = upstream_name3,
})

-- testing various behaviors
-- for each index in weights, create a number of targets,
-- each with its weight as each element of the sub-array
local weights = {
{ 10, 0 }, -- two targets, eventually resulting in down
{ 10, 0, 10 }, -- three targets, eventually resulting in up
{ 10 }, -- one target, up
{ 10, 10 }, -- two targets, up (we should only see one)
{ 10, 50, 0 }, -- three targets, two up in a row, eventually down
{ 10, 0, 20, 0 }, -- four targets, eventually down
}

for i = 1, #weights do
for j = 1, #weights[i] do
ngx.sleep(0.01)
apis[i] = assert(helpers.dao.targets:insert {
target = "api-" .. tostring(i) .. ":80",
weight = weights[i][j],
upstream_id = upstream3.id
})
end
end
end)

it("only shows active targets", function()
for _, append in ipairs({ "", "/" }) do
local res = assert(client:send {
method = "GET",
path = "/upstreams/" .. upstream_name3 .. "/targets" .. append,
})
assert.response(res).has.status(200)
local json = assert.response(res).has.jsonbody()

-- we got three active targets for this upstream
assert.equal(3, #json.data)
assert.equal(3, json.total)

-- when multiple active targets are present, we only see the last one
assert.equal(apis[4].id, json.data[1].id)

-- validate the remaining returned targets
-- note the backwards order, because we walked the targets backwards
assert.equal(apis[3].target, json.data[2].target)
assert.equal(apis[2].target, json.data[3].target)
end
end)
end)
end)

describe("/upstreams/{upstream}/targets/all/", function()
describe("GET", function()
before_each(function()
for i = 1, 10 do
Expand All @@ -184,7 +243,7 @@ describe("Admin API", function()
it("retrieves the first page", function()
local res = assert(client:send {
methd = "GET",
path = "/upstreams/" .. upstream_name .. "/targets/",
path = "/upstreams/" .. upstream_name .. "/targets/all",
})
assert.response(res).has.status(200)
local json = assert.response(res).has.jsonbody()
Expand All @@ -198,7 +257,7 @@ describe("Admin API", function()
for i = 1, 4 do
local res = assert(client:send {
method = "GET",
path = "/upstreams/" .. upstream_name .. "/targets/",
path = "/upstreams/" .. upstream_name .. "/targets/all",
query = {size = 3, offset = offset}
})
assert.response(res).has.status(200)
Expand All @@ -223,7 +282,7 @@ describe("Admin API", function()
it("handles invalid filters", function()
local res = assert(client:send {
method = "GET",
path = "/upstreams/" .. upstream_name .. "/targets/",
path = "/upstreams/" .. upstream_name .. "/targets/all",
query = {foo = "bar"},
})
local body = assert.response(res).has.status(400)
Expand All @@ -233,7 +292,7 @@ describe("Admin API", function()
it("ignores an invalid body", function()
local res = assert(client:send {
methd = "GET",
path = "/upstreams/" .. upstream_name .. "/targets/",
path = "/upstreams/" .. upstream_name .. "/targets/all",
body = "this fails if decoded as json",
headers = {
["Content-Type"] = "application/json",
Expand All @@ -255,7 +314,7 @@ describe("Admin API", function()
it("data property is an empty array", function()
local res = assert(client:send {
method = "GET",
path = "/upstreams/" .. upstream_name2 .. "/targets/",
path = "/upstreams/" .. upstream_name2 .. "/targets/all",
})
local body = assert.response(res).has.status(200)
local json = cjson.decode(body)
Expand All @@ -265,65 +324,6 @@ describe("Admin API", function()
end)
end)

describe("/upstreams/{upstream}/targets/active/", function()
describe("GET", function()
local upstream_name3 = "example.com"
local apis = {}

before_each(function()
local upstream3 = assert(helpers.dao.upstreams:insert {
name = upstream_name3,
})

-- testing various behaviors
-- for each index in weights, create a number of targets,
-- each with its weight as each element of the sub-array
local weights = {
{ 10, 0 }, -- two targets, eventually resulting in down
{ 10, 0, 10 }, -- three targets, eventually resulting in up
{ 10 }, -- one target, up
{ 10, 10 }, -- two targets, up (we should only see one)
{ 10, 50, 0 }, -- three targets, two up in a row, eventually down
{ 10, 0, 20, 0 }, -- four targets, eventually down
}

for i = 1, #weights do
for j = 1, #weights[i] do
ngx.sleep(0.01)
apis[i] = assert(helpers.dao.targets:insert {
target = "api-" .. tostring(i) .. ":80",
weight = weights[i][j],
upstream_id = upstream3.id
})
end
end
end)

it("only shows active targets", function()
for _, append in ipairs({ "", "/" }) do
local res = assert(client:send {
method = "GET",
path = "/upstreams/" .. upstream_name3 .. "/targets/active" .. append,
})
assert.response(res).has.status(200)
local json = assert.response(res).has.jsonbody()

-- we got three active targets for this upstream
assert.equal(3, #json.data)
assert.equal(3, json.total)

-- when multiple active targets are present, we only see the last one
assert.equal(apis[4].id, json.data[1].id)

-- validate the remaining returned targets
-- note the backwards order, because we walked the targets backwards
assert.equal(apis[3].target, json.data[2].target)
assert.equal(apis[2].target, json.data[3].target)
end
end)
end)
end)

describe("/upstreams/{upstream}/targets/{target}", function()
describe("DELETE", function()
local target
Expand Down Expand Up @@ -357,7 +357,7 @@ describe("Admin API", function()

local targets = assert(client:send {
method = "GET",
path = "/upstreams/" .. upstream_name4 .. "/targets/",
path = "/upstreams/" .. upstream_name4 .. "/targets/all",
})
assert.response(targets).has.status(200)
local json = assert.response(targets).has.jsonbody()
Expand All @@ -366,7 +366,7 @@ describe("Admin API", function()

local active = assert(client:send {
method = "GET",
path = "/upstreams/" .. upstream_name4 .. "/targets/active",
path = "/upstreams/" .. upstream_name4 .. "/targets",
})
assert.response(active).has.status(200)
json = assert.response(active).has.jsonbody()
Expand All @@ -384,7 +384,7 @@ describe("Admin API", function()

local targets = assert(client:send {
method = "GET",
path = "/upstreams/" .. upstream_name4 .. "/targets/",
path = "/upstreams/" .. upstream_name4 .. "/targets/all",
})
assert.response(targets).has.status(200)
local json = assert.response(targets).has.jsonbody()
Expand All @@ -393,7 +393,7 @@ describe("Admin API", function()

local active = assert(client:send {
method = "GET",
path = "/upstreams/" .. upstream_name4 .. "/targets/active",
path = "/upstreams/" .. upstream_name4 .. "/targets",
})
assert.response(active).has.status(200)
json = assert.response(active).has.jsonbody()
Expand Down