fix: optional port for nod schema #10477
Conversation
theweakgod
changed the title
theweakgod
changed the title
theweakgod
changed the title
|
cheers |
moonming
requested review from
monkeyDluffy6017,
Revolyssup and
shreemaan-abhishek
|
@theweakgod Could you add a test case to cover this? |
Ok, can you tell me how to add a test case. Or some doc that tells me how to add it |
done! |
t/core/schema_def_optional_port.t
Outdated
| local schema_def = require("apisix.schema_def") | ||
| local core = require("apisix.core") | ||
|
|
||
|
|
t/core/schema_def_optional_port.t
Outdated
There was a problem hiding this comment.
@theweakgod, the test case you added aren't up to the mark. Please demonstrate using domain name in the nodes without port is acceptable and accessing a route that uses such upstream does not fail.
There was a problem hiding this comment.
Yeah, you should prove that the route could work
t/core/schema_def_optional_port.t
Outdated
There was a problem hiding this comment.
No need to add a new test file, add it here: https://github.com/apache/apisix/blob/master/t/admin/schema-validate.t
|
@monkeyDluffy6017 @shreemaan-abhishek all done |
|
please make the ci pass |
monkeyDluffy6017
added
wait for update
ok，updated |
Is this error a problem with the test file? What do I need to do? |
|
@theweakgod It was a flaky test. I rerun the CI and it passed. |
Ok, I see. |
t/admin/schema-validate.t
Outdated
|
|
||
| === TEST 14: Test route upstream | ||
| --- request | ||
| GET /ip |
There was a problem hiding this comment.
@Revolyssup Would you like to update these test cases? it is better not to use httpbin.org, it is unstable, and we could check the response body like the other test cases, and no need to add X-API-KEY in the request header and so on
There was a problem hiding this comment.
okay
I remove some useless code.
t/admin/schema-validate.t
Outdated
| upstream = { | ||
| type = "roundrobin", | ||
| nodes = { | ||
| { host = "nghttp2.org", weight = 1,} |
There was a problem hiding this comment.
use 127.0.0.1:1980 instead
t/admin/schema-validate.t
Outdated
| local code, body = t('/apisix/admin/routes', | ||
| ngx.HTTP_POST, | ||
| { | ||
| uri = "/ip", |
There was a problem hiding this comment.
you could search 127.0.0.1:1980 in the project, and find what uri is better
There was a problem hiding this comment.
OK, done. thanks
t/admin/schema-validate.t
Outdated
|
|
||
| === TEST 14: Test route upstream | ||
| --- request | ||
| GET /ip |
There was a problem hiding this comment.
We should check the response body, like this
=== TEST 8: `url` exist and `route_id` is 1
--- request
GET /hello
--- response_body
hello world
|
@theweakgod Nice job! |
^ ^. thank you very much for your help. |
Description
there is a bug in the schema check of nodes when admin-api saves the upstream configuration.
Fixes # (issue)
Checklist