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.

17 files changed, 131 insertions, 141 deletions

diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
index 3f2dbb93..d6ba90f4 100644
--- a/cloudinit/cmd/main.py
+++ b/cloudinit/cmd/main.py
@@ -187,7 +187,7 @@ def attempt_cmdline_url(path, network=True, cmdline=None):
     data = None
     header = b'#cloud-config'
     try:
-        resp = util.read_file_or_url(**kwargs)
+        resp = url_helper.read_file_or_url(**kwargs)
         if resp.ok():
             data = resp.contents
             if not resp.contents.startswith(header):
diff --git a/cloudinit/config/cc_phone_home.py b/cloudinit/config/cc_phone_home.py
index 878069b7..3be0d1c1 100644
--- a/cloudinit/config/cc_phone_home.py
+++ b/cloudinit/config/cc_phone_home.py
@@ -41,6 +41,7 @@ keys to post. Available keys are:
 """
 
 from cloudinit import templater
+from cloudinit import url_helper
 from cloudinit import util
 
 from cloudinit.settings import PER_INSTANCE
@@ -136,9 +137,9 @@ def handle(name, cfg, cloud, log, args):
     }
     url = templater.render_string(url, url_params)
     try:
-        util.read_file_or_url(url, data=real_submit_keys,
-                              retries=tries, sec_between=3,
-                              ssl_details=util.fetch_ssl_details(cloud.paths))
+        url_helper.read_file_or_url(
+            url, data=real_submit_keys, retries=tries, sec_between=3,
+            ssl_details=util.fetch_ssl_details(cloud.paths))
     except Exception:
         util.logexc(log, "Failed to post phone home data to %s in %s tries",
                     url, tries)
diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py
index 76826e05..3bad8e22 100644
--- a/cloudinit/config/schema.py
+++ b/cloudinit/config/schema.py
@@ -4,7 +4,7 @@
 from __future__ import print_function
 
 from cloudinit import importer
-from cloudinit.util import find_modules, read_file_or_url
+from cloudinit.util import find_modules, load_file
 
 import argparse
 from collections import defaultdict
@@ -139,7 +139,7 @@ def validate_cloudconfig_file(config_path, schema, annotate=False):
     """
     if not os.path.exists(config_path):
         raise RuntimeError('Configfile {0} does not exist'.format(config_path))
