From 3c3013dd737f4d3070f096c23e0126998fef5926 Mon Sep 17 00:00:00 2001 From: oliver Date: Mon, 21 Sep 2026 18:55:59 +0800 Subject: [PATCH] Require explicit VRF bindings and toggle select-all in discover. Drop default-all-VRF mode so collects must bind VRFs manually; discover starts with none selected and select-all toggles to deselect. Co-authored-by: Cursor --- netx_api/biz_state/command_match.py | 22 +++--- netx_api/biz_state/profiles.py | 35 +++------ tests/test_zte_extended_parsers.py | 13 ++-- web/src/i18n/en.ts | 4 +- web/src/i18n/zh.ts | 4 +- web/src/pages/network/BizStatePage.tsx | 101 ++++++++----------------- 6 files changed, 61 insertions(+), 118 deletions(-) diff --git a/netx_api/biz_state/command_match.py b/netx_api/biz_state/command_match.py index 34f0e3f..3780c7c 100644 --- a/netx_api/biz_state/command_match.py +++ b/netx_api/biz_state/command_match.py @@ -88,16 +88,18 @@ def normalize_binding_dicts( return converted -def _optional_discover_placeholders(profile: ParseProfile) -> list[PlaceholderDef]: +def _discover_placeholders(profile: ParseProfile) -> list[PlaceholderDef]: return [ ph for ph in (profile.placeholders or []) - if (not ph.required) - and ph.bind_mode == "discover_select" - and str(ph.discover_profile_id or "").strip() + if ph.bind_mode == "discover_select" and str(ph.discover_profile_id or "").strip() ] +def _optional_discover_placeholders(profile: ParseProfile) -> list[PlaceholderDef]: + return [ph for ph in _discover_placeholders(profile) if not ph.required] + + def filter_discover_records( records: list[dict[str, Any]] | None, ph: PlaceholderDef, @@ -175,12 +177,12 @@ def expand_bindings_from_discover_records( records: list[dict[str, Any]] | None, ) -> list[tuple[str, dict[str, str]]]: """Build concrete commands from discover/parser records (e.g. config_vrf).""" - optional = _optional_discover_placeholders(profile) - if not optional: - raise ValueError(f"profile {profile.profile_id} has no optional discover placeholders") - if len(optional) != 1 or len(profile.placeholders) != 1: - raise ValueError(f"expand-all only supports a single optional placeholder: {profile.profile_id}") - ph = optional[0] + discover_phs = _discover_placeholders(profile) + if not discover_phs: + raise ValueError(f"profile {profile.profile_id} has no discover placeholders") + if len(discover_phs) != 1 or len(profile.placeholders) != 1: + raise ValueError(f"expand-from-discover only supports a single placeholder: {profile.profile_id}") + ph = discover_phs[0] values = filter_discover_records(records, ph) if not values: raise ValueError(f"no discover values for {profile.profile_id} ({ph.discover_profile_id})") diff --git a/netx_api/biz_state/profiles.py b/netx_api/biz_state/profiles.py index 3df317a..0d4ce27 100644 --- a/netx_api/biz_state/profiles.py +++ b/netx_api/biz_state/profiles.py @@ -35,7 +35,7 @@ class PlaceholderDef: discover_profile_id: str = "" discover_value_field: str = "" discover_label_field: str = "" - # When required=False and no bindings: collect expands all discover values. + # When required=False and no bindings: collect may expand all discover values. discover_filter_field: str = "" discover_filter_contains: str = "" @@ -291,21 +291,10 @@ _BGP_VRF_PLACEHOLDER = PlaceholderDef( discover_label_field="vrf_name", ) -# Single-VRF collect: bind selected VRFs, or leave empty → all (from config_vrf). -_VRF_PLACEHOLDER_ALL = PlaceholderDef( - name="vrf", - schema_field="vrf", - required=False, - bind_mode="discover_select", - discover_profile_id="zte.config_vrf", - discover_value_field="vrf_name", - discover_label_field="vrf_name", -) - _VRF_PLACEHOLDER_IPV4 = PlaceholderDef( name="vrf", schema_field="vrf", - required=False, + required=True, bind_mode="discover_select", discover_profile_id="zte.config_vrf", discover_value_field="vrf_name", @@ -317,7 +306,7 @@ _VRF_PLACEHOLDER_IPV4 = PlaceholderDef( _VRF_PLACEHOLDER_IPV6 = PlaceholderDef( name="vrf", schema_field="vrf", - required=False, + required=True, bind_mode="discover_select", discover_profile_id="zte.config_vrf", discover_value_field="vrf_name", @@ -754,8 +743,8 @@ def _zte_status_profiles() -> list[ParseProfile]: match=r"(?i)^\s*show\s+bgp\s+vpnv4\s+unicast\s+vrf\s+(?P\S+)\s+summary(?:\s*\|\s*one-line)?\s*$", textfsm_command="show bgp vpnv4 unicast summary", description=( - "Per-VRF BGP VPNv4 peer summary. Bind VRFs or leave empty for all " - "(from config VRF intent); aux: IPv4 FIB + config_vrf." + "Per-VRF BGP VPNv4 peer summary. Bind one or more VRFs; " + "aux: IPv4 FIB + config_vrf." ), placeholders=[_VRF_PLACEHOLDER_IPV4], fields=list(_BGP_PEER_FIELDS), @@ -786,8 +775,8 @@ def _zte_status_profiles() -> list[ParseProfile]: match=r"(?i)^\s*show\s+bgp\s+vpnv6\s+unicast\s+vrf\s+(?P\S+)\s+summary(?:\s*\|\s*one-line)?\s*$", textfsm_command="show bgp vpnv6 unicast summary", description=( - "Per-VRF BGP VPNv6 peer summary. Bind VRFs or leave empty for all " - "(from config VRF intent); aux: IPv6 FIB + config_vrf." + "Per-VRF BGP VPNv6 peer summary. Bind one or more VRFs; " + "aux: IPv6 FIB + config_vrf." ), placeholders=[_VRF_PLACEHOLDER_IPV6], fields=list(_BGP_PEER_FIELDS), @@ -1128,10 +1117,7 @@ def _zte_status_profiles() -> list[ParseProfile]: command_template="show ip forwarding route vrf | one-line", match=r"(?i)^\s*show\s+ip\s+forwarding\s+route\s+vrf\s+(?P\S+)(?:\s*\|\s*one-line)?\s*$", textfsm_command="show ip forwarding route", - description=( - "IPv4 FIB per VRF. Bind VRFs or leave empty for all ipv4 VRFs " - "(config VRF intent aux)." - ), + description="IPv4 FIB per VRF. Bind one or more VRFs (config VRF intent aux).", placeholders=[_VRF_PLACEHOLDER_IPV4], fields=list(_IP_ROUTE_FIELDS), tags=["route", "ipv4", "vrf"], @@ -1173,10 +1159,7 @@ def _zte_status_profiles() -> list[ParseProfile]: command_template="show ipv6 forwarding route vrf | one-line", match=r"(?i)^\s*show\s+ipv6\s+forwarding\s+route\s+vrf\s+(?P\S+)(?:\s*\|\s*one-line)?\s*$", textfsm_command="show ipv6 forwarding route", - description=( - "IPv6 FIB per VRF. Bind VRFs or leave empty for all ipv6 VRFs " - "(config VRF intent aux)." - ), + description="IPv6 FIB per VRF. Bind one or more VRFs (config VRF intent aux).", placeholders=[_VRF_PLACEHOLDER_IPV6], fields=list(_IPV6_ROUTE_FIELDS), tags=["route", "ipv6", "vrf"], diff --git a/tests/test_zte_extended_parsers.py b/tests/test_zte_extended_parsers.py index 0e03011..b12db95 100644 --- a/tests/test_zte_extended_parsers.py +++ b/tests/test_zte_extended_parsers.py @@ -492,11 +492,8 @@ class ZteExtendedParserTests(unittest.TestCase): self.assertEqual(hit.params.get("vrf"), "CUST_A") self.assertEqual(hit.params.get("neighbor"), "10.0.0.1") - # BGP VRF summary / Forwarding VRF: optional bind + config_vrf aux - from netx_api.biz_state.command_match import ( - EXPAND_ALL_COMMAND, - expand_bindings_from_discover_records, - ) + # BGP VRF summary / Forwarding VRF: required bind + config_vrf aux + from netx_api.biz_state.command_match import expand_bindings_from_discover_records from netx_api.biz_state.collect_session import resolve_aux_command for pid in ( @@ -508,15 +505,15 @@ class ZteExtendedParserTests(unittest.TestCase): prof = get_profile(pid) assert prof is not None self.assertTrue(prof.placeholders) - self.assertFalse(prof.placeholders[0].required) + self.assertTrue(prof.placeholders[0].required) self.assertEqual(prof.placeholders[0].discover_profile_id, "zte.config_vrf") self.assertTrue(any(a.profile_id == "zte.config_vrf" for a in prof.aux_commands)) v4 = get_profile("zte.bgp_vpnv4_vrf_summary") assert v4 is not None self.assertTrue(any(a.key == "ip_route" for a in v4.aux_commands)) - sentinel = expand_from_bindings(profile=v4, bindings=[]) - self.assertEqual(sentinel[0][0], EXPAND_ALL_COMMAND) + with self.assertRaises(ValueError): + expand_from_bindings(profile=v4, bindings=[]) bound = expand_from_bindings(profile=v4, bindings=[{"vrf": "CUST_A"}]) self.assertEqual(bound[0][0], "show bgp vpnv4 unicast vrf CUST_A summary | one-line") ra_ip = resolve_aux_command( diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 956b19b..3a4a687 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -237,13 +237,11 @@ const en = { cancel: "Close", bindTitle: "Select VRF bindings", unbound: "Not bound", - allVrfsDefault: "All VRFs (default)", bindHintRequired: "Bind VRF params before collect", - bindHintOptional: "Bind VRFs, or leave empty for all", discoverLoading: "Discovering VRFs…", discoverEmpty: "No VRFs discovered", selectAllVrfs: "Select all", - clearVrfs: "Clear", + deselectAllVrfs: "Deselect all", batches: "Batches", viewBatch: "Open", export: "Export", diff --git a/web/src/i18n/zh.ts b/web/src/i18n/zh.ts index 2fe3ac3..45b88c5 100644 --- a/web/src/i18n/zh.ts +++ b/web/src/i18n/zh.ts @@ -237,13 +237,11 @@ const zh = { cancel: "关闭", bindTitle: "选择 VRF 绑定", unbound: "未关联", - allVrfsDefault: "默认全部 VRF", bindHintRequired: "需关联 VRF 参数后才可采集", - bindHintOptional: "可关联指定 VRF，未关联则采集全部", discoverLoading: "正在发现 VRF…", discoverEmpty: "未发现可用 VRF", selectAllVrfs: "全选", - clearVrfs: "清空", + deselectAllVrfs: "取消全选", batches: "采集批次", viewBatch: "查看", export: "导出", diff --git a/web/src/pages/network/BizStatePage.tsx b/web/src/pages/network/BizStatePage.tsx index 21a0e2c..3b240c4 100644 --- a/web/src/pages/network/BizStatePage.tsx +++ b/web/src/pages/network/BizStatePage.tsx @@ -800,7 +800,9 @@ export function BizStatePage() { const existing = (item.bindings || []) .filter((b: any) => b.placeholder === ph.name) .map((b: any) => String(b.value)); - setSelectedVrfs(existing.length ? existing : cand.map((c) => c.value)); + // Keep prior bindings if still in candidates; otherwise start with none selected. + const keep = existing.filter((v: string) => cand.some((c) => c.value === v)); + setSelectedVrfs(keep); if (!cand.length) { setDiscoverError(t("bizState.discoverEmpty")); } @@ -819,9 +821,8 @@ export function BizStatePage() { const item = (detail?.items || []).find((it: any) => it.id === bindItemId); const prof = profiles.find((p) => p.profile_id === item?.source_profile_id); const phName = (prof?.placeholders || [])[0]?.name || "vrf"; - const bindOptional = (prof?.placeholders || []).every((ph) => ph.required === false); const picked = values !== undefined ? values : selectedVrfs; - if (!picked.length && !bindOptional) return; + if (!picked.length) return; setBusy(true); try { await bizStateSetBindings( @@ -1352,15 +1353,10 @@ export function BizStatePage() { const enabled = Boolean(it?.enabled); const binds = it?.bindings || []; const needsBind = (prof.placeholders || []).length > 0; - const bindOptional = (prof.placeholders || []).every( - (ph) => ph.required === false, - ); const bindHint = needsBind ? binds.length ? binds.map((b: any) => b.value).join(", ") - : bindOptional - ? t("bizState.allVrfsDefault") - : t("bizState.unbound") + : t("bizState.unbound") : "—"; return ( @@ -1376,22 +1372,12 @@ export function BizStatePage() {
{prof.title}
{prof.description ?
{prof.description}
: null} {needsBind ? ( -
- {bindOptional - ? t("bizState.bindHintOptional") - : t("bizState.bindHintRequired")} -
+
{t("bizState.bindHintRequired")}
) : null} {needsBind ? ( - + {bindHint} ) : ( @@ -1577,17 +1563,17 @@ export function BizStatePage() { size="sm" variant="ghost" isDisabled={busy} - onPress={() => setSelectedVrfs(candidates.map((c) => c.value))} + onPress={() => { + const allSelected = + candidates.length > 0 && + candidates.every((c) => selectedVrfs.includes(c.value)); + setSelectedVrfs(allSelected ? [] : candidates.map((c) => c.value)); + }} > - {t("bizState.selectAllVrfs")} - -
@@ -1614,43 +1600,22 @@ export function BizStatePage() { ) : null} - {(() => { - const item = (detail?.items || []).find((it: any) => it.id === bindItemId); - const prof = profiles.find((p) => p.profile_id === item?.source_profile_id); - const bindOptional = (prof?.placeholders || []).every((ph) => ph.required === false); - return ( - <> - - {bindOptional ? ( - - ) : null} - - - ); - })()} + +