more cleanup, adding docstrings, bit more exception handling.

binder/helpers.py

@@ -1,4 +1,6 @@
-from binder import keyutils
+from binder import keyutils, exceptions
+# TODO: Start using exceptions here, force a record/add/delete on
+#        an unresponsive Bind server.
 
 import re
 import dns.query
@@ -15,10 +17,9 @@ def add_forward_record(dns_server, zone_name, record_name, record_type, record_d
 
     dns_update = dns.update.Update(zone_name, keyring = keyring)
     dns_update.replace(record_name, ttl, record_type, record_data)
+    output = dns.query.tcp(dns_update, dns_server)
 
-    response = dns.query.tcp(dns_update, dns_server)
+    return output
-
-    return response
 
 def add_reverse_record(dns_server, zone_name, record_name, record_data, ttl, keyring):
     """ Given passed arguments, add/update a reverse PTR record."""
@@ -66,10 +67,10 @@ def add_record(form_data):
 def add_cname_record(dns_server, zone_name, originating_record, cname, ttl, key_name):
     """Add a Cname record."""
 
-    if key_name is None:
+    if key_name == "None":
         keyring = None
     else:
-        this_key = models.Key.objects.get(name=str(key_name))
+        this_key = models.Key.objects.get(name=key_name)
         keyring = keyutils.create_keyring(this_key.name, this_key.data)
 
     update = dns.update.Update(zone_name, keyring = keyring)
@@ -98,6 +99,7 @@ def delete_record(form_data, rr_items):
         dns_update = dns.update.Update(domain, keyring = keyring)
         dns_update.delete(record)
         output = dns.query.tcp(dns_update, dns_server)
-        delete_response.append({ "description" : "Delete record %s" % current_rr_item, "output" : output })
+        delete_response.append({ "description" : "Delete record %s" % current_rr_item,
+                                 "output" : output })
 
     return delete_response

binder/keyutils.py

@@ -12,6 +12,6 @@ def create_keyring(key_name, key_data):
 
     keyring = dns.tsigkeyring.from_text({
             key_name : key_data
-    })
+            })
 
     return keyring

binder/models.py

@@ -1,27 +1,38 @@
 from BeautifulSoup import BeautifulStoneSoup as BS
-from django.db import models
 from binder import exceptions
+from django.db import models
 
 import dns.query
-import dns.zone
 import dns.tsig
+import dns.zone
 import keyutils
-import re
 import urllib2
 
-TSIG_ALGORITHMS = (('hmac-md5', 'MD5'),('hmac-sha1', 'SHA1'),('hmac-224', 'SHA224'),('hmac-sha256', 'SHA256'),('hmac-sha384', 'SHA384'),('hmac-sha512', 'SHA512'))
+TSIG_ALGORITHMS = (('hmac-md5', 'MD5'),
+                   ('hmac-sha1', 'SHA1'),
+                   ('hmac-sha256', 'SHA256'),
+                   ('hmac-sha384', 'SHA384'),
+                   ('hmac-sha512', 'SHA512'))
 
 class Key(models.Model):
-    name = models.CharField(max_length=50)
+    """ Class to store and reference TSIG keys.
-    data = models.CharField(max_length=150)
+
-    algorithm = models.CharField(max_length=200, choices=TSIG_ALGORITHMS)
+    TODO: Should/Can we encrypt these DNS keys in the DB?
+    """
+    name = models.CharField(max_length=255)
+    data = models.CharField(max_length=255)
+    algorithm = models.CharField(max_length=255, choices=TSIG_ALGORITHMS)
 
     def __unicode__(self):
         return self.name
 
 
 class BindServer(models.Model):
-    hostname = models.CharField(max_length=100)
+    """ Class to store DNS servers and attributes for referencing their
+        statistics ports. Also reference FK for TSIG transfer keys,
+        if required.
+    """
+    hostname = models.CharField(max_length=255)
     statistics_port = models.IntegerField()
     default_transfer_key = models.ForeignKey(Key, null=True, blank=True)
 
