Load Notification objects after User and UserGroup

[julianbrost]

Notification objects can refer User and Group objects similar to how they can refer Host and Service objects, so that dependency feels quite natural. Note that for evaluating most configuration, this order doesn't really matter, the configuration will successfully evaluate in either case, the difference can be noticed mainly in more advanced configurations, for example when dynamically assigning user based on their groups. When accessing user objects from the Notification object definition (like in the following example), without this change, only groups configured directly in groups attribute of User objects are visible and those added via assign clauses in UserGroup objects are missing. With this commit, these are also visible.

apply Notification "n" to Host {
    for (var u in get_objects(User)) {
        log(u.name + " -> " + Json.encode(u.groups))
    }

    # [...]
}

This was reported by a customer. Their configuration no longer worked as expected with snapshot packages compared to already released versions. I can't share their config here, but in summary, they are populating the users attribute with users that are both member of a group and are configured to receive notifications via a specific communication channel.

Tests

Self-contained Icinga 2 config that adds a User to the UserGroup "g-attr" via its `groups` attribute and adds it to a second UserGroup "g-assign" via the `assign` clause in the group definition. It logs the groups of the user visible from within the Notification definition.

include <itl>

object UserGroup "g-attr" {
}

object UserGroup "g-assign" {
	assign where true
}

object User "u" {
	groups = ["g-attr"]
}

object Host "h" {
	check_command = "dummy"
}

object NotificationCommand "dummy" {
	command = ["true"]
}

apply Notification "n" to Host {
	for (var u in get_objects(User)) {
		log(u.name + " -> " + Json.encode(u.groups))
	}

	command = "dummy"
	users = ["u"]
	assign where true
}

Current master

[2025-04-29 10:28:44 +0000] information/config: u -> ["g-attr"]

This PR

[2025-04-29 10:30:37 +0000] information/config: u -> ["g-attr","g-assign"]

ref/IP/59088
refs #10148 (This behavior was changed by 467e8b1, this PR restores the previous behavior)

[yhabteab]

#10000 would've fixed this with e.g. 687013c before I discarded all the load_after changes for all types there. No, you're fixing this for notifications but shouldn't the problem apply to pretty much every type that has such dependencies without such load_after ordering set?

[julianbrost]

#10000 would've fixed this with e.g. 687013c..f8df6b4

Did you want to point out https://github.com/Icinga/icinga2/compare/687013c4a1b538e3c5a231d97da80b88efb39f42..f8df6b4fa8fc0dd4a6cab0bfaa2bf1b7468db546? At least looking at the individual commits there makes little sense to me.

[yhabteab]

#10000 would've fixed this with e.g. 687013c..f8df6b4

Did you want to point out https://github.com/Icinga/icinga2/compare/687013c4a1b538e3c5a231d97da80b88efb39f42..f8df6b4fa8fc0dd4a6cab0bfaa2bf1b7468db546? At least looking at the individual commits there makes little sense to me.

Oh GitHub 🫩! Yes thank you!

[julianbrost]

So what exactly would be your suggestion for doing here instead? Applying https://github.com/Icinga/icinga2/compare/687013c4a1b538e3c5a231d97da80b88efb39f42..f8df6b4fa8fc0dd4a6cab0bfaa2bf1b7468db546 as-is instead?

I wouldn't resort to #10000 here: that PR was supposed to solve something different, and did so in a different way. It can be a reference, but the suggestion in this PR should be more or less self-contained, as that suggestion is somewhere in the quite long history of that PR.

[yhabteab]

So what exactly would be your suggestion for doing here instead? Applying https://github.com/Icinga/icinga2/compare/687013c4a1b538e3c5a231d97da80b88efb39f42..f8df6b4fa8fc0dd4a6cab0bfaa2bf1b7468db546 as-is instead?

No, not all! I'm asking you if that's all what you want to do here and wait till someone complains about the other types as well or fix it for all of them here.

shouldn't the problem apply to pretty much every type that has such dependencies without such load_after ordering set?

[julianbrost]

The thing is that it's actually not that much about "Notification has an attribute users, hence it should load_after User" (referencing users and groups from a notification already works and doesn't need this change), but rather "is it plausible/useful/expected/desirable (pick whatever you like) to do something like get_objects(User) inside a Notification and expects their groups to be already evaluated".

The problem here is that none of this is really well-specified, the config language allows you to do pretty much everything and hence you may observe all kinds of implementation details. So yes, that might happen:

I'm asking you if that's all what you want to do here and wait till someone complains about the other types

If you see particular reasons why we should add some other load_after, that would be welcome, otherwise, we'd just add with little explanation because they sound somewhat good. Actually, I've just though of #8116, which regards using host.groups in apply Notification, which might actually be similar about this.

[Al2Klimov]

is it plausible/useful/expected/desirable (pick whatever you like) to do something like get_objects(User) inside a Notification and expects their groups to be already evaluated

• Exactly as Object activation order in 2.9.x #6534, just for other types. 👍

[julianbrost]

I've added a short section to the PR description describing what's done in the configuration using which the change in behavior was noticed. So far, I've only described a minimal example to show the difference, now it also describes a real-world use-case.