Project

General

Profile

Actions

Bug #2419

closed

The reporting can't handle component key value with CFEngine variables within

Added by Nicolas CHARLES almost 12 years ago. Updated almost 12 years ago.

Status:
Released
Priority:
1
Category:
Web - Compliance & node report
Target version:
Severity:
UX impact:
User visibility:
Effort required:
Priority:
Name check:
Fix check:
Regression:

Description

If we set in a PI/Directive a Component Key Value with a CFEngine variable, the reporting get confused.
For instance, with the Technique/PT CopyFromSharedFolder, if we set the destination to :

/var/$(genericVariableName.somename)/foo

and $(genericVariableName.somename) is set to "bar", then the agent will report the value /var/bar/foo, but the expected value is still /var/$(genericVariableName.somename)/foo

Note that the example uses the generic variable definition, but it could be a CFEngine system variable (like ${sys.xxx}, ${mon.smth}, ${this.smth}, or even something else)
Note-bis : a CFEngine variable may use ${} or $()

Actions #1

Updated by Nicolas CHARLES almost 12 years ago

Maybe we could, as a workaround for 2.3, consider ${} and $() as a wildcard for .* in the reporting
It is not excellent, but not that bad because :
  1. usually the cfengine variable has some text around
  2. we still have the CR/PI/Serial/Component
  3. we still check the cardinality (modulo the bug #2398)
Actions #2

Updated by Nicolas CHARLES almost 12 years ago

For a more robust solution, we'd need to generate a "key" based on the component key value, and put it in the generated promises, in the reporting and the expected reports.
This key should be 'deterministic' : saving the PI/Directive should not change the generated key if no change were made. Maybe a hash of the "component key value + index + some text" ?

Changing the reports format will be a breaking change (or maybe we could add it at the end, in the human readable message part to keep compatibility, and with a new separator)
As a side effect, it would solve parts of #2398

Actions #3

Updated by Nicolas CHARLES almost 12 years ago

  • Status changed from New to Discussion
  • Assignee changed from Nicolas CHARLES to Jonathan CLARKE

Nicolas CHARLES wrote:

For a more robust solution, we'd need to generate a "key" based on the component key value, and put it in the generated promises, in the reporting and the expected reports.
This key should be 'deterministic' : saving the PI/Directive should not change the generated key if no change were made. Maybe a hash of the "component key value + index + some text" ?

Changing the reports format will be a breaking change (or maybe we could add it at the end, in the human readable message part to keep compatibility, and with a new separator)
As a side effect, it would solve parts of #2398

The component key value + index + some text part is too complex; we already have the index, so we could simply use it (and it would prevent overlong syslog lines), and have a default value of -1 for the Techniques not using it, for compliance with older techniques. The drawback of simply using the index is that the data on the database won't be really differents (much 0, a bit less 1, 2, etc), but that's not a right reason to rule it out

We'd have reports like (the #@ is tentative, any other suggestions welcomed)

@@Policy@@Type@@RuleId@@DirectiveId@@VersionId@@Component@@Key@@ExecutionTimeStamp##NodeId@#KeyIndex#@HumanReadableMessage

In the expectedReports, we would only have to store the highest index expected (and we'd know that we would expect all those between 0 and the index), and set the max index to -1 for the historical data

And we would keep the two reporting, at least during the 2.4 branch:
  • previous reporting, we don't have a KeyIndex, so we set -1, thus we know we'll need the previous reporting system to handle it
  • new reporting, we have a keyIndex, and we chack not by the Key, but by KeyIndex

What do you think of it ?

Actions #4

Updated by Jonathan CLARKE almost 12 years ago

  • Status changed from Discussion to 2
  • Assignee changed from Jonathan CLARKE to Nicolas CHARLES
  • Estimated time set to 5.00 h

Nicolas CHARLES wrote:

Maybe we could, as a workaround for 2.3, consider ${} and $() as a wildcard for .* in the reporting
It is not excellent, but not that bad because :
  1. usually the cfengine variable has some text around
  2. we still have the CR/PI/Serial/Component
  3. we still check the cardinality (modulo the bug #2398)

I agree, let's go with this approach, one #2398 has been fixed. This will be pretty robust, so long as the PT is not broken (but then you're likely to get bad reporting anyway...). We should implement this on 2.3 and 2.4 ASAP, and document the shortcoming as a known problem.

The shortcoming of this approach is as follows. Given a PT that has a multi-valued section using one of the variables in the section as a component key for reporting, if this variable is filled in using a CFEngine variable (ie, ${sys.host} or $(generic_variable_definition.myvar)), then the reports generated will contain the values of these variables on each node, whereas the Rudder server will not compare the value to establish if the reporting is correct. This means that if the CFEngine variables values differ from what is expected, Rudder will still report Success, so long as the right number of variables per PI and component are returned. If a PT was buggy, and send back two success reports instead of one for the first value, then zero instead of one for the second value, this would lead to a false positive in reporting. However, this is a bug in the PT and is highly unlikely because the PT would most likely send a consistent number or reports, and this would be detected during testing with just one value.

Nicolas will work on this once he's finished #2398, so a fix can be expected on Thursday afternoon.

I think any longer term fixes should not be rushed - this deserves some thought. I fear it is too close to the release of 2.4 to include anything more complicated that the fix above.

Actions #5

Updated by Nicolas CHARLES almost 12 years ago

  • Status changed from 2 to Pending technical review
  • % Done changed from 0 to 100

Now when we are expecting a CFEngine variable, the CFEngine variable is replaced by .* at check time and the regexp is matched

Actions #6

Updated by François ARMAND almost 12 years ago

  • Status changed from Pending technical review to Released

That seems to be ok for me, but I believe that we are reaching the limit of our reporting system, and should start to thing to a more structured (pr easier to structure) dataformat - but that would mean stopping using rsyslog)

Actions

Also available in: Atom PDF