Improve ConfigReader::Sysconfig behaviour

src/lib/y2network/config_reader/sysconfig.rb

@@ -19,13 +19,14 @@
 require "y2network/config"
 require "y2network/interface"
 require "y2network/routing_table"
+require "y2network/route"
 
 Yast.import "NetworkInterfaces"
 Yast.import "Routing"
 
 module Y2Network
   module ConfigReader
-    # This class reads the current configuration from the system
+    # This class reads the current configuration from `/etc/sysconfig` files
     class Sysconfig
       # @return [Y2Network::Config] Network configuration
       def config
@@ -36,6 +37,13 @@ def config
 
     private
 
+      MISSING_VALUE = "-".freeze
+      private_constant :MISSING_VALUE
+
+      # Find network interfaces
+      #
+      # @return [Array<Interface>] Detected interfaces
+      # @see Yast::NetworkInterfaces.Read
       def find_interfaces
         Yast::NetworkInterfaces.Read
         # TODO: for the time being, we are just relying in the underlying stuff.
@@ -44,17 +52,45 @@ def find_interfaces
         end
       end
 
+      # Find routing tables
+      #
+      # @note For the time being, only one routing table is considered.
+      #
+      # @param interfaces [Array<Interface>] Detected interfaces
+      # @return [Array<RoutingTable>]
+      #
+      # @see Yast::Routing.Routes
       def find_routing_tables(interfaces)
         Yast::Routing.Read
-
-        routes = Yast::Routing.Routes.map do |route|
-          dest = IPAddr.new(route["destination"]).mask(route["netmask"])
-          iface = interfaces.find { |i| i.name == route["device"] }
-          Y2Network::Route.new(dest, iface, gateway: IPAddr.new(route["gateway"]))
-        end
+        routes = Yast::Routing.Routes.map { |h| build_route(interfaces, h) }
         table = Y2Network::RoutingTable.new(routes)
         [table]
       end
+
+      # Build a route given a hash from the SCR agent
+      #
+      # @param interfaces [Array<Interface>] List of detected interfaces
+      # @param hash [Hash] Hash from the `.routes` SCR agent
+      # @return Route
+      def build_route(interfaces, hash)
+        iface = interfaces.find { |i| i.name == hash["device"] }
+        Y2Network::Route.new(
+          to:        build_ip(hash["destination"], hash["netmask"]) || :default,
+          interface: iface,
+          gateway:   build_ip(hash["gateway"], MISSING_VALUE)
+        )
+      end
+
+      # Given an IP and a netmaks, returns a valid IPAddr objecto
+      #
+      # @param ip_str      [String] IP address; {MISSING_VALUE} means that the IP is not defined
+      # @param netmask_str [String] Netmask; {MISSING_VALUE} means than no netmaks was specified
+      # @return [IPAddr,nil] The IP address or `nil` if the IP is missing
+      def build_ip(ip_str, netmask_str = nil)
+        return nil if ip_str == MISSING_VALUE
+        ip = IPAddr.new(ip_str)
+        netmask_str == MISSING_VALUE ? ip : ip.mask(netmask_str)
+      end
     end
   end
 end

test/y2network/config_reader/sysconfig_test.rb

@@ -33,19 +33,22 @@
   let(:routing) do
     instance_double(
       Yast::RoutingClass,
-      Read: nil,
-      Routes: [
-        {
-          "destination" => "192.168.122.0", "device" => "eth0",
-          "gateway" => "192.168.122.1", "netmask" => "255.255.255.0"
-        }
-      ]
+      Read:   nil,
+      Routes: [scr_route]
     )
   end
 
-  # FIXME: "gateway" => "-", "netmask" => "-"
+  let(:scr_route) do
+    {
+      "destination" => destination, "device" => device, "gateway" => gateway, "netmask" => netmask
+    }
+  end
+  let(:destination) { "192.168.122.1" }
+  let(:device) { "eth0" }
+  let(:gateway) { "192.168.122.1" }
+  let(:netmask) { "255.255.255.0" }
 
-  describe "" do
+  describe "#config" do
     before do
       stub_const("Yast::NetworkInterfaces", network_interfaces)
       stub_const("Yast::Routing", routing)
@@ -56,12 +59,42 @@
       expect(config.interfaces.map(&:name)).to eq(["lo", "eth0", "wlan0"])
     end
 
-    it "returns a configuration including network devices" do
+    it "returns a configuration including routes" do
       config = reader.config
       expect(config.routes.size).to eq(1)
       route = config.routes.first
       expect(route.to).to eq(IPAddr.new("192.168.122.0/24"))
       expect(route.interface.name).to eq("eth0")
     end
+
+    context "when there is not gateway" do
+      let(:gateway) { "-" }
+
+      it "sets the gateway to nil" do
+        config = reader.config
+        route = config.routes.first
+        expect(route.gateway).to be_nil
+      end
+    end
+
+    context "when there is no netmask" do
+      let(:netmask) { "-" }
+
+      it "does not set destination netmask" do
+        config = reader.config
+        route = config.routes.first
+        expect(route.to).to eq(IPAddr.new("192.168.122.1/255.255.255.255"))
+      end
+    end
+
+    context "when there is no destination" do
+      let(:destination) { "-" }
+
+      it "considers the route to be the default one" do
+        config = reader.config
+        route = config.routes.first
+        expect(route.to).to eq(:default)
+      end
+    end
   end
 end