-    content = read_file_or_url('file://{0}'.format(config_path)).contents
+    content = load_file(config_path, decode=False)
     if not content.startswith(CLOUD_CONFIG_HEADER):
         errors = (
             ('header', 'File {0} needs to begin with "{1}"'.format(
diff --git a/cloudinit/ec2_utils.py b/cloudinit/ec2_utils.py
index dc3f0fc3..3b7b17f1 100644
--- a/cloudinit/ec2_utils.py
+++ b/cloudinit/ec2_utils.py
@@ -150,11 +150,9 @@ def get_instance_userdata(api_version='latest',
         # NOT_FOUND occurs) and just in that case returning an empty string.
         exception_cb = functools.partial(_skip_retry_on_codes,
                                          SKIP_USERDATA_CODES)
-        response = util.read_file_or_url(ud_url,
-                                         ssl_details=ssl_details,
-                                         timeout=timeout,
-                                         retries=retries,
-                                         exception_cb=exception_cb)
+        response = url_helper.read_file_or_url(
+            ud_url, ssl_details=ssl_details, timeout=timeout,
+            retries=retries, exception_cb=exception_cb)
         user_data = response.contents
     except url_helper.UrlError as e:
         if e.code not in SKIP_USERDATA_CODES:
@@ -169,9 +167,9 @@ def _get_instance_metadata(tree, api_version='latest',
                            ssl_details=None, timeout=5, retries=5,
                            leaf_decoder=None):
     md_url = url_helper.combine_url(metadata_address, api_version, tree)
-    caller = functools.partial(util.read_file_or_url,
-                               ssl_details=ssl_details, timeout=timeout,
-                               retries=retries)
+    caller = functools.partial(
+        url_helper.read_file_or_url, ssl_details=ssl_details,
+        timeout=timeout, retries=retries)
 
     def mcaller(url):
         return caller(url).contents
diff --git a/cloudinit/sources/DataSourceMAAS.py b/cloudinit/sources/DataSourceMAAS.py
index aa56addb..bcb38544 100644
--- a/cloudinit/sources/DataSourceMAAS.py
+++ b/cloudinit/sources/DataSourceMAAS.py
@@ -198,7 +198,7 @@ def read_maas_seed_url(seed_url, read_file_or_url=None, timeout=None,
     If version is None, then <version>/ will not be used.
     """
     if read_file_or_url is None:
-        read_file_or_url = util.read_file_or_url
+        read_file_or_url = url_helper.read_file_or_url
 
     if seed_url.endswith("/"):
         seed_url = seed_url[:-1]
diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py
index 90c12df1..e5696b1f 100644
--- a/cloudinit/sources/helpers/azure.py
+++ b/cloudinit/sources/helpers/azure.py
@@ -14,6 +14,7 @@ from cloudinit import temp_utils
 from contextlib import contextmanager
 from xml.etree import ElementTree
 
+from cloudinit import url_helper
 from cloudinit import util
 
 LOG = logging.getLogger(__name__)
@@ -55,14 +56,14 @@ class AzureEndpointHttpClient(object):
         if secure:
             headers = self.headers.copy()
             headers.update(self.extra_secure_headers)
-        return util.read_file_or_url(url, headers=headers)
+        return url_helper.read_file_or_url(url, headers=headers)
 
     def post(self, url, data=None, extra_headers=None):
         headers = self.headers
         if extra_headers is not None:
             headers = self.headers.copy()
             headers.update(extra_headers)
-        return util.read_file_or_url(url, data=data, headers=headers)
+        return url_helper.read_file_or_url(url, data=data, headers=headers)
 
 
 class GoalState(object):
diff --git a/cloudinit/tests/test_url_helper.py b/cloudinit/tests/test_url_helper.py
index b778a3a7..113249d9 100644
--- a/cloudinit/tests/test_url_helper.py
+++ b/cloudinit/tests/test_url_helper.py
@@ -1,7 +1,10 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
-from cloudinit.url_helper import oauth_headers
+from cloudinit.url_helper import oauth_headers, read_file_or_url
 from cloudinit.tests.helpers import CiTestCase, mock, skipIf
+from cloudinit import util
+
+import httpretty
 
 
 try:
@@ -38,3 +41,26 @@ class TestOAuthHeaders(CiTestCase):
             'url', 'consumer_key', 'token_key', 'token_secret',
             'consumer_secret')
         self.assertEqual('url', return_value)
+
+
+class TestReadFileOrUrl(CiTestCase):
+    def test_read_file_or_url_str_from_file(self):
+        """Test that str(result.contents) on file is text version of contents.
+        It should not be "b'data'", but just "'data'" """
+        tmpf = self.tmp_path("myfile1")
+        data = b'This is my file content\n'
+        util.write_file(tmpf, data, omode="wb")
+        result = read_file_or_url("file://%s" % tmpf)
+        self.assertEqual(result.contents, data)
+        self.assertEqual(str(result), data.decode('utf-8'))
+
+    @httpretty.activate
+    def test_read_file_or_url_str_from_url(self):
+        """Test that str(result.contents) on url is text version of contents.
+        It should not be "b'data'", but just "'data'" """
+        url = 'http://hostname/path'
+        data = b'This is my url content\n'
+        httpretty.register_uri(httpretty.GET, url, data)
+        result = read_file_or_url(url)
+        self.assertEqual(result.contents, data)
+        self.assertEqual(str(result), data.decode('utf-8'))
diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py
index 1de07b1c..8067979e 100644
--- a/cloudinit/url_helper.py
+++ b/cloudinit/url_helper.py
@@ -15,6 +15,7 @@ import six
 import time
 
 from email.utils import parsedate
+from errno import ENOENT
 from functools import partial
 from itertools import count
 from requests import exceptions
@@ -80,6 +81,32 @@ def combine_url(base, *add_ons):
     return url
 
 
+def read_file_or_url(url, timeout=5, retries=10,
+                     headers=None, data=None, sec_between=1, ssl_details=None,
+                     headers_cb=None, exception_cb=None):
+    url = url.lstrip()
+    if url.startswith("/"):
+        url = "file://%s" % url
+    if url.lower().startswith("file://"):
+        if data:
+            LOG.warning("Unable to post data to file resource %s", url)
+        file_path = url[len("file://"):]
+        try:
+            with open(file_path, "rb") as fp:
+                contents = fp.read()
+        except IOError as e:
+            code = e.errno
+            if e.errno == ENOENT:
+                code = NOT_FOUND
+            raise UrlError(cause=e, code=code, headers=None, url=url)
+        return FileResponse(file_path, contents=contents)
+    else:
+        return readurl(url, timeout=timeout, retries=retries, headers=headers,
+                       headers_cb=headers_cb, data=data,
+                       sec_between=sec_between, ssl_details=ssl_details,
+                       exception_cb=exception_cb)
+
+
 # Made to have same accessors as UrlResponse so that the
 # read_file_or_url can return this or that object and the
 # 'user' of those objects will not need to know the difference.
@@ -96,7 +123,7 @@ class StringResponse(object):
         return True
 
     def __str__(self):
-        return self.contents
+        return self.contents.decode('utf-8')
 
 
 class FileResponse(StringResponse):
diff --git a/cloudinit/user_data.py b/cloudinit/user_data.py
index cc55daf8..8f6aba1e 100644
--- a/cloudinit/user_data.py
+++ b/cloudinit/user_data.py
@@ -19,7 +19,7 @@ import six
 
 from cloudinit import handlers
 from cloudinit import log as logging
-from cloudinit.url_helper import UrlError
+from cloudinit.url_helper import read_file_or_url, UrlError
 from cloudinit import util
 
 LOG = logging.getLogger(__name__)
@@ -224,8 +224,8 @@ class UserDataProcessor(object):
                 content = util.load_file(include_once_fn)
             else:
                 try:
-                    resp = util.read_file_or_url(include_url,
-                                                 ssl_details=self.ssl_details)
+                    resp = read_file_or_url(include_url,
+                                            ssl_details=self.ssl_details)
                     if include_once_on and resp.ok():
                         util.write_file(include_once_fn, resp.contents,
                                         mode=0o600)
diff --git a/cloudinit/util.py b/cloudinit/util.py
index fc30018c..edfedc7d 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -857,37 +857,6 @@ def fetch_ssl_details(paths=None):
     return ssl_details
 
 
-def read_file_or_url(url, timeout=5, retries=10,
-                     headers=None, data=None, sec_between=1, ssl_details=None,
-                     headers_cb=None, exception_cb=None):
-    url = url.lstrip()
-    if url.startswith("/"):
-        url = "file://%s" % url
-    if url.lower().startswith("file://"):
-        if data:
-            LOG.warning("Unable to post data to file resource %s", url)
-        file_path = url[len("file://"):]
-        try:
-            contents = load_file(file_path, decode=False)
-        except IOError as e:
-            code = e.errno
-            if e.errno == ENOENT:
-                code = url_helper.NOT_FOUND
-            raise url_helper.UrlError(cause=e, code=code, headers=None,
-                                      url=url)
-        return url_helper.FileResponse(file_path, contents=contents)
-    else:
-        return url_helper.readurl(url,
-                                  timeout=timeout,
-                                  retries=retries,
-                                  headers=headers,
-                                  headers_cb=headers_cb,
-                                  data=data,
-                                  sec_between=sec_between,
-                                  ssl_details=ssl_details,
-                                  exception_cb=exception_cb)
-
-
 def load_yaml(blob, default=None, allowed=(dict,)):
     loaded = default
     blob = decode_binary(blob)
@@ -925,12 +894,14 @@ def read_seeded(base="", ext="", timeout=5, retries=10, file_retries=0):
         ud_url = "%s%s%s" % (base, "user-data", ext)
         md_url = "%s%s%s" % (base, "meta-data", ext)
 
-    md_resp = read_file_or_url(md_url, timeout, retries, file_retries)
+    md_resp = url_helper.read_file_or_url(md_url, timeout, retries,
+                                          file_retries)
     md = None
     if md_resp.ok():
         md = load_yaml(decode_binary(md_resp.contents), default={})
 
-    ud_resp = read_file_or_url(ud_url, timeout, retries, file_retries)
+    ud_resp = url_helper.read_file_or_url(ud_url, timeout, retries,
+                                          file_retries)
     ud = None
     if ud_resp.ok():
         ud = ud_resp.contents
diff --git a/tests/unittests/test__init__.py b/tests/unittests/test__init__.py
index f1ab02e9..739bbebf 100644
--- a/tests/unittests/test__init__.py
+++ b/tests/unittests/test__init__.py
@@ -182,7 +182,7 @@ class TestCmdlineUrl(CiTestCase):
         self.assertEqual(
             ('url', 'http://example.com'), main.parse_cmdline_url(cmdline))
 
-    @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
+    @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
     def test_invalid_content(self, m_read):
         key = "cloud-config-url"
         url = 'http://example.com/foo'
@@ -196,7 +196,7 @@ class TestCmdlineUrl(CiTestCase):
         self.assertIn(url, msg)
         self.assertFalse(os.path.exists(fpath))
 
-    @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
+    @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
     def test_valid_content(self, m_read):
         url = "http://example.com/foo"
         payload = b"#cloud-config\nmydata: foo\nbar: wark\n"
@@ -210,7 +210,7 @@ class TestCmdlineUrl(CiTestCase):
         self.assertEqual(logging.INFO, lvl)
         self.assertIn(url, msg)
 
-    @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
+    @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
     def test_no_key_found(self, m_read):
         cmdline = "ro mykey=http://example.com/foo root=foo"
         fpath = self.tmp_path("ccpath")
@@ -221,7 +221,7 @@ class TestCmdlineUrl(CiTestCase):
         self.assertFalse(os.path.exists(fpath))
         self.assertEqual(logging.DEBUG, lvl)
 
-    @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
+    @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
     def test_exception_warns(self, m_read):
         url = "http://example.com/foo"
         cmdline = "ro cloud-config-url=%s root=LABEL=bar" % url
diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py
index b42b073f..af9d3e1a 100644
--- a/tests/unittests/test_datasource/test_azure_helper.py
+++ b/tests/unittests/test_datasource/test_azure_helper.py
@@ -195,7 +195,7 @@ class TestAzureEndpointHttpClient(CiTestCase):
         self.addCleanup(patches.close)
 
         self.read_file_or_url = patches.enter_context(
-            mock.patch.object(azure_helper.util, 'read_file_or_url'))
+            mock.patch.object(azure_helper.url_helper, 'read_file_or_url'))
 
     def test_non_secure_get(self):
         client = azure_helper.AzureEndpointHttpClient(mock.MagicMock())
diff --git a/tests/unittests/test_handler/test_handler_apt_conf_v1.py b/tests/unittests/test_handler/test_handler_apt_conf_v1.py
index 83f962a9..6a4b03ee 100644
--- a/tests/unittests/test_handler/test_handler_apt_conf_v1.py
+++ b/tests/unittests/test_handler/test_handler_apt_conf_v1.py
@@ -12,10 +12,6 @@ import shutil
 import tempfile
 
 
-def load_tfile_or_url(*args, **kwargs):
-    return(util.decode_binary(util.read_file_or_url(*args, **kwargs).contents))
-
-
 class TestAptProxyConfig(TestCase):
     def setUp(self):
         super(TestAptProxyConfig, self).setUp()
@@ -36,7 +32,7 @@ class TestAptProxyConfig(TestCase):
         self.assertTrue(os.path.isfile(self.pfile))
         self.assertFalse(os.path.isfile(self.cfile))
 
-        contents = load_tfile_or_url(self.pfile)
+        contents = util.load_file(self.pfile)
         self.assertTrue(self._search_apt_config(contents, "http", "myproxy"))
 
     def test_apt_http_proxy_written(self):
@@ -46,7 +42,7 @@ class TestAptProxyConfig(TestCase):
         self.assertTrue(os.path.isfile(self.pfile))
         self.assertFalse(os.path.isfile(self.cfile))
 
-        contents = load_tfile_or_url(self.pfile)
+        contents = util.load_file(self.pfile)
         self.assertTrue(self._search_apt_config(contents, "http", "myproxy"))
 
     def test_apt_all_proxy_written(self):
@@ -64,7 +60,7 @@ class TestAptProxyConfig(TestCase):
         self.assertTrue(os.path.isfile(self.pfile))
         self.assertFalse(os.path.isfile(self.cfile))
 
-        contents = load_tfile_or_url(self.pfile)
+        contents = util.load_file(self.pfile)
 
         for ptype, pval in values.items():
             self.assertTrue(self._search_apt_config(contents, ptype, pval))
@@ -80,7 +76,7 @@ class TestAptProxyConfig(TestCase):
         cc_apt_configure.apply_apt_config({'proxy': "foo"},
                                           self.pfile, self.cfile)
         self.assertTrue(os.path.isfile(self.pfile))
-        contents = load_tfile_or_url(self.pfile)
+        contents = util.load_file(self.pfile)
         self.assertTrue(self._search_apt_config(contents, "http", "foo"))
 
     def test_config_written(self):
@@ -92,14 +88,14 @@ class TestAptProxyConfig(TestCase):
         self.assertTrue(os.path.isfile(self.cfile))
         self.assertFalse(os.path.isfile(self.pfile))
 
-        self.assertEqual(load_tfile_or_url(self.cfile), payload)
+        self.assertEqual(util.load_file(self.cfile), payload)
 
     def test_config_replaced(self):
         util.write_file(self.pfile, "content doesnt matter")
         cc_apt_configure.apply_apt_config({'conf': "foo"},
                                           self.pfile, self.cfile)
         self.assertTrue(os.path.isfile(self.cfile))
-        self.assertEqual(load_tfile_or_url(self.cfile), "foo")
+        self.assertEqual(util.load_file(self.cfile), "foo")
 
     def test_config_deleted(self):
         # if no 'conf' is provided, delete any previously written file
diff --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
index d2b96f0b..23bd6e10 100644
--- 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
@@ -64,13 +64,6 @@ deb-src http://archive.ubuntu.com/ubuntu/ fakerelease main restricted
 """)
 
 
-def load_tfile_or_url(*args, **kwargs):
-    """load_tfile_or_url
-    load file and return content after decoding
-    """
-    return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents)
-
-
 class TestAptSourceConfigSourceList(t_help.FilesystemMockingTestCase):
     """TestAptSourceConfigSourceList
     Main Class to test sources list rendering
diff --git a/tests/unittests/test_handler/test_handler_apt_source_v1.py b/tests/unittests/test_handler/test_handler_apt_source_v1.py
index 46ca4ce4..a3132fbd 100644
--- a/tests/unittests/test_handler/test_handler_apt_source_v1.py
+++ b/tests/unittests/test_handler/test_handler_apt_source_v1.py
@@ -39,13 +39,6 @@ S0ORP6HXET3+jC8BMG4tBWCTK/XEZw==
 ADD_APT_REPO_MATCH = r"^[\w-]+:\w"
 
 
-def load_tfile_or_url(*args, **kwargs):
-    """load_tfile_or_url
-    load file and return content after decoding
-    """
-    return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents)
-
-
 class FakeDistro(object):
     """Fake Distro helper object"""
     def update_package_sources(self):
@@ -125,7 +118,7 @@ class TestAptSourceConfig(TestCase):
 
         self.assertTrue(os.path.isfile(filename))
 
-        contents = load_tfile_or_url(filename)
+        contents = util.load_file(filename)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", "http://archive.ubuntu.com/ubuntu",
                                    "karmic-backports",
@@ -157,13 +150,13 @@ class TestAptSourceConfig(TestCase):
         self.apt_src_basic(self.aptlistfile, cfg)
 
         # extra verify on two extra files of this test
-        contents = load_tfile_or_url(self.aptlistfile2)
+        contents = util.load_file(self.aptlistfile2)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", "http://archive.ubuntu.com/ubuntu",
                                    "precise-backports",
                                    "main universe multiverse restricted"),
                                   contents, flags=re.IGNORECASE))
-        contents = load_tfile_or_url(self.aptlistfile3)
+        contents = util.load_file(self.aptlistfile3)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", "http://archive.ubuntu.com/ubuntu",
                                    "lucid-backports",
@@ -220,7 +213,7 @@ class TestAptSourceConfig(TestCase):
 
         self.assertTrue(os.path.isfile(filename))
 
-        contents = load_tfile_or_url(filename)
+        contents = util.load_file(filename)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", params['MIRROR'], params['RELEASE'],
                                    "multiverse"),
@@ -241,12 +234,12 @@ class TestAptSourceConfig(TestCase):
 
         # extra verify on two extra files of this test
         params = self._get_default_params()
-        contents = load_tfile_or_url(self.aptlistfile2)
+        contents = util.load_file(self.aptlistfile2)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", params['MIRROR'], params['RELEASE'],
                                    "main"),
                                   contents, flags=re.IGNORECASE))
-        contents = load_tfile_or_url(self.aptlistfile3)
+        contents = util.load_file(self.aptlistfile3)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", params['MIRROR'], params['RELEASE'],
                                    "universe"),
@@ -296,7 +289,7 @@ class TestAptSourceConfig(TestCase):
 
         self.assertTrue(os.path.isfile(filename))
 
-        contents = load_tfile_or_url(filename)
+        contents = util.load_file(filename)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb",
                                    ('http://ppa.launchpad.net/smoser/'
@@ -336,14 +329,14 @@ class TestAptSourceConfig(TestCase):
                 'filename': self.aptlistfile3}
 
         self.apt_src_keyid(self.aptlistfile, [cfg1, cfg2, cfg3], 3)
-        contents = load_tfile_or_url(self.aptlistfile2)
+        contents = util.load_file(self.aptlistfile2)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb",
                                    ('http://ppa.launchpad.net/smoser/'
                                     'cloud-init-test/ubuntu'),
                                    "xenial", "universe"),
                                   contents, flags=re.IGNORECASE))
-        contents = load_tfile_or_url(self.aptlistfile3)
+        contents = util.load_file(self.aptlistfile3)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb",
                                    ('http://ppa.launchpad.net/smoser/'
@@ -375,7 +368,7 @@ class TestAptSourceConfig(TestCase):
 
         self.assertTrue(os.path.isfile(filename))
 
-        contents = load_tfile_or_url(filename)
+        contents = util.load_file(filename)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb",
                                    ('http://ppa.launchpad.net/smoser/'
diff --git a/tests/unittests/test_handler/test_handler_apt_source_v3.py b/tests/unittests/test_handler/test_handler_apt_source_v3.py
index e486862d..7a64c230 100644
--- a/tests/unittests/test_handler/test_handler_apt_source_v3.py
+++ b/tests/unittests/test_handler/test_handler_apt_source_v3.py
@@ -49,13 +49,6 @@ ADD_APT_REPO_MATCH = r"^[\w-]+:\w"
 TARGET = None
 
 
-def load_tfile(*args, **kwargs):
-    """load_tfile_or_url
-    load file and return content after decoding
-    """
-    return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents)
-
-
 class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
     """TestAptSourceConfig
     Main Class to test apt configs
@@ -119,7 +112,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
 
         self.assertTrue(os.path.isfile(filename))
 
-        contents = load_tfile(filename)
+        contents = util.load_file(filename)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", "http://test.ubuntu.com/ubuntu",
                                    "karmic-backports",
@@ -151,13 +144,13 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
         self._apt_src_basic(self.aptlistfile, cfg)
 
         # extra verify on two extra files of this test
-        contents = load_tfile(self.aptlistfile2)
+        contents = util.load_file(self.aptlistfile2)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", "http://test.ubuntu.com/ubuntu",
                                    "precise-backports",
                                    "main universe multiverse restricted"),
                                   contents, flags=re.IGNORECASE))
