From 57ebc42ffd64babb121c940caa3c5ff415062162 Mon Sep 17 00:00:00 2001 From: Daniel Girtler Date: Wed, 28 Jun 2023 21:34:54 +1000 Subject: Fix 1875 (#1880) Co-authored-by: Daniel Girtler --- archinstall/lib/disk/device_handler.py | 41 +++++++++++++---------- archinstall/lib/disk/device_model.py | 19 ++++++++--- archinstall/lib/installer.py | 61 +++++++++++++++++----------------- pyproject.toml | 1 + 4 files changed, 69 insertions(+), 53 deletions(-) diff --git a/archinstall/lib/disk/device_handler.py b/archinstall/lib/disk/device_handler.py index 4341c53c..2c88e382 100644 --- a/archinstall/lib/disk/device_handler.py +++ b/archinstall/lib/disk/device_handler.py @@ -238,41 +238,46 @@ class DeviceHandler(object): info(f'luks2 locking device: {dev_path}') luks_handler.lock() + def _validate(self, device_mod: DeviceModification): + checks = { + # verify that all partitions have a path set (which implies that they have been created) + lambda x: x.dev_path is None: ValueError('When formatting, all partitions must have a path set'), + # crypto luks is not a valid file system type + lambda x: x.fs_type is FilesystemType.Crypto_luks: ValueError('Crypto luks cannot be set as a filesystem type'), + # file system type must be set + lambda x: x.fs_type is None: ValueError('File system type must be set for modification') + } + + for check, exc in checks.items(): + found = next(filter(check, device_mod.partitions), None) + if found is not None: + raise exc + def format( self, - modification: DeviceModification, + device_mod: DeviceModification, enc_conf: Optional['DiskEncryption'] = None ): """ Format can be given an overriding path, for instance /dev/null to test the formatting functionality and in essence the support for the given filesystem. """ - - # verify that all partitions have a path set (which implies that they have been created) - missing_path = next(filter(lambda x: x.dev_path is None, modification.partitions), None) - if missing_path is not None: - raise ValueError('When formatting, all partitions must have a path set') - - # crypto luks is not known to parted and can therefore not - # be used as a filesystem type in that sense; - invalid_fs_type = next(filter(lambda x: x.fs_type is FilesystemType.Crypto_luks, modification.partitions), None) - if invalid_fs_type is not None: - raise ValueError('Crypto luks cannot be set as a filesystem type') + self._validate(device_mod) # make sure all devices are unmounted - self._umount_all_existing(modification) + self._umount_all_existing(device_mod) - for part_mod in modification.partitions: + for part_mod in device_mod.partitions: # partition will be encrypted if enc_conf is not None and part_mod in enc_conf.partitions: self._perform_enc_formatting( part_mod.safe_dev_path, part_mod.mapper_name, - part_mod.fs_type, + part_mod.safe_fs_type, enc_conf ) else: - self._perform_formatting(part_mod.fs_type, part_mod.safe_dev_path) + self._perform_formatting(part_mod.safe_fs_type, part_mod.safe_dev_path) def _perform_partitioning( self, @@ -312,7 +317,7 @@ class DeviceHandler(object): length=length_sector.value ) - filesystem = FileSystem(type=part_mod.fs_type.value, geometry=geometry) + filesystem = FileSystem(type=part_mod.safe_fs_type.value, geometry=geometry) partition = Partition( disk=disk, @@ -325,7 +330,7 @@ class DeviceHandler(object): partition.setFlag(flag.value) debug(f'\tType: {part_mod.type.value}') - debug(f'\tFilesystem: {part_mod.fs_type.value}') + debug(f'\tFilesystem: {part_mod.safe_fs_type.value}') debug(f'\tGeometry: {start_sector.value} start sector, {length_sector.value} length') try: diff --git a/archinstall/lib/disk/device_model.py b/archinstall/lib/disk/device_model.py index 35fbd40c..97623772 100644 --- a/archinstall/lib/disk/device_model.py +++ b/archinstall/lib/disk/device_model.py @@ -283,7 +283,7 @@ class _PartitionInfo: partition: Partition name: str type: PartitionType - fs_type: FilesystemType + fs_type: Optional[FilesystemType] path: Path start: Size length: Size @@ -313,7 +313,7 @@ class _PartitionInfo: def from_partition( cls, partition: Partition, - fs_type: FilesystemType, + fs_type: Optional[FilesystemType], partuuid: str, mountpoints: List[Path], btrfs_subvol_infos: List[_BtrfsSubvolumeInfo] = [] @@ -594,7 +594,7 @@ class PartitionModification: type: PartitionType start: Size length: Size - fs_type: FilesystemType + fs_type: Optional[FilesystemType] mountpoint: Optional[Path] = None mount_options: List[str] = field(default_factory=list) flags: List[PartitionFlag] = field(default_factory=list) @@ -613,6 +613,9 @@ class PartitionModification: if self.is_exists_or_modify() and not self.dev_path: raise ValueError('If partition marked as existing a path must be set') + if self.fs_type is None and self.status == ModificationStatus.Modify: + raise ValueError('FS type must not be empty on modifications with status type modify') + def __hash__(self): return hash(self._obj_id) @@ -628,6 +631,12 @@ class PartitionModification: raise ValueError('Device path was not set') return self.dev_path + @property + def safe_fs_type(self) -> FilesystemType: + if self.fs_type is None: + raise ValueError('File system type is not set') + return self.fs_type + @classmethod def from_existing_partition(cls, partition_info: _PartitionInfo) -> PartitionModification: if partition_info.btrfs_subvol_infos: @@ -714,7 +723,7 @@ class PartitionModification: 'type': self.type.value, 'start': self.start.__dump__(), 'length': self.length.__dump__(), - 'fs_type': self.fs_type.value, + 'fs_type': self.fs_type.value if self.fs_type else '', 'mountpoint': str(self.mountpoint) if self.mountpoint else None, 'mount_options': self.mount_options, 'flags': [f.name for f in self.flags], @@ -731,7 +740,7 @@ class PartitionModification: 'Type': self.type.value, 'Start': self.start.format_size(Unit.MiB), 'Length': self.length.format_size(Unit.MiB), - 'FS type': self.fs_type.value, + 'FS type': self.fs_type.value if self.fs_type else 'Unknown', 'Mountpoint': self.mountpoint if self.mountpoint else '', 'Mount options': ', '.join(self.mount_options), 'Flags': ', '.join([f.name for f in self.flags]), diff --git a/archinstall/lib/installer.py b/archinstall/lib/installer.py index 0680bf6e..f1a7f71a 100644 --- a/archinstall/lib/installer.py +++ b/archinstall/lib/installer.py @@ -667,28 +667,29 @@ class Installer: ): for mod in self._disk_config.device_modifications: for part in mod.partitions: - if (pkg := part.fs_type.installation_pkg) is not None: - self.base_packages.append(pkg) - if (module := part.fs_type.installation_module) is not None: - self.modules.append(module) - if (binary := part.fs_type.installation_binary) is not None: - self._binaries.append(binary) - - # There is not yet an fsck tool for NTFS. If it's being used for the root filesystem, the hook should be removed. - if part.fs_type.fs_type_mount == 'ntfs3' and part.mountpoint == self.target: - if 'fsck' in self._hooks: - self._hooks.remove('fsck') - - if part in self._disk_encryption.partitions: - if self._disk_encryption.hsm_device: - # Required bby mkinitcpio to add support for fido2-device options - self._pacstrap('libfido2') - - if 'sd-encrypt' not in self._hooks: - self._hooks.insert(self._hooks.index('filesystems'), 'sd-encrypt') - else: - if 'encrypt' not in self._hooks: - self._hooks.insert(self._hooks.index('filesystems'), 'encrypt') + if part.fs_type is not None: + if (pkg := part.fs_type.installation_pkg) is not None: + self.base_packages.append(pkg) + if (module := part.fs_type.installation_module) is not None: + self.modules.append(module) + if (binary := part.fs_type.installation_binary) is not None: + self._binaries.append(binary) + + # There is not yet an fsck tool for NTFS. If it's being used for the root filesystem, the hook should be removed. + if part.fs_type.fs_type_mount == 'ntfs3' and part.mountpoint == self.target: + if 'fsck' in self._hooks: + self._hooks.remove('fsck') + + if part in self._disk_encryption.partitions: + if self._disk_encryption.hsm_device: + # Required bby mkinitcpio to add support for fido2-device options + self._pacstrap('libfido2') + + if 'sd-encrypt' not in self._hooks: + self._hooks.insert(self._hooks.index('filesystems'), 'sd-encrypt') + else: + if 'encrypt' not in self._hooks: + self._hooks.insert(self._hooks.index('filesystems'), 'encrypt') if not SysInfo.has_uefi(): self.base_packages.append('grub') @@ -857,7 +858,7 @@ class Installer: # blkid doesn't trigger on loopback devices really well, # so we'll use the old manual method until we get that sorted out. - options_entry = f'rw rootfstype={root_partition.fs_type.fs_type_mount} {" ".join(self._kernel_params)}\n' + options_entry = f'rw rootfstype={root_partition.safe_fs_type.fs_type_mount} {" ".join(self._kernel_params)}\n' for sub_vol in root_partition.btrfs_subvols: if sub_vol.is_root(): @@ -868,7 +869,7 @@ class Installer: if self._zram_enabled: options_entry = "zswap.enabled=0 " + options_entry - if root_partition.fs_type.is_crypto(): + if root_partition.safe_fs_type.is_crypto(): # TODO: We need to detect if the encrypted device is a whole disk encryption, # or simply a partition encryption. Right now we assume it's a partition (and we always have) debug('Root partition is an encrypted device, identifying by PARTUUID: {root_partition.partuuid}') @@ -900,15 +901,15 @@ class Installer: _file = "/etc/default/grub" - if root_partition.fs_type.is_crypto(): + if root_partition.safe_fs_type.is_crypto(): debug(f"Using UUID {root_partition.uuid} as encrypted root identifier") - cmd_line_linux = f"sed -i 's/GRUB_CMDLINE_LINUX=\"\"/GRUB_CMDLINE_LINUX=\"cryptdevice=UUID={root_partition.uuid}:cryptlvm rootfstype={root_partition.fs_type.value}\"/'" + cmd_line_linux = f"sed -i 's/GRUB_CMDLINE_LINUX=\"\"/GRUB_CMDLINE_LINUX=\"cryptdevice=UUID={root_partition.uuid}:cryptlvm rootfstype={root_partition.safe_fs_type.value}\"/'" enable_cryptdisk = "sed -i 's/#GRUB_ENABLE_CRYPTODISK=y/GRUB_ENABLE_CRYPTODISK=y/'" SysCommand(f"/usr/bin/arch-chroot {self.target} {enable_cryptdisk} {_file}") else: - cmd_line_linux = f"sed -i 's/GRUB_CMDLINE_LINUX=\"\"/GRUB_CMDLINE_LINUX=\"rootfstype={root_partition.fs_type.value}\"/'" + cmd_line_linux = f"sed -i 's/GRUB_CMDLINE_LINUX=\"\"/GRUB_CMDLINE_LINUX=\"rootfstype={root_partition.safe_fs_type.value}\"/'" SysCommand(f"/usr/bin/arch-chroot {self.target} {cmd_line_linux} {_file}") @@ -984,14 +985,14 @@ class Installer: # blkid doesn't trigger on loopback devices really well, # so we'll use the old manual method until we get that sorted out. - if root_partition.fs_type.is_crypto(): + if root_partition.safe_fs_type.is_crypto(): # TODO: We need to detect if the encrypted device is a whole disk encryption, # or simply a partition encryption. Right now we assume it's a partition (and we always have) debug(f'Identifying root partition by PARTUUID: {root_partition.partuuid}') - kernel_parameters.append(f'cryptdevice=PARTUUID={root_partition.partuuid}:luksdev root=/dev/mapper/luksdev rw rootfstype={root_partition.fs_type.value} {" ".join(self._kernel_params)}') + kernel_parameters.append(f'cryptdevice=PARTUUID={root_partition.partuuid}:luksdev root=/dev/mapper/luksdev rw rootfstype={root_partition.safe_fs_type.value} {" ".join(self._kernel_params)}') else: debug(f'Root partition is an encrypted device identifying by PARTUUID: {root_partition.partuuid}') - kernel_parameters.append(f'root=PARTUUID={root_partition.partuuid} rw rootfstype={root_partition.fs_type.value} {" ".join(self._kernel_params)}') + kernel_parameters.append(f'root=PARTUUID={root_partition.partuuid} rw rootfstype={root_partition.safe_fs_type.value} {" ".join(self._kernel_params)}') device = disk.device_handler.get_device_by_partition_path(boot_partition.safe_dev_path) diff --git a/pyproject.toml b/pyproject.toml index 8b6ae4c7..f67f1eca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,6 +63,7 @@ packages = ["archinstall"] python_version = "3.10" files = "archinstall/" exclude = "tests" +#check_untyped_defs=true [tool.bandit] targets = ["archinstall"] -- cgit v1.2.3-70-g09d2