bind: Minor fixes and style updates

actions/bind

@@ -27,41 +27,30 @@ from plinth import action_utils
 
 
 CONFIG_FILE = '/etc/bind/named.conf.options'
-default_file = '\n'+\
+
-'acl goodclients {\n'+\
+DEFAULT_CONFIG = '''
-'   localnets;\n'+\
+acl goodclients {
-'};\n'+\
+    localnets;
-'options {\n'+\
+};
-'directory "/var/cache/bind";\n'+\
+options {
-'\n'+\
+directory "/var/cache/bind";
-'recursion yes;\n'+\
+
-'allow-query { goodclients; };\n'+\
+recursion yes;
-'\n'+\
+allow-query { goodclients; };
-'// If there is a firewall between you and nameservers you want\n'+\
+
-'// to talk to, you may need to fix the firewall to allow multiple\n'+\
+forwarders {
-'// ports to talk.  See http://www.kb.cert.org/vuls/id/800113\n'+\
+8.8.8.8; 8.8.4.4;
-'\n'+\
+};
-'// If your ISP provided one or more IP addresses for stable\n'+\
+forward first;
-'// nameservers, you probably want to use them as forwarders.\n'+\
+
-'// Uncomment the following block, and insert the addresses replacing\n'+\
+dnssec-enable yes;
-"// the all-0's placeholder.\n"+\
+dnssec-validation auto;
-'\n'+\
+
-'forwarders {\n'+\
+auth-nxdomain no;    # conform to RFC1035
-'8.8.8.8; 8.8.4.4;\n'+\
+listen-on-v6 { any; };
-'};\n'+\
+};
-'forward first;\n'+\
+'''
-'\n'+\
+
-'dnssec-enable yes;\n'+\
-'//========================================================================\n'+\
-'// If BIND logs error messages about the root key being expired,\n'+\
-'// you will need to update your keys.  See https://www.isc.org/bind-keys\n'+\
-'//========================================================================\n'+\
-'dnssec-validation auto;\n'+\
-'\n'+\
-'auth-nxdomain no;    # conform to RFC1035\n'+\
-'listen-on-v6 { any; };\n'+\
-'};\n'+\
-'\n'
 
 def parse_arguments():
     """Return parsed command line arguments as dictionary"""
@@ -75,31 +64,30 @@ def parse_arguments():
     configure.add_argument('--enable-dnssec', choices=['true', 'false'],
                            help='Set DNSSEC true/false')
 
-    dns = subparsers.add_parser('dns', help='Set up DNS server')
+    dns = subparsers.add_parser('dns', help='Set DNS forwarders')
-    dns.add_argument('--set', help='Set DNS server')
+    dns.add_argument('--set', help='List of IP addresses, separated by space')
 
     return parser.parse_args()
 
 
 def subcommand_setup(_):
     """Setup BIND configuration."""
-    f = open(CONFIG_FILE, "w")
+    conf_file = open(CONFIG_FILE, "w")
-    f.write(default_file)
+    conf_file.write(DEFAULT_CONFIG)
-    f.close()
+    conf_file.close()
     action_utils.service_restart('bind9')
 
+
 def subcommand_dns(arguments):
     """Setting DNS servers"""
-
     if arguments.set:
-        set(arguments.set)
+        set_forwarders(arguments.set)
 
     action_utils.service_restart('bind9')
 
 
 def subcommand_configure(arguments):
     """Configure BIND."""
-
     if arguments.set_forwarding:
         set_forwarding(arguments.set_forwarding)
 
@@ -110,11 +98,12 @@ def subcommand_configure(arguments):
 
 
 def set_forwarding(choice):
+    """Enable or disable DNS forwarding."""
     data = [line.strip() for line in open(CONFIG_FILE, 'r')]
     flag = 0
     if choice == "false":
         if 'forwarders {' in data and not '// forwarders {' in data:
-            f = open(CONFIG_FILE, 'w')
+            conf_file = open(CONFIG_FILE, 'w')
             for line in data:
                 if 'forwarders {' in line and not '// forwarders {' in line:
                     flag = 1
@@ -123,12 +112,12 @@ def set_forwarding(choice):
                 if 'forward first' in line:
                     flag = 0
                 if "0.0.0.0" not in line:
-                    f.write(line+'\n')
+                    conf_file.write(line + '\n')
-            f.close()
+            conf_file.close()
 
     else:
         if '// forwarders {' in data:
-            f = open(CONFIG_FILE, 'w')
+            conf_file = open(CONFIG_FILE, 'w')
             for line in data:
                 if '// forwarders {' in line:
                     flag = 1
@@ -137,47 +126,50 @@ def set_forwarding(choice):
                 if 'forward first' in line:
                     flag = 0
                 if "0.0.0.0" not in line:
