Merge pull request #826 from yast/return_firewall_widget

adapt firewall zone widget

src/lib/y2network/interface_config_builder.rb

@@ -19,6 +19,8 @@
 require "yast"
 
 require "y2network/hwinfo"
+require "y2firewall/firewalld"
+require "y2firewall/firewalld/interface"
 
 Yast.import "LanItems"
 Yast.import "NetworkInterfaces"
@@ -53,10 +55,20 @@ def [](key)
     end
 
     def save
-      return if driver.empty?
+      if !driver.empty?
+        Yast::LanItems.setDriver(driver)
+        Yast::LanItems.driver_options[driver] = driver_options
+      end
+
+      # create new instance as name can change
+      firewall_interface = Y2Firewall::Firewalld::Interface.new(name)
+      if Y2Firewall::Firewalld.instance.installed?
+        Yast::LanItems.firewall_zone = firewall_zone
+        # TODO: should change only if different, but maybe firewall_interface responsibility?
+        firewall_interface.zone = firewall_zone if !firewall_interface.zone || firewall_zone != firewall_interface.zone.name
+      end
 
-      Yast::LanItems.setDriver(driver)
-      Yast::LanItems.driver_options[driver] = driver_options
+      nil
     end
 
     # how many device names is proposed
@@ -84,6 +96,18 @@ def kernel_modules
       Yast::LanItems.GetItemModules("")
     end
 
+    def firewall_zone
+      return @firewall_zone if @firewall_zone
+
+      # TODO: handle renaming
+      firewall_interface = Y2Firewall::Firewalld::Interface.new(name)
+      @firewall_zone = firewall_interface.zone && firewall_interface.zone.name
+    end
+
+    def firewall_zone=(value)
+      @firewall_zone = value
+    end
+
     def driver
       @driver ||= Yast::Ops.get_string(Yast::LanItems.getCurrentItem, ["udev", "driver"], "")
     end

src/lib/y2network/widgets/firewall_zone.rb

@@ -34,11 +34,11 @@ class FirewallZone < ::CWM::CustomWidget
 
       # Constructor
       #
-      # @param name [String]
-      def initialize(name)
+      # @param builder [Y2Network::InterfaceConfigBuilder]
+      def initialize(builder)
         textdomain "network"
-        @value = nil
-        @interface = Y2Firewall::Firewalld::Interface.new(name)
+        @builder = builder
+        @interface = Y2Firewall::Firewalld::Interface.new(builder.name)
       end
 
       # @see CWM::AbstractWidget
@@ -53,7 +53,7 @@ def init
         return unless installed?
 
         populate_select(firewall_zones)
-        self.value = @value
+        select_zone(@builder.firewall_zone) if installed?
       end
 
       # @see CWM::AbstractWidget
@@ -64,24 +64,11 @@ def contents
         Left(zones_widget)
       end
 
-      # Stores the given name and when it is enabled to configure the
-      # interface ZONE through the ifcfg file. It also selects the given zone
-      # in the select list
-      #
-      # @see CWM::AbstractWidget
-      # @param name [String,nil] zone name
-      def value=(name)
-        @value = name
-        return unless installed?
-        select_zone(name)
-      end
-
       # It returns the current ZONE selection or nil in case of not enabled
       # the management through the ifcfg files.
       #
       # @return [String, nil] current zone or nil when not managed
       def value
-        return @value unless Yast::UI.WidgetExists(Id(:zones))
         selected_zone
       end
 
@@ -90,18 +77,7 @@ def value
       # @see CWM::AbstractWidget
       # @return [String, nil]
       def store
-        @value = value
-      end
-
-      # Stores the selected zone permanently when it has change and it is
-      # enabled to be managed through the ifcfg files
-      #
-      # @return [String, nil] the current zone selection
-      def store_permanent
-        return @value unless installed?
-
-        @interface.zone = @value if zone_changed?
-        @value
+        @builder.firewall_zone = value
       end
 
       # @see CWM::AbstractWidget
@@ -119,13 +95,6 @@ def help
 
     private
 
-      # Return whether the permanent ZONE match or not the selected one.
-      #
-      # @return [Boolean]
-      def zone_changed?
-        @value && (current_zone.to_s != @value)
-      end
-
       # @return [String]
       def default_label
         # TRANSLATORS: List item describing an assigment of the interface
@@ -139,13 +108,6 @@ def no_zone_label
         _("Do not assign ZONE")
       end
 
-      # Current {Y2Firewall::Firewalld::Interface} name
-      # @return [String, nil]
-      def current_zone
-        return unless @interface.zone
-        @interface.zone.name
-      end
-
       # @return [Yast::Term] zones select list
       def zones_widget
         ComboBox(Id(:zones), Opt(:notify, :hstretch), label)

