From cc8c22d153d4ca1fae07b3421198a6c3b5de30c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 20:10:16 +0000 Subject: [PATCH 1/4] Initial plan From 114817d411f959e224450abe47375afa19239d1f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 20:15:43 +0000 Subject: [PATCH 2/4] Fix: Skip removing 'switchport trunk allowed vlan 1' as it's the default state Co-authored-by: qkalsky <234014210+qkalsky@users.noreply.github.com> --- .../cisco/command_actions/iface_actions.py | 7 ++ .../command_actions/test_iface_actions.py | 68 +++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/cloudshell/networking/cisco/command_actions/iface_actions.py b/cloudshell/networking/cisco/command_actions/iface_actions.py index 7639b7f..e49b0e6 100644 --- a/cloudshell/networking/cisco/command_actions/iface_actions.py +++ b/cloudshell/networking/cisco/command_actions/iface_actions.py @@ -114,6 +114,13 @@ def clean_interface_switchport_config( for line in current_config.splitlines(): if line.strip(" ").startswith("switchport "): + # Skip removing "switchport trunk allowed vlan 1" as it's the default state + if re.match( + r"^\s*switchport\s+trunk\s+allowed\s+vlan\s+1\s*$", + line, + re.IGNORECASE, + ): + continue line_to_remove = re.sub(r"\s+\d+[-\d+,]+", "", line).strip(" ") CommandTemplateExecutor( self._cli_service, diff --git a/tests/networking/cisco/command_actions/test_iface_actions.py b/tests/networking/cisco/command_actions/test_iface_actions.py index 996013d..3ed70c4 100644 --- a/tests/networking/cisco/command_actions/test_iface_actions.py +++ b/tests/networking/cisco/command_actions/test_iface_actions.py @@ -58,3 +58,71 @@ def test_get_no_l2_protocol_tunnel_cmd(self, vlan_templates_mock, cte_mock): error_map=None, ) self.assertEqual(result, cte_mock.return_value) + + @patch( + "cloudshell.networking.cisco.command_actions.iface_actions" + ".CommandTemplateExecutor" + ) + def test_clean_interface_switchport_config_preserves_vlan_1(self, cte_mock): + """Test that switchport trunk allowed vlan 1 is not removed (default state).""" + current_config = """Building configuration... + +Current configuration : 144 bytes +! +interface GigabitEthernet110/1/0/6 + description KG-255X-06-PT + switchport + switchport trunk allowed vlan 1 + switchport mode dynamic auto +end +""" + executor_mock = MagicMock() + cte_mock.return_value = executor_mock + + self._handler.clean_interface_switchport_config(current_config) + + # Verify that execute_command was called for other switchport lines but not for vlan 1 + calls = executor_mock.execute_command.call_args_list + # Should be called twice: once for "switchport" and once for "switchport mode dynamic auto" + # but NOT for "switchport trunk allowed vlan 1" + self.assertEqual(len(calls), 2) + + # Verify the commands that were issued + called_commands = [call[1]["command"] for call in calls] + self.assertIn("switchport", called_commands) + self.assertIn("switchport mode dynamic auto", called_commands) + # Ensure vlan 1 command was NOT called + self.assertNotIn("switchport trunk allowed vlan", called_commands) + + @patch( + "cloudshell.networking.cisco.command_actions.iface_actions" + ".CommandTemplateExecutor" + ) + def test_clean_interface_switchport_config_removes_other_vlans(self, cte_mock): + """Test that switchport trunk allowed vlan with other VLANs are removed.""" + current_config = """Building configuration... + +Current configuration : 144 bytes +! +interface GigabitEthernet110/1/0/6 + description KG-255X-06-PT + switchport + switchport trunk allowed vlan 100 + switchport mode trunk +end +""" + executor_mock = MagicMock() + cte_mock.return_value = executor_mock + + self._handler.clean_interface_switchport_config(current_config) + + # Verify that execute_command was called for all switchport lines including vlan 100 + calls = executor_mock.execute_command.call_args_list + # Should be called three times: "switchport", "switchport trunk allowed vlan", "switchport mode trunk" + self.assertEqual(len(calls), 3) + + # Verify the commands that were issued + called_commands = [call[1]["command"] for call in calls] + self.assertIn("switchport", called_commands) + self.assertIn("switchport trunk allowed vlan", called_commands) + self.assertIn("switchport mode trunk", called_commands) From cd1256f9d15d14a341c47544ab4496c7519c26d8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 20:20:47 +0000 Subject: [PATCH 3/4] Refactor: Extract VLAN 1 pattern to module-level constant Co-authored-by: qkalsky <234014210+qkalsky@users.noreply.github.com> --- .../networking/cisco/command_actions/iface_actions.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cloudshell/networking/cisco/command_actions/iface_actions.py b/cloudshell/networking/cisco/command_actions/iface_actions.py index e49b0e6..71d032b 100644 --- a/cloudshell/networking/cisco/command_actions/iface_actions.py +++ b/cloudshell/networking/cisco/command_actions/iface_actions.py @@ -12,6 +12,9 @@ iface, ) +# Pattern to match VLAN 1 configuration (default state that should be preserved) +VLAN_1_PATTERN = r"^\s*switchport\s+trunk\s+allowed\s+vlan\s+1\s*$" + class IFaceActions: def __init__(self, cli_service, logger): @@ -115,11 +118,7 @@ def clean_interface_switchport_config( for line in current_config.splitlines(): if line.strip(" ").startswith("switchport "): # Skip removing "switchport trunk allowed vlan 1" as it's the default state - if re.match( - r"^\s*switchport\s+trunk\s+allowed\s+vlan\s+1\s*$", - line, - re.IGNORECASE, - ): + if re.match(VLAN_1_PATTERN, line, re.IGNORECASE): continue line_to_remove = re.sub(r"\s+\d+[-\d+,]+", "", line).strip(" ") CommandTemplateExecutor( From bd8f49d133157b256b575154a94d05c8a1fe9abb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 20:23:44 +0000 Subject: [PATCH 4/4] Fix test assertions and add test for VLAN ranges Co-authored-by: qkalsky <234014210+qkalsky@users.noreply.github.com> --- .../command_actions/test_iface_actions.py | 50 +++++++++++++++---- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/tests/networking/cisco/command_actions/test_iface_actions.py b/tests/networking/cisco/command_actions/test_iface_actions.py index 3ed70c4..bbcf294 100644 --- a/tests/networking/cisco/command_actions/test_iface_actions.py +++ b/tests/networking/cisco/command_actions/test_iface_actions.py @@ -83,16 +83,17 @@ def test_clean_interface_switchport_config_preserves_vlan_1(self, cte_mock): # Verify that execute_command was called for other switchport lines but not for vlan 1 calls = executor_mock.execute_command.call_args_list - # Should be called twice: once for "switchport" and once for "switchport mode dynamic auto" + # Should be called once for "switchport mode dynamic auto" # but NOT for "switchport trunk allowed vlan 1" - self.assertEqual(len(calls), 2) + # Note: "switchport" alone (without trailing space) is not matched by the pattern + self.assertEqual(len(calls), 1) - # Verify the commands that were issued + # Verify the command that was issued called_commands = [call[1]["command"] for call in calls] - self.assertIn("switchport", called_commands) self.assertIn("switchport mode dynamic auto", called_commands) - # Ensure vlan 1 command was NOT called - self.assertNotIn("switchport trunk allowed vlan", called_commands) + # Ensure vlan 1 line was NOT processed (would appear as "switchport trunk allowed vlan" after regex) + for cmd in called_commands: + self.assertNotIn("trunk allowed vlan", cmd) @patch( "cloudshell.networking.cisco.command_actions.iface_actions" @@ -118,11 +119,42 @@ def test_clean_interface_switchport_config_removes_other_vlans(self, cte_mock): # Verify that execute_command was called for all switchport lines including vlan 100 calls = executor_mock.execute_command.call_args_list - # Should be called three times: "switchport", "switchport trunk allowed vlan", "switchport mode trunk" - self.assertEqual(len(calls), 3) + # Should be called twice: "switchport trunk allowed vlan", "switchport mode trunk" + # Note: "switchport" alone (without trailing space) is not matched by the pattern + self.assertEqual(len(calls), 2) # Verify the commands that were issued called_commands = [call[1]["command"] for call in calls] - self.assertIn("switchport", called_commands) + self.assertIn("switchport trunk allowed vlan", called_commands) + self.assertIn("switchport mode trunk", called_commands) + + @patch( + "cloudshell.networking.cisco.command_actions.iface_actions" + ".CommandTemplateExecutor" + ) + def test_clean_interface_switchport_config_removes_vlan_1_in_range(self, cte_mock): + """Test that VLAN 1 in a range or list is still removed (not default state).""" + current_config = """Building configuration... + +Current configuration : 144 bytes +! +interface GigabitEthernet110/1/0/6 + description Test Port + switchport trunk allowed vlan 1,2,3 + switchport mode trunk +end +""" + executor_mock = MagicMock() + cte_mock.return_value = executor_mock + + self._handler.clean_interface_switchport_config(current_config) + + # Verify that execute_command was called for VLAN range including vlan 1 + calls = executor_mock.execute_command.call_args_list + # Should be called twice: "switchport trunk allowed vlan", "switchport mode trunk" + self.assertEqual(len(calls), 2) + + # Verify the commands that were issued - vlan 1,2,3 should be removed + called_commands = [call[1]["command"] for call in calls] self.assertIn("switchport trunk allowed vlan", called_commands) self.assertIn("switchport mode trunk", called_commands)