From f4d9f758785f9ff6d1b8278765a101d1b33c1c4d Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 8 Apr 2025 08:56:26 -0700 Subject: [PATCH] Fix arch guest networking setup Updates network configuration capability in Arch Linux guest to properly handle NetworkManager. Extracts NetworkManager setup into reusable module. --- lib/vagrant/util.rb | 1 + lib/vagrant/util/guest_inspection.rb | 8 + lib/vagrant/util/guest_networks.rb | 107 +++++++++++++ plugins/guests/arch/cap/configure_networks.rb | 4 + .../guests/redhat/cap/configure_networks.rb | 98 +----------- .../network_manager_device.erb | 0 .../arch/cap/configure_networks_test.rb | 13 ++ .../redhat/cap/configure_networks_test.rb | 124 ++------------- test/unit/vagrant/util/guest_networks_spec.rb | 147 ++++++++++++++++++ 9 files changed, 297 insertions(+), 205 deletions(-) create mode 100644 lib/vagrant/util/guest_networks.rb rename templates/{guests/redhat => networking/network_manager}/network_manager_device.erb (100%) create mode 100644 test/unit/vagrant/util/guest_networks_spec.rb diff --git a/lib/vagrant/util.rb b/lib/vagrant/util.rb index 1e68fb02e..3687e1bbe 100644 --- a/lib/vagrant/util.rb +++ b/lib/vagrant/util.rb @@ -21,6 +21,7 @@ module Vagrant autoload :FileMutex, 'vagrant/util/file_mutex' autoload :GuestHosts, 'vagrant/util/guest_hosts' autoload :GuestInspection, 'vagrant/util/guest_inspection' + autoload :GuestNetworks, 'vagrant/util/guest_networks' autoload :HashWithIndifferentAccess, 'vagrant/util/hash_with_indifferent_access' autoload :HCLogFormatter, 'vagrant/util/logging_formatter' autoload :HCLogOutputter, 'vagrant/util/logging_formatter' diff --git a/lib/vagrant/util/guest_inspection.rb b/lib/vagrant/util/guest_inspection.rb index 200c61af6..ff8696652 100644 --- a/lib/vagrant/util/guest_inspection.rb +++ b/lib/vagrant/util/guest_inspection.rb @@ -26,6 +26,14 @@ module Vagrant comm.test("systemctl -q is-active systemd-networkd.service", sudo: true) end + # NetworkManager.service is in use + # + # @param [Vagrant::Plugin::V2::Communicator] comm Guest communicator + # @return [Boolean] + def systemd_network_manager?(comm) + comm.test("systemctl -q is-active NetworkManager.service", sudo: true) + end + # Check if a unit file with the given name is defined. Name can # be a pattern or explicit name. # diff --git a/lib/vagrant/util/guest_networks.rb b/lib/vagrant/util/guest_networks.rb new file mode 100644 index 000000000..670fdff33 --- /dev/null +++ b/lib/vagrant/util/guest_networks.rb @@ -0,0 +1,107 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: BUSL-1.1 + +module Vagrant + module Util + # Helper methods for configuring guest networks + module GuestNetworks + module Linux + NETWORK_MANAGER_DEVICE_DIRECTORY = "/etc/NetworkManager/system-connections".freeze + + def configure_network_manager(machine, networks, **opts) + comm = machine.communicate + nm_directory = opts.fetch(:nm_directory, NETWORK_MANAGER_DEVICE_DIRECTORY) + + interfaces = machine.guest.capability(:network_interfaces) + net_configs = machine.config.vm.networks.find_all { |type, _| type.to_s.end_with?("_network") }.map(&:last) + + # Get IDs of currently configured devices + current_devs = Hash.new.tap do |cd| + comm.execute("nmcli -t c show") do |type, data| + if type == :stdout + _, id, _, dev = data.strip.split(":") + cd[dev] = id + end + end + end + + networks.each.with_index do |network, i| + net_opts = (net_configs[i] || {}).merge(network) + net_opts[:type] = net_opts[:type].to_s + net_opts[:device] = interfaces[network[:interface]] + + if !net_opts[:mac_address] + comm.execute("cat /sys/class/net/#{net_opts[:device]}/address") do |type, data| + net_opts[:mac_address] = data if type == :stdout + end + end + + tmpl_opts = { + interface_name: net_opts[:device], + type: net_opts[:type], + mac_address: net_opts[:mac_address], + uuid: SecureRandom.uuid + } + + if net_opts[:type] != "dhcp" + begin + addr = IPAddr.new("#{net_opts[:ip]}") + if addr.ipv4? + tmpl_opts[:ipv4] = addr.to_string + masked = addr.mask(net_opts[:netmask]) + + tmpl_opts[:ipv4_mask] = masked.prefix + tmpl_opts[:ipv4_gateway] = masked.succ.to_string + else + tmpl_opts[:ipv6] = addr.to_string + masked = addr.mask(net_opts[:netmask]) + + tmpl_opts[:ipv6_mask] = masked.prefix + tmpl_opts[:ipv6_gateway] = masked.succ.to_string + end + rescue IPAddr::Error => err + raise NetworkAddressInvalid, + address: net_opts[:ip], + mask: net_opts[:netmask], + error: err.to_s + end + end + + entry = TemplateRenderer.render("networking/network_manager/network_manager_device", options: tmpl_opts) + remote_path = "/tmp/vagrant-network-entry-#{net_opts[:device]}-#{Time.now.to_i}-#{i}" + final_path = "#{nm_directory}/#{net_opts[:device]}.nmconnection" + + Tempfile.open("vagrant-nm-configure-networks") do |f| + f.binmode + f.write(entry) + f.fsync + f.close + comm.upload(f.path, remote_path) + end + + # Remove the device if it already exists + if device_id = current_devs[net_opts[:device]] + [ + "nmcli d disconnect '#{net_opts[:device]}'", + "nmcli c delete '#{device_id}'", + ].each do |cmd| + comm.sudo(cmd, error_check: false) + end + end + + # Apply the config + [ + "chown root:root '#{remote_path}'", + "chmod 0600 '#{remote_path}'", + "mv '#{remote_path}' '#{final_path}'", + "nmcli c load '#{final_path}'", + "nmcli d connect '#{net_opts[:device]}'" + ].each do |cmd| + comm.sudo(cmd) + end + end + end + end + end + end +end diff --git a/plugins/guests/arch/cap/configure_networks.rb b/plugins/guests/arch/cap/configure_networks.rb index 52ed72f5c..ca14e9eb8 100644 --- a/plugins/guests/arch/cap/configure_networks.rb +++ b/plugins/guests/arch/cap/configure_networks.rb @@ -13,9 +13,13 @@ module VagrantPlugins class ConfigureNetworks include Vagrant::Util extend Vagrant::Util::GuestInspection::Linux + extend Vagrant::Util::GuestNetworks::Linux def self.configure_networks(machine, networks) comm = machine.communicate + + return configure_network_manager(machine, networks) if systemd_network_manager?(comm) + commands = [] uses_systemd_networkd = systemd_networkd?(comm) diff --git a/plugins/guests/redhat/cap/configure_networks.rb b/plugins/guests/redhat/cap/configure_networks.rb index 4886be0c0..7f70ed645 100644 --- a/plugins/guests/redhat/cap/configure_networks.rb +++ b/plugins/guests/redhat/cap/configure_networks.rb @@ -12,6 +12,7 @@ module VagrantPlugins class ConfigureNetworks include Vagrant::Util extend Vagrant::Util::GuestInspection::Linux + extend Vagrant::Util::GuestNetworks::Linux def self.configure_networks(machine, networks) @logger = Log4r::Logger.new("vagrant::guest::redhat::configurenetworks") @@ -23,98 +24,11 @@ module VagrantPlugins # The legacy configuration will handle rhel/centos pre-10 # versions. The newer versions have a different path for # network configuration files. - - return configure_networks_legacy(machine, networks) if network_scripts_dir.end_with?("network-scripts") - - comm = machine.communicate - - interfaces = machine.guest.capability(:network_interfaces) - net_configs = machine.config.vm.networks.find_all { |type, _| type.to_s.end_with?("_network") }.map(&:last) - - # Get IDs of currently configured devices - current_devs = Hash.new.tap do |cd| - comm.execute("nmcli -t c show") do |type, data| - if type == :stdout - _, id, _, dev = data.strip.split(":") - cd[dev] = id - end - end - end - - networks.each.with_index do |network, i| - net_opts = (net_configs[i] || {}).merge(network) - net_opts[:type] = net_opts[:type].to_s - net_opts[:device] = interfaces[network[:interface]] - - if !net_opts[:mac_address] - comm.execute("cat /sys/class/net/#{net_opts[:device]}/address") do |type, data| - net_opts[:mac_address] = data if type == :stdout - end - end - - tmpl_opts = { - interface_name: net_opts[:device], - type: net_opts[:type], - mac_address: net_opts[:mac_address], - uuid: SecureRandom.uuid - } - - if net_opts[:type] != "dhcp" - begin - addr = IPAddr.new("#{net_opts[:ip]}") - if addr.ipv4? - tmpl_opts[:ipv4] = addr.to_string - masked = addr.mask(net_opts[:netmask]) - - tmpl_opts[:ipv4_mask] = masked.prefix - tmpl_opts[:ipv4_gateway] = masked.succ.to_string - else - tmpl_opts[:ipv6] = addr.to_string - masked = addr.mask(net_opts[:netmask]) - - tmpl_opts[:ipv6_mask] = masked.prefix - tmpl_opts[:ipv6_gateway] = masked.succ.to_string - end - rescue IPAddr::Error => err - raise NetworkAddressInvalid, - address: net_opts[:ip], - mask: net_opts[:netmask], - error: err.to_s - end - end - - entry = TemplateRenderer.render("guests/redhat/network_manager_device", options: tmpl_opts) - remote_path = "/tmp/vagrant-network-entry-#{net_opts[:device]}-#{Time.now.to_i}-#{i}" - final_path = "#{network_scripts_dir}/#{net_opts[:device]}.nmconnection" - - Tempfile.open("vagrant-redhat-configure-networks") do |f| - f.binmode - f.write(entry) - f.fsync - f.close - comm.upload(f.path, remote_path) - end - - # Remove the device if it already exists - if device_id = current_devs[net_opts[:device]] - [ - "nmcli d disconnect '#{net_opts[:device]}'", - "nmcli c delete '#{device_id}'", - ].each do |cmd| - comm.sudo(cmd, error_check: false) - end - end - - # Apply the config - [ - "chown root:root '#{remote_path}'", - "chmod 0600 '#{remote_path}'", - "mv '#{remote_path}' '#{final_path}'", - "nmcli c load '#{final_path}'", - "nmcli d connect '#{net_opts[:device]}'" - ].each do |cmd| - comm.sudo(cmd) - end + if network_scripts_dir.end_with?("network-scripts") + configure_networks_legacy(machine, networks) + else + # Recent versions use Network Manager + configure_network_manager(machine, networks) end end diff --git a/templates/guests/redhat/network_manager_device.erb b/templates/networking/network_manager/network_manager_device.erb similarity index 100% rename from templates/guests/redhat/network_manager_device.erb rename to templates/networking/network_manager/network_manager_device.erb diff --git a/test/unit/plugins/guests/arch/cap/configure_networks_test.rb b/test/unit/plugins/guests/arch/cap/configure_networks_test.rb index 7c7713278..b33e391e4 100644 --- a/test/unit/plugins/guests/arch/cap/configure_networks_test.rb +++ b/test/unit/plugins/guests/arch/cap/configure_networks_test.rb @@ -29,6 +29,7 @@ describe "VagrantPlugins::GuestArch::Cap::ConfigureNetworks" do allow(guest).to receive(:capability).with(:network_interfaces) .and_return(["eth1", "eth2"]) allow(cap).to receive(:systemd_networkd?).and_return(true) + allow(cap).to receive(:systemd_network_manager?).and_return(false) end let(:network_1) do @@ -81,5 +82,17 @@ describe "VagrantPlugins::GuestArch::Cap::ConfigureNetworks" do expect(comm.received_commands[0]).to match(/netctl enable 'eth2'/) end end + + context "network is controlled by NetworkManager via systemd" do + before do + expect(cap).to receive(:systemd_network_manager?).and_return(true) + end + + it "should configure for network manager" do + expect(cap).to receive(:configure_network_manager).with(machine, [network_1]) + + cap.configure_networks(machine, [network_1]) + end + end end end diff --git a/test/unit/plugins/guests/redhat/cap/configure_networks_test.rb b/test/unit/plugins/guests/redhat/cap/configure_networks_test.rb index 7aa3098fc..f60e5012c 100644 --- a/test/unit/plugins/guests/redhat/cap/configure_networks_test.rb +++ b/test/unit/plugins/guests/redhat/cap/configure_networks_test.rb @@ -28,20 +28,6 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do context "with systems-connections network configuration path" do let(:cap) { caps.get(:configure_networks) } - before do - allow(guest).to receive(:capability) - .with(:flavor) - .and_return(:rhel) - - allow(guest).to receive(:capability) - .with(:network_scripts_dir) - .and_return("/system-connections") - - allow(guest).to receive(:capability) - .with(:network_interfaces) - .and_return(["eth1", "eth2", "eth3"]) - end - let(:network_1) do { interface: 0, @@ -59,108 +45,20 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do } end - let(:network_3) do - { - interface: 2, - type: "static", - ip: "33.33.33.11", - netmask: "255.255.0.0", - gateway: "33.33.0.1", - } + before do + allow(guest).to receive(:capability) + .with(:flavor) + .and_return(:rhel) + + allow(guest).to receive(:capability) + .with(:network_scripts_dir) + .and_return("/system-connections") end - it "should fetch mac address for devices" do - cap.configure_networks(machine, [network_1, network_2, network_3]) - - expect(comm.received_commands).to include(%r{/net/eth1/address}) - expect(comm.received_commands).to include(%r{/net/eth2/address}) - expect(comm.received_commands).to include(%r{/net/eth3/address}) + it "should configure with network manager" do + expect(cap).to receive(:configure_network_manager).with(machine, [network_1]) + cap.configure_networks(machine, [network_1]) end - - it "should change ownership of files" do - cap.configure_networks(machine, [network_1, network_2, network_3]) - - expect(comm.received_commands).to include(%r{chown root:root '/tmp/vagrant.*eth1.*'}) - expect(comm.received_commands).to include(%r{chown root:root '/tmp/vagrant.*eth2.*'}) - expect(comm.received_commands).to include(%r{chown root:root '/tmp/vagrant.*eth3.*'}) - end - - it "should change mode of files" do - cap.configure_networks(machine, [network_1, network_2, network_3]) - - expect(comm.received_commands).to include(%r{chmod 0600 '/tmp/vagrant.*eth1.*'}) - expect(comm.received_commands).to include(%r{chmod 0600 '/tmp/vagrant.*eth2.*'}) - expect(comm.received_commands).to include(%r{chmod 0600 '/tmp/vagrant.*eth3.*'}) - end - - it "should move configuration files" do - cap.configure_networks(machine, [network_1, network_2, network_3]) - - expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth1.*' '/system-connections/eth1.nmconnection'}) - expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth2.*' '/system-connections/eth2.nmconnection'}) - expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth3.*' '/system-connections/eth3.nmconnection'}) - end - - it "should move configuration files" do - cap.configure_networks(machine, [network_1, network_2, network_3]) - - expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth1.*' '/system-connections/eth1.nmconnection'}) - expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth2.*' '/system-connections/eth2.nmconnection'}) - expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth3.*' '/system-connections/eth3.nmconnection'}) - end - - it "should move load new configuration files" do - cap.configure_networks(machine, [network_1, network_2, network_3]) - - expect(comm.received_commands).to include("nmcli c load '/system-connections/eth1.nmconnection'") - expect(comm.received_commands).to include("nmcli c load '/system-connections/eth2.nmconnection'") - expect(comm.received_commands).to include("nmcli c load '/system-connections/eth3.nmconnection'") - end - - it "should connect new devices" do - cap.configure_networks(machine, [network_1, network_2, network_3]) - - expect(comm.received_commands).to include("nmcli d connect 'eth1'") - expect(comm.received_commands).to include("nmcli d connect 'eth2'") - expect(comm.received_commands).to include("nmcli d connect 'eth3'") - end - - context "network configuration file" do - let(:networks){ [[:public_network, network_1], [:private_network, network_2], [:private_network, network_3]] } - - let(:tempfile) { double("tempfile") } - - before do - allow(tempfile).to receive(:binmode) - allow(tempfile).to receive(:write) - allow(tempfile).to receive(:fsync) - allow(tempfile).to receive(:close) - allow(tempfile).to receive(:path) - allow(Tempfile).to receive(:open).and_yield(tempfile) - end - - it "should generate two configuration files" do - expect(Tempfile).to receive(:open).twice - cap.configure_networks(machine, [network_1, network_2]) - end - - it "should generate three configuration files" do - expect(Tempfile).to receive(:open).thrice - cap.configure_networks(machine, [network_1, network_2, network_3]) - end - - it "should generate configuration with network_2 IP address" do - expect(tempfile).to receive(:write).with(/#{network_2[:ip]}/) - cap.configure_networks(machine, [network_1, network_2, network_3]) - end - - it "should generate configuration with network_3 IP address" do - expect(tempfile).to receive(:write).with(/#{network_3[:ip]}/) - cap.configure_networks(machine, [network_1, network_2, network_3]) - end - end - - end describe ".configure_networks" do diff --git a/test/unit/vagrant/util/guest_networks_spec.rb b/test/unit/vagrant/util/guest_networks_spec.rb new file mode 100644 index 000000000..6b329b837 --- /dev/null +++ b/test/unit/vagrant/util/guest_networks_spec.rb @@ -0,0 +1,147 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: BUSL-1.1 + +require File.expand_path("../../../base", __FILE__) + +require "vagrant/util/guest_networks" + +describe Vagrant::Util::GuestNetworks::Linux do + include_context "unit" + + subject { Class.new { extend Vagrant::Util::GuestNetworks::Linux } } + + let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) } + let(:config) { double("config", vm: vm) } + let(:guest) { double("guest") } + let(:machine) { double("machine", guest: guest, config: config) } + let(:networks){ [[:public_network, network_1], [:private_network, network_2]] } + let(:vm){ double("vm", networks: networks) } + let(:interfaces) { ["eth1", "eth2", "eth3"] } + + before do + allow(machine).to receive(:communicate).and_return(comm) + allow(guest).to receive(:capability).with(:network_interfaces).and_return(interfaces) + end + + after do + comm.verify_expectations! + end + + let(:network_1) do + { + interface: 0, + type: "dhcp", + } + end + + let(:network_2) do + { + interface: 1, + type: "static", + ip: "33.33.33.10", + netmask: "255.255.0.0", + gateway: "33.33.0.1", + } + end + + let(:network_3) do + { + interface: 2, + type: "static", + ip: "33.33.33.11", + netmask: "255.255.0.0", + gateway: "33.33.0.1", + } + end + + it "should fetch mac address for devices" do + subject.configure_network_manager(machine, [network_1, network_2, network_3]) + + expect(comm.received_commands).to include(%r{/net/eth1/address}) + expect(comm.received_commands).to include(%r{/net/eth2/address}) + expect(comm.received_commands).to include(%r{/net/eth3/address}) + end + + it "should change ownership of files" do + subject.configure_network_manager(machine, [network_1, network_2, network_3]) + + expect(comm.received_commands).to include(%r{chown root:root '/tmp/vagrant.*eth1.*'}) + expect(comm.received_commands).to include(%r{chown root:root '/tmp/vagrant.*eth2.*'}) + expect(comm.received_commands).to include(%r{chown root:root '/tmp/vagrant.*eth3.*'}) + end + + it "should change mode of files" do + subject.configure_network_manager(machine, [network_1, network_2, network_3]) + + expect(comm.received_commands).to include(%r{chmod 0600 '/tmp/vagrant.*eth1.*'}) + expect(comm.received_commands).to include(%r{chmod 0600 '/tmp/vagrant.*eth2.*'}) + expect(comm.received_commands).to include(%r{chmod 0600 '/tmp/vagrant.*eth3.*'}) + end + + it "should move configuration files" do + subject.configure_network_manager(machine, [network_1, network_2, network_3]) + + expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth1.*' '/etc/NetworkManager/system-connections/eth1.nmconnection'}) + expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth2.*' '/etc/NetworkManager/system-connections/eth2.nmconnection'}) + expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth3.*' '/etc/NetworkManager/system-connections/eth3.nmconnection'}) + end + + it "should move configuration files" do + subject.configure_network_manager(machine, [network_1, network_2, network_3]) + + expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth1.*' '/etc/NetworkManager/system-connections/eth1.nmconnection'}) + expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth2.*' '/etc/NetworkManager/system-connections/eth2.nmconnection'}) + expect(comm.received_commands).to include(%r{mv '/tmp/vagrant.*eth3.*' '/etc/NetworkManager/system-connections/eth3.nmconnection'}) + end + + it "should move load new configuration files" do + subject.configure_network_manager(machine, [network_1, network_2, network_3]) + + expect(comm.received_commands).to include("nmcli c load '/etc/NetworkManager/system-connections/eth1.nmconnection'") + expect(comm.received_commands).to include("nmcli c load '/etc/NetworkManager/system-connections/eth2.nmconnection'") + expect(comm.received_commands).to include("nmcli c load '/etc/NetworkManager/system-connections/eth3.nmconnection'") + end + + it "should connect new devices" do + subject.configure_network_manager(machine, [network_1, network_2, network_3]) + + expect(comm.received_commands).to include("nmcli d connect 'eth1'") + expect(comm.received_commands).to include("nmcli d connect 'eth2'") + expect(comm.received_commands).to include("nmcli d connect 'eth3'") + end + + context "network configuration file" do + let(:networks){ [[:public_network, network_1], [:private_network, network_2], [:private_network, network_3]] } + + let(:tempfile) { double("tempfile") } + + before do + allow(tempfile).to receive(:binmode) + allow(tempfile).to receive(:write) + allow(tempfile).to receive(:fsync) + allow(tempfile).to receive(:close) + allow(tempfile).to receive(:path) + allow(Tempfile).to receive(:open).and_yield(tempfile) + end + + it "should generate two configuration files" do + expect(Tempfile).to receive(:open).twice + subject.configure_network_manager(machine, [network_1, network_2]) + end + + it "should generate three configuration files" do + expect(Tempfile).to receive(:open).thrice + subject.configure_network_manager(machine, [network_1, network_2, network_3]) + end + + it "should generate configuration with network_2 IP address" do + expect(tempfile).to receive(:write).with(/#{network_2[:ip]}/) + subject.configure_network_manager(machine, [network_1, network_2, network_3]) + end + + it "should generate configuration with network_3 IP address" do + expect(tempfile).to receive(:write).with(/#{network_3[:ip]}/) + subject.configure_network_manager(machine, [network_1, network_2, network_3]) + end + end +end