CFE-4686: Add cancel attribute to classes promises to undefine a class

libpromises/eval_context.h

@@ -169,6 +169,7 @@ bool EvalContextClassPutHard(EvalContext *ctx, const char *name, const char *tag
 Class *EvalContextClassGet(const EvalContext *ctx, const char *ns, const char *name);
 Class *EvalContextClassMatch(const EvalContext *ctx, const char *regex);
 bool EvalContextClassRemove(EvalContext *ctx, const char *ns, const char *name);
+void EvalContextStackFrameRemoveSoft(EvalContext *ctx, const char *context);
 StringSet *EvalContextClassTags(const EvalContext *ctx, const char *ns, const char *name);
 
 ClassTableIterator *EvalContextClassTableIteratorNewGlobal(const EvalContext *ctx, const char *ns, bool is_hard, bool is_soft);

libpromises/mod_common.c

@@ -219,6 +219,7 @@ const ConstraintSyntax CF_CLASSBODY[] =
 {
     ConstraintSyntaxNewOption("scope", "namespace,bundle", "Scope of the class set by this promise", SYNTAX_STATUS_NORMAL),
     ConstraintSyntaxNewContextList("and", "Combine class sources with AND", SYNTAX_STATUS_NORMAL),
+    ConstraintSyntaxNewContext("cancel", "Undefine the promiser class when this string expression of classes evaluates to true", SYNTAX_STATUS_NORMAL),
     ConstraintSyntaxNewRealList("dist", "Generate a probabilistic class distribution (from strategies in cfengine 2)", SYNTAX_STATUS_NORMAL),
     ConstraintSyntaxNewContext("expression", "Evaluate string expression of classes in normal form", SYNTAX_STATUS_NORMAL),
     ConstraintSyntaxNewContextList("or", "Combine class sources with inclusive OR", SYNTAX_STATUS_NORMAL),

libpromises/promises.c

@@ -704,10 +704,14 @@ Promise *ExpandDeRefPromise(EvalContext *ctx, const Promise *pp, bool *excluded)
     pcopy->conlist = SeqNew(10, ConstraintDestroy);
     pcopy->org_pp = pp->org_pp;
 
-    // if this is a class promise, check if it is already set, if so, skip
+    // if this is a class promise, check if it is already set, if so, skip.
+    // Exception: a promise using the 'cancel' attribute must be evaluated
+    // precisely when the class is already defined, since its purpose is to
+    // undefine it.
     if (strcmp("classes", PromiseGetPromiseType(pp)) == 0)
     {
-        if (IsDefinedClass(ctx, CanonifyName(pcopy->promiser)))
+        if (IsDefinedClass(ctx, CanonifyName(pcopy->promiser)) &&
+            PromiseGetConstraint(pp, "cancel") == NULL)
         {
             Log(LOG_LEVEL_DEBUG,
                 "Skipping evaluation of classes promise as class '%s' is already set",

libpromises/verify_classes.c

@@ -38,6 +38,8 @@
 
 
 static bool EvalClassExpression(EvalContext *ctx, Constraint *cp, const Promise *pp);
+static bool EvalClassConstraintRval(EvalContext *ctx, Constraint *cp, const Promise *pp);
+static PromiseResult VerifyClassCancel(EvalContext *ctx, const Promise *pp, Attributes *a);
 
 static bool ValidClassName(const char *str)
 {
@@ -73,6 +75,16 @@ PromiseResult VerifyClassPromise(EvalContext *ctx, const Promise *pp, ARG_UNUSED
         return PROMISE_RESULT_FAIL;
     }
 
+    /* The 'cancel' attribute undefines the promiser class when its expression
+     * evaluates true. It must be handled before the class-defining path
+     * because, by design, it operates on a class that is already defined and
+     * would otherwise be skipped (see EvalClassExpression()). */
+    if (a.context.expression != NULL &&
+        strcmp(a.context.expression->lval, "cancel") == 0)
+    {
+        return VerifyClassCancel(ctx, pp, &a);
+    }
+
     if (a.context.expression == NULL ||
         EvalClassExpression(ctx, a.context.expression, pp))
     {
@@ -295,32 +307,18 @@ static bool EvalBoolCombination(EvalContext *ctx, const Rlist *list,
     return result;
 }
 
-static bool EvalClassExpression(EvalContext *ctx, Constraint *cp, const Promise *pp)
+/*
+  Expand the RHS of a class-context constraint in place and evaluate it as a
+  boolean. Shared by the class-defining attributes (expression, not, and, or,
+  xor, select_class, dist) and by the class-cancelling 'cancel' attribute,
+  whose trigger is evaluated exactly like 'expression'. This deliberately does
+  NOT consider whether the promiser class is already defined; that skip logic
+  is specific to class definition and lives in EvalClassExpression().
+*/
+static bool EvalClassConstraintRval(EvalContext *ctx, Constraint *cp, const Promise *pp)
 {
-    assert(pp);
-
-    if (cp == NULL) // ProgrammingError ?  We'll crash RSN anyway ...
-    {
-        Log(LOG_LEVEL_ERR,
-            "EvalClassExpression internal diagnostic discovered an ill-formed condition");
-    }
-
-    if (!IsDefinedClass(ctx, pp->classes))
-    {
-        return false;
-    }
-
-    if (IsDefinedClass(ctx, pp->promiser))
-    {
-        if (PromiseGetConstraintAsInt(ctx, "persistence", pp) == 0)
-        {
-            Log(LOG_LEVEL_VERBOSE,
-                " ?> Cancelling cached persistent class %s",
-                pp->promiser);
-            EvalContextHeapPersistentRemove(pp->promiser);
-        }
-        return false;
-    }
+    assert(cp != NULL);
+    assert(pp != NULL);
 
     switch (cp->rval.type)
     {
@@ -353,7 +351,11 @@ static bool EvalClassExpression(EvalContext *ctx, Constraint *cp, const Promise
         break;
     }
 
-    if (strcmp(cp->lval, "expression") == 0)
+    /* The 'cancel' trigger is a plain class expression, evaluated like
+     * 'expression': true when the named class(es) resolve to a defined
+     * context. */
+    if (strcmp(cp->lval, "expression") == 0 ||
+        strcmp(cp->lval, "cancel") == 0)
     {
         return (cp->rval.type == RVAL_TYPE_SCALAR &&
                 IsDefinedClass(ctx, RvalScalarValue(cp->rval)));
@@ -401,3 +403,108 @@ static bool EvalClassExpression(EvalContext *ctx, Constraint *cp, const Promise
 
     return false;
 }
+
+static bool EvalClassExpression(EvalContext *ctx, Constraint *cp, const Promise *pp)
+{
+    assert(pp != NULL);
+    assert(cp != NULL);
+
+    /* assert() is compiled out under NDEBUG, so guard the pointer
+     * dereferences below with an explicit runtime check as well. */
+    if (pp == NULL || cp == NULL)
+    {
+        Log(LOG_LEVEL_ERR,
+            "EvalClassExpression internal diagnostic discovered an ill-formed condition");
+        return false;
+    }
+
+    if (!IsDefinedClass(ctx, pp->classes))
+    {
+        return false;
+    }
+
+    if (IsDefinedClass(ctx, pp->promiser))
+    {
+        if (PromiseGetConstraintAsInt(ctx, "persistence", pp) == 0)
+        {
+            Log(LOG_LEVEL_VERBOSE,
+                " ?> Cancelling cached persistent class %s",
+                pp->promiser);
+            EvalContextHeapPersistentRemove(pp->promiser);
+        }
+        return false;
+    }
+
+    return EvalClassConstraintRval(ctx, cp, pp);
+}
+
+/*
+  Handle a classes promise that uses the 'cancel' attribute. Unlike the
+  class-defining attributes, a cancel promise must be evaluated even when the
+  promiser class is already defined - that is the whole point - so it bypasses
+  the "already defined" skip in EvalClassExpression(). When the cancel
+  expression evaluates true and the promiser class is currently defined, the
+  class is undefined, mirroring how classes bodies cancel classes via
+  DeleteAllClasses().
+*/
+static PromiseResult VerifyClassCancel(EvalContext *ctx, const Promise *pp, Attributes *a)
+{
+    assert(ctx != NULL);
+    assert(pp != NULL);
+    assert(a != NULL);
+
+    Constraint *cp = a->context.expression;
+    assert(cp != NULL);
+
+    /* Respect the promise's class context guard. */
+    if (!IsDefinedClass(ctx, pp->classes))
+    {
+        return PROMISE_RESULT_NOOP;
+    }
+
+    if (!ValidClassName(pp->promiser))
+    {
+        cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_FAIL, pp, a,
+             "Attempted to cancel a class '%s', which is an illegal class identifier",
+             pp->promiser);
+        return PROMISE_RESULT_FAIL;
+    }
+
+    /* Hard classes describe the running system and must never be undefined
+     * (consistent with how the cancel_* class body attributes ignore them).
+     * Warn and leave the class in place rather than failing the promise. */
+    const Class *promiser_class = EvalContextClassGet(ctx, NULL, pp->promiser);
+    if (promiser_class != NULL && !promiser_class->is_soft)
+    {
+        Log(LOG_LEVEL_WARNING,
+            "Cannot cancel reserved hard class '%s'", pp->promiser);
+        return PROMISE_RESULT_NOOP;
+    }
+
+    /* Evaluate the cancel trigger. If it is false, leave the class as-is. */
+    if (!EvalClassConstraintRval(ctx, cp, pp))
+    {
+        return PROMISE_RESULT_NOOP;
+    }
+
+    /* Trigger is true: undefine the promiser class if it is defined. */
+    if (!IsDefinedClass(ctx, pp->promiser))
+    {
+        Log(LOG_LEVEL_VERBOSE,
+            "C:     -  Class '%s' targeted by cancel is not defined, nothing to do",
+            pp->promiser);
+        return PROMISE_RESULT_NOOP;
+    }
+
+    Log(LOG_LEVEL_VERBOSE, "C:     -  Cancelling class: %s", pp->promiser);
+
+    EvalContextHeapPersistentRemove(pp->promiser);
+    {
+        ClassRef ref = ClassRefParse(pp->promiser);
+        EvalContextClassRemove(ctx, ref.ns, ref.name);
+        ClassRefDestroy(ref);
+    }
+    EvalContextStackFrameRemoveSoft(ctx, pp->promiser);
+
+    return PROMISE_RESULT_NOOP;
+}

tests/acceptance/02_classes/01_basic/cancel_attribute.cf

@@ -0,0 +1,65 @@
+#######################################################
+#
+# Test the 'cancel' attribute of classes promises (CFE-4686).
+#
+# A classes promise using 'cancel' undefines its promiser class when the
+# cancel expression evaluates to true. Unlike the class-defining attributes,
+# it must be evaluated even when the class is already defined.
+#
+#######################################################
+
+body common control
+{
+      inputs => { "../../default.sub.cf" };
+      bundlesequence  => { default("$(this.promise_filename)") };
+      version => "1.0";
+}
+
+#######################################################
+
+bundle agent init
+{
+}
+
+#######################################################
+
+bundle agent test
+{
+  meta:
+      "description" -> { "CFE-4686" }
+        string => "classes promise 'cancel' attribute undefines a defined class when its expression is true";
+
+  classes:
+      # Define global classes so the check bundle can observe the outcome.
+      "to_cancel"   expression => "any", scope => "namespace";
+      "to_keep"     expression => "any", scope => "namespace";
+      "trigger"     expression => "any", scope => "namespace";
+      "func_cancel" expression => "any", scope => "namespace";
+
+      # trigger is set, so to_cancel must be undefined.
+      "to_cancel"   cancel => "trigger";
+
+      # Condition is an undefined class, so to_keep must be retained.
+      "to_keep"     cancel => "class_that_is_not_defined";
+
+      # Function-based boolean trigger: the file does not exist, so not(...)
+      # is true and func_cancel must be undefined.
+      "func_cancel" cancel => not(fileexists("/this/does/not/exist/cfe4686"));
+
+      # Cancelling a class that was never defined is a harmless no-op.
+      "never_defined" cancel => "trigger";
+}
+
+#######################################################
+
+bundle agent check
+{
+  classes:
+      "ok" expression => "!to_cancel.to_keep.!func_cancel.!never_defined.trigger";
+
+  reports:
+    ok::
+      "$(this.promise_filename) Pass";
+    !ok::
+      "$(this.promise_filename) FAIL";
+}

tests/acceptance/02_classes/01_basic/cancel_attribute_hardclass.cf

@@ -0,0 +1,48 @@
+#######################################################
+#
+# Test that the 'cancel' attribute cannot undefine a hard class (CFE-4686).
+#
+# Cancelling a reserved hard class must be refused (with a warning) and the
+# class left in place, consistent with the cancel_* classes body attributes.
+#
+#######################################################
+
+body common control
+{
+      inputs => { "../../default.sub.cf" };
+      bundlesequence  => { default("$(this.promise_filename)") };
+      version => "1.0";
+}
+
+#######################################################
+
+bundle agent init
+{
+}
+
+#######################################################
+
+bundle agent test
+{
+  meta:
+      "description" -> { "CFE-4686" }
+        string => "classes promise 'cancel' attribute must not undefine a hard class";
+
+  classes:
+      # cfengine is a reserved hard class - the cancel must be ignored.
+      "cfengine" cancel => "any";
+}
+
+#######################################################
+
+bundle agent check
+{
+  classes:
+      "ok" expression => "cfengine";
+
+  reports:
+    ok::
+      "$(this.promise_filename) Pass";
+    !ok::
+      "$(this.promise_filename) FAIL";
+}

tests/acceptance/02_classes/01_basic/cancel_attribute_mutex.cf

@@ -0,0 +1,43 @@
+#######################################################
+#
+# Test that 'cancel' is mutually exclusive with the class-defining
+# attributes of a classes promise (CFE-4686).
+#
+# Combining 'cancel' with expression/and/or/xor/not/dist/select_class must be
+# rejected with "Irreconcilable constraints", exactly like combining any two
+# of the defining attributes. The sub-policy exercises every combination; this
+# test asserts the rejection appears in the agent output.
+#
+#######################################################
+
+body common control
+{
+      inputs => { "../../default.sub.cf" };
+      bundlesequence  => { default("$(this.promise_filename)") };
+      version => "1.0";
+}
+
+#######################################################
+
+bundle agent init
+{
+}
+
+#######################################################
+
+bundle agent test
+{
+  meta:
+      "description" -> { "CFE-4686" }
+        string => "'cancel' must be mutually exclusive with the class-defining attributes";
+}
+
+#######################################################
+
+bundle agent check
+{
+  methods:
+      "" usebundle => dcs_passif_output1(".*Irreconcilable constraints in classes.*",
+                                         "$(sys.cf_agent) -Kf $(this.promise_filename).sub 2>&1",
+                                         $(this.promise_filename));
+}