Merge d6e81a5 into f94d4a7

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
 

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

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

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

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 }