More adjustments/cleanups for the system configuration

helper objects.

1. Add in a parser for the /etc/hostname file that can be shared
2. Adjust the sysconfig configobj parser to not always quote
   fields that it does not need to quote + add in tests around
   this to ensure that we don't go nuts with quoting again.

9 files changed, 315 insertions, 107 deletions

diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py
index c8b13f95..0d5cbac7 100644
--- a/cloudinit/distros/debian.py
+++ b/cloudinit/distros/debian.py
@@ -27,7 +27,7 @@ from cloudinit import helpers
 from cloudinit import log as logging
 from cloudinit import util
 
-from cloudinit.distros.parsers import chop_comment
+from cloudinit.distros.parsers.hostname import HostnameConf
 
 from cloudinit.settings import PER_INSTANCE
 
@@ -84,27 +84,38 @@ class Distro(distros.Distro):
         self._write_hostname(hostname, self.hostname_conf_fn)
         self._apply_hostname(hostname)
 
-    def _write_hostname(self, hostname, out_fn):
-        # "" gives trailing newline.
-        hostname_lines = [
-            str(hostname),
-            "",
-        ]
-        util.write_file(out_fn, "\n".join(hostname_lines), 0644)
+    def _write_hostname(self, your_hostname, out_fn):
+        conf = self._read_hostname_conf(out_fn)
+        if not conf:
+            conf = HostnameConf('')
+            conf.parse()
+        conf.set_hostname(your_hostname)
+        util.write_file(out_fn, str(conf), 0644)
 
     def _read_system_hostname(self):
-        return (self.hostname_conf_fn,
-                self._read_hostname(self.hostname_conf_fn))
+        conf = self._read_hostname_conf(self.hostname_conf_fn)
+        if conf:
+            sys_hostname = conf.hostname
+        else:
+            sys_hostname = None
+        return (self.hostname_conf_fn, sys_hostname)
+
+    def _read_hostname_conf(self, filename):
+        try:
+            conf = HostnameConf(util.load_file(filename))
+            conf.parse()
+            return conf
+        except IOError:
+            util.logexc(LOG, "Error reading hostname from %s", filename)
+            return None
 
     def _read_hostname(self, filename, default=None):
-        contents = util.load_file(filename, quiet=True)
-        for line in contents.splitlines():
-            # Handle inline comments
-            (before_comment, _comment) = chop_comment(line, "#")
-            before_comment = before_comment.strip()
-            if len(before_comment):
-                return before_comment
-        return default
+        conf = self._read_hostname_conf(filename)
+        if not conf:
+            return default
+        if not conf.hostname:
+            return default
+        return conf.hostname
 
     def _get_localhost_ip(self):
         # Note: http://www.leonardoborda.com/blog/127-0-1-1-ubuntu-debian/
