cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:fix/shellify-raise-type-errors into cloud-init:master.

Commit message:
shellify: raise TypeError on bad input.

This makes 2 changes to shellify's behavior:
 a.) raise a TypeError rather than a RuntimeError.
 b.) raise a TypeError if input is not a list or tuple.

Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/341116

see commit message
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/shellify-raise-type-errors into cloud-init:master.

diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py
index ba6bf69..e9b1880 100644
--- a/cloudinit/tests/test_util.py
+++ b/cloudinit/tests/test_util.py
@@ -44,3 +44,27 @@ class TestUtil(CiTestCase):
         m_mount_info.return_value = ('/dev/sda1', 'btrfs', '/', 'ro,relatime')
         is_rw = util.mount_is_read_write('/')
         self.assertEqual(is_rw, False)
+
+
+class TestShellify(CiTestCase):
+
+    def test_input_dict_raises_type_error(self):
+        self.assertRaisesRegex(
+            TypeError, 'Input.*was.*dict.*xpected',
+            util.shellify, {'mykey': 'myval'})
+
+    def test_input_str_raises_type_error(self):
+        self.assertRaisesRegex(
+            TypeError, 'Input.*was.*str.*xpected', util.shellify, "foobar")
+
+    def test_value_with_int_raises_type_error(self):
+        self.assertRaisesRegex(
+            TypeError, 'shellify.*int', util.shellify, ["foo", 1])
+
+    def test_supports_strings_and_lists(self):
+        ret = util.shellify(["echo hi mom", ["echo", "hi dad"]])
+        self.assertEqual(
+            '\n'.join(["#!/bin/sh", "echo hi mom", "'echo' 'hi dad'",
+                       "'echo' 'hi' 'sis'", ""]),
+            util.shellify(["echo hi mom", ["echo", "hi dad"],
+                           ('echo', 'hi', 'sis')]))
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 1d6cd1c..083a8ef 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1922,6 +1922,11 @@ def abs_join(*paths):
 #    if it is an array, shell protect it (with single ticks)
 #    if it is a string, do nothing
 def shellify(cmdlist, add_header=True):
+    if not isinstance(cmdlist, (tuple, list)):
+        raise TypeError(
+            "Input to shellify was type '%s'. Expected list or tuple." %
+            (type_utils.obj_name(cmdlist)))
+
     content = ''
     if add_header:
         content += "#!/bin/sh\n"
@@ -1930,7 +1935,7 @@ def shellify(cmdlist, add_header=True):
     for args in cmdlist:
         # If the item is a list, wrap all items in single tick.
         # If its not, then just write it directly.
-        if isinstance(args, list):
+        if isinstance(args, (list, tuple)):
             fixed = []
             for f in args:
                 fixed.append("'%s'" % (six.text_type(f).replace("'", escaped)))
@@ -1940,9 +1945,10 @@ def shellify(cmdlist, add_header=True):
             content = "%s%s\n" % (content, args)
             cmds_made += 1
         else:
-            raise RuntimeError(("Unable to shellify type %s"
-                                " which is not a list or string")
-                               % (type_utils.obj_name(args)))
+            raise TypeError(
+                "Unable to shellify type '%s'. Expected list, string, tuple. "
+                "Got: %s" % (type_utils.obj_name(args), args))
+
     LOG.debug("Shellified %s commands.", cmds_made)
     return content