report: move facts generator out of PID1 into a separate varlink service

The collection of facts is entirely unprivileged and has very little to
do with PID1. PID1 is privileged and single-threaded and a point of
contention, so we shouldn't put things in PID1 that don't need to be
there. A separate service can be enabled/disabled/started/stopped at
will, is easy to sandbox, etc. If it turns out to be necessary to
collect some facts through PID1 in the future, we can always add a
smaller facts endpoint to PID1 again.

diff --git a/src/core/meson.build b/src/core/meson.build
index 353854dafd9a8c625dec4143577ebb016608189e..391dc45a6b294a2c294362e5efb1cb0284881387 100644 (file)
--- a/src/core/meson.build
+++ b/src/core/meson.build
@@ -70,7 +70,6 @@ libcore_sources = files(
         'varlink-execute.c',
         'varlink-manager.c',
         'varlink-metrics.c',
-        'varlink-facts.c',
         'varlink-unit.c',
 )
 

diff --git a/src/core/varlink.c b/src/core/varlink.c
index e4e598a3dae4628997ac9c1cc5b720450210e464..533d1061b8eb7d583b1635c55576d48275e4b581 100644 (file)
--- a/src/core/varlink.c
+++ b/src/core/varlink.c
@@ -5,7 +5,6 @@
 #include "constants.h"
 #include "errno-util.h"
 #include "manager.h"
-#include "facts.h"
 #include "metrics.h"
 #include "path-util.h"
 #include "pidref.h"
@@ -13,7 +12,6 @@
 #include "unit.h"
 #include "varlink.h"
 #include "varlink-dynamic-user.h"
-#include "varlink-facts.h"
 #include "varlink-io.systemd.ManagedOOM.h"
 #include "varlink-io.systemd.Manager.h"
 #include "varlink-io.systemd.Unit.h"
@@ -435,29 +433,16 @@ int manager_setup_varlink_server(Manager *m) {
 }
 
 int manager_setup_varlink_metrics_server(Manager *m) {
-        int r;
-
         assert(m);
 
         sd_varlink_server_flags_t flags = SD_VARLINK_SERVER_INHERIT_USERDATA;
         if (MANAGER_IS_SYSTEM(m))
                 flags |= SD_VARLINK_SERVER_ACCOUNT_UID;
 
-        r = metrics_setup_varlink_server(&m->metrics_varlink_server, flags,
+        return metrics_setup_varlink_server(&m->metrics_varlink_server, flags,
                                             m->event, EVENT_PRIORITY_IPC,
                                             vl_method_list_metrics, vl_method_describe_metrics,
                                             m);
-        if (r < 0)
-                return r;
-        if (r > 0) {
-                /* Server newly created — also register facts interface on it */
-                int q = facts_add_to_varlink_server(m->metrics_varlink_server,
-                                                     vl_method_list_facts, vl_method_describe_facts);
-                if (q < 0)
-                        return q;
-        }
-
-        return r;
 }
 
 static int varlink_server_listen_many_idempotent_sentinel(

diff --git a/src/report/meson.build b/src/report/meson.build
index 2813f9d033b1640976e57b538771546ec925e567..26d1bbfdc3e9c8ccb939146576019fe65fe9a7ea 100644 (file)
--- a/src/report/meson.build
+++ b/src/report/meson.build
@@ -6,4 +6,13 @@ executables += [
                 'public' : true,
                 'sources' : files('report.c'),
         },
+
+        libexec_template + {
+                'name' : 'systemd-report-basic',
+                'public' : true,
+                'sources' : files(
+                        'report-basic-server.c',
+                        'report-basic.c',
+                ),
+        },
 ]

diff --git a/src/report/report-basic-server.c b/src/report/report-basic-server.c
new file mode 100644 (file)
index 0000000..32ec9b0
--- /dev/null
+++ b/src/report/report-basic-server.c
@@ -0,0 +1,97 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "sd-varlink.h"
+
+#include "ansi-color.h"
+#include "build.h"
+#include "facts.h"
+#include "format-table.h"
+#include "log.h"
+#include "main-func.h"
+#include "options.h"
+#include "report-basic.h"
+#include "varlink-util.h"
+
+static int vl_server(void) {
+        _cleanup_(sd_varlink_server_unrefp) sd_varlink_server *vs = NULL;
+        int r;
+
+        r = varlink_server_new(&vs, /* flags= */ 0, /* userdata= */ NULL);
+        if (r < 0)
+                return log_error_errno(r, "Failed to allocate Varlink server: %m");
+
+        r = facts_add_to_varlink_server(vs, vl_method_list_facts, vl_method_describe_facts);
+        if (r < 0)
+                return log_error_errno(r, "Failed to register Facts varlink interface: %m");
+
+        r = sd_varlink_server_loop_auto(vs);
+        if (r < 0)
+                return log_error_errno(r, "Failed to run Varlink event loop: %m");
+
+        return 0;
+}
+
+static int help(void) {
+        _cleanup_(table_unrefp) Table *options = NULL;
+        int r;
+
+        r = option_parser_get_help_table(&options);
+        if (r < 0)
+                return r;
+
+        printf("%s [OPTIONS...]\n"
+               "\n%sGenerate a report describing the current system%s\n"
+               "\n%sOptions:%s\n",
+               program_invocation_short_name,
+               ansi_highlight(),
+               ansi_normal(),
+               ansi_underline(),
+               ansi_normal());
+        table_print(options, stdout);
+
+        return 0;
+}
+
+static int parse_argv(int argc, char *argv[]) {
+        int r;
+
+        assert(argc >= 0);
+        assert(argv);
+
+        OptionParser state = { argc, argv };
+
+        FOREACH_OPTION(&state, c, /* ret_a= */ NULL, /* on_error= */ return c)
+                switch (c) {
+                OPTION_COMMON_HELP:
+                        return help();
+
+                OPTION_COMMON_VERSION:
+                        return version();
+                }
+
+        if (state.optind < argc)
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                       "This program takes no arguments.");
+
+        r = sd_varlink_invocation(SD_VARLINK_ALLOW_ACCEPT);
+        if (r < 0)
+                return log_error_errno(r, "Failed to check if invoked in Varlink mode: %m");
+        if (r == 0)
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                       "This program can only run as a Varlink service.");
+        return 1;
+}
+
+static int run(int argc, char *argv[]) {
+        int r;
+
+        log_setup();
+
+        r = parse_argv(argc, argv);
+        if (r <= 0)
+                return r;
+
+        return vl_server();
+}
+
+DEFINE_MAIN_FUNCTION(run);

diff --git a/src/core/varlink-facts.c b/src/report/report-basic.c
similarity index 88%
rename from src/core/varlink-facts.c
rename to src/report/report-basic.c
index 695080c819c607ebc2019eba17b5811a5233960d..381262dfd49097c57a5f8135e78009793347c441 100644 (file)
--- a/src/core/varlink-facts.c
+++ b/src/report/report-basic.c
@@ -10,7 +10,7 @@
 #include "architecture.h"
 #include "facts.h"
 #include "hostname-setup.h"
-#include "varlink-facts.h"
+#include "report-basic.h"
 #include "virt.h"
 
 static int architecture_generate(FactFamilyContext *context, void *userdata) {
@@ -98,35 +98,35 @@ static int virtualization_generate(FactFamilyContext *context, void *userdata) {
                         virtualization_to_string(v));
 }
 
-const FactFamily fact_family_table[] = {
+static const FactFamily fact_family_table[] = {
         /* Keep facts ordered alphabetically */
         {
-                .name = FACT_IO_SYSTEMD_MANAGER_PREFIX "Architecture",
+                .name = FACT_IO_SYSTEMD_BASIC "Architecture",
                 .description = "CPU architecture",
                 .generate = architecture_generate,
         },
         {
-                .name = FACT_IO_SYSTEMD_MANAGER_PREFIX "BootID",
+                .name = FACT_IO_SYSTEMD_BASIC "BootID",
                 .description = "Current boot ID",
                 .generate = boot_id_generate,
         },
         {
-                .name = FACT_IO_SYSTEMD_MANAGER_PREFIX "Hostname",
+                .name = FACT_IO_SYSTEMD_BASIC "Hostname",
                 .description = "System hostname",
                 .generate = hostname_generate,
         },
         {
-                .name = FACT_IO_SYSTEMD_MANAGER_PREFIX "KernelVersion",
+                .name = FACT_IO_SYSTEMD_BASIC "KernelVersion",
                 .description = "Kernel version",
                 .generate = kernel_version_generate,
         },
         {
-                .name = FACT_IO_SYSTEMD_MANAGER_PREFIX "MachineID",
+                .name = FACT_IO_SYSTEMD_BASIC "MachineID",
                 .description = "Machine ID",
                 .generate = machine_id_generate,
         },
         {
-                .name = FACT_IO_SYSTEMD_MANAGER_PREFIX "Virtualization",
+                .name = FACT_IO_SYSTEMD_BASIC "Virtualization",
                 .description = "Virtualization type",
                 .generate = virtualization_generate,
         },

diff --git a/src/core/varlink-facts.h b/src/report/report-basic.h
similarity index 78%
rename from src/core/varlink-facts.h
rename to src/report/report-basic.h
index 5c303c84558ad7af6bfb72ef771cee54f9742154..b24613edb62ffd48a28c1c17f8c54e47ac048790 100644 (file)
--- a/src/core/varlink-facts.h
+++ b/src/report/report-basic.h
@@ -1,9 +1,9 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
-#include "core-forward.h"
+#include "shared-forward.h"
 
-#define FACT_IO_SYSTEMD_MANAGER_PREFIX "io.systemd.Manager."
+#define FACT_IO_SYSTEMD_BASIC "io.systemd.Basic."
 
 int vl_method_list_facts(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata);
 int vl_method_describe_facts(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata);

diff --git a/src/shared/facts.c b/src/shared/facts.c
index 5a6882c6ac80a528b931472d526c461a7ed2e4a2..7554126e2b808280d417cf530aa38dcecbe55593 100644 (file)
--- a/src/shared/facts.c
+++ b/src/shared/facts.c
@@ -1,10 +1,11 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
+#include "sd-varlink.h"
+
 #include "facts.h"
 #include "json-util.h"
 #include "log.h"
 #include "varlink-io.systemd.Facts.h"
-#include "varlink-util.h"
 
 int facts_add_to_varlink_server(
                 sd_varlink_server *server,

diff --git a/src/shared/options.h b/src/shared/options.h
index 7980b69448b11b21288486dfe5ad6779d0159640..afa17d9e3006fd552272a14731d5eedded44bcf2 100644 (file)
--- a/src/shared/options.h
+++ b/src/shared/options.h
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
+#include "memory-util.h"
 #include "shared-forward.h"
 
 typedef enum OptionFlags {

diff --git a/src/shared/varlink-io.systemd.Facts.c b/src/shared/varlink-io.systemd.Facts.c
index c7cf10290aa6334b6586353b24078950fadc701f..dad1271c7248b206740399d70a27a322674c40e6 100644 (file)
--- a/src/shared/varlink-io.systemd.Facts.c
+++ b/src/shared/varlink-io.systemd.Facts.c
@@ -9,8 +9,9 @@ static SD_VARLINK_DEFINE_ERROR(NoSuchFact);
 static SD_VARLINK_DEFINE_METHOD_FULL(
                 List,
                 SD_VARLINK_REQUIRES_MORE,
-                SD_VARLINK_FIELD_COMMENT("Fact family name, e.g. io.systemd.Manager.Hostname"),
+                SD_VARLINK_FIELD_COMMENT("Fact family name, e.g. io.systemd.Basic.Hostname"),
                 SD_VARLINK_DEFINE_OUTPUT(name, SD_VARLINK_STRING, 0),
+                /* This is currently an unused placeholder. Add examples when we have them. */
                 SD_VARLINK_FIELD_COMMENT("Fact object name"),
                 SD_VARLINK_DEFINE_OUTPUT(object, SD_VARLINK_STRING, SD_VARLINK_NULLABLE),
                 SD_VARLINK_FIELD_COMMENT("Fact value"),
@@ -19,7 +20,7 @@ static SD_VARLINK_DEFINE_METHOD_FULL(
 static SD_VARLINK_DEFINE_METHOD_FULL(
                 Describe,
                 SD_VARLINK_REQUIRES_MORE,
-                SD_VARLINK_FIELD_COMMENT("Fact family name"),
+                SD_VARLINK_FIELD_COMMENT("Fact family name, e.g. io.systemd.Basic.Hostname"),
                 SD_VARLINK_DEFINE_OUTPUT(name, SD_VARLINK_STRING, 0),
                 SD_VARLINK_FIELD_COMMENT("Fact family description"),
                 SD_VARLINK_DEFINE_OUTPUT(description, SD_VARLINK_STRING, 0));

diff --git a/test/units/TEST-74-AUX-UTILS.report.sh b/test/units/TEST-74-AUX-UTILS.report.sh
index 4edb67a3150635e744f268a4f2b547b7a0585b0b..0b9006e0590e0852cf15d850601bc506a45dfb1c 100755 (executable)
--- a/test/units/TEST-74-AUX-UTILS.report.sh
+++ b/test/units/TEST-74-AUX-UTILS.report.sh
@@ -36,6 +36,9 @@ varlinkctl list-methods /run/systemd/report/io.systemd.Network
 varlinkctl --more call /run/systemd/report/io.systemd.Network io.systemd.Metrics.List {}
 varlinkctl --more call /run/systemd/report/io.systemd.Network io.systemd.Metrics.Describe {}
 
+# Make sure the service for "system facts" is enabled
+systemctl start systemd-report-basic.socket
+
 # Test facts verbs
 "$REPORT" facts
 "$REPORT" facts -j
@@ -53,5 +56,5 @@ varlinkctl --more call /run/systemd/report/io.systemd.Network io.systemd.Metrics
 "$REPORT" describe-facts piff
 
 # Test facts via direct Varlink call on existing socket
-varlinkctl --more call /run/systemd/report/io.systemd.Manager io.systemd.Facts.List {}
-varlinkctl --more call /run/systemd/report/io.systemd.Manager io.systemd.Facts.Describe {}
+varlinkctl --more call /run/systemd/report/io.systemd.Basic io.systemd.Facts.List {}
+varlinkctl --more call /run/systemd/report/io.systemd.Basic io.systemd.Facts.Describe {}

diff --git a/units/meson.build b/units/meson.build
index 774c02c0ac4fdd00c89f418b8e842b012a2848bd..02c2db074c259a47f77fc7782ecb3b5144c46762 100644 (file)
--- a/units/meson.build
+++ b/units/meson.build
@@ -624,6 +624,8 @@ units = [
           'conditions' : ['ENABLE_BOOTLOADER', 'HAVE_OPENSSL', 'HAVE_TPM2'],
           'symlinks' : ['sysinit.target.wants/'],
         },
+        { 'file' : 'systemd-report-basic.socket' },
+        { 'file' : 'systemd-report-basic@.service.in' },
         {
           'file' : 'systemd-tpm2-clear.service.in',
           'conditions' : ['ENABLE_BOOTLOADER', 'HAVE_OPENSSL', 'HAVE_TPM2'],

diff --git a/units/systemd-report-basic.socket b/units/systemd-report-basic.socket
new file mode 100644 (file)
index 0000000..bce9309
--- /dev/null
+++ b/units/systemd-report-basic.socket
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+#
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+[Unit]
+Description=Report System Basic Facts Socket
+DefaultDependencies=no
+Before=sockets.target
+
+[Socket]
+ListenStream=/run/systemd/report/io.systemd.Basic
+FileDescriptorName=varlink
+SocketMode=0666
+Accept=yes
+RemoveOnStop=yes
+
+[Install]
+WantedBy=sockets.target

diff --git a/units/systemd-report-basic@.service.in b/units/systemd-report-basic@.service.in
new file mode 100644 (file)
index 0000000..ad4e3fc
--- /dev/null
+++ b/units/systemd-report-basic@.service.in
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+#
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+[Unit]
+Description=Report System Basic Facts
+
+[Service]
+ExecStart={{LIBEXECDIR}}/systemd-report-basic