[PATCH] Automatic DNS site coverage for sites with no DCs

Garming Sam garming at catalyst.net.nz
Tue Mar 20 00:19:21 UTC 2018


Hi,

In AD sites without any DCs, looking for LDAP SRV DNS records in the
local site will never return any results. This means that eventual LDAP
connections are made using the top level SRV records which might be
located anywhere geographically. In order to resolve this, AD has the
concept of automatic site coverage, where such sites are adopted by
nearby sites (based on site link cost).

https://blogs.technet.microsoft.com/askds/2011/04/29/sites-sites-everywhere/

I've implemented a similar behaviour in Samba where DC-less sites are
covered by DCs in the site connected with the lowest cost present. If
costs are equal between two site links, both will choose to cover the
uncovered site. In the case of site links which connect more than the
uncovered site and one other site, we choose the largest site (and if
that is equal, the earliest alphabetically) to represent the cluster of
sites. This is similar to the behaviour described by Windows and seems
to be generally common-sense behaviour.

There are unit tests for the algorithm that is used to determine which
sites to cover and a simple test of the samba_dnsupdate changes.

Please review and push.

Cheers,

Garming

-------------- next part --------------
From 7852aa0a31322f678c1bea151f735c89c83d12fd Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 12 Mar 2018 14:44:58 +1300
Subject: [PATCH 1/8] join.py: Add missing NTSTATUSError import

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 python/samba/join.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/samba/join.py b/python/samba/join.py
index 22deff0..4eaf05c 100644
--- a/python/samba/join.py
+++ b/python/samba/join.py
@@ -35,7 +35,7 @@ from samba.provision.sambadns import setup_bind9_dns
 from samba import read_and_sub_file
 from samba import werror
 from base64 import b64encode
-from samba import WERRORError
+from samba import WERRORError, NTSTATUSError
 from samba.dnsserver import ARecord, AAAARecord, PTRRecord, CNameRecord, NSRecord, MXRecord, SOARecord, SRVRecord, TXTRecord
 from samba import sd_utils
 import logging
-- 
1.9.1


From 9e129d29e663ec9b9f9eff0980d56610a7bced95 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Tue, 13 Mar 2018 13:04:12 +1300
Subject: [PATCH 2/8] kcc_utils: Add a routine for automatic site coverage

This allows double-coverage if two links exist with the same cost.
Administrators should only connect an DC-less site via a single site
link.

This also allows unnecessary coverage by all sites in the adjoining site
link (to be resolved in the later patches).

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 python/samba/kcc/kcc_utils.py | 72 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/python/samba/kcc/kcc_utils.py b/python/samba/kcc/kcc_utils.py
index 7c292a6..e5a323d 100644
--- a/python/samba/kcc/kcc_utils.py
+++ b/python/samba/kcc/kcc_utils.py
@@ -2252,3 +2252,75 @@ def new_connection_schedule():
 
     schedule.dataArray = [data]
     return schedule
