Merge pull request #630 from teclator/oes2018_fw

SuSEFirewallClass: IsInstalled does not return true if Pkg is selected
yast · Oct 6, 2017 · a42c114 · a42c114
2 parents 7e83f93 + 7ddbeeb
commit a42c114
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 50 deletions.
diff --git a/library/network/src/lib/network/susefirewall.rb b/library/network/src/lib/network/susefirewall.rb
@@ -60,7 +60,7 @@ def GetStartService
     # @param [Boolean] start_service at Write() process
     # @see #GetStartService()
     def SetStartService(start_service)
-      if !SuSEFirewallIsInstalled()
+      if !SuSEFirewallIsSelectedOrInstalled()
         Builtins.y2warning("Cannot set SetStartService")
         return nil
       end
@@ -97,7 +97,7 @@ def GetEnableService
     #
     # @param	boolean start_service at Write() process
     def SetEnableService(enable_service)
-      if !SuSEFirewallIsInstalled()
+      if !SuSEFirewallIsSelectedOrInstalled()
         Builtins.y2warning("Cannot set SetEnableService")
         return nil
       end
@@ -536,21 +536,31 @@ def IsKnownZone(zone)
     # installation)
     #
     # @return [Boolean] whether the selected firewall backend is installed
-    def SuSEFirewallIsInstalled
-      # Always recheck the status in inst-sys, user/solver might have change
-      # the list of packages selected for installation
-      # bnc#892935: in inst_finish, the package is already installed
+    def SuSEFirewallIsSelectedOrInstalled
+      return true if @needed_packages_installed
+
       if Stage.initial
-        @needed_packages_installed = Pkg.IsSelected(@FIREWALL_PACKAGE) || PackageSystem.Installed(@FIREWALL_PACKAGE)
-        log.info "Selected for installation/installed -> #{@needed_packages_installed}"
-      elsif @needed_packages_installed.nil?
-        if Mode.normal
-          @needed_packages_installed = PackageSystem.CheckAndInstallPackages([@FIREWALL_PACKAGE])
-          log.info "CheckAndInstallPackages -> #{@needed_packages_installed}"
-        else
-          @needed_packages_installed = PackageSystem.Installed(@FIREWALL_PACKAGE)
-          log.info "Installed -> #{@needed_packages_installed}"
-        end
+        packages_selected = Pkg.IsSelected(@FIREWALL_PACKAGE)
+        log.info "Selected for installation -> #{packages_selected}"
+
+        return true if packages_selected
+      end
+
+      SuSEFirewallIsInstalled()
+    end
+
+    # Returns whether all needed packages are installed
+    #
+    # @return [Boolean] whether the selected firewall backend is installed
+    def SuSEFirewallIsInstalled
+      return true if @needed_packages_installed
+
+      if Mode.normal
+        @needed_packages_installed = PackageSystem.CheckAndInstallPackages([@FIREWALL_PACKAGE])
+        log.info "CheckAndInstallPackages -> #{@needed_packages_installed}"
+      else
+        @needed_packages_installed = PackageSystem.Installed(@FIREWALL_PACKAGE)
+        log.info "Installed -> #{@needed_packages_installed}"
       end
 
       @needed_packages_installed

diff --git a/library/network/src/lib/network/susefirewall2.rb b/library/network/src/lib/network/susefirewall2.rb
@@ -1526,7 +1526,7 @@ def Read
       end
 
       # bnc #887406
-      if !FileUtils.Exists(CONFIG_FILE) || !SuSEFirewallIsInstalled()
+      if !FileUtils.Exists(CONFIG_FILE) || !SuSEFirewallIsSelectedOrInstalled()
         log.warn "No firewall config -> firewall can't be read"
         FillUpEmptyConfig()
         return false
@@ -2757,7 +2757,6 @@ def full_init_on_boot(new_state)
     publish function: :AddServiceSupportIntoZone, type: "void (string, string)", private: true
     publish variable: :check_and_install_package, type: "boolean", private: true
     publish function: :SetInstallPackagesIfMissing, type: "void (boolean)"
