Merge lp:~harlowja/cloud-init/write-files-fetch-from-somewhere into lp:~cloud-init-dev/cloud-init/trunk

[Joshua Harlow]

[lp:~harlowja/cloud-init/write-files-fetch-from-somewhere]

1091. By Joshua Harlow on 2015-03-31

Fix reading of files for py3.x

util.read_file_or_url needs to have a __str__ that
returns the right type for py3.x; so make sure that if
binary data is read that it is decoded.

1092. By Joshua Harlow on 2015-03-31

Only do the decoding on py3.x

1093. By Joshua Harlow on 2015-03-31

Some contents/text tweaks and handler tests

[Scott Moser (smoser)]

I think better to require user to be explicit that this is a remote file.
I don't like the interpretation of content. The problem being, if it seems quite likely that some config file that someone would want to 'write-files' of might start with 'http://'.
If they try to send such a content, then cloud-init would interpret that content and pull the url.

So, in short, I'd rather:
write_files:
 - path: /myfile-from-remote
   content_url: http://www.yahoo.com
 - path: /odd-use-of-cloud-init-for-copy
   content_url: file:///usr/bin/true
 - path: /myfile-old-write_files
   content: "Hi Mom"

Than:
write_files:
 - path: /myfile-from-remote
   content: http://www.yahoo.com

in the event of an entry having both 'content_url' and 'content', I guess I'd warn and use the 'content' field.

Agree/ disagree?

review: Needs Fixing

[Joshua Harlow (harlowja)]

Thats fine with me; I was debating whether to try to interpret content or not and am fine either way; will adjust that soon.

[lp:~harlowja/cloud-init/write-files-fetch-from-somewhere]

1094. By Joshua Harlow on 2015-04-07

Avoid fetching automagically and add test for this

1095. By Joshua Harlow on 2015-04-07

Fixs log entry/output

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:1095
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~harlowja/cloud-init/write-files-fetch-from-somewhere/+merge/254816/+edit-commit-message

https://server-team-jenkins.canonical.com/job/cloud-init-ci/47/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/lp-vote-on-merge/21/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/cloud-init-ci/47/rebuild

review: Needs Fixing (continuous-integration)

[Scott Moser (smoser)]

Hi,

Josh, Please re-submit this. I like the functionality. Please re-submit in git.

Thank you for taking the time to contribute to cloud-init. Cloud-init has moved its revision control system to git. As a result, we are marking all bzr merge proposals as 'rejected'. If you would like to re-submit this proposal for review, please do so by following the current HACKING documentation at http://cloudinit.readthedocs.io/en/latest/topics/hacking.html .

I'm going to mark this as 'merged', because I think it is actually
upstream at
https://git.launchpad.net/cloud-init/commit/?id=79236a629f1

Unmerged revisions

1095. By Joshua Harlow on 2015-04-07

Fixs log entry/output

1094. By Joshua Harlow on 2015-04-07

Avoid fetching automagically and add test for this

1093. By Joshua Harlow on 2015-03-31

Some contents/text tweaks and handler tests

1092. By Joshua Harlow on 2015-03-31

Only do the decoding on py3.x

1091. By Joshua Harlow on 2015-03-31

Fix reading of files for py3.x

util.read_file_or_url needs to have a __str__ that
returns the right type for py3.x; so make sure that if
binary data is read that it is decoded.

1090. By Joshua Harlow on 2015-03-31

Fix extra space

1089. By Joshua Harlow on 2015-03-31

Allow write_files to fetch content

Instead of requring write_files to be always provided
all the data (which can be big/large blobs); allow it to
fetch that data from either a local file or a http/https
url instead (saving precious user-data space).

=== modified file 'cloudinit/config/cc_write_files.py'
--- cloudinit/config/cc_write_files.py	2015-01-26 17:41:04 +0000
+++ cloudinit/config/cc_write_files.py	2015-04-07 22:54:32 +0000
@@ -25,6 +25,13 @@
 
 frequency = PER_INSTANCE
 
+# Content with these prefix(s) will be read/fetched externally to grab its
+# contents (which will replace any provided contents...)
+READ_EXTERNAL_PREFIXES = [
+    "http://",
+    'file://',
+    "https://",
+]
 DEFAULT_OWNER = "root:root"
 DEFAULT_PERMS = 0o644
 UNKNOWN_ENC = 'text/plain'
@@ -62,7 +69,6 @@
 def write_files(name, files, log):
     if not files:
         return
-
     for (i, f_info) in enumerate(files):
         path = f_info.get('path')
         if not path:
@@ -70,8 +76,26 @@
                      i + 1, name)
             continue
         path = os.path.abspath(path)
+        pre_content = f_info.get('content', '')
+        fetch_external = util.is_true(f_info.get('fetch_external', False))
+        if fetch_external:
+            # Verify prefixes that we will try to fetch...
+            prefix_match = False
+            for p in READ_EXTERNAL_PREFIXES:
+                if pre_content.startswith(p):
+                    prefix_match = True
+                    break
+            if not prefix_match:
+                log.warn("Unable to read from external content '%s' (for"
+                         " entry %s) that does not start with one"
+                         " of %s prefixes", pre_content, i + 1,
+                         READ_EXTERNAL_PREFIXES)
+                continue
+            else:
+                pre_content = util.read_file_or_url(pre_content)
+                pre_content = pre_content.contents
         extractions = canonicalize_extraction(f_info.get('encoding'), log)