-        contents = load_tfile(self.aptlistfile3)
+        contents = util.load_file(self.aptlistfile3)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", "http://test.ubuntu.com/ubuntu",
                                    "lucid-backports",
@@ -174,7 +167,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
 
         self.assertTrue(os.path.isfile(filename))
 
-        contents = load_tfile(filename)
+        contents = util.load_file(filename)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", params['MIRROR'], params['RELEASE'],
                                    "multiverse"),
@@ -201,12 +194,12 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
 
         # extra verify on two extra files of this test
         params = self._get_default_params()
-        contents = load_tfile(self.aptlistfile2)
+        contents = util.load_file(self.aptlistfile2)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", params['MIRROR'], params['RELEASE'],
                                    "main"),
                                   contents, flags=re.IGNORECASE))
-        contents = load_tfile(self.aptlistfile3)
+        contents = util.load_file(self.aptlistfile3)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb", params['MIRROR'], params['RELEASE'],
                                    "universe"),
@@ -240,7 +233,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
 
         self.assertTrue(os.path.isfile(filename))
 
-        contents = load_tfile(filename)
+        contents = util.load_file(filename)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb",
                                    ('http://ppa.launchpad.net/smoser/'
@@ -277,14 +270,14 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
                                    'keyid': "03683F77"}}
 
         self._apt_src_keyid(self.aptlistfile, cfg, 3)