+
+
+##################################################
+# DNS related calls
+##################################################
+
+def uncovered_sites_to_cover(samdb, site_name):
+    """
+    Discover which sites have no DCs and whose lowest single-hop cost
+    distance for any link attached to that site is linked to the site supplied.
+
+    We compare the lowest cost of your single-hop link to this site to all of
+    those available (if it exists). This means that a lower ranked siteLink
+    with only the uncovered site can trump any available links (but this can
+    only be done with specific, poorly enacted user configuration).
+
+    :param samdb database
+    :param site_name origin site (with a DC)
+
+    :return a list of sites this site should be covering (for DNS)
+    """
+    sites_to_cover = []
+
+    server_res = samdb.search(base=samdb.get_config_basedn(),
+                              scope=ldb.SCOPE_SUBTREE,
+                              expression="(&(objectClass=server)(serverReference=*))")
+
+    site_res = samdb.search(base=samdb.get_config_basedn(),
+                            scope=ldb.SCOPE_SUBTREE,
+                            expression="(objectClass=site)")
+
+    sites_in_use = set()
+
+    # Assume server is of form DC,Servers,Site-ABCD because of schema
+    for msg in server_res:
+        sites_in_use.add(msg.dn.parent().parent().canonical_str())
+
+    if len(sites_in_use) != len(site_res):
+        # There is a possible uncovered site
+        sites_uncovered = []
+
+        for msg in site_res:
+            if msg.dn.canonical_str() not in sites_in_use:
+                sites_uncovered.append(msg)
+
+        own_site_dn = "CN=%s,CN=Sites,%s" % (ldb.binary_encode(site_name),
+                                             ldb.binary_encode(str(samdb.get_config_basedn())))
+        for site in sites_uncovered:
+            # Get a sorted list of all siteLinks featuring the uncovered site
+            link_res1 = samdb.search(base=samdb.get_config_basedn(),
+                                     scope=ldb.SCOPE_SUBTREE, attrs=["cost"],
+                                     expression="(&(objectClass=siteLink)"
+                                     "(siteList=%s))" % ldb.binary_encode(str(site.dn)),
+                                     controls=["server_sort:1:0:cost"])
+
+            # Get a sorted list of all siteLinks connecting this an the uncovered site
+            link_res2 = samdb.search(base=samdb.get_config_basedn(),
+                                     scope=ldb.SCOPE_SUBTREE, attrs=["cost"],
+                                     expression="(&(objectClass=siteLink)"
+                                     "(siteList=%s)(siteList=%s))" % (own_site_dn,
+                                                                      ldb.binary_encode(str(site.dn))),
+                                     controls=["server_sort:1:0:cost"])
+
+            # Add to list if your link is equal in cost to lowest cost link
+            if len(link_res1) > 0 and len(link_res2) > 0:
+                cost1 = int(link_res1[0]['cost'][0])
+                cost2 = int(link_res2[0]['cost'][0])
+                if cost1 == cost2:
+                    site_cover_rdn = site.dn.get_rdn_value()
+                    sites_to_cover.append(site_cover_rdn)
+
+    return sites_to_cover
-- 
1.9.1


From 97e509caa3a21ff477ae75a316bf31cf57d35eef Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Tue, 13 Mar 2018 14:11:14 +1300
Subject: [PATCH 3/8] kcc_utils: Keep a count of the DCs in each site

This is useful for ranking which sites are preferable within the same
site link.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 python/samba/kcc/kcc_utils.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/python/samba/kcc/kcc_utils.py b/python/samba/kcc/kcc_utils.py
index e5a323d..da4c99f 100644
--- a/python/samba/kcc/kcc_utils.py
+++ b/python/samba/kcc/kcc_utils.py
@@ -31,6 +31,7 @@ from samba.dcerpc import (
     )
 from samba.common import dsdb_Dn
 from samba.ndr import ndr_unpack, ndr_pack
+from collections import Counter
 
 
 class KCCError(Exception):
@@ -2283,11 +2284,11 @@ def uncovered_sites_to_cover(samdb, site_name):
                             scope=ldb.SCOPE_SUBTREE,
                             expression="(objectClass=site)")
 
-    sites_in_use = set()
+    sites_in_use = Counter()
 
     # Assume server is of form DC,Servers,Site-ABCD because of schema
     for msg in server_res:
-        sites_in_use.add(msg.dn.parent().parent().canonical_str())
+        sites_in_use[msg.dn.parent().parent().canonical_str()] += 1
 
     if len(sites_in_use) != len(site_res):
         # There is a possible uncovered site
-- 
1.9.1


From 5aea8017d915e4fd9b49fae103e47e73bcc3202a Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Tue, 13 Mar 2018 14:41:23 +1300
Subject: [PATCH 4/8] kcc_utils: Prevent multiple sites attached to a sitelink
 covering a site

This avoids trivial duplicates in a similar manner as mentioned in:

https://blogs.technet.microsoft.com/askds/2011/04/29/sites-sites-everywhere/

It prefers the largest sites then the earliest alphabetically, so that
only a single site ever covers an uncovered site (within a site link).
Note that this isn't applicable over multiple site links (like Windows
presumably) and is only a simple mechanism to avoid excessive
registering.  DCs within the site will also still register for each.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 python/samba/kcc/kcc_utils.py | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/python/samba/kcc/kcc_utils.py b/python/samba/kcc/kcc_utils.py
index da4c99f..946ff1c 100644
--- a/python/samba/kcc/kcc_utils.py
+++ b/python/samba/kcc/kcc_utils.py
@@ -2269,6 +2269,10 @@ def uncovered_sites_to_cover(samdb, site_name):
     with only the uncovered site can trump any available links (but this can
     only be done with specific, poorly enacted user configuration).
 
