From e0c67258726b76f1b52023e6d657df462bb49f1a Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 5 Jan 2022 12:45:29 -0700 Subject: [PATCH 01/20] schema: perform validate_cloudcfg_schema once in early boot Validate full schema one time early in boot just after user-data is processed. This avoid 51+ repeated calls to validate_cloudcfg_schema on subsections of the schema. Moving validation to one place against the whole schema will allow for a single concise error/warning about schema violations in one place in logs. --- cloudinit/cmd/main.py | 7 + cloudinit/config/schema.py | 10 +- config/cloud-init-schema-1.0.json | 503 ++++++++++++++++++++++++++ tests/unittests/config/test_schema.py | 62 ++-- 4 files changed, 557 insertions(+), 25 deletions(-) create mode 100644 config/cloud-init-schema-1.0.json diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py index e67edbc35c7..c9be41b3496 100644 --- a/cloudinit/cmd/main.py +++ b/cloudinit/cmd/main.py @@ -22,6 +22,7 @@ patcher.patch_logging() +from cloudinit.config.schema import validate_cloudconfig_schema from cloudinit import log as logging from cloudinit import netinfo from cloudinit import signal_handler @@ -474,6 +475,12 @@ def main_init(name, args): util.logexc(LOG, "Consuming user data failed!") return (init.datasource, ["Consuming user data failed!"]) + # Validate user-data adheres to schema definition + if os.path.exists(init.paths.get_ipath_cur("userdata_raw")): + validate_cloudconfig_schema(config=init.cfg, strict=False) + else: + LOG.debug("Skipping user-data validation. No user-data found.") + apply_reporting_cfg(init.cfg) # Stage 8 - re-read and apply relevant cloud-config to include user-data diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index f54cf18facf..c8f9f34f536 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -166,14 +166,18 @@ def validate_cloudconfig_metaschema(validator, schema: dict, throw=True): def validate_cloudconfig_schema( - config: dict, schema: dict, strict=False, strict_metaschema=False + config: dict, + schema: dict = None, + strict: bool = False, + strict_metaschema: bool = False, ): """Validate provided config meets the schema definition. @param config: Dict of cloud configuration settings validated against schema. Ignored if strict_metaschema=True @param schema: jsonschema dict describing the supported schema definition - for the cloud config module (config.cc_*). + for the cloud config module (config.cc_*). If None, validate against + global schema. @param strict: Boolean, when True raise SchemaValidationErrors instead of logging warnings. @param strict_metaschema: Boolean, when True validates schema using strict @@ -183,6 +187,8 @@ def validate_cloudconfig_schema( against the provided schema. @raises: RuntimeError when provided config sourced from YAML is not a dict. """ + if schema is None: + schema = get_schema() try: (cloudinitValidator, FormatChecker) = get_jsonschema_validator() if strict_metaschema: diff --git a/config/cloud-init-schema-1.0.json b/config/cloud-init-schema-1.0.json new file mode 100644 index 00000000000..97169783c75 --- /dev/null +++ b/config/cloud-init-schema-1.0.json @@ -0,0 +1,503 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "$defs": { + "apt_configure.mirror": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["arches"], + "properties": { + "arches": { + "type": "array", + "items": {"type": "string"}, + "minItems": 1 + }, + "uri": {"type": "string", "format": "uri"}, + "search": { + "type": "array", + "items": {"type": "string", "format": "uri"}, + "minItems": 1 + }, + "search_dns": { + "type": "boolean" + }, + "keyid": {"type": "string"}, + "key": {"type": "string"}, + "keyserver": {"type": "string"} + } + } + }, + "cc_apk_configure": { + "type": "object", + "properties": { + "apk_repos": { + "type": "object", + "properties": { + "preserve_repositories": { + "type": "boolean", + "default": false, + "description": "By default, cloud-init will generate a new repositories file ``/etc/apk/repositories`` based on any valid configuration settings specified within a apk_repos section of cloud config. To disable this behavior and preserve the repositories file from the pristine image, set ``preserve_repositories`` to ``true``.\n\n The ``preserve_repositories`` option overrides all other config keys that would alter ``/etc/apk/repositories``." + }, + "alpine_repo": { + "type": ["object", "null"], + "properties": { + "base_url": { + "type": "string", + "default": "https://alpine.global.ssl.fastly.net/alpine", + "description": "The base URL of an Alpine repository, or mirror, to download official packages from. If not specified then it defaults to ``https://alpine.global.ssl.fastly.net/alpine``" + }, + "community_enabled": { + "type": "boolean", + "default": false, + "description": "Whether to add the Community repo to the repositories file. By default the Community repo is not included." + }, + "testing_enabled": { + "type": "boolean", + "default": false, + "description": "Whether to add the Testing repo to the repositories file. By default the Testing repo is not included. It is only recommended to use the Testing repo on a machine running the ``Edge`` version of Alpine as packages installed from Testing may have dependancies that conflict with those in non-Edge Main or Community repos." + }, + "version": { + "type": "string", + "description": "The Alpine version to use (e.g. ``v3.12`` or ``edge``)" + } + }, + "required": ["version"], + "minProperties": 1, + "additionalProperties": false + }, + "local_repo_base_url": { + "type": "string", + "description": "The base URL of an Alpine repository containing unofficial packages" + } + }, + "minProperties": 1, + "additionalProperties": false + } + } + }, + "cc_apt_configure": { + "properties": { + "apt": { + "type": "object", + "additionalProperties": false, + "properties": { + "preserve_sources_list": { + "type": "boolean", + "default": false, + "description": "By default, cloud-init will generate a new sources list in ``/etc/apt/sources.list.d`` based on any changes specified in cloud config. To disable this behavior and preserve the sources list from the pristine image, set ``preserve_sources_list`` to ``true``.\n\nThe ``preserve_sources_list`` option overrides all other config keys that would alter ``sources.list`` or ``sources.list.d``, **except** for additional sources to be added to ``sources.list.d``." + }, + "disable_suites": { + "type": "array", + "items": {"type": "string"}, + "uniqueItems": true, + "description": "Entries in the sources list can be disabled using ``disable_suites``, which takes a list of suites to be disabled. If the string ``$RELEASE`` is present in a suite in the ``disable_suites`` list, it will be replaced with the release name. If a suite specified in ``disable_suites`` is not present in ``sources.list`` it will be ignored. For convenience, several aliases are provided for`` disable_suites``:\n\n - ``updates`` => ``$RELEASE-updates``\n - ``backports`` => ``$RELEASE-backports``\n - ``security`` => ``$RELEASE-security``\n - ``proposed`` => ``$RELEASE-proposed``\n - ``release`` => ``$RELEASE``.\n\nWhen a suite is disabled using ``disable_suites``, its entry in ``sources.list`` is not deleted; it is just commented out." + }, + "primary": { + "$ref": "#/$defs/apt_configure.mirror", + "description": "The primary and security archive mirrors can be specified using the ``primary`` and ``security`` keys, respectively. Both the ``primary`` and ``security`` keys take a list of configs, allowing mirrors to be specified on a per-architecture basis. Each config is a dictionary which must have an entry for ``arches``, specifying which architectures that config entry is for. The keyword ``default`` applies to any architecture not explicitly listed. The mirror url can be specified with the ``uri`` key, or a list of mirrors to check can be provided in order, with the first mirror that can be resolved being selected. This allows the same configuration to be used in different environment, with different hosts used for a local APT mirror. If no mirror is provided by ``uri`` or ``search``, ``search_dns`` may be used to search for dns names in the format ``-mirror`` in each of the following:\n\n - fqdn of this host per cloud metadata,\n - localdomain,\n - domains listed in ``/etc/resolv.conf``.\n\nIf there is a dns entry for ``-mirror``, then it is assumed that there is a distro mirror at ``http://-mirror./``. If the ``primary`` key is defined, but not the ``security`` key, then then configuration for ``primary`` is also used for ``security``. If ``search_dns`` is used for the ``security`` key, the search pattern will be ``-security-mirror``.\n\nEach mirror may also specify a key to import via any of the following optional keys:\n\n - ``keyid``: a key to import via shortid or fingerprint.\n - ``key``: a raw PGP key.\n - ``keyserver``: alternate keyserver to pull ``keyid`` key from.\n\nIf no mirrors are specified, or all lookups fail, then default mirrors defined in the datasource are used. If none are present in the datasource either the following defaults are used:\n\n - ``primary`` => ``http://archive.ubuntu.com/ubuntu``.\n - ``security`` => ``http://security.ubuntu.com/ubuntu``" + }, + "security": { + "$ref": "#/$defs/apt_configure.mirror", + "description": "Please refer to the primary config documentation" + }, + "add_apt_repo_match": { + "type": "string", + "default": "^[\\w-]+:\\w", + "description": "All source entries in ``apt-sources`` that match regex in ``add_apt_repo_match`` will be added to the system using ``add-apt-repository``. If ``add_apt_repo_match`` is not specified, it defaults to ``^[\\w-]+:\\w``" + }, + "debconf_selections": { + "type": "object", + "items": {"type": "string"}, + "description": "Debconf additional configurations can be specified as a dictionary under the ``debconf_selections`` config key, with each key in the dict representing a different set of configurations. The value of each key must be a string containing all the debconf configurations that must be applied. We will bundle all of the values and pass them to ``debconf-set-selections``. Therefore, each value line must be a valid entry for ``debconf-set-selections``, meaning that they must possess for distinct fields:\n\n``pkgname question type answer``\n\nWhere:\n\n - ``pkgname`` is the name of the package.\n - ``question`` the name of the questions.\n - ``type`` is the type of question.\n - ``answer`` is the value used to answer the question.\n\nFor example: ``ippackage ippackage/ip string 127.0.01``" + }, + "sources_list": { + "type": "string", + "description": "Specifies a custom template for rendering ``sources.list`` . If no ``sources_list`` template is given, cloud-init will use sane default. Within this template, the following strings will be replaced with the appropriate values:\n\n - ``$MIRROR``\n - ``$RELEASE``\n - ``$PRIMARY``\n - ``$SECURITY``\n - ``$KEY_FILE``" + }, + "conf": { + "type": "string", + "description": "Specify configuration for apt, such as proxy configuration. This configuration is specified as a string. For multiline APT configuration, make sure to follow yaml syntax." + }, + "https_proxy": { + "type": "string", + "description": "More convenient way to specify https APT proxy. https proxy url is specified in the format ``https://[[user][:pass]@]host[:port]/``." + }, + "http_proxy": { + "type": "string", + "description": "More convenient way to specify http APT proxy. http proxy url is specified in the format ``http://[[user][:pass]@]host[:port]/``." + }, + "proxy": { + "type": "string", + "description": "Alias for defining a http APT proxy." + }, + "ftp_proxy": { + "type": "string", + "description": "More convenient way to specify ftp APT proxy. ftp proxy url is specified in the format ``ftp://[[user][:pass]@]host[:port]/``." + }, + "sources": { + "type": "object", + "items": {"type": "string"}, + "description": "Source list entries can be specified as a dictionary under the ``sources`` config key, with each key in the dict representing a different source file. The key of each source entry will be used as an id that can be referenced in other config entries, as well as the filename for the source's configuration under ``/etc/apt/sources.list.d``. If the name does not end with ``.list``, it will be appended. If there is no configuration for a key in ``sources``, no file will be written, but the key may still be referred to as an id in other ``sources`` entries.\n\nEach entry under ``sources`` is a dictionary which may contain any of the following optional keys:\n - ``source``: a sources.list entry (some variable replacements apply).\n - ``keyid``: a key to import via shortid or fingerprint.\n - ``key``: a raw PGP key.\n - ``keyserver``: alternate keyserver to pull ``keyid`` key from.\n - ``filename``: specify the name of the list file\n\nThe ``source`` key supports variable replacements for the following strings:\n\n - ``$MIRROR``\n - ``$PRIMARY``\n - ``$SECURITY``\n - ``$RELEASE``\n - ``$KEY_FILE``" + } + } + } + } + }, + "cc_apt_pipelining": { + "type": "object", + "properties": { + "apt_pipelining": { + "type": ["integer", "boolean", "string"], + "oneOf": [ + {"type": "integer"}, + {"type": "boolean"}, + {"type": "string", "enum": ["none", "unchanged", "os"]} + ] + } + } + }, + "cc_bootcmd": { + "type": "object", + "properties": { + "bootcmd": { + "type": "array", + "items": { + "oneOf": [ + {"type": "array", "items": {"type": "string"}}, + {"type": "string"} + ] + }, + "additionalItems": false, + "minItems": 1 + } + } + }, + "cc_byobu": { + "type": "object", + "properties": { + "byobu_by_default": { + "type": "string", + "enum": [ + "enable-system", + "enable-user", + "disable-system", + "disable-user", + "enable", + "disable", + "user", + "system" + ] + } + } + }, + "cc_ca_certs": { + "type": "object", + "properties": { + "ca-certs": { + "type": "object", + "properties": { + "remove-defaults": { + "description": "Remove default CA certificates if true. Default: false", + "type": "boolean" + }, + "trusted": { + "description": "List of trusted CA certificates to add.", + "type": "array", + "items": {"type": "string"} + } + }, + "additionalProperties": false + } + } + }, + "cc_chef": { + "type": "object", + "properties": { + "chef": { + "type": "object", + "additionalProperties": false, + "properties": { + "directories": { + "type": "array", + "items": {"type": "string"}, + "uniqueItems": true, + "description": "Create the necessary directories for chef to run. By default, it creates the following directories:\n\n - ``/etc/chef``\n - ``/var/log/chef``\n - ``/var/lib/chef``\n - ``/var/cache/chef``\n - ``/var/backups/chef``\n - ``/var/run/chef``" + }, + "validation_cert": { + "type": "string", + "description": "Optional string to be written to file validation_key. Special value ``system`` means set use existing file." + }, + "validation_key": { + "type": "string", + "default": "/etc/chef/validation.pem", + "description": "Optional path for validation_cert. default to ``/etc/chef/validation.pem``" + }, + "firstboot_path": { + "type": "string", + "default": "/etc/chef/firstboot.json", + "description": "Path to write run_list and initial_attributes keys that should also be present in this configuration, defaults to ``/etc/chef/firstboot.json``" + }, + "exec": { + "type": "boolean", + "default": false, + "description": "Set true if we should run or not run chef (defaults to false, unless a gem installed is requested where this will then default to true)." + }, + "client_key": { + "type": "string", + "default": "/etc/chef/client.pem", + "description": "Optional path for client_cert. Default to ``/etc/chef/client.pem``." + }, + "encrypted_data_bag_secret": { + "type": "string", + "default": null, + "description": "Specifies the location of the secret key used by chef to encrypt data items. By default, this path is set to null, meaning that chef will have to look at the path ``/etc/chef/encrypted_data_bag_secret`` for it." + }, + "environment": { + "type": "string", + "default": "_default", + "description": "Specifies which environment chef will use. By default, it will use the ``_default`` configuration." + }, + "file_backup_path": { + "type": "string", + "default": "/var/backups/chef", + "description": "Specifies the location in which backup files are stored. By default, it uses the ``/var/backups/chef`` location." + }, + "file_cache_path": { + "type": "string", + "default": "/var/cache/chef", + "description": "Specifies the location in which chef cache files will be saved. By default, it uses the ``/var/cache/chef`` location." + }, + "json_attribs": { + "type": "string", + "default": "/etc/chef/firstboot.json", + "description": "Specifies the location in which some chef json data is stored. By default, it uses the ``/etc/chef/firstboot.json`` location." + }, + "log_level": { + "type": "string", + "default": ":info", + "description": "Defines the level of logging to be stored in the log file. By default this value is set to ``:info``." + }, + "log_location": { + "type": "string", + "default": "/var/log/chef/client.log", + "description": "Specifies the location of the chef lof file. By default, the location is specified at ``/var/log/chef/client.log``." + }, + "node_name": { + "type": "string", + "description": "The name of the node to run. By default, we will use th instance id as the node name." + }, + "omnibus_url": { + "type": "string", + "default": "https://www.chef.io/chef/install.sh", + "description": "Omnibus URL if chef should be installed through Omnibus. By default, it uses the ``https://www.chef.io/chef/install.sh``." + }, + "omnibus_url_retries": { + "type": "integer", + "default": 5, + "description": "The number of retries that will be attempted to reach the Omnibus URL. Default is 5." + }, + "omnibus_version": { + "type": "string", + "description": "Optional version string to require for omnibus install." + }, + "pid_file": { + "type": "string", + "default": "/var/run/chef/client.pid", + "description": "The location in which a process identification number (pid) is saved. By default, it saves in the ``/var/run/chef/client.pid`` location." + }, + "server_url": { + "type": "string", + "description": "The URL for the chef server" + }, + "show_time": { + "type": "boolean", + "default": true, + "description": "Show time in chef logs" + }, + "ssl_verify_mode": { + "type": "string", + "default": ":verify_none", + "description": "Set the verify mode for HTTPS requests. We can have two possible values for this parameter:\n\n - ``:verify_none``: No validation of SSL certificates.\n - ``:verify_peer``: Validate all SSL certificates.\n\nBy default, the parameter is set as ``:verify_none``." + }, + "validation_name": { + "type": "string", + "description": "The name of the chef-validator key that Chef Infra Client uses to access the Chef Infra Server during the initial Chef Infra Client run." + }, + "force_install": { + "type": "boolean", + "default": false, + "description": "If set to ``true``, forces chef installation, even if it is already installed." + }, + "initial_attributes": { + "type": "object", + "items": {"type": "string"}, + "description": "Specify a list of initial attributes used by the cookbooks." + }, + "install_type": { + "type": "string", + "default": "packages", + "description": "The type of installation for chef. It can be one of the following values:\n\n - ``packages``\n - ``gems``\n - ``omnibus``" + }, + "run_list": { + "type": "array", + "items": {"type": "string"}, + "description": "A run list for a first boot json." + }, + "chef_license": { + "type": "string", + "description": "string that indicates if user accepts or not license related to some of chef products" + } + } + } + } + }, + "cc_debug": { + "type": "object", + "properties": { + "debug": { + "additionalProperties": false, + "type": "object", + "properties": { + "verbose": { + "description": "Should always be true for this module", + "type": "boolean" + }, + "output": { + "description": "Location to write output. Defaults to console + log", + "type": "string" + } + } + } + } + }, + "cc_disable_ec2_metadata": { + "type": "object", + "properties": { + "disable_ec2_metadata": { + "default": false, + "description": "Set true to disable IPv4 routes to EC2 metadata. Default: false.", + "type": "boolean" + } + } + }, + "cc_disk_setup": { + "type": "object", + "properties": { + "device_aliases": { + "type": "object", + "patternProperties": { + "^.+$": { + "label": "", + "type": "string", + "description": "Path to disk to be aliased by this name." + } + } + }, + "disk_setup": { + "type": "object", + "patternProperties": { + "^.+$": { + "label": "", + "type": "object", + "additionalProperties": false, + "properties": { + "table_type": { + "type": "string", + "enum": ["mbr", "gpt"], + "description": "Specifies the partition table type, either ``mbr`` or ``gpt``. Default: ``mbr``." + }, + "layout": { + "type": ["string", "boolean", "array"], + "oneOf": [ + {"type": "string", "enum": ["auto", "remove"]}, + {"type": "boolean"}, + { + "type": "array", + "items": { + "oneOf": [ + {"type": "integer"}, + { + "type": "array", + "items": {"type": "integer"} + } + ] + } + } + ], + "description": "If set to ``true``, a single partition using all the space on the device will be created. If set to ``false``, no partitions will be created. Partitions can be specified by providing a list to ``layout``, where each entry in the list is either a size or a list containing a size and the numerical value for a partition type. The size for partitions is specified in **percentage** of disk space, not in bytes (e.g. a size of 33 would take up 1/3 of the disk space). Default: ``false``." + }, + "overwrite": { + "type": "boolean", + "description": "Controls whether this module tries to be safe about writing partition tables or not. If ``overwrite: false`` is set, the device will be checked for a partition table and for a file system and if either is found, the operation will be skipped. If ``overwrite: true`` is set, no checks will be performed. Using ``overwrite: true`` is **dangerous** and can lead to data loss, so double check that the correct device has been specified if using this option. Default: ``false``" + } + } + } + } + }, + "fs_setup": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "label": { + "type": "string", + "description": "Label for the filesystem." + }, + "filesystem": { + "type": "string", + "description": "Filesystem type to create. E.g., ``ext4`` or ``btrfs``" + }, + "device": { + "type": "string", + "description": "Specified either as a path or as an alias in the format ``.`` where ```` denotes the partition number on the device. If specifying device using the ``.`` format, the value of ``partition`` will be overwritten." + }, + "partition": { + "type": ["string", "integer"], + "oneOf": [ + { + "type": "string", + "enum": ["auto", "any", "none"] + }, + {"type": "integer"} + ], + "description": "The partition can be specified by setting ``partition`` to the desired partition number. The ``partition`` option may also be set to ``auto``, in which this module will search for the existence of a filesystem matching the ``label``, ``type`` and ``device`` of the ``fs_setup`` entry and will skip creating the filesystem if one is found. The ``partition`` option may also be set to ``any``, in which case any file system that matches ``type`` and ``device`` will cause this module to skip filesystem creation for the ``fs_setup`` entry, regardless of ``label`` matching or not. To write a filesystem directly to a device, use ``partition: none``. ``partition: none`` will **always** write the filesystem, even when the ``label`` and ``filesystem`` are matched, and ``overwrite`` is ``false``." + }, + "overwrite": { + "type": "boolean", + "description": "If ``true``, overwrite any existing filesystem. Using ``overwrite: true`` for filesystems is **dangerous** and can lead to data loss, so double check the entry in ``fs_setup``. Default: ``false``" + }, + "replace_fs": { + "type": "string", + "description": "Ignored unless ``partition`` is ``auto`` or ``any``. Default ``false``." + }, + "extra_opts": { + "type": ["array", "string"], + "items": {"type": "string"}, + "description": "Optional options to pass to the filesystem creation command. Ignored if you using ``cmd`` directly." + }, + "cmd": { + "type": ["array", "string"], + "items": {"type": "string"}, + "description": "Optional command to run to create the filesystem. Can include string substitutions of the other ``fs_setup`` config keys. This is only necessary if you need to override the default command." + } + } + } + } + } + } + }, + "allOf": [ + { "$ref": "#/$defs/cc_apk_configure" }, + { "$ref": "#/$defs/cc_apt_configure" }, + { "$ref": "#/$defs/cc_apt_pipelining" }, + { "$ref": "#/$defs/cc_bootcmd" }, + { "$ref": "#/$defs/cc_byobu" }, + { "$ref": "#/$defs/cc_ca_certs" }, + { "$ref": "#/$defs/cc_chef" }, + { "$ref": "#/$defs/cc_debug" }, + { "$ref": "#/$defs/cc_disable_ec2_metadata" }, + { "$ref": "#/$defs/cc_disk_setup" } + ] +} diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 93206bddf47..0d942bfc053 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -129,69 +129,85 @@ def test_schema_validation_error_expects_schema_errors(self): self.assertTrue(isinstance(exception, ValueError)) -class ValidateCloudConfigSchemaTest(CiTestCase): +class TestValidateCloudConfigSchema: """Tests for validate_cloudconfig_schema.""" with_logs = True + @pytest.mark.parametrize( + "schema, call_count", + ((None, 1), ({"properties": {"p1": {"type": "string"}}}, 0)), + ) @skipUnlessJsonSchema() - def test_validateconfig_schema_non_strict_emits_warnings(self): + @mock.patch("cloudinit.config.schema.get_schema") + def test_validateconfig_schema_use_full_schema_when_no_schema_param( + self, get_schema, schema, call_count + ): + """Use full schema when schema param is absent.""" + get_schema.return_value = {"properties": {"p1": {"type": "string"}}} + kwargs = {"config": {"p1": "valid"}} + if schema: + kwargs["schema"] = schema + validate_cloudconfig_schema(**kwargs) + assert call_count == get_schema.call_count + + @skipUnlessJsonSchema() + def test_validateconfig_schema_non_strict_emits_warnings(self, caplog): """When strict is False validate_cloudconfig_schema emits warnings.""" schema = {"properties": {"p1": {"type": "string"}}} validate_cloudconfig_schema({"p1": -1}, schema, strict=False) - self.assertIn( - "Invalid config:\np1: -1 is not of type 'string'\n", - self.logs.getvalue(), + assert "Invalid config:\np1: -1 is not of type 'string'\n" in ( + caplog.text ) @skipUnlessJsonSchema() - def test_validateconfig_schema_emits_warning_on_missing_jsonschema(self): + def test_validateconfig_schema_emits_warning_on_missing_jsonschema( + self, caplog + ): """Warning from validate_cloudconfig_schema when missing jsonschema.""" schema = {"properties": {"p1": {"type": "string"}}} with mock.patch.dict("sys.modules", **{"jsonschema": ImportError()}): validate_cloudconfig_schema({"p1": -1}, schema, strict=True) - self.assertIn( - "Ignoring schema validation. jsonschema is not present", - self.logs.getvalue(), + assert "Ignoring schema validation. jsonschema is not present" in ( + caplog.text ) @skipUnlessJsonSchema() def test_validateconfig_schema_strict_raises_errors(self): """When strict is True validate_cloudconfig_schema raises errors.""" schema = {"properties": {"p1": {"type": "string"}}} - with self.assertRaises(SchemaValidationError) as context_mgr: + with pytest.raises(SchemaValidationError) as context_mgr: validate_cloudconfig_schema({"p1": -1}, schema, strict=True) - self.assertEqual( - "Cloud config schema errors: p1: -1 is not of type 'string'", - str(context_mgr.exception), + assert"Cloud config schema errors: p1: -1 is not of type 'string'" == ( + str(context_mgr.value) ) @skipUnlessJsonSchema() def test_validateconfig_schema_honors_formats(self): """With strict True, validate_cloudconfig_schema errors on format.""" schema = {"properties": {"p1": {"type": "string", "format": "email"}}} - with self.assertRaises(SchemaValidationError) as context_mgr: + with pytest.raises(SchemaValidationError) as context_mgr: validate_cloudconfig_schema({"p1": "-1"}, schema, strict=True) - self.assertEqual( - "Cloud config schema errors: p1: '-1' is not a 'email'", - str(context_mgr.exception), + assert "Cloud config schema errors: p1: '-1' is not a 'email'" == ( + str(context_mgr.value) ) @skipUnlessJsonSchema() def test_validateconfig_schema_honors_formats_strict_metaschema(self): """With strict and strict_metaschema True, ensure errors on format""" schema = {"properties": {"p1": {"type": "string", "format": "email"}}} - with self.assertRaises(SchemaValidationError) as context_mgr: + with pytest.raises(SchemaValidationError) as context_mgr: validate_cloudconfig_schema( {"p1": "-1"}, schema, strict=True, strict_metaschema=True ) - self.assertEqual( - "Cloud config schema errors: p1: '-1' is not a 'email'", - str(context_mgr.exception), + assert "Cloud config schema errors: p1: '-1' is not a 'email'" == str( + context_mgr.value ) @skipUnlessJsonSchema() - def test_validateconfig_strict_metaschema_do_not_raise_exception(self): + def test_validateconfig_strict_metaschema_do_not_raise_exception( + self, caplog + ): """With strict_metaschema=True, do not raise exceptions. This flag is currently unused, but is intended for run-time validation. @@ -203,7 +219,7 @@ def test_validateconfig_strict_metaschema_do_not_raise_exception(self): ) assert ( "Meta-schema validation failed, attempting to validate config" - in self.logs.getvalue() + in caplog.text ) From feb5c5ee294b3e7cb34ff8ecc0d6b5a6c7fcde68 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 7 Jan 2022 12:52:41 -0700 Subject: [PATCH 02/20] schema: deliver single JSON schema file and Paths.schema_dir - Define single JSON schema file to allow for validation of full schema for provided user-data. - Package schema file in /etc/cloud/cloud-init-1.0.json - Add Paths.schema_dir and unpickle upgrade test handling --- cloudinit/helpers.py | 7 +- cloudinit/settings.py | 1 + config/cloud-init-schema-1.0.json | 485 +----------------------------- setup.py | 1 + tests/unittests/test_upgrade.py | 3 + 5 files changed, 12 insertions(+), 485 deletions(-) diff --git a/cloudinit/helpers.py b/cloudinit/helpers.py index c2c9e5842bc..263dfe0cf45 100644 --- a/cloudinit/helpers.py +++ b/cloudinit/helpers.py @@ -328,13 +328,14 @@ def items(self): class Paths(persistence.CloudInitPickleMixin): - _ci_pkl_version = 1 + _ci_pkl_version = 2 def __init__(self, path_cfgs, ds=None): self.cfgs = path_cfgs # Populate all the initial paths self.cloud_dir = path_cfgs.get("cloud_dir", "/var/lib/cloud") self.run_dir = path_cfgs.get("run_dir", "/run/cloud-init") + self.schema_dir = path_cfgs.get("schema_dir", "/etc/cloud/schema") self.instance_link = os.path.join(self.cloud_dir, "instance") self.boot_finished = os.path.join(self.instance_link, "boot-finished") self.upstart_conf_d = path_cfgs.get("upstart_dir") @@ -377,6 +378,10 @@ def _unpickle(self, ci_pkl_version: int) -> None: self.run_dir = Paths( path_cfgs=self.cfgs, ds=self.datasource ).run_dir + if not hasattr(self, "schema_dir"): + self.schema_dir = Paths( + path_cfgs=self.cfgs, ds=self.datasource + ).schema_dir # get_ipath_cur: get the current instance path for an item def get_ipath_cur(self, name=None): diff --git a/cloudinit/settings.py b/cloudinit/settings.py index ecc1403bd9b..39067542ea2 100644 --- a/cloudinit/settings.py +++ b/cloudinit/settings.py @@ -53,6 +53,7 @@ "syslog_fix_perms": ["syslog:adm", "root:adm", "root:wheel", "root:root"], "system_info": { "paths": { + "schema_dir": "/etc/cloud/schema", "cloud_dir": "/var/lib/cloud", "templates_dir": "/etc/cloud/templates/", }, diff --git a/config/cloud-init-schema-1.0.json b/config/cloud-init-schema-1.0.json index 97169783c75..b2e8bd21ff9 100644 --- a/config/cloud-init-schema-1.0.json +++ b/config/cloud-init-schema-1.0.json @@ -1,154 +1,10 @@ { "$schema": "http://json-schema.org/draft-04/schema#", "$defs": { - "apt_configure.mirror": { - "type": "array", - "items": { - "type": "object", - "additionalProperties": false, - "required": ["arches"], - "properties": { - "arches": { - "type": "array", - "items": {"type": "string"}, - "minItems": 1 - }, - "uri": {"type": "string", "format": "uri"}, - "search": { - "type": "array", - "items": {"type": "string", "format": "uri"}, - "minItems": 1 - }, - "search_dns": { - "type": "boolean" - }, - "keyid": {"type": "string"}, - "key": {"type": "string"}, - "keyserver": {"type": "string"} - } - } - }, - "cc_apk_configure": { - "type": "object", - "properties": { - "apk_repos": { - "type": "object", - "properties": { - "preserve_repositories": { - "type": "boolean", - "default": false, - "description": "By default, cloud-init will generate a new repositories file ``/etc/apk/repositories`` based on any valid configuration settings specified within a apk_repos section of cloud config. To disable this behavior and preserve the repositories file from the pristine image, set ``preserve_repositories`` to ``true``.\n\n The ``preserve_repositories`` option overrides all other config keys that would alter ``/etc/apk/repositories``." - }, - "alpine_repo": { - "type": ["object", "null"], - "properties": { - "base_url": { - "type": "string", - "default": "https://alpine.global.ssl.fastly.net/alpine", - "description": "The base URL of an Alpine repository, or mirror, to download official packages from. If not specified then it defaults to ``https://alpine.global.ssl.fastly.net/alpine``" - }, - "community_enabled": { - "type": "boolean", - "default": false, - "description": "Whether to add the Community repo to the repositories file. By default the Community repo is not included." - }, - "testing_enabled": { - "type": "boolean", - "default": false, - "description": "Whether to add the Testing repo to the repositories file. By default the Testing repo is not included. It is only recommended to use the Testing repo on a machine running the ``Edge`` version of Alpine as packages installed from Testing may have dependancies that conflict with those in non-Edge Main or Community repos." - }, - "version": { - "type": "string", - "description": "The Alpine version to use (e.g. ``v3.12`` or ``edge``)" - } - }, - "required": ["version"], - "minProperties": 1, - "additionalProperties": false - }, - "local_repo_base_url": { - "type": "string", - "description": "The base URL of an Alpine repository containing unofficial packages" - } - }, - "minProperties": 1, - "additionalProperties": false - } - } - }, - "cc_apt_configure": { - "properties": { - "apt": { - "type": "object", - "additionalProperties": false, - "properties": { - "preserve_sources_list": { - "type": "boolean", - "default": false, - "description": "By default, cloud-init will generate a new sources list in ``/etc/apt/sources.list.d`` based on any changes specified in cloud config. To disable this behavior and preserve the sources list from the pristine image, set ``preserve_sources_list`` to ``true``.\n\nThe ``preserve_sources_list`` option overrides all other config keys that would alter ``sources.list`` or ``sources.list.d``, **except** for additional sources to be added to ``sources.list.d``." - }, - "disable_suites": { - "type": "array", - "items": {"type": "string"}, - "uniqueItems": true, - "description": "Entries in the sources list can be disabled using ``disable_suites``, which takes a list of suites to be disabled. If the string ``$RELEASE`` is present in a suite in the ``disable_suites`` list, it will be replaced with the release name. If a suite specified in ``disable_suites`` is not present in ``sources.list`` it will be ignored. For convenience, several aliases are provided for`` disable_suites``:\n\n - ``updates`` => ``$RELEASE-updates``\n - ``backports`` => ``$RELEASE-backports``\n - ``security`` => ``$RELEASE-security``\n - ``proposed`` => ``$RELEASE-proposed``\n - ``release`` => ``$RELEASE``.\n\nWhen a suite is disabled using ``disable_suites``, its entry in ``sources.list`` is not deleted; it is just commented out." - }, - "primary": { - "$ref": "#/$defs/apt_configure.mirror", - "description": "The primary and security archive mirrors can be specified using the ``primary`` and ``security`` keys, respectively. Both the ``primary`` and ``security`` keys take a list of configs, allowing mirrors to be specified on a per-architecture basis. Each config is a dictionary which must have an entry for ``arches``, specifying which architectures that config entry is for. The keyword ``default`` applies to any architecture not explicitly listed. The mirror url can be specified with the ``uri`` key, or a list of mirrors to check can be provided in order, with the first mirror that can be resolved being selected. This allows the same configuration to be used in different environment, with different hosts used for a local APT mirror. If no mirror is provided by ``uri`` or ``search``, ``search_dns`` may be used to search for dns names in the format ``-mirror`` in each of the following:\n\n - fqdn of this host per cloud metadata,\n - localdomain,\n - domains listed in ``/etc/resolv.conf``.\n\nIf there is a dns entry for ``-mirror``, then it is assumed that there is a distro mirror at ``http://-mirror./``. If the ``primary`` key is defined, but not the ``security`` key, then then configuration for ``primary`` is also used for ``security``. If ``search_dns`` is used for the ``security`` key, the search pattern will be ``-security-mirror``.\n\nEach mirror may also specify a key to import via any of the following optional keys:\n\n - ``keyid``: a key to import via shortid or fingerprint.\n - ``key``: a raw PGP key.\n - ``keyserver``: alternate keyserver to pull ``keyid`` key from.\n\nIf no mirrors are specified, or all lookups fail, then default mirrors defined in the datasource are used. If none are present in the datasource either the following defaults are used:\n\n - ``primary`` => ``http://archive.ubuntu.com/ubuntu``.\n - ``security`` => ``http://security.ubuntu.com/ubuntu``" - }, - "security": { - "$ref": "#/$defs/apt_configure.mirror", - "description": "Please refer to the primary config documentation" - }, - "add_apt_repo_match": { - "type": "string", - "default": "^[\\w-]+:\\w", - "description": "All source entries in ``apt-sources`` that match regex in ``add_apt_repo_match`` will be added to the system using ``add-apt-repository``. If ``add_apt_repo_match`` is not specified, it defaults to ``^[\\w-]+:\\w``" - }, - "debconf_selections": { - "type": "object", - "items": {"type": "string"}, - "description": "Debconf additional configurations can be specified as a dictionary under the ``debconf_selections`` config key, with each key in the dict representing a different set of configurations. The value of each key must be a string containing all the debconf configurations that must be applied. We will bundle all of the values and pass them to ``debconf-set-selections``. Therefore, each value line must be a valid entry for ``debconf-set-selections``, meaning that they must possess for distinct fields:\n\n``pkgname question type answer``\n\nWhere:\n\n - ``pkgname`` is the name of the package.\n - ``question`` the name of the questions.\n - ``type`` is the type of question.\n - ``answer`` is the value used to answer the question.\n\nFor example: ``ippackage ippackage/ip string 127.0.01``" - }, - "sources_list": { - "type": "string", - "description": "Specifies a custom template for rendering ``sources.list`` . If no ``sources_list`` template is given, cloud-init will use sane default. Within this template, the following strings will be replaced with the appropriate values:\n\n - ``$MIRROR``\n - ``$RELEASE``\n - ``$PRIMARY``\n - ``$SECURITY``\n - ``$KEY_FILE``" - }, - "conf": { - "type": "string", - "description": "Specify configuration for apt, such as proxy configuration. This configuration is specified as a string. For multiline APT configuration, make sure to follow yaml syntax." - }, - "https_proxy": { - "type": "string", - "description": "More convenient way to specify https APT proxy. https proxy url is specified in the format ``https://[[user][:pass]@]host[:port]/``." - }, - "http_proxy": { - "type": "string", - "description": "More convenient way to specify http APT proxy. http proxy url is specified in the format ``http://[[user][:pass]@]host[:port]/``." - }, - "proxy": { - "type": "string", - "description": "Alias for defining a http APT proxy." - }, - "ftp_proxy": { - "type": "string", - "description": "More convenient way to specify ftp APT proxy. ftp proxy url is specified in the format ``ftp://[[user][:pass]@]host[:port]/``." - }, - "sources": { - "type": "object", - "items": {"type": "string"}, - "description": "Source list entries can be specified as a dictionary under the ``sources`` config key, with each key in the dict representing a different source file. The key of each source entry will be used as an id that can be referenced in other config entries, as well as the filename for the source's configuration under ``/etc/apt/sources.list.d``. If the name does not end with ``.list``, it will be appended. If there is no configuration for a key in ``sources``, no file will be written, but the key may still be referred to as an id in other ``sources`` entries.\n\nEach entry under ``sources`` is a dictionary which may contain any of the following optional keys:\n - ``source``: a sources.list entry (some variable replacements apply).\n - ``keyid``: a key to import via shortid or fingerprint.\n - ``key``: a raw PGP key.\n - ``keyserver``: alternate keyserver to pull ``keyid`` key from.\n - ``filename``: specify the name of the list file\n\nThe ``source`` key supports variable replacements for the following strings:\n\n - ``$MIRROR``\n - ``$PRIMARY``\n - ``$SECURITY``\n - ``$RELEASE``\n - ``$KEY_FILE``" - } - } - } - } - }, "cc_apt_pipelining": { "type": "object", "properties": { "apt_pipelining": { - "type": ["integer", "boolean", "string"], "oneOf": [ {"type": "integer"}, {"type": "boolean"}, @@ -156,348 +12,9 @@ ] } } - }, - "cc_bootcmd": { - "type": "object", - "properties": { - "bootcmd": { - "type": "array", - "items": { - "oneOf": [ - {"type": "array", "items": {"type": "string"}}, - {"type": "string"} - ] - }, - "additionalItems": false, - "minItems": 1 - } - } - }, - "cc_byobu": { - "type": "object", - "properties": { - "byobu_by_default": { - "type": "string", - "enum": [ - "enable-system", - "enable-user", - "disable-system", - "disable-user", - "enable", - "disable", - "user", - "system" - ] - } - } - }, - "cc_ca_certs": { - "type": "object", - "properties": { - "ca-certs": { - "type": "object", - "properties": { - "remove-defaults": { - "description": "Remove default CA certificates if true. Default: false", - "type": "boolean" - }, - "trusted": { - "description": "List of trusted CA certificates to add.", - "type": "array", - "items": {"type": "string"} - } - }, - "additionalProperties": false - } - } - }, - "cc_chef": { - "type": "object", - "properties": { - "chef": { - "type": "object", - "additionalProperties": false, - "properties": { - "directories": { - "type": "array", - "items": {"type": "string"}, - "uniqueItems": true, - "description": "Create the necessary directories for chef to run. By default, it creates the following directories:\n\n - ``/etc/chef``\n - ``/var/log/chef``\n - ``/var/lib/chef``\n - ``/var/cache/chef``\n - ``/var/backups/chef``\n - ``/var/run/chef``" - }, - "validation_cert": { - "type": "string", - "description": "Optional string to be written to file validation_key. Special value ``system`` means set use existing file." - }, - "validation_key": { - "type": "string", - "default": "/etc/chef/validation.pem", - "description": "Optional path for validation_cert. default to ``/etc/chef/validation.pem``" - }, - "firstboot_path": { - "type": "string", - "default": "/etc/chef/firstboot.json", - "description": "Path to write run_list and initial_attributes keys that should also be present in this configuration, defaults to ``/etc/chef/firstboot.json``" - }, - "exec": { - "type": "boolean", - "default": false, - "description": "Set true if we should run or not run chef (defaults to false, unless a gem installed is requested where this will then default to true)." - }, - "client_key": { - "type": "string", - "default": "/etc/chef/client.pem", - "description": "Optional path for client_cert. Default to ``/etc/chef/client.pem``." - }, - "encrypted_data_bag_secret": { - "type": "string", - "default": null, - "description": "Specifies the location of the secret key used by chef to encrypt data items. By default, this path is set to null, meaning that chef will have to look at the path ``/etc/chef/encrypted_data_bag_secret`` for it." - }, - "environment": { - "type": "string", - "default": "_default", - "description": "Specifies which environment chef will use. By default, it will use the ``_default`` configuration." - }, - "file_backup_path": { - "type": "string", - "default": "/var/backups/chef", - "description": "Specifies the location in which backup files are stored. By default, it uses the ``/var/backups/chef`` location." - }, - "file_cache_path": { - "type": "string", - "default": "/var/cache/chef", - "description": "Specifies the location in which chef cache files will be saved. By default, it uses the ``/var/cache/chef`` location." - }, - "json_attribs": { - "type": "string", - "default": "/etc/chef/firstboot.json", - "description": "Specifies the location in which some chef json data is stored. By default, it uses the ``/etc/chef/firstboot.json`` location." - }, - "log_level": { - "type": "string", - "default": ":info", - "description": "Defines the level of logging to be stored in the log file. By default this value is set to ``:info``." - }, - "log_location": { - "type": "string", - "default": "/var/log/chef/client.log", - "description": "Specifies the location of the chef lof file. By default, the location is specified at ``/var/log/chef/client.log``." - }, - "node_name": { - "type": "string", - "description": "The name of the node to run. By default, we will use th instance id as the node name." - }, - "omnibus_url": { - "type": "string", - "default": "https://www.chef.io/chef/install.sh", - "description": "Omnibus URL if chef should be installed through Omnibus. By default, it uses the ``https://www.chef.io/chef/install.sh``." - }, - "omnibus_url_retries": { - "type": "integer", - "default": 5, - "description": "The number of retries that will be attempted to reach the Omnibus URL. Default is 5." - }, - "omnibus_version": { - "type": "string", - "description": "Optional version string to require for omnibus install." - }, - "pid_file": { - "type": "string", - "default": "/var/run/chef/client.pid", - "description": "The location in which a process identification number (pid) is saved. By default, it saves in the ``/var/run/chef/client.pid`` location." - }, - "server_url": { - "type": "string", - "description": "The URL for the chef server" - }, - "show_time": { - "type": "boolean", - "default": true, - "description": "Show time in chef logs" - }, - "ssl_verify_mode": { - "type": "string", - "default": ":verify_none", - "description": "Set the verify mode for HTTPS requests. We can have two possible values for this parameter:\n\n - ``:verify_none``: No validation of SSL certificates.\n - ``:verify_peer``: Validate all SSL certificates.\n\nBy default, the parameter is set as ``:verify_none``." - }, - "validation_name": { - "type": "string", - "description": "The name of the chef-validator key that Chef Infra Client uses to access the Chef Infra Server during the initial Chef Infra Client run." - }, - "force_install": { - "type": "boolean", - "default": false, - "description": "If set to ``true``, forces chef installation, even if it is already installed." - }, - "initial_attributes": { - "type": "object", - "items": {"type": "string"}, - "description": "Specify a list of initial attributes used by the cookbooks." - }, - "install_type": { - "type": "string", - "default": "packages", - "description": "The type of installation for chef. It can be one of the following values:\n\n - ``packages``\n - ``gems``\n - ``omnibus``" - }, - "run_list": { - "type": "array", - "items": {"type": "string"}, - "description": "A run list for a first boot json." - }, - "chef_license": { - "type": "string", - "description": "string that indicates if user accepts or not license related to some of chef products" - } - } - } - } - }, - "cc_debug": { - "type": "object", - "properties": { - "debug": { - "additionalProperties": false, - "type": "object", - "properties": { - "verbose": { - "description": "Should always be true for this module", - "type": "boolean" - }, - "output": { - "description": "Location to write output. Defaults to console + log", - "type": "string" - } - } - } - } - }, - "cc_disable_ec2_metadata": { - "type": "object", - "properties": { - "disable_ec2_metadata": { - "default": false, - "description": "Set true to disable IPv4 routes to EC2 metadata. Default: false.", - "type": "boolean" - } - } - }, - "cc_disk_setup": { - "type": "object", - "properties": { - "device_aliases": { - "type": "object", - "patternProperties": { - "^.+$": { - "label": "", - "type": "string", - "description": "Path to disk to be aliased by this name." - } - } - }, - "disk_setup": { - "type": "object", - "patternProperties": { - "^.+$": { - "label": "", - "type": "object", - "additionalProperties": false, - "properties": { - "table_type": { - "type": "string", - "enum": ["mbr", "gpt"], - "description": "Specifies the partition table type, either ``mbr`` or ``gpt``. Default: ``mbr``." - }, - "layout": { - "type": ["string", "boolean", "array"], - "oneOf": [ - {"type": "string", "enum": ["auto", "remove"]}, - {"type": "boolean"}, - { - "type": "array", - "items": { - "oneOf": [ - {"type": "integer"}, - { - "type": "array", - "items": {"type": "integer"} - } - ] - } - } - ], - "description": "If set to ``true``, a single partition using all the space on the device will be created. If set to ``false``, no partitions will be created. Partitions can be specified by providing a list to ``layout``, where each entry in the list is either a size or a list containing a size and the numerical value for a partition type. The size for partitions is specified in **percentage** of disk space, not in bytes (e.g. a size of 33 would take up 1/3 of the disk space). Default: ``false``." - }, - "overwrite": { - "type": "boolean", - "description": "Controls whether this module tries to be safe about writing partition tables or not. If ``overwrite: false`` is set, the device will be checked for a partition table and for a file system and if either is found, the operation will be skipped. If ``overwrite: true`` is set, no checks will be performed. Using ``overwrite: true`` is **dangerous** and can lead to data loss, so double check that the correct device has been specified if using this option. Default: ``false``" - } - } - } - } - }, - "fs_setup": { - "type": "array", - "items": { - "type": "object", - "additionalProperties": false, - "properties": { - "label": { - "type": "string", - "description": "Label for the filesystem." - }, - "filesystem": { - "type": "string", - "description": "Filesystem type to create. E.g., ``ext4`` or ``btrfs``" - }, - "device": { - "type": "string", - "description": "Specified either as a path or as an alias in the format ``.`` where ```` denotes the partition number on the device. If specifying device using the ``.`` format, the value of ``partition`` will be overwritten." - }, - "partition": { - "type": ["string", "integer"], - "oneOf": [ - { - "type": "string", - "enum": ["auto", "any", "none"] - }, - {"type": "integer"} - ], - "description": "The partition can be specified by setting ``partition`` to the desired partition number. The ``partition`` option may also be set to ``auto``, in which this module will search for the existence of a filesystem matching the ``label``, ``type`` and ``device`` of the ``fs_setup`` entry and will skip creating the filesystem if one is found. The ``partition`` option may also be set to ``any``, in which case any file system that matches ``type`` and ``device`` will cause this module to skip filesystem creation for the ``fs_setup`` entry, regardless of ``label`` matching or not. To write a filesystem directly to a device, use ``partition: none``. ``partition: none`` will **always** write the filesystem, even when the ``label`` and ``filesystem`` are matched, and ``overwrite`` is ``false``." - }, - "overwrite": { - "type": "boolean", - "description": "If ``true``, overwrite any existing filesystem. Using ``overwrite: true`` for filesystems is **dangerous** and can lead to data loss, so double check the entry in ``fs_setup``. Default: ``false``" - }, - "replace_fs": { - "type": "string", - "description": "Ignored unless ``partition`` is ``auto`` or ``any``. Default ``false``." - }, - "extra_opts": { - "type": ["array", "string"], - "items": {"type": "string"}, - "description": "Optional options to pass to the filesystem creation command. Ignored if you using ``cmd`` directly." - }, - "cmd": { - "type": ["array", "string"], - "items": {"type": "string"}, - "description": "Optional command to run to create the filesystem. Can include string substitutions of the other ``fs_setup`` config keys. This is only necessary if you need to override the default command." - } - } - } - } - } } }, "allOf": [ - { "$ref": "#/$defs/cc_apk_configure" }, - { "$ref": "#/$defs/cc_apt_configure" }, - { "$ref": "#/$defs/cc_apt_pipelining" }, - { "$ref": "#/$defs/cc_bootcmd" }, - { "$ref": "#/$defs/cc_byobu" }, - { "$ref": "#/$defs/cc_ca_certs" }, - { "$ref": "#/$defs/cc_chef" }, - { "$ref": "#/$defs/cc_debug" }, - { "$ref": "#/$defs/cc_disable_ec2_metadata" }, - { "$ref": "#/$defs/cc_disk_setup" } + { "$ref": "#/$defs/cc_apt_pipelining" } ] } diff --git a/setup.py b/setup.py index c98a4703d81..225269ee826 100755 --- a/setup.py +++ b/setup.py @@ -278,6 +278,7 @@ def finalize_options(self): (ETC + "/cloud", [render_tmpl("config/cloud.cfg.tmpl")]), (ETC + "/cloud/cloud.cfg.d", glob("config/cloud.cfg.d/*")), (ETC + "/cloud/templates", glob("templates/*")), + (ETC + "/cloud/schema", glob("config/cloud-init-schema-*")), ( USR_LIB_EXEC + "/cloud-init", [ diff --git a/tests/unittests/test_upgrade.py b/tests/unittests/test_upgrade.py index d7a721a28bd..b73e1cddb2d 100644 --- a/tests/unittests/test_upgrade.py +++ b/tests/unittests/test_upgrade.py @@ -47,6 +47,9 @@ def test_blacklist_drivers_set_on_networking(self, previous_obj_pkl): def test_paths_has_run_dir_attribute(self, previous_obj_pkl): assert previous_obj_pkl.paths.run_dir is not None + def test_paths_has_schema_dir_attribute(self, previous_obj_pkl): + assert previous_obj_pkl.paths.schema_dir is not None + def test_vendordata_exists(self, previous_obj_pkl): assert previous_obj_pkl.vendordata2 is None assert previous_obj_pkl.vendordata2_raw is None From 5c66261f138b84e6e947003d802bb793af10094b Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 7 Jan 2022 13:36:35 -0700 Subject: [PATCH 03/20] schema: support legacy module schema and compisite global schema Ultimately cloud-init wants a single JSON schema file defining composite schema for all cc_* modules instead of each module defining it's own sub-schema. Legacy schema definitions took the form or a "schema" attribute in each cc_* module. The "new" way would be adding schema $defs to the single config/cloud-init-schema- file. End goal is to publish those static composite JSON schema files to schema validation services to allow externals to consume and validate user-data before spending resources on invalid instance launches. During migration from legacy schema definitions in each cc_* module, cloudinit needs to support cc_* modules which define schema localling within the cc_* module as a "schema" attribute. - get_schema: source config/cloud-init-schema* and supplement with legacy cc_*.schema definitions to bridge gap between legacy cc_*.schema defitions and the "new" full config/cloud-init-schema. - Add support for load_doc to handle legacy and combined schema defs - get_meta_doc will validate against global composite schema when 'schema' param absent - get_meta_doc will also limit scope of documentation to the specific meta["id"] attribute as defined in global schema $defs. To allow modules to avoid passing in their own sub-schema - get_meta_doc needs to find_modules containing "meta" instead of "schema" as non-legacy schema definitions exist in config/cloud-init-schema* instead of in python dicts as a cc_*.schema attribute --- cloudinit/config/schema.py | 65 +++++++++++++++++++++------ tests/unittests/config/test_schema.py | 41 ++++++++++++----- 2 files changed, 82 insertions(+), 24 deletions(-) diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index c8f9f34f536..2c370f49977 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -2,6 +2,8 @@ """schema.py: Set of module functions for processing cloud-config schema.""" import argparse +import glob +import json import logging import os import re @@ -455,7 +457,7 @@ def _parse_description(description, prefix) -> str: return description -def _get_property_doc(schema: dict, prefix=" ") -> str: +def _get_property_doc(schema: dict, defs: dict, prefix=" ") -> str: """Return restructured text describing the supported schema properties.""" new_prefix = prefix + " " properties = [] @@ -466,6 +468,10 @@ def _get_property_doc(schema: dict, prefix=" ") -> str: for props in property_keys: for prop_key, prop_config in props.items(): + if "$ref" in prop_config: + # Update the defined references in subschema for doc rendering + ref = defs[prop_config["$ref"].replace("#/$defs/", "")] + prop_config.update(ref) # Define prop_name and description for SCHEMA_PROPERTY_TMPL description = prop_config.get("description", "") @@ -484,7 +490,9 @@ def _get_property_doc(schema: dict, prefix=" ") -> str: if isinstance(items, list): for item in items: properties.append( - _get_property_doc(item, prefix=new_prefix) + _get_property_doc( + item, defs=defs, prefix=new_prefix + ) ) elif isinstance(items, dict) and ( items.get("properties") or items.get("patternProperties") @@ -496,14 +504,16 @@ def _get_property_doc(schema: dict, prefix=" ") -> str: ) new_prefix += " " properties.append( - _get_property_doc(items, prefix=new_prefix) + _get_property_doc(items, defs=defs, prefix=new_prefix) ) if ( "properties" in prop_config or "patternProperties" in prop_config ): properties.append( - _get_property_doc(prop_config, prefix=new_prefix) + _get_property_doc( + prop_config, defs=defs, prefix=new_prefix + ) ) return "\n\n".join(properties) @@ -526,15 +536,18 @@ def _get_examples(meta: MetaSchema) -> str: return rst_content -def get_meta_doc(meta: MetaSchema, schema: dict) -> str: +def get_meta_doc(meta: MetaSchema, schema: dict = None) -> str: """Return reStructured text rendering the provided metadata. @param meta: Dict of metadata to render. + @param schema: Optional module schema, if absent, read global schema. @raise KeyError: If metadata lacks an expected key. """ + if schema is None: + schema = get_schema() if not meta or not schema: - raise ValueError("Expected meta and schema") + raise ValueError("Expected non-empty meta and schema") keys = set(meta.keys()) expected = set( { @@ -563,8 +576,11 @@ def get_meta_doc(meta: MetaSchema, schema: dict) -> str: # cast away type annotation meta_copy = dict(deepcopy(meta)) + defs = schema.get("$defs", {}) + if defs.get(meta["id"]): + schema = defs.get(meta["id"]) try: - meta_copy["property_doc"] = _get_property_doc(schema) + meta_copy["property_doc"] = _get_property_doc(schema, defs=defs) except AttributeError: LOG.warning("Unable to render property_doc due to invalid schema") meta_copy["property_doc"] = "" @@ -599,7 +615,7 @@ def load_doc(requested_modules: list) -> str: for mod_name in all_modules: if "all" in requested_modules or mod_name in requested_modules: (mod_locs, _) = importer.find_module( - mod_name, ["cloudinit.config"], ["schema"] + mod_name, ["cloudinit.config"], ["meta"] ) if mod_locs: mod = importer.import_module(mod_locs[0]) @@ -608,14 +624,35 @@ def load_doc(requested_modules: list) -> str: def get_schema() -> dict: - """Return jsonschema coalesced from all cc_* cloud-config module.""" - full_schema = { - "$schema": "http://json-schema.org/draft-04/schema#", - "id": "cloud-config-schema", - "allOf": [], - } + """Return jsonschema coalesced from all cc_* cloud-config modules.""" + paths = read_cfg_paths() + schema_files = glob.glob( + os.path.join(paths.schema_dir, "cloud-init-schema-*") + ) + full_schema = None + for schema_file in schema_files: + try: + full_schema = json.loads(load_file(schema_file)) + except Exception as e: + LOG.warning("Cannot parse JSON schema file %s. %s", schema_file, e) + if not full_schema: + LOG.warning( + "No base JSON schema files found at %s/cloud-init-schema-*. Setting default empty schema", + paths.schema_dir, + ) + full_schema = { + "$defs": {}, + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [], + } + # TODO( Drop the get_modules loop when all legacy cc_* schema migrates ) + # Supplement base_schema with any legacy modules which still contain a + # "schema" attribute. Legacy cc_* modules will be migrated to use the + # store module schema in the composite cloud-init-schema-.json + # and will drop "schema" at that point. for (_, mod_name) in get_modules().items(): + # All cc_* modules need a "meta" attribute to represent schema defs (mod_locs, _) = importer.find_module( mod_name, ["cloudinit.config"], ["schema"] ) diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 0d942bfc053..c72f5f79474 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -21,6 +21,7 @@ get_jsonschema_validator, get_meta_doc, get_schema, + load_doc, main, validate_cloudconfig_file, validate_cloudconfig_metaschema, @@ -78,14 +79,15 @@ def get_module_variable(var_name) -> dict: return schemas -class GetSchemaTest(CiTestCase): +class TestGetSchema: def test_get_schema_coalesces_known_schema(self): """Every cloudconfig module with schema is listed in allOf keyword.""" schema = get_schema() - self.assertCountEqual( + assert sorted( [ "cc_apk_configure", "cc_apt_configure", + "cc_apt_pipelining", "cc_bootcmd", "cc_keyboard", "cc_locale", @@ -99,14 +101,32 @@ def test_get_schema_coalesces_known_schema(self): "cc_zypper_add_repo", "cc_chef", "cc_install_hotplug", - ], - [meta["id"] for meta in get_metas().values() if meta is not None], + ] + ) == sorted( + [meta["id"] for meta in get_metas().values() if meta is not None] ) - self.assertEqual("cloud-config-schema", schema["id"]) - self.assertEqual( - "http://json-schema.org/draft-04/schema#", schema["$schema"] + assert "http://json-schema.org/draft-04/schema#" == schema["$schema"] + assert ["$defs", "$schema", "allOf"] == sorted( + list(get_schema().keys()) ) - self.assertCountEqual(["id", "$schema", "allOf"], get_schema().keys()) + + +class TestLoadDoc: + + docs = get_module_variable("__doc__") + + # TODO( Drop legacy test when all sub-schemas in cloud-init-schema.json ) + @pytest.mark.parametrize( + "module_name", + ( + "cc_apt_pipelining", # new style composite schema file + "cc_bootcmd", # legacy sub-schema defined in module + ), + ) + def test_report_docs_for_legacy_and_consolidated_schema(self, module_name): + doc = load_doc([module_name]) + assert doc, "Unexpected empty docs for {}".format(module_name) + assert self.docs[module_name] == doc class SchemaValidationErrorTest(CiTestCase): @@ -178,8 +198,9 @@ def test_validateconfig_schema_strict_raises_errors(self): schema = {"properties": {"p1": {"type": "string"}}} with pytest.raises(SchemaValidationError) as context_mgr: validate_cloudconfig_schema({"p1": -1}, schema, strict=True) - assert"Cloud config schema errors: p1: -1 is not of type 'string'" == ( - str(context_mgr.value) + assert ( + "Cloud config schema errors: p1: -1 is not of type 'string'" + == (str(context_mgr.value)) ) @skipUnlessJsonSchema() From 12b9504a4465be77072933433a8f06bd9eeb6aab Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 7 Jan 2022 13:47:22 -0700 Subject: [PATCH 04/20] schema: define schema meta for cc_apt_pipelining --- cloudinit/config/cc_apt_pipelining.py | 62 +++++++++++++++------------ 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/cloudinit/config/cc_apt_pipelining.py b/cloudinit/config/cc_apt_pipelining.py index 569849d1333..556eb4c40b4 100644 --- a/cloudinit/config/cc_apt_pipelining.py +++ b/cloudinit/config/cc_apt_pipelining.py @@ -4,54 +4,60 @@ # # This file is part of cloud-init. See LICENSE file for license information. -""" -Apt Pipelining --------------- -**Summary:** configure apt pipelining +"""Apt Pipelining: configure apt pipelining.""" -This module configures apt's ``Acquite::http::Pipeline-Depth`` option, which -controls how apt handles HTTP pipelining. It may be useful for pipelining to be -disabled, because some web servers, such as S3 do not pipeline properly (LP: -#948461). The ``apt_pipelining`` config key may be set to ``false`` to disable -pipelining altogether. This is the default behavior. If it is set to ``none``, -``unchanged``, or ``os``, no change will be made to apt configuration and the -default setting for the distro will be used. The pipeline depth can also be -manually specified by setting ``apt_pipelining`` to a number. However, this is -not recommended. - -**Internal name:** ``cc_apt_pipelining`` - -**Module frequency:** per instance - -**Supported distros:** ubuntu, debian - -**Config keys**:: - apt_pipelining: -""" +from textwrap import dedent from cloudinit import util +from cloudinit.config.schema import get_meta_doc from cloudinit.settings import PER_INSTANCE frequency = PER_INSTANCE - distros = ["ubuntu", "debian"] - DEFAULT_FILE = "/etc/apt/apt.conf.d/90cloud-init-pipelining" - APT_PIPE_TPL = ( "//Written by cloud-init per 'apt_pipelining'\n" 'Acquire::http::Pipeline-Depth "%s";\n' ) - # Acquire::http::Pipeline-Depth can be a value # from 0 to 5 indicating how many outstanding requests APT should send. # A value of zero MUST be specified if the remote host does not properly linger # on TCP connections - otherwise data corruption will occur. +meta = { + "id": "cc_apt_pipelining", + "name": "Apt Pipelining", + "title": "Configure apt pipelining", + "description": dedent( + """\ + This module configures apt's ``Acquite::http::Pipeline-Depth`` option, + which controls how apt handles HTTP pipelining. It may be useful for + pipelining to be disabled, because some web servers, such as S3 do not + pipeline properly (LP: #948461). + + Value configuration options for this module are: + + * ``false`` (Default): disable pipelining altogether + * ``none``, ``unchanged``, or ``os``: use distro default + * ````: Manually specify pipeline depth. This is not recommended.""" # noqa: E501 + ), + "distros": distros, + "frequency": frequency, + "examples": [ + "apt_pipelining: false", + "apt_pipelining: none", + "apt_pipelining: unchanged", + "apt_pipelining: os", + "apt_pipelining: 3", + ], +} + +__doc__ = get_meta_doc(meta) -def handle(_name, cfg, _cloud, log, _args): +def handle(_name, cfg, _cloud, log, _args): apt_pipe_value = util.get_cfg_option_str(cfg, "apt_pipelining", "os") + apt_pipe_value = cfg.get("apt_pipelining", "os") apt_pipe_value_s = str(apt_pipe_value).lower().strip() if apt_pipe_value_s == "false": From b1cf0aa21f3111862848173e43d16c73601021d2 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 7 Jan 2022 16:07:25 -0700 Subject: [PATCH 05/20] schema: annotation fixes for lists and empty schema Add tests for _schemapath_for_cloudconfig as it was incorrectly constructing paths prefixes for list items. Prevent concatonation of path_prefix to a key if the key is the same indent_depth as prior line. --- cloudinit/config/schema.py | 10 ++++++---- tests/unittests/config/test_schema.py | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index 2c370f49977..e88d2aafcdc 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -355,12 +355,14 @@ def _schemapath_for_cloudconfig(config, original_content): @param config: The yaml.loaded config dictionary of a cloud-config file. @param original_content: The simple file content of the cloud-config file """ - # FIXME Doesn't handle multi-line lists or multi-line strings + # TODO( handle multi-line lists or multi-line strings, inline dicts) content_lines = original_content.decode().split("\n") schema_line_numbers = {} list_index = 0 RE_YAML_INDENT = r"^(\s*)" scopes = [] + if not config: + return {} # No YAML config dict, no schemapaths to annotate for line_number, line in enumerate(content_lines, 1): indent_depth = len(re.match(RE_YAML_INDENT, line).groups()[0]) line = line.strip() @@ -377,7 +379,6 @@ def _schemapath_for_cloudconfig(config, original_content): if path_prefix and path_prefix.endswith(previous_list_idx): path_prefix = path_prefix[: -len(previous_list_idx)] key = str(list_index) - schema_line_numbers[key] = line_number item_indent = len(re.match(RE_YAML_INDENT, line[1:]).groups()[0]) item_indent += 1 # For the leading '-' character previous_depth = indent_depth @@ -388,7 +389,7 @@ def _schemapath_for_cloudconfig(config, original_content): # Process non-list lines setting value if present list_index = 0 key, value = line.split(":", 1) - if path_prefix: + if path_prefix and indent_depth > previous_depth: # Append any existing path_prefix for a fully-pathed key key = path_prefix + "." + key while indent_depth <= previous_depth: @@ -637,7 +638,8 @@ def get_schema() -> dict: LOG.warning("Cannot parse JSON schema file %s. %s", schema_file, e) if not full_schema: LOG.warning( - "No base JSON schema files found at %s/cloud-init-schema-*. Setting default empty schema", + "No base JSON schema files found at %s/cloud-init-schema-*." + " Setting default empty schema", paths.schema_dir, ) full_schema = { diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index c72f5f79474..3a0eb1ad9b5 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -11,12 +11,14 @@ from textwrap import dedent import pytest +import yaml from yaml import safe_load from cloudinit.config.schema import ( CLOUD_CONFIG_HEADER, MetaSchema, SchemaValidationError, + _schemapath_for_cloudconfig, annotated_cloudconfig_file, get_jsonschema_validator, get_meta_doc, @@ -129,6 +131,31 @@ def test_report_docs_for_legacy_and_consolidated_schema(self, module_name): assert self.docs[module_name] == doc +class Test_SchemapathForCloudconfig: + """Coverage tests for supported YAML formats.""" + + @pytest.mark.parametrize( + "source_content, expected", + ( + (b"{}", {}), # assert empty config handled + # Multiple keys account for comments and whitespace lines + (b"#\na: va\n \nb: vb\n#\nc: vc", {"a": 2, "b": 4, "c": 6}), + # List items represented on correct line number + (b"a:\n - a1\n\n - a2\n", {"a": 1, "a.0": 2, "a.1": 4}), + # Nested dicts represented on correct line number + (b"a:\n a1:\n\n aa1: aa1v\n", {"a": 1, "a.a1": 2, "a.a1.aa1": 4}), + ), + ) + def test_schemapaths_representatative_of_source_yaml( + self, source_content, expected + ): + """Validate schemapaths dict accurately represents source YAML line.""" + cfg = yaml.safe_load(source_content) + assert expected == _schemapath_for_cloudconfig( + config=cfg, original_content=source_content + ) + + class SchemaValidationErrorTest(CiTestCase): """Test validate_cloudconfig_schema""" From cfaf67bc3fc8cd49b652fde87ee629d80d50e60c Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 7 Jan 2022 21:17:20 -0700 Subject: [PATCH 06/20] schema: clarify schema warning message --- cloudinit/config/schema.py | 4 +- tests/unittests/config/test_cc_bootcmd.py | 3 +- tests/unittests/config/test_cc_ntp.py | 21 ++++---- tests/unittests/config/test_cc_resizefs.py | 8 +-- tests/unittests/config/test_cc_runcmd.py | 2 +- tests/unittests/config/test_cc_snap.py | 49 ++++++++++--------- .../config/test_cc_ubuntu_advantage.py | 17 ++++--- tests/unittests/config/test_cc_write_files.py | 15 +++--- .../config/test_cc_write_files_deferred.py | 6 ++- tests/unittests/config/test_schema.py | 5 +- 10 files changed, 73 insertions(+), 57 deletions(-) diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index e88d2aafcdc..3fb5dbe8282 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -211,7 +211,9 @@ def validate_cloudconfig_schema( raise SchemaValidationError(errors) else: messages = ["{0}: {1}".format(k, msg) for k, msg in errors] - LOG.warning("Invalid config:\n%s", "\n".join(messages)) + LOG.warning( + "Invalid cloud-config provided:\n%s", "\n".join(messages) + ) def annotated_cloudconfig_file(cloudconfig, original_content, schema_errors): diff --git a/tests/unittests/config/test_cc_bootcmd.py b/tests/unittests/config/test_cc_bootcmd.py index 6d8793b97a0..17033596e3c 100644 --- a/tests/unittests/config/test_cc_bootcmd.py +++ b/tests/unittests/config/test_cc_bootcmd.py @@ -77,7 +77,8 @@ def test_handler_schema_validation_warns_non_array_type(self): with self.assertRaises(TypeError): handle("cc_bootcmd", invalid_config, cc, LOG, []) self.assertIn( - "Invalid config:\nbootcmd: 1 is not of type 'array'", + "Invalid cloud-config provided:\nbootcmd: 1 is not of type" + " 'array'", self.logs.getvalue(), ) self.assertIn("Failed to shellify", self.logs.getvalue()) diff --git a/tests/unittests/config/test_cc_ntp.py b/tests/unittests/config/test_cc_ntp.py index 7da82cee164..fba141aa21f 100644 --- a/tests/unittests/config/test_cc_ntp.py +++ b/tests/unittests/config/test_cc_ntp.py @@ -385,7 +385,9 @@ def test_ntp_handler_schema_validation_allows_empty_ntp_config( "servers []\npools {0}\n".format(pools), util.load_file(confpath), ) - self.assertNotIn("Invalid config:", self.logs.getvalue()) + self.assertNotIn( + "Invalid cloud-config provided:", self.logs.getvalue() + ) @skipUnlessJsonSchema() @mock.patch("cloudinit.config.cc_ntp.select_ntp_client") @@ -404,8 +406,8 @@ def test_ntp_handler_schema_validation_warns_non_string_item_type( m_sel.return_value = ntpconfig cc_ntp.handle("cc_ntp", invalid_config, mycloud, None, []) self.assertIn( - "Invalid config:\nntp.pools.0: 123 is not of type 'string'\n" - "ntp.servers.1: None is not of type 'string'", + "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( @@ -431,8 +433,8 @@ def test_ntp_handler_schema_validation_warns_of_non_array_type( m_select.return_value = ntpconfig cc_ntp.handle("cc_ntp", invalid_config, mycloud, None, []) self.assertIn( - "Invalid config:\nntp.pools: 123 is not of type 'array'\n" - "ntp.servers: 'non-array' is not of type 'array'", + "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( @@ -459,8 +461,9 @@ def test_ntp_handler_schema_validation_warns_invalid_key_present( m_select.return_value = ntpconfig cc_ntp.handle("cc_ntp", invalid_config, mycloud, None, []) self.assertIn( - "Invalid config:\nntp: Additional properties are not " - "allowed ('invalidkey' was unexpected)", + "Invalid cloud-config provided:\nntp: Additional" + " properties are not allowed ('invalidkey' was" + " unexpected)", self.logs.getvalue(), ) self.assertEqual( @@ -488,8 +491,8 @@ def test_ntp_handler_schema_validation_warns_of_duplicates(self, m_select): m_select.return_value = ntpconfig cc_ntp.handle("cc_ntp", invalid_config, mycloud, None, []) self.assertIn( - "Invalid config:\nntp.pools: ['0.mypool.org', '0.mypool.org']" - " has non-unique elements\nntp.servers: " + "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(), ) diff --git a/tests/unittests/config/test_cc_resizefs.py b/tests/unittests/config/test_cc_resizefs.py index 228f1e458ba..9981dcea2d4 100644 --- a/tests/unittests/config/test_cc_resizefs.py +++ b/tests/unittests/config/test_cc_resizefs.py @@ -92,8 +92,8 @@ def test_handle_schema_validation_logs_invalid_resize_rootfs_value(self): handle("cc_resizefs", cfg, _cloud=None, log=LOG, args=[]) logs = self.logs.getvalue() self.assertIn( - "WARNING: Invalid config:\nresize_rootfs: 'junk' is not one of" - " [True, False, 'noblock']", + "WARNING: Invalid cloud-config provided:\nresize_rootfs: 'junk' is" + " not one of [True, False, 'noblock']", logs, ) self.assertIn( @@ -108,7 +108,9 @@ def test_handle_warns_on_unknown_mount_info(self, m_get_mount_info): cfg = {"resize_rootfs": True} handle("cc_resizefs", cfg, _cloud=None, log=LOG, args=[]) logs = self.logs.getvalue() - self.assertNotIn("WARNING: Invalid config:\nresize_rootfs:", logs) + self.assertNotIn( + "WARNING: Invalid cloud-config provided:\nresize_rootfs:", logs + ) self.assertIn( "WARNING: Could not determine filesystem type of /\n", logs ) diff --git a/tests/unittests/config/test_cc_runcmd.py b/tests/unittests/config/test_cc_runcmd.py index 34b3fb77ca5..59490d67377 100644 --- a/tests/unittests/config/test_cc_runcmd.py +++ b/tests/unittests/config/test_cc_runcmd.py @@ -73,7 +73,7 @@ def test_handler_schema_validation_warns_non_array_type(self): with self.assertRaises(TypeError) as cm: handle("cc_runcmd", invalid_config, cc, LOG, []) self.assertIn( - "Invalid config:\nruncmd: 1 is not of type 'array'", + "Invalid cloud-config provided:\nruncmd: 1 is not of type 'array'", self.logs.getvalue(), ) self.assertIn("Failed to shellify", str(cm.exception)) diff --git a/tests/unittests/config/test_cc_snap.py b/tests/unittests/config/test_cc_snap.py index f7e66ad25f0..1632676dbb0 100644 --- a/tests/unittests/config/test_cc_snap.py +++ b/tests/unittests/config/test_cc_snap.py @@ -284,8 +284,8 @@ def test_schema_warns_on_snap_not_as_dict(self): """If the snap configuration is not a dict, emit a warning.""" validate_cloudconfig_schema({"snap": "wrong type"}, schema) self.assertEqual( - "WARNING: Invalid config:\nsnap: 'wrong type' is not of type" - " 'object'\n", + "WARNING: Invalid cloud-config provided:\nsnap: 'wrong type'" + " is not of type 'object'\n", self.logs.getvalue(), ) @@ -296,8 +296,8 @@ def test_schema_disallows_unknown_keys(self, _): {"snap": {"commands": ["ls"], "invalid-key": ""}}, schema ) self.assertIn( - "WARNING: Invalid config:\nsnap: Additional properties are not" - " allowed ('invalid-key' was unexpected)", + "WARNING: Invalid cloud-config provided:\nsnap: Additional" + " properties are not allowed ('invalid-key' was unexpected)", self.logs.getvalue(), ) @@ -305,8 +305,8 @@ def test_warn_schema_requires_either_commands_or_assertions(self): """Warn when snap configuration lacks both commands and assertions.""" validate_cloudconfig_schema({"snap": {}}, schema) self.assertIn( - "WARNING: Invalid config:\nsnap: {} does not have enough" - " properties", + "WARNING: Invalid cloud-config provided:\nsnap: {} does not" + " have enough properties", self.logs.getvalue(), ) @@ -315,8 +315,8 @@ def test_warn_schema_commands_is_not_list_or_dict(self, _): """Warn when snap:commands config is not a list or dict.""" validate_cloudconfig_schema({"snap": {"commands": "broken"}}, schema) self.assertEqual( - "WARNING: Invalid config:\nsnap.commands: 'broken' is not of type" - " 'object', 'array'\n", + "WARNING: Invalid cloud-config provided:\nsnap.commands: 'broken'" + " is not of type 'object', 'array'\n", self.logs.getvalue(), ) @@ -326,9 +326,9 @@ def test_warn_schema_when_commands_is_empty(self, _): validate_cloudconfig_schema({"snap": {"commands": []}}, schema) validate_cloudconfig_schema({"snap": {"commands": {}}}, schema) self.assertEqual( - "WARNING: Invalid config:\nsnap.commands: [] is too short\n" - "WARNING: Invalid config:\nsnap.commands: {} does not have enough" - " properties\n", + "WARNING: Invalid cloud-config provided:\nsnap.commands: [] is" + " too short\nWARNING: Invalid cloud-config provided:\n" + "snap.commands: {} does not have enough properties\n", self.logs.getvalue(), ) @@ -349,10 +349,10 @@ def test_schema_when_commands_values_are_invalid_type(self, _): {"snap": {"commands": {"01": 123}}}, schema ) self.assertEqual( - "WARNING: Invalid config:\n" + "WARNING: Invalid cloud-config provided:\n" "snap.commands.0: 123 is not valid under any of the given" " schemas\n" - "WARNING: Invalid config:\n" + "WARNING: Invalid cloud-config provided:\n" "snap.commands.01: 123 is not valid under any of the given" " schemas\n", self.logs.getvalue(), @@ -368,10 +368,10 @@ def test_schema_when_commands_list_values_are_invalid_type(self, _): {"snap": {"commands": {"01": ["snap", "install", 123]}}}, schema ) self.assertEqual( - "WARNING: Invalid config:\n" + "WARNING: Invalid cloud-config provided:\n" "snap.commands.0: ['snap', 'install', 123] is not valid under any" " of the given schemas\n", - "WARNING: Invalid config:\n" + "WARNING: Invalid cloud-config provided:\n" "snap.commands.0: ['snap', 'install', 123] is not valid under any" " of the given schemas\n", self.logs.getvalue(), @@ -385,9 +385,9 @@ def test_schema_when_assertions_values_are_invalid_type(self, _): {"snap": {"assertions": {"01": 123}}}, schema ) self.assertEqual( - "WARNING: Invalid config:\n" + "WARNING: Invalid cloud-config provided:\n" "snap.assertions.0: 123 is not of type 'string'\n" - "WARNING: Invalid config:\n" + "WARNING: Invalid cloud-config provided:\n" "snap.assertions.01: 123 is not of type 'string'\n", self.logs.getvalue(), ) @@ -397,8 +397,8 @@ def test_warn_schema_assertions_is_not_list_or_dict(self, _): """Warn when snap:assertions config is not a list or dict.""" validate_cloudconfig_schema({"snap": {"assertions": "broken"}}, schema) self.assertEqual( - "WARNING: Invalid config:\nsnap.assertions: 'broken' is not of" - " type 'object', 'array'\n", + "WARNING: Invalid cloud-config provided:\nsnap.assertions:" + " 'broken' is not of type 'object', 'array'\n", self.logs.getvalue(), ) @@ -408,9 +408,10 @@ def test_warn_schema_when_assertions_is_empty(self, _): validate_cloudconfig_schema({"snap": {"assertions": []}}, schema) validate_cloudconfig_schema({"snap": {"assertions": {}}}, schema) self.assertEqual( - "WARNING: Invalid config:\nsnap.assertions: [] is too short\n" - "WARNING: Invalid config:\nsnap.assertions: {} does not have" - " enough properties\n", + "WARNING: Invalid cloud-config provided:\nsnap.assertions: []" + " is too short\n" + "WARNING: Invalid cloud-config provided:\nsnap.assertions: {}" + " does not have enough properties\n", self.logs.getvalue(), ) @@ -574,8 +575,8 @@ def test_handle_validates_schema(self, m_subp): args=None, ) self.assertEqual( - "WARNING: Invalid config:\nsnap: Additional properties are not" - " allowed ('invalid' was unexpected)\n", + "WARNING: Invalid cloud-config provided:\nsnap: Additional" + " properties are not allowed ('invalid' was unexpected)\n", self.logs.getvalue(), ) diff --git a/tests/unittests/config/test_cc_ubuntu_advantage.py b/tests/unittests/config/test_cc_ubuntu_advantage.py index c39e421f680..2037c5ed86b 100644 --- a/tests/unittests/config/test_cc_ubuntu_advantage.py +++ b/tests/unittests/config/test_cc_ubuntu_advantage.py @@ -184,8 +184,8 @@ def test_schema_warns_on_ubuntu_advantage_not_dict(self, _cfg, _): """If ubuntu_advantage configuration is not a dict, emit a warning.""" validate_cloudconfig_schema({"ubuntu_advantage": "wrong type"}, schema) self.assertEqual( - "WARNING: Invalid config:\nubuntu_advantage: 'wrong type' is not" - " of type 'object'\n", + "WARNING: Invalid cloud-config provided:\nubuntu_advantage:" + " 'wrong type' is not of type 'object'\n", self.logs.getvalue(), ) @@ -198,8 +198,9 @@ def test_schema_disallows_unknown_keys(self, _cfg, _): schema, ) self.assertIn( - "WARNING: Invalid config:\nubuntu_advantage: Additional properties" - " are not allowed ('invalid-key' was unexpected)", + "WARNING: Invalid cloud-config provided:\nubuntu_advantage:" + " Additional properties are not allowed ('invalid-key' was" + " unexpected)", self.logs.getvalue(), ) @@ -211,7 +212,7 @@ def test_warn_schema_requires_token(self, _cfg, _): {"ubuntu_advantage": {"enable": ["esm"]}}, schema ) self.assertEqual( - "WARNING: Invalid config:\nubuntu_advantage:" + "WARNING: Invalid cloud-config provided:\nubuntu_advantage:" " 'token' is a required property\n", self.logs.getvalue(), ) @@ -224,9 +225,9 @@ def test_warn_schema_services_is_not_list_or_dict(self, _cfg, _): {"ubuntu_advantage": {"enable": "needslist"}}, schema ) self.assertEqual( - "WARNING: Invalid config:\nubuntu_advantage: 'token' is a" - " required property\nubuntu_advantage.enable: 'needslist'" - " is not of type 'array'\n", + "WARNING: Invalid cloud-config provided:\nubuntu_advantage:" + " 'token' is a required property\nubuntu_advantage.enable:" + " 'needslist' is not of type 'array'\n", self.logs.getvalue(), ) diff --git a/tests/unittests/config/test_cc_write_files.py b/tests/unittests/config/test_cc_write_files.py index 7eea99d3480..faea5885aa2 100644 --- a/tests/unittests/config/test_cc_write_files.py +++ b/tests/unittests/config/test_cc_write_files.py @@ -79,9 +79,11 @@ def test_schema_validation_warns_missing_path(self, m_write_files): cc = self.tmp_cloud("ubuntu") valid_config = {"write_files": [{"path": "/some/path"}]} handle("cc_write_file", valid_config, cc, LOG, []) - self.assertNotIn("Invalid config:", self.logs.getvalue()) + self.assertNotIn( + "Invalid cloud-config provided:", self.logs.getvalue() + ) handle("cc_write_file", INVALID_SCHEMA, cc, LOG, []) - self.assertIn("Invalid config:", self.logs.getvalue()) + self.assertIn("Invalid cloud-config provided:", self.logs.getvalue()) self.assertIn("'path' is a required property", self.logs.getvalue()) def test_schema_validation_warns_non_string_type_for_files( @@ -105,7 +107,7 @@ def test_schema_validation_warns_non_string_type_for_files( "write_files.0.%s: 1 is not of type '%s'" % (key, key_type), self.logs.getvalue(), ) - self.assertIn("Invalid config:", self.logs.getvalue()) + self.assertIn("Invalid cloud-config provided:", self.logs.getvalue()) def test_schema_validation_warns_on_additional_undefined_propertes( self, m_write_files @@ -116,8 +118,8 @@ def test_schema_validation_warns_on_additional_undefined_propertes( invalid_config["write_files"][0]["bogus"] = "value" handle("cc_write_file", invalid_config, cc, LOG, []) self.assertIn( - "Invalid config:\nwrite_files.0: Additional properties" - " are not allowed ('bogus' was unexpected)", + "Invalid cloud-config provided:\nwrite_files.0: Additional" + " properties are not allowed ('bogus' was unexpected)", self.logs.getvalue(), ) @@ -139,7 +141,8 @@ def test_handler_schema_validation_warns_non_array_type(self): with self.assertRaises(TypeError): handle("cc_write_file", invalid_config, cc, LOG, []) self.assertIn( - "Invalid config:\nwrite_files: 1 is not of type 'array'", + "Invalid cloud-config provided:\nwrite_files: 1 is not of type" + " 'array'", self.logs.getvalue(), ) diff --git a/tests/unittests/config/test_cc_write_files_deferred.py b/tests/unittests/config/test_cc_write_files_deferred.py index 3faac1bf20f..172032334bd 100644 --- a/tests/unittests/config/test_cc_write_files_deferred.py +++ b/tests/unittests/config/test_cc_write_files_deferred.py @@ -43,9 +43,11 @@ def test_schema_validation_warns_invalid_value( cc = self.tmp_cloud("ubuntu") handle("cc_write_files_deferred", valid_config, cc, LOG, []) - self.assertNotIn("Invalid config:", self.logs.getvalue()) + self.assertNotIn( + "Invalid cloud-config provided:", self.logs.getvalue() + ) handle("cc_write_files_deferred", invalid_config, cc, LOG, []) - self.assertIn("Invalid config:", self.logs.getvalue()) + self.assertIn("Invalid cloud-config provided:", self.logs.getvalue()) self.assertIn( "defer: 'no' is not of type 'boolean'", self.logs.getvalue() ) diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 3a0eb1ad9b5..8f9d3365ae6 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -203,8 +203,9 @@ def test_validateconfig_schema_non_strict_emits_warnings(self, caplog): """When strict is False validate_cloudconfig_schema emits warnings.""" schema = {"properties": {"p1": {"type": "string"}}} validate_cloudconfig_schema({"p1": -1}, schema, strict=False) - assert "Invalid config:\np1: -1 is not of type 'string'\n" in ( - caplog.text + assert ( + "Invalid cloud-config provided:\np1: -1 is not of type 'string'\n" + in (caplog.text) ) @skipUnlessJsonSchema() From 5e8ff02b4021f7f8c964b7589fb581917bafa85d Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Sat, 8 Jan 2022 16:05:22 -0700 Subject: [PATCH 07/20] integration test --- tests/integration_tests/modules/test_cli.py | 43 ++++++++++++++++++++- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/integration_tests/modules/test_cli.py b/tests/integration_tests/modules/test_cli.py index 97bfe52d7ef..81a5e7a8a6a 100644 --- a/tests/integration_tests/modules/test_cli.py +++ b/tests/integration_tests/modules/test_cli.py @@ -13,11 +13,18 @@ - echo 'hi' > /var/tmp/test """ -INVALID_USER_DATA = """\ +INVALID_USER_DATA_HEADER = """\ runcmd: - echo 'hi' > /var/tmp/test """ +INVALID_USER_DATA_SCHEMA = """\ +#cloud-config +updates: + notnetwork: -1 +apt_pipelining: bogus +""" + @pytest.mark.sru_2020_11 @pytest.mark.user_data(VALID_USER_DATA) @@ -29,10 +36,15 @@ def test_valid_userdata(client: IntegrationInstance): result = client.execute("cloud-init devel schema --system") assert result.ok assert "Valid cloud-config: system userdata" == result.stdout.strip() + result = client.execute("cloud-init status --long") + if not result.ok: + raise AssertionError( + f"Unexpected error from cloud-init status: {result}" + ) @pytest.mark.sru_2020_11 -@pytest.mark.user_data(INVALID_USER_DATA) +@pytest.mark.user_data(INVALID_USER_DATA_HEADER) def test_invalid_userdata(client: IntegrationInstance): """Test `cloud-init devel schema` with invalid userdata. @@ -42,3 +54,30 @@ def test_invalid_userdata(client: IntegrationInstance): assert not result.ok assert "Cloud config schema errors" in result.stderr assert 'needs to begin with "#cloud-config"' in result.stderr + result = client.execute("cloud-init status --long") + if not result.ok: + raise AssertionError( + f"Unexpected error from cloud-init status: {result}" + ) + + +@pytest.mark.user_data(INVALID_USER_DATA_SCHEMA) +def test_invalid_userdata_schema(client: IntegrationInstance): + """Test invalid schema represented as Warnings, not fatal + + PR #1175 + """ + result = client.execute("cloud-init status --long") + assert result.ok + log = client.read_from_file("/var/log/cloud-init.log") + warning = ( + "[WARNING]: Invalid cloud-config provided:\napt_pipelining: 'bogus'" + " is not valid under any of the given schemas\nupdates: Additional" + " properties are not allowed ('notnetwork' was unexpected)" + ) + assert warning in log + result = client.execute("cloud-init status --long") + if not result.ok: + raise AssertionError( + f"Unexpected error from cloud-init status: {result}" + ) From 2ea7fce27a67594177bb313516063e7004b0599f Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 11 Jan 2022 17:15:17 -0700 Subject: [PATCH 08/20] schema: warn when multiple schema files present, but use the first one --- cloudinit/config/schema.py | 14 ++++-- conftest.py | 1 + tests/unittests/config/test_schema.py | 71 +++++++++++++++++++++++++-- 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index 3fb5dbe8282..7e445043378 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -633,11 +633,19 @@ def get_schema() -> dict: os.path.join(paths.schema_dir, "cloud-init-schema-*") ) full_schema = None - for schema_file in schema_files: + if len(schema_files) > 1: + LOG.warning( + "Found multiple cloud-init-schema files in %s. Ignoring %s", + paths.schema_dir, + ", ".join(schema_files[1:]), + ) + elif len(schema_files) == 1: try: - full_schema = json.loads(load_file(schema_file)) + full_schema = json.loads(load_file(schema_files[0])) except Exception as e: - LOG.warning("Cannot parse JSON schema file %s. %s", schema_file, e) + LOG.warning( + "Cannot parse JSON schema file %s. %s", schema_files[0], e + ) if not full_schema: LOG.warning( "No base JSON schema files found at %s/cloud-init-schema-*." diff --git a/conftest.py b/conftest.py index 3979eb0ab26..6ec78b0a92c 100644 --- a/conftest.py +++ b/conftest.py @@ -198,6 +198,7 @@ def paths(tmpdir): (This uses the builtin tmpdir fixture.) """ dirs = { + "schema_dir": tmpdir.mkdir("schema").strpath, "cloud_dir": tmpdir.mkdir("cloud_dir").strpath, "run_dir": tmpdir.mkdir("run_dir").strpath, } diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 8f9d3365ae6..1644f47aaa5 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -29,6 +29,7 @@ validate_cloudconfig_metaschema, validate_cloudconfig_schema, ) +from cloudinit.util import copy as util_copy from cloudinit.util import write_file from tests.unittests.helpers import ( CiTestCase, @@ -82,8 +83,40 @@ def get_module_variable(var_name) -> dict: class TestGetSchema: - def test_get_schema_coalesces_known_schema(self): + @mock.patch("cloudinit.config.schema.read_cfg_paths") + def test_get_schema_warn_on_multiple_schema_files( + self, read_cfg_paths, paths, caplog + ): + """Warn about ignored files when multiple schemas in schema_dir.""" + read_cfg_paths.return_value = paths + + for schema_file in Path(cloud_init_project_dir("config/")).glob( + "cloud-init-schema*.json" + ): + util_copy(schema_file, paths.schema_dir) + # Add a duplicate entry that'll be warned + duplicate_schema = ( + f"{paths.schema_dir}/{schema_file.name}".replace( + ".json", ".2.json" + ) + ) + util_copy(schema_file, duplicate_schema) + get_schema() + assert ( + f"Found multiple cloud-init-schema files in {paths.schema_dir}" + in caplog.text + ) + + @mock.patch("cloudinit.config.schema.read_cfg_paths") + def test_get_schema_coalesces_known_schema(self, read_cfg_paths, paths): """Every cloudconfig module with schema is listed in allOf keyword.""" + read_cfg_paths.return_value = paths + + # Copy existing packages base schema files into paths.schema_dir + for schema_file in Path(cloud_init_project_dir("config/")).glob( + "cloud-init-schema*.json" + ): + util_copy(schema_file, paths.schema_dir) schema = get_schema() assert sorted( [ @@ -108,9 +141,39 @@ def test_get_schema_coalesces_known_schema(self): [meta["id"] for meta in get_metas().values() if meta is not None] ) assert "http://json-schema.org/draft-04/schema#" == schema["$schema"] - assert ["$defs", "$schema", "allOf"] == sorted( - list(get_schema().keys()) - ) + assert ["$defs", "$schema", "allOf"] == sorted(list(schema.keys())) + # New style schema should be defined in static schema file in $defs + expected_subschema_defs = [ + {"$ref": "#/$defs/cc_apt_pipelining"}, + ] + found_subschema_defs = [] + legacy_schema_keys = [] + for subschema in schema["allOf"]: + if "$ref" in subschema: + found_subschema_defs.append(subschema) + else: # Legacy subschema sourced from cc_* module 'schema' attr + 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 [ + "apk_repos", + "apt", + "bootcmd", + "chef", + "drivers", + "locale", + "locale_configfile", + "ntp", + "resize_rootfs", + "runcmd", + "snap", + "ubuntu_advantage", + "updates", + "write_files", + "write_files", + "zypper", + ] == sorted(legacy_schema_keys) class TestLoadDoc: From cf10af4e6cd60b2dcbed9bc7794e046537cb4228 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 11 Jan 2022 21:03:36 -0700 Subject: [PATCH 09/20] tests --- .../config/test_cc_apt_pipelining.py | 14 ++++++++------ tests/unittests/config/test_schema.py | 19 +++++++++++++------ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/tests/unittests/config/test_cc_apt_pipelining.py b/tests/unittests/config/test_cc_apt_pipelining.py index b4497156893..c6a8e68e2a9 100644 --- a/tests/unittests/config/test_cc_apt_pipelining.py +++ b/tests/unittests/config/test_cc_apt_pipelining.py @@ -3,15 +3,15 @@ """Tests cc_apt_pipelining handler""" import cloudinit.config.cc_apt_pipelining as cc_apt_pipelining -from tests.unittests.helpers import CiTestCase, mock +from tests.unittests.helpers import mock -class TestAptPipelining(CiTestCase): +class TestAptPipelining: @mock.patch("cloudinit.config.cc_apt_pipelining.util.write_file") def test_not_disabled_by_default(self, m_write_file): """ensure that default behaviour is to not disable pipelining""" cc_apt_pipelining.handle("foo", {}, None, mock.MagicMock(), None) - self.assertEqual(0, m_write_file.call_count) + assert 0 == m_write_file.call_count @mock.patch("cloudinit.config.cc_apt_pipelining.util.write_file") def test_false_disables_pipelining(self, m_write_file): @@ -19,10 +19,12 @@ def test_false_disables_pipelining(self, m_write_file): cc_apt_pipelining.handle( "foo", {"apt_pipelining": "false"}, None, mock.MagicMock(), None ) - self.assertEqual(1, m_write_file.call_count) + assert 1 == m_write_file.call_count args, _ = m_write_file.call_args - self.assertEqual(cc_apt_pipelining.DEFAULT_FILE, args[0]) - self.assertIn('Pipeline-Depth "0"', args[1]) + assert cc_apt_pipelining.DEFAULT_FILE == args[0] + assert 'Pipeline-Depth "0"' in args[1] + + def test_failure_schema_ # vi: ts=4 expandtab diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 1644f47aaa5..edcbdc77796 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -40,7 +40,7 @@ def get_schemas() -> dict: - """Return all module schemas + """Return all legacy module schemas Assumes that module schemas have the variable name "schema" """ @@ -336,7 +336,6 @@ def test_validateconfig_strict_metaschema_do_not_raise_exception( class TestCloudConfigExamples: - schema = get_schemas() metas = get_metas() params = [ (meta["id"], example) @@ -347,14 +346,22 @@ class TestCloudConfigExamples: @pytest.mark.parametrize("schema_id, example", params) @skipUnlessJsonSchema() - def test_validateconfig_schema_of_example(self, schema_id, example): + @mock.patch("cloudinit.config.schema.read_cfg_paths") + def test_validateconfig_schema_of_example( + self, read_cfg_paths, schema_id, example, paths + ): """For a given example in a config module we test if it is valid according to the unified schema of all config modules """ + read_cfg_paths.return_value = paths + # New-style schema $defs exist in config/cloud-init-schema*.json + for schema_file in Path(cloud_init_project_dir("config/")).glob( + "cloud-init-schema*.json" + ): + util_copy(schema_file, paths.schema_dir) + schema = get_schema() config_load = safe_load(example) - validate_cloudconfig_schema( - config_load, self.schema[schema_id], strict=True - ) + validate_cloudconfig_schema(config_load, schema, strict=True) class ValidateCloudConfigFileTest(CiTestCase): From dd7e36320de526fb6eaa91e1173396b288b0165a Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 12 Jan 2022 06:52:23 -0700 Subject: [PATCH 10/20] tests: apt_pipelining schema tests --- .../config/test_cc_apt_pipelining.py | 54 +++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/tests/unittests/config/test_cc_apt_pipelining.py b/tests/unittests/config/test_cc_apt_pipelining.py index c6a8e68e2a9..d19533b38bc 100644 --- a/tests/unittests/config/test_cc_apt_pipelining.py +++ b/tests/unittests/config/test_cc_apt_pipelining.py @@ -2,8 +2,22 @@ """Tests cc_apt_pipelining handler""" +from pathlib import Path + +import pytest + import cloudinit.config.cc_apt_pipelining as cc_apt_pipelining -from tests.unittests.helpers import mock +from cloudinit import util +from cloudinit.config.schema import ( + SchemaValidationError, + get_schema, + validate_cloudconfig_schema, +) +from tests.unittests.helpers import ( + cloud_init_project_dir, + mock, + skipUnlessJsonSchema, +) class TestAptPipelining: @@ -19,12 +33,46 @@ def test_false_disables_pipelining(self, m_write_file): cc_apt_pipelining.handle( "foo", {"apt_pipelining": "false"}, None, mock.MagicMock(), None ) - assert 1 == m_write_file.call_count + assert 1 == m_write_file.call_count args, _ = m_write_file.call_args assert cc_apt_pipelining.DEFAULT_FILE == args[0] assert 'Pipeline-Depth "0"' in args[1] - def test_failure_schema_ + @pytest.mark.parametrize( + "config, error_msg", + ( + # Valid schemas + ({}, None), + ({"apt_pipelining": 1}, None), + ({"apt_pipelining": True}, None), + ({"apt_pipelining": False}, None), + ({"apt_pipelining": "none"}, None), + ({"apt_pipelining": "unchanged"}, None), + ({"apt_pipelining": "os"}, None), + # Invalid schemas + ( + {"apt_pipelining": "bogus"}, + "Cloud config schema errors: apt_pipelining: 'bogus' is not" + " valid under any of the given schema", + ), + ), + ) + @skipUnlessJsonSchema() + @mock.patch("cloudinit.config.schema.read_cfg_paths") + def test_schema_vaidatoin(self, read_cfg_paths, config, error_msg, paths): + """Assert expected schema validation and error messages.""" + read_cfg_paths.return_value = paths + # New-style schema $defs exist in config/cloud-init-schema*.json + for schema_file in Path(cloud_init_project_dir("config/")).glob( + "cloud-init-schema*.json" + ): + util.copy(schema_file, paths.schema_dir) + schema = get_schema() + if error_msg is None: + validate_cloudconfig_schema(config, schema, strict=True) + else: + with pytest.raises(SchemaValidationError, match=error_msg): + validate_cloudconfig_schema(config, schema, strict=True) # vi: ts=4 expandtab From 1710170c6209e57a862e52956e9834a3aa00e19a Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 12 Jan 2022 16:23:21 -0700 Subject: [PATCH 11/20] tests: apt_pipelining spelling --- tests/unittests/config/test_cc_apt_pipelining.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unittests/config/test_cc_apt_pipelining.py b/tests/unittests/config/test_cc_apt_pipelining.py index d19533b38bc..650f9947485 100644 --- a/tests/unittests/config/test_cc_apt_pipelining.py +++ b/tests/unittests/config/test_cc_apt_pipelining.py @@ -59,7 +59,7 @@ def test_false_disables_pipelining(self, m_write_file): ) @skipUnlessJsonSchema() @mock.patch("cloudinit.config.schema.read_cfg_paths") - def test_schema_vaidatoin(self, read_cfg_paths, config, error_msg, paths): + def test_schema_validation(self, read_cfg_paths, config, error_msg, paths): """Assert expected schema validation and error messages.""" read_cfg_paths.return_value = paths # New-style schema $defs exist in config/cloud-init-schema*.json From 3bf02205ce47d53227a9f96035b6f3a56e52e0e6 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 12 Jan 2022 16:55:47 -0700 Subject: [PATCH 12/20] schema: migrate cc_apk_configure and add schema tests --- cloudinit/config/cc_apk_configure.py | 103 +----------------- config/cloud-init-schema-1.0.json | 49 +++++++++ .../unittests/config/test_cc_apk_configure.py | 96 +++++++++++++++- tests/unittests/config/test_schema.py | 2 +- 4 files changed, 147 insertions(+), 103 deletions(-) diff --git a/cloudinit/config/cc_apk_configure.py b/cloudinit/config/cc_apk_configure.py index a615c814b58..2cb2dad188c 100644 --- a/cloudinit/config/cc_apk_configure.py +++ b/cloudinit/config/cc_apk_configure.py @@ -10,7 +10,7 @@ from cloudinit import log as logging from cloudinit import temp_utils, templater, util -from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema +from cloudinit.config.schema import get_meta_doc from cloudinit.settings import PER_INSTANCE LOG = logging.getLogger(__name__) @@ -102,104 +102,7 @@ "frequency": frequency, } -schema = { - "type": "object", - "properties": { - "apk_repos": { - "type": "object", - "properties": { - "preserve_repositories": { - "type": "boolean", - "default": False, - "description": dedent( - """\ - By default, cloud-init will generate a new repositories - file ``/etc/apk/repositories`` based on any valid - configuration settings specified within a apk_repos - section of cloud config. To disable this behavior and - preserve the repositories file from the pristine image, - set ``preserve_repositories`` to ``true``. - - The ``preserve_repositories`` option overrides - all other config keys that would alter - ``/etc/apk/repositories``. - """ - ), - }, - "alpine_repo": { - "type": ["object", "null"], - "properties": { - "base_url": { - "type": "string", - "default": DEFAULT_MIRROR, - "description": dedent( - """\ - The base URL of an Alpine repository, or - mirror, to download official packages from. - If not specified then it defaults to ``{}`` - """.format( - DEFAULT_MIRROR - ) - ), - }, - "community_enabled": { - "type": "boolean", - "default": False, - "description": dedent( - """\ - Whether to add the Community repo to the - repositories file. By default the Community - repo is not included. - """ - ), - }, - "testing_enabled": { - "type": "boolean", - "default": False, - "description": dedent( - """\ - Whether to add the Testing repo to the - repositories file. By default the Testing - repo is not included. It is only recommended - to use the Testing repo on a machine running - the ``Edge`` version of Alpine as packages - installed from Testing may have dependancies - that conflict with those in non-Edge Main or - Community repos." - """ - ), - }, - "version": { - "type": "string", - "description": dedent( - """\ - The Alpine version to use (e.g. ``v3.12`` or - ``edge``) - """ - ), - }, - }, - "required": ["version"], - "minProperties": 1, - "additionalProperties": False, - }, - "local_repo_base_url": { - "type": "string", - "description": dedent( - """\ - The base URL of an Alpine repository containing - unofficial packages - """ - ), - }, - }, - "minProperties": 1, # Either preserve_repositories or alpine_repo - "additionalProperties": False, - } - }, -} - -__doc__ = get_meta_doc(meta, schema) +__doc__ = get_meta_doc(meta) def handle(name, cfg, cloud, log, _args): @@ -222,8 +125,6 @@ def handle(name, cfg, cloud, log, _args): ) return - validate_cloudconfig_schema(cfg, schema) - # If "preserve_repositories" is explicitly set to True in # the configuration do nothing. if util.get_cfg_option_bool(apk_section, "preserve_repositories", False): diff --git a/config/cloud-init-schema-1.0.json b/config/cloud-init-schema-1.0.json index b2e8bd21ff9..afaed285ece 100644 --- a/config/cloud-init-schema-1.0.json +++ b/config/cloud-init-schema-1.0.json @@ -1,6 +1,54 @@ { "$schema": "http://json-schema.org/draft-04/schema#", "$defs": { + "cc_apk_configure": { + "type": "object", + "properties": { + "apk_repos": { + "type": "object", + "properties": { + "preserve_repositories": { + "type": "boolean", + "default": false, + "description": "By default, cloud-init will generate a new repositories file ``/etc/apk/repositories`` based on any valid configuration settings specified within a apk_repos section of cloud config. To disable this behavior and preserve the repositories file from the pristine image, set ``preserve_repositories`` to ``true``.\n\n The ``preserve_repositories`` option overrides all other config keys that would alter ``/etc/apk/repositories``." + }, + "alpine_repo": { + "type": ["object", "null"], + "properties": { + "base_url": { + "type": "string", + "default": "https://alpine.global.ssl.fastly.net/alpine", + "description": "The base URL of an Alpine repository, or mirror, to download official packages from. If not specified then it defaults to ``https://alpine.global.ssl.fastly.net/alpine``" + }, + "community_enabled": { + "type": "boolean", + "default": false, + "description": "Whether to add the Community repo to the repositories file. By default the Community repo is not included." + }, + "testing_enabled": { + "type": "boolean", + "default": false, + "description": "Whether to add the Testing repo to the repositories file. By default the Testing repo is not included. It is only recommended to use the Testing repo on a machine running the ``Edge`` version of Alpine as packages installed from Testing may have dependancies that conflict with those in non-Edge Main or Community repos." + }, + "version": { + "type": "string", + "description": "The Alpine version to use (e.g. ``v3.12`` or ``edge``)" + } + }, + "required": ["version"], + "minProperties": 1, + "additionalProperties": false + }, + "local_repo_base_url": { + "type": "string", + "description": "The base URL of an Alpine repository containing unofficial packages" + } + }, + "minProperties": 1, + "additionalProperties": false + } + } + }, "cc_apt_pipelining": { "type": "object", "properties": { @@ -15,6 +63,7 @@ } }, "allOf": [ + { "$ref": "#/$defs/cc_apk_configure" }, { "$ref": "#/$defs/cc_apt_pipelining" } ] } diff --git a/tests/unittests/config/test_cc_apk_configure.py b/tests/unittests/config/test_cc_apk_configure.py index 6fbc3dec7e3..c422f80fcf7 100644 --- a/tests/unittests/config/test_cc_apk_configure.py +++ b/tests/unittests/config/test_cc_apk_configure.py @@ -6,11 +6,25 @@ import logging import os +import re import textwrap +from pathlib import Path + +import pytest from cloudinit import cloud, helpers, util from cloudinit.config import cc_apk_configure -from tests.unittests.helpers import FilesystemMockingTestCase, mock +from cloudinit.config.schema import ( + SchemaValidationError, + get_schema, + validate_cloudconfig_schema, +) +from tests.unittests.helpers import ( + FilesystemMockingTestCase, + cloud_init_project_dir, + mock, + skipUnlessJsonSchema, +) REPO_FILE = "/etc/apk/repositories" DEFAULT_MIRROR_URL = "https://alpine.global.ssl.fastly.net/alpine" @@ -322,4 +336,84 @@ def test_edge_main_community_testing_local_repos(self): self.assertEqual(expected_content, util.load_file(REPO_FILE)) +class TestApkConfigureSchema: + @pytest.mark.parametrize( + "config, error_msg", + ( + # Valid schemas + ({"apk_repos": {"preserve_repositories": True}}, None), + ({"apk_repos": {"alpine_repo": None}}, None), + ({"apk_repos": {"alpine_repo": {"version": "v3.21"}}}, None), + ( + { + "apk_repos": { + "alpine_repo": { + "base_url": "http://yep", + "community_enabled": True, + "testing_enabled": True, + "version": "v3.21", + } + } + }, + None, + ), + ({"apk_repos": {"local_repo_base_url": "http://some"}}, None), + # Invalid schemas + ( + {"apk_repos": {"alpine_repo": {"version": False}}}, + "apk_repos.alpine_repo.version: False is not of type" + " 'string'", + ), + ( + { + "apk_repos": { + "alpine_repo": {"version": "v3.12", "bogus": 1} + } + }, + re.escape( + "apk_repos.alpine_repo: Additional properties are not" + " allowed ('bogus' was unexpected)" + ), + ), + ( + {"apk_repos": {"alpine_repo": {}}}, + "apk_repos.alpine_repo: 'version' is a required property," + " apk_repos.alpine_repo: {} does not have enough properties", + ), + ( + {"apk_repos": {"alpine_repo": True}}, + "apk_repos.alpine_repo: True is not of type 'object', 'null'", + ), + ( + {"apk_repos": {"preserve_repositories": "wrongtype"}}, + "apk_repos.preserve_repositories: 'wrongtype' is not of type" + " 'boolean'", + ), + ( + {"apk_repos": {}}, + "apk_repos: {} does not have enough properties", + ), + ( + {"apk_repos": {"local_repo_base_url": None}}, + "apk_repos.local_repo_base_url: None is not of type 'string'", + ), + ), + ) + @skipUnlessJsonSchema() + @mock.patch("cloudinit.config.schema.read_cfg_paths") + def test_schema_validation(self, read_cfg_paths, config, error_msg, paths): + read_cfg_paths.return_value = paths + # New-style schema $defs exist in config/cloud-init-schema*.json + for schema_file in Path(cloud_init_project_dir("config/")).glob( + "cloud-init-schema*.json" + ): + util.copy(schema_file, paths.schema_dir) + schema = get_schema() + if error_msg is None: + validate_cloudconfig_schema(config, schema, strict=True) + else: + with pytest.raises(SchemaValidationError, match=error_msg): + validate_cloudconfig_schema(config, schema, strict=True) + + # vi: ts=4 expandtab diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index edcbdc77796..c0f382900c6 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -144,6 +144,7 @@ def test_get_schema_coalesces_known_schema(self, read_cfg_paths, paths): assert ["$defs", "$schema", "allOf"] == sorted(list(schema.keys())) # New style schema should be defined in static schema file in $defs expected_subschema_defs = [ + {"$ref": "#/$defs/cc_apk_configure"}, {"$ref": "#/$defs/cc_apt_pipelining"}, ] found_subschema_defs = [] @@ -157,7 +158,6 @@ def test_get_schema_coalesces_known_schema(self, read_cfg_paths, paths): assert expected_subschema_defs == found_subschema_defs # This list will dwindle as we move legacy schema to new $defs assert [ - "apk_repos", "apt", "bootcmd", "chef", From 2b43a2ef3fb555b9e0b79b4e1bb43746e5ab7460 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Thu, 13 Jan 2022 14:30:30 -0700 Subject: [PATCH 13/20] schema: allow CLOUD_INIT_SCHEMA_DIR env override for Paths.schema_dir To allow for building docs locally in the project-dir, get_schema needs to source ./config/cloud-init-schema*json which is data files outside of the typical python module paths. get_schema will prefer CLOUD_INIT_SCHEMA_DIR environment variable over Paths.schema_dir if provided. Allow tox.ini to passenv CLOUD_INIT_SCHEMA_DIR into the tox env. To generate docs from project directory: CLOUD_INIT_SCHEMA_DIR=config tox -e doc To validate schema docs from project directory: CLOUD_INIT_SCHEMA_DIR=config PYTHONPATH=. python3 \ -m cloudinit.cmd.main devel schema --docs all --- cloudinit/config/cc_apt_pipelining.py | 1 - cloudinit/config/schema.py | 20 +++++++++++++------- doc/rtd/conf.py | 2 +- tox.ini | 2 ++ 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/cloudinit/config/cc_apt_pipelining.py b/cloudinit/config/cc_apt_pipelining.py index 556eb4c40b4..34b6ac0e46d 100644 --- a/cloudinit/config/cc_apt_pipelining.py +++ b/cloudinit/config/cc_apt_pipelining.py @@ -56,7 +56,6 @@ def handle(_name, cfg, _cloud, log, _args): - apt_pipe_value = util.get_cfg_option_str(cfg, "apt_pipelining", "os") apt_pipe_value = cfg.get("apt_pipelining", "os") apt_pipe_value_s = str(apt_pipe_value).lower().strip() diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index 7e445043378..185df518513 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -420,10 +420,17 @@ def _get_property_type(property_dict: dict) -> str: jsonschema. """ property_type = property_dict.get("type") - if property_type is None and property_dict.get("enum"): - property_type = [ - str(_YAML_MAP.get(k, k)) for k in property_dict["enum"] - ] + if property_type is None: + if property_dict.get("enum"): + property_type = [ + str(_YAML_MAP.get(k, k)) for k in property_dict["enum"] + ] + elif property_dict.get("oneOf"): + property_type = [ + subschema["type"] + for subschema in property_dict.get("oneOf") + if subschema.get("type") + ] if isinstance(property_type, list): property_type = "/".join(property_type) items = property_dict.get("items", {}) @@ -629,9 +636,8 @@ def load_doc(requested_modules: list) -> str: def get_schema() -> dict: """Return jsonschema coalesced from all cc_* cloud-config modules.""" paths = read_cfg_paths() - schema_files = glob.glob( - os.path.join(paths.schema_dir, "cloud-init-schema-*") - ) + schema_dir = os.environ.get("CLOUD_INIT_SCHEMA_DIR", paths.schema_dir) + schema_files = glob.glob(os.path.join(schema_dir, "cloud-init-schema-*")) full_schema = None if len(schema_files) > 1: LOG.warning( diff --git a/doc/rtd/conf.py b/doc/rtd/conf.py index d2cb6ae1a42..bbaa44e3815 100644 --- a/doc/rtd/conf.py +++ b/doc/rtd/conf.py @@ -18,7 +18,7 @@ # General information about the project. project = "cloud-init" -copyright = "2020, Canonical Ltd." +copyright = "2022, Canonical Ltd." # -- General configuration ---------------------------------------------------- diff --git a/tox.ini b/tox.ini index 57d18cdb641..9a93eb33b7f 100644 --- a/tox.ini +++ b/tox.ini @@ -104,6 +104,8 @@ deps = commands = {envpython} -m sphinx {posargs:doc/rtd doc/rtd_html} doc8 doc/rtd +passenv= + CLOUD_INIT_SCHEMA_DIR [testenv:tip-flake8] commands = {envpython} -m flake8 {posargs:cloudinit/ tests/ tools/ setup.py} From 392db85106ce723fc32fb5007ed2cd3e8cd1a2b2 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Thu, 13 Jan 2022 15:12:38 -0700 Subject: [PATCH 14/20] rebase includes keyboard legacy schema --- tests/unittests/config/test_schema.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index c0f382900c6..d2526b4478e 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -162,6 +162,7 @@ def test_get_schema_coalesces_known_schema(self, read_cfg_paths, paths): "bootcmd", "chef", "drivers", + "keyboard", "locale", "locale_configfile", "ntp", From 8c40933b8895604e27a921636d08d503d45fae73 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 14 Jan 2022 12:25:37 -0700 Subject: [PATCH 15/20] tox: use setenv in tox so tox -e doc includes defines schema docs --- tox.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index 9a93eb33b7f..591a3561440 100644 --- a/tox.ini +++ b/tox.ini @@ -104,8 +104,8 @@ deps = commands = {envpython} -m sphinx {posargs:doc/rtd doc/rtd_html} doc8 doc/rtd -passenv= - CLOUD_INIT_SCHEMA_DIR +setenv= + CLOUD_INIT_SCHEMA_DIR=./config [testenv:tip-flake8] commands = {envpython} -m flake8 {posargs:cloudinit/ tests/ tools/ setup.py} From 34736095cc145e19f490b197b38dc6b7de2088c6 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 14 Jan 2022 14:57:50 -0700 Subject: [PATCH 16/20] schema: deliver cloud-init-schema.json with python modules. Drop paths.schema_dir --- .../config}/cloud-init-schema-1.0.json | 0 cloudinit/config/schema.py | 29 +++++++------------ cloudinit/helpers.py | 7 +---- cloudinit/settings.py | 1 - setup.py | 1 - tests/unittests/config/test_schema.py | 24 --------------- tox.ini | 2 -- 7 files changed, 11 insertions(+), 53 deletions(-) rename {config => cloudinit/config}/cloud-init-schema-1.0.json (100%) diff --git a/config/cloud-init-schema-1.0.json b/cloudinit/config/cloud-init-schema-1.0.json similarity index 100% rename from config/cloud-init-schema-1.0.json rename to cloudinit/config/cloud-init-schema-1.0.json diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index 185df518513..299cafc2a14 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -2,7 +2,6 @@ """schema.py: Set of module functions for processing cloud-config schema.""" import argparse -import glob import json import logging import os @@ -635,28 +634,20 @@ def load_doc(requested_modules: list) -> str: def get_schema() -> dict: """Return jsonschema coalesced from all cc_* cloud-config modules.""" - paths = read_cfg_paths() - schema_dir = os.environ.get("CLOUD_INIT_SCHEMA_DIR", paths.schema_dir) - schema_files = glob.glob(os.path.join(schema_dir, "cloud-init-schema-*")) + schema_file = os.path.join( + os.path.dirname(os.path.abspath(__file__)), + "cloud-init-schema-1.0.json", + ) full_schema = None - if len(schema_files) > 1: - LOG.warning( - "Found multiple cloud-init-schema files in %s. Ignoring %s", - paths.schema_dir, - ", ".join(schema_files[1:]), - ) - elif len(schema_files) == 1: - try: - full_schema = json.loads(load_file(schema_files[0])) - except Exception as e: - LOG.warning( - "Cannot parse JSON schema file %s. %s", schema_files[0], e - ) + try: + full_schema = json.loads(load_file(schema_file)) + except Exception as e: + LOG.warning("Cannot parse JSON schema file %s. %s", schema_file, e) if not full_schema: LOG.warning( - "No base JSON schema files found at %s/cloud-init-schema-*." + "No base JSON schema files found at %s." " Setting default empty schema", - paths.schema_dir, + schema_file, ) full_schema = { "$defs": {}, diff --git a/cloudinit/helpers.py b/cloudinit/helpers.py index 263dfe0cf45..c2c9e5842bc 100644 --- a/cloudinit/helpers.py +++ b/cloudinit/helpers.py @@ -328,14 +328,13 @@ def items(self): class Paths(persistence.CloudInitPickleMixin): - _ci_pkl_version = 2 + _ci_pkl_version = 1 def __init__(self, path_cfgs, ds=None): self.cfgs = path_cfgs # Populate all the initial paths self.cloud_dir = path_cfgs.get("cloud_dir", "/var/lib/cloud") self.run_dir = path_cfgs.get("run_dir", "/run/cloud-init") - self.schema_dir = path_cfgs.get("schema_dir", "/etc/cloud/schema") self.instance_link = os.path.join(self.cloud_dir, "instance") self.boot_finished = os.path.join(self.instance_link, "boot-finished") self.upstart_conf_d = path_cfgs.get("upstart_dir") @@ -378,10 +377,6 @@ def _unpickle(self, ci_pkl_version: int) -> None: self.run_dir = Paths( path_cfgs=self.cfgs, ds=self.datasource ).run_dir - if not hasattr(self, "schema_dir"): - self.schema_dir = Paths( - path_cfgs=self.cfgs, ds=self.datasource - ).schema_dir # get_ipath_cur: get the current instance path for an item def get_ipath_cur(self, name=None): diff --git a/cloudinit/settings.py b/cloudinit/settings.py index 39067542ea2..ecc1403bd9b 100644 --- a/cloudinit/settings.py +++ b/cloudinit/settings.py @@ -53,7 +53,6 @@ "syslog_fix_perms": ["syslog:adm", "root:adm", "root:wheel", "root:root"], "system_info": { "paths": { - "schema_dir": "/etc/cloud/schema", "cloud_dir": "/var/lib/cloud", "templates_dir": "/etc/cloud/templates/", }, diff --git a/setup.py b/setup.py index 225269ee826..c98a4703d81 100755 --- a/setup.py +++ b/setup.py @@ -278,7 +278,6 @@ def finalize_options(self): (ETC + "/cloud", [render_tmpl("config/cloud.cfg.tmpl")]), (ETC + "/cloud/cloud.cfg.d", glob("config/cloud.cfg.d/*")), (ETC + "/cloud/templates", glob("templates/*")), - (ETC + "/cloud/schema", glob("config/cloud-init-schema-*")), ( USR_LIB_EXEC + "/cloud-init", [ diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index d2526b4478e..8f399d190e4 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -83,30 +83,6 @@ def get_module_variable(var_name) -> dict: class TestGetSchema: - @mock.patch("cloudinit.config.schema.read_cfg_paths") - def test_get_schema_warn_on_multiple_schema_files( - self, read_cfg_paths, paths, caplog - ): - """Warn about ignored files when multiple schemas in schema_dir.""" - read_cfg_paths.return_value = paths - - for schema_file in Path(cloud_init_project_dir("config/")).glob( - "cloud-init-schema*.json" - ): - util_copy(schema_file, paths.schema_dir) - # Add a duplicate entry that'll be warned - duplicate_schema = ( - f"{paths.schema_dir}/{schema_file.name}".replace( - ".json", ".2.json" - ) - ) - util_copy(schema_file, duplicate_schema) - get_schema() - assert ( - f"Found multiple cloud-init-schema files in {paths.schema_dir}" - in caplog.text - ) - @mock.patch("cloudinit.config.schema.read_cfg_paths") def test_get_schema_coalesces_known_schema(self, read_cfg_paths, paths): """Every cloudconfig module with schema is listed in allOf keyword.""" diff --git a/tox.ini b/tox.ini index 591a3561440..57d18cdb641 100644 --- a/tox.ini +++ b/tox.ini @@ -104,8 +104,6 @@ deps = commands = {envpython} -m sphinx {posargs:doc/rtd doc/rtd_html} doc8 doc/rtd -setenv= - CLOUD_INIT_SCHEMA_DIR=./config [testenv:tip-flake8] commands = {envpython} -m flake8 {posargs:cloudinit/ tests/ tools/ setup.py} From d47ebc31d3b28c03a3c94a7ebda12e685122dfe3 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 14 Jan 2022 15:12:43 -0700 Subject: [PATCH 17/20] schema: do not version cloud-init-schema.json yet --- ...t-schema-1.0.json => cloud-init-schema.json} | 0 cloudinit/config/schema.py | 3 +-- tests/unittests/config/test_cc_apk_configure.py | 11 +---------- .../unittests/config/test_cc_apt_pipelining.py | 17 ++--------------- tests/unittests/config/test_schema.py | 10 +--------- 5 files changed, 5 insertions(+), 36 deletions(-) rename cloudinit/config/{cloud-init-schema-1.0.json => cloud-init-schema.json} (100%) diff --git a/cloudinit/config/cloud-init-schema-1.0.json b/cloudinit/config/cloud-init-schema.json similarity index 100% rename from cloudinit/config/cloud-init-schema-1.0.json rename to cloudinit/config/cloud-init-schema.json diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index 299cafc2a14..1f969c97e71 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -635,8 +635,7 @@ def load_doc(requested_modules: list) -> str: def get_schema() -> dict: """Return jsonschema coalesced from all cc_* cloud-config modules.""" schema_file = os.path.join( - os.path.dirname(os.path.abspath(__file__)), - "cloud-init-schema-1.0.json", + os.path.dirname(os.path.abspath(__file__)), "cloud-init-schema.json" ) full_schema = None try: diff --git a/tests/unittests/config/test_cc_apk_configure.py b/tests/unittests/config/test_cc_apk_configure.py index c422f80fcf7..85dd028f397 100644 --- a/tests/unittests/config/test_cc_apk_configure.py +++ b/tests/unittests/config/test_cc_apk_configure.py @@ -8,7 +8,6 @@ import os import re import textwrap -from pathlib import Path import pytest @@ -21,7 +20,6 @@ ) from tests.unittests.helpers import ( FilesystemMockingTestCase, - cloud_init_project_dir, mock, skipUnlessJsonSchema, ) @@ -400,14 +398,7 @@ class TestApkConfigureSchema: ), ) @skipUnlessJsonSchema() - @mock.patch("cloudinit.config.schema.read_cfg_paths") - def test_schema_validation(self, read_cfg_paths, config, error_msg, paths): - read_cfg_paths.return_value = paths - # New-style schema $defs exist in config/cloud-init-schema*.json - for schema_file in Path(cloud_init_project_dir("config/")).glob( - "cloud-init-schema*.json" - ): - util.copy(schema_file, paths.schema_dir) + def test_schema_validation(self, config, error_msg): schema = get_schema() if error_msg is None: validate_cloudconfig_schema(config, schema, strict=True) diff --git a/tests/unittests/config/test_cc_apt_pipelining.py b/tests/unittests/config/test_cc_apt_pipelining.py index 650f9947485..0f72d32b5bc 100644 --- a/tests/unittests/config/test_cc_apt_pipelining.py +++ b/tests/unittests/config/test_cc_apt_pipelining.py @@ -2,22 +2,15 @@ """Tests cc_apt_pipelining handler""" -from pathlib import Path - import pytest import cloudinit.config.cc_apt_pipelining as cc_apt_pipelining -from cloudinit import util from cloudinit.config.schema import ( SchemaValidationError, get_schema, validate_cloudconfig_schema, ) -from tests.unittests.helpers import ( - cloud_init_project_dir, - mock, - skipUnlessJsonSchema, -) +from tests.unittests.helpers import mock, skipUnlessJsonSchema class TestAptPipelining: @@ -58,15 +51,9 @@ def test_false_disables_pipelining(self, m_write_file): ), ) @skipUnlessJsonSchema() - @mock.patch("cloudinit.config.schema.read_cfg_paths") - def test_schema_validation(self, read_cfg_paths, config, error_msg, paths): + def test_schema_validation(self, config, error_msg): """Assert expected schema validation and error messages.""" - read_cfg_paths.return_value = paths # New-style schema $defs exist in config/cloud-init-schema*.json - for schema_file in Path(cloud_init_project_dir("config/")).glob( - "cloud-init-schema*.json" - ): - util.copy(schema_file, paths.schema_dir) schema = get_schema() if error_msg is None: validate_cloudconfig_schema(config, schema, strict=True) diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 8f399d190e4..0afdb32ba07 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -83,16 +83,8 @@ def get_module_variable(var_name) -> dict: class TestGetSchema: - @mock.patch("cloudinit.config.schema.read_cfg_paths") - def test_get_schema_coalesces_known_schema(self, read_cfg_paths, paths): + def test_get_schema_coalesces_known_schema(self): """Every cloudconfig module with schema is listed in allOf keyword.""" - read_cfg_paths.return_value = paths - - # Copy existing packages base schema files into paths.schema_dir - for schema_file in Path(cloud_init_project_dir("config/")).glob( - "cloud-init-schema*.json" - ): - util_copy(schema_file, paths.schema_dir) schema = get_schema() assert sorted( [ From bbbbc43f43c5370aebc1ba19a0f34d04497c2e0e Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 14 Jan 2022 15:16:18 -0700 Subject: [PATCH 18/20] drop now-irrelevant paths.schema_dir from tests --- doc/rtd/conf.py | 2 +- tests/unittests/config/test_schema.py | 12 +----------- tests/unittests/test_upgrade.py | 3 --- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/doc/rtd/conf.py b/doc/rtd/conf.py index bbaa44e3815..d2cb6ae1a42 100644 --- a/doc/rtd/conf.py +++ b/doc/rtd/conf.py @@ -18,7 +18,7 @@ # General information about the project. project = "cloud-init" -copyright = "2022, Canonical Ltd." +copyright = "2020, Canonical Ltd." # -- General configuration ---------------------------------------------------- diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 0afdb32ba07..5cb00c5d419 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -29,7 +29,6 @@ validate_cloudconfig_metaschema, validate_cloudconfig_schema, ) -from cloudinit.util import copy as util_copy from cloudinit.util import write_file from tests.unittests.helpers import ( CiTestCase, @@ -315,19 +314,10 @@ class TestCloudConfigExamples: @pytest.mark.parametrize("schema_id, example", params) @skipUnlessJsonSchema() - @mock.patch("cloudinit.config.schema.read_cfg_paths") - def test_validateconfig_schema_of_example( - self, read_cfg_paths, schema_id, example, paths - ): + def test_validateconfig_schema_of_example(self, schema_id, example): """For a given example in a config module we test if it is valid according to the unified schema of all config modules """ - read_cfg_paths.return_value = paths - # New-style schema $defs exist in config/cloud-init-schema*.json - for schema_file in Path(cloud_init_project_dir("config/")).glob( - "cloud-init-schema*.json" - ): - util_copy(schema_file, paths.schema_dir) schema = get_schema() config_load = safe_load(example) validate_cloudconfig_schema(config_load, schema, strict=True) diff --git a/tests/unittests/test_upgrade.py b/tests/unittests/test_upgrade.py index b73e1cddb2d..d7a721a28bd 100644 --- a/tests/unittests/test_upgrade.py +++ b/tests/unittests/test_upgrade.py @@ -47,9 +47,6 @@ def test_blacklist_drivers_set_on_networking(self, previous_obj_pkl): def test_paths_has_run_dir_attribute(self, previous_obj_pkl): assert previous_obj_pkl.paths.run_dir is not None - def test_paths_has_schema_dir_attribute(self, previous_obj_pkl): - assert previous_obj_pkl.paths.schema_dir is not None - def test_vendordata_exists(self, previous_obj_pkl): assert previous_obj_pkl.vendordata2 is None assert previous_obj_pkl.vendordata2_raw is None From a77e8794f6bb54214e512bd875f578951e3fd58e Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 14 Jan 2022 15:59:47 -0700 Subject: [PATCH 19/20] setup.py: include schema json data file in packaging --- setup.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/setup.py b/setup.py index c98a4703d81..a9132d2cf37 100755 --- a/setup.py +++ b/setup.py @@ -332,6 +332,9 @@ def finalize_options(self): author="Scott Moser", author_email="scott.moser@canonical.com", url="http://launchpad.net/cloud-init/", + package_data={ + "": ["*.json"], + }, packages=setuptools.find_packages(exclude=["tests.*", "tests"]), scripts=["tools/cloud-init-per"], license="Dual-licensed under GPLv3 or Apache 2.0", From 4ff1b3cb062546177becd443a2a4479df69c592b Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 18 Jan 2022 09:14:35 -0700 Subject: [PATCH 20/20] testing: no longer mkdir schema_dir --- conftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/conftest.py b/conftest.py index 6ec78b0a92c..3979eb0ab26 100644 --- a/conftest.py +++ b/conftest.py @@ -198,7 +198,6 @@ def paths(tmpdir): (This uses the builtin tmpdir fixture.) """ dirs = { - "schema_dir": tmpdir.mkdir("schema").strpath, "cloud_dir": tmpdir.mkdir("cloud_dir").strpath, "run_dir": tmpdir.mkdir("run_dir").strpath, }