This is an automated email from the ASF dual-hosted git repository.
guoqqqi 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 62593c4a3 fix: stop deep-cleaning inside plugin configs, keep nested
discovery_args (#3438)
62593c4a3 is described below
commit 62593c4a3c819b2f92fc6e287f5e09af6e90735a
Author: Yuhan <[email protected]>
AuthorDate: Thu Jul 23 16:20:54 2026 +0800
fix: stop deep-cleaning inside plugin configs, keep nested discovery_args
(#3438)
---
.../form.plugin-config-nested-empties.spec.ts | 113 +++++++++++++++++++++
src/routes/upstreams/detail.$id.tsx | 2 +-
src/utils/producer.test.ts | 95 +++++++++++++++++
src/utils/producer.ts | 83 +++++++++------
4 files changed, 259 insertions(+), 34 deletions(-)
diff --git a/e2e/tests/regression/form.plugin-config-nested-empties.spec.ts
b/e2e/tests/regression/form.plugin-config-nested-empties.spec.ts
new file mode 100644
index 000000000..4623102c8
--- /dev/null
+++ b/e2e/tests/regression/form.plugin-config-nested-empties.spec.ts
@@ -0,0 +1,113 @@
+/**
+ * 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 a data-integrity item of apache/apisix-dashboard#3417:
+// the submit pipeline deep-cleans every empty value ({} / [] / "" /
+// null), and the restore stage only brought back WHOLE plugin entries —
+// meaningful empty members inside a surviving plugin config were
+// silently removed on any edit-save, and `discovery_args: {}` on an
+// INLINE upstream was dropped (the #3376 fix only covered the upstreams
+// page's root-level field). The Admin API accepts and stores all these
+// shapes (verified live); plugin configs now pass through verbatim.
+
+import { routesPom } from '@e2e/pom/routes';
+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 type { APISIXType } from '@/types/schema/apisix';
+
+test.beforeAll(async () => {
+ await deleteAllRoutes(e2eReq);
+});
+
+test.afterAll(async () => {
+ await deleteAllRoutes(e2eReq);
+});
+
+test('no-op edit-save keeps empty members inside a plugin config', async ({
+ page,
+}) => {
+ const pluginConfig = {
+ empty_obj: {},
+ empty_arr: [],
+ empty_str: '',
+ kept: 'x',
+ };
+ const name = randomId('reg-nested-empty');
+ const res = await e2eReq.put<{ value: APISIXType['Route'] }>(
+ `/routes/${name}`,
+ {
+ name,
+ uri: `/reg-nested-empty/${name}`,
+ plugins: { 'key-auth': pluginConfig },
+ upstream: { type: 'roundrobin', nodes: { 'nested-empty.local:80': 1 } },
+ }
+ );
+ const id = res.data.value.id;
+
+ await uiGoto(page, '/routes/detail/$id', { id });
+ await routesPom.isDetailPage(page);
+ 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.plugins?.['key-auth']).toEqual(pluginConfig);
+});
+
+test('no-op edit-save keeps discovery_args on an inline upstream', async ({
+ page,
+}) => {
+ const name = randomId('reg-inline-disc');
+ const res = await e2eReq.put<{ value: APISIXType['Route'] }>(
+ `/routes/${name}`,
+ {
+ name,
+ uri: `/reg-inline-disc/${name}`,
+ upstream: {
+ type: 'roundrobin',
+ discovery_type: 'dns',
+ service_name: 'svc.local',
+ discovery_args: {},
+ },
+ }
+ );
+ const id = res.data.value.id;
+
+ await uiGoto(page, '/routes/detail/$id', { id });
+ await routesPom.isDetailPage(page);
+ 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}`
+ );
+ const upstream = after.data.value.upstream as Record<string, unknown>;
+ expect(upstream.discovery_type).toBe('dns');
+ expect(upstream.discovery_args).toEqual({});
+});
diff --git a/src/routes/upstreams/detail.$id.tsx
b/src/routes/upstreams/detail.$id.tsx
index cdd8a3d90..e05c66bbe 100644
--- a/src/routes/upstreams/detail.$id.tsx
+++ b/src/routes/upstreams/detail.$id.tsx
@@ -91,7 +91,7 @@ const UpstreamDetailForm = (
const putUpstream = useMutation({
mutationFn: (d: APISIXType['Upstream']) => {
// Merge original discovery_args into form data before processing,
- // so pipeProduce's produceRestoreEmptyPlugins can restore
discovery_args: {}
+ // so pipeProduce's produceCleanPreservingUserValues can restore
discovery_args: {}
// even if the field was not touched (and thus absent from d).
const merged = {
...d,
diff --git a/src/utils/producer.test.ts b/src/utils/producer.test.ts
index 224530992..3d8617f8a 100644
--- a/src/utils/producer.test.ts
+++ b/src/utils/producer.test.ts
@@ -76,4 +76,99 @@ describe('pipeProduce', () => {
// owned by the existing null-cleaner / empty-plugin-restore stages
expect(produced?.plugins['key-auth']).toBeTruthy();
});
+
+ // Regression for #3417 D2: plugin configs are user-authored JSON — the
+ // deep-clean silently removed meaningful empty members ({} / [] / "" /
+ // null) from configs that partially survived cleaning (the old restore
+ // only brought back WHOLE plugin entries). The gateway is the only
+ // judge of such members: loose-schema plugins accept and store them
+ // (verified live, 201), strict ones reject with a descriptive 400.
+ it('passes plugin configs through verbatim, empties included', () => {
+ const plugins = {
+ 'key-auth': {
+ empty_obj: {},
+ empty_arr: [],
+ empty_str: '',
+ note: null,
+ kept: 'x',
+ },
+ };
+ const val = {
+ uri: '/r2',
+ plugins,
+ // array-form nodes: this shape (the add-page default) once tripped
+ // an immer set-trap crash when the restore stage assigned the
+ // original base reference back into the draft — keep it here
+ upstream: {
+ type: 'roundrobin',
+ nodes: [{ host: 'a.local', port: 80, weight: 1 }],
+ },
+ };
+ const produced = pipeProduce()(val);
+ expect(produced.plugins).toEqual(plugins);
+ });
+
+ // #3438 review (LiteSun): the __-key cleaner runs over the whole draft
+ // before the plugins subtree is snapshotted, so a plugin config field
+ // that legitimately starts with __ was deleted — the "verbatim" claim
+ // did not hold. Plugin JSON is gateway-owned; nothing in it is a UI flag.
+ it('passes __-prefixed plugin config fields through verbatim', () => {
+ const plugins = { 'my-plugin': { __mode: 'strict', on: true } };
+ const val = {
+ uri: '/r-uu',
+ plugins,
+ upstream: {
+ type: 'roundrobin',
+ nodes: [{ host: 'a.local', port: 80, weight: 1 }],
+ },
+ };
+ const produced = pipeProduce()(val);
+ expect(produced.plugins).toEqual(plugins);
+ });
+
+ it('restores discovery_args on an inline upstream', () => {
+ const val = {
+ uri: '/r3',
+ upstream: {
+ type: 'roundrobin',
+ discovery_type: 'dns',
+ service_name: 'svc.local',
+ discovery_args: {},
+ },
+ };
+ const produced = pipeProduce()(val);
+ expect(produced.upstream.discovery_args).toEqual({});
+ });
+
+ // the routes detail page composes produceRoute — itself a pipeProduce —
+ // as a stage of another pipeProduce, so inner stages receive the outer
+ // DRAFT instead of a plain value; a restore stage that closed over the
+ // pipeline input crashed exactly here (immer set trap / structuredClone
+ // on a live proxy). Keep this composition pinned.
+ it('survives nested pipeProduce composition with plugins verbatim', () => {
+ const plugins = { 'basic-auth': { hide_credentials: true, e: {} } };
+ const val = {
+ uri: '/r4',
+ plugins,
+ upstream: {
+ type: 'roundrobin',
+ nodes: [{ host: 'a.local', port: 80, weight: 1 }],
+ },
+ };
+ const inner = pipeProduce();
+ const produced = pipeProduce((v: object) => inner(v))(val);
+ expect(produced.plugins).toEqual(plugins);
+ });
+
+ it('still restores discovery_args at the root (upstreams page)', () => {
+ const val = {
+ name: 'u1',
+ type: 'roundrobin',
+ discovery_type: 'dns',
+ service_name: 'svc.local',
+ discovery_args: {},
+ };
+ const produced = pipeProduce()(val);
+ expect(produced.discovery_args).toEqual({});
+ });
});
diff --git a/src/utils/producer.ts b/src/utils/producer.ts
index ced4ff78d..f9785b4e2 100644
--- a/src/utils/producer.ts
+++ b/src/utils/producer.ts
@@ -15,7 +15,7 @@
* limitations under the License.
*/
import { clean, type ICleanerOptions } from 'fast-clean';
-import { produce } from 'immer';
+import { current, isDraft, produce } from 'immer';
import { pipe } from 'rambdax';
import { produceTime } from './form-producer';
@@ -35,35 +35,56 @@ export const produceDeepCleanEmptyKeys = (opts:
ICleanerOptions = {}) =>
deepCleanEmptyKeys(draft, opts);
});
+/** plain deep snapshot of a value that may be an immer draft */
+const snapshot = <T>(v: T): T => (isDraft(v) ? (current(v as object) as T) :
v);
+
+const isEmptyObject = (v: unknown): v is object =>
+ !!v && typeof v === 'object' && Object.keys(v).length === 0;
+
/**
- * Preserves plugin entries with empty config ({}) after deep cleaning.
- * APISIX plugins like key-auth have no required fields and are valid with {}.
- * deepCleanEmptyKeys would strip them, so we restore them from the original.
+ * Deep-clean the draft while preserving values the cleaner must not own
+ * (#3417):
+ *
+ * - plugin configs are user-authored JSON, detached before the clean and
+ * reattached VERBATIM: the gateway is the only judge of empty members
+ * ({} / [] / "" / null) — loose-schema plugins accept and store them,
+ * strict ones reject with a descriptive 400 (both verified against a
+ * live Admin API). The old whole-entry-only restore silently removed
+ * empties inside partially-surviving configs (#3269/#3277 siblings).
+ * - discovery_args: {} is meaningful and accepted by the gateway;
+ * preserved at the root (upstreams page, #3376) and on an inline
+ * upstream (routes/services — the #3376 fix missed the nested case).
+ *
+ * Everything happens inside ONE draft with plain snapshots — a restore
+ * stage that closed over the pipeline's original value crashed under
+ * nested pipeProduce composition (the routes detail page composes
+ * produceRoute, itself a pipeProduce, as a stage of another pipeProduce,
+ * so inner stages receive the outer draft, not a plain value).
+ *
+ * The `__`-prefixed UI-flag removal (form-only fields like __checksEnabled)
+ * also runs here, AFTER the plugins subtree is detached — otherwise a
+ * plugin config field that legitimately begins with `__` would be
+ * stripped, breaking the verbatim contract (#3438 review).
*/
-export const produceRestoreEmptyPlugins = (original: object) =>
+export const produceCleanPreservingUserValues = (opts: ICleanerOptions = {}) =>
produce((draft: Record<string, unknown>) => {
- const orig = original as Record<string, unknown>;
- if (orig.plugins && typeof orig.plugins === 'object') {
- const origPlugins = orig.plugins as Record<string, unknown>;
- const draftPlugins = (draft.plugins ?? {}) as Record<string, unknown>;
- Object.keys(origPlugins).forEach((name) => {
- if (!(name in draftPlugins)) {
- draftPlugins[name] = origPlugins[name];
- }
- });
- if (Object.keys(draftPlugins).length > 0) {
- draft.plugins = draftPlugins;
- }
+ const plugins = snapshot(draft.plugins);
+ const rootDiscoveryArgs = snapshot(draft.discovery_args);
+ const upstreamDiscoveryArgs = snapshot(
+ (draft.upstream as Record<string, unknown> | undefined)?.discovery_args
+ );
+ delete draft.plugins;
+ rmDoubleUnderscoreKeys(draft);
+ deepCleanEmptyKeys(draft, opts);
+ if (plugins && typeof plugins === 'object') {
+ draft.plugins = plugins;
}
- // Restore discovery_args: {} if it was present in the original.
- // APISIX accepts empty discovery_args and deepCleanEmptyKeys would strip
it.
- if (
- 'discovery_args' in orig &&
- orig.discovery_args !== null &&
- typeof orig.discovery_args === 'object' &&
- Object.keys(orig.discovery_args as object).length === 0
- ) {
- (draft as Record<string, unknown>).discovery_args = {};
+ if (isEmptyObject(rootDiscoveryArgs)) {
+ draft.discovery_args = {};
+ }
+ const upstream = draft.upstream as Record<string, unknown> | undefined;
+ if (upstream && isEmptyObject(upstreamDiscoveryArgs)) {
+ upstream.discovery_args = {};
}
});
@@ -84,10 +105,6 @@ export const rmDoubleUnderscoreKeys = (obj: object) => {
return obj;
};
-export const produceRmDoubleUnderscoreKeys = produce((draft) => {
- rmDoubleUnderscoreKeys(draft);
-});
-
/**
* FIXME: type error
*/
@@ -100,10 +117,10 @@ export const pipeProduce = (...funcs: ((a: any) =>
unknown)[]) => {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
...fs,
- produceRmDoubleUnderscoreKeys,
produceTime,
- produceDeepCleanEmptyKeys(),
- produceRestoreEmptyPlugins(val as object)
+ // __-flag removal happens inside this stage, after the plugins
+ // subtree is detached (see #3438 review)
+ produceCleanPreservingUserValues()
)(draft) as never;
}) as T;
};