src/lib/y2network/widgets/general_tab.rb

@@ -40,7 +40,7 @@ def contents
                   HBox(Startmode.new(@settings, ifplugd_widget), ifplugd_widget, HStretch())
                 ),
                 VSpacing(0.4),
-                # TODO: Frame(_("Firewall Zone"), HBox("FWZONE", HStretch())),
+                Frame(_("Firewall Zone"), HBox(FirewallZone.new(@settings), HStretch())),
                 VSpacing(0.4),
                 type == "ib" ? HBox(IPoIBMode.new(@settings)) : Empty(),
                 type == "ib" ? VSpacing(0.4) : Empty(),

test/y2network/widgets/firewall_zone_test.rb

@@ -2,11 +2,19 @@
 
 require_relative "../../test_helper.rb"
 require "y2network/widgets/firewall_zone"
+require "y2network/interface_config_builder"
 
 require "cwm/rspec"
 
 describe Y2Network::Widgets::FirewallZone do
-  let(:subject) { described_class.new("eth0") }
+  let(:builder) do
+    res = Y2Network::InterfaceConfigBuilder.new
+    res.type = "eth"
+    res.name = "eth0"
+    res
+  end
+  subject { described_class.new(builder) }
+
   let(:firewalld) { Y2Firewall::Firewalld.instance }
   let(:firewall_zones) { [["", "Default"], ["custom", "custom"]] }
   let(:installed?) { true }
@@ -24,8 +32,8 @@
       subject.init
     end
 
-    it "selects the current zone from the list if it was cached" do
-      subject.value = "custom"
+    it "selects the current zone" do
+      builder.firewall_zone = "custom"
       expect(subject).to receive(:select_zone).with("custom")
       subject.init
     end
@@ -40,70 +48,13 @@
       expect(subject).to receive(:selected_zone).and_return("")
       expect(subject.value).to eql("")
     end
-
-    context "when the select zone widget does not exist" do
-      before do
-        allow(Yast::UI).to receive(:WidgetExists).and_return(false)
-      end
-
-      it "returns the cached value" do
-        expect(subject.value).to eql(nil)
-        subject.value = "external"
-        expect(subject.value).to eql("external")
-      end
-    end
   end
 
   describe "#store" do
-    before do
-      allow(Yast::UI).to receive(:WidgetExists).and_return(true, false)
-    end
-
-    it "caches the current value" do
-      expect(subject).to receive(:selected_zone).and_return("external")
+    it "stores value to builder" do
+      allow(subject).to receive(:selected_zone).and_return("external")
       subject.store
-      expect(subject).to_not receive(:selected_zone?)
-      expect(subject.store).to eql("external")
-    end
-  end
-
-  describe "#store_permanent" do
-    before do
-      subject.value = "custom"
-    end
-
-    context "when firewalld is not installed" do
-      let(:installed?) { false }
-
-      it "returns the cached value" do
-        expect(subject.store_permanent).to eql("custom")
-      end
-    end
-
-    context "when firewalld is installed" do
-      context "but the firewall zone will not be managed by the ifcfg file" do
-        let(:managed?) { false }
-
-        it "returns the cached value" do
-          expect(subject.store_permanent).to eql("custom")
-        end
-      end
-
-      context "and the cached value is not equal to the firewalld interface zone" do
-        it "modifies the interface permanent ZONE" do
-          allow(subject).to receive(:current_zone).and_return("external")
-          expect_any_instance_of(Y2Firewall::Firewalld::Interface).to receive(:zone=).with("custom")
-          subject.store_permanent
-        end
-      end
-
-      context "and the cached value is the same than the firewalld interface zone" do
-        it "does not touch the interface permanent ZONE" do
-          allow(subject).to receive(:current_zone).and_return("custom")
-          expect_any_instance_of(Y2Firewall::Firewalld::Interface).to_not receive(:zone=)
-          subject.store_permanent
-        end
-      end
+      expect(builder.firewall_zone).to eq "external"
     end
   end
 end

test/y2network/widgets/general_tab_test.rb

@@ -24,7 +24,14 @@
 require "y2network/interface_config_builder"
 
 describe Y2Network::Widgets::GeneralTab do
-  subject { described_class.new(Y2Network::InterfaceConfigBuilder.new) }
+  let(:builder) do
+    res = Y2Network::InterfaceConfigBuilder.new
+    res.type = "eth"
+    res.name = "eth0"
+    res
+  end
+
+  subject { described_class.new(builder) }
 
   include_examples "CWM::Tab"
 end