Page MenuHomeVyOS Platform

Add support for default values to the interface-definition format
Closed, ResolvedPublic

Description

Originally, there was a default: option in node.def files for storing default values. Its aim was to make it simple to set default values for leaf nodes, and to abstract them away.
It abstracted them away deep in libvyattacfg—by silently returning the default value from calls to return_value() etc. if a node value is not set in the config.

That approach was proven problematic. Logically, a node exists in the config or it does not. If it does not exist, exists() should return false and return_value() should fail. If it does exist, then exists() returns true and return_value() returns its value. The old approach to defaults created a third virtual "default" state when a node doesn't exist, but has a value.

Apart from breaking the logic, it also made it difficult to produce a reverse diff for reverting changes, which made rollback without reboot impossible.

Does it mean defaults encoded in command definitions is a bad idea? Not quite. There's a better approach where config scripts can query the default from those definitions instead of hardcoding it in the script itself, but existence of a default has no effect on the way exists() and return_value() behave.

For that we can add a new <defaultValue> tag under <leafNode> and then generate default dicts from those definitions.
However, the convertor must not produce default: option in the node.def's it makes!

Details

Difficulty level
Unknown (require assessment)
Version
-
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Perfectly compatible
Issue type
Internal change (not visible to end users)

Event Timeline

Unfortunately this results in an error:

diff --git i/interface-definitions/ssh.xml.in w/interface-definitions/ssh.xml.in
index de926a8..d122e82 100644
--- i/interface-definitions/ssh.xml.in
+++ w/interface-definitions/ssh.xml.in
@@ -161,6 +161,7 @@
                 <format>1-65535</format>
                 <description>Numeric IP port</description>
               </valueHelp>
+              <defaultValue>22</defaultValue>
               <multi/>
               <constraint>
                 <validator name="numeric" argument="--range 1-65535"/>
<string>:0:0:ERROR:RELAXNGV:RELAXNG_ERR_INTEREXTRA: Extra element children in interleave
build/interface-definitions/ssh.xml:5:0:ERROR:RELAXNGV:RELAXNG_ERR_CONTENTVALID: Element node failed to validate content
Interface definition file build/interface-definitions/ssh.xml does not match the schema!
make[2]: *** [Makefile:31: interface_definitions] Error 1

My fault. defaultValue must pe placed outside of properties

diff --git i/interface-definitions/ssh.xml.in w/interface-definitions/ssh.xml.in
index de926a8..c3e5e83 100644
--- i/interface-definitions/ssh.xml.in
+++ w/interface-definitions/ssh.xml.in
@@ -166,6 +166,7 @@
                 <validator name="numeric" argument="--range 1-65535"/>
               </constraint>
             </properties>
+            <defaultValue>22</defaultValue>
           </leafNode>
           <leafNode name="client-keepalive-interval">
             <properties>
c-po reopened this task as In progress.Jun 18 2020, 3:51 PM
Unknown Object (User) added a subscriber: Unknown Object (User).Jun 18 2020, 4:02 PM

A first implementation is already live with the console-server https://github.com/vyos/vyos-1x/blob/current/src/conf_mode/service_console-server.py

Next example will be SSH!

c-po renamed this task from Add support for default values to the interface definition format to Add support for default values to the interface-definition format.Jun 26 2020, 1:29 PM
c-po closed this task as Resolved.
erkin set Issue type to Internal change (not visible to end users).Aug 30 2021, 5:32 AM
erkin removed a subscriber: Active contributors.