+    If the site is connected to more than one other site with the same
+    siteLink, only the largest site (failing that sorted alphabetically)
+    creates the DNS records.
+
     :param samdb database
     :param site_name origin site (with a DC)
 
@@ -2285,10 +2289,15 @@ def uncovered_sites_to_cover(samdb, site_name):
                             expression="(objectClass=site)")
 
     sites_in_use = Counter()
+    dc_count = 0
 
     # Assume server is of form DC,Servers,Site-ABCD because of schema
     for msg in server_res:
-        sites_in_use[msg.dn.parent().parent().canonical_str()] += 1
+        site_dn = msg.dn.parent().parent()
+        sites_in_use[site_dn.canonical_str()] += 1
+
+        if site_dn.get_rdn_value().lower() == site_name.lower():
+            dc_count += 1
 
     if len(sites_in_use) != len(site_res):
         # There is a possible uncovered site
@@ -2310,7 +2319,7 @@ def uncovered_sites_to_cover(samdb, site_name):
 
             # Get a sorted list of all siteLinks connecting this an the uncovered site
             link_res2 = samdb.search(base=samdb.get_config_basedn(),
-                                     scope=ldb.SCOPE_SUBTREE, attrs=["cost"],
+                                     scope=ldb.SCOPE_SUBTREE, attrs=["cost", "siteList"],
                                      expression="(&(objectClass=siteLink)"
                                      "(siteList=%s)(siteList=%s))" % (own_site_dn,
                                                                       ldb.binary_encode(str(site.dn))),
@@ -2320,7 +2329,27 @@ def uncovered_sites_to_cover(samdb, site_name):
             if len(link_res1) > 0 and len(link_res2) > 0:
                 cost1 = int(link_res1[0]['cost'][0])
                 cost2 = int(link_res2[0]['cost'][0])
-                if cost1 == cost2:
+
+                # Own siteLink must match the lowest cost link
+                if cost1 != cost2:
+                    continue
+
+                # In a siteLink with more than 2 sites attached, only pick the
+                # largest site, and if there are multiple, the earliest alphabetically.
+                to_cover = True
+                for site_val in link_res2[0]['siteList']:
+                    site_dn = ldb.Dn(samdb, str(site_val))
+                    site_dn_str = site_dn.canonical_str()
+                    site_rdn = site_dn.get_rdn_value().lower()
+                    if sites_in_use[site_dn_str] > dc_count:
+                        to_cover = False
+                        break
+                    elif (sites_in_use[site_dn_str] == dc_count and
+                          site_rdn < site_name.lower()):
+                        to_cover = False
+                        break
+
+                if to_cover:
                     site_cover_rdn = site.dn.get_rdn_value()
                     sites_to_cover.append(site_cover_rdn)
 
-- 
1.9.1


From dfcd1cb7e7f1fbc31b9895fd49935f15193d8b20 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 14 Mar 2018 16:52:58 +1300
Subject: [PATCH 5/8] kcc_utils: Use lower name in automatic sites covered

This allows easier testing, as well as some consistency in the DNS
record creation.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 python/samba/kcc/kcc_utils.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/samba/kcc/kcc_utils.py b/python/samba/kcc/kcc_utils.py
index 946ff1c..68cf705 100644
--- a/python/samba/kcc/kcc_utils.py
+++ b/python/samba/kcc/kcc_utils.py
@@ -2351,6 +2351,6 @@ def uncovered_sites_to_cover(samdb, site_name):
 
                 if to_cover:
                     site_cover_rdn = site.dn.get_rdn_value()
-                    sites_to_cover.append(site_cover_rdn)
+                    sites_to_cover.append(site_cover_rdn.lower())
 
     return sites_to_cover
-- 
1.9.1


From d33a7af5915eddce2338ded1ca0c6c95b0b3ca22 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 14 Mar 2018 16:53:13 +1300
Subject: [PATCH 6/8] tests/kcc_util: Add unit tests for automatic site
 coverage

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 python/samba/tests/kcc/kcc_utils.py | 352 ++++++++++++++++++++++++++++++++++++
 selftest/tests.py                   |   1 -
 source4/selftest/tests.py           |   2 +
 3 files changed, 354 insertions(+), 1 deletion(-)

diff --git a/python/samba/tests/kcc/kcc_utils.py b/python/samba/tests/kcc/kcc_utils.py
index ac59bee..ae0aee3 100644
--- a/python/samba/tests/kcc/kcc_utils.py
+++ b/python/samba/tests/kcc/kcc_utils.py
@@ -21,6 +21,10 @@
 import samba
 import samba.tests
 from samba.kcc.kcc_utils import *
+from samba.credentials import Credentials
+from samba.auth import system_session
+from samba.samdb import SamDB
+from samba.tests import delete_force
 
 
 class ScheduleTests(samba.tests.TestCase):
@@ -35,3 +39,351 @@ class ScheduleTests(samba.tests.TestCase):
 # OK, this is pathetic, but the rest of it looks really hard, with the
 # classes all intertwingled with each other and the samdb. That is to say:
 # XXX later.
+
+class SiteCoverageTests(samba.tests.TestCase):
+
+    def setUp(self):
+        self.prefix = "kcc_"
+        self.lp = samba.tests.env_loadparm()
+
+        self.sites = {}
+        self.site_links = {}
+
+        self.creds = Credentials()
+        self.creds.guess(self.lp)
+        self.session = system_session()
+
+        self.samdb = SamDB(session_info=self.session,
+                           credentials=self.creds,
+                           lp=self.lp)
+
+    def tearDown(self):
+        self.samdb.transaction_start()
+
+        for site in self.sites:
+            delete_force(self.samdb, site, controls=['tree_delete:1'])
+
+        for site_link in self.site_links:
+            delete_force(self.samdb, site_link)
+
+        self.samdb.transaction_commit()
+
+    def _add_server(self, name, site):
+        dn = "CN={},CN=Servers,{}".format(name, site)
+        self.samdb.add({
+            "dn": dn,
+            "objectClass": "server",
+            "serverReference": self.samdb.domain_dn()
+        })
+        return dn
+
+    def _add_site(self, name):
+        dn = "CN={},CN=Sites,{}".format(
+            name, self.samdb.get_config_basedn()
+        )
+        self.samdb.add({
+            "dn": dn,
+            "objectClass": "site"
+        })
+        self.samdb.add({
+            "dn": "CN=Servers," + dn,
+            "objectClass": ["serversContainer"]
+        })
+
+        self.sites[dn] = name
+        return dn, name.lower()
+
+    def _add_site_link(self, name, links=[], cost=100):
+        dn = "CN={},CN=IP,CN=Inter-Site Transports,CN=Sites,{}".format(
+            name, self.samdb.get_config_basedn()
+        )
+        self.samdb.add({
+            "dn": dn,
+            "objectClass": "siteLink",
+            "cost": str(cost),
+            "siteList": links
+        })
+        self.site_links[dn] = name
+        return dn
+
+    def test_single_site_link_same_dc_count(self):
+        self.samdb.transaction_start()
+        site1, name1 = self._add_site(self.prefix + "ABCD")
+        site2, name2 = self._add_site(self.prefix + "BCDE")
+
+        uncovered_dn, uncovered = self._add_site(self.prefix + "uncovered")
+
+        self._add_server(self.prefix + "ABCD" + '1', site1)
+        self._add_server(self.prefix + "BCDE" + '1', site2)
+
+        self._add_site_link(self.prefix + "link",
+                            [site1, site2, uncovered_dn])
+        self.samdb.transaction_commit()
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name1)
+        to_cover.sort()
+
+        self.assertEqual([uncovered], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name2)
+        to_cover.sort()
+
+        self.assertEqual([], to_cover)
+
+    def test_single_site_link_different_dc_count(self):
+        self.samdb.transaction_start()
+        site1, name1 = self._add_site(self.prefix + "ABCD")
+        site2, name2 = self._add_site(self.prefix + "BCDE")
+
+        uncovered_dn, uncovered = self._add_site(self.prefix + "uncovered")
+
+        self._add_server(self.prefix + "ABCD" + '1', site1)
+        self._add_server(self.prefix + "ABCD" + '2', site1)
+        self._add_server(self.prefix + "BCDE" + '1', site2)
+        self._add_server(self.prefix + "BCDE" + '2', site2)
+        self._add_server(self.prefix + "BCDE" + '3', site2)
+
+        self._add_site_link(self.prefix + "link",
+                            [site1, site2, uncovered_dn])
+        self.samdb.transaction_commit()
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name1)
+        to_cover.sort()
+
+        self.assertEqual([], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name2)
+        to_cover.sort()
+
+        self.assertEqual([uncovered], to_cover)
+
+    def test_two_site_links_same_cost(self):
+        self.samdb.transaction_start()
+        site1, name1 = self._add_site(self.prefix + "ABCD")
+        site2, name2 = self._add_site(self.prefix + "BCDE")
+
+        uncovered_dn, uncovered = self._add_site(self.prefix + "uncovered")
+
+        self._add_server(self.prefix + "ABCD" + '1', site1)
+        self._add_server(self.prefix + "ABCD" + '2', site1)
+        self._add_server(self.prefix + "BCDE" + '1', site2)
+        self._add_server(self.prefix + "BCDE" + '2', site2)
+        self._add_server(self.prefix + "BCDE" + '3', site2)
+
+        self._add_site_link(self.prefix + "link1",
+                            [site1, uncovered_dn])
+        self._add_site_link(self.prefix + "link2",
+                            [site2, uncovered_dn])
+        self.samdb.transaction_commit()
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name1)
+        to_cover.sort()
+
+        self.assertEqual([uncovered], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name2)
+        to_cover.sort()
+
+        self.assertEqual([uncovered], to_cover)
+
+    def test_two_site_links_different_costs(self):
+        self.samdb.transaction_start()
+        site1, name1 = self._add_site(self.prefix + "ABCD")
+        site2, name2 = self._add_site(self.prefix + "BCDE")
+
+        uncovered_dn, uncovered = self._add_site(self.prefix + "uncovered")
+
+        self._add_server(self.prefix + "ABCD" + '1', site1)
+        self._add_server(self.prefix + "BCDE" + '1', site2)
+        self._add_server(self.prefix + "BCDE" + '2', site2)
+
+        self._add_site_link(self.prefix + "link1",
+                            [site1, uncovered_dn],
+                            cost=50)
+        self._add_site_link(self.prefix + "link2",
+                            [site2, uncovered_dn],
+                            cost=75)
+        self.samdb.transaction_commit()
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name1)
+        to_cover.sort()
+
+        self.assertEqual([uncovered], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name2)
+        to_cover.sort()
+
+        self.assertEqual([], to_cover)
+
+    def test_three_site_links_different_costs(self):
+        self.samdb.transaction_start()
+        site1, name1 = self._add_site(self.prefix + "ABCD")
+        site2, name2 = self._add_site(self.prefix + "BCDE")
+        site3, name3 = self._add_site(self.prefix + "CDEF")
+
+        uncovered_dn, uncovered = self._add_site(self.prefix + "uncovered")
+
+        self._add_server(self.prefix + "ABCD" + '1', site1)
+        self._add_server(self.prefix + "BCDE" + '1', site2)
+        self._add_server(self.prefix + "CDEF" + '1', site3)
+        self._add_server(self.prefix + "CDEF" + '2', site3)
+
+        self._add_site_link(self.prefix + "link1",
+                            [site1, uncovered_dn],
+                            cost=50)
+        self._add_site_link(self.prefix + "link2",
+                            [site2, uncovered_dn],
+                            cost=75)
+        self._add_site_link(self.prefix + "link3",
+                            [site3, uncovered_dn],
+                            cost=60)
+        self.samdb.transaction_commit()
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name1)
+        to_cover.sort()
+
+        self.assertEqual([uncovered], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name2)
+        to_cover.sort()
+
+        self.assertEqual([], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name3)
+        to_cover.sort()
+
+        self.assertEqual([], to_cover)
+
+    def test_three_site_links_different_costs(self):
+        self.samdb.transaction_start()
+        site1, name1 = self._add_site(self.prefix + "ABCD")
+        site2, name2 = self._add_site(self.prefix + "BCDE")
+        site3, name3 = self._add_site(self.prefix + "CDEF")
+
+        uncovered_dn, uncovered = self._add_site(self.prefix + "uncovered")
+
+        self._add_server(self.prefix + "ABCD" + '1', site1)
+        self._add_server(self.prefix + "BCDE" + '1', site2)
+        self._add_server(self.prefix + "CDEF" + '1', site3)
+        self._add_server(self.prefix + "CDEF" + '2', site3)
+
+        self._add_site_link(self.prefix + "link1",
+                            [site1, uncovered_dn],
+                            cost=50)
+        self._add_site_link(self.prefix + "link2",
+                            [site2, uncovered_dn],
+                            cost=75)
+        self._add_site_link(self.prefix + "link3",
+                            [site3, uncovered_dn],
+                            cost=50)
+        self.samdb.transaction_commit()
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name1)
+        to_cover.sort()
+
+        self.assertEqual([uncovered], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name2)
+        to_cover.sort()
+
+        self.assertEqual([], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name3)
+        to_cover.sort()
+
+        self.assertEqual([uncovered], to_cover)
+
+    def test_complex_setup_with_multiple_uncovered_sites(self):
+        self.samdb.transaction_start()
+        site1, name1 = self._add_site(self.prefix + "ABCD")
+        site2, name2 = self._add_site(self.prefix + "BCDE")
+        site3, name3 = self._add_site(self.prefix + "CDEF")
+
+        site4, name4 = self._add_site(self.prefix + "1234")
+        site5, name5 = self._add_site(self.prefix + "2345")
+        site6, name6 = self._add_site(self.prefix + "3456")
+
+        uncovered_dn1, uncovered1 = self._add_site(self.prefix + "uncovered1")
+        uncovered_dn2, uncovered2 = self._add_site(self.prefix + "uncovered2")
+        uncovered_dn3, uncovered3 = self._add_site(self.prefix + "uncovered3")
+
+        # Site Link Cluster 1 - Server List
+        self._add_server(self.prefix + "ABCD" + '1', site1)
+
+        self._add_server(self.prefix + "BCDE" + '1', site2)
+        self._add_server(self.prefix + "BCDE" + '2', site2)
+
+        self._add_server(self.prefix + "CDEF" + '1', site3)
+        self._add_server(self.prefix + "CDEF" + '2', site3)
+        self._add_server(self.prefix + "CDEF" + '3', site3)
+
+        # Site Link Cluster 2 - Server List
+        self._add_server(self.prefix + "1234" + '1', site4)
+        self._add_server(self.prefix + "1234" + '2', site4)
+
+        self._add_server(self.prefix + "2345" + '1', site5)
+        self._add_server(self.prefix + "2345" + '2', site5)
+
+        self._add_server(self.prefix + "3456" + '1', site6)
+
+        # Join to Uncovered1 (preference to site link cluster 1)
+        self._add_site_link(self.prefix + "link1A",
+                            [site1, site2, site3, uncovered_dn1],
+                            cost=49)
+        self._add_site_link(self.prefix + "link2A",
+                            [site4, site5, site6, uncovered_dn1],
+                            cost=50)
+
+        # Join to Uncovered2 (no preferene on site links)
+        self._add_site_link(self.prefix + "link1B",
+                            [site1, site2, site3, uncovered_dn2],
+                            cost=50)
+        self._add_site_link(self.prefix + "link2B",
+                            [site4, site5, site6, uncovered_dn2],
+                            cost=50)
+
+        # Join to Uncovered3 (preference to site link cluster 2)
+        self._add_site_link(self.prefix + "link1C",
+                            [site1, site2, site3, uncovered_dn3],
+                            cost=50)
+        self._add_site_link(self.prefix + "link2C",
+                            [site4, site5, site6, uncovered_dn3],
+                            cost=49)
+
+        self.samdb.transaction_commit()
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name1)
+        to_cover.sort()
+
+        self.assertEqual([], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name2)
+        to_cover.sort()
+
+        self.assertEqual([], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name3)
+        to_cover.sort()
+
+        self.assertEqual([uncovered1, uncovered2], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name4)
+        to_cover.sort()
+
+        self.assertEqual([uncovered2, uncovered3], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name5)
+        to_cover.sort()
+
+        self.assertEqual([], to_cover)
+
+        to_cover = uncovered_sites_to_cover(self.samdb, name6)
+        to_cover.sort()
+
+        self.assertEqual([], to_cover)
+
+        for to_check in [uncovered1, uncovered2, uncovered3]:
+            to_cover = uncovered_sites_to_cover(self.samdb, to_check)
+            to_cover.sort()
+
+            self.assertEqual([], to_cover)
diff --git a/selftest/tests.py b/selftest/tests.py
index e69bc31..ccd184f 100644
--- a/selftest/tests.py
+++ b/selftest/tests.py
@@ -147,7 +147,6 @@ planpythontestsuite("none", "samba.tests.ntacls")
 planpythontestsuite("none", "samba.tests.policy")
 planpythontestsuite("none", "samba.tests.kcc.graph")
 planpythontestsuite("none", "samba.tests.kcc.graph_utils")
