diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py index 25bba764ee2..3bc1d3031d5 100644 --- a/cloudinit/config/cc_ntp.py +++ b/cloudinit/config/cc_ntp.py @@ -12,11 +12,7 @@ from cloudinit import log as logging from cloudinit import subp, temp_utils, templater, type_utils, util -from cloudinit.config.schema import ( - MetaSchema, - get_meta_doc, - validate_cloudconfig_schema, -) +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.settings import PER_INSTANCE LOG = logging.getLogger(__name__) @@ -210,137 +206,14 @@ ], "frequency": PER_INSTANCE, } +__doc__ = get_meta_doc(meta) + -schema = { - "type": "object", - "properties": { - "ntp": { - "type": ["object", "null"], - "properties": { - "pools": { - "type": "array", - "items": {"type": "string", "format": "hostname"}, - "uniqueItems": True, - "description": dedent( - """\ - List of ntp pools. If both pools and servers are - empty, 4 default pool servers will be provided of - the format ``{0-3}.{distro}.pool.ntp.org``. NOTE: - for Alpine Linux when using the Busybox NTP client - this setting will be ignored due to the limited - functionality of Busybox's ntpd.""" - ), - }, - "servers": { - "type": "array", - "items": {"type": "string", "format": "hostname"}, - "uniqueItems": True, - "description": dedent( - """\ - List of ntp servers. If both pools and servers are - empty, 4 default pool servers will be provided with - the format ``{0-3}.{distro}.pool.ntp.org``.""" - ), - }, - "ntp_client": { - "type": "string", - "default": "auto", - "description": dedent( - """\ - Name of an NTP client to use to configure system NTP. - When unprovided or 'auto' the default client preferred - by the distribution will be used. The following - built-in client names can be used to override existing - configuration defaults: chrony, ntp, ntpdate, - systemd-timesyncd.""" - ), - }, - "enabled": { - "type": "boolean", - "default": True, - "description": dedent( - """\ - Attempt to enable ntp clients if set to True. If set - to False, ntp client will not be configured or - installed""" - ), - }, - "config": { - "description": dedent( - """\ - Configuration settings or overrides for the - ``ntp_client`` specified.""" - ), - "type": ["object"], - "properties": { - "confpath": { - "type": "string", - "description": dedent( - """\ - The path to where the ``ntp_client`` - configuration is written.""" - ), - }, - "check_exe": { - "type": "string", - "description": dedent( - """\ - The executable name for the ``ntp_client``. - For example, ntp service ``check_exe`` is - 'ntpd' because it runs the ntpd binary.""" - ), - }, - "packages": { - "type": "array", - "items": { - "type": "string", - }, - "uniqueItems": True, - "description": dedent( - """\ - List of packages needed to be installed for the - selected ``ntp_client``.""" - ), - }, - "service_name": { - "type": "string", - "description": dedent( - """\ - The systemd or sysvinit service name used to - start and stop the ``ntp_client`` - service.""" - ), - }, - "template": { - "type": "string", - "description": dedent( - """\ - Inline template allowing users to define their - own ``ntp_client`` configuration template. - The value must start with '## template:jinja' - to enable use of templating support. - """ - ), - }, - }, - # Don't use REQUIRED_NTP_CONFIG_KEYS to allow for override - # of builtin client values. - "minProperties": 1, # If we have config, define something - "additionalProperties": False, - }, - }, - "additionalProperties": False, - } - }, -} REQUIRED_NTP_CONFIG_KEYS = frozenset( ["check_exe", "confpath", "packages", "service_name"] ) -__doc__ = get_meta_doc(meta, schema) # Supplement python help() - - def distro_ntp_client_configs(distro): """Construct a distro-specific ntp client config dictionary by merging distro specific changes into base config. @@ -604,8 +477,6 @@ def handle(name, cfg, cloud, log, _args): " is a {_type} instead".format(_type=type_utils.obj_name(ntp_cfg)) ) - validate_cloudconfig_schema(cfg, schema) - # Allow users to explicitly enable/disable enabled = ntp_cfg.get("enabled", True) if util.is_false(enabled): diff --git a/cloudinit/config/cloud-init-schema.json b/cloudinit/config/cloud-init-schema.json index 7eb05919c82..2d77a843f84 100644 --- a/cloudinit/config/cloud-init-schema.json +++ b/cloudinit/config/cloud-init-schema.json @@ -1131,6 +1131,77 @@ } } }, + "cc_ntp": { + "type": "object", + "properties": { + "ntp": { + "type": ["null", "object"], + "properties": { + "pools": { + "type": "array", + "items": { + "type": "string", + "format": "hostname" + }, + "uniqueItems": true, + "description": "List of ntp pools. If both pools and servers are\nempty, 4 default pool servers will be provided of\nthe format ``{0-3}.{distro}.pool.ntp.org``. NOTE:\nfor Alpine Linux when using the Busybox NTP client\nthis setting will be ignored due to the limited\nfunctionality of Busybox's ntpd." + }, + "servers": { + "type": "array", + "items": { + "type": "string", + "format": "hostname" + }, + "uniqueItems": true, + "description": "List of ntp servers. If both pools and servers are\nempty, 4 default pool servers will be provided with\nthe format ``{0-3}.{distro}.pool.ntp.org``." + }, + "ntp_client": { + "type": "string", + "default": "auto", + "description": "Name of an NTP client to use to configure system NTP.\nWhen unprovided or 'auto' the default client preferred\nby the distribution will be used. The following\nbuilt-in client names can be used to override existing\nconfiguration defaults: chrony, ntp, ntpdate,\nsystemd-timesyncd." + }, + "enabled": { + "type": "boolean", + "default": true, + "description": "Attempt to enable ntp clients if set to True. If set\nto False, ntp client will not be configured or\ninstalled" + }, + "config": { + "description": "Configuration settings or overrides for the\n``ntp_client`` specified.", + "type": "object", + "properties": { + "confpath": { + "type": "string", + "description": "The path to where the ``ntp_client``\nconfiguration is written." + }, + "check_exe": { + "type": "string", + "description": "The executable name for the ``ntp_client``.\nFor example, ntp service ``check_exe`` is\n'ntpd' because it runs the ntpd binary." + }, + "packages": { + "type": "array", + "items": { + "type": "string" + }, + "uniqueItems": true, + "description": "List of packages needed to be installed for the\nselected ``ntp_client``." + }, + "service_name": { + "type": "string", + "description": "The systemd or sysvinit service name used to\nstart and stop the ``ntp_client``\nservice." + }, + "template": { + "type": "string", + "description": "Inline template allowing users to define their\nown ``ntp_client`` configuration template.\nThe value must start with '## template:jinja'\nto enable use of templating support.\n" + } + }, + "minProperties": 1, + "additionalProperties": false + } + }, + "additionalProperties": false + } + } + }, "cc_package_update_upgrade_install": { "type": "object", "properties": { @@ -2174,6 +2245,7 @@ { "$ref": "#/$defs/cc_mcollective" }, { "$ref": "#/$defs/cc_migrator" }, { "$ref": "#/$defs/cc_mounts" }, + { "$ref": "#/$defs/cc_ntp" }, { "$ref": "#/$defs/cc_package_update_upgrade_install" }, { "$ref": "#/$defs/cc_phone_home" }, { "$ref": "#/$defs/cc_power_state_change"}, diff --git a/tests/unittests/config/test_cc_ntp.py b/tests/unittests/config/test_cc_ntp.py index 0af1767095b..c2bce2a3faf 100644 --- a/tests/unittests/config/test_cc_ntp.py +++ b/tests/unittests/config/test_cc_ntp.py @@ -1,12 +1,20 @@ # This file is part of cloud-init. See LICENSE file for license information. import copy import os +import re import shutil from functools import partial from os.path import dirname +import pytest + from cloudinit import helpers, util from cloudinit.config import cc_ntp +from cloudinit.config.schema import ( + SchemaValidationError, + get_schema, + validate_cloudconfig_schema, +) from tests.unittests.helpers import ( CiTestCase, FilesystemMockingTestCase, @@ -389,121 +397,6 @@ def test_ntp_handler_schema_validation_allows_empty_ntp_config( "Invalid cloud-config provided:", self.logs.getvalue() ) - @skipUnlessJsonSchema() - @mock.patch("cloudinit.config.cc_ntp.select_ntp_client") - def test_ntp_handler_schema_validation_warns_non_string_item_type( - self, m_sel - ): - """Ntp schema validation warns of non-strings in pools or servers. - - Schema validation is not strict, so ntp config is still be rendered. - """ - invalid_config = { - "ntp": {"pools": [123], "servers": ["www.example.com", None]} - } - for distro in cc_ntp.distros: - mycloud = self._get_cloud(distro) - ntpconfig = self._mock_ntp_client_config(distro=distro) - confpath = ntpconfig["confpath"] - m_sel.return_value = ntpconfig - cc_ntp.handle("cc_ntp", invalid_config, mycloud, None, []) - self.assertIn( - "Invalid cloud-config provided:\nntp.pools.0: 123 is not of" - " type 'string'\nntp.servers.1: None is not of type 'string'", - self.logs.getvalue(), - ) - self.assertEqual( - "servers ['www.example.com', None]\npools [123]\n", - util.load_file(confpath), - ) - - @skipUnlessJsonSchema() - @mock.patch("cloudinit.config.cc_ntp.select_ntp_client") - def test_ntp_handler_schema_validation_warns_of_non_array_type( - self, m_select - ): - """Ntp schema validation warns of non-array pools or servers types. - - Schema validation is not strict, so ntp config is still be rendered. - """ - invalid_config = {"ntp": {"pools": 123, "servers": "non-array"}} - - for distro in cc_ntp.distros: - mycloud = self._get_cloud(distro) - ntpconfig = self._mock_ntp_client_config(distro=distro) - confpath = ntpconfig["confpath"] - m_select.return_value = ntpconfig - cc_ntp.handle("cc_ntp", invalid_config, mycloud, None, []) - self.assertIn( - "Invalid cloud-config provided:\nntp.pools: 123 is not of type" - " 'array'\nntp.servers: 'non-array' is not of type 'array'", - self.logs.getvalue(), - ) - self.assertEqual( - "servers non-array\npools 123\n", util.load_file(confpath) - ) - - @skipUnlessJsonSchema() - @mock.patch("cloudinit.config.cc_ntp.select_ntp_client") - def test_ntp_handler_schema_validation_warns_invalid_key_present( - self, m_select - ): - """Ntp schema validation warns of invalid keys present in ntp config. - - Schema validation is not strict, so ntp config is still be rendered. - """ - invalid_config = { - "ntp": {"invalidkey": 1, "pools": ["0.mycompany.pool.ntp.org"]} - } - for distro in cc_ntp.distros: - if distro != "alpine": - mycloud = self._get_cloud(distro) - ntpconfig = self._mock_ntp_client_config(distro=distro) - confpath = ntpconfig["confpath"] - m_select.return_value = ntpconfig - cc_ntp.handle("cc_ntp", invalid_config, mycloud, None, []) - self.assertIn( - "Invalid cloud-config provided:\nntp: Additional" - " properties are not allowed ('invalidkey' was" - " unexpected)", - self.logs.getvalue(), - ) - self.assertEqual( - "servers []\npools ['0.mycompany.pool.ntp.org']\n", - util.load_file(confpath), - ) - - @skipUnlessJsonSchema() - @mock.patch("cloudinit.config.cc_ntp.select_ntp_client") - def test_ntp_handler_schema_validation_warns_of_duplicates(self, m_select): - """Ntp schema validation warns of duplicates in servers or pools. - - Schema validation is not strict, so ntp config is still be rendered. - """ - invalid_config = { - "ntp": { - "pools": ["0.mypool.org", "0.mypool.org"], - "servers": ["10.0.0.1", "10.0.0.1"], - } - } - for distro in cc_ntp.distros: - mycloud = self._get_cloud(distro) - ntpconfig = self._mock_ntp_client_config(distro=distro) - confpath = ntpconfig["confpath"] - m_select.return_value = ntpconfig - cc_ntp.handle("cc_ntp", invalid_config, mycloud, None, []) - self.assertIn( - "Invalid cloud-config provided:\nntp.pools: ['0.mypool.org'," - " '0.mypool.org'] has non-unique elements\nntp.servers: " - "['10.0.0.1', '10.0.0.1'] has non-unique elements", - self.logs.getvalue(), - ) - self.assertEqual( - "servers ['10.0.0.1', '10.0.0.1']\n" - "pools ['0.mypool.org', '0.mypool.org']\n", - util.load_file(confpath), - ) - @mock.patch("cloudinit.config.cc_ntp.select_ntp_client") def test_ntp_handler_timesyncd(self, m_select): """Test ntp handler configures timesyncd""" @@ -869,4 +762,59 @@ def test_error_on_non_string_values(self): self.assertIn(error, error_msg) +class TestNTPSchema: + @pytest.mark.parametrize( + "config, error_msg", + ( + # Allow empty ntp config + ({"ntp": None}, None), + ( + { + "ntp": { + "invalidkey": 1, + "pools": ["0.mycompany.pool.ntp.org"], + } + }, + re.escape( + "ntp: Additional properties are not allowed ('invalidkey'" + ), + ), + ( + { + "ntp": { + "pools": ["0.mypool.org", "0.mypool.org"], + "servers": ["10.0.0.1", "10.0.0.1"], + } + }, + re.escape( + "ntp.pools: ['0.mypool.org', '0.mypool.org'] has" + " non-unique elements" + ), + ), + ( + { + "ntp": { + "pools": [123], + "servers": ["www.example.com", None], + } + }, + "ntp.pools.0: 123 is not of type 'string'.*" + "ntp.servers.1: None is not of type 'string'", + ), + ( + {"ntp": {"pools": 123, "servers": "non-array"}}, + "ntp.pools: 123 is not of type 'array'.*" + "ntp.servers: 'non-array' is not of type 'array'", + ), + ), + ) + @skipUnlessJsonSchema() + def test_schema_validation(self, config, error_msg): + if error_msg is None: + validate_cloudconfig_schema(config, get_schema(), strict=True) + else: + with pytest.raises(SchemaValidationError, match=error_msg): + validate_cloudconfig_schema(config, get_schema(), strict=True) + + # vi: ts=4 expandtab diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 19ef0f484be..a367cda1c25 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -183,6 +183,7 @@ def test_get_schema_coalesces_known_schema(self): {"$ref": "#/$defs/cc_mcollective"}, {"$ref": "#/$defs/cc_migrator"}, {"$ref": "#/$defs/cc_mounts"}, + {"$ref": "#/$defs/cc_ntp"}, {"$ref": "#/$defs/cc_package_update_upgrade_install"}, {"$ref": "#/$defs/cc_phone_home"}, {"$ref": "#/$defs/cc_power_state_change"}, @@ -221,10 +222,9 @@ def test_get_schema_coalesces_known_schema(self): legacy_schema_keys.extend(subschema["properties"].keys()) assert expected_subschema_defs == found_subschema_defs - # This list will dwindle as we move legacy schema to new $defs - assert [ - "ntp", - ] == sorted(legacy_schema_keys) + # This list should remain empty unless we induct new modules with + # legacy schema attributes defined within the cc_module. + assert [] == sorted(legacy_schema_keys) class TestLoadDoc: