review comment: better mock and docs for query ci-b64 prefix

cloudinit/cmd/query.py

@@ -1,6 +1,14 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
-"""Query standardized instance metadata from the command line."""
+"""Query standardized instance metadata provided to machine, returning a JSON
+structure.
+
+Some instance-data values may be binary on some platforms, such as userdata and
+vendordata. Attempt to decompress and decode UTF-8 any binary values.
+
+Binary instance-data values which cannot be decompressed or decoded,
+will be base64-encoded and will have the prefix "ci-b64:" on the value.
+"""
 
 import argparse
 from errno import EACCES
@@ -30,7 +38,7 @@ def get_parser(parser=None):
     """
     if not parser:
         parser = argparse.ArgumentParser(
-            prog=NAME, description='Query cloud-init instance data')
+            prog=NAME, description=__doc__)
     parser.add_argument(
         '-d', '--debug', action='store_true', default=False,
         help='Add verbose messages during template render')
@@ -52,8 +60,10 @@ def get_parser(parser=None):
               ' /var/lib/cloud/instance/vendor-data.txt'))
     parser.add_argument(
         'varname', type=str, nargs='?',
-        help=('A dot-delimited instance data variable to query from'
-              ' instance-data query. For example: v2.local_hostname'))
+        help=('A dot-delimited specific instance data variable to query from'
+              ' instance-data query. For example: v1.local_hostname. If the'
+              ' value is not JSON serializable, it will be base64-encoded and'
+              ' will contain the prefix "ci-b64:". '))
     parser.add_argument(
         '-a', '--all', action='store_true', default=False, dest='dump_all',
         help='Dump all available instance-data')

cloudinit/cmd/tests/test_query.py

@@ -24,22 +24,23 @@ def _gzip_data(data):
         return iobuf.getvalue()
 
 
-@mock.patch("cloudinit.cmd.query.addLogHandlerCLI", return_value="")
+@mock.patch("cloudinit.cmd.query.addLogHandlerCLI", lambda *args: "")
 class TestQuery:
 
     args = namedtuple(
         'queryargs',
         ('debug dump_all format instance_data list_keys user_data vendor_data'
          ' varname'))
 
-    def test_handle_args_error_on_missing_param(
-        self, m_cli_log, caplog, capsys
-    ):
+    def test_handle_args_error_on_missing_param(self, caplog, capsys):
         """Error when missing required parameters and print usage."""
         args = self.args(
             debug=False, dump_all=False, format=None, instance_data=None,
             list_keys=False, user_data=None, vendor_data=None, varname=None)
-        assert 1 == query.handle_args('anyname', args)
+        with mock.patch(
+            "cloudinit.cmd.query.addLogHandlerCLI", return_value=""
+        ) as m_cli_log:
+            assert 1 == query.handle_args('anyname', args)
         expected_error = (
             'Expected one of the options: --all, --format, --list-keys'
             ' or varname\n')
@@ -48,9 +49,7 @@ def test_handle_args_error_on_missing_param(
         assert 'usage: query' in out
         assert 1 == m_cli_log.call_count
 
-    def test_handle_args_error_on_missing_instance_data(
-        self, _m_cli_log, caplog, tmpdir
-    ):
+    def test_handle_args_error_on_missing_instance_data(self, caplog, tmpdir):
         """When instance_data file path does not exist, log an error."""
         absent_fn = tmpdir.join('absent')
         args = self.args(
@@ -63,7 +62,7 @@ def test_handle_args_error_on_missing_instance_data(
         assert msg in caplog.text
 
     def test_handle_args_error_when_no_read_permission_instance_data(
-        self, _m_log_cli, caplog, tmpdir
+        self, caplog, tmpdir
     ):
         """When instance_data file is unreadable, log an error."""
         noread_fn = tmpdir.join('unreadable')
@@ -78,9 +77,7 @@ def test_handle_args_error_when_no_read_permission_instance_data(
         msg = "No read permission on '%s'. Try sudo" % noread_fn
         assert msg in caplog.text
 
-    def test_handle_args_defaults_instance_data(
-        self, _m_log_cli, caplog, tmpdir
-    ):
+    def test_handle_args_defaults_instance_data(self, caplog, tmpdir):
         """When no instance_data argument, default to configured run_dir."""
         args = self.args(
             debug=False, dump_all=True, format=None, instance_data=None,
@@ -95,9 +92,7 @@ def test_handle_args_defaults_instance_data(
         msg = 'Missing instance-data file: %s' % json_file.strpath
         assert msg in caplog.text
 
-    def test_handle_args_root_fallsback_to_instance_data(
-        self, _m_log_cli, caplog, tmpdir
-    ):
+    def test_handle_args_root_fallsback_to_instance_data(self, caplog, tmpdir):
         """When no instance_data argument, root falls back to redacted json."""
         args = self.args(
             debug=False, dump_all=True, format=None, instance_data=None,
@@ -135,8 +130,7 @@ def test_handle_args_root_fallsback_to_instance_data(
         )
     )
     def test_handle_args_root_processes_user_data(
-        self, _m_log_cli, ud_src, ud_expected, vd_src, vd_expected, capsys,
-        tmpdir
+        self, ud_src, ud_expected, vd_src, vd_expected, capsys, tmpdir
     ):
         """Support reading multiple user-data file content types"""
         user_data = tmpdir.join('user-data')
@@ -170,7 +164,7 @@ def test_handle_args_root_processes_user_data(
             assert vd_expected == cmd_output['vendordata']
 
     def test_handle_args_root_uses_instance_sensitive_data(
-        self, _m_cli_log, capsys, tmpdir
+        self, capsys, tmpdir
     ):
         """When no instance_data argument, root uses sensitive json."""
         user_data = tmpdir.join('user-data')
@@ -198,9 +192,7 @@ def test_handle_args_root_uses_instance_sensitive_data(
         out, _err = capsys.readouterr()
         assert expected == out
 
-    def test_handle_args_dumps_all_instance_data(
-        self, _m_cli_log, capsys, tmpdir
-    ):
+    def test_handle_args_dumps_all_instance_data(self, capsys, tmpdir):
         """When --all is specified query will dump all instance data vars."""
         instance_data = tmpdir.join('instance-data')
         instance_data.write('{"my-var": "it worked"}')
@@ -220,9 +212,7 @@ def test_handle_args_dumps_all_instance_data(
         out, _err = capsys.readouterr()
         assert expected == out
 
-    def test_handle_args_returns_top_level_varname(
-        self, _m_cli_log, capsys, tmpdir
-    ):
+    def test_handle_args_returns_top_level_varname(self, capsys, tmpdir):
         """When the argument varname is passed, report its value."""
         instance_data = tmpdir.join('instance-data')
         instance_data.write('{"my-var": "it worked"}')
@@ -236,9 +226,7 @@ def test_handle_args_returns_top_level_varname(
         out, _err = capsys.readouterr()
         assert 'it worked\n' == out
 
-    def test_handle_args_returns_nested_varname(
-        self, _m_cli_log, capsys, tmpdir
-    ):
+    def test_handle_args_returns_nested_varname(self, capsys, tmpdir):
         """If user_data file is a jinja template render instance-data vars."""
         instance_data = tmpdir.join('instance-data')
         instance_data.write(
@@ -255,7 +243,7 @@ def test_handle_args_returns_nested_varname(
         assert 'value-2\n' == out
 
     def test_handle_args_returns_standardized_vars_to_top_level_aliases(
-        self, _m_cli_log, capsys, tmpdir
+        self, capsys, tmpdir
     ):
         """Any standardized vars under v# are promoted as top-level aliases."""
         instance_data = tmpdir.join('instance-data')
