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.

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

fix(api) targets upserts using PUT HTTP method #8596

Merged
merged 4 commits into from
Mar 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
execution, avoiding unnecessary concurrent executions.
[#8567](https://github.com/Kong/kong/pull/8567)


#### Plugins

- **ACME**: `auth_method` default value is set to `token`
Expand All @@ -106,6 +105,11 @@
- The cluster listener now uses the value of `admin_error_log` for its log file
instead of `proxy_error_log` [8583](https://github.com/Kong/kong/pull/8583)

#### Admin API

- Insert and update operations on target entities require using the `PUT` HTTP
method now. [#8596](https://github.com/Kong/kong/pull/8596)

### Additions

#### Performance
Expand Down
27 changes: 14 additions & 13 deletions kong/api/routes/upstreams.lua
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ local tostring = tostring
local fmt = string.format


local function post_health(self, db, is_healthy)
local function set_target_health(self, db, is_healthy)
local upstream, _, err_t = endpoints.select_entity(self, db, db.upstreams.schema)
if err_t then
return endpoints.handle_error(err_t)
Expand Down Expand Up @@ -180,9 +180,7 @@ return {
kong.db.upstreams.schema,
"upstream",
"page_for_upstream"),
POST = function(self, db)
-- updating a target using POST is a compatibility with existent API and
-- should be deprecated in next major version
PUT = function(self, db)
local entity, _, err_t = update_existent_target(self, db)
if err_t then
return endpoints.handle_error(err_t)
Expand All @@ -194,7 +192,10 @@ return {
local create = endpoints.post_collection_endpoint(kong.db.targets.schema,
kong.db.upstreams.schema, "upstream")
return create(self, db)
end
end,
POST = function(self, db)
return kong.response.exit(405)
end,
},

["/upstreams/:upstreams/targets/all"] = {
Expand Down Expand Up @@ -232,26 +233,26 @@ return {
},

["/upstreams/:upstreams/targets/:targets/healthy"] = {
POST = function(self, db)
return post_health(self, db, true)
PUT = function(self, db)
return set_target_health(self, db, true)
end,
},

["/upstreams/:upstreams/targets/:targets/unhealthy"] = {
POST = function(self, db)
return post_health(self, db, false)
PUT = function(self, db)
return set_target_health(self, db, false)
end,
},

["/upstreams/:upstreams/targets/:targets/:address/healthy"] = {
POST = function(self, db)
return post_health(self, db, true)
PUT = function(self, db)
return set_target_health(self, db, true)
end,
},

["/upstreams/:upstreams/targets/:targets/:address/unhealthy"] = {
POST = function(self, db)
return post_health(self, db, false)
PUT = function(self, db)
return set_target_health(self, db, false)
end,
},

Expand Down
4 changes: 2 additions & 2 deletions spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ describe("Admin API: #" .. strategy, function()

-- create the target
local res = assert(client:send {
method = "POST",
method = "PUT",
path = "/upstreams/my-upstream/targets",
body = {
target = "127.0.0.1:8000",
Expand Down Expand Up @@ -791,7 +791,7 @@ describe("Admin API: #" .. strategy, function()

-- create the target
local res = assert(client:send {
method = "POST",
method = "PUT",
path = "/upstreams/my-upstream/targets",
body = {
target = "127.0.0.1:8000",
Expand Down
36 changes: 18 additions & 18 deletions spec/02-integration/04-admin_api/08-targets_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ describe("Admin API #" .. strategy, function()
end)

describe("/upstreams/{upstream}/targets/", function()
describe("POST", function()
describe("PUT", function()
it_content_types("creates a target with defaults", function(content_type)
return function()
local upstream = bp.upstreams:insert { slots = 10 }
local res = assert(client:send {
method = "POST",
method = "PUT",
path = "/upstreams/" .. upstream.name .. "/targets/",
body = {
target = "mashape.com",
Expand All @@ -98,7 +98,7 @@ describe("Admin API #" .. strategy, function()
return function()
local upstream = bp.upstreams:insert { slots = 10 }
local res = assert(client:send {
method = "POST",
method = "PUT",
path = "/upstreams/" .. upstream.name .. "/targets/",
body = {
target = "mashape.com:123",
Expand All @@ -119,7 +119,7 @@ describe("Admin API #" .. strategy, function()
return function()
local upstream = bp.upstreams:insert { slots = 10 }
local res = assert(client:send {
method = "POST",
method = "PUT",
path = "/upstreams/" .. upstream.name .. "/targets/",
body = {
target = "zero.weight.test:8080",
Expand All @@ -146,7 +146,7 @@ describe("Admin API #" .. strategy, function()
return function()
local upstream = bp.upstreams:insert { slots = 10 }
local res = assert(client:send {
method = "POST",
method = "PUT",
path = "/upstreams/" .. upstream.name .. "/targets/",
body = {
target = "single-target.test:8080",
Expand All @@ -163,7 +163,7 @@ describe("Admin API #" .. strategy, function()
assert.are.equal(1, json.weight)

local res = assert(client:send {
method = "POST",
method = "PUT",
path = "/upstreams/" .. upstream.name .. "/targets/",
body = {
target = "single-target.test:8080",
Expand All @@ -183,7 +183,7 @@ describe("Admin API #" .. strategy, function()
it("handles malformed JSON body", function()
local upstream = bp.upstreams:insert { slots = 10 }
local res = assert(client:request {
method = "POST",
method = "PUT",
path = "/upstreams/" .. upstream.name .. "/targets/",
body = '{"hello": "world"',
headers = {["Content-Type"] = "application/json"}
Expand All @@ -197,7 +197,7 @@ describe("Admin API #" .. strategy, function()
local upstream = bp.upstreams:insert { slots = 10 }
-- Missing parameter
local res = assert(client:send {
method = "POST",
method = "PUT",
path = "/upstreams/" .. upstream.name .. "/targets/",
body = {
weight = weight_min,
Expand All @@ -211,7 +211,7 @@ describe("Admin API #" .. strategy, function()

-- Invalid target parameter
res = assert(client:send {
method = "POST",
method = "PUT",
path = "/upstreams/" .. upstream.name .. "/targets/",
body = {
target = "some invalid host name",
Expand All @@ -225,7 +225,7 @@ describe("Admin API #" .. strategy, function()

-- Invalid weight parameter
res = assert(client:send {
method = "POST",
method = "PUT",
path = "/upstreams/" .. upstream.name .. "/targets/",
body = {
target = "mashape.com",
Expand All @@ -240,7 +240,7 @@ describe("Admin API #" .. strategy, function()
end
end)

for _, method in ipairs({"PUT", "PATCH", "DELETE"}) do
for _, method in ipairs({"POST", "PATCH", "DELETE"}) do
it_content_types("returns 405 on " .. method, function(content_type)
return function()
local upstream = bp.upstreams:insert { slots = 10 }
Expand Down Expand Up @@ -337,7 +337,7 @@ describe("Admin API #" .. strategy, function()

for i = 1, #weights do
local status, body = client_send({
method = "POST",
method = "PUT",
path = "/upstreams/" .. upstream.name .. "/targets",
headers = {
["Content-Type"] = "application/json",
Expand Down Expand Up @@ -668,7 +668,7 @@ describe("Admin API #" .. strategy, function()
local json = assert(cjson.decode(body))

status, body = assert(client_send({
method = "POST",
method = "PUT",
path = "/upstreams/" .. upstream.id .. "/targets",
headers = {["Content-Type"] = "application/json"},
body = {
Expand All @@ -691,7 +691,7 @@ describe("Admin API #" .. strategy, function()
local expected = (i >= 3 and j >= 4) and 204 or 404
local path = "/upstreams/" .. u .. "/targets/" .. t .. "/" .. e
local status = assert(client_send {
method = "POST",
method = "PUT",
path = "/upstreams/" .. u .. "/targets/" .. t .. "/" .. e
})
assert.same(expected, status, "bad status for path " .. path)
Expand All @@ -703,7 +703,7 @@ describe("Admin API #" .. strategy, function()
it("flips the target status from UNHEALTHY to HEALTHY", function()
local status, body, json
status, body = assert(client_send {
method = "POST",
method = "PUT",
path = target_path .. "/unhealthy"
})
assert.same(204, status, body)
Expand All @@ -716,7 +716,7 @@ describe("Admin API #" .. strategy, function()
assert.same(target.target, json.data[1].target)
assert.same("UNHEALTHY", json.data[1].health)
status = assert(client_send {
method = "POST",
method = "PUT",
path = target_path .. "/healthy"
})
assert.same(204, status)
Expand All @@ -733,7 +733,7 @@ describe("Admin API #" .. strategy, function()
it("flips the target status from HEALTHY to UNHEALTHY", function()
local status, body, json
status = assert(client_send {
method = "POST",
method = "PUT",
path = target_path .. "/healthy"
})
assert.same(204, status)
Expand All @@ -746,7 +746,7 @@ describe("Admin API #" .. strategy, function()
assert.same(target.target, json.data[1].target)
assert.same("HEALTHY", json.data[1].health)
status = assert(client_send {
method = "POST",
method = "PUT",
path = target_path .. "/unhealthy"
})
assert.same(204, status)
Expand Down
2 changes: 1 addition & 1 deletion spec/02-integration/04-admin_api/15-off_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ describe("Admin API #off", function()
assert.response(res).has.status(201)

local res = assert(client:send {
method = "POST",
method = "PUT",
path = "/upstreams/foo/targets/c830b59e-59cc-5392-adfd-b414d13adfc4/10.20.30.40/unhealthy",
})

Expand Down
Loading