From 8381edc12a116eee3a524ee5d060c057cb039f47 Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Mon, 3 Aug 2026 23:14:08 +0200 Subject: ntp: T9159: add source-address option for client requests chrony (the daemon backing VyOS' NTP service) has no way to pin the local address used for outgoing client requests to upstream servers. "interface"/"listen-address" only configure the server/listening role (binddevice/bindaddress); nothing controls the source address chosen for packets chronyd itself originates. On a router with more than one usable egress path to a given NTP server (e.g. multiple transit/peering sessions), the kernel's default source-address selection depends on whichever route wins at query time. If that route happens to egress through an interface whose own address is not globally reachable (a private peering-fabric segment, for example), the request goes out with an address the reply can never route back to - an intermittent, path-dependent failure with no existing workaround at the NTP layer. BGP and friends already solve the equivalent problem via "update-source" bound to the loopback; NTP had no analogous option. Add "service ntp source-address
", reusing the existing source-address-ipv4-ipv6-multi include (same pattern already used for e.g. RADIUS servers), and render it as chrony's "bindacqaddress" directive - the client-side counterpart to "bindaddress" that chrony already supports natively. --- smoketest/scripts/cli/test_service_ntp.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'smoketest/scripts/cli') diff --git a/smoketest/scripts/cli/test_service_ntp.py b/smoketest/scripts/cli/test_service_ntp.py index 76ce6b644..e59e4ea88 100755 --- a/smoketest/scripts/cli/test_service_ntp.py +++ b/smoketest/scripts/cli/test_service_ntp.py @@ -113,6 +113,22 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): for listen in listen_address: self.assertIn(f'bindaddress {listen}', config) + def test_source_address(self): + source_addresses = ['127.0.0.1', '::1'] + for address in source_addresses: + self.cli_set(base_path + ['source-address', address]) + + servers = ['time1.vyos.net', 'time2.vyos.net'] + for server in servers: + self.cli_set(base_path + ['server', server]) + + self.cli_commit() + + # Check generated client source-address configuration + config = read_file(NTP_CONF, sudo=True) + for address in source_addresses: + self.assertIn(f'bindacqaddress {address}', config) + def test_interface(self): interfaces = ['eth0'] for interface in interfaces: -- cgit v1.2.3 From bf25f63d724ca3175a56d63d509fe164d1a29508 Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Mon, 3 Aug 2026 23:33:50 +0200 Subject: ntp: T9159: add negative smoketest coverage for source-address limit Per review feedback: cover the case where more than one IPv4 or IPv6 source-address is configured, which verify() already rejects. --- smoketest/scripts/cli/test_service_ntp.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'smoketest/scripts/cli') diff --git a/smoketest/scripts/cli/test_service_ntp.py b/smoketest/scripts/cli/test_service_ntp.py index e59e4ea88..f2b6b223e 100755 --- a/smoketest/scripts/cli/test_service_ntp.py +++ b/smoketest/scripts/cli/test_service_ntp.py @@ -129,6 +129,26 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): for address in source_addresses: self.assertIn(f'bindacqaddress {address}', config) + def test_source_address_rejects_multiple_ipv4(self): + self.cli_set(base_path + ['source-address', '192.0.2.1']) + self.cli_set(base_path + ['source-address', '192.0.2.2']) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + # fix up and commit successfully so chronyd is left running for tearDown + self.cli_delete(base_path + ['source-address', '192.0.2.2']) + self.cli_commit() + + def test_source_address_rejects_multiple_ipv6(self): + self.cli_set(base_path + ['source-address', '2001:db8::1']) + self.cli_set(base_path + ['source-address', '2001:db8::2']) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + # fix up and commit successfully so chronyd is left running for tearDown + self.cli_delete(base_path + ['source-address', '2001:db8::2']) + self.cli_commit() + def test_interface(self): interfaces = ['eth0'] for interface in interfaces: -- cgit v1.2.3 From e2e14e29245f0b1a2de2567662892702ec4ff6f7 Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Mon, 3 Aug 2026 23:44:38 +0200 Subject: ntp: T9159: fix source-address negative test recovery commit Per review feedback: deleting only the duplicate address left a non-local address (192.0.2.1 / 2001:db8::1) configured as source-address for the recovery commit, which chronyd cannot actually bind to on the test system. Delete the whole source-address subtree instead before recommitting. --- smoketest/scripts/cli/test_service_ntp.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'smoketest/scripts/cli') diff --git a/smoketest/scripts/cli/test_service_ntp.py b/smoketest/scripts/cli/test_service_ntp.py index f2b6b223e..054cbeda9 100755 --- a/smoketest/scripts/cli/test_service_ntp.py +++ b/smoketest/scripts/cli/test_service_ntp.py @@ -135,8 +135,9 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): with self.assertRaises(ConfigSessionError): self.cli_commit() - # fix up and commit successfully so chronyd is left running for tearDown - self.cli_delete(base_path + ['source-address', '192.0.2.2']) + # remove the invalid subtree entirely (rather than leaving a + # non-local address configured) so chronyd is left running for tearDown + self.cli_delete(base_path + ['source-address']) self.cli_commit() def test_source_address_rejects_multiple_ipv6(self): @@ -145,8 +146,9 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): with self.assertRaises(ConfigSessionError): self.cli_commit() - # fix up and commit successfully so chronyd is left running for tearDown - self.cli_delete(base_path + ['source-address', '2001:db8::2']) + # remove the invalid subtree entirely (rather than leaving a + # non-local address configured) so chronyd is left running for tearDown + self.cli_delete(base_path + ['source-address']) self.cli_commit() def test_interface(self): -- cgit v1.2.3 From 21a55fd494ef9c73744da718560ad540dd3fc0bf Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Wed, 5 Aug 2026 08:38:05 +0200 Subject: ntp: T9159: add source-interface option for client requests Complements source-address (bindacqaddress) with a device-based bind (chrony's bindacqdevice) for outgoing NTP client requests. Addresses feedback on T9159 that a source IP alone cannot unambiguously select the egress path in multi-VRF setups with overlapping address ranges. Both options bind different socket properties and can be combined. If "vrf" is set, source-interface must belong to that VRF, mirroring the existing check for "interface". --- data/templates/chrony/chrony.conf.j2 | 15 ++++++--- interface-definitions/service_ntp.xml.in | 1 + smoketest/scripts/cli/test_service_ntp.py | 56 +++++++++++++++++++++++++++++++ src/conf_mode/service_ntp.py | 14 ++++++++ 4 files changed, 81 insertions(+), 5 deletions(-) (limited to 'smoketest/scripts/cli') diff --git a/data/templates/chrony/chrony.conf.j2 b/data/templates/chrony/chrony.conf.j2 index 9de3e9518..271c6de87 100644 --- a/data/templates/chrony/chrony.conf.j2 +++ b/data/templates/chrony/chrony.conf.j2 @@ -72,12 +72,17 @@ binddevice {{ interface }} {% endif %} {% endif %} -{% if source_address is vyos_defined %} -# Source address used for outgoing NTP client requests, independent of -# whichever interface the kernel's routing table selects for a given server -{% for address in source_address %} +{% if source_address is vyos_defined or source_interface is vyos_defined %} +# Source used for outgoing NTP client requests, independent of whichever +# interface the kernel's routing table selects for a given server +{% if source_address is vyos_defined %} +{% for address in source_address %} bindacqaddress {{ address }} -{% endfor %} +{% endfor %} +{% endif %} +{% if source_interface is vyos_defined %} +bindacqdevice {{ source_interface }} +{% endif %} {% endif %} {% if timestamp.interface is vyos_defined %} diff --git a/interface-definitions/service_ntp.xml.in b/interface-definitions/service_ntp.xml.in index 5b481bd6e..40e4a5d15 100644 --- a/interface-definitions/service_ntp.xml.in +++ b/interface-definitions/service_ntp.xml.in @@ -13,6 +13,7 @@ #include #include #include + #include #include diff --git a/smoketest/scripts/cli/test_service_ntp.py b/smoketest/scripts/cli/test_service_ntp.py index 054cbeda9..6f1bb82e2 100755 --- a/smoketest/scripts/cli/test_service_ntp.py +++ b/smoketest/scripts/cli/test_service_ntp.py @@ -151,6 +151,62 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): self.cli_delete(base_path + ['source-address']) self.cli_commit() + def test_source_interface(self): + interface = 'eth0' + self.cli_set(base_path + ['source-interface', interface]) + + servers = ['time1.vyos.net', 'time2.vyos.net'] + for server in servers: + self.cli_set(base_path + ['server', server]) + + self.cli_commit() + + # Check generated client source-interface configuration + config = read_file(NTP_CONF, sudo=True) + self.assertIn(f'bindacqdevice {interface}', config) + + def test_source_address_and_source_interface(self): + # bindacqaddress (IP) and bindacqdevice (interface) bind different + # socket properties and can be configured together + source_addresses = ['127.0.0.1', '::1'] + interface = 'eth0' + for address in source_addresses: + self.cli_set(base_path + ['source-address', address]) + self.cli_set(base_path + ['source-interface', interface]) + + servers = ['time1.vyos.net', 'time2.vyos.net'] + for server in servers: + self.cli_set(base_path + ['server', server]) + + self.cli_commit() + + config = read_file(NTP_CONF, sudo=True) + for address in source_addresses: + self.assertIn(f'bindacqaddress {address}', config) + self.assertIn(f'bindacqdevice {interface}', config) + + def test_source_interface_vrf_mismatch(self): + vrf_name = 'vyos-mgmt' + interface = 'eth0' + + self.cli_set(['vrf', 'name', vrf_name, 'table', '12345']) + self.cli_set(base_path + ['vrf', vrf_name]) + self.cli_set(base_path + ['source-interface', interface]) + + servers = ['time1.vyos.net', 'time2.vyos.net'] + for server in servers: + self.cli_set(base_path + ['server', server]) + + # eth0 does not belong to the VRF - commit must be rejected + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + # fix the invalid combination so chronyd is left running for tearDown + self.cli_delete(base_path + ['source-interface']) + self.cli_delete(base_path + ['vrf']) + self.cli_delete(['vrf', 'name', vrf_name]) + self.cli_commit() + def test_interface(self): interfaces = ['eth0'] for interface in interfaces: diff --git a/src/conf_mode/service_ntp.py b/src/conf_mode/service_ntp.py index 3cb883ae1..adc161a9d 100755 --- a/src/conf_mode/service_ntp.py +++ b/src/conf_mode/service_ntp.py @@ -81,6 +81,20 @@ def verify(ntp): raise ConfigError(f'NTP runs in VRF "{vrf_name}" - "{interface}" '\ f'does not belong to this VRF!') + if 'source_interface' in ntp: + # If outgoing NTP client requests should be bound to a given + # interface (device), ensure it exists + source_interface = ntp['source_interface'] + verify_interface_exists(ntp, source_interface) + + # If we run in a VRF, our source interface must belong to this VRF, too + if 'vrf' in ntp: + tmp = get_interface_config(source_interface) + vrf_name = ntp['vrf'] + if 'master' not in tmp or tmp['master'] != vrf_name: + raise ConfigError(f'NTP runs in VRF "{vrf_name}" - "{source_interface}" '\ + f'does not belong to this VRF!') + if 'listen_address' in ntp: ipv4_addresses = 0 ipv6_addresses = 0 -- cgit v1.2.3 From d798592061b9c5063ae849227d09a3f0c0af78cc Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Wed, 5 Aug 2026 14:42:26 +0200 Subject: ntp: T9159: verify source-address is locally assigned, fix ruff findings - reject non-local source-address, matching the is_addr_assigned pattern already used by system_syslog.py/system_login.py/etc, addressing CodeRabbit review feedback on PR #5371 - fix ruff findings in the touched file: extraneous f-string prefixes, E701 multi-statement line, and the same in the smoketest file - update the two duplicate-address negative tests to use locally assigned addresses (via a dummy interface) so they actually exercise the per-family duplicate check instead of incidentally tripping the new local-assignment check; add a dedicated non-local rejection test --- smoketest/scripts/cli/test_service_ntp.py | 27 ++++++++++++++++++++++++--- src/conf_mode/service_ntp.py | 16 +++++++++++----- 2 files changed, 35 insertions(+), 8 deletions(-) (limited to 'smoketest/scripts/cli') diff --git a/smoketest/scripts/cli/test_service_ntp.py b/smoketest/scripts/cli/test_service_ntp.py index 6f1bb82e2..75fb3172c 100755 --- a/smoketest/scripts/cli/test_service_ntp.py +++ b/smoketest/scripts/cli/test_service_ntp.py @@ -27,6 +27,8 @@ from vyos.xml_ref import default_value PROCESS_NAME = 'chronyd' NTP_CONF = '/run/chrony/chrony.conf' base_path = ['service', 'ntp'] +dummy_interface = 'dum9159' +dummy_if_path = ['interfaces', 'dummy', dummy_interface] class TestSystemNTP(VyOSUnitTestSHIM.TestCase): @classmethod @@ -130,6 +132,10 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): self.assertIn(f'bindacqaddress {address}', config) def test_source_address_rejects_multiple_ipv4(self): + # both addresses must be locally assigned, so it's the duplicate-per- + # family check being tested here, not the local-assignment check + self.cli_set(dummy_if_path + ['address', '192.0.2.1/32']) + self.cli_set(dummy_if_path + ['address', '192.0.2.2/32']) self.cli_set(base_path + ['source-address', '192.0.2.1']) self.cli_set(base_path + ['source-address', '192.0.2.2']) with self.assertRaises(ConfigSessionError): @@ -138,9 +144,14 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): # remove the invalid subtree entirely (rather than leaving a # non-local address configured) so chronyd is left running for tearDown self.cli_delete(base_path + ['source-address']) + self.cli_delete(dummy_if_path) self.cli_commit() def test_source_address_rejects_multiple_ipv6(self): + # both addresses must be locally assigned, so it's the duplicate-per- + # family check being tested here, not the local-assignment check + self.cli_set(dummy_if_path + ['address', '2001:db8::1/128']) + self.cli_set(dummy_if_path + ['address', '2001:db8::2/128']) self.cli_set(base_path + ['source-address', '2001:db8::1']) self.cli_set(base_path + ['source-address', '2001:db8::2']) with self.assertRaises(ConfigSessionError): @@ -148,6 +159,16 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): # remove the invalid subtree entirely (rather than leaving a # non-local address configured) so chronyd is left running for tearDown + self.cli_delete(base_path + ['source-address']) + self.cli_delete(dummy_if_path) + self.cli_commit() + + def test_source_address_rejects_non_local(self): + # source-address must be assigned to a local interface + self.cli_set(base_path + ['source-address', '192.0.2.1']) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + self.cli_delete(base_path + ['source-address']) self.cli_commit() @@ -259,9 +280,9 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): if mode != 'smear': self.assertIn(f'leapsecmode {mode}', config) else: - self.assertIn(f'leapsecmode slew', config) - self.assertIn(f'maxslewrate 1000', config) - self.assertIn(f'smoothtime 400 0.001024 leaponly', config) + self.assertIn('leapsecmode slew', config) + self.assertIn('maxslewrate 1000', config) + self.assertIn('smoothtime 400 0.001024 leaponly', config) def test_interleave_option(self): # "interleave" option differs from some others in that the diff --git a/src/conf_mode/service_ntp.py b/src/conf_mode/service_ntp.py index adc161a9d..7a49be6c1 100755 --- a/src/conf_mode/service_ntp.py +++ b/src/conf_mode/service_ntp.py @@ -25,6 +25,7 @@ from vyos.netlink import timestamp from vyos.utils.process import call from vyos.utils.permission import chmod_750 from vyos.utils.network import get_interface_config +from vyos.utils.network import is_addr_assigned from vyos.template import render from vyos.template import is_ipv4 from vyos import ConfigError @@ -49,7 +50,8 @@ def get_config(config=None): ntp['user'] = user_group tmp = is_node_changed(conf, base + ['vrf']) - if tmp: ntp.update({'restart_required': {}}) + if tmp: + ntp.update({'restart_required': {}}) # We have gathered the dict representation of the CLI, but there are default # options which we need to update into the dictionary retrieved. @@ -104,22 +106,26 @@ def verify(ntp): else: ipv6_addresses += 1 if ipv4_addresses > 1: - raise ConfigError(f'NTP Only admits one ipv4 value for listen-address parameter ') + raise ConfigError('NTP Only admits one ipv4 value for listen-address parameter') if ipv6_addresses > 1: - raise ConfigError(f'NTP Only admits one ipv6 value for listen-address parameter ') + raise ConfigError('NTP Only admits one ipv6 value for listen-address parameter') if 'source_address' in ntp: ipv4_addresses = 0 ipv6_addresses = 0 + vrf = ntp.get('vrf') for address in ntp['source_address']: if is_ipv4(address): ipv4_addresses += 1 else: ipv6_addresses += 1 + if not is_addr_assigned(address, vrf): + raise ConfigError(f'NTP source-address "{address}" not assigned ' + 'to any interface!') if ipv4_addresses > 1: - raise ConfigError(f'NTP Only admits one ipv4 value for source-address parameter ') + raise ConfigError('NTP Only admits one ipv4 value for source-address parameter') if ipv6_addresses > 1: - raise ConfigError(f'NTP Only admits one ipv6 value for source-address parameter ') + raise ConfigError('NTP Only admits one ipv6 value for source-address parameter') if 'server' in ntp: for host, server in ntp['server'].items(): -- cgit v1.2.3 From d42d8ef1976b0ab8d0b498bf7073f8afea8d19e3 Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Wed, 5 Aug 2026 16:09:10 +0200 Subject: ntp: T9159: fix smoketest ordering for source-address dummy interface Commit the dummy interface's addresses on their own first, then set the NTP source-address in a separate commit. Setting both in the same commit made the outcome depend on conf_mode script execution order (interfaces_dummy vs service_ntp) for whether is_addr_assigned() would see the address as already live -- not guaranteed, and the likely cause of the CLI Smoketests CI failure on this branch. --- smoketest/scripts/cli/test_service_ntp.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'smoketest/scripts/cli') diff --git a/smoketest/scripts/cli/test_service_ntp.py b/smoketest/scripts/cli/test_service_ntp.py index 75fb3172c..06d59d398 100755 --- a/smoketest/scripts/cli/test_service_ntp.py +++ b/smoketest/scripts/cli/test_service_ntp.py @@ -132,10 +132,15 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): self.assertIn(f'bindacqaddress {address}', config) def test_source_address_rejects_multiple_ipv4(self): - # both addresses must be locally assigned, so it's the duplicate-per- - # family check being tested here, not the local-assignment check + # commit the dummy addresses on their own first, so they are + # actually live before NTP's verify() checks is_addr_assigned() -- + # otherwise the outcome would depend on conf_mode script ordering + # within a single commit. That way it's the duplicate-per-family + # check being tested here, not the local-assignment check. self.cli_set(dummy_if_path + ['address', '192.0.2.1/32']) self.cli_set(dummy_if_path + ['address', '192.0.2.2/32']) + self.cli_commit() + self.cli_set(base_path + ['source-address', '192.0.2.1']) self.cli_set(base_path + ['source-address', '192.0.2.2']) with self.assertRaises(ConfigSessionError): @@ -148,10 +153,15 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): self.cli_commit() def test_source_address_rejects_multiple_ipv6(self): - # both addresses must be locally assigned, so it's the duplicate-per- - # family check being tested here, not the local-assignment check + # commit the dummy addresses on their own first, so they are + # actually live before NTP's verify() checks is_addr_assigned() -- + # otherwise the outcome would depend on conf_mode script ordering + # within a single commit. That way it's the duplicate-per-family + # check being tested here, not the local-assignment check. self.cli_set(dummy_if_path + ['address', '2001:db8::1/128']) self.cli_set(dummy_if_path + ['address', '2001:db8::2/128']) + self.cli_commit() + self.cli_set(base_path + ['source-address', '2001:db8::1']) self.cli_set(base_path + ['source-address', '2001:db8::2']) with self.assertRaises(ConfigSessionError): -- cgit v1.2.3 From 4432fcf72ca20be430aa9577eee0446d35a2539b Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Wed, 5 Aug 2026 16:13:56 +0200 Subject: ntp: T9159: harden smoketest per CodeRabbit feedback - assertRaisesRegex instead of bare assertRaises for the two duplicate- per-family checks and the non-local-address check, so each test verifies it hit the specific validation branch it's meant to, not just that *some* ConfigSessionError fired - wrap the duplicate-address tests' cleanup in try/finally so a failed assertion doesn't leave the dummy interface / source-address behind for subsequent tests to trip over --- smoketest/scripts/cli/test_service_ntp.py | 46 +++++++++++++++++++------------ 1 file changed, 28 insertions(+), 18 deletions(-) (limited to 'smoketest/scripts/cli') diff --git a/smoketest/scripts/cli/test_service_ntp.py b/smoketest/scripts/cli/test_service_ntp.py index 06d59d398..03caa73b3 100755 --- a/smoketest/scripts/cli/test_service_ntp.py +++ b/smoketest/scripts/cli/test_service_ntp.py @@ -143,15 +143,21 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): self.cli_set(base_path + ['source-address', '192.0.2.1']) self.cli_set(base_path + ['source-address', '192.0.2.2']) - with self.assertRaises(ConfigSessionError): + try: + with self.assertRaisesRegex( + ConfigSessionError, + 'Only admits one ipv4 value for source-address'): + self.cli_commit() + finally: + # remove the invalid subtree entirely (rather than leaving a + # non-local address configured) so chronyd is left running for + # tearDown -- in a finally block so a failed assertion above + # doesn't leave the dummy interface/source-address behind for + # subsequent tests + self.cli_delete(base_path + ['source-address']) + self.cli_delete(dummy_if_path) self.cli_commit() - # remove the invalid subtree entirely (rather than leaving a - # non-local address configured) so chronyd is left running for tearDown - self.cli_delete(base_path + ['source-address']) - self.cli_delete(dummy_if_path) - self.cli_commit() - def test_source_address_rejects_multiple_ipv6(self): # commit the dummy addresses on their own first, so they are # actually live before NTP's verify() checks is_addr_assigned() -- @@ -164,24 +170,28 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): self.cli_set(base_path + ['source-address', '2001:db8::1']) self.cli_set(base_path + ['source-address', '2001:db8::2']) - with self.assertRaises(ConfigSessionError): + try: + with self.assertRaisesRegex( + ConfigSessionError, + 'Only admits one ipv6 value for source-address'): + self.cli_commit() + finally: + # see the ipv4 test above for why this is in a finally block + self.cli_delete(base_path + ['source-address']) + self.cli_delete(dummy_if_path) self.cli_commit() - # remove the invalid subtree entirely (rather than leaving a - # non-local address configured) so chronyd is left running for tearDown - self.cli_delete(base_path + ['source-address']) - self.cli_delete(dummy_if_path) - self.cli_commit() - def test_source_address_rejects_non_local(self): # source-address must be assigned to a local interface self.cli_set(base_path + ['source-address', '192.0.2.1']) - with self.assertRaises(ConfigSessionError): + try: + with self.assertRaisesRegex(ConfigSessionError, + 'not assigned to any interface'): + self.cli_commit() + finally: + self.cli_delete(base_path + ['source-address']) self.cli_commit() - self.cli_delete(base_path + ['source-address']) - self.cli_commit() - def test_source_interface(self): interface = 'eth0' self.cli_set(base_path + ['source-interface', interface]) -- cgit v1.2.3 From d3b48b76bd71fec1d09db6c72963de797f8ccb93 Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Mon, 10 Aug 2026 15:40:48 +0200 Subject: ntp: T9159: apply darker/black formatting --- smoketest/scripts/cli/test_service_ntp.py | 13 +++++++------ src/conf_mode/service_ntp.py | 27 +++++++++++++++++++-------- 2 files changed, 26 insertions(+), 14 deletions(-) (limited to 'smoketest/scripts/cli') diff --git a/smoketest/scripts/cli/test_service_ntp.py b/smoketest/scripts/cli/test_service_ntp.py index 03caa73b3..0b8094963 100755 --- a/smoketest/scripts/cli/test_service_ntp.py +++ b/smoketest/scripts/cli/test_service_ntp.py @@ -145,8 +145,8 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): self.cli_set(base_path + ['source-address', '192.0.2.2']) try: with self.assertRaisesRegex( - ConfigSessionError, - 'Only admits one ipv4 value for source-address'): + ConfigSessionError, 'Only admits one ipv4 value for source-address' + ): self.cli_commit() finally: # remove the invalid subtree entirely (rather than leaving a @@ -172,8 +172,8 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): self.cli_set(base_path + ['source-address', '2001:db8::2']) try: with self.assertRaisesRegex( - ConfigSessionError, - 'Only admits one ipv6 value for source-address'): + ConfigSessionError, 'Only admits one ipv6 value for source-address' + ): self.cli_commit() finally: # see the ipv4 test above for why this is in a finally block @@ -185,8 +185,9 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase): # source-address must be assigned to a local interface self.cli_set(base_path + ['source-address', '192.0.2.1']) try: - with self.assertRaisesRegex(ConfigSessionError, - 'not assigned to any interface'): + with self.assertRaisesRegex( + ConfigSessionError, 'not assigned to any interface' + ): self.cli_commit() finally: self.cli_delete(base_path + ['source-address']) diff --git a/src/conf_mode/service_ntp.py b/src/conf_mode/service_ntp.py index 7a49be6c1..66cf775a3 100755 --- a/src/conf_mode/service_ntp.py +++ b/src/conf_mode/service_ntp.py @@ -94,8 +94,10 @@ def verify(ntp): tmp = get_interface_config(source_interface) vrf_name = ntp['vrf'] if 'master' not in tmp or tmp['master'] != vrf_name: - raise ConfigError(f'NTP runs in VRF "{vrf_name}" - "{source_interface}" '\ - f'does not belong to this VRF!') + raise ConfigError( + f'NTP runs in VRF "{vrf_name}" - "{source_interface}" ' + f'does not belong to this VRF!' + ) if 'listen_address' in ntp: ipv4_addresses = 0 @@ -106,9 +108,13 @@ def verify(ntp): else: ipv6_addresses += 1 if ipv4_addresses > 1: - raise ConfigError('NTP Only admits one ipv4 value for listen-address parameter') + raise ConfigError( + 'NTP Only admits one ipv4 value for listen-address parameter' + ) if ipv6_addresses > 1: - raise ConfigError('NTP Only admits one ipv6 value for listen-address parameter') + raise ConfigError( + 'NTP Only admits one ipv6 value for listen-address parameter' + ) if 'source_address' in ntp: ipv4_addresses = 0 @@ -120,12 +126,17 @@ def verify(ntp): else: ipv6_addresses += 1 if not is_addr_assigned(address, vrf): - raise ConfigError(f'NTP source-address "{address}" not assigned ' - 'to any interface!') + raise ConfigError( + f'NTP source-address "{address}" not assigned ' 'to any interface!' + ) if ipv4_addresses > 1: - raise ConfigError('NTP Only admits one ipv4 value for source-address parameter') + raise ConfigError( + 'NTP Only admits one ipv4 value for source-address parameter' + ) if ipv6_addresses > 1: - raise ConfigError('NTP Only admits one ipv6 value for source-address parameter') + raise ConfigError( + 'NTP Only admits one ipv6 value for source-address parameter' + ) if 'server' in ntp: for host, server in ntp['server'].items(): -- cgit v1.2.3