LiteSun opened a new pull request, #3477:
URL: https://github.com/apache/apisix-dashboard/pull/3477
Please answer these questions before submitting a pull request, **or your PR
will get closed**.
**Why submit this pull request?**
- [x] Bugfix
- [ ] New feature provided
- [ ] Improve performance
- [ ] Backport patches
**What changes will this PR take into?**
A route (or service / stream route) whose inline upstream section the user
never opened could not be saved from the dashboard: the request carried a
half-built `upstream` object and the Admin API rejected it. This is #3395 — a
plugins-only route such as the `public-api` metrics example needs no upstream,
yet the form forced one.
### Root cause
The Route / Service / Stream Route forms always render the inline upstream
section, and its Select / Switch widgets register a value the moment they mount
(`scheme=http`, `type=roundrobin`, `hash_on=vars`, `pass_host=pass`,
`tls.verify=false`, plus `discovery_args={}` from the JSON input).
react-hook-form keeps those in the submit values, so the Add Route form filled
with only Name + URI + `public-api: {}` POSTs
```json
{"uri":"/…","upstream":{"scheme":"http","type":"roundrobin","hash_on":"vars","pass_host":"pass","tls":{"verify":false},"discovery_args":{}},"enable_websocket":false,"priority":0,"status":1,"name":"…","plugins":{"public-api":{}}}
```
and the Admin API answers `400 invalid configuration: property "upstream"
validation failed: value should match only one schema, but matches none` (an
upstream needs `nodes` or discovery). The same happens on a no-op Edit → Save
of a plugins-only route created through the Admin API, so the API workaround
only covers creation.
### Why `master` looked fixed
The issue thread concluded the client-side problem was gone on `master`. It
is not — it was hidden by the React build flavour:
| Build | React | Request body | Result |
| --- | --- | --- | --- |
| APISIX 3.16.0 image (embeds bbe05ad9) | production | `upstream: {scheme,
type, …}` (and the `public-api: {}` entry dropped, pre-#3277) | 400 |
| APISIX 3.17.0 image (embeds c8d3466d) | production | `upstream: {scheme,
type, …}` | 400 |
| `master` (713803e3), plain `vite build` | production | `upstream: {scheme,
type, …, discovery_args: {}}` | 400 |
| `master`, `vite dev` | development | `{uri, name, plugins}` | 201 |
| `master`, `vite dev` with `<StrictMode>` removed | development |
`upstream: {scheme, type, …}` | 400 |
| `master`, e2e image (`NODE_ENV=test` build) | development | `{uri, name,
plugins}` | 201 |
| this branch, `pnpm build --mode test` | production | `{uri, name,
plugins}` | 201 |
Under `<StrictMode>` the double-invoked controller effects run
react-hook-form's `shouldUnregister` cleanup, which wipes the mount-time values
from the form state again. `vite dev` is development React by design, and the
e2e image was too: `ENV NODE_ENV=test` makes Vite bundle development React
(verified by grepping `react-dom.development` in the shipped assets). So
neither the dev server nor CI could ever see this class of bug — the `services
… neither — a service with no upstream at all still saves` case from #3447
passes in CI only for that reason, and fails against a production build.
### Fix
- `src/components/form-slice/FormPartUpstream/util.ts`:
`produceRmEmptyUpstreamFields` — the producer shared by the route, service and
stream-route submit pipelines — now drops an inline `upstream` that holds
nothing but the widgets' mount-time defaults (`isUntouchedInlineUpstream`).
Every value in such an upstream is the gateway default, so nothing is lost.
Anything the user did supply (a node row, even an empty one; a different
scheme; a name; health checks; …) keeps the upstream in the request for the
Admin API to validate — silently discarding user input is the #3417 failure
mode and is deliberately avoided. The defaults are read from the same schema
constants the widgets use, so they cannot drift apart silently.
- `e2e/server/Dockerfile`: build the e2e image with `pnpm build --mode test`
instead of `NODE_ENV=test`, so the suite runs against the production React the
release ships while `import.meta.env.MODE === 'test'` still hides the devtools.
- `src/components/form/Editor.tsx`: the `window.__monacoEditor__` test seam
the Playwright helpers rely on was keyed on `NODE_ENV !== 'production'` (the
reason the image set `NODE_ENV=test`); it is now keyed on `import.meta.env.DEV
|| import.meta.env.MODE === 'test'`.
Not changed here: an inline upstream that *does* have nodes still carries
`discovery_args: {}` from the JSON widget's mount value (kept on purpose by the
#3438 producer). The Admin API accepts and stores it; worth a separate look.
### Tests
- `src/components/form-slice/FormPartUpstream/util.test.ts` (35 cases): the
untouched-upstream predicate across every widget a user can touch,
`produceRmEmptyUpstreamFields` itself, and the three real submit pipelines
(route, service, stream route) fed with the exact values react-hook-form holds
for a plugins-only Add Route form.
- `e2e/tests/regression/routes.plugins-only-no-upstream.spec.ts`: add page
(only Name + URI + `public-api`) and edit page (no-op save of an API-created
plugins-only route); both capture the request body and assert no `upstream` is
sent and none is stored. With the Dockerfile change these run against
production React, where they fail without the fix.
### Verification
- `pnpm test`: 198 passed (163 before + 35 new)
- `pnpm lint`, `pnpm build --mode test` (tsc + vite) clean; the built assets
contain `react-dom-client.production` and no `react-dom.development`; the
rebuilt e2e image embeds the same
- Standalone reproduction (fills the form, captures the XHR) against the
APISIX 3.16.0 and 3.17.0 images and a production build of `master`: 400 as
above; against a production build of this branch: 201 with
`{"uri","name","plugins"}` only
- Full e2e suite against the rebuilt e2e image (production React): in
progress locally, CI run attached to this PR
**Related issues**
fix #3395
**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 documentation impact)
- [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]
