From d32c04348657d18dbac4a52a08bdb630ef8a14ca Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Thu, 24 May 2018 11:05:38 -0700 Subject: [PATCH] [sonic-cfggen]: Protect config_db.json from minigraph misconfig (#1727) * Add noise config for PortChannel & EthernetInterface in simple-sample-graph.xml * Add noise config for PORTCHANNEL_INTERFACE in simple-sample-graph.xml Signed-off-by: Wenda * Add noice config for DEVICE_NEIGHBOR in t0-sample-graph.xml Add unit test against introducing ports not existing in port_config.ini into DEVICE_NEIGHBOR Signed-off-by: Wenda * DeviceInterfaceLink in minigraph.xml can contain port not existing in port_config.ini but contraining non-zero Bandwidth attribute Add noice config in simple-sample-graph.xml to capture the case that such a port is leaked into config_db.json Signed-off-by: Wenda * Protect PORTCHANNEL from ports not existing in port_config.ini Signed-off-by: Wenda * Protect PORTCHANNEL_INTERFACE from portchannels containing ports not existing in port_config.ini Signed-off-by: Wenda * Protect DEVICE_NEIGHBOR from ports not existing in port_config.ini Signed-off-by: Wenda * Add noise config Ethernet1 in DeviceInterfaceLinks in simple-sample-graph.xml as it is in PortChannel1001 Signed-off-by: Wenda * Add noise config Ethernet1 in DeviceInterfaceLinks in simple-sample-graph.xml as it is in PortChannel1001 Signed-off-by: Wenda * Protect PORTCHANNEL from ports not existing in port_config.ini Signed-off-by: Wenda * Protect PORTCHANNEL_INTERFACE from portchannels containing ports not existing in port_config.ini Signed-off-by: Wenda * Protect DEVICE_NEIGHBOR from ports not existing in port_config.ini Signed-off-by: Wenda * Correct space in minigraph.py Signed-off-by: Wenda * Does not allow non-port_config.ini port to get into the port list Signed-off-by: Wenda * Check PORTCHANNEL against PORT list only if port_config_file exists Signed-off-by: Wenda * Correct format Signed-off-by: Wenda * print warning when a port coming from DeviceInterfaceLink is not in port_config.ini Signed-off-by: Wenda * Change Ethernet1 and 2 to fortyGigE0/1 and 2,respectively Signed-off-by: Wenda * Change Ethernet1 and 2 to fortyGigE0/1 and 2,respectively Signed-off-by: Wenda * print warning when ignoring ports, portchannels, portchannel interfaces, and device neighbors Update t0-sample-graph.xml with interface name 'fortyGigE0/2' and the ACL_TABLE output Signed-off-by: Wenda --- src/sonic-config-engine/minigraph.py | 37 +++++++++++-- .../tests/simple-sample-graph.xml | 52 ++++++++++++++++++- .../tests/t0-sample-graph.xml | 11 ++++ src/sonic-config-engine/tests/test_cfggen.py | 11 ++++ 4 files changed, 105 insertions(+), 6 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 443893c66c..595915d9be 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -414,7 +414,7 @@ def parse_xml(filename, platform=None, port_config_file=None): (port_speeds_default, port_descriptions) = parse_deviceinfo(child, hwsku) current_device = [devices[key] for key in devices if key.lower() == hostname.lower()][0] - results = {} + results = {} results['DEVICE_METADATA'] = {'localhost': { 'bgp_asn': bgp_asn, 'deployment_id': deployment_id, @@ -447,7 +447,6 @@ def parse_xml(filename, platform=None, port_config_file=None): results['INTERFACE'] = phyport_intfs results['VLAN_INTERFACE'] = vlan_intfs - results['PORTCHANNEL_INTERFACE'] = pc_intfs for port_name in port_speeds_default: # ignore port not in port_config.ini @@ -457,9 +456,11 @@ def parse_xml(filename, platform=None, port_config_file=None): ports.setdefault(port_name, {})['speed'] = port_speeds_default[port_name] for port_name in port_speed_png: - # if port_name is not in port_config.ini, still consider it. - # and later swss will pick up and behave on-demand port break-up. - # if on-deman port break-up is not supported on a specific platform, swss will return error. + # not consider port not in port_config.ini + if port_name not in ports: + print >> sys.stderr, "Warning: ignore interface '%s' as it is not in the port_config.ini" % port_name + continue + ports.setdefault(port_name, {})['speed'] = port_speed_png[port_name] for port_name, port in ports.items(): @@ -474,10 +475,36 @@ def parse_xml(filename, platform=None, port_config_file=None): ports.setdefault(port_name, {})['description'] = port_descriptions[port_name] results['PORT'] = ports + + if port_config_file: + port_set = set(ports.keys()) + for (pc_name, mbr_map) in pcs.items(): + # remove portchannels that contain ports not existing in port_config.ini + # when port_config.ini exists + if not set(mbr_map['members']).issubset(port_set): + print >> sys.stderr, "Warning: ignore '%s' as part of its member interfaces is not in the port_config.ini" % pc_name + del pcs[pc_name] + results['PORTCHANNEL'] = pcs + + + for pc_intf in pc_intfs.keys(): + # remove portchannels not in PORTCHANNEL dictionary + if pc_intf[0] not in pcs: + print >> sys.stderr, "Warning: ignore '%s' interface '%s' as '%s' is not in the valid PortChannel list" % (pc_intf[0], pc_intf[1], pc_intf[0]) + del pc_intfs[pc_intf] + + results['PORTCHANNEL_INTERFACE'] = pc_intfs + results['VLAN'] = vlans results['VLAN_MEMBER'] = vlan_members + for nghbr in neighbors.keys(): + # remove port not in port_config.ini + if nghbr not in ports: + print >> sys.stderr, "Warning: ignore interface '%s' in DEVICE_NEIGHBOR as it is not in the port_config.ini" % nghbr + del neighbors[nghbr] + results['DEVICE_NEIGHBOR'] = neighbors results['DEVICE_NEIGHBOR_METADATA'] = { key:devices[key] for key in devices if key.lower() != hostname.lower() } results['SYSLOG_SERVER'] = dict((item, {}) for item in syslog_servers) diff --git a/src/sonic-config-engine/tests/simple-sample-graph.xml b/src/sonic-config-engine/tests/simple-sample-graph.xml index 6e351e1cd0..f5054bb084 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph.xml @@ -125,6 +125,11 @@ fortyGigE0/4 + + PortChannel1001 + fortyGigE0/1;fortyGigE0/2 + + @@ -147,6 +152,16 @@ PortChannel01 FC00::71/126 + + + PortChannel1001 + 10.0.0.57/31 + + + + PortChannel1001 + FC00::72/126 + fortyGigE0/0 @@ -193,6 +208,28 @@ fortyGigE0/8 true + + DeviceInterfaceLink + true + 10000 + switch-t0 + fortyGigE0/1 + true + ARISTA05T1 + Ethernet1/32 + true + + + DeviceInterfaceLink + true + 10000 + switch-t0 + fortyGigE0/2 + true + ARISTA06T1 + Ethernet1/33 + true + @@ -240,7 +277,20 @@ true true 1 - fortyGigE0/1 + Ethernet1 + + false + 0 + 0 + 10000 + + + DeviceInterface + + true + true + 1 + Ethernet2 false 0 diff --git a/src/sonic-config-engine/tests/t0-sample-graph.xml b/src/sonic-config-engine/tests/t0-sample-graph.xml index ce0177d4c1..5eace767c4 100644 --- a/src/sonic-config-engine/tests/t0-sample-graph.xml +++ b/src/sonic-config-engine/tests/t0-sample-graph.xml @@ -338,6 +338,17 @@ switch-t0 fortyGigE0/124 + + DeviceInterfaceLink + true + 10000 + switch-t0 + fortyGigE0/2 + true + ARISTA05T1 + Ethernet1/33 + true + diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index 901b882f70..37b66a27b4 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -79,6 +79,8 @@ class TestCfgGen(TestCase): argument = '-m "' + self.sample_graph_t0 + '" -p "' + self.port_config + '" -v ACL_TABLE' output = self.run_script(argument, True) self.assertEqual(output.strip(), "Warning: Ignoring Control Plane ACL NTP_ACL without type\n" + "Warning: ignore interface 'fortyGigE0/2' as it is not in the port_config.ini\n" + "Warning: ignore interface 'fortyGigE0/2' in DEVICE_NEIGHBOR as it is not in the port_config.ini\n" "{'SSH_ACL': {'services': ['SSH'], 'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL'}," " 'SNMP_ACL': {'services': ['SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL'}," " 'DATAACL': {'type': 'L3', 'policy_desc': 'DATAACL', 'ports': ['Ethernet112', 'Ethernet116', 'Ethernet120', 'Ethernet124']}," @@ -130,6 +132,15 @@ class TestCfgGen(TestCase): output = self.run_script(argument) self.assertEqual(output.strip(), "{'name': 'ARISTA04T1', 'port': 'Ethernet1/1'}") + def test_minigraph_extra_neighbors(self): + argument = '-m "' + self.sample_graph_t0 + '" -p "' + self.port_config + '" -v DEVICE_NEIGHBOR' + output = self.run_script(argument) + self.assertEqual(output.strip(), \ + "{'Ethernet116': {'name': 'ARISTA02T1', 'port': 'Ethernet1/1'}, " + "'Ethernet124': {'name': 'ARISTA04T1', 'port': 'Ethernet1/1'}, " + "'Ethernet112': {'name': 'ARISTA01T1', 'port': 'Ethernet1/1'}, " + "'Ethernet120': {'name': 'ARISTA03T1', 'port': 'Ethernet1/1'}}") + def test_minigraph_bgp(self): argument = '-m "' + self.sample_graph_bgp_speaker + '" -p "' + self.port_config + '" -v "BGP_NEIGHBOR[\'10.0.0.59\']"' output = self.run_script(argument)