Allow the Btrfs hook to create and delete snapshots even when running as a non-root user (#1054).

NEWS

@@ -1,4 +1,7 @@
 2.0.13.dev0
+ * #1054: Allow the Btrfs hook to create and delete snapshots even when running
+   as a non-root user. See the documentation for more information:
+   https://torsion.org/borgmatic/reference/configuration/data-sources/btrfs/#non-root-user
  * #1179: Add a "file_list_format" option for setting the "list" action's output format and an
    "archive_list_format" option for setting the "repo-list" action's format.
  * #1192: Fix for over-aggressive deduplication of source directories that contain the borgmatic

borgmatic/hooks/data_source/btrfs.py

@@ -23,33 +23,21 @@ def use_streaming(hook_config, config):  # pragma: no cover
     return False
 
 
+BTRFS_SUBVOLUME_INODE_NUMBER = 256
+
+
 @functools.cache
-def path_is_a_subvolume(btrfs_command, path):
+def path_is_a_subvolume(path):
     '''
-    Given a btrfs command and a path, return whether the path is a Btrfs subvolume. Return False if
-    the btrfs command errors, which probably indicates there isn't a containing Btrfs subvolume for
-    the given path.
+    Given a path, return whether it is a Btrfs subvolume. Return False if the path doesn't exist.
 
-    As a performance optimization, multiple calls to this function with the same arguments are
-    cached.
+    As a performance optimization, multiple calls to this function with the same path are cached.
     '''
     try:
-        borgmatic.execute.execute_command(
-            (
-                *btrfs_command.split(' '),
-                'subvolume',
-                'show',
-                path,
-            ),
-            output_log_level=None,
-            close_fds=True,
-        )
-    # An error from the command (probably) indicates that the path is not actually a subvolume.
-    except subprocess.CalledProcessError:
+        return os.stat(path).st_ino == BTRFS_SUBVOLUME_INODE_NUMBER
+    except FileNotFoundError:
         return False
 
-    return True
-
 
 @functools.cache
 def get_subvolume_property(btrfs_command, subvolume_path, property_name):
@@ -99,7 +87,7 @@ def get_containing_subvolume_path(btrfs_command, path):
         path,
         *tuple(str(ancestor) for ancestor in pathlib.PurePath(path).parents),
     ):
-        if not path_is_a_subvolume(btrfs_command, candidate_path):
+        if not path_is_a_subvolume(candidate_path):
             continue
 
         try:
@@ -274,7 +262,6 @@ def snapshot_subvolume(btrfs_command, subvolume_path, snapshot_path):  # pragma:
             *btrfs_command.split(' '),
             'subvolume',
             'snapshot',
-            '-r',  # Read-only.
             subvolume_path,
             snapshot_path,
         ),

docs/reference/configuration/data-sources/btrfs.md

@@ -101,6 +101,19 @@ allow the Btrfs feature to work. See the comments in the sample systemd service
 file for details.
 
 
