From eab60cce993b45c66796de45d7dbf192fb5aa653 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 12 Dec 2015 15:31:43 +0100 Subject: [PATCH] pass through some global options from client to server new: logging level options refactored: - umask option and remote_path - cleanly separated ssh command from borg command --- borg/archiver.py | 11 +++++----- borg/remote.py | 42 +++++++++++++++++++++--------------- borg/testsuite/repository.py | 26 ++++++++++++++++------ borg/testsuite/upgrader.py | 6 +++--- 4 files changed, 54 insertions(+), 31 deletions(-) diff --git a/borg/archiver.py b/borg/archiver.py index 65819b662..6e143e255 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -34,6 +34,9 @@ from .remote import RepositoryServer, RemoteRepository has_lchflags = hasattr(os, 'lchflags') +# default umask, overriden by --umask, defaults to read/write only for owner +UMASK_DEFAULT = 0o077 + class ToggleAction(argparse.Action): """argparse action to handle "toggle" flags easily @@ -58,7 +61,7 @@ class Archiver: def open_repository(self, args, create=False, exclusive=False, lock=True): location = args.location # note: 'location' must be always present in args if location.proto == 'ssh': - repository = RemoteRepository(location, create=create, lock_wait=self.lock_wait, lock=lock) + repository = RemoteRepository(location, create=create, lock_wait=self.lock_wait, lock=lock, args=args) else: repository = Repository(location.path, create=create, exclusive=exclusive, lock_wait=self.lock_wait, lock=lock) repository._location = location @@ -674,9 +677,9 @@ class Archiver: help='show/log the return code (rc)') common_parser.add_argument('--no-files-cache', dest='cache_files', action='store_false', help='do not load/update the file metadata cache used to detect unchanged files') - common_parser.add_argument('--umask', dest='umask', type=lambda s: int(s, 8), default=RemoteRepository.umask, metavar='M', + common_parser.add_argument('--umask', dest='umask', type=lambda s: int(s, 8), default=UMASK_DEFAULT, metavar='M', help='set umask to M (local and remote, default: %(default)04o)') - common_parser.add_argument('--remote-path', dest='remote_path', default=RemoteRepository.remote_path, metavar='PATH', + common_parser.add_argument('--remote-path', dest='remote_path', default='borg', metavar='PATH', help='set remote path to executable (default: "%(default)s")') parser = argparse.ArgumentParser(prog=prog, description='Borg - Deduplicated Backups') @@ -1188,8 +1191,6 @@ class Archiver: def run(self, args): os.umask(args.umask) # early, before opening files self.lock_wait = args.lock_wait - RemoteRepository.remote_path = args.remote_path - RemoteRepository.umask = args.umask setup_logging(level=args.log_level) # do not use loggers before this! check_extension_modules() keys_dir = get_keys_dir() diff --git a/borg/remote.py b/borg/remote.py index f724b80d7..1363d1707 100644 --- a/borg/remote.py +++ b/borg/remote.py @@ -117,15 +117,12 @@ class RepositoryServer: # pragma: no cover class RemoteRepository: extra_test_args = [] - remote_path = 'borg' - # default umask, overriden by --umask, defaults to read/write only for owner - umask = 0o077 class RPCError(Exception): def __init__(self, name): self.name = name - def __init__(self, location, create=False, lock_wait=None, lock=True): + def __init__(self, location, create=False, lock_wait=None, lock=True, args=None): self.location = location self.preload_ids = [] self.msgid = 0 @@ -135,15 +132,11 @@ class RemoteRepository: self.responses = {} self.unpacker = msgpack.Unpacker(use_list=False) self.p = None - # XXX: ideally, the testsuite would subclass Repository and - # override ssh_cmd() instead of this crude hack, although - # __testsuite__ is not a valid domain name so this is pretty - # safe. - if location.host == '__testsuite__': - args = [sys.executable, '-m', 'borg.archiver', 'serve' ] + self.extra_test_args - else: # pragma: no cover - args = self.ssh_cmd(location) - self.p = Popen(args, bufsize=0, stdin=PIPE, stdout=PIPE) + testing = location.host == '__testsuite__' + borg_cmd = self.borg_cmd(args, testing) + if not testing: + borg_cmd = self.ssh_cmd(location) + borg_cmd + self.p = Popen(borg_cmd, bufsize=0, stdin=PIPE, stdout=PIPE) self.stdin_fd = self.p.stdin.fileno() self.stdout_fd = self.p.stdout.fileno() fcntl.fcntl(self.stdin_fd, fcntl.F_SETFL, fcntl.fcntl(self.stdin_fd, fcntl.F_GETFL) | os.O_NONBLOCK) @@ -165,10 +158,27 @@ class RemoteRepository: def __repr__(self): return '<%s %s>' % (self.__class__.__name__, self.location.canonical_path()) - def umask_flag(self): - return ['--umask', '%03o' % self.umask] + def borg_cmd(self, args, testing): + """return a borg serve command line""" + # give some args/options to "borg serve" process as they were given to us + opts = [] + if args is not None: + opts.append('--umask=%03o' % args.umask) + if args.log_level == 'debug': + opts.append('--debug') + elif args.log_level == 'info': + opts.append('--info') + elif args.log_level == 'warning': + pass # is default + else: + raise ValueError('log level missing, fix this code') + if testing: + return [sys.executable, '-m', 'borg.archiver', 'serve' ] + opts + self.extra_test_args + else: # pragma: no cover + return [args.remote_path, 'serve'] + opts def ssh_cmd(self, location): + """return a ssh command line that can be prefixed to a borg command line""" args = shlex.split(os.environ.get('BORG_RSH', 'ssh')) if location.port: args += ['-p', str(location.port)] @@ -176,8 +186,6 @@ class RemoteRepository: args.append('%s@%s' % (location.user, location.host)) else: args.append('%s' % location.host) - # use local umask also for the remote process - args += [self.remote_path, 'serve'] + self.umask_flag() return args def call(self, cmd, *args, **kw): diff --git a/borg/testsuite/repository.py b/borg/testsuite/repository.py index 4094df407..7b2874c57 100644 --- a/borg/testsuite/repository.py +++ b/borg/testsuite/repository.py @@ -1,5 +1,6 @@ import os import shutil +import sys import tempfile from mock import patch @@ -326,13 +327,26 @@ class RemoteRepositoryTestCase(RepositoryTestCase): self.assert_raises(InvalidRPCMethod, lambda: self.repository.call('__init__', None)) def test_ssh_cmd(self): - assert self.repository.umask is not None - assert self.repository.ssh_cmd(Location('example.com:foo')) == ['ssh', 'example.com', 'borg', 'serve'] + self.repository.umask_flag() - assert self.repository.ssh_cmd(Location('ssh://example.com/foo')) == ['ssh', 'example.com', 'borg', 'serve'] + self.repository.umask_flag() - assert self.repository.ssh_cmd(Location('ssh://user@example.com/foo')) == ['ssh', 'user@example.com', 'borg', 'serve'] + self.repository.umask_flag() - assert self.repository.ssh_cmd(Location('ssh://user@example.com:1234/foo')) == ['ssh', '-p', '1234', 'user@example.com', 'borg', 'serve'] + self.repository.umask_flag() + assert self.repository.ssh_cmd(Location('example.com:foo')) == ['ssh', 'example.com'] + assert self.repository.ssh_cmd(Location('ssh://example.com/foo')) == ['ssh', 'example.com'] + assert self.repository.ssh_cmd(Location('ssh://user@example.com/foo')) == ['ssh', 'user@example.com'] + assert self.repository.ssh_cmd(Location('ssh://user@example.com:1234/foo')) == ['ssh', '-p', '1234', 'user@example.com'] os.environ['BORG_RSH'] = 'ssh --foo' - assert self.repository.ssh_cmd(Location('example.com:foo')) == ['ssh', '--foo', 'example.com', 'borg', 'serve'] + self.repository.umask_flag() + assert self.repository.ssh_cmd(Location('example.com:foo')) == ['ssh', '--foo', 'example.com'] + + def test_borg_cmd(self): + class MockArgs: + remote_path = 'borg' + log_level = 'warning' + umask = 0o077 + + assert self.repository.borg_cmd(None, testing=True) == [sys.executable, '-m', 'borg.archiver', 'serve' ] + args = MockArgs() + assert self.repository.borg_cmd(args, testing=False) == ['borg', 'serve', '--umask=077'] + args.log_level = 'info' + assert self.repository.borg_cmd(args, testing=False) == ['borg', 'serve', '--umask=077', '--info'] + args.remote_path = 'borg-0.28.2' + assert self.repository.borg_cmd(args, testing=False) == ['borg-0.28.2', 'serve', '--umask=077', '--info'] class RemoteRepositoryCheckTestCase(RepositoryCheckTestCase): diff --git a/borg/testsuite/upgrader.py b/borg/testsuite/upgrader.py index 3d0459126..9a1f823f9 100644 --- a/borg/testsuite/upgrader.py +++ b/borg/testsuite/upgrader.py @@ -12,7 +12,7 @@ except ImportError: from ..upgrader import AtticRepositoryUpgrader, AtticKeyfileKey from ..helpers import get_keys_dir from ..key import KeyfileKey -from ..remote import RemoteRepository +from ..archiver import UMASK_DEFAULT from ..repository import Repository @@ -169,7 +169,7 @@ def test_convert_all(tmpdir, attic_repo, attic_key_file, inplace): orig_inode = first_inode(attic_repo.path) repo = AtticRepositoryUpgrader(str(tmpdir), create=False) # replicate command dispatch, partly - os.umask(RemoteRepository.umask) + os.umask(UMASK_DEFAULT) backup = repo.upgrade(dryrun=False, inplace=inplace) if inplace: assert backup is None @@ -179,7 +179,7 @@ def test_convert_all(tmpdir, attic_repo, attic_key_file, inplace): assert first_inode(repo.path) != first_inode(backup) # i have seen cases where the copied tree has world-readable # permissions, which is wrong - assert stat_segment(backup).st_mode & 0o007 == 0 + assert stat_segment(backup).st_mode & UMASK_DEFAULT == 0 assert key_valid(attic_key_file.path) assert repo_valid(tmpdir)