fix: create upstream error when pass host is node and nodes without port #2421
Conversation
Signed-off-by: Wei Jiang <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2421 +/- ##
==========================================
+ Coverage 70.04% 70.43% +0.38%
==========================================
Files 189 189
Lines 7441 7498 +57
Branches 828 845 +17
==========================================
+ Hits 5212 5281 +69
+ Misses 1923 1912 -11
+ Partials 306 305 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
typo in your title
|
jwrookie
changed the title
|
| Headers: map[string]string{"Authorization": base.GetToken()}, | ||
| ExpectStatus: http.StatusOK, | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Can you add test to check the upstream is work ?
There was a problem hiding this comment.
Good idea. it is done now
Co-authored-by: Peter Zhu <[email protected]>
Signed-off-by: Wei Jiang <[email protected]>
Start the depend error, pls rerun it thanks
|
|
@bzp2010 have a check, thanks! |
| hp := strings.Split(key, ":") | ||
| host := hp[0] | ||
| // according to APISIX upstream nodes policy, port is optional | ||
| port := "0" |
There was a problem hiding this comment.
Is it appropriate to use port 0 by default? Should port 80 be used by default? 🤔
There was a problem hiding this comment.
apisix-dashboard/api/internal/core/entity/entity.go
Lines 99 to 105 in 92f46d9
We cannot give default values to port, the value of 0 is assigned because the zero value is ignored during json.Marshal
There was a problem hiding this comment.
When there is no port in host, port 0 will be used by default, which I think is wrong. For HTTP it should be 80 and for HTTPS it should be 443, so I think it should be set to 80 by default.
There was a problem hiding this comment.
Port is not required, when using https and not configured port it will cause problems, if we do not configure the port there is no such problem
If 80 is given, you need to determine the schema type, when it is https and then change the port default back to 443. I think the first one is more convenient, what do you think?
There was a problem hiding this comment.
So to put it another way, if the port is 0, the JSON will not contain the port field when it is encoded in JSON? The port is required.
* upstream: (28 commits) docs: update some tips in develop.md (apache#2451) fix: create upstream error when pass host is node and nodes without port (apache#2421) fix: correct data type of filed Active.checks.active.https_verify_certificate (apache#2422) feat: basic support Apache APISIX 2.13.0 (apache#2428) feat: add page reload judgment (apache#2370) docs: Update RPM install package link (apache#2439) docs: Remove hyperlinks from documents (apache#2431) chore(deps): bump actions/upload-artifact from 2 to 3 (apache#2423) fix: consumer without plugins causes page crashes (apache#2437) chore(deps): bump axios from 0.21.1 to 0.21.4 in /web (apache#2420) feat: Modify plugin preview page (apache#2359) chore(deps): bump moment from 2.29.1 to 2.29.2 in /web (apache#2418) chore: use json schema instead hard code (apache#2399) refactor: migrate route tests to e2enew (apache#2411) chore(deps): bump actions/setup-python from 2.3.2 to 3.1.1 (apache#2414) chore: update the year of copyright and fix typo (apache#2417) docs: update online playground url (apache#2416) chore: update README for online demo (apache#2404) refactor: migrate id compatible tests to e2enew (apache#2400) docs: fix invalid link (apache#2366) ... # Conflicts: # api/internal/route.go # web/src/components/Footer/index.tsx
Signed-off-by: Wei Jiang [email protected]
Please answer these questions before submitting a pull request, or your PR will get closed.
Why submit this pull request?
What changes will this PR take into?
Create upstream should succeed when pass_host is
nodeand nodes withoutportRelated issues
fix/resolve #2409
Checklist: