Page MenuHomeVyOS Platform

multi_to_list fails in certain cases, with root cause an element redundancy in XML interface-definitions
Closed, ResolvedPublicBUG

Description

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

Details

Version
vyos-1.4
Is it a breaking change?
Unspecified (possibly destroys the router)
Issue type
Internal change (not visible to end users)

Event Timeline

jestabro changed the task status from Open to Confirmed.
jestabro triaged this task as High priority.
jestabro created this object in space S1 VyOS Public.
jestabro renamed this task from multi_to_list fails in certain case, with root cause an element redundancy in XML interface-definitions to multi_to_list fails in certain cases, with root cause an element redundancy in XML interface-definitions.Jan 20 2021, 11:12 PM
jestabro moved this task from Open to Finished on the VyOS 1.4 Sagitta board.
jestabro changed the task status from Unknown Status to Resolved.Aug 19 2021, 3:27 PM
SrividyaA set Issue type to Internal change (not visible to end users).Aug 30 2021, 5:30 PM