From 52bb9c361c6bc943249733ece59707a979eb8682 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Mon, 9 Sep 2024 16:47:51 -0700 Subject: [PATCH 1/4] dry-run: add a default SNAP env var definition We need to define the SNAP environment variable in dry-run mode so the `system_scripts_env` function can setup a path to the system_scripts directory. This is necessary for the subiquity-legacy-cloud-init -validate script. --- subiquity/cmd/server.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/subiquity/cmd/server.py b/subiquity/cmd/server.py index ddf017431..0575600ec 100644 --- a/subiquity/cmd/server.py +++ b/subiquity/cmd/server.py @@ -206,6 +206,9 @@ def main(): logdir = opts.output_base if opts.bootloader is None: opts.bootloader = "uefi" + # Set for system_scripts support in dry run + if not os.environ.get("SNAP"): + os.environ["SNAP"] = str(pathlib.Path(__file__).parents[2]) else: dr_cfg = None if opts.socket is None: From efd289dde5fa724720ef49f24513bd11a7a57fdc Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Fri, 6 Sep 2024 16:20:48 -0700 Subject: [PATCH 2/4] cloud: rename validate_cloud_init_schema Rename validate_cloud_init_schema to validate_cloud_init_top_level_keys and rename get_schema_failure_keys to get_unknown_keys. validate_cloud_init_schema wasn't really validating the whole cloud-init schema, it's really just parsing the output for any top-level keys that failed to validate. Let's rename it to what it actually is doing: checking for bad top level keys. Also renames the original CloudInitSchemaValidationError to CloudInitSchemaTopLevelKeyError to reflect this change. --- subiquity/cloudinit.py | 28 +++++++++++++++------------ subiquity/server/server.py | 8 ++++---- subiquity/server/tests/test_server.py | 14 +++++++++----- subiquity/tests/test_cloudinit.py | 22 ++++++++++----------- 4 files changed, 40 insertions(+), 32 deletions(-) diff --git a/subiquity/cloudinit.py b/subiquity/cloudinit.py index 775d0d328..248868077 100644 --- a/subiquity/cloudinit.py +++ b/subiquity/cloudinit.py @@ -29,7 +29,11 @@ class CloudInitSchemaValidationError(NonReportableException): - """Exception for cloud config schema validation failure. + """Exception for cloud config schema validation failure.""" + + +class CloudInitSchemaTopLevelKeyError(CloudInitSchemaValidationError): + """Exception for when cloud-config top level keys fail to validate. Attributes: keys -- List of keys which are the cause of the failure @@ -38,7 +42,7 @@ class CloudInitSchemaValidationError(NonReportableException): def __init__( self, keys: list[str], - message: str = "Cloud config schema failed to validate.", + message: str = "Cloud config schema failed to validate top-level keys.", ) -> None: super().__init__(message) self.keys = keys @@ -100,8 +104,8 @@ def read_legacy_status(stream): return None -async def get_schema_failure_keys() -> list[str]: - """Retrieve the keys causing schema failure.""" +async def get_unknown_keys() -> list[str]: + """Retrieve top-level keys causing schema failures, if any.""" cmd: list[str] = ["cloud-init", "schema", "--system"] status_coro: Awaitable = arun_command(cmd, clean_locale=True) @@ -149,22 +153,22 @@ async def cloud_init_status_wait() -> (bool, Optional[str]): return (True, status) -async def validate_cloud_init_schema() -> None: - """Check for cloud-init schema errors. +async def validate_cloud_init_top_level_keys() -> None: + """Check for cloud-init schema errors in top-level keys. Returns (None) if the cloud-config schema validated OK according to - cloud-init. Otherwise, a CloudInitSchemaValidationError is thrown - which contains a list of the keys which failed to validate. + cloud-init. Otherwise, a CloudInitSchemaTopLevelKeyError is thrown + which contains a list of the top-level keys which failed to validate. Requires cloud-init supporting recoverable errors and extended status. :return: None if cloud-init schema validated successfully. :rtype: None - :raises CloudInitSchemaValidationError: If cloud-init schema did not validate - successfully. + :raises CloudInitSchemaTopLevelKeyError: If cloud-init schema did not + validate successfully. """ - causes: list[str] = await get_schema_failure_keys() + causes: list[str] = await get_unknown_keys() if causes: - raise CloudInitSchemaValidationError(keys=causes) + raise CloudInitSchemaTopLevelKeyError(keys=causes) return None diff --git a/subiquity/server/server.py b/subiquity/server/server.py index 8ac8b0058..705593383 100644 --- a/subiquity/server/server.py +++ b/subiquity/server/server.py @@ -28,12 +28,12 @@ from systemd import journal from subiquity.cloudinit import ( - CloudInitSchemaValidationError, + CloudInitSchemaTopLevelKeyError, cloud_init_status_wait, get_host_combined_cloud_config, legacy_cloud_init_extract, rand_user_password, - validate_cloud_init_schema, + validate_cloud_init_top_level_keys, ) from subiquity.common.api.server import bind, controller_for_request from subiquity.common.apidef import API @@ -790,8 +790,8 @@ async def _extract_autoinstall_from_cloud_config( context.enter() # publish start event try: - await validate_cloud_init_schema() - except CloudInitSchemaValidationError as exc: + await validate_cloud_init_top_level_keys() + except CloudInitSchemaTopLevelKeyError as exc: bad_keys: list[str] = exc.keys raw_keys: list[str] = [f"{key!r}" for key in bad_keys] context.warning( diff --git a/subiquity/server/tests/test_server.py b/subiquity/server/tests/test_server.py index 0c3f269f1..9101f87a8 100644 --- a/subiquity/server/tests/test_server.py +++ b/subiquity/server/tests/test_server.py @@ -23,7 +23,7 @@ import yaml from jsonschema.validators import validator_for -from subiquity.cloudinit import CloudInitSchemaValidationError +from subiquity.cloudinit import CloudInitSchemaTopLevelKeyError from subiquity.common.types import NonReportableError, PasswordKind from subiquity.server.autoinstall import AutoinstallError, AutoinstallValidationError from subiquity.server.nonreportable import NonReportableException @@ -442,11 +442,13 @@ async def test_autoinstall_from_cloud_config(self, cloud_cfg, expected, throws): cloud_data.pop("valid-cloud", None) cloud_data.pop("autoinstall", None) - with patch("subiquity.server.server.validate_cloud_init_schema") as val_mock: + with patch( + "subiquity.server.server.validate_cloud_init_top_level_keys" + ) as val_mock: if len(cloud_data) == 0: val_mock.return_value = True else: - val_mock.side_effect = CloudInitSchemaValidationError( + val_mock.side_effect = CloudInitSchemaTopLevelKeyError( keys=list(cloud_data.keys()) ) @@ -468,8 +470,10 @@ async def test_cloud_config_extract_KeyError(self): self.server.base_schema = SubiquityServer.base_schema self.pseudo_load_controllers() - with patch("subiquity.server.server.validate_cloud_init_schema") as val_mock: - val_mock.side_effect = CloudInitSchemaValidationError( + with patch( + "subiquity.server.server.validate_cloud_init_top_level_keys" + ) as val_mock: + val_mock.side_effect = CloudInitSchemaTopLevelKeyError( keys=["broadcast", "foobar"], ) diff --git a/subiquity/tests/test_cloudinit.py b/subiquity/tests/test_cloudinit.py index 9ca1c87e2..dc0b5b335 100644 --- a/subiquity/tests/test_cloudinit.py +++ b/subiquity/tests/test_cloudinit.py @@ -23,10 +23,10 @@ from subiquity.cloudinit import ( CLOUD_INIT_PW_SET, - CloudInitSchemaValidationError, + CloudInitSchemaTopLevelKeyError, cloud_init_status_wait, cloud_init_version, - get_schema_failure_keys, + get_unknown_keys, legacy_cloud_init_extract, rand_password, rand_user_password, @@ -34,7 +34,7 @@ read_legacy_status, supports_format_json, supports_recoverable_errors, - validate_cloud_init_schema, + validate_cloud_init_top_level_keys, ) from subiquitycore.tests import SubiTestCase from subiquitycore.tests.parameterized import parameterized @@ -144,7 +144,7 @@ async def test_cloud_init_status_wait_legacy(self, m_wait_for): self.assertEqual((True, "done"), await cloud_init_status_wait()) -class TestCloudInitSchemaValidation(SubiTestCase): +class TestCloudInitTopLevelKeyValidation(SubiTestCase): @parameterized.expand( ( ( @@ -178,7 +178,7 @@ async def test_get_schema_failure_keys(self, msg, expected): args=[], returncode=1, stderr=msg ) - bad_keys = await get_schema_failure_keys() + bad_keys = await get_unknown_keys() self.assertEqual(bad_keys, expected) @@ -193,7 +193,7 @@ async def test_get_schema_failure_malformed(self, wait_for_mock): args=[], returncode=1, stderr=error_msg ) - bad_keys = await get_schema_failure_keys() + bad_keys = await get_unknown_keys() self.assertEqual(bad_keys, []) @@ -202,15 +202,15 @@ async def test_get_schema_failure_malformed(self, wait_for_mock): async def test_no_schema_errors(self, wait_for_mock): wait_for_mock.return_value = CompletedProcess(args=[], returncode=0, stderr="") - self.assertEqual(None, await validate_cloud_init_schema()) + self.assertEqual(None, await validate_cloud_init_top_level_keys()) - @patch("subiquity.cloudinit.get_schema_failure_keys") + @patch("subiquity.cloudinit.get_unknown_keys") async def test_validate_cloud_init_schema(self, sources_mock): mock_keys = ["key1", "key2"] sources_mock.return_value = mock_keys - with self.assertRaises(CloudInitSchemaValidationError) as ctx: - await validate_cloud_init_schema() + with self.assertRaises(CloudInitSchemaTopLevelKeyError) as ctx: + await validate_cloud_init_top_level_keys() self.assertEqual(mock_keys, ctx.exception.keys) @@ -219,7 +219,7 @@ async def test_validate_cloud_init_schema(self, sources_mock): @patch("subiquity.cloudinit.log") async def test_get_schema_warn_on_timeout(self, log_mock, wait_for_mock): wait_for_mock.side_effect = asyncio.TimeoutError() - sources = await get_schema_failure_keys() + sources = await get_unknown_keys() log_mock.warning.assert_called() self.assertEqual([], sources) From 859d44e6d56cac4e5c236b137f6f9da9d8a40ed1 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Mon, 9 Sep 2024 15:01:28 -0700 Subject: [PATCH 3/4] cloud: refactor cloud-init validation Extract cloud-init schema validation to a new system_script `subiquity-legacy-cloud-init-validate`. This is called "legacy" because, in the future, we likely want to rely on the cloud-init CLI directly to do schema validation, but there isn't a pressing need to rely on it now. For now, all releases will rely on the "legacy" script. Also let `get_unknown_keys` use `system_scripts_env` for the command environment to rely on the system cloud-init version. --- setup.py | 1 + snapcraft.yaml | 1 + subiquity/cloudinit.py | 69 +++++++- subiquity/models/subiquity.py | 55 +------ subiquity/models/tests/test_subiquity.py | 58 ++----- .../server/controllers/tests/test_userdata.py | 22 +-- subiquity/tests/test_cloudinit.py | 102 ++++++++++++ .../subiquity-legacy-cloud-init-validate | 152 ++++++++++++++++++ 8 files changed, 345 insertions(+), 115 deletions(-) create mode 100755 system_scripts/subiquity-legacy-cloud-init-validate diff --git a/setup.py b/setup.py index 9db0ea8eb..c12d3be61 100644 --- a/setup.py +++ b/setup.py @@ -126,6 +126,7 @@ class build(distutils.command.build.build): 'bin/subiquity-cmd', 'system_scripts/subiquity-umockdev-wrapper', 'system_scripts/subiquity-legacy-cloud-init-extract', + 'system_scripts/subiquity-legacy-cloud-init-validate', ], entry_points={ 'console_scripts': [ diff --git a/snapcraft.yaml b/snapcraft.yaml index c3acef378..d4a1455b0 100644 --- a/snapcraft.yaml +++ b/snapcraft.yaml @@ -157,6 +157,7 @@ parts: bin/subiquity-cmd: usr/bin/subiquity-cmd bin/subiquity-umockdev-wrapper: system_scripts/subiquity-umockdev-wrapper bin/subiquity-legacy-cloud-init-extract: system_scripts/subiquity-legacy-cloud-init-extract + bin/subiquity-legacy-cloud-init-validate: system_scripts/subiquity-legacy-cloud-init-validate build-attributes: - enable-patchelf diff --git a/subiquity/cloudinit.py b/subiquity/cloudinit.py index 248868077..f7f6a3981 100644 --- a/subiquity/cloudinit.py +++ b/subiquity/cloudinit.py @@ -5,7 +5,9 @@ import logging import re import secrets +import tempfile from collections.abc import Awaitable, Sequence +from pathlib import Path from string import ascii_letters, digits from subprocess import CalledProcessError, CompletedProcess from typing import Any, Optional @@ -108,7 +110,11 @@ async def get_unknown_keys() -> list[str]: """Retrieve top-level keys causing schema failures, if any.""" cmd: list[str] = ["cloud-init", "schema", "--system"] - status_coro: Awaitable = arun_command(cmd, clean_locale=True) + status_coro: Awaitable = arun_command( + cmd, + clean_locale=True, + env=system_scripts_env(), + ) try: sp: CompletedProcess = await asyncio.wait_for(status_coro, 10) except asyncio.TimeoutError: @@ -173,6 +179,67 @@ async def validate_cloud_init_top_level_keys() -> None: return None +def validate_cloud_config_schema(data: dict[str, Any], data_source: str) -> None: + """Validate data config adheres to strict cloud-config schema + + Log warnings on any deprecated cloud-config keys used. + + :param data: dict of cloud-config + :param data_source: str to present in logs/errors describing + where this config came from: autoinstall.user-data or system info + :raises CloudInitSchemaValidationError: If cloud-config did not validate + successfully. + :raises CalledProcessError: In the legacy code path if calling the helper + script fails. + """ + with tempfile.TemporaryDirectory() as td: + path = Path(td) / "test-cloud-config.yaml" + path.write_text(yaml.dump(data)) + # Eventually we may want to move to using the CLI when available, + # but we can rely on the "legacy" script for now. + legacy_cloud_init_validation(str(path), data_source) + + +def legacy_cloud_init_validation(config_path: str, data_source: str) -> None: + """Validate cloud-config using helper script. + + :param config_path: path to cloud-config to validate + :param data_source: str to present in logs/errors describing + where this config came from: autoinstall.user-data or system info + :raises CloudInitSchemaValidationError: If cloud-config did not validate + successfully. + :raises CalledProcessError: If calling the helper script fails. + """ + + try: + proc: CompletedProcess = run_command( + [ + "subiquity-legacy-cloud-init-validate", + "--config", + config_path, + "--source", + data_source, + ], + env=system_scripts_env(), + check=True, + ) + except CalledProcessError as cpe: + log_process_streams( + logging.DEBUG, + cpe, + "subiquity-legacy-cloud-init-validate", + ) + raise cpe + + results: dict[str, str] = yaml.safe_load(proc.stdout) + + if warnings := results.get("warnings"): + log.warning(warnings) + + if errors := results.get("errors"): + raise CloudInitSchemaValidationError(errors) + + async def legacy_cloud_init_extract() -> tuple[dict[str, Any], str]: """Load cloud-config from stages.Init() using helper script.""" diff --git a/subiquity/models/subiquity.py b/subiquity/models/subiquity.py index 5d567ef04..101560197 100644 --- a/subiquity/models/subiquity.py +++ b/subiquity/models/subiquity.py @@ -23,22 +23,9 @@ from typing import Any, Dict, List, Set, Tuple import yaml -from cloudinit.config.schema import ( - SchemaValidationError, - get_schema, - validate_cloudconfig_schema, -) - -try: - from cloudinit.config.schema import SchemaProblem -except ImportError: - - def SchemaProblem(x, y): - return (x, y) # TODO(drop on cloud-init 22.3 SRU) - - from curtin.config import merge_config +from subiquity.cloudinit import validate_cloud_config_schema from subiquity.common.pkg import TargetPkg from subiquity.common.resources import get_users_and_groups from subiquity.server.types import InstallerChannels @@ -321,44 +308,8 @@ async def confirm(self): await self.hub.abroadcast(InstallerChannels.INSTALL_CONFIRMED) def validate_cloudconfig_schema(self, data: dict, data_source: str): - """Validate data config adheres to strict cloud-config schema - - Log warnings on any deprecated cloud-config keys used. - - :param data: dict of valid cloud-config - :param data_source: str to present in logs/errors describing - where this config came from: autoinstall.user-data or system info - - :raise SchemaValidationError: on invalid cloud-config schema - """ - # cloud-init v. 22.3 will allow for log_deprecations=True to avoid - # raising errors on deprecated keys. - # In the meantime, iterate over schema_deprecations to log warnings. - try: - validate_cloudconfig_schema(data, schema=get_schema(), strict=True) - except SchemaValidationError as e: - if hasattr(e, "schema_deprecations"): - warnings = [] - deprecations = getattr(e, "schema_deprecations") - if deprecations: - for schema_path, message in deprecations: - warnings.append(message) - if warnings: - log.warning( - "The cloud-init configuration for %s contains" - " deprecated values:\n%s", - data_source, - "\n".join(warnings), - ) - if e.schema_errors: - if data_source == "autoinstall.user-data": - errors = [ - SchemaProblem(f"{data_source}.{path}", message) - for (path, message) in e.schema_errors - ] - else: - errors = e.schema_errors - raise SchemaValidationError(schema_errors=errors) + """Validate data config adheres to strict cloud-config schema.""" + validate_cloud_config_schema(data, data_source) def _cloud_init_config(self): config = { diff --git a/subiquity/models/tests/test_subiquity.py b/subiquity/models/tests/test_subiquity.py index fdf4f298d..21d8dc8ab 100644 --- a/subiquity/models/tests/test_subiquity.py +++ b/subiquity/models/tests/test_subiquity.py @@ -16,23 +16,15 @@ import datetime import fnmatch import json +import os import re import unittest +from pathlib import Path from unittest import mock import yaml -from cloudinit.config.schema import SchemaValidationError - -from subiquitycore.tests.parameterized import parameterized - -try: - from cloudinit.config.schema import SchemaProblem -except ImportError: - - def SchemaProblem(x, y): - return (x, y) # TODO(drop on cloud-init 22.3 SRU) - +from subiquity.cloudinit import CloudInitSchemaValidationError from subiquity.common.types import IdentityData from subiquity.models.subiquity import ( CLOUDINIT_CLEAN_FILE_TMPL, @@ -43,6 +35,8 @@ def SchemaProblem(x, y): from subiquity.server.server import INSTALL_MODEL_NAMES, POSTINSTALL_MODEL_NAMES from subiquity.server.types import InstallerChannels from subiquitycore.pubsub import MessageHub +from subiquitycore.tests import SubiTestCase +from subiquitycore.tests.parameterized import parameterized getent_group_output = """ root:x:0: @@ -77,7 +71,9 @@ def test_all(self): self.assertEqual(model_names.all(), {"a", "b", "c"}) -class TestSubiquityModel(unittest.IsolatedAsyncioTestCase): +# Patch os.environ for system_scripts +@mock.patch.dict(os.environ, {"SNAP": str(Path(__file__).parents[3])}) +class TestSubiquityModel(SubiTestCase): maxDiff = None def writtenFiles(self, config): @@ -257,7 +253,7 @@ def test_cloud_init_user_list_merge(self, run_cmd): with self.subTest("Invalid user-data raises error"): model = self.make_model() model.userdata = {"bootcmd": "nope"} - with self.assertRaises(SchemaValidationError) as ctx: + with self.assertRaises(CloudInitSchemaValidationError) as ctx: model._cloud_init_config() expected_error = ( "Cloud config schema errors: bootcmd: 'nope' is not of type 'array'" @@ -370,38 +366,8 @@ def test_validatecloudconfig_schema(self): data_source="autoinstall.user-data", ) - # Create our own subclass for focal as schema_deprecations - # was not yet defined. - class SchemaDeprecation(SchemaValidationError): - schema_deprecations = () - - def __init__(self, schema_errors=(), schema_deprecations=()): - super().__init__(schema_errors) - self.schema_deprecations = schema_deprecations - - problem = SchemaProblem( - "bogus", "'bogus' is deprecated, use 'notbogus' instead" - ) - with self.subTest("Deprecated cloud-config warns"): - with unittest.mock.patch( - "subiquity.models.subiquity.validate_cloudconfig_schema" - ) as validate: - validate.side_effect = SchemaDeprecation(schema_deprecations=(problem,)) - with self.assertLogs( - "subiquity.models.subiquity", level="INFO" - ) as logs: - model.validate_cloudconfig_schema( - data={"bogus": True}, data_source="autoinstall.user-data" - ) - expected = ( - "WARNING:subiquity.models.subiquity:The cloud-init" - " configuration for autoinstall.user-data contains deprecated" - " values:\n'bogus' is deprecated, use 'notbogus' instead" - ) - self.assertEqual(logs.output, [expected]) - with self.subTest("Invalid cloud-config schema errors"): - with self.assertRaises(SchemaValidationError) as ctx: + with self.assertRaises(CloudInitSchemaValidationError) as ctx: model.validate_cloudconfig_schema( data={"bootcmd": "nope"}, data_source="system info" ) @@ -411,7 +377,7 @@ def __init__(self, schema_errors=(), schema_deprecations=()): self.assertEqual(expected_error, str(ctx.exception)) with self.subTest("Prefix autoinstall.user-data cloud-config errors"): - with self.assertRaises(SchemaValidationError) as ctx: + with self.assertRaises(CloudInitSchemaValidationError) as ctx: model.validate_cloudconfig_schema( data={"bootcmd": "nope"}, data_source="autoinstall.user-data" ) @@ -423,6 +389,8 @@ def __init__(self, schema_errors=(), schema_deprecations=()): self.assertEqual(expected_error, str(ctx.exception)) +# Patch os.environ for system_scripts +@mock.patch.dict(os.environ, {"SNAP": str(Path(__file__).parents[3])}) class TestUserCreationFlows(unittest.IsolatedAsyncioTestCase): """live-server and desktop have a key behavior difference: desktop will permit user creation on first boot, while server will do no such thing. diff --git a/subiquity/server/controllers/tests/test_userdata.py b/subiquity/server/controllers/tests/test_userdata.py index b2501ffc9..b1bc7b1dd 100644 --- a/subiquity/server/controllers/tests/test_userdata.py +++ b/subiquity/server/controllers/tests/test_userdata.py @@ -16,19 +16,12 @@ import unittest import jsonschema -from cloudinit.config.schema import SchemaValidationError from jsonschema.validators import validator_for +from subiquity.cloudinit import CloudInitSchemaValidationError from subiquity.server.controllers.userdata import UserdataController from subiquitycore.tests.mocks import make_app -try: - from cloudinit.config.schema import SchemaProblem -except ImportError: - - def SchemaProblem(x, y): - return (x, y) # TODO(drop on cloud-init 22.3 SRU) - class TestUserdataController(unittest.TestCase): def setUp(self): @@ -42,21 +35,16 @@ def test_load_autoinstall_data(self): self.controller.load_autoinstall_data(valid_schema) self.assertEqual(self.controller.model, valid_schema) - fake_error = SchemaValidationError( - schema_errors=( - SchemaProblem("ssh_import_id", "'wrong' is not of type 'array'"), - ), + fake_error = CloudInitSchemaValidationError( + "ssh_import_id: 'wrong' is not of type 'array'" ) invalid_schema = {"ssh_import_id": "wrong"} validate = self.controller.app.base_model.validate_cloudconfig_schema validate.side_effect = fake_error with self.subTest("Invalid user-data raises error"): - with self.assertRaises(SchemaValidationError) as ctx: + with self.assertRaises(CloudInitSchemaValidationError) as ctx: self.controller.load_autoinstall_data(invalid_schema) - expected_error = ( - "Cloud config schema errors: ssh_import_id: 'wrong' is not of" - " type 'array'" - ) + expected_error = "ssh_import_id: 'wrong' is not of" " type 'array'" self.assertEqual(expected_error, str(ctx.exception)) validate.assert_called_with( data=invalid_schema, data_source="autoinstall.user-data" diff --git a/subiquity/tests/test_cloudinit.py b/subiquity/tests/test_cloudinit.py index dc0b5b335..36911b599 100644 --- a/subiquity/tests/test_cloudinit.py +++ b/subiquity/tests/test_cloudinit.py @@ -24,16 +24,19 @@ from subiquity.cloudinit import ( CLOUD_INIT_PW_SET, CloudInitSchemaTopLevelKeyError, + CloudInitSchemaValidationError, cloud_init_status_wait, cloud_init_version, get_unknown_keys, legacy_cloud_init_extract, + legacy_cloud_init_validation, rand_password, rand_user_password, read_json_extended_status, read_legacy_status, supports_format_json, supports_recoverable_errors, + validate_cloud_config_schema, validate_cloud_init_top_level_keys, ) from subiquitycore.tests import SubiTestCase @@ -144,6 +147,7 @@ async def test_cloud_init_status_wait_legacy(self, m_wait_for): self.assertEqual((True, "done"), await cloud_init_status_wait()) +@patch("subiquity.cloudinit.system_scripts_env", new=Mock()) class TestCloudInitTopLevelKeyValidation(SubiTestCase): @parameterized.expand( ( @@ -254,6 +258,25 @@ def test_rand_string_generation(self): self.assertEqual("a" * 32, rand_password(select_from=choices)) +class TestCloudInitSchemaValidation(SubiTestCase): + """Test cloud-init schema Validation.""" + + @patch("subiquity.cloudinit.legacy_cloud_init_validation") + @patch("subiquity.cloudinit.Path") + @patch("subiquity.cloudinit.tempfile.TemporaryDirectory") + async def test_config_dump(self, tempdir_mock, path_mock, legacy_validate_mock): + """Test the config and source passed correctly.""" + test_config = {"mock key": "mock value"} + validate_cloud_config_schema(test_config, "mock source") + + # Config is the same + result_path = path_mock.return_value.__truediv__.return_value + result_path.write_text.assert_called_with(yaml.dump(test_config)) + + # Source is the same + legacy_validate_mock.assert_called_with(str(result_path), "mock source") + + @patch("subiquity.cloudinit.arun_command") @patch("subiquity.cloudinit.system_scripts_env") class TestCloudInitLegacyExtract(SubiTestCase): @@ -306,3 +329,82 @@ async def test_useful_error( log_mock.assert_called_with( logging.DEBUG, cpe, "subiquity-legacy-cloud-init-extract" ) + + +@patch("subiquity.cloudinit.run_command") +@patch("subiquity.cloudinit.system_scripts_env") +class TestCloudInitLegacyValidation(SubiTestCase): + """Test subiquity-legacy-cloud-init-validation helper function.""" + + def test_called_with_correct_env( + self, + scripts_env_mock, + arun_mock, + ): + """Test legacy script is called with correct parameters and env.""" + prog_output = {"warnings": "", "errors": ""} + arun_mock.return_value.stdout = yaml.dump(prog_output) + + mock_env = {"mock": "env"} + scripts_env_mock.return_value = mock_env + + legacy_cloud_init_validation("mock_config", "mock source") + + scripts_env_mock.assert_called_once() + arun_mock.assert_called_with( + [ + "subiquity-legacy-cloud-init-validate", + "--config", + "mock_config", + "--source", + "mock source", + ], + env=mock_env, + check=True, + ) + + def test_useful_cpe_error( + self, + scripts_env_mock, + arun_mock, + ): + """Test reports CalledProcessError usefully.""" + arun_mock.side_effect = cpe = CalledProcessError( + 1, ["validate"], "stdout", "stderr" + ) + + with ( + self.assertRaises(CalledProcessError), + patch("subiquity.cloudinit.log_process_streams") as log_mock, + ): + legacy_cloud_init_validation("", "") + + log_mock.assert_called_with( + logging.DEBUG, cpe, "subiquity-legacy-cloud-init-validate" + ) + + def test_raise_schema_error( + self, + scripts_env_mock, + arun_mock, + ): + """Test raise CloudInitSchemaValidationError on errors encountered.""" + prog_output = {"warnings": "", "errors": "bad config!"} + arun_mock.return_value.stdout = yaml.dump(prog_output) + + with self.assertRaises(CloudInitSchemaValidationError): + legacy_cloud_init_validation("", "") + + async def test_log_warnings( + self, + scripts_env_mock, + arun_mock, + ): + """Test raise CloudInitSchemaValidationError on errors encountered.""" + prog_output = {"warnings": "deprecated key!", "errors": ""} + arun_mock.return_value.stdout = yaml.dump(prog_output) + + with self.assertLogs("subiquity.cloudinit") as log_mock: + legacy_cloud_init_validation("", "") + + self.assertIn("deprecated key!", log_mock.output[0]) diff --git a/system_scripts/subiquity-legacy-cloud-init-validate b/system_scripts/subiquity-legacy-cloud-init-validate new file mode 100755 index 000000000..fe44a01ec --- /dev/null +++ b/system_scripts/subiquity-legacy-cloud-init-validate @@ -0,0 +1,152 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical, Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, version 3. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +"""Legacy script for compatibility on systems where the 'cloud-init schema' +is unsupported. +""" + +import argparse +import sys +from typing import Any, Dict + +import yaml +from cloudinit import safeyaml +from cloudinit.config.schema import ( + SchemaValidationError, + get_schema, + validate_cloudconfig_schema, +) + +try: + from cloudinit.config.schema import SchemaProblem +except ImportError: + + def SchemaProblem(x, y): + return (x, y) # (not available before cloud-init 22.3) + + +def validate(data: Dict[str, Any], data_source: str) -> Dict[str, str]: + """Validate that `data` adheres to strict cloud-config schema. + + Collect warnings on any deprecated cloud-config keys used, and any errors + generated while trying to validate the cloud-config. + + :param data: dict of valid cloud-config + :param data_source: str to present in logs/errors describing + where this config came from: autoinstall.user-data or system info + + :return: A dict with keys "warnings" and "errors", containing the respective + data as a string. + """ + # cloud-init v. 22.3 will allow for log_deprecations=True to avoid + # raising errors on deprecated keys. + # In the meantime, iterate over schema_deprecations to log warnings. + + results = { + "warnings": "", + "errors": "", + } + + try: + validate_cloudconfig_schema(data, schema=get_schema(), strict=True) + except SchemaValidationError as e: + if hasattr(e, "schema_deprecations"): + warnings = [] + deprecations = getattr(e, "schema_deprecations") + if deprecations: + for schema_path, message in deprecations: + warnings.append(f"{schema_path}: {message}") + if warnings: + combined_warnings = "\n".join(warnings) + results["warnings"] = ( + f"The cloud-init configuration for {data_source} contains" + f" deprecated values:\n" + f"{combined_warnings}" + ) + + if e.schema_errors: + if data_source == "autoinstall.user-data": + errors = [ + SchemaProblem(f"{data_source}.{path}", message) + for (path, message) in e.schema_errors + ] + else: + errors = e.schema_errors + results["errors"] = str(SchemaValidationError(schema_errors=errors)) + + return results + + +def write_data( + results: Dict[str, str], + location: str, +) -> None: + """Write result of cloud-config validation""" + + output = safeyaml.dumps(results) + + if location == "-": + print(output) + else: + with open(location, "w") as fp: + fp.write(output) + + +def parse_args() -> argparse.Namespace: + """Parse arguments.""" + parser = argparse.ArgumentParser( + description=__doc__, + ) + + parser.add_argument( + "-c", + "--config", + type=str, + required=True, + help="Location of cloud-config to validate.", + ) + + parser.add_argument( + "-s", + "--source", + type=str, + required=True, + help="description of the data source for the config.", + ) + + parser.add_argument( + "-o", + "--output", + type=str, + default="-", + help="Location to write result of validation instead of stdout.", + ) + + return parser.parse_args() + + +def main() -> int: + args: argparse.Namespace = parse_args() + + with open(args.config) as fp: + config = yaml.safe_load(fp) + + results = validate(config, args.source) + write_data(results, args.output) + + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 087d02339c9a462bf721d5edfa618c1506717dd2 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Mon, 9 Sep 2024 16:04:44 -0700 Subject: [PATCH 4/4] cloud: top-level key validation skip check The "schema" subcommand was introduced in cloud-init 22.2 (see cloud-init commit [0] ), which was released April 27, 2022. This means older releases of Focal and Jammy, which are currently supported LTSes, will fail this code path after we unstage cloud-init from the Subiquity snap. Add a check to skip this validation if a newer version of cloud-init is detected. [0] 3bcffacb216d683241cf955e4f7f3e89431c1491 --- subiquity/cloudinit.py | 11 +++++++++++ subiquity/tests/test_cloudinit.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/subiquity/cloudinit.py b/subiquity/cloudinit.py index f7f6a3981..95a1a481c 100644 --- a/subiquity/cloudinit.py +++ b/subiquity/cloudinit.py @@ -87,6 +87,10 @@ def supports_recoverable_errors() -> bool: return cloud_init_version() >= "23.4" +def supports_schema_subcommand() -> bool: + return cloud_init_version() >= "22.2" + + def read_json_extended_status(stream): try: status = json.loads(stream) @@ -171,6 +175,13 @@ async def validate_cloud_init_top_level_keys() -> None: :raises CloudInitSchemaTopLevelKeyError: If cloud-init schema did not validate successfully. """ + if not supports_schema_subcommand(): + log.debug( + "Host cloud-config doesn't support 'schema' subcommand. " + "Skipping top-level key cloud-config validation." + ) + return None + causes: list[str] = await get_unknown_keys() if causes: diff --git a/subiquity/tests/test_cloudinit.py b/subiquity/tests/test_cloudinit.py index 36911b599..915136a7a 100644 --- a/subiquity/tests/test_cloudinit.py +++ b/subiquity/tests/test_cloudinit.py @@ -227,6 +227,36 @@ async def test_get_schema_warn_on_timeout(self, log_mock, wait_for_mock): log_mock.warning.assert_called() self.assertEqual([], sources) + @parameterized.expand( + ( + ("20.2", True), + ("22.1", True), + ("22.2", False), + ("23.0", False), + ) + ) + async def test_version_check_and_skip(self, version, should_skip): + """Test that top-level key validation skipped on right versions. + + The "schema" subcommand, which the top-level key validation relies + on, was added in cloud-init version 22.2. This test is to ensure + that it's skipped on older releases. + """ + with ( + patch("subiquity.cloudinit.get_unknown_keys") as keys_mock, + patch("subiquity.cloudinit.cloud_init_version") as version_mock, + ): + version_mock.return_value = version + + if should_skip: + await validate_cloud_init_top_level_keys() + keys_mock.assert_not_called() + + else: + keys_mock.return_value = [] # avoid raise condition + await validate_cloud_init_top_level_keys() + keys_mock.assert_called_once() + class TestCloudInitRandomStrings(SubiTestCase): def test_passwd_constraints(self):