From 0a3dafc03baeb1ad771a45a511eee80d91308e89 Mon Sep 17 00:00:00 2001
From: bingwang-ms <66248323+bingwang-ms@users.noreply.github.com>
Date: Fri, 13 Jan 2023 15:54:25 +0800
Subject: [PATCH] [minigraph]: Support port name in ACL table AttachTo
 attribute (#13105)

Why I did it
This PR is to update minigraph.py to support both port alias and port name as input of AttachTo attribute of ACL table.
Before this change, only port alias is supported.

How I did it
Add a global variable to store port names
Search both port names and port alias wheh parsing the value of AttachTo.

How to verify it
Verified by a new unit test case test_minigraph_acl_attach_to_ports
Verified by copying the new minigraph.py to a testbed and run conflg load_minigraph.
---
 src/sonic-config-engine/minigraph.py                | 13 ++++++++++---
 .../tests/simple-sample-graph-case.xml              |  2 +-
 .../tests/test_minigraph_case.py                    |  8 ++++++++
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py
index ac459ee0f4..72fb8a8bca 100644
--- a/src/sonic-config-engine/minigraph.py
+++ b/src/sonic-config-engine/minigraph.py
@@ -653,10 +653,14 @@ def parse_dpg(dpg, hname):
                         acl_intfs.extend(vlan_member_list[member])
                     else:
                         acl_intfs.append(member)
-                elif member in port_alias_map:
-                    acl_intfs.append(port_alias_map[member])
+                elif (member in port_alias_map) or (member in port_names_map):
+                    if member in port_alias_map:
+                        acl_intf = port_alias_map[member]
+                    else:
+                        acl_intf = member
+                    acl_intfs.append(acl_intf)
                     # Give a warning if trying to attach ACL to a LAG member interface, correct way is to attach ACL to the LAG interface
-                    if port_alias_map[member] in intfs_inpc:
+                    if acl_intf in intfs_inpc:
                         print("Warning: ACL " + aclname + " is attached to a LAG member interface " + port_alias_map[member] + ", instead of LAG interface", file=sys.stderr)
                 elif member.lower().startswith('erspan') or member.lower().startswith('egress_erspan') or member.lower().startswith('erspan_dscp'):
                     if 'dscp' in member.lower():
@@ -1375,6 +1379,8 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
             docker_routing_config_mode = child.text
 
     (ports, alias_map, alias_asic_map) = get_port_config(hwsku=hwsku, platform=platform, port_config_file=port_config_file, asic_name=asic_name, hwsku_config_file=hwsku_config_file)
+    
+    port_names_map.update(ports)
     port_alias_map.update(alias_map)
     port_alias_asic_map.update(alias_asic_map)
 
@@ -2041,6 +2047,7 @@ def parse_asic_meta_get_devices(root):
 
     return local_devices
 
+port_names_map = {}
 port_alias_map = {}
 port_alias_asic_map = {}
 
diff --git a/src/sonic-config-engine/tests/simple-sample-graph-case.xml b/src/sonic-config-engine/tests/simple-sample-graph-case.xml
index 4165647a9a..6692e30665 100644
--- a/src/sonic-config-engine/tests/simple-sample-graph-case.xml
+++ b/src/sonic-config-engine/tests/simple-sample-graph-case.xml
@@ -180,7 +180,7 @@
       <DataAcls/>
       <AclInterfaces>
         <AclInterface>
-          <AttachTo>PortChannel01</AttachTo>
+          <AttachTo>PortChannel01;fortyGigE0/8;Ethernet12</AttachTo>
           <InAcl>DataAcl</InAcl>
           <Type>DataPlane</Type>
         </AclInterface>
diff --git a/src/sonic-config-engine/tests/test_minigraph_case.py b/src/sonic-config-engine/tests/test_minigraph_case.py
index 824220a29e..22966020fa 100644
--- a/src/sonic-config-engine/tests/test_minigraph_case.py
+++ b/src/sonic-config-engine/tests/test_minigraph_case.py
@@ -467,6 +467,14 @@ class TestCfgGenCaseInsensitive(TestCase):
             expected_ports.sort()
         )
 
+    def test_minigraph_acl_attach_to_ports(self):
+        """
+        The test case is to verify ACL table can be bound to both port names and alias
+        """
+        result = minigraph.parse_xml(self.sample_graph, port_config_file=self.port_config)
+        expected_dataacl_ports = ['PortChannel01','fortyGigE0/8','Ethernet12']
+        self.assertEqual(result['ACL_TABLE']['DATAACL']['ports'].sort(), expected_dataacl_ports.sort())
+
     def test_parse_device_desc_xml_mgmt_interface(self):
         # Regular device_desc.xml with both IPv4 and IPv6 mgmt address
         result = minigraph.parse_device_desc_xml(self.sample_simple_device_desc)