Page MenuHomeVyOS Platform

Make session_config not depend on the current edit level
Closed, ResolvedPublic

Description

It seems that now after T1758 / d9ee0b95d1020b6d5412dd011ebb1ef7f6ef3fc7 the session config is not taken from the root level but the current edit level which means that trying to commit will fail if the user is not in the top level.

The config is determined using self._run([self._cli_shell_api, '--show-working-only', '--show-show-defaults', 'showConfig']) which is edit-level dependent:

[edit interfaces ethernet eth0]
vyos@vyos# /bin/cli-shell-api --show-working-only --show-show-defaults showConfig
address 10.0.0.1/24
hw-id 52:54:00:3f:49:25
[edit interfaces ethernet eth0]
vyos@vyos# top
[edit]
vyos@vyos# /bin/cli-shell-api --show-working-only --show-show-defaults showConfig | head
interfaces {
    ethernet eth0 {
        address 10.0.0.1/24
        hw-id 52:54:00:3f:49:25

Originally found out / debugged in T1844.

Details

Difficulty level
Unknown (require assessment)
Version
-
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Unspecified (possibly destroys the router)

Event Timeline

varesa created this task.Dec 4 2019, 10:06 AM
varesa created this object in space S1 VyOS Public.

It would be pretty simple to pass VYATTA_{TEMPLATE,EDIT}_LEVEL=/ as environment variables in Config._run():

def _run(self, cmd):    
    if self.__session_env:    
        p = subprocess.Popen(cmd, stdout=subprocess.PIPE, env=self.__session_env)    
    else:    
        p = subprocess.Popen(cmd, stdout=subprocess.PIPE)

However I'm not sure if something else might actually depend on it being edit level aware.

Mainly these:

out = self._run(self._make_command('showConfig', path))
self._run(self._make_command('isMulti', path))
self._run(self._make_command('isTag', path))
self._run(self._make_command('isLeaf', path))

Is path supposed to be absolute or relative to the current edit level?

varesa updated the task description. (Show Details)Dec 4 2019, 10:17 AM

Seems like that paths are absolute regardless of the environment variables:

[edit interfaces ethernet eth0]
vyos@vyos# /bin/cli-shell-api isLeaf interfaces ethernet eth0 hw-id; echo $?
0

Seems based on the uses of the Config-class, for example this in vyos-http-api-server:

session = ConfigSession(os.getpid())    
env = session.get_session_env()    
config = vyos.config.Config(session_env=env)

that the intention was to have a clean environment unless called with session_env=something.

However by default the Popen call uses the surrounding environment instead of a clean one.

So the appropriate fix to me seems to be:

diff --git a/python/vyos/config.py b/python/vyos/config.py
index 13b2c10..c9c5ea0 100644
--- a/python/vyos/config.py
+++ b/python/vyos/config.py
@@ -137,7 +137,7 @@ class Config(object):
         if self.__session_env:
             p = subprocess.Popen(cmd, stdout=subprocess.PIPE, env=self.__session_env)
         else:
-            p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+            p = subprocess.Popen(cmd, stdout=subprocess.PIPE, env={})
         out = p.stdout.read()
         p.wait()
         if p.returncode != 0:

Okay, that doesn't work. It likely requires some variables like these:

VYATTA_ACTIVE_CONFIGURATION_DIR=/opt/vyatta/config/active
VYATTA_CONFIG_TMP=/opt/vyatta/config/tmp/tmp_1929
VYATTA_CHANGES_ONLY_DIR=/opt/vyatta/config/tmp/changes_only_1929
VYATTA_TEMP_CONFIG_DIR=/opt/vyatta/config/tmp/new_config_1929

Simple fix could be to just override VYATTA_TEMPLATE_LEVEL and VYATTA_EDIT_LEVEL.

A nicer one would be to whitelist the required variables and otherwise use a clean environment

This works better:

diff --git a/python/vyos/config.py b/python/vyos/config.py
index 13b2c10..82483cb 100644
--- a/python/vyos/config.py
+++ b/python/vyos/config.py
@@ -137,7 +137,10 @@ class Config(object):
         if self.__session_env:
             p = subprocess.Popen(cmd, stdout=subprocess.PIPE, env=self.__session_env)
         else:
-            p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+            env = os.environ
+            env['VYATTA_TEMPLATE_LEVEL'] = '/'
+            env['VYATTA_EDIT_LEVEL'] = '/'
+            p = subprocess.Popen(cmd, stdout=subprocess.PIPE, env=env)
         out = p.stdout.read()
         p.wait()
         if p.returncode != 0:
pasik added a subscriber: pasik.Dec 4 2019, 3:23 PM
jestabro claimed this task.Dec 9 2019, 5:29 PM
jestabro changed the task status from Open to In progress.Dec 9 2019, 6:19 PM

A parsimonious fix to this issue has been committed; I will move the status to "In progress" until the related issues mentioned here are sorted through.

varesa removed a subscriber: varesa.Dec 10 2019, 9:58 PM
jestabro reopened this task as Open.Dec 11 2019, 3:47 PM

This requirement needs to be applied to 'running_config' as well, in case of (1) editing at the level of an unset node (parse error); (2) reading effective_value(s) in configuration mode (lacking full paths).

jestabro closed this task as Resolved.Dec 11 2019, 4:39 PM