config: T5228: simplify get_config_dict and add argument with_defaults

[jestabro]

Change Summary

After T5218, we can simplify the key_mangling function used by get_config_dict, and add an argument

get_config_dict(..., with_defaults=True)

to correctly merge default values.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Code style update (formatting, renaming)
• Refactoring (no functional changes)
• Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• Other (please describe):

Related Task(s)

• https://vyos.dev/T5228

Component(s) name

Proposed changes

How to test

Checklist:

• I have read the CONTRIBUTING document
• I have linked this PR to one or more Phabricator Task(s)
• I have run the components SMOKETESTS if applicable
• My commit headlines contain a valid Task id
• My change requires a change to the documentation
• I have updated the documentation accordingly

[dmbaturin]

I like the idea to simplify getting a config with defaults.

However, I feel that such functionality should come with clear guidelines on how to use it properly. Possibly even with safeguards that ensure that it doesn't get in the way.

Here's the infamous...

Problem of defaults

When there is no such thing as a first-class default value in interface definitions, there are only two cases:

1. Config node exists and has a value (unless it's valueless).
2. Config node doesn't exist.

That means that "node doesn't exist in the config dict = node doesn't exist in the config".

First-class defaults come comes at a pretty dire price: existence of a node in the config dict no longer means that it exists in the config itself! It may now mean two things: either that node exists in the config or it simply has a default value.

For values with defaults, it becomes impossible to check if they actually exist or not.
In the old config system where $config->returnValue() would return the default value for nodes that had it, whether they were actually present in the config or not, that created an impassable obstacle to implementing live rollbacks: it made node deletion impossible to detect and thus an inverse of a diff was also impossible to reconstruct.

Now we are getting a light version of that problem. "Light" in that we do have a way to check if a node exists in the config or not.

Config scripts must never use config dicts with defaults for verification because they create an impression that subtrees exist but don't have required options — just because an option in a subtree has a default value. Templates can also get troubles with it.

Possible solution

I'm now thinking that there's one invariant that can save us from the problem of defaults: defaults must never be injected in a subtree whose parent doesn't exist in the config.

In different words: default value for a node must be injected if and only if its parent exists in the config tree.

I don't have a proof that it's sufficient yet but my feeling is that it should be.

[jestabro]

The PR version satisfies the invariant 'defaults of paths (without intervening unspecified tag nodes) are added only if an ancestor exists in the config'. We can choose to only add leaf nodes whose parent is set in the config, which would rule out, say, the recent issue with VRRP 'health-check' (T5215; commit 00b48df); example below. This would correspond with the hypothesis that an intervening non-leaf node would indicate an intentional effect; whether that is always the case is an open question.

I will add an arg 'recursive=False' to the *_defaults functions of PR 1997 to limit to children that are leaf nodes.

Current PR version is recursive from ancestor along paths not containing an unspecified tag node:

>>> from vyos.xml_ref import get_defaults
>>> from pprint import pprint
>>> pprint(get_defaults(['interfaces', 'ethernet', 'eth0']))
{'eth0': {'dhcp-options': {'default-route-distance': '210'},
          'duplex': 'auto',
          'ip': {'arp-cache-timeout': '30'},
          'mtu': '1500',
          'speed': 'auto'}}
>>> pprint(get_defaults(['high-availability', 'vrrp', 'group', 'test']))
{'test': {'advertise-interval': '1',
          'garp': {'interval': '0',
                   'master-delay': '5',
                   'master-refresh': '5',
                   'master-refresh-repeat': '1',
                   'master-repeat': '5'},
          'health-check': {'failure-count': '3', 'interval': '60'},
          'preempt-delay': '0',
          'priority': '100'}}

[jestabro]

The base PR for these simplifications
#1997
has been updated with the solution discussed: default values are collected only for those leaf-nodes whose parent nodes are set in the config; 'non-local' defaults from leaf-nodes who have an ancestor in the config are only available with the option 'recursive=True'.

The non-local behavior is accessible with the explicit functions get_defaults, get_config_defaults (examples below). The function merge_defaults, as used optionally within get_config_dict, returns 'local' results, as it is expected that for more complex requirements the explicit forms will be used and the results adjusted to needs.

>>> from vyos.config import Config
>>> from vyos.xml_ref import get_config_defaults, merge_defaults
>>> from pprint import pprint
>>> conf = Config()
>>> d = conf.get_config_dict(['high-availability', 'vrrp'])
>>> d
{'vrrp': {'group': {'int': {'priority': '200'}}}}
>>> pprint(get_config_defaults(['high-availability', 'vrrp'], d))
{'vrrp': {'group': {'int': {'advertise-interval': '1',
                            'preempt-delay': '0',
                            'priority': '100'}}}}
>>> pprint(merge_defaults(['high-availability', 'vrrp'], d))
{'vrrp': {'group': {'int': {'advertise-interval': '1',
                            'preempt-delay': '0',
                            'priority': '200'}}}}
>>> pprint(get_config_defaults(['high-availability', 'vrrp'], d, recursive=True))
{'vrrp': {'group': {'int': {'advertise-interval': '1',
                            'garp': {'interval': '0',
                                     'master-delay': '5',
                                     'master-refresh': '5',
                                     'master-refresh-repeat': '1',
                                     'master-repeat': '5'},
                            'health-check': {'failure-count': '3',
                                             'interval': '60'},
                            'preempt-delay': '0',
                            'priority': '100'}}}}
>>> d = conf.get_config_dict(['high-availability', 'vrrp'], with_defaults=True)
>>> pprint(d)
{'vrrp': {'group': {'int': {'advertise-interval': '1',
                            'preempt-delay': '0',
                            'priority': '200'}}}}

get_config_dict and get_config_defaults have consistent use of the optional argument get_first_key.

[jestabro]

Temporarily set to draft to fix bug in recursive option.

[jestabro]

Fixed regression caused by introduction of option recursive, and simplified logic. In addition, get_config_defaults now takes optional args consistent with get_config_dict and a smoketest is added to confirm; the smoketest can likely be retired once the application of the revised operations on defaults is complete.

[jestabro]

Note that this is a dependency for #2052, which has been merged; merging now.