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

[nickanderson]

Summary

Adds a cancel attribute to classes promises so policy can undefine a
class. Until now a classes promise could only define a class; the only way
to undefine one from policy was a cancel_* classes body attached to another
promise's outcome. With cancel, the classes promise can undefine its own
promiser directly:

classes:
    # Undefine 'maintenance' once the window has passed
    "maintenance" cancel => "maintenance_window_over";

    # Undefine 'have_config' if the file no longer exists
    "have_config" cancel => not(fileexists("/etc/myapp.conf"));

When the cancel expression (a class expression — string or a function
returning a boolean) evaluates true, the promiser class is undefined if it is
currently defined. False trigger, or an already-undefined class, is a no-op.

Key detail: the class-skipping exception

ExpandDeRefPromise() normally excludes a classes promise whose promiser is
already defined (an optimization). A cancel promise must run precisely in
that case, so the skip now makes an exception when the cancel attribute is
present. This is the crux of the change — without it the promise would never
actuate.

Behaviour

• Mutually exclusive with the class-defining attributes (expression, and,
  or, xor, not, dist, select_class) via the existing "Irreconcilable
  constraints" check — cancel is counted like any other class-body attribute.
• Reserved hard classes cannot be cancelled (warned, left in place),
  consistent with the cancel_* classes body attributes (ENT-7718 / CFE-3647).
• The evaluate-and-undefine path mirrors the cancel_* classes body
  (DeleteAllClasses): persistent removal + EvalContextClassRemove +
  bundle-frame removal.

Tests

New acceptance tests in tests/acceptance/02_classes/01_basic/:
cancel_attribute.cf, cancel_attribute_hardclass.cf, cancel_attribute_mutex.cf(+.sub).
Full 02_classes suite: 117 passed, 0 failed (3 staging-skipped, 3
pre-existing soft failures).

Docs

Documentation PR: cfengine/documentation (CFE-4686) adds the cancel reference
section.

Ticket: https://northerntech.atlassian.net/browse/CFE-4686

🤖 Generated with Claude Code

[nickanderson]

@cf-bottom jenkins please

[cf-bottom]

Alright, I triggered a build:

Jenkins: https://ci.cfengine.com/job/pr-pipeline/13945/

Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-13945/

[basvandervlies]

Thanks nice addition!! Much needed

[olehermanse]

It looks a bit backwards that cancel has a class expression. To me it reads as if what you put as the value to the cancel attribute should be the class you want to cancel. With the proposed behavior I guess something like cancel_if would be a better name.

But why should it have a class expression? We already have conditions (class expressions) in many places: class guard, if, unless, and expression for classes promises. Why do we need one more?

Some other proposals;

bundle agent example
{
  classes:
    some_condition.linux::
      "my_class"
        # true / false, default to false:
        cancel => true;
}

bundle agent example
{
  classes:
    "my_class"
      # define / undefine, default to define:
      do => "undefine",
      if => "some_condition.linux";
}

[nickanderson]

It looks a bit backwards that cancel has a class expression. To me it reads as if what you put as the value to the cancel attribute should be the class you want to cancel. With the proposed behavior I guess something like cancel_if would be a better name.

On the naming, yeah right after I pushed it I was thinking to mysefl that maintenance cancel blah read quite poorly. I was thinking a bit about cancel_when.

classes:
    "maintenance" expression  => "maintenance_window_active";
    "maintenance" cancel_when => "maintenance_window_over";

    "have_config" cancel_when => not(fileexists("/etc/myapp.conf"));

That leaves if/unless still available for further compound constraint.

or maybe invalidated_by ? we already have if/ifvarclass so cancel_if just seemed a bit yeuck.

@basvandervlies @ncharles opinions on language here?

But why should it have a class expression? We already have conditions (class expressions) in many places: class guard, if, unless, and expression for classes promises. Why do we need one more?

Most class-defining attributes accepts full class expressions:

bundle agent main
{
      classes:
        # expression -- a single class expression string
        "via_expression"
          expression => "linux.!windows";

        # and -- list of class expressions, ALL must be true
        "via_and"
          and => { "linux.!windows", "any.!solaris" };

        # or -- list of class expressions, ANY must be true
        "via_or"
          or => { "windows.x86_64", "linux.!windows" };

        # not -- a class expression, NEGATED
        "via_not"
          not => "windows.!linux";

        # xor -- list of class expressions, exactly ONE must be true
        "via_xor"
          xor => { "linux.!windows", "windows.!linux" };

        # dist -- probabilistic weights (not a class expression)
        "dist_a"
          dist => { "100", "0", "0" };

        # select_class -- deterministic host selection (not a class expression)
        "sel_a"
          select_class => { "sel_a", "sel_b", "sel_c" };

      reports:
        "expression => linux.!windows : $(with)"
          with => ifelse("via_expression", "DEFINED", "NOT DEFINED");

        "and => { linux.!windows, any.!solaris } : $(with)"
          with => ifelse("via_and", "DEFINED", "NOT DEFINED");

        "or => { windows.x86_64, linux.!windows } : $(with)"
          with => ifelse("via_or", "DEFINED", "NOT DEFINED");

        "not => windows.!linux : $(with)"
          with => ifelse("via_not", "DEFINED", "NOT DEFINED");

        "xor => { linux.!windows, windows.!linux } : $(with)"
          with => ifelse("via_xor", "DEFINED", "NOT DEFINED");

        "dist => { 100, 0, 0 } : $(with)"
          with => ifelse("dist_a", "DEFINED", "NOT DEFINED");

        "select_class => { sel_a, sel_b, sel_c } : $(with)"
          with => ifelse("sel_a", "DEFINED", "NOT DEFINED");
}

