From 448644365fb3f205f0ecf0122deb2f80880a5f21 Mon Sep 17 00:00:00 2001 From: Victor Moene Date: Thu, 1 Oct 2026 19:09:16 +0200 Subject: [PATCH 1/2] Reset classes and variables between "then" bundle runs in cf-reactor The issue is that classes and variables declared once will stay declared until the next policy reload. So a class can stay defined between runs, when the state it checks changed. Instead, the state of all vars and classes is saved in a snapshot before every event, and restored once the event is handled. Persistent classes are left out and loaded from the state database at every event, so they stay defined across runs until they expire, as in cf-agent. Signed-off-by: Victor Moene --- cf-reactor/Makefile.am | 1 + cf-reactor/reactor_transform.c | 19 ++++++ libpromises/class.c | 20 +++++++ libpromises/class.h | 6 ++ libpromises/eval_context.c | 102 +++++++++++++++++++++++++++++++++ libpromises/eval_context.h | 24 ++++++++ libpromises/variable.c | 26 +++++++++ libpromises/variable.h | 6 ++ 8 files changed, 204 insertions(+) diff --git a/cf-reactor/Makefile.am b/cf-reactor/Makefile.am index 710befaef0c..4036d272376 100644 --- a/cf-reactor/Makefile.am +++ b/cf-reactor/Makefile.am @@ -26,6 +26,7 @@ noinst_LTLIBRARIES = libcf-reactor.la AM_CPPFLAGS = -I$(srcdir)/../libpromises -I$(srcdir)/../libntech/libutils \ -I$(srcdir)/../libcfecompat \ -I$(srcdir)/../libcfnet \ + -I$(srcdir)/../libenv \ -I$(srcdir)/../cf-agent \ $(OPENSSL_CPPFLAGS) \ $(PCRE2_CPPFLAGS) \ diff --git a/cf-reactor/reactor_transform.c b/cf-reactor/reactor_transform.c index 6660a8308aa..d5b116d6bed 100644 --- a/cf-reactor/reactor_transform.c +++ b/cf-reactor/reactor_transform.c @@ -38,6 +38,8 @@ #include #include // ConnCache_Init(), ConnCache_Destroy() #include // Initialize/FinalizeCustomPromises() +#include // SetReferenceTime() +#include // UpdateTimeClasses() /* Promise types evaluated within `bundle reactor NAME { ... }`. */ static const char *const REACTOR_TYPESEQUENCE[] = @@ -258,6 +260,16 @@ void HandleReactorEvent(EvalContext *ctx, const Promise *pp, const char *promise assert(pp != NULL); assert(promiser != NULL); + /* cf-reactor keeps the same EvalContext across events. Like an agent run, + * the bundle runs see the time classes of now and the persistent classes + * that have not expired. The classes and variables they define, cancel or + * change are undone once the event is handled, except the persistent + * classes, which are kept in the state database. */ + UpdateTimeClasses(ctx, SetReferenceTime()); + ClassTable *classes = NULL; + VariableTable *variables = NULL; + EvalContextSnapshotTake(ctx, &classes, &variables); + ReactorEventParam event = { .promiser = promiser }; EvalContextStackPushBundleFrame(ctx, PromiseGetBundle(pp), NULL, false, NULL); @@ -265,6 +277,8 @@ void HandleReactorEvent(EvalContext *ctx, const Promise *pp, const char *promise ExpandPromise(ctx, pp, KeepEventsPromiseOnEvent, &event); EvalContextStackPopFrame(ctx); /* bundle section */ EvalContextStackPopFrame(ctx); /* bundle */ + + EvalContextSnapshotRestore(ctx, classes, variables); } void KeepReactorPromises(EvalContext *ctx, const Policy *policy) @@ -289,4 +303,9 @@ void KeepReactorPromises(EvalContext *ctx, const Policy *policy) EvaluateReactorBundle(ctx, bp); } + + /* Tag the persistent classes the evaluation defined 'source=persistent', + * so that the snapshots taken on events tell them apart even once another + * agent deleted them from the state database, see EvalContextSnapshotTake() */ + EvalContextHeapPersistentLoadAll(ctx); } diff --git a/libpromises/class.c b/libpromises/class.c index dcd4f5c6bd8..df4f7492d99 100644 --- a/libpromises/class.c +++ b/libpromises/class.c @@ -231,6 +231,26 @@ bool ClassTableClear(ClassTable *table) return has_classes; } +ClassTable *ClassTableCopy(const ClassTable *table) +{ + assert(table != NULL); + + ClassTable *copy = ClassTableNew(); + + ClassTableIterator *iter = ClassTableIteratorNew(table, NULL, true, true); + for (const Class *cls = ClassTableIteratorNext(iter); cls != NULL; + cls = ClassTableIteratorNext(iter)) + { + /* The tags of a class are never NULL, see ClassInit() */ + StringSet *tags = StringSetNew(); + StringSetJoin(tags, cls->tags, xstrdup); + ClassTablePut(copy, cls->ns, cls->name, cls->is_soft, cls->scope, tags, cls->comment); + } + ClassTableIteratorDestroy(iter); + + return copy; +} + ClassTableIterator *ClassTableIteratorNew(const ClassTable *table, const char *ns, bool is_hard, bool is_soft) diff --git a/libpromises/class.h b/libpromises/class.h index 67b1d8cb209..918f0df25d6 100644 --- a/libpromises/class.h +++ b/libpromises/class.h @@ -53,6 +53,12 @@ bool ClassTableRemove(ClassTable *table, const char *ns, const char *name); bool ClassTableClear(ClassTable *table); +/** + * @brief Deep copy of the table: all its classes, hard and soft, with their + * tags and comments. The copy is owned by the caller. + */ +ClassTable *ClassTableCopy(const ClassTable *table); + ClassTableIterator *ClassTableIteratorNew(const ClassTable *table, const char *ns, bool is_hard, bool is_soft); Class *ClassTableIteratorNext(ClassTableIterator *iter); void ClassTableIteratorDestroy(ClassTableIterator *iter); diff --git a/libpromises/eval_context.c b/libpromises/eval_context.c index 2fbe3ffddb8..944098790f8 100644 --- a/libpromises/eval_context.c +++ b/libpromises/eval_context.c @@ -893,6 +893,108 @@ void EvalContextHeapPersistentLoadAll(EvalContext *ctx) CloseDB(dbp); } +/*****************************************************************************/ + +/* Whether the class is persistent, expired or not: loaded from the state + * database, which tags it 'source=persistent', or defined by a persistent + * class promise, which saves it in the database without tagging it. The keys + * are the class names as ClassRefToString() makes them, see + * EvalContextHeapPersistentSave(). dbp may be NULL if the database cannot be + * opened. */ +static bool IsPersistentClass(CF_DB *dbp, const Class *cls) +{ + assert(cls != NULL); + + if (StringSetContains(cls->tags, "source=persistent")) + { + return true; + } + if (dbp == NULL) + { + return false; + } + + char *key = ClassRefToString(cls->ns, cls->name); + const bool persistent = HasKeyDB(dbp, key, strlen(key) + 1); + free(key); + return persistent; +} + +void EvalContextSnapshotTake(EvalContext *ctx, ClassTable **classes, VariableTable **variables) +{ + assert(ctx != NULL); + assert(classes != NULL); + assert(variables != NULL); + + /* 1. Copy the global classes and variables, the hard classes and the + * special variables too. The global variables include the variables of + * every bundle, stored under the scope of their bundle. The bundle + * scoped classes are not in the global table, they disappear with the + * frame of their bundle, and so do the 'this', 'body' and 'edit' + * variables. */ + *classes = ClassTableCopy(ctx->global_classes); + *variables = VariableTableCopy(ctx->global_variables); + + /* 2. Remove the persistent classes, expired or not, from both the copy and + * the context: their lifetime is their TTL in the state database, not + * the snapshot nor the context, which may have outlived it. Collect + * first, the context must not change while being iterated. */ + CF_DB *dbp; + if (!OpenDB(&dbp, dbid_state)) + { + dbp = NULL; + } + + Seq *persistent_classes = SeqNew(16, free); + /* ClassTableIteratorNext() does not filter the hard classes out */ + ClassTableIterator *iter = ClassTableIteratorNew(ctx->global_classes, NULL, false, true); + for (const Class *cls = ClassTableIteratorNext(iter); cls != NULL; + cls = ClassTableIteratorNext(iter)) + { + if (cls->is_soft && IsPersistentClass(dbp, cls)) + { + ClassTableRemove(*classes, cls->ns, cls->name); + SeqAppend(persistent_classes, ClassRefToString(cls->ns, cls->name)); + } + } + ClassTableIteratorDestroy(iter); + + if (dbp != NULL) + { + CloseDB(dbp); + } + + for (size_t i = 0; i < SeqLength(persistent_classes); i++) + { + ClassRef ref = ClassRefParse(SeqAt(persistent_classes, i)); + ClassTableRemove(ctx->global_classes, ref.ns, ref.name); + ClassRefDestroy(ref); + } + SeqDestroy(persistent_classes); + + /* 3. Load the persistent classes that have not expired into the context, + * like an agent does when it starts. The expired ones are deleted from + * the database instead. */ + EvalContextHeapPersistentLoadAll(ctx); +} + +void EvalContextSnapshotRestore(EvalContext *ctx, ClassTable *classes, VariableTable *variables) +{ + assert(ctx != NULL); + assert(classes != NULL); + assert(variables != NULL); + + /* Replace the global classes and variables with the snapshot, which + * discards the ones defined, cancelled or changed since, the persistent + * classes too: the next snapshot loads them again from the state + * database, see EvalContextSnapshotTake(). */ + ClassTableDestroy(ctx->global_classes); + ctx->global_classes = classes; + + VariableTableDestroy(ctx->global_variables); + ctx->global_variables = variables; +} + void EvalContextSetNegatedClasses(EvalContext *ctx, StringSet *negated_classes) { assert(ctx != NULL); diff --git a/libpromises/eval_context.h b/libpromises/eval_context.h index 2537c083f65..721c017353b 100644 --- a/libpromises/eval_context.h +++ b/libpromises/eval_context.h @@ -145,6 +145,30 @@ void EvalContextHeapPersistentSave(EvalContext *ctx, const char *name, unsigned void EvalContextHeapPersistentRemove(const char *context); void EvalContextHeapPersistentLoadAll(EvalContext *ctx); +/** + * @brief Save a copy of the global classes, except the persistent ones, and + * of the global variables, so that EvalContextSnapshotRestore() can + * undo what is evaluated in between. Then reset the persistent classes + * of the context to the ones in the state database that have not + * expired, as an agent starting a run does. + * + * @param classes set to the copy of the classes, owned by the caller + * @param variables set to the copy of the variables, owned by the caller + * @note The copied variables refer to the promises of the policy, so the + * snapshot must be restored (or destroyed) before the policy is. + */ +void EvalContextSnapshotTake(EvalContext *ctx, ClassTable **classes, VariableTable **variables); + +/** + * @brief Replace the global classes and variables with the snapshot taken by + * EvalContextSnapshotTake(). The persistent classes are not in it, the + * next snapshot loads them again from the state database. + * + * @param classes the copy of the classes, owned by the context afterwards + * @param variables the copy of the variables, owned by the context afterwards + */ +void EvalContextSnapshotRestore(EvalContext *ctx, ClassTable *classes, VariableTable *variables); + void EvalContextOverrideImmutableSet(EvalContext *ctx, bool should_override); bool EvalContextOverrideImmutableGet(EvalContext *ctx); diff --git a/libpromises/variable.c b/libpromises/variable.c index 213c342531a..d0363e15ba0 100644 --- a/libpromises/variable.c +++ b/libpromises/variable.c @@ -257,6 +257,32 @@ bool VariableTablePut(VariableTable *table, const VarRef *ref, return VarMapInsert(table->vars, var->ref, var); } +VariableTable *VariableTableCopy(const VariableTable *table) +{ + assert(table != NULL); + + VariableTable *copy = VariableTableNew(); + + VariableTableIterator *iter = VariableTableIteratorNew(table, NULL, NULL, NULL); + for (const Variable *var = VariableTableIteratorNext(iter); var != NULL; + var = VariableTableIteratorNext(iter)) + { + /* VariableTablePut() copies the value, but takes the tags and the + * comment. The tags may be NULL. */ + StringSet *tags = NULL; + if (var->tags != NULL) + { + tags = StringSetNew(); + StringSetJoin(tags, var->tags, xstrdup); + } + VariableTablePut(copy, var->ref, &var->rval, var->type, tags, + SafeStringDuplicate(var->comment), var->promise); + } + VariableTableIteratorDestroy(iter); + + return copy; +} + bool VariableTableClear(VariableTable *table, const char *ns, const char *scope, const char *lval) { const size_t vars_num = VarMapSize(table->vars); diff --git a/libpromises/variable.h b/libpromises/variable.h index 1c653a85d40..65b779aef32 100644 --- a/libpromises/variable.h +++ b/libpromises/variable.h @@ -73,6 +73,12 @@ bool VariableTableRemove(VariableTable *table, const VarRef *ref); size_t VariableTableCount(const VariableTable *table, const char *ns, const char *scope, const char *lval); bool VariableTableClear(VariableTable *table, const char *ns, const char *scope, const char *lval); +/** + * @brief Deep copy of the table, with all its variables. The copied variables + * refer to the same promises. + */ +VariableTable *VariableTableCopy(const VariableTable *table); + VariableTableIterator *VariableTableIteratorNew(const VariableTable *table, const char *ns, const char *scope, const char *lval); VariableTableIterator *VariableTableIteratorNewFromVarRef(const VariableTable *table, const VarRef *ref); Variable *VariableTableIteratorNext(VariableTableIterator *iter); From a715a166754894b490648adacbce86d925cf2b2b Mon Sep 17 00:00:00 2001 From: Victor Moene Date: Thu, 8 Oct 2026 14:01:12 +0200 Subject: [PATCH 2/2] Add unit tests for the cf-reactor snapshot functions Adds tests for ClassTableCopy(), VariableTableCopy() and snapshot take/restore, and corrects a comment that wrongly said bundle variables aren't in the global variable table. Signed-off-by: Victor Moene --- tests/unit/class_test.c | 38 +++++++++++++++++++++ tests/unit/eval_context_test.c | 52 +++++++++++++++++++++++++++++ tests/unit/variable_test.c | 60 ++++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+) diff --git a/tests/unit/class_test.c b/tests/unit/class_test.c index 5fcce060045..b25e7771df6 100644 --- a/tests/unit/class_test.c +++ b/tests/unit/class_test.c @@ -106,6 +106,43 @@ static void test_put_replace(void) ClassTableDestroy(t); } +static void test_copy(void) +{ + ClassTable *t = ClassTableNew(); + assert_false(ClassTablePut(t, NULL, "hard", false, CONTEXT_SCOPE_NAMESPACE, NULL, NULL)); + assert_false(ClassTablePut(t, "ns", "soft", true, CONTEXT_SCOPE_NAMESPACE, + StringSetFromString("a,b", ','), "a comment")); + + ClassTable *copy = ClassTableCopy(t); + + /* Removing or adding classes in the original does not change the copy */ + assert_true(ClassTableRemove(t, "ns", "soft")); + assert_false(ClassTablePut(t, NULL, "added", true, CONTEXT_SCOPE_NAMESPACE, NULL, NULL)); + assert_true(ClassTableGet(copy, NULL, "added") == NULL); + + Class *cls = ClassTableGet(copy, NULL, "hard"); + assert_true(cls != NULL); + assert_true(cls->ns == NULL); + assert_false(cls->is_soft); + assert_int_equal(CONTEXT_SCOPE_NAMESPACE, cls->scope); + assert_true(cls->comment == NULL); + + cls = ClassTableGet(copy, "ns", "soft"); + assert_true(cls != NULL); + assert_string_equal("ns", cls->ns); + assert_true(cls->is_soft); + assert_int_equal(CONTEXT_SCOPE_NAMESPACE, cls->scope); + assert_string_equal("a comment", cls->comment); + assert_int_equal(2, StringSetSize(cls->tags)); + assert_true(StringSetContains(cls->tags, "a")); + assert_true(StringSetContains(cls->tags, "b")); + + /* The copy owns its classes, it outlives the original */ + ClassTableDestroy(t); + assert_true(ClassTableGet(copy, NULL, "hard") != NULL); + ClassTableDestroy(copy); +} + int main() { PRINT_TEST_BANNER(); @@ -115,6 +152,7 @@ int main() unit_test(test_ns), unit_test(test_class_ref), unit_test(test_put_replace), + unit_test(test_copy), }; return run_tests(tests); diff --git a/tests/unit/eval_context_test.c b/tests/unit/eval_context_test.c index 88a192bbaf9..bfef1ce475a 100644 --- a/tests/unit/eval_context_test.c +++ b/tests/unit/eval_context_test.c @@ -244,6 +244,57 @@ static void test_secret_container_redacts_indexed_read(void) EvalContextDestroy(ctx); } +/* What a "then" bundle of cf-reactor defines between the snapshot and its + * restoration is gone afterwards, the variables of a bundle included, as they + * are global. What was defined before is kept, and the persistent classes + * come back from the state database with the next snapshot. */ +static void test_snapshot_take_restore(void) +{ + EvalContext *ctx = EvalContextNew(); + + VarRef *before = VarRefParse("default:snapshot_bundle.before"); + VarRef *during = VarRefParse("default:snapshot_bundle.during"); + + assert_true(EvalContextClassPutHard(ctx, "snapshot_hard", "")); + assert_true(EvalContextClassPutSoft(ctx, "snapshot_soft_before", CONTEXT_SCOPE_NAMESPACE, "")); + assert_true(EvalContextVariablePut(ctx, before, "old", CF_DATA_TYPE_STRING, "")); + + ClassTable *classes; + VariableTable *variables; + EvalContextSnapshotTake(ctx, &classes, &variables); + + /* Like a class promise with 'persistence' */ + EvalContextHeapPersistentSave(ctx, "snapshot_persistent", 10, CONTEXT_STATE_POLICY_RESET, ""); + assert_true(EvalContextClassPutSoft(ctx, "snapshot_persistent", CONTEXT_SCOPE_NAMESPACE, "")); + + assert_true(EvalContextClassPutSoft(ctx, "snapshot_soft_during", CONTEXT_SCOPE_NAMESPACE, "")); + assert_true(EvalContextVariablePut(ctx, during, "new", CF_DATA_TYPE_STRING, "")); + assert_true(EvalContextVariablePut(ctx, before, "new", CF_DATA_TYPE_STRING, "")); + assert_true(EvalContextClassRemove(ctx, NULL, "snapshot_soft_before")); + + EvalContextSnapshotRestore(ctx, classes, variables); + + assert_true(EvalContextClassGet(ctx, NULL, "snapshot_hard") != NULL); + assert_true(EvalContextClassGet(ctx, NULL, "snapshot_soft_before") != NULL); + assert_true(EvalContextClassGet(ctx, NULL, "snapshot_soft_during") == NULL); + assert_true(EvalContextVariableGet(ctx, during, NULL, false) == NULL); + assert_string_equal("old", EvalContextVariableGet(ctx, before, NULL, false)); + + /* The persistent class is not in the snapshot, the next one loads it */ + assert_true(EvalContextClassGet(ctx, NULL, "snapshot_persistent") == NULL); + EvalContextSnapshotTake(ctx, &classes, &variables); + { + const Class *cls = EvalContextClassGet(ctx, NULL, "snapshot_persistent"); + assert_true(cls != NULL); + assert_true(StringSetContains(cls->tags, "source=persistent")); + } + EvalContextSnapshotRestore(ctx, classes, variables); + + VarRefDestroy(before); + VarRefDestroy(during); + EvalContextDestroy(ctx); +} + int main() { PRINT_TEST_BANNER(); @@ -256,6 +307,7 @@ int main() unit_test(test_changes_chroot), unit_test(test_eval_with_token_from_list), unit_test(test_secret_container_redacts_indexed_read), + unit_test(test_snapshot_take_restore), }; int ret = run_tests(tests); diff --git a/tests/unit/variable_test.c b/tests/unit/variable_test.c index 7bad4a7c680..ebca58d394e 100644 --- a/tests/unit/variable_test.c +++ b/tests/unit/variable_test.c @@ -242,6 +242,65 @@ static void test_clear(void) } } +static void test_copy(void) +{ + VariableTable *t = ReferenceTable(); + { + VarRef *ref = VarRefParse("scope1.tagged"); + Rval rval = (Rval) { "value", RVAL_TYPE_SCALAR }; + assert_false(VariableTablePut(t, ref, &rval, CF_DATA_TYPE_STRING, + StringSetFromString("a,b", ','), + xstrdup("a comment"), NULL)); + VarRefDestroy(ref); + } + + VariableTable *copy = VariableTableCopy(t); + assert_int_equal(VariableTableCount(t, NULL, NULL, NULL), + VariableTableCount(copy, NULL, NULL, NULL)); + + /* Removing, replacing or adding variables in the original does not change + * the copy */ + { + VarRef *ref = VarRefParse("scope1.lval1"); + assert_true(VariableTableRemove(t, ref)); + VarRefDestroy(ref); + } + { + VarRef *ref = VarRefParse("scope1.lval2"); + Rval rval = (Rval) { "replaced", RVAL_TYPE_SCALAR }; + assert_true(VariableTablePut(t, ref, &rval, CF_DATA_TYPE_STRING, NULL, NULL, NULL)); + VarRefDestroy(ref); + } + assert_false(PutVar(t, "scope1.added")); + { + VarRef *ref = VarRefParse("scope1.added"); + assert_true(VariableTableGet(copy, ref) == NULL); + VarRefDestroy(ref); + } + + /* The copy owns its variables, it outlives the original */ + VariableTableDestroy(t); + + TestGet(copy, "scope1.lval1"); + TestGet(copy, "scope1.lval2"); + TestGet(copy, "scope1.array[two][three]"); + TestGet(copy, "ns1:scope2.lval1"); + { + VarRef *ref = VarRefParse("scope1.tagged"); + Variable *v = VariableTableGet(copy, ref); + assert_true(v != NULL); + assert_string_equal("value", RvalScalarValue(v->rval)); + assert_int_equal(CF_DATA_TYPE_STRING, v->type); + assert_string_equal("a comment", v->comment); + assert_int_equal(2, StringSetSize(v->tags)); + assert_true(StringSetContains(v->tags, "a")); + assert_true(StringSetContains(v->tags, "b")); + VarRefDestroy(ref); + } + + VariableTableDestroy(copy); +} + static void test_counting(void) { VariableTable *t = ReferenceTable(); @@ -383,6 +442,7 @@ int main() unit_test(test_clear), unit_test(test_counting), unit_test(test_iterate_indices), + unit_test(test_copy), }; return run_tests(tests);