diff --git a/package/yast2-firewall.changes b/package/yast2-firewall.changes index 6846f28f..5674c5cf 100644 --- a/package/yast2-firewall.changes +++ b/package/yast2-firewall.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Wed Jan 23 09:52:51 UTC 2019 - knut.anderssen@suse.com + +- Autoyast: do not overwrite imported configuration when editing + and fixed check for not configured summary (fate#324662) +- 4.1.10 + ------------------------------------------------------------------- Thu Jan 17 12:53:20 UTC 2019 - knut.anderssen@suse.com diff --git a/package/yast2-firewall.spec b/package/yast2-firewall.spec index ca35f8ff..bf1715ff 100644 --- a/package/yast2-firewall.spec +++ b/package/yast2-firewall.spec @@ -17,7 +17,7 @@ Name: yast2-firewall -Version: 4.1.9 +Version: 4.1.10 Release: 0 BuildRoot: %{_tmppath}/%{name}-%{version}-build diff --git a/src/lib/y2firewall/clients/auto.rb b/src/lib/y2firewall/clients/auto.rb index 381ba43e..06d69279 100644 --- a/src/lib/y2firewall/clients/auto.rb +++ b/src/lib/y2firewall/clients/auto.rb @@ -69,7 +69,7 @@ def initialize def summary presenter = Y2Firewall::SummaryPresenter.new(firewalld) return presenter.not_installed if !firewalld.installed? - return presenter.not_configured if !modified? + return presenter.not_configured if !firewalld.read? && !firewalld.modified? presenter.create end @@ -107,7 +107,8 @@ def reset end def change - self.class.imported = false + read_keeping_configuration + result = Dialogs::Main.new.run case result when :next, :finish, :ok, :accept @@ -162,6 +163,19 @@ def modified? private + # Read the minimal configuration from firewalld, w/o dropping available configuration + # + # Useful to preserve the already imported, but not written yet, configuration (if any) + def read_keeping_configuration + return unless firewalld.installed? + return if firewalld.read? + + self.class.profile = export + firewalld.reset + firewalld.read(minimal: true) + import(self.class.profile, false) + end + def import_if_needed if ay_config? self.class.profile = export diff --git a/src/lib/y2firewall/dialogs/main.rb b/src/lib/y2firewall/dialogs/main.rb index 36a9314c..b982e8a4 100644 --- a/src/lib/y2firewall/dialogs/main.rb +++ b/src/lib/y2firewall/dialogs/main.rb @@ -36,9 +36,7 @@ def initialize Yast.import "NetworkInterfaces" textdomain "firewall" - if Yast::Mode.config - fw.read(minimal: true) unless fw.read? - else + unless Yast::Mode.config Yast::NetworkInterfaces.Read fw.read unless fw.read? end diff --git a/test/lib/y2firewall/clients/auto_test.rb b/test/lib/y2firewall/clients/auto_test.rb index f47d374a..fdb85877 100755 --- a/test/lib/y2firewall/clients/auto_test.rb +++ b/test/lib/y2firewall/clients/auto_test.rb @@ -20,27 +20,26 @@ # ------------------------------------------------------------------------------ require_relative "../../../test_helper" +require "y2firewall/dialogs/main" require "y2firewall/clients/auto" describe Y2Firewall::Clients::Auto do let(:firewalld) { Y2Firewall::Firewalld.instance } + let(:installed) { true } let(:autoyast) { double("Y2Firewall::Autoyast", import: true, export: {}) } before do - allow_any_instance_of(Y2Firewall::Firewalld::Api).to receive(:running?).and_return(false) subject.class.imported = false + allow(firewalld).to receive(:read) - allow(firewalld).to receive(:installed?).and_return(true) + allow(firewalld).to receive(:installed?).and_return(installed) allow(subject).to receive(:autoyast).and_return(autoyast) + allow_any_instance_of(Y2Firewall::Firewalld::Api).to receive(:running?).and_return(false) end describe "#summary" do let(:installed) { false } - before do - allow(firewalld).to receive(:installed?).and_return(installed) - end - context "when firewalld is not installed" do it "reports when firewalld is not available" do expect(subject.summary).to match(/not available/) @@ -229,6 +228,90 @@ end end + describe "#change" do + let(:result) { :ok } + let(:already_read) { false } + let(:main_dialog) { instance_double("Dialog::Main", run: result) } + + before do + allow(Y2Firewall::Dialogs::Main).to receive(:new).and_return(main_dialog) + allow(firewalld).to receive(:read?).and_return(already_read) + end + + context "when the configuration is accepted" do + [:next, :finish, :ok, :accept].each do |dialog_result| + let(:result) { dialog_result } + + it "sets autoyast config" do + expect(subject.class).to receive(:ay_config=).with(true) + + subject.change + end + end + end + + context "when the configuration is aborted" do + [:back, :abort, :cancel].each do |dialog_result| + let(:result) { dialog_result } + + it "does not set autoyast config" do + expect(subject.class).to_not receive(:ay_config=) + + subject.change + end + end + end + + context "when firewalld is not installed yet" do + let(:installed) { false } + + it "does not read its configuration" do + expect(firewalld).to_not receive(:read) + + subject.change + end + end + + context "when firewalld is installed" do + context "and its configuration was already read" do + let(:already_read) { true } + + it "does not read it again" do + expect(firewalld).to_not receive(:read) + + subject.change + end + end + + context "but its configuration has not been read" do + let(:example_zone) do + { "name" => "example", "services" => ["http", "https", "ssh"] } + end + + let(:imported_zones) do + { "zones" => [example_zone] } + end + + before do + allow(subject).to receive(:autoyast).and_call_original + subject.import(imported_zones, false) + end + + it "reads the minimal configuration" do + expect(firewalld).to receive(:read).with(minimal: true) + + subject.change + end + + it "keeps the configuration previously imported" do + subject.change + + expect(subject.export["zones"]).to include(hash_including(example_zone)) + end + end + end + end + describe "#write" do let(:arguments) do { "FW_MASQUERADE" => "yes", "enable_firewall" => false, "start_firewall" => false }