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

Some Admin APIs' data structure are not unified #6105

Closed
anjia0532 opened this issue Jan 14, 2022 · 14 comments
Closed

Some Admin APIs' data structure are not unified #6105

anjia0532 opened this issue Jan 14, 2022 · 14 comments
Labels
enhancement New feature or request

Comments

@anjia0532
Copy link
Contributor

anjia0532 commented Jan 14, 2022

Issue description

apisix admin api 的略微混乱的数据结构

  1. 部分接口(e.g. /apisix/admin/plugin_metadata/和 /apisix/admin/global_rules)中的 "count" 字段是string,其余admin接口是int
  2. 同key数据结构不一致,常见的比如 node.nodes 无值{},有值[],更奇葩的是upstreamnode.nodes[].nodes,通过接口更新的是{},通过dashboard更新的是[{},{}],就很难受
  3. /apisix/admin/plugins /apisix/admin/plugins?all=true/apisix/admin/plugins/list 返回风格与其他接口返回数据风格不一致
  4. stand-alone 模式不支持(也可能没更新文档 ) /apisix/admin/plugin_configs 接口
  5. stand-alone#配置-plugins 文档中描述 stream: true # stream 插件需要设置 stream 属性为 true 啥样的插件算是 stream 插件?没法从admin api 相关接口中获得这个标识

