CreateObjectHandler: Fix ignore_on_error api flag doesn't work properly

[yhabteab]

The API ignore_on_error flag doesn't quite work yet, as it should. For example, if you don't set the check_command attribute when creating a host object via API, the action will fail during config validation and it won't even go out from the ConfigObjectUtility::CreateObject() method. After that internal server error is sent, even though you have set the flag.

[yhabteab]

Before:

curl -k -s -u root:icinga -H 'Accept: application/json' \                      
 -X PUT 'https://localhost:5665/v1/objects/hosts/example' \
 -d '{ "attrs": { "address": "192.168.1.1", "vars.os" : "Linux" }, "ignore_on_error": true, "pretty": true }'

{
    "results": [
        {
            "code": 500,
            "errors": [
                "Error: Validation failed for object 'example' of type 'Host'; Attribute 'check_command': Attribute must not be empty.\nLocation: in /Users/yhabteab/Workspace/icinga2/prefix/var/lib/icinga2/api/packages/_api/ca7c22f6-3eed-426c-bd2e-04b9cf04c267/conf.d/hosts/example.conf: 1:0-1:32"
            ],
            "status": "Object could not be created."
        }
    ]
}

After:

{
    "results": [
        {
            "code": 200,
            "status": "Object was not created but 'ignore_on_error' was set to true."
        }
    ]
}

[Al2Klimov]

Seems quite logical to me and the code below confirms this.

Apropos: couldn’t you just fix the condition instead of duplicating the mentioned code?

[Al2Klimov]

Have you re-tested it (all cases)?

[julianbrost]

Given the description of #8820, it sound like this PR would introduce a crash. This has to be investigated/fixed before we can merge this. (Posting this "request changes" review so that it's not forgotten.)

[Al2Klimov]

Exactly, so

Blocked by

• ConfigItem: Fix infinite recursion caused by ignore_on_error when … #9406

[julianbrost]

Before:

curl -k -s -u root:icinga -H 'Accept: application/json' \                      
 -X PUT 'https://localhost:5665/v1/objects/hosts/example' \
 -d '{ "attrs": { "address": "192.168.1.1", "vars.os" : "Linux" }, "ignore_on_error": true, "pretty": true }'

{
    "results": [
        {
            "code": 500,
            "errors": [
                "Error: Validation failed for object 'example' of type 'Host'; Attribute 'check_command': Attribute must not be empty.\nLocation: in /Users/yhabteab/Workspace/icinga2/prefix/var/lib/icinga2/api/packages/_api/ca7c22f6-3eed-426c-bd2e-04b9cf04c267/conf.d/hosts/example.conf: 1:0-1:32"
            ],
            "status": "Object could not be created."
        }
    ]
}

Can't reproduce this with the current master (dd7009d):

$ curl -i -k -s -u root:icinga -H 'Accept: application/json' -X PUT 'https://localhost:5665/v1/objects/hosts/example' -d '{ "attrs": { "address": "192.168.1.1", "vars.os" : "Linux" }, "ignore_on_error": true, "pretty": true }'
HTTP/1.1 200 OK
Server: Icinga/v2.13.0-458-gdd7009dc6
Content-Type: application/json
Content-Length: 157

{
    "results": [
        {
            "code": 200,
            "status": "Object was not created but 'ignore_on_error' was set to true"
        }
    ]
}

Also, with your PR rebased onto the current master, the following happens:

$ curl -i -k -s -u root:icinga -H 'Accept: application/json' -X PUT 'https://localhost:5665/v1/objects/hosts/example' -d '{ "attrs": {"invalid":0}, "ignore_on_error": true, "pretty": true }'                                    
HTTP/1.1 500 Internal Server Error
Server: Icinga/v2.13.0-459-g20d705740
Content-Type: application/json
Content-Length: 228

{
    "results": [
        {
            "code": 500,
            "errors": [
                "Error: Invalid attribute specified: invalid\n"
            ],
            "status": "Object could not be created."
        }
    ]
}

I find the documentation for the ignore_on_error parameter inconsistent with what it does. To me this reads like this only changes error reporting, but that's not actually what the implementation does. What really happens is that the object is written to a config file in the _api package with the ignore_on_error attribute set, which means that if the error is gone on the next reload (for example because the error was that it referred to a non-existing host that was created in the meantime), the object will become active at that time. Otherwise, if the error is still present at a reload, the config file is deleted entirely.

To be honest, I don't see many use cases for this flag where you actually want that behavior rather than just ignore error responses. So another option would be to make the documentation more clear on what this parameter actually does.

[Al2Klimov]

Works for me as well, why exactly do we still need this?

[yhabteab]

Fixed by #9406 implicitly.