[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Wed Mar 24 00:47:01 UTC 2021


The branch, master has been updated
       via  17283de8fd9 netcmd: Fix typos in offline domain backup test
       via  05b17c98598 netcmd: Avoid database corruption by opting not to create database files during an offline domain backup
       via  09995f780d1 netcmd: Determine which files are to be copied for an offline domain backup
       via  f52e6e5345b netcmd: Add test for an offline backup of nested directories
       via  542678908ac netcmd: Add test for an offline backup of a directory containing hardlinks
      from  447ad461588 man winbind: Remove untrue statement, you can run winbind without running nmbd.

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 17283de8fd967fbfe68f64b0cacb1e7aadf559fc
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Tue Mar 16 22:46:02 2021 +1300

    netcmd: Fix typos in offline domain backup test
    
    Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Wed Mar 24 00:46:31 UTC 2021 on sn-devel-184

commit 05b17c98598168b6d74a3f2dd0d9973e3bf407c1
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Tue Mar 16 22:20:21 2021 +1300

    netcmd: Avoid database corruption by opting not to create database files during an offline domain backup
    
    If backup dirs contain hardlinks, the backup process could previously
    attempt to open an LMDB database already opened during the backup,
    causing it to be recreated as a new TDB database. This commit ensures
    that new database files are not created during this operation, and that
    the main SamDB database is not modified.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14027
    
    Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz

commit 09995f780d1e75618ffc50b870b32e2375b63747
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Tue Mar 16 16:22:40 2021 +1300

    netcmd: Determine which files are to be copied for an offline domain backup
    
    The old behaviour attempted to check for and remove files with duplicate
    names, but did not do so due to a bug, and would have left undetermined
    which files were given priority when duplicate filenames were present.
    Now when hardlinks are present, only one instance of each file is
    chosen, with files in the private directory having priority. If one
    backup dir is nested inside another, the files contained in the nested
    directory are only added once. Additionally, the BIND DNS database is
    omitted from the backup.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14027
    
    Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz

commit f52e6e5345bd7cf35abe49eb67e9e4ea969493fd
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Thu Mar 18 10:52:52 2021 +1300

    netcmd: Add test for an offline backup of nested directories
    
    This test verifies that when performing an offline backup of a domain
    where one of the directories to be backed up is nested inside another,
    the contained files are only included once in the backup.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14027
    
    Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz

commit 542678908ac9869bca53b83db7c61d53d898c69c
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Tue Mar 16 16:13:05 2021 +1300

    netcmd: Add test for an offline backup of a directory containing hardlinks
    
    This test verifies that when performing an offline backup of a domain
    where the directories to be backed up contain hardlinks, only one
    instance of each file is backed up, and that files in the private
    directory take precedence.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14027
    
    Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz

-----------------------------------------------------------------------

Summary of changes:
 python/samba/netcmd/domain_backup.py        |  44 ++++++++---
 python/samba/tests/domain_backup_offline.py | 109 ++++++++++++++++++++++------
 2 files changed, 120 insertions(+), 33 deletions(-)


Changeset truncated at 500 lines:

diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
index 799fd0593e5..9eae6d3c3cf 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -313,7 +313,8 @@ class cmd_domain_backup_online(samba.netcmd.Command):
             shutil.rmtree(paths.sysvol)
 
             # Edit the downloaded sam.ldb to mark it as a backup
-            samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp)
+            samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp,
+                          flags=ldb.FLG_DONT_CREATE_DB)
             time_str = get_timestamp()
             add_backup_marker(samdb, "backupDate", time_str)
             add_backup_marker(samdb, "sidForRestore", new_sid)
@@ -537,7 +538,8 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
         # open a DB connection to the restored DB
         private_dir = os.path.join(targetdir, 'private')
         samdb_path = os.path.join(private_dir, 'sam.ldb')
-        samdb = SamDB(url=samdb_path, session_info=system_session(), lp=lp)
+        samdb = SamDB(url=samdb_path, session_info=system_session(), lp=lp,
+                      flags=ldb.FLG_DONT_CREATE_DB)
         backup_type = self.get_backup_type(samdb)
 
         if site is None:
