Skip to content

Commit

Permalink
cloud: refactor cloud-init validation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Chris-Peterson444 committed Sep 18, 2024
1 parent efd289d commit 859d44e
Show file tree
Hide file tree
Showing 8 changed files with 345 additions and 115 deletions.
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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': [
Expand Down
1 change: 1 addition & 0 deletions snapcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
69 changes: 68 additions & 1 deletion subiquity/cloudinit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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."""

Expand Down
55 changes: 3 additions & 52 deletions subiquity/models/subiquity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {
Expand Down
58 changes: 13 additions & 45 deletions subiquity/models/tests/test_subiquity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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:
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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'"
Expand Down Expand Up @@ -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"
)
Expand All @@ -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"
)
Expand All @@ -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.
Expand Down
22 changes: 5 additions & 17 deletions subiquity/server/controllers/tests/test_userdata.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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"
Expand Down
Loading

0 comments on commit 859d44e

Please sign in to comment.