-    publish variable: :needed_packages_installed, type: "boolean", private: true
     publish function: :SuSEFirewallIsInstalled, type: "boolean ()"
     publish variable: :fw_service_can_be_configured, type: "boolean", private: true
     publish function: :GetModified, type: "boolean ()"

diff --git a/library/network/test/susefirewall_test.rb b/library/network/test/susefirewall_test.rb
@@ -18,65 +18,82 @@ def reset_SuSEFirewallIsInstalled_cache
 
 describe FakeFirewall do
 
-  describe "#SuSEFirewallIsInstalled" do
-    before(:each) do
-      reset_SuSEFirewallIsInstalled_cache
-    end
-
+  describe "#SuSEFirewallIsSelectedOrInstalled" do
     context "while in inst-sys" do
       it "returns whether SuSEfirewall2 is selected for installation or already installed" do
         expect(Yast::Stage).to receive(:stage).and_return("initial").at_least(:once)
 
         # Value is not cached
         expect(Yast::Pkg).to receive(:IsSelected).and_return(true, false, false).exactly(3).times
         # Fallback: if not selected, checks whether the package is installed
-        expect(Yast::PackageSystem).to receive(:Installed).and_return(false, true).twice
+        expect(subject).to receive(:SuSEFirewallIsInstalled).and_return(false, true).twice
 
         # Selected
-        expect(subject.SuSEFirewallIsInstalled).to eq(true)
+        expect(subject.SuSEFirewallIsSelectedOrInstalled).to eq(true)
         # Not selected and not installed
-        expect(subject.SuSEFirewallIsInstalled).to eq(false)
+        expect(subject.SuSEFirewallIsSelectedOrInstalled).to eq(false)
         # Not selected, but installed
+        expect(subject.SuSEFirewallIsSelectedOrInstalled).to eq(true)
+      end
+    end
+
+    context "while on a running system or AutoYast config" do
+      it "returns whether SuSEfirewall2 was or could have been installed" do
+        expect(Yast::Stage).to receive(:stage).and_return("normal").twice
+
+        expect(subject).to receive(:SuSEFirewallIsInstalled).and_return(false, true).twice
+
+        expect(subject.SuSEFirewallIsSelectedOrInstalled).to eq(false)
+        expect(subject.SuSEFirewallIsSelectedOrInstalled).to eq(true)
+      end
+    end
+  end
+
+  describe "#SuSEFirewallIsInstalled" do
+    before(:each) do
+      reset_SuSEFirewallIsInstalled_cache
+    end
+
+    context "while in inst-sys" do
+      it "returns whether SuSEfirewall2 is installed or not" do
+        expect(Yast::Mode).to receive(:mode).and_return("installation").twice
+
+        # Checks whether the package is installed
+        expect(Yast::PackageSystem).to receive(:Installed).and_return(false, true).twice
+
+        expect(subject.SuSEFirewallIsInstalled).to eq(false)
+        expect(subject.SuSEFirewallIsInstalled).to eq(true)
+        # Value is cached if true
         expect(subject.SuSEFirewallIsInstalled).to eq(true)
       end
     end
 
     context "while on a running system (normal configuration)" do
       it "returns whether SuSEfirewall2 was or could have been installed" do
-        expect(Yast::Stage).to receive(:stage).and_return("normal").at_least(:once)
         expect(Yast::Mode).to receive(:mode).and_return("normal").at_least(:once)
 
         # Value is cached
         expect(Yast::PackageSystem).to receive(:CheckAndInstallPackages).and_return(true, false).twice
 
         expect(subject.SuSEFirewallIsInstalled).to eq(true)
-        expect(subject.SuSEFirewallIsInstalled).to eq(true)
+        # Value is cached if true
         expect(subject.SuSEFirewallIsInstalled).to eq(true)
 
         reset_SuSEFirewallIsInstalled_cache
 
         expect(subject.SuSEFirewallIsInstalled).to eq(false)
-        expect(subject.SuSEFirewallIsInstalled).to eq(false)
-        expect(subject.SuSEFirewallIsInstalled).to eq(false)
       end
     end
 
     context "while in AutoYast config" do
       it "returns whether SuSEfirewall2 is installed" do
