cloud-init

Merge ~smoser/cloud-init:fix/read_file_or_url-contents-should-be-text into cloud-init:master

  1. Git
  2. lp:~smoser/cloud-init
  3. fix/read_file_or_url-contents-should-be-text
  4. Merge into master
Proposed by Scott Moser
Status: Merged
Approved by: Chad Smith
Approved revision: ab62be9aa53f1450c99aa5596aa2232b47f38030
Merge reported by: Chad Smith
Merged at revision: 30e730f7ca111487d243ba9f40c66df6d7a49953
Proposed branch: ~smoser/cloud-init:fix/read_file_or_url-contents-should-be-text
Merge into: cloud-init:master
Diff against target: 847 lines (+131/-141)
17 files modified
cloudinit/cmd/main.py (+1/-1)
cloudinit/config/cc_phone_home.py (+4/-3)
cloudinit/config/schema.py (+2/-2)
cloudinit/ec2_utils.py (+6/-8)
cloudinit/sources/DataSourceMAAS.py (+1/-1)
cloudinit/sources/helpers/azure.py (+3/-2)
cloudinit/tests/test_url_helper.py (+27/-1)
cloudinit/url_helper.py (+28/-1)
cloudinit/user_data.py (+3/-3)
cloudinit/util.py (+4/-33)
tests/unittests/test__init__.py (+4/-4)
tests/unittests/test_datasource/test_azure_helper.py (+1/-1)
tests/unittests/test_handler/test_handler_apt_conf_v1.py (+6/-10)
tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py (+0/-7)
tests/unittests/test_handler/test_handler_apt_source_v1.py (+10/-17)
tests/unittests/test_handler/test_handler_apt_source_v3.py (+10/-17)
tests/unittests/test_handler/test_handler_ntp.py (+21/-30)
Reviewer Review Type Date Requested Status
Chad Smith Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+343127@code.launchpad.net

Commit message

read_file_or_url: move to url_helper, fix bug in its FileResponse.

The result of a read_file_or_url on a file and on a url would differ
in behavior.
  str(UrlResponse) would return UrlResponse.contents.decode('utf-8')
while
  str(FileResponse) would return str(FileResponse.contents)

The difference being "b'foo'" versus "foo".

As part of the general goal of cleaning util, move read_file_or_url
into url_helper.

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:d2de560b2ea0471d9730ab16cd9fa6334be1ffcc
https://jenkins.ubuntu.com/server/job/cloud-init-ci/999/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:a6c448127b13cc34622427505fa84469658db949
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1000/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    FAILED: Ubuntu LTS: Integration

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:162d2022c539f538287d56d3ec1d3f59c32fd3a6
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1037/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

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

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

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

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

review: Approve (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) wrote :

LGTM!

Was able to validate tracebacks without your changeset using the script below and python3.

Traceback on python3 (tip)

Traceback (most recent call last):
  File "test_read.py", line 12, in <module>
    print(f)
TypeError: __str__ returned non-string (type bytes)

With your changes things look good, FileResponse.content and UrlResponse.content are both binary, and str(*Response) is text.

from cloudinit.url_helper import read_file_or_url

web = read_file_or_url('http://ack.io')
print('----------- url -------------')
print(web.contents)
print(web)

print('----------- file -------------')
f = read_file_or_url('file:///tmp/counting')
print(f.contents)
print(f)

