cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #00863
[Merge] lp:~paelzer/cloud-init/test-apt-source into lp:cloud-init
ChristianEhrhardt has proposed merging lp:~paelzer/cloud-init/test-apt-source into lp:cloud-init.
Requested reviews:
cloud init development team (cloud-init-dev)
For more details, see:
https://code.launchpad.net/~paelzer/cloud-init/test-apt-source/+merge/294521
As discussed, the cloud-init portion of key-only (no source) and alternative source.list template.
I refused to do all the major changes you indicated as "might" in your mail.
Especially since IIRC the thought is to SRU it back.
The impact on backward compatibility could be too big.
Instead I tried to take a safe approach.
For none of the already existing features that touched existed unit tests.
So I wrote these, then added functionality and verified at least nothing on the old tests broke.
Finally I added tests for the new functions and also documentations in the example files.
--
Your team cloud init development team is requested to review the proposed merge of lp:~paelzer/cloud-init/test-apt-source into lp:cloud-init.
=== modified file 'cloudinit/config/cc_apt_configure.py'
--- cloudinit/config/cc_apt_configure.py 2016-03-03 22:20:10 +0000
+++ cloudinit/config/cc_apt_configure.py 2016-05-12 14:50:47 +0000
@@ -21,6 +21,7 @@
import glob
import os
import re
+import tempfile
from cloudinit import templater
from cloudinit import util
@@ -70,7 +71,7 @@
if not util.get_cfg_option_bool(cfg,
'apt_preserve_sources_list', False):
- generate_sources_list(release, mirrors, cloud, log)
+ generate_sources_list(cfg, release, mirrors, cloud, log)
old_mirrors = cfg.get('apt_old_mirrors',
{"primary": "archive.ubuntu.com/ubuntu",
"security": "security.ubuntu.com/ubuntu"})
@@ -149,7 +150,17 @@
return stdout.strip()
-def generate_sources_list(codename, mirrors, cloud, log):
+def generate_sources_list(cfg, codename, mirrors, cloud, log):
+ params = {'codename': codename}
+ for k in mirrors:
+ params[k] = mirrors[k]
+
+ custtmpl = cfg.get('apt_custom_sources_list', None)
+ if custtmpl is not None:
+ templater.render_string_to_file(custtmpl,
+ '/etc/apt/sources.list', params)
+ return
+
template_fn = cloud.get_template_filename('sources.list.%s' %
(cloud.distro.name))
if not template_fn:
@@ -158,12 +169,32 @@
log.warn("No template found, not rendering /etc/apt/sources.list")
return
- params = {'codename': codename}
- for k in mirrors:
- params[k] = mirrors[k]
templater.render_to_file(template_fn, '/etc/apt/sources.list', params)
+def add_key(ent, errorlist):
+ """
+ add key to the system as defiend in entry (if any)
+ suppords raw keys or keyid's
+ The latter will as a first step fetched to get the raw key
+ """
+ if ('keyid' in ent and 'key' not in ent):
+ keyserver = "keyserver.ubuntu.com"
+ if 'keyserver' in ent:
+ keyserver = ent['keyserver']
+ try:
+ ent['key'] = getkeybyid(ent['keyid'], keyserver)
+ except:
+ errorlist.append([ent, "failed to get key from %s" % keyserver])
+ return
+
+ if 'key' in ent:
+ try:
+ util.subp(('apt-key', 'add', '-'), ent['key'])
+ except:
+ errorlist.append([ent, "failed add key"])
+
+
def add_sources(srclist, template_params=None, aa_repo_match=None):
"""
add entries in /etc/apt/sources.list.d for each abbreviated
@@ -179,6 +210,9 @@
errorlist = []
for ent in srclist:
+ # keys can be added without specifying a source
+ add_key(ent, errorlist)
+
if 'source' not in ent:
errorlist.append(["", "missing source"])
continue
@@ -201,22 +235,6 @@
ent['filename'] = os.path.join("/etc/apt/sources.list.d/",
ent['filename'])
- if ('keyid' in ent and 'key' not in ent):
- ks = "keyserver.ubuntu.com"
- if 'keyserver' in ent:
- ks = ent['keyserver']
- try:
- ent['key'] = getkeybyid(ent['keyid'], ks)
- except:
- errorlist.append([source, "failed to get key from %s" % ks])
- continue
-
- if 'key' in ent:
- try:
- util.subp(('apt-key', 'add', '-'), ent['key'])
- except:
- errorlist.append([source, "failed add key"])
-
try:
contents = "%s\n" % (source)
util.write_file(ent['filename'], contents, omode="ab")
=== modified file 'cloudinit/templater.py'
--- cloudinit/templater.py 2015-01-23 02:21:04 +0000
+++ cloudinit/templater.py 2016-05-12 14:50:47 +0000
@@ -142,6 +142,11 @@
util.write_file(outfn, contents, mode=mode)
+def render_string_to_file(content, outfn, params, mode=0o644):
+ contents = render_string(content, params)
+ util.write_file(outfn, contents, mode=mode)
+
+
def render_string(content, params):
if not params:
params = {}
=== modified file 'doc/examples/cloud-config.txt'
--- doc/examples/cloud-config.txt 2015-03-04 17:42:34 +0000
+++ doc/examples/cloud-config.txt 2016-05-12 14:50:47 +0000
@@ -72,6 +72,36 @@
# then apt_mirror above will have no effect
apt_preserve_sources_list: true
+# Provide a custom template for rednering sources.list
+# Default: a default template for Ubuntu/Debain will be used as packaged in
+# Ubuntu: /etc/cloud/templates/sources.list.ubuntu.tmpl
+# Debian: /etc/cloud/templates/sources.list.debian.tmpl
+# Others: n/a
+# This will follow the normal mirror/codename replacement rules before
+# being written to disk.
+apt_custom_sources_list: |
+ ## template:jinja
+ ## Note, this file is written by cloud-init on first boot of an instance
+ ## modifications made here will not survive a re-bundle.
+ ## if you wish to make changes you can:
+ ## a.) add 'apt_preserve_sources_list: true' to /etc/cloud/cloud.cfg
+ ## or do the same in user-data
+ ## b.) add sources in /etc/apt/sources.list.d
+ ## c.) make changes to template file /etc/cloud/templates/sources.list.tmpl
+ deb {{mirror}} {{codename}} main restricted
+ deb-src {{mirror}} {{codename}} main restricted
+
+ # could drop some of the usually used entries
+
+ # could refer to other mirrors
+ deb http://ddebs.ubuntu.com {{codename}} main restricted universe multiverse
+ deb http://ddebs.ubuntu.com {{codename}}-updates main restricted universe multiverse
+ deb http://ddebs.ubuntu.com {{codename}}-proposed main restricted universe multiverse
+
+ # or even more uncommon examples like local or NFS mounted repos,
+ # eventually whatever is compatible with sources.list syntax
+ deb file:/home/apt/debian unstable main contrib non-free
+
# 'source' entries in apt-sources that match this python regex
# expression will be passed to add-apt-repository
add_apt_repo_match: '^[\w-]+:\w'
@@ -111,12 +141,14 @@
keyid: F430BBA5 # GPG key ID published on a key server
filename: byobu-ppa.list
+ # this would only import the key without adding a ppa or other source spec
+ - keyid: F430BBA5 # GPG key ID published on a key server
+
# Custom apt repository:
# * The apt signing key can also be specified
# by providing a pgp public key block
# * Providing the PBG key here is the most robust method for
# specifying a key, as it removes dependency on a remote key server
-
- source: deb http://ppa.launchpad.net/alestic/ppa/ubuntu karmic main
key: | # The value needs to start with -----BEGIN PGP PUBLIC KEY BLOCK-----
-----BEGIN PGP PUBLIC KEY BLOCK-----
@@ -132,6 +164,24 @@
=Y2oI
-----END PGP PUBLIC KEY BLOCK-----
+ # Custom gpg key:
+ # * As the keyid also a key can be specified withut a related source
+ # * all other facts mentioned above still apply
+ - key: | # The value needs to start with -----BEGIN PGP PUBLIC KEY BLOCK-----
+ -----BEGIN PGP PUBLIC KEY BLOCK-----
+ Version: SKS 1.0.10
+
+ mI0ESpA3UQEEALdZKVIMq0j6qWAXAyxSlF63SvPVIgxHPb9Nk0DZUixn+akqytxG4zKCONz6
+ qLjoBBfHnynyVLfT4ihg9an1PqxRnTO+JKQxl8NgKGz6Pon569GtAOdWNKw15XKinJTDLjnj
+ 9y96ljJqRcpV9t/WsIcdJPcKFR5voHTEoABE2aEXABEBAAG0GUxhdW5jaHBhZCBQUEEgZm9y
+ IEFsZXN0aWOItgQTAQIAIAUCSpA3UQIbAwYLCQgHAwIEFQIIAwQWAgMBAh4BAheAAAoJEA7H
+ 5Qi+CcVxWZ8D/1MyYvfj3FJPZUm2Yo1zZsQ657vHI9+pPouqflWOayRR9jbiyUFIn0VdQBrP
+ t0FwvnOFArUovUWoKAEdqR8hPy3M3APUZjl5K4cMZR/xaMQeQRZ5CHpS4DBKURKAHC0ltS5o
+ uBJKQOZm5iltJp15cgyIkBkGe8Mx18VFyVglAZey
+ =Y2oI
+ -----END PGP PUBLIC KEY BLOCK-----
+
+
## apt config via system_info:
# under the 'system_info', you can further customize cloud-init's interaction
# with apt.
=== added file 'tests/unittests/test_handler/test_handler_apt_configure_sources_list.py'
--- tests/unittests/test_handler/test_handler_apt_configure_sources_list.py 1970-01-01 00:00:00 +0000
+++ tests/unittests/test_handler/test_handler_apt_configure_sources_list.py 2016-05-12 14:50:47 +0000
@@ -0,0 +1,163 @@
+""" test_handler_apt_configure_sources_list
+Test templating of sources list
+"""
+import os
+import shutil
+import tempfile
+import re
+
+import logging
+
+try:
+ from unittest import mock
+except ImportError:
+ import mock
+
+from cloudinit import cloud
+from cloudinit import distros
+from cloudinit import util
+from cloudinit import helpers
+from cloudinit import templater
+
+from cloudinit.sources import DataSourceNone
+from cloudinit.config import cc_apt_configure
+
+from .. import helpers as t_help
+
+LOG = logging.getLogger(__name__)
+
+YAML_TEXT_CUSTOM_SL = """
+apt_mirror: http://archive.ubuntu.com/ubuntu/
+apt_custom_sources_list: |
+ ## template:jinja
+ ## Note, this file is written by cloud-init on first boot of an instance
+ ## modifications made here will not survive a re-bundle.
+ ## if you wish to make changes you can:
+ ## a.) add 'apt_preserve_sources_list: true' to /etc/cloud/cloud.cfg
+ ## or do the same in user-data
+ ## b.) add sources in /etc/apt/sources.list.d
+ ## c.) make changes to template file /etc/cloud/templates/sources.list.tmpl
+
+ # See http://help.ubuntu.com/community/UpgradeNotes for how to upgrade to
+ # newer versions of the distribution.
+ deb {{mirror}} {{codename}} main restricted
+ deb-src {{mirror}} {{codename}} main restricted
+ # FIND_SOMETHING_SPECIAL
+"""
+
+
+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
+ """
+ def setUp(self):
+ super(TestAptSourceConfigSourceList, self).setUp()
+ self.subp = util.subp
+ self.new_root = tempfile.mkdtemp()
+ self.addCleanup(shutil.rmtree, self.new_root)
+
+ def _get_cloud(self, distro, metadata=None):
+ self.patchUtils(self.new_root)
+ paths = helpers.Paths({})
+ cls = distros.fetch(distro)
+ mydist = cls(distro, {}, paths)
+ myds = DataSourceNone.DataSourceNone({}, mydist, paths)
+ if metadata:
+ myds.metadata.update(metadata)
+ return cloud.Cloud(myds, paths, {}, mydist, None)
+
+ def apt_source_list(self, distro, mirror, mirrorcheck=None):
+ """ apt_source_list
+ Test rendering of a source.list from template for a given distro
+ """
+ if mirrorcheck is None:
+ mirrorcheck = mirror
+
+ if isinstance(mirror, list):
+ cfg = {'apt_mirror_search': mirror}
+ else:
+ cfg = {'apt_mirror': mirror}
+ mycloud = self._get_cloud(distro)
+
+ with mock.patch.object(templater, 'render_to_file') as mocktmpl:
+ with mock.patch.object(os.path, 'isfile',
+ return_value=True) as mockisfile:
+ cc_apt_configure.handle("notimportant", cfg, mycloud,
+ LOG, None)
+
+ mockisfile.assert_any_call(('/etc/cloud/templates/'
+ 'sources.list.%s.tmpl' % distro))
+ mocktmpl.assert_called_once_with(('/etc/cloud/templates/'
+ 'sources.list.%s.tmpl' % distro),
+ '/etc/apt/sources.list',
+ {'codename': '',
+ 'primary':
+ mirrorcheck,
+ 'mirror':
+ mirrorcheck})
+
+ def test_apt_source_list_debian(self):
+ """ test_apt_source_list_debian
+ Test rendering of a source.list from template for debian
+ """
+ self.apt_source_list('debian', 'http://httpredir.debian.org/debian')
+
+ def test_apt_source_list_ubuntu(self):
+ """ test_apt_source_list_ubuntu
+ Test rendering of a source.list from template for ubuntu
+ """
+ self.apt_source_list('ubuntu', 'http://archive.ubuntu.com/ubuntu/')
+
+ def test_apt_srcl_debian_mirrorfail(self):
+ """ test_apt_source_list_debian_mirrorfail
+ Test rendering of a source.list from template for debian
+ """
+ self.apt_source_list('debian', ['http://does.not.exist',
+ 'http://httpredir.debian.org/debian'],
+ 'http://httpredir.debian.org/debian')
+
+ def test_apt_srcl_ubuntu_mirrorfail(self):
+ """ test_apt_source_list_ubuntu_mirrorfail
+ Test rendering of a source.list from template for ubuntu
+ """
+ self.apt_source_list('ubuntu', ['http://does.not.exist',
+ 'http://archive.ubuntu.com/ubuntu/'],
+ 'http://archive.ubuntu.com/ubuntu/')
+
+ def test_apt_srcl_custom(self):
+ """ test_apt_srcl_custom
+ Test rendering from a custom source.list template
+ """
+ cfg = util.load_yaml(YAML_TEXT_CUSTOM_SL)
+ mycloud = self._get_cloud('ubuntu')
+
+ # the second mock restores the original subp
+ with mock.patch.object(util, 'write_file') as mockwrite:
+ with mock.patch.object(util, 'subp', self.subp) as mocksubp:
+ cc_apt_configure.handle("notimportant", cfg, mycloud,
+ LOG, None)
+
+ mockwrite.assert_called_once_with(
+ '/etc/apt/sources.list',
+ ("## Note, this file is written by cloud-init on first boot of an"
+ " instance\n## modifications made here will not survive a re-bun"
+ "dle.\n## if you wish to make changes you can:\n## a.) add 'apt_"
+ "preserve_sources_list: true' to /etc/cloud/cloud.cfg\n## or"
+ " do the same in user-data\n## b.) add sources in /etc/apt/sourc"
+ "es.list.d\n## c.) make changes to template file /etc/cloud/temp"
+ "lates/sources.list.tmpl\n\n# See http://help.ubuntu.com/communi";
+ "ty/UpgradeNotes for how to upgrade to\n# newer versions of the "
+ "distribution.\ndeb http://archive.ubuntu.com/ubuntu/ xenial mai"
+ "n restricted\ndeb-src http://archive.ubuntu.com/ubuntu/ xenial "
+ "main restricted\n# FIND_SOMETHING_SPECIAL\n"),
+ mode=420)
+
+
+# vi: ts=4 expandtab
=== added file 'tests/unittests/test_handler/test_handler_apt_source.py'
--- tests/unittests/test_handler/test_handler_apt_source.py 1970-01-01 00:00:00 +0000
+++ tests/unittests/test_handler/test_handler_apt_source.py 2016-05-12 14:50:47 +0000
@@ -0,0 +1,196 @@
+""" test_handler_apt_source
+Testing various config variations of the apt_source config
+"""
+import os
+import shutil
+import tempfile
+import re
+
+try:
+ from unittest import mock
+except ImportError:
+ import mock
+
+from cloudinit import distros
+from cloudinit import util
+from cloudinit.config import cc_apt_configure
+
+from ..helpers import TestCase
+
+
+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 TestAptSourceConfig(TestCase):
+ """ TestAptSourceConfig
+ Main Class to test apt_source configs
+ """
+ def setUp(self):
+ super(TestAptSourceConfig, self).setUp()
+ self.tmp = tempfile.mkdtemp()
+ self.addCleanup(shutil.rmtree, self.tmp)
+ self.aptlistfile = os.path.join(self.tmp, "single-deb.list")
+
+ @staticmethod
+ def _get_default_params():
+ """ get_default_params
+ Get the most basic default mrror and release info to be used in tests
+ """
+ params = {}
+ params['RELEASE'] = cc_apt_configure.get_release()
+ params['MIRROR'] = "http://archive.ubuntu.com/ubuntu";
+ return params
+
+ def test_apt_source_basic(self):
+ """ test_apt_source_basic
+ Test Fix deb source string, has to overwrite mirror conf in params
+ """
+ params = self._get_default_params()
+ cfg = {'source': ('deb http://archive.ubuntu.com/ubuntu'
+ ' karmic-backports'
+ ' main universe multiverse restricted'),
+ 'filename': self.aptlistfile}
+
+ cc_apt_configure.add_sources([cfg], params)
+
+ self.assertTrue(os.path.isfile(self.aptlistfile))
+
+ contents = load_tfile_or_url(self.aptlistfile)
+ self.assertTrue(re.search(r"%s %s %s %s\n" %
+ ("deb", "http://archive.ubuntu.com/ubuntu";,
+ "karmic-backports",
+ "main universe multiverse restricted"),
+ contents, flags=re.IGNORECASE))
+
+ def test_apt_source_replacement(self):
+ """ test_apt_source_replace
+ Test Autoreplacement of MIRROR and RELEASE in source specs
+ """
+ params = self._get_default_params()
+ cfg = {'source': 'deb $MIRROR $RELEASE multiverse',
+ 'filename': self.aptlistfile}
+
+ cc_apt_configure.add_sources([cfg], params)
+
+ self.assertTrue(os.path.isfile(self.aptlistfile))
+
+ contents = load_tfile_or_url(self.aptlistfile)
+ self.assertTrue(re.search(r"%s %s %s %s\n" %
+ ("deb", params['MIRROR'], params['RELEASE'],
+ "multiverse"),
+ contents, flags=re.IGNORECASE))
+
+ def test_apt_source_keyid(self):
+ """ test_apt_source_keyid
+ Test specification of a source + keyid
+ """
+ params = self._get_default_params()
+ cfg = {'source': ('deb '
+ 'http://ppa.launchpad.net/'
+ 'smoser/cloud-init-test/ubuntu'
+ ' xenial main'),
+ 'keyid': "03683F77",
+ 'filename': self.aptlistfile}
+
+ with mock.patch.object(util, 'subp',
+ return_value=('fakekey 1234', '')) as mockobj:
+ cc_apt_configure.add_sources([cfg], params)
+
+ mockobj.assert_called_with(('apt-key', 'add', '-'), 'fakekey 1234')
+
+ self.assertTrue(os.path.isfile(self.aptlistfile))
+
+ contents = load_tfile_or_url(self.aptlistfile)
+ self.assertTrue(re.search(r"%s %s %s %s\n" %
+ ("deb",
+ ('http://ppa.launchpad.net/smoser/'
+ 'cloud-init-test/ubuntu'),
+ "xenial", "main"),
+ contents, flags=re.IGNORECASE))
+
+ def test_apt_source_key(self):
+ """ test_apt_source_key
+ Test specification of a source + key
+ """
+ params = self._get_default_params()
+ cfg = {'source': ('deb '
+ 'http://ppa.launchpad.net/'
+ 'smoser/cloud-init-test/ubuntu'
+ ' xenial main'),
+ 'key': "fakekey 4321",
+ 'filename': self.aptlistfile}
+
+ with mock.patch.object(util, 'subp') as mockobj:
+ cc_apt_configure.add_sources([cfg], params)
+
+ mockobj.assert_called_with(('apt-key', 'add', '-'), 'fakekey 4321')
+
+ self.assertTrue(os.path.isfile(self.aptlistfile))
+
+ contents = load_tfile_or_url(self.aptlistfile)
+ self.assertTrue(re.search(r"%s %s %s %s\n" %
+ ("deb",
+ ('http://ppa.launchpad.net/smoser/'
+ 'cloud-init-test/ubuntu'),
+ "xenial", "main"),
+ contents, flags=re.IGNORECASE))
+
+ def test_apt_source_keyonly(self):
+ """ test_apt_source_keyonly
+ Test specification key without source (not yet supported)
+ """
+ params = self._get_default_params()
+ cfg = {'key': "fakekey 4242",
+ 'filename': self.aptlistfile}
+
+ with mock.patch.object(util, 'subp') as mockobj:
+ cc_apt_configure.add_sources([cfg], params)
+
+ mockobj.assert_called_once_with(('apt-key', 'add', '-'),
+ 'fakekey 4242')
+
+ # filename should be ignored on key only
+ self.assertFalse(os.path.isfile(self.aptlistfile))
+
+ def test_apt_source_keyidonly(self):
+ """ test_apt_source_keyidonly
+ Test specification of a keyid without source (not yet supported)
+ """
+ params = self._get_default_params()
+ cfg = {'keyid': "03683F77",
+ 'filename': self.aptlistfile}
+
+ with mock.patch.object(util, 'subp',
+ return_value=('fakekey 1212', '')) as mockobj:
+ cc_apt_configure.add_sources([cfg], params)
+
+ mockobj.assert_called_with(('apt-key', 'add', '-'), 'fakekey 1212')
+
+ # filename should be ignored on key only
+ self.assertFalse(os.path.isfile(self.aptlistfile))
+
+ def test_apt_source_ppa(self):
+ """ test_apt_source_ppa
+ Test specification of a ppa
+ """
+ params = self._get_default_params()
+ cfg = {'source': 'ppa:smoser/cloud-init-test',
+ 'filename': self.aptlistfile}
+
+ # default matcher needed for ppa
+ matcher = re.compile(r'^[\w-]+:\w').search
+
+ with mock.patch.object(util, 'subp') as mockobj:
+ cc_apt_configure.add_sources([cfg], params, aa_repo_match=matcher)
+ mockobj.assert_called_once_with(['add-apt-repository',
+ 'ppa:smoser/cloud-init-test'])
+
+ # adding ppa should ignore filename (uses add-apt-repository)
+ self.assertFalse(os.path.isfile(self.aptlistfile))
+
+
+# vi: ts=4 expandtab
Follow ups