@@ -29,12 +40,16 @@ class BindServer(models.Model):
         return self.hostname
 
     def list_zones(self):
-        """ Take a DNS server, and list the DNS zones it provides resolution for. """
+        """ List the DNS zones and attributes.
-        # I should take the dns_hostname here, get the object from the DB,
+
-        # and use the status port attribute for the urllib2 query.
+        TODO: Parse these XML more intelligently. Grab the view name. Any other data available?
-        myreq = urllib2.Request("http://%s:%s" % (self.hostname, self.statistics_port))
+
+        Returns:
+          List of Dicts { String zone_name, String zone_serial }
+        """
+        zone_req  = urllib2.Request("http://%s:%s" % (self.hostname, self.statistics_port))
         try:
-            http_request = urllib2.urlopen(myreq)
+            http_request = urllib2.urlopen(zone_req)
         except urllib2.URLError, err:
             raise exceptions.ZoneException(err)
 
@@ -42,7 +57,7 @@ class BindServer(models.Model):
         xmloutput = http_request.read()
         mysoup = BS(xmloutput)
         zones = mysoup.findAll('zone')
-        for current_zone in zones: # Interate over found zones
+        for current_zone in zones:
             zone_name = current_zone.find("name").string.split("/IN")[0]
             zone_serial = current_zone.find("serial").string
             zone_class = current_zone.find("rdataclass").string
@@ -51,9 +66,18 @@ class BindServer(models.Model):
 
         return return_array
 
-    def list_zone_records(self, zone):
+    def list_zone_records(self, zone_name):
-        """Given a zone, produce an array of dicts containing
+        """ List all records in a specific zone.
-        each RR record and its attributes."""
+
+        TODO: Print out current_record in the loop and see if we can parse this more programatically,
+              rather than just splitting on space. What is the difference between class and type?
+        Arguments:
+          String zone_name: Name of the zone
+
+        Returns:
+          List of Dicts { String rr_name, String rr_ttl, String rr_class, String rr_type, String rr_data }
+        """
+
         if self.default_transfer_key:
             keyring = keyutils.create_keyring(self.default_transfer_key.name,
                                               self.default_transfer_key.data)
@@ -61,7 +85,7 @@ class BindServer(models.Model):
             keyring = None
 
         try:
-            zone = dns.zone.from_xfr(dns.query.xfr(self.hostname, zone, keyring=keyring))
+            zone = dns.zone.from_xfr(dns.query.xfr(self.hostname, zone_name, keyring=keyring))
         except dns.exception.FormError, err:
             raise exceptions.TransferException("There was an error attempting to list zone records.")
         except dns.tsig.PeerBadKey:
@@ -72,11 +96,16 @@ class BindServer(models.Model):
         record_array = []
         for current_name in names:
             current_record = zone[current_name].to_text(current_name)
-            for split_record in current_record.split("\n"): # Split the records on the newline
+            for split_record in current_record.split("\n"):
-                record_array.append({'rr_name'  : split_record.split(" ")[0],
+                current_record = split_record.split(" ")
-                                     'rr_ttl'  : split_record.split(" ")[1],
+                rr_dict = {}
-                                     'rr_class' : split_record.split(" ")[2],
+                rr_dict["rr_name"] = current_record[0]
-                                     'rr_type'  : split_record.split(" ")[3],
+                rr_dict["rr_ttl"] = current_record[1]
-                                     'rr_data'  : split_record.split(" ")[4]})
+                rr_dict["rr_class"] = current_record[2]
+                rr_dict["rr_type"] = current_record[3]
+                rr_dict["rr_data"] = current_record[4]
+
+                record_array.append(rr_dict)
+
         return record_array
 

binder/views.py

@@ -1,6 +1,5 @@
-from django.shortcuts import redirect, render
-
 from binder import exceptions, forms, helpers, models
