Read-write ports, via PortsForProtocols

src/lib/y2firewall/widgets/allowed_services.rb

@@ -31,6 +31,7 @@ class AllowedServices < ::CWM::CustomWidget
       #
       # @param zone [Y2Firewall::Firewalld::Zone] Zone
       def initialize(zone)
+        textdomain "firewall"
         @zone = zone
         self.widget_id = "allowed_services"
         @available_svcs_table = ServicesTable.new

src/lib/y2firewall/widgets/pages/interfaces.rb

@@ -73,6 +73,7 @@ def contents
         class ZoneBox < CWM::SelectionBox
           # @param interface [Hash<String,String>] "id", "name" and "zone"
           def initialize(interface)
+            textdomain "firewall"
             @interface = interface
             @zones = Y2Firewall::Firewalld.instance.zones
           end

src/lib/y2firewall/widgets/pages/zone.rb

@@ -20,7 +20,9 @@
 # ------------------------------------------------------------------------------
 
 require "yast"
+require "cwm/common_widgets"
 require "cwm/page"
+require "cwm/table"
 require "cwm/tabs"
 require "y2firewall/widgets/allowed_services"
 
@@ -72,60 +74,96 @@ def label
 
         def contents
           VBox(
-            # TRANSLATORS: TCP is the Transmission Control Protocol
-            PortsForProtocol.new(@zone, _("TCP Ports"), :tcp),
-            # TRANSLATORS: UDP is the User Datagram Protocol
-            PortsForProtocol.new(@zone, _("UDP Ports"), :udp),
-            # TRANSLATORS: SCTP is the Stream Control Transmission Protocol
-            PortsForProtocol.new(@zone, _("SCTP Ports"), :sctp),
-            # TRANSLATORS: DCCP is the Datagram Congestion Control Protocol
-            PortsForProtocol.new(@zone, _("DCCP Ports"), :dccp),
+            PortsForProtocols.new(@zone),
             VStretch()
           )
         end
 
-        def help
-          "FIXME: ports or port ranges, separated by spaces and/or commas <br>" \
-          "a port is an integer <br>" \
-          "a port range is port-dash-port (with no spaces)"
-        end
-
-        def init
-          log.info "INIT #{widget_id}"
-        end
+        # A group of InputFields to specify the open TCP, UDP, SCTP and DCCP ports.
+        class PortsForProtocols < CWM::CustomWidget
+          extend Yast::I18n
 
-        def store
-          log.info "STORE #{widget_id}"
-        end
+          PROTOCOLS = {
+            # TRANSLATORS: TCP is the Transmission Control Protocol
+            tcp:  N_("TCP Ports"),
+            # TRANSLATORS: UDP is the User Datagram Protocol
+            udp:  N_("UDP Ports"),
+            # TRANSLATORS: SCTP is the Stream Control Transmission Protocol
+            sctp: N_("SCTP Ports"),
+            # TRANSLATORS: DCCP is the Datagram Congestion Control Protocol
+            dccp: N_("DCCP Ports")
+          }.freeze
 
-        # FIXME: separate objects like this do not work well with the
-        # single zone.ports attribute mixing all protos. Rather make a
-        # CustomWidget that handles it all
-        class PortsForProtocol < CWM::InputField
-          # @param proto [:tcp,:udp,:sctp,:dccp]
-          def initialize(zone, label, proto)
+          def initialize(zone)
+            textdomain "firewall"
             @zone = zone
-            @proto = proto
-            @label = label
-            self.widget_id = "#{proto}_ports"
           end
 
-          attr_reader :label
+          def contents
+            fields = PROTOCOLS.map do |sym, label|
+              InputField(Id(sym), Opt(:hstretch), _(label))
+            end
+            VBox(* fields)
+          end
+
+          def help
+            "FIXME: ports or port ranges, separated by spaces and/or commas <br>" \
+            "a port is an integer <br>" \
+            "a port range is port-dash-port (with no spaces)"
+          end
 
           def init
-            log.info "INIT #{widget_id}"
-            # FIXME: factor out and test
-            self.value = @zone.ports
-                              .map { |p| p.split("/") }
-                              .find_all { |_port, proto| proto == @proto.to_s }
-                              .map { |port, _proto| port }
-                              .join(", ")
+            by_proto = ports_from_array(@zone.ports)
+            PROTOCOLS.each do |sym, _label|
+              Yast::UI.ChangeWidget(Id(sym), :Value, by_proto.fetch(sym, []).join(", "))
+            end
           end
 
+          # FIXME: validation, cleanup, error reporting
           def store