-planpythontestsuite("none", "samba.tests.kcc.kcc_utils")
 planpythontestsuite("none", "samba.tests.kcc.ldif_import_export")
 planpythontestsuite("none", "samba.tests.graph")
 plantestsuite("wafsamba.duplicate_symbols", "none", [os.path.join(srcdir(), "buildtools/wafsamba/test_duplicate_symbol.sh")])
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index ef752a5..a5543b5 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -997,6 +997,8 @@ for env in [
                            )
     planpythontestsuite(env, "samba.tests.samba_tool.visualize_drs")
 
+planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.kcc.kcc_utils")
+
 for env in [ "simpleserver", "fileserver", "nt4_dc", "ad_dc", "ad_dc_ntvfs", "ad_member"]:
     planoldpythontestsuite(env, "netlogonsvc",
                            extra_path=[os.path.join(srcdir(), 'python/samba/tests')],
-- 
1.9.1


From e664e684ce72e9c3ee6b5fb876c3e9de6693a72e Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 19 Mar 2018 16:50:36 +1300
Subject: [PATCH 7/8] tests/samba_dnsupdate: Add a trivial test of automatic
 site coverage

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 python/samba/tests/blackbox/samba_dnsupdate.py | 54 ++++++++++++++++++++++++++
 selftest/knownfail.d/autosite_coverage         |  1 +
 2 files changed, 55 insertions(+)
 create mode 100644 selftest/knownfail.d/autosite_coverage

diff --git a/python/samba/tests/blackbox/samba_dnsupdate.py b/python/samba/tests/blackbox/samba_dnsupdate.py
index 7ddaab7..e6cad3b 100644
--- a/python/samba/tests/blackbox/samba_dnsupdate.py
+++ b/python/samba/tests/blackbox/samba_dnsupdate.py
@@ -17,6 +17,12 @@
 #
 
 import samba.tests
+from StringIO import StringIO
+from samba.netcmd.main import cmd_sambatool
+from samba.credentials import Credentials
+from samba.auth import system_session
+from samba.samdb import SamDB
+import ldb
 
 class SambaDnsUpdateTests(samba.tests.BlackboxTestCase):
     """Blackbox test case for samba_dnsupdate."""
@@ -57,3 +63,51 @@ class SambaDnsUpdateTests(samba.tests.BlackboxTestCase):
         self.assertTrue(" DNS deletes needed" in rpc_out, rpc_out)
         out = self.check_output("samba_dnsupdate --verbose")
         self.assertTrue("No DNS updates needed" in out, out + rpc_out)
+
+    def test_add_new_uncovered_site(self):
+        name = 'sites'
+        cmd = cmd_sambatool.subcommands[name]
+        cmd.outf = StringIO()
+        cmd.errf = StringIO()
+
+        site_name = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'
+
+        # Clear out any existing site
+        cmd._run("samba-tool %s" % name, 'remove', site_name)
+
+        result = cmd._run("samba-tool %s" % name, 'create', site_name)
+        if result is not None:
+            self.fail("Error creating new site")
+
+        self.lp = samba.tests.env_loadparm()
+        self.creds = Credentials()
+        self.creds.guess(self.lp)
+        self.session = system_session()
+
+        self.samdb = SamDB(session_info=self.session,
+                           credentials=self.creds,
+                           lp=self.lp)
+
+        m = ldb.Message()
+        m.dn = ldb.Dn(self.samdb, 'CN=DEFAULTIPSITELINK,CN=IP,'
+                      'CN=Inter-Site Transports,CN=Sites,{}'.format(
+                          self.samdb.get_config_basedn()))
+        m['siteList'] = ldb.MessageElement("CN={},CN=Sites,{}".format(
+            site_name,
+            self.samdb.get_config_basedn()),
+            ldb.FLAG_MOD_ADD, "siteList")
+
+        out = self.check_output("samba_dnsupdate --verbose")
+        self.assertTrue("No DNS updates needed" in out, out)
+
+        self.samdb.modify(m)
+
+        out = self.check_output("samba_dnsupdate --verbose --use-samba-tool"
+                                " --rpc-server-ip={}".format(self.server_ip))
+
+        self.assertFalse("No DNS updates needed" in out, out)
+        self.assertTrue(site_name.lower() in out, out)
+
+        result = cmd._run("samba-tool %s" % name, 'remove', site_name)
+        if result is not None:
+            self.fail("Error deleting site")
diff --git a/selftest/knownfail.d/autosite_coverage b/selftest/knownfail.d/autosite_coverage
new file mode 100644
index 0000000..1dd2df4
--- /dev/null
+++ b/selftest/knownfail.d/autosite_coverage
@@ -0,0 +1 @@
+^samba.tests.blackbox.samba_dnsupdate.samba.tests.blackbox.samba_dnsupdate.SambaDnsUpdateTests.test_add_new_uncovered_site
-- 
1.9.1


From 707a4ad86625995107e86538b79b73279f520cd7 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 12 Mar 2018 14:45:48 +1300
Subject: [PATCH 8/8] samba_dnsupdate: Introduce automatic site coverage

This uses the underlying function in kcc_utils.py which already has
tests.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 selftest/knownfail.d/autosite_coverage |  1 -
 source4/scripting/bin/samba_dnsupdate  | 27 +++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)
 delete mode 100644 selftest/knownfail.d/autosite_coverage

