Merge ~chad.smith/cloud-init:bug/180134-openstack-random-seed-encoding into cloud-init:master

Proposed by Chad Smith on 2019-09-27
Status: Merged
Approved by: Chad Smith on 2019-09-27
Approved revision: 4cdfdca07d8181d37d092d43c7d6806cec1e26b7
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~chad.smith/cloud-init:bug/180134-openstack-random-seed-encoding
Merge into: cloud-init:master
Diff against target: 108 lines (+50/-9)
3 files modified
cloudinit/sources/tests/test_init.py (+5/-7)
cloudinit/tests/test_util.py (+20/-0)
cloudinit/util.py (+25/-2)
Reviewer Review Type Date Requested Status
Robert Schweikert (community) 2019-09-27 Approve on 2019-09-27
Server Team CI bot continuous-integration Approve on 2019-09-27
cloud-init Commiters 2019-09-27 Pending
Review via email: mp+373291@code.launchpad.net

Commit message

util: json.dumps on python 2.7 will handle UnicodeDecodeError on binary

Since python 2.7 doesn't handle UnicodeDecodeErrors with the default
handler

LP: #1801364

To post a comment you must log in.

FAILED: Continuous integration, rev:45e8ac7edc90bde12416380daf72822224080204
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1184/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)
Scott Moser (smoser) wrote :

doing an import in an exception handling path doesn't seem right.
other than that, good work.

PASSED: Continuous integration, rev:4cdfdca07d8181d37d092d43c7d6806cec1e26b7
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1185/
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/1185//rebuild

review: Approve (continuous-integration)
Robert Schweikert (rjschwei) wrote :

LGTM, thanks

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py
2index 6378e98..9698261 100644
3--- a/cloudinit/sources/tests/test_init.py
4+++ b/cloudinit/sources/tests/test_init.py
5@@ -457,19 +457,17 @@ class TestDataSource(CiTestCase):
6 instance_json['ds']['meta_data'])
7
8 @skipIf(not six.PY2, "Only python2 hits UnicodeDecodeErrors on non-utf8")
9- def test_non_utf8_encoding_logs_warning(self):
10- """When non-utf-8 values exist in py2 instance-data is not written."""
11+ def test_non_utf8_encoding_gets_b64encoded(self):
12+ """When non-utf-8 values exist in py2 instance-data is b64encoded."""
13 tmp = self.tmp_dir()
14 datasource = DataSourceTestSubclassNet(
15 self.sys_cfg, self.distro, Paths({'run_dir': tmp}),
16 custom_metadata={'key1': 'val1', 'key2': {'key2.1': b'ab\xaadef'}})
17 self.assertTrue(datasource.get_data())
18 json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp)
19- self.assertFalse(os.path.exists(json_file))
20- self.assertIn(
21- "WARNING: Error persisting instance-data.json: 'utf8' codec can't"
22- " decode byte 0xaa in position 2: invalid start byte",
23- self.logs.getvalue())
24+ instance_json = util.load_json(util.load_file(json_file))
25+ key21_value = instance_json['ds']['meta_data']['key2']['key2.1']
26+ self.assertEqual('ci-b64:' + util.b64e(b'ab\xaadef'), key21_value)
27
28 def test_get_hostname_subclass_support(self):
29 """Validate get_hostname signature on all subclasses of DataSource."""
30diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py
31index e3d2dba..f4f95e9 100644
32--- a/cloudinit/tests/test_util.py
33+++ b/cloudinit/tests/test_util.py
34@@ -2,7 +2,9 @@
35
36 """Tests for cloudinit.util"""
37
38+import base64
39 import logging
40+import json
41 import platform
42
43 import cloudinit.util as util
44@@ -528,6 +530,24 @@ class TestGetLinuxDistro(CiTestCase):
45 self.assertEqual(('foo', '1.1', 'aarch64'), dist)
46
47
48+class TestJsonDumps(CiTestCase):
49+ def test_is_str(self):
50+ """json_dumps should return a string."""
51+ self.assertTrue(isinstance(util.json_dumps({'abc': '123'}), str))
52+
53+ def test_utf8(self):
54+ smiley = '\\ud83d\\ude03'
55+ self.assertEqual(
56+ {'smiley': smiley},
57+ json.loads(util.json_dumps({'smiley': smiley})))
58+
59+ def test_non_utf8(self):
60+ blob = b'\xba\x03Qx-#y\xea'
61+ self.assertEqual(
62+ {'blob': 'ci-b64:' + base64.b64encode(blob).decode('utf-8')},
63+ json.loads(util.json_dumps({'blob': blob})))
64+
65+
66 @mock.patch('os.path.exists')
67 class TestIsLXD(CiTestCase):
68
69diff --git a/cloudinit/util.py b/cloudinit/util.py
70index aa23b3f..6e8e73b 100644
71--- a/cloudinit/util.py
72+++ b/cloudinit/util.py
73@@ -1599,10 +1599,33 @@ def json_serialize_default(_obj):
74 return 'Warning: redacted unserializable type {0}'.format(type(_obj))
75
76
77+def json_preserialize_binary(data):
78+ """Preserialize any discovered binary values to avoid json.dumps issues.
79+
80+ Used only on python 2.7 where default type handling is not honored for
81+ failure to encode binary data. LP: #1801364.
82+ TODO(Drop this function when py2.7 support is dropped from cloud-init)
83+ """
84+ data = obj_copy.deepcopy(data)
85+ for key, value in data.items():
86+ if isinstance(value, (dict)):
87+ data[key] = json_preserialize_binary(value)
88+ if isinstance(value, bytes):
89+ data[key] = 'ci-b64:{0}'.format(b64e(value))
90+ return data
91+
92+
93 def json_dumps(data):
94 """Return data in nicely formatted json."""
95- return json.dumps(data, indent=1, sort_keys=True,
96- separators=(',', ': '), default=json_serialize_default)
97+ try:
98+ return json.dumps(
99+ data, indent=1, sort_keys=True, separators=(',', ': '),
100+ default=json_serialize_default)
101+ except UnicodeDecodeError:
102+ if sys.version_info[:2] == (2, 7):
103+ data = json_preserialize_binary(data)
104+ return json.dumps(data)
105+ raise
106
107
108 def yaml_dumps(obj, explicit_start=True, explicit_end=True, noalias=False):

Subscribers

People subscribed via source and target branches