From 3f38059d1397ed8e16000d2246f1b1f01929bf5e Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 08:56:55 +0200 Subject: unify testing of real gpg key adds --- .../test_handler/test_handler_apt_source.py | 43 +++++++++------------- 1 file changed, 17 insertions(+), 26 deletions(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index fa28c916..fc767ece 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -441,45 +441,36 @@ class TestAptSourceConfig(TestCase): # filename should be ignored on key only self.assertFalse(os.path.isfile(self.aptlistfile)) - def test_apt_src_keyid_real(self): - """test_apt_src_keyid_real - Test specification of a keyid without source incl - up to addition of the key (add_key_raw, getkeybyid mocked) + def apt_src_keyid_real(self, cfg, expectedkey): + """apt_src_keyid_real + Test specification of a keyid without source including + up to addition of the key (nothing but add_key_raw mocked to keep the + environment as is) """ - keyid = "03683F77" params = self._get_default_params() - cfg = {'keyid': keyid, - 'filename': self.aptlistfile} - with mock.patch.object(cc_apt_configure, 'add_key_raw') as mockobj: - with mock.patch.object(cc_apt_configure, 'getkeybyid') as gkbi: - gkbi.return_value = EXPECTEDKEY - cc_apt_configure.add_sources([cfg], params) + cc_apt_configure.add_sources([cfg], params) - mockobj.assert_called_with(EXPECTEDKEY) + mockobj.assert_called_with(expectedkey) # filename should be ignored on key only self.assertFalse(os.path.isfile(self.aptlistfile)) - def test_apt_src_longkeyid_real(self): - """test_apt_src_longkeyid_real - Test specification of a long key fingerprint without source incl - up to addition of the key (nothing but add_key_raw mocked) - """ - keyid = "B59D 5F15 97A5 04B7 E230 6DCA 0620 BBCF 0368 3F77" - params = self._get_default_params() + def test_apt_src_keyid_real(self): + """test_apt_src_keyid_real - Test keyid including key add""" + keyid = "03683F77" cfg = {'keyid': keyid, 'filename': self.aptlistfile} - with mock.patch.object(cc_apt_configure, 'add_key_raw') as mockobj: - with mock.patch.object(cc_apt_configure, 'getkeybyid') as gkbi: - gkbi.return_value = EXPECTEDKEY - cc_apt_configure.add_sources([cfg], params) + self.apt_src_keyid_real(cfg, EXPECTEDKEY) - mockobj.assert_called_with(EXPECTEDKEY) + def test_apt_src_longkeyid_real(self): + """test_apt_src_longkeyid_real - Test long keyid including key add""" + keyid = "B59D 5F15 97A5 04B7 E230 6DCA 0620 BBCF 0368 3F77" + cfg = {'keyid': keyid, + 'filename': self.aptlistfile} - # filename should be ignored on key only - self.assertFalse(os.path.isfile(self.aptlistfile)) + self.apt_src_keyid_real(cfg, EXPECTEDKEY) def test_apt_src_ppa(self): """test_apt_src_ppa -- cgit v1.2.3 From 1b32f10f431aa9138346f20e421442bf935399b5 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 08:57:48 +0200 Subject: make _get_default_params method static --- tests/unittests/test_handler/test_handler_apt_source.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index fc767ece..4d8ae46a 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -62,7 +62,8 @@ class TestAptSourceConfig(TestCase): get_rel.return_value = self.release self.addCleanup(patcher.stop) - def _get_default_params(self): + @staticmethod + def _get_default_params(): """get_default_params Get the most basic default mrror and release info to be used in tests """ -- cgit v1.2.3 From eee526be37d47bae4196b88c3b2ecd4fdf0bbc8c Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 09:04:29 +0200 Subject: test alternate keyserver --- tests/unittests/test_handler/test_handler_apt_source.py | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index 4d8ae46a..a4878124 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -473,6 +473,15 @@ class TestAptSourceConfig(TestCase): self.apt_src_keyid_real(cfg, EXPECTEDKEY) + def test_apt_src_longkeyid_ks_real(self): + """test_apt_src_longkeyid_ks_real - Test long keyid from other ks""" + keyid = "B59D 5F15 97A5 04B7 E230 6DCA 0620 BBCF 0368 3F77" + cfg = {'keyid': keyid, + 'keyserver': 'keys.gnupg.net', + 'filename': self.aptlistfile} + + self.apt_src_keyid_real(cfg, EXPECTEDKEY) + def test_apt_src_ppa(self): """test_apt_src_ppa Test specification of a ppa -- cgit v1.2.3 From 89688593e9358433cd9383d1bdfae12dbcd58f72 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 13:36:23 +0200 Subject: add_key_raw - make exceptions more specific --- cloudinit/config/cc_apt_configure.py | 4 +-- cloudinit/util.py | 1 + .../test_handler/test_handler_apt_source.py | 38 +++++++++++++++++++--- 3 files changed, 36 insertions(+), 7 deletions(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index 2f270662..a9ac6ea8 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -154,7 +154,7 @@ def add_key_raw(key): try: util.subp(('apt-key', 'add', '-'), key) except util.ProcessExecutionError: - raise Exception('failed add key') + raise ValueError('failed to add apt GPG Key to apt keyring') def add_key(ent): @@ -221,7 +221,7 @@ def add_sources(srclist, template_params=None, aa_repo_match=None): # keys can be added without specifying a source try: add_key(ent) - except Exception as detail: + except ValueError as detail: errorlist.append([ent, detail]) if 'source' not in ent: diff --git a/cloudinit/util.py b/cloudinit/util.py index d3b14f72..a1622946 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2244,6 +2244,7 @@ def gpg_export_armour(key): def gpg_recv_key(key, keyserver): """Receive gpg key from the specified keyserver""" + print("ORIGINAL gpg_recv_key") try: subp(["gpg", "--keyserver", keyserver, "--recv", key], capture=True) diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index a4878124..65e375a0 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -4,6 +4,7 @@ Testing various config variations of the apt_source config import os import re import shutil +import socket import tempfile try: @@ -56,6 +57,7 @@ class TestAptSourceConfig(TestCase): # mock fallback filename into writable tmp dir self.fallbackfn = os.path.join(self.tmp, "etc/apt/sources.list.d/", "cloud_config_sources.list") + self.orig_gpg_recv_key = util.gpg_recv_key patcher = mock.patch("cloudinit.config.cc_apt_configure.get_release") get_rel = patcher.start() @@ -445,14 +447,40 @@ class TestAptSourceConfig(TestCase): def apt_src_keyid_real(self, cfg, expectedkey): """apt_src_keyid_real Test specification of a keyid without source including - up to addition of the key (nothing but add_key_raw mocked to keep the + up to addition of the key (add_key_raw mocked to keep the environment as is) """ params = self._get_default_params() - with mock.patch.object(cc_apt_configure, 'add_key_raw') as mockobj: - cc_apt_configure.add_sources([cfg], params) - mockobj.assert_called_with(expectedkey) + def fake_gpg_recv_key(self, key, keyserver): + """try original gpg_recv_key, but allow fall back""" + try: + print("Try orig orig_gpg_recv_key") + self.orig_gpg_recv_key(key, keyserver) + except ValueError: + print("Fail, test net") + # if this is a networking issue mock it's effect + testsock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + try: + testsock.connect((keyserver, 80)) + testsock.close() + except socket.error: + # as fallback add the known key as a working recv would + print("Fallback import expectedkey") + util.subp(("gpg", "--import", "-"), EXPECTEDKEY) + + print("FOO") + with mock.patch.object(cc_apt_configure, 'add_key_raw') as mockkey: + with mock.patch.object(util, 'gpg_recv_key', + side_effect=fake_gpg_recv_key) as mockrecv: + cc_apt_configure.add_sources([cfg], params) + + # since we might mock the recv path ensure it is called right + mockrecv.assert_called_with(cfg['keyid'], + keyserver=cfg.get('keyserver', + 'keyserver.ubuntu.com')) + # no matter if really imported or faked, ensure we add the right key + mockkey.assert_called_with(expectedkey) # filename should be ignored on key only self.assertFalse(os.path.isfile(self.aptlistfile)) @@ -477,7 +505,7 @@ class TestAptSourceConfig(TestCase): """test_apt_src_longkeyid_ks_real - Test long keyid from other ks""" keyid = "B59D 5F15 97A5 04B7 E230 6DCA 0620 BBCF 0368 3F77" cfg = {'keyid': keyid, - 'keyserver': 'keys.gnupg.net', + 'keyserver': 'knorz.gnupg.net', 'filename': self.aptlistfile} self.apt_src_keyid_real(cfg, EXPECTEDKEY) -- cgit v1.2.3 From e9b333b49954d5863f8b53581454179b0363d978 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 13:52:31 +0200 Subject: fixup key tests for cases where network isn't available --- cloudinit/util.py | 1 - tests/unittests/test_handler/test_handler_apt_source.py | 8 ++------ 2 files changed, 2 insertions(+), 7 deletions(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index a1622946..d3b14f72 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2244,7 +2244,6 @@ def gpg_export_armour(key): def gpg_recv_key(key, keyserver): """Receive gpg key from the specified keyserver""" - print("ORIGINAL gpg_recv_key") try: subp(["gpg", "--keyserver", keyserver, "--recv", key], capture=True) diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index 65e375a0..c7eeb64c 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -452,13 +452,11 @@ class TestAptSourceConfig(TestCase): """ params = self._get_default_params() - def fake_gpg_recv_key(self, key, keyserver): + def fake_gpg_recv_key(key, keyserver): """try original gpg_recv_key, but allow fall back""" try: - print("Try orig orig_gpg_recv_key") self.orig_gpg_recv_key(key, keyserver) - except ValueError: - print("Fail, test net") + except ValueError as error: # if this is a networking issue mock it's effect testsock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) try: @@ -466,10 +464,8 @@ class TestAptSourceConfig(TestCase): testsock.close() except socket.error: # as fallback add the known key as a working recv would - print("Fallback import expectedkey") util.subp(("gpg", "--import", "-"), EXPECTEDKEY) - print("FOO") with mock.patch.object(cc_apt_configure, 'add_key_raw') as mockkey: with mock.patch.object(util, 'gpg_recv_key', side_effect=fake_gpg_recv_key) as mockrecv: -- cgit v1.2.3 From c1ad09609805c6c4f5262eb21533799af814379a Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 14:23:55 +0200 Subject: rename add_key / add_source to add_apt_key / add_apt_source The functions clearly are apt specific so the name should reflect that. --- cloudinit/config/cc_apt_configure.py | 14 ++++++------ .../test_handler/test_handler_apt_source.py | 25 +++++++++++----------- 2 files changed, 20 insertions(+), 19 deletions(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index a9ac6ea8..91a117f5 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -79,8 +79,8 @@ def handle(name, cfg, cloud, log, _args): def matcher(x): return False - errors = add_sources(cfg['apt_sources'], params, - aa_repo_match=matcher) + errors = add_apt_sources(cfg['apt_sources'], params, + aa_repo_match=matcher) for e in errors: log.warn("Add source error: %s", ':'.join(e)) @@ -146,7 +146,7 @@ def generate_sources_list(cfg, codename, mirrors, cloud, log): templater.render_to_file(template_fn, '/etc/apt/sources.list', params) -def add_key_raw(key): +def add_apt_key_raw(key): """ actual adding of a key as defined in key argument to the system @@ -157,7 +157,7 @@ def add_key_raw(key): raise ValueError('failed to add apt GPG Key to apt keyring') -def add_key(ent): +def add_apt_key(ent): """ add key to the system as defined in ent (if any) supports raw keys or keyid's @@ -170,7 +170,7 @@ def add_key(ent): ent['key'] = util.getkeybyid(ent['keyid'], keyserver) if 'key' in ent: - add_key_raw(ent['key']) + add_apt_key_raw(ent['key']) def convert_to_new_format(srclist): @@ -197,7 +197,7 @@ def convert_to_new_format(srclist): return srcdict -def add_sources(srclist, template_params=None, aa_repo_match=None): +def add_apt_sources(srclist, template_params=None, aa_repo_match=None): """ add entries in /etc/apt/sources.list.d for each abbreviated sources.list entry in 'srclist'. When rendering template, also @@ -220,7 +220,7 @@ def add_sources(srclist, template_params=None, aa_repo_match=None): # keys can be added without specifying a source try: - add_key(ent) + add_apt_key(ent) except ValueError as detail: errorlist.append([ent, detail]) diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index c7eeb64c..4a720213 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -89,7 +89,7 @@ class TestAptSourceConfig(TestCase): """ params = self._get_default_params() - cc_apt_configure.add_sources(cfg, params) + cc_apt_configure.add_apt_sources(cfg, params) self.assertTrue(os.path.isfile(filename)) @@ -200,7 +200,7 @@ class TestAptSourceConfig(TestCase): Test Autoreplacement of MIRROR and RELEASE in source specs """ params = self._get_default_params() - cc_apt_configure.add_sources(cfg, params) + cc_apt_configure.add_apt_sources(cfg, params) self.assertTrue(os.path.isfile(filename)) @@ -283,7 +283,7 @@ class TestAptSourceConfig(TestCase): with mock.patch.object(util, 'subp', return_value=('fakekey 1234', '')) as mockobj: - cc_apt_configure.add_sources(cfg, params) + cc_apt_configure.add_apt_sources(cfg, params) # check if it added the right ammount of keys calls = [] @@ -372,7 +372,7 @@ class TestAptSourceConfig(TestCase): params = self._get_default_params() with mock.patch.object(util, 'subp') as mockobj: - cc_apt_configure.add_sources([cfg], params) + cc_apt_configure.add_apt_sources([cfg], params) mockobj.assert_called_with(('apt-key', 'add', '-'), 'fakekey 4321') @@ -419,7 +419,7 @@ class TestAptSourceConfig(TestCase): 'filename': self.aptlistfile} with mock.patch.object(util, 'subp') as mockobj: - cc_apt_configure.add_sources([cfg], params) + cc_apt_configure.add_apt_sources([cfg], params) mockobj.assert_called_once_with(('apt-key', 'add', '-'), 'fakekey 4242') @@ -437,7 +437,7 @@ class TestAptSourceConfig(TestCase): with mock.patch.object(util, 'subp', return_value=('fakekey 1212', '')) as mockobj: - cc_apt_configure.add_sources([cfg], params) + cc_apt_configure.add_apt_sources([cfg], params) mockobj.assert_called_with(('apt-key', 'add', '-'), 'fakekey 1212') @@ -447,7 +447,7 @@ class TestAptSourceConfig(TestCase): def apt_src_keyid_real(self, cfg, expectedkey): """apt_src_keyid_real Test specification of a keyid without source including - up to addition of the key (add_key_raw mocked to keep the + up to addition of the key (add_apt_key_raw mocked to keep the environment as is) """ params = self._get_default_params() @@ -466,10 +466,10 @@ class TestAptSourceConfig(TestCase): # as fallback add the known key as a working recv would util.subp(("gpg", "--import", "-"), EXPECTEDKEY) - with mock.patch.object(cc_apt_configure, 'add_key_raw') as mockkey: + with mock.patch.object(cc_apt_configure, 'add_apt_key_raw') as mockkey: with mock.patch.object(util, 'gpg_recv_key', side_effect=fake_gpg_recv_key) as mockrecv: - cc_apt_configure.add_sources([cfg], params) + cc_apt_configure.add_apt_sources([cfg], params) # since we might mock the recv path ensure it is called right mockrecv.assert_called_with(cfg['keyid'], @@ -518,7 +518,8 @@ class TestAptSourceConfig(TestCase): 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) + cc_apt_configure.add_apt_sources([cfg], params, + aa_repo_match=matcher) mockobj.assert_called_once_with(['add-apt-repository', 'ppa:smoser/cloud-init-test']) @@ -541,8 +542,8 @@ class TestAptSourceConfig(TestCase): matcher = re.compile(r'^[\w-]+:\w').search with mock.patch.object(util, 'subp') as mockobj: - cc_apt_configure.add_sources([cfg1, cfg2, cfg3], params, - aa_repo_match=matcher) + cc_apt_configure.add_apt_sources([cfg1, cfg2, cfg3], params, + aa_repo_match=matcher) calls = [call(['add-apt-repository', 'ppa:smoser/cloud-init-test']), call(['add-apt-repository', 'ppa:smoser/cloud-init-test2']), call(['add-apt-repository', 'ppa:smoser/cloud-init-test3'])] -- cgit v1.2.3 From a3b234357ce1e1c7b8d9411d0928873752e2d107 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 14:34:45 +0200 Subject: only test the apt feature in environments with apt --- .../test_handler/test_handler_apt_configure_sources_list.py | 4 ++++ tests/unittests/test_handler/test_handler_apt_source.py | 5 +++++ 2 files changed, 9 insertions(+) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py b/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py index 5f1dd427..27e332d5 100644 --- a/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py +++ b/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py @@ -24,6 +24,8 @@ from .. import helpers as t_help LOG = logging.getLogger(__name__) +BIN_APT = "/usr/bin/apt" + YAML_TEXT_CUSTOM_SL = """ apt_mirror: http://archive.ubuntu.com/ubuntu/ apt_custom_sources_list: | @@ -67,6 +69,8 @@ def load_tfile_or_url(*args, **kwargs): return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents) +# This feature is apt specific and thereby is disabled in environments without +@t_help.skipIf(not os.path.isfile(BIN_APT), "no apt") class TestAptSourceConfigSourceList(t_help.FilesystemMockingTestCase): """TestAptSourceConfigSourceList Main Class to test sources list rendering diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index 4a720213..223af764 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -17,6 +17,9 @@ from cloudinit.config import cc_apt_configure from cloudinit import util from ..helpers import TestCase +from .. import helpers as t_help + +BIN_APT = "/usr/bin/apt" EXPECTEDKEY = """-----BEGIN PGP PUBLIC KEY BLOCK----- Version: GnuPG v1 @@ -40,6 +43,8 @@ def load_tfile_or_url(*args, **kwargs): return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents) +# This feature is apt specific and thereby is disabled in environments without +@t_help.skipIf(not os.path.isfile(BIN_APT), "no apt") class TestAptSourceConfig(TestCase): """TestAptSourceConfig Main Class to test apt_source configs -- cgit v1.2.3 From 1210eee8d6bd70817df10f2865af10c13553102b Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 14:48:21 +0200 Subject: fix alternate keyserver dns The intentionally false name was meant for testing of abug, now we can and should use a proper third party keyserver. --- tests/unittests/test_handler/test_handler_apt_source.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index 223af764..a184e7de 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -506,7 +506,7 @@ class TestAptSourceConfig(TestCase): """test_apt_src_longkeyid_ks_real - Test long keyid from other ks""" keyid = "B59D 5F15 97A5 04B7 E230 6DCA 0620 BBCF 0368 3F77" cfg = {'keyid': keyid, - 'keyserver': 'knorz.gnupg.net', + 'keyserver': 'keys.gnupg.net', 'filename': self.aptlistfile} self.apt_src_keyid_real(cfg, EXPECTEDKEY) -- cgit v1.2.3 From 01c47d9f3366a1535fbd50bd7be8f4e51f3f3b4b Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 14:57:56 +0200 Subject: apt_src_keyid_real: drop unused exception variable --- tests/unittests/test_handler/test_handler_apt_source.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index a184e7de..0b2aa9e5 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -461,7 +461,7 @@ class TestAptSourceConfig(TestCase): """try original gpg_recv_key, but allow fall back""" try: self.orig_gpg_recv_key(key, keyserver) - except ValueError as error: + except ValueError: # if this is a networking issue mock it's effect testsock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) try: -- cgit v1.2.3 From 9874eff179410d1cf48c7a02623c3b6bda545344 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 15:14:34 +0200 Subject: remove check that fails if user has installed tested keys The overall check for the expected key is a superset and would spot an issue. --- tests/unittests/test_handler/test_handler_apt_source.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index 0b2aa9e5..9aa6ff71 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -473,13 +473,9 @@ class TestAptSourceConfig(TestCase): with mock.patch.object(cc_apt_configure, 'add_apt_key_raw') as mockkey: with mock.patch.object(util, 'gpg_recv_key', - side_effect=fake_gpg_recv_key) as mockrecv: + side_effect=fake_gpg_recv_key): cc_apt_configure.add_apt_sources([cfg], params) - # since we might mock the recv path ensure it is called right - mockrecv.assert_called_with(cfg['keyid'], - keyserver=cfg.get('keyserver', - 'keyserver.ubuntu.com')) # no matter if really imported or faked, ensure we add the right key mockkey.assert_called_with(expectedkey) -- cgit v1.2.3 From bb0bf81cb373fde0c59464127633ebc7527f2be5 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 15:48:25 +0200 Subject: capture output of gpg calls to avoid messing up stdout/stderr --- cloudinit/util.py | 2 +- tests/unittests/test_handler/test_handler_apt_source.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index d3b14f72..6d16532d 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2254,7 +2254,7 @@ def gpg_recv_key(key, keyserver): def gpg_delete_key(key): """Delete the specified key from the local gpg ring""" - subp(["gpg", "--batch", "--yes", "--delete-keys", key], capture=False) + subp(["gpg", "--batch", "--yes", "--delete-keys", key], capture=True) def getkeybyid(keyid, keyserver): diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index 9aa6ff71..ea8aa17a 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -469,7 +469,8 @@ class TestAptSourceConfig(TestCase): testsock.close() except socket.error: # as fallback add the known key as a working recv would - util.subp(("gpg", "--import", "-"), EXPECTEDKEY) + util.subp(("gpg", "--import", "-"), EXPECTEDKEY, + capture=True) with mock.patch.object(cc_apt_configure, 'add_apt_key_raw') as mockkey: with mock.patch.object(util, 'gpg_recv_key', -- cgit v1.2.3 From 5d74451a5838c346fcbb4b0eee0d817fbf84128a Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 6 Jun 2016 16:47:42 +0200 Subject: make unittests more readable By moving the "what it does" into the first line of the docstrings output becomes much more useful. --- .../test_handler_apt_configure_sources_list.py | 20 ++--- .../test_handler/test_handler_apt_source.py | 92 +++++----------------- 2 files changed, 24 insertions(+), 88 deletions(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py b/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py index 27e332d5..20e61995 100644 --- a/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py +++ b/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py @@ -119,37 +119,27 @@ class TestAptSourceConfigSourceList(t_help.FilesystemMockingTestCase): {'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 - """ + """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 - """ + """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 - """ + """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 - """ + """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 - """ + """Test rendering from a custom source.list template""" cfg = util.load_yaml(YAML_TEXT_CUSTOM_SL) mycloud = self._get_cloud('ubuntu') diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index ea8aa17a..1f76e770 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -106,10 +106,7 @@ class TestAptSourceConfig(TestCase): contents, flags=re.IGNORECASE)) def test_apt_src_basic(self): - """test_apt_src_basic - Test Fix deb source string, has to overwrite mirror conf in params. - Test with a filename provided in config. - """ + """Test deb source string, overwrite mirror and filename""" cfg = {'source': ('deb http://archive.ubuntu.com/ubuntu' ' karmic-backports' ' main universe multiverse restricted'), @@ -117,11 +114,7 @@ class TestAptSourceConfig(TestCase): self.apt_src_basic(self.aptlistfile, [cfg]) def test_apt_src_basic_dict(self): - """test_apt_src_basic_dict - Test Fix deb source string, has to overwrite mirror conf in params. - Test with a filename provided in config. - Provided in a dictionary with filename being the key (new format) - """ + """Test deb source string, overwrite mirror and filename (dict)""" cfg = {self.aptlistfile: {'source': ('deb http://archive.ubuntu.com/ubuntu' ' karmic-backports' @@ -151,10 +144,7 @@ class TestAptSourceConfig(TestCase): contents, flags=re.IGNORECASE)) def test_apt_src_basic_tri(self): - """test_apt_src_basic_tri - Test Fix three deb source string, has to overwrite mirror conf in - params. Test with filenames provided in config. - """ + """Test Fix three deb source string with filenames""" cfg1 = {'source': ('deb http://archive.ubuntu.com/ubuntu' ' karmic-backports' ' main universe multiverse restricted'), @@ -170,11 +160,7 @@ class TestAptSourceConfig(TestCase): self.apt_src_basic_tri([cfg1, cfg2, cfg3]) def test_apt_src_basic_dict_tri(self): - """test_apt_src_basic_dict_tri - Test Fix three deb source string, has to overwrite mirror conf in - params. Test with filenames provided in config. - Provided in a dictionary with filename being the key (new format) - """ + """Test Fix three deb source string with filenames (dict)""" cfg = {self.aptlistfile: {'source': ('deb http://archive.ubuntu.com/ubuntu' ' karmic-backports' @@ -190,10 +176,7 @@ class TestAptSourceConfig(TestCase): self.apt_src_basic_tri(cfg) def test_apt_src_basic_nofn(self): - """test_apt_src_basic_nofn - Test Fix deb source string, has to overwrite mirror conf in params. - Test without a filename provided in config and test for known fallback. - """ + """Test Fix three deb source string without filenames (dict)""" cfg = {'source': ('deb http://archive.ubuntu.com/ubuntu' ' karmic-backports' ' main universe multiverse restricted')} @@ -216,10 +199,7 @@ class TestAptSourceConfig(TestCase): contents, flags=re.IGNORECASE)) def test_apt_src_replace(self): - """test_apt_src_replace - Test Autoreplacement of MIRROR and RELEASE in source specs with - Filename being set - """ + """Test Autoreplacement of MIRROR and RELEASE in source specs""" cfg = {'source': 'deb $MIRROR $RELEASE multiverse', 'filename': self.aptlistfile} self.apt_src_replacement(self.aptlistfile, [cfg]) @@ -245,10 +225,7 @@ class TestAptSourceConfig(TestCase): contents, flags=re.IGNORECASE)) def test_apt_src_replace_tri(self): - """test_apt_src_replace_tri - Test three autoreplacements of MIRROR and RELEASE in source specs with - Filename being set - """ + """Test triple Autoreplacement of MIRROR and RELEASE in source specs""" cfg1 = {'source': 'deb $MIRROR $RELEASE multiverse', 'filename': self.aptlistfile} cfg2 = {'source': 'deb $MIRROR $RELEASE main', @@ -258,13 +235,7 @@ class TestAptSourceConfig(TestCase): self.apt_src_replace_tri([cfg1, cfg2, cfg3]) def test_apt_src_replace_dict_tri(self): - """test_apt_src_replace_dict_tri - Test three autoreplacements of MIRROR and RELEASE in source specs with - Filename being set - Provided in a dictionary with filename being the key (new format) - We also test a new special conditions of the new format that allows - filenames to be overwritten inside the directory entry. - """ + """Test triple Autoreplacement in source specs (dict)""" cfg = {self.aptlistfile: {'source': 'deb $MIRROR $RELEASE multiverse'}, 'notused': {'source': 'deb $MIRROR $RELEASE main', 'filename': self.aptlistfile2}, @@ -272,10 +243,7 @@ class TestAptSourceConfig(TestCase): self.apt_src_replace_tri(cfg) def test_apt_src_replace_nofn(self): - """test_apt_src_replace_nofn - Test Autoreplacement of MIRROR and RELEASE in source specs with - No filename being set - """ + """Test Autoreplacement of MIRROR and RELEASE in source specs nofile""" cfg = {'source': 'deb $MIRROR $RELEASE multiverse'} with mock.patch.object(os.path, 'join', side_effect=self.myjoin): self.apt_src_replacement(self.fallbackfn, [cfg]) @@ -307,9 +275,7 @@ class TestAptSourceConfig(TestCase): contents, flags=re.IGNORECASE)) def test_apt_src_keyid(self): - """test_apt_src_keyid - Test specification of a source + keyid with filename being set - """ + """Test specification of a source + keyid with filename being set""" cfg = {'source': ('deb ' 'http://ppa.launchpad.net/' 'smoser/cloud-init-test/ubuntu' @@ -319,10 +285,7 @@ class TestAptSourceConfig(TestCase): self.apt_src_keyid(self.aptlistfile, [cfg], 1) def test_apt_src_keyid_tri(self): - """test_apt_src_keyid_tri - Test specification of a source + keyid with filename being set - Setting three of such, check for content and keys - """ + """Test 3x specification of a source + keyid with filename being set""" cfg1 = {'source': ('deb ' 'http://ppa.launchpad.net/' 'smoser/cloud-init-test/ubuntu' @@ -359,9 +322,7 @@ class TestAptSourceConfig(TestCase): contents, flags=re.IGNORECASE)) def test_apt_src_keyid_nofn(self): - """test_apt_src_keyid_nofn - Test specification of a source + keyid without filename being set - """ + """Test specification of a source + keyid without filename being set""" cfg = {'source': ('deb ' 'http://ppa.launchpad.net/' 'smoser/cloud-init-test/ubuntu' @@ -392,9 +353,7 @@ class TestAptSourceConfig(TestCase): contents, flags=re.IGNORECASE)) def test_apt_src_key(self): - """test_apt_src_key - Test specification of a source + key with filename being set - """ + """Test specification of a source + key with filename being set""" cfg = {'source': ('deb ' 'http://ppa.launchpad.net/' 'smoser/cloud-init-test/ubuntu' @@ -404,9 +363,7 @@ class TestAptSourceConfig(TestCase): self.apt_src_key(self.aptlistfile, cfg) def test_apt_src_key_nofn(self): - """test_apt_src_key_nofn - Test specification of a source + key without filename being set - """ + """Test specification of a source + key without filename being set""" cfg = {'source': ('deb ' 'http://ppa.launchpad.net/' 'smoser/cloud-init-test/ubuntu' @@ -416,9 +373,7 @@ class TestAptSourceConfig(TestCase): self.apt_src_key(self.fallbackfn, cfg) def test_apt_src_keyonly(self): - """test_apt_src_keyonly - Test specification key without source - """ + """Test specifying key without source""" params = self._get_default_params() cfg = {'key': "fakekey 4242", 'filename': self.aptlistfile} @@ -433,9 +388,7 @@ class TestAptSourceConfig(TestCase): self.assertFalse(os.path.isfile(self.aptlistfile)) def test_apt_src_keyidonly(self): - """test_apt_src_keyidonly - Test specification of a keyid without source - """ + """Test specification of a keyid without source""" params = self._get_default_params() cfg = {'keyid': "03683F77", 'filename': self.aptlistfile} @@ -509,9 +462,7 @@ class TestAptSourceConfig(TestCase): self.apt_src_keyid_real(cfg, EXPECTEDKEY) def test_apt_src_ppa(self): - """test_apt_src_ppa - Test specification of a ppa - """ + """Test adding a ppa""" params = self._get_default_params() cfg = {'source': 'ppa:smoser/cloud-init-test', 'filename': self.aptlistfile} @@ -529,9 +480,7 @@ class TestAptSourceConfig(TestCase): self.assertFalse(os.path.isfile(self.aptlistfile)) def test_apt_src_ppa_tri(self): - """test_apt_src_ppa_tri - Test specification of a ppa - """ + """Test adding three ppa's""" params = self._get_default_params() cfg1 = {'source': 'ppa:smoser/cloud-init-test', 'filename': self.aptlistfile} @@ -557,10 +506,7 @@ class TestAptSourceConfig(TestCase): self.assertFalse(os.path.isfile(self.aptlistfile3)) def test_convert_to_new_format(self): - """test_convert_to_new_format - Test the conversion of old to new format - And the noop conversion of new to new format as well - """ + """Test the conversion of old to new format""" cfg1 = {'source': 'deb $MIRROR $RELEASE multiverse', 'filename': self.aptlistfile} cfg2 = {'source': 'deb $MIRROR $RELEASE main', -- cgit v1.2.3 From 1f05fd90f38b8ff1f1b2d6418030545d502b3965 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Tue, 7 Jun 2016 08:23:14 +0200 Subject: drop gpg activity from aot-source key tests While the unittest gets a bit less real by that change, it will work in protected environment (e.g. sbuild) and leave the developers .gpg keyring alone. --- .../test_handler/test_handler_apt_source.py | 24 +++++----------------- 1 file changed, 5 insertions(+), 19 deletions(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index 1f76e770..6223674b 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -4,7 +4,6 @@ Testing various config variations of the apt_source config import os import re import shutil -import socket import tempfile try: @@ -410,27 +409,14 @@ class TestAptSourceConfig(TestCase): """ params = self._get_default_params() - def fake_gpg_recv_key(key, keyserver): - """try original gpg_recv_key, but allow fall back""" - try: - self.orig_gpg_recv_key(key, keyserver) - except ValueError: - # if this is a networking issue mock it's effect - testsock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - try: - testsock.connect((keyserver, 80)) - testsock.close() - except socket.error: - # as fallback add the known key as a working recv would - util.subp(("gpg", "--import", "-"), EXPECTEDKEY, - capture=True) - with mock.patch.object(cc_apt_configure, 'add_apt_key_raw') as mockkey: - with mock.patch.object(util, 'gpg_recv_key', - side_effect=fake_gpg_recv_key): + with mock.patch.object(util, 'getkeybyid', + return_value=expectedkey) as mockgetkey: cc_apt_configure.add_apt_sources([cfg], params) - # no matter if really imported or faked, ensure we add the right key + mockgetkey.assert_called_with(cfg['keyid'], + cfg.get('keyserver', + 'keyserver.ubuntu.com')) mockkey.assert_called_with(expectedkey) # filename should be ignored on key only -- cgit v1.2.3 From f4df4fbf200ef04c841bc36baa559bce265fcb38 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Tue, 7 Jun 2016 10:58:38 +0200 Subject: enable test_handler_apt_source unittests on CentOS Tested and working on CentOS7 (container), so we can enable the tests --- tests/unittests/test_handler/test_handler_apt_source.py | 3 --- 1 file changed, 3 deletions(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index 6223674b..3f87fad3 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -16,7 +16,6 @@ from cloudinit.config import cc_apt_configure from cloudinit import util from ..helpers import TestCase -from .. import helpers as t_help BIN_APT = "/usr/bin/apt" @@ -42,8 +41,6 @@ def load_tfile_or_url(*args, **kwargs): return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents) -# This feature is apt specific and thereby is disabled in environments without -@t_help.skipIf(not os.path.isfile(BIN_APT), "no apt") class TestAptSourceConfig(TestCase): """TestAptSourceConfig Main Class to test apt_source configs -- cgit v1.2.3 From b89ac6b7caeeade5ad21137773ac4496cdaea2c5 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Thu, 9 Jun 2016 09:18:35 +0200 Subject: move gpg functions into gpg.py This helps for cleaner code structuring. ALong that makeing sure all these functions have a gpg_prefix. --- cloudinit/config/cc_apt_configure.py | 3 +- cloudinit/gpg.py | 64 ++++++++++++++++++++++ cloudinit/util.py | 38 ------------- .../test_handler/test_handler_apt_source.py | 4 +- 4 files changed, 68 insertions(+), 41 deletions(-) create mode 100644 cloudinit/gpg.py (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index be6324a2..ba080930 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -24,6 +24,7 @@ import re from cloudinit import templater from cloudinit import util +from cloudinit import gpg distros = ['ubuntu', 'debian'] @@ -167,7 +168,7 @@ def add_apt_key(ent): keyserver = "keyserver.ubuntu.com" if 'keyserver' in ent: keyserver = ent['keyserver'] - ent['key'] = util.getkeybyid(ent['keyid'], keyserver) + ent['key'] = gpg.gpg_getkeybyid(ent['keyid'], keyserver) if 'key' in ent: add_apt_key_raw(ent['key']) diff --git a/cloudinit/gpg.py b/cloudinit/gpg.py new file mode 100644 index 00000000..620dfb19 --- /dev/null +++ b/cloudinit/gpg.py @@ -0,0 +1,64 @@ +"""gpg.py - Collection of gpg key related functions""" +# vi: ts=4 expandtab +# +# Copyright (C) 2016 Canonical Ltd. +# +# Author: Scott Moser +# Author: Juerg Haefliger +# Author: Joshua Harlow +# Author: Christian Ehrhardt +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from cloudinit import util +from cloudinit import log as logging + +LOG = logging.getLogger(__name__) + + +def gpg_export_armour(key): + """Export gpg key, armoured key gets returned""" + (armour, _) = util.subp(["gpg", "--export", "--armour", key], capture=True) + return armour + + +def gpg_recv_key(key, keyserver): + """Receive gpg key from the specified keyserver""" + try: + util.subp(["gpg", "--keyserver", keyserver, "--recv", key], + capture=True) + except util.ProcessExecutionError as error: + raise ValueError('Failed to import key %s from server %s - error %s' % + (key, keyserver, error)) + + +def gpg_delete_key(key): + """Delete the specified key from the local gpg ring""" + util.subp(["gpg", "--batch", "--yes", "--delete-keys", key], capture=True) + + +def gpg_getkeybyid(keyid, keyserver): + """get gpg keyid from keyserver""" + armour = gpg_export_armour(keyid) + if not armour: + try: + gpg_recv_key(keyid, keyserver=keyserver) + except ValueError: + LOG.exception('Failed to obtain gpg key %s', keyid) + raise + + armour = gpg_export_armour(keyid) + # delete just imported key to leave environment as it was before + gpg_delete_key(keyid) + + return armour.rstrip('\n') diff --git a/cloudinit/util.py b/cloudinit/util.py index 6d16532d..d6b80dbe 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2234,41 +2234,3 @@ def message_from_string(string): if sys.version_info[:2] < (2, 7): return email.message_from_file(six.StringIO(string)) return email.message_from_string(string) - - -def gpg_export_armour(key): - """Export gpg key, armoured key gets returned""" - (armour, _) = subp(["gpg", "--export", "--armour", key], capture=True) - return armour - - -def gpg_recv_key(key, keyserver): - """Receive gpg key from the specified keyserver""" - try: - subp(["gpg", "--keyserver", keyserver, "--recv", key], - capture=True) - except ProcessExecutionError as error: - raise ValueError('Failed to import key %s from server %s - error %s' % - (key, keyserver, error)) - - -def gpg_delete_key(key): - """Delete the specified key from the local gpg ring""" - subp(["gpg", "--batch", "--yes", "--delete-keys", key], capture=True) - - -def getkeybyid(keyid, keyserver): - """get gpg keyid from keyserver""" - armour = gpg_export_armour(keyid) - if not armour: - try: - gpg_recv_key(keyid, keyserver=keyserver) - except ValueError: - LOG.exception('Failed to obtain gpg key %s', keyid) - raise - - armour = gpg_export_armour(keyid) - # delete just imported key to leave environment as it was before - gpg_delete_key(keyid) - - return armour.rstrip('\n') diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index 3f87fad3..b09dd8ab 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -14,6 +14,7 @@ from mock import call from cloudinit.config import cc_apt_configure from cloudinit import util +from cloudinit import gpg from ..helpers import TestCase @@ -58,7 +59,6 @@ class TestAptSourceConfig(TestCase): # mock fallback filename into writable tmp dir self.fallbackfn = os.path.join(self.tmp, "etc/apt/sources.list.d/", "cloud_config_sources.list") - self.orig_gpg_recv_key = util.gpg_recv_key patcher = mock.patch("cloudinit.config.cc_apt_configure.get_release") get_rel = patcher.start() @@ -407,7 +407,7 @@ class TestAptSourceConfig(TestCase): params = self._get_default_params() with mock.patch.object(cc_apt_configure, 'add_apt_key_raw') as mockkey: - with mock.patch.object(util, 'getkeybyid', + with mock.patch.object(gpg, 'gpg_getkeybyid', return_value=expectedkey) as mockgetkey: cc_apt_configure.add_apt_sources([cfg], params) -- cgit v1.2.3 From 2d7f4673db55d5111d5e31e32eea3ca64c4e5f79 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Thu, 9 Jun 2016 10:01:32 +0200 Subject: remove unused BIN_APT constant --- tests/unittests/test_handler/test_handler_apt_configure_sources_list.py | 2 -- tests/unittests/test_handler/test_handler_apt_source.py | 2 -- 2 files changed, 4 deletions(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py b/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py index 2e604216..bedef51c 100644 --- a/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py +++ b/tests/unittests/test_handler/test_handler_apt_configure_sources_list.py @@ -27,8 +27,6 @@ from .. import helpers as t_help LOG = logging.getLogger(__name__) -BIN_APT = "/usr/bin/apt" - YAML_TEXT_CUSTOM_SL = """ apt_mirror: http://archive.ubuntu.com/ubuntu/ apt_custom_sources_list: | diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index b09dd8ab..5b52f709 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -18,8 +18,6 @@ from cloudinit import gpg from ..helpers import TestCase -BIN_APT = "/usr/bin/apt" - EXPECTEDKEY = """-----BEGIN PGP PUBLIC KEY BLOCK----- Version: GnuPG v1 -- cgit v1.2.3 From 7e527b1b2f3fda558fb0f3a6958c42dde4716079 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Fri, 10 Jun 2016 13:22:59 -0400 Subject: minor changes prior to merge a.) remove 'gpg_' from function names in new gpg module. b.) use --recv-keys rather than --recv --recv-keys is more obvious and works back to precise at least. c.) do not trim trailing '\n' from a armour'd key. --- cloudinit/config/cc_apt_configure.py | 4 ++-- cloudinit/gpg.py | 24 ++++++++++------------ .../test_handler/test_handler_apt_source.py | 4 ++-- 3 files changed, 15 insertions(+), 17 deletions(-) (limited to 'tests/unittests/test_handler/test_handler_apt_source.py') diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index ba080930..96c4a43d 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -22,9 +22,9 @@ import glob import os import re +from cloudinit import gpg from cloudinit import templater from cloudinit import util -from cloudinit import gpg distros = ['ubuntu', 'debian'] @@ -168,7 +168,7 @@ def add_apt_key(ent): keyserver = "keyserver.ubuntu.com" if 'keyserver' in ent: keyserver = ent['keyserver'] - ent['key'] = gpg.gpg_getkeybyid(ent['keyid'], keyserver) + ent['key'] = gpg.get_key_by_id(ent['keyid'], keyserver) if 'key' in ent: add_apt_key_raw(ent['key']) diff --git a/cloudinit/gpg.py b/cloudinit/gpg.py index baa8b534..6a76d785 100644 --- a/cloudinit/gpg.py +++ b/cloudinit/gpg.py @@ -4,8 +4,6 @@ # Copyright (C) 2016 Canonical Ltd. # # Author: Scott Moser -# Author: Juerg Haefliger -# Author: Joshua Harlow # Author: Christian Ehrhardt # # This program is free software: you can redistribute it and/or modify @@ -20,13 +18,13 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from cloudinit import util from cloudinit import log as logging +from cloudinit import util LOG = logging.getLogger(__name__) -def gpg_export_armour(key): +def export_armour(key): """Export gpg key, armoured key gets returned""" try: (armour, _) = util.subp(["gpg", "--export", "--armour", key], @@ -38,11 +36,11 @@ def gpg_export_armour(key): return armour -def gpg_recv_key(key, keyserver): +def receive_key(key, keyserver): """Receive gpg key from the specified keyserver""" LOG.debug('Receive gpg key "%s"', key) try: - util.subp(["gpg", "--keyserver", keyserver, "--recv", key], + util.subp(["gpg", "--keyserver", keyserver, "--recv-keys", key], capture=True) except util.ProcessExecutionError as error: raise ValueError(('Failed to import key "%s" ' @@ -50,7 +48,7 @@ def gpg_recv_key(key, keyserver): (key, keyserver, error)) -def gpg_delete_key(key): +def delete_key(key): """Delete the specified key from the local gpg ring""" try: util.subp(["gpg", "--batch", "--yes", "--delete-keys", key], @@ -59,18 +57,18 @@ def gpg_delete_key(key): LOG.warn('Failed delete key "%s": %s', key, error) -def gpg_getkeybyid(keyid, keyserver): +def get_key_by_id(keyid, keyserver="keyserver.ubuntu.com"): """get gpg keyid from keyserver""" - armour = gpg_export_armour(keyid) + armour = export_armour(keyid) if not armour: try: - gpg_recv_key(keyid, keyserver=keyserver) - armour = gpg_export_armour(keyid) + receive_key(keyid, keyserver=keyserver) + armour = export_armour(keyid) except ValueError: LOG.exception('Failed to obtain gpg key %s', keyid) raise finally: # delete just imported key to leave environment as it was before - gpg_delete_key(keyid) + delete_key(keyid) - return armour.rstrip('\n') + return armour diff --git a/tests/unittests/test_handler/test_handler_apt_source.py b/tests/unittests/test_handler/test_handler_apt_source.py index 5b52f709..99a4d860 100644 --- a/tests/unittests/test_handler/test_handler_apt_source.py +++ b/tests/unittests/test_handler/test_handler_apt_source.py @@ -13,8 +13,8 @@ except ImportError: from mock import call from cloudinit.config import cc_apt_configure -from cloudinit import util from cloudinit import gpg +from cloudinit import util from ..helpers import TestCase @@ -405,7 +405,7 @@ class TestAptSourceConfig(TestCase): params = self._get_default_params() with mock.patch.object(cc_apt_configure, 'add_apt_key_raw') as mockkey: - with mock.patch.object(gpg, 'gpg_getkeybyid', + with mock.patch.object(gpg, 'get_key_by_id', return_value=expectedkey) as mockgetkey: cc_apt_configure.add_apt_sources([cfg], params) -- cgit v1.2.3