diff --git a/selftest/knownfail.d/autosite_coverage b/selftest/knownfail.d/autosite_coverage
deleted file mode 100644
index 1dd2df4..0000000
--- a/selftest/knownfail.d/autosite_coverage
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.blackbox.samba_dnsupdate.samba.tests.blackbox.samba_dnsupdate.SambaDnsUpdateTests.test_add_new_uncovered_site
diff --git a/source4/scripting/bin/samba_dnsupdate b/source4/scripting/bin/samba_dnsupdate
index ac6cf61..d6f77d4 100755
--- a/source4/scripting/bin/samba_dnsupdate
+++ b/source4/scripting/bin/samba_dnsupdate
@@ -24,6 +24,7 @@ import fcntl
 import sys
 import tempfile
 import subprocess
+from samba.kcc import kcc_utils
 
 # ensure we get messages out immediately, so they get in the samba logs,
 # and don't get swallowed by a timeout
@@ -48,6 +49,7 @@ from samba.samdb import SamDB
 from samba.dcerpc import netlogon, winbind
 from samba.netcmd.dns import cmd_dns
 from samba import gensec
+import ldb
 
 samba.ensure_third_party_module("dns", "dnspython")
 import dns.resolver
@@ -775,9 +777,15 @@ for line in cfile:
         cache_list.append(c)
         cache_set.add(str(c))
 
+site_specific_rec = []
+
 # read each line, and check that the DNS name exists
 for line in file:
     line = line.strip()
+
+    if '${SITE}' in line:
+        site_specific_rec.append(line)
+
     if line == '' or line[0] == "#":
         continue
     d = parse_dns_line(line, sub_vars)
@@ -791,6 +799,25 @@ for line in file:
         dns_list.append(d)
         dup_set.add(str(d))
 
+# Perform automatic site coverage by default
+auto_coverage = True
+
+if not am_rodc and auto_coverage:
+    site_names = kcc_utils.uncovered_sites_to_cover(samdb,
+                                                    samdb.server_site_name())
+
+    # Duplicate all site specific records for the uncovered site
+    for site in site_names:
+        to_add = [samba.substitute_var(line, {'SITE': site})
+                  for line in site_specific_rec]
+
+        for site_line in to_add:
+            d = parse_dns_line(site_line,
+                               sub_vars=sub_vars)
+            if d is not None and str(d) not in dup_set:
+                dns_list.append(d)
+                dup_set.add(str(d))
+
 # now expand the entries, if any are A record with ip set to $IP
 # then replace with multiple entries, one for each interface IP
 for d in dns_list:
-- 
1.9.1



More information about the samba-technical mailing list