@@ -288,7 +276,7 @@ def test_handle_args_returns_standardized_vars_to_top_level_aliases(
         assert expected == out
 
     def test_handle_args_list_keys_sorts_top_level_keys_when_no_varname(
-        self, _m_cli_log, capsys, tmpdir
+        self, capsys, tmpdir
     ):
         """Sort all top-level keys when only --list-keys provided."""
         instance_data = tmpdir.join('instance-data')
@@ -307,7 +295,7 @@ def test_handle_args_list_keys_sorts_top_level_keys_when_no_varname(
         assert expected == out
 
     def test_handle_args_list_keys_sorts_nested_keys_when_varname(
-        self, _m_cli_log, capsys, tmpdir
+        self, capsys, tmpdir
     ):
         """Sort all nested keys of varname object when --list-keys provided."""
         instance_data = tmpdir.join('instance-data')
@@ -326,7 +314,7 @@ def test_handle_args_list_keys_sorts_nested_keys_when_varname(
         assert expected == out
 
     def test_handle_args_list_keys_errors_when_varname_is_not_a_dict(
-        self, _m_cli_log, caplog, tmpdir
+        self, caplog, tmpdir
     ):
         """Raise an error when --list-keys and varname specify a non-list."""
         instance_data = tmpdir.join('instance-data')