cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Chad Smith has proposed merging ~chad.smith/cloud-init:cleanup/report-unreadable-instance-data into cloud-init:master.

Commit message:
query: better error when missing read permission on instance-data

Emit a permissions error instead of "Missing instance-data.json" when
non-root user doesn't have read-permission on
/run/cloud-init/instance-data.json

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

For more details, see:
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/358041
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:cleanup/report-unreadable-instance-data into cloud-init:master.

diff --git a/cloudinit/cmd/devel/render.py b/cloudinit/cmd/devel/render.py
index 4d3ec95..1bc2240 100755
--- a/cloudinit/cmd/devel/render.py
+++ b/cloudinit/cmd/devel/render.py
@@ -71,10 +71,14 @@ def handle_args(name, args):
     except IOError:
         LOG.error('Missing user-data file: %s', args.user_data)
         return 1
-    rendered_payload = render_jinja_payload_from_file(
-        payload=user_data, payload_fn=args.user_data,
-        instance_data_file=instance_data_fn,
-        debug=True if args.debug else False)
+    try:
+        rendered_payload = render_jinja_payload_from_file(
+            payload=user_data, payload_fn=args.user_data,
+            instance_data_file=instance_data_fn,
+            debug=True if args.debug else False)
+    except RuntimeError as e:
+        LOG.error('Cannot render from instance data: %s', str(e))
+        return 1
     if not rendered_payload:
         LOG.error('Unable to render user-data file: %s', args.user_data)
         return 1
diff --git a/cloudinit/cmd/query.py b/cloudinit/cmd/query.py
index ff03de9..650bc0a 100644
--- a/cloudinit/cmd/query.py
+++ b/cloudinit/cmd/query.py
@@ -3,6 +3,7 @@
 """Query standardized instance metadata from the command line."""
 
 import argparse
+from errno import EACCES
 import os
 import six
 import sys
@@ -106,8 +107,11 @@ def handle_args(name, args):
 
     try:
         instance_json = util.load_file(instance_data_fn)
-    except IOError:
-        LOG.error('Missing instance-data.json file: %s', instance_data_fn)
+    except (IOError, OSError) as e:
+        if e.errno == EACCES:
+            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
 
     instance_data = util.load_json(instance_json)
diff --git a/cloudinit/cmd/tests/test_query.py b/cloudinit/cmd/tests/test_query.py
index 241f541..3df04ca 100644
--- a/cloudinit/cmd/tests/test_query.py
+++ b/cloudinit/cmd/tests/test_query.py
@@ -1,5 +1,6 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
+import errno
 from six import StringIO
 from textwrap import dedent
 import os
@@ -51,10 +52,28 @@ class TestQuery(CiTestCase):
         with mock.patch('sys.stderr', new_callable=StringIO) as m_stderr:
             self.assertEqual(1, query.handle_args('anyname', args))
         self.assertIn(
-            'ERROR: Missing instance-data.json file: %s' % absent_fn,
+            'ERROR: Missing instance-data file: %s' % absent_fn,
             self.logs.getvalue())
         self.assertIn(
-            'ERROR: Missing instance-data.json file: %s' % absent_fn,
+            'ERROR: Missing instance-data file: %s' % absent_fn,
+            m_stderr.getvalue())
+
+    def test_handle_args_error_when_no_read_permission_instance_data(self):
+        """When instance_data file is unreadable, log an error."""
+        noread_fn = self.tmp_path('unreadable', dir=self.tmp)
+        write_file(noread_fn, 'thou shall not pass')
+        args = self.args(
+            debug=False, dump_all=True, format=None, instance_data=noread_fn,
+            list_keys=False, user_data='ud', vendor_data='vd', varname=None)
+        with mock.patch('sys.stderr', new_callable=StringIO) as m_stderr:
+            with mock.patch('cloudinit.cmd.query.util.load_file') as m_load:
+                m_load.side_effect = OSError(errno.EACCES, 'Not allowed')
+                self.assertEqual(1, query.handle_args('anyname', args))
+        self.assertIn(
+            'ERROR: No read permission on %s. Try sudo' % noread_fn,
+            self.logs.getvalue())
+        self.assertIn(
+            'ERROR: No read permission on %s. Try sudo' % noread_fn,
             m_stderr.getvalue())
 
     def test_handle_args_defaults_instance_data(self):
