From a25f2e03c11b40cc377a4ef126c061499d5271f7 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Fri, 22 Mar 2024 16:05:52 -0700 Subject: [PATCH 1/5] server: refactor reading in autoinstall Factor out reading the autoinstall data from the loading function. Also add context logging to both the loading and reading functions. --- subiquity/server/server.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/subiquity/server/server.py b/subiquity/server/server.py index c39cebd5..61f2aa39 100644 --- a/subiquity/server/server.py +++ b/subiquity/server/server.py @@ -668,7 +668,19 @@ class SubiquityServer(Application): context=ctx, ) - def load_autoinstall_config(self, *, only_early): + @with_context(name="read_config") + def _read_config(self, *, cfg_path: str, context: Context) -> dict[str, Any]: + with open(cfg_path) as fp: + config: dict[str, Any] = yaml.safe_load(fp) + + autoinstall_config: dict[str, Any] = dict() + + autoinstall_config = config + + return autoinstall_config + + @with_context() + def load_autoinstall_config(self, *, only_early, context): log.debug( "load_autoinstall_config only_early %s file %s", only_early, @@ -689,8 +701,9 @@ class SubiquityServer(Application): self.interactive = True return - with open(self.autoinstall) as fp: - self.autoinstall_config = yaml.safe_load(fp) + self.autoinstall_config = self._read_config( + cfg_path=self.autoinstall, context=context + ) # Check every time self.interactive = bool(self.autoinstall_config.get("interactive-sections")) From ba3dbb52ef3b0e46ddbc7f6c657db50d0b50a921 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Fri, 22 Mar 2024 16:49:19 -0700 Subject: [PATCH 2/5] autoinstall: Allow "autoinstall" as top-level key --- subiquity/server/server.py | 23 +++++++++++-- subiquity/server/tests/test_server.py | 49 +++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/subiquity/server/server.py b/subiquity/server/server.py index 61f2aa39..77e943b7 100644 --- a/subiquity/server/server.py +++ b/subiquity/server/server.py @@ -673,9 +673,28 @@ class SubiquityServer(Application): with open(cfg_path) as fp: config: dict[str, Any] = yaml.safe_load(fp) - autoinstall_config: dict[str, Any] = dict() + autoinstall_config: dict[str, Any] - autoinstall_config = config + # Support "autoinstall" as a top-level key + if "autoinstall" in config: + autoinstall_config = config.pop("autoinstall") + + # but the only top level key + if len(config) != 0: + self.interactive = bool(autoinstall_config.get("interactive-sections")) + msg: str = ( + "autoinstall.yaml is not a valid cloud config datasource.\n" + "No other keys may be present alongside 'autoinstall' at " + "the top level." + ) + context.error(msg) + raise AutoinstallValidationError( + owner="top-level keys", + details="autoinstall.yaml is not a valid cloud config datasource", + ) + + else: + autoinstall_config = config return autoinstall_config diff --git a/subiquity/server/tests/test_server.py b/subiquity/server/tests/test_server.py index e776fae9..a9dced2a 100644 --- a/subiquity/server/tests/test_server.py +++ b/subiquity/server/tests/test_server.py @@ -20,6 +20,7 @@ from typing import Any from unittest.mock import AsyncMock, Mock, patch import jsonschema +import yaml from jsonschema.validators import validator_for from subiquity.cloudinit import CloudInitSchemaValidationError @@ -156,6 +157,7 @@ early-commands: ["{cmd}"] class TestAutoinstallValidation(SubiTestCase): async def asyncSetUp(self): + self.tempdir = self.tmp_dir() opts = Mock() opts.dry_run = True opts.output_base = self.tmp_dir() @@ -171,6 +173,15 @@ class TestAutoinstallValidation(SubiTestCase): } self.server.make_apport_report = Mock() + def path(self, relative_path): + return self.tmp_path(relative_path, dir=self.tempdir) + + def create(self, path, contents): + path = self.path(path) + with open(path, "w") as fp: + fp.write(contents) + return path + # 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): @@ -445,6 +456,44 @@ class TestAutoinstallValidation(SubiTestCase): self.assertEqual(cfg, expected) + async def test_autoinstall_validation__top_level_autoinstall(self): + """Test allow autoinstall as top-level key""" + + new_style = { + "autoinstall": { + "version": 1, + "interactive-sections": ["identity"], + "apt": "...", + } + } + old_style = new_style["autoinstall"] + + # Read new style correctly + path = self.create("autoinstall.yaml", yaml.dump(new_style)) + self.assertEqual(self.server._read_config(cfg_path=path), old_style) + + # No changes to old style + path = self.create("autoinstall.yaml", yaml.dump(old_style)) + self.assertEqual(self.server._read_config(cfg_path=path), old_style) + + async def test_autoinstall_validation__not_cloudinit_datasource(self): + """Test no cloud init datasources in new style autoinstall""" + + new_style = { + "autoinstall": { + "version": 1, + "interactive-sections": ["identity"], + "apt": "...", + }, + "cloudinit-data": "I am data", + } + + with self.assertRaises(AutoinstallValidationError) as ctx: + path = self.create("autoinstall.yaml", yaml.dump(new_style)) + self.server._read_config(cfg_path=path) + + self.assertEqual("top-level keys", ctx.exception.owner) + class TestMetaController(SubiTestCase): async def test_interactive_sections_not_present(self): From ecfc3a4df0f4f9f0cabcc3a37eeb6a203d13899f Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Mon, 25 Mar 2024 21:30:54 -0700 Subject: [PATCH 3/5] scripts: support top-level autoinstall in validator --- scripts/validate-autoinstall-user-data.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/validate-autoinstall-user-data.py b/scripts/validate-autoinstall-user-data.py index e59c3671..33bffbd2 100755 --- a/scripts/validate-autoinstall-user-data.py +++ b/scripts/validate-autoinstall-user-data.py @@ -60,7 +60,13 @@ def main() -> None: assert user_data.readline() == "#cloud-config\n" def get_autoinstall_data(data): return data["autoinstall"] else: - def get_autoinstall_data(data): return data + def get_autoinstall_data(data): + try: + cfg = data["autoinstall"] + except KeyError: + cfg = data + return cfg + # Verify autoinstall doc link is in the file From aed2905db01b0867b28686a502cb1652422e4290 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Mon, 25 Mar 2024 21:30:39 -0700 Subject: [PATCH 4/5] doc: consistent top-level autoinstall keyword Also deletes a note to users about not needing the autoinstall keyword in non cloud-config delivery methods. We should keep a hint in the reference that this is available, but generally let's always suggest to use the top-level autoinstall keyword. --- doc/intro-to-autoinstall.rst | 12 - doc/reference/autoinstall-reference.rst | 294 ++++++++++++++---------- 2 files changed, 174 insertions(+), 132 deletions(-) diff --git a/doc/intro-to-autoinstall.rst b/doc/intro-to-autoinstall.rst index 3371ff16..1cc5a3d7 100644 --- a/doc/intro-to-autoinstall.rst +++ b/doc/intro-to-autoinstall.rst @@ -94,18 +94,6 @@ path is relative to the rootfs of the installation system. For example: * :code:`subiquity.autoinstallpath=path/to/autoinstall.yaml` -.. note:: - - Directly specifying autoinstall as a :code:`autoinstall.yaml` file does not - require a :code:`#cloud-config` header, and does not use a top level - ``autoinstall:`` key. The autoinstall directives are placed at the top - level. For example: - - .. code-block:: yaml - - version: 1 - .... - Order precedence of the autoinstall locations ============================================= diff --git a/doc/reference/autoinstall-reference.rst b/doc/reference/autoinstall-reference.rst index 9fdca7c5..4462d0ff 100644 --- a/doc/reference/autoinstall-reference.rst +++ b/doc/reference/autoinstall-reference.rst @@ -3,10 +3,40 @@ 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 in version 1, but will cause a fatal validation error in future -versions. +The autoinstall file uses the YAML format. At the top level is a +single key ``autoinstall`` which contains a mapping of the keys described in +this document. Unrecognised keys are ignored in version 1, but will cause a +fatal validation error in future versions. + +Here is an example of a minimal autoinstall configuration: + +.. code-block:: yaml + + autoinstall: + version: 1 + identity: + ... + + +At the top level is the ``autoinstall`` keyword, which contains a version section +and an (incomplete) identity section which are explained in more detail below. +Any other key at the level of ``autoinstall``, will result in an autoinstall +validation error at runtime. + +.. warning:: + + This behaviour was first introduced during 24.04 (Noble). On any ISOs built + before this, you will need to refresh the installer to see this behaviour. + Please the note below about the old format. + +.. note:: + + Technically, in all but one case the top level ``autoinstall`` keyword is + strictly unnecessary. This keyword is only necessary when serving autoinstall + via cloud-config. For backwards compatibility this format is still supported + for non-cloud-config based delivery methods; however, it is + **highly recommended** to use the format with a top-level ``autoinstall`` + keyword as mistakes in this formatting are a common source of confusion. .. _ai-schema: @@ -29,6 +59,10 @@ Several configuration keys are lists of commands to be executed. Each command ca Top-level keys -------------- +The following keys can be used to configure various aspects of the installation. +If the global ``autoinstall`` key is provided, then all "top-level keys" must +be provided underneath it and "top-level" refers to this sub-level. The +examples below demonstrate this structure. .. warning:: In version 1, Subiquity will emit warnings when encountering unrecognised @@ -57,12 +91,13 @@ A list of configuration keys to still show in the user interface (UI). For examp .. code-block:: yaml - version: 1 - interactive-sections: - - network - identity: - username: ubuntu - password: $crypted_pass + autoinstall: + version: 1 + interactive-sections: + - network + identity: + username: ubuntu + password: $crypted_pass This example stops on the network screen and allows the user to change the defaults. If a value is provided for an interactive section, it is used as the default. @@ -221,23 +256,25 @@ For example, to run DHCP version 6 on a specific network interface: .. code-block:: yaml - network: - version: 2 - ethernets: - enp0s31f6: - dhcp6: true - -Note that in the 20.04 GA release of Subiquity, the behaviour is slightly different and requires you to write this with an extra ``network:`` key: - -.. code-block:: yaml - - network: + autoinstall: network: version: 2 ethernets: enp0s31f6: dhcp6: true +Note that in the 20.04 GA release of Subiquity, the behaviour is slightly different and requires you to write this with an extra ``network:`` key: + +.. code-block:: yaml + + autoinstall: + network: + network: + version: 2 + ethernets: + enp0s31f6: + dhcp6: true + Versions later than 20.04 support this syntax, too (for compatibility). When using a newer version, use the regular syntax. .. _ai-proxy: @@ -274,17 +311,18 @@ The default is: .. code-block:: yaml - apt: - preserve_sources_list: false - mirror-selection: - primary: - - country-mirror - - arches: [i386, amd64] - uri: "http://archive.ubuntu.com/ubuntu" - - arches: [s390x, arm64, armhf, powerpc, ppc64el, riscv64] - uri: "http://ports.ubuntu.com/ubuntu-ports" - fallback: abort - geoip: true + autoinstall: + apt: + preserve_sources_list: false + mirror-selection: + primary: + - country-mirror + - arches: [i386, amd64] + uri: "http://archive.ubuntu.com/ubuntu" + - arches: [s390x, arm64, armhf, powerpc, ppc64el, riscv64] + uri: "http://ports.ubuntu.com/ubuntu-ports" + fallback: abort + geoip: true mirror-selection ^^^^^^^^^^^^^^^^ @@ -330,21 +368,23 @@ To specify a mirror, use a configuration like this: .. code-block:: yaml - apt: - mirror-selection: - primary: - - uri: YOUR_MIRROR_GOES_HERE - - country-mirror - - uri: http://archive.ubuntu.com/ubuntu + autoinstall: + apt: + mirror-selection: + primary: + - uri: YOUR_MIRROR_GOES_HERE + - country-mirror + - uri: http://archive.ubuntu.com/ubuntu To add a PPA: .. code-block:: yaml - apt: - sources: - curtin-ppa: - source: ppa:curtin-dev/test-archive + autoinstall: + apt: + sources: + curtin-ppa: + source: ppa:curtin-dev/test-archive .. _ai-storage: @@ -364,31 +404,33 @@ The three supported layouts at the time of writing are ``lvm``, ``direct`` and ` .. code-block:: yaml - storage: - layout: - name: lvm - storage: - layout: - name: direct - storage: - layout: - name: zfs + autoinstall: + storage: + layout: + name: lvm + storage: + layout: + name: direct + storage: + layout: + name: zfs By default, these layouts install to the largest disk in a system, but you can supply a match spec (see below) to indicate which disk to use: .. code-block:: yaml - storage: - layout: - name: lvm - match: - serial: CT* - storage: - layout: - name: direct - match: - ssd: true + autoinstall: + storage: + layout: + name: lvm + match: + serial: CT* + storage: + layout: + name: direct + match: + ssd: true .. note:: Match spec -- using ``match: {}`` matches an arbitrary disk. @@ -396,10 +438,11 @@ When using the ``lvm`` layout, LUKS encryption can be enabled by supplying a pas .. code-block:: yaml - storage: - layout: - name: lvm - password: LUKS_PASSPHRASE + autoinstall: + storage: + layout: + name: lvm + password: LUKS_PASSPHRASE The default is to use the ``lvm`` layout. @@ -427,11 +470,12 @@ Example with no size scaling and a passphrase: .. code-block:: yaml - storage: - layout: - name: lvm - sizing-policy: all - password: LUKS_PASSPHRASE + autoinstall: + storage: + layout: + name: lvm + sizing-policy: all + password: LUKS_PASSPHRASE Reset Partition ^^^^^^^^^^^^^^^ @@ -444,29 +488,32 @@ An example to enable Reset Partition: .. code-block:: yaml - storage: - layout: - name: direct - reset-partition: true + autoinstall: + storage: + layout: + name: direct + reset-partition: true The size of the reset partition can also be fixed to a specified size. This is an example to fix Reset Partition to 12 GiB: .. code-block:: yaml - storage: - layout: - name: direct - reset-partition: 12G + autoinstall: + storage: + layout: + name: direct + reset-partition: 12G The installer can also install Reset Partition without installing the system. To do this, set ``reset-partition-only`` to ``true``: .. code-block:: yaml - storage: - layout: - name: direct - reset-partition: true - reset-partition-only: true + autoinstall: + storage: + layout: + name: direct + reset-partition: true + reset-partition-only: true Action-based configuration ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -482,15 +529,16 @@ An example storage section: .. code-block:: yaml - storage: - swap: - size: 0 - config: - - type: disk - id: disk0 - serial: ADATA_SX8200PNP_XXXXXXXXXXX - - type: partition - ... + autoinstall: + storage: + swap: + size: 0 + config: + - type: disk + id: disk0 + serial: ADATA_SX8200PNP_XXXXXXXXXXX + - type: partition + ... The extensions to the curtin syntax allow for disk selection and partition or logical-volume sizing. @@ -613,11 +661,12 @@ Example: .. code-block:: yaml - identity: - realname: 'Ubuntu User' - username: ubuntu - password: '$6$wdAcoXrU039hKYPd$508Qvbe7ObUnxoj15DRCkzC3qO7edjH0VV7BPNRDYK4QR8ofJaEEF2heacn0QgD.f8pO8SNp83XNdWG6tocBM1' - hostname: ubuntu + autoinstall: + identity: + realname: 'Ubuntu User' + username: ubuntu + password: '$6$wdAcoXrU039hKYPd$508Qvbe7ObUnxoj15DRCkzC3qO7edjH0VV7BPNRDYK4QR8ofJaEEF2heacn0QgD.f8pO8SNp83XNdWG6tocBM1' + hostname: ubuntu .. _ai-active-directory: @@ -758,10 +807,11 @@ A list of snaps to install. Each snap is represented as a mapping with a require .. code-block:: yaml - snaps: - - name: etcd - channel: edge - classic: false + autoinstall: + snaps: + - name: etcd + channel: edge + classic: false .. _ai-debconf-selections: @@ -898,41 +948,45 @@ The default configuration is: .. code-block:: yaml - reporting: - builtin: - type: print + autoinstall: + reporting: + builtin: + type: print Report to rsyslog: .. code-block:: yaml - reporting: - central: - type: rsyslog - destination: "@192.168.0.1" + autoinstall: + reporting: + central: + type: rsyslog + destination: "@192.168.0.1" Suppress the default output: .. code-block:: yaml - reporting: - builtin: - type: none + autoinstall: + reporting: + builtin: + type: none Report to a curtin-style webhook: .. code-block:: yaml - reporting: - hook: - type: webhook - endpoint: http://example.com/endpoint/path - consumer_key: "ck_value" - consumer_secret: "cs_value" - token_key: "tk_value" - token_secret: "tk_secret" - level: INFO + autoinstall: + reporting: + hook: + type: webhook + endpoint: http://example.com/endpoint/path + consumer_key: "ck_value" + consumer_secret: "cs_value" + token_key: "tk_value" + token_secret: "tk_secret" + level: INFO .. _ai-user-data: From 26761b4f1a7f1ff8077e43ba27785f3bde387556 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Tue, 26 Mar 2024 10:56:52 -0700 Subject: [PATCH 5/5] doc: update wordlist --- doc/.custom_wordlist.txt | 1 + doc/.wordlist.txt | 2 ++ 2 files changed, 3 insertions(+) diff --git a/doc/.custom_wordlist.txt b/doc/.custom_wordlist.txt index b5cb8b9f..e5b9e8de 100644 --- a/doc/.custom_wordlist.txt +++ b/doc/.custom_wordlist.txt @@ -73,6 +73,7 @@ pw realname rootfs rsyslog +runtime subvolume subvolumes superset diff --git a/doc/.wordlist.txt b/doc/.wordlist.txt index 828beee9..8a0ec8a1 100644 --- a/doc/.wordlist.txt +++ b/doc/.wordlist.txt @@ -8,6 +8,8 @@ EBS EKS Grafana IAM +ISO +ISOs JSON Jira Juju