@@ -645,7 +647,8 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
                                    host_ip, host_ip6, site)
 
         secrets_path = os.path.join(private_dir, 'secrets.ldb')
-        secrets_ldb = Ldb(secrets_path, session_info=system_session(), lp=lp)
+        secrets_ldb = Ldb(secrets_path, session_info=system_session(), lp=lp,
+                          flags=ldb.FLG_DONT_CREATE_DB)
         secretsdb_self_join(secrets_ldb, domain=ctx.domain_name,
                             realm=ctx.realm, dnsdomain=ctx.dnsdomain,
                             netbiosname=ctx.myname, domainsid=ctx.domsid,
@@ -937,7 +940,8 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
 
         # connect to the local DB (making sure we use the new/renamed config)
         lp.load(paths.smbconf)
-        samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp)
+        samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp,
+                      flags=ldb.FLG_DONT_CREATE_DB)
 
         # Edit the cloned sam.ldb to mark it as a backup
         time_str = get_timestamp()
@@ -1025,7 +1029,8 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
     # on the secrets.ldb file before backing up that file and secrets.tdb
     def backup_secrets(self, private_dir, lp, logger):
         secrets_path = os.path.join(private_dir, 'secrets')
-        secrets_obj = Ldb(secrets_path + '.ldb', lp=lp)
+        secrets_obj = Ldb(secrets_path + '.ldb', lp=lp,
+                          flags=ldb.FLG_DONT_CREATE_DB)
         logger.info('Starting transaction on ' + secrets_path)
         secrets_obj.transaction_start()
         self.offline_tdb_copy(secrets_path + '.ldb')
@@ -1050,7 +1055,7 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
         else:
             logger.info('Starting transaction on ' + sam_ldb_path)
             copy_function = self.offline_tdb_copy
-            sam_obj = Ldb(sam_ldb_path, lp=lp)
+            sam_obj = Ldb(sam_ldb_path, lp=lp, flags=ldb.FLG_DONT_CREATE_DB)
             sam_obj.transaction_start()
 
         logger.info('   backing up ' + sam_ldb_path)
@@ -1102,9 +1107,14 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
 
         check_targetdir(logger, targetdir)
 
-        samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp)
+        samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp,
+                      flags=ldb.FLG_RDONLY)
         sid = get_sid_for_restore(samdb, logger)
 
+        # Iterating over the directories in this specific order ensures that
+        # when the private directory contains hardlinks that are also contained
+        # in other directories to be backed up (such as in paths.binddns_dir),
+        # the hardlinks in the private directory take precedence.
         backup_dirs = [paths.private_dir, paths.state_dir,
                        os.path.dirname(paths.smbconf)]  # etc dir
         logger.info('running backup on dirs: {0}'.format(' '.join(backup_dirs)))
@@ -1117,22 +1127,31 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
                     continue
                 if working_dir.endswith('.sock') or '.sock/' in working_dir:
                     continue
+                # The BIND DNS database can be regenerated, so it doesn't need
+                # to be backed up.
+                if working_dir.startswith(os.path.join(paths.binddns_dir, 'dns')):
+                    continue
 
                 for filename in filenames:
-                    if filename in all_files:
+                    full_path = os.path.join(working_dir, filename)
+
+                    # Ignore files that have already been added. This prevents
+                    # duplicates if one backup dir is a subdirectory of another,
+                    # or if backup dirs contain hardlinks.
+                    if any(os.path.samefile(full_path, file) for file in all_files):
                         continue
 
                     # Assume existing backup files are from a previous backup.
                     # Delete and ignore.
                     if filename.endswith(self.backup_ext):
-                        os.remove(os.path.join(working_dir, filename))
+                        os.remove(full_path)
                         continue
 
                     # Sock files are autogenerated at runtime, ignore.
                     if filename.endswith('.sock'):
                         continue
 
-                    all_files.append(os.path.join(working_dir, filename))
+                    all_files.append(full_path)
 
         # Backup secrets, sam.ldb and their downstream files
         self.backup_secrets(paths.private_dir, lp, logger)
