From 4b910d7bca47cb4fd26394fb4d14ebc72b1a73f8 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 20 Sep 2012 14:02:34 -0700 Subject: Add a resolv.conf parser that can be easily used for adjusting a resolv.conf formatted file and use this to adjust the resolv.conf in the redhat distro instead of replacing the previous resolv.conf completely. --- tests/unittests/test_distros/test_resolv.py | 61 +++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 tests/unittests/test_distros/test_resolv.py (limited to 'tests') diff --git a/tests/unittests/test_distros/test_resolv.py b/tests/unittests/test_distros/test_resolv.py new file mode 100644 index 00000000..5f122833 --- /dev/null +++ b/tests/unittests/test_distros/test_resolv.py @@ -0,0 +1,61 @@ +from mocker import MockerTestCase + +from cloudinit.distros import helpers + + +BASE_RESOLVE = ''' +; generated by /sbin/dhclient-script +search blah.yahoo.com yahoo.com +nameserver 10.15.44.14 +nameserver 10.15.30.92 +''' +BASE_RESOLVE = BASE_RESOLVE.strip() + + +class TestResolvHelper(MockerTestCase): + def test_parse_same(self): + rp = helpers.ResolvConf(BASE_RESOLVE) + rp_r = str(rp).strip() + self.assertEquals(BASE_RESOLVE, rp_r) + + def test_local_domain(self): + rp = helpers.ResolvConf(BASE_RESOLVE) + self.assertEquals(None, rp.local_domain) + + rp.local_domain = "bob" + self.assertEquals('bob', rp.local_domain) + self.assertIn('domain bob', str(rp)) + + def test_nameservers(self): + rp = helpers.ResolvConf(BASE_RESOLVE) + self.assertIn('10.15.44.14', rp.nameservers) + self.assertIn('10.15.30.92', rp.nameservers) + rp.add_nameserver('10.2') + self.assertIn('10.2', rp.nameservers) + self.assertIn('nameserver 10.2', str(rp)) + self.assertNotIn('10.3', rp.nameservers) + self.assertEquals(len(rp.nameservers), 3) + rp.add_nameserver('10.2') + with self.assertRaises(ValueError): + rp.add_nameserver('10.3') + self.assertNotIn('10.3', rp.nameservers) + + def test_search_domains(self): + rp = helpers.ResolvConf(BASE_RESOLVE) + self.assertIn('yahoo.com', rp.search_domains) + self.assertIn('blah.yahoo.com', rp.search_domains) + rp.add_search_domain('bbb.y.com') + self.assertIn('bbb.y.com', rp.search_domains) + self.assertRegexpMatches(str(rp), r'search(.*)bbb.y.com(.*)') + self.assertIn('bbb.y.com', rp.search_domains) + rp.add_search_domain('bbb.y.com') + self.assertEquals(len(rp.search_domains), 3) + rp.add_search_domain('bbb2.y.com') + self.assertEquals(len(rp.search_domains), 4) + rp.add_search_domain('bbb3.y.com') + self.assertEquals(len(rp.search_domains), 5) + rp.add_search_domain('bbb4.y.com') + self.assertEquals(len(rp.search_domains), 6) + with self.assertRaises(ValueError): + rp.add_search_domain('bbb5.y.com') + self.assertEquals(len(rp.search_domains), 6) -- cgit v1.2.3 From 6e0f6f3db662751c2b0bf837f2affe51b1f328a3 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 21 Sep 2012 18:05:17 -0700 Subject: Don't forget the etc/hosts test. --- tests/unittests/test_distros/test_hosts.py | 41 ++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 tests/unittests/test_distros/test_hosts.py (limited to 'tests') diff --git a/tests/unittests/test_distros/test_hosts.py b/tests/unittests/test_distros/test_hosts.py new file mode 100644 index 00000000..621837ab --- /dev/null +++ b/tests/unittests/test_distros/test_hosts.py @@ -0,0 +1,41 @@ +from mocker import MockerTestCase + +from cloudinit.distros import helpers + + +BASE_ETC = ''' +# Example +127.0.0.1 localhost +192.168.1.10 foo.mydomain.org foo +192.168.1.10 bar.mydomain.org bar +146.82.138.7 master.debian.org master +209.237.226.90 www.opensource.org +''' +BASE_ETC = BASE_ETC.strip() + + +class TestHostsHelper(MockerTestCase): + def test_parse(self): + eh = helpers.HostsConf(BASE_ETC) + self.assertEquals(eh.get_entry('127.0.0.1'), [['localhost']]) + self.assertEquals(eh.get_entry('192.168.1.10'), + [['foo.mydomain.org', 'foo'], + ['bar.mydomain.org', 'bar']]) + eh = str(eh) + self.assertTrue(eh.startswith('# Example')) + + def test_add(self): + eh = helpers.HostsConf(BASE_ETC) + eh.add_entry('127.0.0.0', 'blah') + self.assertEquals(eh.get_entry('127.0.0.0'), [['blah']]) + eh.add_entry('127.0.0.3', 'blah', 'blah2', 'blah3') + self.assertEquals(eh.get_entry('127.0.0.3'), + [['blah', 'blah2', 'blah3']]) + + def test_del(self): + eh = helpers.HostsConf(BASE_ETC) + eh.add_entry('127.0.0.0', 'blah') + self.assertEquals(eh.get_entry('127.0.0.0'), [['blah']]) + + eh.del_entries('127.0.0.0') + self.assertEquals(eh.get_entry('127.0.0.0'), []) -- cgit v1.2.3 From 80eb53deb9f80694c5fd0e0190197de1ff496716 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Wed, 10 Oct 2012 16:21:22 -0700 Subject: System config niceness! 1. Move out the old helpers that provided oop access/reading/writing to various standard conf files and place those in parsers instead. 2. Unify the 'update_hostname' which varied very little between distros and make it generic so that subclasses can only provide a couple of functions to obtain the hostname updating functionality 3. Implement that new set of functions in rhel/debian 4. Use the new parsers chop_comment function for similar use cases as well as add a new utils make header function that can be used for configuration files that are newly generated to use (less duplication here of this same thing being done in multiple places. 5. Add in a distro '_apply_hostname' which calls out to the 'hostname' program to set the system hostname (more duplication elimination). 6. Make the 'constant' filenames being written to for configuration by the various distros be instance members instead of string constants 'sprinkled' throughout the code --- cloudinit/distros/__init__.py | 89 ++++++--- cloudinit/distros/debian.py | 88 ++++----- cloudinit/distros/helpers.py | 252 ------------------------- cloudinit/distros/parsers/__init__.py | 27 +++ cloudinit/distros/parsers/hosts.py | 92 +++++++++ cloudinit/distros/parsers/quoting_conf.py | 80 ++++++++ cloudinit/distros/parsers/resolv_conf.py | 171 +++++++++++++++++ cloudinit/distros/rhel.py | 149 ++++----------- cloudinit/util.py | 15 +- tests/unittests/test_distros/test_hosts.py | 8 +- tests/unittests/test_distros/test_netconfig.py | 2 +- tests/unittests/test_distros/test_resolv.py | 14 +- 12 files changed, 532 insertions(+), 455 deletions(-) delete mode 100644 cloudinit/distros/helpers.py create mode 100644 cloudinit/distros/parsers/__init__.py create mode 100644 cloudinit/distros/parsers/hosts.py create mode 100644 cloudinit/distros/parsers/quoting_conf.py create mode 100644 cloudinit/distros/parsers/resolv_conf.py (limited to 'tests') diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index a5d41bdb..07f03159 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -33,7 +33,7 @@ from cloudinit import log as logging from cloudinit import ssh_util from cloudinit import util -from cloudinit.distros import helpers +from cloudinit.distros.parsers import hosts LOG = logging.getLogger(__name__) @@ -43,6 +43,8 @@ class Distro(object): __metaclass__ = abc.ABCMeta default_user = None default_user_groups = None + hosts_fn = "/etc/hosts" + ci_sudoers_fn = "/etc/sudoers.d/90-cloud-init-users" def __init__(self, name, cfg, paths): self._paths = paths @@ -66,10 +68,6 @@ class Distro(object): def set_hostname(self, hostname): raise NotImplementedError() - @abc.abstractmethod - def update_hostname(self, hostname, prev_hostname_fn): - raise NotImplementedError() - @abc.abstractmethod def package_command(self, cmd, args=None): raise NotImplementedError() @@ -117,14 +115,62 @@ class Distro(object): def _get_localhost_ip(self): return "127.0.0.1" + @abc.abstractmethod + def _read_hostname(self, filename, default=None): + raise NotImplementedError() + + @abc.abstractmethod + def _write_hostname(self, hostname, filename): + raise NotImplementedError() + + @abc.abstractmethod + def _read_system_hostname(self): + raise NotImplementedError() + + def _apply_hostname(self, hostname): + LOG.debug("Setting system hostname to %s", hostname) + util.subp(['hostname', hostname]) + + def update_hostname(self, hostname, prev_hostname_fn): + if not hostname: + return + + prev_hostname = self._read_hostname(prev_hostname_fn) + (sys_fn, sys_hostname) = self._read_system_hostname() + update_files = [] + if not prev_hostname or prev_hostname != hostname: + update_files.append(prev_hostname_fn) + + if (not sys_hostname) or (sys_hostname == prev_hostname + and sys_hostname != hostname): + update_files.append(sys_fn) + + update_files = set([f for f in update_files if f]) + LOG.debug("Attempting to update hostname to %s in %s files", + hostname, len(update_files)) + + for fn in update_files: + try: + self._write_hostname(hostname, fn) + except IOError: + util.logexc(LOG, "Failed to write hostname %s to %s", + hostname, fn) + + if (sys_hostname and prev_hostname and + sys_hostname != prev_hostname): + LOG.debug("%s differs from %s, assuming user maintained hostname.", + prev_hostname_fn, sys_fn) + + if sys_fn in update_files: + self._apply_hostname(hostname) + def update_etc_hosts(self, hostname, fqdn): header = '' - if os.path.exists('/etc/hosts'): - eh = helpers.HostsConf(util.load_file("/etc/hosts")) + if os.path.exists(self.hosts_fn): + eh = hosts.HostsConf(util.load_file(self.hosts_fn)) else: - eh = helpers.HostsConf('') - header = "# Added by cloud-init" - header = "%s on %s" % (header, util.time_rfc2822()) + eh = hosts.HostsConf('') + header = util.make_header(base="added") local_ip = self._get_localhost_ip() prev_info = eh.get_entry(local_ip) need_change = False @@ -154,7 +200,7 @@ class Distro(object): if header: contents.write("%s\n" % (header)) contents.write("%s\n" % (eh)) - util.write_file("/etc/hosts", contents.getvalue(), mode=0644) + util.write_file(self.hosts_fn, contents.getvalue(), mode=0644) def _bring_up_interface(self, device_name): cmd = ['ifup', device_name] @@ -302,30 +348,31 @@ class Distro(object): return True - def write_sudo_rules(self, - user, - rules, - sudo_file="/etc/sudoers.d/90-cloud-init-users", - ): + def write_sudo_rules(self, user, rules, sudo_file=None): + if not sudo_file: + sudo_file = self.ci_sudoers_fn - content_header = "# user rules for %s" % user + content_header = "# User rules for %s" % user content = "%s\n%s %s\n\n" % (content_header, user, rules) - if isinstance(rules, list): + if isinstance(rules, (list, tuple, set)): content = "%s\n" % content_header for rule in rules: content += "%s %s\n" % (user, rule) content += "\n" if not os.path.exists(sudo_file): - util.write_file(sudo_file, content, 0440) - + contents = [ + util.make_header(), + content, + ] + util.write_file(sudo_file, "\n".join(contents), 0440) else: try: with open(sudo_file, 'a') as f: f.write(content) except IOError as e: - util.logexc(LOG, "Failed to write %s" % sudo_file, e) + util.logexc(LOG, "Failed to write sudoers file %s", sudo_file) raise e def create_group(self, name, members): diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py index 88f4e978..20962937 100644 --- a/cloudinit/distros/debian.py +++ b/cloudinit/distros/debian.py @@ -27,12 +27,20 @@ from cloudinit import helpers from cloudinit import log as logging from cloudinit import util +from cloudinit.distros.parsers import chop_comment + from cloudinit.settings import PER_INSTANCE LOG = logging.getLogger(__name__) class Distro(distros.Distro): + hostname_conf_fn = "/etc/hostname" + locale_conf_fn = "/etc/default/locale" + network_conf_fn = "/etc/network/interfaces" + tz_conf_fn = "/etc/timezone" + tz_local_fn = "/etc/localtime" + tz_zone_dir = "/usr/share/zoneinfo" def __init__(self, name, cfg, paths): distros.Distro.__init__(self, name, cfg, paths) @@ -43,10 +51,15 @@ class Distro(distros.Distro): def apply_locale(self, locale, out_fn=None): if not out_fn: - out_fn = self._paths.join(False, '/etc/default/locale') + out_fn = self.locale_conf_fn util.subp(['locale-gen', locale], capture=False) util.subp(['update-locale', locale], capture=False) - lines = ["# Created by cloud-init", 'LANG="%s"' % (locale), ""] + # "" provides trailing newline during join + lines = [ + util.make_header(), + 'LANG="%s"' % (locale), + "", + ] util.write_file(out_fn, "\n".join(lines)) def install_packages(self, pkglist): @@ -54,8 +67,7 @@ class Distro(distros.Distro): self.package_command('install', pkglist) def _write_network(self, settings): - net_fn = self._paths.join(False, "/etc/network/interfaces") - util.write_file(net_fn, settings) + util.write_file(self.network_conf_fn, settings) return ['all'] def _bring_up_interfaces(self, device_names): @@ -69,54 +81,29 @@ class Distro(distros.Distro): return distros.Distro._bring_up_interfaces(self, device_names) def set_hostname(self, hostname): - out_fn = self._paths.join(False, "/etc/hostname") - self._write_hostname(hostname, out_fn) - if out_fn == '/etc/hostname': - # Only do this if we are running in non-adjusted root mode - LOG.debug("Setting hostname to %s", hostname) - util.subp(['hostname', hostname]) + self._write_hostname(hostname, self.hostname_conf_fn) + self._apply_hostname(hostname) def _write_hostname(self, hostname, out_fn): # "" gives trailing newline. - util.write_file(out_fn, "%s\n" % str(hostname), 0644) - - def update_hostname(self, hostname, prev_fn): - hostname_prev = self._read_hostname(prev_fn) - read_fn = self._paths.join(True, "/etc/hostname") - hostname_in_etc = self._read_hostname(read_fn) - update_files = [] - if not hostname_prev or hostname_prev != hostname: - update_files.append(prev_fn) - if (not hostname_in_etc or - (hostname_in_etc == hostname_prev and - hostname_in_etc != hostname)): - write_fn = self._paths.join(False, "/etc/hostname") - update_files.append(write_fn) - for fn in update_files: - try: - self._write_hostname(hostname, fn) - except: - util.logexc(LOG, "Failed to write hostname %s to %s", - hostname, fn) - if (hostname_in_etc and hostname_prev and - hostname_in_etc != hostname_prev): - LOG.debug(("%s differs from /etc/hostname." - " Assuming user maintained hostname."), prev_fn) - if "/etc/hostname" in update_files: - # Only do this if we are running in non-adjusted root mode - LOG.debug("Setting hostname to %s", hostname) - util.subp(['hostname', hostname]) + hostname_lines = [ + str(hostname), + "", + ] + util.write_file(out_fn, "\n".join(hostname_lines), 0644) + + def _read_system_hostname(self): + return (self.hostname_conf_fn, + self._read_hostname(self.hostname_conf_fn)) def _read_hostname(self, filename, default=None): contents = util.load_file(filename, quiet=True) for line in contents.splitlines(): - c_pos = line.find("#") # Handle inline comments - if c_pos != -1: - line = line[0:c_pos] - line_c = line.strip() - if line_c: - return line_c + (before_comment, _comment) = chop_comment(line, "#") + before_comment = before_comment.strip() + if len(before_comment): + return before_comment return default def _get_localhost_ip(self): @@ -124,15 +111,18 @@ class Distro(distros.Distro): return "127.0.1.1" def set_timezone(self, tz): - tz_file = os.path.join("/usr/share/zoneinfo", tz) + tz_file = os.path.join(self.tz_zone_dir, tz) if not os.path.isfile(tz_file): raise RuntimeError(("Invalid timezone %s," " no file found at %s") % (tz, tz_file)) # "" provides trailing newline during join - tz_lines = ["# Created by cloud-init", str(tz), ""] - tz_fn = self._paths.join(False, "/etc/timezone") - util.write_file(tz_fn, "\n".join(tz_lines)) - util.copy(tz_file, self._paths.join(False, "/etc/localtime")) + tz_lines = [ + util.make_header(), + str(tz), + "", + ] + util.write_file(self.tz_conf_fn, "\n".join(tz_lines)) + util.copy(tz_file, self.tz_local_fn) def package_command(self, command, args=None): e = os.environ.copy() diff --git a/cloudinit/distros/helpers.py b/cloudinit/distros/helpers.py deleted file mode 100644 index 916935eb..00000000 --- a/cloudinit/distros/helpers.py +++ /dev/null @@ -1,252 +0,0 @@ -# vi: ts=4 expandtab -# -# Copyright (C) 2012 Canonical Ltd. -# Copyright (C) 2012 Yahoo! Inc. -# -# Author: Scott Moser -# Author: Joshua Harlow -# -# 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 . - -from StringIO import StringIO - -from cloudinit import util - - -def _chop_comment(text, comment_chars): - comment_locations = [text.find(c) for c in comment_chars] - comment_locations = [c for c in comment_locations if c != -1] - if not comment_locations: - return (text, '') - min_comment = min(comment_locations) - before_comment = text[0:min_comment] - comment = text[min_comment:] - return (before_comment, comment) - - -# See: man hosts -# or http://unixhelp.ed.ac.uk/CGI/man-cgi?hosts -class HostsConf(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 get_entry(self, ip): - self.parse() - options = [] - for (line_type, components) in self._contents: - if line_type == 'option': - (pieces, _tail) = components - if len(pieces) and pieces[0] == ip: - options.append(pieces[1:]) - return options - - def del_entries(self, ip): - self.parse() - n_entries = [] - for (line_type, components) in self._contents: - if line_type != 'option': - n_entries.append((line_type, components)) - continue - else: - (pieces, _tail) = components - if len(pieces) and pieces[0] == ip: - pass - elif len(pieces): - n_entries.append((line_type, list(components))) - self._contents = n_entries - - def add_entry(self, ip, canonical_hostname, *aliases): - self.parse() - self._contents.append(('option', - ([ip, canonical_hostname] + list(aliases), ''))) - - def _parse(self, contents): - entries = [] - 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(('option', [head.split(None), tail])) - return entries - - def __str__(self): - self.parse() - contents = StringIO() - for (line_type, components) in self._contents: - if line_type == 'blank': - contents.write("%s\n") - elif line_type == 'all_comment': - contents.write("%s\n" % (components[0])) - elif line_type == 'option': - (pieces, tail) = components - pieces = [str(p) for p in pieces] - pieces = "\t".join(pieces) - contents.write("%s%s\n" % (pieces, tail)) - return contents.getvalue() - - -# See: man resolv.conf -class ResolvConf(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) - - @property - def nameservers(self): - self.parse() - return self._retr_option('nameserver') - - @property - def local_domain(self): - self.parse() - dm = self._retr_option('domain') - if dm: - return dm[0] - return None - - @property - def search_domains(self): - self.parse() - current_sds = self._retr_option('search') - flat_sds = [] - for sdlist in current_sds: - for sd in sdlist.split(None): - if sd: - flat_sds.append(sd) - return flat_sds - - def __str__(self): - self.parse() - contents = StringIO() - for (line_type, components) in self._contents: - if line_type == 'blank': - contents.write("\n") - elif line_type == 'all_comment': - contents.write("%s\n" % (components[0])) - elif line_type == 'option': - (cfg_opt, cfg_value, comment_tail) = components - line = "%s %s" % (cfg_opt, cfg_value) - if len(comment_tail): - line += comment_tail - contents.write("%s\n" % (line)) - return contents.getvalue() - - def _retr_option(self, opt_name): - found = [] - for (line_type, components) in self._contents: - if line_type == 'option': - (cfg_opt, cfg_value, _comment_tail) = components - if cfg_opt == opt_name: - found.append(cfg_value) - return found - - def add_nameserver(self, ns): - self.parse() - current_ns = self._retr_option('nameserver') - new_ns = list(current_ns) - new_ns.append(str(ns)) - new_ns = util.uniq_list(new_ns) - if len(new_ns) == len(current_ns): - return current_ns - if len(current_ns) >= 3: - # Hard restriction on only 3 name servers - raise ValueError(("Adding %r would go beyond the " - "'3' maximum name servers") % (ns)) - self._remove_option('nameserver') - for n in new_ns: - self._contents.append(('option', ['nameserver', n, ''])) - return new_ns - - def _remove_option(self, opt_name): - - def remove_opt(item): - line_type, components = item - if line_type != 'option': - return False - (cfg_opt, _cfg_value, _comment_tail) = components - if cfg_opt != opt_name: - return False - return True - - new_contents = [] - for c in self._contents: - if not remove_opt(c): - new_contents.append(c) - self._contents = new_contents - - def add_search_domain(self, search_domain): - flat_sds = self.search_domains - new_sds = list(flat_sds) - new_sds.append(str(search_domain)) - new_sds = util.uniq_list(new_sds) - if len(flat_sds) == len(new_sds): - return new_sds - if len(flat_sds) >= 6: - # Hard restriction on only 6 search domains - raise ValueError(("Adding %r would go beyond the " - "'6' maximum search domains") % (search_domain)) - s_list = " ".join(new_sds) - if len(s_list) > 256: - # Some hard limit on 256 chars total - raise ValueError(("Adding %r would go beyond the " - "256 maximum search list character limit") - % (search_domain)) - self._remove_option('search') - self._contents.append(('option', ['search', s_list, ''])) - return flat_sds - - @local_domain.setter - def local_domain(self, domain): - self.parse() - self._remove_option('domain') - self._contents.append(('option', ['domain', str(domain), ''])) - return domain - - def _parse(self, contents): - entries = [] - for (i, line) in enumerate(contents.splitlines()): - sline = line.strip() - if not sline: - entries.append(('blank', [line])) - continue - (head, tail) = _chop_comment(line, ';#') - if not len(head.strip()): - entries.append(('all_comment', [line])) - continue - if not tail: - tail = '' - try: - (cfg_opt, cfg_values) = head.split(None, 1) - except (IndexError, ValueError): - raise IOError("Incorrectly formatted resolv.conf line %s" - % (i + 1)) - if cfg_opt not in ['nameserver', 'domain', - 'search', 'sortlist', 'options']: - raise IOError("Unexpected resolv.conf option %s" % (cfg_opt)) - entries.append(("option", [cfg_opt, cfg_values, tail])) - return entries - - diff --git a/cloudinit/distros/parsers/__init__.py b/cloudinit/distros/parsers/__init__.py new file mode 100644 index 00000000..8334a5e6 --- /dev/null +++ b/cloudinit/distros/parsers/__init__.py @@ -0,0 +1,27 @@ +# vi: ts=4 expandtab +# +# Copyright (C) 2012 Yahoo! Inc. +# +# Author: Joshua Harlow +# +# 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 . + +def chop_comment(text, comment_chars): + comment_locations = [text.find(c) for c in comment_chars] + comment_locations = [c for c in comment_locations if c != -1] + if not comment_locations: + return (text, '') + min_comment = min(comment_locations) + before_comment = text[0:min_comment] + comment = text[min_comment:] + return (before_comment, comment) diff --git a/cloudinit/distros/parsers/hosts.py b/cloudinit/distros/parsers/hosts.py new file mode 100644 index 00000000..5374ab0b --- /dev/null +++ b/cloudinit/distros/parsers/hosts.py @@ -0,0 +1,92 @@ +# vi: ts=4 expandtab +# +# Copyright (C) 2012 Yahoo! Inc. +# +# Author: Joshua Harlow +# +# 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 . + +from StringIO import StringIO + +from cloudinit.distros.parsers import chop_comment + + +# See: man hosts +# or http://unixhelp.ed.ac.uk/CGI/man-cgi?hosts +class HostsConf(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 get_entry(self, ip): + self.parse() + options = [] + for (line_type, components) in self._contents: + if line_type == 'option': + (pieces, _tail) = components + if len(pieces) and pieces[0] == ip: + options.append(pieces[1:]) + return options + + def del_entries(self, ip): + self.parse() + n_entries = [] + for (line_type, components) in self._contents: + if line_type != 'option': + n_entries.append((line_type, components)) + continue + else: + (pieces, _tail) = components + if len(pieces) and pieces[0] == ip: + pass + elif len(pieces): + n_entries.append((line_type, list(components))) + self._contents = n_entries + + def add_entry(self, ip, canonical_hostname, *aliases): + self.parse() + self._contents.append(('option', + ([ip, canonical_hostname] + list(aliases), ''))) + + def _parse(self, contents): + entries = [] + 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(('option', [head.split(None), tail])) + return entries + + def __str__(self): + self.parse() + contents = StringIO() + for (line_type, components) in self._contents: + if line_type == 'blank': + contents.write("%s\n") + elif line_type == 'all_comment': + contents.write("%s\n" % (components[0])) + elif line_type == 'option': + (pieces, tail) = components + pieces = [str(p) for p in pieces] + pieces = "\t".join(pieces) + contents.write("%s%s\n" % (pieces, tail)) + return contents.getvalue() + diff --git a/cloudinit/distros/parsers/quoting_conf.py b/cloudinit/distros/parsers/quoting_conf.py new file mode 100644 index 00000000..953ccfe9 --- /dev/null +++ b/cloudinit/distros/parsers/quoting_conf.py @@ -0,0 +1,80 @@ +# vi: ts=4 expandtab +# +# Copyright (C) 2012 Yahoo! Inc. +# +# Author: Joshua Harlow +# +# 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 . + +# 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/resolv_conf.py b/cloudinit/distros/parsers/resolv_conf.py new file mode 100644 index 00000000..377ada6b --- /dev/null +++ b/cloudinit/distros/parsers/resolv_conf.py @@ -0,0 +1,171 @@ +# vi: ts=4 expandtab +# +# Copyright (C) 2012 Yahoo! Inc. +# +# Author: Joshua Harlow +# +# 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 . + +from StringIO import StringIO + +from cloudinit import util + +from cloudinit.distros.parsers import chop_comment + + +# See: man resolv.conf +class ResolvConf(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) + + @property + def nameservers(self): + self.parse() + return self._retr_option('nameserver') + + @property + def local_domain(self): + self.parse() + dm = self._retr_option('domain') + if dm: + return dm[0] + return None + + @property + def search_domains(self): + self.parse() + current_sds = self._retr_option('search') + flat_sds = [] + for sdlist in current_sds: + for sd in sdlist.split(None): + if sd: + flat_sds.append(sd) + return flat_sds + + def __str__(self): + self.parse() + contents = StringIO() + for (line_type, components) in self._contents: + if line_type == 'blank': + contents.write("\n") + elif line_type == 'all_comment': + contents.write("%s\n" % (components[0])) + elif line_type == 'option': + (cfg_opt, cfg_value, comment_tail) = components + line = "%s %s" % (cfg_opt, cfg_value) + if len(comment_tail): + line += comment_tail + contents.write("%s\n" % (line)) + return contents.getvalue() + + def _retr_option(self, opt_name): + found = [] + for (line_type, components) in self._contents: + if line_type == 'option': + (cfg_opt, cfg_value, _comment_tail) = components + if cfg_opt == opt_name: + found.append(cfg_value) + return found + + def add_nameserver(self, ns): + self.parse() + current_ns = self._retr_option('nameserver') + new_ns = list(current_ns) + new_ns.append(str(ns)) + new_ns = util.uniq_list(new_ns) + if len(new_ns) == len(current_ns): + return current_ns + if len(current_ns) >= 3: + # Hard restriction on only 3 name servers + raise ValueError(("Adding %r would go beyond the " + "'3' maximum name servers") % (ns)) + self._remove_option('nameserver') + for n in new_ns: + self._contents.append(('option', ['nameserver', n, ''])) + return new_ns + + def _remove_option(self, opt_name): + + def remove_opt(item): + line_type, components = item + if line_type != 'option': + return False + (cfg_opt, _cfg_value, _comment_tail) = components + if cfg_opt != opt_name: + return False + return True + + new_contents = [] + for c in self._contents: + if not remove_opt(c): + new_contents.append(c) + self._contents = new_contents + + def add_search_domain(self, search_domain): + flat_sds = self.search_domains + new_sds = list(flat_sds) + new_sds.append(str(search_domain)) + new_sds = util.uniq_list(new_sds) + if len(flat_sds) == len(new_sds): + return new_sds + if len(flat_sds) >= 6: + # Hard restriction on only 6 search domains + raise ValueError(("Adding %r would go beyond the " + "'6' maximum search domains") % (search_domain)) + s_list = " ".join(new_sds) + if len(s_list) > 256: + # Some hard limit on 256 chars total + raise ValueError(("Adding %r would go beyond the " + "256 maximum search list character limit") + % (search_domain)) + self._remove_option('search') + self._contents.append(('option', ['search', s_list, ''])) + return flat_sds + + @local_domain.setter + def local_domain(self, domain): + self.parse() + self._remove_option('domain') + self._contents.append(('option', ['domain', str(domain), ''])) + return domain + + def _parse(self, contents): + entries = [] + for (i, line) in enumerate(contents.splitlines()): + sline = line.strip() + if not sline: + entries.append(('blank', [line])) + continue + (head, tail) = chop_comment(line, ';#') + if not len(head.strip()): + entries.append(('all_comment', [line])) + continue + if not tail: + tail = '' + try: + (cfg_opt, cfg_values) = head.split(None, 1) + except (IndexError, ValueError): + raise IOError("Incorrectly formatted resolv.conf line %s" + % (i + 1)) + if cfg_opt not in ['nameserver', 'domain', + 'search', 'sortlist', 'options']: + raise IOError("Unexpected resolv.conf option %s" % (cfg_opt)) + entries.append(("option", [cfg_opt, cfg_values, tail])) + return entries + + diff --git a/cloudinit/distros/rhel.py b/cloudinit/distros/rhel.py index 13fd5ec8..21f2216e 100644 --- a/cloudinit/distros/rhel.py +++ b/cloudinit/distros/rhel.py @@ -23,41 +23,17 @@ import os from cloudinit import distros -from cloudinit.distros import helpers as d_helpers + +from cloudinit.distros.parsers import (resolv_conf, quoting_conf) from cloudinit import helpers from cloudinit import log as logging from cloudinit import util -from cloudinit import version from cloudinit.settings import PER_INSTANCE LOG = logging.getLogger(__name__) -NETWORK_FN_TPL = '/etc/sysconfig/network-scripts/ifcfg-%s' - -# See: http://tiny.cc/6r99fw -# For what alot of these files that are being written -# are and the format of them - -# 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 = { - "\"": "\\\"", - "(": "\\(", - ")": "\\)", - "$": '\$', - '`': '\`', -} - def _make_sysconfig_bool(val): if val: @@ -66,12 +42,15 @@ def _make_sysconfig_bool(val): return 'no' -def _make_header(): - ci_ver = version.version_string() - return '# Created by cloud-init v. %s' % (ci_ver) - - class Distro(distros.Distro): + # See: http://tiny.cc/6r99fw + clock_conf_fn = "/etc/sysconfig/clock" + locale_conf_fn = '/etc/sysconfig/i18n' + network_conf_fn = "/etc/sysconfig/network" + network_script_tpl = '/etc/sysconfig/network-scripts/ifcfg-%s' + resolve_conf_fn = "/etc/resolv.conf" + tz_local_fn = "/etc/localtime" + tz_zone_dir = "/usr/share/zoneinfo" def __init__(self, name, cfg, paths): distros.Distro.__init__(self, name, cfg, paths) @@ -84,14 +63,14 @@ class Distro(distros.Distro): self.package_command('install', pkglist) def _adjust_resolve(self, dns_servers, search_servers): - r_conf = d_helpers.ResolvConf(util.load_file("/etc/resolv.conf")) + r_conf = resolv_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", - "/etc/resolv.conf") - r_conf = d_helpers.ResolvConf('') + self.resolve_conf_fn) + r_conf = resolv_conf.ResolvConf('') r_conf.parse() if dns_servers: for s in dns_servers: @@ -105,7 +84,7 @@ class Distro(distros.Distro): r_conf.add_search_domain(s) except ValueError: util.logexc(LOG, "Failed at adding search domain %s", s) - util.write_file("/etc/resolv.conf", str(r_conf), 0644) + util.write_file(self.resolve_conf_fn, str(r_conf), 0644) def _write_network(self, settings): # TODO(harlowja) fix this... since this is the ubuntu format @@ -117,7 +96,7 @@ class Distro(distros.Distro): searchservers = [] dev_names = entries.keys() for (dev, info) in entries.iteritems(): - net_fn = NETWORK_FN_TPL % (dev) + net_fn = self.network_script_tpl % (dev) net_cfg = { 'DEVICE': dev, 'NETMASK': info.get('netmask'), @@ -134,12 +113,12 @@ class Distro(distros.Distro): if 'dns-search' in info: searchservers.extend(info['dns-search']) if nameservers or searchservers: - self._write_resolve(nameservers, searchservers) + self._adjust_resolve(nameservers, searchservers) if dev_names: net_cfg = { 'NETWORKING': _make_sysconfig_bool(True), } - self._update_sysconfig_file("/etc/sysconfig/network", net_cfg) + self._update_sysconfig_file(self.network_conf_fn, net_cfg) return dev_names def _update_sysconfig_file(self, fn, adjustments, allow_empty=False): @@ -158,17 +137,16 @@ class Distro(distros.Distro): if updated_am: lines = contents.write() if not exists: - lines.insert(0, _make_header()) + lines.insert(0, util.make_header()) util.write_file(fn, "\n".join(lines), 0644) def set_hostname(self, hostname): - self._write_hostname(hostname, '/etc/sysconfig/network') - LOG.debug("Setting hostname to %s", hostname) - util.subp(['hostname', hostname]) + self._write_hostname(hostname, self.network_conf_fn) + self._apply_hostname(hostname) def apply_locale(self, locale, out_fn=None): if not out_fn: - out_fn = '/etc/sysconfig/i18n' + out_fn = self.locale_conf_fn locale_cfg = { 'LANG': locale, } @@ -180,30 +158,9 @@ class Distro(distros.Distro): } self._update_sysconfig_file(out_fn, host_cfg) - def update_hostname(self, hostname, prev_file): - hostname_prev = self._read_hostname(prev_file) - hostname_in_sys = self._read_hostname("/etc/sysconfig/network") - update_files = [] - if not hostname_prev or hostname_prev != hostname: - update_files.append(prev_file) - if (not hostname_in_sys or - (hostname_in_sys == hostname_prev - and hostname_in_sys != hostname)): - update_files.append("/etc/sysconfig/network") - for fn in update_files: - try: - self._write_hostname(hostname, fn) - except: - util.logexc(LOG, "Failed to write hostname %s to %s", - hostname, fn) - if (hostname_in_sys and hostname_prev and - hostname_in_sys != hostname_prev): - LOG.debug(("%s differs from /etc/sysconfig/network." - " Assuming user maintained hostname."), prev_file) - if "/etc/sysconfig/network" in update_files: - # Only do this if we are running in non-adjusted root mode - LOG.debug("Setting hostname to %s", hostname) - util.subp(['hostname', hostname]) + def _read_system_hostname(self): + return (self.network_conf_fn, + self._read_hostname(self.network_conf_fn)) def _read_hostname(self, filename, default=None): (_exists, contents) = self._read_conf(filename) @@ -219,7 +176,8 @@ class Distro(distros.Distro): exists = True else: contents = [] - return (exists, QuotingConfigObj(contents)) + return (exists, + quoting_conf.QuotingConfigObj(contents)) def _bring_up_interfaces(self, device_names): if device_names and 'all' in device_names: @@ -228,17 +186,19 @@ class Distro(distros.Distro): return distros.Distro._bring_up_interfaces(self, device_names) def set_timezone(self, tz): - tz_file = os.path.join("/usr/share/zoneinfo", tz) + # Ensure that this timezone is actually + # available on this system, if not give up + tz_file = os.path.join(self.tz_zone_dir, str(tz)) if not os.path.isfile(tz_file): raise RuntimeError(("Invalid timezone %s," " no file found at %s") % (tz, tz_file)) # Adjust the sysconfig clock zone setting clock_cfg = { - 'ZONE': tz, + 'ZONE': str(tz), } - self._update_sysconfig_file("/etc/sysconfig/clock", clock_cfg) + self._update_sysconfig_file(self.clock_conf_fn, clock_cfg) # This ensures that the correct tz will be used for the system - util.copy(tz_file, "/etc/localtime") + util.copy(tz_file, self.tz_local_fn) def package_command(self, command, args=None): cmd = ['yum'] @@ -262,51 +222,6 @@ class Distro(distros.Distro): ["makecache"], freq=PER_INSTANCE) -# 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) - - # This is a util function to translate a ubuntu /etc/network/interfaces 'blob' # to a rhel equiv. that can then be written to /etc/sysconfig/network-scripts/ # TODO(harlowja) remove when we have python-netcf active... diff --git a/cloudinit/util.py b/cloudinit/util.py index 79676305..918deba2 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -52,6 +52,7 @@ from cloudinit import importer from cloudinit import log as logging from cloudinit import safeyaml from cloudinit import url_helper as uhelp +from cloudinit import version from cloudinit.settings import (CFG_BUILTIN) @@ -272,11 +273,7 @@ def uniq_merge(*lists): # Kickout the empty ones a_list = [a for a in a_list if len(a)] combined_list.extend(a_list) - uniq_list = [] - for i in combined_list: - if i not in uniq_list: - uniq_list.append(i) - return uniq_list + return uniq_list(combined_list) def clean_filename(fn): @@ -1429,6 +1426,14 @@ def subp(args, data=None, rcs=None, env=None, capture=True, shell=False, return (out, err) +def make_header(comment_char="#", base='created'): + ci_ver = version.version_string() + header = str(comment_char) + header += " %s by cloud-init v. %s" % (base.title(), ci_ver) + header += " on %s" % time_rfc2822() + return header + + def abs_join(*paths): return os.path.abspath(os.path.join(*paths)) diff --git a/tests/unittests/test_distros/test_hosts.py b/tests/unittests/test_distros/test_hosts.py index 621837ab..687a0dab 100644 --- a/tests/unittests/test_distros/test_hosts.py +++ b/tests/unittests/test_distros/test_hosts.py @@ -1,6 +1,6 @@ from mocker import MockerTestCase -from cloudinit.distros import helpers +from cloudinit.distros.parsers import hosts BASE_ETC = ''' @@ -16,7 +16,7 @@ BASE_ETC = BASE_ETC.strip() class TestHostsHelper(MockerTestCase): def test_parse(self): - eh = helpers.HostsConf(BASE_ETC) + eh = hosts.HostsConf(BASE_ETC) self.assertEquals(eh.get_entry('127.0.0.1'), [['localhost']]) self.assertEquals(eh.get_entry('192.168.1.10'), [['foo.mydomain.org', 'foo'], @@ -25,7 +25,7 @@ class TestHostsHelper(MockerTestCase): self.assertTrue(eh.startswith('# Example')) def test_add(self): - eh = helpers.HostsConf(BASE_ETC) + eh = hosts.HostsConf(BASE_ETC) eh.add_entry('127.0.0.0', 'blah') self.assertEquals(eh.get_entry('127.0.0.0'), [['blah']]) eh.add_entry('127.0.0.3', 'blah', 'blah2', 'blah3') @@ -33,7 +33,7 @@ class TestHostsHelper(MockerTestCase): [['blah', 'blah2', 'blah3']]) def test_del(self): - eh = helpers.HostsConf(BASE_ETC) + eh = hosts.HostsConf(BASE_ETC) eh.add_entry('127.0.0.0', 'blah') self.assertEquals(eh.get_entry('127.0.0.0'), [['blah']]) diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py index 55765f0c..b7ce6fea 100644 --- a/tests/unittests/test_distros/test_netconfig.py +++ b/tests/unittests/test_distros/test_netconfig.py @@ -83,7 +83,7 @@ class TestNetCfgDistro(MockerTestCase): self.assertEquals(write_buf.mode, 0644) def assertCfgEquals(self, blob1, blob2): - cfg_tester = distros.rhel.QuotingConfigObj + cfg_tester = distros.parsers.quoting_conf.QuotingConfigObj b1 = dict(cfg_tester(blob1.strip().splitlines())) b2 = dict(cfg_tester(blob2.strip().splitlines())) self.assertEquals(b1, b2) diff --git a/tests/unittests/test_distros/test_resolv.py b/tests/unittests/test_distros/test_resolv.py index 5f122833..d947dda0 100644 --- a/tests/unittests/test_distros/test_resolv.py +++ b/tests/unittests/test_distros/test_resolv.py @@ -1,6 +1,8 @@ from mocker import MockerTestCase -from cloudinit.distros import helpers +from cloudinit.distros.parsers import resolv_conf + +import re BASE_RESOLVE = ''' @@ -14,12 +16,12 @@ BASE_RESOLVE = BASE_RESOLVE.strip() class TestResolvHelper(MockerTestCase): def test_parse_same(self): - rp = helpers.ResolvConf(BASE_RESOLVE) + rp = resolv_conf.ResolvConf(BASE_RESOLVE) rp_r = str(rp).strip() self.assertEquals(BASE_RESOLVE, rp_r) def test_local_domain(self): - rp = helpers.ResolvConf(BASE_RESOLVE) + rp = resolv_conf.ResolvConf(BASE_RESOLVE) self.assertEquals(None, rp.local_domain) rp.local_domain = "bob" @@ -27,7 +29,7 @@ class TestResolvHelper(MockerTestCase): self.assertIn('domain bob', str(rp)) def test_nameservers(self): - rp = helpers.ResolvConf(BASE_RESOLVE) + rp = resolv_conf.ResolvConf(BASE_RESOLVE) self.assertIn('10.15.44.14', rp.nameservers) self.assertIn('10.15.30.92', rp.nameservers) rp.add_nameserver('10.2') @@ -41,12 +43,12 @@ class TestResolvHelper(MockerTestCase): self.assertNotIn('10.3', rp.nameservers) def test_search_domains(self): - rp = helpers.ResolvConf(BASE_RESOLVE) + rp = resolv_conf.ResolvConf(BASE_RESOLVE) self.assertIn('yahoo.com', rp.search_domains) self.assertIn('blah.yahoo.com', rp.search_domains) rp.add_search_domain('bbb.y.com') self.assertIn('bbb.y.com', rp.search_domains) - self.assertRegexpMatches(str(rp), r'search(.*)bbb.y.com(.*)') + self.assertTrue(re.search(r'search(.*)bbb.y.com(.*)', str(rp))) self.assertIn('bbb.y.com', rp.search_domains) rp.add_search_domain('bbb.y.com') self.assertEquals(len(rp.search_domains), 3) -- cgit v1.2.3 From 0f1a2cbe434cba243ce65ff43a88722c2bcf6f2c Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 11 Oct 2012 12:49:45 -0700 Subject: 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. --- cloudinit/distros/debian.py | 47 ++++++++------ cloudinit/distros/parsers/hostname.py | 90 ++++++++++++++++++++++++++ cloudinit/distros/parsers/hosts.py | 3 +- cloudinit/distros/parsers/quoting_conf.py | 80 ----------------------- cloudinit/distros/parsers/sys_conf.py | 85 ++++++++++++++++++++++++ cloudinit/distros/rhel.py | 13 ++-- tests/unittests/test_distros/test_hostname.py | 38 +++++++++++ tests/unittests/test_distros/test_netconfig.py | 7 +- tests/unittests/test_distros/test_sysconfig.py | 59 +++++++++++++++++ 9 files changed, 315 insertions(+), 107 deletions(-) create mode 100644 cloudinit/distros/parsers/hostname.py delete mode 100644 cloudinit/distros/parsers/quoting_conf.py create mode 100644 cloudinit/distros/parsers/sys_conf.py create mode 100644 tests/unittests/test_distros/test_hostname.py create mode 100644 tests/unittests/test_distros/test_sysconfig.py (limited to 'tests') 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 +# +# 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 . + +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 -# -# 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 . - -# 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 +# +# 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 . + +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) + -- cgit v1.2.3 From 66f6d2d4fd1e8c1b397c9533ee0596d7b09e9824 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 11 Oct 2012 18:48:50 -0700 Subject: Update to use pipes.quote to ensure that variables adjusted in sysconfig files are properly quoted for there common use case, that being sourced into shell scripts. --- cloudinit/distros/parsers/sys_conf.py | 24 ++++++++++-------------- tests/unittests/test_distros/test_sysconfig.py | 14 +++++++++++--- 2 files changed, 21 insertions(+), 17 deletions(-) (limited to 'tests') diff --git a/cloudinit/distros/parsers/sys_conf.py b/cloudinit/distros/parsers/sys_conf.py index 3d8802b8..7549c7a3 100644 --- a/cloudinit/distros/parsers/sys_conf.py +++ b/cloudinit/distros/parsers/sys_conf.py @@ -18,6 +18,7 @@ from StringIO import StringIO +import pipes import re # This library is used to parse/write @@ -30,6 +31,7 @@ import re import configobj + class SysConf(configobj.ConfigObj): def __init__(self, contents): configobj.ConfigObj.__init__(self, contents, @@ -50,24 +52,18 @@ class SysConf(configobj.ConfigObj): 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) + quot_func = (lambda x: str(x)) + if value[0] in ['"', "'"] and value[-1] in ['"', "'"]: + if len(value) == 1: + quot_func = self._get_single_quote else: - # Quote whitespace if it isn't the start+end of a shell command + # 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) + if re.search(r"[\t\r\n ]", value) and not white_space_ok: + quot_func = pipes.quote + return quot_func(value) def _write_line(self, indent_string, entry, this_entry, comment): # Ensure it is formatted fine for diff --git a/tests/unittests/test_distros/test_sysconfig.py b/tests/unittests/test_distros/test_sysconfig.py index 196d090d..a07a251e 100644 --- a/tests/unittests/test_distros/test_sysconfig.py +++ b/tests/unittests/test_distros/test_sysconfig.py @@ -1,5 +1,7 @@ from mocker import MockerTestCase +import re + from cloudinit.distros.parsers.sys_conf import SysConf @@ -7,6 +9,11 @@ from cloudinit.distros.parsers.sys_conf import SysConf # http://content.hccfl.edu/pollock/AUnix1/SysconfigFilesDesc.txt class TestSysConfHelper(MockerTestCase): + def assertRegexpMatches(self, text, regexp): + regexp = re.compile(regexp) + self.assertTrue(regexp.search(text), + msg="%s must match %s!" % (text, regexp.pattern)) + def test_parse_no_change(self): contents = '''# A comment USESMBAUTH=no @@ -16,8 +23,8 @@ 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" +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') @@ -25,6 +32,7 @@ USEMD5=no''' # Should be unquoted self.assertEquals(conf['ETHTOOL_OPTS'], ('-K ${DEVICE} tso on; ' '-G ${DEVICE} rx 256 tx 256')) + # This is harmless convertion self.assertEquals(contents, str(conf)) def test_parse_adjust(self): @@ -36,7 +44,7 @@ USEMD5=no''' conf['IPV6TO4_ROUTING'] = "blah \tblah" contents2 = str(conf).strip() # Should be requoted due to whitespace - self.assertEquals('IPV6TO4_ROUTING="blah \tblah"', contents2) + self.assertRegexpMatches(contents2, r'IPV6TO4_ROUTING=["\']blah \tblah["\']') def test_parse_no_adjust_shell(self): conf = SysConf(''.splitlines()) -- cgit v1.2.3 From 3cf8b618f9efcdb2e9cca695c2930a2bf14d42ec Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 11 Oct 2012 19:10:42 -0700 Subject: Handle the case where the value contains shell variables to be expanded which when using pipes.quote will now not be expanded, so add some checks to ensure that this case will still happen. --- cloudinit/distros/parsers/sys_conf.py | 17 ++++++++++++++++- tests/unittests/test_distros/test_sysconfig.py | 5 ++--- 2 files changed, 18 insertions(+), 4 deletions(-) (limited to 'tests') diff --git a/cloudinit/distros/parsers/sys_conf.py b/cloudinit/distros/parsers/sys_conf.py index 7549c7a3..80c305fe 100644 --- a/cloudinit/distros/parsers/sys_conf.py +++ b/cloudinit/distros/parsers/sys_conf.py @@ -31,6 +31,12 @@ import re import configobj +def _contains_shell_variable(text): + if (re.search(r"\$\{.+\}", text) or + re.search(r"\$[a-zA-Z_]+[a-zA-Z0-9_]*", text)): + return True + return False + class SysConf(configobj.ConfigObj): def __init__(self, contents): @@ -62,7 +68,16 @@ class SysConf(configobj.ConfigObj): if value.strip().startswith("$(") and value.strip().endswith(")"): white_space_ok = True if re.search(r"[\t\r\n ]", value) and not white_space_ok: - quot_func = pipes.quote + if _contains_shell_variable(value): + # If it contains shell variables then we likely want to + # leave it alone since the pipes.quote function likes to + # use single quotes which won't get expanded... + if re.search(r"[\n\"']", value): + quot_func = (lambda x: self._get_triple_quote(x) % x) + else: + quot_func = (lambda x: self._get_single_quote(x) % x) + else: + quot_func = pipes.quote return quot_func(value) def _write_line(self, indent_string, entry, this_entry, comment): diff --git a/tests/unittests/test_distros/test_sysconfig.py b/tests/unittests/test_distros/test_sysconfig.py index a07a251e..21e161ad 100644 --- a/tests/unittests/test_distros/test_sysconfig.py +++ b/tests/unittests/test_distros/test_sysconfig.py @@ -24,7 +24,7 @@ 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' +ETHTOOL_OPTS="-K ${DEVICE} tso on; -G ${DEVICE} rx 256 tx 256" USEMD5=no''' conf = SysConf(contents.splitlines()) self.assertEquals(conf['HOSTNAME'], 'blahblah') @@ -32,7 +32,6 @@ USEMD5=no''' # Should be unquoted self.assertEquals(conf['ETHTOOL_OPTS'], ('-K ${DEVICE} tso on; ' '-G ${DEVICE} rx 256 tx 256')) - # This is harmless convertion self.assertEquals(contents, str(conf)) def test_parse_adjust(self): @@ -44,7 +43,7 @@ USEMD5=no''' conf['IPV6TO4_ROUTING'] = "blah \tblah" contents2 = str(conf).strip() # Should be requoted due to whitespace - self.assertRegexpMatches(contents2, r'IPV6TO4_ROUTING=["\']blah \tblah["\']') + self.assertRegexpMatches(contents2, r'IPV6TO4_ROUTING=[\']blah\s+blah[\']') def test_parse_no_adjust_shell(self): conf = SysConf(''.splitlines()) -- cgit v1.2.3 From bbe325c902ef3a3b8845cd3c1bb8bee0c3c74a89 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 11 Oct 2012 21:24:30 -0700 Subject: Add some more sysconfig tests + pylint cleanups. --- cloudinit/distros/__init__.py | 3 +- cloudinit/distros/parsers/sys_conf.py | 59 +++++++++++++++++--------- tests/unittests/test_distros/test_sysconfig.py | 21 ++++++++- 3 files changed, 59 insertions(+), 24 deletions(-) (limited to 'tests') diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index bafa69d3..fa7cc1ca 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -132,7 +132,8 @@ class Distro(object): # temporarily (until reboot so it should # not be depended on). Use the write # hostname functions for 'permanent' adjustments. - LOG.debug("Non-persistently setting the system hostname to %s", hostname) + LOG.debug("Non-persistently setting the system hostname to %s", + hostname) try: util.subp(['hostname', hostname]) except util.ProcessExecutionError: diff --git a/cloudinit/distros/parsers/sys_conf.py b/cloudinit/distros/parsers/sys_conf.py index 1cefb8bc..5cd765fc 100644 --- a/cloudinit/distros/parsers/sys_conf.py +++ b/cloudinit/distros/parsers/sys_conf.py @@ -22,7 +22,7 @@ import pipes import re # This library is used to parse/write -# out the various sysconfig files edited +# out the various sysconfig files edited (best attempt effort) # # It has to be slightly modified though # to ensure that all values are quoted/unquoted correctly @@ -30,11 +30,26 @@ import re # bash scripts... import configobj +# See: http://pubs.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap08.html +# or look at the 'param_expand()' function in the subst.c file in the bash +# source tarball... +SHELL_VAR_RULE = r'[a-zA-Z_]+[a-zA-Z0-9_]*' +SHELL_VAR_REGEXES = [ + # Basic variables + re.compile(r"\$" + SHELL_VAR_RULE), + # Things like $?, $0, $-, $@ + re.compile(r"\$[0-9#\?\-@\*]"), + # Things like ${blah:1} - but this one + # gets very complex so just try the + # simple path + re.compile(r"\$\{.+\}"), +] + def _contains_shell_variable(text): - if (re.search(r"\$\{.+\}", text) or - re.search(r"\$[a-zA-Z_]+[a-zA-Z0-9_]*", text)): - return True + for r in SHELL_VAR_REGEXES: + if r.search(text): + return True return False @@ -58,33 +73,36 @@ class SysConf(configobj.ConfigObj): raise ValueError('Value "%s" is not a string' % (value)) if len(value) == 0: return '' - quot_func = (lambda x: str(x)) + quot_func = None if value[0] in ['"', "'"] and value[-1] in ['"', "'"]: if len(value) == 1: - quot_func = (lambda x: self._get_single_quote(x) % x) + quot_func = (lambda x: + self._get_single_quote(x) % x) 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\r\n ]", value) and not white_space_ok: - if _contains_shell_variable(value): - # If it contains shell variables then we likely want to - # leave it alone since the pipes.quote function likes to - # use single quotes which won't get expanded... - if re.search(r"[\n\"']", value): - quot_func = (lambda x: self._get_triple_quote(x) % x) + pass + else: + if re.search(r"[\t\r\n ]", value): + if _contains_shell_variable(value): + # If it contains shell variables then we likely want to + # leave it alone since the pipes.quote function likes + # to use single quotes which won't get expanded... + if re.search(r"[\n\"']", value): + quot_func = (lambda x: + self._get_triple_quote(x) % x) + else: + quot_func = (lambda x: + self._get_single_quote(x) % x) else: - quot_func = (lambda x: self._get_single_quote(x) % x) - else: - quot_func = pipes.quote + quot_func = pipes.quote + if not quot_func: + return value return quot_func(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) @@ -93,4 +111,3 @@ class SysConf(configobj.ConfigObj): self._a_to_u('='), val, cmnt) - diff --git a/tests/unittests/test_distros/test_sysconfig.py b/tests/unittests/test_distros/test_sysconfig.py index 21e161ad..1e34909d 100644 --- a/tests/unittests/test_distros/test_sysconfig.py +++ b/tests/unittests/test_distros/test_sysconfig.py @@ -9,7 +9,8 @@ from cloudinit.distros.parsers.sys_conf import SysConf # http://content.hccfl.edu/pollock/AUnix1/SysconfigFilesDesc.txt class TestSysConfHelper(MockerTestCase): - def assertRegexpMatches(self, text, regexp): + # This function was added in 2.7, make it work for 2.6 + def assertRegMatches(self, text, regexp): regexp = re.compile(regexp) self.assertTrue(regexp.search(text), msg="%s must match %s!" % (text, regexp.pattern)) @@ -34,6 +35,21 @@ USEMD5=no''' '-G ${DEVICE} rx 256 tx 256')) self.assertEquals(contents, str(conf)) + def test_parse_shell_vars(self): + contents = 'USESMBAUTH=$XYZ' + conf = SysConf(contents.splitlines()) + self.assertEquals(contents, str(conf)) + conf = SysConf('') + conf['B'] = '${ZZ}d apples' + # Should be quoted + self.assertEquals('B="${ZZ}d apples"', str(conf)) + conf = SysConf('') + conf['B'] = '$? d apples' + self.assertEquals('B="$? d apples"', str(conf)) + contents = 'IPMI_WATCHDOG_OPTIONS="timeout=60"' + conf = SysConf(contents.splitlines()) + self.assertEquals('IPMI_WATCHDOG_OPTIONS=timeout=60', str(conf)) + def test_parse_adjust(self): contents = 'IPV6TO4_ROUTING="eth0-:0004::1/64 eth1-:0005::1/64"' conf = SysConf(contents.splitlines()) @@ -43,7 +59,8 @@ USEMD5=no''' conf['IPV6TO4_ROUTING'] = "blah \tblah" contents2 = str(conf).strip() # Should be requoted due to whitespace - self.assertRegexpMatches(contents2, r'IPV6TO4_ROUTING=[\']blah\s+blah[\']') + self.assertRegMatches(contents2, + r'IPV6TO4_ROUTING=[\']blah\s+blah[\']') def test_parse_no_adjust_shell(self): conf = SysConf(''.splitlines()) -- cgit v1.2.3