feat(admin) /targets endpoint return active ones only

Breaking change:

The endpoints to list active and all Targets associated
with an Upstream have been changed as follows:

* `/upstreams/:upstream_name_or_id/targets/active` is
removed.

* `/upstreams/:upstream_name_or_id/targets/` now
only lists the active Targets instead of listing all of them. 

* `/upstreams/:upstream_name_or_id/targets/all` is
introduced, which returns a list of all the Targets (old and
current), as per the previous behavior of the `/targets/`
endpoint.

From #3049 
Closes #2791

kong/api/routes/upstreams.lua

@@ -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
 
@@ -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
   },
 

spec/02-integration/04-admin_api/08-targets_routes_spec.lua

@@ -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
@@ -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()
@@ -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)
@@ -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)
@@ -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",
@@ -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)
@@ -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
@@ -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()
@@ -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()
@@ -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()
@@ -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()