diff --git a/service/lib/agama/storage/config_checker.rb b/service/lib/agama/storage/config_checker.rb index ff991d0af..25a32e27f 100644 --- a/service/lib/agama/storage/config_checker.rb +++ b/service/lib/agama/storage/config_checker.rb @@ -170,8 +170,49 @@ def partition_issues(config) ].flatten.compact end + # Issues from volume groups. + # + # @return [Array] def volume_groups_issues - config.volume_groups.flat_map { |v| volume_group_issues(v) } + [ + 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. @@ -187,6 +228,10 @@ def volume_group_issues(config) ].flatten end + # 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 @@ -228,6 +273,10 @@ def missing_thin_pool_issue(lv_config, vg_config) ) end + # 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 @@ -249,12 +298,20 @@ def missing_physical_volume_issue(pv_alias) ) end + # Issues from physical volumes devices (target devices). + # + # @param config [Configs::VolumeGroup] + # @return [Array] def physical_volumes_devices_issues(config) config.physical_volumes_devices .map { |d| missing_physical_volumes_device_issue(d) } .compact end + # @see #physical_volumes_devices_issue + # + # @param device_alias [String] + # @return [Issue, nil] def missing_physical_volumes_device_issue(device_alias) return if config.drives.any? { |d| d.alias == device_alias } @@ -267,7 +324,7 @@ def missing_physical_volumes_device_issue(device_alias) ) end - # @see {#volume_group_issues} + # Issues from physical volumes encryption. # # @param config [Configs::VolumeGroup] # @return [Array] @@ -282,7 +339,7 @@ def physical_volumes_encryption_issues(config) ].compact end - # @see {#physical_volumes_encryption_issues} + # @see #physical_volumes_encryption_issues # # @param config [Configs::Encryption] # @return [Issue, nil] @@ -295,17 +352,24 @@ def wrong_physical_volumes_encryption_method_issue(config) # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device # (e.g., 'luks1'). _("'%{crypt_method}' is not a suitable method to encrypt the physical volumes."), - crypt_method: method.id.to_s + 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) - suitable = [Y2Storage::EncryptionMethod::LUKS1, Y2Storage::EncryptionMethod::LUKS2] - - suitiable.include?(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. diff --git a/service/test/agama/storage/config_checker_test.rb b/service/test/agama/storage/config_checker_test.rb index 95ba28d90..bb9f4eefe 100644 --- a/service/test/agama/storage/config_checker_test.rb +++ b/service/test/agama/storage/config_checker_test.rb @@ -210,7 +210,7 @@ drives: [ { search: { - condition: { name: "/dev/vdd" }, + condition: { name: "/dev/vdd" }, ifNotFound: if_not_found } } @@ -289,7 +289,7 @@ let(:partition) do { search: { - condition: { name: "/dev/vdb1" }, + condition: { name: "/dev/vdb1" }, ifNotFound: if_not_found } } @@ -353,7 +353,7 @@ logical_volume, { alias: "pool", - pool: true + pool: true } ] } @@ -401,7 +401,7 @@ context "if a volume group has an unknown physical volume" do let(:config_json) do { - drives: [ + drives: [ { alias: "first-disk" } @@ -427,7 +427,7 @@ context "if a volume group has an unknown target device for physical volumes" do let(:config_json) do { - drives: [ + drives: [ { alias: "first-disk" } @@ -460,7 +460,7 @@ context "if a volume group has encryption for physical volumes" do let(:config_json) do { - drives: [ + drives: [ { alias: "first-disk" } @@ -471,9 +471,7 @@ { generate: { targetDevices: ["first-disk"], - encryption: { - - } + encryption: encryption } } ] @@ -500,14 +498,14 @@ context "with unavailable method" do let(:encryption) do { - pervasiveLuks2: { + luks2: { password: "12345" } } end before do - allow_any_instance_of(Y2Storage::EncryptionMethod::PervasiveLuks2) + allow_any_instance_of(Y2Storage::EncryptionMethod::Luks2) .to(receive(:available?)) .and_return(false) end @@ -518,20 +516,104 @@ issue = issues.first expect(issue.error?).to eq(true) - expect(issue.description).to match("'Pervasive Volume Encryption' is not available") + 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) - it "includes the expected issue" do + 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 + 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("no target device for LVM physical volumes with alias 'second-disk'")) + 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