-        contents = load_tfile(self.aptlistfile2)
+        contents = util.load_file(self.aptlistfile2)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb",
                                    ('http://ppa.launchpad.net/smoser/'
                                     'cloud-init-test/ubuntu'),
                                    "xenial", "universe"),
                                   contents, flags=re.IGNORECASE))
-        contents = load_tfile(self.aptlistfile3)
+        contents = util.load_file(self.aptlistfile3)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb",
                                    ('http://ppa.launchpad.net/smoser/'
@@ -310,7 +303,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
 
         self.assertTrue(os.path.isfile(self.aptlistfile))
 
-        contents = load_tfile(self.aptlistfile)
+        contents = util.load_file(self.aptlistfile)
         self.assertTrue(re.search(r"%s %s %s %s\n" %
                                   ("deb",
                                    ('http://ppa.launchpad.net/smoser/'
diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
index 6da4564e..6fe3659d 100644
--- a/tests/unittests/test_handler/test_handler_ntp.py
+++ b/tests/unittests/test_handler/test_handler_ntp.py
@@ -155,9 +155,9 @@ class TestNtp(FilesystemMockingTestCase):
                                              path=confpath,
                                              template_fn=template_fn,
                                              template=None)
-        content = util.read_file_or_url('file://' + confpath).contents
         self.assertEqual(
-            "servers []\npools ['10.0.0.1', '10.0.0.2']\n", content.decode())
+            "servers []\npools ['10.0.0.1', '10.0.0.2']\n",
+            util.load_file(confpath))
 
     def test_write_ntp_config_template_defaults_pools_w_empty_lists(self):
         """write_ntp_config_template defaults pools servers upon empty config.
@@ -176,10 +176,9 @@ class TestNtp(FilesystemMockingTestCase):
                                              path=confpath,
                                              template_fn=template_fn,
                                              template=None)
-        content = util.read_file_or_url('file://' + confpath).contents
         self.assertEqual(
             "servers []\npools {0}\n".format(pools),
-            content.decode())
+            util.load_file(confpath))
 
     def test_defaults_pools_empty_lists_sles(self):
         """write_ntp_config_template defaults opensuse pools upon empty config.
@@ -196,11 +195,11 @@ class TestNtp(FilesystemMockingTestCase):
                                          path=confpath,
                                          template_fn=template_fn,
                                          template=None)
-        content = util.read_file_or_url('file://' + confpath).contents
         for pool in default_pools:
             self.assertIn('opensuse', pool)
         self.assertEqual(
-            "servers []\npools {0}\n".format(default_pools), content.decode())
+            "servers []\npools {0}\n".format(default_pools),
+            util.load_file(confpath))
         self.assertIn(
             "Adding distro default ntp pool servers: {0}".format(
                 ",".join(default_pools)),
@@ -217,10 +216,9 @@ class TestNtp(FilesystemMockingTestCase):
                                          path=confpath,
                                          template_fn=template_fn,
                                          template=None)
-        content = util.read_file_or_url('file://' + confpath).contents
         self.assertEqual(
             "[Time]\nNTP=%s %s \n" % (" ".join(servers), " ".join(pools)),
-            content.decode())
+            util.load_file(confpath))
 
     def test_distro_ntp_client_configs(self):
         """Test we have updated ntp client configs on different distros"""
