lxbme opened a new pull request, #3433:
URL: https://github.com/apache/apisix-dashboard/pull/3433
**Why submit this pull request?**
- [x] Bugfix
- [ ] New feature provided
- [ ] Improve performance
- [ ] Backport patches
**What changes will this PR take into?**
Part of #3417 (Data-integrity section): "Object-form nodes conversion
corrupts IPv6 hosts and invents `port: 1` / `priority: 0` (`FormItemNodes.tsx`
`key.split(':')`)."
Object-form nodes (`{"host:port": weight}`) are the Admin API's documented
shorthand, also valid without a port and with IPv6 keys. `objToUpstreamNodes`
parsed those keys with `key.split(':')`:
- an IPv6 key like `"[::1]:1980"` became host `"["` with port 1;
- a port-less key (valid — the scheme decides the port at runtime) got an
invented `port: 1` via `Number(port) || 1`;
- `priority: 0` was added out of thin air.
The nodes table's flush-on-mousedown sync (from the #3293 fix) pushes the
displayed values into the form on any edit, so a **no-op Edit → Save silently
persisted the corruption** — turning a working upstream into one that routes to
port 1. Verified live: the Admin API accepts both corrupted shapes (it even
stores host `"["`), so nothing downstream catches it.
**Fix.** The conversion (extracted to `nodes-conversion.ts` solely so it is
unit-testable) now parses all four gateway-verified key shapes and never
invents fields:
| object key | before (corrupted) | after |
|---|---|---|
| `"host:80"` | ok by luck (plus invented `priority: 0`) | `{host, port:
80}` |
| `"host"` | invented `port: 1` | `{host}` — port stays runtime-resolved |
| `"[::1]:1980"` | `{host: "[", port: 1}` | `{host: "[::1]", port: 1980}` |
| `"::1"` (bare IPv6) | `{host: ":", port: 1}` | `{host: "[::1]"}`
(bracketed — the array form rejects unbracketed IPv6 with 400) |
Supporting changes, each forced by the fix and verified against the live
gateway:
- `UpstreamNode.port` is now `.optional()` — the Admin API accepts
array-form nodes without a port (201). Requiring it both rejected valid
API-created nodes and was the reason the converter invented ports. This also
fixes a hidden crash: the detail page rendered `entity.port.toString()`, which
throws for API-created port-less array nodes.
- The port cell renders `-` for missing ports (same guard the priority
column already had).
- The DOM-flush sync no longer falls back to `port: 80` for port-less rows
(it would have re-invented a port on save).
**Data-plane verification** (beyond Admin-API acceptance, which proved too
weak a signal — it accepts host `"["`): both transformed shapes were exercised
end-to-end on a live gateway. A port-less array node forwarded to an nginx
backend on the scheme default (HTTP 200), and a bracketed-IPv6 node (`{host:
"[::1]", port: 9080}`, with `enable_ipv6: true`) was dialed successfully by the
proxy (HTTP 200 through a fault-injection echo route).
**Tests** (red on the unfixed build at the intended assertions, green after):
- Unit `nodes-conversion.test.ts`: 8 cases covering every key shape, no
invented port/priority, non-numeric tails kept as part of the host.
- E2E regression `upstreams.object-form-nodes.spec.ts`: an upstream seeded
with `{"httpbin.org": 1, "[::1]:1980": 2}` displays correctly (no shredded
host, no invented port) and survives a no-op Edit → Save with the port-less
node still port-less and the IPv6 node intact.
Blast radius: full local e2e suite — 169 passed; the 4 failures are
documented environment items unrelated to this change (2 known Monaco read-back
races that reproduce on pristine master, 1 late-serial-run load flake green on
isolated + 4× repeat, and `stream_routes.show-disabled-error` which cannot run
outside the repo's own compose project). Unit tests 18/18, lint and build clean.
**Related issues**
Part of #3417 (please do not auto-close the tracking issue)
**Checklist:**
- [x] Did you explain what problem does this PR solve? Or what new features
have been added?
- [x] Have you added corresponding test cases?
- [ ] Have you modified the corresponding document? (no user-facing document
covers the nodes editor)
- [x] Is this PR backward compatible? If it is not backward compatible,
please discuss on the mailing list first
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
