Merge pull request #819 from yast/merge_SLE-15-GA

Merge SLE-15-GA into master

library/network/src/Makefile.am

@@ -5,7 +5,6 @@ module_DATA = \
   modules/SuSEFirewall.rb \
   modules/NetworkConfig.rb \
   modules/Internet.rb \
-  modules/SuSEFirewallProposal.rb \
   modules/PortRanges.rb \
   modules/NetworkInterfaces.rb \
   modules/CWMFirewallInterfaces.rb \
@@ -45,7 +44,8 @@ yfwdlib_DATA = \
   lib/y2firewall/firewalld/relations.rb \
   lib/y2firewall/firewalld/service.rb \
   lib/y2firewall/firewalld/zone.rb \
-  lib/y2firewall/firewalld/zone_parser.rb
+  lib/y2firewall/firewalld/zone_reader.rb \
+  lib/y2firewall/firewalld/service_reader.rb
 
 yfwdapilibdir = @ylibdir@/y2firewall/firewalld/api/
 yfwdapilib_DATA = \

library/network/src/lib/y2firewall/firewalld.rb

@@ -23,9 +23,11 @@
 # ***************************************************************************
 
 require "y2firewall/firewalld/api"
+require "y2firewall/firewalld/relations"
 require "y2firewall/firewalld/service"
 require "y2firewall/firewalld/zone"
-require "y2firewall/firewalld/zone_parser"
+require "y2firewall/firewalld/zone_reader"
+require "y2firewall/firewalld/service_reader"
 require "singleton"
 
 Yast.import "PackageSystem"
@@ -49,29 +51,30 @@ class Firewalld
     include Singleton
     include Yast::Logger
     extend Forwardable
+    extend Relations
 
-    # @return Y2Firewall::Firewalld::Api instance
-    attr_accessor :api
-    # @return [Array <Y2Firewall::Firewalld::Zone>] firewalld zones
+    attr_writer :api
+    # @return [Array<Y2Firewall::Firewalld::Zone>] firewalld zones
     attr_accessor :zones
-    # @return [Array <Y2Firewall::Firewalld::Service>] firewalld services. To
-    # avoid performance problems it is empty by default and the services are
-    # added when needed by the find_service method.
+    # @return [Array<String>] current zone names.
+    attr_accessor :current_zone_names
+    # @return [Array<String>] current service names.
+    attr_accessor :current_service_names
+    # @return [Array<Y2Firewall::Firewalld::Service>] firewalld services. To
+    #   avoid performance problems it is empty by default and the services are
+    #   added when needed by the find_service method.
     attr_accessor :services
-    # @return [String] Type of log denied packets (reject & drop rules).
-    # Possible values are: all, unicast, broadcast, multicast and off
-    attr_accessor :log_denied_packets
-    # @return [String] firewalld default zone name
-    attr_accessor :default_zone
 
     PACKAGE = "firewalld".freeze
     SERVICE = "firewalld".freeze
 
-    def_delegators :@api, :enable!, :disable!, :reload, :running?
+    def_delegators :api, :enable!, :disable!, :reload, :running?
+    has_attributes :log_denied_packets, :default_zone, cache: true
 
     # Constructor
     def initialize
-      @api = Api.new
+      @current_zone_names = []
+      @current_service_names = []
       @zones = []
       @services = []
       @read = false
@@ -83,9 +86,10 @@ def initialize
     # @return [Boolean] true
     def read
       return false unless installed?
-      @zones = ZoneParser.new(api.zones, api.list_all_zones).parse
-      @log_denied_packets = api.log_denied_packets
-      @default_zone       = api.default_zone
+      @zones = zone_reader.read
+      @current_zone_names = api.zones
+      @current_service_names = api.services
+      read_attributes
       # The list of services is not read or initialized because takes time and
       # affects to the performance and also the services are rarely touched.
       @read = true
@@ -95,7 +99,7 @@ def read
     #
     # @param name [String] the zone name
     # @return [Y2Firewall::Firewalld::Zone, nil] the firewalld zone with the
-    # given name
+    #   given name
     def find_zone(name)
       zones.find { |z| z.name == name }
     end
@@ -104,7 +108,7 @@ def find_zone(name)
     #
     # @param name [String] the service name
     # @return [Y2Firewall::Firewalld::Service] the firewalld service with
