cloud-init

Merge ~chad.smith/cloud-init:bug/1798189-instance-data-fallback into cloud-init:master

  1. Git
  2. lp:~chad.smith/cloud-init
  3. bug/1798189-instance-data-fallback
  4. Merge into master
Proposed by Chad Smith
Status: Merged
Approved by: Scott Moser
Approved revision: 5900f42a3ae3ed2c5fd90dbbccb037e15d7d5966
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~chad.smith/cloud-init:bug/1798189-instance-data-fallback
Merge into: cloud-init:master
Diff against target: 220 lines (+126/-19)
4 files modified
cloudinit/cmd/devel/render.py (+16/-7)
cloudinit/cmd/devel/tests/test_render.py (+44/-1)
cloudinit/cmd/query.py (+18/-10)
cloudinit/cmd/tests/test_query.py (+48/-1)
Reviewer Review Type Date Requested Status
Scott Moser Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+357793@code.launchpad.net

Commit message

instance-data: fallback to instance-data.json if sensitive is absent.

On cloud-init upgrade path from 18.3 to 18.4 cloud-init changed how
instance-data is written. Cloud-init changes instance-data.json from root
read-only to redacted world-readable content, and provided a separate
unredacted instance-data-sensitive.json which is read-only root.
Since instance-data is only rewritten from cache on
reboot, the query and render tools needed fallback to use the 'old'
instance-data.json if the new sensitive file isn't yet present.

This avoids error messages from tools about an absebt
/run/instance-data-sensitive.json file.