@@ -1144,7 +1163,8 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
         #          Writing to a .bak file only works because the DN being
         #          written to happens to be top level.
         samdb = SamDB(url=paths.samdb + self.backup_ext,
-                      session_info=system_session(), lp=lp)
+                      session_info=system_session(), lp=lp,
+                      flags=ldb.FLG_DONT_CREATE_DB)
         time_str = get_timestamp()
         add_backup_marker(samdb, "backupDate", time_str)
         add_backup_marker(samdb, "sidForRestore", sid)
@@ -1156,7 +1176,7 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
             if not os.path.exists(path + self.backup_ext):
                 if path.endswith('.ldb'):
                     logger.info('Starting transaction on solo db: ' + path)
-                    ldb_obj = Ldb(path, lp=lp)
+                    ldb_obj = Ldb(path, lp=lp, flags=ldb.FLG_DONT_CREATE_DB)
                     ldb_obj.transaction_start()
                     logger.info('   running tdbbackup on the same file')
                     self.offline_tdb_copy(path)
diff --git a/python/samba/tests/domain_backup_offline.py b/python/samba/tests/domain_backup_offline.py
index 8b7209ec24d..23300ac91b7 100644
--- a/python/samba/tests/domain_backup_offline.py
+++ b/python/samba/tests/domain_backup_offline.py
@@ -21,6 +21,7 @@ import shutil
 import tempfile
 from samba.tests import BlackboxTestCase
 from samba.netcmd import CommandError
+from samba.param import LoadParm
 
 # The backup tests require that a completely clean LoadParm object gets used
 # for the restore. Otherwise the same global LP gets re-used, and the LP
@@ -31,25 +32,86 @@ from samba.netcmd import CommandError
 # so that we never inadvertently use .runcmd() by accident.
 class DomainBackupOfflineCmp(BlackboxTestCase):
 
+    def test_domain_backup_offline_nested_tdb(self):
+        self.nested_testcase('tdb')
+
+    def test_domain_backup_offline_nested_mdb(self):
+        self.nested_testcase('mdb')
+
+    def nested_testcase(self, backend):
+        self.prov_dir = self.provision(backend)
+        self.extract_dir = None
+
+        src = os.path.join(self.prov_dir, "private")
+        dst = os.path.join(self.prov_dir, "state", "private")
+
+        # Move private directory inside state directory
+        shutil.move(src, dst)
+
+        smbconf = os.path.join(self.prov_dir, "etc", "smb.conf")
+
+        # Update the conf file
+        lp = LoadParm(filename_for_non_global_lp=smbconf)
+        lp.set("private dir", dst)
+        lp.dump(False, smbconf)
+
+        backup_file = self.backup(self.prov_dir)
+
+        # Ensure each file is only present once in the tar file
+        tf = tarfile.open(backup_file)
+        names = tf.getnames()
+        self.assertEqual(len(names), len(set(names)))
+
+    def test_domain_backup_offline_hard_link_tdb(self):
+        self.hard_link_testcase('tdb')
+
+    def test_domain_backup_offline_hard_link_mdb(self):
+        self.hard_link_testcase('mdb')
+
+    def hard_link_testcase(self, backend):
+        self.prov_dir = self.provision(backend)
+        self.extract_dir = None
+
+        # Create hard links in the private and state directories
+        os.link(os.path.join(self.prov_dir, "private", "krb5.conf"),
+                os.path.join(self.prov_dir, "state", "krb5.conf"))
+
+        backup_file = self.backup(self.prov_dir)
+
+        # Extract the backup
+        self.extract_dir = tempfile.mkdtemp(dir=self.tempdir)
+        tf = tarfile.open(backup_file)
+        tf.extractall(self.extract_dir)
+
+        # Ensure that the hard link in the private directory was backed up,
+        # while the one in the state directory was not.
+        self.assertTrue(os.path.exists(os.path.join(self.extract_dir,
+                                                    "private", "krb5.conf")))
+        self.assertFalse(os.path.exists(os.path.join(self.extract_dir,
+                                                    "statedir", "krb5.conf")))
+
     def test_domain_backup_offline_untar_tdb(self):
         self.untar_testcase('tdb')
 
-    def test_domain_backup_offline_untar_mbd(self):
+    def test_domain_backup_offline_untar_mdb(self):
         self.untar_testcase('mdb')
 
     def test_domain_backup_offline_restore_tdb(self):
         self.restore_testcase('tdb')
 