-    # the given name
+    #   the given name
     def find_service(name)
       services.find { |s| s.name == name } || read_service(name)
     end
@@ -116,22 +120,19 @@ def find_service(name)
     # @return [Y2Firewall::Firewalld::Service] the recently added service
     def read_service(name)
       raise(Service::NotFound, name) unless installed?
-      service = Y2Firewall::Firewalld::Service.new(name: name)
-      raise(Service::NotFound, name) if !service.supported?
-
-      service.read
-      @services << service
+      service = ServiceReader.new.read(name)
+      services << service
       service
     end
 
     # Return true if the logging config or any of the zones where modified
     # since read
     #
     # @return [Boolean] true if the config was modified; false otherwise
-    def modified?
-      default_zone != api.default_zone ||
-        log_denied_packets != api.log_denied_packets ||
-        zones.any?(&:modified?)
+    def modified?(*item)
+      return modified.include?(item.first) if !item.empty?
+
+      !modified.empty? || zones.any?(&:modified?)
     end
 
     # Apply the changes to the modified zones and sets the logging option
@@ -143,12 +144,18 @@ def write
     def write_only
       return false unless installed?
       read unless read?
-      zones.each { |z| z.apply_changes! if z.modified? }
-      api.log_denied_packets = log_denied_packets if log_denied_packets != api.log_denied_packets
-      api.default_zone       = default_zone if default_zone != api.default_zone
+      apply_zones_changes!
+      apply_attributes_changes!
+      untouched!
       true
     end
 
+    # Apply the changes done in each of the modified zones. It will create or
+    # delete all the new or removed zones depending on each case.
+    def apply_zones_changes!
+      zones.select(&:modified?).each(&:apply_changes!)
+    end
+
     # Return a map with current firewalld settings.
     #
     # @return [Hash] dump firewalld settings
@@ -211,9 +218,23 @@ def start
     # Return whether the configuration has been read
     #
     # @return [Boolean] true if the configuration has been read; false
-    # otherwise
+    #   otherwise
     def read?
       @read
     end
+
+    # Convenience method to instantiate the firewalld API
+    def api
+      @api ||= Api.new
+    end
+
+  private
+
+    # Convenience method to instantiate a new zone reader
+    #
+    # @return [ZoneReader]
+    def zone_reader
+      ZoneReader.new(api.zones, api.list_all_zones(verbose: true))
+    end
   end
 end

library/network/src/lib/y2firewall/firewalld/api.rb

@@ -52,20 +52,22 @@ class Api
       COMMAND = { offline: "firewall-offline-cmd", running: "firewall-cmd" }.freeze
       # FIXME: Do not like to define twice
       PACKAGE = "firewalld".freeze
+      # Modification commands were applied successfully
+      SUCCESS = "success".freeze
 
       # Determines the mode in which firewalld is running and as consequence the
       # command to be used.
       attr_accessor :mode
 
       # Constructor
+      #
+      # @param mode [Symbol, nil] defines which cmdline should be used if the
+      #   running or the offline one. Possible values are: :offline, :running
+      # @param permanent [Boolean] whether the configuration should be written
+      #   permanently or in runtime when firewalld is running.
       def initialize(mode: nil, permanent: true)
-        @mode =
-          if mode == :running || running?
-            :running
-          else
-            :offline
-          end
-        @permanent = permanent
+        @mode = mode || (running? ? :running : :offline)
+        @permanent = !offline? && permanent
       end
 
       # Whether the mode is :offline or not
@@ -95,10 +97,12 @@ def running?
         state == "running"
       end
 
+      # Enables the firewalld service
       def enable!
         offline? ? run_command("--enable") : Yast::Service.Enable("firewalld")
       end
 
+      # Disables the firewalld service
       def disable!
         offline? ? run_command("--disable") : Yast::Service.Disable("firewalld")
       end
@@ -120,7 +124,7 @@ def state
       end
 
       # Return the default zone
-      #
+
       # @return [String] default zone
       def default_zone
         string_command("--get-default-zone")
@@ -129,44 +133,41 @@ def default_zone
       # Set the default zone
       #
       # @param zone [String] The firewall zone
