Prevent user from adding more then a single untagged VLAN to an interface (sonic-net#1382)

deran1980 · web-flow · commit 7a8024a585a1 · 2021-01-27T16:54:23.000+02:00
Fix for sonic-net#6421 **- What I did** User was able to add an interface to multiple VLANs as untagged. Added a validation to a single untagged member. **- How I did it** Added a validation when adding a port as untagged to check in DB if is already member as untagged in a different vlan. **- How to verify it** Add vlan, add interface as untagged member in this vlan, add another vlan, try to add the same interface as untagged member in the new vlan. **- Previous command output (if the output of a command-line utility has changed)** Command was successful. **- New command output (if the output of a command-line utility has changed)** Fail in VLAN membership. Error: Ethernet8 is already untagged member!
diff --git a/config/vlan.py b/config/vlan.py
@@ -136,6 +136,9 @@ def add_vlan_member(db, vid, port, untagged):
     if (is_port and clicommon.is_port_router_interface(db.cfgdb, port)) or \
        (not is_port and clicommon.is_pc_router_interface(db.cfgdb, port)):
         ctx.fail("{} is a router interface!".format(port))
+        
+    if (clicommon.interface_is_untagged_member(db.cfgdb, port) and untagged):
+        ctx.fail("{} is already untagged member!".format(port))
 
     db.cfgdb.set_entry('VLAN_MEMBER', (vlan, port), {'tagging_mode': "untagged" if untagged else "tagged" })
 
diff --git a/tests/vlan_test.py b/tests/vlan_test.py
@@ -597,6 +597,35 @@ def test_config_vlan_proxy_arp_disable(self):
 
         assert result.exit_code == 0
         assert db.cfgdb.get_entry("VLAN_INTERFACE", "Vlan2000") == {"proxy_arp": "disabled"}
+        
+    def test_config_2_untagged_vlan_on_same_interface(self):
+        runner = CliRunner()
+        db = Db()
+        
+        # add Ethernet4 to vlan 2000 as untagged - should fail as ethrnet4 is already untagged member in 1000
+        result = runner.invoke(config.config.commands["vlan"].commands["member"].commands["add"],
+                ["2000", "Ethernet4", "--untagged"], obj=db)
+        print(result.exit_code)
+        assert result.exit_code != 0
+        
+        # add Ethernet4 to vlan 2000 as tagged - should succeed
+        result = runner.invoke(config.config.commands["vlan"].commands["member"].commands["add"],
+                ["2000", "Ethernet4" ], obj=db)
+        print(result.exit_code)
+        assert result.exit_code == 0
+        
+    def test_config_set_router_port_on_member_interface(self):        
+        db = Db()
+        runner = CliRunner()
+        obj = {'config_db':db.cfgdb}
+        
+        # intf enable
+        result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"],
+                               ["Ethernet4", "10.10.10.1/24"], obj=obj)        
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert 'Interface Ethernet4 is a member of vlan' in result.output
+        
 
     @classmethod
     def teardown_class(cls):
diff --git a/utilities_common/cli.py b/utilities_common/cli.py
@@ -544,3 +544,13 @@ def json_dump(data):
     return json.dumps(
         data, sort_keys=True, indent=2, ensure_ascii=False
     )
+    
+def interface_is_untagged_member(db, interface_name):
+    """ Check if interface is already untagged member"""    
+    vlan_member_table = db.get_table('VLAN_MEMBER')
+    
+    for key,val in vlan_member_table.items():
+        if(key[1] == interface_name):
+            if (val['tagging_mode'] == 'untagged'):
+                return True
+    return False
