From f307b874260fefea8be85f1c974d93d8c626d19c Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Fri, 22 Mar 2024 23:13:59 -0700 Subject: [PATCH 1/3] context: also send events to log --- subiquitycore/context.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/subiquitycore/context.py b/subiquitycore/context.py index bd1ca641..fffd5d00 100644 --- a/subiquitycore/context.py +++ b/subiquitycore/context.py @@ -17,6 +17,8 @@ import asyncio import enum import functools import inspect +from logging import Logger +from typing import Optional class Status(enum.Enum): @@ -118,13 +120,19 @@ class Context: c = c.parent return default - def info(self, message: str) -> None: + def info(self, message: str, log: Optional[Logger] = None) -> None: + if log is not None: + log.info(message) self.app.report_info_event(self, message) - def warning(self, message: str) -> None: + def warning(self, message: str, log: Optional[Logger] = None) -> None: + if log is not None: + log.warning(message) self.app.report_warning_event(self, message) - def error(self, message: str) -> None: + def error(self, message: str, log: Optional[Logger] = None) -> None: + if log is not None: + log.error(message) self.app.report_error_event(self, message) From 8a31c7e72f9f3566b91c099363be04f1b8353a55 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Mon, 25 Mar 2024 15:28:56 -0700 Subject: [PATCH 2/3] autoinstall: Query cloud-init for schema failures Users attempting to do autoinstall may incorrectly send autoinstall directives as cloud-config, which will result in cloud-init schema validation errors. When loading autoinstall from cloud-config, we now check to see if there are any cloud-init schema validation errors and warn the user. Additionally, if the source of the error is from a known autoinstall error, we inform the user and halt the installation with a nonreportable AutoinstallError. --- subiquity/cloudinit.py | 67 ++++++++++++++++++++ subiquity/server/server.py | 81 +++++++++++++++++++++--- subiquity/server/tests/test_server.py | 90 ++++++++++++++++++++++++++- subiquity/tests/test_cloudinit.py | 83 ++++++++++++++++++++++++ 4 files changed, 313 insertions(+), 8 deletions(-) diff --git a/subiquity/cloudinit.py b/subiquity/cloudinit.py index 168d2f76..84713f6e 100644 --- a/subiquity/cloudinit.py +++ b/subiquity/cloudinit.py @@ -4,13 +4,32 @@ import asyncio import json import logging import re +from collections.abc import Awaitable +from subprocess import CompletedProcess from typing import Optional +from subiquity.server.nonreportable import NonReportableException from subiquitycore.utils import arun_command, run_command log = logging.getLogger("subiquity.cloudinit") +class CloudInitSchemaValidationError(NonReportableException): + """Exception for cloud config schema validation failure. + + Attributes: + keys -- List of keys which are the cause of the failure + """ + + def __init__( + self, + keys: list[str], + message: str = "Cloud config schema failed to validate.", + ) -> None: + super().__init__(message) + self.keys = keys + + def get_host_combined_cloud_config() -> dict: """Return the host system /run/cloud-init/combined-cloud-config.json""" try: @@ -67,6 +86,34 @@ def read_legacy_status(stream): return None +async def get_schema_failure_keys() -> list[str]: + """Retrieve the keys causing schema failure.""" + + cmd: list[str] = ["cloud-init", "schema", "--system"] + status_coro: Awaitable = arun_command(cmd, clean_locale=True) + try: + sp: CompletedProcess = await asyncio.wait_for(status_coro, 10) + except asyncio.TimeoutError: + log.warning("cloud-init schema --system timed out") + return [] + + error: str = sp.stderr # Relies on arun_command decoding to utf-8 str by default + + # Matches: + # ('some-key' was unexpected) + # ('some-key', 'another-key' were unexpected) + pattern = r"\((?P'[^']+'(,\s'[^']+')*) (?:was|were) unexpected\)" + search_result = re.search(pattern, error) + + if search_result is None: + return [] + + args_list: list[str] = search_result.group("args").split(", ") + no_quotes: list[str] = [arg.strip("'") for arg in args_list] + + return no_quotes + + async def cloud_init_status_wait() -> (bool, Optional[str]): """Wait for cloud-init completion, and return if timeout ocurred and best available status information. @@ -86,3 +133,23 @@ async def cloud_init_status_wait() -> (bool, Optional[str]): else: status = read_legacy_status(sp.stdout) return (True, status) + + +async def validate_cloud_init_schema() -> None: + """Check for cloud-init schema errors. + Returns (None) if the cloud-config schmea validated OK according to + cloud-init. Otherwise, a CloudInitSchemaValidationError is thrown + which contains a list of the keys which failed to validate. + Requires cloud-init supporting recoverable errors and extended status. + + :return: None if cloud-init schema validated succesfully. + :rtype: None + :raises CloudInitSchemaValidationError: If cloud-init schema did not validate + succesfully. + """ + causes: list[str] = await get_schema_failure_keys() + + if causes: + raise CloudInitSchemaValidationError(keys=causes) + + return None diff --git a/subiquity/server/server.py b/subiquity/server/server.py index 6f68be4f..c39cebd5 100644 --- a/subiquity/server/server.py +++ b/subiquity/server/server.py @@ -28,7 +28,12 @@ from cloudinit.config.cc_set_passwords import rand_user_password from jsonschema.exceptions import ValidationError from systemd import journal -from subiquity.cloudinit import cloud_init_status_wait, get_host_combined_cloud_config +from subiquity.cloudinit import ( + CloudInitSchemaValidationError, + cloud_init_status_wait, + get_host_combined_cloud_config, + validate_cloud_init_schema, +) from subiquity.common.api.server import bind, controller_for_request from subiquity.common.apidef import API from subiquity.common.errorreport import ErrorReport, ErrorReporter, ErrorReportKind @@ -43,7 +48,7 @@ from subiquity.common.types import ( PasswordKind, ) from subiquity.models.subiquity import ModelNames, SubiquityModel -from subiquity.server.autoinstall import AutoinstallValidationError +from subiquity.server.autoinstall import AutoinstallError, AutoinstallValidationError from subiquity.server.controller import SubiquityController from subiquity.server.dryrun import DRConfig from subiquity.server.errors import ErrorController @@ -723,7 +728,66 @@ class SubiquityServer(Application): def base_relative(self, path): return os.path.join(self.base_model.root, path) - def load_cloud_config(self): + @with_context(name="extract_autoinstall") + async def _extract_autoinstall_from_cloud_config( + self, + *, + cloud_cfg: dict[str, Any], + context: Context, + ) -> dict[str, Any]: + """Extract autoinstall passed via cloud config.""" + + # Not really is-install-context but set to force event reporting + context.set("is-install-context", True) + context.enter() # publish start event + + try: + await validate_cloud_init_schema() + except CloudInitSchemaValidationError as exc: + bad_keys: list[str] = exc.keys + raw_keys: list[str] = [f"{key!r}" for key in bad_keys] + context.warning( + f"cloud-init schema validation failure for: {', '.join(raw_keys)}", + log=log, + ) + + # Raise AutoinstallError if we found any autoinstall as a cause + # of the schema validation error, otherwise continue + + # Filter only the bad keys + potential_autoinstall: dict[str, Any] = dict( + ((key, cloud_cfg[key]) for key in bad_keys) + ) + autoinstall, other = self.filter_autoinstall(potential_autoinstall) + + if len(autoinstall) != 0: + for key in autoinstall: + context.error( + message=( + f"{key!r} is valid autoinstall but not " + "found under 'autoinstall'." + ), + log=log, + ) + + raise AutoinstallError( + ( + "Misplaced autoinstall directives resulted in a cloud-init " + "schema validation failure." + ) + ) from exc + + else: + log.debug( + "No autoinstall keys found among bad cloud config. Continuing." + ) + + cfg: dict[str, Any] = cloud_cfg.get("autoinstall", {}) + + return cfg + + @with_context() + async def load_cloud_config(self, *, context: Context): # cloud-init 23.3 introduced combined-cloud-config, which helps to # prevent subiquity from having to go load cloudinit modules. # This matters because a downgrade pickle deserialization issue may @@ -755,13 +819,16 @@ class SubiquityServer(Application): users = ug_util.normalize_users_groups(cloud_cfg, cloud.distro)[0] self.installer_user_name = ug_util.extract_default(users)[0] - if "autoinstall" in cloud_cfg: + autoinstall = await self._extract_autoinstall_from_cloud_config( + cloud_cfg=cloud_cfg, context=context + ) + + if autoinstall != {}: log.debug("autoinstall found in cloud-config") - cfg = cloud_cfg["autoinstall"] target = self.base_relative(cloud_autoinstall_path) from cloudinit import safeyaml - write_file(target, safeyaml.dumps(cfg)) + write_file(target, safeyaml.dumps(autoinstall)) else: log.debug("no autoinstall found in cloud-config") @@ -778,7 +845,7 @@ class SubiquityServer(Application): if "disabled" in status: log.debug("Skip cloud-init autoinstall, cloud-init is disabled") else: - self.load_cloud_config() + await self.load_cloud_config() def select_autoinstall(self): # precedence diff --git a/subiquity/server/tests/test_server.py b/subiquity/server/tests/test_server.py index a6a199f3..e776fae9 100644 --- a/subiquity/server/tests/test_server.py +++ b/subiquity/server/tests/test_server.py @@ -13,6 +13,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +import copy import os import shlex from typing import Any @@ -21,8 +22,9 @@ from unittest.mock import AsyncMock, Mock, patch import jsonschema from jsonschema.validators import validator_for +from subiquity.cloudinit import CloudInitSchemaValidationError from subiquity.common.types import NonReportableError, PasswordKind -from subiquity.server.autoinstall import AutoinstallValidationError +from subiquity.server.autoinstall import AutoinstallError, AutoinstallValidationError from subiquity.server.nonreportable import NonReportableException from subiquity.server.server import ( NOPROBERARG, @@ -357,6 +359,92 @@ class TestAutoinstallValidation(SubiTestCase): self.assertEqual(valid, good) self.assertEqual(invalid, bad) + @parameterized.expand( + ( + # Has valid cloud config, no autoinstall + ({"valid-cloud": "data"}, {}, False), + # Has valid cloud config and autoinstall, no valid ai in cloud cfg + ( + { + "valid-cloud": "data", + "autoinstall": { + "version": 1, + "interactive-sections": ["identity"], + }, + }, + { + "version": 1, + "interactive-sections": ["identity"], + }, + False, + ), + # Has valid autoinstall directive in cloud config + ( + { + "interactive-sections": "data", + "autoinstall": { + "version": 1, + "interactive-sections": ["identity"], + }, + }, + None, # Doesn't return + True, + ), + # Invalid cloud config key is autoinstall and no autoinstall + ( + { + "interactive-sections": ["identity"], + }, + None, # Doesn't return + True, + ), + # Has invalid cloud config key but is not valid autoinstall either + ( + { + "something-else": "data", + "autoinstall": { + "version": 1, + "interactive-sections": ["identity"], + }, + }, + { + "version": 1, + "interactive-sections": ["identity"], + }, + False, + ), + ) + ) + async def test_autoinstall_from_cloud_config(self, cloud_cfg, expected, throws): + """Test autoinstall extract from cloud config.""" + + self.server.base_schema = SubiquityServer.base_schema + self.pseudo_load_controllers() + + cloud_data = copy.copy(cloud_cfg) + cloud_data.pop("valid-cloud", None) + cloud_data.pop("autoinstall", None) + + with patch("subiquity.server.server.validate_cloud_init_schema") as val_mock: + if len(cloud_data) == 0: + val_mock.return_value = True + else: + val_mock.side_effect = CloudInitSchemaValidationError( + keys=list(cloud_data.keys()) + ) + + if throws: + with self.assertRaises(AutoinstallError): + cfg = await self.server._extract_autoinstall_from_cloud_config( + cloud_cfg=cloud_cfg + ) + else: + cfg = await self.server._extract_autoinstall_from_cloud_config( + cloud_cfg=cloud_cfg + ) + + self.assertEqual(cfg, expected) + class TestMetaController(SubiTestCase): async def test_interactive_sections_not_present(self): diff --git a/subiquity/tests/test_cloudinit.py b/subiquity/tests/test_cloudinit.py index 8f4b3ee6..109a5c44 100644 --- a/subiquity/tests/test_cloudinit.py +++ b/subiquity/tests/test_cloudinit.py @@ -19,12 +19,15 @@ from unittest import skipIf from unittest.mock import Mock, patch from subiquity.cloudinit import ( + CloudInitSchemaValidationError, cloud_init_status_wait, cloud_init_version, + get_schema_failure_keys, read_json_extended_status, read_legacy_status, supports_format_json, supports_recoverable_errors, + validate_cloud_init_schema, ) from subiquitycore.tests import SubiTestCase from subiquitycore.tests.parameterized import parameterized @@ -132,3 +135,83 @@ class TestCloudInitVersion(SubiTestCase): args=[], returncode=0, stdout="status: done\n" ) self.assertEqual((True, "done"), await cloud_init_status_wait()) + + +class TestCloudInitSchemaValidation(SubiTestCase): + @parameterized.expand( + ( + ( + ( + " Error: Cloud config schema errors: : Additional " + "properties are not allowed ('bad-key', 'late-commands' " + "were unexpected)\n\nError: Invalid schema: user-data\n\n" + ), + ["bad-key", "late-commands"], + ), + ( + ( + " Error: Cloud config schema errors: : Additional " + "properties are not allowed ('bad-key' " + "was unexpected)\n\nError: Invalid schema: user-data\n\n" + ), + ["bad-key"], + ), + ("('key_1', 'key-2', 'KEY3' were unexpected)", ["key_1", "key-2", "KEY3"]), + ("('key_.-;!)1!' was unexpected)", ["key_.-;!)1!"]), + ) + ) + async def test_get_schema_failure_keys(self, msg, expected): + """Test 1 or more keys are extracted correctly.""" + + with ( + patch("subiquity.cloudinit.arun_command", new=Mock()), + patch("subiquity.cloudinit.asyncio.wait_for") as wait_for_mock, + ): + wait_for_mock.return_value = CompletedProcess( + args=[], returncode=1, stderr=msg + ) + + bad_keys = await get_schema_failure_keys() + + self.assertEqual(bad_keys, expected) + + @patch("subiquity.cloudinit.arun_command", new=Mock()) + @patch("subiquity.cloudinit.asyncio.wait_for") + async def test_get_schema_failure_malformed(self, wait_for_mock): + """Test graceful failure if output changes.""" + + error_msg = "('key_1', 'key-2', 'KEY3', were unexpected)" + + wait_for_mock.return_value = CompletedProcess( + args=[], returncode=1, stderr=error_msg + ) + + bad_keys = await get_schema_failure_keys() + + self.assertEqual(bad_keys, []) + + @patch("subiquity.cloudinit.arun_command", new=Mock()) + @patch("subiquity.cloudinit.asyncio.wait_for") + 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()) + + @patch("subiquity.cloudinit.get_schema_failure_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() + + self.assertEqual(mock_keys, ctx.exception.keys) + + @patch("subiquity.cloudinit.arun_command", new=Mock()) + @patch("subiquity.cloudinit.asyncio.wait_for") + @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() + log_mock.warning.assert_called() + self.assertEqual([], sources) From 1b2c6befbbee3b6cf4b08d2d3ef7126ba8e78337 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Tue, 2 Apr 2024 13:04:15 -0700 Subject: [PATCH 3/3] client: CloudInitSchemaValidationError in error overlay --- subiquity/ui/views/error.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/subiquity/ui/views/error.py b/subiquity/ui/views/error.py index a5acaa7e..3fbf3634 100644 --- a/subiquity/ui/views/error.py +++ b/subiquity/ui/views/error.py @@ -426,6 +426,7 @@ class ErrorReportListStretchy(Stretchy): nonreportable_titles: dict[str, str] = { "AutoinstallError": _("an Autoinstall error"), "AutoinstallValidationError": _("an Autoinstall validation error"), + "CloudInitSchemaValidationError": _("a cloud-init schema validation error"), } nonreportable_footers: dict[str, str] = { @@ -437,6 +438,11 @@ nonreportable_footers: dict[str, str] = { "The installer has detected an issue with the provided Autoinstall " "file. Please modify it and try again." ), + "CloudInitSchemaValidationError": _( + "The installer has detected a cloud-init schema validation error " + "that will likely cause the installation to not proceed as intended. " + "Please address the validation errors and try again." + ), }