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 directly. Until now a classes promise could only define a class;
undefining one required a cancel_* classes body attached to another promise's
outcome.

classes:
  trigger::
    # Undefine 'maintenance' when the 'trigger' guard holds
    "maintenance" cancel => "true";

cancel is a boolean (cfbool). When it is true and the promise's
class-context guard is satisfied, the promiser class is undefined. cancel => "false", an unmet guard, or an already-undefined class are all no-ops.

Key detail: the class-skipping exception

ExpandDeRefPromise() normally skips 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 cancel is present. This is
the crux of the change — without it the promise would never actuate.

Behaviour

• The trigger is the promise's own class guard, not a cancel expression.
• Mutually exclusive with the class-defining attributes (expression, and,
  or, xor, not, dist, select_class) via the existing "Irreconcilable
  constraints" check, and rejects the class-modifier attributes (persistence,
  scope, timer_policy), which are meaningless alongside it.
• Reserved hard classes cannot be cancelled (warned, left in place),
  consistent with the cancel_* body attributes (ENT-7718 / CFE-3647).
• Undefining mirrors the cancel_* path: persistent-record removal from the
  cf_state LMDB + EvalContextClassRemove + bundle-frame removal.

Tests

New acceptance tests in tests/acceptance/02_classes/01_basic/:

• cancel_attribute.cf — core semantics (cfbool true/aliases/false, guard,
  no-op on an undefined class).
• cancel_attribute_hardclass.cf — refuses to cancel a reserved hard class.
• cancel_attribute_mutex.cf (+.sub) — rejects the class-defining attributes.
• cancel_attribute_modifier_mutex.cf (+.sub) — rejects persistence /
  scope / timer_policy.
• cancel_attribute_persistent_lmdb.cf (+.sub/.sub2) — verifies the
  persistent record is removed from the cf_state LMDB, checked at the source
  with cf-check dump.

All CI checks green.

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

[larsewi]

When the cancel expression (a class expression — string or a function
returning a boolean) evaluates true...

Please update PR description. It seems to only support boolean.

[nickanderson]

When the cancel expression (a class expression — string or a function
returning a boolean) evaluates true...

Please update PR description. It seems to only support boolean.

Will do, yes it got stale, unfortunate byproduct of herman.