@@ -267,17 +265,17 @@ class TestNtp(FilesystemMockingTestCase):
                 cc_ntp.write_ntp_config_template(distro, servers=servers,
                                                  pools=pools, path=confpath,
                                                  template_fn=template_fn)
-                content = util.read_file_or_url('file://' + confpath).contents
+                content = util.load_file(confpath)
                 if client in ['ntp', 'chrony']:
                     expected_servers = '\n'.join([
                         'server {0} iburst'.format(srv) for srv in servers])
                     print('distro=%s client=%s' % (distro, client))
-                    self.assertIn(expected_servers, content.decode('utf-8'),
+                    self.assertIn(expected_servers, content,
                                   ('failed to render {0} conf'
                                    ' for distro:{1}'.format(client, distro)))
                     expected_pools = '\n'.join([
                         'pool {0} iburst'.format(pool) for pool in pools])
-                    self.assertIn(expected_pools, content.decode('utf-8'),
+                    self.assertIn(expected_pools, content,
                                   ('failed to render {0} conf'
                                    ' for distro:{1}'.format(client, distro)))
                 elif client == 'systemd-timesyncd':
@@ -286,7 +284,7 @@ class TestNtp(FilesystemMockingTestCase):
                         "# See timesyncd.conf(5) for details.\n\n" +
                         "[Time]\nNTP=%s %s \n" % (" ".join(servers),
                                                   " ".join(pools)))
-                    self.assertEqual(expected_content, content.decode())
+                    self.assertEqual(expected_content, content)
 
     def test_no_ntpcfg_does_nothing(self):
         """When no ntp section is defined handler logs a warning and noops."""
@@ -308,10 +306,10 @@ class TestNtp(FilesystemMockingTestCase):
                 confpath = ntpconfig['confpath']
                 m_select.return_value = ntpconfig
                 cc_ntp.handle('cc_ntp', valid_empty_config, mycloud, None, [])
-                content = util.read_file_or_url('file://' + confpath).contents
                 pools = cc_ntp.generate_server_names(mycloud.distro.name)
                 self.assertEqual(
-                    "servers []\npools {0}\n".format(pools), content.decode())
+                    "servers []\npools {0}\n".format(pools),
+                    util.load_file(confpath))
             self.assertNotIn('Invalid config:', self.logs.getvalue())
 
     @skipUnlessJsonSchema()