diff --git a/cloudinit/distros/parsers/hostname.py b/cloudinit/distros/parsers/hostname.py
new file mode 100644
index 00000000..7e19f017
--- /dev/null
+++ b/cloudinit/distros/parsers/hostname.py
@@ -0,0 +1,90 @@
+# vi: ts=4 expandtab
+#
+#    Copyright (C) 2012 Yahoo! Inc.
+#
+#    Author: Joshua Harlow <harlowja@yahoo-inc.com>
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU General Public License version 3, as
+#    published by the Free Software Foundation.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU General Public License for more details.
+#
+#    You should have received a copy of the GNU General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from StringIO import StringIO
+
+from cloudinit.distros.parsers import chop_comment
+
+
+# Parser that knows how to work with /etc/hostname format
+class HostnameConf(object):
+    def __init__(self, text):
+        self._text = text
+        self._contents = None
+
+    def parse(self):
+        if self._contents is None:
+            self._contents = self._parse(self._text)
+
+    def __str__(self):
+        self.parse()
+        contents = StringIO()
+        for (line_type, components) in self._contents:
+            if line_type == 'blank':
+                contents.write("%s\n" % (components[0]))
+            elif line_type == 'all_comment':
+                contents.write("%s\n" % (components[0]))
+            elif line_type == 'hostname':
+                (hostname, tail) = components
+                contents.write("%s%s\n" % (hostname, tail))
+        # Ensure trailing newline
+        contents = contents.getvalue()
+        if not contents.endswith("\n"):
+            contents += "\n"
+        return contents
+
+    @property
+    def hostname(self):
+        self.parse()
+        for (line_type, components) in self._contents:
+            if line_type == 'hostname':
+                return components[0]
+        return None
+
+    def set_hostname(self, your_hostname):
+        your_hostname = your_hostname.strip()
+        if not your_hostname:
+            return
+        self.parse()
+        replaced = False
+        for (line_type, components) in self._contents:
+            if line_type == 'hostname':
+                components[0] = str(your_hostname)
+                replaced = True
+        if not replaced:
+            self._contents.append(('hostname', [str(your_hostname), '']))
+
+    def _parse(self, contents):
+        entries = []
+        hostnames_found = set()
+        for line in contents.splitlines():
+            if not len(line.strip()):
+                entries.append(('blank', [line]))
+                continue
+            (head, tail) = chop_comment(line.strip(), '#')
+            if not len(head):
+                entries.append(('all_comment', [line]))
+                continue
+            entries.append(('hostname', [head, tail]))
+            hostnames_found.add(head)
+        if len(hostnames_found) > 1:
+            raise IOError("Multiple hostnames (%s) found!"
+                           % (hostnames_found))
+        return entries
+    
+    
diff --git a/cloudinit/distros/parsers/hosts.py b/cloudinit/distros/parsers/hosts.py
index 5374ab0b..958a7c31 100644
--- a/cloudinit/distros/parsers/hosts.py
+++ b/cloudinit/distros/parsers/hosts.py
@@ -23,6 +23,7 @@ from cloudinit.distros.parsers import chop_comment
 
 # See: man hosts
 # or http://unixhelp.ed.ac.uk/CGI/man-cgi?hosts
+# or http://tinyurl.com/6lmox3
 class HostsConf(object):
     def __init__(self, text):
         self._text = text
@@ -80,7 +81,7 @@ class HostsConf(object):
         contents = StringIO()
         for (line_type, components) in self._contents:
             if line_type == 'blank':
-                contents.write("%s\n")
+                contents.write("%s\n" % (components[0]))
             elif line_type == 'all_comment':
                 contents.write("%s\n" % (components[0]))
             elif line_type == 'option':
