Merge pull request #5608 from garlick/issue#5475

history: track root jobs
flux-framework · Dec 7, 2023 · a96f7fa · a96f7fa
2 parents 85d8e2d + a8c4882
commit a96f7fa
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 8 deletions.
diff --git a/src/modules/job-manager/plugins/history.c b/src/modules/job-manager/plugins/history.c
@@ -37,6 +37,24 @@ struct history {
 
 #define COMPARE_NUM_REVERSE(a,b) ((a)>(b)?-1:(a)<(b)?1:0)
 
+/* Keys in history->users are calculated from int2ptr(uid), but that won't
+ * work for root.  Substitute (uid_t)-1 in that case as it's reserved per
+ * POSIX.  See also: flux-framework/flux-core#5475.
+ */
+static const void *userid2key (int userid)
+{
+    if (userid == 0)
+        return int2ptr ((uid_t)-1);
+    return int2ptr (userid);
+}
+
+static int key2userid (const void *key)
+{
+    if (key == int2ptr ((uid_t)-1))
+        return 0;
+    return ptr2int (key);
+}
+
 static void job_entry_destroy (struct job_entry *entry)
 {
     if (entry) {
@@ -76,13 +94,13 @@ static int job_entry_comparator (const void *item1, const void *item2)
 // zhashx_hash_fn footprint
 static size_t userid_hasher (const void *key)
 {
-    return ptr2int (key);
+    return key2userid (key);
 }
 
 // zhashx_comparator_fn footprint
 static int userid_comparator (const void *item1, const void *item2)
 {
-    return COMPARE_NUM_REVERSE (ptr2int (item1), ptr2int (item2));
+    return COMPARE_NUM_REVERSE (key2userid (item1), key2userid (item2));
 }
 
 static void history_destroy (struct history *hist)
@@ -124,6 +142,7 @@ static int jobtap_cb (flux_plugin_t *p,
     struct history *hist = arg;
     struct job_entry *entry;
     int userid;
+    const void *key;
 
     if (!(entry = job_entry_create ()))
         return -1;
@@ -134,24 +153,25 @@ static int jobtap_cb (flux_plugin_t *p,
                                 "t_submit", &entry->t_submit,
                                 "userid", &userid) < 0)
         return -1;
+    key = userid2key (userid);
     if (streq (topic, "job.inactive-remove")) {
         void *handle;
-        if ((handle = hola_list_find (hist->users, int2ptr (userid), entry)))
-            hola_list_delete (hist->users, int2ptr (userid), handle);
+        if ((handle = hola_list_find (hist->users, key, entry)))
+            hola_list_delete (hist->users, key, handle);
         job_entry_destroy (entry);
     }
     else if (streq (topic, "job.inactive-add")) {
-        if (hola_list_find (hist->users, int2ptr (userid), entry)) {
+        if (hola_list_find (hist->users, key, entry)) {
             job_entry_destroy (entry);
             return 0;
         }
-        if (!hola_list_insert (hist->users, int2ptr (userid), entry, false)) {
+        if (!hola_list_insert (hist->users, key, entry, false)) {
             job_entry_destroy (entry);
             return -1;
         }
     }
     else if (streq (topic, "job.new")) {
-        if (!hola_list_insert (hist->users, int2ptr (userid), entry, false)) {
+        if (!hola_list_insert (hist->users, key, entry, false)) {
             job_entry_destroy (entry);
             return -1;
         }
@@ -221,7 +241,7 @@ static json_t *history_slice (struct history *hist,
     size_t list_size = 0;
     int rc;
 
-    if ((l = hola_hash_lookup (hist->users, int2ptr (userid))))
+    if ((l = hola_hash_lookup (hist->users, userid2key (userid))))
         list_size = zlistx_size (l);
     if (slice_parse (&sl, slice, list_size) < 0) {
         errprintf (error, "could not parse python-style slice expression");

diff --git a/t/t2812-flux-job-last.t b/t/t2812-flux-job-last.t
@@ -4,6 +4,8 @@ test_description='Test the flux job last command'
 
 . $(dirname $0)/sharness.sh
 
+flux version | grep -q libflux-security && test_set_prereq FLUX_SECURITY
+
 test_under_flux 1
 
 test_expect_success 'flux-job last fails on invalid arguments' '
@@ -50,4 +52,22 @@ test_expect_success 'flux-job last does not list purged jobs' '
 	grep "job history is empty" nojob2.err
 '
 
+submit_as_root()
+{
+        FAKE_USERID=0
+        test_debug "echo running flux run $@ as userid $FAKE_USERID"
+        flux run --dry-run "$@" | \
+          flux python ${SHARNESS_TEST_SRCDIR}/scripts/sign-as.py $FAKE_USERID \
+            >job.signed
+        FLUX_HANDLE_USERID=$FAKE_USERID \
+          flux job submit --flags=signed job.signed
+}
+
+# issue #5475
+# Execution may fail but submission should work - enough for this test
+test_expect_success FLUX_SECURITY 'run a job as fake root' '
+	submit_as_root true &&
+	FLUX_HANDLE_USERID=0 flux job last
+'
+
 test_done