Skip to content

Allow assisted configuration of notifications - #1396

Merged
BastianLedererIcinga merged 2 commits into
mainfrom
configure-notifications
Sep 4, 2026
Merged

Allow assisted configuration of notifications#1396
BastianLedererIcinga merged 2 commits into
mainfrom
configure-notifications

Conversation

@BastianLedererIcinga

@BastianLedererIcinga BastianLedererIcinga commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

resolve #1356

This PR introduces the General Settings section, which contains the functionality of the previous Database section and also allows to configure Icinga Notifications. GeneralConfigForm is introduced to support this.

For the configuration of Icinga Notifications a toggle is added which, allows to enable/disable the Icinga Notifications integration, as long as it runs on the same system as Icinga DB and the two are connected by a Unix domain socket.

To achieve this this PR relies on a new table and modifications to the instance table.

Enabling the toggle simply writes the discovered socket path(s) to in icingadb_config and ensures Icinga Notifications has a matching source for each configured service user. Disabling it removes the rows from icingadb_config, but will not touch the sources.
Once row is written per endpoint found in icingadb_instance, if the endpoint_id is null a a default value of 20 zero bytes is used, matching the daemons fallback.

The toggle is disabled with a callout explaining why if:

  • Instances of multiple environments exist
  • The schema is outdated
  • The daemon did not provide a socket path
  • icingadb_config has a row with locked = y, because a static configuration already exists.
  • no responsible icingadb_instance is found at all
  • ICINGADB_NOTIFICATIONS_SYNCHRONIZE_WITH_DATABASE is false
  • No database is configure yet

@cla-bot cla-bot Bot added the cla/signed CLA is signed by all contributors of a PR label Jul 23, 2026
@BastianLedererIcinga
BastianLedererIcinga force-pushed the configure-notifications branch 3 times, most recently from 861327a to 914dd1b Compare July 23, 2026 09:00

@sukhwinder33445 sukhwinder33445 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The general Settings tab now shows two forms; please add a separator to visually distinguish them. For example , see the /account page.

Comment thread application/controllers/ConfigController.php Outdated
Comment thread application/controllers/ConfigController.php Outdated
Comment thread application/controllers/ConfigController.php Outdated
Comment thread configuration.php Outdated
Comment thread application/forms/NotificationsConfigForm.php Outdated
Comment thread application/forms/NotificationsConfigForm.php Outdated
Comment thread library/Icingadb/Common/NotificationsIntegration.php Outdated
Comment thread library/Icingadb/Model/IcingaDBConfig.php
Comment thread library/Icingadb/Model/Config.php
Comment thread library/Icingadb/Common/NotificationsIntegration.php Outdated
Comment thread application/forms/NotificationsConfigForm.php Outdated
Comment thread application/controllers/ConfigController.php Outdated
Comment thread library/Icingadb/Model/IcingaDBConfig.php Outdated
Comment thread application/forms/NotificationsConfigForm.php Outdated
Comment thread application/forms/NotificationsConfigForm.php Outdated
Comment thread application/forms/NotificationsConfigForm.php Outdated

@sukhwinder33445 sukhwinder33445 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far.

I'll approve this PR as soon as the IcingaDB part has been implemented and this branch can be properly tested.

@BastianLedererIcinga
BastianLedererIcinga marked this pull request as ready for review August 14, 2026 11:43
Comment thread library/Icingadb/Model/Config.php Outdated
Comment thread library/Icingadb/Model/Config.php
Comment thread application/controllers/ConfigController.php
Comment thread application/forms/GeneralConfigForm.php Outdated
Comment thread application/forms/GeneralConfigForm.php Outdated
Comment thread application/forms/GeneralConfigForm.php Outdated
Comment thread application/forms/GeneralConfigForm.php Outdated
Comment thread application/forms/GeneralConfigForm.php
Comment thread application/forms/GeneralConfigForm.php Outdated
Comment thread application/forms/GeneralConfigForm.php Outdated
Comment thread application/forms/GeneralConfigForm.php
Comment thread application/forms/GeneralConfigForm.php Outdated
Comment thread application/forms/GeneralConfigForm.php Outdated
Comment thread public/css/form/general-config-form.less

@nilmerg nilmerg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Managed to get this:

Image

And the reason is that there is no source configured due to a database change. The toggle should not be on in this case and an appropriate warning should appear.

We should also think about a notice that tells the user the configuration looks fine, although the connection is unhealthy.

Comment thread application/forms/GeneralConfigForm.php Outdated
Comment thread public/css/form/general-config-form.less Outdated
Comment thread application/forms/GeneralConfigForm.php Outdated
Comment thread application/forms/GeneralConfigForm.php
Comment thread application/forms/GeneralConfigForm.php

@nilmerg nilmerg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😓

Add the `Config` model to represent the `icingadb_config` table, and
declare the relation in `Environment`.

Add columns necessary for assisted Icinga Notifications configuration
to `Instance` guarded by `Backend::supportsNotifications()`
Rename the existing `Database` tab to `General Settings` and introduce
`GeneralConfigForm`.

In addition to the database configuration a toggle is added to (un)-configure
Icinga Notifications.

If the assisted configuration is not possible the toggle is locked with a
callout explaining the reason.
@BastianLedererIcinga
BastianLedererIcinga merged commit c4d4928 into main Sep 4, 2026
13 checks passed
@BastianLedererIcinga
BastianLedererIcinga deleted the configure-notifications branch September 4, 2026 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla/signed CLA is signed by all contributors of a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow to configure Icinga Notifications

3 participants