R: expression => linux.!windows : DEFINED
R: and => { linux.!windows, any.!solaris } : DEFINED
R: or => { windows.x86_64, linux.!windows } : DEFINED
R: not => windows.!linux : DEFINED
R: xor => { linux.!windows, windows.!linux } : DEFINED
R: dist => { 100, 0, 0 } : DEFINED
R: select_class => { sel_a, sel_b, sel_c } : DEFINED

R: expression => linux.!windows : DEFINED
R: and => { linux.!windows, any.!solaris } : DEFINED
R: or => { windows.x86_64, linux.!windows } : DEFINED
R: not => windows.!linux : DEFINED
R: xor => { linux.!windows, windows.!linux } : DEFINED
R: dist => { 100, 0, 0 } : DEFINED
R: select_class => { sel_a, sel_b, sel_c } : DEFINED

expression, and, or, not, and xor all accept full class expressions (compound boolean strings with . / | / !), not just bare class names. dist and select_class are the exceptions but I think they are very different from canceling as well.

[nickanderson]

Another idea, expire_when

[nickanderson]

expire is growing on me (I don't need the _when suffix, but im ok with it or even _if I guess.)

[basvandervlies]

I like what @olehermanse suggested as syntax:

  classes:
    "my_class"
      # define / undefine, default to define:
      do => "undefine",
      if => "some_condition.linux";

Only I suggest action here.

If we got for the class <attribute> => <statement>. The unset or undefine are the attribute types.

[nickanderson]

I like what @olehermanse suggested as syntax:

  classes:
    "my_class"
      # define / undefine, default to define:
      do => "undefine",
      if => "some_condition.linux";

Only I suggest action here.

If we got for the class <attribute> => <statement>. The unset or undefine are the attribute types.

An attribute that specifies define/undefined would change how all classes promises are processed instead of adding a new canceling expression.

    classes:

        "my_dist"
          WORD => "undefine",
          dist => { "10", "20", "40", "50" };

        "selection"
          WORD => "undefine",
          select_class => { "one", "two" };

What would be un-defined?. What about the distribution and selected class? Yes, the attribute could be made mutually exclusive. The attributes that are currently involved in determining if the class should be defined (and, dist, expression, or, not, select_class, and xorg are already mutually exclusive. I don't know if that is that is cleaner. it feels like more to remember.

Persistent classes were on my mind when I suggested expire. It doesn't fit as well with non-persistent classes. So I am back to preferring undefine => "<expression>" or cancel => "<expression>" Of these I prefer to use undefine, but the pre-existing stuff uses cancel.

[olehermanse]

@nickanderson the exact same problems exist with your suggestion. We should make all those other attributes mutually exclusive with the new cancel attribute regardless.

  classes:
    "my_dist"
      cancel => "Hr01|Hr02.linux",
      dist => { "10", "20", "40", "50" };

It's not at all clear what should happen here or how dist and cancel should interact. We should block the attribute combinations that don't make sense, so it is not allowed to write cancel promises which are overly complicated, ambiguous, and hard to read / understand.

expire should refer to time, so not appropriate here. Since we try to consistently say "define a class", undefine seems appropriate. (Though cancel also works). I don't see a good reason to allow the complexity of a class expression. (And I see many good reasons to keep policy language simple to read and evaluate). A simple do => undefine or cancel => true achieves what we need and is easier for the reader.

[nickanderson]

@nickanderson the exact same problems exist with your suggestion. We should make all those other attributes mutually exclusive with the new cancel attribute regardless.

  classes:
    "my_dist"
      cancel => "Hr01|Hr02.linux",
      dist => { "10", "20", "40", "50" };

ugh, true.

A simple do => undefine or cancel => true achieves what we need and is easier for the reader.

I still don't like it and I still think it's wrong. We use expressions all over the place, it's weird to not take an expression.

 classes:
   "my_dist"  undefine => "Hr01|Hr02.linux";

[nickanderson]

Updated the branch (force-pushed, rebased onto current master):

• cancel is now a boolean (cancel => "true", cfbool like create) instead of a class expression. The trigger is the promise's own class-context guard; false/no is a no-op. The EvalClassExpression split was reverted, so that function is unchanged vs master.
• cancel is now also mutually exclusive with persistence, scope, and timer_policy (they only tune how a class is defined), in addition to the class-defining attributes.
• Added an acceptance test asserting the persistent-class record is removed from the cf_state LMDB on cancel.

Docs PR cfengine/documentation#3683 updated to match.

[nickanderson]

@cf-bottom jenkins, please.

[nickanderson]

@cf-bottom jenkins please

[cf-bottom]

Alright, I triggered a build:

Jenkins: https://ci.cfengine.com/job/pr-pipeline/13955/

Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-13955/

addressed and implemented your bad idea