diff --git a/cloudinit/distros/parsers/quoting_conf.py b/cloudinit/distros/parsers/quoting_conf.py
deleted file mode 100644
index 953ccfe9..00000000
--- a/cloudinit/distros/parsers/quoting_conf.py
+++ /dev/null
@@ -1,80 +0,0 @@
-# vi: ts=4 expandtab
-#
-#    Copyright (C) 2012 Yahoo! Inc.
-#
-#    Author: Joshua Harlow <harlowja@yahoo-inc.com>
-#
-#    This program is free software: you can redistribute it and/or modify
-#    it under the terms of the GNU General Public License version 3, as
-#    published by the Free Software Foundation.
-#
-#    This program is distributed in the hope that it will be useful,
-#    but WITHOUT ANY WARRANTY; without even the implied warranty of
-#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-#    GNU General Public License for more details.
-#
-#    You should have received a copy of the GNU General Public License
-#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-# This library is used to parse/write
-# out the various sysconfig files edited
-#
-# It has to be slightly modified though
-# to ensure that all values are quoted
-# since these configs are usually sourced into
-# bash scripts...
-from configobj import ConfigObj
-
-# See: http://tiny.cc/oezbgw
-D_QUOTE_CHARS = {
-    "\"": "\\\"",
-    "(": "\\(",
-    ")": "\\)",
-    "$": '\$',
-    '`': '\`',
-}
-
-# This class helps adjust the configobj
-# writing to ensure that when writing a k/v
-# on a line, that they are properly quoted
-# and have no spaces between the '=' sign.
-# - This is mainly due to the fact that
-# the sysconfig scripts are often sourced
-# directly into bash/shell scripts so ensure
-# that it works for those types of use cases.
-class QuotingConfigObj(ConfigObj):
-    def __init__(self, lines):
-        ConfigObj.__init__(self, lines,
-                           interpolation=False,
-                           write_empty_values=True)
-
-    def _quote_posix(self, text):
-        if not text:
-            return ''
-        for (k, v) in D_QUOTE_CHARS.iteritems():
-            text = text.replace(k, v)
-        return '"%s"' % (text)
-
-    def _quote_special(self, text):
-        if text.lower() in ['yes', 'no', 'true', 'false']:
-            return text
-        else:
-            return self._quote_posix(text)
-
-    def _write_line(self, indent_string, entry, this_entry, comment):
-        # Ensure it is formatted fine for
-        # how these sysconfig scripts are used
-        val = self._decode_element(self._quote(this_entry))
-        # Single quoted strings should
-        # always work.
-        if not val.startswith("'"):
-            # Perform any special quoting
-            val = self._quote_special(val)
-        key = self._decode_element(self._quote(entry, multiline=False))
-        cmnt = self._decode_element(comment)
-        return '%s%s%s%s%s' % (indent_string,
-                               key,
-                               "=",
-                               val,
-                               cmnt)
-
diff --git a/cloudinit/distros/parsers/sys_conf.py b/cloudinit/distros/parsers/sys_conf.py
new file mode 100644
index 00000000..3d8802b8
--- /dev/null
+++ b/cloudinit/distros/parsers/sys_conf.py
@@ -0,0 +1,85 @@
+# vi: ts=4 expandtab
+#
+#    Copyright (C) 2012 Yahoo! Inc.
+#
+#    Author: Joshua Harlow <harlowja@yahoo-inc.com>
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU General Public License version 3, as
+#    published by the Free Software Foundation.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU General Public License for more details.
+#
+#    You should have received a copy of the GNU General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from StringIO import StringIO
+
+import re
+
+# This library is used to parse/write
+# out the various sysconfig files edited
+#
+# It has to be slightly modified though
+# to ensure that all values are quoted/unquoted correctly
+# since these configs are usually sourced into
+# bash scripts...
+import configobj
+
+
+class SysConf(configobj.ConfigObj):
+    def __init__(self, contents):
+        configobj.ConfigObj.__init__(self, contents,
+                                     interpolation=False,
+                                     write_empty_values=True)
+
+    def __str__(self):
+        contents = self.write()
+        out_contents = StringIO()
+        if isinstance(contents, (list, tuple)):
+            out_contents.write("\n".join(contents))
+        else:
+            out_contents.write(str(contents))
+        return out_contents.getvalue()
+
+    def _quote(self, value, multiline=False):
+        if not isinstance(value, (str, basestring)):
+            raise ValueError('Value "%s" is not a string' % (value))
+        if len(value) == 0:
+            return ''
+        if re.search(r"[\n\r]", value):
+            raise ValueError('Value "%s" cannot be safely quoted.' % (value))
+        quot = "%s"
+        if '#' in value:
+            quot = self._get_single_quote(value)
+        elif value[0] in ['"', "'"] and value[-1] in ['"', "'"]:
+            # Already quoted, leave it be
+            pass
+        elif "'" in value and '"' in value:
+            quot = self._get_triple_quote(value)
+        else:
+            # Quote whitespace if it isn't the start+end of a shell command
+            white_space_ok = False
+            if value.strip().startswith("$(") and value.strip().endswith(")"):
+                white_space_ok = True
+            if re.search(r"[\t ]", value) and not white_space_ok:
+                quot = self._get_single_quote(value)
+        return quot % (value)
+
+    def _write_line(self, indent_string, entry, this_entry, comment):
+        # Ensure it is formatted fine for
+        # how these sysconfig scripts are used
+        if this_entry.startswith("'") or this_entry.startswith('"'):
+            val = this_entry
+        val = self._decode_element(self._quote(this_entry))
+        key = self._decode_element(self._quote(entry))
+        cmnt = self._decode_element(comment)
+        return '%s%s%s%s%s' % (indent_string,
+                               key,
+                               self._a_to_u('='),
+                               val,
+                               cmnt)
+
diff --git a/cloudinit/distros/rhel.py b/cloudinit/distros/rhel.py
index 45c85fbb..039215c8 100644
--- a/cloudinit/distros/rhel.py
+++ b/cloudinit/distros/rhel.py
@@ -24,7 +24,8 @@ import os
 
 from cloudinit import distros
 
