Project

General

Profile

Actions

Bug #7459

closed

Architecture #7155: Stable class identifier

Architecture #7156: Modify generic methods to define and use the new class_prefix

Reporting is broken for several ncf generic methods because the new class_prefix is not computed without this.callers_promisers, and any conditions using class_prefix are incorrectly evaluated

Added by Jonathan CLARKE about 9 years ago. Updated about 3 years ago.

Status:
Released
Priority:
N/A
Category:
-
Target version:
-
Severity:
UX impact:
User visibility:
Effort required:
Priority:
0
Name check:
To do
Fix check:
To do
Regression:

Description

For example, in command_execution.cf:

  vars:
      "old_class_prefix"  string => canonify("command_execution_${command_name}");
      "class_prefix"      string => canonify(join("_", "this.callers_promisers"));
      "args"               slist => { "${command_name}" };

  methods:
      "report"                                                                                                                                                                                                                        
        usebundle  => _log("Execute the command ${command_name}", "${old_class_prefix}", "${class_prefix}", @{args}),
        ifvarclass => "(!has_promiser_stack.${old_class_prefix}_reached)|(has_promiser_stack.${class_prefix}_reached)";

In this case, the last ifvarclass will evaluate to FALSE because ${class_prefix} is an undefined variable, because "this.callers_promisers" does not exist. This obvisouly breaks reporting, since "_log" is never called.


Related issues 1 (0 open1 closed)

Related to Rudder - Bug #7461: classes_* methods should refuse to copy classes to a prefix which is an empty stringReleasedBenoît PECCATTEActions
Actions #1

Updated by Jonathan CLARKE about 9 years ago

The best solution we have found for this is to slightly change the boiler plate lines at the top of each generic method as follows:

  vars:
      "old_class_prefix"  string => canonify("command_execution_${command_name}");
      "promisers"          slist => { @{this.callers_promisers}, cf_null }, policy => "ifdefined";
      "class_prefix"      string => canonify(join("_", "promisers"));
      "args"               slist => { "${command_name}" };

The change is the second line (whose result is used in the 3rd line, instead of using "this.callers_promisers" directly). This line uses several CFEngine tricks:

  • It uses 'policy => "ifdefined"' to include the values of @{this.callers_promisers} if and only if that list exists.
  • It uses the special value "cf_null" to avoid this new list from being empty, in case @{this.callers_promisers} does not exist
  • "promisers" becomes an empty list (when "this.callers_promisers", as currently it does not)
Actions #2

Updated by Jonathan CLARKE about 9 years ago

  • Status changed from New to In progress
  • Assignee set to Jonathan CLARKE
Actions #3

Updated by Jonathan CLARKE about 9 years ago

  • Status changed from In progress to Pending technical review
  • Assignee changed from Jonathan CLARKE to Benoît PECCATTE
  • Pull Request set to https://github.com/Normation/ncf/pull/271
Actions #4

Updated by Jonathan CLARKE about 9 years ago

  • Related to Bug #7461: classes_* methods should refuse to copy classes to a prefix which is an empty string added
Actions #5

Updated by Jonathan CLARKE about 9 years ago

  • Status changed from Pending technical review to Pending release
  • % Done changed from 0 to 100
Actions #7

Updated by Benoît PECCATTE about 9 years ago

  • Subject changed from Reporting is broken for several ncf generic methods because the new class_prefix is not computer without this.callers_promisers, and any conditions using class_prefix are incorrectly evaluated to Reporting is broken for several ncf generic methods because the new class_prefix is not computed without this.callers_promisers, and any conditions using class_prefix are incorrectly evaluated
Actions #8

Updated by Vincent MEMBRÉ over 8 years ago

  • Status changed from Pending release to Released
Actions

Also available in: Atom PDF