[BUG] value_raw set to a variable

[clagio]

Describe the bug I'm trying to set the value of value_raw to a variable instead of hardcoding it in the task, so I can reuse the same task for multiple rules. Unfortunately I get the error "Unsupported type. Field must be string". The same task with the value_raw explicitly set works fine. Seems to be an escaping issue, and I'm not 100% the issue is with the collection honestly.. but I'm banging my head on this so any help is appreciated.

Component Name tribe29.checkmk.rule

Ansible Version

$ ansible --version
ansible 2.10.8
python version = 3.9.2 (default, Feb 28 2021, 17:03:44) [GCC 10.2.1 20210110]

Checkmk Version

Checkmk Raw Edition 2.1.0p13

Collection Version

$ ansible-galaxy collection list
tribe29.checkmk   0.11.0

To Reproduce Steps to reproduce the behavior: Execute this task:

    - name: "Create a rule."
      tribe29.checkmk.rule:
        server_url: "{{ hostvars['checkmk'].checkmk_url }}"
        site: "{{checkmk_site}}"
        automation_user: "{{ hostvars['checkmk'].checkmk_user }}"
        automation_secret: "{{ hostvars['checkmk'].checkmk_secret }}"
        ruleset: "checkgroup_parameters:memory_percentage_used"
        rule:
            properties: {
                "comment": "Warning at 80%\nCritical at 90%\n",
                "description": "Allow higher memory usage",
                "disabled": false
            }
            value_raw: "{{ myvariable }}"
        state: "present"
      vars:
        - myvariable: "{'levels': (80.0, 90.0)}"

If I replace value_raw: "{{ myvariable }}" to value_raw: "{'levels': (80.0, 90.0)}" the task works fine.

Expected behavior Rule to be created using the value in "myvariable"

Actual behavior The following error is printed:

"msg": "Error calling API. HTTP code 400. Details: b'{\"title\": \"Bad Request\", \"status\": 400, \"detail\": \"These fields have problems: value_raw\", \"fields\": {\"value_raw\": [\"Unsupported type. Field must be string.\"]}}', "

Additional context I suspect the issue is caused by the curly bracket required by the API which make ansible interpret the variable's value as a dictionary instead of a string. But I couldn't find a way to escape that and make it works with the API.

[muehlings]

Hello clagio,

can you please try to write the rule as dict(key:list)? vars: - myvariable: {'levels': [80,90]}

Tuples and ansible are kind of impossible and I suspect that the API call will be JSON-encoded anyways. JSON doesn't know tuples.

The debug output in ansible is the output of json.dumps() and thus the original type of the object isn't preserved. At the moment I have no 2.1 instance available for testing and 2.0-API does not provide the rules.

[clagio]

Hi, I tried removing the double quotes s you asked, but I get the same error "Unsupported type. Field must be string" but in this case I see from the debug info that all the output is mixed up. there is a way to see the raw output in ansible before any transformation?

