Merge lp:~mpontillo/maas/remove-iscpy-1.8 into lp:maas/1.8

[Mike Pontillo]

Remove dependency on iscpy. Add MAAS utility (based on iscpy) for parsing named.options. Add tests for iscpy-derived code.

See https://code.launchpad.net/~mpontillo/maas/remove-iscpy-1.7/+merge/263450

[Mike Pontillo (mpontillo)]

This is the same code that was approved for 1.7 and trunk, and landed on trunk. (Self-review might be appropriate in this circumstance?)

[Mike Pontillo (mpontillo)]

Self-approving. 1.7 and trunk have already landed. I don't want 1.8 to feel left out. =)

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~mpontillo/maas/remove-iscpy-1.8 into lp:maas/1.8 failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Get:2 http://security.ubuntu.com trusty-security Release [63.5 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [63.5 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [87.4 kB]
Get:6 http://security.ubuntu.com trusty-security/universe Sources [28.1 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [304 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:8 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [212 kB]
Get:9 http://security.ubuntu.com trusty-security/universe amd64 Packages [111 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [123 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [562 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [292 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Fetched 1,848 kB in 3s (600 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm pep8 phantomjs postgresql pyflakes python-apt python-bson python-bzrlib python-convoy python-coverage python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-iscpy python-jinja2 python-jsonschema python-lockfile python-lxml python-mock python-netaddr python-netifaces p...

=== modified file 'buildout.cfg'
--- buildout.cfg	2015-05-29 12:30:52 +0000
+++ buildout.cfg	2015-07-04 09:40:25 +0000
@@ -71,7 +71,6 @@
   djorm-ext-pgarray
   docutils
   crochet
-  iscpy
 entry-points =
   maas-region-admin=maasserver:execute_from_command_line
   twistd.region=twisted.scripts.twistd:run
 
=== modified file 'src/maasserver/management/commands/edit_named_options.py'
--- src/maasserver/management/commands/edit_named_options.py	2015-05-07 18:14:38 +0000
+++ src/maasserver/management/commands/edit_named_options.py	2015-07-04 09:40:25 +0000
@@ -28,9 +28,9 @@
     BaseCommand,
     CommandError,
 )
