[BUG] Rule Module not correct?

[elwood218]

Hi, we have tried the rule module created by @diademiemi . First of all thanks for that work! But we have tested and debugged it and we aren't sure if that is working correctly. Maybe we are missing a point.

If you get for example r["extensions"]["conditions"] it only got the keys but not the values for comparing.
Also if the value_raw is sorted it not really sorts the keys, instead it sorts every character. Maybe the last is working anyway kinda but the other are not working.

We have debugged it with adjusting the following function (reason for debug was to find out if the API output was changed):

def get_existing_rule(module, base_url, headers, ruleset, rule):
    # Get rules in ruleset
    rules = get_rules_in_ruleset(module, base_url, headers, ruleset)
    a_list = []
    b_list = []
    if rules is not None:
        # Loop through all rules
        for r in rules.get("value"):
    #        rule_exist = sorted(r)
    #        rule_defined = sorted(rule["properties"])
    #        exit_failed(module, "Rule_exist: %s, Rule_defined: %s , " % (rule_exist, rule_defined),)
            foo = sorted(r["extensions"]["conditions"])
            bar = sorted(rule["conditions"])
            a_list.append(foo)
            b_list.append(bar)
            # Check if conditions, properties and values are the same
            if (
                sorted(r["extensions"]["conditions"]) == sorted(rule["conditions"])
    #            and sorted(r["extensions"]["properties"]) == sorted(rule["properties"])
                and sorted(r["extensions"]["value_raw"]) == sorted(rule["value_raw"])
            ):
                # If they are the same, return the ID
                return r["id"]
        exit_failed(module, "Rule_exist: %s, Rule_defined: %s , " % (a_list, b_list),)
    return None

PS: I also wonder why the json output was choosed with ().. This is not really the standard way like other tools how jq could read it by default.

[diademiemi]

What version of the Checkmk server are you using?

[elwood218]

2.1.0p11

[diademiemi]

The API output has changed in 2.1.0p10. What exactly does the rule module not do? Is the idem potency not correct?

[elwood218]

As I already wrote that only the keys are compared and not the values.

[diademiemi]

I'm busy for a few days but I'll have a look at this in the weekend and resolve it

[elwood218]

@diademiemi @robin-tribe29 I also saw that the reason for API changes was this module. Why () brackets are used in the output instead of official [] brackets ?
In Ansible when using from_yaml was able to read directly the [] but now it is not working anymore. The same is for tools like jq or any other.

[diademiemi]

This is because this is how they are stored and what the API expects as input. There was an inconsistency with the API requiring Python as input and the web UI exporting Python, but the API exporting JSON.

[elwood218]

Maybe I miss something but why it was not changed other way around? The API expects a JSON and a JSON would be [].

[diademiemi]

This is how the API expected it as input. Only the output from exporting it was changed in these changes for parity with the web UI.

[elwood218]

I have just put a rule via API with a JSON with correct [] brackets.. So I still don't know what you mean with "expected input"

[diademiemi]

If you go into the UI and look at the rule you will see that the rule is not actually in effect since it will not correctly parse the value

[elwood218]

All values are set correct in my case. And if that would be the case there is still the option to adjust the API that it is reading JSON correctly. Because [] brackets are the correct ones which can be read and used by other tools.

[diademiemi]

There was a discussion about this in the PR #79.

This was the solution chosen by the API developers. In any case this isn't related to the Ansible Collection.

[elwood218]

Yeah and I bet checkmk isn't caring anyway..

[lgetwan]

Happy new year, everybody!

Today, I talked to a REST API developer, and he told me that properly using JSON for the value_raw would need a major change in the complete setup part of the CMK backend. It's not planned to do that in the next few releases.

Looks like you have to keep on using some extra manual steps to deal with this parameter.

[robin-checkmk]

As this is an upstream issue, and we put some work into the rule module in version 0.16.0 of the collection, I will close this issue.
If something comes up with the most recent collection version, please open a new issue. Thanks!

[kain88-de]

Maybe a quick note as a developer of checkmk. We do care about using JSON in the API. That we do not have pure JSON in the body of API requests is also annoying for the developers of checkmk. There are a few historical reasons why it is not straight forward to make the raw_value field a json string.

Workaround

The string in raw_value is pure python. You can convert it to a python object using ast.literal_eval, like the current api code does. Converting it back is done using repr, the linked code example masks secrets.

Why can we not just use JSON?

There are two classes that deal with rules ValueSpecs and RuleSpec.

Rulespec is just a thin wrapper around a python object, stored in the variable value. The Rulespec is used to determine if a rule applies to a given service/check plugin. The check plugin later only gets the value. We have a large number of plugins that behave differently based on the type of value, example in the gcp utils. The check plugins always worked like this. Since pretty much all mkps rely on this behavior we cannot change it.

ValueSpecs are kind of a schema definition for RuleSpecs, kind of because they also do a lot more. The API only uses them for validation. Because we cannot change the type of RuleSpecs without breaking almost all MKPs the API would need to convert JSON-arrays to a python tuple in a parsing step. This could be added. In fact we do have buggy support for parsing JSON. For example here is a bug in the Alternative ValueSpec. Here is an example using Alternative to define different option to configure levels. In that example the length of the tuple decides if you want to apply fixed levels or predictive levels. This is not ideal and also a pain for us developers. Unfortunately a lot of legacy code and MKPs depend on this behavior.

In short touching this is difficult because a lot of legacy code depends on the current behavior and making changes to the python API of RuleSpec of ValueSpec is risky because it might break a lot of existing code from customers.

We really would like to use JSON ourselves. But it is not straight forward to find a solution that works with the constraints we have due to RuleSpec and ValueSpec.