@@ -71,10 +90,10 @@ class TestQuery(CiTestCase):
             self.assertEqual(1, query.handle_args('anyname', args))
         json_file = os.path.join(run_dir, INSTANCE_JSON_FILE)
         self.assertIn(
-            'ERROR: Missing instance-data.json file: %s' % json_file,
+            'ERROR: Missing instance-data file: %s' % json_file,
             self.logs.getvalue())
         self.assertIn(
-            'ERROR: Missing instance-data.json file: %s' % json_file,
+            'ERROR: Missing instance-data file: %s' % json_file,
             m_stderr.getvalue())
 
     def test_handle_args_root_fallsback_to_instance_data(self):
diff --git a/cloudinit/handlers/jinja_template.py b/cloudinit/handlers/jinja_template.py
index 3fa4097..06e931b 100644
--- a/cloudinit/handlers/jinja_template.py
+++ b/cloudinit/handlers/jinja_template.py
@@ -1,5 +1,6 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
+from errno import EACCES
 import os
 import re
 
@@ -76,7 +77,14 @@ def render_jinja_payload_from_file(
         raise RuntimeError(
             'Cannot render jinja template vars. Instance data not yet'
             ' present at %s' % instance_data_file)
-    instance_data = load_json(load_file(instance_data_file))
+    try:
+        instance_data = load_json(load_file(instance_data_file))
+    except (IOError, OSError) as e:
+        if e.errno == EACCES:
+            raise RuntimeError(
+                'Cannot render jinja template vars. No read permission on %s.'
+                ' Try sudo' % instance_data_file)
+
     rendered_payload = render_jinja_payload(
         payload, payload_fn, instance_data, debug)
     if not rendered_payload:
diff --git a/doc/rtd/topics/network-config-format-v1.rst b/doc/rtd/topics/network-config-format-v1.rst
index 8352000..3b0148c 100644
--- a/doc/rtd/topics/network-config-format-v1.rst
+++ b/doc/rtd/topics/network-config-format-v1.rst
@@ -332,7 +332,7 @@ the following keys:
            - type: static
              address: 192.168.23.14/27
              gateway: 192.168.23.1
-      - type: nameserver
+      - type: nameserver:
         address:
           - 192.168.23.2
           - 8.8.8.8
diff --git a/tests/unittests/test_builtin_handlers.py b/tests/unittests/test_builtin_handlers.py
index abe820e..dfa3c9b 100644
--- a/tests/unittests/test_builtin_handlers.py
+++ b/tests/unittests/test_builtin_handlers.py
@@ -3,6 +3,7 @@
 """Tests of the built-in user data handlers."""
 
 import copy
+import errno
 import os
 import shutil
 import tempfile
@@ -202,6 +203,30 @@ class TestJinjaTemplatePartHandler(CiTestCase):
             os.path.exists(script_file),
             'Unexpected file created %s' % script_file)
 
+    def test_jinja_template_handle_errors_on_unreadable_instance_data(self):
+        """If instance-data is unreadable, raise an error from handle_part."""
+        script_handler = ShellScriptPartHandler(self.paths)
+        instance_json = os.path.join(self.run_dir, 'instance-data.json')
+        util.write_file(instance_json, util.json_dumps({}))
+        h = JinjaTemplatePartHandler(
+            self.paths, sub_handlers=[script_handler])
+        with mock.patch(self.mpath + 'load_file') as m_load:
+            with self.assertRaises(RuntimeError) as context_manager:
+                m_load.side_effect = OSError(errno.EACCES, 'Not allowed')
+                h.handle_part(
+                    data='data', ctype="!" + handlers.CONTENT_START,
+                    filename='part01',
+                    payload='## template: jinja  \n#!/bin/bash\necho himom',
+                    frequency='freq', headers='headers')
+        script_file = os.path.join(script_handler.script_dir, 'part01')
+        self.assertEqual(
+            'Cannot render jinja template vars. No read permission on'
+            ' {rdir}/instance-data.json. Try sudo'.format(rdir=self.run_dir),
+            str(context_manager.exception))
+        self.assertFalse(
+            os.path.exists(script_file),
+            'Unexpected file created %s' % script_file)
+
     @skipUnlessJinja()
     def test_jinja_template_handle_renders_jinja_content(self):
         """When present, render jinja variables from instance-data.json."""