+from django.shortcuts import redirect, render
 
 import re
 
@@ -8,12 +7,12 @@ RE_UNICODEARRAY = re.compile(r"u'(.*?)'")
 
 def home_index(request):
     """ List the main index page for Binder. """
-    return render(request, 'index.htm')
+    return render(request, "index.htm")
 
 def view_server_list(request):
-    """ List the DNS servers configured in the Django DB. """
+    """ List the DNS servers configured in the database. """
-    server_list = models.BindServer.objects.all().order_by('hostname')
+    server_list = models.BindServer.objects.all().order_by("hostname")
-    return render(request, 'bcommon/list_servers.htm',
+    return render(request, "bcommon/list_servers.htm",
                   { "server_list" : server_list})
 
 def view_server_zones(request, dns_server):
@@ -24,35 +23,35 @@ def view_server_zones(request, dns_server):
         this_server = models.BindServer.objects.get(hostname=dns_server)
         zone_array = this_server.list_zones()
     except models.BindServer.DoesNotExist, err:
-        errors = "Configured server does not exist: %s" % dns_server
+        errors = "There is no configured server by that name: %s" % dns_server
     except exceptions.ZoneException, err:
         errors = "Unable to list server zones. Error: %s" % err
 
-    return render(request, 'bcommon/list_server_zones.htm',
+    return render(request, "bcommon/list_server_zones.htm",
                   { "errors" : errors,
                     "dns_server" : dns_server,
                     "zone_array" : zone_array})
 
 def view_zone_records(request, dns_server, zone_name):
-    """ Display the list of records for a a particular zone."""
+    """ Display the list of records for a particular zone. """
+    # Have we tried here to transfer for a zone that does not exist?
+    # Try with a transfer key and without.
     try:
         this_server = models.BindServer.objects.get(hostname=dns_server)
         zone_array = this_server.list_zone_records(zone_name)
     except exceptions.TransferException, err:
-        return render(request, 'bcommon/list_zone.htm',
+        return render(request, "bcommon/list_zone.htm",
-                      { 'errors' : err,
+                      { "errors" : err,
-                        'zone_name' : zone_name})
+                        "zone_name" : zone_name})
 
-    return render(request, 'bcommon/list_zone.htm',
+    return render(request, "bcommon/list_zone.htm",
-                  { 'zone_array' : zone_array,
+                  { "zone_array" : zone_array,
-                    'dns_server' : dns_server,
+                    "dns_server" : dns_server,
-                    'zone_name' : zone_name})
+                    "zone_name" : zone_name})
 
 def view_add_record(request, dns_server, zone_name):
     """ View to provide form to add a DNS record. """
-    print "keys: %r" % models.Key.objects.all()
+    return render(request, "bcommon/add_record_form.htm",
-    form = forms.FormAddRecord()
-    return render(request, 'bcommon/add_record_form.htm',
                   { "dns_server" : dns_server,
                     "zone_name" : zone_name,
                     "tsig_keys" : models.Key.objects.all() })
@@ -61,21 +60,22 @@ def view_add_record_result(request):
     """ Process the input given to add a DNS record. """
     errors = ""
     if request.method == "GET":
-        return redirect('/')
+        return redirect("/")
 
     form = forms.FormAddRecord(request.POST)
     if form.is_valid():
-        cd = form.cleaned_data
+        form_cleaned = form.cleaned_data
         try:
-            add_record_response = helpers.add_record(cd)
+            add_record_response = helpers.add_record(form_cleaned)
         except exceptions.RecordException, err:
+            # TODO: Start using this exception.
             errors = err
 
-        return render(request, 'bcommon/response_result.htm',
+        return render(request, "bcommon/response_result.htm",
                       { "errors" : errors,
                         "response" : add_record_response })
 
-    return render(request, 'bcommon/add_record_form.htm',
+    return render(request, "bcommon/add_record_form.htm",
                   { "dns_server" : request.POST["dns_server"],
                     "zone_name" : request.POST["zone_name"],
                     "form_errors" : form.errors,
@@ -92,25 +92,22 @@ def view_add_cname_record(request, dns_server, zone_name, record_name):
 def view_add_cname_result(request):
     """ Process input on the CNAME form and provide a response."""
     if request.method == "GET":
-        return redirect('/')
+        return redirect("/")
 
-    form = forms.FormAddCnameRecord(request.POST)
     errors = ""
+    form = forms.FormAddCnameRecord(request.POST)
     if form.is_valid():
         cd = form.cleaned_data
-        try:
+        add_cname_response = helpers.add_cname_record(
-            add_cname_response = helpers.add_cname_record(
+            str(cd["dns_server"]),
-                str(cd["dns_server"]),
+            str(cd["zone_name"]),
-                str(cd["zone_name"]),
+            str(cd["originating_record"]),
-                str(cd["originating_record"]),
+            str(cd["cname"]),
-                str(cd["cname"]),
+            str(cd["ttl"]),
-                str(cd["ttl"]),
+            str(cd["key_name"]))
-                str(cd["key_name"]))
-        except:
-            print "hit exception in view_add_cname_result"
 
-        return render(request, 'bcommon/response_result.htm',
+        return render(request, "bcommon/response_result.htm",
-                      {'response' : add_cname_response })
+                      {"response" : add_cname_response })
 
     return render(request, "bcommon/add_cname_record_form.htm",
                   { "dns_server" : request.POST["dns_server"],
@@ -123,40 +120,37 @@ def view_add_cname_result(request):
 
 
 def view_delete_record(request):
-    """Provide the initial form for deleting records."""
+    """ Provide the initial form for deleting records. """
     if request.method == "GET":
         # Return home. You shouldn't trying to directly acces
         # the url for deleting records.
-        return redirect('/')
+        return redirect("/")
 
-    dns_server = request.POST['dns_server']
+    dns_server = request.POST["dns_server"]
-    zone_name = request.POST['zone_name']
+    zone_name = request.POST["zone_name"]
-    rr_array = request.POST.getlist('rr_array')
+    rr_array = request.POST.getlist("rr_array")
 
-    return render(request, 'bcommon/delete_record_initial.htm',
+    return render(request, "bcommon/delete_record_initial.htm",
-                  { 'dns_server' : dns_server,
+                  { "dns_server" : dns_server,
-                    'zone_name' : zone_name,
+                    "zone_name" : zone_name,
-                    'rr_array' :  rr_array,
+                    "rr_array" :  rr_array,
-                    'tsig_keys' : models.Key.objects.all() })
+                    "tsig_keys" : models.Key.objects.all() })
 
 
 def view_delete_result(request):
-    """ View that deletes records and returns the response."""
+    """ View that deletes records and returns the response. """
     if request.method == "GET":
         # Return home. You shouldn't trying to directly access
         # the url for deleting records.
-        return redirect('/')
+        return redirect("/")
 
     # What seems like an ugly hack to get around the fact
     # that the array comes back as Unicode values.
-    rr_unicode_array = request.POST.getlist('rr_array')[0]
+    # TODO: Can we make this cleaner?
+    rr_unicode_array = request.POST.getlist("rr_array")[0]
     rr_items = RE_UNICODEARRAY.findall(rr_unicode_array)
 
-    try:
+    delete_result = helpers.delete_record(request.POST, rr_items)
-        delete_result = helpers.delete_record(request.POST, rr_items)
-    except exceptions.RecordException, err:
-        return render(request, 'bcommon/response_result.htm.htm',
-                      { "errors" : err })
 
-    return render(request, 'bcommon/response_result.htm',
+    return render(request, "bcommon/response_result.htm",
-                  { 'response' : delete_result })
+                  { "response" : delete_result })