fix(api) upsert of targets now require the PUT HTTP method

Kong · Mar 28, 2022 · b4dd909 · b4dd909
1 parent 6983215
commit b4dd909
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 68 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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`
@@ -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

diff --git a/kong/api/routes/upstreams.lua b/kong/api/routes/upstreams.lua
@@ -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)
@@ -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)
@@ -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"] = {
@@ -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,
   },
 

diff --git a/spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua b/spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua
@@ -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",
@@ -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",

diff --git a/spec/02-integration/04-admin_api/08-targets_routes_spec.lua b/spec/02-integration/04-admin_api/08-targets_routes_spec.lua
@@ -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",
@@ -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",
@@ -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",
@@ -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",
@@ -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",
@@ -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"}
@@ -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,
@@ -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",
@@ -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",
@@ -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 }
@@ -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",
@@ -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 = {
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)

diff --git a/spec/02-integration/04-admin_api/15-off_spec.lua b/spec/02-integration/04-admin_api/15-off_spec.lua
@@ -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",
       })