Page MenuHomeVyOS Platform

vrrp transition-script validator makes warning fatal and also causes a python NameError exception
Closed, ResolvedPublicBUG

Description

When setting a transition script to something outside of /config the commit will fail with:

[ high-availability vrrp group eth0-5 transition-script backup /etc/init.d/radvd stop ]
Traceback (most recent call last):
  File "/usr/libexec/vyos/validators/script", line 40, in <module>
    f'Warning: file {path} is outside the / config directory\n'
NameError: name 'path' is not defined

[ high-availability vrrp group eth0-5 transition-script backup /etc/init.d/radvd stop ]
Invalid value

In /usr/libexec/vyos/validators/script it has

f'Warning: file {path} is outside the / config directory\n'

changing {path} to {script} here in https://github.com/vyos/vyos-1x/blob/current/src/validators/script#L40 fixes the traceback error but the warning is still fatal. If I change it to print() instead of sys.exit() the warning doesn't even get printed.

Details

Difficulty level
Unknown (require assessment)
Version
1.3/1.4
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Behavior change
Issue type
Bug (incorrect behavior)

Event Timeline

Try to restart vyos-configd after changing script file

sudo systemctl restart vyos-configd

Or just stop it.

This doesn't seem to help, whatever is calling the validator script seems to hide the output unless the exit status is non-zero.

From what I understand this looks to be due to https://github.com/vyos/vyos-utils/blob/master/src/validate_value.ml catching both stdout and stderr output from the validators and only printing the captured output if the validator exit status is 0 so there isn't a way to print warnings unless it always prints the output or handling for a special 'warning' exit code was added.

jestabro added a subscriber: jestabro.

I will take a look; thanks for the report !

This brings up an interesting issue: validate_value.ml could easily be modified to print warnings, while maintaining T2759 (namely, only print fatal errors if _all_ validators fail for a given setting), however, is this reasonable behaviour ? One would think that a 'validator' is either pass or fail, and if it is just giving a warning, it is no longer a validator.

If we choose not to have strict enforcement of the path here, then an alternative is to move the 'script' check to the verify step in the conf_mode script, providing either a warning or failure depending on what is appropriate for the case. Currently, 'script' is only called for validation by vrrp.xml.in and service_pppoe-server.xml.in.

The plan is to weaken this to a warning in 'verify'; as summarized above, validators are pass/fail and warnings are not an appropriate response for a validator

The errors here were fixed in:
https://vyos.dev/T4052
https://vyos.dev/T4053
in equuleus and subsequent.