ENT-14731: Unit tests and syntax validation for cf-reactor

cf-reactor/reactor_transform.c

@@ -115,19 +115,15 @@ static void RecordEventsPromiseOutcome(EvalContext *ctx, const Promise *pp, Prom
 
 /* Returns the list of bundle references of the (expanded) 'then' attribute,
  * which is always a list after expansion, see ExpandBundleListReference().
- * Logs an error and returns NULL if there is no 'then' attribute or it is
- * empty. */
+ * Logs an error and returns NULL if it is empty. */
 static const Rlist *GetThenBundles(const Promise *pp)
 {
     assert(pp != NULL);
 
+    /* A promise without 'then' is rejected by the policy checks, see
+     * EventsParseTreeCheck() */
     const Constraint *then_constraint = PromiseGetConstraint(pp, "then");
-    if (then_constraint == NULL)
-    {
-        Log(LOG_LEVEL_ERR, "Reactor events promise '%s' does not specify a 'then' bundle", pp->promiser);
-        return NULL;
-    }
-
+    assert(then_constraint != NULL);
     assert(then_constraint->rval.type == RVAL_TYPE_LIST);
     const Rlist *bundles = RvalRlistValue(then_constraint->rval);
     if (bundles == NULL)

libpromises/mod_reactor.c

@@ -26,6 +26,15 @@
 
 #include <syntax.h>
 #include <policy.h>
+#include <rlist.h>
+#include <string_lib.h>
+
+static const char *const POLICY_ERROR_EVENTS_MISSING_ATTRIBUTE =
+    "Missing required attribute '%s' for promise type 'events'";
+static const char *const POLICY_ERROR_EVENTS_EMPTY_THEN =
+    "Empty list of bundles in attribute 'then' for promise type 'events'";
+static const char *const POLICY_ERROR_WHEN_BODY_NO_EVENT =
+    "Body when %s requires an event attribute, such as " REACTOR_FILE_DELETED;
 
 static const ConstraintSyntax when_constraints[] =
 {
@@ -37,7 +46,34 @@ static const ConstraintSyntax when_constraints[] =
     ConstraintSyntaxNewNull()
 };
 
-static const BodySyntax when_body = BodySyntaxNew("when", when_constraints, NULL, SYNTAX_STATUS_NORMAL);
+/* Checks that a 'when' body has an event attribute, i.e. any attribute other
+ * than the global ones (meta, inherit_from) and 'interval', which only sets
+ * how the event is checked for. A body inheriting from another one may get
+ * its event from it, and that one is checked on its own. */
+static bool WhenBodyCheck(const Body *body, Seq *errors)
+{
+    assert(body != NULL);
+
+    if (BodyHasConstraint(body, "inherit_from"))
+    {
+        return true;
+    }
+
+    for (size_t i = 0; i < SeqLength(body->conlist); i++)
+    {
+        const Constraint *cp = SeqAt(body->conlist, i);
+        if (!StringEqual(cp->lval, "meta") && !StringEqual(cp->lval, "interval"))
+        {
+            return true;
+        }
+    }
+
+    SeqAppend(errors, PolicyErrorNew(POLICY_ELEMENT_TYPE_BODY, body,
+                                     POLICY_ERROR_WHEN_BODY_NO_EVENT, body->name));
+    return false;
+}
+
+static const BodySyntax when_body = BodySyntaxNew("when", when_constraints, &WhenBodyCheck, SYNTAX_STATUS_NORMAL);
 
 static const ConstraintSyntax CF_EVENT_BODIES[] =
 {
@@ -48,7 +84,33 @@ static const ConstraintSyntax CF_EVENT_BODIES[] =
 
 static bool EventsParseTreeCheck(const Promise *pp, Seq *errors)
 {
-    return PromiseCheckBundleCallArity(pp, "then", errors);
+    bool success = true;
+
+    if (PromiseGetConstraint(pp, "when") == NULL)
+    {
+        SeqAppend(errors, PolicyErrorNew(POLICY_ELEMENT_TYPE_PROMISE, pp,
+                                         POLICY_ERROR_EVENTS_MISSING_ATTRIBUTE, "when"));
+        success = false;
+    }
+
+    /* Only a literally empty list can be caught here, a list made empty by an
+     * empty @(list) is caught on evaluation. */
+    const Constraint *then = PromiseGetConstraint(pp, "then");
+    if (then == NULL)
+    {
+        SeqAppend(errors, PolicyErrorNew(POLICY_ELEMENT_TYPE_PROMISE, pp,
+                                         POLICY_ERROR_EVENTS_MISSING_ATTRIBUTE, "then"));
+        success = false;
+    }
+    else if (then->rval.type == RVAL_TYPE_LIST && RvalRlistValue(then->rval) == NULL)
+    {
+        SeqAppend(errors, PolicyErrorNew(POLICY_ELEMENT_TYPE_CONSTRAINT, then,
+                                         POLICY_ERROR_EVENTS_EMPTY_THEN));
+        success = false;
+    }
+
+    success &= PromiseCheckBundleCallArity(pp, "then", errors);
+    return success;
 }
 
 const PromiseTypeSyntax CF_REACTOR_PROMISE_TYPES[] =

tests/unit/Makefile.am

@@ -157,6 +157,7 @@ check_PROGRAMS = \
 	iteration_test \
 	protocol_recv_overflow_test \
 	watcher_test \
+	file_watcher_test \
 	stoppable_thread_test \
 	wakeup_channel_test
 
@@ -236,6 +237,9 @@ watcher_test_SOURCES = watcher_test.c \
 	../../cf-reactor/stoppable_thread.c \
 	../../cf-reactor/wakeup_channel.c
 
+file_watcher_test_SOURCES = file_watcher_test.c \
+	../../cf-reactor/file_watcher.c
+
 stoppable_thread_test_SOURCES = stoppable_thread_test.c \
 	../../cf-reactor/stoppable_thread.c
 

tests/unit/file_watcher_test.c

@@ -0,0 +1,219 @@
+#include <test.h>
+
+#include <file_watcher.h>       /* FileWatcherStateNew(), FileWatcherCheckFileDeleted() */
+#include <rlist.h>              /* RlistAppendScalar() */
+#include <logging_priv.h>       /* LoggingPrivContext, LoggingPrivSetContext() */
+
+#include <string.h>             /* strstr() */
+#include <stdio.h>              /* snprintf() */
+#include <stdlib.h>             /* mkdtemp() */
+#include <unistd.h>             /* close(), unlink(), rmdir(), symlink() */
+#include <fcntl.h>              /* open() */
+
+static int captured_err_count = 0;
+static char captured_err_message[256];
+
+static char *CaptureErrorLogHook(ARG_UNUSED LoggingPrivContext *pctx, LogLevel level, const char *message)
+{
+    if (level == LOG_LEVEL_ERR)
+    {
+        captured_err_count++;
+        strlcpy(captured_err_message, message, sizeof(captured_err_message));
+    }
+    return (char *) message;
+}
+
+/* Returns a new state of a file_deleted watcher of the file at path */
+static void *NewFileDeletedState(const char *path)
+{
+    Rlist *args = NULL;
+    RlistAppendScalar(&args, path);
+    void *state = FileWatcherStateNew(args);
+    RlistDestroy(args);
+    return state;
+}
+
+/* Creates the file 'file' in a new temporary directory dir */
+static void CreateTempFile(char *dir, char *path, size_t path_size)
+{
+    assert_true(mkdtemp(dir) != NULL);
+    snprintf(path, path_size, "%s/file", dir);
+    int file_fd = open(path, O_CREAT | O_WRONLY, 0600);
+    assert_true(file_fd >= 0);
+    close(file_fd);
+}
+
+/* Sets up dir/loop, a symlink to itself, so that lstat() of dir/loop/file
+ * fails with ELOOP (even as root), i.e. whether the file was deleted can't
+ * be told. Unlinking the loop makes lstat() work again (ENOENT). */
+static void SetUpSymlinkLoop(char *dir, char *loop, size_t loop_size, char *path, size_t path_size)
+{
+    assert_true(mkdtemp(dir) != NULL);
+    snprintf(loop, loop_size, "%s/loop", dir);
+    assert_int_equal(symlink(loop, loop), 0);
+    snprintf(path, path_size, "%s/file", loop);
+}
+
+static void TearDownSymlinkLoop(const char *dir, const char *loop)
+{
+    unlink(loop);
+    assert_int_equal(rmdir(dir), 0);
+}
+
+static void test_check_file_deleted_transitions(void)
+{
+    char dir[] = "/tmp/file_watcher_test.XXXXXX";
+    char path[PATH_MAX];
+    CreateTempFile(dir, path, sizeof(path));
+
+    void *state = NewFileDeletedState(path);
+
+    /* Present (the first check is the reference) */
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_NO_EVENT);
+
+    /* Deleted: the event happens once, not while the file stays absent */
+    assert_int_equal(unlink(path), 0);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_EVENT);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_NO_EVENT);
+
+    /* Created again, then deleted again: the event happens again */
+    int file_fd = open(path, O_CREAT | O_WRONLY, 0600);
+    assert_true(file_fd >= 0);
+    close(file_fd);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_NO_EVENT);
+    assert_int_equal(unlink(path), 0);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_EVENT);
+
+    FileWatcherStateDestroy(state);
+    assert_int_equal(rmdir(dir), 0);
+}
+
+static void test_check_file_deleted_absent_at_start(void)
+{
+    char dir[] = "/tmp/file_watcher_test.XXXXXX";
+    char path[PATH_MAX];
+    assert_true(mkdtemp(dir) != NULL);
+    snprintf(path, sizeof(path), "%s/file", dir);
+
+    void *state = NewFileDeletedState(path);
+
+    /* The first check is the reference, at which the file is absent */
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_NO_EVENT);
+
+    /* Still absent: never seen, so not deleted */
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_NO_EVENT);
+
+    /* Seen once created, so deleted afterwards */
+    int file_fd = open(path, O_CREAT | O_WRONLY, 0600);
+    assert_true(file_fd >= 0);
+    close(file_fd);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_NO_EVENT);
+    assert_int_equal(unlink(path), 0);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_EVENT);
+
+    FileWatcherStateDestroy(state);
+    assert_int_equal(rmdir(dir), 0);
+}
+
+static void test_check_file_deleted_dangling_symlink(void)
+{
+    char dir[] = "/tmp/file_watcher_test.XXXXXX";
+    char path[PATH_MAX];
+    char link[PATH_MAX];
+    CreateTempFile(dir, path, sizeof(path));
+    snprintf(link, sizeof(link), "%s/link", dir);
+    assert_int_equal(symlink(path, link), 0);
+
+    void *state = NewFileDeletedState(link);
+
+    /* The first check is the reference */
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_NO_EVENT);
+
+    /* Symlinks are not followed: the link is not deleted when its target
+     * is, only when the link itself is */
+    assert_int_equal(unlink(path), 0);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_NO_EVENT);
+    assert_int_equal(unlink(link), 0);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_EVENT);
+
+    FileWatcherStateDestroy(state);
+    assert_int_equal(rmdir(dir), 0);
+}
+
+static void test_check_file_deleted_error(void)
+{
+    char dir[] = "/tmp/file_watcher_test.XXXXXX";
+    char loop[PATH_MAX];
+    char path[PATH_MAX];
+    SetUpSymlinkLoop(dir, loop, sizeof(loop), path, sizeof(path));
+
+    const LogLevel old_level = LogGetGlobalLevel();
+    LogSetGlobalLevel(LOG_LEVEL_CRIT);
+
+    void *state = NewFileDeletedState(path);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_ERROR);
+    FileWatcherStateDestroy(state);
+
+    LogSetGlobalLevel(old_level);
+    TearDownSymlinkLoop(dir, loop);
+}
+
+static void test_check_file_deleted_logs_error_once(void)
+{
+    char dir[] = "/tmp/file_watcher_test.XXXXXX";
+    char loop[PATH_MAX];
+    char path[PATH_MAX];
+    SetUpSymlinkLoop(dir, loop, sizeof(loop), path, sizeof(path));
+
+    captured_err_count = 0;
+    captured_err_message[0] = '\0';
+    LoggingPrivContext log_ctx = {
+        .log_hook = CaptureErrorLogHook,
+        .force_hook_level = LOG_LEVEL_ERR,
+    };
+    LoggingPrivSetContext(&log_ctx);
+    const LogLevel old_level = LogGetGlobalLevel();
+    LogSetGlobalLevel(LOG_LEVEL_CRIT);
+
+    /* The failure of the first check is logged */
+    void *state = NewFileDeletedState(path);
+    assert_int_equal(captured_err_count, 0);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_ERROR);
+    assert_int_equal(captured_err_count, 1);
+    assert_true(strstr(captured_err_message, "Unable to lstat") != NULL);
+
+    /* The following failures are not logged again */
+    for (int i = 0; i < 3; i++)
+    {
+        assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_ERROR);
+    }
+    assert_int_equal(captured_err_count, 1);
+
+    /* Once lstat() works again, the next failure is logged again */
+    assert_int_equal(unlink(loop), 0);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_NO_EVENT);
+    assert_int_equal(symlink(loop, loop), 0);
+    assert_int_equal(FileWatcherCheckFileDeleted(state), WATCHER_CHECK_ERROR);
+    assert_int_equal(captured_err_count, 2);
+
+    FileWatcherStateDestroy(state);
+
+    LogSetGlobalLevel(old_level);
+    LoggingPrivSetContext(NULL);
+    TearDownSymlinkLoop(dir, loop);
+}
+
+int main()
+{
+    PRINT_TEST_BANNER();
+    const UnitTest tests[] =
+    {
+        unit_test(test_check_file_deleted_transitions),
+        unit_test(test_check_file_deleted_absent_at_start),
+        unit_test(test_check_file_deleted_dangling_symlink),
+        unit_test(test_check_file_deleted_error),
+        unit_test(test_check_file_deleted_logs_error_once),
+    };
+
+    return run_tests(tests);
+}