I get the same error also with a different rule (ruleset special_agents:aws) if I try to move the access_key_id in a variable`. As before, if I hardcode the value, it works fine:

value_raw: "{
              'access_key_id': '{{ hostvars['aws-rds'].aws_access_key }}',
              'assume_role': {},
              'global_services': {},
              'regions': ['eu-west-1'],
              'secret_access_key': ('password', 'xxxxxxxxxxxxxxxxxxxx'), 
              'services': {
                'rds': {'limits': True, 'selection': 'all'}
              }"

[muehlings]

Have you removed the "()" and replaced them with "[]", too? Ansible doesn't know the tuple-notation (tup1,tup2) of python. The line in YAML is then interpreted as string and mixed up.

Edit: Does not work. See next comment for solution.

[muehlings]

I set up a 2.1 instance, modified the source and achieved this:

  - name: "Create a rule."
    tribe29.checkmk.rule:
      server_url: "http://127.0.0.1/"
      site: "test"
      automation_user: "automation"
      automation_secret: "$SECRET"
      ruleset: "checkgroup_parameters:memory_percentage_used"
      rule:
          properties: {
              "comment": "Warning at 81%\nCritical at 91%\n",
              "description": "Allow higher memory usage",
              "disabled": false
          }
          value_raw: "{{ myvariable }}" #"{'levels': (80.0, 90.0)}"  #"{{ myvariable }}"
      state: "present"
    vars:
      - myvariable: {'levels': [81,91]}

=>

changed: [debian] => {
    "changed": true,
    "invocation": {
        "module_args": {
            "automation_secret": "$SECRET",
            "automation_user": "automation",
            "rule": {
                "conditions": {
                    "host_labels": [],
                    "host_tags": [],
                    "service_labels": []
                },
                "folder": "/",
                "properties": {
                    "comment": "Warning at 81%\nCritical at 91%\n",
                    "description": "Allow higher memory usage",
                    "disabled": false
                },
                "value_raw": {
                    "levels": [
                        81,
                        91
                    ]
                }
            },
            "ruleset": "checkgroup_parameters:memory_percentage_used",
            "server_url": "http://127.0.0.1/",
            "site": "test",
            "state": "present",
            "validate_certs": true
        }
    },
    "msg": "Created rule"

Source modification:

def get_existing_rule(module, base_url, headers, ruleset, rule):
    # Get rules in ruleset
    rules = get_rules_in_ruleset(module, base_url, headers, ruleset)
    if rules is not None:
        # Loop through all rules
        for r in rules.get("value"):
            # Check if conditions, properties and values are the same
            if type(rule["value_raw"]) == str:
                pass
            else:
                # Input is YAML variable. Need to convert to string...
                rule["value_raw"] = str(rule["value_raw"]).replace('[','(').replace(']',')')
            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"]
    return None

and

def create_rule(module, base_url, headers, ruleset, rule):
    api_endpoint = "/domain-types/rule/collections/all"

    params = {
        "ruleset": ruleset,
        "folder": rule["folder"],
        "properties": rule["properties"],
        "conditions": rule["conditions"],
    }

    if type(rule["value_raw"]) == str:
        params["value_raw"] = rule["value_raw"]
    else:
        # Input is YAML variable. Need to convert to string...
        params["value_raw"] = str(rule["value_raw"]).replace('[','(').replace(']',')')

    url = base_url + api_endpoint

    response, info = fetch_url(
        module, url, module.jsonify(params), headers=headers, method="POST"
    )

    if info["status"] != 200:
        exit_failed(
            module,
            "Error calling API. HTTP code %d. Details: %s, %s"
            % (info["status"], info["body"], module.jsonify(params)),
        )

Now I have two options:

1. Write value_raw directly as string => value_raw: "{'levels': (80.0, 90.0)}"
2. Define a YAML(!) variable and reference it in my playbook => value_raw: "{{ myvariable }}" with

    vars:
      - myvariable: {'levels': [81,91]}

My modifications construct the proper string for the API including replacement of the square brackets (=python/YAML list) with the round brackets (=python tuple).

[clagio]

thanks, I modified the source as you did, and indeed now the first task( myvariable: {'levels': [81,91]}) works, but I get an error with the following :

value_raw: "{
              'access_key_id': '{{ hostvars['aws-rds'].aws_access_key }}',
              'assume_role': {},
              'global_services': {},
              'regions': ['eu-west-1'],
              'secret_access_key': ('password', 'xxxxxxxxxxxxxxxxxxxx'), 
              'services': {
                'rds': {'limits': True, 'selection': 'all'}
              }"

I will have a look later at the sources to see if I can find why

[clagio]

I had a better look, your change replace all squared brackets with rounded brackets, which means this: 'regions': ['eu-west-1'], becomes 'regions': ('eu-west-1'), and it fails. It's tricky because depending on the rule we may need a replace or not..

I have also an issue where it keep creating the rule even if already exists, but I haven't narrowed down the problem yet. Would make sense to use the description field to detect if the rule already exists instead of matching conditions/properties/value_raw?

[muehlings]

Yes, you are right.

The only clean way to get this resolved in my opinion is that the API of Checkmk should accept and deliver JSON and not a string which is "evaluated" into a python object. There was some discussion about that in https://github.com/tribe29/ansible-collection-tribe29.checkmk/issues/153 I don't know the reason for the decision to not modifying the API.

The second possibility will be to get rid of all tuples inside the rules. But this is a lot more work to do I think.

I have problems to update the rule without my modifications, too. If only a single character or value differs in "conditions", "properties" or "value_raw" a new rule is created.

[clagio]

I agree the cleanest way is to fix it at the API level.. I think the different parameters in the value_raw are handled differently by the API, for example: 'regions': ['eu-west-1', 'eu-west-2'], need to stay with the squared brackets (will fails with the rounded brackets ) 'secret_access_key': ['password', 'xxxxxxxxxxxxxxxxxxxx'], need to be with rounded brackets or will fail. But from my understanding there is no difference between the two. I can only suppose secret_access_key is handled different in the API.

Anyway I did some changes and now I have a rule.py that works in all my cases. It's not super clean, but I'm not sure there is a better way without modifying the API. I also changed so that the rule matching (if already exists) is done only comparing the description.

There are several problem trying to compare conditions, properties and value_raw. Value raw has all the problems with escaping chars, and since there are different possible parameters I suspect there is an high chance of mismatch. Conditions seems to be problematic as well.. below the conditions returned by an existing rule and the one passed by the new rule. This mismatch doesn't happen for all the rules, not sure why, maybe depends on the ruleset type?

existing_rule - compare11:['host_labels', 'host_tags', 'service_labels']
new_rule - compare12:['host_labels']

It's not impossible to match against all the properties, but I think requires a lot of work to make it work for all the possible cases. I believe matching by description it's simple and effective.

I created a new function transform_value_raw, where I take the value_raw and change the brackets only for secret_access_key. Eventually new keys can be added if needed,

def transform_value_raw(value_raw): 
    if type(value_raw) == dict:
        concat_value_raw= "{"
        for key, value in value_raw.items():
            if key == "secret_access_key":
                concat_value_raw = concat_value_raw + "'" +str(key) + "': " +str(value).replace('[','(').replace(']',')')+ ", "
            else:
                if str(value).startswith('[') or str(value).startswith('{'):
                    concat_value_raw = concat_value_raw + "'" +str(key) + "': " +str(value)+ ", "
                else:
                    concat_value_raw = concat_value_raw + "'" +str(key) + "': '" +str(value)+ "', "
        concat_value_raw= concat_value_raw + "}"
        return concat_value_raw
    return value_raw

Modified the create_rule:

def create_rule(module, base_url, headers, ruleset, rule):
    api_endpoint = "/domain-types/rule/collections/all"
    
    value_raw = transform_value_raw(rule["value_raw"])

    params = {
        "ruleset": ruleset,
        "folder": rule["folder"],
        "properties": rule["properties"],
        "conditions": rule["conditions"],
        "value_raw": value_raw
    }
    params_json = json.dumps(params)

    url = base_url + api_endpoint

    response, info = fetch_url(
        module, url, params_json, headers=headers, method="POST"
    )

    if info["status"] != 200:
        exit_failed(
            module,
            "Error calling API. HTTP code %d. Details: %s, %s"
            % (info["status"], info["body"], module.jsonify(params)),
        )

Modified the get_existing_rule:

def get_existing_rule(module, base_url, headers, ruleset, rule):
    # Get rules in ruleset
    rules = get_rules_in_ruleset(module, base_url, headers, ruleset)
    if rules.get("extensions").get("found_rules") != 0 :
        # Loop through all rules
        for r in rules.get("value"):
            if ( r["extensions"]["properties"]["description"] == rule["properties"]["description"]):
                # If they have same description, return the ID
                return r["id"]
    return None

I'm happy to submit the change if is ok

[muehlings]

For all password fields I found out the following: You can just write the password as string without the ('password', 'YOURPASSWORD') notation: /omd/sites/TEST/lib/python3/cmk/gui/plugins/wato/utils/init.py has a function to transform the string into the required tuple: forth=lambda v: ("password", v) if not isinstance(v, tuple) else v

This is done automatically and removes at least one type of tuples you have to care about, if the API won't be changed in the future.

I try to find the relevant part in the API of https://github.com/tribe29/checkmk to make a PR there.

[clagio]

that's right! thanks, it's helpful at least for the passwords

[clagio]

Another tuple issue with the overall_tags:

'overall_tags': [('{{aws_tag_key}}', ['{{aws_tag_value}}'])],

The playbook runs fine and the rule is created but when I check the rule in checkmk the option "Restrict monitoring services by one of these AWS tags" is not flagged and I get the following message:

Unable to read current options of this rule. Falling back to default values. When saving this rule now, your previous settings will be overwritten. Problem was: Restrict monitoring services by one of these AWS tags: The datatype must be a tuple, but is list.

[msekania]

@muehlings,

Unfortunately string substitution does not solve the problem and creates other ones, because there are arrays (lists) too, not only tuples! for example : expect_response: ['302', '403'] it should be an array exactly with [ , ] and not tuple with ( , ), otherwise it's again an error!

With this substitution for example

I agree though that API should deliver correctly formatted JSON.

Otherwise:

• one can also use [[ and ]] as an escape for tuple ( and ).
• additionally one can also solve the YAML to python string conversion for value_raw with jinja2 macro

[msekania]

Probably this is also helpful,

jinja2 template file with macros value_raw_gen.j2

{#- Generate value_raw for rule from yaml nested dictionary                                                                     -#}
{#- ########################################################################################################################### -#}
{%- macro generateline(elem)                                                                                                    -%}
{%-   if elem is mapping                                                                                                        -%}
{{-     '{' -}}
{%-      for el in elem | dict2items                                                                                            -%}
{%-        if el.value | lower != 'none'                                                                                        -%}
'{{          el.key }}': {{ generateline(el.value) }}
{%-        endif                                                                                                                -%}
{%-      endfor                                                                                                                 -%}
{{-     '}' -}},
{%-   elif ( elem is iterable ) and ( elem is not string )                                                                      -%}
{#-     check if it is real array, escaped, '[[]]' or just a substitute for tuple '[]'                                          -#}
{%-     if ( elem | count == 1 ) and ( elem[0] is not mapping ) and ( elem[0] is iterable ) and ( elem[0] is not string )       -%}
{{-       '(' -}}
{%-       for el in elem[0]                                                                                                     -%}
{{          generateline(el)[:(-1 if loop.last else none):] }}
{%-       endfor                                                                                                                -%}
{{-       ')' -}},
{%-     else                                                                                                                    -%}
{{-       '[' -}}
{%-       for el in elem                                                                                                        -%}
{{          generateline(el)[:(-1 if loop.last else none):] }}
{%-       endfor                                                                                                                -%}
{{-       ']' -}},
{%-     endif                                                                                                                   -%}
{%-   else                                                                                                                      -%}
{%-     if elem |int(-1) == -1                                                                                                  -%}
{%        if '"' in elem or "'" in elem                                                                                         -%}
{{-         elem }},
{%-       else                                                                                                                  -%}
{{          elem | to_json }},
{%-       endif                                                                                                                 -%}
{%-     else                                                                                                                    -%}
{{        elem }},
{%-     endif                                                                                                                   -%}
{%-   endif                                                                                                                     -%}
{%- endmacro                                                                                                                    -%}
{#- ########################################################################################################################### -#}
{#-                                                                                                                             -#}
{#- ########################################################################################################################### -#}
{%- if value_raw is defined                                                                                                     -%}
{{    generateline(value_raw)[:-1:] }}
{%- endif                                                                                                                       -%}

in ansible then

value_raw: "{{ lookup('template', 'value_raw_gen.j2') | string }}"

[robin-checkmk]

I talked to one of our developers, and it looks like this is an upstream issue, which we cannot solve in the collection. A lot of programming details in here I do not understand, but it sounds like some workarounds were already discussed. I will keep this open for some time, maybe I can get some feedback from our API team.

Maybe someone can write up some sort of bottom line, where we are right now?

[msekania]

If one will not get REST API fixed, so that value_raw format is either JSON (or any ansible compatible format) or can be unambiguously converted to JSON (or any ansible compatible format), then:

• either one writes converter/parser (inside Collection), with "well" defined escape symbols
• employs jinja2 template macros (not necessarily the above listed example), that can be shipped together with tribe29 Collection

Before, it might be also useful to get some obvious bug fixes, I'll be glad to help.

[robin-checkmk]

Thanks, Michael, we are on the same page. I would like to avoid building workarounds in the collection. I am open to point users to possible approaches though. What bug fixes are you talking about, and how would you help?

[msekania]

1. One is already in "pipeline", issue #195, I have some straightforward solution there, bit it brings me to the following question/issue,
2. which are the fields/properties that uniquely identify the rule. Those, which one requires in order to modify the rest of the properties, or, even simpler, to entirely delete this rule?

Both are closely tied to the idempotency of Ansible and the underlying REST API in modules.

If there is a clear answer to the second question, or alternatively a decision what the primary and secondary properties of the rule are, I think the rest can be addressed more systematically and hence relatively easy.

[clagio]

from my understanding there is a ID field that uniquely identify a rule, but a new random ID it's created automatically when you call the API, so you can have multiple rules with all the same values/properties but different ID. To achieve idempotency, in the collection has been arbitrarily decided to check against "conditions", "properties" and "value_raw". Personally I think we should have a "name" field for the rule (that's why I suggested to use the description since it's basically a name for the rule), and check against that, but again should be changed in the API.

[msekania]

@clagio, there is one key problem in your proposal,

how can one then apply the same rule to different folders?

possible solutions:

• either splitting rest API in two sets one for rule management and other for rule applications --- I think one should convince lots of people for it (although this is a better solution)
• or (and I think it's easier to implement) include the folder in checks.

Therefore I still think that there should be a reasonable set of primary fields/properties that are used to Identify the rule, like rule-set, folder, conditions, properties, and may be some subset of value_raw (although latter is causing most of the problems)

ID-s are for internal management and you do not know it apriori from Ansible or API side!

[geof77]

Hi! Two problems related to this issue I came across while developping custom installation playbooks:

1. Maybe remove escaped characters (i.e. \n,\r,\', \",...) from the value_raw before the comparison because it's very difficult (or impossible) to have this right for some rules. For example, I want to create a rule in "ruleset: agent_config:cmk_update_agent". There we must pass the entire certificate in a specific format, with each line surrounded by single-quotes. This is different from what is returned by the API, which does not have the single quotes. So a new rule is created each time the playbook is run! I don't know why we have to send the entire certificate instead of an ID, but that's another story.

2. If there are no conditions in our rule, we still have to specify empty conditions, i.e.:

conditions: { "host_tags": [], "host_labels": [], "service_labels": [] }

If we do not, the comparison fails. It would be great to add these (empty conditions) by default.

I have edited the code to check only the description like suggested above, even if it's not a universal solution, it works for me.

[msekania]

@geof77,

there is even more fundamental problem in the following comparison in rule module:

sorted(r["extensions"]["value_raw"]) == sorted(rule["value_raw"])

it cannot distinguish between the following variants (just an example, one can also make some others):

• expect_response: ['302', '403']
• expect_response: ['402', '303']
• or any possible permutation of parameters, expect_response: ['432', '300']

To avoid all related complication, even for such a simple case, one should parse value_raw in any reasonable format and not as sorted string.

As for now, as I understand, each of us has it's own workaround, which are not suitable to cover general cases and hnece to be integrated in the rule module, IMHO.

Therefor, I will still repeat my question @robin-tribe29: which parameters are primary and which secondary on the REST API level? Implementation in ansible should repeat the same logic as REST API! As an example, if field host_tag is used in primary group in REST API it should be also in primary group in Ansible module -- used as a member of the parameter set that uniquely identifies the rule.

folder for example, is in the primary group in REST API but not in current implementation of the rule module #195.

[lgetwan]

First, Happy new year everybody, and sorry for my late reply. I had a very busy December and the had some time to relax.

The "ID" is only key the REST API uses to identify rules. That doesn't fit very well to the way how idem potency works in Ansible, though. In Ansible, there are two possible solutions:

1. We only can check if ruleset, folder, conditions, properties, value_raw are equal. Probably, we have to remove all "()[]{}' from value_raw vefore comparison.
2. In most environments, it might also be a solution to only compare description and use this as an ID in your Ansible variables.

What do you think of implementing both solutions by inventing a parameter "keys" that has "[ ruleset, folder, conditions, properties, value_raw]" as a default, but can also be set to "[ description ]"? The function get_existing_rule() then can take care of that choice.

@clagio, @muehlings, @geof77 & @msekania: What's your opinion on this?

[muehlings]

Hello @lgetwan,

Adding flags or mechanisms to circumvent the problem are not my preferred way...

I'm going to analyze the logic of the API endpoints (e.g. cmk/gui/plugins/openapi/endpoints/rule/init.py) soon. It should even be possible to programmatically "write" all ansible modules ("parse('endpoints/rule/init.py') -> endpoint_module.py").

But I think I need a newer development vm. I try to get this snippet

from cmk.gui.watolib.rulesets import (
    AllRulesets,
    FolderRulesets,
    Rule,
    RuleConditions,
    RuleOptions,
    Ruleset,
)

all_rulesets = AllRulesets.load_all_rulesets()
print(all_rulesets)

to work, but "match" in [https://raw.githubusercontent.com/tribe29/checkmk/627515db4ad05457276325806f1948841382c112/cmk/gui/watolib/rulesets.py] is python3.10 and my Debian 10 has python3.9. I'm interested in the structure of the rulesets and how the API itself is checking against "AllRulesets".

@lgetwan Perhaps we should define a "target version" we want the ansible modules to work against (2.1.0pXY.*, python3.10+)?

[geof77]

Hello and happy new year! Just an idea: since 1) checkmk has code to format the rule as returned by the API call and 2) nothing stops us from creating identical rules, what about first creating the new rule in disabled state. The API will return the formated rule back to the module. The API representation of the new rule could then be compared with other existing rules. Then if it does not exist, simply recreate it with the enabled state (or leave it alone if the disabled state is desired). Another solution would be to use the cmk code to pre-format the rule before comparison. I don't know if that would work... Edit: @muehlings looks like we're looking at the same file :)

[geof77]

Well, Ok... that idea would work only for "create", not for "delete". Creating a temporary rule just to get the format correct (and delete both if it exists)... I don't like it...

[lgetwan]

@geof77: Even if it feels odd to create an additional rule to delete itself and its clone afterwards, it is still a valid workaround. And unfortunately, it's the only possible workaround, right now.

But as we're planning to release a new version of the collection tomorrow, we have to postpone this workaround. For the planned release, I will reduce the function get_existing_rule() to only compare "properties" and "conditions". Also, I'll write a comment into the docs, saying that the rule module currently doesn't behave as idempotent as it should be.

After the release, we can implement your workaround.

The developers are currently still discussing how to change the Rest API to reply "raw_data" formatted in JSON. If they can do that, that'll make a long-term solution possible.

[geof77]

@lgetwan : what about the code I proposed in #224 ? The current implementation does not work at all, unless we provide a very precise value_raw string that's exactly identical as the representation returned by the API, and empty values (see #231). My version works all the time (except corner cases I'm try to adress now), although I'm not sure that using eval() is secure...

[lgetwan]

@geof77: using eval() is probably not a good idea. @robin-tribe29, what do you think of that?

But besides that: Unfortunately, we do have use cases where we have rules with

• identical conditions
• identical folder
• different settings. In all rule sets that do a "merge", this might occur.

[geof77]

@lgetwan indeed it's probably a bad idea, even "safer" alternatives have big warnings attached. I have code implementing the workaround for the "create" operation and it works perfectly. If you think it's ok to do the same trick with "delete", I will give it a try.

[lgetwan]

@geof77 You mean the "create new rule and delete both if equal"? Yes, you can give it a try. Can you do that today?