-from iscpy import (
-    MakeISC,
-    ParseISCString,
+from maasserver.utils.isc import (
+    make_isc_string,
+    parse_isc_string,
 )
 from provisioningserver.dns.config import MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME
 
@@ -61,12 +61,12 @@
         return options_file
 
     def parse_file(self, config_path, options_file):
-        """Read the named.conf.options file and parse it with iscpy.
+        """Read the named.conf.options file and parse it.
 
-        We also use iscpy to insert the include statement that we need.
+        Then insert the include statement that we need.
         """
         try:
-            config_dict = ParseISCString(options_file)
+            config_dict = parse_isc_string(options_file)
         except Exception as e:
             # Yes, it throws bare exceptions :(
             raise CommandError("Failed to parse %s: %s" % (
@@ -81,7 +81,7 @@
         return config_dict
 
     def set_up_include_statement(self, options_block, config_path):
-        """Insert the 'include' directive into the iscpy-parsed options."""
+        """Insert the 'include' directive into the parsed options."""
         dir = os.path.join(os.path.dirname(config_path), "maas")
         options_block['include'] = '"%s%s%s"' % (
             dir, os.path.sep, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
@@ -128,7 +128,7 @@
         self.set_up_include_statement(options_block, config_path)
         self.remove_forwarders(options_block)
         self.remove_dnssec_validation(options_block)
-        new_content = MakeISC(config_dict)
+        new_content = make_isc_string(config_dict)
 
         # Back up and write new file.
         self.back_up_existing_file(config_path)
 
=== added file 'src/maasserver/utils/isc.py'
--- src/maasserver/utils/isc.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/utils/isc.py	2015-07-04 09:40:25 +0000
@@ -0,0 +1,283 @@
+# Copyright (c) 2009, Purdue University
+# Copyright (c) 2015, Canonical Ltd.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+# Redistributions of source code must retain the above copyright notice, this
+# list of conditions and the following disclaimer.
+#
+# Redistributions in binary form must reproduce the above copyright notice,
+# this list of conditions and the following disclaimer in the documentation
+# and/or other materials provided with the distribution.
+#
+# Neither the name of the Purdue University nor the names of its contributors
+# may be used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+# POSSIBILITY OF SUCH DAMAGE.
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+)
+
+str = None
+
+__metaclass__ = type
+__all__ = [
+    'make_isc_string',
+    'parse_isc_string',
+]
+
+import copy
+
+
+def _clip(char_list):
+    """Clips char_list to individual stanza.
+
+    Inputs:
+      char_list: partial of char_list from _parse_tokens
+
+    Outputs:
+      tuple: (int: skip to char list index, list: shortened char_list)
+    """
+    assert char_list[0] == '{'
+    char_list.pop(0)
+    skip = 0
+    for index, item in enumerate(char_list):
+        if item == '{':
+            skip += 1
+        elif item == '}' and skip == 0:
+            return index, char_list[:index]
+        elif item == '}':
+            skip -= 1
+    raise Exception("Invalid brackets.")
+
+
+def _parse_tokens(char_list):
+    """Parses exploded isc named.conf portions.
+
+    Inputs:
+      char_list: List of isc file parts
+
+    Outputs:
+      dict: fragment or full isc file dict
+      Recursive dictionary of isc file, dict values can be of 3 types,
+      dict, string and bool. Boolean values are always true. Booleans are false
+      if key is absent. Booleans represent situations in isc files such as:
+        acl "registered" { 10.1.0/32; 10.1.1:/32;}}
+
+      Example:
+
+      {'stanza1 "new"': 'test_info', 'stanza1 "embedded"': {'acl "registered"':
+          {'10.1.0/32': True, '10.1.1/32': True}}}
+    """
+    index = 0
+    dictionary_fragment = {}
+    new_char_list = copy.deepcopy(char_list)
+    if type(new_char_list) == str:
+        return new_char_list
+    if type(new_char_list) == dict:
+        return new_char_list
+    last_open = None
+    continuous_line = False
+    temp_list = []
+
+    # Prevent "may be referenced before assignment" error
+    key = None
+
+    while index < len(new_char_list):
+        if new_char_list[index] == '{':
+            last_open = index
+        if new_char_list[index] == ';' and continuous_line:
+            dictionary_fragment = temp_list
+            temp_list = []
+            continuous_line = False
+        if new_char_list[index] == ';':
+            continuous_line = False
+        if (len(new_char_list) > index + 1 and
+           new_char_list[index] == '}' and
+           new_char_list[index + 1] != ';'):
+            skip, value = _clip(new_char_list[last_open:])
+            temp_list.append({key: copy.deepcopy(_parse_tokens(value))})
+            continuous_line = True
+        if len(new_char_list) > index + 1 and new_char_list[index + 1] == '{':
+            # assert key is not None
+            key = new_char_list.pop(index)
+            skip, dict_value = _clip(new_char_list[index:])
+            if continuous_line:
+                temp_list.append(
+                    {key: copy.deepcopy(_parse_tokens(dict_value))})
+            else:
+                dictionary_fragment[key] = copy.deepcopy(
+                    _parse_tokens(dict_value))
+            index += skip
+        else:
+            if len(new_char_list[
+                   index].split()) == 1 and '{' not in new_char_list:
+                for item in new_char_list:
+                    if item in [';']:
+                        continue
+                    dictionary_fragment[item] = True
+
+            # If there are more than 1 'keywords' at new_char_list[index]
+            # ex - "recursion no;"
+            elif len(new_char_list[index].split()) >= 2:
+                dictionary_fragment[
+                    new_char_list[index].split()[0]] = (
+                    ' '.join(new_char_list[index].split()[1:]))
+                index += 1
+
+            # If there is just 1 'keyword' at new_char_list[index]
+            # ex "recursion;" (not a valid option, but for example's sake it's
+            # fine)
+            elif new_char_list[index] not in ['{', ';', '}']:
+                key = new_char_list[index]
+                dictionary_fragment[key] = ''
+                index += 1
+            index += 1
+
+    return dictionary_fragment
+
+
+def _scrub_comments(isc_string):
+    """Clears comments from an isc file
+
+    Inputs:
+      isc_string: string of isc file
+    Outputs:
+      string: string of scrubbed isc file
+    """
+    isc_list = []
+    if isc_string is None:
+        return ''
+    expanded_comment = False
+    for line in isc_string.split('\n'):
+        no_comment_line = ""
+        # Vet out any inline comments
+        if '/*' in line.strip():
+            try:
+                striped_line = line.strip()
+                chars = enumerate(striped_line)
+                while True:
+                    i, c = chars.next()
+                    try:
+                        if c == '/' and striped_line[i + 1] == '*':
+                            expanded_comment = True
+                            chars.next()  # Skip '*'
+                            continue
+                        elif c == '*' and striped_line[i + 1] == '/':
+                            expanded_comment = False
+                            chars.next()  # Skip '/'
+                            continue
+                    except IndexError:
+                        continue  # We are at the end of the line
+                    if expanded_comment:
+                        continue
+                    else:
+                        no_comment_line += c
+            except StopIteration:
+                if no_comment_line:
+                    isc_list.append(no_comment_line)
+                continue
+
+        if expanded_comment:
+            if '*/' in line.strip():
+                expanded_comment = False
+                isc_list.append(line.split('*/')[-1])
+                continue
+            else:
+                continue
+        if line.strip().startswith(('#', '//')):
+            continue
+        else:
+            isc_list.append(line.split('#')[0].split('//')[0].strip())
+    return '\n'.join(isc_list)
+
+
+def _explode(isc_string):
+    """Explodes isc file into relevant tokens.
+
+    Inputs:
+      isc_string: String of isc file
+
+    Outputs:
+      list: list of isc file tokens delimited by brackets and semicolons
+        ['stanza1 "new"', '{', 'test_info', ';', '}']
+    """
+    str_array = []
+    temp_string = []
+    for char in isc_string:
+        if char in ['\n']:
+            continue
+        if char in ['{', '}', ';']:
+            if ''.join(temp_string).strip() == '':
+                str_array.append(char)
+            else:
+                str_array.append(''.join(temp_string).strip())
+                str_array.append(char)
+                temp_string = []
+        else:
+            temp_string.append(char)
+    return str_array
+
+
+def parse_isc_string(isc_string):
+    """Makes a dictionary from an ISC file string
+
+    Inputs:
+      isc_string: string of isc file
+
+    Outputs:
+      dict: dictionary of ISC file representation
+    """
+    return _parse_tokens(_explode(_scrub_comments(isc_string)))
+
+
+def make_isc_string(isc_dict, terminate=True):
+    """Outputs an isc formatted file string from a dict
+
+  Inputs:
+    isc_dict: a recursive dictionary to be turned into an isc file
+              (from ParseTokens)
+
+  Outputs:
+    str: string of isc file without indentation
+  """
+    if terminate:
+        terminator = ';'
+    else:
+        terminator = ''
+    if type(isc_dict) == str:
+        return isc_dict
+    isc_list = []
+    for option in isc_dict:
+        if type(isc_dict[option]) == bool:
+            isc_list.append('%s%s' % (option, terminator))
+        elif (type(isc_dict[option]) == str or
+              type(isc_dict[option]) == unicode):
+            isc_list.append('%s %s%s' % (option, isc_dict[option], terminator))
+        elif type(isc_dict[option]) == list:
+            new_list = []
+            for item in isc_dict[option]:
+                new_list.append(make_isc_string(item, terminate=False))
+            new_list[-1] = '%s%s' % (new_list[-1], terminator)
+            isc_list.append(
+                '%s { %s }%s' % (option, ' '.join(new_list), terminator))
+        elif type(isc_dict[option]) == dict:
+            isc_list.append('%s { %s }%s' % (
+                option, make_isc_string(isc_dict[option]), terminator))
+    return '\n'.join(isc_list)
 
=== added file 'src/maasserver/utils/tests/test_isc.py'
--- src/maasserver/utils/tests/test_isc.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/utils/tests/test_isc.py	2015-07-04 09:40:25 +0000
@@ -0,0 +1,179 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test ISC configuration file parser/generator."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+)
+
+str = None
+
+__metaclass__ = type
+__all__ = []
+
+
+from textwrap import dedent
+
+from maasserver.utils.isc import (
+    make_isc_string,
+    parse_isc_string,
+)
+from maastesting.testcase import MAASTestCase
+
+
+class TestParseISCString(MAASTestCase):
+
+    def test_parses_simple_bind_options(self):
+        testdata = dedent("""\
+            options {
+                directory "/var/cache/bind";
+
+                dnssec-validation auto;
+
+                auth-nxdomain no;    # conform to RFC1035
+                listen-on-v6 { any; };
+            };
+            """)
+        options = parse_isc_string(testdata)
+        self.assertEqual(
+            {u'options': {u'auth-nxdomain': u'no',
+                          u'directory': u'"/var/cache/bind"',
+                          u'dnssec-validation': u'auto',
+                          u'listen-on-v6': {u'any': True}}}, options)
+
+    def test_parses_bind_acl(self):
+        testdata = dedent("""\
+            acl goodclients {
+                192.0.2.0/24;
+                localhost;
+                localnets;
+            };
+            """)
+        acl = parse_isc_string(testdata)
+        self.assertEqual(
+            {u'acl goodclients': {u'192.0.2.0/24': True,
+                                  u'localhost': True,
+                                  u'localnets': True}}, acl)
+
+    def test_parses_multiple_forwarders(self):
+        testdata = dedent("""\
+            forwarders {
+                91.189.94.2;
+                91.189.94.3;
+                91.189.94.4;
+                91.189.94.5;
+                91.189.94.6;
+            };
+            """)
+        forwarders = parse_isc_string(testdata)
+        self.assertEqual(
+            {u'forwarders': {u'91.189.94.2': True,
+                             u'91.189.94.3': True,
+                             u'91.189.94.4': True,
+                             u'91.189.94.5': True,
+                             u'91.189.94.6': True}}, forwarders)
+
+    def test_parses_bug_1413388_config(self):
+        testdata = dedent("""\
+            acl canonical-int-ns { 91.189.90.151; 91.189.89.192;  };
+
+            options {
+                directory "/var/cache/bind";
+
+                forwarders {
+                    91.189.94.2;
+                    91.189.94.2;
+                };
+
+                dnssec-validation auto;
+
+                auth-nxdomain no;    # conform to RFC1035
+                listen-on-v6 { any; };
+
+                allow-query { any; };
+                allow-transfer { 10.222.64.1; canonical-int-ns; };
+
+                notify explicit;
+                also-notify { 91.189.90.151; 91.189.89.192;  };
+
+                allow-query-cache { 10.222.64.0/18; };
+                recursion yes;
+            };
+
+            zone "."  { type master; file "/etc/bind/db.special"; };
+            """)
+        config = parse_isc_string(testdata)
+        self.assertEqual(
+            {u'acl canonical-int-ns':
+             {u'91.189.89.192': True, u'91.189.90.151': True},
+             u'options': {u'allow-query': {u'any': True},
+                          u'allow-query-cache': {u'10.222.64.0/18': True},
+                          u'allow-transfer': {u'10.222.64.1': True,
+                                              u'canonical-int-ns': True},
+                          u'also-notify': {u'91.189.89.192': True,
+                                           u'91.189.90.151': True},
+                          u'auth-nxdomain': u'no',
+                          u'directory': u'"/var/cache/bind"',
+                          u'dnssec-validation': u'auto',
+                          u'forwarders': {u'91.189.94.2': True},
+                          u'listen-on-v6': {u'any': True},
+                          u'notify': u'explicit',
+                          u'recursion': u'yes'},
+             u'zone "."':
+             {u'file': u'"/etc/bind/db.special"', u'type': u'master'}},
+            config)
+
+    def test_parse_then_make_then_parse_generates_identical_config(self):
+        testdata = dedent("""\
+            acl canonical-int-ns { 91.189.90.151; 91.189.89.192;  };
+
+            options {
+                directory "/var/cache/bind";
+
+                forwarders {
+                    91.189.94.2;
+                    91.189.94.2;
+                };
+
+                dnssec-validation auto;
+
+                auth-nxdomain no;    # conform to RFC1035
+                listen-on-v6 { any; };
+
+                allow-query { any; };
+                allow-transfer { 10.222.64.1; canonical-int-ns; };
+
+                notify explicit;
+                also-notify { 91.189.90.151; 91.189.89.192;  };
+
+                allow-query-cache { 10.222.64.0/18; };
+                recursion yes;
+            };
+
+            zone "."  { type master; file "/etc/bind/db.special"; };
+            """)
+        config = parse_isc_string(testdata)
+        config_string = make_isc_string(config)
+        config = parse_isc_string(config_string)
+        self.assertEqual(
+            {u'acl canonical-int-ns':
+             {u'91.189.89.192': True, u'91.189.90.151': True},
+             u'options': {u'allow-query': {u'any': True},
+                          u'allow-query-cache': {u'10.222.64.0/18': True},
+                          u'allow-transfer': {u'10.222.64.1': True,
+                                              u'canonical-int-ns': True},
+                          u'also-notify': {u'91.189.89.192': True,
+                                           u'91.189.90.151': True},
+                          u'auth-nxdomain': u'no',
+                          u'directory': u'"/var/cache/bind"',
+                          u'dnssec-validation': u'auto',
+                          u'forwarders': {u'91.189.94.2': True},
+                          u'listen-on-v6': {u'any': True},
+                          u'notify': u'explicit',
+                          u'recursion': u'yes'},
+             u'zone "."':
+             {u'file': u'"/etc/bind/db.special"', u'type': u'master'}},
+            config)
 
=== modified file 'versions.cfg'
--- versions.cfg	2015-05-06 09:51:19 +0000
+++ versions.cfg	2015-07-04 09:40:25 +0000
@@ -35,7 +35,6 @@
 extras = 0.0.3
 fixtures = 0.3.14
 httplib2 = 0.8
-iscpy = 1.05
 iso8601 = 0.1.4
 junitxml = 0.6
 nose = 1.3.1