Fix service inheritance with overrides on multiple nested levels

[eurotux-tec]

Currently when you have a service inside a host template with defined values, if you create a host importing that template and override other values, when deploying you lose the values that (according to the hints in the web form) should have been inherited.

For example, if the service accepts 2 values valueA and valueB, and the parent template has valueA=1, when you edit the service in the host you can see valueA has the "default" value 1 (in gray). If you then set valueB to something on the host service and save, you can still see valueA with 1 on the form. However when you deploy and access that host's service in Icinga monitoring module, you'll see that valueA is now undefined. If you unset valueB and deploy again, you'll see that the host's valueA has the template's value 1 in the monitoring module.

The reason for this is because when overriding, the generated configuration tries to merge with += all of vars["_override_servicevars"] with a dictionary containing each overriden service as the key, and as its value another dictionary with each field's values. However, the operator += is not recursive and instead replaces the vars["_override_servicevars"][servicename] element.

This commit fixes that by replacing the += operator with a Icinga 2 DSL helper function that takes proper care of merging, with one extra level of nesting, the configuration for each of the services with the values already imported from the parent template, making the deployed configuration behave as expected, according with what is displayed on the web forms (or mostly, see below [1]).

We named the function directorMergeOverrideConfig and defined it in the generated zones.d/director-global/001-director-basics.conf alongside other functions already and templates defined by Director, but you can rename the function to something you find more appropriate.

So the generated config, instead of becoming:

    import "parent-template"

    vars["_override_servicevars"] += {
        service = {
            valueB = 2
        }
    })

will become:

    import "parent-template"

    vars["_override_servicevars"] = directorMergeOverrideConfig(vars["_override_servicevars"], {
        service = {
            valueB = 2
        }
    })

which may not be as pretty, but it's not much worse either, most important is that it now behaves as expected.

[1] We say "mostly" because when the overridden value is an array, the web form still doesn't show correctly what will be deployed. An example: suppose you have valueA = ['a'] on the parent template's service, with a corresponding array field associated. When you edit that service in a host that imports that template, you'll see the field has a gray value of "a (inherited from servicename)". If you then try to override that value with, let's say ['b'], the web interface now shows the value "b" but still shows "a (inherited from servicename)" in the second line of the field. The deployed configuration will only deploy ['b'] (this is what happens now, and this patch doesn't change that behaviour), which makes sense because otherwise you'd always be forced to keep the templates ['a'] without any means of fully overriding it, so we'd say that is a different bug, this time on the web forms (if you agree, then a separate issue should be opened, correct?).

So this patch only tries to solve the most "serious" bug that is the host services losing configuration from parent imports when overriding service values, but there still is a additional (mostly cosmetic) bug in the web forms GUI when the field is of an array type with a value that has been defined on the parent template and you override it on the host.

Can you review this commit and see if it's good to merge? (the patch also applies well to 1.8.1 stable version, should anyone be interested until a new stable Director version comes out)

[araujorm]

Good morning.
The previous commit was incompatible with old (ancient...) icinga2 versions. A new version has been force pushed that should work with most icinga2 versions out there (this one was just tested with 2.8.4).

[araujorm]

Hello.

Pull request was updated to merge without issues in current master.

As far as we know, without this fix, the issue still occurs. Can this be reviewed?
If any adjustments are needed, please tell.

Thanks and best regards.

[eurotux-tec]

Hello.

We've again refreshed the proposed commit so that it merges with the current master. We also adjusted the commit message so that it provides a bit more detail, and adjusted the title of the PR to match it.

Could we have any feedback on whether it's acceptable, or if there is any other approach you'd prefer instead?

[lippserd]

@eurotux-tec I just wanted to send you a quick note because it looks like you are still actively using Icinga and need these changes. At the end of this year we will start planning the feature release and will take this PR into account. Please excuse the long wait.