From 7e2042320541b91829965b8ee1d6f53627b8ac38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 30 Sep 2024 12:30:23 +0100 Subject: [PATCH 01/10] schema: 'generate' section for pvs --- .../share/examples/storage/generate_pvs.json | 48 ++++++++++++++++ rust/agama-lib/share/profile.schema.json | 55 +++++++++++++++++-- 2 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 rust/agama-lib/share/examples/storage/generate_pvs.json diff --git a/rust/agama-lib/share/examples/storage/generate_pvs.json b/rust/agama-lib/share/examples/storage/generate_pvs.json new file mode 100644 index 000000000..709406bb8 --- /dev/null +++ b/rust/agama-lib/share/examples/storage/generate_pvs.json @@ -0,0 +1,48 @@ +{ + "storage": { + "drives": [ + { + "alias": "first-disk" + }, + { + "partitions": [ + { + "alias": "pv1", + "id": "lvm", + "size": { "min": "10 GiB" } + } + ] + } + ], + "volumeGroups": [ + { + "name": "system", + "physicalVolumes": ["pv1"], + "logicalVolumes": [ + { + "filesystem": { "path": "/" } + } + ] + }, + { + "name": "logs", + "physicalVolumes": [ + { "generate": ["first-disk"] } + ] + }, + { + "name": "data", + "physicalVolumes": [ + { + "generate": { + "targetDevices": ["first-disk"], + "encryption": { + "luks2": { "password": "12345" } + } + } + } + ] + } + ] + } +} diff --git a/rust/agama-lib/share/profile.schema.json b/rust/agama-lib/share/profile.schema.json index dab5ccc21..8081fff53 100644 --- a/rust/agama-lib/share/profile.schema.json +++ b/rust/agama-lib/share/profile.schema.json @@ -529,11 +529,56 @@ "physicalVolumes": { "title": "Physical volumes", "description": "Devices to use as physical volumes.", - "type": "array", - "items": { - "title": "Device alias", - "type": "string" - } + "$comment": "In the future it would be possible to indicate both aliases and 'generate' items together.", + "anyOf": [ + { + "type": "array", + "items": { + "title": "Device alias", + "type": "string" + } + }, + { + "type": "array", + "items": { + "title": "Generate physical volumes", + "description": "Automatically creates the needed physical volumes in the indicated devices.", + "type": "object", + "additionalProperties": false, + "required": ["generate"], + "properties": { + "generate": { + "anyOf": [ + { + "type": "array", + "items": { + "title": "Device alias", + "type": "string" + } + }, + { + "type": "object", + "additionalProperties": false, + "required": ["targetDevices"], + "properties": { + "targetDevices": { + "type": "array", + "items": { + "title": "Device alias", + "type": "string" + } + }, + "encryption": { + "$ref": "#/$defs/encryption" + } + } + } + ] + } + } + } + } + ] }, "logicalVolumes": { "title": "Logical volumes", From 7e7bc4d10576de069c21cf465c06d37efce785a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 3 Oct 2024 12:35:14 +0100 Subject: [PATCH 02/10] storage: add solvers for filesystem and encryption --- .../agama/storage/config_encryption_solver.rb | 94 ++++++++++++++++ .../agama/storage/config_filesystem_solver.rb | 106 ++++++++++++++++++ .../lib/agama/storage/config_size_solver.rb | 4 + service/lib/agama/storage/config_solver.rb | 11 +- service/lib/agama/storage/configs/btrfs.rb | 19 +++- 5 files changed, 226 insertions(+), 8 deletions(-) create mode 100644 service/lib/agama/storage/config_encryption_solver.rb create mode 100644 service/lib/agama/storage/config_filesystem_solver.rb diff --git a/service/lib/agama/storage/config_encryption_solver.rb b/service/lib/agama/storage/config_encryption_solver.rb new file mode 100644 index 000000000..4a5996af8 --- /dev/null +++ b/service/lib/agama/storage/config_encryption_solver.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "agama/storage/config_builder" + +module Agama + module Storage + # Solver for the encryption configs. + # + # The encryption configs are solved by assigning the default encryption values defined by the + # productd, if needed. + class ConfigEncryptionSolver + # @param product_config [Agama::Config] + def initialize(product_config) + @product_config = product_config + end + + # Solves all the encryption configs within a given config. + # + # @note The config object is modified. + # + # @param config [Config] + def solve(config) + @config = config + + configs_with_encryption.each { |c| solve_encryption(c) } + end + + private + + # @return [Agama::Config] + attr_reader :product_config + + # @return [Config] + attr_reader :config + + # @param config [#encryption] + def solve_encryption(config) + return unless config.encryption + + encryption = config.encryption + encryption.method ||= default_encryption.method + + # Recovering values from the default encryption only makes sense if the encryption method is + # the same. + solve_encryption_values(encryption) if encryption.method == default_encryption.method + end + + # @param config [Configs::Encryption] + def solve_encryption_values(config) + config.password ||= default_encryption.password + config.pbkd_function ||= default_encryption.pbkd_function + config.label ||= default_encryption.label + config.cipher ||= default_encryption.cipher + config.key_size ||= default_encryption.key_size + end + + # @return [Array<#encryption>] + def configs_with_encryption + config.drives + config.partitions + config.logical_volumes + end + + # Default encryption defined by the product. + # + # @return [Configs::Encryption] + def default_encryption + @default_encryption ||= config_builder.default_encryption + end + + # @return [ConfigBuilder] + def config_builder + @config_builder ||= ConfigBuilder.new(product_config) + end + end + end +end diff --git a/service/lib/agama/storage/config_filesystem_solver.rb b/service/lib/agama/storage/config_filesystem_solver.rb new file mode 100644 index 000000000..abb9ba423 --- /dev/null +++ b/service/lib/agama/storage/config_filesystem_solver.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "agama/storage/config_builder" + +module Agama + module Storage + # Solver for the filesystem configs. + # + # The filesystem configs are solved by assigning the default filesystem values defined by the + # productd, if needed. + class ConfigFilesystemSolver + # @param product_config [Agama::Config] + def initialize(product_config) + @product_config = product_config + end + + # Solves all the filesystem configs within a given config. + # + # @note The config object is modified. + # + # @param config [Config] + def solve(config) + @config = config + + configs_with_filesystem.each { |c| solve_filesystem(c) } + end + + private + + # @return [Agama::Config] + attr_reader :product_config + + # @return [Config] + attr_reader :config + + # @param config [#filesystem] + def solve_filesystem(config) + return unless config.filesystem + + default_filesystem = default_filesystem(config.filesystem.path) + + config.filesystem.type ||= default_filesystem.type + config.filesystem.type.btrfs ||= default_filesystem.type.btrfs + solve_btrfs_values(config) + end + + # @param config [#filesystem] + def solve_btrfs_values(config) + btrfs = config.filesystem.type.btrfs + return unless btrfs + + default_btrfs = default_btrfs(config.filesystem.path) + + btrfs.snapshots = default_btrfs.snapshots? if btrfs.snapshots.nil? + btrfs.read_only = default_btrfs.read_only? if btrfs.read_only.nil? + btrfs.subvolumes ||= default_btrfs.subvolumes + btrfs.default_subvolume ||= (default_btrfs.default_subvolume || "") + end + + # @return [Array<#filesystem>] + def configs_with_filesystem + config.drives + config.partitions + config.logical_volumes + end + + # Default filesystem defined by the product. + # + # @param path [String, nil] + # @return [Configs::Filesystem] + def default_filesystem(path = nil) + config_builder.default_filesystem(path) + end + + # Default btrfs config defined by the product. + # + # @param path [String, nil] + # @return [Configs::Btrfs] + def default_btrfs(path = nil) + default_filesystem(path).type.btrfs + end + + # @return [ConfigBuilder] + def config_builder + @config_builder ||= ConfigBuilder.new(product_config) + end + end + end +end diff --git a/service/lib/agama/storage/config_size_solver.rb b/service/lib/agama/storage/config_size_solver.rb index 69449ae8c..bb8ffbc5b 100644 --- a/service/lib/agama/storage/config_size_solver.rb +++ b/service/lib/agama/storage/config_size_solver.rb @@ -175,6 +175,10 @@ def with_current_size?(config) !config.size.default? && (config.size.min.nil? || config.size.max.nil?) end + # Whether the config has to be considered. + # + # Note that a config could be ignored if a device is not found for its search. + # # @param config [Object] Any config from {Configs}. # @return [Boolean] def valid?(config) diff --git a/service/lib/agama/storage/config_solver.rb b/service/lib/agama/storage/config_solver.rb index 505dc06a8..36f8a9995 100644 --- a/service/lib/agama/storage/config_solver.rb +++ b/service/lib/agama/storage/config_solver.rb @@ -19,6 +19,8 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. +require "agama/storage/config_encryption_solver" +require "agama/storage/config_filesystem_solver" require "agama/storage/config_search_solver" require "agama/storage/config_size_solver" @@ -26,7 +28,9 @@ module Agama module Storage # Class for solving a storage config. # - # It assigns proper devices and size values according to the product and the system. + # Solving a config means to assign proper values according to the product and the system. For + # example, the sizes of a partition config taking into account its fallbacks, assigning a + # specific device when a config has a search, etc. class ConfigSolver # @param devicegraph [Y2Storage::Devicegraph] # @param product_config [Agama::Config] @@ -35,13 +39,16 @@ def initialize(devicegraph, product_config) @product_config = product_config end - # Solves all the search and size configs within a given config. + # Solves the config according to the product and the system. # # @note The config object is modified. # # @param config [Config] def solve(config) + ConfigEncryptionSolver.new(product_config).solve(config) + ConfigFilesystemSolver.new(product_config).solve(config) ConfigSearchSolver.new(devicegraph).solve(config) + # Sizes must be solved once the searches are solved. ConfigSizeSolver.new(devicegraph, product_config).solve(config) end diff --git a/service/lib/agama/storage/configs/btrfs.rb b/service/lib/agama/storage/configs/btrfs.rb index 1dd3cf4de..21f7f65fe 100644 --- a/service/lib/agama/storage/configs/btrfs.rb +++ b/service/lib/agama/storage/configs/btrfs.rb @@ -26,26 +26,33 @@ module Configs class Btrfs # Whether there are snapshots. # - # @return [Boolean] + # @return [Boolean, nil] attr_accessor :snapshots - alias_method :snapshots?, :snapshots - # @return [Boolean] + # @return [Boolean, nil] attr_accessor :read_only - alias_method :read_only?, :read_only # @return [Array, nil] if nil, a historical fallback list # may be applied depending on the mount path of the volume attr_accessor :subvolumes - # @return [String] + # @return [String, nil] attr_accessor :default_subvolume # Constructor def initialize @snapshots = false @read_only = false - @default_subvolume = "" + end + + # @return [Boolean] + def snapshots? + !!snapshots + end + + # @return [Boolean] + def read_only? + !!read_only end end end From a37a516e2d903f57c9129470ebcbf6a7f31b1e56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 3 Oct 2024 12:37:21 +0100 Subject: [PATCH 03/10] refactor(storage): remove solving values from JSON conversions - Solving encryption and filesystem configs is now done by the config solver. - JSON conversion only applies the values provided by the JSON data and expands 'generate' for partitions and logical volumes. --- service/lib/agama/config.rb | 23 + service/lib/agama/storage/config.rb | 10 - .../storage/config_conversions/from_json.rb | 40 +- .../from_json_conversions/base.rb | 25 +- .../from_json_conversions/boot.rb | 19 +- .../from_json_conversions/btrfs.rb | 19 +- .../from_json_conversions/config.rb | 33 +- .../from_json_conversions/drive.rb | 30 +- .../from_json_conversions/encryption.rb | 32 +- .../from_json_conversions/filesystem.rb | 36 +- .../from_json_conversions/filesystem_type.rb | 26 +- .../from_json_conversions/logical_volume.rb | 26 +- .../from_json_conversions/partition.rb | 27 +- .../from_json_conversions/search.rb | 19 +- .../from_json_conversions/size.rb | 19 +- .../from_json_conversions/volume_group.rb | 26 +- .../from_json_conversions/with_encryption.rb | 11 +- .../from_json_conversions/with_filesystem.rb | 14 +- .../from_json_conversions/with_partitions.rb | 9 +- .../from_json_conversions/with_ptable_type.rb | 6 +- .../from_json_conversions/with_search.rb | 10 +- .../from_json_conversions/with_size.rb | 9 +- .../lib/agama/storage/config_json_solver.rb | 44 +- .../config_conversions/from_json_test.rb | 2361 ++++++++--------- 24 files changed, 1327 insertions(+), 1547 deletions(-) diff --git a/service/lib/agama/config.rb b/service/lib/agama/config.rb index 3b4c2ba81..4712b5c55 100644 --- a/service/lib/agama/config.rb +++ b/service/lib/agama/config.rb @@ -186,8 +186,31 @@ def arch_elements_from(*keys, property: nil, default: []) end.compact end + # Default paths to be created for the product. + # + # @return [Array] + def default_paths + data.dig("storage", "volumes") || [] + end + + # Mandatory paths to be created for the product. + # + # @return [Array] + def mandatory_paths + default_paths.select { |p| mandatory_path?(p) } + end + private + def mandatory_path?(path) + templates = data.dig("storage", "volume_templates") || [] + template = templates.find { |t| t["mount_path"] == path } + + return false unless template + + template.dig("outline", "required") || false + end + # Simple deep merge # # @param a_hash [Hash] Default values diff --git a/service/lib/agama/storage/config.rb b/service/lib/agama/storage/config.rb index 05e532675..ab8166010 100644 --- a/service/lib/agama/storage/config.rb +++ b/service/lib/agama/storage/config.rb @@ -55,16 +55,6 @@ def initialize @nfs_mounts = [] end - # Creates a config from JSON hash according to schema. - # - # @param config_json [Hash] - # @param product_config [Agama::Config] - # - # @return [Storage::Config] - def self.new_from_json(config_json, product_config:) - ConfigConversions::FromJSON.new(config_json, product_config: product_config).convert - end - # Name of the device that will presumably be used to boot the target system # # @return [String, nil] nil if there is no enough information to infer a possible boot disk diff --git a/service/lib/agama/storage/config_conversions/from_json.rb b/service/lib/agama/storage/config_conversions/from_json.rb index 26e210765..86cddab38 100644 --- a/service/lib/agama/storage/config_conversions/from_json.rb +++ b/service/lib/agama/storage/config_conversions/from_json.rb @@ -20,7 +20,6 @@ # find current contact information at www.suse.com. require "agama/config" -require "agama/storage/config_builder" require "agama/storage/config_conversions/from_json_conversions/config" require "agama/storage/config_json_solver" @@ -29,14 +28,13 @@ module Storage module ConfigConversions # Config conversion from JSON hash according to schema. class FromJSON - # TODO: Replace product_config param by a ProductDefinition. - # # @param config_json [Hash] - # @param product_config [Agama::Config, nil] - def initialize(config_json, product_config: nil) - # Copies the JSON hash to avoid changes in the given parameter, see {ConfigJSONSolver}. - @config_json = json_dup(config_json) - @product_config = product_config || Agama::Config.new + # @param default_paths [Array] Default paths of the product. + # @param mandatory_paths [Array] Mandatory paths of the product. + def initialize(config_json, default_paths: [], mandatory_paths: []) + @config_json = config_json + @default_paths = default_paths + @mandatory_paths = mandatory_paths end # Performs the conversion from Hash according to the JSON schema. @@ -44,17 +42,15 @@ def initialize(config_json, product_config: nil) # @return [Storage::Config] def convert # TODO: Raise error if config_json does not match the JSON schema. - # Implementation idea: ConfigJSONChecker class which reports issues if: - # * The JSON does not match the schema. - # * The JSON contains more than one "generate" for partitions and logical volumes. - # * The JSON contains invalid aliases (now checked by ConfigChecker). + + # Copies the JSON hash to avoid changes in the given config, see {ConfigJSONSolver}. + config_json = json_dup(self.config_json) + ConfigJSONSolver - .new(product_config) + .new(default_paths: default_paths, mandatory_paths: mandatory_paths) .solve(config_json) - FromJSONConversions::Config - .new(config_json, config_builder: config_builder) - .convert + FromJSONConversions::Config.new(config_json).convert end private @@ -62,8 +58,11 @@ def convert # @return [Hash] attr_reader :config_json - # @return [Agama::Config] - attr_reader :product_config + # @return [Array] + attr_reader :default_paths + + # @return [Array] + attr_reader :mandatory_paths # Deep dup of the given JSON. # @@ -72,11 +71,6 @@ def convert def json_dup(json) Marshal.load(Marshal.dump(json)) end - - # @return [ConfigBuilder] - def config_builder - @config_builder ||= ConfigBuilder.new(product_config) - end end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/base.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/base.rb index c295b9242..e3e449f0f 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/base.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/base.rb @@ -25,35 +25,32 @@ module ConfigConversions module FromJSONConversions # Base class for conversions from JSON hash according to schema. class Base - # @param config_builder [ConfigBuilder, nil] - def initialize(config_builder = nil) - @config_builder = config_builder + def initialize(config_json) + @config_json = config_json end # Performs the conversion from Hash according to the JSON schema. # - # @param default [Object] A {Config} or any its configs from {Storage::Configs}. + # @param config [Object] A {Config} or any of its configs from {Storage::Configs}. # @return [Object] A {Config} or any its configs from {Storage::Configs}. - def convert(default) - default.dup.tap do |config| - conversions(config).each do |property, value| - next if value.nil? + def convert(config) + conversions.each do |property, value| + next if value.nil? - config.public_send("#{property}=", value) - end + config.public_send("#{property}=", value) end + + config end private - # @return [ConfigBuilder, nil] - attr_reader :config_builder + attr_reader :config_json # Values to apply to the config. # - # @param _default [Object] A {Config} or any its configs from {Storage::Configs}. # @return [Hash] e.g., { name: "/dev/vda" }. - def conversions(_default) + def conversions {} end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/boot.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/boot.rb index 259d12ec1..e2d2096ab 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/boot.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/boot.rb @@ -28,30 +28,19 @@ module ConfigConversions module FromJSONConversions # Boot conversion from JSON hash according to schema. class Boot < Base - # @param boot_json [Hash] - def initialize(boot_json) - super() - @boot_json = boot_json - end - # @see Base#convert - # - # @param default [Configs::Boot, nil] # @return [Configs::Boot] - def convert(default = nil) - super(default || Configs::Boot.new) + def convert + super(Configs::Boot.new) end private - # @return [Hash] - attr_reader :boot_json + alias_method :boot_json, :config_json # @see Base#conversions - # - # @param _default [Configs::Boot] # @return [Hash] - def conversions(_default) + def conversions { configure: boot_json[:configure], device: boot_json[:device] diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/btrfs.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/btrfs.rb index 0e95b0a24..f658287b7 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/btrfs.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/btrfs.rb @@ -28,30 +28,19 @@ module ConfigConversions module FromJSONConversions # Btrfs conversion from JSON hash according to schema. class Btrfs < Base - # @param btrfs_json [Hash] - def initialize(btrfs_json) - super() - @btrfs_json = btrfs_json - end - # @see Base#convert - # - # @param default [Configs::Btrfs, nil] # @return [Configs::Btrfs] - def convert(default = nil) - super(default || Configs::Btrfs.new) + def convert + super(Configs::Btrfs.new) end private - # @return [String] - attr_reader :btrfs_json + alias_method :btrfs_json, :config_json # @see Base#conversions - # - # @param _default [Configs::Btrfs] # @return [Hash] - def conversions(_default) + def conversions { snapshots: btrfs_json[:snapshots] } diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/config.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/config.rb index fbb4c6fd3..4e86fef67 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/config.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/config.rb @@ -31,45 +31,30 @@ module ConfigConversions module FromJSONConversions # Config conversion from JSON hash according to schema. class Config < Base - # @param config_json [Hash] - # @param config_builder [ConfigBuilder, nil] - def initialize(config_json, config_builder: nil) - super(config_builder) - @config_json = config_json - end - # @see Base#convert - # - # @param default [Config, nil] # @return [Config] - def convert(default = nil) - super(default || Storage::Config.new) + def convert + super(Storage::Config.new) end private - # @return [Hash] - attr_reader :config_json - # @see Base#conversions - # - # @param default [Config] # @return [Hash] - def conversions(default) + def conversions { - boot: convert_boot(default.boot), + boot: convert_boot, drives: convert_drives, volume_groups: convert_volume_groups } end - # @param default [Configs::Boot, nil] # @return [Configs::Boot, nil] - def convert_boot(default = nil) + def convert_boot boot_json = config_json[:boot] return unless boot_json - FromJSONConversions::Boot.new(boot_json).convert(default) + FromJSONConversions::Boot.new(boot_json).convert end # @return [Array, nil] @@ -83,7 +68,7 @@ def convert_drives # @param drive_json [Hash] # @return [Configs::Drive] def convert_drive(drive_json) - FromJSONConversions::Drive.new(drive_json, config_builder: config_builder).convert + FromJSONConversions::Drive.new(drive_json).convert end # @return [Array, nil] @@ -97,9 +82,7 @@ def convert_volume_groups # @param volume_group_json [Hash] # @return [Configs::VolumeGroup] def convert_volume_group(volume_group_json) - FromJSONConversions::VolumeGroup - .new(volume_group_json, config_builder: config_builder) - .convert + FromJSONConversions::VolumeGroup.new(volume_group_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/drive.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/drive.rb index 8146da620..8fd661dd6 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/drive.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/drive.rb @@ -39,38 +39,26 @@ class Drive < Base include WithPtableType include WithPartitions - # @param drive_json [Hash] - # @param config_builder [ConfigBuilder, nil] - def initialize(drive_json, config_builder: nil) - super(config_builder) - @drive_json = drive_json - end - # @see Base#convert - # - # @param default [Configs::Drive, nil] # @return [Configs::Drive] - def convert(default = nil) - super(default || Configs::Drive.new) + def convert + super(Configs::Drive.new) end private - # @return [Hash] - attr_reader :drive_json + alias_method :drive_json, :config_json # @see Base#conversions - # - # @param default [Configs::Drive] # @return [Hash] - def conversions(default) + def conversions { - search: convert_search(drive_json, default: default.search), + search: convert_search, alias: drive_json[:alias], - encryption: convert_encryption(drive_json, default: default.encryption), - filesystem: convert_filesystem(drive_json, default: default.filesystem), - ptable_type: convert_ptable_type(drive_json), - partitions: convert_partitions(drive_json) + encryption: convert_encryption, + filesystem: convert_filesystem, + ptable_type: convert_ptable_type, + partitions: convert_partitions } end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb index c280f0ebf..0bafd6a99 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb @@ -30,34 +30,19 @@ module ConfigConversions module FromJSONConversions # Encryption conversion from JSON hash according to schema. class Encryption < Base - # @param encryption_json [Hash, String] - # @param config_builder [ConfigBuilder, nil] - def initialize(encryption_json, config_builder: nil) - super(config_builder) - @encryption_json = encryption_json - end - # @see Base#convert - # - # @param default [Configs::Encryption, nil] # @return [Configs::Encryption] - def convert(default = nil) - super(default || self.default) + def convert + super(Configs::Encryption.new) end private - # @return [Hash, String] - attr_reader :encryption_json - - # @return [Configs::Encryption] - attr_reader :default_config + alias_method :encryption_json, :config_json # @see Base#conversions - # - # @param _default [Configs::Encryption] # @return [Hash] - def conversions(_default) + def conversions return luks1_conversions if luks1? return luks2_conversions if luks2? return pervasive_luks2_conversions if pervasive_luks2? @@ -159,15 +144,6 @@ def convert_label def convert_pbkd_function Y2Storage::PbkdFunction.find(encryption_json.dig(:luks2, :pbkdFunction)) end - - # Default encryption config. - # - # @return [Configs::Encryption] - def default - return Configs::Encryption.new unless config_builder - - config_builder.default_encryption - end end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem.rb index 68b605c7f..b5aac1405 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem.rb @@ -30,31 +30,19 @@ module ConfigConversions module FromJSONConversions # Filesystem conversion from JSON hash according to schema. class Filesystem < Base - # @param filesystem_json [Hash] - # @param config_builder [ConfigBuilder, nil] - def initialize(filesystem_json, config_builder: nil) - super(config_builder) - @filesystem_json = filesystem_json - end - # @see Base#convert - # - # @param default [Configs::Filesystem, nil] # @return [Configs::Filesystem] - def convert(default = nil) - super(default || self.default) + def convert + super(Configs::Filesystem.new) end private - # @return [Hash] - attr_reader :filesystem_json + alias_method :filesystem_json, :config_json # @see Base#conversions - # - # @param default [Configs::Filesystem] # @return [Hash] - def conversions(default) + def conversions { reuse: filesystem_json[:reuseIfPossible], label: filesystem_json[:label], @@ -62,7 +50,7 @@ def conversions(default) mount_options: filesystem_json[:mountOptions], mkfs_options: filesystem_json[:mkfsOptions], mount_by: convert_mount_by, - type: convert_type(default.type) + type: convert_type } end @@ -74,22 +62,12 @@ def convert_mount_by Y2Storage::Filesystems::MountByType.find(value.to_sym) end - # @param default [Configs::FilesystemType, nil] # @return [Configs::FilesystemType, nil] - def convert_type(default = nil) + def convert_type filesystem_type_json = filesystem_json[:type] return unless filesystem_type_json - FromJSONConversions::FilesystemType.new(filesystem_type_json).convert(default) - end - - # Default filesystem config. - # - # @return [Configs::Filesystem] - def default - return Configs::Filesystem.new unless config_builder - - config_builder.default_filesystem(filesystem_json[:path]) + FromJSONConversions::FilesystemType.new(filesystem_type_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem_type.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem_type.rb index 094e9976c..14e032d28 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem_type.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem_type.rb @@ -31,33 +31,22 @@ module ConfigConversions module FromJSONConversions # Filesystem type conversion from JSON hash according to schema. class FilesystemType < Base - # @param filesystem_type_json [Hash, String] - def initialize(filesystem_type_json) - super() - @filesystem_type_json = filesystem_type_json - end - # @see Base#convert - # - # @param default [Configs::FilesystemType, nil] # @return [Configs::FilesystemType] - def convert(default = nil) - super(default || Configs::FilesystemType.new) + def convert + super(Configs::FilesystemType.new) end private - # @return [Hash, String] - attr_reader :filesystem_type_json + alias_method :filesystem_type_json, :config_json # @see Base#conversions - # - # @param default [Configs::FilesystemType] # @return [Hash] - def conversions(default) + def conversions { fs_type: convert_type, - btrfs: convert_btrfs(default.btrfs) + btrfs: convert_btrfs } end @@ -67,15 +56,14 @@ def convert_type Y2Storage::Filesystems::Type.find(value.to_sym) end - # @param default [Configs::Btrfs, nil] # @return [Configs::Btrfs, nil] - def convert_btrfs(default = nil) + def convert_btrfs return if filesystem_type_json.is_a?(String) btrfs_json = filesystem_type_json[:btrfs] return unless btrfs_json - FromJSONConversions::Btrfs.new(btrfs_json).convert(default) + FromJSONConversions::Btrfs.new(btrfs_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/logical_volume.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/logical_volume.rb index cd48f8de1..cbd98c3e3 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/logical_volume.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/logical_volume.rb @@ -36,36 +36,24 @@ class LogicalVolume < Base include WithFilesystem include WithSize - # @param logical_volume_json [Hash] - # @param config_builder [ConfigBuilder, nil] - def initialize(logical_volume_json, config_builder: nil) - super(config_builder) - @logical_volume_json = logical_volume_json - end - # @see Base#convert - # - # @param default [Configs::LogicalVolume, nil] # @return [Configs::LogicalVolume] - def convert(default = nil) - super(default || Configs::LogicalVolume.new) + def convert + super(Configs::LogicalVolume.new) end private - # @return [Hash] - attr_reader :logical_volume_json + alias_method :logical_volume_json, :config_json # @see Base#conversions - # - # @param default [Configs::LogicalVolume] # @return [Hash] - def conversions(default) + def conversions { alias: logical_volume_json[:alias], - encryption: convert_encryption(logical_volume_json, default: default.encryption), - filesystem: convert_filesystem(logical_volume_json, default: default.filesystem), - size: convert_size(logical_volume_json, default: default.size), + encryption: convert_encryption, + filesystem: convert_filesystem, + size: convert_size, name: logical_volume_json[:name], stripes: logical_volume_json[:stripes], stripe_size: convert_stripe_size, diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/partition.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/partition.rb index 81a487057..00823a0c4 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/partition.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/partition.rb @@ -38,36 +38,25 @@ class Partition < Base include WithFilesystem include WithSize - # @param partition_json [Hash] - def initialize(partition_json, config_builder: nil) - super(config_builder) - @partition_json = partition_json - end - # @see Base#convert - # - # @param default [Configs::Partition, nil] # @return [Configs::Partition] - def convert(default = nil) - super(default || Configs::Partition.new) + def convert + super(Configs::Partition.new) end private - # @return [Hash] - attr_reader :partition_json + alias_method :partition_json, :config_json # @see Base#conversions - # - # @param default [Configs::Partition] # @return [Hash] - def conversions(default) + def conversions { - search: convert_search(partition_json, default: default.search), + search: convert_search, alias: partition_json[:alias], - encryption: convert_encryption(partition_json, default: default.encryption), - filesystem: convert_filesystem(partition_json, default: default.filesystem), - size: convert_size(partition_json, default: default.size), + encryption: convert_encryption, + filesystem: convert_filesystem, + size: convert_size, id: convert_id, delete: partition_json[:delete], delete_if_needed: partition_json[:deleteIfNeeded] diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/search.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/search.rb index 327568a36..5df1bfb67 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/search.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/search.rb @@ -28,30 +28,19 @@ module ConfigConversions module FromJSONConversions # Search conversion from JSON hash according to schema. class Search < Base - # @param search_json [Hash, String] - def initialize(search_json) - super() - @search_json = search_json - end - # @see Base#convert - # - # @param default [Configs::Search, nil] # @return [Configs::Search] - def convert(default = nil) - super(default || Configs::Search.new) + def convert + super(Configs::Search.new) end private - # @return [Hash, String] - attr_reader :search_json + alias_method :search_json, :config_json # @see Base#conversions - # - # @param _default [Configs::Partition] # @return [Hash] - def conversions(_default) + def conversions { name: convert_name, if_not_found: convert_not_found diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/size.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/size.rb index cb02433aa..c1cf93881 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/size.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/size.rb @@ -29,30 +29,19 @@ module ConfigConversions module FromJSONConversions # Size conversion from JSON hash according to schema. class Size < Base - # @param size_json [Hash] - def initialize(size_json) - super() - @size_json = size_json - end - # @see Base#convert - # - # @param default [Configs::Size, nil] # @return [Configs::Size] - def convert(default = nil) - super(default || Configs::Size.new) + def convert + super(Configs::Size.new) end private - # @return [Hash] - attr_reader :size_json + alias_method :size_json, :config_json # @see Base#conversions - # - # @param _default [Configs::Size] # @return [Hash] - def conversions(_default) + def conversions { default: false, min: convert_min_size, diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb index b88e26212..153331ced 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb @@ -21,7 +21,7 @@ require "agama/storage/config_conversions/from_json_conversions/base" require "agama/storage/config_conversions/from_json_conversions/logical_volume" -require "agama/storage/configs/drive" +require "agama/storage/configs/volume_group" module Agama module Storage @@ -29,31 +29,19 @@ module ConfigConversions module FromJSONConversions # Volume group conversion from JSON hash according to schema. class VolumeGroup < Base - # @param volume_group_json [Hash] - # @param config_builder [ConfigBuilder, nil] - def initialize(volume_group_json, config_builder: nil) - super(config_builder) - @volume_group_json = volume_group_json - end - # @see Base#convert - # - # @param default [Configs::VolumeGroup, nil] # @return [Configs::VolumeGroup] - def convert(default = nil) - super(default || Configs::VolumeGroup.new) + def convert + super(Configs::VolumeGroup.new) end private - # @return [Hash] - attr_reader :volume_group_json + alias_method :volume_group_json, :config_json # @see Base#conversions - # - # @param _default [Configs::VolumeGroup] # @return [Hash] - def conversions(_default) + def conversions { name: volume_group_json[:name], extent_size: convert_extent_size, @@ -81,9 +69,7 @@ def convert_logical_volumes # @param logical_volume_json [Hash] # @return [Configs::LogicalVolume] def convert_logical_volume(logical_volume_json) - FromJSONConversions::LogicalVolume - .new(logical_volume_json, config_builder: config_builder) - .convert + FromJSONConversions::LogicalVolume.new(logical_volume_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_encryption.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_encryption.rb index 2dce8c39d..3bfdd2dd6 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_encryption.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_encryption.rb @@ -27,17 +27,12 @@ module ConfigConversions module FromJSONConversions # Mixin for encryption conversion. module WithEncryption - # @param json [Hash] - # @param default [Configs::Encryption, nil] - # # @return [Configs::Encryption, nil] - def convert_encryption(json, default: nil) - encryption_json = json[:encryption] + def convert_encryption + encryption_json = config_json[:encryption] return unless encryption_json - FromJSONConversions::Encryption - .new(encryption_json, config_builder: config_builder) - .convert(default) + FromJSONConversions::Encryption.new(encryption_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_filesystem.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_filesystem.rb index 9f5e84678..9c8b60be9 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_filesystem.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_filesystem.rb @@ -27,20 +27,12 @@ module ConfigConversions module FromJSONConversions # Mixin for filesystem conversion. module WithFilesystem - # @param json [Hash] - # @param default [Configs::Filesystem, nil] - # # @return [Configs::Filesystem, nil] - def convert_filesystem(json, default: nil) - filesystem_json = json[:filesystem] + def convert_filesystem + filesystem_json = config_json[:filesystem] return unless filesystem_json - # @todo Check whether the given filesystem can be used for the mount point. - # @todo Check whether snapshots can be configured and restore to default if needed. - - FromJSONConversions::Filesystem - .new(filesystem_json, config_builder: config_builder) - .convert(default) + FromJSONConversions::Filesystem.new(filesystem_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_partitions.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_partitions.rb index 432fb9ab6..9c5dad1e5 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_partitions.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_partitions.rb @@ -27,10 +27,9 @@ module ConfigConversions module FromJSONConversions # Mixin for partitions conversion. module WithPartitions - # @param json [Hash] # @return [Array, nil] - def convert_partitions(json) - partitions_json = json[:partitions] + def convert_partitions + partitions_json = config_json[:partitions] return unless partitions_json partitions_json.map { |p| convert_partition(p) } @@ -39,9 +38,7 @@ def convert_partitions(json) # @param partition_json [Hash] # @return [Configs::Partition] def convert_partition(partition_json) - FromJSONConversions::Partition - .new(partition_json, config_builder: config_builder) - .convert + FromJSONConversions::Partition.new(partition_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_ptable_type.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_ptable_type.rb index d6c8d6b6f..9c50f0cd0 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_ptable_type.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_ptable_type.rb @@ -27,11 +27,9 @@ module ConfigConversions module FromJSONConversions # Mixin for partition table type conversion. module WithPtableType - # @param json [Hash] - # # @return [Y2Storage::PartitionTables::Type, nil] - def convert_ptable_type(json) - value = json[:ptableType] + def convert_ptable_type + value = config_json[:ptableType] return unless value Y2Storage::PartitionTables::Type.find(value) diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_search.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_search.rb index 7a25dbc25..cf72d7642 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_search.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_search.rb @@ -27,16 +27,12 @@ module ConfigConversions module FromJSONConversions # Mixin for search conversion. module WithSearch - # @param json [Hash] - # @param default [Configs::Search, nil] - # # @return [Configs::Search, nil] - def convert_search(json, default: nil) - search_json = json[:search] + def convert_search + search_json = config_json[:search] return unless search_json - converter = FromJSONConversions::Search.new(search_json) - converter.convert(default) + FromJSONConversions::Search.new(search_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_size.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_size.rb index 33e9baa81..bde583b87 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_size.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_size.rb @@ -27,15 +27,12 @@ module ConfigConversions module FromJSONConversions # Mixin for size conversion. module WithSize - # @param json [Hash] - # @param default [Configs::Size, nil] - # # @return [Configs::Size, nil] - def convert_size(json, default: nil) - size_json = json[:size] + def convert_size + size_json = config_json[:size] return unless size_json - FromJSONConversions::Size.new(size_json).convert(default) + FromJSONConversions::Size.new(size_json).convert end end end diff --git a/service/lib/agama/storage/config_json_solver.rb b/service/lib/agama/storage/config_json_solver.rb index cd210b662..3f05dda4a 100644 --- a/service/lib/agama/storage/config_json_solver.rb +++ b/service/lib/agama/storage/config_json_solver.rb @@ -27,9 +27,9 @@ module Storage # Class for solving a storage JSON config. # # A storage JSON config can contain a "generate" section for automatically generating partitions - # or logical volumes according to the product definition. That section is solved by replacing it - # with the corresponding configs. The solver takes into account other paths already present in - # the rest of the config. + # or logical volumes according to the indicated default and mandatory paths. The "generate" + # section is solved by replacing it with the corresponding configs. The solver takes into + # account other paths already present in the rest of the config. # # @example # config_json = { @@ -58,9 +58,11 @@ module Storage # ] # } class ConfigJSONSolver - # @param product_config [Agama::Config] - def initialize(product_config = nil) - @product_config = product_config || Agama::Config.new + # @param default_paths [Array] Default paths of the product. + # @param mandatory_paths [Array] Mandatory paths of the product. + def initialize(default_paths: [], mandatory_paths: []) + @default_paths = default_paths + @mandatory_paths = mandatory_paths end # Solves the generate section within a given JSON config. @@ -76,12 +78,15 @@ def solve(config_json) private + # @return [Array] + attr_reader :default_paths + + # @return [Array] + attr_reader :mandatory_paths + # @return [Hash] attr_reader :config_json - # @return [Agama::Config] - attr_reader :product_config - def solve_generate configs = configs_with_generate return unless configs.any? @@ -127,11 +132,6 @@ def missing_default_paths default_paths - current_paths end - # @return [Array] - def default_paths - product_config.data.dig("storage", "volumes") || [] - end - # @return [Array] def current_paths configs_with_filesystem @@ -151,17 +151,6 @@ def missing_mandatory_paths mandatory_paths - current_paths end - # @return [Array] - def mandatory_paths - default_paths.select { |p| mandatory_path?(p) } - end - - # @param path [String] - # @return [Volume] - def mandatory_path?(path) - volume_builder.for(path).outline.required? - end - # @param config [Hash] e.g., { generate: "default" } # @param path [String] # @@ -260,11 +249,6 @@ def with_generate_value?(config, value) generate[:partitions] == value || generate[:logicalVolumes] == value end - - # @return [VolumeTemplatesBuilder] - def volume_builder - @volume_builder ||= VolumeTemplatesBuilder.new_from_config(product_config) - end end end end diff --git a/service/test/agama/storage/config_conversions/from_json_test.rb b/service/test/agama/storage/config_conversions/from_json_test.rb index a7f5b8631..e45beb63c 100644 --- a/service/test/agama/storage/config_conversions/from_json_test.rb +++ b/service/test/agama/storage/config_conversions/from_json_test.rb @@ -30,267 +30,626 @@ using Y2Storage::Refinements::SizeCasts -describe Agama::Storage::ConfigConversions::FromJSON do - subject { described_class.new(config_json, product_config: product_config) } +shared_examples "without search" do |config_proc| + it "does not set #search" do + config = config_proc.call(subject.convert) + expect(config.search).to be_nil + end +end + +shared_examples "without alias" do |config_proc| + it "does not set #alias" do + config = config_proc.call(subject.convert) + expect(config.alias).to be_nil + end +end + +shared_examples "without encryption" do |config_proc| + it "does not set #encryption" do + config = config_proc.call(subject.convert) + expect(config.encryption).to be_nil + end +end + +shared_examples "without filesystem" do |config_proc| + it "does not set #filesystem" do + config = config_proc.call(subject.convert) + expect(config.filesystem).to be_nil + end +end + +shared_examples "without ptableType" do |config_proc| + it "does not set #ptable_type" do + config = config_proc.call(subject.convert) + expect(config.ptable_type).to be_nil + end +end + +shared_examples "without partitions" do |config_proc| + it "sets #partitions to the expected value" do + config = config_proc.call(subject.convert) + expect(config.partitions).to eq([]) + end +end + +shared_examples "without size" do |config_proc| + it "sets #size to default size" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(true) + expect(config.size.min).to be_nil + expect(config.size.max).to be_nil + end +end + +shared_examples "without delete" do |config_proc| + it "sets #delete to false" do + config = config_proc.call(subject.convert) + expect(config.delete?).to eq(false) + end +end + +shared_examples "without deleteIfNeeded" do |config_proc| + it "sets #delete_if_needed to false" do + config = config_proc.call(subject.convert) + expect(config.delete_if_needed?).to eq(false) + end +end + +shared_examples "with search" do |config_proc| + context "with a device name" do + let(:search) { "/dev/vda1" } - let(:product_config) { Agama::Config.new(product_data) } + it "sets #search to the expected value" do + config = config_proc.call(subject.convert) + expect(config.search).to be_a(Agama::Storage::Configs::Search) + expect(config.search.name).to eq("/dev/vda1") + expect(config.search.if_not_found).to eq(:error) + end + end + + context "with a search section" do + let(:search) do + { + condition: { name: "/dev/vda1" }, + ifNotFound: "skip" + } + end + + it "sets #search to the expected value" do + config = config_proc.call(subject.convert) + expect(config.search).to be_a(Agama::Storage::Configs::Search) + expect(config.search.name).to eq("/dev/vda1") + expect(config.search.if_not_found).to eq(:skip) + end + end +end - let(:product_data) do +shared_examples "with alias" do |config_proc| + let(:device_alias) { "test" } + + it "sets #alias to the expected value" do + config = config_proc.call(subject.convert) + expect(config.alias).to eq("test") + end +end + +shared_examples "with encryption" do |config_proc| + let(:encryption) do { - "storage" => { - "lvm" => false, - "space_policy" => "delete", - "encryption" => { - "method" => "luks2", - "pbkd_function" => "argon2id" - }, - "volumes" => ["/", "swap"], - "volume_templates" => [ - { - "mount_path" => "/", "filesystem" => "btrfs", "size" => { "auto" => true }, - "btrfs" => { - "snapshots" => true, "default_subvolume" => "@", - "subvolumes" => ["home", "opt", "root", "srv"] - }, - "outline" => { - "required" => true, "snapshots_configurable" => true, - "auto_size" => { - "base_min" => "5 GiB", "base_max" => "10 GiB", - "min_fallback_for" => ["/home"], "max_fallback_for" => ["/home"], - "snapshots_increment" => "300%" - } - } - }, - { - "mount_path" => "/home", "size" => { "auto" => false, "min" => "5 GiB" }, - "filesystem" => "xfs", "outline" => { "required" => false } - }, - { - "mount_path" => "swap", "filesystem" => "swap", - "outline" => { "required" => false } - }, - { "mount_path" => "", "filesystem" => "ext4", - "size" => { "min" => "100 MiB" } } - ] + luks2: { + password: "12345", + keySize: 256, + pbkdFunction: "argon2i", + cipher: "twofish", + label: "test" } } end - shared_examples "omitting sizes" do |result| - let(:example_configs) do - [ - { filesystem: { path: "/", type: { btrfs: { snapshots: false } } } }, - { filesystem: { path: "/home" } }, - { filesystem: { path: "/opt" } }, - { filesystem: { path: "swap" } } - ] + it "sets #encryption to the expected value" do + config = config_proc.call(subject.convert) + encryption = config.encryption + expect(encryption).to be_a(Agama::Storage::Configs::Encryption) + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(encryption.password).to eq("12345") + expect(encryption.key_size).to eq(256) + expect(encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::ARGON2I) + expect(encryption.cipher).to eq("twofish") + expect(encryption.label).to eq("test") + end + + context "if 'encryption' only specifies 'password'" do + let(:encryption) do + { + luks2: { + password: "12345" + } + } end - it "uses default sizes" do - config = subject.convert - devices = result.call(config) - expect(devices).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - size: have_attributes(default: true, min: be_nil, max: be_nil) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - size: have_attributes(default: true, min: be_nil, max: be_nil) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/opt"), - size: have_attributes(default: true, min: be_nil, max: be_nil) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - size: have_attributes(default: true, min: be_nil, max: be_nil) - ) - ) + it "sets #encryption to the expected value" do + config = config_proc.call(subject.convert) + encryption = config.encryption + expect(encryption).to be_a(Agama::Storage::Configs::Encryption) + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(encryption.password).to eq("12345") + expect(encryption.key_size).to be_nil + expect(encryption.pbkd_function).to be_nil + expect(encryption.cipher).to be_nil + expect(encryption.label).to be_nil end end +end - shared_examples "fixed sizes" do |result| - let(:example_configs) do - [ - { filesystem: { path: "/" }, size: "10 GiB" }, - { filesystem: { path: "/home" }, size: "6Gb" }, - { filesystem: { path: "/opt" }, size: 3221225472 }, - { filesystem: { path: "swap" }, size: "6 Gib" } - ] +shared_examples "with filesystem" do |config_proc| + let(:filesystem) do + { + reuseIfPossible: true, + type: "xfs", + label: "test", + path: "/test", + mountBy: "device", + mkfsOptions: ["version=2"], + mountOptions: ["rw"] + } + end + + it "sets #filesystem to the expected value" do + config = config_proc.call(subject.convert) + filesystem = config.filesystem + expect(filesystem).to be_a(Agama::Storage::Configs::Filesystem) + expect(filesystem.reuse?).to eq(true) + expect(filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::XFS) + expect(filesystem.type.btrfs).to be_nil + expect(filesystem.label).to eq("test") + expect(filesystem.path).to eq("/test") + expect(filesystem.mount_by).to eq(Y2Storage::Filesystems::MountByType::DEVICE) + expect(filesystem.mkfs_options).to contain_exactly("version=2") + expect(filesystem.mount_options).to contain_exactly("rw") + end + + context "if 'filesystem' specifies a 'type' with a btrfs section" do + let(:filesystem) do + { + type: { + btrfs: { + snapshots: true + } + } + } end - it "sets both min and max to the same value if a string is used" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - size: have_attributes(default: false, min: 10.GiB, max: 10.GiB) - ) - ) + it "sets #filesystem to the expected value" do + config = config_proc.call(subject.convert) + filesystem = config.filesystem + expect(filesystem).to be_a(Agama::Storage::Configs::Filesystem) + expect(filesystem.reuse?).to eq(false) + expect(filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) + expect(filesystem.type.btrfs.snapshots?).to eq(true) + expect(filesystem.label).to be_nil + expect(filesystem.path).to be_nil + expect(filesystem.mount_by).to be_nil + expect(filesystem.mkfs_options).to eq([]) + expect(filesystem.mount_options).to eq([]) end + end - it "sets both min and max to the same value if an integer is used" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/opt"), - size: have_attributes(default: false, min: 3.GiB, max: 3.GiB) - ) - ) + context "if 'filesystem' is an empty section" do + let(:filesystem) { {} } + + it "sets #filesystem to the expected value" do + config = config_proc.call(subject.convert) + filesystem = config.filesystem + expect(filesystem).to be_a(Agama::Storage::Configs::Filesystem) + expect(filesystem.reuse?).to eq(false) + expect(filesystem.type).to be_nil + expect(filesystem.label).to be_nil + expect(filesystem.path).to be_nil + expect(filesystem.mount_by).to be_nil + expect(filesystem.mkfs_options).to eq([]) + expect(filesystem.mount_options).to eq([]) end + end +end - it "makes a difference between SI units and binary units" do - config = subject.convert - devices = result.call(config) - home_size = devices.find { |d| d.filesystem.path == "/home" }.size - swap_size = devices.find { |d| d.filesystem.path == "swap" }.size - expect(swap_size.min.to_i).to eq 6 * 1024 * 1024 * 1024 - expect(home_size.max.to_i).to eq 6 * 1000 * 1000 * 1000 +shared_examples "with ptableType" do |config_proc| + let(:ptableType) { "gpt" } + + it "sets #ptable_type to the expected value" do + config = config_proc.call(subject.convert) + expect(config.ptable_type).to eq(Y2Storage::PartitionTables::Type::GPT) + end +end + +shared_examples "with size" do |config_proc| + context "if 'size' is a string" do + let(:size) { "10 GiB" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(10.GiB) + expect(config.size.max).to eq(10.GiB) end end - shared_examples "size limits" do |result| - shared_examples "limit tests" do - it "sets both min and max limits as requested if strings are used" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - size: have_attributes(default: false, min: 6.GiB, max: 9.GiB) - ) - ) + context "if 'size' is a number" do + let(:size) { 3221225472 } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(3.GiB) + expect(config.size.max).to eq(3.GiB) + end + end + + shared_examples "min size" do + context "and the value is a string" do + let(:min_size) { "10 GiB" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(10.GiB) + expect(config.size.max).to eq(Y2Storage::DiskSize.unlimited) end + end - it "makes a difference between SI units and binary units" do - config = subject.convert - devices = result.call(config) - home_size = devices.find { |d| d.filesystem.path == "/home" }.size - swap_size = devices.find { |d| d.filesystem.path == "swap" }.size - expect(home_size.min.to_i).to eq 6 * 1024 * 1024 * 1024 - expect(swap_size.max.to_i).to eq 6 * 1000 * 1000 * 1000 + context "and the value is a number" do + let(:min_size) { 3221225472 } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(3.GiB) + expect(config.size.max).to eq(Y2Storage::DiskSize.unlimited) end + end - it "sets both min and max limits as requested if numbers are used" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - size: have_attributes(default: false, min: 1.GiB) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/opt"), - size: have_attributes(default: false, min: 1.GiB, max: 3.GiB) - ) - ) + context "and the value is 'current'" do + let(:min_size) { "current" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to be_nil + expect(config.size.max).to eq(Y2Storage::DiskSize.unlimited) end + end + end - it "uses unlimited for the omitted max sizes" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - size: have_attributes(default: false, min: 3.GiB, - max: Y2Storage::DiskSize.unlimited) - ) - ) + shared_examples "min and max sizes" do + context "and the values are strings" do + let(:min_size) { "10 GiB" } + let(:max_size) { "20 GiB" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(10.GiB) + expect(config.size.max).to eq(20.GiB) end + end - it "uses nil for min size as current" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/data1"), - size: have_attributes(default: false, min: be_nil, - max: Y2Storage::DiskSize.unlimited) - ) - ) + context "and the values are numbers" do + let(:min_size) { 3221225472 } + let(:max_size) { 10737418240 } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(3.GiB) + expect(config.size.max).to eq(10.GiB) end + end - it "uses nil for max size as current" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/data2"), - size: have_attributes(default: false, min: 10.GiB, max: be_nil) - ) - ) + context "and the values mixes string and number" do + let(:min_size) { 3221225472 } + let(:max_size) { "10 Gib" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(3.GiB) + expect(config.size.max).to eq(10.GiB) end end - context "using a hash" do - let(:example_configs) do - [ - { - filesystem: { path: "/", type: { btrfs: { snapshots: false } } }, - size: { min: "3 GiB" } - }, - { - filesystem: { path: "/home" }, - size: { min: "6 GiB", max: "9 GiB" } - }, - { - filesystem: { path: "swap" }, - size: { min: 1073741824, max: "6 GB" } - }, - { - filesystem: { path: "/opt" }, - size: { min: "1073741824", max: 3221225472 } - }, - { - filesystem: { path: "/data1" }, - size: { min: "current" } - }, - { - filesystem: { path: "/data2" }, - size: { min: "10 GiB", max: "current" } - } - ] + context "and the min value is 'current'" do + let(:min_size) { "current" } + let(:max_size) { "10 GiB" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to be_nil + expect(config.size.max).to eq(10.GiB) end + end - include_examples "limit tests" + context "and the max value is 'current'" do + let(:min_size) { "10 GiB" } + let(:max_size) { "current" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(10.GiB) + expect(config.size.max).to be_nil + end end - context "using an array" do - let(:example_configs) do - [ - { - filesystem: { path: "/", type: { btrfs: { snapshots: false } } }, - size: ["3 GiB"] - }, - { - filesystem: { path: "/home" }, - size: ["6 GiB", "9 GiB"] - }, - { - filesystem: { path: "swap" }, - size: [1073741824, "6 GB"] - }, - { - filesystem: { path: "/opt" }, - size: ["1073741824", 3221225472] - }, - { - filesystem: { path: "/data1" }, - size: ["current"] - }, - { - filesystem: { path: "/data2" }, - size: ["10 GiB", "current"] + context "and both values are 'current'" do + let(:min_size) { "current" } + let(:max_size) { "current" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to be_nil + expect(config.size.max).to be_nil + end + end + end + + context "if 'size' is an array" do + context "and only contains one value" do + let(:size) { [min_size] } + include_examples "min size" + end + + context "and contains two values" do + let(:size) { [min_size, max_size] } + include_examples "min and max sizes" + end + end + + context "if 'size' is a hash" do + context "and only specifies 'min'" do + let(:size) { { min: min_size } } + include_examples "min size" + end + + context "and specifies 'min' and 'max'" do + let(:size) do + { + min: min_size, + max: max_size + } + end + + include_examples "min and max sizes" + end + end +end + +shared_examples "with delete" do |config_proc| + it "sets #delete to true" do + config = config_proc.call(subject.convert) + expect(config.delete?).to eq(true) + end +end + +shared_examples "with deleteIfNeeded" do |config_proc| + it "sets #delete_if_needed to true" do + config = config_proc.call(subject.convert) + expect(config.delete_if_needed?).to eq(true) + end +end + +shared_examples "with partitions" do |config_proc| + let(:partitions) do + [ + partition, + { + filesystem: { path: "/test" } + } + ] + end + + let(:partition) do + { + filesystem: { path: "/" } + } + end + + context "with an empty list" do + let(:partitions) { [] } + + it "sets #partitions to empty" do + config = config_proc.call(subject.convert) + expect(config.partitions).to eq([]) + end + end + + context "with a list of partitions" do + it "sets #partitions to the expected value" do + config = config_proc.call(subject.convert) + partitions = config.partitions + expect(partitions.size).to eq(2) + + partition1, partition2 = partitions + expect(partition1).to be_a(Agama::Storage::Configs::Partition) + expect(partition1.filesystem.path).to eq("/") + expect(partition2).to be_a(Agama::Storage::Configs::Partition) + expect(partition2.filesystem.path).to eq("/test") + end + end + + partition_proc = proc { |c| config_proc.call(c).partitions.first } + + context "if a partition does not spicify 'search'" do + let(:partition) { {} } + include_examples "without search", partition_proc + end + + context "if a partition does not spicify 'alias'" do + let(:partition) { {} } + include_examples "without alias", partition_proc + end + + context "if a partition does not spicify 'id'" do + let(:partition) { {} } + + it "does not set #id" do + partition = partition_proc.call(subject.convert) + expect(partition.id).to be_nil + end + end + + context "if a partition does not spicify 'size'" do + let(:partition) { {} } + include_examples "without size", partition_proc + end + + context "if a partition does not spicify 'encryption'" do + let(:partition) { {} } + include_examples "without encryption", partition_proc + end + + context "if a partition does not spicify 'filesystem'" do + let(:partition) { {} } + include_examples "without filesystem", partition_proc + end + + context "if a partition does not spicify 'delete'" do + let(:partition) { {} } + include_examples "without delete", partition_proc + end + + context "if a partition does not spicify 'deleteIfNeeded'" do + let(:partition) { {} } + include_examples "without deleteIfNeeded", partition_proc + end + + context "if a partition specifies 'search'" do + let(:partition) { { search: search } } + include_examples "with search", partition_proc + end + + context "if a partition specifies 'alias'" do + let(:partition) { { alias: device_alias } } + include_examples "with alias", partition_proc + end + + context "if a partition spicifies 'id'" do + let(:partition) { { id: "esp" } } + + it "sets #id to the expected value" do + partition = partition_proc.call(subject.convert) + expect(partition.id).to eq(Y2Storage::PartitionId::ESP) + end + end + + context "if a partition spicifies 'size'" do + let(:partition) { { size: size } } + include_examples "with size", partition_proc + end + + context "if a partition specifies 'encryption'" do + let(:partition) { { encryption: encryption } } + include_examples "with encryption", partition_proc + end + + context "if a partition specifies 'filesystem'" do + let(:partition) { { filesystem: filesystem } } + include_examples "with filesystem", partition_proc + end + + context "if a partition specifies 'delete'" do + let(:partition) { { delete: true } } + include_examples "with delete", partition_proc + end + + context "if a partition specifies 'deleteIfNeeded'" do + let(:partition) { { deleteIfNeeded: true } } + include_examples "with deleteIfNeeded", partition_proc + end + + context "if a partition specifies 'generate'" do + let(:partition) { { generate: generate } } + + partitions_proc = proc { |c| config_proc.call(c).partitions } + include_examples "with generate", partitions_proc + + context "with a generate section" do + let(:generate) do + { + partitions: "default", + encryption: { + luks2: { password: "12345" } } - ] + } end - include_examples "limit tests" + let(:default_paths) { ["/", "swap"] } + + it "adds the expected partitions" do + partitions = config_proc.call(subject.convert).partitions + expect(partitions.size).to eq(3) + + root_part = partitions.find { |p| p.filesystem.path == "/" } + swap_part = partitions.find { |p| p.filesystem.path == "swap" } + test_part = partitions.find { |p| p.filesystem.path == "/test" } + + expect(root_part).to_not be_nil + expect(root_part.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(root_part.encryption.password).to eq("12345") + + expect(swap_part).to_not be_nil + expect(swap_part.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(swap_part.encryption.password).to eq("12345") + + expect(test_part).to_not be_nil + expect(test_part.encryption).to be_nil + end end end +end + +shared_examples "with generate" do |configs_proc| + context "with 'default' value" do + let(:generate) { "default" } + + let(:default_paths) { ["/default1", "/default2"] } + + it "adds volumes for the default paths" do + configs = configs_proc.call(subject.convert) + + default1 = configs.find { |c| c.filesystem.path == "/default1" } + expect(default1).to_not be_nil + expect(default1.encryption).to be_nil + + default2 = configs.find { |c| c.filesystem.path == "/default2" } + expect(default2).to_not be_nil + expect(default2.encryption).to be_nil + end + end + + context "with 'mandatory' value" do + let(:generate) { "mandatory" } + + let(:mandatory_paths) { ["/mandatory1"] } + + it "adds volumes for the mandatory paths" do + configs = configs_proc.call(subject.convert) + + mandatory1 = configs.find { |c| c.filesystem.path == "/mandatory1" } + expect(mandatory1).to_not be_nil + expect(mandatory1.encryption).to be_nil + end + end +end + +describe Agama::Storage::ConfigConversions::FromJSON do + subject do + described_class.new(config_json, default_paths: default_paths, mandatory_paths: mandatory_paths) + end + + let(:default_paths) { [] } + + let(:mandatory_paths) { [] } before do # Speed up tests by avoding real check of TPM presence. @@ -298,1202 +657,828 @@ end describe "#convert" do - using Y2Storage::Refinements::SizeCasts + let(:config_json) { {} } - context "with an empty JSON configuration" do + it "returns a storage config" do + config = subject.convert + expect(config).to be_a(Agama::Storage::Config) + end + + context "with an empty JSON" do let(:config_json) { {} } - it "generates a storage configuration" do + it "sets #boot to the expected value" do config = subject.convert - expect(config).to be_a(Agama::Storage::Config) + expect(config.boot).to be_a(Agama::Storage::Configs::Boot) + expect(config.boot.configure).to eq(true) + expect(config.boot.device).to be_nil end - it "calculates default boot settings" do + it "sets #drives to the expected value" do config = subject.convert - expect(config.boot).to be_a(Agama::Storage::Configs::Boot) - expect(config.boot.configure).to eq true - expect(config.boot.device).to eq nil + expect(config.drives).to be_empty end - # @todo Generate default drive/LVM from product descripton. - it "does not include any device in the configuration" do + it "sets #volume_groups to the expected value" do config = subject.convert expect(config.drives).to be_empty + expect(config.volume_groups).to be_empty end end - context "with some drives and boot configuration at JSON" do + context "with a JSON specifying 'boot'" do let(:config_json) do { - boot: { configure: true, device: "/dev/sdb" }, - drives: [ - { - alias: "first-disk", - ptableType: "gpt", - partitions: [ - { - alias: "root", - filesystem: { path: "/" } - } - ] - } - ] + boot: { + configure: true, + device: "/dev/sdb" + } } end - it "generates a storage configuration" do - config = subject.convert - expect(config).to be_a(Agama::Storage::Config) - end - - it "calculates the corresponding boot settings" do + it "sets #boot to the expected value" do config = subject.convert expect(config.boot).to be_a(Agama::Storage::Configs::Boot) expect(config.boot.configure).to eq true expect(config.boot.device).to eq "/dev/sdb" end + end - it "includes the corresponding drives" do - config = subject.convert - expect(config.drives.size).to eq 1 - drive = config.drives.first - expect(drive).to be_a(Agama::Storage::Configs::Drive) - expect(drive.alias).to eq "first-disk" - expect(drive.ptable_type).to eq Y2Storage::PartitionTables::Type::GPT - expect(drive.partitions.size).to eq 1 - partition = drive.partitions.first - expect(partition.alias).to eq "root" - expect(partition.filesystem.path).to eq "/" + context "with a JSON specifying 'drives'" do + let(:config_json) do + { drives: drives } end - context "omitting search for a drive" do - let(:config_json) do - { - drives: [ - { - partitions: [] - } - ] - } + let(:drives) do + [ + drive, + { alias: "second-disk" } + ] + end + + let(:drive) do + { alias: "first-disk" } + end + + context "with an empty list" do + let(:drives) { [] } + + it "sets #drives to the expected value" do + config = subject.convert + expect(config.drives).to eq([]) end + end - it "sets the default search" do + context "with a list of drives" do + it "sets #drives to the expected value" do config = subject.convert - drive = config.drives.first + expect(config.drives.size).to eq(2) + expect(config.drives).to all(be_a(Agama::Storage::Configs::Drive)) + + drive1, drive2 = config.drives + expect(drive1.alias).to eq("first-disk") + expect(drive1.partitions).to eq([]) + expect(drive2.alias).to eq("second-disk") + expect(drive2.partitions).to eq([]) + end + end + + drive_proc = proc { |c| c.drives.first } + + context "if a drive does not specify 'search'" do + let(:drive) { {} } + + it "sets #search to the expected value" do + drive = drive_proc.call(subject.convert) expect(drive.search).to be_a(Agama::Storage::Configs::Search) expect(drive.search.name).to be_nil expect(drive.search.if_not_found).to eq(:error) end end - context "specifying search for a drive" do - let(:config_json) do - { - drives: [ - { - search: search, - partitions: [] - } - ] - } - end + context "if a drive does not spicify 'alias'" do + let(:drive) { {} } + include_examples "without alias", drive_proc + end - context "with a device name" do - let(:search) { "/dev/vda" } + context "if a drive does not spicify 'encryption'" do + let(:drive) { {} } + include_examples "without encryption", drive_proc + end - it "sets the expected search" do - config = subject.convert - drive = config.drives.first - expect(drive.search).to be_a(Agama::Storage::Configs::Search) - expect(drive.search.name).to eq("/dev/vda") - expect(drive.search.if_not_found).to eq(:error) - end - end + context "if a drive does not spicify 'filesystem'" do + let(:drive) { {} } + include_examples "without filesystem", drive_proc + end - context "with a search section" do - let(:search) do - { - condition: { name: "/dev/vda" }, - ifNotFound: "skip" - } - end + context "if a drive does not spicify 'ptableType'" do + let(:drive) { {} } + include_examples "without ptableType", drive_proc + end - it "sets the expected search" do - config = subject.convert - drive = config.drives.first - expect(drive.search).to be_a(Agama::Storage::Configs::Search) - expect(drive.search.name).to eq("/dev/vda") - expect(drive.search.if_not_found).to eq(:skip) - end - end + context "if a drive does not spicify 'partitions'" do + let(:drive) { {} } + include_examples "without partitions", drive_proc + end + + context "if a drive specifies 'search'" do + let(:drive) { { search: search } } + include_examples "with search", drive_proc + end + + context "if a drive specifies 'alias'" do + let(:drive) { { alias: device_alias } } + include_examples "with alias", drive_proc + end + + context "if a drive specifies 'encryption'" do + let(:drive) { { encryption: encryption } } + include_examples "with encryption", drive_proc + end + + context "if a drive specifies 'filesystem'" do + let(:drive) { { filesystem: filesystem } } + include_examples "with filesystem", drive_proc + end + + context "if a drive specifies 'ptableType'" do + let(:drive) { { ptableType: ptableType } } + include_examples "with ptableType", drive_proc + end + + context "if a drive specifies 'partitions'" do + let(:drive) { { partitions: partitions } } + include_examples "with partitions", drive_proc end end - context "specifying a filesystem for a drive" do + context "with a JSON specifying 'volumeGroups'" do let(:config_json) do - { - drives: [{ filesystem: filesystem }] - } + { volumeGroups: volume_groups } end - let(:filesystem) do - { - reuseIfPossible: true, - path: "/", - type: "xfs", - label: "root", - mkfsOptions: ["version=2"], - mountOptions: ["rw"], - mountBy: "label" - } + let(:volume_groups) do + [ + volume_group, + { name: "vg2" } + ] end - it "uses the specified attributes" do - config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.reuse?).to eq true - expect(filesystem.path).to eq "/" - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::XFS - expect(filesystem.label).to eq "root" - expect(filesystem.mkfs_options).to eq ["version=2"] - expect(filesystem.mount_options).to eq ["rw"] - expect(filesystem.mount_by).to eq Y2Storage::Filesystems::MountByType::LABEL - end + let(:volume_group) { { name: "vg1" } } - context "if the filesystem specification only contains a path" do - let(:filesystem) { { path: "/" } } + context "with an empty list" do + let(:volume_groups) { [] } - it "uses the default type and btrfs attributes for that path" do + it "sets #volume_groups to the expected value" do config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.reuse?).to eq false - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS - expect(filesystem.type.btrfs.snapshots).to eq true - expect(filesystem.type.btrfs.default_subvolume).to eq "@" - expect(filesystem.type.btrfs.subvolumes.map(&:path)).to eq ["home", "opt", "root", "srv"] + expect(config.volume_groups).to eq([]) end end - context "if the filesystem specification contains some btrfs settings" do - let(:filesystem) do - { path: "/", - type: { btrfs: { snapshots: false, default_subvolume: "", subvolumes: ["tmp"] } } } - end - - it "uses the specified btrfs attributes" do + context "with a list of volume groups" do + it "sets #volume_groups to the expected value" do config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS - expect(filesystem.type.btrfs.snapshots).to eq false - # TODO: none of the following attributes are specified at the schema. Intentional? - # expect(filesystem.type.btrfs.default_subvolume).to eq "" - # expect(filesystem.type.btrfs.subvolumes.map(&:path)).to eq ["tmp"] + expect(config.volume_groups.size).to eq(2) + expect(config.volume_groups).to all(be_a(Agama::Storage::Configs::VolumeGroup)) + + volume_group1, volume_group2 = config.volume_groups + expect(volume_group1.name).to eq("vg1") + expect(volume_group1.logical_volumes).to eq([]) + expect(volume_group2.name).to eq("vg2") + expect(volume_group2.logical_volumes).to eq([]) end + end - context "and the default filesystem type is not btrfs" do - let(:filesystem) do - { path: "/home", type: { btrfs: { snapshots: false } } } - end + vg_proc = proc { |c| c.volume_groups.first } - it "uses btrfs filesystem" do - config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS - end + context "if a volume group does not spicify 'name'" do + let(:volume_group) { {} } + + it "does not set #name" do + vg = vg_proc.call(subject.convert) + expect(vg.name).to be_nil end end - end - context "omitting search for a partition" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - filesystem: { - path: "/" - } - } - ] - } - ] - } + context "if a volume group does not spicify 'extentSize'" do + let(:volume_group) { {} } + + it "does not set #extent_size" do + vg = vg_proc.call(subject.convert) + expect(vg.extent_size).to be_nil + end end - it "does not set a search" do - config = subject.convert - drive = config.drives.first - partition = drive.partitions.first - expect(partition.search).to be_nil + context "if a volume group does not spicify 'physicalVolumes'" do + let(:volume_group) { {} } + + it "sets #physical_volumes to the expected vale" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes).to eq([]) + end end - end - context "specifying search for a partition" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - search: search, - filesystem: { - path: "/" - } - } - ] - } - ] - } + context "if a volume group does not spicify 'logicalVolumes'" do + let(:volume_group) { {} } + + it "sets #logical_volumes to the expected vale" do + vg = vg_proc.call(subject.convert) + expect(vg.logical_volumes).to eq([]) + end end - context "with a device name" do - let(:search) { "/dev/vda1" } + context "if a volume group spicifies 'name'" do + let(:volume_group) { { name: "test" } } - it "sets the expected search" do - config = subject.convert - drive = config.drives.first - partition = drive.partitions.first - expect(partition.search).to be_a(Agama::Storage::Configs::Search) - expect(partition.search.name).to eq("/dev/vda1") - expect(partition.search.if_not_found).to eq(:error) + it "sets #name to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.name).to eq("test") end end - context "with a search section" do - let(:search) do - { - condition: { name: "/dev/vda1" }, - ifNotFound: "skip" - } + context "if a volume group spicifies 'extentSize'" do + let(:volume_group) { { extentSize: size } } + + context "if 'extentSize' is a string" do + let(:size) { "4 KiB" } + + it "sets #extent_size to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.extent_size).to eq(4.KiB) + end end - it "sets the expected search" do - config = subject.convert - drive = config.drives.first - partition = drive.partitions.first - expect(partition.search).to be_a(Agama::Storage::Configs::Search) - expect(partition.search.name).to eq("/dev/vda1") - expect(partition.search.if_not_found).to eq(:skip) + context "if 'extentSize' is a number" do + let(:size) { 4096 } + + it "sets #extent_size to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.extent_size).to eq(4.KiB) + end end end - end - context "setting delete for a partition" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - search: "/dev/vda1", - delete: true - }, - { - filesystem: { path: "/" } - } - ] - } - ] - } - end + context "if a volume group spicifies 'physicalVolumes'" do + let(:volume_group) { { physicalVolumes: physical_volumes } } - it "sets #delete to true" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - search: have_attributes(name: "/dev/vda1"), - delete: true, - delete_if_needed: false - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - delete: false, - delete_if_needed: false - ) - ) - end - end + context "with an empty list" do + let(:physical_volumes) { [] } - context "setting delete if needed for a partition" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - search: "/dev/vda1", - deleteIfNeeded: true - }, - { - filesystem: { path: "/" } - } - ] - } - ] - } - end + it "sets #physical_volumes to empty" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes).to eq([]) + end + end - it "sets #delete_if_needed to true" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - search: have_attributes(name: "/dev/vda1"), - delete: false, - delete_if_needed: true - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - delete: false, - delete_if_needed: false - ) - ) + context "with a list of aliases" do + let(:physical_volumes) { ["pv1", "pv2"] } + + it "sets #physical_volumes to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes).to contain_exactly("pv1", "pv2") + end + end end - end - context "omitting sizes for the partitions" do - let(:config_json) do - { - drives: [ - { - partitions: example_configs - } + context "if a volume group spicifies 'logicalVolumes'" do + let(:volume_group) { { logicalVolumes: logical_volumes } } + + let(:logical_volumes) do + [ + logical_volume, + { name: "test" } ] - } - end + end - result = proc { |config| config.drives.first.partitions } + let(:logical_volume) { { name: "root" } } - include_examples "omitting sizes", result - end + context "with an empty list" do + let(:logical_volumes) { [] } - context "setting fixed sizes for the partitions" do - let(:config_json) do - { - drives: [ - { - partitions: example_configs - } - ] - } - end + it "sets #logical_volumes to empty" do + vg = vg_proc.call(subject.convert) + expect(vg.logical_volumes).to eq([]) + end + end - result = proc { |config| config.drives.first.partitions } + context "with a list of logical volumes" do + it "sets #logical_volumes to the expected value" do + vg = vg_proc.call(subject.convert) + lvs = vg.logical_volumes + expect(lvs.size).to eq(2) + + lv1, lv2 = lvs + expect(lv1).to be_a(Agama::Storage::Configs::LogicalVolume) + expect(lv1.name).to eq("root") + expect(lv2).to be_a(Agama::Storage::Configs::LogicalVolume) + expect(lv2.name).to eq("test") + end + end - include_examples "fixed sizes", result - end + lv_proc = proc { |c| c.volume_groups.first.logical_volumes.first } - context "specifying size limits for the partitions" do - let(:config_json) do - { - drives: [ - { - partitions: example_configs - } - ] - } - end + context "if a logical volume does not specify 'name'" do + let(:logical_volume) { {} } - result = proc { |config| config.drives.first.partitions } + it "does not set #name" do + lv = lv_proc.call(subject.convert) + expect(lv.name).to be_nil + end + end - include_examples "size limits", result - end + context "if a logical volume does not specify 'stripes'" do + let(:logical_volume) { {} } - context "configuring partial information for several mount points" do - let(:config_json) { { drives: [{ partitions: partitions }] } } - let(:partitions) do - [ - { filesystem: { path: "/" } }, - { filesystem: { path: "swap" } }, - { filesystem: { path: "/opt" } } - ] - end + it "does not set #stripes" do + lv = lv_proc.call(subject.convert) + expect(lv.stripes).to be_nil + end + end - it "configures the filesystem types according to the product configuration" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes( - path: "/", type: have_attributes(fs_type: Y2Storage::Filesystems::Type::BTRFS) - ) - ), - an_object_having_attributes( - filesystem: have_attributes( - path: "swap", type: have_attributes(fs_type: Y2Storage::Filesystems::Type::SWAP) - ) - ), - an_object_having_attributes( - filesystem: have_attributes( - path: "/opt", type: have_attributes(fs_type: Y2Storage::Filesystems::Type::EXT4) - ) - ) - ) - end - end + context "if a logical volume does not specify 'stripeSize'" do + let(:logical_volume) { {} } - context "when some partition is configured to be encrypted" do - let(:config_json) do - { - drives: [{ partitions: partitions }] - } - end + it "does not set #stripe_size" do + lv = lv_proc.call(subject.convert) + expect(lv.stripe_size).to be_nil + end + end - let(:partitions) do - [ - { - id: "linux", size: { min: "10 GiB" }, - filesystem: { type: "xfs", path: "/home" }, - encryption: encryption_home - }, - { - size: { min: "2 GiB" }, - filesystem: { type: "swap", path: "swap" }, - encryption: encryption_swap - } - ] - end + context "if a logical volume does not specify 'pool'" do + let(:logical_volume) { {} } + + it "sets #pool? to false" do + lv = lv_proc.call(subject.convert) + expect(lv.pool?).to eq(false) + end + end + + context "if a logical volume does not specify 'usedPool'" do + let(:logical_volume) { {} } + + it "does not set #used_pool" do + lv = lv_proc.call(subject.convert) + expect(lv.used_pool).to be_nil + end + end + + context "if a logical volume does not specify 'alias'" do + let(:logical_volume) { {} } + include_examples "without alias", lv_proc + end + + context "if a logical volume does not specify 'size'" do + let(:logical_volume) { {} } + include_examples "without size", lv_proc + end + + context "if a logical volume does not specify 'encryption'" do + let(:logical_volume) { {} } + include_examples "without encryption", lv_proc + end + + context "if a logical volume does not specify 'filesystem'" do + let(:logical_volume) { {} } + include_examples "without filesystem", lv_proc + end + + context "if a logical volume specifies 'stripes'" do + let(:logical_volume) { { stripes: 10 } } + + it "sets #stripes to the expected value" do + lv = lv_proc.call(subject.convert) + expect(lv.stripes).to eq(10) + end + end + + context "if a logical volume specifies 'stripeSize'" do + let(:logical_volume) { { stripeSize: size } } + + context "if 'stripeSize' is a string" do + let(:size) { "4 KiB" } + + it "sets #stripe_size to the expected value" do + lv = lv_proc.call(subject.convert) + expect(lv.stripe_size).to eq(4.KiB) + end + end + + context "if 'stripeSize' is a number" do + let(:size) { 4096 } + + it "sets #stripe_size to the expected value" do + lv = lv_proc.call(subject.convert) + expect(lv.stripe_size).to eq(4.KiB) + end + end + end + + context "if a logical volume specifies 'pool'" do + let(:logical_volume) { { pool: true } } + + it "sets #pool? to the expected value" do + lv = lv_proc.call(subject.convert) + expect(lv.pool?).to eq(true) + end + end + + context "if a logical volume specifies 'usedPool'" do + let(:logical_volume) { { usedPool: "pool" } } + + it "sets #used_pool to the expected value" do + lv = lv_proc.call(subject.convert) + expect(lv.used_pool).to eq("pool") + end + end + + context "if a logical volume specifies 'alias'" do + let(:logical_volume) { { alias: device_alias } } + include_examples "with alias", lv_proc + end + + context "if a logical volume specifies 'size'" do + let(:logical_volume) { { size: size } } + include_examples "with size", lv_proc + end + + context "if a logical volume specifies 'encryption'" do + let(:logical_volume) { { encryption: encryption } } + include_examples "with encryption", lv_proc + end + + context "if a logical volume specifies 'filesystem'" do + let(:logical_volume) { { filesystem: filesystem } } + include_examples "with filesystem", lv_proc + end + + context "if a logical volume specifies 'generate'" do + let(:logical_volume) { { generate: generate } } + + logical_volumes_proc = proc { |c| c.volume_groups.first.logical_volumes } + include_examples "with generate", logical_volumes_proc - let(:encryption_home) do - { luks2: { password: "notsecret", keySize: 256 } } - end + context "with a generate section" do + let(:generate) do + { + logicalVolumes: "default", + encryption: { + luks2: { password: "12345" } + }, + stripes: 8, + stripeSize: "16 KiB" + } + end - let(:encryption_swap) { nil } + let(:default_paths) { ["/", "swap"] } - it "sets the encryption settings for the corresponding partition" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - encryption: have_attributes( - password: "notsecret", method: Y2Storage::EncryptionMethod::LUKS2, key_size: 256 - ) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - encryption: nil - ) - ) - end + it "adds the expected logical volumes" do + lvs = subject.convert.volume_groups.first.logical_volumes + expect(lvs.size).to eq(3) - context "if only the password is provided" do - let(:encryption_home) { { luks2: { password: "notsecret" } } } - let(:encryption_swap) { nil } + root_lv = lvs.find { |v| v.filesystem.path == "/" } + swap_lv = lvs.find { |v| v.filesystem.path == "swap" } + test_lv = lvs.find { |v| v.name == "test" } - it "uses the default derivation function" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - encryption: have_attributes( - password: "notsecret", - method: Y2Storage::EncryptionMethod::LUKS2, - pbkd_function: Y2Storage::PbkdFunction::ARGON2ID - ) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - encryption: nil - ) - ) - end - end + expect(root_lv).to_not be_nil + expect(root_lv.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(root_lv.encryption.password).to eq("12345") - context "if random encryption is configured for swap" do - let(:encryption_home) { nil } - let(:encryption_swap) { "random_swap" } + expect(swap_lv).to_not be_nil + expect(swap_lv.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(swap_lv.encryption.password).to eq("12345") - it "sets the corresponding configuration" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - encryption: nil - ), - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - encryption: have_attributes( - password: nil, - label: nil, - cipher: nil, - method: Y2Storage::EncryptionMethod::RANDOM_SWAP - ) - ) - ) + expect(test_lv).to_not be_nil + expect(test_lv.encryption).to be_nil + end + end end end end - context "when the id of some partition is specified" do + context "generating partitions" do let(:config_json) do { - drives: [{ partitions: partitions }] + drives: drives, + volumeGroups: volume_groups } end - let(:partitions) do - [ - { - id: "Esp", size: { min: "10 GiB" }, - filesystem: { type: "xfs", path: "/home" } - }, - { - size: { min: "2 GiB" }, - filesystem: { type: "swap", path: "swap" } - } - ] - end + let(:drives) { [] } - it "configures the corresponding id" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - id: Y2Storage::PartitionId::ESP - ), - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - id: nil - ) - ) - end - end + let(:volume_groups) { [] } - context "with some LVM volume groups" do - let(:config_json) do - { - volumeGroups: [ + let(:default_paths) { ["/", "swap", "/home"] } + + let(:mandatory_paths) { ["/", "swap"] } + + context "if the device already specifies any of the partitions" do + let(:drives) do + [ { - name: "vg0", - extentSize: "2 MiB", - physicalVolumes: ["alias1", "alias2"], - logicalVolumes: [ - { - name: "root", - filesystem: { path: "/" }, - encryption: { - luks2: { password: "12345" } - } - }, - { - alias: "thin-pool", - name: "pool", - pool: true, - size: "100 GiB", - stripes: 10, - stripeSize: "4 KiB" - }, - { - name: "data", - size: "50 GiB", - usedPool: "thin-pool", - filesystem: { type: "xfs" } - } + partitions: [ + { generate: "default" }, + { filesystem: { path: "/home" } } ] - }, - { - name: "vg1" } ] - } - end - - it "generates the corresponding volume groups and logical volumes" do - config = subject.convert + end - expect(config.volume_groups).to contain_exactly( - an_object_having_attributes( - name: "vg0", - extent_size: 2.MiB, - physical_volumes: ["alias1", "alias2"] - ), - an_object_having_attributes( - name: "vg1", - extent_size: be_nil, - physical_volumes: be_empty, - logical_volumes: be_empty - ) - ) - - logical_volumes = config.volume_groups - .find { |v| v.name == "vg0" } - .logical_volumes - - expect(logical_volumes).to include( - an_object_having_attributes( - alias: be_nil, - name: "root", - encryption: have_attributes( - password: "12345", - method: Y2Storage::EncryptionMethod::LUKS2, - pbkd_function: Y2Storage::PbkdFunction::ARGON2ID - ), - filesystem: have_attributes( - path: "/", - type: have_attributes( - fs_type: Y2Storage::Filesystems::Type::BTRFS - ) - ), - size: have_attributes( - default: true, - min: be_nil, - max: be_nil - ), - stripes: be_nil, - stripe_size: be_nil, - pool: false, - used_pool: be_nil - ), - an_object_having_attributes( - alias: "thin-pool", - name: "pool", - encryption: be_nil, - filesystem: be_nil, - size: have_attributes( - default: false, - min: 100.GiB, - max: 100.GiB - ), - stripes: 10, - stripe_size: 4.KiB, - pool: true, - used_pool: be_nil - ), - an_object_having_attributes( - alias: be_nil, - name: "data", - encryption: be_nil, - filesystem: have_attributes( - path: be_nil, - type: have_attributes( - fs_type: Y2Storage::Filesystems::Type::XFS - ) - ), - size: have_attributes( - default: false, - min: 50.GiB, - max: 50.GiB - ), - stripes: be_nil, - stripe_size: be_nil, - pool: false, - used_pool: "thin-pool" - ) - ) + it "only adds partitions for the the missing paths" do + config = subject.convert + partitions = config.drives.first.partitions + expect(partitions.size).to eq(3) + + root_part = partitions.find { |p| p.filesystem.path == "/" } + swap_part = partitions.find { |p| p.filesystem.path == "swap" } + home_part = partitions.find { |p| p.filesystem.path == "/home" } + expect(root_part).to_not be_nil + expect(swap_part).to_not be_nil + expect(home_part).to_not be_nil + end end - end - context "omitting sizes for the logical volumes" do - let(:config_json) do - { - volumeGroups: [ + context "if other device already specifies any of the partitions" do + let(:drives) do + [ { - logicalVolumes: example_configs + partitions: [ + { generate: "default" } + ] + }, + { + partitions: [ + { filesystem: { path: "/home" } } + ] } ] - } - end + end - result = proc { |config| config.volume_groups.first.logical_volumes } + it "only adds partitions for the the missing paths" do + config = subject.convert + partitions = config.drives.first.partitions + expect(partitions.size).to eq(2) - include_examples "omitting sizes", result - end + root_part = partitions.find { |p| p.filesystem.path == "/" } + swap_part = partitions.find { |p| p.filesystem.path == "swap" } + expect(root_part).to_not be_nil + expect(swap_part).to_not be_nil + end + end - context "setting fixed sizes for the logical volumes" do - let(:config_json) do - { - volumeGroups: [ + context "if a volume group already specifies any of the paths" do + let(:drives) do + [ { - logicalVolumes: example_configs + partitions: [ + { generate: "mandatory" } + ] } ] - } - end - - result = proc { |config| config.volume_groups.first.logical_volumes } - - include_examples "fixed sizes", result - end + end - context "specifying size limits for the logical volumes" do - let(:config_json) do - { - volumeGroups: [ + let(:volume_groups) do + [ { - logicalVolumes: example_configs + logicalVolumes: [ + { filesystem: { path: "swap" } } + ] } ] - } - end + end - result = proc { |config| config.volume_groups.first.logical_volumes } + it "only adds partitions for the the missing paths" do + config = subject.convert + partitions = config.drives.first.partitions + expect(partitions.size).to eq(1) - include_examples "size limits", result - end + root_part = partitions.find { |p| p.filesystem.path == "/" } + expect(root_part).to_not be_nil + end + end - context "using 'generate' with 'default' for partitions in a drive" do - let(:config_json) do - { - drives: [ + context "if the device specifies several partitions with 'generate'" do + let(:drives) do + [ { partitions: [ + { generate: "mandatory" }, { generate: "default" } ] } ] - } - end - - it "includes the default partitions defined by the product" do - config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(2) - - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - - swap = partitions.find { |p| p.filesystem.path == "swap" } - expect(swap.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::SWAP) - expect(swap.size.default?).to eq(true) - end - - context "if the drive already defines some of the default paths" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "default" }, - { - filesystem: { path: "swap" }, - size: "2 GiB" - } - ] - } - ] - } end - it "only includes the missing default partitions" do + it "only adds partitions for the first 'generate'" do config = subject.convert partitions = config.drives.first.partitions - expect(partitions.size).to eq(2) - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - - swap = partitions.find { |p| p.filesystem.path == "swap" } - expect(swap.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::SWAP) - expect(swap.size.default?).to eq(false) - expect(swap.size.min).to eq(2.GiB) - expect(swap.size.max).to eq(2.GiB) + root_part = partitions.find { |p| p.filesystem.path == "/" } + swap_part = partitions.find { |p| p.filesystem.path == "swap" } + expect(root_part).to_not be_nil + expect(swap_part).to_not be_nil end end - context "if there are more than one 'generate'" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "default" }, - { generate: "default" } - ] - } - ] - } + context "if several devices specify partitions with 'generate'" do + let(:drives) do + [ + { + partitions: [ + { generate: "mandatory" } + ] + }, + { + partitions: [ + { generate: "default" } + ] + } + ] end - it "does not include the same partition twice" do + it "only adds partitions to the first device with a 'generate'" do config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(2) - - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root).to_not be_nil - - swap = partitions.find { |p| p.filesystem.path == "swap" } - expect(swap).to_not be_nil + drive1, drive2 = config.drives + expect(drive1.partitions.size).to eq(2) + expect(drive2.partitions.size).to eq(0) end end + end - context "if there is a 'generate' with 'mandatory'" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "default" }, - { generate: "mandatory" } - ] - } - ] - } - end + context "generating logical volumes" do + let(:config_json) do + { + drives: drives, + volumeGroups: volume_groups + } + end - it "does not include the same partition twice" do - config = subject.convert - partitions = config.drives.first.partitions + let(:drives) { [] } - expect(partitions.size).to eq(2) + let(:volume_groups) { [] } - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root).to_not be_nil + let(:default_paths) { ["/", "swap", "/home"] } - swap = partitions.find { |p| p.filesystem.path == "swap" } - expect(swap).to_not be_nil - end - end + let(:mandatory_paths) { ["/", "swap"] } - context "if other drive already defines some of the default paths" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "default" } - ] - }, - { - partitions: [ - { - filesystem: { path: "swap" }, - size: "2 GiB" - } - ] - } - ] - } + context "if the volume group already specifies any of the logical volumes" do + let(:volume_groups) do + [ + { + logicalVolumes: [ + { generate: "default" }, + { filesystem: { path: "/home" } } + ] + } + ] end - it "only includes the missing default partitions" do + it "only adds logical volumes for the the missing paths" do config = subject.convert - partitions0 = config.drives[0].partitions - partitions1 = config.drives[1].partitions - - expect(partitions0.size).to eq(1) - - root = partitions0.first - expect(root.filesystem.path).to eq("/") - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - - expect(partitions1.size).to eq(1) - - swap = partitions1.first - expect(swap.filesystem.path).to eq("swap") - expect(swap.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::SWAP) - expect(swap.size.default?).to eq(false) - expect(swap.size.min).to eq(2.GiB) - expect(swap.size.max).to eq(2.GiB) + lvs = config.volume_groups.first.logical_volumes + expect(lvs.size).to eq(3) + + root_lv = lvs.find { |v| v.filesystem.path == "/" } + swap_lv = lvs.find { |v| v.filesystem.path == "swap" } + home_lv = lvs.find { |v| v.filesystem.path == "/home" } + expect(root_lv).to_not be_nil + expect(swap_lv).to_not be_nil + expect(home_lv).to_not be_nil end end - context "if other drive also contains a 'generate'" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "default" } - ] - }, - { - partitions: [ - { generate: "default" } - ] - } - ] - } + context "if other volume group already specifies any of the logical volumes" do + let(:volume_groups) do + [ + { + logicalVolumes: [ + { generate: "default" } + ] + }, + { + logicalVolumes: [ + { filesystem: { path: "/home" } } + ] + } + ] end - it "only includes the default partitions in the first drive" do + it "only adds logical volumes for the the missing paths" do config = subject.convert - partitions0 = config.drives[0].partitions - partitions1 = config.drives[1].partitions + lvs = config.volume_groups.first.logical_volumes + expect(lvs.size).to eq(2) - expect(partitions0.size).to eq(2) - - root = partitions0.find { |p| p.filesystem.path == "/" } - expect(root).to_not be_nil - - swap = partitions0.find { |p| p.filesystem.path == "swap" } - expect(swap).to_not be_nil - - expect(partitions1.size).to eq(0) + root_lv = lvs.find { |v| v.filesystem.path == "/" } + swap_lv = lvs.find { |v| v.filesystem.path == "swap" } + expect(root_lv).to_not be_nil + expect(swap_lv).to_not be_nil end end - end - context "using 'generate' with more properties for partitions in a drive" do - let(:config_json) do - { - drives: [ + context "if a device already specifies a partition for any of the paths" do + let(:drives) do + [ { partitions: [ - { - generate: { - partitions: "default", - encryption: { - luks1: { password: "12345" } - } - } - } + { filesystem: { path: "swap" } } ] } ] - } - end - - it "includes the default partitions defined by the product with the given properties" do - config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(2) - - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - expect(root.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS1) - expect(root.encryption.password).to eq("12345") - - swap = partitions.find { |p| p.filesystem.path == "swap" } - expect(swap.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::SWAP) - expect(swap.size.default?).to eq(true) - expect(swap.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS1) - expect(swap.encryption.password).to eq("12345") - end - end + end - context "using 'generate' with 'mandatory' for partitions in a drive" do - let(:config_json) do - { - drives: [ + let(:volume_groups) do + [ { - partitions: [ + logicalVolumes: [ { generate: "mandatory" } ] } ] - } - end - - it "includes the mandatory partitions defined by the product" do - config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(1) - - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - end - - context "if other device already defines some of the mandatory paths" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "mandatory" } - ] - } - ], - volumeGroups: [ - { - logicalVolumes: [ - { - filesystem: { path: "/" } - } - ] - } - ] - } end - it "does not include the already defined mandatory paths" do + it "only adds logical volumes for the the missing paths" do config = subject.convert - partitions = config.drives.first.partitions - logical_volumes = config.volume_groups.first.logical_volumes + lvs = config.volume_groups.first.logical_volumes + expect(lvs.size).to eq(1) - expect(partitions.size).to eq(0) + root_lv = lvs.find { |v| v.filesystem.path == "/" } + expect(root_lv).to_not be_nil end end - end - context "using 'generate' with 'default' for logical volumes" do - let(:config_json) do - { - volumeGroups: [ + context "if the volume group specifies several logical volumes with 'generate'" do + let(:volume_groups) do + [ { logicalVolumes: [ + { generate: "mandatory" }, { generate: "default" } ] } ] - } - end - - it "includes the default logical volumes defined by the product" do - config = subject.convert - logical_volumes = config.volume_groups.first.logical_volumes - - expect(logical_volumes.size).to eq(2) - - root = logical_volumes.find { |v| v.filesystem.path == "/" } - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - - swap = logical_volumes.find { |v| v.filesystem.path == "swap" } - expect(swap.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::SWAP) - expect(swap.size.default?).to eq(true) - end - - context "if other device already defines any of the default paths" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - filesystem: { path: "/" } - } - ] - } - ], - volumeGroups: [ - { - logicalVolumes: [ - { generate: "default" } - ] - } - ] - } end - it "does not include the already defined default paths" do + it "only adds logical volumes for the first 'generate'" do config = subject.convert - logical_volumes = config.volume_groups.first.logical_volumes - - expect(logical_volumes.size).to eq(1) + lvs = config.volume_groups.first.logical_volumes + expect(lvs.size).to eq(2) - swap = logical_volumes.first - expect(swap.filesystem.path).to eq("swap") + root_lv = lvs.find { |v| v.filesystem.path == "/" } + swap_lv = lvs.find { |v| v.filesystem.path == "swap" } + expect(root_lv).to_not be_nil + expect(swap_lv).to_not be_nil end end - end - context "using 'generate' with 'mandatory' for logical volumes" do - let(:config_json) do - { - volumeGroups: [ + context "if several volume groups specify logical volumes with 'generate'" do + let(:volume_groups) do + [ { logicalVolumes: [ { generate: "mandatory" } ] + }, + { + logicalVolumes: [ + { generate: "default" } + ] } ] - } - end - - it "includes the mandatory logical volumes defined by the product" do - config = subject.convert - logical_volumes = config.volume_groups.first.logical_volumes - - expect(logical_volumes.size).to eq(1) - - root = logical_volumes.first - expect(root.filesystem.path).to eq("/") - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - end - - context "if other device already defines any of the mandatory paths" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - filesystem: { path: "/" } - } - ] - } - ], - volumeGroups: [ - { - logicalVolumes: [ - { generate: "mandatory" } - ] - } - ] - } end - it "does not include the already defined mandatory paths" do + it "only adds logical volumes to the first volume group with a 'generate'" do config = subject.convert - logical_volumes = config.volume_groups.first.logical_volumes - - expect(logical_volumes.size).to eq(0) + vg1, vg2 = config.volume_groups + expect(vg1.logical_volumes.size).to eq(2) + expect(vg2.logical_volumes.size).to eq(0) end end - end - context "using both 'generate' with 'default' and with 'mandatory'" do - let(:config_json) do - { - drives: [ + context "if a drive specifies a partition with 'generate'" do + let(:drives) do + [ { partitions: [ - first_generate, - second_generate + { generate: "mandatory" } ] } ] - } - end - - context "if 'default' appears first" do - let(:first_generate) { { generate: "default" } } - let(:second_generate) { { generate: "mandatory" } } - - it "includes the default partitions defined by the product" do - config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(2) - - root = partitions.find { |p| p.filesystem.path == "/" } - swap = partitions.find { |p| p.filesystem.path == "swap" } - - expect(root).to_not be_nil - expect(swap).to_not be_nil end - end - context "if 'mandatory' appears first" do - let(:first_generate) { { generate: "mandatory" } } - let(:second_generate) { { generate: "default" } } + let(:volume_groups) do + [ + { + logicalVolumes: [ + { generate: "mandatory" } + ] + } + ] + end - it "includes the mandatory partitions defined by the product" do + it "does not add logical volumes to the volume group" do config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(1) - - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root).to_not be_nil + vg = config.volume_groups.first + expect(vg.logical_volumes.size).to eq(0) end end end From b751c4ceee109b2fb7cce58ea753d02ad91b9523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 3 Oct 2024 12:40:43 +0100 Subject: [PATCH 04/10] storage: adapt proposal to new JSON conversion API --- service/lib/agama/storage/proposal.rb | 9 ++++++--- service/test/agama/dbus/storage/manager_test.rb | 4 ++-- service/test/y2storage/agama_proposal_test.rb | 6 +++++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/service/lib/agama/storage/proposal.rb b/service/lib/agama/storage/proposal.rb index f35a869c3..572458ce8 100644 --- a/service/lib/agama/storage/proposal.rb +++ b/service/lib/agama/storage/proposal.rb @@ -234,9 +234,12 @@ def calculate_guided_from_json(guided_json) # @param storage_json [Hash] e.g., { "drives": [] }. # @return [Boolean] Whether the proposal successes. def calculate_agama_from_json(storage_json) - storage_config = ConfigConversions::FromJSON - .new(storage_json, product_config: config) - .convert + storage_config = ConfigConversions::FromJSON.new( + storage_json, + default_paths: config.default_paths, + mandatory_paths: config.mandatory_paths + ).convert + calculate_agama(storage_config) end diff --git a/service/test/agama/dbus/storage/manager_test.rb b/service/test/agama/dbus/storage/manager_test.rb index 89cafd221..f099957bd 100644 --- a/service/test/agama/dbus/storage/manager_test.rb +++ b/service/test/agama/dbus/storage/manager_test.rb @@ -592,10 +592,10 @@ def serialize(value) context "if an agama proposal has been calculated" do before do - proposal.calculate_agama(config) + proposal.calculate_agama(storage_config) end - let(:config) do + let(:storage_config) do fs_type = Agama::Storage::Configs::FilesystemType.new.tap do |t| t.fs_type = Y2Storage::Filesystems::Type::BTRFS end diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index e31e7902a..7d97b4d17 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -83,7 +83,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) let(:config_from_json) do Agama::Storage::ConfigConversions::FromJSON - .new(config_json, product_config: product_config) + .new(config_json, default_paths: default_paths, mandatory_paths: mandatory_paths) .convert end @@ -137,6 +137,10 @@ def partition_config(name: nil, filesystem: nil, size: nil) } end + let(:default_paths) { product_config.default_paths } + + let(:mandatory_paths) { product_config.mandatory_paths } + let(:issues_list) { [] } let(:drives) { [drive0] } From 1ff12350f88162ace9a31f0931aa87ed0ad72b14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 4 Oct 2024 10:42:45 +0100 Subject: [PATCH 05/10] storage: add tests for config solver --- .../test/agama/storage/config_solver_test.rb | 728 ++++++++++++++++++ 1 file changed, 728 insertions(+) create mode 100644 service/test/agama/storage/config_solver_test.rb diff --git a/service/test/agama/storage/config_solver_test.rb b/service/test/agama/storage/config_solver_test.rb new file mode 100644 index 000000000..278e3a1a2 --- /dev/null +++ b/service/test/agama/storage/config_solver_test.rb @@ -0,0 +1,728 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require_relative "./storage_helpers" +require "agama/config" +require "agama/storage/config_conversions/from_json" +require "agama/storage/config_solver" +require "y2storage" +require "y2storage/refinements" + +using Y2Storage::Refinements::SizeCasts + +describe Agama::Storage::ConfigSolver do + include Agama::RSpec::StorageHelpers + + let(:product_data) do + { + "storage" => { + "lvm" => false, + "space_policy" => "delete", + "encryption" => { + "method" => "luks2", + "pbkd_function" => "argon2i" + }, + "volumes" => ["/", "swap"], + "volume_templates" => [ + { + "mount_path" => "/", "filesystem" => "btrfs", + "size" => { "auto" => true, "min" => "5 GiB", "max" => "10 GiB" }, + "btrfs" => { + "snapshots" => true, "default_subvolume" => "@", + "subvolumes" => ["home", "opt", "root", "srv"] + }, + "outline" => { + "required" => true, "snapshots_configurable" => true, + "auto_size" => { + "base_min" => "5 GiB", "base_max" => "10 GiB", + "min_fallback_for" => min_fallbacks, "max_fallback_for" => max_fallbacks, + "snapshots_increment" => snapshots_increment + } + } + }, + { + "mount_path" => "/home", "size" => { "auto" => false, "min" => "5 GiB" }, + "filesystem" => "xfs", "outline" => { "required" => false } + }, + { + "mount_path" => "swap", "filesystem" => "swap", "size" => { "auto" => true }, + "outline" => { + "auto_size" => { + "adjust_by_ram" => true, + "base_min" => "2 GiB", + "base_max" => "4 GiB" + } + } + }, + { "mount_path" => "", "filesystem" => "ext4", + "size" => { "min" => "100 MiB" } } + ] + } + } + end + + let(:min_fallbacks) { [] } + + let(:max_fallbacks) { [] } + + let(:snapshots_increment) { nil } + + let(:product_config) { Agama::Config.new(product_data) } + + let(:default_paths) { product_config.default_paths } + + let(:mandatory_paths) { product_config.mandatory_paths } + + let(:config_json) { nil } + + let(:config) do + Agama::Storage::ConfigConversions::FromJSON + .new(config_json) + .convert + end + + let(:devicegraph) { Y2Storage::StorageManager.instance.probed } + + before do + mock_storage(devicegraph: scenario) + # To speed-up the tests + allow(Y2Storage::EncryptionMethod::TPM_FDE) + .to(receive(:possible?)) + .and_return(true) + end + + subject { described_class.new(devicegraph, product_config) } + + describe "#solve" do + let(:scenario) { "empty-hd-50GiB.yaml" } + + context "if a config does not specify all the encryption properties" do + let(:config_json) do + { + drives: [ + { + encryption: { + luks2: { password: "12345" } + } + } + ] + } + end + + it "completes the encryption config according to the product info" do + subject.solve(config) + + drive = config.drives.first + encryption = drive.encryption + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(encryption.password).to eq("12345") + expect(encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::ARGON2I) + end + end + + context "if a config does not specify all the filesystem properties" do + let(:config_json) do + { + drives: [ + { + filesystem: { path: "/" } + } + ] + } + end + + it "completes the filesystem config according to the product info" do + subject.solve(config) + + drive = config.drives.first + filesystem = drive.filesystem + expect(filesystem.type).to be_a(Agama::Storage::Configs::FilesystemType) + expect(filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) + expect(filesystem.type.btrfs).to be_a(Agama::Storage::Configs::Btrfs) + expect(filesystem.type.btrfs.snapshots?).to eq(true) + expect(filesystem.type.btrfs.read_only?).to eq(false) + expect(filesystem.type.btrfs.default_subvolume).to eq("@") + expect(filesystem.type.btrfs.subvolumes).to all(be_a(Y2Storage::SubvolSpecification)) + end + end + + partition_proc = proc { |c| c.drives.first.partitions.first } + + context "if a config does not specify size" do + let(:config_json) do + { + drives: [ + { + partitions: partitions + } + ] + } + end + + let(:partitions) do + [ + { + filesystem: { path: "/" } + }, + { + filesystem: { path: "/home" } + }, + {} + ] + end + + let(:scenario) { "disks.yaml" } + + it "sets a size according to the product info" do + subject.solve(config) + + drive = config.drives.first + p1, p2, p3 = drive.partitions + + expect(p1.size.default?).to eq(true) + expect(p1.size.min).to eq(5.GiB) + expect(p1.size.max).to eq(10.GiB) + + expect(p2.size.default?).to eq(true) + expect(p2.size.min).to eq(5.GiB) + expect(p2.size.max).to eq(Y2Storage::DiskSize.unlimited) + + expect(p3.size.default?).to eq(true) + expect(p3.size.min).to eq(100.MiB) + expect(p3.size.max).to eq(Y2Storage::DiskSize.unlimited) + end + + context "and there is a device assigned" do + let(:partitions) do + [ + { + search: "/dev/vda2", + filesystem: { path: "/" } + } + ] + end + + # Enable fallbacks and snapshots to check they don't affect in this case. + let(:min_fallbacks) { ["/home"] } + let(:max_fallbacks) { ["/home"] } + let(:snapshots_increment) { "300%" } + + it "sets the device size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(false) + expect(partition.size.min).to eq(20.GiB) + expect(partition.size.max).to eq(20.GiB) + end + end + + context "and the product defines size fallbacks" do + let(:min_fallbacks) { ["/home"] } + let(:max_fallbacks) { ["/home"] } + let(:snapshots_increment) { "300%" } + + context "and the config does not specify some of the paths" do + let(:partitions) do + [ + { + filesystem: { + type: "xfs", + path: "/" + } + } + ] + end + + it "sets a size adding the fallback sizes" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(10.GiB) + expect(partition.size.max).to eq(Y2Storage::DiskSize.unlimited) + end + + context "and snapshots are enabled" do + let(:partitions) do + [ + { + filesystem: { + type: "btrfs", + path: "/" + } + } + ] + end + + it "sets a size adding the fallback and snapshots sizes" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(40.GiB) + expect(partition.size.max).to eq(Y2Storage::DiskSize.unlimited) + end + end + end + + context "and the config specifies the fallback paths" do + let(:partitions) do + [ + { + filesystem: { + type: filesystem, + path: "/" + } + }, + { + filesystem: { path: "/home" } + } + ] + end + + let(:filesystem) { "xfs" } + + it "sets a size ignoring the fallback sizes" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(5.GiB) + expect(partition.size.max).to eq(10.GiB) + end + + context "and snapshots are enabled" do + let(:filesystem) { "btrfs" } + + it "sets a size adding the snapshots size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(20.GiB) + expect(partition.size.max).to eq(40.GiB) + end + end + end + end + + context "and the volume has to be enlarged according to RAM size" do + before do + allow_any_instance_of(Y2Storage::Arch).to receive(:ram_size).and_return(8.GiB) + end + + let(:partitions) do + [ + { + filesystem: { path: "swap" } + } + ] + end + + it "sets the RAM size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(8.GiB) + expect(partition.size.max).to eq(8.GiB) + end + end + end + + context "if a config specifies a size" do + let(:config_json) do + { + drives: [ + { + partitions: [ + { + search: search, + filesystem: { path: "/" }, + size: { + min: "10 GiB", + max: "15 GiB" + } + } + ] + } + ] + } + end + + let(:scenario) { "disks.yaml" } + + # Enable fallbacks and snapshots to check they don't affect in this case. + let(:min_fallbacks) { ["/home"] } + let(:max_fallbacks) { ["/home"] } + let(:snapshots_increment) { "300%" } + + context "and there is no device assigned" do + let(:search) { nil } + + it "sets the given size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(false) + expect(partition.size.min).to eq(10.GiB) + expect(partition.size.max).to eq(15.GiB) + end + end + + context "and there is a device assigned" do + let(:search) { "/dev/vda2" } + + it "sets the given size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(false) + expect(partition.size.min).to eq(10.GiB) + expect(partition.size.max).to eq(15.GiB) + end + end + end + + context "if a config specifies 'current' for min size" do + let(:config_json) do + { + drives: [ + { + partitions: [ + { + search: search, + filesystem: { path: "/" }, + size: { + min: "current", + max: "40 GiB" + } + } + ] + } + ] + } + end + + context "and there is no device assigned" do + let(:search) { nil } + + it "sets a size according to the product info" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(5.GiB) + expect(partition.size.max).to eq(10.GiB) + end + end + + context "and there is a device assigned" do + let(:scenario) { "disks.yaml" } + + let(:search) { "/dev/vda2" } + + it "sets the device size as min size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(false) + expect(partition.size.min).to eq(20.GiB) + expect(partition.size.max).to eq(40.GiB) + end + end + end + + context "if a config specifies 'current' for max size" do + let(:config_json) do + { + drives: [ + { + partitions: [ + { + search: search, + filesystem: { path: "/" }, + size: { + min: "10 GiB", + max: "current" + } + } + ] + } + ] + } + end + + context "and there is no device assigned" do + let(:search) { nil } + + it "sets a size according to the product info" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(5.GiB) + expect(partition.size.max).to eq(10.GiB) + end + end + + context "and there is a device assigned" do + let(:scenario) { "disks.yaml" } + + let(:search) { "/dev/vda2" } + + it "sets the device size as max size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(false) + expect(partition.size.min).to eq(10.GiB) + expect(partition.size.max).to eq(20.GiB) + end + end + end + end + + context "if a drive has a search without a device name" do + let(:config_json) { { drives: drives } } + + let(:drives) do + [ + {}, + {}, + {} + ] + end + + let(:scenario) { "disks.yaml" } + + it "sets the first unassigned device to the drive" do + subject.solve(config) + search1, search2, search3 = config.drives.map(&:search) + expect(search1.solved?).to eq(true) + expect(search1.device.name).to eq("/dev/vda") + expect(search2.solved?).to eq(true) + expect(search2.device.name).to eq("/dev/vdb") + expect(search3.solved?).to eq(true) + expect(search3.device.name).to eq("/dev/vdc") + end + + context "and there is not unassigned device" do + let(:drives) do + [ + {}, + {}, + {}, + {} + ] + end + + it "does not set a device to the drive" do + subject.solve(config) + search = config.drives[3].search + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + end + + context "if a drive has a search with a device name" do + let(:config_json) { { drives: drives } } + + let(:drives) do + [ + { search: search } + ] + end + + let(:scenario) { "disks.yaml" } + + context "and the device is found" do + let(:search) { "/dev/vdb" } + + it "sets the device to the drive" do + subject.solve(config) + search = config.drives.first.search + expect(search.solved?).to eq(true) + expect(search.device.name).to eq("/dev/vdb") + end + end + + context "and the device is not found" do + let(:search) { "/dev/vdd" } + + it "does not set a device to the drive" do + subject.solve(config) + search = config.drives.first.search + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + + context "and the device was already assigned" do + let(:drives) do + [ + {}, + { search: "/dev/vda" } + ] + end + + it "does not set a device to the drive" do + subject.solve(config) + search = config.drives[1].search + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + + context "and there is other drive with the same device" do + let(:drives) do + [ + { search: "/dev/vdb" }, + { search: "/dev/vdb" } + ] + end + + it "only sets the device to the first drive" do + subject.solve(config) + search1, search2 = config.drives.map(&:search) + expect(search1.solved?).to eq(true) + expect(search1.device.name).to eq("/dev/vdb") + expect(search2.solved?).to eq(true) + expect(search2.device).to be_nil + end + end + end + + context "if a partition has a search without a device name" do + let(:config_json) do + { + drives: [ + { partitions: partitions } + ] + } + end + + let(:partitions) do + [ + { search: {} }, + { search: {} }, + { search: {} } + ] + end + + let(:scenario) { "disks.yaml" } + + it "sets the first unassigned partition to the config" do + subject.solve(config) + search1, search2, search3 = config.drives.first.partitions.map(&:search) + expect(search1.solved?).to eq(true) + expect(search1.device.name).to eq("/dev/vda1") + expect(search2.solved?).to eq(true) + expect(search2.device.name).to eq("/dev/vda2") + expect(search3.solved?).to eq(true) + expect(search3.device.name).to eq("/dev/vda3") + end + + context "and there is not unassigned partition" do + let(:partitions) do + [ + { search: {} }, + { search: {} }, + { search: {} }, + { search: {} } + ] + end + + it "does not set a partition to the config" do + subject.solve(config) + search = config.drives.first.partitions[3].search + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + end + + context "if a partition has a search with a device name" do + let(:config_json) do + { + drives: [ + { partitions: partitions } + ] + } + end + + let(:partitions) do + [ + { search: search } + ] + end + + let(:scenario) { "disks.yaml" } + + search_proc = proc { |c| c.drives.first.partitions.first.search } + + context "and the partition is found" do + let(:search) { "/dev/vda2" } + + it "sets the partition to the config" do + subject.solve(config) + search = search_proc.call(config) + expect(search.solved?).to eq(true) + expect(search.device.name).to eq("/dev/vda2") + end + end + + context "and the device is not found" do + let(:search) { "/dev/vdb1" } + + it "does not set a partition to the config" do + subject.solve(config) + search = search_proc.call(config) + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + + context "and the device was already assigned" do + let(:partitions) do + [ + { search: {} }, + { search: "/dev/vda1" } + ] + end + + it "does not set a partition to the config" do + subject.solve(config) + search = config.drives.first.partitions[1].search + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + + context "and there is other partition with the same device" do + let(:partitions) do + [ + { search: "/dev/vda2" }, + { search: "/dev/vda2" } + ] + end + + it "only sets the partition to the first config" do + subject.solve(config) + search1, search2 = config.drives.first.partitions.map(&:search) + expect(search1.solved?).to eq(true) + expect(search1.device.name).to eq("/dev/vda2") + expect(search2.solved?).to eq(true) + expect(search2.device).to be_nil + end + end + end +end From 09998fb95c12030212a85e1e513c50e3fe0935a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 4 Oct 2024 16:04:45 +0100 Subject: [PATCH 06/10] storage: generate physical volumes config --- .../from_json_conversions/volume_group.rb | 48 ++++++- .../agama/storage/config_encryption_solver.rb | 23 +++- .../lib/agama/storage/configs/volume_group.rb | 13 ++ .../config_conversions/from_json_test.rb | 118 +++++++++++++++++- .../test/agama/storage/config_solver_test.rb | 30 +++++ service/test/y2storage/agama_proposal_test.rb | 1 - 6 files changed, 226 insertions(+), 7 deletions(-) diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb index 153331ced..f1516456b 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "agama/storage/config_conversions/from_json_conversions/base" +require "agama/storage/config_conversions/from_json_conversions/encryption" require "agama/storage/config_conversions/from_json_conversions/logical_volume" require "agama/storage/configs/volume_group" @@ -43,10 +44,12 @@ def convert # @return [Hash] def conversions { - name: volume_group_json[:name], - extent_size: convert_extent_size, - physical_volumes: volume_group_json[:physicalVolumes], - logical_volumes: convert_logical_volumes + name: volume_group_json[:name], + extent_size: convert_extent_size, + physical_volumes_devices: convert_physical_volumes_devices, + physical_volumes_encryption: convert_physical_volumes_encryption, + physical_volumes: convert_physical_volumes, + logical_volumes: convert_logical_volumes } end @@ -58,6 +61,43 @@ def convert_extent_size Y2Storage::DiskSize.new(value) end + # @return [Array, nil] + def convert_physical_volumes_devices + generate_json = physical_volume_generate_json&.fetch(:generate) + return unless generate_json + + generate_json.is_a?(Array) ? generate_json : generate_json[:targetDevices] + end + + # @return [Configs::Encryption, nil] + def convert_physical_volumes_encryption + generate_json = physical_volume_generate_json&.fetch(:generate) + return unless generate_json.is_a?(Hash) + + encryption_json = generate_json[:encryption] + return unless encryption_json + + FromJSONConversions::Encryption.new(encryption_json).convert + end + + # JSON of the physical volume with a 'generate'. + # + # @return [Hash, nil] + def physical_volume_generate_json + physical_volumes_json = volume_group_json[:physicalVolumes] + return unless physical_volumes_json + + physical_volumes_json.find { |p| p.is_a?(Hash) } + end + + # @return [Array, nil] + def convert_physical_volumes + physical_volumes_json = volume_group_json[:physicalVolumes] + return unless physical_volumes_json + + physical_volumes_json.select { |c| c.is_a?(String) } + end + # @return [Array, nil] def convert_logical_volumes logical_volumes_json = volume_group_json[:logicalVolumes] diff --git a/service/lib/agama/storage/config_encryption_solver.rb b/service/lib/agama/storage/config_encryption_solver.rb index 4a5996af8..60a02e8a7 100644 --- a/service/lib/agama/storage/config_encryption_solver.rb +++ b/service/lib/agama/storage/config_encryption_solver.rb @@ -41,7 +41,8 @@ def initialize(product_config) def solve(config) @config = config - configs_with_encryption.each { |c| solve_encryption(c) } + solve_encryptions + solve_physical_volumes_encryptions end private @@ -52,6 +53,10 @@ def solve(config) # @return [Config] attr_reader :config + def solve_encryptions + configs_with_encryption.each { |c| solve_encryption(c) } + end + # @param config [#encryption] def solve_encryption(config) return unless config.encryption @@ -64,6 +69,22 @@ def solve_encryption(config) solve_encryption_values(encryption) if encryption.method == default_encryption.method end + def solve_physical_volumes_encryptions + config.volume_groups.each { |c| solve_physical_volumes_encryption(c) } + end + + # @param config [Configs::VolumeGroup] + def solve_physical_volumes_encryption(config) + return unless config.physical_volumes_encryption + + encryption = config.physical_volumes_encryption + encryption.method ||= default_encryption.method + + # Recovering values from the default encryption only makes sense if the encryption method is + # the same. + solve_encryption_values(encryption) if encryption.method == default_encryption.method + end + # @param config [Configs::Encryption] def solve_encryption_values(config) config.password ||= default_encryption.password diff --git a/service/lib/agama/storage/configs/volume_group.rb b/service/lib/agama/storage/configs/volume_group.rb index 0939c5710..c595b6e53 100644 --- a/service/lib/agama/storage/configs/volume_group.rb +++ b/service/lib/agama/storage/configs/volume_group.rb @@ -30,6 +30,18 @@ class VolumeGroup # @return [Y2Storage::DiskSize, nil] attr_accessor :extent_size + # Aliases of the devices used for automatically creating new physical volumes. + # + # @return [Array] + attr_accessor :physical_volumes_devices + + # Encryption for the new physical volumes created at the {physical_volumes_devices}. + # + # @return [Encryption, nil] + attr_accessor :physical_volumes_encryption + + # Aliases of the devices used as physical volumes. + # # @return [Array] attr_accessor :physical_volumes @@ -37,6 +49,7 @@ class VolumeGroup attr_accessor :logical_volumes def initialize + @physical_volumes_devices = [] @physical_volumes = [] @logical_volumes = [] end diff --git a/service/test/agama/storage/config_conversions/from_json_test.rb b/service/test/agama/storage/config_conversions/from_json_test.rb index e45beb63c..119c8b8d7 100644 --- a/service/test/agama/storage/config_conversions/from_json_test.rb +++ b/service/test/agama/storage/config_conversions/from_json_test.rb @@ -924,10 +924,20 @@ context "with an empty list" do let(:physical_volumes) { [] } - it "sets #physical_volumes to empty" do + it "sets #physical_volumes to the expected value" do vg = vg_proc.call(subject.convert) expect(vg.physical_volumes).to eq([]) end + + it "sets #physical_volumes_devices to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_devices).to eq([]) + end + + it "sets #physical_volumes_encryption to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_encryption).to be_nil + end end context "with a list of aliases" do @@ -937,6 +947,112 @@ vg = vg_proc.call(subject.convert) expect(vg.physical_volumes).to contain_exactly("pv1", "pv2") end + + it "sets #physical_volumes_devices to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_devices).to eq([]) + end + + it "sets #physical_volumes_encryption to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_encryption).to be_nil + end + end + + context "with a list including a physical volume with 'generate' array" do + let(:physical_volumes) do + [ + "pv1", + { generate: ["disk1", "disk2"] }, + "pv2" + ] + end + + it "sets #physical_volumes to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes).to contain_exactly("pv1", "pv2") + end + + it "sets #physical_volumes_devices to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_devices).to contain_exactly("disk1", "disk2") + end + + it "does not set #physical_volumes_encryption" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_encryption).to be_nil + end + end + + context "with a list including a physical volume with 'generate' section" do + let(:physical_volumes) do + [ + "pv1", + { + generate: { + targetDevices: target_devices, + encryption: encryption + } + }, + "pv2" + ] + end + + let(:target_devices) { nil } + + let(:encryption) { nil } + + it "sets #physical_volumes to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes).to contain_exactly("pv1", "pv2") + end + + context "if the physical volume does not specify 'targetDevices'" do + let(:target_devices) { nil } + + it "sets #physical_volumes_devices to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_devices).to eq([]) + end + end + + context "if the physical volume does not specify 'encryption'" do + let(:target_devices) { nil } + + it "does not set #physical_volumes_encryption" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_encryption).to be_nil + end + end + + context "if the physical volume specifies 'targetDevices'" do + let(:target_devices) { ["disk1"] } + + it "sets #physical_volumes_devices to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_devices).to contain_exactly("disk1") + end + end + + context "if the physical volume specifies 'encryption'" do + let(:encryption) do + { + luks1: { password: "12345" } + } + end + + it "sets #physical_volumes_encryption to the expected value" do + vg = vg_proc.call(subject.convert) + encryption = vg.physical_volumes_encryption + expect(encryption).to be_a(Agama::Storage::Configs::Encryption) + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS1) + expect(encryption.password).to eq("12345") + expect(encryption.pbkd_function).to be_nil + expect(encryption.label).to be_nil + expect(encryption.cipher).to be_nil + expect(encryption.key_size).to be_nil + end + end end end diff --git a/service/test/agama/storage/config_solver_test.rb b/service/test/agama/storage/config_solver_test.rb index 278e3a1a2..4973011e5 100644 --- a/service/test/agama/storage/config_solver_test.rb +++ b/service/test/agama/storage/config_solver_test.rb @@ -138,6 +138,36 @@ end end + context "if a volume group does not specify all the pv encryption properties" do + let(:config_json) do + { + volumeGroups: [ + { + physicalVolumes: [ + { + generate: { + encryption: { + luks2: { password: "12345" } + } + } + } + ] + } + ] + } + end + + it "completes the encryption config according to the product info" do + subject.solve(config) + + volume_group = config.volume_groups.first + encryption = volume_group.physical_volumes_encryption + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(encryption.password).to eq("12345") + expect(encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::ARGON2I) + end + end + context "if a config does not specify all the filesystem properties" do let(:config_json) do { diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index 7d97b4d17..2022e2b86 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -73,7 +73,6 @@ def partition_config(name: nil, filesystem: nil, size: nil) describe Y2Storage::AgamaProposal do include Agama::RSpec::StorageHelpers - using Y2Storage::Refinements::SizeCasts subject(:proposal) do described_class.new(config, product_config: product_config, issues_list: issues_list) From ebce78c28bba9fec0b93c730abed9670853b4fca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 8 Oct 2024 11:13:37 +0100 Subject: [PATCH 07/10] storage: add checks for physical volumes config --- service/lib/agama/storage/config_checker.rb | 281 ++++++--- .../from_json_conversions/encryption.rb | 4 +- .../test/agama/storage/config_checker_test.rb | 574 ++++++++++++++++++ .../config_conversions/from_json_test.rb | 22 + service/test/y2storage/agama_proposal_test.rb | 8 +- 5 files changed, 814 insertions(+), 75 deletions(-) create mode 100644 service/test/agama/storage/config_checker_test.rb diff --git a/service/lib/agama/storage/config_checker.rb b/service/lib/agama/storage/config_checker.rb index 281fb4447..453e33c1a 100644 --- a/service/lib/agama/storage/config_checker.rb +++ b/service/lib/agama/storage/config_checker.rb @@ -39,8 +39,7 @@ def initialize(config) # # @return [Array] def issues - config.drives.flat_map { |d| drive_issues(d) } + - config.volume_groups.flat_map { |v| volume_group_issues(v) } + drives_issues + volume_groups_issues end private @@ -48,6 +47,13 @@ def issues # @return [Storage::Config] attr_reader :config + # Issues from drives. + # + # @return [Array] + def drives_issues + config.drives.flat_map { |d| drive_issues(d) } + end + # Issues from a drive config. # # @param config [Configs::Drive] @@ -60,6 +66,94 @@ def drive_issues(config) ].flatten.compact end + # Issue for not found device. + # + # @param config [Configs::Drive, Configs::Partition] + # @return [Agama::Issue] + def search_issue(config) + return if !config.search || config.found_device + + if config.is_a?(Agama::Storage::Configs::Drive) + if config.search.skip_device? + warning(_("No device found for an optional drive")) + else + error(_("No device found for a mandatory drive")) + end + elsif config.search.skip_device? + warning(_("No device found for an optional partition")) + else + error(_("No device found for a mandatory partition")) + end + end + + # Issues related to encryption. + # + # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] + # @return [Array] + def encryption_issues(config) + return [] unless config.encryption + + [ + missing_encryption_password_issue(config.encryption), + unavailable_encryption_method_issue(config.encryption), + wrong_encryption_method_issue(config) + ].compact + end + + # @see #encryption_issues + # + # @param config [Configs::Encryption] + # @return [Issue, nil] + def missing_encryption_password_issue(config) + return unless config.missing_password? + + error( + format( + # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device + # (e.g., 'luks1', 'random_swap'). + _("No passphrase provided (required for using the method '%{crypt_method}')."), + crypt_method: config.method.to_human_string + ) + ) + end + + # @see #encryption_issues + # + # @param config [Configs::Encryption] + # @return [Issue, nil] + def unavailable_encryption_method_issue(config) + method = config.method + return if !method || method.available? + + error( + format( + # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device + # (e.g., 'luks1', 'random_swap'). + _("Encryption method '%{crypt_method}' is not available in this system."), + crypt_method: method.to_human_string + ) + ) + end + + # @see #encryption_issues + # + # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] + # @return [Issue, nil] + def wrong_encryption_method_issue(config) + method = config.encryption&.method + return unless method&.only_for_swap? + return if config.filesystem&.path == Y2Storage::MountPoint::SWAP_PATH.to_s + + error( + format( + # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device + # (e.g., 'luks1', 'random_swap'). + _("'%{crypt_method}' is not a suitable method to encrypt the device."), + crypt_method: method.to_human_string + ) + ) + end + # Issues from partitions. # # @param config [Configs::Drive] @@ -79,15 +173,70 @@ def partition_issues(config) ].flatten.compact end + # Issues from volume groups. + # + # @return [Array] + def volume_groups_issues + [ + overused_physical_volumes_devices_issues, + config.volume_groups.flat_map { |v| volume_group_issues(v) } + ].flatten + end + + # Issues for overused target devices for physical volumes. + # + # @note The Agama proposal is not able to calculate if the same target device is used by more + # than one volume group having several target devices. + # + # @return [Array] + def overused_physical_volumes_devices_issues + overused = overused_physical_volumes_devices + return [] if overused.none? + + overused.map do |device| + error( + format( + # TRANSLATORS: %s is the replaced by a device alias (e.g., "disk1"). + _("The device '%s' is used several times as target device for physical volumes"), + device + ) + ) + end + end + + # Aliases of overused target devices for physical volumes. + # + # @return [Array] + def overused_physical_volumes_devices + config.volume_groups + .map(&:physical_volumes_devices) + .map(&:uniq) + .select { |d| d.size > 1 } + .flatten + .tally + .select { |_, v| v > 1 } + .keys + end + # Issues from a volume group config. # # @param config [Configs::VolumeGroup] # @return [Array] def volume_group_issues(config) - lvs_issues = config.logical_volumes.flat_map { |v| logical_volume_issues(v, config) } - pvs_issues = config.physical_volumes.map { |v| missing_physical_volume_issue(v) }.compact + [ + logical_volumes_issues(config), + physical_volumes_issues(config), + physical_volumes_devices_issues(config), + physical_volumes_encryption_issues(config) + ].flatten + end - lvs_issues + pvs_issues + # Issues from a logical volumes. + # + # @param config [Configs::VolumeGroup] + # @return [Array] + def logical_volumes_issues(config) + config.logical_volumes.flat_map { |v| logical_volume_issues(v, config) } end # Issues from a logical volume config. @@ -103,26 +252,6 @@ def logical_volume_issues(lv_config, vg_config) ].compact.flatten end - # Issue for not found device. - # - # @param config [Configs::Drive, Configs::Partition] - # @return [Agama::Issue] - def search_issue(config) - return if !config.search || config.found_device - - if config.is_a?(Agama::Storage::Configs::Drive) - if config.search.skip_device? - warning(_("No device found for an optional drive")) - else - error(_("No device found for a mandatory drive")) - end - elsif config.search.skip_device? - warning(_("No device found for an optional partition")) - else - error(_("No device found for a mandatory partition")) - end - end - # @see #logical_volume_issues # # @param lv_config [Configs::LogicalVolume] @@ -141,13 +270,21 @@ def missing_thin_pool_issue(lv_config, vg_config) error( format( # TRANSLATORS: %s is the replaced by a device alias (e.g., "pv1"). - _("There is no LVM thin pool volume with alias %s"), + _("There is no LVM thin pool volume with alias '%s'"), lv_config.used_pool ) ) end - # @see #logical_volume_issues + # Issues from physical volumes. + # + # @param config [Configs::VolumeGroup] + # @return [Array] + def physical_volumes_issues(config) + config.physical_volumes.map { |v| missing_physical_volume_issue(v) }.compact + end + + # @see #physical_volumes_issues # # @param pv_alias [String] # @return [Issue, nil] @@ -158,80 +295,86 @@ def missing_physical_volume_issue(pv_alias) error( format( # TRANSLATORS: %s is the replaced by a device alias (e.g., "pv1"). - _("There is no LVM physical volume with alias %s"), + _("There is no LVM physical volume with alias '%s'"), pv_alias ) ) end - # Issues related to encryption. + # Issues from physical volumes devices (target devices). # - # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] + # @param config [Configs::VolumeGroup] # @return [Array] - def encryption_issues(config) - return [] unless config.encryption - - [ - missing_encryption_password_issue(config), - available_encryption_method_issue(config), - wrong_encryption_method_issue(config) - ].compact + def physical_volumes_devices_issues(config) + config.physical_volumes_devices + .map { |d| missing_physical_volumes_device_issue(d) } + .compact end - # @see #encryption_issues + # @see #physical_volumes_devices_issue # - # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] + # @param device_alias [String] # @return [Issue, nil] - def missing_encryption_password_issue(config) - return unless config.encryption&.missing_password? + def missing_physical_volumes_device_issue(device_alias) + return if config.drives.any? { |d| d.alias == device_alias } error( format( - # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device - # (e.g., 'luks1', 'random_swap'). - _("No passphrase provided (required for using the method '%{crypt_method}')."), - crypt_method: config.encryption.method.id.to_s + # TRANSLATORS: %s is the replaced by a device alias (e.g., "disk1"). + _("There is no target device for LVM physical volumes with alias '%s'"), + device_alias ) ) end - # @see #encryption_issues + # Issues from physical volumes encryption. # - # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] - # @return [Issue, nil] - def available_encryption_method_issue(config) - method = config.encryption&.method - return if !method || method.available? + # @param config [Configs::VolumeGroup] + # @return [Array] + def physical_volumes_encryption_issues(config) + encryption = config.physical_volumes_encryption + return [] unless encryption - error( - format( - # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device - # (e.g., 'luks1', 'random_swap'). - _("Encryption method '%{crypt_method}' is not available in this system."), - crypt_method: method.id.to_s - ) - ) + [ + missing_encryption_password_issue(encryption), + unavailable_encryption_method_issue(encryption), + wrong_physical_volumes_encryption_method_issue(encryption) + ].compact end - # @see #encryption_issues + # @see #physical_volumes_encryption_issues # - # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] + # @param config [Configs::Encryption] # @return [Issue, nil] - def wrong_encryption_method_issue(config) - method = config.encryption&.method - return unless method&.only_for_swap? - return if config.filesystem&.path == Y2Storage::MountPoint::SWAP_PATH.to_s + def wrong_physical_volumes_encryption_method_issue(config) + method = config.method + return if method.nil? || valid_physical_volumes_encryption_method?(method) error( format( # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device - # (e.g., 'luks1', 'random_swap'). - _("'%{crypt_method}' is not a suitable method to encrypt the device."), - crypt_method: method.id.to_s + # (e.g., 'luks1'). + _("'%{crypt_method}' is not a suitable method to encrypt the physical volumes."), + crypt_method: method.to_human_string ) ) end + # Whether an encryption method can be used for encrypting physical volumes. + # + # @param method [Y2Storage::EncryptionMethod] + # @return [Boolean] + def valid_physical_volumes_encryption_method?(method) + valid_methods = [ + Y2Storage::EncryptionMethod::LUKS1, + Y2Storage::EncryptionMethod::LUKS2, + Y2Storage::EncryptionMethod::PERVASIVE_LUKS2, + Y2Storage::EncryptionMethod::TPM_FDE + ] + + valid_methods.include?(method) + end + # Creates a warning issue. # # @param message [String] diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb index 0bafd6a99..1956c458c 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb @@ -65,7 +65,7 @@ def luks2? def pervasive_luks2? return false unless encryption_json.is_a?(Hash) - !encryption_json[:pervasive_luks2].nil? + !encryption_json[:pervasiveLuks2].nil? end # @return [Hash] @@ -96,7 +96,7 @@ def luks2_conversions # @return [Hash] def pervasive_luks2_conversions - pervasive_json = encryption_json[:pervasive_luks2] + pervasive_json = encryption_json[:pervasiveLuks2] { method: Y2Storage::EncryptionMethod::PERVASIVE_LUKS2, diff --git a/service/test/agama/storage/config_checker_test.rb b/service/test/agama/storage/config_checker_test.rb new file mode 100644 index 000000000..7582d199f --- /dev/null +++ b/service/test/agama/storage/config_checker_test.rb @@ -0,0 +1,574 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require_relative "./storage_helpers" +require "agama/config" +require "agama/storage/config_conversions/from_json" +require "agama/storage/config_checker" +require "agama/storage/config_solver" +require "y2storage" +require "y2storage/refinements" + +using Y2Storage::Refinements::SizeCasts + +shared_examples "encryption issues" do + let(:filesystem) { nil } + + context "without password" do + let(:encryption) do + { luks1: {} } + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("No passphrase") + end + end + + context "with unavailable method" do + let(:encryption) do + { + pervasiveLuks2: { + password: "12345" + } + } + end + + before do + allow_any_instance_of(Y2Storage::EncryptionMethod::PervasiveLuks2) + .to(receive(:available?)) + .and_return(false) + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("'Pervasive Volume Encryption' is not available") + end + end + + context "with invalid method" do + let(:encryption) { "protected_swap" } + let(:filesystem) { { path: "/" } } + + before do + allow_any_instance_of(Y2Storage::EncryptionMethod::ProtectedSwap) + .to(receive(:available?)) + .and_return(true) + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description) + .to(match("'Encryption with Volatile Protected Key' is not a suitable")) + end + end + + context "with a valid encryption" do + let(:encryption) do + { + luks1: { + password: "12345" + } + } + end + + let(:filesystem) { { path: "/" } } + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end +end + +describe Agama::Storage::ConfigChecker do + include Agama::RSpec::StorageHelpers + + subject { described_class.new(config) } + + let(:config) do + Agama::Storage::ConfigConversions::FromJSON + .new(config_json) + .convert + end + + let(:config_json) { nil } + + before do + mock_storage(devicegraph: scenario) + # To speed-up the tests + allow(Y2Storage::EncryptionMethod::TPM_FDE) + .to(receive(:possible?)) + .and_return(true) + end + + describe "#issues" do + before do + # Solves the config before checking. + devicegraph = Y2Storage::StorageManager.instance.probed + product_config = Agama::Config.new + + Agama::Storage::ConfigSolver + .new(devicegraph, product_config) + .solve(config) + end + + let(:scenario) { "disks.yaml" } + + context "if a drive has not found device" do + let(:config_json) do + { + drives: [ + { + search: { + condition: { name: "/dev/vdd" }, + ifNotFound: if_not_found + } + } + ] + } + end + + context "and the drive should be skipped" do + let(:if_not_found) { "skip" } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(false) + expect(issue.description).to eq("No device found for an optional drive") + end + end + + context "and the drive should not be skipped" do + let(:if_not_found) { "error" } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to eq("No device found for a mandatory drive") + end + end + end + + context "if a drive has a found device" do + let(:config_json) do + { + drives: [ + { search: "/dev/vda" } + ] + } + end + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + + context "if a drive has encryption" do + let(:config_json) do + { + drives: [ + { + encryption: encryption, + filesystem: filesystem + } + ] + } + end + + include_examples "encryption issues" + end + + context "if a drive has partitions" do + let(:config_json) do + { + drives: [ + { + partitions: [partition] + } + ] + } + end + + context "and a partition has not found device" do + let(:partition) do + { + search: { + condition: { name: "/dev/vdb1" }, + ifNotFound: if_not_found + } + } + end + + context "and the partition should be skipped" do + let(:if_not_found) { "skip" } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(false) + expect(issue.description).to eq("No device found for an optional partition") + end + end + + context "and the partition should not be skipped" do + let(:if_not_found) { "error" } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to eq("No device found for a mandatory partition") + end + end + end + + context "and the partition has a found device" do + let(:partition) do + { search: "/dev/vda1" } + end + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + + context "and a partition has encryption" do + let(:partition) do + { + encryption: encryption, + filesystem: filesystem + } + end + + include_examples "encryption issues" + end + end + + context "if a volume group has logical volumes" do + let(:config_json) do + { + volumeGroups: [ + { + logicalVolumes: [ + logical_volume, + { + alias: "pool", + pool: true + } + ] + } + ] + } + end + + context "and a logical volume has encryption" do + let(:logical_volume) do + { + encryption: encryption, + filesystem: filesystem + } + end + + include_examples "encryption issues" + end + + context "and a logical volume has an unknown pool" do + let(:logical_volume) do + { usedPool: "unknown" } + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("no LVM thin pool") + end + end + + context "and a logical volume has a known pool" do + let(:logical_volume) do + { usedPool: "pool" } + end + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + end + + context "if a volume group has an unknown physical volume" do + let(:config_json) do + { + drives: [ + { + alias: "first-disk" + } + ], + volumeGroups: [ + { + physicalVolumes: ["first-disk", "pv1"] + } + ] + } + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("no LVM physical volume with alias 'pv1'") + end + end + + context "if a volume group has an unknown target device for physical volumes" do + let(:config_json) do + { + drives: [ + { + alias: "first-disk" + } + ], + volumeGroups: [ + { + physicalVolumes: [ + { + generate: { + targetDevices: ["first-disk", "second-disk"] + } + } + ] + } + ] + } + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description) + .to(match("no target device for LVM physical volumes with alias 'second-disk'")) + end + end + + context "if a volume group has encryption for physical volumes" do + let(:config_json) do + { + drives: [ + { + alias: "first-disk" + } + ], + volumeGroups: [ + { + physicalVolumes: [ + { + generate: { + targetDevices: ["first-disk"], + encryption: encryption + } + } + ] + } + ] + } + end + + context "without password" do + let(:encryption) do + { luks1: {} } + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("No passphrase") + end + end + + context "with unavailable method" do + let(:encryption) do + { + luks2: { + password: "12345" + } + } + end + + before do + allow_any_instance_of(Y2Storage::EncryptionMethod::Luks2) + .to(receive(:available?)) + .and_return(false) + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("'Regular LUKS2' is not available") + end + end + + context "with invalid method" do + let(:encryption) { "random_swap" } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description) + .to(match("'Encryption with Volatile Random Key' is not a suitable method")) + end + end + + context "with a valid encryption" do + let(:encryption) do + { + luks1: { + password: "12345" + } + } + end + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + end + + context "if there are overused physical volumes devices" do + let(:config_json) do + { + drives: [ + { alias: "disk1" }, + { alias: "disk2" }, + { alias: "disk3" } + ], + volumeGroups: [ + { + physicalVolumes: [ + { + generate: { + targetDevices: ["disk1", "disk2"] + } + } + ] + }, + { + physicalVolumes: [ + { + generate: { + targetDevices: ["disk2"] + } + } + ] + }, + { + physicalVolumes: [ + { + generate: { + targetDevices: ["disk1", "disk3", "disk3"] + } + } + ] + } + ] + } + end + + it "includes the expected issues" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to(match("The device 'disk1' is used several times")) + end + end + + context "if the config has several issues" do + let(:config_json) do + { + drives: [ + { + search: "/dev/vdd", + encryption: { luks2: {} } + } + ], + volumeGroups: [ + { + physicalVolumes: ["pv1"] + } + ] + } + end + + it "includes the expected issues" do + expect(subject.issues).to contain_exactly( + an_object_having_attributes( + description: match(/No device found for a mandatory drive/) + ), + an_object_having_attributes( + description: match(/No passphrase provided/) + ), + an_object_having_attributes( + description: match(/There is no LVM physical volume with alias 'pv1'/) + ) + ) + end + end + end +end diff --git a/service/test/agama/storage/config_conversions/from_json_test.rb b/service/test/agama/storage/config_conversions/from_json_test.rb index 119c8b8d7..87792bdb7 100644 --- a/service/test/agama/storage/config_conversions/from_json_test.rb +++ b/service/test/agama/storage/config_conversions/from_json_test.rb @@ -179,6 +179,28 @@ expect(encryption.label).to be_nil end end + + context "if 'encryption' is 'pervasiveLuks2'" do + let(:encryption) do + { + pervasiveLuks2: { + password: "12345" + } + } + end + + it "sets #encryption to the expected value" do + config = config_proc.call(subject.convert) + encryption = config.encryption + expect(encryption).to be_a(Agama::Storage::Configs::Encryption) + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::PERVASIVE_LUKS2) + expect(encryption.password).to eq("12345") + expect(encryption.key_size).to be_nil + expect(encryption.pbkd_function).to be_nil + expect(encryption.cipher).to be_nil + expect(encryption.label).to be_nil + end + end end shared_examples "with filesystem" do |config_proc| diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index 2022e2b86..f193bf4f0 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -396,7 +396,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) it "reports the corresponding error" do proposal.propose expect(proposal.issues_list).to include an_object_having_attributes( - description: /method 'luks2' is not available/, + description: /method 'Regular LUKS2' is not available/, severity: Agama::Issue::Severity::ERROR ) end @@ -413,7 +413,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) it "reports the corresponding error" do proposal.propose expect(proposal.issues_list).to include an_object_having_attributes( - description: /'random_swap' is not a suitable method/, + description: /'Encryption with Volatile Random Key' is not a suitable method/, severity: Agama::Issue::Severity::ERROR ) end @@ -1459,7 +1459,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) it "reports the corresponding error" do proposal.propose expect(proposal.issues_list).to include an_object_having_attributes( - description: /no LVM physical volume with alias pv2/, + description: /no LVM physical volume with alias 'pv2'/, severity: Agama::Issue::Severity::ERROR ) end @@ -1511,7 +1511,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) it "reports the corresponding error" do proposal.propose expect(proposal.issues_list).to include an_object_having_attributes( - description: /no LVM thin pool volume with alias pool/, + description: /no LVM thin pool volume with alias 'pool'/, severity: Agama::Issue::Severity::ERROR ) end From cf82e99884b824b4629184a54632d1c41b3abbbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 7 Oct 2024 08:01:39 +0100 Subject: [PATCH 08/10] service: changelog --- service/package/rubygem-agama-yast.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index 24704a441..cfd7f956c 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Mon Oct 7 06:58:48 UTC 2024 - José Iván López González + +- Storage: add support to the storage config for automatically + creating physical volumes (gh#agama-project/agama#1652). + ------------------------------------------------------------------- Fri Sep 27 14:15:16 UTC 2024 - José Iván López González From e4fe3dbc42a1cafb945bdc1aff41c205bb0a8abf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 8 Oct 2024 16:34:10 +0100 Subject: [PATCH 09/10] storage: add checks for filesystem config --- service/lib/agama/storage/config_checker.rb | 94 +++++++++++- service/lib/y2storage/agama_proposal.rb | 5 +- .../test/agama/storage/config_checker_test.rb | 139 +++++++++++++++++- service/test/y2storage/agama_proposal_test.rb | 55 +++++-- 4 files changed, 266 insertions(+), 27 deletions(-) diff --git a/service/lib/agama/storage/config_checker.rb b/service/lib/agama/storage/config_checker.rb index 453e33c1a..25c3c6a47 100644 --- a/service/lib/agama/storage/config_checker.rb +++ b/service/lib/agama/storage/config_checker.rb @@ -20,19 +20,24 @@ # find current contact information at www.suse.com. require "agama/issue" +require "agama/storage/volume_templates_builder" require "yast/i18n" require "y2storage/mount_point" module Agama module Storage # Class for checking a storage config. - class ConfigChecker + # + # TODO: Split in smaller checkers, for example: ConfigFilesystemChecker, etc. + class ConfigChecker # rubocop:disable Metrics/ClassLength include Yast::I18n # @param config [Storage::Config] - def initialize(config) + # @param product_config [Agama::Config] + def initialize(config, product_config) textdomain "agama" @config = config + @product_config = product_config || Agama::Config.new end # Issues detected in the config. @@ -47,6 +52,9 @@ def issues # @return [Storage::Config] attr_reader :config + # @return [Agama::Config] + attr_reader :product_config + # Issues from drives. # # @return [Array] @@ -61,6 +69,7 @@ def drives_issues def drive_issues(config) [ search_issue(config), + filesystem_issues(config), encryption_issues(config), partitions_issues(config) ].flatten.compact @@ -86,16 +95,76 @@ def search_issue(config) end end + # Issues related to the filesystem. + # + # @param config [#filesystem] + # @return [Array] + def filesystem_issues(config) + filesystem = config.filesystem + return [] unless filesystem + + [ + missing_filesystem_issue(filesystem), + invalid_filesystem_issue(filesystem) + ].compact + end + + # @see #filesystem_issues + # + # @param config [Configs::Filesystem] + # @return [Issue, nil] + def missing_filesystem_issue(config) + return if config.reuse? + return if config.type&.fs_type + + error( + format( + # TRANSLATORS: %s is the replaced by a mount path (e.g., "/home"). + _("Missing file system type for '%s'"), + config.path + ) + ) + end + + # @see #filesystem_issues + # + # @param config [Configs::Filesystem] + # @return [Issue, nil] + def invalid_filesystem_issue(config) + return if config.reuse? + + type = config.type&.fs_type + return unless type + + path = config.path + types = suitable_filesystem_types(path) + return if types.include?(type) + + # Let's consider a type as valid if the product does not define any suitable type. + return if types.empty? + + error( + format( + # TRANSLATORS: %{filesystem} is replaced by a file system type (e.g., "Btrfs") and + # %{path} is replaced by a mount path (e.g., "/home"). + _("The file system type '%{filesystem}' is not suitable for '%{path}'"), + filesystem: type.to_human_string, + path: path + ) + ) + end + # Issues related to encryption. # # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] # @return [Array] def encryption_issues(config) - return [] unless config.encryption + encryption = config.encryption + return [] unless encryption [ - missing_encryption_password_issue(config.encryption), - unavailable_encryption_method_issue(config.encryption), + missing_encryption_password_issue(encryption), + unavailable_encryption_method_issue(encryption), wrong_encryption_method_issue(config) ].compact end @@ -169,6 +238,7 @@ def partitions_issues(config) def partition_issues(config) [ search_issue(config), + filesystem_issues(config), encryption_issues(config) ].flatten.compact end @@ -247,6 +317,7 @@ def logical_volumes_issues(config) # @return [Array] def logical_volume_issues(lv_config, vg_config) [ + filesystem_issues(lv_config), encryption_issues(lv_config), missing_thin_pool_issue(lv_config, vg_config) ].compact.flatten @@ -375,6 +446,19 @@ def valid_physical_volumes_encryption_method?(method) valid_methods.include?(method) end + # Suitable file system types for the given path. + # + # @param path [String, nil] + # @return [Array] + def suitable_filesystem_types(path = nil) + volume_builder.for(path || "").outline.filesystems + end + + # @return [VolumeTemplatesBuilder] + def volume_builder + @volume_builder ||= VolumeTemplatesBuilder.new_from_config(product_config) + end + # Creates a warning issue. # # @param message [String] diff --git a/service/lib/y2storage/agama_proposal.rb b/service/lib/y2storage/agama_proposal.rb index a386da7e4..13675f089 100644 --- a/service/lib/y2storage/agama_proposal.rb +++ b/service/lib/y2storage/agama_proposal.rb @@ -96,7 +96,10 @@ def calculate_proposal .new(initial_devicegraph, product_config) .solve(config) - issues = Agama::Storage::ConfigChecker.new(config).issues + issues = Agama::Storage::ConfigChecker + .new(config, product_config) + .issues + issues_list.concat(issues) if fatal_error? diff --git a/service/test/agama/storage/config_checker_test.rb b/service/test/agama/storage/config_checker_test.rb index 7582d199f..bf818958a 100644 --- a/service/test/agama/storage/config_checker_test.rb +++ b/service/test/agama/storage/config_checker_test.rb @@ -25,9 +25,6 @@ require "agama/storage/config_checker" require "agama/storage/config_solver" require "y2storage" -require "y2storage/refinements" - -using Y2Storage::Refinements::SizeCasts shared_examples "encryption issues" do let(:filesystem) { nil } @@ -110,10 +107,91 @@ end end +shared_examples "filesystem issues" do |filesystem_proc| + context "with invalid type" do + let(:filesystem) do + { + path: "/", + type: "vfat", + reuseIfPossible: reuse + } + end + + context "and without reusing the filesystem" do + let(:reuse) { false } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("type 'FAT' is not suitable for '/'") + end + end + + context "and reusing the filesystem" do + let(:reuse) { true } + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + end + + context "with valid type" do + let(:filesystem) do + { + path: "/", + type: "btrfs" + } + end + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + + context "without a filesystem type" do + let(:filesystem) do + { + path: "/", + reuseIfPossible: reuse + } + end + + before do + # Explicitly remove the filesystem type. Otherwise the JSON conversion assigns a default type. + filesystem_proc.call(config).type = nil + end + + context "and without reusing the filesystem" do + let(:reuse) { false } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to eq("Missing file system type for '/'") + end + end + + context "and reusing the filesystem" do + let(:reuse) { true } + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + end +end + describe Agama::Storage::ConfigChecker do include Agama::RSpec::StorageHelpers - subject { described_class.new(config) } + subject { described_class.new(config, product_config) } let(:config) do Agama::Storage::ConfigConversions::FromJSON @@ -123,6 +201,22 @@ let(:config_json) { nil } + let(:product_config) { Agama::Config.new(product_data) } + + let(:product_data) do + { + "storage" => { + "volume_templates" => [ + { + "mount_path" => "/", + "filesystem" => "btrfs", + "outline" => { "filesystems" => ["btrfs", "xfs"] } + } + ] + } + } + end + before do mock_storage(devicegraph: scenario) # To speed-up the tests @@ -135,7 +229,6 @@ before do # Solves the config before checking. devicegraph = Y2Storage::StorageManager.instance.probed - product_config = Agama::Config.new Agama::Storage::ConfigSolver .new(devicegraph, product_config) @@ -214,6 +307,22 @@ include_examples "encryption issues" end + context "if a drive has filesystem" do + let(:config_json) do + { + drives: [ + { + filesystem: filesystem + } + ] + } + end + + filesystem_proc = proc { |c| c.drives.first.filesystem } + + include_examples "filesystem issues", filesystem_proc + end + context "if a drive has partitions" do let(:config_json) do { @@ -272,6 +381,16 @@ end end + context "if a partition has filesystem" do + let(:partition) do + { filesystem: filesystem } + end + + filesystem_proc = proc { |c| c.drives.first.partitions.first.filesystem } + + include_examples "filesystem issues", filesystem_proc + end + context "and a partition has encryption" do let(:partition) do { @@ -301,6 +420,16 @@ } end + context "if a logical volume has filesystem" do + let(:logical_volume) do + { filesystem: filesystem } + end + + filesystem_proc = proc { |c| c.volume_groups.first.logical_volumes.first.filesystem } + + include_examples "filesystem issues", filesystem_proc + end + context "and a logical volume has encryption" do let(:logical_volume) do { diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index f193bf4f0..33783ab5c 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -107,30 +107,52 @@ def partition_config(name: nil, filesystem: nil, size: nil) "volumes" => ["/", "swap"], "volume_templates" => [ { - "mount_path" => "/", "filesystem" => "btrfs", "size" => { "auto" => true }, - "btrfs" => { - "snapshots" => true, "default_subvolume" => "@", - "subvolumes" => ["home", "opt", "root", "srv"] + "mount_path" => "/", + "filesystem" => "btrfs", + "size" => { "auto" => true }, + "btrfs" => { + "snapshots" => true, + "default_subvolume" => "@", + "subvolumes" => ["home", "opt", "root", "srv"] }, - "outline" => { - "required" => true, "snapshots_configurable" => true, - "auto_size" => { - "base_min" => "5 GiB", "base_max" => "10 GiB", - "min_fallback_for" => ["/home"], "max_fallback_for" => ["/home"], + "outline" => { + "required" => true, + "snapshots_configurable" => true, + "filesystems" => ["btrfs", "xfs", "ext3", "ext4"], + "auto_size" => { + "base_min" => "5 GiB", + "base_max" => "10 GiB", + "min_fallback_for" => ["/home"], + "max_fallback_for" => ["/home"], "snapshots_increment" => "300%" } } }, { - "mount_path" => "/home", "size" => { "auto" => false, "min" => "5 GiB" }, - "filesystem" => "xfs", "outline" => { "required" => false } + "mount_path" => "/home", + "size" => { "auto" => false, "min" => "5 GiB" }, + "filesystem" => "xfs", + "outline" => { + "required" => false, + "filesystems" => ["xfs", "ext4"] + } }, { - "mount_path" => "swap", "filesystem" => "swap", - "outline" => { "required" => false } + "mount_path" => "swap", + "filesystem" => "swap", + "outline" => { + "required" => false, + "filesystems" => ["swap"] + } }, - { "mount_path" => "", "filesystem" => "ext4", - "size" => { "min" => "100 MiB" } } + { + "mount_path" => "", + "filesystem" => "ext4", + "size" => { "min" => "100 MiB" }, + "outline" => { + "filesystems" => ["xfs", "ext4"] + } + } ] } } @@ -1049,7 +1071,8 @@ def partition_config(name: nil, filesystem: nil, size: nil) "filesystem" => "swap", "size" => { "auto" => true }, "outline" => { - "auto_size" => { + "filesystems" => ["swap"], + "auto_size" => { "adjust_by_ram" => true, "base_min" => "2 GiB", "base_max" => "4 GiB" From 4bd5eacbbebf70665bbe2ca8d84a958766d583f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 9 Oct 2024 07:22:22 +0100 Subject: [PATCH 10/10] doc: document generate physical volumes --- doc/auto_storage.md | 75 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/doc/auto_storage.md b/doc/auto_storage.md index 3aea6a7ed..fa4aae674 100644 --- a/doc/auto_storage.md +++ b/doc/auto_storage.md @@ -944,6 +944,81 @@ The *mandatory* keyword can be used for only generating the mandatory partitions } ``` +### Generating Physical Volumes + +Volume groups can be configured to explicitly use a set of devices as physical volumes. The aliases +of the devices to use are added to the list of physical volumes: + +```json +"storage": { + "drives": [ + { + "search": "/dev/vda", + "partitions": [ + { "alias": "pv2" }, + { "alias": "pv1" } + ] + } + ], + "volumeGroups": [ + { + "physicalVolumes": ["pv1", "pv2"] + } + ] +} +``` + +The physical volumes can be automatically generated too, by simply indicating the target devices in +which to create the partitions. For that, a *generate* section is added to the list of physical +volumes: + +```json +"storage": { + "drives": [ + { + "search": "/dev/vda", + "alias": "pvs-disk" + } + ], + "volumeGroups": [ + { + "physicalVolumes": [ + { "generate": ["pvs-disk"] } + ] + } + ] +} +``` + +If the auto-generated physical volumes have to be encrypted, then the encryption config is added to +the *generate* section: + + +```json +"storage": { + "drives": [ + { + "search": "/dev/vda", + "alias": "pvs-disk" + } + ], + "volumeGroups": [ + { + "physicalVolumes": [ + { + "generate": { + "targetDevices": ["pvs-disk"], + "encryption": { + "luks2": { "password": "12345" } + } + } + } + ] + } + ] +} +``` + ### Using the Automatic Proposal On the first implementations, Agama can rely on the process known as Guided Proposal to calculate