Merge pull request #2079 from Chris-Peterson444/refactor-cloud-init-v…

…alidation

Replace cloud-init validation with external script

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': [

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

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
@@ -29,7 +31,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 +44,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
@@ -81,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)
@@ -100,11 +110,15 @@ 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)
+    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:
@@ -149,26 +163,94 @@ 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()
+    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:
-        raise CloudInitSchemaValidationError(keys=causes)
+        raise CloudInitSchemaTopLevelKeyError(keys=causes)
 
     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."""
 

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:

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 = {

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.