-      # @return [String] default zone
-      def default_zone=(zone)
-        run_command("--set-default-zone=#{zone}")
+      def modify_default_zone(zone)
+        modify_command("--set-default-zone=#{zone}")
       end
 
       # Do a reload of the firewall if running. In offline mode just return
       # true as a reload is not needed to apply the changes.
       #
-      # @return [Boolean] The firewalld reload result (exit code)
+      # @return [Boolean] true if the firewall was reloaded successfully
       def reload
         return true if offline?
-        run_command("--reload")
+        modify_command("--reload")
       end
 
       # Do a complete reload of the firewall if running. In offline mode just
       # return true as a reload is not needed to apply the changes
       #
-      # @return [Boolean] The firewalld complete-reload result (exit code)
+      # @return [Boolean] true if the firewall was reloaded completely with success
       def complete_reload
         return true if offline?
-        run_command("--complete-reload")
+        modify_command("--complete-reload")
       end
 
       ### Logging ###
 
       # @param kind [String] Denied packets to log. Possible values are:
-      # all, unicast, broadcast, multicast and off
+      #   all, unicast, broadcast, multicast and off
       # @return [Boolean] True if desired packet type is being logged when denied
       def log_denied_packets?(kind)
-        string_command("--get-log-denied").strip == kind ? true : false
+        string_command("--get-log-denied").strip == kind
       end
 
       # @param kind [String] Denied packets to log. Possible values are:
-      # all, unicast, broadcast, multicast and off
-      # @return [Boolean] True if desired packet type was set to being logged
-      # when denied
-      def log_denied_packets=(kind)
-        run_command("--set-log-denied=#{kind}")
+      #   all, unicast, broadcast, multicast and off
+      def modify_log_denied_packets(kind)
+        modify_command("--set-log-denied=#{kind}")
       end
 
       # @return [String] packet type which is being logged when denied
@@ -187,13 +188,12 @@ def command
       # @see #command
       # @see Yast::Execute
       # @param args [Array<String>] list of command optional arguments
-      # @param permanent [Boolean] if true it adds the --permanent option the
-      # command to be executed
+      # @param permanent [Boolean] if true and firewalld is running it
+      #   operates over the permanent configuration
       # @param allowed_exitstatus [Fixnum, .include?, nil] allowed exit codes
-      # which do not cause an exception.
-      # command to be executed
+      #   which do not cause an exception.
       def run_command(*args, permanent: false, allowed_exitstatus: nil)
-        arguments = permanent ? ["--permanent"] : []
+        arguments = !offline? && permanent ? ["--permanent"] : []
         arguments.concat(args)
         log.info("Executing #{command} with #{arguments.inspect}")
 
@@ -206,22 +206,37 @@ def run_command(*args, permanent: false, allowed_exitstatus: nil)
       # the output as a string and chomping it
       #
       # @see #run_command
-      # @return [String] the chomped output of the run command
       # @param args [Array<String>] list of command optional arguments
-      # @param permanent [Boolean] if true it adds the --permanent option the
-      # command to be executed
+      # @param permanent [Boolean] if true and firewalld is running it
+      #   operates over the permanent configuration
+      # @return [String] the chomped output of the run command
       def string_command(*args, permanent: false)
         run_command(*args, permanent: permanent).to_s.chomp
       end
 
+      # Convenience method that run the given modification command for the
+      # current mode returning whether it was applied successfully or not
+      #
+      #
+      # @see #run_command
+      # @param args [Array<String>] list of command optional arguments
+      # @param permanent [Boolean] if true and firewalld is running it
+      #   operates over the permanent configuration
+      # @return [Boolean] true if the executed command returns succesfully
+      def modify_command(*args, permanent: false)
+        string_command(*args, permanent: permanent) == SUCCESS
+      end
+
       # Convenience method which return true whether the run command for the
       # current mode return the exit status 0.
       #
       # @see #run_command
-      # @return [Boolean] true if the exit status of the executed command is 0
       # @param args [Array<String>] list of command optional arguments
-      def query_command(*args)
-        _output, exit_status = run_command(*args, allowed_exitstatus: [0, 1])
+      # @param permanent [Boolean] if true and firewalld is running it
+      #   operates over the permanent configuration
+      # @return [Boolean] true if the exit status of the executed command is 0
+      def query_command(*args, permanent: false)
+        _output, exit_status = run_command(*args, allowed_exitstatus: [0, 1], permanent: permanent)
 
         exit_status == 0
       end