@@ -333,9 +331,8 @@ class TestNtp(FilesystemMockingTestCase):
                 "Invalid config:\nntp.pools.0: 123 is not of type 'string'\n"
                 "ntp.servers.1: None is not of type 'string'",
                 self.logs.getvalue())
-            content = util.read_file_or_url('file://' + confpath).contents
             self.assertEqual("servers ['valid', None]\npools [123]\n",
-                             content.decode())
+                             util.load_file(confpath))
 
     @skipUnlessJsonSchema()
     @mock.patch('cloudinit.config.cc_ntp.select_ntp_client')
@@ -357,9 +354,8 @@ class TestNtp(FilesystemMockingTestCase):
                 "Invalid config:\nntp.pools: 123 is not of type 'array'\n"
                 "ntp.servers: 'non-array' is not of type 'array'",
                 self.logs.getvalue())
-            content = util.read_file_or_url('file://' + confpath).contents
             self.assertEqual("servers non-array\npools 123\n",
-                             content.decode())
+                             util.load_file(confpath))
 
     @skipUnlessJsonSchema()
     @mock.patch('cloudinit.config.cc_ntp.select_ntp_client')
@@ -381,10 +377,9 @@ class TestNtp(FilesystemMockingTestCase):
                 "Invalid config:\nntp: Additional properties are not allowed "
                 "('invalidkey' was unexpected)",
                 self.logs.getvalue())
