You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/01/12 06:52:03 UTC
[GitHub] [apisix-dashboard] imjoey opened a new issue #1275: bug: the script field in Route is limited to map, instead of string
imjoey opened a new issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275
# Bug report
## Describe the bug
In APISIX, we can see the `script` field in `Route` is of type `string`.
https://github.com/apache/apisix/blob/3e19b06293463f52c8d26f0958542fc475ff0fcd/apisix/schema_def.lua#L456
While as creating a Route in manager-api, we forcibly cast the script to `map[string]interface{}`.
https://github.com/apache/apisix-dashboard/blob/0f322e1d4a03e3416da1ae989fcfa49232035648/api/internal/handler/route/route.go#L332
So if sending a request to manager-api as below,
```
curl http://127.0.0.1:8000/apisix/admin/routes -X POST -H "Authorization: auth-header" -d '{"uri":"/hello","script":"local _M = {} \n function _M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M","upstream":{"type":"roundrobin","nodes":{"127.0.0.1:1980":1}}}'
```
manager-api will always complain with the following error:
```
errοΌ interface conversion: interface {} is string, not map[string]interface {}
[Recovery] 2021/01/12 - 14:23:53 panic recovered:
interface conversion: interface {} is string, not map[string]interface {}
/usr/local/opt/go/libexec/src/runtime/iface.go:261 (0x100cd6e)
panicdottypeE: panic(&TypeAssertionError{iface, have, want, ""})
/Users/imjoey/Work/apache-apisix/apisix-dashboard/api/internal/handler/route/route.go:332 (0x193d9fe)
```
As a contrast, the similar request can be successfully processed by APISIX Admin API.
Is such situation mentioned above as expected? Much appreciated.
## Expected behavior
manager-api could also accept the `script` field of `string` type.
## System information
- OS: macOS
- Version: latest in master branch
## Additional
This issue is also related to https://github.com/apache/apisix-dashboard/issues/1085, put here for tracking.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] imjoey commented on issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
imjoey commented on issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275#issuecomment-758519103
> oh, please have a look at test cases for plugin orchestration:
> https://github.com/apache/apisix-dashboard/blob/master/api/test/e2e/route_with_plugin_orchestration_test.go
>
> and you will know why.
@nic-chen
Thanks for pointing this out. I guess you mean the `Script` of `map` type is only for plugin orchestration in FE, just as the comment **-- The 'script' fields below are used by dashboard for plugin orchestration** in https://github.com/apache/apisix/blob/3e19b06293463f52c8d26f0958542fc475ff0fcd/apisix/schema_def.lua#L455 ? Is that correct?
If yes, may I ask that how we could add a script like `"local _M = {} \n function _M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"` in Route via manager-api, to implement the serverless services?
Much appreciated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
nic-chen commented on issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275#issuecomment-758512183
oh, please have a look at test cases for plugin orchestration:
https://github.com/apache/apisix-dashboard/blob/master/api/test/e2e/route_with_plugin_orchestration_test.go
and you will know why.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz edited a comment on issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
starsz edited a comment on issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275#issuecomment-758470436
Good capture. I think we need to fix it.
~~By the way, I think we need middleware to recover the panic. Let me create another issue.~~
Already support.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan closed issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
juzhiyuan closed issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
nic-chen commented on issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275#issuecomment-759144645
> > oh, please have a look at test cases for plugin orchestration:
> > https://github.com/apache/apisix-dashboard/blob/master/api/test/e2e/route_with_plugin_orchestration_test.go
> > and you will know why.
>
> @nic-chen
>
> Thanks for pointing this out. I guess you mean the `Script` of `map` type is only for plugin orchestration in FE, just as the comment **-- The 'script' fields below are used by dashboard for plugin orchestration** in https://github.com/apache/apisix/blob/3e19b06293463f52c8d26f0958542fc475ff0fcd/apisix/schema_def.lua#L455 ? Is that correct?
>
> If yes, may I ask that how we could add a script like `"local _M = {} \n function _M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"` in Route via manager-api, to implement the serverless services?
>
> Please let me know if I missed anything important. π Much appreciated.
Yes. The threshold for manually writing Lua code to implement plugin orchestration is too high. We prefer that users use the FE to drag for plugin orchestration, not write Lua code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz edited a comment on issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
starsz edited a comment on issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275#issuecomment-758470436
Good capture. I think we need to fix it.
By the way, I think we need middleware to recover the panic. Let me create another issue. : )
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz commented on issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
starsz commented on issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275#issuecomment-758470436
Good capture. I think we need to fix it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] imjoey commented on issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
imjoey commented on issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275#issuecomment-759185875
> > > oh, please have a look at test cases for plugin orchestration:
> > > https://github.com/apache/apisix-dashboard/blob/master/api/test/e2e/route_with_plugin_orchestration_test.go
> > > and you will know why.
> >
> >
> > @nic-chen
> > Thanks for pointing this out. I guess you mean the `Script` of `map` type is only for plugin orchestration in FE, just as the comment **-- The 'script' fields below are used by dashboard for plugin orchestration** in https://github.com/apache/apisix/blob/3e19b06293463f52c8d26f0958542fc475ff0fcd/apisix/schema_def.lua#L455 ? Is that correct?
> > If yes, may I ask that how we could add a script like `"local _M = {} \n function _M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"` in Route via manager-api, to implement the serverless services?
> > Please let me know if I missed anything important. π Much appreciated.
>
> Yes. The threshold for manually writing Lua code to implement plugin orchestration is too high. We prefer that users use the FE to drag for plugin orchestration, not write Lua code.
@nic-chen got it. Much appreciated for the explanation. I will propose a PR that also supports `string` type script alongside the existing `map` type, in order to eliminate the errors. Pls assign this to me, if possible. Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] imjoey commented on issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
imjoey commented on issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275#issuecomment-758487451
@juzhiyuan @starsz
I've one more question, when creating a `Route`, manager-api will store the script field into etcd with prefix `/apisix/script`, treating `Script` as top level resource type. While, AFAIK, not the same as APISIX, since it only regards `Script` as an inner property of `Route`, only stored with in Route object. This definitely introduces inconsistency between manager-api and Admin API. Is this also as expected, or would be a feature in future? Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] imjoey edited a comment on issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
imjoey edited a comment on issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275#issuecomment-758519103
> oh, please have a look at test cases for plugin orchestration:
> https://github.com/apache/apisix-dashboard/blob/master/api/test/e2e/route_with_plugin_orchestration_test.go
>
> and you will know why.
@nic-chen
Thanks for pointing this out. I guess you mean the `Script` of `map` type is only for plugin orchestration in FE, just as the comment **-- The 'script' fields below are used by dashboard for plugin orchestration** in https://github.com/apache/apisix/blob/3e19b06293463f52c8d26f0958542fc475ff0fcd/apisix/schema_def.lua#L455 ? Is that correct?
If yes, may I ask that how we could add a script like `"local _M = {} \n function _M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"` in Route via manager-api, to implement the serverless services?
Please let me know if I missed anything important. π Much appreciated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275#issuecomment-758493997
cc @nic-chen
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on issue #1275: bug: the script field in Route is limited to map, instead of string
Posted by GitBox <gi...@apache.org>.
nic-chen commented on issue #1275:
URL: https://github.com/apache/apisix-dashboard/issues/1275#issuecomment-759191192
> > > > oh, please have a look at test cases for plugin orchestration:
> > > > https://github.com/apache/apisix-dashboard/blob/master/api/test/e2e/route_with_plugin_orchestration_test.go
> > > > and you will know why.
> > >
> > >
> > > @nic-chen
> > > Thanks for pointing this out. I guess you mean the `Script` of `map` type is only for plugin orchestration in FE, just as the comment **-- The 'script' fields below are used by dashboard for plugin orchestration** in https://github.com/apache/apisix/blob/3e19b06293463f52c8d26f0958542fc475ff0fcd/apisix/schema_def.lua#L455 ? Is that correct?
> > > If yes, may I ask that how we could add a script like `"local _M = {} \n function _M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"` in Route via manager-api, to implement the serverless services?
> > > Please let me know if I missed anything important. π Much appreciated.
> >
> >
> > Yes. The threshold for manually writing Lua code to implement plugin orchestration is too high. We prefer that users use the FE to drag for plugin orchestration, not write Lua code.
>
> @nic-chen got it. Much appreciated for the explanation. I will propose a PR that also supports `string` type script alongside the existing `map` type, in order to eliminate the errors. Pls assign this to me, if possible. Thanks.
Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org