Merge pull request flux-framework#6331 from garlick/issue#6325

flux job info: improve error messages

src/cmd/job/info.c

@@ -55,6 +55,7 @@ static void info_usage (void)
 int cmd_info (optparse_t *p, int argc, char **argv)
 {
     flux_t *h;
+    flux_error_t error;
     int optindex = optparse_option_index (p);
     flux_jobid_t id;
     const char *id_str;
@@ -72,8 +73,8 @@ int cmd_info (optparse_t *p, int argc, char **argv)
     id = parse_jobid (id_str);
     key = argv[optindex++];
 
-    if (!(h = flux_open (NULL, 0)))
-        log_err_exit ("flux_open");
+    if (!(h = flux_open_ex (NULL, 0, &error)))
+        log_msg_exit ("flux_open: %s", error.text);
 
     /* The --original (pre-frobnication) jobspec is obtained by fetching J.
      * J is the original jobspec, signed, so we must unwrap it to get to the

src/modules/job-info/allow.c

@@ -20,50 +20,37 @@
 #include "allow.h"
 
 #include "src/common/libeventlog/eventlog.h"
+#include "src/common/libutil/errno_safe.h"
 #include "ccan/str/str.h"
 
 /* Parse the submit userid from the event log.
- * Assume "submit" is the first event.
+ * RFC 18 defines the structure of eventlogs.
+ * RFC 21 requires that the first entry is "submit" and defines its context.
  */
-static int eventlog_get_userid (struct info_ctx *ctx, const char *s,
+static int eventlog_get_userid (struct info_ctx *ctx,
+                                const char *s,
                                 uint32_t *useridp)
 {
-    json_t *a = NULL;
-    json_t *entry = NULL;
-    const char *name = NULL;
-    json_t *context = NULL;
+    json_t *o = NULL;
+    const char *name;
     int userid;
     int rv = -1;
 
-    if (!(a = eventlog_decode (s))) {
-        flux_log_error (ctx->h, "%s: eventlog_decode", __FUNCTION__);
-        /* if eventlog improperly formatted, we'll consider this a
-         * protocol error */
-        if (errno == EINVAL)
-            errno = EPROTO;
-        goto error;
-    }
-    if (!(entry = json_array_get (a, 0))) {
-        errno = EPROTO;
-        goto error;
-    }
-    if (eventlog_entry_parse (entry, NULL, &name, &context) < 0) {
-        flux_log_error (ctx->h, "%s: eventlog_decode", __FUNCTION__);
-        goto error;
-    }
-    if (!streq (name, "submit") || !context) {
-        flux_log (ctx->h, LOG_ERR, "%s: invalid event: %s", __FUNCTION__, name);
-        errno = EPROTO;
-        goto error;
-    }
-    if (json_unpack (context, "{ s:i }", "userid", &userid) < 0) {
+    if (!s
+        || !(o = json_loads (s, JSON_DISABLE_EOF_CHECK, NULL))
+        || json_unpack (o,
+                        "{s:s s:{s:i}}",
+                        "name", &name,
+                        "context",
+                          "userid", &userid) < 0
+        || !streq (name, "submit")) {
         errno = EPROTO;
         goto error;
     }
     (*useridp) = userid;
     rv = 0;
 error:
-    json_decref (a);
+    ERRNO_SAFE_WRAP (json_decref, o);
     return rv;
 }
 
@@ -87,29 +74,17 @@ static void store_lru (struct info_ctx *ctx, flux_jobid_t id, uint32_t userid)
     return;
 }
 
-/* Optimization:
- * Avoid calling eventlog_get_userid() if message cred has OWNER role.
- */
-int eventlog_allow (struct info_ctx *ctx, const flux_msg_t *msg,
-                    flux_jobid_t id, const char *s)
+int eventlog_allow (struct info_ctx *ctx,
+                    const flux_msg_t *msg,
+                    flux_jobid_t id,
+                    const char *s)
 {
-    struct flux_msg_cred cred;
-
-    if (flux_msg_get_cred (msg, &cred) < 0)
+    uint32_t userid;
+    if (eventlog_get_userid (ctx, s, &userid) < 0)
+        return -1;
+    store_lru (ctx, id, userid);
+    if (flux_msg_authorize (msg, userid) < 0)
         return -1;
-    if (!(cred.rolemask & FLUX_ROLE_OWNER)) {
-        uint32_t userid;
-        /* RFC18: empty eventlog not allowed */
-        if (!s) {
-            errno = EPROTO;
-            return -1;
-        }
-        if (eventlog_get_userid (ctx, s, &userid) < 0)
-            return -1;
-        store_lru (ctx, id, userid);
-        if (flux_msg_cred_authorize (cred, userid) < 0)
-            return -1;
-    }
     return 0;
 }
 

src/modules/job-info/allow.h

@@ -20,8 +20,10 @@
  * event which records the job owner.  Will cache recently looked
  * up job owners in an LRU cache.
  */
-int eventlog_allow (struct info_ctx *ctx, const flux_msg_t *msg,
-                    flux_jobid_t id, const char *s);
+int eventlog_allow (struct info_ctx *ctx,
+                    const flux_msg_t *msg,
+                    flux_jobid_t id,
+                    const char *s);
 
 /* Determine if user who sent request 'msg' is allowed to access job
  * eventlog via LRU cache.  Returns 1 if access allowed, 0 if

src/modules/job-info/job-info.c

@@ -22,25 +22,31 @@
 #include "guest_watch.h"
 #include "update.h"
 
-static void disconnect_cb (flux_t *h, flux_msg_handler_t *mh,
-                           const flux_msg_t *msg, void *arg)
+static void disconnect_cb (flux_t *h,
+                           flux_msg_handler_t *mh,
+                           const flux_msg_t *msg,
+                           void *arg)
 {
     struct info_ctx *ctx = arg;
     watchers_cancel (ctx, msg, false);
     guest_watchers_cancel (ctx, msg, false);
     update_watchers_cancel (ctx, msg, false);
 }
 
-static void stats_cb (flux_t *h, flux_msg_handler_t *mh,
-                      const flux_msg_t *msg, void *arg)
+static void stats_cb (flux_t *h,
+                      flux_msg_handler_t *mh,
+                      const flux_msg_t *msg,
+                      void *arg)
 {
     struct info_ctx *ctx = arg;
     int lookups = zlist_size (ctx->lookups);
     int watchers = zlist_size (ctx->watchers);
     int guest_watchers = zlist_size (ctx->guest_watchers);
     int update_lookups = 0;     /* no longer supported */
     int update_watchers = update_watch_count (ctx);
-    if (flux_respond_pack (h, msg, "{s:i s:i s:i s:i s:i}",
+    if (flux_respond_pack (h,
+                           msg,
+                           "{s:i s:i s:i s:i s:i}",
                            "lookups", lookups,
                            "watchers", watchers,
                            "guest_watchers", guest_watchers,