-                    f.write(line+'\n')
+                    conf_file.write(line + '\n')
-            f.close()
+            conf_file.close()
 
 
 def enable_dnssec(choice):
+    """Enable or disable DNSSEC."""
     data = [line.strip() for line in open(CONFIG_FILE, 'r')]
     if choice == "false":
         if '//dnssec-enable yes;' not in data:
-            f = open(CONFIG_FILE, 'w')
+            conf_file = open(CONFIG_FILE, 'w')
             for line in data:
                 if 'dnssec-enable yes;' in line:
                     line = '//' + line
-                f.write(line+'\n')
+                conf_file.write(line+'\n')
-            f.close()
+            conf_file.close()
 
     else:
         if '//dnssec-enable yes;' in data:
-            f = open(CONFIG_FILE, 'w')
+            conf_file = open(CONFIG_FILE, 'w')
             for line in data:
                 if '//dnssec-enable yes;' in line:
                     line = line[2:]
-                f.write(line+'\n')
+                conf_file.write(line + '\n')
-            f.close()
+            conf_file.close()
 
 
-def set(DNS):
+def set_forwarders(forwarders):
+    """Set DNS forwarders."""
     flag = 0
     data = [line.strip() for line in open(CONFIG_FILE, 'r')]
-    f = open(CONFIG_FILE, 'w')
+    conf_file = open(CONFIG_FILE, 'w')
     for line in data:
         if 'forwarders {' in line:
-            f.write(line+'\n')
+            conf_file.write(line + '\n')
-            for dns in DNS.split():
+            for dns in forwarders.split():
-                f.write(dns+'; ')
+                conf_file.write(dns + '; ')
-            f.write('\n')
+            conf_file.write('\n')
             flag = 1
-        elif '};' and flag == 1:
+        elif '};' in line and flag == 1:
+            conf_file.write(line + '\n')
             flag = 0
         elif flag == 0:
-            f.write(line+'\n')
+            conf_file.write(line + '\n')
-    f.close()
+    conf_file.close()
 
 
 def main():

plinth/modules/bind/__init__.py

@@ -21,16 +21,14 @@ Plinth module to configure BIND server
 
 import re
 
-from django.urls import reverse_lazy
 from django.utils.translation import ugettext_lazy as _
+from django.core.exceptions import ValidationError
 from django.core.validators import validate_ipv46_address
 
 from plinth import actions
 from plinth import action_utils
 from plinth import cfg
-from plinth import frontpage
 from plinth import service as service_module
-from plinth.views import ServiceView
 
 
 version = 1
@@ -73,12 +71,6 @@ def init():
             )
 
 
-class BindServiceView(ServiceView):
-    service_id = managed_services[0]
-    diagnostics_module_name = "bind"
-    description = description
-
-
 def setup(helper, old_version=None):
     """Install and configure the module."""
     helper.install(managed_packages)
@@ -151,10 +143,11 @@ def get_default():
     return conf
 
 
-def validate(IP):
+def validate(ips):
-    for ip in IP.split():
+    """Validate that ips is a list of IP addresses, separated by space."""
+    for ip_addr in ips.split():
         try:
-            validate_ipv46_address(ip)
+            validate_ipv46_address(ip_addr)
-        except:
+        except ValidationError:
             return False
     return True

plinth/modules/bind/forms.py

@@ -24,8 +24,6 @@ from django.utils.translation import ugettext_lazy as _
 
 from plinth.forms import ServiceForm
 
-from . import get_default
-
 
 class BindForm(ServiceForm):
     """BIND configuration form"""
@@ -41,4 +39,4 @@ class BindForm(ServiceForm):
 
     forwarders = forms.CharField(
         required=False,
-        help_text=_('Set new DNS server'))
+        help_text=_('A list of IP addresses, separated by space'))

plinth/modules/bind/views.py

@@ -65,9 +65,8 @@ class BindServiceView(ServiceView): # pylint: disable=too-many-ancestors
             messages.success(self.request,
                              _('Enable DNSSEC configuration updated'))
 
-
+        if old_config['forwarders'] != data['forwarders'] \
-
+           and old_config['forwarders'] is not '':
-        if old_config['forwarders'] != data['forwarders'] and old_config['forwarders'] is not '':
             if validate(data['forwarders']) is True:
                 actions.superuser_run(
                     'bind',
@@ -77,8 +76,10 @@ class BindServiceView(ServiceView): # pylint: disable=too-many-ancestors
             else:
                 messages.error(self.request,
                                _('Enter a valid IPv4 or IPv6 address.'))
-        elif old_config['forwarders'] is '' and old_config['forwarders'] != data['forwarders']:
+        elif old_config['forwarders'] is '' \
-            messages.error(self.request,
+             and old_config['forwarders'] != data['forwarders']:
+            messages.error(
+                self.request,
                 _('Enable forwarding to set forwarding DNS servers'))
 
         return super().form_valid(form)