autoinstall: enforce top-level keys
Subiquity currently ignores invalid top-level keys, but this has likely been a major source of confusion of autoinstall capabilities. In future versions, the following autoinstall config will throw an AutoinstallValidationError: version: 2 interactive-sections: - identity literally-anything: lmao This patch adds warnings for version 1 and will begin to throw an AutoinstallValidationError on these instances in version 2 once it has been enabled.
This commit is contained in:
parent
735771a494
commit
d812919366
|
@ -3,7 +3,11 @@
|
||||||
Autoinstall configuration reference manual
|
Autoinstall configuration reference manual
|
||||||
==========================================
|
==========================================
|
||||||
|
|
||||||
The autoinstall file uses the YAML format. At the top level, it must be a mapping containing the keys described in this document. Unrecognised keys are ignored.
|
The autoinstall file uses the YAML format. At the top level, it must be a
|
||||||
|
mapping containing the keys described in this document. Unrecognised keys
|
||||||
|
are ignored in version 1, but will cause a fatal validation error in future
|
||||||
|
versions.
|
||||||
|
|
||||||
|
|
||||||
.. _ai-schema:
|
.. _ai-schema:
|
||||||
|
|
||||||
|
@ -25,6 +29,12 @@ Several configuration keys are lists of commands to be executed. Each command ca
|
||||||
Top-level keys
|
Top-level keys
|
||||||
--------------
|
--------------
|
||||||
|
|
||||||
|
|
||||||
|
.. warning::
|
||||||
|
In version 1, Subiquity will emit warnings when encountering unrecognised
|
||||||
|
keys. In later versions, a fatal validation error is thrown and the
|
||||||
|
installation will halt.
|
||||||
|
|
||||||
.. _ai-version:
|
.. _ai-version:
|
||||||
|
|
||||||
version
|
version
|
||||||
|
|
|
@ -14,11 +14,12 @@
|
||||||
# along with this program. If not, see <http://www.gnu.org/licenses/>.
|
# along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
|
import copy
|
||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
import sys
|
import sys
|
||||||
import time
|
import time
|
||||||
from typing import List, Optional
|
from typing import Any, List, Optional
|
||||||
|
|
||||||
import jsonschema
|
import jsonschema
|
||||||
import yaml
|
import yaml
|
||||||
|
@ -553,8 +554,97 @@ class SubiquityServer(Application):
|
||||||
await controller.apply_autoinstall_config()
|
await controller.apply_autoinstall_config()
|
||||||
await controller.configured()
|
await controller.configured()
|
||||||
|
|
||||||
|
def filter_autoinstall(
|
||||||
|
self,
|
||||||
|
autoinstall_config: dict[str, Any],
|
||||||
|
) -> tuple[dict[str, Any], dict[str, Any]]:
|
||||||
|
"""Separates autoinstall config by known and unknown keys"""
|
||||||
|
|
||||||
|
invalid_config: dict[str, Any] = copy.deepcopy(autoinstall_config)
|
||||||
|
|
||||||
|
# Pop keys of all loaded controllers, if they exists
|
||||||
|
for controller in self.controllers.instances:
|
||||||
|
invalid_config.pop(controller.autoinstall_key, None)
|
||||||
|
|
||||||
|
# Pop server keys, if they exist
|
||||||
|
for key in self.base_schema["properties"]:
|
||||||
|
invalid_config.pop(key, None)
|
||||||
|
|
||||||
|
valid_config: dict[str, Any] = copy.deepcopy(autoinstall_config)
|
||||||
|
|
||||||
|
# Remove the keys we isolated
|
||||||
|
for key in invalid_config:
|
||||||
|
valid_config.pop(key)
|
||||||
|
|
||||||
|
return (valid_config, invalid_config)
|
||||||
|
|
||||||
|
@with_context(name="top_level_keys")
|
||||||
|
def _enforce_top_level_keys(
|
||||||
|
self,
|
||||||
|
*,
|
||||||
|
autoinstall_config: dict[str, Any],
|
||||||
|
context: Context,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Enforces usage of known top-level keys.
|
||||||
|
|
||||||
|
In Autoinstall v1, unknown top-level keys are removed from
|
||||||
|
the config and a cleaned config is returned.
|
||||||
|
|
||||||
|
In Autoinstall v2, unknown top-level keys result in a fatal
|
||||||
|
AutoinstallValidationError
|
||||||
|
|
||||||
|
Only checks for unrecognized keys, doesn't validate them.
|
||||||
|
Requires a version number, so this should be called after
|
||||||
|
validating against the schema.
|
||||||
|
|
||||||
|
"""
|
||||||
|
|
||||||
|
valid_config, invalid_config = self.filter_autoinstall(autoinstall_config)
|
||||||
|
|
||||||
|
# If no bad keys, return early
|
||||||
|
if len(invalid_config.keys()) == 0:
|
||||||
|
return autoinstall_config
|
||||||
|
|
||||||
|
# If the version is early enough, only warn
|
||||||
|
version: int = autoinstall_config["version"]
|
||||||
|
|
||||||
|
ctx = context
|
||||||
|
if version == 1:
|
||||||
|
# Warn then clean out bad keys and return
|
||||||
|
|
||||||
|
for key in invalid_config:
|
||||||
|
warning = f"Unrecognized top-level key {key!r}"
|
||||||
|
log.warning(warning)
|
||||||
|
ctx.warning(warning)
|
||||||
|
|
||||||
|
warning = (
|
||||||
|
"Unrecognized top-level keys will cause Autoinstall to "
|
||||||
|
"throw an error in future versions."
|
||||||
|
)
|
||||||
|
log.warning(warning)
|
||||||
|
ctx.warning(warning)
|
||||||
|
|
||||||
|
return valid_config
|
||||||
|
|
||||||
|
else:
|
||||||
|
for key in invalid_config:
|
||||||
|
error = f"Unrecognized top-level key {key!r}"
|
||||||
|
log.error(error)
|
||||||
|
ctx.error(error)
|
||||||
|
|
||||||
|
error = "Unrecognized top-level keys are unsupported"
|
||||||
|
log.error(error)
|
||||||
|
ctx.error(error)
|
||||||
|
|
||||||
|
raw_keys = (f"{key!r}" for key in invalid_config)
|
||||||
|
details: str = f"Unrecognized top-level key(s): {', '.join(raw_keys)}"
|
||||||
|
raise AutoinstallValidationError(
|
||||||
|
owner="top-level keys",
|
||||||
|
details=details,
|
||||||
|
)
|
||||||
|
|
||||||
def validate_autoinstall(self):
|
def validate_autoinstall(self):
|
||||||
with self.context.child("core_validation", level="INFO"):
|
with self.context.child("core_validation", level="INFO") as ctx:
|
||||||
try:
|
try:
|
||||||
jsonschema.validate(self.autoinstall_config, self.base_schema)
|
jsonschema.validate(self.autoinstall_config, self.base_schema)
|
||||||
except ValidationError as original_exception:
|
except ValidationError as original_exception:
|
||||||
|
@ -567,6 +657,12 @@ class SubiquityServer(Application):
|
||||||
|
|
||||||
raise new_exception from original_exception
|
raise new_exception from original_exception
|
||||||
|
|
||||||
|
# Enforce top level keys after ensuring we have a version number
|
||||||
|
self.autoinstall_config = self._enforce_top_level_keys(
|
||||||
|
autoinstall_config=self.autoinstall_config,
|
||||||
|
context=ctx,
|
||||||
|
)
|
||||||
|
|
||||||
def load_autoinstall_config(self, *, only_early):
|
def load_autoinstall_config(self, *, only_early):
|
||||||
log.debug(
|
log.debug(
|
||||||
"load_autoinstall_config only_early %s file %s",
|
"load_autoinstall_config only_early %s file %s",
|
||||||
|
|
|
@ -15,6 +15,7 @@
|
||||||
|
|
||||||
import os
|
import os
|
||||||
import shlex
|
import shlex
|
||||||
|
from typing import Any
|
||||||
from unittest.mock import AsyncMock, Mock, patch
|
from unittest.mock import AsyncMock, Mock, patch
|
||||||
|
|
||||||
import jsonschema
|
import jsonschema
|
||||||
|
@ -125,7 +126,9 @@ class TestAutoinstallLoad(SubiTestCase):
|
||||||
with self.assertRaises(Exception):
|
with self.assertRaises(Exception):
|
||||||
self.server.select_autoinstall()
|
self.server.select_autoinstall()
|
||||||
|
|
||||||
def test_early_commands_changes_autoinstall(self):
|
# Only care about changes to autoinstall, not validity
|
||||||
|
@patch("subiquity.server.server.SubiquityServer.validate_autoinstall")
|
||||||
|
def test_early_commands_changes_autoinstall(self, validate_mock):
|
||||||
self.server.controllers = Mock()
|
self.server.controllers = Mock()
|
||||||
self.server.controllers.instances = []
|
self.server.controllers.instances = []
|
||||||
rootpath = self.path(root_autoinstall_path)
|
rootpath = self.path(root_autoinstall_path)
|
||||||
|
@ -166,6 +169,33 @@ class TestAutoinstallValidation(SubiTestCase):
|
||||||
}
|
}
|
||||||
self.server.make_apport_report = Mock()
|
self.server.make_apport_report = Mock()
|
||||||
|
|
||||||
|
# Pseudo Load Controllers to avoid patching the loading logic for each
|
||||||
|
# controller when we still want access to class attributes
|
||||||
|
def pseudo_load_controllers(self):
|
||||||
|
controller_classes = []
|
||||||
|
for prefix in self.server.controllers.controller_names:
|
||||||
|
controller_classes.append(
|
||||||
|
self.server.controllers._get_controller_class(prefix)
|
||||||
|
)
|
||||||
|
self.server.controllers.instances = controller_classes
|
||||||
|
|
||||||
|
def load_config_and_controllers(
|
||||||
|
self, config: dict[str, Any]
|
||||||
|
) -> tuple[dict[str, Any], dict[str, Any]]:
|
||||||
|
"""Loads an autoinstall config and controllers.
|
||||||
|
|
||||||
|
Loads the provided autoinstall config and the controllers.
|
||||||
|
Returns the valid and invalid portions of the config.
|
||||||
|
"""
|
||||||
|
# Reset base schema
|
||||||
|
self.server.base_schema = SubiquityServer.base_schema
|
||||||
|
|
||||||
|
self.server.autoinstall_config = config
|
||||||
|
|
||||||
|
self.pseudo_load_controllers()
|
||||||
|
|
||||||
|
return self.server.filter_autoinstall(config)
|
||||||
|
|
||||||
def test_valid_schema(self):
|
def test_valid_schema(self):
|
||||||
"""Test that the expected autoinstall JSON schema is valid"""
|
"""Test that the expected autoinstall JSON schema is valid"""
|
||||||
|
|
||||||
|
@ -232,6 +262,101 @@ class TestAutoinstallValidation(SubiTestCase):
|
||||||
error = NonReportableError.from_exception(exception)
|
error = NonReportableError.from_exception(exception)
|
||||||
self.assertEqual(error, self.server.nonreportable_error)
|
self.assertEqual(error, self.server.nonreportable_error)
|
||||||
|
|
||||||
|
@patch("subiquity.server.server.log")
|
||||||
|
async def test_autoinstall_validation__strict_top_level_keys_warn(self, log_mock):
|
||||||
|
"""Test strict top-level key enforcement warnings in v1"""
|
||||||
|
|
||||||
|
bad_ai_data = {
|
||||||
|
"version": 1,
|
||||||
|
"interactive-sections": ["identity"],
|
||||||
|
"apt": "Invalid but deferred",
|
||||||
|
"literally-anything": "lmao",
|
||||||
|
}
|
||||||
|
|
||||||
|
good, bad = self.load_config_and_controllers(bad_ai_data)
|
||||||
|
|
||||||
|
# OK in Version 1 but ensure warnings and stripped config
|
||||||
|
self.server.validate_autoinstall()
|
||||||
|
log_mock.warning.assert_called()
|
||||||
|
log_mock.error.assert_not_called()
|
||||||
|
self.assertEqual(self.server.autoinstall_config, good)
|
||||||
|
self.assertEqual(bad, {"literally-anything": "lmao"})
|
||||||
|
|
||||||
|
@patch("subiquity.server.server.log")
|
||||||
|
async def test_autoinstall_validation__strict_top_level_keys_error(self, log_mock):
|
||||||
|
"""Test strict top-level key enforcement errors in v2 or greater"""
|
||||||
|
|
||||||
|
bad_ai_data = {
|
||||||
|
"version": 2,
|
||||||
|
"interactive-sections": ["identity"],
|
||||||
|
"apt": "Invalid but deferred",
|
||||||
|
"literally-anything": "lmao",
|
||||||
|
}
|
||||||
|
|
||||||
|
self.load_config_and_controllers(bad_ai_data)
|
||||||
|
|
||||||
|
# TODO: remove once V2 is enabled
|
||||||
|
self.server.base_schema["properties"]["version"]["maximum"] = 2
|
||||||
|
|
||||||
|
# Not OK in Version >= 2
|
||||||
|
with self.assertRaises(AutoinstallValidationError) as ctx:
|
||||||
|
self.server.validate_autoinstall()
|
||||||
|
|
||||||
|
self.assertIn("top-level keys", str(ctx.exception))
|
||||||
|
|
||||||
|
log_mock.error.assert_called()
|
||||||
|
log_mock.warning.assert_not_called()
|
||||||
|
|
||||||
|
@parameterized.expand(
|
||||||
|
(
|
||||||
|
(
|
||||||
|
# Case 1: extra key "some-bad-key"
|
||||||
|
{
|
||||||
|
"version": 1,
|
||||||
|
"interactive-sections": ["identity"],
|
||||||
|
"apt": "...",
|
||||||
|
"some-bad-key": "...",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"version": 1,
|
||||||
|
"interactive-sections": ["identity"],
|
||||||
|
"apt": "...",
|
||||||
|
},
|
||||||
|
{"some-bad-key": "..."},
|
||||||
|
),
|
||||||
|
(
|
||||||
|
# Case 2: no bad keys
|
||||||
|
{
|
||||||
|
"version": 1,
|
||||||
|
"interactive-sections": ["identity"],
|
||||||
|
"apt": "...",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"version": 1,
|
||||||
|
"interactive-sections": ["identity"],
|
||||||
|
"apt": "...",
|
||||||
|
},
|
||||||
|
{},
|
||||||
|
),
|
||||||
|
(
|
||||||
|
# Case 3: no good keys
|
||||||
|
{"some-bad-key": "..."},
|
||||||
|
{},
|
||||||
|
{"some-bad-key": "..."},
|
||||||
|
),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
async def test_autoinstall_validation__filter_autoinstall(self, config, good, bad):
|
||||||
|
"""Test autoinstall config filtering"""
|
||||||
|
|
||||||
|
self.server.base_schema = SubiquityServer.base_schema
|
||||||
|
self.pseudo_load_controllers()
|
||||||
|
|
||||||
|
valid, invalid = self.server.filter_autoinstall(config)
|
||||||
|
|
||||||
|
self.assertEqual(valid, good)
|
||||||
|
self.assertEqual(invalid, bad)
|
||||||
|
|
||||||
|
|
||||||
class TestMetaController(SubiTestCase):
|
class TestMetaController(SubiTestCase):
|
||||||
async def test_interactive_sections_not_present(self):
|
async def test_interactive_sections_not_present(self):
|
||||||
|
|
Loading…
Reference in New Issue