diff options
| -rw-r--r-- | data/templates/dns-dynamic/ddclient.conf.j2 | 3 | ||||
| -rw-r--r-- | data/templates/dns-dynamic/override.conf.j2 | 3 | ||||
| -rwxr-xr-x | debian/vyos-1x-smoketest.postinst | 10 | ||||
| -rw-r--r-- | interface-definitions/container.xml.in | 2 | ||||
| -rw-r--r-- | interface-definitions/include/haproxy/rule-backend.xml.i | 2 | ||||
| -rw-r--r-- | interface-definitions/include/haproxy/rule-frontend.xml.i | 5 | ||||
| -rwxr-xr-x | smoketest/scripts/cli/test_nat.py | 2 | ||||
| -rwxr-xr-x | smoketest/scripts/cli/test_service_dns_dynamic.py | 19 | ||||
| -rw-r--r-- | src/tests/test_configd_inspect.py | 210 |
9 files changed, 161 insertions, 95 deletions
diff --git a/data/templates/dns-dynamic/ddclient.conf.j2 b/data/templates/dns-dynamic/ddclient.conf.j2 index 23aad4cb8..b209c8c81 100644 --- a/data/templates/dns-dynamic/ddclient.conf.j2 +++ b/data/templates/dns-dynamic/ddclient.conf.j2 @@ -21,10 +21,7 @@ if{{ ipv }}={{ address }}, \ {{ host }} {% endmacro %} ### Autogenerated by service_dns_dynamic.py ### -daemon={{ interval }} -syslog=yes ssl=yes -cache={{ config_file | replace('.conf', '.cache') }} {# ddclient default (web=dyndns) doesn't support ssl and results in process lockup #} web=googledomains {# ddclient default (use=ip) results in confusing warning message in log #} diff --git a/data/templates/dns-dynamic/override.conf.j2 b/data/templates/dns-dynamic/override.conf.j2 index c0edd8f05..aaed4ff35 100644 --- a/data/templates/dns-dynamic/override.conf.j2 +++ b/data/templates/dns-dynamic/override.conf.j2 @@ -1,4 +1,5 @@ {% set vrf_command = 'ip vrf exec ' ~ vrf ~ ' ' if vrf is vyos_defined else '' %} +{% set cache_file = config_file.replace('.conf', '.cache') %} [Unit] ConditionPathExists={{ config_file }} Wants= @@ -7,5 +8,5 @@ After=vyos-router.service [Service] EnvironmentFile= ExecStart= -ExecStart={{ vrf_command }}/usr/bin/ddclient --file {{ config_file }} --foreground +ExecStart={{ vrf_command }}/usr/bin/ddclient --file {{ config_file }} --cache {{ cache_file }} --foreground --daemon {{ interval }} Restart=always diff --git a/debian/vyos-1x-smoketest.postinst b/debian/vyos-1x-smoketest.postinst index 57149af82..08d6d7d4f 100755 --- a/debian/vyos-1x-smoketest.postinst +++ b/debian/vyos-1x-smoketest.postinst @@ -2,14 +2,12 @@ BUSYBOX_TAG="docker.io/library/busybox:stable" BUSYBOX_PATH="/usr/share/vyos/busybox-stable.tar" -if [[ -f $BUSYBOX_PATH ]]; then - rm -f $BUSYBOX_PATH +if [[ ! -f $BUSYBOX_PATH ]]; then + skopeo copy --additional-tag "$BUSYBOX_TAG" "docker://$BUSYBOX_TAG" "docker-archive:/$BUSYBOX_PATH" fi -skopeo copy --additional-tag "$BUSYBOX_TAG" "docker://$BUSYBOX_TAG" "docker-archive:/$BUSYBOX_PATH" TACPLUS_TAG="docker.io/lfkeitel/tacacs_plus:alpine" TACPLUS_PATH="/usr/share/vyos/tacplus-alpine.tar" -if [[ -f $TACPLUS_PATH ]]; then - rm -f $TACPLUS_PATH +if [[ ! -f $TACPLUS_PATH ]]; then + skopeo copy --additional-tag "$TACPLUS_TAG" "docker://$TACPLUS_TAG" "docker-archive:/$TACPLUS_PATH" fi -skopeo copy --additional-tag "$TACPLUS_TAG" "docker://$TACPLUS_TAG" "docker-archive:/$TACPLUS_PATH" diff --git a/interface-definitions/container.xml.in b/interface-definitions/container.xml.in index ad1815604..04318a7c9 100644 --- a/interface-definitions/container.xml.in +++ b/interface-definitions/container.xml.in @@ -131,7 +131,7 @@ <properties> <help>Add custom environment variables</help> <constraint> - <regex>[-_a-zA-Z0-9]+</regex> + #include <include/constraint/alpha-numeric-hyphen-underscore.xml.i> </constraint> <constraintErrorMessage>Environment variable name must be alphanumeric and can contain hyphen and underscores</constraintErrorMessage> </properties> diff --git a/interface-definitions/include/haproxy/rule-backend.xml.i b/interface-definitions/include/haproxy/rule-backend.xml.i index b2be4fde4..1df9d5dcf 100644 --- a/interface-definitions/include/haproxy/rule-backend.xml.i +++ b/interface-definitions/include/haproxy/rule-backend.xml.i @@ -47,7 +47,7 @@ <properties> <help>Server name</help> <constraint> - <regex>[-_a-zA-Z0-9]+</regex> + #include <include/constraint/alpha-numeric-hyphen-underscore.xml.i> </constraint> <constraintErrorMessage>Server name must be alphanumeric and can contain hyphen and underscores</constraintErrorMessage> </properties> diff --git a/interface-definitions/include/haproxy/rule-frontend.xml.i b/interface-definitions/include/haproxy/rule-frontend.xml.i index 001ae2d80..eabdd8632 100644 --- a/interface-definitions/include/haproxy/rule-frontend.xml.i +++ b/interface-definitions/include/haproxy/rule-frontend.xml.i @@ -47,9 +47,12 @@ <properties> <help>Backend name</help> <constraint> - <regex>[-_a-zA-Z0-9]+</regex> + #include <include/constraint/alpha-numeric-hyphen-underscore.xml.i> </constraint> <constraintErrorMessage>Server name must be alphanumeric and can contain hyphen and underscores</constraintErrorMessage> + <completionHelp> + <path>load-balancing haproxy backend</path> + </completionHelp> </properties> </leafNode> </children> diff --git a/smoketest/scripts/cli/test_nat.py b/smoketest/scripts/cli/test_nat.py index 0beafcc6c..b33ef2617 100755 --- a/smoketest/scripts/cli/test_nat.py +++ b/smoketest/scripts/cli/test_nat.py @@ -84,7 +84,7 @@ class TestNAT(VyOSUnitTestSHIM.TestCase): address_group = 'smoketest_addr' address_group_member = '192.0.2.1' interface_group = 'smoketest_ifaces' - interface_group_member = 'bond.99' + interface_group_member = 'eth0' self.cli_set(['firewall', 'group', 'address-group', address_group, 'address', address_group_member]) self.cli_set(['firewall', 'group', 'interface-group', interface_group, 'interface', interface_group_member]) diff --git a/smoketest/scripts/cli/test_service_dns_dynamic.py b/smoketest/scripts/cli/test_service_dns_dynamic.py index 9fbc931de..522102e67 100755 --- a/smoketest/scripts/cli/test_service_dns_dynamic.py +++ b/smoketest/scripts/cli/test_service_dns_dynamic.py @@ -20,8 +20,10 @@ import tempfile from base_vyostest_shim import VyOSUnitTestSHIM from vyos.configsession import ConfigSessionError +from vyos.utils.file import read_file from vyos.utils.process import cmd from vyos.utils.process import process_named_running +from vyos.xml_ref import default_value DDCLIENT_SYSTEMD_UNIT = '/run/systemd/system/ddclient.service.d/override.conf' DDCLIENT_CONF = '/run/ddclient/ddclient.conf' @@ -29,6 +31,7 @@ DDCLIENT_PNAME = 'ddclient' base_path = ['service', 'dns', 'dynamic'] name_path = base_path + ['name'] +default_interval = default_value(base_path + ['interval']) server = 'ddns.vyos.io' hostname = 'test.ddns.vyos.io' zone = 'vyos.io' @@ -95,12 +98,14 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase): # Check the generating config parameters ddclient_conf = cmd(f'sudo cat {DDCLIENT_CONF}') - # default value 300 seconds - self.assertIn(f'daemon=300', ddclient_conf) self.assertIn(f'usev4=ifv4', ddclient_conf) self.assertIn(f'ifv4={interface}', ddclient_conf) self.assertIn(f'password=\'{password}\'', ddclient_conf) + # Check default interval of 300 seconds + systemd_override = read_file(DDCLIENT_SYSTEMD_UNIT) + self.assertIn(f'--daemon {default_interval}', systemd_override) + for opt in details.keys(): if opt == 'username': login = details[opt] @@ -140,7 +145,6 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase): # Check the generating config parameters ddclient_conf = cmd(f'sudo cat {DDCLIENT_CONF}') - self.assertIn(f'daemon={interval}', ddclient_conf) self.assertIn(f'usev6=ifv6', ddclient_conf) self.assertIn(f'ifv6={interface}', ddclient_conf) self.assertIn(f'protocol={proto}', ddclient_conf) @@ -150,6 +154,10 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase): self.assertIn(f'min-interval={wait_time}', ddclient_conf) self.assertIn(f'max-interval={expiry_time_good}', ddclient_conf) + # default value 300 seconds + systemd_override = read_file(DDCLIENT_SYSTEMD_UNIT) + self.assertIn(f'--daemon {interval}', systemd_override) + # IPv4+IPv6 dual DDNS service configuration def test_03_dyndns_service_dual_stack(self): services = {'cloudflare': {'protocol': 'cloudflare', 'zone': zone}, @@ -339,9 +347,10 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase): self.cli_commit() # Check for process in VRF - systemd_override = cmd(f'cat {DDCLIENT_SYSTEMD_UNIT}') + systemd_override = read_file(DDCLIENT_SYSTEMD_UNIT) self.assertIn(f'ExecStart=ip vrf exec {vrf_name} /usr/bin/ddclient ' \ - f'--file {DDCLIENT_CONF} --foreground', systemd_override) + f'--file {DDCLIENT_CONF} --cache {DDCLIENT_CONF.replace("conf", "cache")} ' \ + f'--foreground --daemon {default_interval}', systemd_override) # Check for process in VRF proc = cmd(f'ip vrf pids {vrf_name}') diff --git a/src/tests/test_configd_inspect.py b/src/tests/test_configd_inspect.py index ccd631893..a0470221d 100644 --- a/src/tests/test_configd_inspect.py +++ b/src/tests/test_configd_inspect.py @@ -1,4 +1,4 @@ -# Copyright (C) 2020-2024 VyOS maintainers and contributors +# Copyright (C) 2020-2025 VyOS maintainers and contributors # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License version 2 or later as @@ -12,93 +12,151 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. -import os -import re +import ast import json -import warnings -import importlib.util -from inspect import signature -from inspect import getsource -from functools import wraps from unittest import TestCase INC_FILE = 'data/configd-include.json' CONF_DIR = 'src/conf_mode' -f_list = ['get_config', 'verify', 'generate', 'apply'] - -def import_script(s): - path = os.path.join(CONF_DIR, s) - name = os.path.splitext(s)[0].replace('-', '_') - spec = importlib.util.spec_from_file_location(name, path) - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - return module - -# importing conf_mode scripts imports jinja2 with deprecation warning -def ignore_deprecation_warning(f): - @wraps(f) - def decorated_function(*args, **kwargs): - with warnings.catch_warnings(): - warnings.simplefilter("ignore") - f(*args, **kwargs) - return decorated_function +funcs = ['get_config', 'verify', 'generate', 'apply'] + + +class FunctionSig(ast.NodeVisitor): + def __init__(self): + self.func_sig_len = dict.fromkeys(funcs, None) + self.get_config_default_values = [] + + def visit_FunctionDef(self, node): + func_name = node.name + if func_name in funcs: + self.func_sig_len[func_name] = len(node.args.args) + + if func_name == 'get_config': + for default in node.args.defaults: + if isinstance(default, ast.Constant): + self.get_config_default_values.append(default.value) + + self.generic_visit(node) + + def get_sig_lengths(self): + return self.func_sig_len + + def get_config_default(self): + return self.get_config_default_values[0] + + +class LegacyCall(ast.NodeVisitor): + def __init__(self): + self.legacy_func_count = 0 + + def visit_Constant(self, node): + value = node.value + if isinstance(value, str): + if 'my_set' in value or 'my_delete' in value: + self.legacy_func_count += 1 + + self.generic_visit(node) + + def get_legacy_func_count(self): + return self.legacy_func_count + + +class ConfigInstance(ast.NodeVisitor): + def __init__(self): + self.count = 0 + + def visit_Call(self, node): + if isinstance(node.func, ast.Name): + name = node.func.id + if name == 'Config': + self.count += 1 + self.generic_visit(node) + + def get_count(self): + return self.count + + +class FunctionConfigInstance(ast.NodeVisitor): + def __init__(self): + self.func_config_instance = dict.fromkeys(funcs, 0) + + def visit_FunctionDef(self, node): + func_name = node.name + if func_name in funcs: + config_instance = ConfigInstance() + config_instance.visit(node) + self.func_config_instance[func_name] = config_instance.get_count() + self.generic_visit(node) + + def get_func_config_instance(self): + return self.func_config_instance + class TestConfigdInspect(TestCase): def setUp(self): + self.ast_list = [] + with open(INC_FILE) as f: self.inc_list = json.load(f) - @ignore_deprecation_warning - def test_signatures(self): for s in self.inc_list: - m = import_script(s) - for i in f_list: - f = getattr(m, i, None) - self.assertIsNotNone(f, f"'{s}': missing function '{i}'") - sig = signature(f) - par = sig.parameters - l = len(par) - self.assertEqual(l, 1, - f"'{s}': '{i}' incorrect signature") - if i == 'get_config': - for p in par.values(): - self.assertTrue(p.default is None, - f"'{s}': '{i}' incorrect signature") - - @ignore_deprecation_warning - def test_function_instance(self): - for s in self.inc_list: - m = import_script(s) - for i in f_list: - f = getattr(m, i, None) - if not f: - continue - str_f = getsource(f) - # Regex not XXXConfig() T3108 - n = len(re.findall(r'[^a-zA-Z]Config\(\)', str_f)) - if i == 'get_config': - self.assertEqual(n, 1, - f"'{s}': '{i}' no instance of Config") - if i != 'get_config': - self.assertEqual(n, 0, - f"'{s}': '{i}' instance of Config") - - @ignore_deprecation_warning - def test_file_instance(self): - for s in self.inc_list: - m = import_script(s) - str_m = getsource(m) - # Regex not XXXConfig T3108 - n = len(re.findall(r'[^a-zA-Z]Config\(\)', str_m)) - self.assertEqual(n, 1, - f"'{s}' more than one instance of Config") - - @ignore_deprecation_warning + s_path = f'{CONF_DIR}/{s}' + with open(s_path) as f: + s_str = f.read() + s_tree = ast.parse(s_str) + self.ast_list.append((s, s_tree)) + + def test_signatures(self): + for s, t in self.ast_list: + visitor = FunctionSig() + visitor.visit(t) + sig_lens = visitor.get_sig_lengths() + + for f in funcs: + self.assertIsNotNone(sig_lens[f], f"'{s}': '{f}' missing") + self.assertEqual(sig_lens[f], 1, f"'{s}': '{f}' incorrect signature") + + self.assertEqual( + visitor.get_config_default(), + None, + f"'{s}': 'get_config' incorrect signature", + ) + + def test_file_config_instance(self): + for s, t in self.ast_list: + visitor = ConfigInstance() + visitor.visit(t) + count = visitor.get_count() + + self.assertEqual(count, 1, f"'{s}' more than one instance of Config") + + def test_function_config_instance(self): + for s, t in self.ast_list: + visitor = FunctionConfigInstance() + visitor.visit(t) + func_config_instance = visitor.get_func_config_instance() + + for f in funcs: + if f == 'get_config': + self.assertTrue( + func_config_instance[f] > 0, + f"'{s}': '{f}' no instance of Config", + ) + self.assertTrue( + func_config_instance[f] < 2, + f"'{s}': '{f}' more than one instance of Config", + ) + else: + self.assertEqual( + func_config_instance[f], 0, f"'{s}': '{f}' instance of Config" + ) + def test_config_modification(self): - for s in self.inc_list: - m = import_script(s) - str_m = getsource(m) - n = str_m.count('my_set') - self.assertEqual(n, 0, f"'{s}' modifies config") + for s, t in self.ast_list: + visitor = LegacyCall() + visitor.visit(t) + legacy_func_count = visitor.get_legacy_func_count() + + self.assertEqual(legacy_func_count, 0, f"'{s}' modifies config") |