review: Approve
Revision history for this message
Chad Smith (chad.smith) wrote :

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=30e730f7

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
2index 3f2dbb9..d6ba90f 100644
3--- a/cloudinit/cmd/main.py
4+++ b/cloudinit/cmd/main.py
5@@ -187,7 +187,7 @@ def attempt_cmdline_url(path, network=True, cmdline=None):
6 data = None
7 header = b'#cloud-config'
8 try:
9- resp = util.read_file_or_url(**kwargs)
10+ resp = url_helper.read_file_or_url(**kwargs)
11 if resp.ok():
12 data = resp.contents
13 if not resp.contents.startswith(header):
14diff --git a/cloudinit/config/cc_phone_home.py b/cloudinit/config/cc_phone_home.py
15index 878069b..3be0d1c 100644
16--- a/cloudinit/config/cc_phone_home.py
17+++ b/cloudinit/config/cc_phone_home.py
18@@ -41,6 +41,7 @@ keys to post. Available keys are:
19 """
20
21 from cloudinit import templater
22+from cloudinit import url_helper
23 from cloudinit import util
24
25 from cloudinit.settings import PER_INSTANCE
26@@ -136,9 +137,9 @@ def handle(name, cfg, cloud, log, args):
27 }
28 url = templater.render_string(url, url_params)
29 try:
30- util.read_file_or_url(url, data=real_submit_keys,
31- retries=tries, sec_between=3,
32- ssl_details=util.fetch_ssl_details(cloud.paths))
33+ url_helper.read_file_or_url(
34+ url, data=real_submit_keys, retries=tries, sec_between=3,
35+ ssl_details=util.fetch_ssl_details(cloud.paths))
36 except Exception:
37 util.logexc(log, "Failed to post phone home data to %s in %s tries",
38 url, tries)
39diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py
40index 76826e0..3bad8e2 100644
41--- a/cloudinit/config/schema.py
42+++ b/cloudinit/config/schema.py
43@@ -4,7 +4,7 @@
44 from __future__ import print_function
45
46 from cloudinit import importer
47-from cloudinit.util import find_modules, read_file_or_url
48+from cloudinit.util import find_modules, load_file
49
50 import argparse
51 from collections import defaultdict
52@@ -139,7 +139,7 @@ def validate_cloudconfig_file(config_path, schema, annotate=False):
53 """
54 if not os.path.exists(config_path):
55 raise RuntimeError('Configfile {0} does not exist'.format(config_path))
56- content = read_file_or_url('file://{0}'.format(config_path)).contents
57+ content = load_file(config_path, decode=False)
58 if not content.startswith(CLOUD_CONFIG_HEADER):
59 errors = (
60 ('header', 'File {0} needs to begin with "{1}"'.format(
61diff --git a/cloudinit/ec2_utils.py b/cloudinit/ec2_utils.py
62index dc3f0fc..3b7b17f 100644
63--- a/cloudinit/ec2_utils.py
64+++ b/cloudinit/ec2_utils.py
65@@ -150,11 +150,9 @@ def get_instance_userdata(api_version='latest',
66 # NOT_FOUND occurs) and just in that case returning an empty string.
67 exception_cb = functools.partial(_skip_retry_on_codes,
68 SKIP_USERDATA_CODES)
69- response = util.read_file_or_url(ud_url,
70- ssl_details=ssl_details,
71- timeout=timeout,
72- retries=retries,
73- exception_cb=exception_cb)
74+ response = url_helper.read_file_or_url(
75+ ud_url, ssl_details=ssl_details, timeout=timeout,
76+ retries=retries, exception_cb=exception_cb)
77 user_data = response.contents
78 except url_helper.UrlError as e:
79 if e.code not in SKIP_USERDATA_CODES:
80@@ -169,9 +167,9 @@ def _get_instance_metadata(tree, api_version='latest',
81 ssl_details=None, timeout=5, retries=5,
82 leaf_decoder=None):
83 md_url = url_helper.combine_url(metadata_address, api_version, tree)
84- caller = functools.partial(util.read_file_or_url,
85- ssl_details=ssl_details, timeout=timeout,
86- retries=retries)
87+ caller = functools.partial(
88+ url_helper.read_file_or_url, ssl_details=ssl_details,
89+ timeout=timeout, retries=retries)
90
91 def mcaller(url):
92 return caller(url).contents
93diff --git a/cloudinit/sources/DataSourceMAAS.py b/cloudinit/sources/DataSourceMAAS.py
94index aa56add..bcb3854 100644
95--- a/cloudinit/sources/DataSourceMAAS.py
96+++ b/cloudinit/sources/DataSourceMAAS.py
97@@ -198,7 +198,7 @@ def read_maas_seed_url(seed_url, read_file_or_url=None, timeout=None,
98 If version is None, then <version>/ will not be used.
99 """
100 if read_file_or_url is None:
101- read_file_or_url = util.read_file_or_url
102+ read_file_or_url = url_helper.read_file_or_url
103
104 if seed_url.endswith("/"):
105 seed_url = seed_url[:-1]
106diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py
107index 90c12df..e5696b1 100644
108--- a/cloudinit/sources/helpers/azure.py
109+++ b/cloudinit/sources/helpers/azure.py
110@@ -14,6 +14,7 @@ from cloudinit import temp_utils
111 from contextlib import contextmanager
112 from xml.etree import ElementTree
113
114+from cloudinit import url_helper
115 from cloudinit import util
116
117 LOG = logging.getLogger(__name__)
118@@ -55,14 +56,14 @@ class AzureEndpointHttpClient(object):
119 if secure:
120 headers = self.headers.copy()
121 headers.update(self.extra_secure_headers)
122- return util.read_file_or_url(url, headers=headers)
123+ return url_helper.read_file_or_url(url, headers=headers)
124
125 def post(self, url, data=None, extra_headers=None):
126 headers = self.headers
127 if extra_headers is not None:
128 headers = self.headers.copy()
129 headers.update(extra_headers)
130- return util.read_file_or_url(url, data=data, headers=headers)
131+ return url_helper.read_file_or_url(url, data=data, headers=headers)
132
133
134 class GoalState(object):
135diff --git a/cloudinit/tests/test_url_helper.py b/cloudinit/tests/test_url_helper.py
136index b778a3a..113249d 100644
137--- a/cloudinit/tests/test_url_helper.py
138+++ b/cloudinit/tests/test_url_helper.py
139@@ -1,7 +1,10 @@
140 # This file is part of cloud-init. See LICENSE file for license information.
141
142-from cloudinit.url_helper import oauth_headers
143+from cloudinit.url_helper import oauth_headers, read_file_or_url
144 from cloudinit.tests.helpers import CiTestCase, mock, skipIf
145+from cloudinit import util
146+
147+import httpretty
148
149
150 try:
151@@ -38,3 +41,26 @@ class TestOAuthHeaders(CiTestCase):
152 'url', 'consumer_key', 'token_key', 'token_secret',
153 'consumer_secret')
154 self.assertEqual('url', return_value)
155+
156+
157+class TestReadFileOrUrl(CiTestCase):
158+ def test_read_file_or_url_str_from_file(self):
159+ """Test that str(result.contents) on file is text version of contents.
160+ It should not be "b'data'", but just "'data'" """
161+ tmpf = self.tmp_path("myfile1")
162+ data = b'This is my file content\n'
163+ util.write_file(tmpf, data, omode="wb")
164+ result = read_file_or_url("file://%s" % tmpf)
165+ self.assertEqual(result.contents, data)
166+ self.assertEqual(str(result), data.decode('utf-8'))
167+
168+ @httpretty.activate
169+ def test_read_file_or_url_str_from_url(self):
170+ """Test that str(result.contents) on url is text version of contents.
171+ It should not be "b'data'", but just "'data'" """
172+ url = 'http://hostname/path'
173+ data = b'This is my url content\n'
174+ httpretty.register_uri(httpretty.GET, url, data)
175+ result = read_file_or_url(url)
176+ self.assertEqual(result.contents, data)
177+ self.assertEqual(str(result), data.decode('utf-8'))
178diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py
179index 1de07b1..8067979 100644
180--- a/cloudinit/url_helper.py
181+++ b/cloudinit/url_helper.py
182@@ -15,6 +15,7 @@ import six
183 import time
184
185 from email.utils import parsedate
186+from errno import ENOENT
187 from functools import partial
188 from itertools import count
189 from requests import exceptions
190@@ -80,6 +81,32 @@ def combine_url(base, *add_ons):
191 return url
192
193
194+def read_file_or_url(url, timeout=5, retries=10,
195+ headers=None, data=None, sec_between=1, ssl_details=None,
196+ headers_cb=None, exception_cb=None):
197+ url = url.lstrip()
198+ if url.startswith("/"):
199+ url = "file://%s" % url
200+ if url.lower().startswith("file://"):
201+ if data:
202+ LOG.warning("Unable to post data to file resource %s", url)
203+ file_path = url[len("file://"):]
204+ try:
205+ with open(file_path, "rb") as fp:
206+ contents = fp.read()
207+ except IOError as e:
208+ code = e.errno
209+ if e.errno == ENOENT:
210+ code = NOT_FOUND
211+ raise UrlError(cause=e, code=code, headers=None, url=url)
212+ return FileResponse(file_path, contents=contents)
213+ else:
214+ return readurl(url, timeout=timeout, retries=retries, headers=headers,
215+ headers_cb=headers_cb, data=data,
216+ sec_between=sec_between, ssl_details=ssl_details,
217+ exception_cb=exception_cb)
218+
219+
220 # Made to have same accessors as UrlResponse so that the
221 # read_file_or_url can return this or that object and the
222 # 'user' of those objects will not need to know the difference.
223@@ -96,7 +123,7 @@ class StringResponse(object):
224 return True
225
226 def __str__(self):
227- return self.contents
228+ return self.contents.decode('utf-8')
229
230
231 class FileResponse(StringResponse):
232diff --git a/cloudinit/user_data.py b/cloudinit/user_data.py
233index cc55daf..8f6aba1 100644
234--- a/cloudinit/user_data.py
235+++ b/cloudinit/user_data.py
236@@ -19,7 +19,7 @@ import six
237
238 from cloudinit import handlers
239 from cloudinit import log as logging
240-from cloudinit.url_helper import UrlError
241+from cloudinit.url_helper import read_file_or_url, UrlError
242 from cloudinit import util
243
244 LOG = logging.getLogger(__name__)
245@@ -224,8 +224,8 @@ class UserDataProcessor(object):
246 content = util.load_file(include_once_fn)
247 else:
248 try:
249- resp = util.read_file_or_url(include_url,
250- ssl_details=self.ssl_details)
251+ resp = read_file_or_url(include_url,
252+ ssl_details=self.ssl_details)
253 if include_once_on and resp.ok():
254 util.write_file(include_once_fn, resp.contents,
255 mode=0o600)
256diff --git a/cloudinit/util.py b/cloudinit/util.py
257index 310758d..efcbf9f 100644
258--- a/cloudinit/util.py
259+++ b/cloudinit/util.py
260@@ -857,37 +857,6 @@ def fetch_ssl_details(paths=None):
261 return ssl_details
262
263
264-def read_file_or_url(url, timeout=5, retries=10,
265- headers=None, data=None, sec_between=1, ssl_details=None,
266- headers_cb=None, exception_cb=None):
267- url = url.lstrip()
268- if url.startswith("/"):
269- url = "file://%s" % url
270- if url.lower().startswith("file://"):
271- if data:
272- LOG.warning("Unable to post data to file resource %s", url)
273- file_path = url[len("file://"):]
274- try:
275- contents = load_file(file_path, decode=False)
276- except IOError as e:
277- code = e.errno
278- if e.errno == ENOENT:
279- code = url_helper.NOT_FOUND
280- raise url_helper.UrlError(cause=e, code=code, headers=None,
281- url=url)
282- return url_helper.FileResponse(file_path, contents=contents)
283- else:
284- return url_helper.readurl(url,
285- timeout=timeout,
286- retries=retries,
287- headers=headers,
288- headers_cb=headers_cb,
289- data=data,
290- sec_between=sec_between,
291- ssl_details=ssl_details,
292- exception_cb=exception_cb)
293-
294-
295 def load_yaml(blob, default=None, allowed=(dict,)):
296 loaded = default
297 blob = decode_binary(blob)
298@@ -925,12 +894,14 @@ def read_seeded(base="", ext="", timeout=5, retries=10, file_retries=0):
299 ud_url = "%s%s%s" % (base, "user-data", ext)
300 md_url = "%s%s%s" % (base, "meta-data", ext)
301
302- md_resp = read_file_or_url(md_url, timeout, retries, file_retries)
303+ md_resp = url_helper.read_file_or_url(md_url, timeout, retries,
304+ file_retries)
305 md = None
306 if md_resp.ok():
307 md = load_yaml(decode_binary(md_resp.contents), default={})
308
309- ud_resp = read_file_or_url(ud_url, timeout, retries, file_retries)
310+ ud_resp = url_helper.read_file_or_url(ud_url, timeout, retries,
311+ file_retries)
312 ud = None
313 if ud_resp.ok():
314 ud = ud_resp.contents
315diff --git a/tests/unittests/test__init__.py b/tests/unittests/test__init__.py
316index f1ab02e..739bbeb 100644
317--- a/tests/unittests/test__init__.py
318+++ b/tests/unittests/test__init__.py
319@@ -182,7 +182,7 @@ class TestCmdlineUrl(CiTestCase):
320 self.assertEqual(
321 ('url', 'http://example.com'), main.parse_cmdline_url(cmdline))
322
323- @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
324+ @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
325 def test_invalid_content(self, m_read):
326 key = "cloud-config-url"
327 url = 'http://example.com/foo'
328@@ -196,7 +196,7 @@ class TestCmdlineUrl(CiTestCase):
329 self.assertIn(url, msg)
330 self.assertFalse(os.path.exists(fpath))
331
332- @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
333+ @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
334 def test_valid_content(self, m_read):
335 url = "http://example.com/foo"
336 payload = b"#cloud-config\nmydata: foo\nbar: wark\n"
337@@ -210,7 +210,7 @@ class TestCmdlineUrl(CiTestCase):
338 self.assertEqual(logging.INFO, lvl)
339 self.assertIn(url, msg)
340
341- @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
342+ @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
343 def test_no_key_found(self, m_read):
344 cmdline = "ro mykey=http://example.com/foo root=foo"
345 fpath = self.tmp_path("ccpath")
346@@ -221,7 +221,7 @@ class TestCmdlineUrl(CiTestCase):
347 self.assertFalse(os.path.exists(fpath))
348 self.assertEqual(logging.DEBUG, lvl)
349
350- @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
351+ @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
352 def test_exception_warns(self, m_read):
353 url = "http://example.com/foo"
354 cmdline = "ro cloud-config-url=%s root=LABEL=bar" % url
355diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py
356index b42b073..af9d3e1 100644
357--- a/tests/unittests/test_datasource/test_azure_helper.py
358+++ b/tests/unittests/test_datasource/test_azure_helper.py
359@@ -195,7 +195,7 @@ class TestAzureEndpointHttpClient(CiTestCase):
360 self.addCleanup(patches.close)
361
362 self.read_file_or_url = patches.enter_context(
363- mock.patch.object(azure_helper.util, 'read_file_or_url'))
364+ mock.patch.object(azure_helper.url_helper, 'read_file_or_url'))
365
366 def test_non_secure_get(self):
367 client = azure_helper.AzureEndpointHttpClient(mock.MagicMock())
368diff --git a/tests/unittests/test_handler/test_handler_apt_conf_v1.py b/tests/unittests/test_handler/test_handler_apt_conf_v1.py
369index 83f962a..6a4b03e 100644
370--- a/tests/unittests/test_handler/test_handler_apt_conf_v1.py
371+++ b/tests/unittests/test_handler/test_handler_apt_conf_v1.py
372@@ -12,10 +12,6 @@ import shutil
373 import tempfile
374
375
376-def load_tfile_or_url(*args, **kwargs):
377- return(util.decode_binary(util.read_file_or_url(*args, **kwargs).contents))
378-
379-
380 class TestAptProxyConfig(TestCase):
381 def setUp(self):
382 super(TestAptProxyConfig, self).setUp()
383@@ -36,7 +32,7 @@ class TestAptProxyConfig(TestCase):
384 self.assertTrue(os.path.isfile(self.pfile))
385 self.assertFalse(os.path.isfile(self.cfile))
386
387- contents = load_tfile_or_url(self.pfile)
388+ contents = util.load_file(self.pfile)
389 self.assertTrue(self._search_apt_config(contents, "http", "myproxy"))
390
391 def test_apt_http_proxy_written(self):
392@@ -46,7 +42,7 @@ class TestAptProxyConfig(TestCase):
393 self.assertTrue(os.path.isfile(self.pfile))
394 self.assertFalse(os.path.isfile(self.cfile))
395
396- contents = load_tfile_or_url(self.pfile)
397+ contents = util.load_file(self.pfile)
398 self.assertTrue(self._search_apt_config(contents, "http", "myproxy"))
399
400 def test_apt_all_proxy_written(self):
401@@ -64,7 +60,7 @@ class TestAptProxyConfig(TestCase):
402 self.assertTrue(os.path.isfile(self.pfile))
403 self.assertFalse(os.path.isfile(self.cfile))
404
405- contents = load_tfile_or_url(self.pfile)
406+ contents = util.load_file(self.pfile)
407
408 for ptype, pval in values.items():
409 self.assertTrue(self._search_apt_config(contents, ptype, pval))
410@@ -80,7 +76,7 @@ class TestAptProxyConfig(TestCase):
411 cc_apt_configure.apply_apt_config({'proxy': "foo"},
412 self.pfile, self.cfile)
413 self.assertTrue(os.path.isfile(self.pfile))
414- contents = load_tfile_or_url(self.pfile)
415+ contents = util.load_file(self.pfile)
416 self.assertTrue(self._search_apt_config(contents, "http", "foo"))
417
418 def test_config_written(self):
419@@ -92,14 +88,14 @@ class TestAptProxyConfig(TestCase):
420 self.assertTrue(os.path.isfile(self.cfile))
421 self.assertFalse(os.path.isfile(self.pfile))
422
423- self.assertEqual(load_tfile_or_url(self.cfile), payload)
424+ self.assertEqual(util.load_file(self.cfile), payload)
425
426 def test_config_replaced(self):
427 util.write_file(self.pfile, "content doesnt matter")
428 cc_apt_configure.apply_apt_config({'conf': "foo"},
429 self.pfile, self.cfile)
430 self.assertTrue(os.path.isfile(self.cfile))
431- self.assertEqual(load_tfile_or_url(self.cfile), "foo")
432+ self.assertEqual(util.load_file(self.cfile), "foo")
433
434 def test_config_deleted(self):
435 # if no 'conf' is provided, delete any previously written file
436diff --git a/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py b/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py
437index d2b96f0..23bd6e1 100644
438--- a/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py
439+++ b/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py
440@@ -64,13 +64,6 @@ deb-src http://archive.ubuntu.com/ubuntu/ fakerelease main restricted
441 """)
442
443
444-def load_tfile_or_url(*args, **kwargs):
445- """load_tfile_or_url
446- load file and return content after decoding
447- """
448- return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents)
449-
450-
451 class TestAptSourceConfigSourceList(t_help.FilesystemMockingTestCase):
452 """TestAptSourceConfigSourceList
453 Main Class to test sources list rendering
454diff --git a/tests/unittests/test_handler/test_handler_apt_source_v1.py b/tests/unittests/test_handler/test_handler_apt_source_v1.py
455index 46ca4ce..a3132fb 100644
456--- a/tests/unittests/test_handler/test_handler_apt_source_v1.py
457+++ b/tests/unittests/test_handler/test_handler_apt_source_v1.py
458@@ -39,13 +39,6 @@ S0ORP6HXET3+jC8BMG4tBWCTK/XEZw==
459 ADD_APT_REPO_MATCH = r"^[\w-]+:\w"
460
461
462-def load_tfile_or_url(*args, **kwargs):
463- """load_tfile_or_url
464- load file and return content after decoding
465- """
466- return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents)
467-
468-
469 class FakeDistro(object):
470 """Fake Distro helper object"""
471 def update_package_sources(self):
472@@ -125,7 +118,7 @@ class TestAptSourceConfig(TestCase):
473
474 self.assertTrue(os.path.isfile(filename))
475
476- contents = load_tfile_or_url(filename)
477+ contents = util.load_file(filename)
478 self.assertTrue(re.search(r"%s %s %s %s\n" %
479 ("deb", "http://archive.ubuntu.com/ubuntu",
480 "karmic-backports",
481@@ -157,13 +150,13 @@ class TestAptSourceConfig(TestCase):
482 self.apt_src_basic(self.aptlistfile, cfg)
483
484 # extra verify on two extra files of this test
485- contents = load_tfile_or_url(self.aptlistfile2)
486+ contents = util.load_file(self.aptlistfile2)
487 self.assertTrue(re.search(r"%s %s %s %s\n" %
488 ("deb", "http://archive.ubuntu.com/ubuntu",
489 "precise-backports",
490 "main universe multiverse restricted"),
491 contents, flags=re.IGNORECASE))
492- contents = load_tfile_or_url(self.aptlistfile3)
493+ contents = util.load_file(self.aptlistfile3)
494 self.assertTrue(re.search(r"%s %s %s %s\n" %
495 ("deb", "http://archive.ubuntu.com/ubuntu",
496 "lucid-backports",
497@@ -220,7 +213,7 @@ class TestAptSourceConfig(TestCase):
498
499 self.assertTrue(os.path.isfile(filename))
500
501- contents = load_tfile_or_url(filename)
502+ contents = util.load_file(filename)
503 self.assertTrue(re.search(r"%s %s %s %s\n" %
504 ("deb", params['MIRROR'], params['RELEASE'],
505 "multiverse"),
506@@ -241,12 +234,12 @@ class TestAptSourceConfig(TestCase):
507
508 # extra verify on two extra files of this test
509 params = self._get_default_params()
510- contents = load_tfile_or_url(self.aptlistfile2)
511+ contents = util.load_file(self.aptlistfile2)
512 self.assertTrue(re.search(r"%s %s %s %s\n" %
513 ("deb", params['MIRROR'], params['RELEASE'],
514 "main"),
515 contents, flags=re.IGNORECASE))
516- contents = load_tfile_or_url(self.aptlistfile3)
517+ contents = util.load_file(self.aptlistfile3)
518 self.assertTrue(re.search(r"%s %s %s %s\n" %
519 ("deb", params['MIRROR'], params['RELEASE'],
520 "universe"),
521@@ -296,7 +289,7 @@ class TestAptSourceConfig(TestCase):
522
523 self.assertTrue(os.path.isfile(filename))
524
525- contents = load_tfile_or_url(filename)
526+ contents = util.load_file(filename)
527 self.assertTrue(re.search(r"%s %s %s %s\n" %
528 ("deb",
529 ('http://ppa.launchpad.net/smoser/'
530@@ -336,14 +329,14 @@ class TestAptSourceConfig(TestCase):
531 'filename': self.aptlistfile3}
532
533 self.apt_src_keyid(self.aptlistfile, [cfg1, cfg2, cfg3], 3)
534- contents = load_tfile_or_url(self.aptlistfile2)
535+ contents = util.load_file(self.aptlistfile2)
536 self.assertTrue(re.search(r"%s %s %s %s\n" %
537 ("deb",
538 ('http://ppa.launchpad.net/smoser/'
539 'cloud-init-test/ubuntu'),
540 "xenial", "universe"),
541 contents, flags=re.IGNORECASE))
542- contents = load_tfile_or_url(self.aptlistfile3)
543+ contents = util.load_file(self.aptlistfile3)
544 self.assertTrue(re.search(r"%s %s %s %s\n" %
545 ("deb",
546 ('http://ppa.launchpad.net/smoser/'
547@@ -375,7 +368,7 @@ class TestAptSourceConfig(TestCase):
548
549 self.assertTrue(os.path.isfile(filename))
550
551- contents = load_tfile_or_url(filename)
552+ contents = util.load_file(filename)
553 self.assertTrue(re.search(r"%s %s %s %s\n" %
554 ("deb",
555 ('http://ppa.launchpad.net/smoser/'
556diff --git a/tests/unittests/test_handler/test_handler_apt_source_v3.py b/tests/unittests/test_handler/test_handler_apt_source_v3.py
557index e486862..7a64c23 100644
558--- a/tests/unittests/test_handler/test_handler_apt_source_v3.py
559+++ b/tests/unittests/test_handler/test_handler_apt_source_v3.py
560@@ -49,13 +49,6 @@ ADD_APT_REPO_MATCH = r"^[\w-]+:\w"
561 TARGET = None
562
563
564-def load_tfile(*args, **kwargs):
565- """load_tfile_or_url
566- load file and return content after decoding
567- """
568- return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents)
569-
570-
571 class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
572 """TestAptSourceConfig
573 Main Class to test apt configs
574@@ -119,7 +112,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
575
576 self.assertTrue(os.path.isfile(filename))
577
578- contents = load_tfile(filename)
579+ contents = util.load_file(filename)
580 self.assertTrue(re.search(r"%s %s %s %s\n" %
581 ("deb", "http://test.ubuntu.com/ubuntu",
582 "karmic-backports",
583@@ -151,13 +144,13 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
584 self._apt_src_basic(self.aptlistfile, cfg)
585
586 # extra verify on two extra files of this test
587- contents = load_tfile(self.aptlistfile2)
588+ contents = util.load_file(self.aptlistfile2)
589 self.assertTrue(re.search(r"%s %s %s %s\n" %
590 ("deb", "http://test.ubuntu.com/ubuntu",
591 "precise-backports",
592 "main universe multiverse restricted"),
593 contents, flags=re.IGNORECASE))
594- contents = load_tfile(self.aptlistfile3)
595+ contents = util.load_file(self.aptlistfile3)
596 self.assertTrue(re.search(r"%s %s %s %s\n" %
597 ("deb", "http://test.ubuntu.com/ubuntu",
598 "lucid-backports",
599@@ -174,7 +167,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
600
601 self.assertTrue(os.path.isfile(filename))
602
603- contents = load_tfile(filename)
604+ contents = util.load_file(filename)
605 self.assertTrue(re.search(r"%s %s %s %s\n" %
606 ("deb", params['MIRROR'], params['RELEASE'],
607 "multiverse"),
608@@ -201,12 +194,12 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
609
610 # extra verify on two extra files of this test
611 params = self._get_default_params()
612- contents = load_tfile(self.aptlistfile2)
613+ contents = util.load_file(self.aptlistfile2)
614 self.assertTrue(re.search(r"%s %s %s %s\n" %
615 ("deb", params['MIRROR'], params['RELEASE'],
616 "main"),
617 contents, flags=re.IGNORECASE))
618- contents = load_tfile(self.aptlistfile3)
619+ contents = util.load_file(self.aptlistfile3)
620 self.assertTrue(re.search(r"%s %s %s %s\n" %
621 ("deb", params['MIRROR'], params['RELEASE'],
622 "universe"),
623@@ -240,7 +233,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
624
625 self.assertTrue(os.path.isfile(filename))
626
627- contents = load_tfile(filename)
628+ contents = util.load_file(filename)
629 self.assertTrue(re.search(r"%s %s %s %s\n" %
630 ("deb",
631 ('http://ppa.launchpad.net/smoser/'
632@@ -277,14 +270,14 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
633 'keyid': "03683F77"}}
634
635 self._apt_src_keyid(self.aptlistfile, cfg, 3)
636- contents = load_tfile(self.aptlistfile2)
637+ contents = util.load_file(self.aptlistfile2)
638 self.assertTrue(re.search(r"%s %s %s %s\n" %
639 ("deb",
640 ('http://ppa.launchpad.net/smoser/'
641 'cloud-init-test/ubuntu'),
642 "xenial", "universe"),
643 contents, flags=re.IGNORECASE))
644- contents = load_tfile(self.aptlistfile3)
645+ contents = util.load_file(self.aptlistfile3)
646 self.assertTrue(re.search(r"%s %s %s %s\n" %
647 ("deb",
648 ('http://ppa.launchpad.net/smoser/'
649@@ -310,7 +303,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
650
651 self.assertTrue(os.path.isfile(self.aptlistfile))
652
653- contents = load_tfile(self.aptlistfile)
654+ contents = util.load_file(self.aptlistfile)
655 self.assertTrue(re.search(r"%s %s %s %s\n" %
656 ("deb",
657 ('http://ppa.launchpad.net/smoser/'
658diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
659index 17c5355..f109c0e 100644
660--- a/tests/unittests/test_handler/test_handler_ntp.py
661+++ b/tests/unittests/test_handler/test_handler_ntp.py
662@@ -155,9 +155,9 @@ class TestNtp(FilesystemMockingTestCase):
663 path=confpath,
664 template_fn=template_fn,
665 template=None)
666- content = util.read_file_or_url('file://' + confpath).contents
667 self.assertEqual(
668- "servers []\npools ['10.0.0.1', '10.0.0.2']\n", content.decode())
669+ "servers []\npools ['10.0.0.1', '10.0.0.2']\n",
670+ util.load_file(confpath))
671
672 def test_write_ntp_config_template_defaults_pools_w_empty_lists(self):
673 """write_ntp_config_template defaults pools servers upon empty config.
674@@ -176,10 +176,9 @@ class TestNtp(FilesystemMockingTestCase):
675 path=confpath,
676 template_fn=template_fn,
677 template=None)
678- content = util.read_file_or_url('file://' + confpath).contents
679 self.assertEqual(
680 "servers []\npools {0}\n".format(pools),
681- content.decode())
682+ util.load_file(confpath))
683
684 def test_defaults_pools_empty_lists_sles(self):
685 """write_ntp_config_template defaults opensuse pools upon empty config.
686@@ -196,11 +195,11 @@ class TestNtp(FilesystemMockingTestCase):
687 path=confpath,
688 template_fn=template_fn,
689 template=None)
690- content = util.read_file_or_url('file://' + confpath).contents
691 for pool in default_pools:
692 self.assertIn('opensuse', pool)
693 self.assertEqual(
694- "servers []\npools {0}\n".format(default_pools), content.decode())
695+ "servers []\npools {0}\n".format(default_pools),
696+ util.load_file(confpath))
697 self.assertIn(
698 "Adding distro default ntp pool servers: {0}".format(
699 ",".join(default_pools)),
700@@ -217,10 +216,9 @@ class TestNtp(FilesystemMockingTestCase):
701 path=confpath,
702 template_fn=template_fn,
703 template=None)
704- content = util.read_file_or_url('file://' + confpath).contents
705 self.assertEqual(
706 "[Time]\nNTP=%s %s \n" % (" ".join(servers), " ".join(pools)),
707- content.decode())
708+ util.load_file(confpath))
709
710 def test_distro_ntp_client_configs(self):
711 """Test we have updated ntp client configs on different distros"""
712@@ -267,17 +265,17 @@ class TestNtp(FilesystemMockingTestCase):
713 cc_ntp.write_ntp_config_template(distro, servers=servers,
714 pools=pools, path=confpath,
715 template_fn=template_fn)
716- content = util.read_file_or_url('file://' + confpath).contents
717+ content = util.load_file(confpath)
718 if client in ['ntp', 'chrony']:
719 expected_servers = '\n'.join([
720 'server {0} iburst'.format(srv) for srv in servers])
721 print('distro=%s client=%s' % (distro, client))
722- self.assertIn(expected_servers, content.decode('utf-8'),
723+ self.assertIn(expected_servers, content,
724 ('failed to render {0} conf'
725 ' for distro:{1}'.format(client, distro)))
726 expected_pools = '\n'.join([
727 'pool {0} iburst'.format(pool) for pool in pools])
728- self.assertIn(expected_pools, content.decode('utf-8'),
729+ self.assertIn(expected_pools, content,
730 ('failed to render {0} conf'
731 ' for distro:{1}'.format(client, distro)))
732 elif client == 'systemd-timesyncd':
733@@ -286,7 +284,7 @@ class TestNtp(FilesystemMockingTestCase):
734 "# See timesyncd.conf(5) for details.\n\n" +
735 "[Time]\nNTP=%s %s \n" % (" ".join(servers),
736 " ".join(pools)))
737- self.assertEqual(expected_content, content.decode())
738+ self.assertEqual(expected_content, content)
739
740 def test_no_ntpcfg_does_nothing(self):
741 """When no ntp section is defined handler logs a warning and noops."""
742@@ -308,10 +306,10 @@ class TestNtp(FilesystemMockingTestCase):
743 confpath = ntpconfig['confpath']
744 m_select.return_value = ntpconfig
745 cc_ntp.handle('cc_ntp', valid_empty_config, mycloud, None, [])
746- content = util.read_file_or_url('file://' + confpath).contents
747 pools = cc_ntp.generate_server_names(mycloud.distro.name)
748 self.assertEqual(
749- "servers []\npools {0}\n".format(pools), content.decode())
750+ "servers []\npools {0}\n".format(pools),
751+ util.load_file(confpath))
752 self.assertNotIn('Invalid config:', self.logs.getvalue())
753
754 @skipUnlessJsonSchema()
755@@ -333,9 +331,8 @@ class TestNtp(FilesystemMockingTestCase):
756 "Invalid config:\nntp.pools.0: 123 is not of type 'string'\n"
757 "ntp.servers.1: None is not of type 'string'",
758 self.logs.getvalue())
759- content = util.read_file_or_url('file://' + confpath).contents
760 self.assertEqual("servers ['valid', None]\npools [123]\n",
761- content.decode())
762+ util.load_file(confpath))
763
764 @skipUnlessJsonSchema()
765 @mock.patch('cloudinit.config.cc_ntp.select_ntp_client')
766@@ -357,9 +354,8 @@ class TestNtp(FilesystemMockingTestCase):
767 "Invalid config:\nntp.pools: 123 is not of type 'array'\n"
768 "ntp.servers: 'non-array' is not of type 'array'",
769 self.logs.getvalue())
770- content = util.read_file_or_url('file://' + confpath).contents
771 self.assertEqual("servers non-array\npools 123\n",
772- content.decode())
773+ util.load_file(confpath))
774
775 @skipUnlessJsonSchema()
776 @mock.patch('cloudinit.config.cc_ntp.select_ntp_client')
777@@ -381,10 +377,9 @@ class TestNtp(FilesystemMockingTestCase):
778 "Invalid config:\nntp: Additional properties are not allowed "
779 "('invalidkey' was unexpected)",
780 self.logs.getvalue())
781- content = util.read_file_or_url('file://' + confpath).contents
782 self.assertEqual(
783 "servers []\npools ['0.mycompany.pool.ntp.org']\n",
784- content.decode())
785+ util.load_file(confpath))
786
787 @skipUnlessJsonSchema()
788 @mock.patch('cloudinit.config.cc_ntp.select_ntp_client')
789@@ -407,10 +402,10 @@ class TestNtp(FilesystemMockingTestCase):
790 " has non-unique elements\nntp.servers: "
791 "['10.0.0.1', '10.0.0.1'] has non-unique elements",
792 self.logs.getvalue())
793- content = util.read_file_or_url('file://' + confpath).contents
794 self.assertEqual(
795 "servers ['10.0.0.1', '10.0.0.1']\n"
796- "pools ['0.mypool.org', '0.mypool.org']\n", content.decode())
797+ "pools ['0.mypool.org', '0.mypool.org']\n",
798+ util.load_file(confpath))
799
800 @mock.patch('cloudinit.config.cc_ntp.select_ntp_client')
801 def test_ntp_handler_timesyncd(self, m_select):
802@@ -426,10 +421,9 @@ class TestNtp(FilesystemMockingTestCase):
803 confpath = ntpconfig['confpath']
804 m_select.return_value = ntpconfig
805 cc_ntp.handle('cc_ntp', cfg, mycloud, None, [])
806- content = util.read_file_or_url('file://' + confpath).contents
807 self.assertEqual(
808 "[Time]\nNTP=192.168.2.1 192.168.2.2 0.mypool.org \n",
809- content.decode())
810+ util.load_file(confpath))
811
812 @mock.patch('cloudinit.config.cc_ntp.select_ntp_client')
813 def test_ntp_handler_enabled_false(self, m_select):
814@@ -466,10 +460,9 @@ class TestNtp(FilesystemMockingTestCase):
815 m_util.subp.assert_called_with(
816 ['systemctl', 'reload-or-restart',
817 service_name], capture=True)
818- content = util.read_file_or_url('file://' + confpath).contents
819 self.assertEqual(
820 "servers []\npools {0}\n".format(pools),
821- content.decode())
822+ util.load_file(confpath))
823
824 def test_opensuse_picks_chrony(self):
825 """Test opensuse picks chrony or ntp on certain distro versions"""
826@@ -638,10 +631,9 @@ class TestNtp(FilesystemMockingTestCase):
827 mock_path = 'cloudinit.config.cc_ntp.temp_utils._TMPDIR'
828 with mock.patch(mock_path, self.new_root):
829 cc_ntp.handle('notimportant', cfg, mycloud, None, None)
830- content = util.read_file_or_url('file://' + confpath).contents
831 self.assertEqual(
832 "servers []\npools ['mypool.org']\n%s" % custom,
833- content.decode())
834+ util.load_file(confpath))
835
836 @mock.patch('cloudinit.config.cc_ntp.supplemental_schema_validation')
837 @mock.patch('cloudinit.config.cc_ntp.reload_ntp')
838@@ -675,10 +667,9 @@ class TestNtp(FilesystemMockingTestCase):
839 with mock.patch(mock_path, self.new_root):
840 cc_ntp.handle('notimportant',
841 {'ntp': cfg}, mycloud, None, None)
842- content = util.read_file_or_url('file://' + confpath).contents
843 self.assertEqual(
844 "servers []\npools ['mypool.org']\n%s" % custom,
845- content.decode())
846+ util.load_file(confpath))
847 m_schema.assert_called_with(expected_merged_cfg)
848
849

Subscribers

People subscribed via source and target branches