borgmatic-collective/borgmatic
borgmatic-collective/borgmatic
9
74
Fork 56
Code Issues 66 Pull requests 5 Releases 161 Packages 1 Activity Actions 1

Order of operations between normalise and override #561

New issue
Closed
opened 2022-07-05 21:05:55 +00:00 by g-a-c · 4 comments
g-a-c commented 2022-07-05 21:05:55 +00:00
Contributor
Copy link

What I'm trying to do and why

Since commit 02781662f85bcdbab795bf67fdecf28f85cb50d7 the healthchecks configuration changed from a simple string to an object. This is backwards compatible within the configuration file because the normalize function will upgrade the configuration on-the-fly.

However this normalisation happens before overrides are applied (borgmatic/config/validate.py), so if you have an override to ping a different Healthcheck (which I currently do before I switch to ntfy) then this will fail "silently" without sending a failure ping because the Healthchecks config is invalid

Maybe it would be better to override before normalising so that these dynamic changes can apply in both cases instead of silently failing in one of them?

[borgmatic/config/validate.py](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/master/borgmatic/config/validate.py#L101-L102)

Steps to reproduce (if a bug)

hooks:
  healthchecks: https://hc-ping.com/UUID1

borgmatic check --config /etc/borgmatic/config.yaml -v 1 --override hooks.healthchecks=https://hc-ping.com/UUID2

Actual behavior (if a bug)

Jul 05 21:10:18 server borgmatic[8923]: CRITICAL An error occurred while parsing a configuration file at /etc/borgmatic/config.yaml:
    At 'hooks.healthchecks': 'https://hc-ping.com/UUID2' is not of type 'object'
Jul 05 21:10:18 server pipenv[8923]: /etc/borgmatic/config.yaml: No valid configuration files found
Jul 05 21:10:18 server borgmatic[8923]: CRITICAL /etc/borgmatic/config.yaml: No valid configuration files found       
Jul 05 21:10:18 server borgmatic[8923]: CRITICAL
Jul 05 21:10:18 server pipenv[8923]: Need some help? https://torsion.org/borgmatic/#issues
Jul 05 21:10:18 server borgmatic[8923]: CRITICAL Need some help? https://torsion.org/borgmatic/#issues
Jul 05 21:10:18 server systemd[1]: borgmatic.service: Main process exited, code=exited, status=1/FAILURE
Jul 05 21:10:18 server systemd[1]: borgmatic.service: Failed with result 'exit-code'.
Jul 05 21:10:18 server systemd[1]: Failed to start Run system-wide Borgmatic backups.

Expected behavior (if a bug)

That the command line --override is also normalised in the same way, which makes it compatible with hooks.healthchecks being an object, not a string

Other notes / implementation ideas

Environment

borgmatic version: 1.6.3

Use sudo borgmatic --version or sudo pip show borgmatic | grep ^Version

borgmatic installation method: pipenv

Borg version: 1.2.1

Use sudo borg --version

Python version: 3.9.2

Use python3 --version

Database version (if applicable): n/a

Use psql --version or mysql --version on client and server.

operating system and version: Debian 11

#### What I'm trying to do and why Since commit `02781662f85bcdbab795bf67fdecf28f85cb50d7` the `healthchecks` configuration changed from a simple string to an object. This is backwards compatible within the configuration file because the `normalize` function will upgrade the configuration on-the-fly. However this normalisation happens before overrides are applied ([`borgmatic/config/validate.py`](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/master/borgmatic/config/validate.py#L101-L102)), so if you have an override to ping a different Healthcheck (which I currently do before I switch to `ntfy`) then this will fail "silently" without sending a failure ping because the Healthchecks config is invalid Maybe it would be better to override _before_ normalising so that these dynamic changes can apply in both cases instead of silently failing in one of them? `[borgmatic/config/validate.py](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/master/borgmatic/config/validate.py#L101-L102)` #### Steps to reproduce (if a bug) ```yaml hooks: healthchecks: https://hc-ping.com/UUID1 ``` ```sh borgmatic check --config /etc/borgmatic/config.yaml -v 1 --override hooks.healthchecks=https://hc-ping.com/UUID2 ``` #### Actual behavior (if a bug) ``` Jul 05 21:10:18 server borgmatic[8923]: CRITICAL An error occurred while parsing a configuration file at /etc/borgmatic/config.yaml: At 'hooks.healthchecks': 'https://hc-ping.com/UUID2' is not of type 'object' Jul 05 21:10:18 server pipenv[8923]: /etc/borgmatic/config.yaml: No valid configuration files found Jul 05 21:10:18 server borgmatic[8923]: CRITICAL /etc/borgmatic/config.yaml: No valid configuration files found Jul 05 21:10:18 server borgmatic[8923]: CRITICAL Jul 05 21:10:18 server pipenv[8923]: Need some help? https://torsion.org/borgmatic/#issues Jul 05 21:10:18 server borgmatic[8923]: CRITICAL Need some help? https://torsion.org/borgmatic/#issues Jul 05 21:10:18 server systemd[1]: borgmatic.service: Main process exited, code=exited, status=1/FAILURE Jul 05 21:10:18 server systemd[1]: borgmatic.service: Failed with result 'exit-code'. Jul 05 21:10:18 server systemd[1]: Failed to start Run system-wide Borgmatic backups. ``` #### Expected behavior (if a bug) That the command line `--override` is also normalised in the same way, which makes it compatible with `hooks.healthchecks` being an object, not a string #### Other notes / implementation ideas #### Environment **borgmatic version:** 1.6.3 Use `sudo borgmatic --version` or `sudo pip show borgmatic | grep ^Version` **borgmatic installation method:** `pipenv` **Borg version:** 1.2.1 Use `sudo borg --version` **Python version:** 3.9.2 Use `python3 --version` **Database version (if applicable):** n/a Use `psql --version` or `mysql --version` on client and server. **operating system and version:** Debian 11
witten commented 2022-07-05 23:06:26 +00:00
Owner
Copy link

Good catch! I apologize for the breakage. I think the solution you suggest (apply overrides before normalization) makes sense. I'd just have to make sure that if you pass in old-style overrides using a new-style configuration file, the override would still replace the existing configuration.

Good catch! I apologize for the breakage. I think the solution you suggest (apply overrides before normalization) makes sense. I'd just have to make sure that if you pass in old-style overrides using a new-style configuration file, the override would still replace the existing configuration.
g-a-c commented 2022-07-06 09:06:51 +00:00
Author
Contributor
Copy link

No need to apologise, it's a breakage of my own making since I probably didn't bother reading the release notes last time I upgraded :)