-    def test_domain_backup_offline_restore_mbd(self):
+    def test_domain_backup_offline_restore_mdb(self):
         self.restore_testcase('mdb')
 
     def restore_testcase(self, backend):
-        prov_dir, backup_file = self.provision_and_backup(backend)
+        self.prov_dir = self.provision(backend)
+        self.extract_dir = None
+        backup_file = self.backup(self.prov_dir)
 
-        extract_dir = tempfile.mkdtemp(dir=self.tempdir)
+        self.extract_dir = tempfile.mkdtemp(dir=self.tempdir)
         cmd = ("samba-tool domain backup restore --backup-file={f}"
                " --targetdir={d} "
-               "--newservername=NEWSERVER").format(f=backup_file, d=extract_dir)
+               "--newservername=NEWSERVER").format(f=backup_file,
+                                                   d=self.extract_dir)
         self.check_output(cmd)
 
         # attrs that are altered by the restore process
@@ -61,22 +123,18 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
                         "interSiteTopologyGenerator"]
         filter_arg = "--filter=" + ",".join(ignore_attrs)
         args = ["--two", filter_arg]
-        self.ldapcmp(prov_dir, extract_dir, args)
-
-        shutil.rmtree(prov_dir)
-        shutil.rmtree(extract_dir)
+        self.ldapcmp(self.prov_dir, self.extract_dir, args)
 
     def untar_testcase(self, backend):
-        prov_dir, backup_file = self.provision_and_backup(backend)
+        self.prov_dir = self.provision(backend)
+        self.extract_dir = None
+        backup_file = self.backup(self.prov_dir)
 
-        extract_dir = tempfile.mkdtemp(dir=self.tempdir)
+        self.extract_dir = tempfile.mkdtemp(dir=self.tempdir)
         tf = tarfile.open(backup_file)
-        tf.extractall(extract_dir)
-
-        self.ldapcmp(prov_dir, extract_dir)
+        tf.extractall(self.extract_dir)
 
-        shutil.rmtree(prov_dir)
-        shutil.rmtree(extract_dir)
+        self.ldapcmp(self.prov_dir, self.extract_dir)
 
     def ldapcmp(self, prov_dir, ex_dir, args=[]):
         sam_fn = os.path.join("private", "sam.ldb")
@@ -90,8 +148,8 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
             self.check_output(cmd)
 
     # Test the "samba-tool domain backup" command with ldapcmp
-    def provision_and_backup(self, backend):
-        prov_dir = tempfile.mkdtemp(dir=self.tempdir)
+    def provision(self, backend):
+        target = tempfile.mkdtemp(dir=self.tempdir)
 
         # Provision domain.  Use fake ACLs and store xattrs in tdbs so that
         # NTACL backup will work inside the testenv.
@@ -100,13 +158,16 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
         # circumstances, causing the ldapcmp to fail.
         prov_cmd = "samba-tool domain provision " +\
                    "--domain FOO --realm foo.example.com " +\
-                   "--targetdir {prov_dir} " +\
+                   "--targetdir {target} " +\
                    "--backend-store {backend} " +\
                    "--host-name OLDSERVER "+\
                    "--option=\"vfs objects=fake_acls xattr_tdb\""
-        prov_cmd = prov_cmd.format(prov_dir=prov_dir, backend=backend)
+        prov_cmd = prov_cmd.format(target=target, backend=backend)
         self.check_output(prov_cmd)
 
+        return target
+
+    def backup(self, prov_dir):
         # Run the backup and check we got one backup tar file
         cmd = ("samba-tool domain backup offline --targetdir={prov_dir} "
                "-s {prov_dir}/etc/smb.conf").format(prov_dir=prov_dir)
@@ -120,4 +181,10 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
                                " file but got {0}".format(len(tar_files)))
 
         backup_file = os.path.join(prov_dir, tar_files[0])
-        return prov_dir, backup_file
+        return backup_file
+
+    def tearDown(self):
+        # Remove temporary directories
+        shutil.rmtree(self.prov_dir)
+        if self.extract_dir:
+            shutil.rmtree(self.extract_dir)


-- 
Samba Shared Repository



More information about the samba-cvs mailing list