-from cloudinit.distros.parsers import (resolv_conf, quoting_conf)
+from cloudinit.distros.parsers.resolv_conf import ResolvConf
+from cloudinit.distros.parsers.sys_conf import SysConf
 
 from cloudinit import helpers
 from cloudinit import log as logging
@@ -63,14 +64,14 @@ class Distro(distros.Distro):
         self.package_command('install', pkglist)
 
     def _adjust_resolve(self, dns_servers, search_servers):
-        r_conf = resolv_conf.ResolvConf(util.load_file(self.resolve_conf_fn))
+        r_conf = ResolvConf(util.load_file(self.resolve_conf_fn))
         try:
             r_conf.parse()
         except IOError:
             util.logexc(LOG, 
                         "Failed at parsing %s reverting to an empty instance",
                         self.resolve_conf_fn)
-            r_conf = resolv_conf.ResolvConf('')
+            r_conf = ResolvConf('')
             r_conf.parse()
         if dns_servers:
             for s in dns_servers:
@@ -135,7 +136,9 @@ class Distro(distros.Distro):
             contents[k] = v
             updated_am += 1
         if updated_am:
-            lines = contents.write()
+            lines = [
+                str(contents),
+            ]
             if not exists:
                 lines.insert(0, util.make_header())
             util.write_file(fn, "\n".join(lines), 0644)
@@ -177,7 +180,7 @@ class Distro(distros.Distro):
         else:
             contents = []
         return (exists,
-                quoting_conf.QuotingConfigObj(contents))
+                SysConf(contents))
 
     def _bring_up_interfaces(self, device_names):
         if device_names and 'all' in device_names:
diff --git a/tests/unittests/test_distros/test_hostname.py b/tests/unittests/test_distros/test_hostname.py
new file mode 100644
index 00000000..8e644f4d
--- /dev/null
+++ b/tests/unittests/test_distros/test_hostname.py
@@ -0,0 +1,38 @@
+from mocker import MockerTestCase
+
+from cloudinit.distros.parsers import hostname
+
+
+BASE_HOSTNAME = '''
+# My super-duper-hostname
+
+blahblah
+
+'''
+BASE_HOSTNAME = BASE_HOSTNAME.strip()
+
+
+class TestHostnameHelper(MockerTestCase):
+    def test_parse_same(self):
+        hn = hostname.HostnameConf(BASE_HOSTNAME)
+        self.assertEquals(str(hn).strip(), BASE_HOSTNAME)
+        self.assertEquals(hn.hostname, 'blahblah')
+
+    def test_no_adjust_hostname(self):
+        hn = hostname.HostnameConf(BASE_HOSTNAME)
+        prev_name = hn.hostname
+        hn.set_hostname("")
+        self.assertEquals(hn.hostname, prev_name)
+
+    def test_adjust_hostname(self):
+        hn = hostname.HostnameConf(BASE_HOSTNAME)
+        prev_name = hn.hostname
+        self.assertEquals(prev_name, 'blahblah')
+        hn.set_hostname("bbbbd")
+        self.assertEquals(hn.hostname, 'bbbbd')
+        expected_out = '''
+# My super-duper-hostname
+
+bbbbd
+'''
+        self.assertEquals(str(hn).strip(), expected_out.strip())
diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py
index b7ce6fea..9763b14b 100644
--- a/tests/unittests/test_distros/test_netconfig.py
+++ b/tests/unittests/test_distros/test_netconfig.py
@@ -9,6 +9,8 @@ from cloudinit import helpers
 from cloudinit import settings
 from cloudinit import util
 
