|
The classifier already specifies classes in an order, and except for the partitioning we currently do between with-parameters classes and without-parameters classes, that order is preserved as the evaluation order, relative to each partition.
Backwards Compatibility Analysis
Consider the following.
|
Puppet Classes
|
class c1 ($parameter) {
|
notify { "example": message => $c2::variable; }
|
}
|
|
class c2 {
|
$variable = "value"
|
}
|
|
Classification
|
{ "classes": {
|
"c1": { "parameter": "value" },
|
"c2": {}
|
}}
|
If a customer today writes code such as the above and classifies a node with both of these classes, at present the evaluation order will be c2, c1 because classes specified without parameters are evaluated before all classes with parameters, regardless of the order given.
The biggest risk I can dream up (and this is complete fiction) is the following.
A user writes a class using the params pattern, setting class parameters to defaults inherited from the params class. Because the variables will not have values prior to the evaluation of the params class, this is a difficult pattern to get right, and the user may at some point stumble into the realization that if in the Enterprise Console, if they explicitly classify a node with the params class, all their default parameter values in the other class seem to be correct. The user then proceeds to use this pattern, always explicitly including the params class alongside their main class in their classifier, not really understanding why this makes it work, but rolling with it because it does.
|
Fictional Puppet Code
|
class util (
|
$required_parameter,
|
$message = $util::params::default_message,
|
) {
|
notify { $message: }
|
}
|
|
class util::params {
|
$default_message = "hello"
|
}
|
|
Classification
|
{ "classes": {
|
"util": { "required_parameter": "value" },
|
"util::params": {}
|
}}
|
Note that this would only work if they never set parameters on the params class and ALWAYS set parameters on the util class.
Opinion
The worst-case fictional scenario is only realized when people do Bad Things (TM) that shouldn't work in the first place, but are able to fiddle with permutations long enough to slot their use case into the coincidental undefined behavior.
Any time non-parameterized classes are being used to define variables, the best-practices patterns ALL account for appropriate below-classification reference and use of params classes by requiring that if a variable from an external class is going to be referenced, either that external class is include'd prior to referencing the variable, or at worst (questionable practice) a "private" parameterized class may make reference to variables from its intended declarer.
Today, because the behavior is not clearly defined, ENCs should not be relied upon for evaluation-order dependent classification. Our documentation goes into great detail about the data structure that should be returned by a node classifier, but makes no reference at all to ordering or evaluation order guarantees.
Fixing this bug will potentially cause behavior changes for two groups of people. People who currently work around the bug intentionally, and potentially people who may have stumbled onto the buggy behavior and now rely on it (more than likely unknowingly).
The problem of course is that fixing the bug is effectively a transition from unintentional, undefined behavior to intentional, defined behavior. I have to be realistic and recognize that this calls into question whether this is a bug fix or a feature request.
I would like it to be considered a bug and fixed in a .z release. I suspect though that that's a difficult classification to make and I might have to wait for at minimum a .y.
100% Compliant Ordering
Regardless of when it is deemed acceptable to introduce this fix, it is trivial to achieve 100% compliance with ordering if it is acceptable to just call evaluate_classes once per node class.
def evaluate_node_classes
|
if @node.classes.is_a? Hash
|
@node.classes.each do |name,params|
|
if params.empty?
|
evaluate_classes([name], @node_scope || topscope)
|
else
|
evaluate_classes({name => params}, @node_scope || topscope)
|
end
|
end
|
else
|
evaluate_classes(@node.classes, @node_scope || topscope)
|
end
|
end
|
|
