From 6f2c1037d5ac3d252fd693d9ce46d80bdbb8186f Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sat, 23 Nov 2024 21:34:07 +0100 Subject: [PATCH 1/9] catch errors during execution of hook, do not raise exception but log error --- radicale/storage/multifilesystem/lock.py | 32 +++++++++++++++--------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/radicale/storage/multifilesystem/lock.py b/radicale/storage/multifilesystem/lock.py index 7e8143912..68a92792c 100644 --- a/radicale/storage/multifilesystem/lock.py +++ b/radicale/storage/multifilesystem/lock.py @@ -75,27 +75,35 @@ def acquire_lock(self, mode: str, user: str = "") -> Iterator[None]: preexec_fn = os.setpgrp command = self._hook % { "user": shlex.quote(user or "Anonymous")} - logger.debug("Running storage hook") - p = subprocess.Popen( - command, stdin=subprocess.DEVNULL, - stdout=subprocess.PIPE if debug else subprocess.DEVNULL, - stderr=subprocess.PIPE if debug else subprocess.DEVNULL, - shell=True, universal_newlines=True, preexec_fn=preexec_fn, - cwd=self._filesystem_folder, creationflags=creationflags) + logger.debug("Executing storage hook: '%s'" % command) + try: + p = subprocess.Popen( + command, stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE if debug else subprocess.DEVNULL, + stderr=subprocess.PIPE if debug else subprocess.DEVNULL, + shell=True, universal_newlines=True, preexec_fn=preexec_fn, + cwd=self._filesystem_folder, creationflags=creationflags) + except Exception as e: + logger.error("Execution of storage hook not successful on 'Popen': %s" % e) + return + logger.debug("Executing storage hook started 'Popen'") try: stdout_data, stderr_data = p.communicate() - except BaseException: # e.g. KeyboardInterrupt or SystemExit + except BaseException as e: # e.g. KeyboardInterrupt or SystemExit + logger.error("Execution of storage hook not successful on 'communicate': %s" % e) p.kill() p.wait() - raise + return finally: if sys.platform != "win32": # Kill remaining children identified by process group with contextlib.suppress(OSError): os.killpg(p.pid, signal.SIGKILL) + logger.debug("Executing storage hook finished") if stdout_data: - logger.debug("Captured stdout from hook:\n%s", stdout_data) + logger.debug("Captured stdout from storage hook:\n%s", stdout_data) if stderr_data: - logger.debug("Captured stderr from hook:\n%s", stderr_data) + logger.debug("Captured stderr from storage hook:\n%s", stderr_data) if p.returncode != 0: - raise subprocess.CalledProcessError(p.returncode, p.args) + logger.error("Execution of storage hook not successful: %s" % subprocess.CalledProcessError(p.returncode, p.args)) + return From 4781b48a1c3b2698fe6d2110ec4829224ad95c76 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sat, 23 Nov 2024 21:35:58 +0100 Subject: [PATCH 2/9] review storage hook git part --- DOCUMENTATION.md | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index d2d8a451a..e90e2d8ab 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -578,14 +578,30 @@ authentication over HTTP. This tutorial describes how to keep track of all changes to calendars and address books with **git** (or any other version control system). -The repository must be initialized by running `git init` in the file -system folder. Internal files of Radicale can be excluded by creating the -file `.gitignore` with the following content: +The repository must be initialized in the collection base directory +of the user running `radicale` daemon. -```gitignore +```bash +## assuming "radicale" user is starting "radicale" service +# change to user "radicale" +su -l -s /bin/bash radicale + +# change to collection base directory, assumed /var/lib/radicale/collections +cd /var/lib/radicale/collections + +# initialize git repository +git init + +# set user and e-mail, here minimum example +git config user.name "$USER" +git config user.email "$USER@$HOSTNAME" + +# define ignore of cache/lock/tmp files +cat <<'END' >.gitignore .Radicale.cache .Radicale.lock .Radicale.tmp-* +END ``` The configuration option `hook` in the `storage` section must be set to @@ -598,16 +614,16 @@ git add -A && (git diff --cached --quiet || git commit -m "Changes by \"%(user)s The command gets executed after every change to the storage and commits the changes into the **git** repository. -For the hook to not cause errors either **git** user details need to be set and match the owner of the collections directory or the repository needs to be marked as safe. - -When using the systemd unit file from the [Running as a service](#running-as-a-service) section this **cannot** be done via a `.gitconfig` file in the users home directory, as Radicale won't have read permissions! +Log of `git` can be investigated using -In `/var/lib/radicale/collections/.git` run: ```bash -git config user.name "radicale" -git config user.email "radicale@example.com" +su -l -s /bin/bash radicale +cd /var/lib/radicale/collections +git log ``` +In case of error messages in log, check SELinux status and related audit log and file/directory permissions. + ## Documentation ### Configuration From 69780dd0ee954162f5b4f2203fdfa84c80dd4c26 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 24 Nov 2024 15:53:53 +0100 Subject: [PATCH 3/9] adjust test: verify that a request succeeded if the hook still fails (anyhow no rollback implemented) --- radicale/tests/test_storage.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/radicale/tests/test_storage.py b/radicale/tests/test_storage.py index 9072a354b..b03dadf75 100644 --- a/radicale/tests/test_storage.py +++ b/radicale/tests/test_storage.py @@ -80,9 +80,9 @@ def test_hook_principal_collection_creation(self) -> None: self.propfind("/created_by_hook/") def test_hook_fail(self) -> None: - """Verify that a request fails if the hook fails.""" + """Verify that a request succeeded if the hook still fails (anyhow no rollback implemented).""" self.configure({"storage": {"hook": "exit 1"}}) - self.mkcalendar("/calendar.ics/", check=500) + self.mkcalendar("/calendar.ics/", check=200) def test_item_cache_rebuild(self) -> None: """Delete the item cache and verify that it is rebuild.""" From 5b64ef9fe7925980c41a48adceebdba9343ba3e8 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 24 Nov 2024 16:29:14 +0100 Subject: [PATCH 4/9] extend hook doc --- DOCUMENTATION.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index e90e2d8ab..44654d4ce 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -1017,6 +1017,11 @@ Command that is run after changes to storage. Take a look at the Default: +Supported placeholders: + - `%(user)`: logged-in user + +Command will be executed with base directory defined in `filesystem_folder` (see above) + ##### predefined_collections Create predefined user collections From 6fa15dae4a2b24222bad55e45a57911a20a6f9d7 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 24 Nov 2024 16:29:48 +0100 Subject: [PATCH 5/9] extend hook doc in config --- config | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/config b/config index 8415807b5..bf134fb79 100644 --- a/config +++ b/config @@ -145,7 +145,11 @@ #skip_broken_item = True # Command that is run after changes to storage -# Example: ([ -d .git ] || git init) && git add -A && (git diff --cached --quiet || git commit -m "Changes by \"%(user)s\"") +# Supported placeholders: +# %(user): logged-in user +# Command will be executed with base directory defined in filesystem_folder +# For "git" check DOCUMENTATION.md for bootstrap instructions +# Example: git add -A && (git diff --cached --quiet || git commit -m "Changes by \"%(user)s\"") #hook = # Create predefined user collections From 92e5032278490a74a0cdc54872e51fc1982a4140 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 24 Nov 2024 16:30:13 +0100 Subject: [PATCH 6/9] fix result code --- radicale/tests/test_storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radicale/tests/test_storage.py b/radicale/tests/test_storage.py index b03dadf75..22cc1f4cc 100644 --- a/radicale/tests/test_storage.py +++ b/radicale/tests/test_storage.py @@ -82,7 +82,7 @@ def test_hook_principal_collection_creation(self) -> None: def test_hook_fail(self) -> None: """Verify that a request succeeded if the hook still fails (anyhow no rollback implemented).""" self.configure({"storage": {"hook": "exit 1"}}) - self.mkcalendar("/calendar.ics/", check=200) + self.mkcalendar("/calendar.ics/", check=201) def test_item_cache_rebuild(self) -> None: """Delete the item cache and verify that it is rebuild.""" From 19f5aa0eddfc2102543c82b2a45099257e882428 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 24 Nov 2024 16:37:35 +0100 Subject: [PATCH 7/9] extend doc as partially suggested by https://github.com/Kozea/Radicale/pull/914 --- DOCUMENTATION.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 44654d4ce..cfc415581 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -586,7 +586,8 @@ of the user running `radicale` daemon. # change to user "radicale" su -l -s /bin/bash radicale -# change to collection base directory, assumed /var/lib/radicale/collections +# change to collection base directory defined in [storage] -> filesystem_folder +# assumed here /var/lib/radicale/collections cd /var/lib/radicale/collections # initialize git repository @@ -622,7 +623,14 @@ cd /var/lib/radicale/collections git log ``` -In case of error messages in log, check SELinux status and related audit log and file/directory permissions. +In case of problems, make sure you run radicale with ``--debug`` switch and +inspect the log output. For more information, please visit +[section on logging.]({{ site.baseurl }}/logging/) . + +Reason for problems can be + - SELinux status -> check related audit log + - problematic file/directory permissions + - command is not fond or cannot be executed or argument problem ## Documentation From 82064f823a0fd993eece83c7af06a2d21cf1665a Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 24 Nov 2024 16:39:40 +0100 Subject: [PATCH 8/9] add doc hint from https://github.com/Kozea/Radicale/pull/913 --- config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config b/config index bf134fb79..2b02d0b09 100644 --- a/config +++ b/config @@ -144,7 +144,7 @@ # Skip broken item instead of triggering an exception #skip_broken_item = True -# Command that is run after changes to storage +# Command that is run after changes to storage, default is emtpy # Supported placeholders: # %(user): logged-in user # Command will be executed with base directory defined in filesystem_folder From fbb6b1684a35860c51e6207d14bb55eeb10b66ae Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 24 Nov 2024 16:46:19 +0100 Subject: [PATCH 9/9] replace eol URL --- DOCUMENTATION.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index cfc415581..58ae17190 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -1557,7 +1557,7 @@ The ``radicale`` package offers the following modules. `ìtem` : Internal representation of address book and calendar entries. Based on - [VObject](https://eventable.github.io/vobject/). + [VObject](https://github.com/py-vobject/vobject/). `log` : The logger for Radicale based on the default Python logging module.