Merge pull request #1368 from yast/write_default_route

Assign the global default route to an specific connection when possible

package/yast2-network.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Thu Nov  7 10:10:01 UTC 2024 - Knut Anderssen <[email protected]>
+
+- Try to assign default global routes to an specific connection 
+  when  possible (bsc#1232531).
+- 4.5.25
+
 -------------------------------------------------------------------
 Wed Mar 13 14:20:25 UTC 2024 - Stefan Hundhammer <[email protected]>
 

package/yast2-network.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-network
-Version:        4.5.24
+Version:        4.5.25
 Release:        0
 Summary:        YaST2 - Network Configuration
 License:        GPL-2.0-only

src/lib/y2network/config_writer.rb

@@ -95,18 +95,21 @@ def write(config, old_config = nil, only: nil)
     #
     # @param _config     [Y2Network::Config] configuration to write
     # @param _old_config [Y2Network::Config, nil] original configuration used for detecting changes
+    # @param _issues_list [Y2Issues::List] list of issues detected until the method is call
     def write_routing(_config, _old_config, _issues_list); end
 
     # Writes the connection configurations
     #
     # @param _config     [Y2Network::Config] configuration to write
     # @param _old_config [Y2Network::Config, nil] original configuration used for detecting changes
+    # @param _issues_list [Y2Issues::List] list of issues detected until the method is call
     def write_connections(_config, _old_config, _issues_list); end
 
     # Updates the DNS configuration
     #
     # @param config     [Y2Network::Config] Current config object
     # @param old_config [Y2Network::Config,nil] Config object with original configuration
+    # @param _issues_list [Y2Issues::List] list of issues detected until the method is call
     def write_dns(config, old_config, _issues_list)
       old_dns = old_config.dns if old_config
       writer = Y2Network::ConfigWriters::DNSWriter.new
@@ -117,6 +120,7 @@ def write_dns(config, old_config, _issues_list)
     #
     # @param config     [Y2Network::Config] Current config object
     # @param old_config [Y2Network::Config,nil] Config object with original configuration
+    # @param _issues_list [Y2Issues::List] list of issues detected until the method is call
     def write_hostname(config, old_config, _issues_list)
       old_hostname = old_config.hostname if old_config
       writer = Y2Network::ConfigWriters::HostnameWriter.new
@@ -128,6 +132,7 @@ def write_hostname(config, old_config, _issues_list)
     #
     # @param config     [Y2Network::Config] Current config object
     # @param _old_config [Y2Network::Config,nil] Config object with original configuration
+    # @param _issues_list [Y2Issues::List] list of issues detected until the method is call
     def write_interfaces(config, _old_config, _issues_list)
       writer = Y2Network::ConfigWriters::InterfacesWriter.new(reload: !Yast::Lan.write_only)
       writer.write(config.interfaces)
@@ -137,6 +142,7 @@ def write_interfaces(config, _old_config, _issues_list)
     #
     # @param config     [Y2Network::Config] Current config object
     # @param _old_config [Y2Network::Config,nil] Config object with original configuration
+    # @param _issues_list [Y2Issues::List] list of issues detected until the method is call
     def write_drivers(config, _old_config, _issues_list)
       Y2Network::Driver.write_options(config.drivers)
     end
@@ -146,6 +152,7 @@ def write_drivers(config, _old_config, _issues_list)
     # TODO: extract this behaviour to a separate class.
     #
     # @param routing [Y2Network::Routing] routing configuration
+    # @param issues_list [Y2Issues::List] list of issues detected until the method is call
     def write_ip_forwarding(routing, issues_list)
       sysctl_config = CFA::SysctlConfig.new
       sysctl_config.load

src/lib/y2network/connection_config/base.rb

@@ -196,6 +196,17 @@ def static?
         bootproto ? bootproto.static? : true
       end
 
+      # Convenience method to check whether a connection is configured using
+      # the static bootproto and a valid IP address.
+      #
+      # @return [Boolean] whether the connection is configured with a valid
+      #   static IP address or not
+      def static_valid_ip?
+        return false unless bootproto.static?
+
+        ip && ip.address.to_s != "0.0.0.0"
+      end
+
       # Return the first hostname associated with the primary IP address.
       #
       # @return [String, nil] returns the hostname associated with the primary

src/lib/y2network/network_manager/config_writer.rb

@@ -19,22 +19,40 @@
 
 require "y2network/config_writer"
 require "y2network/network_manager/connection_config_writer"
+require "y2issues"
 
 module Y2Network
   module NetworkManager
     # This class configures NetworkManager according to a given configuration
     class ConfigWriter < Y2Network::ConfigWriter
     private # rubocop:disable Layout/IndentationWidth
 
+      # Updates the ip forwarding as the routes are written when writing the connections
+      #
+      # @param config     [Y2Network::Config] Current config object
+      # @param _old_config [Y2Network::Config,nil] Config object with original configuration
+      # @param issues_list [Y2Issues::List] list of issues detected until the method is call
+      def write_routing(config, _old_config, issues_list)
+        write_ip_forwarding(config.routing, issues_list)
+
+        routes = routes_for(nil, config.routing.routes)
+        return if routes.empty?
+
+        log.error "There are some routes that could need to be written manually: #{routes}"
+      end
+
       # Writes connections configuration
       #
       # @todo Handle old connections (removing those that are not needed, etc.)
       #
       # @param config     [Y2Network::Config] Current config object
       # @param _old_config [Y2Network::Config,nil] Config object with original configuration
+      # @param _issues_list [Y2Issues::List] list of issues detected until the method is call
       def write_connections(config, _old_config, _issues_list)
         writer = Y2Network::NetworkManager::ConnectionConfigWriter.new
         config.connections.each do |conn|
+          routes_for(conn, config.routing.routes)
+
           opts = {
             routes: routes_for(conn, config.routing.routes),
             parent: conn.find_parent(config.connections)
@@ -49,8 +67,9 @@ def write_connections(config, _old_config, _issues_list)
       # to the configuration file (see bsc#1181701).
       #
       # @param config     [Y2Network::Config] Current config object
-      # @param old_config [Y2Network::Config,nil] Config object with original configuration
-      def write_dns(config, old_config, _issues_list)
+      # @param _old_config [Y2Network::Config,nil] Config object with original configuration
+      # @param _issues_list [Y2Issues::List] list of issues detected until the method is call
+      def write_dns(config, _old_config, _issues_list)
         static = config.connections.by_bootproto(Y2Network::BootProtocol::STATIC)
         return super if static.empty? || config.dns.nameservers.empty?
 
@@ -68,7 +87,23 @@ def write_dns(config, old_config, _issues_list)
       # @param routes [Array<Route>] List of routes to search in
       # @return [Array<Route>] List of routes for the given connection
       def routes_for(conn, routes)
-        routes.select { |r| r.interface&.name == conn.name }
+        return routes.reject(&:interface) if conn.nil?
+
+        explicit = routes.select { |r| r.interface&.name == conn.name }
+
+        return explicit unless conn.static_valid_ip?
+
+        conn_network = (IPAddr.new conn.ip.address.to_s).to_range
+
+        # select the routes without an specific interface and which gateway belongs to the
+        # same network
+        global = routes.select do |r|
+          next if r.interface || !r.default? || !r.gateway
+
+          conn_network.include?(IPAddr.new(r.gateway.to_s))
+        end
+
+        explicit + global
       end
 
       # Add the DNS settings to the nmconnection file corresponding to the give conn

src/lib/y2network/wicked/connection_config_writers/base.rb

@@ -81,7 +81,7 @@ def dhclient_set_hostname(conn)
         def add_ips(conn)
           file.ipaddrs.clear
           ips_to_add = conn.ip_aliases.clone
-          ips_to_add << conn.ip if static_valid_ip?(conn)
+          ips_to_add << conn.ip if conn.static_valid_ip?
           ips_to_add.each { |i| add_ip(i) }
         end
 
@@ -105,18 +105,6 @@ def add_hostname(conn)
           Yast::Host.Update("", conn.hostname, conn.ip.address.address.to_s)
         end
 
-        # Convenience method to check whether a connection is configured using
-        # the static bootproto and a valid IP address.
-        #
-        # @param conn [Y2Network::ConnectionConfig::Base] Connection to take settings from
-        # @return [Boolean] whether the connection is configured with a valid
-        #   static IP address or not
-        def static_valid_ip?(conn)
-          return false unless conn.bootproto.static?
-
-          conn.ip && conn.ip.address.address.to_s != "0.0.0.0"
-        end
-
         # Converts the value into a string (or nil if empty)
         #
         # @param [String] value

test/y2network/network_manager/config_writer_test.rb

@@ -24,6 +24,7 @@
 require "y2network/connection_configs_collection"
 require "y2network/interface"
 require "y2network/interfaces_collection"
+require "y2network/ip_address"
 require "y2network/boot_protocol"
 require "y2network/route"
 require "y2network/routing"
@@ -62,12 +63,20 @@
     end
 
     let(:routes) { [] }
+    let(:gateway) { IPAddr.new("192.168.122.1") }
 
-    let(:route) do
+    let(:eth0_route) do
       Y2Network::Route.new(
         to:        :default,
         interface: eth0,
-        gateway:   IPAddr.new("192.168.122.1")
+        gateway:   gateway
+      )
+    end
+
+    let(:default_route) do
+      Y2Network::Route.new(
+        to:      :default,
+        gateway: gateway
       )
     end
 
@@ -90,35 +99,36 @@
       )
     end
 
-    let(:ip) { Y2Network::ConnectionConfig::IPConfig.new(IPAddr.new("192.168.122.2")) }
+    let(:ip) do
+      address = Y2Network::IPAddress.from_string("192.168.122.2/24")
+      Y2Network::ConnectionConfig::IPConfig.new(address)
+    end
     let(:eth0) { Y2Network::Interface.new("eth0") }
 
     let(:conn_config_writer) { Y2Network::NetworkManager::ConnectionConfigWriter.new }
 
-    let(:file) do
-      CFA::NmConnection.new(File.join(DATA_PATH, "some_wifi.nmconnection"))
-    end
+    let(:file_path) { File.join(DATA_PATH, "eth0_test.nmconnection") }
 
-    before do
-      allow(CFA::NmConnection).to receive(:for).and_return(file)
-      allow(file).to receive(:save)
-    end
+    let(:file) { CFA::NmConnection.new(file_path) }
 
     before do
+      allow(CFA::NmConnection).to receive(:for).and_return(file)
       allow(Y2Network::NetworkManager::ConnectionConfigWriter).to receive(:new)
         .and_return(conn_config_writer)
       allow(writer).to receive(:write_drivers)
       allow(writer).to receive(:write_hostname)
     end
 
+    after { FileUtils.rm_f(file_path) }
+
     it "writes connections configuration" do
       options = { routes: [], parent: nil }
       expect(conn_config_writer).to receive(:write).with(eth0_conn, nil, options)
       writer.write(config)
     end
 
-    context "when there is some route to be configured" do
-      let(:routes) { [route] }
+    context "when there is some device route to be configured" do
+      let(:routes) { [eth0_route] }
 
       context "and it contains a gateway" do
         it "writes the connection gateway" do
@@ -128,7 +138,7 @@
       end
 
       context "and it does not contain a gateway" do
-        let(:route) do
+        let(:eth0_route) do
           Y2Network::Route.new(
             to:        :default,
             interface: eth0
@@ -142,6 +152,53 @@
       end
     end
 
+    context "when there is some global route to be configured" do
+      let(:routes) { [default_route] }
+
+      context "and it contains a gateway" do
+        context "and the connections are configured by DHCP" do
+          it "does not write any gateway" do
+            writer.write(config)
+            expect(file.ipv4["gateway"]).to be_nil
+          end
+        end
+
+        context "and there is some connection configured with an static IP" do
+          let(:eth0_conn) do
+            Y2Network::ConnectionConfig::Ethernet.new.tap do |conn|
+              conn.interface = "eth0"
+              conn.name = "eth0"
+              conn.bootproto = Y2Network::BootProtocol::STATIC
+              conn.ip = ip
+            end
+          end
+
+          context "when the gateway does not belongs to the same network than the connection" do
+            let(:gateway) { IPAddr.new("192.168.1.1") }
+
+            it "does not write any gateway" do
+              writer.write(config)
+              expect(file.ipv4["gateway"]).to be_nil
+            end
+          end
+
+          context "when the gateway belongs to the same network than the connection" do
+            it "writes the connection gateway" do
+              writer.write(config)
+              expect(file.ipv4["gateway"]).to eq("192.168.122.1")
+            end
+          end
+        end
+      end
+
+      context "and it does not contain a gateway" do
+        it "does not write any gateway" do
+          writer.write(config)
+          expect(file.ipv4["gateway"]).to be_nil
+        end
+      end
+    end
+
     context "writes DNS configuration" do
       context "when a connection has static configuration" do
         let(:eth0_conn) do