Avoid inconsistent modified attributes state after object updates were rejected due to validation errors

[julianbrost]

Update original_attributes only after validation of the new value. Otherwise, the value may be rejected, leaving original_attributes in the already changed but now inconsistent state.

I noticed that issue while working on #10940, but I've created this as a separate PR as this can also be viewed as an independent issue. As shown by the tests below, this can already be triggered even without that PR.

Tests

The following test creates an object, then tries to modify a custom variable with an invalid value and then observes the resulting behavior.

Before (master as of 8ff3d58)

Create an object for testing (having non-empty vars seems to be important for reproducing this):

$ curl -Ssku root:icinga https://127.0.0.1:5665/v1/objects/hosts/inconsistent-attrs \
    -X PUT --json '{"attrs": {"check_command": "dummy", "vars.location": "Earth"}, "pretty": true}'
{
    "results": [
        {
            "code": 200,
            "status": "Object was created"
        }
    ]
}

Attempt to modify its custom variables without success (as expected, it fails):

$ curl -Ssku root:icinga https://127.0.0.1:5665/v1/objects/hosts/inconsistent-attrs \
    -X POST --json '{"attrs": {"vars.oops": "$invalid_macro_string"}, "pretty": true}'
{
    "results": [
        {
            "code": 500,
            "name": "inconsistent-attrs",
            "status": "Attribute 'vars.oops' could not be set: Error: Validation failed for object 'inconsistent-attrs' of type 'Host'; Attribute 'vars' -> 'oops': Closing $ not found in macro format string '$invalid_macro_string'.\nLocation: in /data/var/lib/icinga2/api/packages/_api/77e78991-06d4-4589-aa0c-ee60e1d7724e/conf.d/hosts/inconsistent-attrs.conf: 1:0-1:31\n/data/var/lib/icinga2/api/packages/_api/77e78991-06d4-4589-aa0c-ee60e1d7724e/conf.d/hosts/inconsistent-attrs.conf(1): object Host \"inconsistent-attrs\" {\n                                                                                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n/data/var/lib/icinga2/api/packages/_api/77e78991-06d4-4589-aa0c-ee60e1d7724e/conf.d/hosts/inconsistent-attrs.conf(2):  check_command = \"dummy\"\n/data/var/lib/icinga2/api/packages/_api/77e78991-06d4-4589-aa0c-ee60e1d7724e/conf.d/hosts/inconsistent-attrs.conf(3):  vars[\"location\"] = \"Earth\"\n",
            "type": "Host"
        }
    ]
}

Now query the object, in particular the original_attributes, you'll see that even though the update was rejected, vars.oops was added there:

$ curl -Ssku root:icinga https://127.0.0.1:5665/v1/objects/hosts/inconsistent-attrs \
    -X GET --json '{"attrs": ["vars", "original_attributes"], "pretty": true}' 
{
    "results": [
        {
            "attrs": {
                "original_attributes": {
                    "vars.oops": null
                },
                "vars": {
                    "location": "Earth"
                }
            },
            "joins": {},
            "meta": {},
            "name": "inconsistent-attrs",
            "type": "Host"
        }
    ]
}

Now restart Icinga 2:

$ curl -Ssku root:icinga https://127.0.0.1:5665/v1/actions/restart-process \
    -X POST --json '{"pretty": true}'                                                   
{
    "results": [
        {
            "code": 200,
            "status": "Restarting Icinga 2."
        }
    ]
}

And perform the very same query again, where you can observe, that now the object has a mysterious vars.oops with value null:

% curl -Ssku root:icinga https://127.0.0.1:5665/v1/objects/hosts/inconsistent-attrs \
    -X GET --json '{"attrs": ["vars", "original_attributes"], "pretty": true}' 
{
    "results": [
        {
            "attrs": {
                "original_attributes": {
                    "vars.oops": null
                },
                "vars": {
                    "location": "Earth",
                    "oops": null
                }
            },
            "joins": {},
            "meta": {},
            "name": "inconsistent-attrs",
            "type": "Host"
        }
    ]
}

This PR (as of 0332ead)

If the object is still there from the previous test, make sure to delete it.

Creating (PUT) and attempting to update (POST) produces the same responses, the difference only starts to show with the following GET request, where nothing was added to original_attributes:

$ curl -Ssku root:icinga https://127.0.0.1:5665/v1/objects/hosts/inconsistent-attrs \
    -X GET --json '{"attrs": ["vars", "original_attributes"], "pretty": true}'

{
    "results": [
        {
            "attrs": {
                "original_attributes": {},
                "vars": {
                    "location": "Earth"
                }
            },
            "joins": {},
            "meta": {},
            "name": "inconsistent-attrs",
            "type": "Host"
        }
    ]
}

Which - in contrast to the previous behavior - also stays the same across a restart of Icinga 2.

Additional Information

The value of original_attributes is important here as this is used to create /var/lib/icinga2/modified-attributes.conf which then contains something like this with the unfixed version, explaining why this suddenly appears within vars after the restart:

var obj = get_object("Host", "inconsistent-attrs")
if (obj) {
	obj.modify_attribute("vars.oops", null)
	obj.version = 1783613466.662146
}

[jschmidt-icinga]

I've looked at the test commands and confirmed the problem and solution.

One small issue I had with reproducing this is that a podman compose restart on my container setup seems to wipe modified-attributes.conf but not the object in api, but that's very likely unrelated to this PR and with the API restart I could reproduce it immediately.

Two small suggestions below:

[jschmidt-icinga]

Maybe this declaration can be moved a little closer to where it's used.

[yhabteab]

Also using snake_case as a variable name in 2026 :).

[jschmidt-icinga]

Maybe std::move(kv.second) here and std::move(oldValue) in the two cases below. Probably doesn't matter much unless the Value contains a really big string, but still...