It's just an unfortunate one because the config being invalid means that borgmatic never runs, so it won't even send a failure ping. Also that second UUID I set up with Healthchecks doesn't nag every day if the maintenance (check, prune) phases don't run so I hadn't noticed they weren't running for a short while.

No need to apologise, it's a breakage of my own making since I probably didn't bother reading the release notes last time I upgraded :) It's just an unfortunate one because the config being invalid means that `borgmatic` never runs, so it won't even send a failure ping. Also that second UUID I set up with Healthchecks doesn't nag every day if the maintenance (check, prune) phases don't run so I hadn't noticed they weren't running for a short while.
witten commented 2022-07-07 01:21:29 +00:00
Owner
Copy link

This is fixed in master and will be part of the next release. Thanks again for reporting it!

This is fixed in master and will be part of the next release. Thanks again for reporting it!
witten commented 2022-07-20 03:35:52 +00:00
Owner
Copy link

This has been released in borgmatic 1.6.6!

This has been released in borgmatic 1.6.6!
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
bootstrap-multiple-latest-archives
parallel-borgmatic
command-hook-steps
2.1.6
2.1.5
2.1.4
2.1.3
2.1.2
2.1.1
2.1.0
2.0.13
2.0.12
2.0.11
2.0.10
2.0.9
2.0.8
2.0.7
2.0.6
2.0.5
2.0.4
2.0.3
2.0.2
2.0.1
2.0.0
1.9.14
1.9.13
1.9.12
1.9.11
1.9.10
1.9.9
1.9.8
1.9.7
1.9.6
1.9.5
1.9.4
1.9.3
1.9.2
1.9.1
1.9.0
1.8.14
1.8.13
1.8.12
1.8.11
1.8.10
1.8.9
1.8.8
1.8.7
1.8.6
1.8.5
1.8.4
1.8.3
1.8.2
1.8.1
1.8.0
1.7.15
1.7.14
1.7.13
1.7.12
1.7.11
1.7.10
1.7.9
1.7.8
1.7.7
1.7.6
1.7.5
1.7.4
1.7.3
1.7.2
1.7.1
1.7.0
1.6.6
1.6.5
1.6.4
1.6.3
1.6.2
1.6.1
1.6.0
1.5.24
1.5.23
1.5.22
1.5.21
1.5.20
1.5.19
1.5.18
1.5.17
1.5.16
1.5.15
1.5.14
1.5.13
1.5.12
1.5.11
1.5.10
1.5.9
1.5.8
1.5.7
1.5.7.dev0
1.5.6
1.5.5
1.5.4
1.5.3
1.5.2
1.5.1
1.5.0
1.4.22
1.4.21
1.4.20
1.4.19
1.4.18
1.4.17
1.4.16
1.4.15
1.4.14
1.4.13
1.4.12
1.4.11
1.4.10
1.4.9
1.4.8
1.4.7
1.4.6
1.4.5
1.4.4
1.4.3
1.4.2
1.4.1
1.4.0
1.3.26
1.3.25
1.3.24
1.3.23
1.3.22
1.3.21
1.3.20
1.3.19
1.3.18
1.3.17
1.3.16
1.3.15
1.3.14
1.3.13
1.3.12
1.3.11
1.3.10
1.3.9
1.3.8
1.3.7
1.3.6
1.3.5
1.3.4
1.3.3
1.3.2
1.3.1
1.3.0
1.2.18
1.2.17
1.2.16
1.2.15
1.2.14
1.2.13
1.2.12
1.2.11
1.2.10
1.2.9
1.2.8
1.2.7
1.2.6
1.2.5
1.2.4
1.2.3
1.2.2
1.2.1
1.2.0
1.1.15
1.1.14
1.1.13
1.1.12
1.1.11
1.1.10
1.1.9
1.1.7
1.1.6
1.1.5
1.1.4
1.1.3
1.1.2
1.1.1
1.1.0
1.0.3
1.0.2
1.0.1
1.0.0
0.1.8
0.1.7
0.1.6
0.1.5
0.1.4
0.1.3
0.1.2
0.1.1
0.1.0
0.0.7
0.0.6
0.0.5
0.0.4
0.0.3
0.0.2
Labels
Clear labels   
blocked

   
breaking

   
bug

   
data loss

   
design finalized

   
good first issue

   
new feature area

   
question / support

   
security

   
waiting for response
No labels
blocked
breaking
bug
data loss
design finalized
good first issue
new feature area
question / support
security
waiting for response
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
borgmatic-collective/borgmatic#561
No description provided.