[PATCH]add dns cleanup subcommand for samba-tools
Garming Sam
garming at catalyst.net.nz
Wed Jan 31 03:35:12 UTC 2018
I'm okay with putting this patch in, but there's still the caveat which
isn't mentioned where removed dns host names which have no direct
records cannot be deleted from SRV records for instance. I would've
expected the subcommand to have removed those too. Would there be any
standard cases where the A record would be deleted but not those
referencing it? In any case, it seems like an expected behaviour.
I've attached some patches which I think make the command behave as I
would expect.
Cheers,
Garming
On 24/01/18 09:45, Andrew Bartlett wrote:
> Thanks Joe. This is looking great!
>
> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
>
> If I could get a second team reviewer please, it would be great to see
> this merged into master.
>
> Andrew Bartlett
>
>
-------------- next part --------------
From 929b66f609870c70557edcb4b43399ed6137c212 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Fri, 12 Jan 2018 14:14:00 +1300
Subject: [PATCH 1/6] samba-tool: add dns cleanup cmd
1. Add new command to cleanup dns records for a dns host name
2. Add test to verify the command is working
Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/netcmd/dns.py | 46 ++++++++++++++
python/samba/tests/samba_tool/dnscmd.py | 103 ++++++++++++++++++++++++++++++++
2 files changed, 149 insertions(+)
diff --git a/python/samba/netcmd/dns.py b/python/samba/netcmd/dns.py
index fd8db93..cbad2ef 100644
--- a/python/samba/netcmd/dns.py
+++ b/python/samba/netcmd/dns.py
@@ -15,6 +15,7 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
+import logging
import samba.getopt as options
from samba import WERRORError
@@ -26,6 +27,10 @@ from socket import AF_INET
from socket import AF_INET6
import shlex
+from samba import remove_dc
+from samba.samdb import SamDB
+from samba.auth import system_session
+
from samba.netcmd import (
Command,
CommandError,
@@ -1068,6 +1073,46 @@ class cmd_delete_record(Command):
self.outf.write('Record deleted successfully\n')
+class cmd_cleanup_record(Command):
+ """Cleanup DNS records for a DNS host.
+
+ example:
+
+ samba-tool dns cleanup dc1 dc1.samdom.test.site -U USER%PASSWORD
+
+ NOTE: This command doesn't delete the DNS records,
+ it only mark the `dNSTombstoned` attr as `TRUE`.
+ """
+
+ synopsis = '%prog <server> <dnshostname>'
+
+ takes_args = ['server', 'dnshostname']
+
+ takes_optiongroups = {
+ "sambaopts": options.SambaOptions,
+ "versionopts": options.VersionOptions,
+ "credopts": options.CredentialsOptions,
+ }
+
+ def run(self, server, dnshostname, sambaopts=None, credopts=None, versionopts=None, verbose=False, quiet=False):
+ lp = sambaopts.get_loadparm()
+ creds = credopts.get_credentials(lp)
+
+ logger = self.get_logger()
+ if verbose:
+ logger.setLevel(logging.DEBUG)
+ elif quiet:
+ logger.setLevel(logging.WARNING)
+ else:
+ logger.setLevel(logging.INFO)
+
+ samdb = SamDB(url="ldap://%s" % server,
+ session_info=system_session(),
+ credentials=creds, lp=lp)
+
+ remove_dc.remove_dns_references(samdb, logger, dnshostname)
+
+
class cmd_dns(SuperCommand):
"""Domain Name Service (DNS) management."""
@@ -1082,3 +1127,4 @@ class cmd_dns(SuperCommand):
subcommands['add'] = cmd_add_record()
subcommands['update'] = cmd_update_record()
subcommands['delete'] = cmd_delete_record()
+ subcommands['cleanup'] = cmd_cleanup_record()
diff --git a/python/samba/tests/samba_tool/dnscmd.py b/python/samba/tests/samba_tool/dnscmd.py
index 1712c0e..9b942e5 100644
--- a/python/samba/tests/samba_tool/dnscmd.py
+++ b/python/samba/tests/samba_tool/dnscmd.py
@@ -660,6 +660,109 @@ class DnsCmdTestCase(SambaToolCmdTest):
"A", self.testip, self.creds_string)
self.assertCmdFail(result)
+ def test_cleanup_record(self):
+ """
+ Test dns cleanup command is working fine.
+ """
+
+ # add a A record
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ 'testa', "A", self.testip, self.creds_string)
+
+ # the above A record points to this host
+ dnshostname = '{}.{}'.format('testa', self.zone.lower())
+
+ # add a CNAME record points to above host
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ 'testcname', "CNAME", dnshostname, self.creds_string)
+
+ # add a NS record
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ 'testns', "NS", dnshostname, self.creds_string)
+
+ # add a PTR record points to above host
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ 'testptr', "PTR", dnshostname, self.creds_string)
+
+ # add a SRV record points to above host
+ srv_record = "{} 65530 65530 65530".format(dnshostname)
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ 'testsrv', "SRV", srv_record, self.creds_string)
+
+ # cleanup record for this dns host
+ self.runsubcmd("dns", "cleanup", os.environ["SERVER"],
+ dnshostname, self.creds_string)
+
+ # all records should be marked as dNSTombstoned
+ for record_name in ['testa', 'testcname', 'testns', 'testptr', 'testsrv']:
+
+ records = self.samdb.search(
+ base="DC=DomainDnsZones,{}".format(self.samdb.get_default_basedn()),
+ scope=ldb.SCOPE_SUBTREE,
+ expression="(&(objectClass=dnsNode)(name={}))".format(record_name),
+ attrs=["dNSTombstoned"])
+
+ self.assertEqual(len(records), 1)
+ for record in records:
+ self.assertEqual(str(record['dNSTombstoned']), 'TRUE')
+
+ def test_cleanup_multi_srv_record(self):
+ """
+ Test dns cleanup command for multi-valued SRV record.
+
+ Steps:
+ - Add 2 A records host1 and host2
+ - Add a SRV record srv1 and points to both host1 and host2
+ - Run cleanup command for host1
+ - Check records for srv1, data for host1 should be gone and host2 is kept.
+ """
+
+ hosts = ['host1', 'host2'] # A record names
+ srv_name = 'srv1'
+
+ # add A records
+ for host in hosts:
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ host, "A", self.testip, self.creds_string)
+
+ # the above A record points to this host
+ dnshostname = '{}.{}'.format(host, self.zone.lower())
+
+ # add a SRV record points to above host
+ srv_record = "{} 65530 65530 65530".format(dnshostname)
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ srv_name, "SRV", srv_record, self.creds_string)
+
+ records = self.samdb.search(
+ base="DC=DomainDnsZones,{}".format(self.samdb.get_default_basedn()),
+ scope=ldb.SCOPE_SUBTREE,
+ expression="(&(objectClass=dnsNode)(name={}))".format(srv_name),
+ attrs=['dnsRecord'])
+ # should have 2 records here
+ self.assertEqual(len(records[0]['dnsRecord']), 2)
+
+ # cleanup record for dns host1
+ dnshostname1 = 'host1.{}'.format(self.zone.lower())
+ self.runsubcmd("dns", "cleanup", os.environ["SERVER"],
+ dnshostname1, self.creds_string)
+
+ records = self.samdb.search(
+ base="DC=DomainDnsZones,{}".format(self.samdb.get_default_basedn()),
+ scope=ldb.SCOPE_SUBTREE,
+ expression="(&(objectClass=dnsNode)(name={}))".format(srv_name),
+ attrs=['dnsRecord'])
+
+ # dnsRecord for host1 should be deleted
+ self.assertEqual(len(records[0]['dnsRecord']), 1)
+
+ # unpack data
+ dns_record_bin = records[0]['dnsRecord'][0]
+ dns_record_obj = ndr_unpack(dnsp.DnssrvRpcRecord, dns_record_bin)
+
+ # dnsRecord for host2 is still there and is the only one
+ dnshostname2 = 'host2.{}'.format(self.zone.lower())
+ self.assertEqual(dns_record_obj.data.nameTarget, dnshostname2)
+
def test_dns_wildcards(self):
"""
Ensure that DNS wild card entries can be added deleted and queried
--
1.9.1
From a0eec7ade3f7e71843677856982e21795211a94c Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 31 Jan 2018 11:52:34 +1300
Subject: [PATCH 2/6] remove_dc: Allow remove_dns_references to ignore missing
server names
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/netcmd/dns.py | 3 ++-
python/samba/remove_dc.py | 14 ++++++++++++--
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/python/samba/netcmd/dns.py b/python/samba/netcmd/dns.py
index cbad2ef..1ca68fb 100644
--- a/python/samba/netcmd/dns.py
+++ b/python/samba/netcmd/dns.py
@@ -1110,7 +1110,8 @@ class cmd_cleanup_record(Command):
session_info=system_session(),
credentials=creds, lp=lp)
- remove_dc.remove_dns_references(samdb, logger, dnshostname)
+ remove_dc.remove_dns_references(samdb, logger, dnshostname,
+ ignore_no_name=True)
class cmd_dns(SuperCommand):
diff --git a/python/samba/remove_dc.py b/python/samba/remove_dc.py
index 4c8ee89..f273a51 100644
--- a/python/samba/remove_dc.py
+++ b/python/samba/remove_dc.py
@@ -84,7 +84,7 @@ def remove_sysvol_references(samdb, logger, dc_name):
raise
-def remove_dns_references(samdb, logger, dnsHostName):
+def remove_dns_references(samdb, logger, dnsHostName, ignore_no_name=False):
# Check we are using in-database DNS
zones = samdb.search(base="", scope=ldb.SCOPE_SUBTREE,
@@ -100,7 +100,11 @@ def remove_dns_references(samdb, logger, dnsHostName):
(dn, primary_recs) = samdb.dns_lookup(dnsHostName)
except RuntimeError as (enum, estr):
if enum == werror.WERR_DNS_ERROR_NAME_DOES_NOT_EXIST:
- return
+ if ignore_no_name:
+ remove_hanging_dns_references(samdb, logger,
+ dnsHostNameUpper,
+ zones)
+ return
raise DemoteException("lookup of %s failed: %s" % (dnsHostName, estr))
samdb.dns_replace(dnsHostName, [])
@@ -154,6 +158,11 @@ def remove_dns_references(samdb, logger, dnsHostName):
(a_name, len(a_recs), orig_num_recs - len(a_recs)))
samdb.dns_replace(a_name, a_recs)
+ remove_hanging_dns_references(samdb, logger, dnsHostNameUpper, zones)
+
+
+def remove_hanging_dns_references(samdb, logger, dnsHostNameUpper, zones):
+
# Find all the CNAME, NS, PTR and SRV records that point at the
# name we are removing
@@ -194,6 +203,7 @@ def remove_dns_references(samdb, logger, dnsHostName):
# has been done in the list comprehension above
samdb.dns_replace_by_dn(record.dn, values)
+
def offline_remove_server(samdb, logger,
server_dn,
remove_computer_obj=False,
--
1.9.1
From a10e69f067263985cf624c9289f6795a4c5aadb4 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 31 Jan 2018 11:53:40 +1300
Subject: [PATCH 3/6] tests/samba-tool: dns cleanup should work with a missing
name
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/tests/samba_tool/dnscmd.py | 50 +++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/python/samba/tests/samba_tool/dnscmd.py b/python/samba/tests/samba_tool/dnscmd.py
index 9b942e5..780af4f 100644
--- a/python/samba/tests/samba_tool/dnscmd.py
+++ b/python/samba/tests/samba_tool/dnscmd.py
@@ -706,6 +706,56 @@ class DnsCmdTestCase(SambaToolCmdTest):
for record in records:
self.assertEqual(str(record['dNSTombstoned']), 'TRUE')
+ def test_cleanup_record_no_A_record(self):
+ """
+ Test dns cleanup command works with no A record.
+ """
+
+ # add a A record
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ 'notesta', "A", self.testip, self.creds_string)
+
+ # the above A record points to this host
+ dnshostname = '{}.{}'.format('testa', self.zone.lower())
+
+ # add a CNAME record points to above host
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ 'notestcname', "CNAME", dnshostname, self.creds_string)
+
+ # add a NS record
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ 'notestns', "NS", dnshostname, self.creds_string)
+
+ # add a PTR record points to above host
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ 'notestptr', "PTR", dnshostname, self.creds_string)
+
+ # add a SRV record points to above host
+ srv_record = "{} 65530 65530 65530".format(dnshostname)
+ self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
+ 'notestsrv', "SRV", srv_record, self.creds_string)
+
+ # Remove the initial A record (leading to hanging references)
+ self.runsubcmd("dns", "delete", os.environ["SERVER"], self.zone,
+ 'notesta', "A", self.testip, self.creds_string)
+
+ # cleanup record for this dns host
+ self.runsubcmd("dns", "cleanup", os.environ["SERVER"],
+ dnshostname, self.creds_string)
+
+ # all records should be marked as dNSTombstoned
+ for record_name in ['notestcname', 'notestns', 'notestptr', 'notestsrv']:
+
+ records = self.samdb.search(
+ base="DC=DomainDnsZones,{}".format(self.samdb.get_default_basedn()),
+ scope=ldb.SCOPE_SUBTREE,
+ expression="(&(objectClass=dnsNode)(name={}))".format(record_name),
+ attrs=["dNSTombstoned"])
+
+ self.assertEqual(len(records), 1)
+ for record in records:
+ self.assertEqual(str(record['dNSTombstoned']), 'TRUE')
+
def test_cleanup_multi_srv_record(self):
"""
Test dns cleanup command for multi-valued SRV record.
--
1.9.1
From 4ffbcbdd797091c60027f2fa20a9a1a93b1442e3 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 31 Jan 2018 16:12:05 +1300
Subject: [PATCH 4/6] samba-tool/dns: Clarify the cleanup subcommand
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/netcmd/dns.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/python/samba/netcmd/dns.py b/python/samba/netcmd/dns.py
index 1ca68fb..78c1261 100644
--- a/python/samba/netcmd/dns.py
+++ b/python/samba/netcmd/dns.py
@@ -1080,8 +1080,9 @@ class cmd_cleanup_record(Command):
samba-tool dns cleanup dc1 dc1.samdom.test.site -U USER%PASSWORD
- NOTE: This command doesn't delete the DNS records,
- it only mark the `dNSTombstoned` attr as `TRUE`.
+ NOTE: This command in many cases will only mark the `dNSTombstoned` attr
+ as `TRUE` on the DNS records. Querying will no longer return results but
+ there may still be some placeholder entries in the database.
"""
synopsis = '%prog <server> <dnshostname>'
--
1.9.1
From 519b87bed4386f55e89e0f5f88a9f1186ac909a7 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 31 Jan 2018 16:13:14 +1300
Subject: [PATCH 5/6] samba-tool/tests: Check that dns cleanup does not
spuriously remove entries
This might happen in the multi-record case.
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/tests/samba_tool/dnscmd.py | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/python/samba/tests/samba_tool/dnscmd.py b/python/samba/tests/samba_tool/dnscmd.py
index 780af4f..bcd2c73 100644
--- a/python/samba/tests/samba_tool/dnscmd.py
+++ b/python/samba/tests/samba_tool/dnscmd.py
@@ -800,7 +800,7 @@ class DnsCmdTestCase(SambaToolCmdTest):
base="DC=DomainDnsZones,{}".format(self.samdb.get_default_basedn()),
scope=ldb.SCOPE_SUBTREE,
expression="(&(objectClass=dnsNode)(name={}))".format(srv_name),
- attrs=['dnsRecord'])
+ attrs=['dnsRecord', 'dNSTombstoned'])
# dnsRecord for host1 should be deleted
self.assertEqual(len(records[0]['dnsRecord']), 1)
@@ -813,6 +813,10 @@ class DnsCmdTestCase(SambaToolCmdTest):
dnshostname2 = 'host2.{}'.format(self.zone.lower())
self.assertEqual(dns_record_obj.data.nameTarget, dnshostname2)
+ # assert that the record isn't spuriously tombstoned
+ self.assertTrue('dNSTombstoned' not in records[0] or
+ str(record['dNSTombstoned']) == 'FALSE')
+
def test_dns_wildcards(self):
"""
Ensure that DNS wild card entries can be added deleted and queried
--
1.9.1
From 67a077b5de3fb9b75cf9f6e8b9ee34e8629047a9 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 31 Jan 2018 16:16:06 +1300
Subject: [PATCH 6/6] netcmd: Wrap an overly long line in dns.py
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/netcmd/dns.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/python/samba/netcmd/dns.py b/python/samba/netcmd/dns.py
index 78c1261..33f81ee 100644
--- a/python/samba/netcmd/dns.py
+++ b/python/samba/netcmd/dns.py
@@ -1095,7 +1095,8 @@ class cmd_cleanup_record(Command):
"credopts": options.CredentialsOptions,
}
- def run(self, server, dnshostname, sambaopts=None, credopts=None, versionopts=None, verbose=False, quiet=False):
+ def run(self, server, dnshostname, sambaopts=None, credopts=None,
+ versionopts=None, verbose=False, quiet=False):
lp = sambaopts.get_loadparm()
creds = credopts.get_credentials(lp)
--
1.9.1
More information about the samba-technical
mailing list