Merge lp:~bloodearnest/charm-helpers/ppa-tools into lp:charm-helpers

[Simon Davy]

Added enable_private_ppas and add_apt_key helperss

Added enable_private_ppas and add_apt_key helperss

[Michael Nelson (michael.nelson)]

14:56 <bloodearnest> https://code.launchpad.net/~bloodearnest/charm-helpers/ppa-tools/+merge/175308
14:56 <bloodearnest> be nice if we could a "shared pipeline" for a fast moving dep branch like that
14:57 <noodles> Yeah
14:57 <noodles> Nice branch!
14:58 <bloodearnest> simple functions
14:58 <noodles> I think mock expectations are used a bit much in charmhelpers... what I've been doing...
14:58 * noodles checks on his own MP
14:58 <bloodearnest> I think a mock filesystem would be better
15:00 <noodles> Yeah - well, I'm not sure if it's what you mean, but you can do that really easily...
15:00 <noodles> If you look at test_updates_existing_values()
15:00 <noodles> Teh actual code writes a file to /etc/salt/grains
15:01 <noodles> But the test can just mock the path to the grains file with a tmp directory, then the rest of the test is on the actual filesystem.
15:01 <bloodearnest> looking
15:31 <bloodearnest> hmm - not sure how to apply the tmp dir to my test, as the path is a hardcoded one
15:31 <noodles> Oh - you can't make it a module-level path? Yeah, that doesn't work then.
15:32 <bloodearnest> I could, I guess
15:32 <bloodearnest> python needs better whole fs mocking tools
15:32 <noodles> I love mock - so much better than what was available before...
15:35 <bloodearnest> mock is great
15:35 <bloodearnest> I loved how easy it was to mock my new hooks logic with self.mock_charmhelpers
15:35 <noodles> Yeah!

review: Approve

[lp:~bloodearnest/charm-helpers/ppa-tools]

48. By Simon Davy on 2013-07-18

Refactor tests to use filesystem, and fix check_call args in add_apt_key

[Matthew Wedgwood (mew)]

Hi, Simon. Thanks for the MP!

Although there are vestigial helpers for package management in charmhelpers.core, this functionality should live in charmhelpers.fetch. Would you mind reshuffling and resubmitting?

Thanks.

review: Needs Fixing

Unmerged revisions

48. By Simon Davy on 2013-07-18

Refactor tests to use filesystem, and fix check_call args in add_apt_key

47. By Simon Davy on 2013-07-17

typo

46. By Simon Davy on 2013-07-17

Added enable_private_ppas and add_apt_key helpers

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2013-07-18 13:24:23 +0000
+++ charmhelpers/core/host.py	2013-07-18 13:24:23 +0000
@@ -192,6 +192,27 @@
         apt_update(fatal=fatal)
 
 
+DISABLE_PROXY_PPA_APTCONF = '/etc/apt/apt.conf.d/97disable-ppa-proxy'
+
+
+def enable_private_ppas():
+    """Bypass apt proxies for private-ppa.launchpad.net to allow auth"""
+    if not os.path.exists(DISABLE_PROXY_PPA_APTCONF):
+        write_file(
+            DISABLE_PROXY_PPA_APTCONF,
+            'Acquire::HTTP::Proxy::private-ppa.launchpad.net "DIRECT";',
+            perms=0644
+        )
+
+
+def add_apt_key(key):
+    """Add an apt key. Useful for ppas"""
+    cmd = ['apt-key', 'adv',
+           '--keyserver', 'keyserver.ubuntu.com',
+           '--recv-keys', key]
+    subprocess.check_call(cmd)
+
+
 def mount(device, mountpoint, options=None, persist=False):
     '''Mount a filesystem'''
     cmd_args = ['mount']

=== modified file 'tests/core/test_host.py'
--- tests/core/test_host.py	2013-07-18 13:24:23 +0000
+++ tests/core/test_host.py	2013-07-18 13:24:23 +0000
@@ -2,6 +2,9 @@
 from contextlib import contextmanager
 import subprocess
 import io
+import tempfile
+import os.path
+import shutil
 
 from mock import patch, call, MagicMock
 from testtools import TestCase
@@ -556,6 +559,51 @@
             call(['apt-get', 'update']),
         ])
 
+    def _setup_etc_with_path(self, *paths):
+        etc_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, etc_dir)
+        file_path = os.path.join(etc_dir, *paths)
+        dir_path = os.path.dirname(file_path)
+        os.makedirs(dir_path)
+        return file_path
+
+    @patch.object(host, 'execution_environment')
+    @patch('os.fchown', MagicMock())
+    @patch('os.fchmod', MagicMock())
+    def test_enable_private_ppa_conf_not_exist(self, mock_exec_env):
+        mock_exec_env.return_value = {}
+        aptpath = self._setup_etc_with_path('apt', 'apt.conf.d', 'test')
+        with patch.object(host, 'DISABLE_PROXY_PPA_APTCONF', aptpath):
+            host.enable_private_ppas()
+
+        self.assertTrue(os.path.exists(aptpath))
+        aptconfig = open(aptpath).read()
+        self.assertEqual(
+            aptconfig,
+            'Acquire::HTTP::Proxy::private-ppa.launchpad.net "DIRECT";'
+        )
+
+    def test_enable_private_ppa_conf_does_exist(self):
+        aptpath = self._setup_etc_with_path('apt', 'apt.conf.d', 'test')
+        with open(aptpath, 'w+') as aptconf:
+            aptconf.write("ORIGNAL")
+        with patch.object(host, 'DISABLE_PROXY_PPA_APTCONF', aptpath):
+            host.enable_private_ppas()
+
+        self.assertTrue(os.path.exists(aptpath))
+        aptconfig = open(aptpath).read()
+        self.assertEqual(aptconfig, "ORIGNAL")
+
+    @patch('subprocess.check_call')
+    def test_add_apt_key_calls_correctly(self, mock_call):
+        host.add_apt_key('key')
+        expected = [
+            'apt-key', 'adv',
+            '--keyserver', 'keyserver.ubuntu.com',
+            '--recv-keys', 'key'
+        ]
+        mock_call.assert_called_once_with(expected)
+
     @patch('apt_pkg.Cache')
     def test_filter_packages_missing(self, cache):
         cache.side_effect = fake_apt_cache