liuxiran commented 3 years ago

Route restful API proposal

According to our discussion on mail list: https://lists.apache.org/thread.html/r29f4b901d9779349bfcc3516f8f15ad73506bc16c3c3600666cbf7ef%40%3Cdev.apisix.apache.org%3E

I tried to list the entire Route module API in detail, and the goal is to serve as a sample of modifications to other modules, here they are:

Create route

Method and URL

POST /apisix/admin/routes

Request parameters

name	type	In	required	description
X-API-KEY	string	Header	true	access token got by login

and other request body parameters, please refer to https://github.com/apache/apisix/blob/master/docs/en/latest/admin-api.md#request-body-parameters

notice

only keep uris, remove uri, since uris contains uri, there is no need to keep two of them to Increase the difficulty of choice for users
only keep hosts, remove host, same reason with before
only keep remote_addrs, remove remote_addr, same reason with before
should not accept create_time and update_time from the request
only keep POST method to create a route, remove PUT(PUT /apisix/admin/routes/{id}) to reduce the difficulty of choice for users

Response parameters

Status: 200 OK

return the new created route info

{
     "id":"368819495659307851",
     "create_time":1629363150,
     "update_time":1630332911,
     "uri":"/libraries/jquery",
     "name":"CDN",
     "methods":["GET"],
     "vars":[["http_fff","==","fff"]],
     "plugins":{},
     "upstream":{"nodes":[{"host":"api.cdnjs.com","port":443,"weight":1}],"retries":1,"timeout":{"connect":6,"read":6,"send":6},
     "type":"roundrobin",
     "scheme":"https",
     "pass_host":"node",
     "labels": {"0":"npm","API_VERSION":"1.0"},
     "status":0
}

Status: 400 Bad Request

{
    "message":  "error details"
}

Status: 401 Unauthorized

{
    "message":  "error details"
}

Fetch routes list

Method and URL

GET /apisix/admin/routes

Request parameters

name	type	In	required	description
page	int	Query	false	search route by pagination：page index
page_size	int	Query	false	query the number of routes per page
name	string	Query	false	search route by name.
uri	string	Query	false	search route by uri. ｜
status	string	Query	false	route status, `1` for enabled, `0` for disable。
labels	string	Query	false	search route by labels
X-API-KEY	string	Header	true	access token got by login

notice this part has two changes:

change label to labels, for labels is the field name, and we support searching route with multi labels.
change Authorization to X-API-KEY to achieve the authentication.

Response parameters

Status: 200 OK

 {
     "items":[{
         "id":"368819495659307851",
         "create_time":1629363150,
         "update_time":1630332911,
         "uri":"/libraries/jquery",
         "name":"CDN",
         "methods":["GET"],
         "vars":[["http_fff","==","fff"]],
         "plugins":{},
         "upstream":{"nodes":[{"host":"api.cdnjs.com","port":443,"weight":1}],"retries":1,"timeout":{"connect":6,"read":6,"send":6},
         "type":"roundrobin",
         "scheme":"https",
         "pass_host":"node",
         "labels": {"0":"npm","API_VERSION":"1.0"},
         "status":0
     }],
     "total_size": 1,
}

Status: 401 Unauthorized

{
    "message":  "error details"
}

Get route info

Method and URL

GET /apisix/admin/routes/{id}

Request parameters

name	type	In	required	description
id	string	Query	true	route id
X-API-KEY	string	Header	true	access token got by login

Response parameters

Status: 200 OK

{
     "id":"368819495659307851",
     "create_time":1629363150,
     "update_time":1630332911,
     "uri":"/libraries/jquery",
     "name":"CDN",
     "methods":["GET"],
     "vars":[["http_fff","==","fff"]],
     "plugins":{},
     "upstream":{"nodes":[{"host":"api.cdnjs.com","port":443,"weight":1}],"retries":1,"timeout":{"connect":6,"read":6,"send":6},
     "type":"roundrobin",
     "scheme":"https",
     "pass_host":"node",
     "labels": {"0":"npm","API_VERSION":"1.0"},
     "status":0
}

Status: 404 Not Found

{
     "message": "Not found"
}

Status: 401 Unauthorized

{
    "message":  "error details"
}

Update route info

Method and URL

PATCH /apisix/admin/routes/{id}

Request parameters

name	type	In	required	description
id	string	Query	true	route id
X-API-KEY	string	Header	true	access token got by login

and other request body parameters, please refer to https://github.com/apache/apisix/blob/master/docs/en/latest/admin-api.md#request-body-parameters, and the same notice with create route

Response parameters

Status: 200 OK

return the updated route info

{
     "id":"368819495659307851",
     "create_time":1629363150,
     "update_time":1630332911,
     "uri":"/libraries/jquery",
     "name":"CDN",
     "methods":["GET"],
     "vars":[["http_fff","==","fff"]],
     "plugins":{},
     "upstream":{"nodes":[{"host":"api.cdnjs.com","port":443,"weight":1}],"retries":1,"timeout":{"connect":6,"read":6,"send":6},
     "type":"roundrobin",
     "scheme":"https",
     "pass_host":"node",
     "labels": {"0":"npm","API_VERSION":"1.0"},
     "status":0
}

Status: 400 Bad Request

{
    "message": "xxxx"
}

Status: 401 Unauthorized

{
    "message":  "error details"
}

Delete route

Method and URL

DELETE /apisix/admin/routes/{id}

Request parameters

name	type	In	required	description
id	string	Query	true	route id
X-API-KEY	string	Header	true	access token got by login

Response parameters

Status: 200 OK

Status: 400 Bad Request

{
    "message": "xxxx"
}

Status: 401 Unauthorized

{
    "message":  "error details"
}

Update specify property

Method and URL

PUT /apisix/admin/routes/{id}/{path}

notice

I prefer to use PUT method instead of PATCH, reason: this API aimed to update the specify property avoid entering the entire parameter structure in the body, and can only update the property defined in path parameters, so PUT is more suitable.

Request parameters

name	type	In	required	description
X-API-KEY	string	Header	true	access token got by login

and the specify property values.

Response parameters

Status: 200 OK

return the updated route info

{
     "id":"368819495659307851",
     "create_time":1629363150,
     "update_time":1630332911,
     "uri":"/libraries/jquery",
     "name":"CDN",
     "methods":["GET"],
     "vars":[["http_fff","==","fff"]],
     "plugins":{},
     "upstream":{"nodes":[{"host":"api.cdnjs.com","port":443,"weight":1}],"retries":1,"timeout":{"connect":6,"read":6,"send":6},
     "type":"roundrobin",
     "scheme":"https",
     "pass_host":"node",
     "labels": {"0":"npm","API_VERSION":"1.0"},
     "status":0
}

Status: 400 Bad Request

{
    "message": "xxxx"
}

Status: 401 Unauthorized

{
    "message":  "error details"
}

liuxiran commented 3 years ago

cc @nic-chen @starsz @imjoey @bzp2010 @spacewander @membphis @moonming @juzhiyuan to have a look when you have time, thanks a lot

nic-chen commented 3 years ago

Others LGTM, but why not keep the PUT API?

liuxiran commented 3 years ago

Others LGTM, but why not keep the PUT API?

I tend to guide the user to complete a target operation using the specified API, without ambiguity @nic-chen

For create, we can use POST, for update, we can use PATCH,

As our doc says, PUT API is used to create a route by ID, ID is used as a unique identifier and would ideally be better generated by default. And on the other hand, it can also update the already existed route, and if we want to update route, PATCH would be more flexible to use.

starsz commented 3 years ago

Hi, @liuxiran Thank you very much. After reading the proposal. I have some problems as follows.

I think we should keep the put method since we have the situation that specified the route ID.
The delete method should return the deleted route info?
We will return the route info directly rather than including it in a field?

liuxiran commented 3 years ago

Hi all, in order to make it more clear about what are the changes between the new design and the current manager-api admin-api, I made more detailed comparison:

method and url

this is the comparison of current Manager-API and Admin-API, we can see that the method and url are almost the same, only one exception, Manager-API has a API to check is the route name is exist, actually this is only a logic in UI, and in our APISIX only route id is the unique Identification, we allow repeated names, so it would be better to remove this API and the Validation rules.

the new design of route API is:

differences are:

keep the manager-api and admin-api the same
remove PUT /apisix/admin/routes/{id}, the reason comment here: https://github.com/apache/apisix-dashboard/issues/2105#issuecomment-909758567
change PATCH /apisix/admin/routes/{id}/{path} to PUT /apisix/admin/routes/{id}/{path}, the reason: when user use this API they can easier update the whole specified path in the route, refer to https://en.wikipedia.org/wiki/Patch_verb, PUT method is more suitable.

request parameters for create a route

here is the comparison of the new design and the current Admin-API(Manager-API should the same parameters with Admin-API)

differences are:

only keep uris, remove uri, since uris contains uri, there is no need to keep two of them to Increase the difficulty of choice for users
only keep hosts, remove host, same reason with before
only keep remote_addrs, remove remote_addr, same reason with before

Uniform response parameter format

actually the response parameters struct is different between the Admin-API and Manager-API, here is the comparison,

and here is the new design as we talked about in the mail list

liuxiran commented 3 years ago

I think we should keep the put method since we have the situation that specified the route ID.

I am actually hesitant here, the fact that the ID is our unique identifier, customization increases flexibility while inviting the risk of duplication, In case the user defines two routes with duplicate IDs, using the PUT method, the latter will override the former. if I've missed any details, please point them out @starsz

The delete method should return the deleted route info?

Other APIs were referenced during the design process

Gthub API does not return the deleted info
kubesphere API does

Probably for application scenarios, I prefer to keep these return values

We will return the route info directly rather than including it in a field?

For the get list API, info will be included in a field called items, but for the info API, IMO we mainly want to get the route info, return them directly is a better way, what do you think?

liuxiran commented 3 years ago

Hi @starsz @nic-chen any updates?

starsz commented 3 years ago

Hi @liuxiran, sorry for the late reply.

In case the user defines two routes with duplicate IDs, using the PUT method, the latter will override the former. if I've missed any details, please point them out

It's right, this may be another way that is called update.

kubesphere API does This is a special API IMO, if you look at others like,they don't return the delete info:

https://kubesphere.io/api/kubesphere/#operation/DeleteGroup https://kubesphere.io/api/kubesphere/#operation/DeleteNamespace https://kubesphere.io/api/kubesphere/#operation/DeleteClusterRole https://kubesphere.io/api/kubesphere/#operation/DeleteGlobalRole https://kubesphere.io/api/kubesphere/#operation/DeleteClusterRole

IMO we mainly want to get the route info, return them directly is a better way

It's not a good way. Please refer to other API docs like github, kubesphere, etc.

liuxiran commented 3 years ago

It's not a good way. Please refer to other API docs like github, kubesphere, etc.

Got you, thanks for the detailed examples, @starsz then for the delete API, just return HTTP code 200 is enough

github-actions[bot] commented 3 years ago

This issue has been marked as stale due to 30 days of inactivity. It will be closed in 2 weeks if no further activity occurs. If this issue is still relevant, please simply write any comment. Even if closed, you can still revive the issue at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.

moonming commented 3 years ago

any update？ @liuxiran

zaunist commented 2 years ago

Hi, community. My understanding, is this proposal to convert the manager-api into an adminapi?

juzhiyuan commented 2 years ago

Yes, will continue this proposal in V3.

apache / apisix-dashboard

Proposal: restful API for Route module #2105

Route restful API proposal

Create route

Method and URL

Request parameters

Response parameters

Fetch routes list

Method and URL

Request parameters

Response parameters

Get route info

Method and URL

Request parameters

Response parameters

Update route info

Method and URL

Request parameters

Response parameters

Delete route

Method and URL

Request parameters

Response parameters

Update specify property

Method and URL

Request parameters

Response parameters

method and url

request parameters for create a route

Uniform response parameter format