-            # FIXME: do modify immediately?
-            # DUH, clumsy
-            log.info "STORE #{widget_id}"
+            by_proto = PROTOCOLS.map do |sym, _label|
+              line = Yast::UI.QueryWidget(Id(sym), :Value)
+              [sym, items_from_ui(line)]
+            end
+            @zone.ports = ports_to_array(by_proto.to_h)
+          end
+
+        private
+
+          def items_from_ui(s)
+            # the separator is at least one comma or space, surrounded by optional spaces
+            s.split(/ *[, ] */)
+          end
+
+          # @param hash [Hash{Symbol => Array<String>}] ports specification
+          #   categorized by protocol
+          # @return [Array] ports specification
+          #   as array for {Y2Firewall::Firewalld::Zone#ports}
+          # @example
+          #   h = { tcp: ["55555-55666", "44444"], udp: ["33333"] }
+          #   a = ["55555-55666/tcp", "44444/tcp", "33333/udp"]
+          #   ports_to_array(h) # => a
+          def ports_to_array(hash)
+            hash.map { |sym, ports| ports.map { |p| "#{p}/#{sym}" } }.flatten
+          end
+
+          # @param a [Array] ports specification
+          #   as array from {Y2Firewall::Firewalld::Zone#ports}
+          # @return [Hash{Symbol => Array<String>}] ports specification
+          #   categorized by protocol
+          # @example
+          #   a = ["55555-55666/tcp", "44444/tcp", "33333/udp"]
+          #   h = { tcp: ["55555-55666", "44444"], udp: ["33333"] }
+          #   ports_from_array(a) # => h
+          def ports_from_array(a)
+            a
+              .map { |p| p.split("/") }
+              .each_with_object({}) do |i, acc|
+                ports, proto = *i
+                proto = proto.to_sym
+                acc[proto] ||= []
+                acc[proto] << ports
+              end
           end
         end
       end
@@ -176,5 +214,5 @@ def init
         end
       end
     end
- end
+  end
 end

src/lib/y2firewall/widgets/pages/zones.rb

@@ -27,6 +27,8 @@
 module Y2Firewall
   module Widgets
     module Pages
+      # A page for firewall zones:
+      #   contains {ZonesTable}, has {Zone} as subpages.
       class Zones < CWM::Page
         # Constructor
         #

src/lib/y2firewall/widgets/services_table.rb

@@ -23,6 +23,7 @@
 
 module Y2Firewall
   module Widgets
+    # A table with all firewall services.
     class ServicesTable < ::CWM::Table
       # @!attribute [r] services
       #   @return [Array<String>] Services to be displayed

src/lib/y2firewall/widgets/zones_table.rb

@@ -21,6 +21,7 @@
 
 module Y2Firewall
   module Widgets
+    # A table with all {Y2Firewall::Firewalld::Zone}s.
     class ZonesTable < ::CWM::Table
       # @!attribute [r] zone
       #   @return [Array<Y2Firewall::Firewalld::Zone>] Zones

test/lib/y2firewall/widgets/pages/zone_test.rb

@@ -0,0 +1,61 @@
+#!/usr/bin/env rspec
+
+# ------------------------------------------------------------------------------
+# Copyright (c) 2018 SUSE LLC
+#
+#
+# This program is free software; you can redistribute it and/or modify it under
+# the terms of version 2 of the GNU General Public License as published by the
+# Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, contact SUSE.
+#
+# To contact SUSE about this file by physical or electronic mail, you may find
+# current contact information at www.suse.com.
+# ------------------------------------------------------------------------------
+
+require_relative "../../../../test_helper.rb"
+require "cwm/rspec"
+require "y2firewall/widgets/pages/zone"
+
+describe Y2Firewall::Widgets::Pages::PortsTab::PortsForProtocols do
+  subject { described_class.new(double("fake zone")) }
+
+  describe "#items_from_ui" do
+    let(:expected) do
+      ["11/tcp", "12/udp", "13/udp"]
+    end
+
+    it "parses comma separated items" do
+      expect(subject.send(:items_from_ui, "11/tcp,12/udp,13/udp")).to eq(expected)
+    end
+
+    it "parses space separated items" do
+      expect(subject.send(:items_from_ui, "11/tcp 12/udp 13/udp")).to eq(expected)
+    end
+
+    it "parses clumsily separated items" do
+      expect(subject.send(:items_from_ui, "11/tcp  12/udp , 13/udp")).to eq(expected)
+    end
+  end
+
+  let(:ports_h1) { { tcp: ["55555-55666", "44444"], udp: ["33333"] } }
+  let(:ports_a1) { ["55555-55666/tcp", "44444/tcp", "33333/udp"] }
+
+  describe "#ports_from_array" do
+    it "parses a regular case" do
+      expect(subject.send(:ports_from_array, ports_a1)).to eq(ports_h1)
+    end
+  end
+
+  describe "#ports_to_array" do
+    it "formats a regular case" do
+      expect(subject.send(:ports_to_array, ports_h1)).to eq(ports_a1)
+    end
+  end
+end