Change Details

tldr: we need a design decision on a certain style of XML include files, which causes a failure in the python xml dictionary cache: the decision is whether to accommodate this style, or avoid it; details below. The function 'multi_to_list' was introduced in order to return a consistent form for values of multi nodes, specifically, return a list even in the case of a single value (T2636, T2849). It was noticed by @c-po in the context of T3192 that this failed on the multi node ['system', 'login', 'radius', 'source-address']. The proximate cause of this is that the test 'is_multi' returns None, instead of a boolean. The test 'is_multi' fails because this path is not included in the python xml dictionary cache. The path is not included due to the fact that, after preprocessing of XML include files, there are multiple nodes of the same name and path (in this case, ['system', 'login', 'radius']) in the XML, and the creation of the dictionary cache contains only the final element's content. There exist ~10 instances of such redundancy (all but 2 involving 'radius'), due to the XML include files repeating ancestor nodes in their definition, which then overlap with those of the including file. Note that this is not a problem for build-command-templates, as each subelement produces a separate node.def file; it may be an issue when we move beyond the legacy system. So the question is whether to add a preprocessor to merge these multiple node elements, our avoid the practice that produces them. A script to list the occurrence of redundancies is scripts/check-xml-element-redundancy, here: https://github.com/vyos/vyos-1x/compare/current...jestabro:check-xml-element-redundancy To run, one should 'make interface_definitions' and then 'scripts/check-xml-element-redundancy'.

tldr: we need a design decision on a certain style of XML include files, which causes a failure in the python xml dictionary cache: the decision is whether to accommodate this style, or avoid it; details below. The function 'multi_to_list' was introduced in order to return a consistent form for values of multi nodes, specifically, return a list even in the case of a single value (T2636, T2849). It was noticed by @c-po in the context of T3192 that this failed on the multi node ['system', 'login', 'radius', 'source-address']. The proximate cause of this is that the test 'is_multi' returns None, instead of a boolean. The test 'is_multi' fails because this path is not included in the python xml dictionary cache. The path is not included due to the fact that, after preprocessing of XML include files, there are multiple nodes of the same name and path (in this case, ['system', 'login', 'radius']) in the XML, and the creation of the dictionary cache contains only the final element's content. There exist ~10 instances of such redundancy (all but 2 involving 'radius'), due to the XML include files repeating ancestor nodes in their definition, which then overlap with those of the including file. Note that this is not a problem for build-command-templates, as each subelement produces a separate node.def file; it may be an issue when we move beyond the legacy system. So the question is whether to add a preprocessor to merge these multiple node elements, our avoid the practice that produces them. A script to list the occurrence of redundancies is scripts/check-xml-element-redundancy, here: https://github.com/vyos/vyos-1x/compare/current...jestabro:check-xml-element-redundancy To run, one should 'make interface_definitions' and then 'scripts/check-xml-element-redundancy'. Output: Differing content for elements radius at path ['vpn', 'sstp', 'authentication'] in file build/interface-definitions/vpn_sstp.xml Differing content for elements radius at path ['vpn', 'pptp', 'remote-access', 'authentication'] in file build/interface-definitions/vpn_pptp.xml Differing content for elements radius at path ['system', 'login'] in file build/interface-definitions/system-login.xml Differing content for elements radius at path ['vpn', 'openconnect', 'authentication'] in file build/interface-definitions/vpn_openconnect.xml Differing content for elements radius at path ['service', 'ipoe-server', 'authentication'] in file build/interface-definitions/service_ipoe-server.xml Differing content for elements radius at path ['service', 'pppoe-server', 'authentication'] in file build/interface-definitions/service_pppoe-server.xml Differing content for elements radius at path ['vpn', 'l2tp', 'remote-access', 'authentication'] in file build/interface-definitions/vpn_l2tp.xml Differing content for elements radius at path ['interfaces', 'wireless', 'security', 'wpa'] in file build/interface-definitions/interfaces-wireless.xml Differing content for elements rule at path ['nat', 'destination'] in file build/interface-definitions/nat.xml Differing content for elements rule at path ['nat', 'source'] in file build/interface-definitions/nat.xml

tldr: we need a design decision on a certain style of XML include files, which causes a failure in the python xml dictionary cache: the decision is whether to accommodate this style, or avoid it; details below. The function 'multi_to_list' was introduced in order to return a consistent form for values of multi nodes, specifically, return a list even in the case of a single value (T2636, T2849). It was noticed by @c-po in the context of T3192 that this failed on the multi node ['system', 'login', 'radius', 'source-address']. The proximate cause of this is that the test 'is_multi' returns None, instead of a boolean. The test 'is_multi' fails because this path is not included in the python xml dictionary cache. The path is not included due to the fact that, after preprocessing of XML include files, there are multiple nodes of the same name and path (in this case, ['system', 'login', 'radius']) in the XML, and the creation of the dictionary cache contains only the final element's content. There exist ~10 instances of such redundancy (all but 2 involving 'radius'), due to the XML include files repeating ancestor nodes in their definition, which then overlap with those of the including file. Note that this is not a problem for build-command-templates, as each subelement produces a separate node.def file; it may be an issue when we move beyond the legacy system. So the question is whether to add a preprocessor to merge these multiple node elements, our avoid the practice that produces them. A script to list the occurrence of redundancies is scripts/check-xml-element-redundancy, here: https://github.com/vyos/vyos-1x/compare/current...jestabro:check-xml-element-redundancy To run, one should 'make interface_definitions' and then 'scripts/check-xml-element-redundancy'. Output: Differing content for elements radius at path ['vpn', 'sstp', 'authentication'] in file build/interface-definitions/vpn_sstp.xml Differing content for elements radius at path ['vpn', 'pptp', 'remote-access', 'authentication'] in file build/interface-definitions/vpn_pptp.xml Differing content for elements radius at path ['system', 'login'] in file build/interface-definitions/system-login.xml Differing content for elements radius at path ['vpn', 'openconnect', 'authentication'] in file build/interface-definitions/vpn_openconnect.xml Differing content for elements radius at path ['service', 'ipoe-server', 'authentication'] in file build/interface-definitions/service_ipoe-server.xml Differing content for elements radius at path ['service', 'pppoe-server', 'authentication'] in file build/interface-definitions/service_pppoe-server.xml Differing content for elements radius at path ['vpn', 'l2tp', 'remote-access', 'authentication'] in file build/interface-definitions/vpn_l2tp.xml Differing content for elements radius at path ['interfaces', 'wireless', 'security', 'wpa'] in file build/interface-definitions/interfaces-wireless.xml Differing content for elements rule at path ['nat', 'destination'] in file build/interface-definitions/nat.xml Differing content for elements rule at path ['nat', 'source'] in file build/interface-definitions/nat.xml