From 9f0442d60e111459a37e0ebf7a8ba79171d28c09 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Wed, 28 Jun 2023 13:59:55 +0200 Subject: [PATCH 01/12] Honor proposal encryption_password from ProductFeatures --- src/lib/y2storage/proposal_settings.rb | 10 ++++++++++ test/y2storage/proposal_settings_test.rb | 9 +++++++++ 2 files changed, 19 insertions(+) diff --git a/src/lib/y2storage/proposal_settings.rb b/src/lib/y2storage/proposal_settings.rb index 2aefaa18d..da760e51e 100644 --- a/src/lib/y2storage/proposal_settings.rb +++ b/src/lib/y2storage/proposal_settings.rb @@ -418,6 +418,16 @@ def load_features load_feature(:proposal, :multidisk_first) load_size_feature(:proposal, :lvm_vg_size) load_volumes_feature(:volumes) + load_encryption + end + + # Loads the default encryption settings + # + # The encryption settings are not part of control.xml, but can be injected by a previous step of + # the installation, eg. the dialog of the Common Criteria system role + def load_encryption + load_feature(:proposal, :encryption_password) + @encryption_password = nil if encryption_password&.empty? end def validated_delete_mode(mode) diff --git a/test/y2storage/proposal_settings_test.rb b/test/y2storage/proposal_settings_test.rb index 46c383bca..cb655016c 100755 --- a/test/y2storage/proposal_settings_test.rb +++ b/test/y2storage/proposal_settings_test.rb @@ -336,6 +336,15 @@ def read_feature(feature, value) expect(settings.lvm).to eq false end + it "sets 'encryption_password' based on the feature in the 'proposal' section" do + read_feature("encryption_password", "") + expect(settings.use_encryption).to eq false + expect(settings.encryption_password).to eq nil + read_feature("encryption_password", "SuperSecret") + expect(settings.use_encryption).to eq true + expect(settings.encryption_password).to eq "SuperSecret" + end + it "sets 'delete_resize_configurable' based on the feature in the 'proposal' section" do read_feature("delete_resize_configurable", true) expect(settings.delete_resize_configurable).to eq true From ff8d97690ca33f4e885ecbd65112ec10aafcb299 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Wed, 28 Jun 2023 14:39:04 +0200 Subject: [PATCH 02/12] Please rubocop (although is wrong this time) https://msp-greg.github.io/rubocop/RuboCop/Cop/Lint/SafeNavigationWithEmpty.html says my code "will actually do the opposite of what the author intends" which is not true in this case (as proven by the unit tests). --- src/lib/y2storage/proposal_settings.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/y2storage/proposal_settings.rb b/src/lib/y2storage/proposal_settings.rb index da760e51e..1c061afa8 100644 --- a/src/lib/y2storage/proposal_settings.rb +++ b/src/lib/y2storage/proposal_settings.rb @@ -427,7 +427,9 @@ def load_features # the installation, eg. the dialog of the Common Criteria system role def load_encryption load_feature(:proposal, :encryption_password) - @encryption_password = nil if encryption_password&.empty? + return if encryption_password.nil? + + @encryption_password = nil if encryption_password.empty? end def validated_delete_mode(mode) From a502eeefd99e95c92a9c6e25cd850d77bafdc14b Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Thu, 29 Jun 2023 12:48:56 +0200 Subject: [PATCH 03/12] ProductFeatures: drop encryption_password in favor of encryption --- src/lib/y2storage/proposal_settings.rb | 11 ++++++++--- test/y2storage/proposal_settings_test.rb | 19 ++++++++++++++++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/lib/y2storage/proposal_settings.rb b/src/lib/y2storage/proposal_settings.rb index 1c061afa8..8210f4218 100644 --- a/src/lib/y2storage/proposal_settings.rb +++ b/src/lib/y2storage/proposal_settings.rb @@ -426,10 +426,15 @@ def load_features # The encryption settings are not part of control.xml, but can be injected by a previous step of # the installation, eg. the dialog of the Common Criteria system role def load_encryption - load_feature(:proposal, :encryption_password) - return if encryption_password.nil? + enc = feature(:proposal, :encryption) - @encryption_password = nil if encryption_password.empty? + return unless enc + return unless enc.respond_to?(:password) + + passwd = enc.password.to_s + return if passwd.nil? || passwd.empty? + + self.encryption_password = passwd end def validated_delete_mode(mode) diff --git a/test/y2storage/proposal_settings_test.rb b/test/y2storage/proposal_settings_test.rb index cb655016c..5051a5d88 100755 --- a/test/y2storage/proposal_settings_test.rb +++ b/test/y2storage/proposal_settings_test.rb @@ -35,6 +35,19 @@ def stub_partitioning_features(features = {}) stub_features("partitioning" => initial_partitioning_features.merge(features)) end + # Used to test the mechanism to inject an encryption password into the settings + class TestInjectedPassword + include Y2Storage::SecretAttributes + + # Real password + secret_attr :password + + # Constructor + def initialize(passwd) + self.password = passwd + end + end + before do Y2Storage::StorageManager.create_test_instance end @@ -336,11 +349,11 @@ def read_feature(feature, value) expect(settings.lvm).to eq false end - it "sets 'encryption_password' based on the feature in the 'proposal' section" do - read_feature("encryption_password", "") + it "sets 'encryption_password' based on the 'encryption' feature in the 'proposal' section" do + read_feature("encryption", "SuperSecret") expect(settings.use_encryption).to eq false expect(settings.encryption_password).to eq nil - read_feature("encryption_password", "SuperSecret") + read_feature("encryption", TestInjectedPassword.new("SuperSecret")) expect(settings.use_encryption).to eq true expect(settings.encryption_password).to eq "SuperSecret" end From ad32141bfbf42d7ad4a9f7b5b31f422a3c5133ce Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Thu, 29 Jun 2023 13:44:14 +0200 Subject: [PATCH 04/12] Version and changelog --- package/yast2-storage-ng.changes | 7 +++++++ package/yast2-storage-ng.spec | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes index fcc33b236..255f14bae 100644 --- a/package/yast2-storage-ng.changes +++ b/package/yast2-storage-ng.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Thu Jun 29 11:42:21 UTC 2023 - Ancor Gonzalez Sosa + +- Honor encryption settings if they are set into ProductFeatures + by the Common Critera role (jsc#PED-4166, jsc#PED-4474). +- 4.4.44 + ------------------------------------------------------------------- Wed Jun 7 08:03:52 UTC 2023 - Stefan Hundhammer diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec index 76cb0f298..2ba93bca6 100644 --- a/package/yast2-storage-ng.spec +++ b/package/yast2-storage-ng.spec @@ -16,7 +16,7 @@ # Name: yast2-storage-ng -Version: 4.4.43 +Version: 4.4.44 Release: 0 Summary: YaST2 - Storage Configuration License: GPL-2.0-only OR GPL-3.0-only From 95bca984c649eea23b7a8f3ea76d6e0aeccff224 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 10:34:24 +0200 Subject: [PATCH 05/12] Factored out required vs. optional used features --- src/lib/y2storage/devicegraph.rb | 17 +++++++++++++++++ test/y2storage/devicegraph_test.rb | 30 ++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/lib/y2storage/devicegraph.rb b/src/lib/y2storage/devicegraph.rb index cc0fade22..5c6c006d3 100644 --- a/src/lib/y2storage/devicegraph.rb +++ b/src/lib/y2storage/devicegraph.rb @@ -615,6 +615,23 @@ def used_features(required_only: false) StorageFeaturesList.from_bitfield(storage_used_features(type)) end + # List of required (mandatory) storage features used by the devicegraph + # + # @return [StorageFeaturesList] + def required_used_features + used_features(required_only: true) + end + + # List of optional storage features used by the devicegraph + # + # @return [StorageFeaturesList] + def optional_used_features + all = storage_used_features(Storage::UsedFeaturesDependencyType_SUGGESTED) + required = storage_used_features(Storage::UsedFeaturesDependencyType_REQUIRED) + # Using binary XOR in those bit fields to calculate the difference + StorageFeaturesList.from_bitfield(all ^ required) + end + private # Copy of a device tree where hashes have been substituted by sorted diff --git a/test/y2storage/devicegraph_test.rb b/test/y2storage/devicegraph_test.rb index 2600a836b..2050ad9e0 100755 --- a/test/y2storage/devicegraph_test.rb +++ b/test/y2storage/devicegraph_test.rb @@ -1331,4 +1331,34 @@ def with_sda2_deleted(initial_graph) end end end + + describe "#required_used_features" do + before { fake_scenario(scenario) } + + context "with local devices combining several filesystem types" do + let(:scenario) { "mixed_disks" } + + it "returns only the features for mounted filesystems" do + features = fake_devicegraph.required_used_features + expect(features).to be_a Y2Storage::StorageFeaturesList + expect(features.map(&:id)) + .to contain_exactly(:UF_BTRFS, :UF_XFS, :UF_SWAP) + end + end + end + + describe "#optional_used_features" do + before { fake_scenario(scenario) } + + context "with local devices combining several filesystem types" do + let(:scenario) { "mixed_disks" } + + it "returns only the features for filesystems that are not mounted" do + features = fake_devicegraph.optional_used_features + expect(features).to be_a Y2Storage::StorageFeaturesList + expect(features.map(&:id)) + .to contain_exactly(:UF_EXT4, :UF_NTFS) + end + end + end end From 7d03f853afc67b4286b398c2e9f33ce273c9d28f Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 11:17:13 +0200 Subject: [PATCH 06/12] Factored out storage pkg proposal handling to PackageHandler --- .../y2storage/clients/inst_disk_proposal.rb | 18 +----------------- .../y2storage/clients/partitions_proposal.rb | 5 +++-- src/lib/y2storage/package_handler.rb | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/lib/y2storage/clients/inst_disk_proposal.rb b/src/lib/y2storage/clients/inst_disk_proposal.rb index 606f5dd5d..b6f29de0f 100755 --- a/src/lib/y2storage/clients/inst_disk_proposal.rb +++ b/src/lib/y2storage/clients/inst_disk_proposal.rb @@ -162,23 +162,7 @@ def actions_after_partitioner(devicegraph, dialog_result) # Add storage-related software packages (filesystem tools etc.) to the # set of packages to be installed. def add_storage_packages - features = storage_manager.staging.used_features - required_features = storage_manager.staging.used_features(required_only: true) - - required_packages = required_features.pkg_list - optional_packages = features.pkg_list - required_packages - - set_proposal_packages(required_packages, false) - set_proposal_packages(optional_packages, true) - end - - # @see #add_storage_packages - # - # @param pkgs [Array] list of packages - # @param optional [Boolean] whether the packages in the list are optional - def set_proposal_packages(pkgs, optional) - pkg_handler = Y2Storage::PackageHandler.new(pkgs, optional: optional) - pkg_handler.set_proposal_packages + Y2Storage::PackageHandler.set_proposal_packages_for(storage_manager.staging) end # Save the list of filesystem to /etc/sysconfig/storage. diff --git a/src/lib/y2storage/clients/partitions_proposal.rb b/src/lib/y2storage/clients/partitions_proposal.rb index eadcc21d4..43a528e5b 100755 --- a/src/lib/y2storage/clients/partitions_proposal.rb +++ b/src/lib/y2storage/clients/partitions_proposal.rb @@ -120,8 +120,9 @@ def update_state staging = storage_manager.staging actiongraph = staging ? staging.actiongraph : nil self.actions_presenter = ActionsPresenter.new(actiongraph) - Y2Storage::DumpManager.dump(staging) - Y2Storage::DumpManager.dump(actions_presenter) + DumpManager.dump(staging) + DumpManager.dump(actions_presenter) + PackageHandler.set_proposal_packages_for(staging) end end diff --git a/src/lib/y2storage/package_handler.rb b/src/lib/y2storage/package_handler.rb index 2c59f5832..afb6157d4 100755 --- a/src/lib/y2storage/package_handler.rb +++ b/src/lib/y2storage/package_handler.rb @@ -116,6 +116,21 @@ def set_proposal_packages success end + # Add the proposal packages for storage that are needed for the specified + # devicegraph's used features. This marks the packages for installation; + # it does not install them yet. + # + # @param devicegraph [Devicegraph] usually StorageManager.instance.staging + # @param optional + def self.set_proposal_packages_for(devicegraph, optional: true) + required_packages = devicegraph.required_used_features.pkg_list + PackageHandler.new(required_packages, optional: false).set_proposal_packages + return unless optional + + optional_packages = devicegraph.optional_used_features.pkg_list + PackageHandler.new(optional_packages, optional: true).set_proposal_packages + end + private # Whether the packages should be considered as optional when adding them to the From c6769261eb4e9f07a4a70987f08003b99ff95964 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 14:22:32 +0200 Subject: [PATCH 07/12] Extended unit test for fringe cases --- test/y2storage/devicegraph_test.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/y2storage/devicegraph_test.rb b/test/y2storage/devicegraph_test.rb index 2050ad9e0..98aaeeaf3 100755 --- a/test/y2storage/devicegraph_test.rb +++ b/test/y2storage/devicegraph_test.rb @@ -1345,6 +1345,16 @@ def with_sda2_deleted(initial_graph) .to contain_exactly(:UF_BTRFS, :UF_XFS, :UF_SWAP) end end + + context "with an empty disk" do + let(:scenario) { "empty_disks" } + + it "Survives not having any storage features" do + features = fake_devicegraph.required_used_features + expect(features).to be_a Y2Storage::StorageFeaturesList + expect(features.map(&:id)).to be == [] + end + end end describe "#optional_used_features" do @@ -1360,5 +1370,15 @@ def with_sda2_deleted(initial_graph) .to contain_exactly(:UF_EXT4, :UF_NTFS) end end + + context "with an empty disk" do + let(:scenario) { "empty_disks" } + + it "Survives not having any storage features" do + features = fake_devicegraph.optional_used_features + expect(features).to be_a Y2Storage::StorageFeaturesList + expect(features.map(&:id)).to be == [] + end + end end end From 0247f7e4be4eac609513c2ffa253833de24cbfe4 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 4 Jul 2023 14:43:09 +0200 Subject: [PATCH 08/12] Added unit test for new PackageHandler.set_proposal_packages_for method --- test/y2storage/package_handler_test.rb | 42 ++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/y2storage/package_handler_test.rb b/test/y2storage/package_handler_test.rb index dc7bb7bb0..a2efb1ba2 100755 --- a/test/y2storage/package_handler_test.rb +++ b/test/y2storage/package_handler_test.rb @@ -135,4 +135,46 @@ end end end + + describe "#set_proposal_packages_for" do + before do + fake_scenario(scenario) + allow(Yast::Mode).to receive(:installation).and_return(installation) + allow(Yast::Package).to receive(:Installed).and_return(false) + allow(Yast::Package).to receive(:Available).and_return(true) + end + + context "with local devices combining several filesystem types" do + PROPOSAL_ID = "storage_proposal" + let(:scenario) { "mixed_disks" } + let(:installation) { true } + + it "Adds the correct required and optional storage packages to the proposal" do + expect(Yast::PackagesProposal).to receive(:SetResolvables) + .with(PROPOSAL_ID, :package, ["btrfsprogs", "e2fsprogs", "xfsprogs"], optional: false) + .and_return true + expect(Yast::PackagesProposal).to receive(:SetResolvables) + .with(PROPOSAL_ID, :package, ["e2fsprogs", "ntfs-3g", "ntfsprogs"], optional: true) + .and_return true + described_class.set_proposal_packages_for(fake_devicegraph) + end + + it "Adds only required storage packages to the proposal if 'optional' is 'false" do + expect(Yast::PackagesProposal).to receive(:SetResolvables) + .with(PROPOSAL_ID, :package, ["btrfsprogs", "e2fsprogs", "xfsprogs"], optional: false) + .and_return true + described_class.set_proposal_packages_for(fake_devicegraph, optional: false) + end + end + + context "with empty disks" do + let(:scenario) { "empty_disks" } + let(:installation) { true } + + it "Does not add any storage packages to the proposal" do + expect(Yast::PackagesProposal).not_to receive(:SetResolvables) + described_class.set_proposal_packages_for(fake_devicegraph, optional: true) + end + end + end end From 1a8c57249d02085a8cf70a4e20dc031e06d0345d Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Wed, 5 Jul 2023 15:47:00 +0200 Subject: [PATCH 09/12] Version bump and change log --- package/yast2-storage-ng.changes | 9 +++++++++ package/yast2-storage-ng.spec | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes index 255f14bae..fc4a0b1b2 100644 --- a/package/yast2-storage-ng.changes +++ b/package/yast2-storage-ng.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Wed Jul 5 13:42:12 UTC 2023 - Stefan Hundhammer + +- Ensure adding storage support software packages for MicroOS + which uses its custom partitions_proposal client, not the + standard inst_disk_proposal client (bsc#1212452) + https://github.com/yast/yast-storage-ng/pull/1346 +- 4.4.45 + ------------------------------------------------------------------- Thu Jun 29 11:42:21 UTC 2023 - Ancor Gonzalez Sosa diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec index 2ba93bca6..8323ba9fe 100644 --- a/package/yast2-storage-ng.spec +++ b/package/yast2-storage-ng.spec @@ -16,7 +16,7 @@ # Name: yast2-storage-ng -Version: 4.4.44 +Version: 4.4.45 Release: 0 Summary: YaST2 - Storage Configuration License: GPL-2.0-only OR GPL-3.0-only From f2e67b5fd14b1c1bbc6683eab3fd1f96cb2b6a6e Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Mon, 10 Jul 2023 10:40:02 +0200 Subject: [PATCH 10/12] Use class method notation for class method unit tests --- test/y2storage/package_handler_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/y2storage/package_handler_test.rb b/test/y2storage/package_handler_test.rb index a2efb1ba2..48cc36582 100755 --- a/test/y2storage/package_handler_test.rb +++ b/test/y2storage/package_handler_test.rb @@ -136,7 +136,7 @@ end end - describe "#set_proposal_packages_for" do + describe ".set_proposal_packages_for" do before do fake_scenario(scenario) allow(Yast::Mode).to receive(:installation).and_return(installation) From fcc76021e62e3d11040930b5a4cf68ecade02ccd Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Mon, 10 Jul 2023 13:04:00 +0200 Subject: [PATCH 11/12] Drop cache for reproducable results for parallel unit tests --- src/lib/y2storage/storage_feature.rb | 8 ++++++++ test/y2storage/package_handler_test.rb | 2 ++ test/y2storage/storage_features_list_test.rb | 2 ++ 3 files changed, 12 insertions(+) diff --git a/src/lib/y2storage/storage_feature.rb b/src/lib/y2storage/storage_feature.rb index 55cd999e9..ca80de157 100755 --- a/src/lib/y2storage/storage_feature.rb +++ b/src/lib/y2storage/storage_feature.rb @@ -124,6 +124,14 @@ def self.all end end + # Drop the cache of storage packages that are available. + # + # This is only ever needed if the available packages might have changed + # since the last use of this class. + def self.drop_cache + @all = nil + end + # Constructor # # This looks up a constant in the ::Storage namespace to make sure the id diff --git a/test/y2storage/package_handler_test.rb b/test/y2storage/package_handler_test.rb index 48cc36582..92eb4aa78 100755 --- a/test/y2storage/package_handler_test.rb +++ b/test/y2storage/package_handler_test.rb @@ -23,6 +23,7 @@ require_relative "spec_helper" require "y2storage/package_handler" +require "y2storage/storage_feature" describe Y2Storage::PackageHandler do let(:feature_pkg) { ["lvm2", "btrfsprogs", "e2fsprogs"] } @@ -139,6 +140,7 @@ describe ".set_proposal_packages_for" do before do fake_scenario(scenario) + Y2Storage::StorageFeature.drop_cache allow(Yast::Mode).to receive(:installation).and_return(installation) allow(Yast::Package).to receive(:Installed).and_return(false) allow(Yast::Package).to receive(:Available).and_return(true) diff --git a/test/y2storage/storage_features_list_test.rb b/test/y2storage/storage_features_list_test.rb index 8f53fd49b..ffa61b556 100755 --- a/test/y2storage/storage_features_list_test.rb +++ b/test/y2storage/storage_features_list_test.rb @@ -22,6 +22,7 @@ # find current contact information at www.suse.com. require_relative "spec_helper" +require "y2storage/storage_feature" require "y2storage/storage_features_list" describe Y2Storage::StorageFeaturesList do @@ -84,6 +85,7 @@ let(:bits) { Storage::UF_NTFS | Storage::UF_EXT3 } before do + Y2Storage::StorageFeature.drop_cache allow(Yast::Package).to receive(:Available).and_return false allow(Yast::Package).to receive(:Available).with("ntfsprogs").and_return true end From 985006eb068162588ec383c1a0da7a5bb97cff24 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Mon, 10 Jul 2023 17:12:49 +0200 Subject: [PATCH 12/12] Fixed PR URL --- package/yast2-storage-ng.changes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes index e557282c0..daee9bf1d 100644 --- a/package/yast2-storage-ng.changes +++ b/package/yast2-storage-ng.changes @@ -4,7 +4,7 @@ Wed Jul 5 13:42:12 UTC 2023 - Stefan Hundhammer - Ensure adding storage support software packages for MicroOS which uses its custom partitions_proposal client, not the standard inst_disk_proposal client (bsc#1212452) - https://github.com/yast/yast-storage-ng/pull/1347 + https://github.com/yast/yast-storage-ng/pull/1350 - 4.5.24 -------------------------------------------------------------------