You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "kayx23 (via GitHub)" <gi...@apache.org> on 2023/05/30 09:53:57 UTC
[GitHub] [apisix] kayx23 opened a new issue, #9577: Admin API URL Parameters
kayx23 opened a new issue, #9577:
URL: https://github.com/apache/apisix/issues/9577
### Description
Open to discussion for room for improvements of the following.
## Current State
There are 77 "HTTP" / L7 plugins:
```
curl -s "http://127.0.0.1:9180/apisix/admin/plugins?all=true" | jq 'length'
77
```
By default, if `subsystem` is not specified, the response defaults to returning L7 plugins. This can be validated with:
```
curl -s "http://127.0.0.1:9180/apisix/admin/plugins?all=true&subsystem=http" | jq 'length'
77
```
If we want to get stream plugins, we use `subsystem=stream` to query:
```
curl -s "http://127.0.0.1:9180/apisix/admin/plugins?all=true&subsystem=stream" | jq 'length'
4
```
## Issue
If you pass other random strings into the `subsystem` query, it still defaults to the result for `http`:
```
curl "http://127.0.0.1:9180/apisix/admin/plugins?all=true&subsystem=strm" | jq 'length'
77
```
```
curl "http://127.0.0.1:9180/apisix/admin/plugins?all=true&subsystem=htt" | jq 'length'
77
```
```
curl "http://127.0.0.1:9180/apisix/admin/plugins?all=true&subsystem=random" | jq 'length'
77
```
While this may or may not be the intended behaviour, it is not very straightforward. I personally think it makes sense to add a checker to see if the `subsystem` value is valid (i.e. `http` or `stream`)
### Environment
APISIX version: 3.3.0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kayx23 commented on issue #9577: Admin API: Plugins URL Query `subsystem`
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on issue #9577:
URL: https://github.com/apache/apisix/issues/9577#issuecomment-1581809353
> Shouldn't we make this interface a little better once and for all, for example the all=true parameter is quite strange to look at
If we're good with this breaking change then absolutely, let's factor in `all=true` in the re-design as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Revolyssup commented on issue #9577: Admin API: Plugins URL Query `subsystem`
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on issue #9577:
URL: https://github.com/apache/apisix/issues/9577#issuecomment-1581984384
Proposed design for GET endpoint:
/apisix/admin/plugins/{name}
Query params:
- subsystem: Only this subsystem will be searched for {name} when name is provided. The default value of subsystem when not provided in query parameter is to search in all subsystems. Possible values are : http, stream. This is so that by default when subsystem is unspecified, you search through all subsystems to remove ambiguity and to search across a given subsystem - you pass it in query param
---
**Removing redundant all=true**
this query params adds confusion and is usually not found in REST APIs. The default behaviour of any RESTful endpoint is to return all results for a given resource unless specified to filter via a query parameter. So here I propose to get rid of `all` to stick to convention.
/apisix/admin/plugins is equivalent to name not being passed. and When {name} is not provided then by default all plugins for the given subsystem(default: all subsystems) will be listed.
**Backwards compatibility:**
Currently the output of:
`/apisix/admin/plugins` is `{"error_msg":"not found plugin name"}` so no one would be using it.
Instead currently to get all plugins, the endpoint used is: `/apisix/admin/plugins?all=true`. With the proposed design all=true will simply be ignored and there would be no breaking change.
By default, if subsystem is not specified, the response defaults to returning L7 plugins. This is a breaking change since the proposal is to return all plugins when the subsystem is unspecified to remove ambiguity. So if somewhere this API is used in a way where it assume to only get L7 plugins, after this change they will have to add subsystem=http to make things explicit.
Currently when name is specified like http://127.0.0.1:9180/apisix/admin/plugins/http-logger?subsystem=stream, the subsystem field is ignored all together. The proposed design is to have uniform behaviour. Currently the above specified path returns http-logger even when there is no such plugin in stream subsystem. After the proposed changes, the search will be strict and the above endpoint will return no results.
---
**Proposal Summary**
Search via name requires the name to be passed in URI and filter by subsystem requires the subsystem to be specified in query params. These two are orthogonal search vectors.
- `/apisix/admin/plugins/{name}` = Find `{name}` plugin across all subsystems
- `/apisix/admin/plugins/{name}?subsystem={xyz}` = Find `{name}` plugin in `{xyz}` subsystems
- `/apisix/admin/plugins` = Return all plugins across all subsystems
- `/apisix/admin/plugins?subsystem={xyz}` = Return all plugins across `{xyz}` subsystem
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 closed issue #9577: Admin API: Plugins URL Query `subsystem`
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 closed issue #9577: Admin API: Plugins URL Query `subsystem`
URL: https://github.com/apache/apisix/issues/9577
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Revolyssup commented on issue #9577: Admin API: Plugins URL Query `subsystem`
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on issue #9577:
URL: https://github.com/apache/apisix/issues/9577#issuecomment-1586585350
>
Conventionally this list should also be returned by GET `/apisix/admin/plugins` but with `?trim=true`. The name of the HTTP verb should not be in the URI. But this can be a breaking change as client's might be using this API. Should I remove this API entirely or leave it as it is for now and add `?trim=true` with same behaviour. Then gradually as we remove /list from clients, later we can drop that endpoint from admin api as well. What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kayx23 commented on issue #9577: Admin API: Plugins URL Query `subsystem`
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on issue #9577:
URL: https://github.com/apache/apisix/issues/9577#issuecomment-1579772186
I only brought up the issues in this issue. **It's not a comprehensive solutioning thread**. As we already [have a discussion going in the PR](https://github.com/apache/apisix/pull/9580#issuecomment-1579716589) (though I think discussion should go before the PR) I suggest we just stick to one place for further discussion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Revolyssup commented on issue #9577: Admin API: Plugins URL Query `subsystem`
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on issue #9577:
URL: https://github.com/apache/apisix/issues/9577#issuecomment-1623413861
Conclusion:
`/apisix/admin/plugins?all=true` will be deprecated in future but is supported now. To get the names of all plugins, users can use `/apisix/admin/plugins/list` or `/apisix/admin/plugins/list?subsystem=stream`.
And then for fetching the full plugin in a particular subsystem with a schema, users can, for example:
`/apisix/admin/plugins/http-logger` or `/apisix/admin/plugins/mqtt-logger?subsystem=stream`.
This PR reflects the state of this conclusion: https://github.com/apache/apisix/pull/9580/files#diff-0143af7fae95d5cd58bb1e460c89bbe37e8c35826b5ce783d9e2c3a13ba91775
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kayx23 commented on issue #9577: Admin API: Plugins URL Query `subsystem`
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on issue #9577:
URL: https://github.com/apache/apisix/issues/9577#issuecomment-1568145036
As a side note (and as mentioned in https://github.com/apache/apisix/pull/9576) I feel that `http` might not be the best term for L7 plugins as there are plugins for other protocols such as grpc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Revolyssup commented on issue #9577: Admin API: Plugins URL Query `subsystem`
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on issue #9577:
URL: https://github.com/apache/apisix/issues/9577#issuecomment-1581976190
> Shouldn't we make this interface a little better once and for all, for example the `all=true` parameter is quite strange to look at
Yes I agree generally in REST APIs, the default behaviour is of all=true when nothing is specified in filter query. Adding `all` itself as a query parameter is weird. Below this commend I have a proposal for new design with all caveats and consideration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on issue #9577: Admin API: Plugins URL Query `subsystem`
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on issue #9577:
URL: https://github.com/apache/apisix/issues/9577#issuecomment-1579777923
Let's talk in this issue, it's more convenient.
We have a few questions about this API, so it is best to have a detailed design before we submit a PR.
cc @Revolyssup
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on issue #9577: Admin API: Plugins URL Query `subsystem`
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on issue #9577:
URL: https://github.com/apache/apisix/issues/9577#issuecomment-1579732141
Shouldn't we make this interface a little better once and for all, for example the all=true parameter is quite strange to look at
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on issue #9577: Admin API: Plugins URL Query `subsystem`
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on issue #9577:
URL: https://github.com/apache/apisix/issues/9577#issuecomment-1586488505
@Revolyssup will you change the API `/apisix/admin/plugins/list`? it now fetches a list of all plugins with attributes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org