Environment

  • apisix version (cmd: apisix version):
  • OS (cmd: uname -a):
  • OpenResty / Nginx version (cmd: nginx -V or openresty -V):
  • etcd version, if have (cmd: run curl http://127.0.0.1:9090/v1/server_info to get the info from server-info API):
  • apisix-dashboard version, if have:
  • the plugin runner version, if the issue is about a plugin runner (cmd: depended on the kind of runner):
  • luarocks version, if the issue is about installation (cmd: luarocks --version):

Steps to reproduce

感觉很多是历史遗留问题,但是希望最好能解决下

Actual result

以上

Error log

Expected result

No response

@anjia0532
Copy link
Contributor Author

不是大问题,不影响使用。只是看着别扭而已

@spacewander
Copy link
Member

/apisix/admin/plugins /apisix/admin/plugins?all=true 和/apisix/admin/plugins/list 返回风格与其他接口返回数据风格不一致

Yes, these APIs don't fetch data from etcd, so they are different.

BTW, /apisix/admin/plugins/list should be marked as deprecated.
PR is welcome!

stand-alone 模式不支持(也可能没更新文档 ) /apisix/admin/plugin_configs 接口

stand-alone mode doesn't have Admin API.

stand-alone#配置-plugins 文档中描述 stream: true # stream 插件需要设置 stream 属性为 true 啥样的插件算是 stream 插件?没法从admin api 相关接口中获得这个标识

You can get stream plugin list via /apisix/admin/plugins?all=true&subsystem=stream. The API doesn't provide room for stream plugins, so we have to use an extra subsystem argument.

@spacewander
Copy link
Member

部分接口(e.g. /apisix/admin/plugin_metadata/和 /apisix/admin/global_rules)中的 "count" 字段是string,其余admin接口是int

These resources forget to call utils.fix_count in the get method like route:

utils.fix_count(res.body, id)

Let's fix it in a separate PR!

You may ask why we don't call fix_count on all resources. Because not all the resources come from etcd.

@spacewander
Copy link
Member

同key数据结构不一致,常见的比如 node.nodes 无值{},有值[],更奇葩的是upstream的node.nodes[].nodes,通过接口更新的是{},通过dashboard更新的是[{},{}],就很难受

Could you provide detailed steps to reproduce it?

It's a bit weird in this case, because both {} and [] are valid nodes values. The array format is preferable, because:

  1. it can be PATCHed
  2. it is extendable

@anjia0532
Copy link
Contributor Author

anjia0532 commented Jan 14, 2022

/apisix/admin/plugins /apisix/admin/plugins?all=true 和/apisix/admin/plugins/list 返回风格与其他接口返回数据风格不一致

Yes, these APIs don't fetch data from etcd, so they are different.

BTW, /apisix/admin/plugins/list should be marked as deprecated. PR is welcome!

stand-alone 模式不支持(也可能没更新文档 ) /apisix/admin/plugin_configs 接口

stand-alone mode doesn't have Admin API.

stand-alone#配置-plugins 文档中描述 stream: true # stream 插件需要设置 stream 属性为 true 啥样的插件算是 stream 插件?没法从admin api 相关接口中获得这个标识

You can get stream plugin list via /apisix/admin/plugins?all=true&subsystem=stream. The API doesn't provide room for stream plugins, so we have to use an extra subsystem argument.

In my test docker apache/apisix:2.9-alpine subsystem=stream not work.

apisix/admin-api/#plugin

Description: all the attributes of all plugins, each plugin includes name, priority, type, schema, consumer_schema and version.
Not have name(maybe it's object key?), type, consumer_schema field

    "mqtt-proxy": {
        "priority": 1000,
        "version": 0.1,
        "schema": {
            // 忽略
        }
    }

@anjia0532
Copy link
Contributor Author

anjia0532 commented Jan 14, 2022

同key数据结构不一致,常见的比如 node.nodes 无值{},有值[],更奇葩的是upstream的node.nodes[].nodes,通过接口更新的是{},通过dashboard更新的是[{},{}],就很难受

Could you provide detailed steps to reproduce it?

It's a bit weird in this case, because both {} and [] are valid nodes values. The array format is preferable, because:

  1. it can be PATCHed
  2. it is extendable

Could you provide detailed steps to reproduce it?

My env apache/apisix:2.9-alpine and apache/apisix-dashboard:2.8

ref doc docs/apisix/admin-api/#request-body-parameters-3
nodes's Description: Hash table or array. If it is a hash table, the key of the internal element is the upstream machine address list, the format is Address + (optional) Port, where the address part can be IP or domain name, such as 192.168.1.100:80, foo.com:80, etc. The value is the weight of node. If it is an array, each item is a hash table with key host/weight and optional port/priority. The nodes can be empty, which means it is a placeholder and will be filled later. Clients use such an upstream will get 502 response. means { "ip:port": weight} or [{"host":"", "weight ":1, "port":80}]

根据文档的描述,upstream的nodes这个字段,同时支持 lua的hash table和数组两种格式,dashboard通过ui修改的都是数组形式。而我不确定 stand-alone#how-to-configure-router--upstream stand-alone 的upstream支不支持数组形式。

  1. it can be PATCHed
  2. it is extendable

用golang解析这种可变类型的结构,很痛苦。更喜欢某些接口返回 host/hosts,uri/uris 这种不同key返回不同类型的方式

@juzhiyuan juzhiyuan changed the title bug: admin api data structure of the chaos/admin api 数据结构混乱 Some Admin APIs' data structure are not unified Jan 16, 2022
@juzhiyuan juzhiyuan added the enhancement New feature or request label Jan 16, 2022
@spacewander
Copy link
Member

In my test docker apache/apisix:2.9-alpine subsystem=stream not work.

Yes, it is added in 2.10

@anjia0532
Copy link
Contributor Author

anjia0532 commented Jan 17, 2022

In my test docker apache/apisix:2.9-alpine subsystem=stream not work.

Yes, it is added in 2.10

Maybe add since version to doc is a better experience?

在文档中对于新增特性标注起始版本,可能会是更好的体验?

@spacewander
Copy link
Member

We currently provide multi-version doc on the doc site: https://apisix.apache.org/docs/apisix/2.9/admin-api

Maybe add since version to doc is a better experience?

We have tried it before, for example in the control-API's doc.
But it's hard to maintain it as new features are added into APISIX very frequently.

@anjia0532
Copy link
Contributor Author

We currently provide multi-version doc on the doc site: https://apisix.apache.org/docs/apisix/2.9/admin-api

Maybe add since version to doc is a better experience?

We have tried it before, for example in the control-API's doc. But it's hard to maintain it as new features are added into APISIX very frequently.

Yes,This is a feasible solution.

@mangoGoForward
Copy link
Contributor

These resources forget to call utils.fix_count in the get method like route:

utils.fix_count(res.body, id)

Let's fix it in a separate PR!

You may ask why we don't call fix_count on all resources. Because not all the resources come from etcd.

Is there any works I can do it to improve this issue?

@leslie-tsang
Copy link
Member

These resources forget to call utils.fix_count in the get method like route:

utils.fix_count(res.body, id)

Let's fix it in a separate PR!
You may ask why we don't call fix_count on all resources. Because not all the resources come from etcd.

Is there any works I can do it to improve this issue?

AFAIK, not yet, Would you like to submit a PR to fix it ? Thanks.

@mangoGoForward
Copy link
Contributor

These resources forget to call utils.fix_count in the get method like route:

utils.fix_count(res.body, id)

Let's fix it in a separate PR!
You may ask why we don't call fix_count on all resources. Because not all the resources come from etcd.

Is there any works I can do it to improve this issue?

AFAIK, not yet, Would you like to submit a PR to fix it ? Thanks.

I'd like to work on this, let's submit a PR.

@leslie-tsang
Copy link
Member

@juzhiyuan Consider solved. Feel free to reopen it if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants