From 2de10ba184d2816d2bdd3de4f745175069dc8a19 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Thu, 23 Apr 2026 08:12:36 -0700 Subject: [PATCH 01/10] flux-imp-exec-helper: convert to C Problem: the IMP helper needs to support device containment options, which will be difficult to implement in a shell script. Convert to a built-in C command with no functional changes. The hyphens in the name won't work in a built-in, so use underscores. Update tests and job-exec to use the new name. --- src/cmd/Makefile.am | 4 +- src/cmd/builtin/imp_exec_helper.c | 88 ++++++++++++++++++++++++++++ src/cmd/flux-imp-exec-helper | 20 ------- src/modules/job-exec/exec.c | 2 +- src/test/docker/poison-libflux.sh.in | 2 +- t/t0800-completion.t | 2 +- 6 files changed, 93 insertions(+), 25 deletions(-) create mode 100644 src/cmd/builtin/imp_exec_helper.c delete mode 100755 src/cmd/flux-imp-exec-helper diff --git a/src/cmd/Makefile.am b/src/cmd/Makefile.am index 33777b6ef991..ab86ee48cadf 100644 --- a/src/cmd/Makefile.am +++ b/src/cmd/Makefile.am @@ -81,7 +81,8 @@ flux_SOURCES = \ builtin/shutdown.c \ builtin/lptest.c \ builtin/fsck.c \ - builtin/cgroup.c + builtin/cgroup.c \ + builtin/imp_exec_helper.c nodist_flux_SOURCES = \ builtin-cmds.c @@ -123,7 +124,6 @@ dist_fluxcmd_SCRIPTS = \ flux-cancel.py \ flux-watch.py \ flux-update.py \ - flux-imp-exec-helper \ py-runner.py \ flux-hostlist.py \ flux-post-job-event.py \ diff --git a/src/cmd/builtin/imp_exec_helper.c b/src/cmd/builtin/imp_exec_helper.c new file mode 100644 index 000000000000..f880acfeaeb7 --- /dev/null +++ b/src/cmd/builtin/imp_exec_helper.c @@ -0,0 +1,88 @@ +/************************************************************\ + * Copyright 2026 Lawrence Livermore National Security, LLC + * (c.f. AUTHORS, NOTICE.LLNS, COPYING) + * + * This file is part of the Flux resource manager framework. + * For details, see https://github.com/flux-framework. + * + * SPDX-License-Identifier: LGPL-3.0 +\************************************************************/ + +/* imp_exec_helper.c - produce IMP exec helper JSON on stdout + * + * Usage: flux imp_exec_helper JOBID + * + * Fetch the signed jobspec (J) for JOBID and emit it as JSON to stdout + * for consumption by the IMP exec helper protocol (RFC 15). + */ + +#if HAVE_CONFIG_H +#include "config.h" +#endif +#include +#include + +#include "builtin.h" + +static int helper (optparse_t *p, int ac, char *av[]) +{ + int optindex; + flux_t *h; + flux_future_t *f; + flux_jobid_t id; + const char *idstr = NULL; + const char *J; + json_t *imp_input; + + log_init ("imp_exec_helper"); + + optindex = optparse_option_index (p); + if (optindex != ac && optindex != ac - 1) { + optparse_print_usage (p); + exit (1); + } + if (optindex < ac) + idstr = av[optindex]; + else if (!(idstr = getenv ("FLUX_JOB_ID"))) + log_msg_exit ("Unable to determine jobid"); + if (flux_job_id_parse (idstr, &id) < 0) + log_err_exit ("invalid jobid: %s", idstr); + + h = builtin_get_flux_handle (p); + + if (!(f = flux_rpc_pack (h, + "job-info.lookup", + FLUX_NODEID_ANY, + 0, + "{s:I s:[s] s:i}", + "id", id, + "keys", "J", + "flags", 0)) + || flux_rpc_get_unpack (f, "{s:s}", "J", &J) < 0) + log_err_exit ("error looking up J: %s", future_strerror (f, errno)); + + if (!(imp_input = json_pack ("{s:s}", "J", J))) + log_msg_exit ("error creating imp input object"); + + if (json_dumpf (imp_input, stdout, JSON_COMPACT) < 0) + log_msg_exit ("error writing imp input object to stdout"); + + json_decref (imp_input); + flux_future_destroy (f); + return 0; +} + +int subcommand_imp_exec_helper_register (optparse_t *p) +{ + optparse_err_t e; + e = optparse_reg_subcommand (p, + "imp_exec_helper", + helper, + "JOBID", + "Emit IMP exec helper JSON for JOBID", + 0, + NULL); + return (e == OPTPARSE_SUCCESS ? 0 : -1); +} + +// vi: ts=4 sw=4 expandtab diff --git a/src/cmd/flux-imp-exec-helper b/src/cmd/flux-imp-exec-helper deleted file mode 100755 index 917514a152b9..000000000000 --- a/src/cmd/flux-imp-exec-helper +++ /dev/null @@ -1,20 +0,0 @@ -#!/bin/sh -############################################################## -# Copyright 2023 Lawrence Livermore National Security, LLC -# (c.f. AUTHORS, NOTICE.LLNS, COPYING) -# -# This file is part of the Flux resource manager framework. -# For details, see https://github.com/flux-framework. -# -# SPDX-License-Identifier: LGPL-3.0 -############################################################## -# -# Helper for flux-imp exec functionality. -# Emit input to IMP for jobid on stdout given jobid in $1 -# -JOBID=${1:-$FLUX_JOB_ID} -if test -z "$JOBID"; then - echo "flux-imp-exec-helper: Unable to determine jobid" >&2 - exit 1 -fi -printf '{"J": "%s"}' $(flux job info --orig $JOBID J) diff --git a/src/modules/job-exec/exec.c b/src/modules/job-exec/exec.c index d79ebffb2494..759ca5e96503 100644 --- a/src/modules/job-exec/exec.c +++ b/src/modules/job-exec/exec.c @@ -651,7 +651,7 @@ static int exec_init (struct jobinfo *job) if (flux_cmd_setenvf (cmd, 1, "FLUX_IMP_EXEC_HELPER", - "flux imp-exec-helper %ju", + "flux imp_exec_helper %ju", (uintmax_t) job->id) < 0) { flux_log_error (job->h, "exec_init: flux_cmd_setenvf"); goto err; diff --git a/src/test/docker/poison-libflux.sh.in b/src/test/docker/poison-libflux.sh.in index 7a4267aa5155..05b76d540316 100644 --- a/src/test/docker/poison-libflux.sh.in +++ b/src/test/docker/poison-libflux.sh.in @@ -101,7 +101,7 @@ for cmd in \ dump module-python-exec start env multi-prog startlog event overlay \ submit exec terminus fortune top fsck pgrep update getattr ping uptime \ heaptrace pkill uri help pmi hostlist post-job-event housekeeping \ - proxy imp-exec-helper pstree job python version job-exec-override watch + proxy imp_exec_helper pstree job python version job-exec-override watch do cat <<-EOF >${CMDDIR}/flux-${cmd} #!/bin/sh diff --git a/t/t0800-completion.t b/t/t0800-completion.t index f647f3d6e63d..df4fe3ae9f10 100755 --- a/t/t0800-completion.t +++ b/t/t0800-completion.t @@ -489,7 +489,7 @@ test_expect_success 'completion coverage: all commands covered or explicitly exc job-validator \ multi-prog \ slurm-expiration-sync \ - imp-exec-helper \ + imp_exec_helper \ run-epilog \ run-prolog \ run-housekeeping \ From 93224b90f40f5196f31bcfe94cb4170784544618 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Thu, 23 Apr 2026 15:23:36 -0700 Subject: [PATCH 02/10] sdbus: add read support for a(ss) variants Problem: the DeviceAllow systemd unit property has D-Bus type a(ss) (array of string-pair structs), which sdbus cannot currently decode from a variant wrapper. Add sdmsg_get_struct_ss to decode a single (ss) struct into a two-element JSON array, extend sdmsg_get to dispatch on "(ss)", and handle the a(ss) variant case explicitly in sdmsg_get_variant and sdmsg_read. Add a unit test and update the capability table in the sdbus guide. Co-Authored-By: Claude Sonnet 4.6 --- doc/guide/sdbus.rst | 9 +++--- src/modules/sdbus/message.c | 26 ++++++++++++++++ src/modules/sdbus/test/message.c | 52 ++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/doc/guide/sdbus.rst b/doc/guide/sdbus.rst index ccfc811ec230..e584f5b7dbdc 100644 --- a/doc/guide/sdbus.rst +++ b/doc/guide/sdbus.rst @@ -89,7 +89,8 @@ variant arrays. - integer * - variant - v - - ``["type", value]``, e.g. ``["s", "active"]`` + - ``["type", value]``, e.g. ``["s", "active"]``; + ``a(ss)`` variants are also supported * - array of basic type - aX - JSON array, e.g. ``[1, 2, 3]`` for ``ai`` @@ -118,10 +119,10 @@ extending ``message.c`` and registering the new signature in ``interface.c``. * - Basic types (y, b, n, q, i, u, x, t, h, d, s, g, o) - yes - yes - * - Variant (v) with basic or simple array content + * - Variant (v) with basic or simple array content, or a(ss) content - yes - yes - * - Variant (v) with complex content + * - Variant (v) with other complex content - null - yes * - Array of basic type (aX) @@ -130,7 +131,7 @@ extending ``message.c`` and registering the new signature in ``interface.c``. * - Property dictionary (a{sv}) - yes - no - * - a(sv), a(sasb), a(ss) + * - a(sv), a(sasb) - no - yes * - Arbitrary struct (...) diff --git a/src/modules/sdbus/message.c b/src/modules/sdbus/message.c index e0dc9592b65e..b6daa2d7cfd6 100644 --- a/src/modules/sdbus/message.c +++ b/src/modules/sdbus/message.c @@ -360,6 +360,26 @@ static int sdmsg_get_array (sd_bus_message *m, const char *fmt, json_t **op) return 1; } +static int sdmsg_get_struct_ss (sd_bus_message *m, json_t **op) +{ + const char *s1, *s2; + json_t *o; + int e; + + if ((e = sd_bus_message_enter_container (m, 'r', "ss")) <= 0) + return e; + if ((e = sd_bus_message_read (m, "ss", &s1, &s2)) <= 0) + return e; + if (!(o = json_pack ("[ss]", s1, s2))) + return -ENOMEM; + if ((e = sd_bus_message_exit_container (m)) < 0) { + json_decref (o); + return e; + } + *op = o; + return 1; +} + static int sdmsg_get_unknown (sd_bus_message *m, const char *fmt, json_t **op) { int e; @@ -387,6 +407,8 @@ static int sdmsg_get_variant (sd_bus_message *m, json_t **op) e = sdmsg_get_basic (m, contents[0], &val); else if (strlen (contents) == 2 && contents[0] == 'a') e = sdmsg_get_array (m, contents + 1, &val); + else if (streq (contents, "a(ss)")) + e = sdmsg_get_array (m, "(ss)", &val); else e = sdmsg_get_unknown (m, contents, &val); if (e <= 0) @@ -449,6 +471,8 @@ int sdmsg_get (sd_bus_message *m, const char *fmt, json_t **op) e = sdmsg_get_array (m, fmt + 1, op); else if (streq (fmt, "v")) e = sdmsg_get_variant (m, op); + else if (streq (fmt, "(ss)")) + e = sdmsg_get_struct_ss (m, op); else if (strlen (fmt) == 1) e = sdmsg_get_basic (m, fmt[0], op); else @@ -465,6 +489,8 @@ int sdmsg_read (sd_bus_message *m, const char *fmt, json_t *o) if (strstarts (&fmt[i], "a{sv}")) efmt = strndup (&fmt[i], 5); + else if (strstarts (&fmt[i], "(ss)")) + efmt = strndup (&fmt[i], 4); else if (fmt[i] == 'a' && strlen (&fmt[i]) > 1) efmt = strndup (&fmt[i], 2); else diff --git a/src/modules/sdbus/test/message.c b/src/modules/sdbus/test/message.c index 2b656daff977..c324fb764f0d 100644 --- a/src/modules/sdbus/test/message.c +++ b/src/modules/sdbus/test/message.c @@ -304,6 +304,57 @@ void test_variant_as (sd_bus *bus) sd_bus_message_unref (m); } +/* Convert an a(ss) variant (e.g. DeviceAllow property) from dbus->json. + * Verify that each (ss) pair is decoded as a two-element JSON array. + */ +void test_variant_ass (sd_bus *bus) +{ + sd_bus_message *m; + json_t *out; + const char *fmt = "v"; + int rc; + + if (sd_bus_message_new (bus, &m, SD_BUS_MESSAGE_METHOD_RETURN) < 0 + || sd_bus_message_open_container (m, 'v', "a(ss)") < 0 + || sd_bus_message_open_container (m, 'a', "(ss)") < 0 + || sd_bus_message_open_container (m, 'r', "ss") < 0 + || sd_bus_message_append (m, "ss", "/dev/nvidiactl", "rw") < 0 + || sd_bus_message_close_container (m) < 0 + || sd_bus_message_open_container (m, 'r', "ss") < 0 + || sd_bus_message_append (m, "ss", "/dev/nvidia0", "r") < 0 + || sd_bus_message_close_container (m) < 0 + || sd_bus_message_close_container (m) < 0 + || sd_bus_message_close_container (m) < 0 + || sd_bus_message_seal (m, 42, 0) < 0 + || sd_bus_message_rewind (m, true) < 0) + BAIL_OUT ("could not create a(ss) variant message"); + diagmsg (m); + + if (!(out = json_array ())) + BAIL_OUT ("could not create json array"); + rc = sdmsg_read (m, fmt, out); + diag ("sdmsg_read returned %d", rc); + ok (rc == 1, + "sdmsg_read works on a(ss) variant"); + diagjson (out); + + const char *type, *path1, *perms1, *path2, *perms2; + ok (json_unpack (out, + "[[s[[ss][ss]]]]", + &type, + &path1, &perms1, + &path2, &perms2) == 0 + && streq (type, "a(ss)") + && streq (path1, "/dev/nvidiactl") + && streq (perms1, "rw") + && streq (path2, "/dev/nvidia0") + && streq (perms2, "r"), + "a(ss) variant decoded as expected"); + + json_decref (out); + sd_bus_message_unref (m); +} + /* In property dicts (e.g. GetAll) we don't know how to decode all values yet. * It seems most sane to decode keys with a JSON null value rather than omit * those keys. Create an sdbus message containing complex variants, then @@ -537,6 +588,7 @@ int main (int argc, char **argv) test_struct_sasb (bus); test_variant (bus); test_variant_as (bus); + test_variant_ass (bus); test_variant_unknown (bus); test_property_array (bus); test_str_pair_array (bus); From 64a707dc7a0944ab6582e132ce61050733b97c75 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Thu, 23 Apr 2026 15:39:16 -0700 Subject: [PATCH 03/10] sdbus: add GetUnitByInvocationID to manager interface table Problem: fetching device containment properties for a running systemd unit requires resolving the unit object path from its invocation ID first, but GetUnitByInvocationID is not yet registered in the sdbus interface table. Add the entry using the generic callbacks; the method takes a byte array (ay) and returns an object path (o), both of which are already handled generically. Co-Authored-By: Claude Sonnet 4.6 --- src/modules/sdbus/interface.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/modules/sdbus/interface.c b/src/modules/sdbus/interface.c index 11437b85d658..53907d24e886 100644 --- a/src/modules/sdbus/interface.c +++ b/src/modules/sdbus/interface.c @@ -169,6 +169,10 @@ static const struct xtab managertab[] = { "ssa(sv)a(sa(sv))", start_transient_unit_fromjson, "o", generic_tojson, }, + { "GetUnitByInvocationID", + "ay", generic_fromjson, + "o", generic_tojson, + }, }; static const struct xtab dbustab[] = { From d772bfffb25a80a0189aa555b8135b26d5c5344c Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Thu, 23 Apr 2026 15:42:28 -0700 Subject: [PATCH 04/10] sdbus: document supported D-Bus methods Problem: the sdbus guide describes type translation and the RPC protocol but does not enumerate which (interface, member) pairs are actually registered, leaving users to read interface.c to find out what can be called. Add a Supported Methods table listing all registered interfaces and members with their input/output type signatures, and add the new method names to the spellcheck wordlist. Co-Authored-By: Claude Sonnet 4.6 --- doc/guide/sdbus.rst | 68 +++++++++++++++++++++++++++++++++++++++++++ doc/test/spell.en.pws | 2 ++ 2 files changed, 70 insertions(+) diff --git a/doc/guide/sdbus.rst b/doc/guide/sdbus.rst index e584f5b7dbdc..9cc508627b62 100644 --- a/doc/guide/sdbus.rst +++ b/doc/guide/sdbus.rst @@ -202,6 +202,74 @@ RPC Interface Signal parameters as a JSON array. +Supported Methods +================= + +The following ``(interface, member)`` pairs are registered in ``interface.c`` +and may be called via ``sdbus.call``. Signatures use D-Bus notation; ``o`` +is an object path, ``ay`` is a byte array. + +.. list-table:: + :header-rows: 1 + :widths: 45 27 10 10 + + * - Interface + - Member + - In + - Out + * - org.freedesktop.systemd1.Manager + - Subscribe + - + - + * - org.freedesktop.systemd1.Manager + - Unsubscribe + - + - + * - org.freedesktop.systemd1.Manager + - ListUnitsByPatterns + - asas + - a(ssssssouso) + * - org.freedesktop.systemd1.Manager + - KillUnit + - ssi + - + * - org.freedesktop.systemd1.Manager + - StopUnit + - ss + - o + * - org.freedesktop.systemd1.Manager + - ResetFailedUnit + - s + - + * - org.freedesktop.systemd1.Manager + - StartTransientUnit + - ssa(sv)a(sa(sv)) + - o + * - org.freedesktop.systemd1.Manager + - GetUnitByInvocationID + - ay + - o + * - org.freedesktop.DBus + - AddMatch + - s + - + * - org.freedesktop.DBus + - RemoveMatch + - s + - + * - org.freedesktop.DBus.Properties + - GetAll + - s + - a{sv} + * - org.freedesktop.DBus.Properties + - Get + - ss + - v + * - org.freedesktop.DBus.Properties + - PropertiesChanged (signal) + - + - sa{sv}as + Exploring the D-Bus Interface ============================= diff --git a/doc/test/spell.en.pws b/doc/test/spell.en.pws index 2106f59d9058..0fd102290f3c 100644 --- a/doc/test/spell.en.pws +++ b/doc/test/spell.en.pws @@ -1117,6 +1117,7 @@ ExecMainStatus freedesktop frominfo GetAll +GetUnitByInvocationID ioencode KillUnit libsdexec @@ -1125,6 +1126,7 @@ LINEBUF ListUnitsByPatterns objpath PropertiesChanged +RemoveMatch ResetFailedUnit sa sdbus's From 10a12af81b5147ad11f833b4e197649053b7ad23 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Sat, 25 Apr 2026 06:27:59 -0700 Subject: [PATCH 05/10] sdexec: don't propagate INVOCATION_ID Problem: INVOCATION_ID is wrong in the systemd unit when job-exec starts it. job-exec sets up the IMP's environment starting with the leader broker environment. When the broker is started by systemd, INVOCATION_ID refers to the leader flux system unit. Clear INVOCATION_ID so that the value set by systemd for the transient unit is not clobbered, similar to how NOTIFY_SOCKET is handled. --- src/modules/sdexec/sdexec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/modules/sdexec/sdexec.c b/src/modules/sdexec/sdexec.c index 9f35cbf80a39..58535253b7ab 100644 --- a/src/modules/sdexec/sdexec.c +++ b/src/modules/sdexec/sdexec.c @@ -745,10 +745,12 @@ static struct sdproc *sdproc_create (struct sdexec_ctx *ctx, if (!(proc->unit = sdexec_unit_create (name))) goto error; /* Ensure that FLUX_URI refers to the local broker. + * Ensure systemd variables refer to the local systemd. */ if (set_dict (proc->cmd, "env", "FLUX_URI", ctx->local_uri) < 0) goto error; - unset_dict (proc->cmd, "env", "NOTIFY_SOCKET"); // see sd_notify(3) + unset_dict (proc->cmd, "env", "NOTIFY_SOCKET"); + unset_dict (proc->cmd, "env", "INVOCATION_ID"); /* Create channels for stdio as required by flags. */ if (!(proc->in = sdexec_channel_create_input (ctx->h, "stdin"))) From e3197373f8cb00ee2b0fe042f942613a6c785723 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Thu, 23 Apr 2026 15:52:54 -0700 Subject: [PATCH 06/10] imp_exec_helper: add device containment options Problem: the IMP exec helper doesn't pass DevicePolicy and DeviceAllow from systemd to the IMP, so the IMP doesn't know how to restrict device access. If INVOCATION_ID is set in the environment, assume the IMP is being launched in a flux-owned systemd unit and the DevicePolicy and DeviceAllow properties should be relayed to the IMP. Look up the unit name using INVOCATION_ID, then fetch the two properties and add them to the options key (see RFC 15) in the IMP input object. If INVOCATION_ID is not set, skip setting these options, which the IMP will interpret as "allow all". Co-Authored-By: Claude Sonnet 4.6 --- src/cmd/builtin/imp_exec_helper.c | 195 ++++++++++++++++++++++++++---- 1 file changed, 171 insertions(+), 24 deletions(-) diff --git a/src/cmd/builtin/imp_exec_helper.c b/src/cmd/builtin/imp_exec_helper.c index f880acfeaeb7..d0aff6a02a83 100644 --- a/src/cmd/builtin/imp_exec_helper.c +++ b/src/cmd/builtin/imp_exec_helper.c @@ -14,6 +14,11 @@ * * Fetch the signed jobspec (J) for JOBID and emit it as JSON to stdout * for consumption by the IMP exec helper protocol (RFC 15). + * + * If INVOCATION_ID is set (i.e. we are running inside a systemd unit), + * also query the unit's DevicePolicy and DeviceAllow properties via + * sdbus and include an "options" object so the IMP can configure device + * containment. */ #if HAVE_CONFIG_H @@ -22,33 +27,133 @@ #include #include +#include "ccan/str/str.h" +#include "ccan/str/hex/hex.h" #include "builtin.h" -static int helper (optparse_t *p, int ac, char *av[]) +static const char *unit_interface = "org.freedesktop.systemd1.Service"; +static const char *prop_interface = "org.freedesktop.DBus.Properties"; + +static struct optparse_option helper_opts[] = { + { .name = "test-nojob", .key = 0, .has_arg = 0, + .usage = "Skip J lookup, emit options only (for testing)" }, + OPTPARSE_TABLE_END, +}; + +/* Decode a 32-char hex invocation ID string to a JSON array of 16 bytes. + * The caller must json_decref() the result. + */ +static json_t *invocation_id_decode (const char *hex) { - int optindex; - flux_t *h; + unsigned char data[16]; + json_t *a; + + if (!hex_decode (hex, strlen (hex), data, sizeof (data))) + log_msg_exit ("invalid INVOCATION_ID: %s", hex); + if (!(a = json_array ())) + log_err_exit ("error creating INVOCATION_ID byte array"); + for (int i = 0; i < sizeof (data); i++) { + if (json_array_append_new (a, json_integer (data[i])) < 0) + log_err_exit ("error building INVOCATION_ID byte array"); + } + return a; +} + +/* Call GetUnitByInvocationID and return the decoded unit object path. + * The caller must free() the result. + */ +static char *get_unit_path (flux_t *h, const char *invocation_id) +{ + json_t *bytes; + flux_future_t *f; + const char *path; + char *result; + + bytes = invocation_id_decode (invocation_id); + if (!(f = flux_rpc_pack (h, + "sdbus.call", + FLUX_NODEID_ANY, + 0, + "{s:s s:[o]}", + "member", "GetUnitByInvocationID", + "params", bytes)) + || flux_rpc_get_unpack (f, "{s:[s]}", "params", &path) < 0) + log_msg_exit ("GetUnitByInvocationID: %s", future_strerror (f, errno)); + if (!(result = strdup (path))) + log_err_exit ("error duplicating unit path"); + json_decref (bytes); + flux_future_destroy (f); + return result; +} + +/* Add DevicePolicy, DeviceAllow to options if the IMP is running + * under systemd. INVOCATION_ID is set in the environment if so. + */ +static void options_add_device (json_t *options, flux_t *h) +{ + const char *invocation_id; + char *unit_path; flux_future_t *f; + flux_future_t *f2; + const char *vtype; + const char *policy; + json_t *allow; + + if (!(invocation_id = getenv ("INVOCATION_ID"))) + return; + unit_path = get_unit_path (h, invocation_id); + + if (!(f = flux_rpc_pack (h, + "sdbus.call", + FLUX_NODEID_ANY, + 0, + "{s:s s:s s:s s:[ss]}", + "path", unit_path, + "interface", prop_interface, + "member", "Get", + "params", unit_interface, "DevicePolicy")) + || flux_rpc_get_unpack (f, + "{s:[[ss]]}", + "params", &vtype, &policy) < 0) + log_msg_exit ("Get DevicePolicy: %s", future_strerror (f, errno)); + + if (!(f2 = flux_rpc_pack (h, + "sdbus.call", + FLUX_NODEID_ANY, + 0, + "{s:s s:s s:s s:[ss]}", + "path", unit_path, + "interface", prop_interface, + "member", "Get", + "params", unit_interface, "DeviceAllow")) + || flux_rpc_get_unpack (f2, + "{s:[[so]]}", + "params", &vtype, &allow) < 0) + log_msg_exit ("Get DeviceAllow: %s", future_strerror (f2, errno)); + + if (json_object_set_new (options, "DevicePolicy", json_string (policy)) < 0 + || json_object_set (options, "DeviceAllow", allow) < 0) + log_msg_exit ("error adding device properties to options"); + + flux_future_destroy (f2); + flux_future_destroy (f); + free (unit_path); +} + +/* Fetch the signed jobspec J for jobid. + * Caller must json_decref() the result. + */ +static json_t *J_from_jobid (flux_t *h, const char *jobid) +{ flux_jobid_t id; - const char *idstr = NULL; + flux_future_t *f; const char *J; - json_t *imp_input; + json_t *result; - log_init ("imp_exec_helper"); - - optindex = optparse_option_index (p); - if (optindex != ac && optindex != ac - 1) { - optparse_print_usage (p); - exit (1); - } - if (optindex < ac) - idstr = av[optindex]; - else if (!(idstr = getenv ("FLUX_JOB_ID"))) + if (!jobid) log_msg_exit ("Unable to determine jobid"); - if (flux_job_id_parse (idstr, &id) < 0) - log_err_exit ("invalid jobid: %s", idstr); - - h = builtin_get_flux_handle (p); + if (flux_job_id_parse (jobid, &id) < 0) + log_err_exit ("invalid jobid: %s", jobid); if (!(f = flux_rpc_pack (h, "job-info.lookup", @@ -60,15 +165,57 @@ static int helper (optparse_t *p, int ac, char *av[]) "flags", 0)) || flux_rpc_get_unpack (f, "{s:s}", "J", &J) < 0) log_err_exit ("error looking up J: %s", future_strerror (f, errno)); + if (!(result = json_string (J))) + log_msg_exit ("error creating JSON string for J"); + flux_future_destroy (f); + return result; +} + +static int helper (optparse_t *p, int ac, char *av[]) +{ + int optindex; + const char *jobid; + flux_t *h; + json_t *imp_input; + json_t *options; + + log_init ("imp_exec_helper"); + + optindex = optparse_option_index (p); + + if (optparse_hasopt (p, "test-nojob") && optindex != ac) + log_msg_exit ("--test-nojob cannot be used with JOBID argument"); + if (ac - optindex > 1) { + optparse_print_usage (p); + exit (1); + } + if (optindex < ac) + jobid = av[optindex]; + else + jobid = getenv ("FLUX_JOB_ID"); + + h = builtin_get_flux_handle (p); - if (!(imp_input = json_pack ("{s:s}", "J", J))) - log_msg_exit ("error creating imp input object"); + if (!(imp_input = json_object ()) || !(options = json_object ())) + log_msg_exit ("error creating json object"); + + if (!optparse_hasopt (p, "test-nojob")) { + if (json_object_set_new (imp_input, "J", J_from_jobid (h, jobid)) < 0) + log_msg_exit ("error adding J to imp input"); + } + + options_add_device (options, h); + + if (json_object_size (options) > 0) { + if (json_object_set (imp_input, "options", options) < 0) + log_msg_exit ("error adding options to imp input"); + } if (json_dumpf (imp_input, stdout, JSON_COMPACT) < 0) log_msg_exit ("error writing imp input object to stdout"); + json_decref (options); json_decref (imp_input); - flux_future_destroy (f); return 0; } @@ -78,10 +225,10 @@ int subcommand_imp_exec_helper_register (optparse_t *p) e = optparse_reg_subcommand (p, "imp_exec_helper", helper, - "JOBID", + "[JOBID]", "Emit IMP exec helper JSON for JOBID", 0, - NULL); + helper_opts); return (e == OPTPARSE_SUCCESS ? 0 : -1); } From a97f8f203b1cff10704d22a4ca77a7a79fdf2b97 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Fri, 24 Apr 2026 05:28:57 -0700 Subject: [PATCH 07/10] testsuite: cover IMP exec helper Problem: the IMP exec helper has no test coverage. Add a sharness test that covers usage when not run under systemd, and some error paths. Co-Authored-By: Claude Sonnet 4.6 --- t/Makefile.am | 1 + t/t2414-imp-exec-helper.t | 69 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100755 t/t2414-imp-exec-helper.t diff --git a/t/Makefile.am b/t/Makefile.am index 2cbfc940b783..5dfa3813bd3b 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -216,6 +216,7 @@ TESTSCRIPTS = \ t2411-sdexec-job.t \ t2412-sdmon.t \ t2413-sdmon-resource.t \ + t2414-imp-exec-helper.t \ t2500-job-attach.t \ t2501-job-status.t \ t2600-job-shell-rcalc.t \ diff --git a/t/t2414-imp-exec-helper.t b/t/t2414-imp-exec-helper.t new file mode 100755 index 000000000000..c4710fd5c68f --- /dev/null +++ b/t/t2414-imp-exec-helper.t @@ -0,0 +1,69 @@ +#!/bin/sh + +test_description='Test flux imp_exec_helper builtin' + +. $(dirname $0)/sharness.sh + +test_under_flux 1 job + +test_expect_success 'imp_exec_helper: too many args fails' ' + test_must_fail flux imp_exec_helper f1 f2 +' + +test_expect_success 'imp_exec_helper: no jobid without FLUX_JOB_ID fails' ' + test_must_fail env -u FLUX_JOB_ID flux imp_exec_helper +' + +test_expect_success 'imp_exec_helper: unparsable jobid fails' ' + test_must_fail flux imp_exec_helper notajobid +' + +test_expect_success 'imp_exec_helper: submit a test job' ' + jobid=$(flux submit true) && + flux job wait-event -t 10 ${jobid} clean +' + +test_expect_success 'imp_exec_helper: non-existent jobid fails' ' + test_must_fail flux imp_exec_helper 9999999999 +' + +test_expect_success 'imp_exec_helper: output is valid JSON on stdout with J key' ' + flux imp_exec_helper ${jobid} >out.json && + jq -e .J out.json +' + +test_expect_success 'imp_exec_helper: no options key without INVOCATION_ID' ' + flux imp_exec_helper ${jobid} >out.json && + test "$(jq "has(\"options\")" out.json)" = "false" +' + +test_expect_success 'imp_exec_helper: FLUX_JOB_ID env var fallback works' ' + FLUX_JOB_ID=${jobid} flux imp_exec_helper >out.json && + jq -e .J out.json +' + +test_expect_success 'imp_exec_helper: INVOCATION_ID wrong length is fatal' ' + test_must_fail env INVOCATION_ID=notvalidhex \ + flux imp_exec_helper ${jobid} +' + +test_expect_success 'imp_exec_helper: INVOCATION_ID invalid hex chars is fatal' ' + test_must_fail env INVOCATION_ID=GGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGG \ + flux imp_exec_helper ${jobid} +' + +test_expect_success 'imp_exec_helper: INVOCATION_ID set but sdbus unavailable is fatal' ' + test_must_fail env INVOCATION_ID=deadbeefdeadbeefdeadbeefdeadbeef \ + flux imp_exec_helper ${jobid} +' + +test_expect_success 'imp_exec_helper: --test-nojob with JOBID arg fails' ' + test_must_fail flux imp_exec_helper --test-nojob ${jobid} +' + +test_expect_success 'imp_exec_helper: --test-nojob without INVOCATION_ID outputs empty object' ' + flux imp_exec_helper --test-nojob >nojob.json && + jq -e "type == \"object\" and length == 0" nojob.json +' + +test_done From fb3ed74a3f4cec09531e6bdbfdf8dea0dd426f72 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Fri, 24 Apr 2026 08:05:15 -0700 Subject: [PATCH 08/10] testsuite: cover IMP exec helper under systemd Problem: there is no test coverage for the IMP exec helper's interaction with systemd. Add t2415-sdexec-device.t, which uses the helper's --test-nojob option under flux exec --service=sdexec to check its handling of DeviceAllow and DevicePolicy property settings. Co-Authored-By: Claude Sonnet 4.6 --- t/Makefile.am | 1 + t/t2415-sdexec-device.t | 98 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100755 t/t2415-sdexec-device.t diff --git a/t/Makefile.am b/t/Makefile.am index 5dfa3813bd3b..a83c3aa948d3 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -217,6 +217,7 @@ TESTSCRIPTS = \ t2412-sdmon.t \ t2413-sdmon-resource.t \ t2414-imp-exec-helper.t \ + t2415-sdexec-device.t \ t2500-job-attach.t \ t2501-job-status.t \ t2600-job-shell-rcalc.t \ diff --git a/t/t2415-sdexec-device.t b/t/t2415-sdexec-device.t new file mode 100755 index 000000000000..87c85b7e84f0 --- /dev/null +++ b/t/t2415-sdexec-device.t @@ -0,0 +1,98 @@ +#!/bin/sh +# ci=system +test_description='Test imp_exec_helper device containment with systemd' + +. $(dirname $0)/sharness.sh + +if ! flux version | grep systemd; then + skip_all="flux was not built with systemd" + test_done +fi +if ! systemctl --user show --property Version; then + skip_all="user systemd is not running" + test_done +fi +if ! busctl --user status >/dev/null; then + skip_all="user dbus is not running" + test_done +fi + +test_under_flux 1 minimal -Slog-stderr-level=1 + +sdexec="flux exec --service sdexec" +# Capture full path: sdexec units may not inherit the build-tree PATH +flux_cmd=$(command -v flux) +# Use --test-nojob to skip J lookup, so we can test device options in isolation +imp_helper="$flux_cmd imp_exec_helper --test-nojob" + +test_expect_success 'load sdbus,sdexec modules' ' + flux exec flux module load sdbus && + flux exec flux module load sdexec +' + +# +# DevicePolicy=auto, DeviceAllow empty (default): options present, IMP decides +# +test_expect_success 'auto policy with no DeviceAllow: options present with empty allow' ' + $sdexec -r 0 $imp_helper >auto-empty.json && + jq -e ".options.DevicePolicy == \"auto\"" auto-empty.json && + jq -e ".options.DeviceAllow | length == 0" auto-empty.json +' + +# +# DevicePolicy=closed, DeviceAllow empty: options present with empty allow list +# +test_expect_success 'closed policy: options.DevicePolicy is closed' ' + $sdexec -r 0 \ + --setopt=SDEXEC_PROP_DevicePolicy=closed \ + $imp_helper >closed-empty.json && + jq -e ".options.DevicePolicy == \"closed\"" closed-empty.json +' +test_expect_success 'closed policy: options.DeviceAllow is empty array' ' + jq -e ".options.DeviceAllow | length == 0" closed-empty.json +' + +# +# DevicePolicy=strict, explicit DeviceAllow: raw [specifier, access] tuple passed through +# +test_expect_success 'strict policy with DeviceAllow passes through raw entry' ' + $sdexec -r 0 \ + --setopt=SDEXEC_PROP_DevicePolicy=strict \ + --setopt="SDEXEC_PROP_DeviceAllow=/dev/null rw" \ + $imp_helper >strict.json && + jq -e ".options.DeviceAllow | length == 1" strict.json +' +test_expect_success 'strict policy entry is raw [specifier, access] tuple' ' + jq -e ".options.DeviceAllow[0][0] == \"/dev/null\" and + .options.DeviceAllow[0][1] == \"rw\"" strict.json +' + +# +# DevicePolicy=auto, non-empty DeviceAllow: only the listed entry, no standard devices +# +test_expect_success 'auto policy with non-empty DeviceAllow passes through raw entry' ' + $sdexec -r 0 \ + --setopt="SDEXEC_PROP_DeviceAllow=/dev/null rw" \ + $imp_helper >auto-nonempty.json && + jq -e ".options.DeviceAllow | length == 1" auto-nonempty.json +' + +# +# char-SUBSYSTEM specifier: passed through verbatim, IMP resolves /proc/devices +# +test_expect_success 'strict policy with char-pts passes through raw specifier' ' + $sdexec -r 0 \ + --setopt=SDEXEC_PROP_DevicePolicy=strict \ + --setopt="SDEXEC_PROP_DeviceAllow=char-pts rw" \ + $imp_helper >char-pts.json && + jq -e ".options.DeviceAllow | length == 1" char-pts.json && + jq -e ".options.DeviceAllow[0][0] == \"char-pts\" and + .options.DeviceAllow[0][1] == \"rw\"" char-pts.json +' + +test_expect_success 'remove sdexec,sdbus modules' ' + flux exec flux module remove sdexec && + flux exec flux module remove sdbus +' + +test_done From 30bccc7ae9dd930daaa8ccf6e9a0d46342c9a039 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Sun, 26 Apr 2026 10:34:02 -0700 Subject: [PATCH 09/10] flux-environment(7): add FLUX_IMP_EXEC_HELPER Problem: the FLUX_IMP_EXEC_HELPER environment variable is undocumented. Add it to the environment man page. --- doc/man7/flux-environment.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/man7/flux-environment.rst b/doc/man7/flux-environment.rst index 84788ef2fd09..4d4ab3afd0a3 100644 --- a/doc/man7/flux-environment.rst +++ b/doc/man7/flux-environment.rst @@ -590,6 +590,12 @@ MISCELLANEOUS plugins from those paths, with plugins in the ``flux.cli.plugins`` namespace loaded last. +.. envvar:: FLUX_IMP_EXEC_HELPER + + The IMP optionally takes its input from the output of an unprivileged + helper program when this variable is set to its path. It is set by the + job-exec system in the IMP environment at job launch. + .. _sub_command_environment: SUB-COMMAND ENVIRONMENT From 9dcbe1d956819337364480eae9dc74f74fe8984a Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Sun, 26 Apr 2026 11:00:06 -0700 Subject: [PATCH 10/10] rexec: don't propagate INVOCATION_ID Problem: the IMP may inherit a bogus INVOCATION_ID when not launched from systemd, if it is set in the leader broker environment. Clear INVOCATION_ID in the rexec server, similar to how NOTIFY_SOCKET is handled. --- src/common/libsubprocess/server.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/libsubprocess/server.c b/src/common/libsubprocess/server.c index 9ca1d3c31596..c2eedb6d8848 100644 --- a/src/common/libsubprocess/server.c +++ b/src/common/libsubprocess/server.c @@ -776,6 +776,7 @@ static void server_exec_cb (flux_t *h, */ flux_cmd_unsetenv (cmd, "FLUX_PROXY_REMOTE"); flux_cmd_unsetenv (cmd, "NOTIFY_SOCKET"); // see sd_notify(3) + flux_cmd_unsetenv (cmd, "INVOCATION_ID"); if (!(p = flux_local_exec_ex (flux_get_reactor (s->h), local_flags,