From bd6096a018c14e2e9866f77950371359eb8fc3d2 Mon Sep 17 00:00:00 2001 From: Alexander Allen Date: Sun, 23 May 2021 16:40:43 -0400 Subject: [PATCH] [ntp] Fix ntp.conf template to allow setting of source port in CONFIG_DB (#7586) Why I did it Currently, there is a bug in the ntp.conf jinja2 template where it will ignore the src_intf directive in CONFIG_DB if there are multiple IP addresses associated with an interface. This code change fixes that bug and allows the template to select the correct source interface for NTP. How I did it I did this by modifying the macro in ntp.conf.j2 which determines if there is an ip address associated with an interface to set a state variable when it detects a valid interface entry in CONFIG_DB instead of outputting "true" directly (which could result in multiple "trues" outputted for interfaces with multiple valid IP addresses). How to verify it Add two ipv4 addresses to an interface in SONiC Add the following configuration to config_db.json { "NTP": { "global": { "src_intf": "Ethernet1" } } } Replace Ethernet1 with the interface name of the one you assigned the IP addresses to. Run sudo config reload -y Open /etc/ntp.conf and verify that the following line exists ... interface listen Ethernet1 ... The interface specified should be the one set in the previous steps. Description for the changelog [ntp] Fix ntp.conf template to allow setting of source port in CONFIG_DB --- files/image_config/ntp/ntp.conf.j2 | 4 +- .../tests/data/ntp/ntp_interfaces.json | 12 ++++ src/sonic-config-engine/tests/ntp.conf.j2 | 1 + .../tests/sample_output/py2/ntp.conf | 72 +++++++++++++++++++ .../tests/sample_output/py3/ntp.conf | 72 +++++++++++++++++++ src/sonic-config-engine/tests/test_j2files.py | 9 +++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 src/sonic-config-engine/tests/data/ntp/ntp_interfaces.json create mode 120000 src/sonic-config-engine/tests/ntp.conf.j2 create mode 100644 src/sonic-config-engine/tests/sample_output/py2/ntp.conf create mode 100644 src/sonic-config-engine/tests/sample_output/py3/ntp.conf diff --git a/files/image_config/ntp/ntp.conf.j2 b/files/image_config/ntp/ntp.conf.j2 index 4b76321932..280b46a426 100644 --- a/files/image_config/ntp/ntp.conf.j2 +++ b/files/image_config/ntp/ntp.conf.j2 @@ -41,13 +41,15 @@ interface ignore wildcard # if the source interface is configured but no ip on that interface, then listen on another # interface based on existing logic {%- macro check_ip_on_interface(interface_name, table_name) %} + {%- set ns = namespace(valid_intf = 'false') %} {%- if table_name %} {%- for (name, source_prefix) in table_name|pfx_filter %} {%- if source_prefix and name == interface_name %} -true + {%- set ns.valid_intf = 'true' %} {%- endif %} {%- endfor %} {%- endif %} +{{ ns.valid_intf }} {%- endmacro %} {% set ns = namespace(source_intf = "") %} diff --git a/src/sonic-config-engine/tests/data/ntp/ntp_interfaces.json b/src/sonic-config-engine/tests/data/ntp/ntp_interfaces.json new file mode 100644 index 0000000000..2847583014 --- /dev/null +++ b/src/sonic-config-engine/tests/data/ntp/ntp_interfaces.json @@ -0,0 +1,12 @@ +{ + "NTP": { + "global": { + "src_intf": "Ethernet0" + } + }, + "INTERFACE": { + "Ethernet0": {}, + "Ethernet0|10.0.0.0/31": {}, + "Ethernet0|192.168.0.122/24": {} + } +} diff --git a/src/sonic-config-engine/tests/ntp.conf.j2 b/src/sonic-config-engine/tests/ntp.conf.j2 new file mode 120000 index 0000000000..bc52df834e --- /dev/null +++ b/src/sonic-config-engine/tests/ntp.conf.j2 @@ -0,0 +1 @@ +../../../files/image_config/ntp/ntp.conf.j2 \ No newline at end of file diff --git a/src/sonic-config-engine/tests/sample_output/py2/ntp.conf b/src/sonic-config-engine/tests/sample_output/py2/ntp.conf new file mode 100644 index 0000000000..bc98019e88 --- /dev/null +++ b/src/sonic-config-engine/tests/sample_output/py2/ntp.conf @@ -0,0 +1,72 @@ +############################################################################### +# Managed by Ansible +# file: ansible/roles/acs/templates/ntp.conf.j2 +############################################################################### + +# /etc/ntp.conf, configuration for ntpd; see ntp.conf(5) for help + +# To avoid ntpd from panic and exit if the drift between new time and +# current system time is large. +tinker panic 0 + +driftfile /var/lib/ntp/ntp.drift + + +# Enable this if you want statistics to be logged. +#statsdir /var/log/ntpstats/ + +statistics loopstats peerstats clockstats +filegen loopstats file loopstats type day enable +filegen peerstats file peerstats type day enable +filegen clockstats file clockstats type day enable + + +# You do need to talk to an NTP server or two (or three). +#server ntp.your-provider.example + +# pool.ntp.org maps to about 1000 low-stratum NTP servers. Your server will +# pick a different set every time it starts up. Please consider joining the +# pool: + +#listen on source interface if configured, else +#only listen on MGMT_INTERFACE, LOOPBACK_INTERFACE ip when MGMT_INTERFACE is not defined, or eth0 +# if we don't have both of them (default is to listen on all ip addresses) +interface ignore wildcard + +# set global variable for configured source interface name +# set global boolean to indicate if the ip of the configured source interface is configured +# if the source interface is configured but no ip on that interface, then listen on another +# interface based on existing logic + +interface listen Ethernet0 +interface listen 127.0.0.1 + +# Access control configuration; see /usr/share/doc/ntp-doc/html/accopt.html for +# details. The web page +# might also be helpful. +# +# Note that "restrict" applies to both servers and clients, so a configuration +# that might be intended to block requests from certain clients could also end +# up blocking replies from your own upstream servers. + +# By default, exchange time with everybody, but don't allow configuration. +restrict -4 default kod notrap nomodify nopeer noquery +restrict -6 default kod notrap nomodify nopeer noquery + +# Local users may interrogate the ntp server more closely. +restrict 127.0.0.1 +restrict ::1 + +# Clients from this (example!) subnet have unlimited access, but only if +# cryptographically authenticated. +#restrict 192.168.123.0 mask 255.255.255.0 notrust + + +# If you want to provide time to your local subnet, change the next line. +# (Again, the address is an example only.) +#broadcast 192.168.123.255 + +# If you want to listen to time broadcasts on your local subnet, de-comment the +# next lines. Please do this only if you trust everybody on the network! +#disable auth +#broadcastclient diff --git a/src/sonic-config-engine/tests/sample_output/py3/ntp.conf b/src/sonic-config-engine/tests/sample_output/py3/ntp.conf new file mode 100644 index 0000000000..bc98019e88 --- /dev/null +++ b/src/sonic-config-engine/tests/sample_output/py3/ntp.conf @@ -0,0 +1,72 @@ +############################################################################### +# Managed by Ansible +# file: ansible/roles/acs/templates/ntp.conf.j2 +############################################################################### + +# /etc/ntp.conf, configuration for ntpd; see ntp.conf(5) for help + +# To avoid ntpd from panic and exit if the drift between new time and +# current system time is large. +tinker panic 0 + +driftfile /var/lib/ntp/ntp.drift + + +# Enable this if you want statistics to be logged. +#statsdir /var/log/ntpstats/ + +statistics loopstats peerstats clockstats +filegen loopstats file loopstats type day enable +filegen peerstats file peerstats type day enable +filegen clockstats file clockstats type day enable + + +# You do need to talk to an NTP server or two (or three). +#server ntp.your-provider.example + +# pool.ntp.org maps to about 1000 low-stratum NTP servers. Your server will +# pick a different set every time it starts up. Please consider joining the +# pool: + +#listen on source interface if configured, else +#only listen on MGMT_INTERFACE, LOOPBACK_INTERFACE ip when MGMT_INTERFACE is not defined, or eth0 +# if we don't have both of them (default is to listen on all ip addresses) +interface ignore wildcard + +# set global variable for configured source interface name +# set global boolean to indicate if the ip of the configured source interface is configured +# if the source interface is configured but no ip on that interface, then listen on another +# interface based on existing logic + +interface listen Ethernet0 +interface listen 127.0.0.1 + +# Access control configuration; see /usr/share/doc/ntp-doc/html/accopt.html for +# details. The web page +# might also be helpful. +# +# Note that "restrict" applies to both servers and clients, so a configuration +# that might be intended to block requests from certain clients could also end +# up blocking replies from your own upstream servers. + +# By default, exchange time with everybody, but don't allow configuration. +restrict -4 default kod notrap nomodify nopeer noquery +restrict -6 default kod notrap nomodify nopeer noquery + +# Local users may interrogate the ntp server more closely. +restrict 127.0.0.1 +restrict ::1 + +# Clients from this (example!) subnet have unlimited access, but only if +# cryptographically authenticated. +#restrict 192.168.123.0 mask 255.255.255.0 notrust + + +# If you want to provide time to your local subnet, change the next line. +# (Again, the address is an example only.) +#broadcast 192.168.123.255 + +# If you want to listen to time broadcasts on your local subnet, de-comment the +# next lines. Please do this only if you trust everybody on the network! +#disable auth +#broadcastclient diff --git a/src/sonic-config-engine/tests/test_j2files.py b/src/sonic-config-engine/tests/test_j2files.py index 8a6fdf5c39..8b4e9c686b 100644 --- a/src/sonic-config-engine/tests/test_j2files.py +++ b/src/sonic-config-engine/tests/test_j2files.py @@ -297,6 +297,15 @@ class TestJ2Files(TestCase): self.run_script(argument) assert filecmp.cmp(expected, self.output_file), self.run_diff(expected, self.output_file) + def test_ntp_conf(self): + conf_template = os.path.join(self.test_dir, "ntp.conf.j2") + ntp_interfaces_json = os.path.join(self.test_dir, "data", "ntp", "ntp_interfaces.json") + expected = os.path.join(self.test_dir, "sample_output", utils.PYvX_DIR, "ntp.conf") + + argument = '-j {} -t {} > {}'.format(ntp_interfaces_json, conf_template, self.output_file) + self.run_script(argument) + assert filecmp.cmp(expected, self.output_file), self.run_diff(expected, self.output_file) + def tearDown(self): try: os.remove(self.output_file)