-        expect(Yast::Stage).to receive(:stage).and_return("normal").at_least(:once)
         expect(Yast::Mode).to receive(:mode).and_return("autoinst_config").at_least(:once)
 
-        # Value is cached
         expect(Yast::PackageSystem).to receive(:Installed).and_return(false, true).twice
 
         expect(subject.SuSEFirewallIsInstalled).to eq(false)
-        expect(subject.SuSEFirewallIsInstalled).to eq(false)
-        expect(subject.SuSEFirewallIsInstalled).to eq(false)
-
-        reset_SuSEFirewallIsInstalled_cache
-
-        expect(subject.SuSEFirewallIsInstalled).to eq(true)
         expect(subject.SuSEFirewallIsInstalled).to eq(true)
+        # Value is cached if true
         expect(subject.SuSEFirewallIsInstalled).to eq(true)
       end
     end

diff --git a/library/network/test/susefirewalld_test.rb b/library/network/test/susefirewalld_test.rb
@@ -34,26 +34,19 @@ def reset_FirewallDIsInstalled_cache
     end
 
     context "while in inst-sys" do
-      it "returns whether FirewallD is selected for installation or already installed" do
-        expect(Yast::Stage).to receive(:stage).and_return("initial").at_least(:once)
+      it "returns whether FirewallD is installed or not" do
+        expect(Yast::Mode).to receive(:mode).and_return("installation").at_least(:twice)
 
-        # Value is not cached
-        expect(Yast::Pkg).to receive(:IsSelected).and_return(true, false, false).exactly(3).times
-        # Fallback: if not selected, checks whether the package is installed
+        # Checks whether the package is installed
         expect(Yast::PackageSystem).to receive(:Installed).and_return(false, true).twice
 
-        # Selected
-        expect(subject.SuSEFirewallIsInstalled).to eq(true)
-        # Not selected and not installed
         expect(subject.SuSEFirewallIsInstalled).to eq(false)
-        # Not selected, but installed
         expect(subject.SuSEFirewallIsInstalled).to eq(true)
       end
     end
 
     context "while on a running system (normal configuration)" do
       it "returns whether FirewallD was or could have been installed" do
-        expect(Yast::Stage).to receive(:stage).and_return("normal").at_least(:once)
         expect(Yast::Mode).to receive(:mode).and_return("normal").at_least(:once)
 
         expect(Yast::PackageSystem).to receive(:CheckAndInstallPackages).and_return(true, false)
@@ -70,7 +63,6 @@ def reset_FirewallDIsInstalled_cache
 
     context "while in AutoYast config" do
       it "returns whether FirewallD is installed" do
-        expect(Yast::Stage).to receive(:stage).and_return("normal").at_least(:once)
         expect(Yast::Mode).to receive(:mode).and_return("autoinst_config").at_least(:once)
 
         expect(Yast::PackageSystem).to receive(:Installed).and_return(false, true)

diff --git a/package/yast2.changes b/package/yast2.changes
@@ -1,3 +1,16 @@
+-------------------------------------------------------------------
+Fri Oct  6 11:02:12 UTC 2017 - knut.anderssen@suse.com
+
+- Adapted SuSEFirewallIsInstalled() to return true only when the
+  package is already installed or checked and installed in normal
+  mode.
+- Added SuSEFirewallIsSelectedOrInstalled() which behaves as the
+  old SuSEFirewallIsInstalled() method.
+  (bnc#1037214)
+- Adapted calls to use SuSEFirewallIsSelectedOrInstalled() when
+  the methods can be called even with just Pkg selection.
+- 3.1.217
+
 -------------------------------------------------------------------
 Tue May 30 12:05:21 UTC 2017 - gsouza@suse.com
 

diff --git a/package/yast2.spec b/package/yast2.spec
@@ -17,7 +17,7 @@
 
 
 Name:           yast2
-Version:        3.1.216
+Version:        3.1.217
 Release:        0
 Summary:        YaST2 - Main Package
 License:        GPL-2.0