[PATCH] Add new DRS GetNCChanges tests
Tim Beale
timbeale at catalyst.net.nz
Tue Jun 20 01:08:52 UTC 2017
Hi,
The attached change-set adds new tests for exercising the server-side
GetNCChanges() code.
I wrote these tests to:
- Check whether we could get missing objects or linked attributes if
objects were modified during a replication cycle.
- Test the GET_TGT/GET_ANC behaviour of a Windows DC.
The branch is here:
http://git.catalyst.net.nz/gw?p=samba.git;a=shortlog;h=refs/heads/tim-tgt-tests
Thanks,
Tim
-------------- next part --------------
From 1d9535feabe8344e5f3edfabb0d50c84ab02a78a Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 6 Jun 2017 18:06:22 +1200
Subject: [PATCH 01/14] getncchanges.py: Add a new test for replication
This adds a new test to check that if objects are modified during a
replication, then those objects don't wind up missing from the
replication data.
Note that when this scenario occurs, samba returns the objects in a
different order to Windows. This test doesn't care what order the
replicated objects get returned in, so long as they all have been
received by the end of the test.
As part of this, I've refactored _check_replication() in drs_base.py so
it can be reused in new tests. In these cases, the objects are split up
over multiple different chunks. So asserting that the objects are returned
in a specific order makes it difficult to run the same test on both Samba
and Windows.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/drs_base.py | 70 +++++++++----
source4/torture/drs/python/getncchanges.py | 152 +++++++++++++++++++++++++++++
2 files changed, 203 insertions(+), 19 deletions(-)
create mode 100644 source4/torture/drs/python/getncchanges.py
diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index f07592d..35bb10d 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -176,17 +176,15 @@ class DrsBaseTestCase(SambaToolCmdTest):
id.dn = str(res[0].dn)
return id
- def _check_replication(self, expected_dns, replica_flags, expected_links=[],
- drs_error=drsuapi.DRSUAPI_EXOP_ERR_NONE, drs=None, drs_handle=None,
- highwatermark=None, uptodateness_vector=None,
- more_flags=0, more_data=False,
- dn_ordered=True, links_ordered=True,
- max_objects=133, exop=0,
- dest_dsa=drsuapi.DRSUAPI_DS_BIND_GUID_W2K3,
- source_dsa=None, invocation_id=None, nc_dn_str=None,
- nc_object_count=0, nc_linked_attributes_count=0):
+ def _get_replication(self, replica_flags,
+ drs_error=drsuapi.DRSUAPI_EXOP_ERR_NONE, drs=None, drs_handle=None,
+ highwatermark=None, uptodateness_vector=None,
+ more_flags=0, max_objects=133, exop=0,
+ dest_dsa=drsuapi.DRSUAPI_DS_BIND_GUID_W2K3,
+ source_dsa=None, invocation_id=None, nc_dn_str=None):
"""
- Makes sure that replication returns the specific error given.
+ Builds a DsGetNCChanges request based on the information provided
+ and returns the response received from the DC.
"""
if source_dsa is None:
source_dsa = self.ldb_dc1.get_ntds_GUID()
@@ -230,12 +228,51 @@ class DrsBaseTestCase(SambaToolCmdTest):
self.assertEqual(level, 6, "expected level 6 response!")
self.assertEqual(ctr.source_dsa_guid, misc.GUID(source_dsa))
self.assertEqual(ctr.source_dsa_invocation_id, misc.GUID(invocation_id))
- ctr6 = ctr
- self.assertEqual(ctr6.extended_ret, drs_error)
+ self.assertEqual(ctr.extended_ret, drs_error)
+
+ return ctr
+
+ def _check_replication(self, expected_dns, replica_flags, expected_links=[],
+ drs_error=drsuapi.DRSUAPI_EXOP_ERR_NONE, drs=None, drs_handle=None,
+ highwatermark=None, uptodateness_vector=None,
+ more_flags=0, more_data=False,
+ dn_ordered=True, links_ordered=True,
+ max_objects=133, exop=0,
+ dest_dsa=drsuapi.DRSUAPI_DS_BIND_GUID_W2K3,
+ source_dsa=None, invocation_id=None, nc_dn_str=None,
+ nc_object_count=0, nc_linked_attributes_count=0):
+ """
+ Makes sure that replication returns the specific error given.
+ """
+
+ # send a DsGetNCChanges to the DC
+ ctr6 = self._get_replication(replica_flags,
+ drs_error, drs, drs_handle,
+ highwatermark, uptodateness_vector,
+ more_flags, max_objects, exop, dest_dsa,
+ source_dsa, invocation_id, nc_dn_str)
+
+ # check the response is what we expect
self._check_ctr6(ctr6, expected_dns, expected_links,
- nc_object_count=nc_object_count)
+ nc_object_count=nc_object_count, more_data=more_data,
+ dn_ordered=dn_ordered)
return (ctr6.new_highwatermark, ctr6.uptodateness_vector)
+
+ def _get_ctr6_dn_list(self, ctr6):
+ """
+ Returns the DNs contained in a DsGetNCChanges response.
+ """
+ dn_list = []
+ next_object = ctr6.first_object
+ for i in range(0, ctr6.object_count):
+ dn_list.append(next_object.object.identifier.dn)
+ next_object = next_object.next_object
+ self.assertEqual(next_object, None)
+
+ return dn_list
+
+
def _check_ctr6(self, ctr6, expected_dns=[], expected_links=[],
dn_ordered=True, links_ordered=True,
more_data=False, nc_object_count=0,
@@ -250,12 +287,7 @@ class DrsBaseTestCase(SambaToolCmdTest):
self.assertEqual(ctr6.nc_linked_attributes_count, nc_linked_attributes_count)
self.assertEqual(ctr6.drs_error[0], drs_error)
- ctr6_dns = []
- next_object = ctr6.first_object
- for i in range(0, ctr6.object_count):
- ctr6_dns.append(next_object.object.identifier.dn)
- next_object = next_object.next_object
- self.assertEqual(next_object, None)
+ ctr6_dns = self._get_ctr6_dn_list(ctr6)
i = 0
for dn in expected_dns:
diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
new file mode 100644
index 0000000..b604239
--- /dev/null
+++ b/source4/torture/drs/python/getncchanges.py
@@ -0,0 +1,152 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+#
+# Tests various schema replication scenarios
+#
+# Copyright (C) Kamen Mazdrashki <kamenim at samba.org> 2011
+# Copyright (C) Andrew Bartlett <abartlet at samba.org> 2016
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+#
+# Usage:
+# export DC1=dc1_dns_name
+# export DC2=dc2_dns_name
+# export SUBUNITRUN=$samba4srcdir/scripting/bin/subunitrun
+# PYTHONPATH="$PYTHONPATH:$samba4srcdir/torture/drs/python" $SUBUNITRUN getnc_exop -U"$DOMAIN/$DC_USERNAME"%"$DC_PASSWORD"
+#
+
+import drs_base
+
+import samba.tests
+
+import ldb
+from ldb import SCOPE_BASE
+
+from samba.dcerpc import drsuapi, misc, drsblobs
+from samba.drs_utils import drs_DsBind
+from samba.ndr import ndr_unpack, ndr_pack
+
+class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
+ def setUp(self):
+ super(DrsReplicaSyncIntegrityTestCase, self).setUp()
+ self.base_dn = self.ldb_dc1.get_default_basedn()
+ self.ou = "OU=uptodateness_test,%s" % self.base_dn
+ self.ldb_dc1.add({
+ "dn": self.ou,
+ "objectclass": "organizationalUnit"})
+ (self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1)
+ (self.default_hwm, self.default_utdv) = self._get_highest_hwm_utdv(self.ldb_dc1)
+ self._debug = True
+
+ def tearDown(self):
+ super(DrsReplicaSyncIntegrityTestCase, self).tearDown()
+ # tidyup groups and users
+ try:
+ self.ldb_dc1.delete(self.ou, ["tree_delete:1"])
+ except ldb.LdbError as (enum, string):
+ if enum == ldb.ERR_NO_SUCH_OBJECT:
+ pass
+
+ def add_object(self, dn):
+ """Adds an OU object"""
+ self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalunit"})
+ res = self.ldb_dc1.search(base=dn, scope=SCOPE_BASE)
+ self.assertEquals(len(res), 1)
+
+ def modify_object(self, dn, attr, value):
+ """Modifies an object's USN by adding an attribute value to it"""
+ m = ldb.Message()
+ m.dn = ldb.Dn(self.ldb_dc1, dn)
+ m[attr] = ldb.MessageElement(value, ldb.FLAG_MOD_ADD, attr)
+ self.ldb_dc1.modify(m)
+
+ def create_object_range(self, start, end, prefix=""):
+ """
+ Creates a block of objects. Object names are numbered sequentially,
+ using the optional prefix supplied.
+ """
+ dn_list = []
+
+ # Create the parents first, then the children.
+ # This makes it easier to see in debug when GET_ANC takes effect
+ # because the parent/children become interleaved (by default,
+ # this approach means the objects are organized into blocks of
+ # parents and blocks of children together)
+ for x in range(start, end):
+ ou = "OU=test_ou_%s%d,%s" % (prefix, x, self.ou)
+ self.add_object(ou)
+ dn_list.append(ou)
+
+ return dn_list
+
+ def assert_expected_data(self, received_list, expected_list):
+ """
+ Asserts that we received all the DNs that we expected and
+ none are missing.
+ """
+
+ # Note that with GET_ANC Windows can end up sending the same parent
+ # object multiple times, so this might be noteworthy but doesn't
+ # warrant failing the test
+ if (len(received_list) != len(expected_list)):
+ print("Note: received %d objects but expected %d" %(len(received_list),
+ len(expected_list)))
+
+ # Check that we received every object that we were expecting
+ for dn in expected_list:
+ self.assertTrue(dn in received_list, "DN '%s' missing from replication." % dn)
+
+ def test_repl_integrity(self):
+ """
+ Modify the objects being replicated while the replication is still
+ in progress and check that no object loss occurs.
+ """
+
+ # The server behaviour differs between samba and Windows. Samba returns
+ # the objects in the original order (up to the pre-modify HWM). Windows
+ # incorporates the modified objects and returns them in the new order
+ # (i.e. modified objects last), up to the post-modify HWM. The Microsoft
+ # docs state the Windows behaviour is optional.
+
+ # Create a range of objects to replicate.
+ expected_dn_list = self.create_object_range(0, 400)
+ (orig_hwm, unused) = self._get_highest_hwm_utdv(self.ldb_dc1)
+
+ # We ask for the first page of 100 objects.
+ # For this test, we don't care what order we receive the objects in,
+ # so long as by the end we've received everything
+ rxd_dn_list = []
+ ctr6 = self._get_replication(drsuapi.DRSUAPI_DRS_WRIT_REP, max_objects=100)
+ rxd_dn_list = self._get_ctr6_dn_list(ctr6)
+
+ # Modify some of the second page of objects. This should bump the highwatermark
+ for x in range(100, 200):
+ self.modify_object(expected_dn_list[x], "displayName", "OU%d" % x)
+
+ (post_modify_hwm, unused) = self._get_highest_hwm_utdv(self.ldb_dc1)
+ self.assertTrue(post_modify_hwm.highest_usn > orig_hwm.highest_usn)
+
+ # Get the remaining blocks of data
+ while ctr6.more_data:
+ ctr6 = self._get_replication(drsuapi.DRSUAPI_DRS_WRIT_REP, max_objects=100,
+ highwatermark=ctr6.new_highwatermark,
+ uptodateness_vector=ctr6.uptodateness_vector)
+ rxd_dn_list += self._get_ctr6_dn_list(ctr6)
+
+ # Check we still receive all the objects we're expecting
+ self.assert_expected_data(rxd_dn_list, expected_dn_list)
+
+
--
2.7.4
From aaa02b1c11a45286000354e0fb607d0556c9368d Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 6 Jun 2017 18:21:40 +1200
Subject: [PATCH 02/14] getncchanges.py: Add GET_ANC test case
This test:
- creates blocks of parent/child objects
- modifies the parents, so the child gets received first in the
replication (which means the client has to use GET_ANC)
- checks that we always receive the parent before the child (if not, it
either retries with GET_ANC, or asserts if GET_ANC is already set)
- modifies the parent objects to change their USN while the
replication is in progress
- checks that all expected objects are received by the end of the
test
I've added a repl_get_next() function to help simulate a client's
behaviour - if it encounters an object it doesn't know the parent of,
then it retries with GET_ANC.
Also added some debug to drs_base.py that developers can turn on to make
it easier to see what objects we're actually receiving in the
responses.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/drs_base.py | 25 ++++
source4/torture/drs/python/getncchanges.py | 192 +++++++++++++++++++++++++++--
2 files changed, 209 insertions(+), 8 deletions(-)
diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index 35bb10d..391c81f 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -68,6 +68,9 @@ class DrsBaseTestCase(SambaToolCmdTest):
self.dnsname_dc1 = self.info_dc1["dnsHostName"][0]
self.dnsname_dc2 = self.info_dc2["dnsHostName"][0]
+ # for debugging the test code
+ self._debug = False
+
def tearDown(self):
super(DrsBaseTestCase, self).tearDown()
@@ -176,6 +179,27 @@ class DrsBaseTestCase(SambaToolCmdTest):
id.dn = str(res[0].dn)
return id
+ def _ctr6_debug(self, ctr6):
+ """
+ Displays basic info contained in a DsGetNCChanges response.
+ Having this debug code allows us to see the difference in behaviour
+ between Samba and Windows easier. Turn on the self._debug flag to see it.
+ """
+
+ if self._debug:
+ print("------------ recvd CTR6 -------------")
+
+ next_object = ctr6.first_object
+ for i in range(0, ctr6.object_count):
+ print("Obj %d: %s %s" %(i, next_object.object.identifier.dn[:22],
+ next_object.object.identifier.guid))
+ next_object = next_object.next_object
+
+ print("Linked Attributes: %d" % ctr6.linked_attributes_count)
+ print("HWM: %d" %(ctr6.new_highwatermark.highest_usn))
+ print("Tmp HWM: %d" %(ctr6.new_highwatermark.tmp_highest_usn))
+ print("More data: %d" %(ctr6.more_data))
+
def _get_replication(self, replica_flags,
drs_error=drsuapi.DRSUAPI_EXOP_ERR_NONE, drs=None, drs_handle=None,
highwatermark=None, uptodateness_vector=None,
@@ -224,6 +248,7 @@ class DrsBaseTestCase(SambaToolCmdTest):
uptodateness_vector_v1.cursors = cursors
req10.uptodateness_vector = uptodateness_vector_v1
(level, ctr) = drs.DsGetNCChanges(drs_handle, 10, req10)
+ self._ctr6_debug(ctr)
self.assertEqual(level, 6, "expected level 6 response!")
self.assertEqual(ctr.source_dsa_guid, misc.GUID(source_dsa))
diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
index b604239..6dcf174 100644
--- a/source4/torture/drs/python/getncchanges.py
+++ b/source4/torture/drs/python/getncchanges.py
@@ -49,7 +49,14 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
"objectclass": "organizationalUnit"})
(self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1)
(self.default_hwm, self.default_utdv) = self._get_highest_hwm_utdv(self.ldb_dc1)
- self._debug = True
+
+ # 100 is the minimum max_objects that Microsoft seems to honour
+ # (the max honoured is 400ish), so we use that in these tests
+ self.max_objects = 100
+ self.last_ctr = None
+
+ # store whether we used GET_ANC flags in the requests
+ self.used_get_anc = False
def tearDown(self):
super(DrsReplicaSyncIntegrityTestCase, self).tearDown()
@@ -73,13 +80,23 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
m[attr] = ldb.MessageElement(value, ldb.FLAG_MOD_ADD, attr)
self.ldb_dc1.modify(m)
- def create_object_range(self, start, end, prefix=""):
+ def create_object_range(self, start, end, prefix="",
+ children=None, parent_list=None):
"""
Creates a block of objects. Object names are numbered sequentially,
- using the optional prefix supplied.
+ using the optional prefix supplied. If the children parameter is
+ supplied it will create a parent-child hierarchy and return the
+ top-level parents separately.
"""
dn_list = []
+ # Use dummy/empty lists if we're not creating a parent/child hierarchy
+ if children is None:
+ children = []
+
+ if parent_list is None:
+ parent_list = []
+
# Create the parents first, then the children.
# This makes it easier to see in debug when GET_ANC takes effect
# because the parent/children become interleaved (by default,
@@ -90,6 +107,16 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
self.add_object(ou)
dn_list.append(ou)
+ # keep track of the top-level parents (if needed)
+ parent_list.append(ou)
+
+ # create the block of children (if needed)
+ for x in range(start, end):
+ for child in children:
+ ou = "OU=test_ou_child%s%d,%s" % (child, x, parent_list[x])
+ self.add_object(ou)
+ dn_list.append(ou)
+
return dn_list
def assert_expected_data(self, received_list, expected_list):
@@ -129,7 +156,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# For this test, we don't care what order we receive the objects in,
# so long as by the end we've received everything
rxd_dn_list = []
- ctr6 = self._get_replication(drsuapi.DRSUAPI_DRS_WRIT_REP, max_objects=100)
+ ctr6 = self.repl_get_next(rxd_dn_list)
rxd_dn_list = self._get_ctr6_dn_list(ctr6)
# Modify some of the second page of objects. This should bump the highwatermark
@@ -140,13 +167,162 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
self.assertTrue(post_modify_hwm.highest_usn > orig_hwm.highest_usn)
# Get the remaining blocks of data
- while ctr6.more_data:
- ctr6 = self._get_replication(drsuapi.DRSUAPI_DRS_WRIT_REP, max_objects=100,
- highwatermark=ctr6.new_highwatermark,
- uptodateness_vector=ctr6.uptodateness_vector)
+ while not self.replication_complete():
+ ctr6 = self.repl_get_next(rxd_dn_list)
rxd_dn_list += self._get_ctr6_dn_list(ctr6)
# Check we still receive all the objects we're expecting
self.assert_expected_data(rxd_dn_list, expected_dn_list)
+ def is_parent_known(self, dn, known_dn_list):
+ """
+ Returns True if the parent of the dn specified is in known_dn_list
+ """
+ # Remove the child portion from the name to get the parent's DN
+ name_substrings = dn.split(",")
+ del name_substrings[0]
+
+ parent_dn = ",".join(name_substrings)
+
+ # check either this object is a parent (it's parent is the top-level
+ # test object), or its parent has been seen previously
+ return parent_dn == self.ou or parent_dn in known_dn_list
+
+ def repl_get_next(self, initial_objects, get_anc=False):
+ """
+ Requests the next block of replication data. This tries to simulate
+ client behaviour - if we receive a replicated object that we don't know
+ the parent of, then re-request the block with the GET_ANC flag set.
+ """
+
+ # we're just trying to mimic regular client behaviour here, so just
+ # use the highwatermark in the last response we received
+ if self.last_ctr:
+ highwatermark = self.last_ctr.new_highwatermark
+ uptodateness_vector = self.last_ctr.uptodateness_vector
+ else:
+ # this is the initial replication, so we're starting from the start
+ highwatermark = None
+ uptodateness_vector = None
+
+ # we'll add new objects as we discover them, so take a copy to modify
+ known_objects = initial_objects[:]
+
+ # Ask for the next block of replication data
+ replica_flags = drsuapi.DRSUAPI_DRS_WRIT_REP
+
+ if get_anc:
+ replica_flags = drsuapi.DRSUAPI_DRS_WRIT_REP | drsuapi.DRSUAPI_DRS_GET_ANC
+ self.used_get_anc = True
+
+ ctr6 = self._get_replication(replica_flags,
+ max_objects=self.max_objects,
+ highwatermark=highwatermark,
+ uptodateness_vector=uptodateness_vector)
+
+ # check that we know the parent for every object received
+ rxd_dn_list = self._get_ctr6_dn_list(ctr6)
+
+ for i in range(0, len(rxd_dn_list)):
+
+ dn = rxd_dn_list[i]
+
+ if self.is_parent_known(dn, known_objects):
+
+ # the new DN is now known so add it to the list.
+ # It may be the parent of another child in this block
+ known_objects.append(dn)
+ else:
+ # If we've already set the GET_ANC flag then it should mean
+ # we receive the parents before the child
+ self.assertFalse(get_anc, "Unknown parent for object %s" % dn)
+
+ if self._debug:
+ print("Unknown parent for %s - try GET_ANC" % dn)
+
+ # try the same thing again with the GET_ANC flag set this time
+ ctr6 = self.repl_get_next (initial_objects,
+ get_anc=True)
+ break
+
+ # store the last successful result so we know what HWM to request next
+ self.last_ctr = ctr6
+
+ return ctr6
+
+ def replication_complete(self):
+ """Returns True if the current/last replication cycle is complete"""
+
+ if self.last_ctr is None or self.last_ctr.more_data:
+ return False
+ else:
+ return True
+
+ def test_repl_integrity_get_anc(self):
+ """
+ Modify the parent objects being replicated while the replication is still
+ in progress (using GET_ANC) and check that no object loss occurs.
+ """
+
+ # Note that GET_ANC behaviour varies between Windows and Samba.
+ # On Samba GET_ANC results in the replication restarting from the very
+ # beginning. After that, Samba remembers GET_ANC and also sends the
+ # parents in subsequent requests (regardless of whether GET_ANC is
+ # specified in the later request).
+ # Windows only sends the parents if GET_ANC was specified in the last
+ # request. It will also resend a parent, even if it's already sent the
+ # parent in a previous response (whereas Samba doesn't).
+
+ # Create a small block of 50 parents, each with 2 children (A and B)
+ # This is so that we receive some children in the first block, so we
+ # can resend with GET_ANC before we learn too many parents
+ parent_dn_list = []
+ expected_dn_list = self.create_object_range(0, 50, prefix="parent",
+ children=("A", "B"),
+ parent_list=parent_dn_list)
+
+ # create the remaining parents and children
+ expected_dn_list += self.create_object_range(50, 150, prefix="parent",
+ children=("A", "B"),
+ parent_list=parent_dn_list)
+
+ # We've now got objects in the following order:
+ # [50 parents][100 children][100 parents][200 children]
+
+ # Modify the first parent so that it's now ordered last by USN
+ # This means we set the GET_ANC flag pretty much straight away
+ # because we receive the first child before the first parent
+ self.modify_object(parent_dn_list[0], "displayName", "OU0")
+
+ # modify a later block of parents so they also get reordered
+ for x in range(50, 100):
+ self.modify_object(parent_dn_list[x], "displayName", "OU%d" % x)
+
+ # Get the first block of objects - this should resend the request with
+ # GET_ANC set because we won't know about the first child's parent.
+ # On samba GET_ANC essentially starts the sync from scratch again, so
+ # we get this over with early before we learn too many parents
+ rxd_dn_list = []
+ ctr6 = self.repl_get_next(rxd_dn_list)
+ rxd_dn_list = self._get_ctr6_dn_list(ctr6)
+
+ # modify the last chunk of parents. They should now have a USN higher
+ # than the highwater-mark for the replication cycle
+ for x in range(100, 150):
+ self.modify_object(parent_dn_list[x], "displayName", "OU%d" % x)
+
+ # Get the remaining blocks of data - this will resend the request with
+ # GET_ANC if it encounters an object it doesn't have the parent for.
+ while not self.replication_complete():
+ ctr6 = self.repl_get_next(rxd_dn_list)
+ rxd_dn_list += self._get_ctr6_dn_list(ctr6)
+
+ # The way the test objects have been created should force
+ # self.repl_get_next() to use the GET_ANC flag. If this doesn't
+ # actually happen, then the test isn't doing its job properly
+ self.assertTrue(self.used_get_anc,
+ "Test didn't use the GET_ANC flag as expected")
+
+ # Check we get all the objects we're expecting
+ self.assert_expected_data(rxd_dn_list, expected_dn_list)
--
2.7.4
From 4c98f568cba31d2976d759aebcccc67b7e67a975 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 7 Jun 2017 09:03:22 +1200
Subject: [PATCH 03/14] getncchanges.py: Add a basic GET_TGT test
This new test case does the following:
- Adds 2 sets of objects
- Links one set to the other
- Changes the order so the target object comes last in the
replication (which means the client has to use GET_TGT)
- Checks that when GET_TGT is used that we have received all target
objects we need to resolve the linked attibutes
- Checks that we expect to receive the linked attributes *before*
the last chunk is sent (Samba currently sends all the links at
the end, so this fails)
- Checks that we eventually receive all expected objects, and all
links we receive match what is expected
Note that this test demonstrates the GET_TGT behaviour on Windows,
but fails on the current Samba implementation. If we don't end up
implementing GET_TGT support on the Samba server, then I'll fix up the
test in a later patch.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/drs_base.py | 57 ++++++---
source4/torture/drs/python/getncchanges.py | 182 ++++++++++++++++++++++++++++-
2 files changed, 219 insertions(+), 20 deletions(-)
diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index 391c81f..a53058c 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -179,6 +179,38 @@ class DrsBaseTestCase(SambaToolCmdTest):
id.dn = str(res[0].dn)
return id
+ def _get_ctr6_links(self, ctr6):
+ """
+ Unpacks the linked attributes from a DsGetNCChanges response
+ and returns them as a list.
+ """
+ ctr6_links = []
+ for lidx in range(0, ctr6.linked_attributes_count):
+ l = ctr6.linked_attributes[lidx]
+ try:
+ target = ndr_unpack(drsuapi.DsReplicaObjectIdentifier3,
+ l.value.blob)
+ except:
+ target = ndr_unpack(drsuapi.DsReplicaObjectIdentifier3Binary,
+ l.value.blob)
+ al = AbstractLink(l.attid, l.flags,
+ l.identifier.guid,
+ target.guid, target.dn)
+ ctr6_links.append(al)
+
+ return ctr6_links
+
+ def _get_ctr6_object_guids(self, ctr6):
+ """Returns all the object GUIDs in a GetNCChanges response"""
+ guid_list = []
+
+ obj = ctr6.first_object
+ for i in range(0, ctr6.object_count):
+ guid_list.append(str(obj.object.identifier.guid))
+ obj = obj.next_object
+
+ return guid_list
+
def _ctr6_debug(self, ctr6):
"""
Displays basic info contained in a DsGetNCChanges response.
@@ -196,6 +228,11 @@ class DrsBaseTestCase(SambaToolCmdTest):
next_object = next_object.next_object
print("Linked Attributes: %d" % ctr6.linked_attributes_count)
+ ctr6_links = self._get_ctr6_links(ctr6)
+ for link in ctr6_links:
+ print("Link Tgt %s... <-- Src %s"
+ %(link.targetDN[:22], link.identifier))
+
print("HWM: %d" %(ctr6.new_highwatermark.highest_usn))
print("Tmp HWM: %d" %(ctr6.new_highwatermark.tmp_highest_usn))
print("More data: %d" %(ctr6.more_data))
@@ -325,21 +362,9 @@ class DrsBaseTestCase(SambaToolCmdTest):
else:
self.assertTrue(dn in ctr6_dns, "Couldn't find DN '%s' anywhere in ctr6 response." % dn)
- ctr6_links = []
+ # Extract the links from the response
+ ctr6_links = self._get_ctr6_links(ctr6)
expected_links.sort()
- lidx = 0
- for lidx in range(0, ctr6.linked_attributes_count):
- l = ctr6.linked_attributes[lidx]
- try:
- target = ndr_unpack(drsuapi.DsReplicaObjectIdentifier3,
- l.value.blob)
- except:
- target = ndr_unpack(drsuapi.DsReplicaObjectIdentifier3Binary,
- l.value.blob)
- al = AbstractLink(l.attid, l.flags,
- l.identifier.guid,
- target.guid)
- ctr6_links.append(al)
lidx = 0
for el in expected_links:
@@ -420,13 +445,15 @@ class DrsBaseTestCase(SambaToolCmdTest):
class AbstractLink:
- def __init__(self, attid, flags, identifier, targetGUID):
+ def __init__(self, attid, flags, identifier, targetGUID,
+ targetDN=""):
self.attid = attid
self.flags = flags
self.identifier = str(identifier)
self.selfGUID_blob = ndr_pack(identifier)
self.targetGUID = str(targetGUID)
self.targetGUID_blob = ndr_pack(targetGUID)
+ self.targetDN = targetDN
def __repr__(self):
return "AbstractLink(0x%08x, 0x%08x, %s, %s)" % (
diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
index 6dcf174..021a6ed 100644
--- a/source4/torture/drs/python/getncchanges.py
+++ b/source4/torture/drs/python/getncchanges.py
@@ -55,7 +55,8 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
self.max_objects = 100
self.last_ctr = None
- # store whether we used GET_ANC flags in the requests
+ # store whether we used GET_TGT/GET_ANC flags in the requests
+ self.used_get_tgt = False
self.used_get_anc = False
def tearDown(self):
@@ -188,11 +189,14 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# test object), or its parent has been seen previously
return parent_dn == self.ou or parent_dn in known_dn_list
- def repl_get_next(self, initial_objects, get_anc=False):
+ def repl_get_next(self, initial_objects, get_anc=False, get_tgt=False,
+ initial_guids=None, assert_links=False):
"""
Requests the next block of replication data. This tries to simulate
client behaviour - if we receive a replicated object that we don't know
the parent of, then re-request the block with the GET_ANC flag set.
+ If we don't know the target object for a linked attribute, then
+ re-request with GET_TGT.
"""
# we're just trying to mimic regular client behaviour here, so just
@@ -207,31 +211,45 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# we'll add new objects as we discover them, so take a copy to modify
known_objects = initial_objects[:]
+ known_guids = []
+
+ # Likewise with GUIDs - we only really need them for checking links
+ if initial_guids:
+ known_guids = initial_guids[:]
# Ask for the next block of replication data
replica_flags = drsuapi.DRSUAPI_DRS_WRIT_REP
+ more_flags = 0
if get_anc:
replica_flags = drsuapi.DRSUAPI_DRS_WRIT_REP | drsuapi.DRSUAPI_DRS_GET_ANC
self.used_get_anc = True
+ if get_tgt:
+ more_flags = drsuapi.DRSUAPI_DRS_GET_TGT
+ self.used_get_tgt = True
+
ctr6 = self._get_replication(replica_flags,
max_objects=self.max_objects,
highwatermark=highwatermark,
- uptodateness_vector=uptodateness_vector)
+ uptodateness_vector=uptodateness_vector,
+ more_flags=more_flags)
# check that we know the parent for every object received
rxd_dn_list = self._get_ctr6_dn_list(ctr6)
+ rxd_guid_list = self._get_ctr6_object_guids(ctr6)
for i in range(0, len(rxd_dn_list)):
dn = rxd_dn_list[i]
+ guid = rxd_guid_list[i]
if self.is_parent_known(dn, known_objects):
# the new DN is now known so add it to the list.
# It may be the parent of another child in this block
known_objects.append(dn)
+ known_guids.append(guid)
else:
# If we've already set the GET_ANC flag then it should mean
# we receive the parents before the child
@@ -241,10 +259,59 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
print("Unknown parent for %s - try GET_ANC" % dn)
# try the same thing again with the GET_ANC flag set this time
- ctr6 = self.repl_get_next (initial_objects,
- get_anc=True)
+ ctr6 = self.repl_get_next (initial_objects, get_anc=True,
+ get_tgt=get_tgt,
+ initial_guids=initial_guids,
+ assert_links=assert_links)
break
+ # check we know about references to any objects in the linked attritbutes
+ received_links = self._get_ctr6_links(ctr6)
+
+ # This is so that older versions of Samba fail - we want the links to be
+ # sent roughly with the objects, rather than getting all links at the end
+ if assert_links:
+ self.assertTrue(len(received_links) > 0,
+ "Links were expected in the GetNCChanges response")
+
+ for link in received_links:
+
+ # check the source object is known (Windows can actually send links
+ # where we don't know the source object yet). Samba shouldn't ever
+ # hit this case because it gets the links based on the source
+ if link.identifier not in known_guids:
+
+ # If we've already set the GET_ANC flag then it should mean
+ # this case doesn't happen
+ self.assertFalse(get_anc, "Unknown source object for GUID %s"
+ % link.identifier)
+
+ if self._debug:
+ print("Unknown source GUID %s - try GET_ANC" % link.identifier)
+
+ # try the same thing again with the GET_ANC flag set this time
+ return self.repl_get_next (initial_objects, get_anc=True,
+ get_tgt=get_tgt,
+ initial_guids=initial_guids,
+ assert_links=assert_links)
+
+ # check we know the target object
+ if link.targetGUID not in known_guids:
+
+ # If we've already set the GET_TGT flag then we should have
+ # already received any objects we need to know about
+ self.assertFalse(get_tgt, "Unknown linked target for object %s"
+ % link.targetDN)
+
+ if self._debug:
+ print("Unknown target for %s - try GET_TGT" % link.targetDN)
+
+ # try the same thing again with the GET_TGT flag set this time
+ return self.repl_get_next (initial_objects, get_anc=get_anc,
+ get_tgt=True,
+ initial_guids=initial_guids,
+ assert_links=assert_links)
+
# store the last successful result so we know what HWM to request next
self.last_ctr = ctr6
@@ -326,3 +393,108 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# Check we get all the objects we're expecting
self.assert_expected_data(rxd_dn_list, expected_dn_list)
+ def assert_expected_links(self, received_links, objects_with_links,
+ link_attr="managedBy"):
+ """
+ Asserts that a GetNCChanges response contains any expected links
+ for the objects it contains.
+ """
+
+ self.assertTrue(len(received_links) == len(objects_with_links),
+ "Received %d links but expected %d"
+ %(len(received_links), len(objects_with_links)))
+
+ for dn in objects_with_links:
+ self.assert_object_has_link(dn, link_attr, received_links)
+
+ def assert_object_has_link(self, dn, link_attr, received_links):
+ """
+ Queries the object in the DB and asserts there is a link in the
+ GetNCChanges response that matches.
+ """
+
+ # Look up the link attribute in the DB
+ # The extended_dn option will dump the GUID info for the link
+ # attribute (as a hex blob)
+ res = self.ldb_dc1.search(ldb.Dn(self.ldb_dc1, dn), attrs=[link_attr],
+ controls=['extended_dn:1:0'], scope=ldb.SCOPE_BASE)
+
+ # We didn't find the expected link attribute in the DB for the object.
+ # Something has gone wrong somewhere...
+ self.assertTrue(link_attr in res[0], "%s in DB doesn't have attribute %s"
+ %(dn, link_attr))
+
+ # find the received link in the list and assert that the target and
+ # source GUIDs match what's in the DB
+ found = False
+
+ for val in res[0][link_attr]:
+ # Work out the expected source and target GUIDs for the DB link
+ target_dn = ldb.Dn(self.ldb_dc1, val)
+ targetGUID_blob = target_dn.get_extended_component("GUID")
+ sourceGUID_blob = res[0].dn.get_extended_component("GUID")
+
+ for link in received_links:
+ if link.selfGUID_blob == sourceGUID_blob and \
+ link.targetGUID_blob == targetGUID_blob:
+
+ found = True
+
+ if self._debug:
+ print("Link %s --> %s" %(dn[:25], link.targetDN[:25]))
+ break
+
+ self.assertTrue(found, "Did not receive expected link for DN %s" % dn)
+
+
+ def test_repl_get_tgt(self):
+ """
+ Creates a scenario where we should receive the linked attribute before
+ we know about the target object, and therefore need to use GET_TGT.
+ Note: Samba currently avoids this problem by sending all its links last
+ """
+
+ # create the test objects
+ reportees = self.create_object_range(0, 100, prefix="reportee")
+ managers = self.create_object_range(0, 100, prefix="manager")
+ all_objects = managers + reportees
+ expected_links = reportees
+
+ # add a link attribute to each reportee object that points to the
+ # corresponding manager object as the target
+ for i in range(0, 100):
+ self.modify_object(reportees[i], "managedBy", managers[i])
+
+ # touch the managers (the link-target objects) again to make sure the
+ # reportees (link source objects) get returned first by the replication
+ for i in range(0, 100):
+ self.modify_object(managers[i], "displayName", "OU%d" % i)
+
+ rxd_dn_list = []
+ rxd_links = []
+ rxd_guids = []
+ links_expected = True
+
+ # Get all the replication data - this code should resend the requests
+ # with GET_TGT
+ while not self.replication_complete():
+
+ # get the next block of replication data (this sets GET_TGT if needed)
+ ctr6 = self.repl_get_next(rxd_dn_list, initial_guids=rxd_guids,
+ assert_links=links_expected)
+ rxd_dn_list += self._get_ctr6_dn_list(ctr6)
+ rxd_links += self._get_ctr6_links(ctr6)
+ rxd_guids += self._get_ctr6_object_guids(ctr6)
+ links_expected = len(rxd_links) < len(expected_links)
+
+ # The way the test objects have been created should force
+ # self.repl_get_next() to use the GET_TGT flag. If this doesn't
+ # actually happen, then the test isn't doing its job properly
+ self.assertTrue(self.used_get_tgt,
+ "Test didn't use the GET_TGT flag as expected")
+
+ # Check we get all the objects we're expecting
+ self.assert_expected_data(rxd_dn_list, all_objects)
+
+ # Check we received links for all the reportees
+ self.assert_expected_links(rxd_links, expected_links)
--
2.7.4
From a124e660cdfb090a423ba56becc163bd99b677f6 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 7 Jun 2017 09:10:33 +1200
Subject: [PATCH 04/14] getncchanges.py: Add a more complicated GET_TGT test
This adds the linked attributes in a more complicated chain. We add 300
objects, but the links for 100 objects will point to a linked chain of
200 objects. This was mainly to determine whether or not Windows follows
the target object (i.e. whether it sends all the links for the target
object as well). It turns out Windows maintains its own linked attribute
DB, so it sends the links based on USN.
This test passes on Windows, but fails on the current Samba
implementation. If we don't implement Samba GET_TGT server-side support
then this test will need to be fixed up.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/getncchanges.py | 63 ++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
index 021a6ed..2e58e60 100644
--- a/source4/torture/drs/python/getncchanges.py
+++ b/source4/torture/drs/python/getncchanges.py
@@ -498,3 +498,66 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# Check we received links for all the reportees
self.assert_expected_links(rxd_links, expected_links)
+
+ def test_repl_get_tgt_chain(self):
+ """
+ Tests the behaviour of GET_TGT with a more complicated scenario.
+ Here we create a chain of objects linked together, so if we follow
+ the link target, then we'd traverse ~200 objects each time.
+ """
+
+ # create the test objects
+ objectsA = self.create_object_range(0, 100, prefix="AAA")
+ objectsB = self.create_object_range(0, 100, prefix="BBB")
+ objectsC = self.create_object_range(0, 100, prefix="CCC")
+
+ # create a complex set of object links:
+ # A0-->B0-->C1-->B2-->C3-->B4-->and so on...
+ # Basically each object-A should link to a circular chain of 200 B/C
+ # objects. We create the links in separate chunks here, as it makes it
+ # clearer what happens with the USN (links on Windows have their own
+ # USN, so this approach means the A->B/B->C links aren't interleaved)
+ for i in range(0, 100):
+ self.modify_object(objectsA[i], "managedBy", objectsB[i])
+
+ for i in range(0, 100):
+ self.modify_object(objectsB[i], "managedBy", objectsC[(i + 1) % 100])
+
+ for i in range(0, 100):
+ self.modify_object(objectsC[i], "managedBy", objectsB[(i + 1) % 100])
+
+ all_objects = objectsA + objectsB + objectsC
+ expected_links = all_objects
+
+ # the default order the objects now get returned in should be:
+ # [A0-A99][B0-B99][C0-C99]
+
+ rxd_dn_list = []
+ rxd_links = []
+ rxd_guids = []
+ links_expected = True
+
+ # Get all the replication data - this code should resend the requests
+ # with GET_TGT
+ while not self.replication_complete():
+
+ # get the next block of replication data (this sets GET_TGT if needed)
+ ctr6 = self.repl_get_next(rxd_dn_list, initial_guids=rxd_guids,
+ assert_links=links_expected)
+ rxd_dn_list += self._get_ctr6_dn_list(ctr6)
+ rxd_links += self._get_ctr6_links(ctr6)
+ rxd_guids += self._get_ctr6_object_guids(ctr6)
+ links_expected = len(rxd_links) < len(expected_links)
+
+ # The way the test objects have been created should force
+ # self.repl_get_next() to use the GET_TGT flag. If this doesn't
+ # actually happen, then the test isn't doing its job properly
+ self.assertTrue(self.used_get_tgt,
+ "Test didn't use the GET_TGT flag as expected")
+
+ # Check we get all the objects we're expecting
+ self.assert_expected_data(rxd_dn_list, all_objects)
+
+ # Check we received links for all the reportees
+ self.assert_expected_links(rxd_links, expected_links)
+
--
2.7.4
From 6d75ef977eae75338dea9cc0f205e1cee974a8d8 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 23 May 2017 14:37:56 +1200
Subject: [PATCH 05/14] getnc_exop.py: Fix GET_TGT behaviour in DRS tests
The existing code never passed the more_flags parameter into the
actual getNCChanges request, i.e. _getnc_req10(). This meant the
existing GET_TGT tests effectively did nothing.
Passing the flag through properly means we have to now change the tests
as the DNs returned by Windows now include any target objects in the
linked attributes. These changes mean the test passes once more on
Windows, but it will fail on Samba now (because Samba doesn't support
GET_TGT yet).
Also added comments to the tests to help explain what they are actually
doing.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/drs_base.py | 3 ++-
source4/torture/drs/python/getnc_exop.py | 16 ++++++++++++++--
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index a53058c..f5a4a21 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -270,7 +270,8 @@ class DrsBaseTestCase(SambaToolCmdTest):
nc_dn_str=nc_dn_str,
exop=exop,
max_objects=max_objects,
- replica_flags=replica_flags)
+ replica_flags=replica_flags,
+ more_flags=more_flags)
req10.highwatermark = highwatermark
if uptodateness_vector is not None:
uptodateness_vector_v1 = drsuapi.DsReplicaCursorCtrEx()
diff --git a/source4/torture/drs/python/getnc_exop.py b/source4/torture/drs/python/getnc_exop.py
index 26786be..5c4fa17 100644
--- a/source4/torture/drs/python/getnc_exop.py
+++ b/source4/torture/drs/python/getnc_exop.py
@@ -410,11 +410,16 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
drsuapi.DRSUAPI_DRS_GET_ANC,
expected_links=[dc3_managedBy_ou1])
- self._check_replication([dc3],
+ # GET_TGT seems to override DRS_CRITICAL_ONLY and also returns any
+ # object(s) that relate to the linked attributes (similar to GET_ANC)
+ self._check_replication([ou1, dc3],
drsuapi.DRSUAPI_DRS_CRITICAL_ONLY,
more_flags=drsuapi.DRSUAPI_DRS_GET_TGT,
expected_links=[dc3_managedBy_ou1])
+ # Change DC3's managedBy to OU2 instead of OU1
+ # Note that the OU1 managedBy linked attribute will still exist as
+ # a tombstone object (and so will be returned in the replication still)
m = ldb.Message()
m.dn = ldb.Dn(self.ldb_dc1, dc3)
m["managedBy"] = ldb.MessageElement(ou2, ldb.FLAG_MOD_REPLACE, "managedBy")
@@ -443,11 +448,16 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
drsuapi.DRSUAPI_DRS_GET_ANC,
expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2])
- self._check_replication([dc3],
+ # GET_TGT will also return any DNs referenced by the linked attributes
+ # (including the Tombstone attribute)
+ self._check_replication([ou1, ou2, dc3],
drsuapi.DRSUAPI_DRS_CRITICAL_ONLY,
more_flags=drsuapi.DRSUAPI_DRS_GET_TGT,
expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2])
+ # Use the highwater-mark prior to changing ManagedBy - this should
+ # only return the old/Tombstone and new linked attributes (we already
+ # know all the DNs)
self._check_replication([],
drsuapi.DRSUAPI_DRS_WRIT_REP,
expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2],
@@ -476,6 +486,8 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2],
highwatermark=hwm7)
+ # Repeat the above set of tests using the uptodateness_vector
+ # instead of the highwater-mark
self._check_replication([],
drsuapi.DRSUAPI_DRS_WRIT_REP,
expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2],
--
2.7.4
From 505a5acd8db6ad2080f2647436bbeaab6ae39136 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 8 Jun 2017 14:27:51 +1200
Subject: [PATCH 06/14] getnc_exop.py: Disable the GET_TGT test cases for now
These tests now fail on Samba, but work on Windows. Add a flag so
they're not run by default. When we eventually add GET_TGT support
to Samba we can just revert this patch.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/getnc_exop.py | 44 +++++++++++++++++++-------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/source4/torture/drs/python/getnc_exop.py b/source4/torture/drs/python/getnc_exop.py
index 5c4fa17..3b8f9ed 100644
--- a/source4/torture/drs/python/getnc_exop.py
+++ b/source4/torture/drs/python/getnc_exop.py
@@ -410,12 +410,17 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
drsuapi.DRSUAPI_DRS_GET_ANC,
expected_links=[dc3_managedBy_ou1])
+ # Samba doesn't currently support GET_TGT, so these tests will fail.
+ # They run successfully against Windows though
+ support_get_tgt = False
+
# GET_TGT seems to override DRS_CRITICAL_ONLY and also returns any
# object(s) that relate to the linked attributes (similar to GET_ANC)
- self._check_replication([ou1, dc3],
- drsuapi.DRSUAPI_DRS_CRITICAL_ONLY,
- more_flags=drsuapi.DRSUAPI_DRS_GET_TGT,
- expected_links=[dc3_managedBy_ou1])
+ if support_get_tgt:
+ self._check_replication([ou1, dc3],
+ drsuapi.DRSUAPI_DRS_CRITICAL_ONLY,
+ more_flags=drsuapi.DRSUAPI_DRS_GET_TGT,
+ expected_links=[dc3_managedBy_ou1])
# Change DC3's managedBy to OU2 instead of OU1
# Note that the OU1 managedBy linked attribute will still exist as
@@ -450,10 +455,11 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
# GET_TGT will also return any DNs referenced by the linked attributes
# (including the Tombstone attribute)
- self._check_replication([ou1, ou2, dc3],
- drsuapi.DRSUAPI_DRS_CRITICAL_ONLY,
- more_flags=drsuapi.DRSUAPI_DRS_GET_TGT,
- expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2])
+ if support_get_tgt:
+ self._check_replication([ou1, ou2, dc3],
+ drsuapi.DRSUAPI_DRS_CRITICAL_ONLY,
+ more_flags=drsuapi.DRSUAPI_DRS_GET_TGT,
+ expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2])
# Use the highwater-mark prior to changing ManagedBy - this should
# only return the old/Tombstone and new linked attributes (we already
@@ -480,11 +486,12 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2],
highwatermark=hwm7)
- self._check_replication([],
- drsuapi.DRSUAPI_DRS_CRITICAL_ONLY,
- more_flags=drsuapi.DRSUAPI_DRS_GET_TGT,
- expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2],
- highwatermark=hwm7)
+ if support_get_tgt:
+ self._check_replication([],
+ drsuapi.DRSUAPI_DRS_CRITICAL_ONLY,
+ more_flags=drsuapi.DRSUAPI_DRS_GET_TGT,
+ expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2],
+ highwatermark=hwm7)
# Repeat the above set of tests using the uptodateness_vector
# instead of the highwater-mark
@@ -510,11 +517,12 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2],
uptodateness_vector=utdv7)
- self._check_replication([],
- drsuapi.DRSUAPI_DRS_CRITICAL_ONLY,
- more_flags=drsuapi.DRSUAPI_DRS_GET_TGT,
- expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2],
- uptodateness_vector=utdv7)
+ if support_get_tgt:
+ self._check_replication([],
+ drsuapi.DRSUAPI_DRS_CRITICAL_ONLY,
+ more_flags=drsuapi.DRSUAPI_DRS_GET_TGT,
+ expected_links=[dc3_managedBy_ou1,dc3_managedBy_ou2],
+ uptodateness_vector=utdv7)
def test_FSMONotOwner(self):
"""Test role transfer with against DC not owner of the role"""
--
2.7.4
From 6166063a4b1531ec5de914e14dc1ddfb2495152e Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 7 Jun 2017 11:13:52 +1200
Subject: [PATCH 07/14] getnc_exop.py: Fix typo in function name
This drove me crazy when I tried to search for it.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/drs_base.py | 2 +-
source4/torture/drs/python/getnc_exop.py | 14 +++++++-------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index f5a4a21..49971a9 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -169,7 +169,7 @@ class DrsBaseTestCase(SambaToolCmdTest):
utdv.cursors = cursors
return (hwm, utdv)
- def _get_indentifier(self, ldb_conn, dn):
+ def _get_identifier(self, ldb_conn, dn):
res = ldb_conn.search(dn, scope=ldb.SCOPE_BASE,
attrs=["objectGUID", "objectSid"])
id = drsuapi.DsReplicaObjectIdentifier()
diff --git a/source4/torture/drs/python/getnc_exop.py b/source4/torture/drs/python/getnc_exop.py
index 3b8f9ed..cf9396d 100644
--- a/source4/torture/drs/python/getnc_exop.py
+++ b/source4/torture/drs/python/getnc_exop.py
@@ -146,20 +146,20 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
"dn": ou1,
"objectclass": "organizationalUnit"
})
- ou1_id = self._get_indentifier(self.ldb_dc1, ou1)
+ ou1_id = self._get_identifier(self.ldb_dc1, ou1)
ou2 = "OU=get_anc2,%s" % ou1
self.ldb_dc1.add({
"dn": ou2,
"objectclass": "organizationalUnit"
})
- ou2_id = self._get_indentifier(self.ldb_dc1, ou2)
+ ou2_id = self._get_identifier(self.ldb_dc1, ou2)
dc3 = "CN=test_anc_dc_%u,%s" % (random.randint(0, 4294967295), ou2)
self.ldb_dc1.add({
"dn": dc3,
"objectclass": "computer",
"userAccountControl": "%d" % (samba.dsdb.UF_ACCOUNTDISABLE | samba.dsdb.UF_SERVER_TRUST_ACCOUNT)
})
- dc3_id = self._get_indentifier(self.ldb_dc1, dc3)
+ dc3_id = self._get_identifier(self.ldb_dc1, dc3)
req8 = self._exop_req8(dest_dsa=None,
invocation_id=self.ldb_dc1.get_invocation_id(),
@@ -201,20 +201,20 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
"dn": ou1,
"objectclass": "organizationalUnit"
})
- ou1_id = self._get_indentifier(self.ldb_dc1, ou1)
+ ou1_id = self._get_identifier(self.ldb_dc1, ou1)
ou2 = "OU=get_anc2,%s" % ou1
self.ldb_dc1.add({
"dn": ou2,
"objectclass": "organizationalUnit"
})
- ou2_id = self._get_indentifier(self.ldb_dc1, ou2)
+ ou2_id = self._get_identifier(self.ldb_dc1, ou2)
dc3 = "CN=test_anc_dc_%u,%s" % (random.randint(0, 4294967295), ou2)
self.ldb_dc1.add({
"dn": dc3,
"objectclass": "computer",
"userAccountControl": "%d" % (samba.dsdb.UF_ACCOUNTDISABLE | samba.dsdb.UF_SERVER_TRUST_ACCOUNT)
})
- dc3_id = self._get_indentifier(self.ldb_dc1, dc3)
+ dc3_id = self._get_identifier(self.ldb_dc1, dc3)
(hwm1, utdv1) = self._check_replication([ou1,ou2,dc3],
drsuapi.DRSUAPI_DRS_WRIT_REP)
@@ -325,7 +325,7 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
"dn": cn3,
"objectclass": "container",
})
- cn3_id = self._get_indentifier(self.ldb_dc1, cn3)
+ cn3_id = self._get_identifier(self.ldb_dc1, cn3)
(hwm5, utdv5) = self._check_replication([dc3,ou1,ou2,self.ou,cn3],
drsuapi.DRSUAPI_DRS_WRIT_REP)
--
2.7.4
From 6b1e2e95d0a2727063aafe8c2b3c2c802ae5b4e1 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 7 Jun 2017 12:22:38 +1200
Subject: [PATCH 08/14] getncchanges.py: Add test for GET_ANC and linked
attributes
Add a basic test that when we use GET_ANC and the parents have linked
attributes, then we receive all the expected links and all the expected
objects by the end of the test.
Note that this shouldn't exercise GET_TGT at all.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/getncchanges.py | 44 ++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
index 2e58e60..3eb42ed 100644
--- a/source4/torture/drs/python/getncchanges.py
+++ b/source4/torture/drs/python/getncchanges.py
@@ -561,3 +561,47 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# Check we received links for all the reportees
self.assert_expected_links(rxd_links, expected_links)
+ def test_repl_get_anc_link_attr(self):
+ """
+ A basic GET_ANC test where the parents have linked attributes
+ """
+
+ # Create a block of 100 parents and 100 children
+ parent_dn_list = []
+ expected_dn_list = self.create_object_range(0, 100, prefix="parent",
+ children=("A"),
+ parent_list=parent_dn_list)
+
+ # Add links from the parents to the children
+ for x in range(0, 100):
+ self.modify_object(parent_dn_list[x], "managedBy", expected_dn_list[x + 100])
+
+ # add some filler objects at the end. This allows us to easily see
+ # which chunk the links get sent in
+ expected_dn_list += self.create_object_range(0, 100, prefix="filler")
+
+ # We've now got objects in the following order:
+ # [100 x children][100 x parents][100 x filler]
+
+ # Get the replication data - because the block of children come first,
+ # this should retry the request with GET_ANC
+ rxd_dn_list = []
+ rxd_links = []
+ rxd_guids = []
+
+ while not self.replication_complete():
+ ctr6 = self.repl_get_next(rxd_dn_list, initial_guids=rxd_guids)
+ rxd_dn_list += self._get_ctr6_dn_list(ctr6)
+ rxd_links += self._get_ctr6_links(ctr6)
+ rxd_guids += self._get_ctr6_object_guids(ctr6)
+
+ self.assertTrue(self.used_get_anc,
+ "Test didn't use the GET_ANC flag as expected")
+
+ # Check we get all the objects we're expecting
+ self.assert_expected_data(rxd_dn_list, expected_dn_list)
+
+ # Check we received links for all the parents
+ self.assert_expected_links(rxd_links, parent_dn_list)
+
+
--
2.7.4
From 2678911b56656ed5a5647613a23c7015b5b4dc6e Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 8 Jun 2017 18:04:30 +1200
Subject: [PATCH 09/14] getncchanges.py: Refactor tests to minimize repeated
code
Basically every test wants to keep track of the object DNs, GUIDs, and
links (if any) that were received in the test. We already use a
centralized function to get each replication chunk, so store the
received objects/links each time we successfully receive a GETNCChanges
response.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/getncchanges.py | 110 +++++++++++------------------
1 file changed, 41 insertions(+), 69 deletions(-)
diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
index 3eb42ed..3dfb51c 100644
--- a/source4/torture/drs/python/getncchanges.py
+++ b/source4/torture/drs/python/getncchanges.py
@@ -50,6 +50,10 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
(self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1)
(self.default_hwm, self.default_utdv) = self._get_highest_hwm_utdv(self.ldb_dc1)
+ self.rxd_dn_list = []
+ self.rxd_links = []
+ self.rxd_guids = []
+
# 100 is the minimum max_objects that Microsoft seems to honour
# (the max honoured is 400ish), so we use that in these tests
self.max_objects = 100
@@ -120,11 +124,12 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
return dn_list
- def assert_expected_data(self, received_list, expected_list):
+ def assert_expected_data(self, expected_list):
"""
Asserts that we received all the DNs that we expected and
none are missing.
"""
+ received_list = self.rxd_dn_list
# Note that with GET_ANC Windows can end up sending the same parent
# object multiple times, so this might be noteworthy but doesn't
@@ -156,9 +161,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# We ask for the first page of 100 objects.
# For this test, we don't care what order we receive the objects in,
# so long as by the end we've received everything
- rxd_dn_list = []
- ctr6 = self.repl_get_next(rxd_dn_list)
- rxd_dn_list = self._get_ctr6_dn_list(ctr6)
+ self.repl_get_next()
# Modify some of the second page of objects. This should bump the highwatermark
for x in range(100, 200):
@@ -169,11 +172,10 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# Get the remaining blocks of data
while not self.replication_complete():
- ctr6 = self.repl_get_next(rxd_dn_list)
- rxd_dn_list += self._get_ctr6_dn_list(ctr6)
+ self.repl_get_next()
# Check we still receive all the objects we're expecting
- self.assert_expected_data(rxd_dn_list, expected_dn_list)
+ self.assert_expected_data(expected_dn_list)
def is_parent_known(self, dn, known_dn_list):
"""
@@ -189,8 +191,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# test object), or its parent has been seen previously
return parent_dn == self.ou or parent_dn in known_dn_list
- def repl_get_next(self, initial_objects, get_anc=False, get_tgt=False,
- initial_guids=None, assert_links=False):
+ def repl_get_next(self, get_anc=False, get_tgt=False, assert_links=False):
"""
Requests the next block of replication data. This tries to simulate
client behaviour - if we receive a replicated object that we don't know
@@ -209,13 +210,10 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
highwatermark = None
uptodateness_vector = None
- # we'll add new objects as we discover them, so take a copy to modify
- known_objects = initial_objects[:]
- known_guids = []
-
- # Likewise with GUIDs - we only really need them for checking links
- if initial_guids:
- known_guids = initial_guids[:]
+ # we'll add new objects as we discover them, so take a copy of the
+ # ones we already know about, so we can modify it safely
+ known_objects = self.rxd_dn_list[:]
+ known_guids = self.rxd_guids[:]
# Ask for the next block of replication data
replica_flags = drsuapi.DRSUAPI_DRS_WRIT_REP
@@ -259,11 +257,8 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
print("Unknown parent for %s - try GET_ANC" % dn)
# try the same thing again with the GET_ANC flag set this time
- ctr6 = self.repl_get_next (initial_objects, get_anc=True,
- get_tgt=get_tgt,
- initial_guids=initial_guids,
- assert_links=assert_links)
- break
+ return self.repl_get_next(get_anc=True, get_tgt=get_tgt,
+ assert_links=assert_links)
# check we know about references to any objects in the linked attritbutes
received_links = self._get_ctr6_links(ctr6)
@@ -290,10 +285,8 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
print("Unknown source GUID %s - try GET_ANC" % link.identifier)
# try the same thing again with the GET_ANC flag set this time
- return self.repl_get_next (initial_objects, get_anc=True,
- get_tgt=get_tgt,
- initial_guids=initial_guids,
- assert_links=assert_links)
+ return self.repl_get_next(get_anc=True, get_tgt=get_tgt,
+ assert_links=assert_links)
# check we know the target object
if link.targetGUID not in known_guids:
@@ -307,14 +300,17 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
print("Unknown target for %s - try GET_TGT" % link.targetDN)
# try the same thing again with the GET_TGT flag set this time
- return self.repl_get_next (initial_objects, get_anc=get_anc,
- get_tgt=True,
- initial_guids=initial_guids,
- assert_links=assert_links)
+ return self.repl_get_next(get_anc=get_anc, get_tgt=True,
+ assert_links=assert_links)
# store the last successful result so we know what HWM to request next
self.last_ctr = ctr6
+ # store the objects, GUIDs, and links we received
+ self.rxd_dn_list += self._get_ctr6_dn_list(ctr6)
+ self.rxd_links += self._get_ctr6_links(ctr6)
+ self.rxd_guids += self._get_ctr6_object_guids(ctr6)
+
return ctr6
def replication_complete(self):
@@ -369,9 +365,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# GET_ANC set because we won't know about the first child's parent.
# On samba GET_ANC essentially starts the sync from scratch again, so
# we get this over with early before we learn too many parents
- rxd_dn_list = []
- ctr6 = self.repl_get_next(rxd_dn_list)
- rxd_dn_list = self._get_ctr6_dn_list(ctr6)
+ self.repl_get_next()
# modify the last chunk of parents. They should now have a USN higher
# than the highwater-mark for the replication cycle
@@ -381,8 +375,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# Get the remaining blocks of data - this will resend the request with
# GET_ANC if it encounters an object it doesn't have the parent for.
while not self.replication_complete():
- ctr6 = self.repl_get_next(rxd_dn_list)
- rxd_dn_list += self._get_ctr6_dn_list(ctr6)
+ self.repl_get_next()
# The way the test objects have been created should force
# self.repl_get_next() to use the GET_ANC flag. If this doesn't
@@ -391,14 +384,14 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
"Test didn't use the GET_ANC flag as expected")
# Check we get all the objects we're expecting
- self.assert_expected_data(rxd_dn_list, expected_dn_list)
+ self.assert_expected_data(expected_dn_list)
- def assert_expected_links(self, received_links, objects_with_links,
- link_attr="managedBy"):
+ def assert_expected_links(self, objects_with_links, link_attr="managedBy"):
"""
Asserts that a GetNCChanges response contains any expected links
for the objects it contains.
"""
+ received_links = self.rxd_links
self.assertTrue(len(received_links) == len(objects_with_links),
"Received %d links but expected %d"
@@ -470,9 +463,6 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
for i in range(0, 100):
self.modify_object(managers[i], "displayName", "OU%d" % i)
- rxd_dn_list = []
- rxd_links = []
- rxd_guids = []
links_expected = True
# Get all the replication data - this code should resend the requests
@@ -480,12 +470,8 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
while not self.replication_complete():
# get the next block of replication data (this sets GET_TGT if needed)
- ctr6 = self.repl_get_next(rxd_dn_list, initial_guids=rxd_guids,
- assert_links=links_expected)
- rxd_dn_list += self._get_ctr6_dn_list(ctr6)
- rxd_links += self._get_ctr6_links(ctr6)
- rxd_guids += self._get_ctr6_object_guids(ctr6)
- links_expected = len(rxd_links) < len(expected_links)
+ self.repl_get_next(assert_links=links_expected)
+ links_expected = len(self.rxd_links) < len(expected_links)
# The way the test objects have been created should force
# self.repl_get_next() to use the GET_TGT flag. If this doesn't
@@ -494,10 +480,10 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
"Test didn't use the GET_TGT flag as expected")
# Check we get all the objects we're expecting
- self.assert_expected_data(rxd_dn_list, all_objects)
+ self.assert_expected_data(all_objects)
# Check we received links for all the reportees
- self.assert_expected_links(rxd_links, expected_links)
+ self.assert_expected_links(expected_links)
def test_repl_get_tgt_chain(self):
"""
@@ -532,9 +518,6 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# the default order the objects now get returned in should be:
# [A0-A99][B0-B99][C0-C99]
- rxd_dn_list = []
- rxd_links = []
- rxd_guids = []
links_expected = True
# Get all the replication data - this code should resend the requests
@@ -542,12 +525,8 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
while not self.replication_complete():
# get the next block of replication data (this sets GET_TGT if needed)
- ctr6 = self.repl_get_next(rxd_dn_list, initial_guids=rxd_guids,
- assert_links=links_expected)
- rxd_dn_list += self._get_ctr6_dn_list(ctr6)
- rxd_links += self._get_ctr6_links(ctr6)
- rxd_guids += self._get_ctr6_object_guids(ctr6)
- links_expected = len(rxd_links) < len(expected_links)
+ self.repl_get_next(assert_links=links_expected)
+ links_expected = len(self.rxd_links) < len(expected_links)
# The way the test objects have been created should force
# self.repl_get_next() to use the GET_TGT flag. If this doesn't
@@ -556,10 +535,10 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
"Test didn't use the GET_TGT flag as expected")
# Check we get all the objects we're expecting
- self.assert_expected_data(rxd_dn_list, all_objects)
+ self.assert_expected_data(all_objects)
# Check we received links for all the reportees
- self.assert_expected_links(rxd_links, expected_links)
+ self.assert_expected_links(expected_links)
def test_repl_get_anc_link_attr(self):
"""
@@ -585,23 +564,16 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# Get the replication data - because the block of children come first,
# this should retry the request with GET_ANC
- rxd_dn_list = []
- rxd_links = []
- rxd_guids = []
-
while not self.replication_complete():
- ctr6 = self.repl_get_next(rxd_dn_list, initial_guids=rxd_guids)
- rxd_dn_list += self._get_ctr6_dn_list(ctr6)
- rxd_links += self._get_ctr6_links(ctr6)
- rxd_guids += self._get_ctr6_object_guids(ctr6)
+ self.repl_get_next()
self.assertTrue(self.used_get_anc,
"Test didn't use the GET_ANC flag as expected")
# Check we get all the objects we're expecting
- self.assert_expected_data(rxd_dn_list, expected_dn_list)
+ self.assert_expected_data(expected_dn_list)
# Check we received links for all the parents
- self.assert_expected_links(rxd_links, parent_dn_list)
+ self.assert_expected_links(parent_dn_list)
--
2.7.4
From 95cc7f5bf16a061852255eed3cfc1633d9064cf1 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 12 Jun 2017 10:26:07 +1200
Subject: [PATCH 10/14] getncchanges.py: Split some repl_get_next() code into
separate function
repl_get_next() is quite big and complex. Split out the part that sends
the actual request as this is an easy chunk of code to separate.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/getncchanges.py | 40 ++++++++++++++++++------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
index 3dfb51c..708fdca 100644
--- a/source4/torture/drs/python/getncchanges.py
+++ b/source4/torture/drs/python/getncchanges.py
@@ -191,14 +191,8 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# test object), or its parent has been seen previously
return parent_dn == self.ou or parent_dn in known_dn_list
- def repl_get_next(self, get_anc=False, get_tgt=False, assert_links=False):
- """
- Requests the next block of replication data. This tries to simulate
- client behaviour - if we receive a replicated object that we don't know
- the parent of, then re-request the block with the GET_ANC flag set.
- If we don't know the target object for a linked attribute, then
- re-request with GET_TGT.
- """
+ def _repl_send_request(self, get_anc=False, get_tgt=False):
+ """Sends a GetNCChanges request for the next block of replication data."""
# we're just trying to mimic regular client behaviour here, so just
# use the highwatermark in the last response we received
@@ -206,15 +200,10 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
highwatermark = self.last_ctr.new_highwatermark
uptodateness_vector = self.last_ctr.uptodateness_vector
else:
- # this is the initial replication, so we're starting from the start
+ # this is the first replication chunk
highwatermark = None
uptodateness_vector = None
- # we'll add new objects as we discover them, so take a copy of the
- # ones we already know about, so we can modify it safely
- known_objects = self.rxd_dn_list[:]
- known_guids = self.rxd_guids[:]
-
# Ask for the next block of replication data
replica_flags = drsuapi.DRSUAPI_DRS_WRIT_REP
more_flags = 0
@@ -227,16 +216,35 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
more_flags = drsuapi.DRSUAPI_DRS_GET_TGT
self.used_get_tgt = True
- ctr6 = self._get_replication(replica_flags,
+ # return the response from the DC
+ return self._get_replication(replica_flags,
max_objects=self.max_objects,
highwatermark=highwatermark,
uptodateness_vector=uptodateness_vector,
more_flags=more_flags)
- # check that we know the parent for every object received
+ def repl_get_next(self, get_anc=False, get_tgt=False, assert_links=False):
+ """
+ Requests the next block of replication data. This tries to simulate
+ client behaviour - if we receive a replicated object that we don't know
+ the parent of, then re-request the block with the GET_ANC flag set.
+ If we don't know the target object for a linked attribute, then
+ re-request with GET_TGT.
+ """
+
+ # send a request to the DC and get the response
+ ctr6 = self._repl_send_request(get_anc=get_anc, get_tgt=get_tgt)
+
+ # extract the object DNs and their GUIDs from the response
rxd_dn_list = self._get_ctr6_dn_list(ctr6)
rxd_guid_list = self._get_ctr6_object_guids(ctr6)
+ # we'll add new objects as we discover them, so take a copy of the
+ # ones we already know about, so we can modify these lists safely
+ known_objects = self.rxd_dn_list[:]
+ known_guids = self.rxd_guids[:]
+
+ # check that we know the parent for every object received
for i in range(0, len(rxd_dn_list)):
dn = rxd_dn_list[i]
--
2.7.4
From 997a23a1e81a38cb43a77531e0a346ab1736803d Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 13 Jun 2017 12:14:45 +1200
Subject: [PATCH 11/14] getncchanges.py: Add test for adding links during
replication
We have identified a case where the Samba server can send linked
attributes but not the target object. In this case, the Samba DRS client
would hit the "Failed to re-resolve GUID" case in replmd and silently
discard the linked attribute.
However, Samba will resend the linked attribute in the next cycle
(because its USN is still higher than the committed HWM), so it should
recover OK. On older releases, this may have caused problems if the
first error resulting in a hanging link (which might mean the second
time it's processed it still fails to be added).
Note that this test currently passes on Samba, because the test code
retries with GET_TGT, which causes the Samba DRS sever to restart the
replication cycle from the beginning again.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/getncchanges.py | 42 ++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
index 708fdca..7d21994 100644
--- a/source4/torture/drs/python/getncchanges.py
+++ b/source4/torture/drs/python/getncchanges.py
@@ -548,6 +548,48 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# Check we received links for all the reportees
self.assert_expected_links(expected_links)
+ def test_repl_integrity_link_attr(self):
+ """
+ Tests adding links to new objects while a replication is in progress.
+ """
+
+ # create some source objects for the linked attributes, sandwiched
+ # between 2 blocks of filler objects
+ filler = self.create_object_range(0, 100, prefix="filler")
+ reportees = self.create_object_range(0, 100, prefix="reportee")
+ filler += self.create_object_range(100, 200, prefix="filler")
+
+ # Start the replication and get the first block of filler objects
+ self.repl_get_next()
+
+ # create the target objects and add the links. These objects should be
+ # outside the scope of the Samba replication cycle, but the links should
+ # still get sent with the source object
+ managers = self.create_object_range(0, 100, prefix="manager")
+
+ for i in range(0, 100):
+ self.modify_object(reportees[i], "managedBy", managers[i])
+
+ expected_objects = managers + reportees + filler
+ expected_links = reportees
+
+ # complete the first replication
+ while not self.replication_complete():
+ self.repl_get_next()
+
+ # Start another replication cycle to get the new objects from Samba
+ # (this is unnecessary on Windows - it's already sent everything)
+ self.repl_get_next()
+
+ while not self.replication_complete():
+ self.repl_get_next()
+
+ # Check we get all the objects we're expecting
+ self.assert_expected_data(expected_objects)
+
+ # Check we received links for all the parents
+ self.assert_expected_links(expected_links)
+
def test_repl_get_anc_link_attr(self):
"""
A basic GET_ANC test where the parents have linked attributes
--
2.7.4
From d6454cc6a099cbba705da0718a1a862424b62943 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 19 Jun 2017 17:01:56 +1200
Subject: [PATCH 12/14] getncchanges.py: Update tests to pass against current
Samba
The current Samba implementation doesn't support GET_TGT and so the
tests are currently failing. This patch adds a toggle to turn the
failing behaviour on/off. This allows us to run the tests as part of
'make test' successfully, but also means we have some stricter tests
for when we implement GET_TGT.
The current GET_TGT tests won't actually test the GET_TGT flag on Samba,
but it's still probably worth having some tests that exercise linked
attributes across larger chunks of replication data.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/getncchanges.py | 79 ++++++++++++++++++------------
1 file changed, 49 insertions(+), 30 deletions(-)
diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
index 7d21994..4b02e3c 100644
--- a/source4/torture/drs/python/getncchanges.py
+++ b/source4/torture/drs/python/getncchanges.py
@@ -54,6 +54,10 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
self.rxd_links = []
self.rxd_guids = []
+ # Samba currently sends its links last. Toggle this flag to enable some
+ # extra asserts against Windows (or against future GET_TGT Samba code)
+ self.sends_links_last = True
+
# 100 is the minimum max_objects that Microsoft seems to honour
# (the max honoured is 400ish), so we use that in these tests
self.max_objects = 100
@@ -223,7 +227,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
uptodateness_vector=uptodateness_vector,
more_flags=more_flags)
- def repl_get_next(self, get_anc=False, get_tgt=False, assert_links=False):
+ def repl_get_next(self, get_anc=False, get_tgt=False):
"""
Requests the next block of replication data. This tries to simulate
client behaviour - if we receive a replicated object that we don't know
@@ -265,18 +269,11 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
print("Unknown parent for %s - try GET_ANC" % dn)
# try the same thing again with the GET_ANC flag set this time
- return self.repl_get_next(get_anc=True, get_tgt=get_tgt,
- assert_links=assert_links)
+ return self.repl_get_next(get_anc=True, get_tgt=get_tgt)
# check we know about references to any objects in the linked attritbutes
received_links = self._get_ctr6_links(ctr6)
- # This is so that older versions of Samba fail - we want the links to be
- # sent roughly with the objects, rather than getting all links at the end
- if assert_links:
- self.assertTrue(len(received_links) > 0,
- "Links were expected in the GetNCChanges response")
-
for link in received_links:
# check the source object is known (Windows can actually send links
@@ -293,8 +290,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
print("Unknown source GUID %s - try GET_ANC" % link.identifier)
# try the same thing again with the GET_ANC flag set this time
- return self.repl_get_next(get_anc=True, get_tgt=get_tgt,
- assert_links=assert_links)
+ return self.repl_get_next(get_anc=True, get_tgt=get_tgt)
# check we know the target object
if link.targetGUID not in known_guids:
@@ -308,8 +304,7 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
print("Unknown target for %s - try GET_TGT" % link.targetDN)
# try the same thing again with the GET_TGT flag set this time
- return self.repl_get_next(get_anc=get_anc, get_tgt=True,
- assert_links=assert_links)
+ return self.repl_get_next(get_anc=get_anc, get_tgt=True)
# store the last successful result so we know what HWM to request next
self.last_ctr = ctr6
@@ -447,6 +442,39 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
self.assertTrue(found, "Did not receive expected link for DN %s" % dn)
+ def check_get_tgt_used(self):
+ """Checks that the GET_TGT flag was used, if it's supported"""
+
+ if self.used_get_tgt:
+ pass
+ elif self.sends_links_last:
+ # Samba currently sends the linked attributes last, which means the
+ # GET_TGT flag isn't required. These test cases are still testing
+ # linked attribute replication, but not the GET_TGT flag itself
+ print("Note that test case did not set GET_TGT flag")
+ else:
+ # If the implementation supports GET_TGT but it wasn't used, then
+ # something is not quite right with the test case
+ self.assertTrue(self.used_get_tgt,
+ "Test didn't use the GET_TGT flag as expected")
+
+ def check_links_were_received(self, ctr, total_links_expected):
+ """
+ Checks that the last received chunk of replication data contains linked
+ attributes (if the implemetation supports that).
+ """
+
+ # Because Samba currently sends all the links at the end of the cycle
+ # cycle it avoids the need to use GET_TGT (because the target objects
+ # are always known). We can be mean and assert that we expect to receive
+ # links in each chunk. The following flag allows us to toggle the test
+ # behaviour (so 'make test' passes by default)
+ if not self.sends_links_last:
+
+ # check we haven't already received all the links
+ if len(self.rxd_links) < total_links_expected:
+ self.assertTrue(ctr.linked_attributes_count > 0,
+ "Links were expected in the GetNCChanges response")
def test_repl_get_tgt(self):
"""
@@ -471,21 +499,17 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
for i in range(0, 100):
self.modify_object(managers[i], "displayName", "OU%d" % i)
- links_expected = True
+ total_expected_links = len(expected_links)
# Get all the replication data - this code should resend the requests
# with GET_TGT
while not self.replication_complete():
# get the next block of replication data (this sets GET_TGT if needed)
- self.repl_get_next(assert_links=links_expected)
- links_expected = len(self.rxd_links) < len(expected_links)
+ ctr = self.repl_get_next()
+ self.check_links_were_received(ctr, total_expected_links)
- # The way the test objects have been created should force
- # self.repl_get_next() to use the GET_TGT flag. If this doesn't
- # actually happen, then the test isn't doing its job properly
- self.assertTrue(self.used_get_tgt,
- "Test didn't use the GET_TGT flag as expected")
+ self.check_get_tgt_used()
# Check we get all the objects we're expecting
self.assert_expected_data(all_objects)
@@ -522,25 +546,20 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
all_objects = objectsA + objectsB + objectsC
expected_links = all_objects
+ num_expected_links = len(expected_links)
# the default order the objects now get returned in should be:
# [A0-A99][B0-B99][C0-C99]
- links_expected = True
-
# Get all the replication data - this code should resend the requests
# with GET_TGT
while not self.replication_complete():
# get the next block of replication data (this sets GET_TGT if needed)
- self.repl_get_next(assert_links=links_expected)
- links_expected = len(self.rxd_links) < len(expected_links)
+ ctr = self.repl_get_next()
+ self.check_links_were_received(ctr, num_expected_links)
- # The way the test objects have been created should force
- # self.repl_get_next() to use the GET_TGT flag. If this doesn't
- # actually happen, then the test isn't doing its job properly
- self.assertTrue(self.used_get_tgt,
- "Test didn't use the GET_TGT flag as expected")
+ self.check_get_tgt_used()
# Check we get all the objects we're expecting
self.assert_expected_data(all_objects)
--
2.7.4
From de450ba4193dce161cb0e7cd428bf4d8709ab8a1 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 12 Jun 2017 09:47:42 +1200
Subject: [PATCH 13/14] getncchanges.py: Add a manual test for multivalue
linked attributes
The easiest way to do this is to add a bunch of users to groups.
Unfortunately, adding users/groups causes RID allocation, and there is
some concern that this can make the automated tests flakey. So for now,
this is just a test case that developers can run manually.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/torture/drs/python/getncchanges.py | 66 +++++++++++++++++++++++++++---
1 file changed, 60 insertions(+), 6 deletions(-)
diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
index 4b02e3c..0578201 100644
--- a/source4/torture/drs/python/getncchanges.py
+++ b/source4/torture/drs/python/getncchanges.py
@@ -76,9 +76,9 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
if enum == ldb.ERR_NO_SUCH_OBJECT:
pass
- def add_object(self, dn):
+ def add_object(self, dn, obj_class="organizationalunit"):
"""Adds an OU object"""
- self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalunit"})
+ self.ldb_dc1.add({"dn": dn, "objectclass": obj_class})
res = self.ldb_dc1.search(base=dn, scope=SCOPE_BASE)
self.assertEquals(len(res), 1)
@@ -185,6 +185,11 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
"""
Returns True if the parent of the dn specified is in known_dn_list
"""
+ # we can sometimes get system objects like the RID Manager returned.
+ # Ignore anything that is not under the test OU we created
+ if self.ou not in dn:
+ return True
+
# Remove the child portion from the name to get the parent's DN
name_substrings = dn.split(",")
del name_substrings[0]
@@ -389,14 +394,21 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# Check we get all the objects we're expecting
self.assert_expected_data(expected_dn_list)
- def assert_expected_links(self, objects_with_links, link_attr="managedBy"):
+ def assert_expected_links(self, objects_with_links, link_attr="managedBy",
+ num_expected=0):
"""
Asserts that a GetNCChanges response contains any expected links
for the objects it contains.
"""
received_links = self.rxd_links
- self.assertTrue(len(received_links) == len(objects_with_links),
+ # if we're dealing with single-value linked attributes, then the number
+ # of objects that have links should match the number of links we receive.
+ # This doesn't work with multi-valued attributes like 'member'
+ if num_expected == 0:
+ num_expected = len(objects_with_links)
+
+ self.assertTrue(len(received_links) == num_expected,
"Received %d links but expected %d"
%(len(received_links), len(objects_with_links)))
@@ -422,14 +434,14 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# find the received link in the list and assert that the target and
# source GUIDs match what's in the DB
- found = False
-
for val in res[0][link_attr]:
# Work out the expected source and target GUIDs for the DB link
target_dn = ldb.Dn(self.ldb_dc1, val)
targetGUID_blob = target_dn.get_extended_component("GUID")
sourceGUID_blob = res[0].dn.get_extended_component("GUID")
+ found = False
+
for link in received_links:
if link.selfGUID_blob == sourceGUID_blob and \
link.targetGUID_blob == targetGUID_blob:
@@ -645,4 +657,46 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# Check we received links for all the parents
self.assert_expected_links(parent_dn_list)
+ # Adding users/groups causes RID allocation, which can lead to flakiness
+ # in the automated tests (e.g. exhausting the RID pool may alter the test
+ # behaviour). For now, just run this test manually
+ def manual_test_replication_groups(self):
+ """
+ Tests replication with multi-valued link attributes. The simplest way
+ to do this is to add a bunch of users to groups.
+ """
+ group_list = []
+ user_list = []
+
+ # create a few groups and a bunch of users
+ for x in range(0, 10):
+ dn = "CN=test_grp%d,%s" %(x, self.ou)
+ self.add_object(dn, obj_class="group")
+ group_list.append(dn)
+
+ for x in range(0, 500):
+ dn = "CN=test_usr%d,%s" % (x, self.ou)
+ self.add_object(dn, obj_class="user")
+ user_list.append(dn)
+
+ # add all the users to all the groups
+ for grp in group_list:
+ for usr in user_list:
+ self.modify_object(grp, "member", usr)
+
+ expected_dn_list = user_list + group_list
+
+ # add some filler objects at the end. This allows us to easily see
+ # which chunk the links get sent in
+ expected_dn_list += self.create_object_range(0, 200, prefix="filler")
+
+ # do the replication
+ while not self.replication_complete():
+ self.repl_get_next()
+
+ # Check we get all the objects we're expecting
+ self.assert_expected_data(expected_dn_list)
+
+ # Check we received links for all the groups
+ self.assert_expected_links(group_list, link_attr="member", num_expected=5000)
--
2.7.4
From c838a994754812c265ed62d741d8106577727b0b Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 20 Jun 2017 08:52:16 +1200
Subject: [PATCH 14/14] tests: Add new getncchanges.py tests to tests.py
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/selftest/tests.py | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index f61c93f..777c2cb 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -807,6 +807,11 @@ for env in ['vampire_dc', 'promoted_dc']:
name="samba4.drs.getnc_exop.python(%s)" % env,
environ={'DC1': "$DC_SERVER", 'DC2': '$%s_SERVER' % env.upper()},
extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
+ planoldpythontestsuite(env, "getncchanges",
+ extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
+ name="samba4.drs.getncchanges.python(%s)" % env,
+ environ={'DC1': "$DC_SERVER", 'DC2': '$%s_SERVER' % env.upper()},
+ extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
planoldpythontestsuite(env, "linked_attributes_drs",
extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
name="samba4.drs.linked_attributes_drs.python(%s)" % env,
--
2.7.4
More information about the samba-technical
mailing list