+## non-root user
+
+<span class="minilink minilink-addedin">New in version 2.0.13</span> If you'd
+like borgmatic to snapshot a Btrfs subvolume when running as a non-root user,
+make the following changes to the subvolume in question:
+
+1. `chown` the subvolume to be owned by your non-root user.
+2. Mount the subvolume with the `user_subvol_rm_allowed` mount option.
+
+These changes allow the non-root user to create and delete snapshots of the
+subvolume, which is necessary for the borgmatic Btrfs hook to work.
+
+
 ## Full configuration
 
 ```yaml

tests/unit/hooks/data_source/test_btrfs.py

@@ -5,36 +5,37 @@ from borgmatic.borg.pattern import Pattern, Pattern_source, Pattern_style, Patte
 from borgmatic.hooks.data_source import btrfs as module
 
 
-def test_path_is_a_subvolume_with_btrfs_success_call_returns_true():
+def test_path_is_a_subvolume_with_btrfs_inode_returns_true():
     module.path_is_a_subvolume.cache_clear()
-    flexmock(module.borgmatic.execute).should_receive(
-        'execute_command',
-    ).with_args(('btrfs', 'subvolume', 'show', '/mnt0'), output_log_level=None, close_fds=True)
+    flexmock(module.os).should_receive('stat').and_return(
+        flexmock(st_ino=module.BTRFS_SUBVOLUME_INODE_NUMBER)
+    )
 
-    assert module.path_is_a_subvolume('btrfs', '/mnt0') is True
+    assert module.path_is_a_subvolume('/mnt0') is True
 
 
-def test_path_is_a_subvolume_with_btrfs_error_returns_false():
+def test_path_is_a_subvolume_with_non_btrfs_inode_returns_false():
     module.path_is_a_subvolume.cache_clear()
-    flexmock(module.borgmatic.execute).should_receive(
-        'execute_command',
-    ).with_args(
-        ('btrfs', 'subvolume', 'show', '/mnt0'), output_log_level=None, close_fds=True
-    ).and_raise(
-        module.subprocess.CalledProcessError(1, 'btrfs'),
-    )
+    flexmock(module.os).should_receive('stat').and_return(flexmock(st_ino=12345))
 
-    assert module.path_is_a_subvolume('btrfs', '/mnt0') is False
+    assert module.path_is_a_subvolume('/mnt0') is False
 
 
 def test_path_is_a_subvolume_caches_result_after_first_call():
     module.path_is_a_subvolume.cache_clear()
-    flexmock(module.borgmatic.execute).should_receive(
-        'execute_command',
+    flexmock(module.os).should_receive('stat').and_return(
+        flexmock(st_ino=module.BTRFS_SUBVOLUME_INODE_NUMBER)
     ).once()
 
-    assert module.path_is_a_subvolume('btrfs', '/mnt0') is True
-    assert module.path_is_a_subvolume('btrfs', '/mnt0') is True
+    assert module.path_is_a_subvolume('/mnt0') is True
+    assert module.path_is_a_subvolume('/mnt0') is True
+
+
+def test_path_is_a_subvolume_with_missing_path_returns_false():
+    module.path_is_a_subvolume.cache_clear()
+    flexmock(module.os).should_receive('stat').and_raise(FileNotFoundError)
+
+    assert module.path_is_a_subvolume('/mnt0') is False
 
 
 def test_get_subvolume_property_with_invalid_btrfs_output_errors():
@@ -85,76 +86,66 @@ def test_get_subvolume_property_caches_result_after_first_call():
 
 
 def test_get_containing_subvolume_path_with_subvolume_self_returns_it():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
-    ).and_return(True)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/foo/bar').never()
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/foo').never()
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/').never()
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo/bar/baz').and_return(
+        True
+    )
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo/bar').never()
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo').never()
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/').never()
     flexmock(module).should_receive('get_subvolume_property').and_return(False)
 
     assert module.get_containing_subvolume_path('btrfs', '/foo/bar/baz') == '/foo/bar/baz'
 
 
 def test_get_containing_subvolume_path_with_subvolume_parent_returns_it():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
-    ).and_return(False)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar'
-    ).and_return(True)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/foo').never()
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/').never()
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo/bar/baz').and_return(
+        False
+    )
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo/bar').and_return(True)
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo').never()
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/').never()
     flexmock(module).should_receive('get_subvolume_property').and_return(False)
 
     assert module.get_containing_subvolume_path('btrfs', '/foo/bar/baz') == '/foo/bar'
 
 
 def test_get_containing_subvolume_path_with_subvolume_grandparent_returns_it():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
-    ).and_return(False)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar'
-    ).and_return(False)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/foo').and_return(
-        True
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo/bar/baz').and_return(
+        False
     )
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/').never()
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo/bar').and_return(False)
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo').and_return(True)
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/').never()
     flexmock(module).should_receive('get_subvolume_property').and_return(False)
 
     assert module.get_containing_subvolume_path('btrfs', '/foo/bar/baz') == '/foo'
 
 
 def test_get_containing_subvolume_path_without_subvolume_ancestor_returns_none():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
-    ).and_return(False)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar'
-    ).and_return(False)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/foo').and_return(
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo/bar/baz').and_return(
         False
     )
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/').and_return(False)
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo/bar').and_return(False)
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo').and_return(False)
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/').and_return(False)
     flexmock(module).should_receive('get_subvolume_property').and_return(False)
 
     assert module.get_containing_subvolume_path('btrfs', '/foo/bar/baz') is None
 
 
 def test_get_containing_subvolume_path_with_read_only_subvolume_returns_none():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
-    ).and_return(True)
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo/bar/baz').and_return(
+        True
+    )
     flexmock(module).should_receive('get_subvolume_property').and_return(True)
 
     assert module.get_containing_subvolume_path('btrfs', '/foo/bar/baz') is None
 
 
 def test_get_containing_subvolume_path_with_read_only_error_returns_none():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
-    ).and_return(True)
+    flexmock(module).should_receive('path_is_a_subvolume').with_args('/foo/bar/baz').and_return(
+        True
+    )
     flexmock(module).should_receive('get_subvolume_property').and_raise(
         module.subprocess.CalledProcessError(1, 'wtf')
     )