Attachment 12918 Details for Bug 12577 – Patches for v4-5-test

[patch] Patches for v4-5-test

tmp45.diff.txt (text/plain), 9.48 KB, created by Stefan Metzmacher on 2017-02-13 07:41:12 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2017-02-13 07:41:12 UTC

Size: 9.48 KB

patch

obsolete

>From 80be9a04a0a9276c0d2dac25db702647294ffb4e Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Thu, 2 Feb 2017 16:27:35 +1300
>Subject: [PATCH 1/2] dbcheck: Do not regard old one-way-links as errors
>
>Samba does not maintain one way links when the target is deleted or renamed
>so do not fail dbcheck because of such links, but allow them to be updated.
>
>This matters because administrators and make test expect that normal Samba
>operation do NOT cause the database to become corrupt, and any error from
>dbcheck tends to trigger alarms (or test failures).
>
>If an object pointed at by a one way link is renamed or deleted in normal
>operations (such as intersiteTopologyGenerator pointing at a demoted DC),
>or make test, then this could trigger.
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12577
>(cherry picked from commit 35bfc62a31c9ad73449594ddd48f76f50e0abade)
>---
> python/samba/dbchecker.py      | 47 +++++++++++++++++++++++++++++++++++-------
> testprogs/blackbox/renamedc.sh |  6 ++++--
> 2 files changed, 43 insertions(+), 10 deletions(-)
>
>diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
>index 3fcfbc05..22819de 100644
>--- a/python/samba/dbchecker.py
>+++ b/python/samba/dbchecker.py
>@@ -59,6 +59,7 @@ class dbcheck(object):
>         self.fix_all_string_dn_component_mismatch = False
>         self.fix_all_GUID_dn_component_mismatch = False
>         self.fix_all_SID_dn_component_mismatch = False
>+        self.fix_all_old_dn_string_component_mismatch = False
>         self.fix_all_metadata = False
>         self.fix_time_metadata = False
>         self.fix_undead_linked_attributes = False
>@@ -574,6 +575,23 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>                           "Failed to fix %s on attribute %s" % (errstr, attrname)):
>             self.report("Fixed %s on attribute %s" % (errstr, attrname))
> 
>+    def err_dn_string_component_old(self, dn, attrname, val, dsdb_dn, correct_dn):
>+        """handle a DN string being incorrect"""
>+        self.report("NOTE: old (due to rename or delete) DN string component for %s in object %s - %s" % (attrname, dn, val))
>+        dsdb_dn.dn = correct_dn
>+
>+        if not self.confirm_all('Change DN to %s?' % str(dsdb_dn),
>+                                'fix_all_old_dn_string_component_mismatch'):
>+            self.report("Not fixing old string component")
>+            return
>+        m = ldb.Message()
>+        m.dn = dn
>+        m['old_value'] = ldb.MessageElement(val, ldb.FLAG_MOD_DELETE, attrname)
>+        m['new_value'] = ldb.MessageElement(str(dsdb_dn), ldb.FLAG_MOD_ADD, attrname)
>+        if self.do_modify(m, ["show_recycled:1"],
>+                          "Failed to fix old DN string on attribute %s" % (attrname)):
>+            self.report("Fixed old DN string on attribute %s" % (attrname))
>+
>     def err_dn_component_target_mismatch(self, dn, attrname, val, dsdb_dn, correct_dn, mismatch_type):
>         """handle a DN string being incorrect"""
>         self.report("ERROR: incorrect DN %s component for %s in object %s - %s" % (mismatch_type, attrname, dn, val))
>@@ -914,12 +932,16 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>                 if rmd_flags & 1:
>                     continue
> 
>-            # check the DN matches in string form
>-            if str(res[0].dn) != str(dsdb_dn.dn):
>-                error_count += 1
>-                self.err_dn_component_target_mismatch(obj.dn, attrname, val, dsdb_dn,
>-                                                      res[0].dn, "string")
>-                continue
>+            # assert the DN matches in string form, where a reverse
>+            # link exists, otherwise (below) offer to fix it as a non-error.
>+            # The string form is essentially only kept for forensics,
>+            # as we always re-resolve by GUID in normal operations.
>+            if reverse_link_name is not None:
>+                if str(res[0].dn) != str(dsdb_dn.dn):
>+                    error_count += 1
>+                    self.err_dn_component_target_mismatch(obj.dn, attrname, val, dsdb_dn,
>+                                                          res[0].dn, "string")
>+                    continue
> 
>             if res[0].dn.get_extended_component("GUID") != dsdb_dn.dn.get_extended_component("GUID"):
>                 error_count += 1
>@@ -933,9 +955,18 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
>                                                       res[0].dn, "SID")
>                 continue
> 
>+            # Now we have checked the GUID and SID, offer to fix old
>+            # DN strings as a non-error (for forward links with no
>+            # backlink).  Samba does not maintain this string
>+            # otherwise, so we don't increment error_count.
>+            if reverse_link_name is None:
>+                if str(res[0].dn) != str(dsdb_dn.dn):
>+                    self.err_dn_string_component_old(obj.dn, attrname, val, dsdb_dn,
>+                                                     res[0].dn)
>+                continue
> 
>-            # check the reverse_link is correct if there should be one
>-            if reverse_link_name is not None:
>+            else:
>+                # check the reverse_link is correct if there should be one
>                 match_count = 0
>                 if reverse_link_name in res[0]:
>                     for v in res[0][reverse_link_name]:
>diff --git a/testprogs/blackbox/renamedc.sh b/testprogs/blackbox/renamedc.sh
>index 3eb5817..7767d9d 100755
>--- a/testprogs/blackbox/renamedc.sh
>+++ b/testprogs/blackbox/renamedc.sh
>@@ -65,8 +65,10 @@ testrenamedc2() {
> }
> 
> dbcheck_fix() {
>+        # Unlike most calls to dbcheck --fix, this will not trigger an error, as
>+        # we do not flag an error count for this old DN string case.
> 	$BINDIR/samba-tool dbcheck --cross-ncs -s $PREFIX/renamedc_test/etc/smb.conf --fix \
>-		--quiet --yes fix_all_string_dn_component_mismatch \
>+		--quiet --yes fix_all_old_dn_string_component_mismatch \
> 		--attrs="fsmoRoleOwner interSiteTopologyGenerator msDS-NC-Replica-Locations"
> }
> 
>@@ -83,7 +85,7 @@ testit "confirmrenamedc_sAMAccountName" confirmrenamedc_sAMAccountName || failed
> testit "confirmrenamedc_dNSHostName" confirmrenamedc_dNSHostName || failed=`expr $failed + 1`
> testit "confirmrenamedc_rootdse_dnsHostName" confirmrenamedc_rootdse_dnsHostName || failed=`expr $failed + 1`
> testit "confirmrenamedc_rootdse_dsServiceName" confirmrenamedc_rootdse_dsServiceName || failed=`expr $failed + 1`
>-testit_expect_failure "dbcheck_fix" dbcheck_fix || failed=`expr $failed + 1`
>+testit "dbcheck_fix" dbcheck_fix || failed=`expr $failed + 1`
> testit "dbcheck" dbcheck || failed=`expr $failed + 1`
> testit "renamedc2" testrenamedc2 || failed=`expr $failed + 1`
> 
>-- 
>1.9.1
>
>
>From 8bb8cb196a622d1f1f57bf9d67ed0a89d960896e Mon Sep 17 00:00:00 2001
>From: Garming Sam <garming@catalyst.net.nz>
>Date: Wed, 8 Feb 2017 15:24:14 +1300
>Subject: [PATCH 2/2] dbcheck-links: Test that dbcheck against one-way links
> does not error
>
>Signed-off-by: Garming Sam <garming@catalyst.net.nz>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12577
>Pair-programmed-with: Bob Campbell <bobcampbell@catalyst.net.nz>
>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>
>Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
>Autobuild-Date(master): Mon Feb 13 07:33:08 CET 2017 on sn-devel-144
>
>(cherry picked from commit 44ee31675afd277d429cb246525741110f8fceec)
>---
> .../release-4-5-0-pre1/dangling-one-way-link.ldif         | 15 +++++++++++++++
> testprogs/blackbox/dbcheck-links.sh                       | 10 ++++++++++
> 2 files changed, 25 insertions(+)
> create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/dangling-one-way-link.ldif
>
>diff --git a/source4/selftest/provisions/release-4-5-0-pre1/dangling-one-way-link.ldif b/source4/selftest/provisions/release-4-5-0-pre1/dangling-one-way-link.ldif
>new file mode 100644
>index 0000000..c215c06
>--- /dev/null
>+++ b/source4/selftest/provisions/release-4-5-0-pre1/dangling-one-way-link.ldif
>@@ -0,0 +1,15 @@
>+dn: CN=secretary,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+changetype: add
>+objectclass: user
>+samaccountname: secretary
>+
>+dn: CN=dangling-one-way,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+changetype: add
>+objectclass: user
>+samaccountname: dangling-one-way
>+secretary: CN=secretary,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+
>+dn: CN=secretary,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp
>+changetype: modrdn
>+newrdn: cn=new-secretary
>+deleteoldrdn: 1
>diff --git a/testprogs/blackbox/dbcheck-links.sh b/testprogs/blackbox/dbcheck-links.sh
>index 11592f0..0799a50 100755
>--- a/testprogs/blackbox/dbcheck-links.sh
>+++ b/testprogs/blackbox/dbcheck-links.sh
>@@ -157,6 +157,14 @@ check_expected_after_objects() {
>     fi
> }
> 
>+dangling_one_way() {
>+    ldif=$release_dir/dangling-one-way-link.ldif
>+    TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb $ldif
>+    if [ "$?" != "0" ]; then
>+        return 1
>+    fi
>+}
>+
> if [ -d $release_dir ]; then
>     testit $RELEASE undump
>     testit "add_two_more_users" add_two_more_users
>@@ -169,6 +177,8 @@ if [ -d $release_dir ]; then
>     testit "check_expected_after_deleted_links" check_expected_after_deleted_links
>     testit "check_expected_after_links" check_expected_after_links
>     testit "check_expected_after_objects" check_expected_after_objects
>+    testit "dangling_one_way" dangling_one_way
>+    testit "dbcheck_clean" dbcheck_clean
> else
>     subunit_start_test $RELEASE
>     subunit_skip_test $RELEASE <<EOF
>-- 
>1.9.1
>

Flags:

abartlet: review+
metze: review? (garming)

Actions: View

Attachments on bug 12577: 12917 | 12918