This is an automated email from the ASF dual-hosted git repository. moonming pushed a commit to branch fix/nested-upstream-checks-loss in repository https://gitbox.apache.org/repos/asf/apisix-dashboard.git
commit a22f91d40dbf086cc0a79b11c0ca9f27697b549a Author: Ming Wen <[email protected]> AuthorDate: Fri Jul 10 15:38:01 2026 +0800 fix: no-op edit-save deleted inline upstream health checks Opening a route/service/stream-route whose inline upstream has health checks, clicking Edit, and saving WITHOUT changing anything silently deleted upstream.checks. The green success toast made the data loss invisible. Root cause: the detail pages reset the form with produceToNestedUpstreamForm, which wrote the __checksEnabled UI flags nested under 'upstream.' while FormSectionChecks registers and reads them at the form ROOT. After the reset (triggered by the mount-time background refetch) the root flag read undefined, the checks section rendered as disabled, its inputs unmounted, and shouldUnregister: true dropped upstream.checks from the PUT body. - produceToNestedUpstreamForm now derives the root-level flags from the nested upstream's checks - stream_routes detail resets through the producer (it reset with the raw API value, so the flags were never computed there) - regression e2e loads the detail page directly, waits for the detail queries to settle (the reset that wiped the flags), then performs a no-op edit-save and asserts checks survive via the Admin API — verified to FAIL on the unfixed code and pass with the fix Fixes #3414 --- .../form.noop-edit-preserves-checks.spec.ts | 159 +++++++++++++++++++++ src/components/form-slice/FormPartUpstream/util.ts | 12 +- src/routes/stream_routes/detail.$id.tsx | 6 +- 3 files changed, 175 insertions(+), 2 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..7c60d066c --- /dev/null +++ b/e2e/tests/regression/form.noop-edit-preserves-checks.spec.ts @@ -0,0 +1,159 @@ +/** + * 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'; + +const checks = { + active: { + type: 'http', + http_path: '/health', + healthy: { interval: 2, successes: 2 }, + unhealthy: { interval: 2, http_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); + + 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' + ); +}); + +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); + + 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' + ); +}); 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/stream_routes/detail.$id.tsx b/src/routes/stream_routes/detail.$id.tsx index facb08f7d..f0c46d603 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'; @@ -66,7 +67,10 @@ const StreamRouteDetailForm = (props: Props) => { }); 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({
