From 6f48466ac4d19451786141864628a4dac5abd6c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 29 Dec 2022 12:06:42 +0000 Subject: [PATCH 01/24] [service] Use a separate D-Bus server --- service/lib/dinstaller/dbus/bus.rb | 52 ++++++++ service/lib/dinstaller/dbus/server_manager.rb | 121 ++++++++++++++++++ service/share/dbus.conf | 62 ++++++++- service/test/dinstaller/dbus/bus_test.rb | 43 +++++++ .../dinstaller/dbus/server_manager_test.rb | 118 +++++++++++++++++ 5 files changed, 393 insertions(+), 3 deletions(-) create mode 100644 service/lib/dinstaller/dbus/bus.rb create mode 100644 service/lib/dinstaller/dbus/server_manager.rb create mode 100644 service/test/dinstaller/dbus/bus_test.rb create mode 100644 service/test/dinstaller/dbus/server_manager_test.rb diff --git a/service/lib/dinstaller/dbus/bus.rb b/service/lib/dinstaller/dbus/bus.rb new file mode 100644 index 0000000000..faa88aafe2 --- /dev/null +++ b/service/lib/dinstaller/dbus/bus.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +# Copyright (c) [2022] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "dbus" +require "dinstaller/dbus/server_manager" + +module DInstaller + module DBus + # Represents the D-Installer bus + class Bus < ::DBus::Connection + class << self + # Returns the current D-Bus connection + # + # @return [Bus] D-Bus connection + def current + return @current if @current + + dbus_manager = ServerManager.new + dbus_manager.find_or_start_server + @current = new(dbus_manager.address) + end + + def reset + @current = nil + end + end + + def initialize(address = nil) + super(address) + send_hello + end + end + end +end diff --git a/service/lib/dinstaller/dbus/server_manager.rb b/service/lib/dinstaller/dbus/server_manager.rb new file mode 100644 index 0000000000..16a2c920f0 --- /dev/null +++ b/service/lib/dinstaller/dbus/server_manager.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +# Copyright (c) [2022] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "cheetah" +require "fileutils" + +module DInstaller + module DBus + # This class takes care of setting up the D-Installer D-Bus server + # + # @example Find the current server or start a new if it is not running + # manager = DBusManager.new + # manager.find_or_start_server + class ServerManager + # @return [String] Command to start the D-Bus server + START_CMD = "/usr/bin/dbus-daemon --print-address " \ + "--config-file %{config} --fork --systemd-activation" + private_constant :START_CMD + + # @return [String] Default run directory path + DEFAULT_RUN_DIRECTORY = "/run/d-installer" + private_constant :DEFAULT_RUN_DIRECTORY + + attr_reader :run_directory + + def initialize(run_directory: DEFAULT_RUN_DIRECTORY) + @run_directory = run_directory + end + + # Finds the current D-Bus server or starts a new one + # + # @return [Integer,nil] PID of the server process or nil if it + # was not possible to start a new one + def find_or_start_server + find_server || start_server + end + + # Starts a D-Bus server + # + # @return [Integer,nil] PID of the new server. Returns nil if it failed to start + # the server. + def start_server + FileUtils.mkdir_p(run_directory) + + output = Cheetah.run( + "/usr/bin/dbus-daemon", + "--config-file", config_file, + "--address", address, + "--fork", "--systemd-activation", + "--print-pid", + stdout: :capture + ) + pid = output.strip + File.write(pid_file, pid) + pid.to_i + rescue Cheetah::ExecutionFailed => e + puts "Could not start the DBus daemon: #{e.message}" + nil + end + + # Gets the PID of the running server + # + # @return [Integer,nil] PID of the process if it exists, nil otherwise. + def find_server + return nil unless File.exist?(pid_file) + + pid = File.read(pid_file).to_i + return nil if pid.zero? + + begin + Process.getpgid(pid) + pid + rescue Errno::ESRCH + nil + end + end + + # Returns the D-Bus address + # + # @return [String] + def address + @address ||= "unix:path=#{File.join(run_directory, "bus")}" + end + + private + + # Returns the path to the configuration file + # + # It prefers a local configuration under `share/dbus.conf`. Otherwise, it falls back to a + # system-wide location. + def config_file + file = File.join(Dir.pwd, "share", "dbus.conf") + return file if File.exist?(file) + + "/usr/share/dbus-1/d-installer.conf" + end + + def pid_file + @pid_file ||= File.join(run_directory, "bus.pid") + end + end + end +end diff --git a/service/share/dbus.conf b/service/share/dbus.conf index 13cab74f47..62f4fbfa37 100644 --- a/service/share/dbus.conf +++ b/service/share/dbus.conf @@ -1,8 +1,35 @@ - - + org.opensuse.DInstaller + + + + + unix:tmpdir=/tmp + + + EXTERNAL + + + + + + + + + + + + @@ -22,4 +49,33 @@ + + contexts/dbus_contexts + + + + + 1000000000 + 250000000 + 1000000000 + 250000000 + 1000000000 + + 120000 + 240000 + 150000 + 100000 + 10000 + 100000 + 10000 + 50000 + 50000 + 50000 diff --git a/service/test/dinstaller/dbus/bus_test.rb b/service/test/dinstaller/dbus/bus_test.rb new file mode 100644 index 0000000000..40ad409a3c --- /dev/null +++ b/service/test/dinstaller/dbus/bus_test.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +# Copyright (c) [2022] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require_relative "../../test_helper" +require "dinstaller/dbus/bus" + +describe DInstaller::DBus::Bus do + describe ".current" do + let(:server_manager) do + DInstaller::DBus::ServerManager.new(run_directory: "/tmp") + end + + before do + allow(DInstaller::DBus::ServerManager).to receive(:new).and_return(server_manager) + allow(server_manager).to receive(:find_or_start_server) + end + + it "returns a connection to the current server" do + bus = instance_double(DInstaller::DBus::Bus) + expect(described_class).to receive(:new).with(server_manager.address) + .and_return(bus) + expect(described_class.current).to eq(bus) + end + end +end diff --git a/service/test/dinstaller/dbus/server_manager_test.rb b/service/test/dinstaller/dbus/server_manager_test.rb new file mode 100644 index 0000000000..090335904f --- /dev/null +++ b/service/test/dinstaller/dbus/server_manager_test.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +# Copyright (c) [2022] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require_relative "../../test_helper" +require "tmpdir" +require "dinstaller/dbus/server_manager" + +describe DInstaller::DBus::ServerManager do + subject do + described_class.new(run_directory: tmpdir) + end + + let(:tmpdir) { Dir.mktmpdir } + + after do + FileUtils.remove_entry(tmpdir) + end + + describe "#find_or_start_server" do + context "when the server is started" do + before do + allow(subject).to receive(:find_server).and_return(1000) + end + + it "returns the PID of the running server" do + expect(subject.find_or_start_server).to eq(1000) + end + end + + context "when the server is not started" do + before do + allow(subject).to receive(:find_server).and_return(nil) + end + + it "starts and returns the PID of the new server" do + expect(subject).to receive(:start_server).and_return(1001) + expect(subject.find_or_start_server).to eq(1001) + end + end + end + + describe "#start_server" do + it "starts the dbus-daemon and returns the PID" do + expect(Cheetah).to receive(:run) + .with(/dbus-daemon/, "--config-file", /dbus.conf/, any_args) + .and_return("1000") + expect(subject.start_server).to eq(1000) + end + end + + describe "#find_server" do + context "when a PID file exists" do + let(:pid_file_content) { "1000" } + + before do + File.write(File.join(tmpdir, "bus.pid"), pid_file_content) + end + + context "and the process exists" do + before do + allow(Process).to receive(:getpgid).with(1000).and_return(1000) + end + + it "returns the PID" do + expect(subject.find_server).to eq(1000) + end + end + + context "but it does not contain a PID" do + let(:pid_file_content) { "something" } + + it "returns nil" do + expect(subject.find_server).to be_nil + end + end + + context "but the process does not exist" do + before do + allow(Process).to receive(:getpgid).and_raise(Errno::ESRCH) + end + + it "returns nil" do + expect(subject.find_server).to be_nil + end + end + end + + context "when no PID file exists" do + it "returns nil" do + expect(subject.find_server).to be_nil + end + end + end + + describe "#address" do + it "returns the server address" do + expect(subject.address).to eq("unix:path=#{tmpdir}/bus") + end + end +end From a90d3c1816dd00f89c3bccc76b70b5f82272faa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 29 Dec 2022 12:07:44 +0000 Subject: [PATCH 02/24] [service] Stop the D-Bus server on "d-installer -k" --- service/bin/d-installer | 3 +++ 1 file changed, 3 insertions(+) diff --git a/service/bin/d-installer b/service/bin/d-installer index 51c346bfee..be20a33630 100755 --- a/service/bin/d-installer +++ b/service/bin/d-installer @@ -103,6 +103,9 @@ elsif ["-s", "--status"].include?(ARGV[0]) system "busctl | grep -E '^NAME|DInstaller'" elsif ["-k", "--kill"].include?(ARGV[0]) system "pkill -f bin/d-installer.[a-z]" + # stop the d-bus daemon + # FIXME: use the ServerManager (or the Bus.current instance) to stop the bus + system "cat /run/d-installer/bus.pid | xargs kill -9" else name = ARGV[0] Signal.trap("SIGINT") do From 6c5de31e945e5a093a9cca24294c38ca0b21615c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 29 Dec 2022 12:31:36 +0000 Subject: [PATCH 03/24] [service] Use the own D-Bus server * Replace DBus::SystemBus instances with DInstaller::DBus::Bus. --- service/lib/dinstaller/dbus/clients/base.rb | 3 ++- service/lib/dinstaller/dbus/language_service.rb | 3 ++- service/lib/dinstaller/dbus/manager_service.rb | 3 ++- service/lib/dinstaller/dbus/questions_service.rb | 3 ++- service/lib/dinstaller/dbus/software_service.rb | 3 ++- service/lib/dinstaller/dbus/storage_service.rb | 3 ++- service/lib/dinstaller/dbus/users_service.rb | 3 ++- service/test/dinstaller/dbus/clients/language_test.rb | 4 ++-- service/test/dinstaller/dbus/clients/manager_test.rb | 4 ++-- service/test/dinstaller/dbus/clients/question_test.rb | 4 ++-- .../test/dinstaller/dbus/clients/questions_manager_test.rb | 4 ++-- service/test/dinstaller/dbus/clients/software_test.rb | 4 ++-- service/test/dinstaller/dbus/clients/storage_test.rb | 4 ++-- service/test/dinstaller/dbus/clients/users_test.rb | 4 ++-- service/test/dinstaller/dbus/manager_service_test.rb | 4 ++-- 15 files changed, 30 insertions(+), 23 deletions(-) diff --git a/service/lib/dinstaller/dbus/clients/base.rb b/service/lib/dinstaller/dbus/clients/base.rb index a3bf9d17b5..f17b7b2295 100644 --- a/service/lib/dinstaller/dbus/clients/base.rb +++ b/service/lib/dinstaller/dbus/clients/base.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "dbus" +require "dinstaller/dbus/bus" require "abstract_method" module DInstaller @@ -63,7 +64,7 @@ def on_properties_change(dbus_object, &block) end def bus - @bus ||= ::DBus::SystemBus.instance + @bus ||= Bus.current end end end diff --git a/service/lib/dinstaller/dbus/language_service.rb b/service/lib/dinstaller/dbus/language_service.rb index 2f58b4145f..62d9be0102 100644 --- a/service/lib/dinstaller/dbus/language_service.rb +++ b/service/lib/dinstaller/dbus/language_service.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "dbus" +require "dinstaller/dbus/bus" require "dinstaller/dbus/language" require "dinstaller/language" @@ -45,7 +46,7 @@ class LanguageService # @param logger [Logger] def initialize(_config, logger = nil) @logger = logger || Logger.new($stdout) - @bus = ::DBus::SystemBus.instance + @bus = Bus.current end # Exports the installer object through the D-Bus service diff --git a/service/lib/dinstaller/dbus/manager_service.rb b/service/lib/dinstaller/dbus/manager_service.rb index e722935170..c225378c77 100644 --- a/service/lib/dinstaller/dbus/manager_service.rb +++ b/service/lib/dinstaller/dbus/manager_service.rb @@ -22,6 +22,7 @@ require "dbus" require "dinstaller/manager" require "dinstaller/cockpit_manager" +require "dinstaller/dbus/bus" require "dinstaller/dbus/manager" require "dinstaller/dbus/storage/proposal" @@ -54,7 +55,7 @@ def initialize(config, logger = nil) @config = config @manager = DInstaller::Manager.new(config, logger) @logger = logger || Logger.new($stdout) - @bus = ::DBus::SystemBus.instance + @bus = Bus.current end # Initializes and exports the D-Bus API diff --git a/service/lib/dinstaller/dbus/questions_service.rb b/service/lib/dinstaller/dbus/questions_service.rb index 57a932e19d..dd7083f62f 100644 --- a/service/lib/dinstaller/dbus/questions_service.rb +++ b/service/lib/dinstaller/dbus/questions_service.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "dbus" +require "dinstaller/dbus/bus" require "dinstaller/dbus/questions" require "dinstaller/questions_manager" @@ -42,7 +43,7 @@ class QuestionsService # @param logger [Logger] def initialize(_config, logger = nil) @logger = logger || Logger.new($stdout) - @bus = ::DBus::SystemBus.instance + @bus = Bus.current @backend ||= DInstaller::QuestionsManager.new(logger) end diff --git a/service/lib/dinstaller/dbus/software_service.rb b/service/lib/dinstaller/dbus/software_service.rb index 7d25398ad8..c047f6ff6a 100644 --- a/service/lib/dinstaller/dbus/software_service.rb +++ b/service/lib/dinstaller/dbus/software_service.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "dbus" +require "dinstaller/dbus/bus" require "dinstaller/dbus/software" require "dinstaller/software" @@ -42,7 +43,7 @@ class SoftwareService # @param logger [Logger] def initialize(config, logger = nil) @logger = logger || Logger.new($stdout) - @bus = ::DBus::SystemBus.instance + @bus = Bus.current @backend = DInstaller::Software::Manager.new(config, logger) @backend.on_progress_change { dispatch } end diff --git a/service/lib/dinstaller/dbus/storage_service.rb b/service/lib/dinstaller/dbus/storage_service.rb index 72df1bf4ec..2ecbec64ae 100644 --- a/service/lib/dinstaller/dbus/storage_service.rb +++ b/service/lib/dinstaller/dbus/storage_service.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "dbus" +require "dinstaller/dbus/bus" require "dinstaller/dbus/storage" require "dinstaller/storage" @@ -42,7 +43,7 @@ class StorageService # @param logger [Logger] def initialize(config, logger = nil) @logger = logger || Logger.new($stdout) - @bus = ::DBus::SystemBus.instance + @bus = Bus.current @backend = DInstaller::Storage::Manager.new(config, logger) @backend.on_progress_change { dispatch } end diff --git a/service/lib/dinstaller/dbus/users_service.rb b/service/lib/dinstaller/dbus/users_service.rb index 7a79464010..2a602b2718 100644 --- a/service/lib/dinstaller/dbus/users_service.rb +++ b/service/lib/dinstaller/dbus/users_service.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "dbus" +require "dinstaller/dbus/bus" require "dinstaller/dbus/users" require "dinstaller/users" @@ -45,7 +46,7 @@ class UsersService # @param logger [Logger] def initialize(_config, logger = nil) @logger = logger || Logger.new($stdout) - @bus = ::DBus::SystemBus.instance + @bus = Bus.current end # Exports the installer object through the D-Bus service diff --git a/service/test/dinstaller/dbus/clients/language_test.rb b/service/test/dinstaller/dbus/clients/language_test.rb index 53ffa564d8..a003ac43d8 100644 --- a/service/test/dinstaller/dbus/clients/language_test.rb +++ b/service/test/dinstaller/dbus/clients/language_test.rb @@ -25,7 +25,7 @@ describe DInstaller::DBus::Clients::Language do before do - allow(::DBus::SystemBus).to receive(:instance).and_return(bus) + allow(DInstaller::DBus::Bus).to receive(:current).and_return(bus) allow(bus).to receive(:service).with("org.opensuse.DInstaller.Language").and_return(service) allow(service).to receive(:object).with("/org/opensuse/DInstaller/Language1") .and_return(dbus_object) @@ -34,7 +34,7 @@ .and_return(lang_iface) end - let(:bus) { instance_double(::DBus::SystemBus) } + let(:bus) { instance_double(DInstaller::DBus::Bus) } let(:service) { instance_double(::DBus::Service) } let(:dbus_object) { instance_double(::DBus::ProxyObject) } let(:lang_iface) { instance_double(::DBus::ProxyObjectInterface) } diff --git a/service/test/dinstaller/dbus/clients/manager_test.rb b/service/test/dinstaller/dbus/clients/manager_test.rb index 00366cd2ce..ffab48cbea 100644 --- a/service/test/dinstaller/dbus/clients/manager_test.rb +++ b/service/test/dinstaller/dbus/clients/manager_test.rb @@ -29,7 +29,7 @@ describe DInstaller::DBus::Clients::Manager do before do - allow(::DBus::SystemBus).to receive(:instance).and_return(bus) + allow(DInstaller::DBus::Bus).to receive(:current).and_return(bus) allow(bus).to receive(:service).with("org.opensuse.DInstaller").and_return(service) allow(service).to receive(:[]).with("/org/opensuse/DInstaller/Manager1") .and_return(dbus_object) @@ -40,7 +40,7 @@ .and_return(properties_iface) end - let(:bus) { instance_double(::DBus::SystemBus) } + let(:bus) { instance_double(DInstaller::DBus::Bus) } let(:service) { instance_double(::DBus::Service) } let(:dbus_object) { instance_double(::DBus::ProxyObject) } let(:manager_iface) { instance_double(::DBus::ProxyObjectInterface) } diff --git a/service/test/dinstaller/dbus/clients/question_test.rb b/service/test/dinstaller/dbus/clients/question_test.rb index 4a516422b6..fedb471f5e 100644 --- a/service/test/dinstaller/dbus/clients/question_test.rb +++ b/service/test/dinstaller/dbus/clients/question_test.rb @@ -25,7 +25,7 @@ describe DInstaller::DBus::Clients::Question do before do - allow(::DBus::SystemBus).to receive(:instance).and_return(bus) + allow(DInstaller::DBus::Bus).to receive(:current).and_return(bus) allow(bus).to receive(:service).with("org.opensuse.DInstaller.Questions").and_return(service) allow(service).to receive(:[]).with("/org/opensuse/DInstaller/Questions1/23") .and_return(dbus_object) @@ -36,7 +36,7 @@ allow(dbus_object).to receive(:has_iface?).with(/LuksActivation1/).and_return(luks_iface?) end - let(:bus) { instance_double(::DBus::SystemBus) } + let(:bus) { instance_double(DInstaller::DBus::Bus) } let(:service) { instance_double(::DBus::Service) } let(:dbus_object) { instance_double(::DBus::ProxyObject) } let(:question_iface) { instance_double(::DBus::ProxyObjectInterface) } diff --git a/service/test/dinstaller/dbus/clients/questions_manager_test.rb b/service/test/dinstaller/dbus/clients/questions_manager_test.rb index 9c69a5395f..ec3e4417b0 100644 --- a/service/test/dinstaller/dbus/clients/questions_manager_test.rb +++ b/service/test/dinstaller/dbus/clients/questions_manager_test.rb @@ -26,14 +26,14 @@ describe DInstaller::DBus::Clients::QuestionsManager do before do - allow(::DBus::SystemBus).to receive(:instance).and_return(bus) + allow(DInstaller::DBus::Bus).to receive(:current).and_return(bus) allow(bus).to receive(:service).with("org.opensuse.DInstaller.Questions").and_return(service) allow(service).to receive(:[]).with("/org/opensuse/DInstaller/Questions1") .and_return(dbus_object) allow(dbus_object).to receive(:default_iface=) end - let(:bus) { instance_double(::DBus::SystemBus) } + let(:bus) { instance_double(DInstaller::DBus::Bus) } let(:service) { instance_double(::DBus::Service) } let(:dbus_object) { instance_double(::DBus::ProxyObject) } let(:properties_iface) { instance_double(::DBus::ProxyObjectInterface) } diff --git a/service/test/dinstaller/dbus/clients/software_test.rb b/service/test/dinstaller/dbus/clients/software_test.rb index 31f7af3530..b573196227 100644 --- a/service/test/dinstaller/dbus/clients/software_test.rb +++ b/service/test/dinstaller/dbus/clients/software_test.rb @@ -29,7 +29,7 @@ describe DInstaller::DBus::Clients::Software do before do - allow(::DBus::SystemBus).to receive(:instance).and_return(bus) + allow(DInstaller::DBus::Bus).to receive(:current).and_return(bus) allow(bus).to receive(:service).with("org.opensuse.DInstaller.Software").and_return(service) allow(service).to receive(:[]).with("/org/opensuse/DInstaller/Software1") .and_return(dbus_object) @@ -42,7 +42,7 @@ .and_return(dbus_proposal) end - let(:bus) { instance_double(::DBus::SystemBus) } + let(:bus) { instance_double(DInstaller::DBus::Bus) } let(:service) { instance_double(::DBus::Service) } let(:dbus_object) { instance_double(::DBus::ProxyObject) } let(:dbus_proposal) { instance_double(::DBus::ProxyObject, introspect: nil) } diff --git a/service/test/dinstaller/dbus/clients/storage_test.rb b/service/test/dinstaller/dbus/clients/storage_test.rb index 5d2ad6d000..8527c62f02 100644 --- a/service/test/dinstaller/dbus/clients/storage_test.rb +++ b/service/test/dinstaller/dbus/clients/storage_test.rb @@ -26,7 +26,7 @@ describe DInstaller::DBus::Clients::Storage do before do - allow(::DBus::SystemBus).to receive(:instance).and_return(bus) + allow(DInstaller::DBus::Bus).to receive(:current).and_return(bus) allow(bus).to receive(:service).with("org.opensuse.DInstaller.Storage").and_return(service) allow(service).to receive(:[]).with("/org/opensuse/DInstaller/Storage1") @@ -42,7 +42,7 @@ .and_return(proposal_iface) end - let(:bus) { instance_double(::DBus::SystemBus) } + let(:bus) { instance_double(DInstaller::DBus::Bus) } let(:service) { instance_double(::DBus::Service) } let(:dbus_object) { instance_double(::DBus::ProxyObject) } diff --git a/service/test/dinstaller/dbus/clients/users_test.rb b/service/test/dinstaller/dbus/clients/users_test.rb index e49af44ece..eda1d95dad 100644 --- a/service/test/dinstaller/dbus/clients/users_test.rb +++ b/service/test/dinstaller/dbus/clients/users_test.rb @@ -28,7 +28,7 @@ describe DInstaller::DBus::Clients::Users do before do - allow(::DBus::SystemBus).to receive(:instance).and_return(bus) + allow(DInstaller::DBus::Bus).to receive(:current).and_return(bus) allow(bus).to receive(:service).with("org.opensuse.DInstaller.Users").and_return(service) allow(service).to receive(:[]).with("/org/opensuse/DInstaller/Users1") .and_return(dbus_object) @@ -39,7 +39,7 @@ .and_return(service_status_iface) end - let(:bus) { instance_double(::DBus::SystemBus) } + let(:bus) { instance_double(DInstaller::DBus::Bus) } let(:service) { instance_double(::DBus::Service) } let(:dbus_object) { instance_double(::DBus::ProxyObject) } let(:users_iface) { instance_double(::DBus::ProxyObjectInterface) } diff --git a/service/test/dinstaller/dbus/manager_service_test.rb b/service/test/dinstaller/dbus/manager_service_test.rb index 68c4f308dc..c136d5ef65 100644 --- a/service/test/dinstaller/dbus/manager_service_test.rb +++ b/service/test/dinstaller/dbus/manager_service_test.rb @@ -29,7 +29,7 @@ let(:config) { DInstaller::Config.new } let(:logger) { Logger.new($stdout, level: :warn) } let(:manager) { DInstaller::Manager.new(config, logger) } - let(:bus) { instance_double(::DBus::SystemBus) } + let(:bus) { instance_double(DInstaller::DBus::Bus) } let(:bus_service) do instance_double(::DBus::Service, export: nil) end @@ -39,7 +39,7 @@ end before do - allow(::DBus::SystemBus).to receive(:instance).and_return(bus) + allow(DInstaller::DBus::Bus).to receive(:current).and_return(bus) allow(bus).to receive(:request_service).and_return(bus_service) allow(DInstaller::Manager).to receive(:new).with(config, logger).and_return(manager) allow(DInstaller::CockpitManager).to receive(:new).and_return(cockpit) From 18a95908a757ea4880ff0694b3dc44e772605f06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 29 Dec 2022 12:33:37 +0000 Subject: [PATCH 04/24] [cli] Update Gemfile.lock --- cli/Gemfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/Gemfile.lock b/cli/Gemfile.lock index 0816bbea11..12e4cebc1e 100644 --- a/cli/Gemfile.lock +++ b/cli/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../service specs: - d-installer (0.6.1) + d-installer (0.6.2) cfa (~> 1.0.2) cfa_grub2 (~> 2.0.0) cheetah (~> 1.0.0) @@ -39,7 +39,7 @@ GEM racc (~> 1.4) packaging_rake_tasks (1.5.1) rake - racc (1.6.1) + racc (1.6.2) rake (13.0.6) rexml (3.2.5) rspec (3.11.0) @@ -81,4 +81,4 @@ DEPENDENCIES simplecov-lcov (~> 0.8.0) BUNDLED WITH - 2.3.24 + 2.3.26 From d8c9e0504deafb22b88534210763305f058ef4f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 30 Dec 2022 08:25:21 +0000 Subject: [PATCH 05/24] [web] Connect to the D-Installer D-Bus server --- web/src/client/dbus.js | 13 +++++++++++-- web/src/client/network/network_manager.js | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/web/src/client/dbus.js b/web/src/client/dbus.js index 1d07c44b4a..25f6184385 100644 --- a/web/src/client/dbus.js +++ b/web/src/client/dbus.js @@ -66,13 +66,22 @@ import cockpit from "../lib/cockpit"; * @property {string} [arg0] - First element of the D-Bus message */ +/* TODO: get the real address from the system */ +const DEFAULT_DBUS_ADDRESS = "unix:path=/run/d-installer/bus"; + /** Wrapper class around cockpit D-Bus client */ class DBusClient { /** * @param {string} service - service name + * @param {string|undefined} bus - bus name ("system" or undefined) */ - constructor(service) { - this.client = cockpit.dbus(service, { bus: "system", superuser: "try" }); + constructor(service, bus = undefined) { + this.client = cockpit.dbus(service, { + bus: bus || "none", + address: bus === undefined ? DEFAULT_DBUS_ADDRESS : undefined, + superuser: "try" + } + ); } /** diff --git a/web/src/client/network/network_manager.js b/web/src/client/network/network_manager.js index 1d88fbdc51..e320cc663a 100644 --- a/web/src/client/network/network_manager.js +++ b/web/src/client/network/network_manager.js @@ -171,7 +171,7 @@ class NetworkManagerAdapter { * @param {DBusClient} [dbusClient] - D-Bus client */ constructor(dbusClient) { - this.client = dbusClient || new DBusClient(SERVICE_NAME); + this.client = dbusClient || new DBusClient(SERVICE_NAME, "system"); /** @type {{[k: string]: string}} */ this.connectionIds = {}; this.proxies = { From 3fd005f268a7048cc25031178f4cb21a3e6ec1ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 3 Jan 2023 07:47:20 +0000 Subject: [PATCH 06/24] [service] Fix several documentation issues Co-authored-by: Martin Vidner --- service/lib/dinstaller/dbus/bus.rb | 6 ++++-- web/src/client/dbus.js | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/service/lib/dinstaller/dbus/bus.rb b/service/lib/dinstaller/dbus/bus.rb index faa88aafe2..b997ffd9ee 100644 --- a/service/lib/dinstaller/dbus/bus.rb +++ b/service/lib/dinstaller/dbus/bus.rb @@ -24,7 +24,7 @@ module DInstaller module DBus - # Represents the D-Installer bus + # Represents the D-Installer bus, a distinct one from the system and session buses. class Bus < ::DBus::Connection class << self # Returns the current D-Bus connection @@ -43,7 +43,9 @@ def reset end end - def initialize(address = nil) + # @param address [String] a connectable address + # @see https://dbus.freedesktop.org/doc/dbus-specification.html#addresses + def initialize(address) super(address) send_hello end diff --git a/web/src/client/dbus.js b/web/src/client/dbus.js index 25f6184385..af4f2facfb 100644 --- a/web/src/client/dbus.js +++ b/web/src/client/dbus.js @@ -73,7 +73,9 @@ const DEFAULT_DBUS_ADDRESS = "unix:path=/run/d-installer/bus"; class DBusClient { /** * @param {string} service - service name - * @param {string|undefined} bus - bus name ("system" or undefined) + * @param {string|undefined} bus - bus name ("system" or undefined); + * undefined means use the bus where D-Installer lives (a dedicated one) + * "system" is for other services like NetworkManager */ constructor(service, bus = undefined) { this.client = cockpit.dbus(service, { From cbbf9441872a63113e88c52749d076fa12376e33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 3 Jan 2023 07:51:17 +0000 Subject: [PATCH 07/24] [service] Remove the START_CMD constant --- service/lib/dinstaller/dbus/server_manager.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/service/lib/dinstaller/dbus/server_manager.rb b/service/lib/dinstaller/dbus/server_manager.rb index 16a2c920f0..e6dd118fa6 100644 --- a/service/lib/dinstaller/dbus/server_manager.rb +++ b/service/lib/dinstaller/dbus/server_manager.rb @@ -30,11 +30,6 @@ module DBus # manager = DBusManager.new # manager.find_or_start_server class ServerManager - # @return [String] Command to start the D-Bus server - START_CMD = "/usr/bin/dbus-daemon --print-address " \ - "--config-file %{config} --fork --systemd-activation" - private_constant :START_CMD - # @return [String] Default run directory path DEFAULT_RUN_DIRECTORY = "/run/d-installer" private_constant :DEFAULT_RUN_DIRECTORY From 36970b7257ac397bfa03b214e1efe0b5085a65d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 3 Jan 2023 07:54:19 +0000 Subject: [PATCH 08/24] [service] Fix the path to the D-Bus configuration file --- service/lib/dinstaller/dbus/server_manager.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/lib/dinstaller/dbus/server_manager.rb b/service/lib/dinstaller/dbus/server_manager.rb index e6dd118fa6..565d0a8a52 100644 --- a/service/lib/dinstaller/dbus/server_manager.rb +++ b/service/lib/dinstaller/dbus/server_manager.rb @@ -105,7 +105,7 @@ def config_file file = File.join(Dir.pwd, "share", "dbus.conf") return file if File.exist?(file) - "/usr/share/dbus-1/d-installer.conf" + "/usr/share/dbus-1/system.d/org.opensuse.DInstaller.conf" end def pid_file From c8e358dc5e2a37bb7b8fb49a3fb68d8a99d9b503 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 3 Jan 2023 09:52:51 +0000 Subject: [PATCH 09/24] [service] Fix and document the D-Bus configuration --- service/share/dbus.conf | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/service/share/dbus.conf b/service/share/dbus.conf index 62f4fbfa37..cde93cfd35 100644 --- a/service/share/dbus.conf +++ b/service/share/dbus.conf @@ -1,3 +1,9 @@ + @@ -21,15 +27,6 @@ - - - - - - - - - From 3eff53b5c2babed4d69c136420882f55aedf81c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 9 Jan 2023 12:06:42 +0000 Subject: [PATCH 10/24] [service] Write the D-Bus address to a file --- service/lib/dinstaller/dbus/server_manager.rb | 11 +++++++++++ service/test/dinstaller/dbus/server_manager_test.rb | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/service/lib/dinstaller/dbus/server_manager.rb b/service/lib/dinstaller/dbus/server_manager.rb index 565d0a8a52..7daf10da9e 100644 --- a/service/lib/dinstaller/dbus/server_manager.rb +++ b/service/lib/dinstaller/dbus/server_manager.rb @@ -63,6 +63,7 @@ def start_server "--print-pid", stdout: :capture ) + File.write(address_file, address) pid = output.strip File.write(pid_file, pid) pid.to_i @@ -108,9 +109,19 @@ def config_file "/usr/share/dbus-1/system.d/org.opensuse.DInstaller.conf" end + # Returns the path to the file containing the PID + # + # @return [String] def pid_file @pid_file ||= File.join(run_directory, "bus.pid") end + + # Returns the path to the file containing the D-Bus address + # + # @return [String] + def address_file + @address_file ||= File.join(run_directory, "bus.address") + end end end end diff --git a/service/test/dinstaller/dbus/server_manager_test.rb b/service/test/dinstaller/dbus/server_manager_test.rb index 090335904f..9a69a05999 100644 --- a/service/test/dinstaller/dbus/server_manager_test.rb +++ b/service/test/dinstaller/dbus/server_manager_test.rb @@ -64,6 +64,12 @@ .and_return("1000") expect(subject.start_server).to eq(1000) end + + it "writes the address to a file in the /run directory" do + subject.start_server + address = File.read(File.join(tmpdir, "bus.address")) + expect(address).to eq("unix:path=#{tmpdir}/bus") + end end describe "#find_server" do From 027744996c0f86923b913fb315b42434b00be8c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 9 Jan 2023 11:57:55 +0000 Subject: [PATCH 11/24] [web] Read the D-Bus address from /run/d-installer --- web/src/client/dbus.js | 24 ++++----- web/src/client/dbus.test.js | 2 +- web/src/client/index.js | 23 ++++---- web/src/client/language.js | 8 +-- web/src/client/language.test.js | 14 ++--- web/src/client/manager.js | 10 ++-- web/src/client/manager.test.js | 25 ++++----- web/src/client/mixins.js | 2 +- web/src/client/monitor.js | 8 +-- web/src/client/network/network_manager.js | 9 ++-- .../client/network/network_manager.test.js | 53 ++++++++++--------- web/src/client/questions.js | 8 +-- web/src/client/questions.test.js | 24 ++++----- web/src/client/software.js | 8 +-- web/src/client/software.test.js | 18 ++++--- web/src/client/storage.js | 8 +-- web/src/client/storage.test.js | 22 +++++--- web/src/client/users.js | 8 +-- web/src/client/users.test.js | 43 ++++++++------- web/src/context/installer.jsx | 18 ++++++- web/src/index.js | 4 +- 21 files changed, 187 insertions(+), 152 deletions(-) diff --git a/web/src/client/dbus.js b/web/src/client/dbus.js index af4f2facfb..1686b3fef8 100644 --- a/web/src/client/dbus.js +++ b/web/src/client/dbus.js @@ -66,24 +66,24 @@ import cockpit from "../lib/cockpit"; * @property {string} [arg0] - First element of the D-Bus message */ -/* TODO: get the real address from the system */ -const DEFAULT_DBUS_ADDRESS = "unix:path=/run/d-installer/bus"; - /** Wrapper class around cockpit D-Bus client */ class DBusClient { /** * @param {string} service - service name - * @param {string|undefined} bus - bus name ("system" or undefined); - * undefined means use the bus where D-Installer lives (a dedicated one) - * "system" is for other services like NetworkManager + * @param {string|undefined} address - D-Bus address. If no address is given, it connects + * to the "system" bus. */ - constructor(service, bus = undefined) { - this.client = cockpit.dbus(service, { - bus: bus || "none", - address: bus === undefined ? DEFAULT_DBUS_ADDRESS : undefined, + constructor(service, address = undefined) { + const options = { + bus: address === undefined ? "system" : "none", superuser: "try" + }; + + if (address) { + options.address = address; } - ); + + this.client = cockpit.dbus(service, options); } /** @@ -167,4 +167,4 @@ class DBusClient { } } -export { DBusClient }; +export default DBusClient; diff --git a/web/src/client/dbus.test.js b/web/src/client/dbus.test.js index 5cce5f0848..c137092615 100644 --- a/web/src/client/dbus.test.js +++ b/web/src/client/dbus.test.js @@ -21,7 +21,7 @@ // @ts-check -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; import cockpit from "../lib/cockpit"; const proxyObject = { diff --git a/web/src/client/index.js b/web/src/client/index.js index be7f19a90c..8442409e8e 100644 --- a/web/src/client/index.js +++ b/web/src/client/index.js @@ -21,7 +21,6 @@ // @ts-check -import { DBusClient } from "./dbus"; import { LanguageClient } from "./language"; import { ManagerClient } from "./manager"; import { Monitor } from "./monitor"; @@ -31,6 +30,7 @@ import { UsersClient } from "./users"; import phase from "./phase"; import { QuestionsClient } from "./questions"; import { NetworkClient } from "./network"; +import cockpit from "../lib/cockpit"; const SERVICE_NAME = "org.opensuse.DInstaller"; @@ -49,20 +49,21 @@ const SERVICE_NAME = "org.opensuse.DInstaller"; /** * Creates a D-Installer client * - * @return {InstallerClient} + * @return {Promise} */ -const createClient = () => { - const client = new DBusClient(SERVICE_NAME); +const createClient = async () => { + const file = cockpit.file("/run/d-installer/bus.address", { binary: false }); + const address = await file.read(); return { - language: new LanguageClient(), - manager: new ManagerClient(client), - monitor: new Monitor(client, SERVICE_NAME), + language: new LanguageClient(address), + manager: new ManagerClient(address), + monitor: new Monitor(address, SERVICE_NAME), network: new NetworkClient(), - software: new SoftwareClient(), - storage: new StorageClient(), - users: new UsersClient(), - questions: new QuestionsClient() + software: new SoftwareClient(address), + storage: new StorageClient(address), + users: new UsersClient(address), + questions: new QuestionsClient(address) }; }; diff --git a/web/src/client/language.js b/web/src/client/language.js index ffb83d008d..89a3955bed 100644 --- a/web/src/client/language.js +++ b/web/src/client/language.js @@ -20,7 +20,7 @@ */ // @ts-check -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; const LANGUAGE_SERVICE = "org.opensuse.DInstaller.Language"; const LANGUAGE_IFACE = "org.opensuse.DInstaller.Language1"; @@ -37,10 +37,10 @@ const LANGUAGE_PATH = "/org/opensuse/DInstaller/Language1"; */ class LanguageClient { /** - * @param {DBusClient} [dbusClient] - D-Bus client + * @param {string|undefined} address - D-Bus address; if it is undefined, it uses the system bus. */ - constructor(dbusClient) { - this.client = dbusClient || new DBusClient(LANGUAGE_SERVICE); + constructor(address = undefined) { + this.client = new DBusClient(LANGUAGE_SERVICE, address); } /** diff --git a/web/src/client/language.test.js b/web/src/client/language.test.js index b95772f4fa..f8063faf97 100644 --- a/web/src/client/language.test.js +++ b/web/src/client/language.test.js @@ -21,12 +21,11 @@ // @ts-check +import DBusClient from "./dbus"; import { LanguageClient } from "./language"; -import { DBusClient } from "./dbus"; -const LANGUAGE_IFACE = "org.opensuse.DInstaller.Language1"; +jest.mock("./dbus"); -const dbusClient = new DBusClient(""); const langProxy = { wait: jest.fn(), AvailableLanguages: [ @@ -34,15 +33,18 @@ const langProxy = { ] }; +jest.mock("./dbus"); + beforeEach(() => { - dbusClient.proxy = jest.fn().mockImplementation(iface => { - if (iface === LANGUAGE_IFACE) return langProxy; + // @ts-ignore + DBusClient.mockImplementation(() => { + return { proxy: () => langProxy }; }); }); describe("#getLanguages", () => { it("returns the list of available languages", async () => { - const client = new LanguageClient(dbusClient); + const client = new LanguageClient(); const availableLanguages = await client.getLanguages(); expect(availableLanguages).toEqual([{ id: "cs_CZ", name: "Cestina" }]); }); diff --git a/web/src/client/manager.js b/web/src/client/manager.js index 2828bba18b..ea2ed117e1 100644 --- a/web/src/client/manager.js +++ b/web/src/client/manager.js @@ -21,11 +21,11 @@ // @ts-check -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; import { WithStatus, WithProgress } from "./mixins"; import cockpit from "../lib/cockpit"; -const MANAGER_SERVICE = "org.opensuse.DInstaller.Manager"; +const MANAGER_SERVICE = "org.opensuse.DInstaller"; const MANAGER_IFACE = "org.opensuse.DInstaller.Manager1"; const MANAGER_PATH = "/org/opensuse/DInstaller/Manager1"; @@ -36,10 +36,10 @@ const MANAGER_PATH = "/org/opensuse/DInstaller/Manager1"; */ class ManagerBaseClient { /** - * @param {DBusClient} [dbusClient] - D-Bus client + * @param {string|undefined} address - D-Bus address; if it is undefined, it uses the system bus. */ - constructor(dbusClient) { - this.client = dbusClient || new DBusClient(MANAGER_SERVICE); + constructor(address = undefined) { + this.client = new DBusClient(MANAGER_SERVICE, address); } /** diff --git a/web/src/client/manager.test.js b/web/src/client/manager.test.js index f9826e4010..07d7eb5a95 100644 --- a/web/src/client/manager.test.js +++ b/web/src/client/manager.test.js @@ -22,16 +22,16 @@ // @ts-check import { ManagerClient } from "./manager"; -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; import cockpit from "../lib/cockpit"; jest.mock("../lib/cockpit"); +jest.mock("./dbus"); const MANAGER_IFACE = "org.opensuse.DInstaller.Manager1"; const SERVICE_IFACE = "org.opensuse.DInstaller.ServiceStatus1"; const PROGRESS_IFACE = "org.opensuse.DInstaller.Progress1"; -const dbusClient = new DBusClient(""); const managerProxy = { wait: jest.fn(), Commit: jest.fn(), @@ -59,14 +59,15 @@ const proxies = { }; beforeEach(() => { - dbusClient.proxy = jest.fn().mockImplementation(iface => { - return proxies[iface]; + // @ts-ignore + DBusClient.mockImplementation(() => { + return { proxy: (iface) => proxies[iface] }; }); }); describe("#getStatus", () => { it("returns the installer status", async () => { - const client = new ManagerClient(dbusClient); + const client = new ManagerClient(); const status = await client.getStatus(); expect(status).toEqual(0); }); @@ -74,7 +75,7 @@ describe("#getStatus", () => { describe("#getProgress", () => { it("returns the manager service progress", async () => { - const client = new ManagerClient(dbusClient); + const client = new ManagerClient(); const status = await client.getProgress(); expect(status).toEqual({ message: "Installing software", @@ -87,7 +88,7 @@ describe("#getProgress", () => { describe("#startProbing", () => { it("(re)starts the probing process", async () => { - const client = new ManagerClient(dbusClient); + const client = new ManagerClient(); await client.startProbing(); expect(managerProxy.Probe).toHaveBeenCalledWith(); }); @@ -95,7 +96,7 @@ describe("#startProbing", () => { describe("#getPhase", () => { it("resolves to the current phase", () => { - const client = new ManagerClient(dbusClient); + const client = new ManagerClient(); const phase = client.getPhase(); expect(phase).resolves.toEqual(0); }); @@ -103,7 +104,7 @@ describe("#getPhase", () => { describe("#startInstallation", () => { it("starts the installation", async () => { - const client = new ManagerClient(dbusClient); + const client = new ManagerClient(); await client.startInstallation(); expect(managerProxy.Commit).toHaveBeenCalledWith(); }); @@ -115,7 +116,7 @@ describe("#rebootSystem", () => { }); it("returns whether the system reboot command was called or not", async () => { - const client = new ManagerClient(dbusClient); + const client = new ManagerClient(); const reboot = await client.rebootSystem(); expect(cockpit.spawn).toHaveBeenCalledWith(["/usr/sbin/shutdown", "-r", "now"]); expect(reboot).toEqual(true); @@ -129,7 +130,7 @@ describe("#canInstall", () => { }); it("returns true", async () => { - const client = new ManagerClient(dbusClient); + const client = new ManagerClient(); const install = await client.canInstall(); expect(install).toEqual(true); }); @@ -141,7 +142,7 @@ describe("#canInstall", () => { }); it("returns false", async () => { - const client = new ManagerClient(dbusClient); + const client = new ManagerClient(); const install = await client.canInstall(); expect(install).toEqual(false); }); diff --git a/web/src/client/mixins.js b/web/src/client/mixins.js index 44f64a0e37..021e533eb5 100644 --- a/web/src/client/mixins.js +++ b/web/src/client/mixins.js @@ -48,7 +48,7 @@ const STATUS_IFACE = "org.opensuse.DInstaller.ServiceStatus1"; */ /** - * @typedef {GConstructor<{ client: import("./dbus").DBusClient }>} WithDBusClient + * @typedef {GConstructor<{ client: import("./dbus").default }>} WithDBusClient */ /** diff --git a/web/src/client/monitor.js b/web/src/client/monitor.js index a3986d2b8a..aeb9fa5476 100644 --- a/web/src/client/monitor.js +++ b/web/src/client/monitor.js @@ -21,7 +21,7 @@ // @ts-check -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; const DBUS_SERVICE = "org.freedesktop.DBus"; const MATCHER = { interface: DBUS_SERVICE, member: "NameOwnerChanged" }; @@ -31,12 +31,12 @@ const MATCHER = { interface: DBUS_SERVICE, member: "NameOwnerChanged" }; */ class Monitor { /** - * @param {DBusClient} dbusClient - D-Bus client + * @param {string|undefined} address - D-Bus address; if it is undefined, it uses the system bus. * @param {string} serviceName - name of the service to monitor */ - constructor(dbusClient, serviceName) { + constructor(address, serviceName) { this.serviceName = serviceName; - this.client = dbusClient || new DBusClient("org.freedesktop.DBus"); + this.client = new DBusClient("org.freedesktop.DBus", address); } /** diff --git a/web/src/client/network/network_manager.js b/web/src/client/network/network_manager.js index e320cc663a..f20a998f54 100644 --- a/web/src/client/network/network_manager.js +++ b/web/src/client/network/network_manager.js @@ -21,7 +21,7 @@ // @ts-check // -import { DBusClient } from "../dbus"; +import DBusClient from "../dbus"; import cockpit from "../../lib/cockpit"; import { intToIPString, stringToIPInt } from "./utils"; import { NetworkEventTypes } from "./index"; @@ -167,11 +167,8 @@ const mergeConnectionSettings = (settings, connection) => { * D-Bus. Its interface is modeled to serve NetworkClient requirements. */ class NetworkManagerAdapter { - /** - * @param {DBusClient} [dbusClient] - D-Bus client - */ - constructor(dbusClient) { - this.client = dbusClient || new DBusClient(SERVICE_NAME, "system"); + constructor() { + this.client = new DBusClient(SERVICE_NAME); /** @type {{[k: string]: string}} */ this.connectionIds = {}; this.proxies = { diff --git a/web/src/client/network/network_manager.test.js b/web/src/client/network/network_manager.test.js index 587c2ebb59..dfa433ede5 100644 --- a/web/src/client/network/network_manager.test.js +++ b/web/src/client/network/network_manager.test.js @@ -22,9 +22,11 @@ import { securityFromFlags, mergeConnectionSettings, NetworkManagerAdapter } from "./network_manager"; import { createConnection } from "./model"; import { ConnectionState, ConnectionTypes } from "./index"; -import { DBusClient } from "../dbus"; +import DBusClient from "../dbus"; import cockpit from "../../lib/cockpit"; +jest.mock("../dbus"); + const NM_IFACE = "org.freedesktop.NetworkManager"; const NM_SETTINGS_IFACE = "org.freedesktop.NetworkManager.Settings"; const IP4CONFIG_IFACE = "org.freedesktop.NetworkManager.IP4Config"; @@ -32,8 +34,6 @@ const NM_CONNECTION_IFACE = "org.freedesktop.NetworkManager.Settings.Connection" const ACTIVE_CONNECTION_IFACE = "org.freedesktop.NetworkManager.Connection.Active"; const ACCESS_POINT_IFACE = "org.freedesktop.NetworkManager.AccessPoint"; -const dbusClient = new DBusClient(""); - const accessPoints = { "/org/freedesktop/NetworkManager/AccessPoint/11" : { Flags: 3, @@ -182,24 +182,27 @@ const connectionSettingsProxy = () => connectionSettingsMock; describe("NetworkManagerAdapter", () => { beforeEach(() => { - dbusClient.proxy = jest.fn().mockImplementation(iface => { - if (iface === NM_IFACE) return networkProxy; - if (iface === NM_SETTINGS_IFACE) return networkSettingsProxy; - if (iface === NM_CONNECTION_IFACE) return connectionSettingsProxy(); - }); - - dbusClient.proxies = jest.fn().mockImplementation(iface => { - if (iface === ACCESS_POINT_IFACE) return accessPoints; - if (iface === ACTIVE_CONNECTION_IFACE) return activeConnections; - if (iface === NM_CONNECTION_IFACE) return connections; - if (iface === IP4CONFIG_IFACE) return addressesData; - return {}; + DBusClient.mockImplementation(() => { + return { + proxy: (iface) => { + if (iface === NM_IFACE) return networkProxy; + if (iface === NM_SETTINGS_IFACE) return networkSettingsProxy; + if (iface === NM_CONNECTION_IFACE) return connectionSettingsProxy(); + }, + proxies: (iface) => { + if (iface === ACCESS_POINT_IFACE) return accessPoints; + if (iface === ACTIVE_CONNECTION_IFACE) return activeConnections; + if (iface === NM_CONNECTION_IFACE) return connections; + if (iface === IP4CONFIG_IFACE) return addressesData; + return {}; + } + }; }); }); describe("#accessPoints", () => { it("returns the list of last scanned access points", async () => { - const client = new NetworkManagerAdapter(dbusClient); + const client = new NetworkManagerAdapter(); await client.setUp(); const accessPoints = client.accessPoints(); @@ -216,7 +219,7 @@ describe("NetworkManagerAdapter", () => { describe("#activeConnections", () => { it("returns the list of active connections", async () => { - const client = new NetworkManagerAdapter(dbusClient); + const client = new NetworkManagerAdapter(); await client.setUp(); const availableConnections = client.activeConnections(); @@ -242,7 +245,7 @@ describe("NetworkManagerAdapter", () => { describe("#connections", () => { it("returns the list of settings (profiles)", async () => { - const client = new NetworkManagerAdapter(dbusClient); + const client = new NetworkManagerAdapter(); await client.setUp(); const connections = await client.connections(); @@ -261,7 +264,7 @@ describe("NetworkManagerAdapter", () => { describe("#getConnection", () => { it("returns the connection with the given ID", async () => { - const client = new NetworkManagerAdapter(dbusClient); + const client = new NetworkManagerAdapter(); const connection = await client.getConnection("uuid-wifi-1"); expect(connection).toEqual({ id: "uuid-wifi-1", @@ -279,7 +282,7 @@ describe("NetworkManagerAdapter", () => { describe("#addConnection", () => { it("adds a connection and activates it", async () => { - const client = new NetworkManagerAdapter(dbusClient); + const client = new NetworkManagerAdapter(); const connection = createConnection({ name: "Wired connection 1" }); await client.addConnection(connection); expect(AddConnectionFn).toHaveBeenCalledWith( @@ -292,7 +295,7 @@ describe("NetworkManagerAdapter", () => { describe("#updateConnection", () => { it("updates the connection", async () => { - const client = new NetworkManagerAdapter(dbusClient); + const client = new NetworkManagerAdapter(); const connection = await client.getConnection("uuid-wifi-1"); connection.ipv4 = { ...connection.ipv4, @@ -321,7 +324,7 @@ describe("NetworkManagerAdapter", () => { describe("#connectTo", () => { it("activates the given connection", async () => { - const client = new NetworkManagerAdapter(dbusClient); + const client = new NetworkManagerAdapter(); await client.setUp(); const [wifi] = await client.connections(); await client.connectTo(wifi); @@ -331,7 +334,7 @@ describe("NetworkManagerAdapter", () => { describe("#addAndConnectTo", () => { it("activates the given connection", async () => { - const client = new NetworkManagerAdapter(dbusClient); + const client = new NetworkManagerAdapter(); await client.setUp(); client.addConnection = jest.fn(); await client.addAndConnectTo("Testing", { security: "wpa-psk", password: "testing.1234" }); @@ -347,7 +350,7 @@ describe("NetworkManagerAdapter", () => { describe("#deleteConnection", () => { it("deletes the given connection", async () => { - const client = new NetworkManagerAdapter(dbusClient); + const client = new NetworkManagerAdapter(); await client.setUp(); const [wifi] = await client.connections(); await client.deleteConnection(wifi); @@ -358,7 +361,7 @@ describe("NetworkManagerAdapter", () => { describe("#settings", () => { it("returns the Network Manager settings hostname", async() => { - const client = new NetworkManagerAdapter(dbusClient); + const client = new NetworkManagerAdapter(); await client.setUp(); expect(client.settings().hostname).toEqual("testing-machine"); expect(client.settings().wireless).toEqual(false); diff --git a/web/src/client/questions.js b/web/src/client/questions.js index 04b6d85030..70bde45091 100644 --- a/web/src/client/questions.js +++ b/web/src/client/questions.js @@ -21,7 +21,7 @@ // @ts-check -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; const QUESTIONS_SERVICE = "org.opensuse.DInstaller.Questions"; @@ -112,10 +112,10 @@ function buildQuestion(dbusQuestion) { */ class QuestionsClient { /** - * @param {DBusClient} [dbusClient] - D-Bus client + * @param {string|undefined} address - D-Bus address; if it is undefined, it uses the system bus. */ - constructor(dbusClient) { - this.client = dbusClient || new DBusClient(QUESTIONS_SERVICE); + constructor(address) { + this.client = new DBusClient(QUESTIONS_SERVICE, address); } /** diff --git a/web/src/client/questions.test.js b/web/src/client/questions.test.js index 4e91d0ac0f..912c328c5a 100644 --- a/web/src/client/questions.test.js +++ b/web/src/client/questions.test.js @@ -19,9 +19,11 @@ * find current contact information at www.suse.com. */ -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; import { QuestionsClient } from "./questions"; +jest.mock("./dbus"); + // NOTE: should we export them? const QUESTION_IFACE = "org.opensuse.DInstaller.Question1"; const LUKS_ACTIVATION_IFACE = "org.opensuse.DInstaller.Question.LuksActivation1"; @@ -98,20 +100,18 @@ const proxies = { [LUKS_ACTIVATION_IFACE]: luksActivationProxy }; -// const dbusClient = { -// call: () => getManagedObjectsMock -// }; - -const dbusClient = new DBusClient(""); - beforeEach(() => { - dbusClient.proxy = jest.fn().mockImplementation(iface => proxies[iface]); - dbusClient.call = () => getManagedObjectsMock; + DBusClient.mockImplementation(() => { + return { + proxy: (iface) => proxies[iface], + call: () => getManagedObjectsMock + }; + }); }); describe("#getQuestions", () => { it("returns pending questions", async () => { - const client = new QuestionsClient(dbusClient); + const client = new QuestionsClient(); const questions = await client.getQuestions(); expect(questions).toEqual(expectedQuestions); }); @@ -125,7 +125,7 @@ describe("#answer", () => { }); it("sets given answer", async () => { - const client = new QuestionsClient(dbusClient); + const client = new QuestionsClient(); await client.answer(question); expect(questionsProxy).toMatchObject({ Answer: 'the-answer' }); @@ -137,7 +137,7 @@ describe("#answer", () => { }); it("sets given password", async () => { - const client = new QuestionsClient(dbusClient); + const client = new QuestionsClient(); await client.answer(question); expect(luksActivationProxy).toMatchObject({ Password: "notSecret" }); diff --git a/web/src/client/software.js b/web/src/client/software.js index 74af8824ad..ec87f2cd68 100644 --- a/web/src/client/software.js +++ b/web/src/client/software.js @@ -21,7 +21,7 @@ // @ts-check -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; import { WithStatus, WithProgress } from "./mixins"; const SOFTWARE_SERVICE = "org.opensuse.DInstaller.Software"; @@ -42,10 +42,10 @@ const SOFTWARE_PATH = "/org/opensuse/DInstaller/Software1"; */ class SoftwareBaseClient { /** - * @param {DBusClient} [dbusClient] - D-Bus client + * @param {string|undefined} address - D-Bus address; if it is undefined, it uses the system bus. */ - constructor(dbusClient) { - this.client = dbusClient || new DBusClient(SOFTWARE_SERVICE); + constructor(address = undefined) { + this.client = new DBusClient(SOFTWARE_SERVICE, address); } /** diff --git a/web/src/client/software.test.js b/web/src/client/software.test.js index 53b1526bbf..6758b36ba0 100644 --- a/web/src/client/software.test.js +++ b/web/src/client/software.test.js @@ -21,9 +21,11 @@ // @ts-check -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; import { SoftwareClient } from "./software"; +jest.mock("./dbus"); + const SOFTWARE_IFACE = "org.opensuse.DInstaller.Software1"; const softProxy = { @@ -35,16 +37,20 @@ const softProxy = { SelectedBaseProduct: "MicroOS" }; -const dbusClient = new DBusClient(""); beforeEach(() => { - dbusClient.proxy = jest.fn().mockImplementation(iface => { - if (iface === SOFTWARE_IFACE) return softProxy; + // @ts-ignore + DBusClient.mockImplementation(() => { + return { + proxy: (iface) => { + if (iface === SOFTWARE_IFACE) return softProxy; + } + }; }); }); describe("#getProducts", () => { it("returns the list of available products", async () => { - const client = new SoftwareClient(dbusClient); + const client = new SoftwareClient(); const availableProducts = await client.getProducts(); expect(availableProducts).toEqual([ { id: "MicroOS", name: "openSUSE MicroOS" }, @@ -55,7 +61,7 @@ describe("#getProducts", () => { describe('#getSelectedProduct', () => { it("returns the selected product", async () => { - const client = new SoftwareClient(dbusClient); + const client = new SoftwareClient(); const selectedProduct = await client.getSelectedProduct(); expect(selectedProduct).toEqual( { id: "MicroOS", name: "openSUSE MicroOS" } diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 2b0266e8ac..4ad89d2be5 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -21,7 +21,7 @@ // @ts-check -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; import { WithStatus, WithValidation } from "./mixins"; import cockpit from "../lib/cockpit"; @@ -37,10 +37,10 @@ const STORAGE_PROPOSAL_PATH = "/org/opensuse/DInstaller/Storage/Proposal1"; */ class StorageBaseClient { /** - * @param {DBusClient} [dbusClient] - D-Bus client + * @param {string|undefined} address - D-Bus address; if it is undefined, it uses the system bus. */ - constructor(dbusClient) { - this.client = dbusClient || new DBusClient(STORAGE_SERVICE); + constructor(address = undefined) { + this.client = new DBusClient(STORAGE_SERVICE, address); } /** diff --git a/web/src/client/storage.test.js b/web/src/client/storage.test.js index 1941e564dc..5a6ad5a56d 100644 --- a/web/src/client/storage.test.js +++ b/web/src/client/storage.test.js @@ -19,15 +19,18 @@ * find current contact information at www.suse.com. */ -import { DBusClient } from "./dbus"; +// @ts-check + +import DBusClient from "./dbus"; import { StorageClient } from "./storage"; +jest.mock("./dbus"); + // NOTE: should we export them? const STORAGE_PROPOSAL_IFACE = "org.opensuse.DInstaller.Storage.Proposal1"; const calculateFn = jest.fn(); -const dbusClient = new DBusClient(""); const storageProposalProxy = { wait: jest.fn(), AvailableDevices: [ @@ -68,14 +71,19 @@ const storageProposalProxy = { }; beforeEach(() => { - dbusClient.proxy = jest.fn().mockImplementation(iface => { - if (iface === STORAGE_PROPOSAL_IFACE) return storageProposalProxy; + // @ts-ignore + DBusClient.mockImplementation(() => { + return { + proxy: (iface) => { + if (iface === STORAGE_PROPOSAL_IFACE) return storageProposalProxy; + } + }; }); }); describe("#getProposal", () => { it("returns the storage proposal settings and actions", async () => { - const client = new StorageClient(dbusClient); + const client = new StorageClient(); const proposal = await client.getProposal(); expect(proposal.availableDevices).toEqual([ { id: "/dev/sda", label: "/dev/sda, 950 GiB, Windows" }, @@ -109,14 +117,14 @@ describe("#getProposal", () => { describe("#calculate", () => { it("calculates a default proposal when no settings are given", async () => { - const client = new StorageClient(dbusClient); + const client = new StorageClient(); await client.calculateProposal({}); expect(calculateFn).toHaveBeenCalledWith({}); }); it("calculates a proposal with the given settings", async () => { - const client = new StorageClient(dbusClient); + const client = new StorageClient(); await client.calculateProposal({ candidateDevices: ["/dev/vda"], encryptionPassword: "12345", diff --git a/web/src/client/users.js b/web/src/client/users.js index b02bf24583..7c451cc610 100644 --- a/web/src/client/users.js +++ b/web/src/client/users.js @@ -21,7 +21,7 @@ // @ts-check -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; import { WithValidation } from "./mixins"; const USERS_SERVICE = "org.opensuse.DInstaller.Users"; @@ -56,10 +56,10 @@ const USERS_PATH = "/org/opensuse/DInstaller/Users1"; */ class UsersBaseClient { /** - * @param {DBusClient} [dbusClient] - D-Bus client + * @param {string|undefined} address - D-Bus address; if it is undefined, it uses the system bus. */ - constructor(dbusClient) { - this.client = dbusClient || new DBusClient(USERS_SERVICE); + constructor(address = undefined) { + this.client = new DBusClient(USERS_SERVICE, address); } /** diff --git a/web/src/client/users.test.js b/web/src/client/users.test.js index 3ee8ad0c9e..e2be9b678f 100644 --- a/web/src/client/users.test.js +++ b/web/src/client/users.test.js @@ -21,12 +21,12 @@ // @ts-check -import { DBusClient } from "./dbus"; +import DBusClient from "./dbus"; import { UsersClient } from "./users"; -const USERS_IFACE = "org.opensuse.DInstaller.Users1"; +jest.mock("./dbus"); -const dbusClient = new DBusClient(""); +const USERS_IFACE = "org.opensuse.DInstaller.Users1"; let setFirstUserResult = [true, []]; @@ -43,14 +43,19 @@ const usersProxy = { }; beforeEach(() => { - dbusClient.proxy = jest.fn().mockImplementation(iface => { - if (iface === USERS_IFACE) return usersProxy; + // @ts-ignore + DBusClient.mockImplementation(() => { + return { + proxy: (iface) => { + if (iface === USERS_IFACE) return usersProxy; + } + }; }); }); describe("#getUser", () => { it("returns the defined first user", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const user = await client.getUser(); expect(user).toEqual({ fullName: "Jane Doe", userName: "jane", autologin: false }); }); @@ -63,7 +68,7 @@ describe("#isRootPasswordSet", () => { }); it("returns true", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.isRootPasswordSet(); expect(result).toEqual(true); }); @@ -75,7 +80,7 @@ describe("#isRootPasswordSet", () => { }); it("returns false", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.isRootPasswordSet(); expect(result).toEqual(false); }); @@ -84,7 +89,7 @@ describe("#isRootPasswordSet", () => { describe("#getRootSSHKey", () => { it("returns the SSH key for the root user", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.getRootSSHKey(); expect(result).toEqual("ssh-key"); }); @@ -92,7 +97,7 @@ describe("#getRootSSHKey", () => { describe("#setUser", () => { it("sets the values of the first user and returns whether succeeded or not an errors found", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.setUser({ fullName: "Jane Doe", userName: "jane", @@ -111,7 +116,7 @@ describe("#setUser", () => { }); it("returns an object with the result as false and the issues found", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.setUser({ fullName: "Jane Doe", userName: "jane", @@ -126,7 +131,7 @@ describe("#setUser", () => { describe("#removeUser", () => { it("removes the first user and returns true", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.removeUser(); expect(usersProxy.RemoveFirstUser).toHaveBeenCalled(); expect(result).toEqual(true); @@ -136,7 +141,7 @@ describe("#removeUser", () => { beforeEach(() => (usersProxy.RemoveFirstUser = jest.fn().mockResolvedValue(1))); it("returns false", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.removeUser(); expect(result).toEqual(false); }); @@ -145,7 +150,7 @@ describe("#removeUser", () => { describe("#setRootPassword", () => { it("sets the root password and returns true", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.setRootPassword("12345"); expect(usersProxy.SetRootPassword).toHaveBeenCalledWith("12345", false); expect(result).toEqual(true); @@ -155,7 +160,7 @@ describe("#setRootPassword", () => { beforeEach(() => (usersProxy.SetRootPassword = jest.fn().mockResolvedValue(1))); it("returns false", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.setRootPassword("12345"); expect(result).toEqual(false); }); @@ -164,7 +169,7 @@ describe("#setRootPassword", () => { describe("#removeRootPassword", () => { it("removes the root password", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.removeRootPassword(); expect(usersProxy.RemoveRootPassword).toHaveBeenCalled(); expect(result).toEqual(true); @@ -174,7 +179,7 @@ describe("#removeRootPassword", () => { beforeEach(() => (usersProxy.RemoveRootPassword = jest.fn().mockResolvedValue(1))); it("returns false", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.removeRootPassword(); expect(result).toEqual(false); }); @@ -183,7 +188,7 @@ describe("#removeRootPassword", () => { describe("#setRootSSHKey", () => { it("sets the root password and returns true", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.setRootSSHKey("ssh-key"); expect(usersProxy.SetRootSSHKey).toHaveBeenCalledWith("ssh-key"); expect(result).toEqual(true); @@ -193,7 +198,7 @@ describe("#setRootSSHKey", () => { beforeEach(() => (usersProxy.SetRootSSHKey = jest.fn().mockResolvedValue(1))); it("returns false", async () => { - const client = new UsersClient(dbusClient); + const client = new UsersClient(); const result = await client.setRootSSHKey("ssh-key"); expect(result).toEqual(false); }); diff --git a/web/src/context/installer.jsx b/web/src/context/installer.jsx index 677dca9bd7..8e978f75be 100644 --- a/web/src/context/installer.jsx +++ b/web/src/context/installer.jsx @@ -21,7 +21,7 @@ // @ts-check -import React from "react"; +import React, { useState, useEffect } from "react"; const InstallerClientContext = React.createContext(undefined); @@ -40,8 +40,22 @@ function useInstallerClient() { } function InstallerClientProvider({ client, children }) { + const [value, setValue] = useState(null); + + useEffect(() => { + if (typeof client === "function") { + client().then(setValue); + } else { + setValue(client); + } + }, [client]); + + if (!value) { + return null; + } + return ( - {children} + {children} ); } diff --git a/web/src/index.js b/web/src/index.js index 174f0098ea..c75599d894 100644 --- a/web/src/index.js +++ b/web/src/index.js @@ -47,11 +47,9 @@ import { ProposalPage as StoragePage } from "@components/storage"; import "./patternfly.scss"; import "./app.scss"; -const client = createClient(); - ReactDOM.render( - + From 4b97d027ba2fe4e9c9b604b772a5e97288b8ddea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 9 Jan 2023 16:00:29 +0000 Subject: [PATCH 12/24] [service] Fix D-Bus permissions --- service/share/dbus.conf | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/service/share/dbus.conf b/service/share/dbus.conf index cde93cfd35..909a61af6c 100644 --- a/service/share/dbus.conf +++ b/service/share/dbus.conf @@ -29,16 +29,20 @@ + + + + + + - - - + From 3fbe599f4bdcfb74f4e7f5fa49419fe03e8e9e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 9 Jan 2023 16:24:29 +0000 Subject: [PATCH 13/24] [service] --- service/bin/d-installer | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/service/bin/d-installer b/service/bin/d-installer index be20a33630..9c48e8d5a4 100755 --- a/service/bin/d-installer +++ b/service/bin/d-installer @@ -63,6 +63,7 @@ def start_service(name) end ORDERED_SERVICES = [:questions, :language, :software, :storage, :users, :manager].freeze +BUS_ADDRESS_FILE = "/run/d-installer/bus.address" # Normally the services are started by D-Bus activation. # This starts all the services sequentially without relying on that. @@ -100,7 +101,10 @@ elsif ["-s", "--status"].include?(ARGV[0]) system "pgrep -fl bin/d-installer.[a-z]" # busctl is prettier but will find nothing until the service files are installed, # hence the pgrep above - system "busctl | grep -E '^NAME|DInstaller'" + exit unless File.exist?(BUS_ADDRESS_FILE) + + address = File.read(BUS_ADDRESS_FILE) + system "busctl --address #{address} | grep -E '^NAME|DInstaller'" elsif ["-k", "--kill"].include?(ARGV[0]) system "pkill -f bin/d-installer.[a-z]" # stop the d-bus daemon From 7446abf81e3c989aa906396e61c3a9674fa69745 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 9 Jan 2023 16:31:14 +0000 Subject: [PATCH 14/24] [service] Improve d-installer -s and -k handling --- service/bin/d-installer | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/service/bin/d-installer b/service/bin/d-installer index 9c48e8d5a4..5f91935540 100755 --- a/service/bin/d-installer +++ b/service/bin/d-installer @@ -64,6 +64,7 @@ end ORDERED_SERVICES = [:questions, :language, :software, :storage, :users, :manager].freeze BUS_ADDRESS_FILE = "/run/d-installer/bus.address" +BUS_PID_FILE = "/run/d-installer/bus.pid" # Normally the services are started by D-Bus activation. # This starts all the services sequentially without relying on that. @@ -109,7 +110,10 @@ elsif ["-k", "--kill"].include?(ARGV[0]) system "pkill -f bin/d-installer.[a-z]" # stop the d-bus daemon # FIXME: use the ServerManager (or the Bus.current instance) to stop the bus - system "cat /run/d-installer/bus.pid | xargs kill -9" + exit unless File.exist?(BUS_PID_FILE) + + pid = File.read(BUS_PID_FILE) + Process.kill("KILL", pid.to_i) else name = ARGV[0] Signal.trap("SIGINT") do From a199091e3c6ba9c5b5c9d915efcaf90d900d333d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 9 Jan 2023 16:34:25 +0000 Subject: [PATCH 15/24] [service] Avoid running the D-Bus servers on tests --- service/test/dinstaller/dbus/server_manager_test.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/service/test/dinstaller/dbus/server_manager_test.rb b/service/test/dinstaller/dbus/server_manager_test.rb index 9a69a05999..63dd2f2753 100644 --- a/service/test/dinstaller/dbus/server_manager_test.rb +++ b/service/test/dinstaller/dbus/server_manager_test.rb @@ -30,6 +30,10 @@ let(:tmpdir) { Dir.mktmpdir } + before do + allow(Cheetah).to receive(:run).and_return("9999") + end + after do FileUtils.remove_entry(tmpdir) end From e9d28971975381fcfb54dd42dd39681cc272f255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 10 Jan 2023 07:49:30 +0000 Subject: [PATCH 16/24] [service] createClient receives the address * The createClient is not async anymore. * The responsability of reading the bus address is moved to the installer context. --- web/src/client/index.js | 8 ++------ web/src/context/installer.jsx | 22 +++++++++++++++++----- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/web/src/client/index.js b/web/src/client/index.js index 8442409e8e..6256dfdae7 100644 --- a/web/src/client/index.js +++ b/web/src/client/index.js @@ -30,7 +30,6 @@ import { UsersClient } from "./users"; import phase from "./phase"; import { QuestionsClient } from "./questions"; import { NetworkClient } from "./network"; -import cockpit from "../lib/cockpit"; const SERVICE_NAME = "org.opensuse.DInstaller"; @@ -49,12 +48,9 @@ const SERVICE_NAME = "org.opensuse.DInstaller"; /** * Creates a D-Installer client * - * @return {Promise} + * @return {InstallerClient} */ -const createClient = async () => { - const file = cockpit.file("/run/d-installer/bus.address", { binary: false }); - const address = await file.read(); - +const createClient = (address = "unix:path=/run/d-installer/bus") => { return { language: new LanguageClient(address), manager: new ManagerClient(address), diff --git a/web/src/context/installer.jsx b/web/src/context/installer.jsx index 8e978f75be..140e4c16aa 100644 --- a/web/src/context/installer.jsx +++ b/web/src/context/installer.jsx @@ -22,6 +22,8 @@ // @ts-check import React, { useState, useEffect } from "react"; +import cockpit from "../lib/cockpit"; +import { createClient } from "@/client"; const InstallerClientContext = React.createContext(undefined); @@ -39,14 +41,24 @@ function useInstallerClient() { return context; } +const BUS_ADDRESS_FILE = "/run/d-installer/bus.address" + +/** + * @param {object} props + * @param {import("@client").InstallerClient|undefined} [props.client] client to connect to + * D-Installer service; if it is undefined, it instantiates a new one using the address + * registered in /run/d-installer/bus.address. + * @param {React.ReactNode} [props.children] - content to display within the provider + */ function InstallerClientProvider({ client, children }) { - const [value, setValue] = useState(null); + const [value, setValue] = useState(client); useEffect(() => { - if (typeof client === "function") { - client().then(setValue); - } else { - setValue(client); + if (client !== undefined) { + const file = cockpit.file(BUS_ADDRESS_FILE); + file.read().then(address => { + setValue(createClient(address)); + }); } }, [client]); From c7fdcf50bf2d90f5181201633822b38886ee151c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 10 Jan 2023 07:53:54 +0000 Subject: [PATCH 17/24] [web] Make ESLint happy --- web/src/client/storage.test.js | 35 ++++++++++--------- web/src/components/network/Network.test.jsx | 2 +- .../components/storage/ProposalPage.test.jsx | 1 - .../storage/ProposalSettingsSection.test.jsx | 8 ++--- .../storage/ProposalTargetSection.test.jsx | 2 +- web/src/context/installer.jsx | 2 +- 6 files changed, 26 insertions(+), 24 deletions(-) diff --git a/web/src/client/storage.test.js b/web/src/client/storage.test.js index 5a6ad5a56d..e538fb4ec0 100644 --- a/web/src/client/storage.test.js +++ b/web/src/client/storage.test.js @@ -45,7 +45,7 @@ const storageProposalProxy = { Optional: { t: "b", v: true }, DeviceType: { t: "s", v: "partition" }, Encrypted: { t: "b", v: false }, - FsTypes: { t: "as", v: [{ t: "s" , v: "Btrfs"} , { t: "s", v: "Ext3"}] }, + FsTypes: { t: "as", v: [{ t: "s", v: "Btrfs" }, { t: "s", v: "Ext3" }] }, FsType: { t: "s", v: "Btrfs" }, MinSize: { t: "x", v: 1024 }, MaxSize: { t: "x", v: 2048 }, @@ -150,21 +150,24 @@ describe("#calculate", () => { CandidateDevices: { t: "as", v: ["/dev/vda"] }, EncryptionPassword: { t: "s", v: "12345" }, LVM: { t: "b", v: true }, - Volumes: { t: "aa{sv}", v: [ - { - MountPoint: { t: "s", v: "/test1" }, - Encrypted: { t: "b", v: false }, - FsType: { t: "s", v: "Btrfs" }, - MinSize: { t: "x", v: 1024 }, - MaxSize: { t: "x", v: 2048 }, - FixedSizeLimits: { t: "b", v: false }, - Snapshots: { t: "b", v: true } - }, - { - MountPoint: { t: "s", v: "/test2" }, - MinSize: { t: "x", v: 1024 } - } - ]} + Volumes: { + t: "aa{sv}", + v: [ + { + MountPoint: { t: "s", v: "/test1" }, + Encrypted: { t: "b", v: false }, + FsType: { t: "s", v: "Btrfs" }, + MinSize: { t: "x", v: 1024 }, + MaxSize: { t: "x", v: 2048 }, + FixedSizeLimits: { t: "b", v: false }, + Snapshots: { t: "b", v: true } + }, + { + MountPoint: { t: "s", v: "/test2" }, + MinSize: { t: "x", v: 1024 } + } + ] + } }); }); }); diff --git a/web/src/components/network/Network.test.jsx b/web/src/components/network/Network.test.jsx index f7c58783ed..78164adf9a 100644 --- a/web/src/components/network/Network.test.jsx +++ b/web/src/components/network/Network.test.jsx @@ -23,7 +23,7 @@ import React from "react"; import { screen, within, waitFor } from "@testing-library/react"; import { installerRender } from "@/test-utils"; import Network from "@components/network/Network"; -import { ConnectionState, ConnectionTypes } from "@client/network"; +import { ConnectionTypes } from "@client/network"; import { createClient } from "@client"; jest.mock("@client"); diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index fdcc2d801a..fe08a337b0 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -70,7 +70,6 @@ describe("when there is no proposal yet", () => { await screen.findByText("Loading proposal"); }); - }); describe("when there is a proposal", () => { diff --git a/web/src/components/storage/ProposalSettingsSection.test.jsx b/web/src/components/storage/ProposalSettingsSection.test.jsx index ab6ba4ffb5..1fd82132a2 100644 --- a/web/src/components/storage/ProposalSettingsSection.test.jsx +++ b/web/src/components/storage/ProposalSettingsSection.test.jsx @@ -31,11 +31,11 @@ const FakeProposalSettingsForm = ({ id, onSubmit }) => { }; return
; -} +}; jest.mock("@components/storage/ProposalSettingsForm", () => FakeProposalSettingsForm); -let proposal = { +const proposal = { candidateDevices: ["/dev/sda"], encryptionPassword: "", lvm: false, @@ -135,7 +135,7 @@ describe("when lvm is selected", () => { describe("when encryption is selected", () => { beforeEach(() => { proposal.lvm = false; - proposal.encryptionPassword = "12345" + proposal.encryptionPassword = "12345"; }); it("renders the proper description for the current settings", () => { @@ -148,7 +148,7 @@ describe("when encryption is selected", () => { describe("when LVM and encryption are selected", () => { beforeEach(() => { proposal.lvm = true; - proposal.encryptionPassword = "12345" + proposal.encryptionPassword = "12345"; }); it("renders the proper description for the current settings", () => { diff --git a/web/src/components/storage/ProposalTargetSection.test.jsx b/web/src/components/storage/ProposalTargetSection.test.jsx index 61f3ee867e..22f20b7417 100644 --- a/web/src/components/storage/ProposalTargetSection.test.jsx +++ b/web/src/components/storage/ProposalTargetSection.test.jsx @@ -31,7 +31,7 @@ const FakeProposalTargetForm = ({ id, onSubmit }) => { }; return ; -} +}; jest.mock("@components/storage/ProposalSummary", () => () => "Proposal summary"); jest.mock("@components/storage/ProposalTargetForm", () => FakeProposalTargetForm); diff --git a/web/src/context/installer.jsx b/web/src/context/installer.jsx index 140e4c16aa..7925ad7273 100644 --- a/web/src/context/installer.jsx +++ b/web/src/context/installer.jsx @@ -41,7 +41,7 @@ function useInstallerClient() { return context; } -const BUS_ADDRESS_FILE = "/run/d-installer/bus.address" +const BUS_ADDRESS_FILE = "/run/d-installer/bus.address"; /** * @param {object} props From 3eafc102a2c1625d3d171748cb68e7c32809ad55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 10 Jan 2023 09:03:33 +0000 Subject: [PATCH 18/24] [doc] Mention that D-Installer uses a dedicated D-Bus server --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a89e36d3b5..c4f1732287 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,8 @@ SUSE is working on its next generation operating system called ALP (Adaptable Li ## Architecture -This project is designed as a service-client system, using D-Bus for process communication. +This project is designed as a service-client system, using a dedicated D-Bus server for process +communication. ![Architecture](./doc/images/architecture.png) From 30705886667b21ecdf0f9158533db241349237b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 10 Jan 2023 10:07:32 +0000 Subject: [PATCH 19/24] [service] Adjust the D-Bus configuration file paths --- README.md | 4 ++-- Rakefile | 7 +++++-- service/lib/dinstaller/dbus/server_manager.rb | 2 +- service/package/gem2rpm.yml | 11 ++++++----- setup.sh | 5 +++-- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index c4f1732287..6c40f74094 100644 --- a/README.md +++ b/README.md @@ -110,8 +110,8 @@ $ sudo zypper in gcc gcc-c++ make openssl-devel ruby-devel augeas-devel npm cock * Setup the D-Installer services: ~~~ -$ sudo cp service/share/dbus.conf /usr/share/dbus-1/system.d/org.opensuse.DInstaller.conf -$ cd service; +$ sudo cp service/share/dbus.conf /usr/share/dbus-1/d-installer.conf +$ cd service $ bundle config set --local path 'vendor/bundle'; $ bundle install $ cd - diff --git a/Rakefile b/Rakefile index 9c0b500e28..c26b038912 100644 --- a/Rakefile +++ b/Rakefile @@ -84,6 +84,8 @@ task package: [] do end end +SERVICES_DIR = "/usr/share/dbus-1/d-installer-services" + # support for patching by the yupdate script, # only when running in the inst-sys or live medium if File.exist?("/.packages.initrd") || `mount`.match?(/^[\w]+ on \/ type overlay/) @@ -97,8 +99,9 @@ if File.exist?("/.packages.initrd") || `mount`.match?(/^[\w]+ on \/ type overlay sh "gem install --local --force --no-format-exec --no-doc --build-root #{destdir.shellescape} d-installer-*.gem" # update the DBus configuration files - sh "cp share/org.opensuse.DInstaller*.service /usr/share/dbus-1/system-services" - sh "cp share/dbus.conf /usr/share/dbus-1/system.d/org.opensuse.DInstaller.conf" + FileUtils.mkdir_p(SERVICES_DIR) + sh "cp share/org.opensuse.DInstaller*.service #{SERVICES_DIR}" + sh "cp share/dbus.conf /usr/share/dbus-1/d-installer.conf" # update the systemd service file source_file = "share/systemd.service" diff --git a/service/lib/dinstaller/dbus/server_manager.rb b/service/lib/dinstaller/dbus/server_manager.rb index 7daf10da9e..4e80e78bc2 100644 --- a/service/lib/dinstaller/dbus/server_manager.rb +++ b/service/lib/dinstaller/dbus/server_manager.rb @@ -106,7 +106,7 @@ def config_file file = File.join(Dir.pwd, "share", "dbus.conf") return file if File.exist?(file) - "/usr/share/dbus-1/system.d/org.opensuse.DInstaller.conf" + "/usr/share/dbus-1/d-installer.conf" end # Returns the path to the file containing the PID diff --git a/service/package/gem2rpm.yml b/service/package/gem2rpm.yml index f3e2975461..291ffeb9fc 100644 --- a/service/package/gem2rpm.yml +++ b/service/package/gem2rpm.yml @@ -16,14 +16,15 @@ Requires: yast2 >= 4.5.20 # ## used by gem2rpm :post_install: |- - install -D -m 0644 %{buildroot}%{gem_base}/gems/%{mod_full_name}/share/dbus.conf %{buildroot}%{_datadir}/dbus-1/system.d/org.opensuse.DInstaller.conf - install --directory %{buildroot}%{_datadir}/dbus-1/system-services - install -m 0644 --target-directory=%{buildroot}%{_datadir}/dbus-1/system-services %{buildroot}%{gem_base}/gems/%{mod_full_name}/share/org.opensuse.DInstaller*.service + install -D -m 0644 %{buildroot}%{gem_base}/gems/%{mod_full_name}/share/dbus.conf %{buildroot}%{_datadir}/dbus-1/d-installer.conf + install --directory %{buildroot}%{_datadir}/dbus-1/d-installer-services + install -m 0644 --target-directory=%{buildroot}%{_datadir}/dbus-1/d-installer-services %{buildroot}%{gem_base}/gems/%{mod_full_name}/share/org.opensuse.DInstaller*.service install -D -m 0644 %{buildroot}%{gem_base}/gems/%{mod_full_name}/share/systemd.service %{buildroot}%{_unitdir}/d-installer.service install -D -m 0644 %{buildroot}%{gem_base}/gems/%{mod_full_name}/etc/d-installer.yaml %{buildroot}%{_sysconfdir}/d-installer.yaml # ## used by gem_packages :main: - :filelist: "%{_datadir}/dbus-1/system.d/org.opensuse.DInstaller.conf\n - %{_datadir}/dbus-1/system-services/org.opensuse.DInstaller*.service\n + :filelist: "%{_datadir}/dbus-1/d-installer.conf\n + %dir %{_datadir}/dbus-1/d-installer-services\n + %{_datadir}/dbus-1/d-installer-services/org.opensuse.DInstaller*.service\n %{_unitdir}/d-installer.service\n %{_sysconfdir}/d-installer.yaml\n" diff --git a/setup.sh b/setup.sh index 61cb709b9e..1b13dacee6 100755 --- a/setup.sh +++ b/setup.sh @@ -16,11 +16,12 @@ sudosed() { # set up the d-installer service MYDIR=$(realpath $(dirname $0)) -sudo cp -v $MYDIR/service/share/dbus.conf /usr/share/dbus-1/system.d/org.opensuse.DInstaller.conf +sudo cp -v $MYDIR/service/share/dbus.conf /usr/share/dbus-1/d-installer.conf ( # D-Bus service activation cd $MYDIR/service/share - DBUSDIR=/usr/share/dbus-1/system-services + DBUSDIR=/usr/share/dbus-1/d-installer-services + mkdir -p $DBUSDIR for SVC in org.opensuse.DInstaller*.service; do sudosed "s@\(Exec\)=/usr/bin/@\1=$MYDIR/service/bin/@" $SVC $DBUSDIR/$SVC done From 47d0f738d29a3d3a62b86178f92262c99e986617 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 10 Jan 2023 10:30:36 +0000 Subject: [PATCH 20/24] Update the changes files --- service/package/rubygem-d-installer.changes | 5 +++++ web/package/cockpit-d-installer.changes | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/service/package/rubygem-d-installer.changes b/service/package/rubygem-d-installer.changes index 3ff65a1c5a..0ec6ac0ca9 100644 --- a/service/package/rubygem-d-installer.changes +++ b/service/package/rubygem-d-installer.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Tue Jan 10 10:29:00 UTC 2023 - Imobach Gonzalez Sosa + +- Use a dedicated D-Bus server (gh#yast/d-installer#384). + ------------------------------------------------------------------- Thu Dec 15 13:15:10 UTC 2022 - Imobach Gonzalez Sosa diff --git a/web/package/cockpit-d-installer.changes b/web/package/cockpit-d-installer.changes index a83353531c..373cff12e6 100644 --- a/web/package/cockpit-d-installer.changes +++ b/web/package/cockpit-d-installer.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Tue Jan 10 10:30:05 UTC 2023 - Imobach Gonzalez Sosa + +- Connect to the dedicated D-Bus server (gh#yast/d-installer#384). + ------------------------------------------------------------------- Mon Jan 2 12:53:50 UTC 2023 - David Diaz From 6f41e604f6b7cc9146996af1360382be3ae99f74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 10 Jan 2023 11:43:37 +0000 Subject: [PATCH 21/24] [service] Fix D-Bus activation --- service/bin/d-installer | 13 +++++++++++++ service/share/dbus.conf | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/service/bin/d-installer b/service/bin/d-installer index 5f91935540..3efe3f5322 100755 --- a/service/bin/d-installer +++ b/service/bin/d-installer @@ -33,6 +33,7 @@ Dir.chdir(__dir__) do require "bundler/setup" end require "dinstaller/dbus/service_runner" +require "dinstaller/dbus/server_manager" # Set up a {Logger} suitable either for terminal (started manually) # or syslog (D-Bus activated) @@ -62,6 +63,13 @@ def start_service(name) service_runner.run end +# Finds the D-Bus server or starts a new instance if it is not running +# +# @return [Fixnum] PID +def find_or_start_dbus_server + DInstaller::DBus::ServerManager.new.find_or_start_server +end + ORDERED_SERVICES = [:questions, :language, :software, :storage, :users, :manager].freeze BUS_ADDRESS_FILE = "/run/d-installer/bus.address" BUS_PID_FILE = "/run/d-installer/bus.pid" @@ -95,6 +103,7 @@ elsif ["-h", "--help"].include?(ARGV[0]) puts "#{me} -h|--help" puts "#{me} -s|--status" puts "#{me} -k|--kill" + puts "#{me} -d|--daemon" puts "#{me} #{ORDERED_SERVICES.sort.join("|")}" elsif ["-s", "--status"].include?(ARGV[0]) # The .[a-z] ensures matching the forked processes @@ -114,7 +123,11 @@ elsif ["-k", "--kill"].include?(ARGV[0]) pid = File.read(BUS_PID_FILE) Process.kill("KILL", pid.to_i) +elsif ["-d", "--daemon"].include?(ARGV[0]) + pid = find_or_start_dbus_server # start the D-Bus server (service will start on demand) + puts pid else + find_or_start_dbus_server # make sure that the D-Bus server is running name = ARGV[0] Signal.trap("SIGINT") do puts "Stopping #{name} service..." diff --git a/service/share/dbus.conf b/service/share/dbus.conf index 909a61af6c..b50c7bb4a5 100644 --- a/service/share/dbus.conf +++ b/service/share/dbus.conf @@ -25,7 +25,7 @@ comment out the element to allow fallback to DBUS_COOKIE_SHA1. --> EXTERNAL - + /usr/share/dbus-1/d-installer-services From 0a8b4aee22a6a5e7774cc779aa8474940334ecb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 10 Jan 2023 15:42:38 +0000 Subject: [PATCH 22/24] [service] Move some logic from bin/d-installer to ServerManager --- service/bin/d-installer | 26 +++++-------------- service/lib/dinstaller/dbus/server_manager.rb | 8 ++++++ .../dinstaller/dbus/server_manager_test.rb | 24 +++++++++++++++++ 3 files changed, 38 insertions(+), 20 deletions(-) diff --git a/service/bin/d-installer b/service/bin/d-installer index 3efe3f5322..4bfa6376ce 100755 --- a/service/bin/d-installer +++ b/service/bin/d-installer @@ -63,16 +63,7 @@ def start_service(name) service_runner.run end -# Finds the D-Bus server or starts a new instance if it is not running -# -# @return [Fixnum] PID -def find_or_start_dbus_server - DInstaller::DBus::ServerManager.new.find_or_start_server -end - ORDERED_SERVICES = [:questions, :language, :software, :storage, :users, :manager].freeze -BUS_ADDRESS_FILE = "/run/d-installer/bus.address" -BUS_PID_FILE = "/run/d-installer/bus.pid" # Normally the services are started by D-Bus activation. # This starts all the services sequentially without relying on that. @@ -90,6 +81,8 @@ def start_all_services end end +dbus_server_manager = DInstaller::DBus::ServerManager.new + if ARGV.empty? start_all_services Signal.trap("SIGINT") do @@ -111,23 +104,16 @@ elsif ["-s", "--status"].include?(ARGV[0]) system "pgrep -fl bin/d-installer.[a-z]" # busctl is prettier but will find nothing until the service files are installed, # hence the pgrep above - exit unless File.exist?(BUS_ADDRESS_FILE) - - address = File.read(BUS_ADDRESS_FILE) - system "busctl --address #{address} | grep -E '^NAME|DInstaller'" + system "busctl --address #{dbus_server_manager.address} | grep -E '^NAME|DInstaller'" elsif ["-k", "--kill"].include?(ARGV[0]) system "pkill -f bin/d-installer.[a-z]" # stop the d-bus daemon - # FIXME: use the ServerManager (or the Bus.current instance) to stop the bus - exit unless File.exist?(BUS_PID_FILE) - - pid = File.read(BUS_PID_FILE) - Process.kill("KILL", pid.to_i) + dbus_server_manager.stop_server elsif ["-d", "--daemon"].include?(ARGV[0]) - pid = find_or_start_dbus_server # start the D-Bus server (service will start on demand) + pid = dbus_server_manager.find_or_start_server # start the D-Bus server (service will start on demand) puts pid else - find_or_start_dbus_server # make sure that the D-Bus server is running + dbus_server_manager.find_or_start_server name = ARGV[0] Signal.trap("SIGINT") do puts "Stopping #{name} service..." diff --git a/service/lib/dinstaller/dbus/server_manager.rb b/service/lib/dinstaller/dbus/server_manager.rb index 4e80e78bc2..e43549c6be 100644 --- a/service/lib/dinstaller/dbus/server_manager.rb +++ b/service/lib/dinstaller/dbus/server_manager.rb @@ -89,6 +89,14 @@ def find_server end end + # Stops the running server + def stop_server + pid = find_server + return unless pid + + Process.kill("KILL", pid.to_i) + end + # Returns the D-Bus address # # @return [String] diff --git a/service/test/dinstaller/dbus/server_manager_test.rb b/service/test/dinstaller/dbus/server_manager_test.rb index 63dd2f2753..200f06e10a 100644 --- a/service/test/dinstaller/dbus/server_manager_test.rb +++ b/service/test/dinstaller/dbus/server_manager_test.rb @@ -76,6 +76,30 @@ end end + describe "#stop_server" do + before do + allow(subject).to receive(:find_server).and_return(pid) + end + + context "when a D-Bus server is running" do + let(:pid) { 1000 } + + it "stops the process" do + expect(Process).to receive(:kill).with("KILL", pid) + subject.stop_server + end + end + + context "when no D-Bus server is running" do + let(:pid) { nil } + + it "does not try to stop the process" do + expect(Process).to_not receive(:kill) + subject.stop_server + end + end + end + describe "#find_server" do context "when a PID file exists" do let(:pid_file_content) { "1000" } From 8dddfd3602ff04e9af0e340cbebd3d29b4d1f5a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 10 Jan 2023 15:57:01 +0000 Subject: [PATCH 23/24] [service] Make RuboCop happy --- service/bin/d-installer | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/service/bin/d-installer b/service/bin/d-installer index 4bfa6376ce..849b658c8c 100755 --- a/service/bin/d-installer +++ b/service/bin/d-installer @@ -110,7 +110,8 @@ elsif ["-k", "--kill"].include?(ARGV[0]) # stop the d-bus daemon dbus_server_manager.stop_server elsif ["-d", "--daemon"].include?(ARGV[0]) - pid = dbus_server_manager.find_or_start_server # start the D-Bus server (service will start on demand) + # start the D-Bus server + pid = dbus_server_manager.find_or_start_server puts pid else dbus_server_manager.find_or_start_server From 144c60c09036566dcb1c8f65ff8ddea737986284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 11 Jan 2023 08:51:08 +0000 Subject: [PATCH 24/24] Update from code review --- service/lib/dinstaller/dbus/server_manager.rb | 2 +- web/src/client/dbus.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/service/lib/dinstaller/dbus/server_manager.rb b/service/lib/dinstaller/dbus/server_manager.rb index e43549c6be..5f05a9a21a 100644 --- a/service/lib/dinstaller/dbus/server_manager.rb +++ b/service/lib/dinstaller/dbus/server_manager.rb @@ -84,7 +84,7 @@ def find_server begin Process.getpgid(pid) pid - rescue Errno::ESRCH + rescue Errno::ESRCH # the process is not found nil end end diff --git a/web/src/client/dbus.js b/web/src/client/dbus.js index 1686b3fef8..47f0256cbd 100644 --- a/web/src/client/dbus.js +++ b/web/src/client/dbus.js @@ -74,13 +74,13 @@ class DBusClient { * to the "system" bus. */ constructor(service, address = undefined) { - const options = { - bus: address === undefined ? "system" : "none", - superuser: "try" - }; + const options = { superuser: "try" }; if (address) { + options.bus = "none"; options.address = address; + } else { + options.bus = "system"; } this.client = cockpit.dbus(service, options);