diff --git a/src/cmd/job/info.c b/src/cmd/job/info.c index d611c2fbc6a2..dd314be60cd2 100644 --- a/src/cmd/job/info.c +++ b/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 diff --git a/src/modules/job-info/allow.c b/src/modules/job-info/allow.c index d34971923e45..4c8256c2186b 100644 --- a/src/modules/job-info/allow.c +++ b/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; } diff --git a/src/modules/job-info/allow.h b/src/modules/job-info/allow.h index a8d23c7e13ce..f78e05bd0dda 100644 --- a/src/modules/job-info/allow.h +++ b/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 diff --git a/src/modules/job-info/job-info.c b/src/modules/job-info/job-info.c index 8e27031fa42e..730bc4c7f1cd 100644 --- a/src/modules/job-info/job-info.c +++ b/src/modules/job-info/job-info.c @@ -22,8 +22,10 @@ #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); @@ -31,8 +33,10 @@ static void disconnect_cb (flux_t *h, flux_msg_handler_t *mh, 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); @@ -40,7 +44,9 @@ static void stats_cb (flux_t *h, flux_msg_handler_t *mh, 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, diff --git a/src/modules/job-info/lookup.c b/src/modules/job-info/lookup.c index e754d4928133..54a05796b6c1 100644 --- a/src/modules/job-info/lookup.c +++ b/src/modules/job-info/lookup.c @@ -20,6 +20,7 @@ #include "src/common/libczmqcontainers/czmq_containers.h" #include "src/common/libeventlog/eventlog.h" #include "src/common/libjob/idf58.h" +#include "src/common/libutil/errprintf.h" #include "ccan/str/str.h" #include "job-info.h" @@ -43,12 +44,15 @@ static void info_lookup_continuation (flux_future_t *fall, void *arg); static void lookup_ctx_destroy (void *data) { - if (data) { - struct lookup_ctx *ctx = data; + struct lookup_ctx *ctx = data; + + if (ctx) { + int saved_errno = errno; flux_msg_decref (ctx->msg); json_decref (ctx->keys); flux_future_destroy (ctx->f); free (ctx); + errno = saved_errno; } } @@ -59,7 +63,6 @@ static struct lookup_ctx *lookup_ctx_create (struct info_ctx *ctx, int flags) { struct lookup_ctx *l = calloc (1, sizeof (*l)); - int saved_errno; if (!l) return NULL; @@ -78,9 +81,7 @@ static struct lookup_ctx *lookup_ctx_create (struct info_ctx *ctx, return l; error: - saved_errno = errno; lookup_ctx_destroy (l); - errno = saved_errno; return NULL; } @@ -95,26 +96,13 @@ static int lookup_key (struct lookup_ctx *l, if (flux_future_get_child (fall, key) != NULL) return 0; - if (flux_job_kvs_key (path, sizeof (path), l->id, key) < 0) { - flux_log_error (l->ctx->h, "%s: flux_job_kvs_key", __FUNCTION__); - goto error; - } - - if (!(f = flux_kvs_lookup (l->ctx->h, NULL, 0, path))) { - flux_log_error (l->ctx->h, "%s: flux_kvs_lookup", __FUNCTION__); - goto error; - } - - if (flux_future_push (fall, key, f) < 0) { - flux_log_error (l->ctx->h, "%s: flux_future_push", __FUNCTION__); - goto error; + if (flux_job_kvs_key (path, sizeof (path), l->id, key) < 0 + || !(f = flux_kvs_lookup (l->ctx->h, NULL, 0, path)) + || flux_future_push (fall, key, f) < 0) { + flux_future_destroy (f); + return -1; } - return 0; - -error: - flux_future_destroy (f); - return -1; } static int lookup_keys (struct lookup_ctx *l) @@ -123,10 +111,8 @@ static int lookup_keys (struct lookup_ctx *l) size_t index; json_t *key; - if (!(fall = flux_future_wait_all_create ())) { - flux_log_error (l->ctx->h, "%s: flux_wait_all_create", __FUNCTION__); - goto error; - } + if (!(fall = flux_future_wait_all_create ())) + return -1; flux_future_set_flux (fall, l->ctx->h); if (l->lookup_eventlog) { @@ -139,13 +125,8 @@ static int lookup_keys (struct lookup_ctx *l) goto error; } - if (flux_future_then (fall, - -1, - info_lookup_continuation, - l) < 0) { - flux_log_error (l->ctx->h, "%s: flux_future_then", __FUNCTION__); + if (flux_future_then (fall, -1, info_lookup_continuation, l) < 0) goto error; - } l->f = fall; return 0; @@ -182,15 +163,18 @@ static int lookup_current (struct lookup_ctx *l, } if (!(f_eventlog = flux_future_get_child (fall, "eventlog"))) { - flux_log_error (l->ctx->h, "%s: flux_future_get_child", + flux_log_error (l->ctx->h, + "%s: flux_future_get_child", __FUNCTION__); goto error; } if (flux_kvs_lookup_get (f_eventlog, &s_eventlog) < 0) { - if (errno != ENOENT) - flux_log_error (l->ctx->h, "%s: flux_kvs_lookup_get", + if (errno != ENOENT) { + flux_log_error (l->ctx->h, + "%s: flux_kvs_lookup_get", __FUNCTION__); + } goto error; } @@ -243,32 +227,41 @@ static void info_lookup_continuation (flux_future_t *fall, void *arg) json_t *key; json_t *o = NULL; json_t *tmp = NULL; - char *data = NULL; + flux_error_t error; if (!l->allow) { flux_future_t *f; if (!(f = flux_future_get_child (fall, "eventlog"))) { - flux_log_error (ctx->h, "%s: flux_future_get_child", __FUNCTION__); + errprintf (&error, + "internal error: flux_future_get_child eventlog: %s", + strerror (errno)); goto error; } if (flux_kvs_lookup_get (f, &s) < 0) { - if (errno != ENOENT) - flux_log_error (l->ctx->h, "%s: flux_kvs_lookup_get", __FUNCTION__); + errprintf (&error, + "%s", + errno == ENOENT ? "invalid job id" : strerror (errno)); goto error; } - if (eventlog_allow (ctx, l->msg, l->id, s) < 0) + if (eventlog_allow (ctx, l->msg, l->id, s) < 0) { + char *errmsg; + if (errno == EPERM) + errmsg = "access is restricted to job/instance owner"; + else + errmsg = "error parsing eventlog"; + errprintf (&error, "%s", errmsg); goto error; + } l->allow = true; } - if (!(o = json_object ())) - goto enomem; - - tmp = json_integer (l->id); - if (json_object_set_new (o, "id", tmp) < 0) { + if (!(o = json_object ()) + || !(tmp = json_integer (l->id)) + || json_object_set_new (o, "id", tmp) < 0) { + errprintf (&error, "error creating response object"); json_decref (tmp); goto enomem; } @@ -279,47 +272,53 @@ static void info_lookup_continuation (flux_future_t *fall, void *arg) json_t *val = NULL; if (!(f = flux_future_get_child (fall, keystr))) { - flux_log_error (ctx->h, "%s: flux_future_get_child", __FUNCTION__); + errprintf (&error, + "internal error: flux_future_get_child %s: %s", + keystr, + strerror (errno)); goto error; } if (flux_kvs_lookup_get (f, &s) < 0) { - if (errno != ENOENT) - flux_log_error (l->ctx->h, "%s: flux_kvs_lookup_get", __FUNCTION__); + errprintf (&error, + "%s: %s", + keystr, + errno == ENOENT ? "key not found" : strerror (errno)); goto error; } /* treat empty value as invalid */ if (!s) { + errprintf (&error, "%s: value is unexpectedly empty", keystr); errno = EPROTO; goto error; } if ((l->flags & FLUX_JOB_LOOKUP_CURRENT) - && (streq (keystr, "R") - || streq (keystr, "jobspec"))) { - if (lookup_current (l, fall, keystr, s, ¤t_value) < 0) + && (streq (keystr, "R") || streq (keystr, "jobspec"))) { + if (lookup_current (l, fall, keystr, s, ¤t_value) < 0) { + errprintf (&error, + "%s: error applying eventlog to original value: %s", + keystr, + strerror (errno)); goto error; + } s = current_value; } /* check for JSON_DECODE flag last, as changes above could affect * desired value */ if ((l->flags & FLUX_JOB_LOOKUP_JSON_DECODE) - && (streq (keystr, "jobspec") - || streq (keystr, "R"))) { + && (streq (keystr, "jobspec") || streq (keystr, "R"))) { /* We assume if it was stored in the KVS it's valid JSON, * so failure is ENOMEM */ - if (!(val = json_loads (s, 0, NULL))) - goto enomem; - } - else { - if (!(val = json_string (s))) - goto enomem; + val = json_loads (s, 0, NULL); } - - if (json_object_set_new (o, keystr, val) < 0) { + else + val = json_string (s); + if (!val || json_object_set_new (o, keystr, val) < 0) { json_decref (val); + errprintf (&error, "%s: error adding value to response", keystr); goto enomem; } @@ -331,73 +330,39 @@ static void info_lookup_continuation (flux_future_t *fall, void *arg) * taken error path */ assert (l->allow); - if (!(data = json_dumps (o, JSON_COMPACT))) - goto enomem; - - if (flux_respond (ctx->h, l->msg, data) < 0) { + if (flux_respond_pack (ctx->h, l->msg, "O", o) < 0) flux_log_error (ctx->h, "%s: flux_respond", __FUNCTION__); - goto error; - } goto done; enomem: errno = ENOMEM; error: - if (flux_respond_error (ctx->h, l->msg, errno, NULL) < 0) + if (flux_respond_error (ctx->h, l->msg, errno, error.text) < 0) flux_log_error (ctx->h, "%s: flux_respond_error", __FUNCTION__); done: /* flux future destroyed in lookup_ctx_destroy, which is called * via zlist_remove() */ json_decref (o); - free (data); free (current_value); zlist_remove (ctx->lookups, l); } -/* Check if lookup allowed, either b/c message is from instance owner - * or previous lookup verified it's ok. - */ -static int check_allow (struct lookup_ctx *l) -{ - int ret; - - /* if rpc from owner, no need to do guest access check */ - if (flux_msg_authorize (l->msg, FLUX_USERID_UNKNOWN) == 0) { - l->allow = true; - return 0; - } - - if ((ret = eventlog_allow_lru (l->ctx, - l->msg, - l->id)) < 0) - return -1; - - if (ret) { - l->allow = true; - return 0; - } - - return 0; -} - /* If we need the eventlog for an allow check or for update-lookup * we need to add it to the key lookup list. */ -static int check_to_lookup_eventlog (struct lookup_ctx *l) +static void check_to_lookup_eventlog (struct lookup_ctx *l) { if (!l->allow || (l->flags & FLUX_JOB_LOOKUP_CURRENT)) { size_t index; json_t *key; json_array_foreach (l->keys, index, key) { if (streq (json_string_value (key), "eventlog")) - return 0; + return; } l->lookup_eventlog = true; } - - return 0; } static json_t *get_json_string (json_t *o) @@ -462,7 +427,9 @@ static int lookup_cached (struct lookup_ctx *l) if (ret) { if (l->flags & FLUX_JOB_LOOKUP_JSON_DECODE) { - if (flux_respond_pack (l->ctx->h, l->msg, "{s:I s:O}", + if (flux_respond_pack (l->ctx->h, + l->msg, + "{s:I s:O}", "id", l->id, key_str, current_object) < 0) { flux_log_error (l->ctx->h, "%s: flux_respond", __FUNCTION__); @@ -477,7 +444,9 @@ static int lookup_cached (struct lookup_ctx *l) errno = ENOMEM; goto cleanup; } - if (flux_respond_pack (l->ctx->h, l->msg, "{s:I s:O}", + if (flux_respond_pack (l->ctx->h, + l->msg, + "{s:I s:O}", "id", l->id, key_str, o) < 0) { json_decref (o); @@ -501,33 +470,62 @@ static int lookup (flux_t *h, struct info_ctx *ctx, flux_jobid_t id, json_t *keys, - int flags) + int flags, + flux_error_t *error) { struct lookup_ctx *l = NULL; int ret; - if (!(l = lookup_ctx_create (ctx, msg, id, keys, flags))) + if (!(l = lookup_ctx_create (ctx, msg, id, keys, flags))) { + errprintf (error, + "could not create lookup context: %s", + strerror (errno)); goto error; + } - if (check_allow (l) < 0) - goto error; + /* If authorization is indeterminate at this stage (l->allow == false), + * look up the eventlog and authorize in the continuation. N.B. we could + * summarily allow the instance owner without looking up the eventlog, + * but then we could not differentiate an invalid job ID and a missing key. + * Since keys are looked up in parallel, this should not be too costly. + * See also: flux-framework/flux-core#6325 + */ + switch (eventlog_allow_lru (l->ctx, l->msg, l->id)) { + case -1: // entry found - DENY + errprintf (error, "access is restricted to job/instance owner"); + goto error; + case 1: // entry found - ALLOW + l->allow = true; + break; + case 0: // indeterminate + break; + } - if ((ret = lookup_cached (l)) < 0) + if ((ret = lookup_cached (l)) < 0) { + errprintf (error, + "internal error attempting to use update-watch cache: %s", + strerror (errno)); goto error; + } if (ret) { lookup_ctx_destroy (l); return 0; } - if (check_to_lookup_eventlog (l) < 0) - goto error; + check_to_lookup_eventlog (l); - if (lookup_keys (l) < 0) + if (lookup_keys (l) < 0) { + errprintf (error, + "error sending KVS lookup request(s): %s", + strerror (errno)); goto error; + } if (zlist_append (ctx->lookups, l) < 0) { - flux_log_error (h, "%s: zlist_append", __FUNCTION__); + errprintf (error, + "internal error saving lookup context: out of memory"); + errno = ENOMEM; goto error; } zlist_freefn (ctx->lookups, l, lookup_ctx_destroy, true); @@ -538,8 +536,10 @@ static int lookup (flux_t *h, return -1; } -void lookup_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg) +void lookup_cb (flux_t *h, + flux_msg_handler_t *mh, + const flux_msg_t *msg, + void *arg) { struct info_ctx *ctx = arg; size_t index; @@ -548,15 +548,16 @@ void lookup_cb (flux_t *h, flux_msg_handler_t *mh, flux_jobid_t id; int flags; int valid_flags = FLUX_JOB_LOOKUP_JSON_DECODE | FLUX_JOB_LOOKUP_CURRENT; + flux_error_t error; const char *errmsg = NULL; - if (flux_request_unpack (msg, NULL, "{s:I s:o s:i}", + if (flux_request_unpack (msg, + NULL, + "{s:I s:o s:i}", "id", &id, "keys", &keys, - "flags", &flags) < 0) { - flux_log_error (h, "%s: flux_request_unpack", __FUNCTION__); + "flags", &flags) < 0) goto error; - } if (flags & ~valid_flags) { errno = EPROTO; @@ -577,8 +578,10 @@ void lookup_cb (flux_t *h, flux_msg_handler_t *mh, } } - if (lookup (h, msg, ctx, id, keys, flags) < 0) + if (lookup (h, msg, ctx, id, keys, flags, &error) < 0) { + errmsg = error.text; goto error; + } return; @@ -588,8 +591,10 @@ void lookup_cb (flux_t *h, flux_msg_handler_t *mh, } /* legacy rpc target */ -void update_lookup_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg) +void update_lookup_cb (flux_t *h, + flux_msg_handler_t *mh, + const flux_msg_t *msg, + void *arg) { struct info_ctx *ctx = arg; flux_jobid_t id; @@ -597,15 +602,16 @@ void update_lookup_cb (flux_t *h, flux_msg_handler_t *mh, json_t *keys = NULL; int flags; int valid_flags = 0; + flux_error_t error; const char *errmsg = NULL; - if (flux_request_unpack (msg, NULL, "{s:I s:s s:i}", + if (flux_request_unpack (msg, + NULL, + "{s:I s:s s:i}", "id", &id, "key", &key, - "flags", &flags) < 0) { - flux_log_error (h, "%s: flux_request_unpack", __FUNCTION__); + "flags", &flags) < 0) goto error; - } if ((flags & ~valid_flags)) { errno = EPROTO; errmsg = "update-lookup request rejected with invalid flag"; @@ -627,9 +633,11 @@ void update_lookup_cb (flux_t *h, flux_msg_handler_t *mh, ctx, id, keys, - FLUX_JOB_LOOKUP_JSON_DECODE | FLUX_JOB_LOOKUP_CURRENT) < 0) + FLUX_JOB_LOOKUP_JSON_DECODE | FLUX_JOB_LOOKUP_CURRENT, + &error) < 0) { + errmsg = error.text; goto error; - + } return; error: diff --git a/src/modules/job-info/lookup.h b/src/modules/job-info/lookup.h index e1a0f4fadde9..4b69238e7e59 100644 --- a/src/modules/job-info/lookup.h +++ b/src/modules/job-info/lookup.h @@ -13,12 +13,16 @@ #include -void lookup_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg); +void lookup_cb (flux_t *h, + flux_msg_handler_t *mh, + const flux_msg_t *msg, + void *arg); /* legacy rpc target */ -void update_lookup_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg); +void update_lookup_cb (flux_t *h, + flux_msg_handler_t *mh, + const flux_msg_t *msg, + void *arg); #endif /* ! _FLUX_JOB_INFO_LOOKUP_H */ diff --git a/src/modules/job-info/update.c b/src/modules/job-info/update.c index 29753f0302ce..51b4fcc0dc46 100644 --- a/src/modules/job-info/update.c +++ b/src/modules/job-info/update.c @@ -191,7 +191,9 @@ static void eventlog_continuation (flux_future_t *f, void *arg) msg = flux_msglist_first (uc->msglist); while (msg) { - if (flux_respond_pack (uc->ctx->h, msg, "{s:O}", + if (flux_respond_pack (uc->ctx->h, + msg, + "{s:O}", uc->key, uc->update_object) < 0) { flux_log_error (ctx->h, "%s: flux_respond", __FUNCTION__); goto error_cancel; @@ -275,7 +277,8 @@ static void lookup_continuation (flux_future_t *f, void *arg) bool submit_parsed = false; const flux_msg_t *msg; - if (flux_rpc_get_unpack (f, "{s:s s:s}", + if (flux_rpc_get_unpack (f, + "{s:s s:s}", uc->key, &key_str, "eventlog", &eventlog_str) < 0) { if (errno != ENOENT && errno != EPERM) @@ -459,8 +462,10 @@ static int update_lookup (struct info_ctx *ctx, return -1; } -void update_watch_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg) +void update_watch_cb (flux_t *h, + flux_msg_handler_t *mh, + const flux_msg_t *msg, + void *arg) { struct info_ctx *ctx = arg; struct update_ctx *uc = NULL; @@ -471,13 +476,13 @@ void update_watch_cb (flux_t *h, flux_msg_handler_t *mh, const char *errmsg = NULL; char *index_key = NULL; - if (flux_request_unpack (msg, NULL, "{s:I s:s s:i}", + if (flux_request_unpack (msg, + NULL, + "{s:I s:s s:i}", "id", &id, "key", &key, - "flags", &flags) < 0) { - flux_log_error (h, "%s: flux_request_unpack", __FUNCTION__); + "flags", &flags) < 0) goto error; - } if ((flags & ~valid_flags)) { errno = EPROTO; errmsg = "update-watch request rejected with invalid flag"; @@ -510,7 +515,9 @@ void update_watch_cb (flux_t *h, flux_msg_handler_t *mh, if (uc->update_object) { if (flux_msg_authorize (msg, uc->userid) < 0) goto error; - if (flux_respond_pack (uc->ctx->h, msg, "{s:O}", + if (flux_respond_pack (uc->ctx->h, + msg, + "{s:O}", uc->key, uc->update_object) < 0) { flux_log_error (ctx->h, "%s: flux_respond", __FUNCTION__); goto error; @@ -593,8 +600,10 @@ void update_watchers_cancel (struct info_ctx *ctx, } } -void update_watch_cancel_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg) +void update_watch_cancel_cb (flux_t *h, + flux_msg_handler_t *mh, + const flux_msg_t *msg, + void *arg) { struct info_ctx *ctx = arg; update_watchers_cancel (ctx, msg, true); @@ -609,9 +618,11 @@ void update_watch_cleanup (struct info_ctx *ctx) eventlog_watch_cancel (uc); msg = flux_msglist_first (uc->msglist); while (msg) { - if (flux_respond_error (ctx->h, msg, ENOSYS, NULL) < 0) - flux_log_error (ctx->h, "%s: flux_respond_error", + if (flux_respond_error (ctx->h, msg, ENOSYS, NULL) < 0) { + flux_log_error (ctx->h, + "%s: flux_respond_error", __FUNCTION__); + } msg = flux_msglist_next (uc->msglist); } update_ctx_destroy (uc); diff --git a/src/modules/job-info/update.h b/src/modules/job-info/update.h index 85e3328470b5..eb5151415a82 100644 --- a/src/modules/job-info/update.h +++ b/src/modules/job-info/update.h @@ -14,11 +14,15 @@ #include #include -void update_watch_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg); +void update_watch_cb (flux_t *h, + flux_msg_handler_t *mh, + const flux_msg_t *msg, + void *arg); -void update_watch_cancel_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg); +void update_watch_cancel_cb (flux_t *h, + flux_msg_handler_t *mh, + const flux_msg_t *msg, + void *arg); /* returns 1 on found, 0 if not, -1 on error */ int update_watch_get_cached (struct info_ctx *ctx, diff --git a/src/modules/job-info/util.c b/src/modules/job-info/util.c index 146ccdc33210..95a34d126cb7 100644 --- a/src/modules/job-info/util.c +++ b/src/modules/job-info/util.c @@ -118,10 +118,14 @@ void apply_updates_R (flux_t *h, if (streq (ckey, "expiration")) if (jpath_set (R, "execution.expiration", - value) < 0) - flux_log (h, LOG_INFO, + value) < 0) { + flux_log (h, + LOG_INFO, "%s: failed to update job %s %s", - __FUNCTION__, idf58 (id), key); + __FUNCTION__, + idf58 (id), + key); + } } } @@ -137,10 +141,14 @@ void apply_updates_jobspec (flux_t *h, json_object_foreach (context, ckey, value) { if (jpath_set (jobspec, ckey, - value) < 0) - flux_log (h, LOG_INFO, + value) < 0) { + flux_log (h, + LOG_INFO, "%s: failed to update job %s %s", - __FUNCTION__, idf58 (id), key); + __FUNCTION__, + idf58 (id), + key); + } } } diff --git a/src/modules/job-info/util.h b/src/modules/job-info/util.h index a4679401795b..51f442a544d1 100644 --- a/src/modules/job-info/util.h +++ b/src/modules/job-info/util.h @@ -23,7 +23,8 @@ flux_msg_t *cred_msg_pack (const char *topic, ...); /* helper to parse next eventlog entry when whole eventlog is read */ -bool get_next_eventlog_entry (const char **pp, const char **tok, +bool get_next_eventlog_entry (const char **pp, + const char **tok, size_t *toklen); /* parse chunk from eventlog_parse_next, 'entry' is required and diff --git a/src/modules/job-info/watch.c b/src/modules/job-info/watch.c index a2d3e6487d60..e0ccf3966bd9 100644 --- a/src/modules/job-info/watch.c +++ b/src/modules/job-info/watch.c @@ -132,7 +132,10 @@ static int watch_key (struct watch_ctx *w) pathptr = w->path; } else { - if (flux_job_kvs_key (fullpath, sizeof (fullpath), w->id, w->path) < 0) { + if (flux_job_kvs_key (fullpath, + sizeof (fullpath), + w->id, + w->path) < 0) { flux_log_error (w->ctx->h, "%s: flux_job_kvs_key", __FUNCTION__); return -1; } @@ -256,10 +259,12 @@ static void watch_continuation (flux_future_t *f, void *arg) input = s; while (get_next_eventlog_entry (&input, &tok, &toklen)) { - if (flux_respond_pack (ctx->h, w->msg, + if (flux_respond_pack (ctx->h, + w->msg, "{s:s#}", "event", tok, toklen) < 0) { - flux_log_error (ctx->h, "%s: flux_respond_pack", + flux_log_error (ctx->h, + "%s: flux_respond_pack", __FUNCTION__); goto error_cancel; } @@ -273,7 +278,8 @@ static void watch_continuation (flux_future_t *f, void *arg) if (!w->guest && streq (w->path, "eventlog")) { if (check_eventlog_end (w, tok, toklen) > 0) { if (flux_kvs_lookup_cancel (w->watch_f) < 0) { - flux_log_error (ctx->h, "%s: flux_kvs_lookup_cancel", + flux_log_error (ctx->h, + "%s: flux_kvs_lookup_cancel", __FUNCTION__); goto error; } @@ -294,7 +300,8 @@ static void watch_continuation (flux_future_t *f, void *arg) if (!w->kvs_watch_canceled) { int save_errno = errno; if (flux_kvs_lookup_cancel (w->watch_f) < 0) - flux_log_error (ctx->h, "%s: flux_kvs_lookup_cancel", + flux_log_error (ctx->h, + "%s: flux_kvs_lookup_cancel", __FUNCTION__); errno = save_errno; } @@ -371,8 +378,10 @@ static int watch (struct info_ctx *ctx, return -1; } -void watch_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg) +void watch_cb (flux_t *h, + flux_msg_handler_t *mh, + const flux_msg_t *msg, + void *arg) { struct info_ctx *ctx = arg; struct watch_ctx *w = NULL; @@ -383,13 +392,13 @@ void watch_cb (flux_t *h, flux_msg_handler_t *mh, int valid_flags = FLUX_JOB_EVENT_WATCH_WAITCREATE; const char *errmsg = NULL; - if (flux_request_unpack (msg, NULL, "{s:I s:s s:i}", + if (flux_request_unpack (msg, + NULL, + "{s:I s:s s:i}", "id", &id, "path", &path, - "flags", &flags) < 0) { - flux_log_error (h, "%s: flux_request_unpack", __FUNCTION__); + "flags", &flags) < 0) goto error; - } if ((flags & ~valid_flags)) { errno = EPROTO; errmsg = "eventlog-watch request rejected with invalid flag"; @@ -440,9 +449,11 @@ static void send_kvs_watch_cancel (struct info_ctx *ctx, /* if the watching hasn't started yet, no need to cancel */ if (w->watch_f) { - if (flux_kvs_lookup_cancel (w->watch_f) < 0) - flux_log_error (ctx->h, "%s: flux_kvs_lookup_cancel", + if (flux_kvs_lookup_cancel (w->watch_f) < 0) { + flux_log_error (ctx->h, + "%s: flux_kvs_lookup_cancel", __FUNCTION__); + } } } } @@ -472,13 +483,14 @@ void watch_cleanup (struct info_ctx *ctx) while ((w = zlist_pop (ctx->watchers))) { if (w->watch_f) { - if (flux_kvs_lookup_cancel (w->watch_f) < 0) - flux_log_error (ctx->h, "%s: flux_kvs_lookup_cancel", + if (flux_kvs_lookup_cancel (w->watch_f) < 0) { + flux_log_error (ctx->h, + "%s: flux_kvs_lookup_cancel", __FUNCTION__); + } } if (flux_respond_error (ctx->h, w->msg, ENOSYS, NULL) < 0) - flux_log_error (ctx->h, "%s: flux_respond_error", - __FUNCTION__); + flux_log_error (ctx->h, "%s: flux_respond_error", __FUNCTION__); watch_ctx_destroy (w); } } diff --git a/src/modules/job-info/watch.h b/src/modules/job-info/watch.h index 415eff1964f7..fd5916f1de73 100644 --- a/src/modules/job-info/watch.h +++ b/src/modules/job-info/watch.h @@ -13,11 +13,15 @@ #include -void watch_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg); +void watch_cb (flux_t *h, + flux_msg_handler_t *mh, + const flux_msg_t *msg, + void *arg); -void watch_cancel_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg); +void watch_cancel_cb (flux_t *h, + flux_msg_handler_t *mh, + const flux_msg_t *msg, + void *arg); /* Cancel all lookups that match msg. * match credentials & matchtag if cancel true diff --git a/t/issues/t4413-empty-eventlog.sh b/t/issues/t4413-empty-eventlog.sh index af0bcdd596c9..09681b0aa1d1 100755 --- a/t/issues/t4413-empty-eventlog.sh +++ b/t/issues/t4413-empty-eventlog.sh @@ -7,6 +7,6 @@ jobpath=`flux job id --to=kvs 123456789` flux kvs put "${jobpath}.eventlog"="" # Issue 4413, previously would return Cannot allocate memory -flux job info 123456789 eventlog 2>&1 | grep "Protocol error" +flux job info 123456789 eventlog 2>&1 | grep "error parsing eventlog" diff --git a/t/t2230-job-info-lookup.t b/t/t2230-job-info-lookup.t index 79cfced80abd..c29b56bcf69e 100755 --- a/t/t2230-job-info-lookup.t +++ b/t/t2230-job-info-lookup.t @@ -381,4 +381,16 @@ test_expect_success 'lookup request with invalid flags fails with EPROTO(71)' ' | ${RPC} job-info.lookup 71 ' +# +# issue 6325 +# +test_expect_success 'flux job info on bad job id gives good error message' ' + test_must_fail flux job info fuzzybunny R 2>fuzzy.err && + grep "invalid job id" fuzzy.err +' +test_expect_success 'flux job info on bad key gives good error message' ' + test_must_fail flux job info $(flux job last) badkey 2>badkey.err && + grep "key not found" badkey.err +' + test_done diff --git a/t/t2232-job-info-security.t b/t/t2232-job-info-security.t index 9276a8bd0387..58272fc78b9e 100755 --- a/t/t2232-job-info-security.t +++ b/t/t2232-job-info-security.t @@ -4,53 +4,19 @@ test_description='Test flux job info service security' . $(dirname $0)/sharness.sh -test_under_flux 4 job - -# We have to fake a job submission by a guest into the KVS. -# This method of editing the eventlog preserves newline separators. - -update_job_userid() { - local userid=$1 - if test -n "$userid"; then - local kvsdir=$(flux job id --to=kvs $jobid) - flux kvs get --raw ${kvsdir}.eventlog \ - | sed -e 's/\("userid":\)-*[0-9]*/\1'${userid}/ \ - | flux kvs put --raw ${kvsdir}.eventlog=- - fi -} - -# Usage: submit_job [userid] -# To ensure robustness of tests despite future job manager changes, -# cancel the job, and wait for clean event. Optionally, edit the -# userid -submit_job() { - local jobid=$(flux job submit sleeplong.json) && - flux job wait-event $jobid start >/dev/null && - flux cancel $jobid && - flux job wait-event $jobid clean >/dev/null && - update_job_userid $1 && - echo $jobid -} - -# Unlike above, do not cancel the job, the test will cancel the job -submit_job_live() { - local jobid=$(flux job submit sleeplong.json) && - flux job wait-event $jobid start >/dev/null && - update_job_userid $1 && - echo $jobid -} - -# Usage: bad_first_event -# Wait for job eventlog to include 'depend' event, then mangle submit event name -bad_first_event() { - local jobid=$(flux job submit sleeplong.json) && - flux job wait-event $jobid depend >/dev/null && - local kvsdir=$(flux job id --to=kvs $jobid) && - flux kvs get --raw ${kvsdir}.eventlog \ - | sed -e s/submit/foobar/ \ - | flux kvs put --raw ${kvsdir}.eventlog=- && - flux cancel $jobid && - echo $jobid +test_under_flux 1 job + +flux version | grep -q libflux-security && test_set_prereq FLUX_SECURITY + +# Usage: submit_as_alternate_user cmd... +submit_as_alternate_user() +{ + FAKE_USERID=42 + 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 } set_userid() { @@ -63,182 +29,188 @@ unset_userid() { unset FLUX_HANDLE_ROLEMASK } -test_expect_success 'job-info: generate jobspec for simple test job' ' - flux run --dry-run -n1 -N1 sleep 300 > sleeplong.json +test_expect_success 'configure to allow guests to use test execution' ' + flux config load <<-EOT + exec.testexec.allow-guests = true + EOT +' + +test_expect_success 'submit a job as instance owner and wait for clean' ' + jobid=$(flux submit -n1 true) && + flux job wait-event $jobid clean && + echo $jobid >jobid.owner +' + +test_expect_success FLUX_SECURITY 'submit a job as guest and wait for clean' ' + jobid=$(submit_as_alternate_user -n1 true) && + flux job wait-event $jobid clean && + echo $jobid >jobid.guest ' # # job lookup (via eventlog) # -test_expect_success 'flux job eventlog works (owner)' ' - jobid=$(submit_job) && - flux job eventlog $jobid +test_expect_success 'flux job eventlog works as instance owner' ' + flux job eventlog $(cat jobid.owner) ' -test_expect_success 'flux job eventlog works (user)' ' - jobid=$(submit_job 9000) && - set_userid 9000 && - flux job eventlog $jobid && - unset_userid +test_expect_success FLUX_SECURITY 'flux job eventlog works as guest' ' + set_userid 42 && + flux job eventlog $(cat jobid.guest) ' -test_expect_success 'flux job eventlog fails (wrong user)' ' - jobid=$(submit_job 9000) && +test_expect_success FLUX_SECURITY 'flux job eventlog fails as wrong guest' ' + unset_userid && set_userid 9999 && - ! flux job eventlog $jobid && - unset_userid + test_must_fail flux job eventlog $(cat jobid.guest) ' -test_expect_success 'flux job eventlog fails on bad first event (user)' ' - jobid=$(bad_first_event 9000) && - set_userid 9999 && - ! flux job eventlog $jobid && - unset_userid +test_expect_success FLUX_SECURITY 'flux job eventlog as owner works on guest job' ' + unset_userid && + flux job eventlog $(cat jobid.guest) ' -test_expect_success 'flux job guest.exec.eventlog works via -p (owner)' ' - jobid=$(submit_job) && - flux job eventlog -p exec $jobid +test_expect_success 'flux job eventlog -p exec works as instance owner' ' + unset_userid && + flux job eventlog -p exec $(cat jobid.owner) ' -test_expect_success 'flux job guest.exec.eventlog works via -p (user)' ' - jobid=$(submit_job 9000) && - set_userid 9000 && - flux job eventlog -p exec $jobid && - unset_userid +test_expect_success FLUX_SECURITY 'flux job eventlog -p exec works as guest' ' + set_userid 42 && + flux job eventlog -p exec $(cat jobid.guest) ' -test_expect_success 'flux job guest.exec.eventlog fails via -p (wrong user)' ' - jobid=$(submit_job 9000) && +test_expect_success FLUX_SECURITY 'flux job eventlog -p exec fails as wrong guest' ' + unset_userid && set_userid 9999 && - ! flux job eventlog -p exec $jobid && - unset_userid + test_must_fail flux job eventlog -p exec $(cat jobid.guest) ' # # job info lookup (via 'info') # -test_expect_success 'flux job info eventlog works (owner)' ' - jobid=$(submit_job) && - flux job info $jobid eventlog +test_expect_success 'flux job info eventlog works as instance owner' ' + unset_userid && + flux job info $(cat jobid.owner) eventlog +' + +test_expect_success FLUX_SECURITY 'flux job info eventlog works as guest ' ' + set_userid 42 && + flux job info $(cat jobid.guest) eventlog ' -test_expect_success 'flux job info eventlog works (user)' ' - jobid=$(submit_job 9000) && - set_userid 9000 && - flux job info $jobid eventlog && - unset_userid +test_expect_success FLUX_SECURITY 'flux job info eventlog as owner works on guest job' ' + unset_userid && + flux job info $(cat jobid.guest) eventlog ' -test_expect_success 'flux job info eventlog fails (wrong user)' ' - jobid=$(submit_job 9000) && +test_expect_success FLUX_SECURITY 'flux job info eventlog fails as wrong guest' ' + unset_userid && set_userid 9999 && - ! flux job info $jobid eventlog && - unset_userid + test_must_fail flux job info $(cat jobid.guest) eventlog ' -test_expect_success 'flux job info jobspec works (owner)' ' - jobid=$(submit_job) && - flux job info $jobid jobspec +test_expect_success 'flux job info jobspec works as instance owner' ' + unset_userid && + flux job info $(cat jobid.owner) jobspec ' -test_expect_success 'flux job info jobspec works (user)' ' - jobid=$(submit_job 9000) && - set_userid 9000 && - flux job info $jobid jobspec && - unset_userid +test_expect_success FLUX_SECURITY 'flux job info jobspec works as guest' ' + unset_userid && + set_userid 42 && + flux job info $(cat jobid.guest) jobspec ' -test_expect_success 'flux job info jobspec fails (wrong user)' ' - jobid=$(submit_job 9000) && +test_expect_success FLUX_SECURITY 'flux job info jobspec fails as wrong guest' ' + unset_userid && set_userid 9999 && - ! flux job info $jobid jobspec && - unset_userid + test_must_fail flux job info $(cat jobid.guest) jobspec ' -test_expect_success 'flux job info foobar fails (owner)' ' - jobid=$(submit_job) && - ! flux job info $jobid foobar +test_expect_success 'flux job info foobar fails as instance owner' ' + unset_userid && + test_must_fail flux job info $(cat jobid.owner) foobar ' -test_expect_success 'flux job info foobar fails (user)' ' - jobid=$(submit_job 9000) && - set_userid 9000 && - ! flux job info $jobid foobar && - unset_userid +test_expect_success 'flux job info foobar fails as guest' ' + set_userid 42 && + test_must_fail flux job info $(cat jobid.guest) foobar ' -test_expect_success 'flux job info foobar fails (wrong user)' ' - jobid=$(submit_job 9000) && +test_expect_success 'flux job info foobar fails as wrong guest' ' + unset_userid && set_userid 9999 && - ! flux job info $jobid foobar && - unset_userid + test_must_fail flux job info $(cat jobid.guest) foobar ' # # job eventlog watch (via wait-event) # -test_expect_success 'flux job wait-event works (owner)' ' - jobid=$(submit_job) && - flux job wait-event $jobid submit +test_expect_success 'flux job wait-event works as instance owner' ' + unset_userid && + flux job wait-event $(cat jobid.owner) submit ' -test_expect_success 'flux job wait-event works (user)' ' - jobid=$(submit_job 9000) && - set_userid 9000 && - flux job wait-event $jobid submit && - unset_userid +test_expect_success FLUX_SECURITY 'flux job wait-event works guest' ' + set_userid 42 && + flux job wait-event $(cat jobid.guest) submit ' -test_expect_success 'flux job wait-event fails (wrong user)' ' - jobid=$(submit_job 9000) && +test_expect_success FLUX_SECURITY 'flux job wait-event fails as wrong guest' ' + unset_userid && set_userid 9999 && - ! flux job wait-event $jobid submit && - unset_userid + test_must_fail flux job wait-event $(cat jobid.guest) submit ' -test_expect_success 'flux job wait-event guest.exec.eventlog works via -p (owner)' ' - jobid=$(submit_job) && - flux job wait-event -p exec $jobid done +test_expect_success 'flux job wait-event -p exec works as instance owner' ' + unset_userid && + flux job wait-event -p exec $(cat jobid.owner) done ' -test_expect_success 'flux job wait-event guest.exec.eventlog works via -p (user)' ' - jobid=$(submit_job 9000) && - set_userid 9000 && - flux job wait-event -p exec $jobid done && - unset_userid +test_expect_success FLUX_SECURITY 'flux job wait-event -p exec as guest' ' + set_userid 42 && + flux job wait-event -p exec $(cat jobid.guest) done ' -test_expect_success 'flux job wait-event guest.exec.eventlog fails via -p (wrong user)' ' - jobid=$(submit_job 9000) && +test_expect_success FLUX_SECURITY 'flux job wait-event -p exec fails as wrong guest' ' + unset_userid && set_userid 9999 && - ! flux job wait-event -p exec $jobid done && - unset_userid + test_must_fail flux job wait-event -p exec $(cat jobid.guest) done ' -test_expect_success 'flux job wait-event guest.exec.eventlog works via -p (live job, owner)' ' - jobid=$(submit_job_live) && +test_expect_success 'flux job wait-event -p exec on running job works as instance owner' ' + unset_userid && + jobid=$(flux submit -n1 sleep 300) && flux job wait-event -p exec $jobid init && - flux cancel $jobid + echo $jobid >jobid.running ' -test_expect_success 'flux job wait-event guest.exec.eventlog works via -p (live job, user)' ' - jobid=$(submit_job_live 9000) && - set_userid 9000 && - flux job wait-event -p exec $jobid init && +test_expect_success 'cancel job' ' + unset_userid && + flux cancel $(cat jobid.running) +' + +test_expect_success FLUX_SECURITY 'flux job wait-event -p exec on running job works as guest' ' unset_userid && - flux cancel $jobid + jobid=$(submit_as_alternate_user -n1 \ + --setattr=system.exec.test.run_duration=5m true) && + set_userid 42 && + flux job wait-event -p exec $jobid init && + echo $jobid >jobid-guest.running ' -test_expect_success 'flux job wait-event guest.exec.eventlog fails via -p (live job, wrong user)' ' - jobid=$(submit_job_live 9000) && +test_expect_success FLUX_SECURITY 'flux job wait-event -p exec on running job fails as wrong guest' ' + unset_userid && set_userid 9999 && - ! flux job wait-event -p exec $jobid init && + test_must_fail flux job wait-event -p exec $(cat jobid-guest.running) init +' + +test_expect_success FLUX_SECURITY 'cancel job' ' unset_userid && - flux cancel $jobid + flux cancel $(cat jobid-guest.running) ' # @@ -255,61 +227,47 @@ test_expect_success 'create empty eventlog for job' ' flux kvs put "${jobpath}.dummy"="foobar" ' -test_expect_success 'flux job info dummy works (owner)' ' - flux job info $jobpath dummy -' - -test_expect_success 'flux job info dummy fails (user)' ' - set_userid 9000 && - flux job info 123456789 dummy 2>&1 | grep "Protocol error" && - unset_userid +test_expect_success 'flux job info dummy fails as owner' ' + test_must_fail flux job info $jobpath dummy 2>malevent.err && + grep "error parsing eventlog" malevent.err ' -test_expect_success 'create eventlog with invalid data / not JSON' ' - jobpath=`flux job id --to=kvs 123456789`&& - flux kvs put "${jobpath}.eventlog"="foobar" +test_expect_success 'flux job info dummy fails as guest' ' + set_userid 42 && + test_must_fail flux job info 123456789 dummy 2>malevent2.err && + grep "error parsing eventlog" malevent2.err ' -test_expect_success 'flux job info dummy fails (user)' ' - set_userid 9000 && - flux job info 123456789 dummy 2>&1 | grep "Protocol error" && - unset_userid +test_expect_success 'flux job info fails on malformed eventlog (not json)' ' + unset_userid && + jobpath=`flux job id --to=kvs 123456789` && + flux kvs put "${jobpath}.eventlog"="foobar" && + test_must_fail flux job info 123456789 dummy 2>malevent3.err && + grep "error parsing eventlog" malevent3.err ' -test_expect_success 'create eventlog without submit context' ' +test_expect_success 'flux job info fails on malformed eventlog (no submit event)' ' submitstr="{\"timestamp\":123.4,\"name\":\"submit\"}" && jobpath=`flux job id --to=kvs 123456789` && - echo $submitstr | flux kvs put --raw "${jobpath}.eventlog"=- + echo $submitstr | flux kvs put --raw "${jobpath}.eventlog"=- && + test_must_fail flux job info 123456789 dummy 2>malevent4.err && + grep "error parsing eventlog" malevent4.err ' -test_expect_success 'flux job info dummy fails (user)' ' - set_userid 9000 && - flux job info 123456789 dummy 2>&1 | grep "Protocol error" && - unset_userid -' - -test_expect_success 'create eventlog without submit userid' ' +test_expect_success 'flux job info fails on malformed eventlog (no submit userid)' ' submitstr="{\"timestamp\":123.4,\"name\":\"submit\",\"context\":{}}" && jobpath=`flux job id --to=kvs 123456789` && - echo $submitstr | flux kvs put --raw "${jobpath}.eventlog"=- -' - -test_expect_success 'flux job info dummy fails (user)' ' - set_userid 9000 && - flux job info 123456789 dummy 2>&1 | grep "Protocol error" && - unset_userid + echo $submitstr | flux kvs put --raw "${jobpath}.eventlog"=- && + test_must_fail flux job info 123456789 dummy 2>malevent5.err && + grep "error parsing eventlog" malevent5.err ' -test_expect_success 'create eventlog that is binary garbage' ' +test_expect_success 'flux job info fails on malformed eventlog (random garbage)' ' jobpath=`flux job id --to=kvs 123456789` && dd if=/dev/urandom bs=64 count=1 > binary.out && - flux kvs put --raw "${jobpath}.eventlog"=- < binary.out -' - -test_expect_success 'flux job info dummy fails (user)' ' - set_userid 9000 && - flux job info 123456789 dummy 2>&1 | grep "Protocol error" && - unset_userid + flux kvs put --raw "${jobpath}.eventlog"=- < binary.out && + test_must_fail flux job info 123456789 dummy 2>malevent6.err && + grep "error parsing eventlog" malevent6.err ' test_done