This is an automated email from the ASF dual-hosted git repository.
Yilialinn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-dashboard.git
The following commit(s) were added to refs/heads/master by this push:
new 0302b31cd fix: no-op edit-save deleted inline upstream health checks
(#3423)
0302b31cd is described below
commit 0302b31cd69791df30611acfb05634f71b9295e7
Author: Ming Wen <[email protected]>
AuthorDate: Mon Jul 13 17:21:37 2026 +0800
fix: no-op edit-save deleted inline upstream health checks (#3423)
---
.../form.noop-edit-preserves-checks.spec.ts | 180 +++++++++++++++++++++
src/components/form-slice/FormPartUpstream/util.ts | 12 +-
src/routes/services/detail.$id/index.tsx | 8 +-
src/routes/stream_routes/detail.$id.tsx | 12 +-
src/types/schema/apisix/upstreams.ts | 6 +-
5 files changed, 212 insertions(+), 6 deletions(-)
diff --git a/e2e/tests/regression/form.noop-edit-preserves-checks.spec.ts
b/e2e/tests/regression/form.noop-edit-preserves-checks.spec.ts
new file mode 100644
index 000000000..da44885d6
--- /dev/null
+++ b/e2e/tests/regression/form.noop-edit-preserves-checks.spec.ts
@@ -0,0 +1,180 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Regression for apache/apisix-dashboard#3414: opening a route/service whose
+// INLINE upstream has health checks, clicking Edit, and saving WITHOUT
+// changing anything silently deleted `upstream.checks`. Root cause: the
+// detail pages reset the form with `produceToNestedUpstreamForm`, which set
+// the `__checksEnabled` UI flag nested under `upstream.` while the checks
+// section reads it from the form ROOT — the section unmounted and
+// `shouldUnregister: true` dropped `checks` from the PUT body.
+//
+// A save that changes nothing must be a no-op.
+
+import { routesPom } from '@e2e/pom/routes';
+import { servicesPom } from '@e2e/pom/services';
+import { randomId } from '@e2e/utils/common';
+import { e2eReq } from '@e2e/utils/req';
+import { test } from '@e2e/utils/test';
+import { uiGoto } from '@e2e/utils/ui';
+import { expect } from '@playwright/test';
+
+import { deleteAllRoutes } from '@/apis/routes';
+import { deleteAllServices } from '@/apis/services';
+import type { APISIXType } from '@/types/schema/apisix';
+
+// deliberately sparse: no active.type, no active.healthy, no passive.type —
+// this mirrors what the UI's own create flow stores (only the unhealthy
+// side configured) and pins the zod over-constraint that required
+// active.healthy and silently blocked every save of such a route
+const checks = {
+ active: {
+ http_path: '/health',
+ unhealthy: { interval: 2, http_failures: 3 },
+ },
+ passive: {
+ unhealthy: { http_failures: 3, tcp_failures: 3 },
+ },
+};
+
+const upstream = {
+ type: 'roundrobin',
+ nodes: { 'noop-edit.local:80': 1 },
+ checks,
+};
+
+// The detail pages reset the form again when the mount-time background
+// refetch returns — that reset is exactly what used to wipe the checks
+// flags. Waiting until the resource GETs have settled makes the spec
+// exercise the post-reset state instead of racing ahead of it.
+const waitForDetailQueriesToSettle = async (getCount: () => number) => {
+ let lastSeen = -1;
+ await expect
+ .poll(
+ () => {
+ const settled = getCount() > 0 && getCount() === lastSeen;
+ lastSeen = getCount();
+ return settled;
+ },
+ { timeout: 15000, intervals: [1000] }
+ )
+ .toBe(true);
+};
+
+test.beforeAll(async () => {
+ await deleteAllRoutes(e2eReq);
+ await deleteAllServices(e2eReq);
+});
+
+test.afterAll(async () => {
+ await deleteAllRoutes(e2eReq);
+ await deleteAllServices(e2eReq);
+});
+
+test('no-op edit-save preserves route inline upstream health checks', async ({
+ page,
+}) => {
+ const name = randomId('reg-noop-route');
+ const res = await e2eReq.put<{
+ value: APISIXType['Route'];
+ }>(`/routes/${name}`, { name, uri: `/reg-noop/${name}`, upstream });
+ const id = res.data.value.id;
+
+ // load the detail page directly (full page load): the bug only
+ // reproduces on a fresh mount, not via SPA navigation from the list
+ let routeGets = 0;
+ page.on('response', (res) => {
+ if (
+ res.request().method() === 'GET' &&
+ res.url().includes(`/apisix/admin/routes/${id}`)
+ )
+ routeGets += 1;
+ });
+
+ await uiGoto(page, '/routes/detail/$id', { id });
+ await routesPom.isDetailPage(page);
+ await waitForDetailQueriesToSettle(() => routeGets);
+ // deterministic discriminator: after the post-refetch reset, the form
+ // must still show health checks as enabled — the bug's first visible
+ // symptom was this switch reading off
+ await expect(page.getByTestId('checksEnabled')).toBeChecked();
+
+ await page.getByRole('button', { name: 'Edit' }).click();
+ await page.getByRole('button', { name: 'Save' }).click();
+ await expect(
+ page.getByRole('alert').filter({ hasText: /success/i })
+ ).toBeVisible();
+
+ const after = await e2eReq.get<{ value: APISIXType['Route'] }>(
+ `/routes/${id}`
+ );
+ expect(after.data.value.upstream?.checks).toBeTruthy();
+ expect(after.data.value.upstream?.checks?.active?.http_path).toBe(
+ '/health'
+ );
+ // the passive block must survive too (its own flag drives a separate
+ // sub-section that can unmount independently)
+ expect(
+ after.data.value.upstream?.checks?.passive?.unhealthy?.http_failures
+ ).toBe(3);
+});
+
+test('no-op edit-save preserves service inline upstream health checks', async
({
+ page,
+}) => {
+ const name = randomId('reg-noop-service');
+ const res = await e2eReq.put<{
+ value: APISIXType['Service'];
+ }>(`/services/${name}`, { name, upstream });
+ const id = res.data.value.id;
+
+ // load the detail page directly (full page load): the bug only
+ // reproduces on a fresh mount, not via SPA navigation from the list
+ let serviceGets = 0;
+ page.on('response', (res) => {
+ if (
+ res.request().method() === 'GET' &&
+ res.url().includes(`/apisix/admin/services/${id}`)
+ )
+ serviceGets += 1;
+ });
+
+ await uiGoto(page, '/services/detail/$id', { id });
+ await servicesPom.isDetailPage(page);
+ await waitForDetailQueriesToSettle(() => serviceGets);
+ // deterministic discriminator (see the route variant above)
+ await expect(page.getByTestId('checksEnabled')).toBeChecked();
+
+ await page.getByRole('button', { name: 'Edit' }).click();
+ await page.getByRole('button', { name: 'Save' }).click();
+ await expect(
+ page.getByRole('alert').filter({ hasText: /success/i })
+ ).toBeVisible();
+
+ const after = await e2eReq.get<{ value: APISIXType['Service'] }>(
+ `/services/${id}`
+ );
+ expect(after.data.value.upstream?.checks).toBeTruthy();
+ expect(after.data.value.upstream?.checks?.active?.http_path).toBe(
+ '/health'
+ );
+ // the passive block must survive too (its own flag drives a separate
+ // sub-section that can unmount independently)
+ expect(
+ after.data.value.upstream?.checks?.passive?.unhealthy?.http_failures
+ ).toBe(3);
+});
diff --git a/src/components/form-slice/FormPartUpstream/util.ts
b/src/components/form-slice/FormPartUpstream/util.ts
index 7229e5749..84702863e 100644
--- a/src/components/form-slice/FormPartUpstream/util.ts
+++ b/src/components/form-slice/FormPartUpstream/util.ts
@@ -39,7 +39,17 @@ export const produceToNestedUpstreamForm = produce((draft:
Record<string, unknow
__checksPassiveEnabled?: boolean;
};
if (d.upstream && typeof d.upstream === 'object' &&
!Array.isArray(d.upstream)) {
- d.upstream = produceToUpstreamForm(d.upstream, d.upstream) as
Record<string, unknown>;
+ const upstream = d.upstream as Partial<APISIXType['Upstream']>;
+ d.upstream = produceToUpstreamForm(upstream, upstream) as Record<string,
unknown>;
+ // The health-check switches live at the form ROOT: FormSectionChecks
+ // registers `__checksEnabled` unprefixed and reads it from the root.
+ // They must be derived from the nested upstream's checks — otherwise a
+ // reset with this producer leaves them undefined, the checks section
+ // renders "disabled", its inputs unmount, and `shouldUnregister: true`
+ // silently drops `upstream.checks` from the next PUT (#3414).
+ d.__checksEnabled = !!upstream.checks && isNotEmpty(upstream.checks);
+ d.__checksPassiveEnabled =
+ !!upstream.checks?.passive && isNotEmpty(upstream.checks.passive);
}
// Also handle top-level checks if they exist
if (d.checks) {
diff --git a/src/routes/services/detail.$id/index.tsx
b/src/routes/services/detail.$id/index.tsx
index daee276d6..a0a5a70f9 100644
--- a/src/routes/services/detail.$id/index.tsx
+++ b/src/routes/services/detail.$id/index.tsx
@@ -66,7 +66,13 @@ const ServiceDetailForm = (props: Props) => {
shouldFocusError: true,
mode: 'all',
disabled: readOnly,
- defaultValues: serviceData.value,
+ // must run through the producer so the root-level __checksEnabled
+ // flags exist from the very first mount — otherwise the checks
+ // sections mount only after the reset below and their fields miss
+ // the reset values (passive block was dropped from the next PUT)
+ defaultValues: produceToNestedUpstreamForm(
+ serviceData.value
+ ) as typeof serviceData.value,
});
useEffect(() => {
diff --git a/src/routes/stream_routes/detail.$id.tsx
b/src/routes/stream_routes/detail.$id.tsx
index facb08f7d..6682a7a65 100644
--- a/src/routes/stream_routes/detail.$id.tsx
+++ b/src/routes/stream_routes/detail.$id.tsx
@@ -33,6 +33,7 @@ import { putStreamRouteReq } from '@/apis/stream_routes';
import { FormSubmitBtn } from '@/components/form/Btn';
import { produceRoute } from '@/components/form-slice/FormPartRoute/util';
import { FormPartStreamRoute } from
'@/components/form-slice/FormPartStreamRoute';
+import { produceToNestedUpstreamForm } from
'@/components/form-slice/FormPartUpstream/util';
import { FormTOCBox } from '@/components/form-slice/FormSection';
import { FormSectionGeneral } from
'@/components/form-slice/FormSectionGeneral';
import { DeleteResourceBtn } from '@/components/page/DeleteResourceBtn';
@@ -62,11 +63,18 @@ const StreamRouteDetailForm = (props: Props) => {
shouldFocusError: true,
mode: 'all',
disabled: readOnly,
- defaultValues: streamRouteData.value,
+ // producer required here too: the root-level __checksEnabled flags
+ // must exist from the first mount (see services detail)
+ defaultValues: produceToNestedUpstreamForm(
+ streamRouteData.value
+ ) as typeof streamRouteData.value,
});
useEffect(() => {
- form.reset(streamRouteData.value);
+ // derive the root-level __checksEnabled flags from the nested
+ // upstream, or the checks section unmounts and drops upstream.checks
+ // from the next PUT (#3414)
+ form.reset(produceToNestedUpstreamForm(streamRouteData.value));
}, [streamRouteData, form]);
const putStreamRoute = useMutation({
diff --git a/src/types/schema/apisix/upstreams.ts
b/src/types/schema/apisix/upstreams.ts
index ed347d3f6..232c3ca3b 100644
--- a/src/types/schema/apisix/upstreams.ts
+++ b/src/types/schema/apisix/upstreams.ts
@@ -128,8 +128,10 @@ const UpstreamHealthCheckActive = z.object({
http_path: z.string().optional(),
https_verify_certificate: z.boolean().optional(),
http_request_headers: z.array(z.string()).optional(),
- healthy: UpstreamHealthCheckActiveHealthy.partial(),
- unhealthy: UpstreamHealthCheckActiveUnhealthy.partial(),
+ // both are optional in the Admin API (defaults apply); requiring them
+ // rejected API-stored checks that only configure one side (#3414 CI)
+ healthy: UpstreamHealthCheckActiveHealthy.partial().optional(),
+ unhealthy: UpstreamHealthCheckActiveUnhealthy.partial().optional(),
});
const UpstreamHealthCheckPassiveType = UpstreamHealthCheckActiveType;
