jinja: provide and document jinja-safe key aliases in instance-data (SC-622) (#1123)

Allow #cloud-config and cloud-init query to use underscore-delimited
"jinja-safe" key aliases for any instance-data.json keys
containing jinja operator characters.

This provides a means to use Jinja's dot-notation instead of square brackets
and quoting to reference "unsafe" obtain attribute names.

Support for these aliased keys is available to both #cloud-config user-data and
`cloud-init query`.

For example #cloud-config alias access can look like:
  {{ ds.config.user_network_config }}

  - instead of -

  {{ ds.config["user.network-config"] }}

2 files changed, 166 insertions, 39 deletions

diff --git a/cloudinit/cmd/query.py b/cloudinit/cmd/query.py
index 07db9552..e53cd855 100644
--- a/cloudinit/cmd/query.py
+++ b/cloudinit/cmd/query.py
@@ -19,7 +19,10 @@ import os
 import sys
 
 from cloudinit.handlers.jinja_template import (
-    convert_jinja_instance_data, render_jinja_payload)
+    convert_jinja_instance_data,
+    get_jinja_variable_alias,
+    render_jinja_payload
+)
 from cloudinit.cmd.devel import addLogHandlerCLI, read_cfg_paths
 from cloudinit import log
 from cloudinit.sources import (
@@ -93,22 +96,24 @@ def load_userdata(ud_file_path):
         return util.decomp_gzip(bdata, quiet=False, decode=True)
 
 
-def handle_args(name, args):
-    """Handle calls to 'cloud-init query' as a subcommand."""
-    paths = None
-    addLogHandlerCLI(LOG, log.DEBUG if args.debug else log.WARNING)
-    if not any([args.list_keys, args.varname, args.format, args.dump_all]):
-        LOG.error(
-            'Expected one of the options: --all, --format,'
-            ' --list-keys or varname')
-        get_parser().print_help()
-        return 1
+def _read_instance_data(instance_data, user_data, vendor_data) -> dict:
+    """Return a dict of merged instance-data, vendordata and userdata.
 
+    The dict will contain supplemental userdata and vendordata keys sourced
+    from default user-data and vendor-data files.
+
+    Non-root users will have redacted INSTANCE_JSON_FILE content and redacted
+    vendordata and userdata values.
+
+    :raise: IOError/OSError on absence of instance-data.json file or invalid
+        access perms.
+    """
+    paths = None
     uid = os.getuid()
-    if not all([args.instance_data, args.user_data, args.vendor_data]):
+    if not all([instance_data, user_data, vendor_data]):
         paths = read_cfg_paths()
-    if args.instance_data:
-        instance_data_fn = args.instance_data
+    if instance_data:
+        instance_data_fn = instance_data
     else:
         redacted_data_fn = os.path.join(paths.run_dir, INSTANCE_JSON_FILE)
         if uid == 0:
@@ -124,12 +129,12 @@ def handle_args(name, args):
                 instance_data_fn = redacted_data_fn
         else:
             instance_data_fn = redacted_data_fn
-    if args.user_data:
-        user_data_fn = args.user_data
+    if user_data:
+        user_data_fn = user_data
     else:
         user_data_fn = os.path.join(paths.instance_link, 'user-data.txt')
-    if args.vendor_data:
-        vendor_data_fn = args.vendor_data
+    if vendor_data:
+        vendor_data_fn = vendor_data
     else:
         vendor_data_fn = os.path.join(paths.instance_link, 'vendor-data.txt')
 
@@ -140,7 +145,7 @@ def handle_args(name, args):
             LOG.error("No read permission on '%s'. Try sudo", instance_data_fn)
         else:
             LOG.error('Missing instance-data file: %s', instance_data_fn)
-        return 1
+        raise
 
     instance_data = util.load_json(instance_json)
     if uid != 0:
@@ -151,6 +156,65 @@ def handle_args(name, args):
     else:
         instance_data['userdata'] = load_userdata(user_data_fn)
         instance_data['vendordata'] = load_userdata(vendor_data_fn)
+    return instance_data
+
+
+def _find_instance_data_leaf_by_varname_path(
+    jinja_vars_without_aliases: dict, jinja_vars_with_aliases: dict,
+    varname: str, list_keys: bool
+):
+    """Return the value of the dot-delimited varname path in instance-data
+
+    Split a dot-delimited jinja variable name path into components, walk the
+    path components into the instance_data and look up a matching jinja
+    variable name or cloud-init's underscore-delimited key aliases.
+
+    :raises: ValueError when varname represents an invalid key name or path or
+        if list-keys is provided by varname isn't a dict object.
+    """
+    walked_key_path = ""
+    response = jinja_vars_without_aliases
+    for key_path_part in varname.split('.'):
+        try:
+            # Walk key path using complete aliases dict, yet response
+            # should only contain jinja_without_aliases
+            jinja_vars_with_aliases = jinja_vars_with_aliases[key_path_part]
+        except KeyError as e:
+            if walked_key_path:
+                msg = "instance-data '{key_path}' has no '{leaf}'".format(
+                    leaf=key_path_part, key_path=walked_key_path
+                )
+            else:
+                msg = "Undefined instance-data key '{}'".format(varname)
+            raise ValueError(msg) from e
+        if key_path_part in response:
+            response = response[key_path_part]
+        else:  # We are an underscore_delimited key alias
+            for key in response:
+                if get_jinja_variable_alias(key) == key_path_part:
+                    response = response[key]
+                    break
+        if walked_key_path:
+            walked_key_path += "."
+        walked_key_path += key_path_part
+    return response
+
+
+def handle_args(name, args):
+    """Handle calls to 'cloud-init query' as a subcommand."""
+    addLogHandlerCLI(LOG, log.DEBUG if args.debug else log.WARNING)
+    if not any([args.list_keys, args.varname, args.format, args.dump_all]):
+        LOG.error(
+            'Expected one of the options: --all, --format,'
+            ' --list-keys or varname')
+        get_parser().print_help()
+        return 1
+    try:
+        instance_data = _read_instance_data(
+            args.instance_data, args.user_data, args.vendor_data
+        )
+    except (IOError, OSError):
+        return 1
     if args.format:
         payload = '## template: jinja\n{fmt}'.format(fmt=args.format)
         rendered_payload = render_jinja_payload(
@@ -162,20 +226,32 @@ def handle_args(name, args):
             return 0
         return 1
 
+    # If not rendering a structured format above, query output will be either:
+    #  - JSON dump of all instance-data/jinja variables
+    #  - JSON dump of a value at an dict path into the instance-data dict.
+    #  - a list of keys for a specific dict path into the instance-data dict.
     response = convert_jinja_instance_data(instance_data)
     if args.varname:
+        jinja_vars_with_aliases = convert_jinja_instance_data(
+            instance_data, include_key_aliases=True
+        )
         try:
-            for var in args.varname.split('.'):
-                response = response[var]
-        except KeyError:
-            LOG.error('Undefined instance-data key %s', args.varname)
+            response = _find_instance_data_leaf_by_varname_path(
+                jinja_vars_without_aliases=response,
+                jinja_vars_with_aliases=jinja_vars_with_aliases,
+                varname=args.varname,
+                list_keys=args.list_keys
+            )
+        except (KeyError, ValueError) as e:
+            LOG.error(e)
+            return 1
+    if args.list_keys:
+        if not isinstance(response, dict):
+            LOG.error(
+                "--list-keys provided but '%s' is not a dict",
+                args.varname
+            )
             return 1
-        if args.list_keys:
-            if not isinstance(response, dict):
-                LOG.error("--list-keys provided but '%s' is not a dict", var)
-                return 1
-            response = '\n'.join(sorted(response.keys()))
-    elif args.list_keys:
         response = '\n'.join(sorted(response.keys()))
     if not isinstance(response, str):
         response = util.json_dumps(response)
diff --git a/cloudinit/cmd/tests/test_query.py b/cloudinit/cmd/tests/test_query.py
index c258d321..d96c3945 100644
--- a/cloudinit/cmd/tests/test_query.py
+++ b/cloudinit/cmd/tests/test_query.py
@@ -75,6 +75,40 @@ class TestQuery:
         assert 'usage: query' in out
         assert 1 == m_cli_log.call_count
 
+    @pytest.mark.parametrize(
+        "inst_data,varname,expected_error", (
+            (
+                '{"v1": {"key-2": "value-2"}}',
+                'v1.absent_leaf',
+                "instance-data 'v1' has no 'absent_leaf'\n"
+            ),
+            (
+                '{"v1": {"key-2": "value-2"}}',
+                'absent_key',
+                "Undefined instance-data key 'absent_key'\n"
+            ),
+        )
+    )
+    def test_handle_args_error_on_invalid_vaname_paths(
+        self, inst_data, varname, expected_error, caplog, tmpdir
+    ):
+        """Error when varname is not a valid instance-data variable path."""
+        instance_data = tmpdir.join('instance-data')
+        instance_data.write(inst_data)
+        args = self.args(
+            debug=False, dump_all=False, format=None,
+            instance_data=instance_data.strpath,
+            list_keys=False, user_data=None, vendor_data=None, varname=varname
+        )
+        paths, _, _, _ = self._setup_paths(tmpdir)
+        with mock.patch('cloudinit.cmd.query.read_cfg_paths') as m_paths:
+            m_paths.return_value = paths
+            with mock.patch(
+                "cloudinit.cmd.query.addLogHandlerCLI", return_value=""
+            ):
+                assert 1 == query.handle_args('anyname', args)
+        assert expected_error in caplog.text
+
     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')
@@ -166,7 +200,7 @@ class TestQuery:
                 assert 0 == query.handle_args('anyname', args)
         out, _err = capsys.readouterr()
         cmd_output = json.loads(out)
-        assert "it worked" == cmd_output['my_var']
+        assert "it worked" == cmd_output['my-var']
         if ud_expected == "ci-b64:":
             ud_expected = "ci-b64:{}".format(b64e(ud_src))
         if vd_expected == "ci-b64:":
@@ -193,8 +227,8 @@ class TestQuery:
                 m_getuid.return_value = 0
                 assert 0 == query.handle_args('anyname', args)
         expected = (
-            '{\n "my_var": "it worked",\n "userdata": "ud",\n '
-            '"vendordata": "vd"\n}\n'
+            '{\n "my-var": "it worked",\n '
+            '"userdata": "ud",\n "vendordata": "vd"\n}\n'
         )
         out, _err = capsys.readouterr()
         assert expected == out
@@ -211,7 +245,7 @@ class TestQuery:
             m_getuid.return_value = 100
             assert 0 == query.handle_args('anyname', args)
         expected = (
-            '{\n "my_var": "it worked",\n "userdata": "<%s> file:ud",\n'
+            '{\n "my-var": "it worked",\n "userdata": "<%s> file:ud",\n'
             ' "vendordata": "<%s> file:vd"\n}\n' % (
                 REDACT_SENSITIVE_VALUE, REDACT_SENSITIVE_VALUE
             )
@@ -233,21 +267,38 @@ class TestQuery:
         out, _err = capsys.readouterr()
         assert 'it worked\n' == out
 
-    def test_handle_args_returns_nested_varname(self, capsys, tmpdir):
+    @pytest.mark.parametrize(
+        'inst_data,varname,expected',
+        (
+            (
+                '{"v1": {"key-2": "value-2"}, "my-var": "it worked"}',
+                'v1.key_2',
+                'value-2\n'
+            ),
+            # Assert no jinja underscore-delimited aliases are reported on CLI
+            (
+                '{"v1": {"something-hyphenated": {"no.underscores":"x",'
+                ' "no-alias": "y"}}, "my-var": "it worked"}',
+                'v1.something_hyphenated',
+                '{\n "no-alias": "y",\n "no.underscores": "x"\n}\n'
+            ),
+        )
+    )
+    def test_handle_args_returns_nested_varname(
+        self, inst_data, varname, expected, capsys, tmpdir
+    ):
         """If user_data file is a jinja template render instance-data vars."""
         instance_data = tmpdir.join('instance-data')
-        instance_data.write(
-            '{"v1": {"key-2": "value-2"}, "my-var": "it worked"}'
-        )
+        instance_data.write(inst_data)
         args = self.args(
             debug=False, dump_all=False, format=None,
             instance_data=instance_data.strpath, user_data='ud',
-            vendor_data='vd', list_keys=False, varname='v1.key_2')
+            vendor_data='vd', list_keys=False, varname=varname)
         with mock.patch('os.getuid') as m_getuid:
             m_getuid.return_value = 100
             assert 0 == query.handle_args('anyname', args)
         out, _err = capsys.readouterr()
-        assert 'value-2\n' == out
+        assert expected == out
 
     def test_handle_args_returns_standardized_vars_to_top_level_aliases(
         self, capsys, tmpdir