From 12b8a4e96f1ce8d7bf5a365de15991114a2160e6 Mon Sep 17 00:00:00 2001 From: Tim Simmons Date: Tue, 2 Jun 2015 16:41:24 +0000 Subject: [PATCH] Agent: Optional middleware to rate limit NOTIFYs Currently, the Agent naively does an AXFR/backend call for every NOTIFY that it receives. It should be able to get a NOTIFY for a zone, and then ignore successive NOTIFYs for a time period, and then do the zone transfer, so as to catch all of the updates that came in. This middleware accomplishes that by implementing a small locking dictionary that doesn't allow more than one NOTIFY to kick off AXFR per zone, per process for a configurable time period. The Agent gracefully hanldes the situation where it's sleeping on a NOTIFY, and a DELETE zone come through. When the NOTIFY wakes up, and the zone is already gone, it refuses the NOTIFY because the domain doesn't exist. Change-Id: If5655f8da201202482fa8c44af9b1c8496bf3281 --- designate/agent/__init__.py | 3 ++ designate/agent/service.py | 2 + designate/dnsutils.py | 74 +++++++++++++++++++++++++++++ designate/tests/test_dnsutils.py | 69 +++++++++++++++++++++++++++ etc/designate/designate.conf.sample | 1 + 5 files changed, 149 insertions(+) diff --git a/designate/agent/__init__.py b/designate/agent/__init__.py index 030a8c090..54df108cd 100644 --- a/designate/agent/__init__.py +++ b/designate/agent/__init__.py @@ -40,6 +40,9 @@ OPTS = [ help='The backend driver to use'), cfg.StrOpt('transfer-source', default=None, help='An IP address to be used to fetch zones transferred in'), + cfg.FloatOpt('notify-delay', default=0.0, + help='Delay after a NOTIFY arrives for a zone that the Agent ' + 'will pause and drop subsequent NOTIFYs for that zone'), ] cfg.CONF.register_opts(OPTS, group='service:agent') diff --git a/designate/agent/service.py b/designate/agent/service.py index 62f356931..6ba48e069 100644 --- a/designate/agent/service.py +++ b/designate/agent/service.py @@ -43,6 +43,8 @@ class Service(service.DNSService, service.Service): def _dns_application(self): # Create an instance of the RequestHandler class application = handler.RequestHandler() + if cfg.CONF['service:agent'].notify_delay > 0.0: + application = dnsutils.LimitNotifyMiddleware(application) application = dnsutils.SerializationMiddleware(application) return application diff --git a/designate/dnsutils.py b/designate/dnsutils.py index 460de7f3d..aa5bfae44 100644 --- a/designate/dnsutils.py +++ b/designate/dnsutils.py @@ -16,6 +16,8 @@ import random import socket import base64 +import time +from threading import Lock import dns import dns.exception @@ -182,6 +184,78 @@ class TsigKeyring(object): return default +class ZoneLock(object): + """A Lock across all zones that enforces a rate limit on NOTIFYs""" + + def __init__(self, delay): + self.lock = Lock() + self.data = {} + self.delay = delay + + def acquire(self, zone): + with self.lock: + # If no one holds the lock for the zone, grant it + if zone not in self.data: + self.data[zone] = time.time() + return True + + # Otherwise, get the time that it was locked + locktime = self.data[zone] + now = time.time() + + period = now - locktime + + # If it has been locked for longer than the allowed period + # give the lock to the new requester + if period > self.delay: + self.data[zone] = now + return True + + LOG.debug('Lock for %(zone)s can\'t be releaesed for %(period)s' + 'seconds' % {'zone': zone, + 'period': str(self.delay - period)}) + + # Don't grant the lock for the zone + return False + + def release(self, zone): + # Release the lock + with self.lock: + try: + self.data.pop(zone) + except KeyError: + pass + + +class LimitNotifyMiddleware(DNSMiddleware): + """Middleware that rate limits NOTIFYs to the Agent""" + + def __init__(self, application): + super(LimitNotifyMiddleware, self).__init__(application) + + self.delay = cfg.CONF['service:agent'].notify_delay + self.locker = ZoneLock(self.delay) + + def process_request(self, request): + opcode = request.opcode() + if opcode != dns.opcode.NOTIFY: + return None + + zone_name = request.question[0].name.to_text() + + if self.locker.acquire(zone_name): + time.sleep(self.delay) + self.locker.release(zone_name) + return None + else: + LOG.debug('Threw away NOTIFY for %(zone)s, already ' + 'working on an update.' % {'zone': zone_name}) + response = dns.message.make_response(request) + # Provide an authoritative answer + response.flags |= dns.flags.AA + return (response,) + + def from_dnspython_zone(dnspython_zone): # dnspython never builds a zone with more than one SOA, even if we give # it a zonefile that contains more than one diff --git a/designate/tests/test_dnsutils.py b/designate/tests/test_dnsutils.py index dc40e9eab..3da05bf91 100644 --- a/designate/tests/test_dnsutils.py +++ b/designate/tests/test_dnsutils.py @@ -13,7 +13,11 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import mock from dns import zone as dnszone +import dns.message +import dns.rdatatype +import dns.rcode from designate import dnsutils from designate.tests import TestCase @@ -95,3 +99,68 @@ class TestUtils(TestCase): self.assertEqual(len(SAMPLES), len(zone.recordsets)) self.assertEqual('example.com.', zone.name) + + def test_zone_lock(self): + # Initialize a ZoneLock + lock = dnsutils.ZoneLock(0.1) + + # Ensure there's no lock for different zones + for zone_name in ['foo.com.', 'bar.com.', 'example.com.']: + self.assertEqual(True, lock.acquire(zone_name)) + + # Ensure a lock for successive calls for the same zone + self.assertEqual(True, lock.acquire('example2.com.')) + self.assertEqual(False, lock.acquire('example2.com.')) + + # Acquire, release, and reacquire + self.assertEqual(True, lock.acquire('example3.com.')) + lock.release('example3.com.') + self.assertEqual(True, lock.acquire('example3.com.')) + + def test_limit_notify_middleware(self): + # Set the delay + self.config(notify_delay=.1, + group='service:agent') + + # Initialize the middlware + placeholder_app = None + middleware = dnsutils.LimitNotifyMiddleware(placeholder_app) + + # Prepare a NOTIFY + zone_name = 'example.com.' + notify = dns.message.make_query(zone_name, dns.rdatatype.SOA) + notify.flags = 0 + notify.set_opcode(dns.opcode.NOTIFY) + notify.flags |= dns.flags.AA + + # Send the NOTIFY through the middleware + # No problem, middleware should return None to pass it on + self.assertEqual(middleware.process_request(notify), None) + + @mock.patch('designate.dnsutils.ZoneLock.acquire', return_value=False) + def test_limit_notify_middleware_no_acquire(self, acquire): + # Set the delay + self.config(notify_delay=.1, + group='service:agent') + + # Initialize the middlware + placeholder_app = None + middleware = dnsutils.LimitNotifyMiddleware(placeholder_app) + + # Prepare a NOTIFY + zone_name = 'example.com.' + notify = dns.message.make_query(zone_name, dns.rdatatype.SOA) + notify.flags = 0 + notify.set_opcode(dns.opcode.NOTIFY) + notify.flags |= dns.flags.AA + + # Make a response object to match the middleware's return + response = dns.message.make_response(notify) + # Provide an authoritative answer + response.flags |= dns.flags.AA + + # Send the NOTIFY through the middleware + # Lock can't be acquired, a NOTIFY is already being worked on + # so just return what would have come back for a successful NOTIFY + # This needs to be a one item tuple for the serialization middleware + self.assertEqual(middleware.process_request(notify), (response,)) diff --git a/etc/designate/designate.conf.sample b/etc/designate/designate.conf.sample index 0fe35d7f5..1062fd15a 100644 --- a/etc/designate/designate.conf.sample +++ b/etc/designate/designate.conf.sample @@ -197,6 +197,7 @@ debug = False #masters = 127.0.0.1:5354 #backend_driver = fake #transfer_source = None +#notify_delay = 0 #-----------------------