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

Difficulty level
Unknown (require assessment)
Version
vyos-1.4
Why the issue appeared?
Will be filled on close
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.Jan 20 2021, 10:40 PM
jestabro triaged this task as High priority.
jestabro created this task.
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 changed the task status from Confirmed to Backport pending.Aug 13 2021, 4:32 PM
jestabro moved this task from Need Triage to Finished on the VyOS 1.4 Sagitta board.
SrividyaA set Issue type to Internal change (not visible to end users).Aug 30 2021, 5:30 PM