cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Chad Smith has proposed merging ~chad.smith/cloud-init:fix-modules-cmdline-help into cloud-init:master.

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1736600 in cloud-init: "cloud-init modules -h documents unsupported --mode init"
  https://bugs.launchpad.net/cloud-init/+bug/1736600

For more details, see:
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/335094

cli: Drop unsupported init option from cloud-init module --mode.

The help docs and argument parser were incorrectly allowing 'init' mode
which caused a traceback because the modules subcommand only allows config
and final modes. Add a check in the cli to raise a ValueError if a new
subcommand ends up allowing an unsupported/unimplemented mode.

LP: #1736600
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:fix-modules-cmdline-help into cloud-init:master.

diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
index aa56225..66627c1 100644
--- a/cloudinit/cmd/main.py
+++ b/cloudinit/cmd/main.py
@@ -604,6 +604,9 @@ def status_wrapper(name, args, data_d=None, link_d=None):
         raise ValueError("unknown name: %s" % name)
 
     modes = ('init', 'init-local', 'modules-config', 'modules-final')
+    if mode not in modes:
+        raise ValueError(
+            "Invalid cloud init mode specified '{0}'".format(mode))
 
     status = None
     if mode == 'init-local':
@@ -722,7 +725,7 @@ def main(sysv_args=None):
                             help=("module configuration name "
                                   "to use (default: %(default)s)"),
                             default='config',
-                            choices=('init', 'config', 'final'))
+                            choices=('config', 'final'))
     parser_mod.set_defaults(action=('modules', main_modules))
 
     # This subcommand allows you to run a single module
diff --git a/tests/unittests/test_cli.py b/tests/unittests/test_cli.py
index a8d28ae..48b25e5 100644
--- a/tests/unittests/test_cli.py
+++ b/tests/unittests/test_cli.py
@@ -1,5 +1,6 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
+from collections import namedtuple
 import six
 
 from cloudinit.cmd import main as cli
@@ -24,6 +25,23 @@ class TestCLI(test_helpers.FilesystemMockingTestCase):
         except SystemExit as e:
             return e.code
 
+    def test_status_wrapper_errors_on_invalid_modes(self):
+        """status_wrapper will error is a parameter combination is invalid."""
+        tmpd = self.tmp_dir()
+        data_d = self.tmp_path('data', tmpd)
+        link_d = self.tmp_path('link', tmpd)
+        FakeArgs = namedtuple('FakeArgs', ['action', 'local', 'mode'])
+
+        def myaction():
+            raise 'Should not be called yet'
+
+        myargs = FakeArgs(('modules_name', myaction), False, 'bogusmode')
+        with self.assertRaises(ValueError) as cm:
+            cli.status_wrapper('modules', myargs, data_d, link_d)
+        self.assertEqual(
+            "Invalid cloud init mode specified 'modules-bogusmode'",
+            str(cm.exception))
+
     def test_no_arguments_shows_usage(self):
         exit_code = self._call_main()
         self.assertIn('usage: cloud-init', self.stderr.getvalue())