-            content = util.read_file_or_url('file://' + confpath).contents
             self.assertEqual(
                 "servers []\npools ['0.mycompany.pool.ntp.org']\n",
-                content.decode())
+                util.load_file(confpath))
 
     @skipUnlessJsonSchema()
     @mock.patch('cloudinit.config.cc_ntp.select_ntp_client')
@@ -407,10 +402,10 @@ class TestNtp(FilesystemMockingTestCase):
                 " has non-unique elements\nntp.servers: "
                 "['10.0.0.1', '10.0.0.1'] has non-unique elements",
                 self.logs.getvalue())
-            content = util.read_file_or_url('file://' + confpath).contents
             self.assertEqual(
                 "servers ['10.0.0.1', '10.0.0.1']\n"
-                "pools ['0.mypool.org', '0.mypool.org']\n", content.decode())
+                "pools ['0.mypool.org', '0.mypool.org']\n",
+                util.load_file(confpath))
 
     @mock.patch('cloudinit.config.cc_ntp.select_ntp_client')
     def test_ntp_handler_timesyncd(self, m_select):
@@ -426,10 +421,9 @@ class TestNtp(FilesystemMockingTestCase):
             confpath = ntpconfig['confpath']
             m_select.return_value = ntpconfig
             cc_ntp.handle('cc_ntp', cfg, mycloud, None, [])