LP: #1798189

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:5900f42a3ae3ed2c5fd90dbbccb037e15d7d5966
https://jenkins.ubuntu.com/server/job/cloud-init-ci/411/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/411/rebuild

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/cloudinit/cmd/devel/render.py b/cloudinit/cmd/devel/render.py
index 2ba6b68..4d3ec95 100755
--- a/cloudinit/cmd/devel/render.py
+++ b/cloudinit/cmd/devel/render.py
@@ -8,11 +8,10 @@ import sys
88
9from cloudinit.handlers.jinja_template import render_jinja_payload_from_file9from cloudinit.handlers.jinja_template import render_jinja_payload_from_file
10from cloudinit import log10from cloudinit import log
11from cloudinit.sources import INSTANCE_JSON_FILE11from cloudinit.sources import INSTANCE_JSON_FILE, INSTANCE_JSON_SENSITIVE_FILE
12from . import addLogHandlerCLI, read_cfg_paths12from . import addLogHandlerCLI, read_cfg_paths
1313
14NAME = 'render'14NAME = 'render'
15DEFAULT_INSTANCE_DATA = '/run/cloud-init/instance-data.json'
1615
17LOG = log.getLogger(NAME)16LOG = log.getLogger(NAME)
1817
@@ -47,12 +46,22 @@ def handle_args(name, args):
47 @return 0 on success, 1 on failure.46 @return 0 on success, 1 on failure.
48 """47 """
49 addLogHandlerCLI(LOG, log.DEBUG if args.debug else log.WARNING)48 addLogHandlerCLI(LOG, log.DEBUG if args.debug else log.WARNING)
50 if not args.instance_data:49 if args.instance_data:
51 paths = read_cfg_paths()
52 instance_data_fn = os.path.join(
53 paths.run_dir, INSTANCE_JSON_FILE)
54 else:
55 instance_data_fn = args.instance_data50 instance_data_fn = args.instance_data
51 else:
52 paths = read_cfg_paths()
53 uid = os.getuid()
54 redacted_data_fn = os.path.join(paths.run_dir, INSTANCE_JSON_FILE)
55 if uid == 0:
56 instance_data_fn = os.path.join(
57 paths.run_dir, INSTANCE_JSON_SENSITIVE_FILE)
58 if not os.path.exists(instance_data_fn):
59 LOG.warning(
60 'Missing root-readable %s. Using redacted %s instead.',
61 instance_data_fn, redacted_data_fn)
62 instance_data_fn = redacted_data_fn
63 else:
64 instance_data_fn = redacted_data_fn
56 if not os.path.exists(instance_data_fn):65 if not os.path.exists(instance_data_fn):
57 LOG.error('Missing instance-data.json file: %s', instance_data_fn)66 LOG.error('Missing instance-data.json file: %s', instance_data_fn)
58 return 167 return 1
diff --git a/cloudinit/cmd/devel/tests/test_render.py b/cloudinit/cmd/devel/tests/test_render.py
index fc5d2c0..988bba0 100644
--- a/cloudinit/cmd/devel/tests/test_render.py
+++ b/cloudinit/cmd/devel/tests/test_render.py
@@ -6,7 +6,7 @@ import os
6from collections import namedtuple6from collections import namedtuple
7from cloudinit.cmd.devel import render7from cloudinit.cmd.devel import render
8from cloudinit.helpers import Paths8from cloudinit.helpers import Paths
9from cloudinit.sources import INSTANCE_JSON_FILE9from cloudinit.sources import INSTANCE_JSON_FILE, INSTANCE_JSON_SENSITIVE_FILE
10from cloudinit.tests.helpers import CiTestCase, mock, skipUnlessJinja10from cloudinit.tests.helpers import CiTestCase, mock, skipUnlessJinja
11from cloudinit.util import ensure_dir, write_file11from cloudinit.util import ensure_dir, write_file
1212
@@ -63,6 +63,49 @@ class TestRender(CiTestCase):
63 'Missing instance-data.json file: %s' % json_file,63 'Missing instance-data.json file: %s' % json_file,
64 self.logs.getvalue())64 self.logs.getvalue())
6565
66 def test_handle_args_root_fallback_from_sensitive_instance_data(self):
67 """When root user defaults to sensitive.json."""
68 user_data = self.tmp_path('user-data', dir=self.tmp)
69 run_dir = self.tmp_path('run_dir', dir=self.tmp)
70 ensure_dir(run_dir)
71 paths = Paths({'run_dir': run_dir})
72 self.add_patch('cloudinit.cmd.devel.render.read_cfg_paths', 'm_paths')
73 self.m_paths.return_value = paths
74 args = self.args(
75 user_data=user_data, instance_data=None, debug=False)
76 with mock.patch('sys.stderr', new_callable=StringIO):
77 with mock.patch('os.getuid') as m_getuid:
78 m_getuid.return_value = 0
79 self.assertEqual(1, render.handle_args('anyname', args))
80 json_file = os.path.join(run_dir, INSTANCE_JSON_FILE)
81 json_sensitive = os.path.join(run_dir, INSTANCE_JSON_SENSITIVE_FILE)
82 self.assertIn(
83 'WARNING: Missing root-readable %s. Using redacted %s' % (
84 json_sensitive, json_file), self.logs.getvalue())
85 self.assertIn(
86 'ERROR: Missing instance-data.json file: %s' % json_file,
87 self.logs.getvalue())
88
89 def test_handle_args_root_uses_sensitive_instance_data(self):
90 """When root user, and no instance-data arg, use sensitive.json."""
91 user_data = self.tmp_path('user-data', dir=self.tmp)
92 write_file(user_data, '##template: jinja\nrendering: {{ my_var }}')
93 run_dir = self.tmp_path('run_dir', dir=self.tmp)
94 ensure_dir(run_dir)
95 json_sensitive = os.path.join(run_dir, INSTANCE_JSON_SENSITIVE_FILE)
96 write_file(json_sensitive, '{"my-var": "jinja worked"}')
97 paths = Paths({'run_dir': run_dir})
98 self.add_patch('cloudinit.cmd.devel.render.read_cfg_paths', 'm_paths')
99 self.m_paths.return_value = paths
100 args = self.args(
101 user_data=user_data, instance_data=None, debug=False)
102 with mock.patch('sys.stderr', new_callable=StringIO):
103 with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:
104 with mock.patch('os.getuid') as m_getuid:
105 m_getuid.return_value = 0
106 self.assertEqual(0, render.handle_args('anyname', args))
107 self.assertIn('rendering: jinja worked', m_stdout.getvalue())
108
66 @skipUnlessJinja()109 @skipUnlessJinja()
67 def test_handle_args_renders_instance_data_vars_in_template(self):110 def test_handle_args_renders_instance_data_vars_in_template(self):
68 """If user_data file is a jinja template render instance-data vars."""111 """If user_data file is a jinja template render instance-data vars."""
diff --git a/cloudinit/cmd/query.py b/cloudinit/cmd/query.py
index 7d2d4fe..ff03de9 100644
--- a/cloudinit/cmd/query.py
+++ b/cloudinit/cmd/query.py
@@ -79,22 +79,30 @@ def handle_args(name, args):
79 uid = os.getuid()79 uid = os.getuid()
80 if not all([args.instance_data, args.user_data, args.vendor_data]):80 if not all([args.instance_data, args.user_data, args.vendor_data]):
81 paths = read_cfg_paths()81 paths = read_cfg_paths()
82 if not args.instance_data:82 if args.instance_data:
83 instance_data_fn = args.instance_data
84 else:
85 redacted_data_fn = os.path.join(paths.run_dir, INSTANCE_JSON_FILE)
83 if uid == 0:86 if uid == 0:
84 default_json_fn = INSTANCE_JSON_SENSITIVE_FILE87 sensitive_data_fn = os.path.join(
88 paths.run_dir, INSTANCE_JSON_SENSITIVE_FILE)
89 if os.path.exists(sensitive_data_fn):
90 instance_data_fn = sensitive_data_fn
91 else:
92 LOG.warning(
93 'Missing root-readable %s. Using redacted %s instead.',
94 sensitive_data_fn, redacted_data_fn)
95 instance_data_fn = redacted_data_fn
85 else:96 else:
86 default_json_fn = INSTANCE_JSON_FILE # World readable97 instance_data_fn = redacted_data_fn
87 instance_data_fn = os.path.join(paths.run_dir, default_json_fn)98 if args.user_data:
99 user_data_fn = args.user_data
88 else:100 else:
89 instance_data_fn = args.instance_data
90 if not args.user_data:
91 user_data_fn = os.path.join(paths.instance_link, 'user-data.txt')101 user_data_fn = os.path.join(paths.instance_link, 'user-data.txt')
102 if args.vendor_data:
103 vendor_data_fn = args.vendor_data
92 else:104 else:
93 user_data_fn = args.user_data
94 if not args.vendor_data:
95 vendor_data_fn = os.path.join(paths.instance_link, 'vendor-data.txt')105 vendor_data_fn = os.path.join(paths.instance_link, 'vendor-data.txt')
96 else:
97 vendor_data_fn = args.vendor_data
98106
99 try:107 try:
100 instance_json = util.load_file(instance_data_fn)108 instance_json = util.load_file(instance_data_fn)
diff --git a/cloudinit/cmd/tests/test_query.py b/cloudinit/cmd/tests/test_query.py
index fb87c6a..241f541 100644
--- a/cloudinit/cmd/tests/test_query.py
+++ b/cloudinit/cmd/tests/test_query.py
@@ -7,7 +7,8 @@ import os
7from collections import namedtuple7from collections import namedtuple
8from cloudinit.cmd import query8from cloudinit.cmd import query
9from cloudinit.helpers import Paths9from cloudinit.helpers import Paths
10from cloudinit.sources import REDACT_SENSITIVE_VALUE, INSTANCE_JSON_FILE10from cloudinit.sources import (
11 REDACT_SENSITIVE_VALUE, INSTANCE_JSON_FILE, INSTANCE_JSON_SENSITIVE_FILE)
11from cloudinit.tests.helpers import CiTestCase, mock12from cloudinit.tests.helpers import CiTestCase, mock
12from cloudinit.util import ensure_dir, write_file13from cloudinit.util import ensure_dir, write_file
1314
@@ -76,6 +77,52 @@ class TestQuery(CiTestCase):
76 'ERROR: Missing instance-data.json file: %s' % json_file,77 'ERROR: Missing instance-data.json file: %s' % json_file,
77 m_stderr.getvalue())78 m_stderr.getvalue())
7879
80 def test_handle_args_root_fallsback_to_instance_data(self):
81 """When no instance_data argument, root falls back to redacted json."""
82 args = self.args(
83 debug=False, dump_all=True, format=None, instance_data=None,
84 list_keys=False, user_data=None, vendor_data=None, varname=None)
85 run_dir = self.tmp_path('run_dir', dir=self.tmp)
86 ensure_dir(run_dir)
87 paths = Paths({'run_dir': run_dir})
88 self.add_patch('cloudinit.cmd.query.read_cfg_paths', 'm_paths')
89 self.m_paths.return_value = paths
90 with mock.patch('sys.stderr', new_callable=StringIO) as m_stderr:
91 with mock.patch('os.getuid') as m_getuid:
92 m_getuid.return_value = 0
93 self.assertEqual(1, query.handle_args('anyname', args))
94 json_file = os.path.join(run_dir, INSTANCE_JSON_FILE)
95 sensitive_file = os.path.join(run_dir, INSTANCE_JSON_SENSITIVE_FILE)
96 self.assertIn(
97 'WARNING: Missing root-readable %s. Using redacted %s instead.' % (
98 sensitive_file, json_file),
99 m_stderr.getvalue())
100
101 def test_handle_args_root_uses_instance_sensitive_data(self):
102 """When no instance_data argument, root uses semsitive json."""
103 user_data = self.tmp_path('user-data', dir=self.tmp)
104 vendor_data = self.tmp_path('vendor-data', dir=self.tmp)
105 write_file(user_data, 'ud')
106 write_file(vendor_data, 'vd')
107 run_dir = self.tmp_path('run_dir', dir=self.tmp)
108 sensitive_file = os.path.join(run_dir, INSTANCE_JSON_SENSITIVE_FILE)
109 write_file(sensitive_file, '{"my-var": "it worked"}')
110 ensure_dir(run_dir)
111 paths = Paths({'run_dir': run_dir})
112 self.add_patch('cloudinit.cmd.query.read_cfg_paths', 'm_paths')
113 self.m_paths.return_value = paths
114 args = self.args(
115 debug=False, dump_all=True, format=None, instance_data=None,
116 list_keys=False, user_data=vendor_data, vendor_data=vendor_data,
117 varname=None)
118 with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:
119 with mock.patch('os.getuid') as m_getuid:
120 m_getuid.return_value = 0
121 self.assertEqual(0, query.handle_args('anyname', args))
122 self.assertEqual(
123 '{\n "my_var": "it worked",\n "userdata": "vd",\n '
124 '"vendordata": "vd"\n}\n', m_stdout.getvalue())
125
79 def test_handle_args_dumps_all_instance_data(self):126 def test_handle_args_dumps_all_instance_data(self):
80 """When --all is specified query will dump all instance data vars."""127 """When --all is specified query will dump all instance data vars."""
81 write_file(self.instance_data, '{"my-var": "it worked"}')128 write_file(self.instance_data, '{"my-var": "it worked"}')

Subscribers

People subscribed via source and target branches