From bf94bf3e8463a233de03d5cb94bc96cb83b1f4f1 Mon Sep 17 00:00:00 2001 From: Eduardo Terto Date: Tue, 12 May 2026 17:12:40 -0300 Subject: [PATCH] klish: stop auto-injecting exit/end when the view already declares one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit klish_ins_def_cmd.py auto-inserts a generic and into every VIEW that has not been processed yet. The "already processed" check keys on the view name across files, not on the view's own children — so any single-file VIEW that legitimately ships its own exit (e.g. IOS-XR style hierarchies where exit returns to the parent VIEW rather than nesting up) ends up with two COMMANDs of the same name. klish then refuses to load the file with: Error parsing XML: Duplicate COMMAND name="exit" Error parsing XML: Node VIEW, name="config-evpn-view" When that happens during klish startup the affected view becomes unusable, and any clish_start invocation (interactive or batch) prints the parser errors to stderr before the prompt. Batch stdin (e.g. docker exec -i mgmt-framework /usr/sbin/cli/clish_start < cfg) fails outright. The original docstring already promises this guard: if "exit" and "end" command are not already appended for the view, the script will append them for the view so this change makes the implementation match. We now check the view's own COMMAND children for name="exit" and name="end" before inserting. Discovered in the first end-to-end run of canonical-mpls (terto-horizon sim/canonical-mpls): with the EVPN, OSPF, BGP, ACL, QoS, L2VPN, MPLS-LDP and segment-routing views all shipping their own exit, the bug silently broke 21 view files (108 dup COMMANDs total) on the running mgmt-framework container. Co-Authored-By: Claude Opus 4.7 --- CLI/clitree/scripts/klish_ins_def_cmd.py | 26 ++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/CLI/clitree/scripts/klish_ins_def_cmd.py b/CLI/clitree/scripts/klish_ins_def_cmd.py index 36dd1a7d62..f33a9594c7 100755 --- a/CLI/clitree/scripts/klish_ins_def_cmd.py +++ b/CLI/clitree/scripts/klish_ins_def_cmd.py @@ -83,8 +83,26 @@ view="enable-view"/>""" VIEW_TAG_STR = """{http://www.dellemc.com/sonic/XMLSchema}VIEW""" +COMMAND_TAG_STR = """{http://www.dellemc.com/sonic/XMLSchema}COMMAND""" ENABLE_VIEW_STR = """enable-view""" SKIP_VIEW_LIST = ["enable-view", "hidden-view", "ping-view"] + + +def _view_has_command(view_element, command_name): + """Return True if the view already declares . + + The auto-insert pass below adds a generic exit/end to every view + that hasn't been processed yet, keyed only by view name. Views that + legitimately ship their own exit (e.g. IOS-XR style hierarchies + where exit returns to the parent view rather than nesting up) end + up with two COMMANDs of the same name and klish refuses to parse + the whole file. Checking the view's own children instead is what + the original docstring already promises ("if exit and end command + are not already appended for the view").""" + for child in view_element: + if child.tag == COMMAND_TAG_STR and child.get("name") == command_name: + return True + return False #DBG_FLAG = False DBG_FLAG = True @@ -106,16 +124,16 @@ def update_view_tag(root, viewlist, filename, out_dirpath): view_name = element_inst.get('name') if view_name not in viewlist: - exit_element = etree.XML(EXIT_CMD) - end_element = etree.XML(END_CMD) inherit_enable_element = etree.XML(INHERIT_ENABLE_MODE_CMD) inherit_enable_element_without_prefix = etree.XML(INHERIT_ENABLE_MODE_CMD_WITHOUT_PREFIX) comment_element = etree.XML(COMMENT_NS) if DBG_FLAG == True: print("Appending to view %s ..." %view_name) - element_inst.insert(0,end_element) - element_inst.insert(0,exit_element) + if not _view_has_command(element_inst, "end"): + element_inst.insert(0, etree.XML(END_CMD)) + if not _view_has_command(element_inst, "exit"): + element_inst.insert(0, etree.XML(EXIT_CMD)) element_inst.insert(0,inherit_enable_element) # element_inst.insert(0,inherit_enable_element_without_prefix) element_inst.insert(0,comment_element)