Add notifications_schema table & verify on daemon startup#422
Conversation
|
Focusing on the schema differences here: That the I too find it confusing that the schema table of icingadb uses a integer that differs from the file name or product version… |
yhabteab
force-pushed
the
schema-table
branch
2 times, most recently
from
9f5dd65 to
1408de6
Compare
|
Changed the variable len of the |
There was a problem hiding this comment.
Pull request overview
Introduces a notifications_schema table that records the applied schema version (as a semver string like v1.0), plus a SQL assert_correct_schema_version procedure so that future upgrade scripts can refuse to run out of order. The daemon now verifies on startup that the schema version stored in the DB matches the version the binary expects, failing fast otherwise. This resolves #343 and lays the groundwork for safer future migrations.
Changes:
- Add
notifications_schematable andassert_correct_schema_versionPL/pgSQL / MySQL stored procedure in both the base schema and a newupgrades/1.0.sqlfor PostgreSQL and MySQL. - Add
internal.CheckSchemawhich compares the latestnotifications_schema.versionrow against a per-driver expected version usinggolang.org/x/mod/semver. - Wire
CheckSchemainto the daemon startup incmd/icinga-notifications/main.goand pull ingolang.org/x/modas a new dependency.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| schema/pgsql/schema.sql | Defines the assert procedure and the notifications_schema table; seeds v1.0. |
| schema/pgsql/upgrades/1.0.sql | Upgrade script equivalent that creates the procedure, table and seed row for existing installs. |
| schema/mysql/schema.sql | MySQL counterpart of the procedure/table/seed, using SIGNAL SQLSTATE '45000' for errors. |
| schema/mysql/upgrades/1.0.sql | MySQL upgrade script mirroring the base schema additions. |
| internal/schema.go | New CheckSchema helper that verifies the table exists and the latest version matches per driver, with retry. |
| cmd/icinga-notifications/main.go | Calls internal.CheckSchema after connecting to the DB; fatals on mismatch. |
| go.mod / go.sum | Adds golang.org/x/mod v0.35.0 for semver.Compare. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This PR adds a new
notifications_schematable to the database, which is used to store the schema version for notifications. It quite similar to the one used in Icinga DB, but with a small but important difference: theversioncolumn is of typeTEXTinstead ofINTEGERas opposed to Icinga DB. Instead of using an independent integer versioning system that can't be easily determined to which Icinga Notifications version it corresponds, we will use the actual semver version exactly as its upgrade script is named.In order to prevent users from applying the upgrade scripts out of order or the same script multiple times, any upgrade script after Icinga Notifications
v1.0will have use an SQL procedure introduced in this PR, which will check if the current version in thenotifications_schematable matches the expected one before applying the upgrade. If the version does not match, the procedure will raise an error and prevent the upgrade from being applied. For instance, if we have an upgrade script for versionv1.1, at the top of the script we will have a call to the procedure like this:CALL assert_correct_schema_version('v1.0');This ensures that the current upgrade script can only be applied if the current version in the
notifications_schematable isv1.0, which means that the previous upgrade script has been applied. If the current version is notv1.0, the procedure will raise an error and prevent the upgrade from being applied, thus ensuring that the upgrade scripts are applied in the correct order and that no script is applied multiple times. And of course, Icinga Notifications performs a check at startup to ensure that the expected schema version is applied before doing anything else, but it doesn't go through thenotifications_schemaentries to determine for missing upgrade scripts since that is supposed to be handled by the upgrade scripts themselves.resolves #343