From 3fe7f5fdede8c49f299838b6bd8b2b5216a01653 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Wed, 6 Nov 2024 13:45:44 +0000 Subject: [PATCH] Assign the global default route when possible --- .../network_manager/config_writer.rb | 43 +++++++++- .../network_manager/config_writer_test.rb | 83 ++++++++++++++++--- 2 files changed, 112 insertions(+), 14 deletions(-) diff --git a/src/lib/y2network/network_manager/config_writer.rb b/src/lib/y2network/network_manager/config_writer.rb index e15eb9fa5..facfc0a43 100644 --- a/src/lib/y2network/network_manager/config_writer.rb +++ b/src/lib/y2network/network_manager/config_writer.rb @@ -26,6 +26,19 @@ module NetworkManager class ConfigWriter < Y2Network::ConfigWriter private + # 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 + 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.) @@ -35,6 +48,8 @@ class ConfigWriter < Y2Network::ConfigWriter 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) @@ -68,7 +83,33 @@ def write_dns(config, old_config, _issues_list) # @param routes [Array] List of routes to search in # @return [Array] 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 if !static_valid_ip?(conn) + + # 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 + + (IPAddr.new conn.ip.address.to_s).to_range.include?(IPAddr.new(r.gateway.to_s)) + end + + explicit + global + 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.to_s != "0.0.0.0" end # Add the DNS settings to the nmconnection file corresponding to the give conn diff --git a/test/y2network/network_manager/config_writer_test.rb b/test/y2network/network_manager/config_writer_test.rb index 3b944a1b7..212e7b15b 100644 --- a/test/y2network/network_manager/config_writer_test.rb +++ b/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