-            content = util.read_file_or_url('file://' + confpath).contents
             self.assertEqual(
                 "[Time]\nNTP=192.168.2.1 192.168.2.2 0.mypool.org \n",
-                content.decode())
+                util.load_file(confpath))
 
     @mock.patch('cloudinit.config.cc_ntp.select_ntp_client')
     def test_ntp_handler_enabled_false(self, m_select):
@@ -466,10 +460,9 @@ class TestNtp(FilesystemMockingTestCase):
                 m_util.subp.assert_called_with(
                     ['systemctl', 'reload-or-restart',
                      service_name], capture=True)
-            content = util.read_file_or_url('file://' + confpath).contents
             self.assertEqual(
                 "servers []\npools {0}\n".format(pools),
-                content.decode())
+                util.load_file(confpath))
 
     def test_opensuse_picks_chrony(self):
         """Test opensuse picks chrony or ntp on certain distro versions"""
@@ -638,10 +631,9 @@ class TestNtp(FilesystemMockingTestCase):
             mock_path = 'cloudinit.config.cc_ntp.temp_utils._TMPDIR'
             with mock.patch(mock_path, self.new_root):
                 cc_ntp.handle('notimportant', cfg, mycloud, None, None)
-            content = util.read_file_or_url('file://' + confpath).contents
             self.assertEqual(
                 "servers []\npools ['mypool.org']\n%s" % custom,
-                content.decode())
+                util.load_file(confpath))
 
     @mock.patch('cloudinit.config.cc_ntp.supplemental_schema_validation')
     @mock.patch('cloudinit.config.cc_ntp.reload_ntp')
@@ -675,10 +667,9 @@ class TestNtp(FilesystemMockingTestCase):
             with mock.patch(mock_path, self.new_root):
                 cc_ntp.handle('notimportant',
                               {'ntp': cfg}, mycloud, None, None)
-            content = util.read_file_or_url('file://' + confpath).contents
             self.assertEqual(
                 "servers []\npools ['mypool.org']\n%s" % custom,
-                content.decode())
+                util.load_file(confpath))
         m_schema.assert_called_with(expected_merged_cfg)