-        contents = extract_contents(f_info.get('content', ''), extractions)
+        contents = extract_contents(pre_content, extractions)
         (u, g) = util.extract_usergroup(f_info.get('owner', DEFAULT_OWNER))
         perms = decode_perms(f_info.get('permissions'), DEFAULT_PERMS, log)
         util.write_file(path, contents, mode=perms)
@@ -92,7 +116,7 @@
 
 
 def extract_contents(contents, extraction_types):
-    result = str(contents)
+    result = contents
     for t in extraction_types:
         if t == 'application/x-gzip':
             result = util.decomp_gzip(result, quiet=False)

=== modified file 'cloudinit/url_helper.py'
--- cloudinit/url_helper.py	2015-03-02 20:48:42 +0000
+++ cloudinit/url_helper.py	2015-04-07 22:54:32 +0000
@@ -92,11 +92,18 @@
 # read_file_or_url can return this or that object and the
 # 'user' of those objects will not need to know the difference.
 class StringResponse(object):
-    def __init__(self, contents, code=200):
+    def __init__(self, contents, code=200, encoding='utf-8'):
         self.code = code
         self.headers = {}
-        self.contents = contents
         self.url = None
+        self.encoding = encoding
+        # Get the contents and text in the right encoding(s)...
+        if not isinstance(contents, six.text_type):
+            self.contents = contents
+            self.text = contents.decode(encoding)
+        else:
+            self.text = contents
+            self.contents = contents.encode(encoding)
 
     def ok(self, *args, **kwargs):
         if self.code != 200:
@@ -104,12 +111,14 @@
         return True
 
     def __str__(self):
-        return self.contents
+        """The text/unicode contents."""
+        return self.text
 
 
 class FileResponse(StringResponse):
-    def __init__(self, path, contents, code=200):
-        StringResponse.__init__(self, contents, code=code)
+    def __init__(self, path, contents, code=200, encoding='utf-8'):
+        StringResponse.__init__(self, contents,
+                                code=code, encoding=encoding)
         self.url = path
 
 
@@ -119,8 +128,13 @@
 
     @property
     def contents(self):
+        """The binary contents."""
         return self._response.content
 
+    def text(self):
+        """The text/unicode contents."""
+        return self._response.text
+
     @property
     def url(self):
         return self._response.url
@@ -143,7 +157,7 @@
         return self._response.status_code
 
     def __str__(self):
-        return self._response.text
+        return self.text
 
 
 class UrlError(IOError):

=== added file 'tests/unittests/test_handler/test_handler_write_files.py'
--- tests/unittests/test_handler/test_handler_write_files.py	1970-01-01 00:00:00 +0000
+++ tests/unittests/test_handler/test_handler_write_files.py	2015-04-07 22:54:32 +0000
@@ -0,0 +1,85 @@
+from cloudinit import util
+
+from cloudinit.config import cc_write_files
+
+from .. import helpers
+
+import grp
+import logging
+import os
+import pwd
+import shutil
+import tempfile
+
+LOG = logging.getLogger(__name__)
+
+hp = helpers.import_httpretty()
+
+
+def get_owner():
+    user = pwd.getpwuid(os.getuid())[0]
+    gid = pwd.getpwnam(user).pw_gid
+    group = grp.getgrgid(gid).gr_name
+    return "%s:%s" % (user, group)
+
+
+class TestWriteFiles(helpers.FilesystemMockingTestCase):
+    def setUp(self):
+        super(TestWriteFiles, self).setUp()
+        self.tmp = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.tmp)
+
+    def test_local_write(self):
+        path = os.path.join(self.tmp, 'stuff.txt')
+        files = [
+            {
+                'path': path,
+                'content': 'blahblah',
+                'permissions': '0755',
+                'owner': get_owner(),
+            },
+        ]
+        cfg = {
+            'write_files': files,
+        }
+        cc_write_files.handle('write_files', cfg, None, LOG, [])
+        blob = util.load_file(path)
+        self.assertEqual("blahblah", blob)
+
+    @hp.activate
+    def test_remote_read(self):
+        hp.register_uri(hp.GET, 'http://www.yahoo.com',
+                        status=200, body='yahooooooo!')
+        path = os.path.join(self.tmp, 'stuff.txt')
+        files = [
+            {
+                'path': path,
+                'content': 'http://www.yahoo.com',
+                'permissions': '0755',
+                'owner': get_owner(),
+                'fetch_external': 1,
+            },
+        ]
+        cfg = {
+            'write_files': files,
+        }
+        cc_write_files.handle('write_files', cfg, None, LOG, [])
+        blob = util.load_file(path)
+        self.assertEqual("yahooooooo!", blob)
+
+    def test_skipped_remote_read(self):
+        path = os.path.join(self.tmp, 'stuff.txt')
+        files = [
+            {
+                'path': path,
+                'content': 'blah://www.yahoo.com',
+                'permissions': '0755',
+                'owner': get_owner(),
+                'fetch_external': 1,
+            },
+        ]
+        cfg = {
+            'write_files': files,
+        }
+        cc_write_files.handle('write_files', cfg, None, LOG, [])
+        self.assertFalse(os.path.isfile(path))