+from cloudinit.distros.parsers.sys_conf import SysConf
+
 from StringIO import StringIO
 
 
@@ -83,9 +85,8 @@ class TestNetCfgDistro(MockerTestCase):
         self.assertEquals(write_buf.mode, 0644)
 
     def assertCfgEquals(self, blob1, blob2):
-        cfg_tester = distros.parsers.quoting_conf.QuotingConfigObj
-        b1 = dict(cfg_tester(blob1.strip().splitlines()))
-        b2 = dict(cfg_tester(blob2.strip().splitlines()))
+        b1 = dict(SysConf(blob1.strip().splitlines()))
+        b2 = dict(SysConf(blob2.strip().splitlines()))
         self.assertEquals(b1, b2)
         for (k, v) in b1.items():
             self.assertIn(k, b2)
diff --git a/tests/unittests/test_distros/test_sysconfig.py b/tests/unittests/test_distros/test_sysconfig.py
new file mode 100644
index 00000000..196d090d
--- /dev/null
+++ b/tests/unittests/test_distros/test_sysconfig.py
@@ -0,0 +1,59 @@
+from mocker import MockerTestCase
+
+from cloudinit.distros.parsers.sys_conf import SysConf
+
+
+# Lots of good examples @
+# http://content.hccfl.edu/pollock/AUnix1/SysconfigFilesDesc.txt
+
+class TestSysConfHelper(MockerTestCase):
+    def test_parse_no_change(self):
+        contents = '''# A comment
+USESMBAUTH=no
+KEYTABLE=/usr/lib/kbd/keytables/us.map
+SHORTDATE=$(date +%y:%m:%d:%H:%M)
+HOSTNAME=blahblah
+NETMASK0=255.255.255.0
+# Inline comment
+LIST=$LOGROOT/incremental-list
+IPV6TO4_ROUTING="eth0-:0004::1/64 eth1-:0005::1/64"
+ETHTOOL_OPTS="-K ${DEVICE} tso on; -G ${DEVICE} rx 256 tx 256"
+USEMD5=no'''
+        conf = SysConf(contents.splitlines())
+        self.assertEquals(conf['HOSTNAME'], 'blahblah')
+        self.assertEquals(conf['SHORTDATE'], '$(date +%y:%m:%d:%H:%M)')
+        # Should be unquoted
+        self.assertEquals(conf['ETHTOOL_OPTS'], ('-K ${DEVICE} tso on; '
+                                                 '-G ${DEVICE} rx 256 tx 256'))
+        self.assertEquals(contents, str(conf))
+
+    def test_parse_adjust(self):
+        contents = 'IPV6TO4_ROUTING="eth0-:0004::1/64 eth1-:0005::1/64"'
+        conf = SysConf(contents.splitlines())
+        # Should be unquoted
+        self.assertEquals('eth0-:0004::1/64 eth1-:0005::1/64',
+                          conf['IPV6TO4_ROUTING'])
+        conf['IPV6TO4_ROUTING'] = "blah \tblah"
+        contents2 = str(conf).strip()
+        # Should be requoted due to whitespace
+        self.assertEquals('IPV6TO4_ROUTING="blah \tblah"', contents2)
+
+    def test_parse_no_adjust_shell(self):
+        conf = SysConf(''.splitlines())
+        conf['B'] = ' $(time)'
+        contents = str(conf)
+        self.assertEquals('B= $(time)', contents)
+
+    def test_parse_empty(self):
+        contents = ''
+        conf = SysConf(contents.splitlines())
+        self.assertEquals('', str(conf).strip())
+
+    def test_parse_add_new(self):
+        contents = 'BLAH=b'
+        conf = SysConf(contents.splitlines())
+        conf['Z'] = 'd'
+        contents = str(conf)
+        self.assertIn("Z=d", contents)
+        self.assertIn("BLAH=b", contents)
+