Merge lp:~adam-collard/landscape-charm/install-sources-keys into lp:~landscape/landscape-charm/trunk

[Adam Collard]

Add support for the harmonised configuration keys for setting sources.list: install_sources, install_keys.

Integration tests pass modulo a test for update_security_db.sh cron job which we can look into separately.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 393
Branch: lp:~adam-collard/landscape-charm/install-sources-keys
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3915/

review: Approve (test results)

[lp:~adam-collard/landscape-charm/install-sources-keys]

394. By Adam Collard on 2019-01-17

Noop change

395. By Adam Collard on 2019-01-17

Merge from trunk

396. By Adam Collard on 2019-01-17

392 alive again

397. By Adam Collard on 2019-01-17

393 alive again

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 397
Branch: lp:~adam-collard/landscape-charm/install-sources-keys
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3916/

review: Approve (test results)

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 397
Branch: lp:~adam-collard/landscape-charm/install-sources-keys
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3917/

review: Approve (test results)

[Simon Poirier (simpoir)]

+1 LGTM.

Also bundletester is a pain to get working nowadays

review: Approve

=== modified file 'Makefile'
--- Makefile	2019-01-17 14:18:28 +0000
+++ Makefile	2019-01-17 14:24:17 +0000
@@ -45,7 +45,7 @@
 	fi
 
 integration-test: test-depends
-	~/.local/bin/bundletester -v -l DEBUG --skip-implicit -t .
+	python2 ~/.local/bin/bundletester -v -l DEBUG --skip-implicit -t .
 
 # Run integration tests using the LDS package from the lds-trunk PPA
 integration-test-trunk: secrets

=== modified file 'config.yaml'
--- config.yaml	2019-01-17 14:18:28 +0000
+++ config.yaml	2019-01-17 14:24:17 +0000
@@ -1,8 +1,29 @@
 options:
+    install_sources:
+        default: "['ppa:landscape/18.03']"
+        type: string
+        description: >
+          List of extra apt sources, per charm-helpers standard format (a
+          yaml list of strings encoded as a string). Each source may be either a line
+          that can be added directly to sources.list(5), or in the form ppa:<user>/<ppa-name>
+          for adding Personal Package Archives, or a distribution component to enable.
+    install_keys:
+        default: ""
+        type: string
+        description: >
+          List of signing keys for install_sources package sources, per
+          charmhelpers standard format (a yaml list of strings encoded as a string).
+          The keys should be the full ASCII armoured GPG public keys. While GPG key
+          ids are also supported and looked up on a keyserver, operators should be aware
+          that this mechanism is insecure. null can be used if a standard package signing
+          key is used that will already be installed on the machine, and for PPA sources
+          where the package signing key is securely retrieved from Launchpad.\n"
     source:
-        default: "ppa:landscape/18.03"
+        default: ""
         type: string
         description: |
+            *DEPRECATED*. See install_sources
+
             The source from which to install the landscape packages. Possible
             values are:
                 - A version number: "16.06"
@@ -16,6 +37,8 @@
         default: ""
         type: string
         description: |
+            *DEPRECATED*. See install_keys
+
             Key ID to import to the apt keyring to support use with arbitary source
             configuration from outside of Launchpad archives or PPA's.
             In case multiple repositories are specified in the 'source' config key,

=== modified file 'dev/ubuntu-deps'
--- dev/ubuntu-deps	2019-01-17 14:18:28 +0000
+++ dev/ubuntu-deps	2019-01-17 14:24:17 +0000
@@ -14,7 +14,8 @@
     python3-yaml curl python3-zope.testrunner \
     python3-amulet python3-fixtures python3-jinja2 python3-fixtures \
     python3-psutil python3-twisted python3-flake8 \
-    python3-path python3-libcharmstore python3-zope.testrunner python3-six
+    python3-path python3-libcharmstore python3-zope.testrunner python3-six \
+    python-twisted-core
 
 sudo apt-get -y install python-fixtures python-six python-yaml dpkg-dev \
     pbuilder dh-make python-jinja2 python-psutil python-six

=== modified file 'lib/apt.py'
--- lib/apt.py	2019-01-17 14:18:28 +0000
+++ lib/apt.py	2019-01-17 14:24:17 +0000
@@ -53,13 +53,22 @@
 
 
 class AptSourceAndKeyDontMatchError(CharmError):
-    """Provided config values for 'source' and 'key' do not match.."""
+    """Provided config values for 'source' and 'key' do not match."""
 
     def __init__(self):
         message = "The 'source' and 'key' lists have different lengths"
         super(AptSourceAndKeyDontMatchError, self).__init__(message)
 
 
+class SourceConflictError(CharmError):
+    """Config values for 'source' and 'install_sources' are present."""
+
+    def __init__(self, install_sources, source):
+        message = ("install_sources: {!r} and source: {!r} are "
+                   "mutually exclusive.").format(install_sources, source)
+        super(SourceConflictError, self).__init__(message)
+
+
 class Apt(object):
     """Perform APT-related tasks as setting sources and installing packages.
 
@@ -130,6 +139,14 @@
         """Set the remote APT repository to use, if new or changed."""
         config = self._hookenv.config()
         source = config.get("source")
+
+        # New style install sources
+        install_sources = config.get("install_sources")
+        if install_sources and source:
+            raise SourceConflictError(install_sources, source)
+        if install_sources:
+            self._fetch.configure_sources()
+            return
         if not source:
             raise AptNoSourceConfigError()
 

=== modified file 'lib/callbacks/tests/test_apt.py'
--- lib/callbacks/tests/test_apt.py	2019-01-17 14:18:28 +0000
+++ lib/callbacks/tests/test_apt.py	2019-01-17 14:24:17 +0000
@@ -7,7 +7,7 @@
 
     def setUp(self):
         super(SetAPTSourcesTest, self).setUp()
-        self.fetch = FetchStub()
+        self.fetch = FetchStub(self.hookenv.config)
         self.subprocess = SubprocessStub()
         self.callback = SetAPTSources(
             hookenv=self.hookenv, fetch=self.fetch, subprocess=self.subprocess)

=== modified file 'lib/tests/stubs.py'
--- lib/tests/stubs.py	2019-01-17 14:18:28 +0000
+++ lib/tests/stubs.py	2019-01-17 14:24:17 +0000
@@ -107,7 +107,9 @@
 class FetchStub(object):
     """Provide a testable stub for C{charmhelpers.fetch}."""
 
-    def __init__(self):
+    def __init__(self, config):
+        self._config = config
+        self.configured_sources = []
         self.sources = []
         self.updates = []
         self.filtered = []
@@ -126,6 +128,11 @@
     def apt_install(self, packages, options=None, fatal=False):
         self.installed.append((packages, options, fatal))
 
+    def configure_sources(self):
+        install_sources = self._config().get('install_sources')
+        install_keys = self._config().get('install_keys')
+        self.configured_sources.append((install_sources, install_keys))
+
 
 class HostStub(object):
     """Testable stub for C{charmhelpers.core.host}."""

=== modified file 'lib/tests/test_apt.py'
--- lib/tests/test_apt.py	2019-01-17 14:18:28 +0000
+++ lib/tests/test_apt.py	2019-01-17 14:24:17 +0000
@@ -5,8 +5,8 @@
 
 from lib.apt import (
     Apt, AptNoSourceConfigError, AptSourceAndKeyDontMatchError,
-    INSTALL_PACKAGES, DEFAULT_INSTALL_OPTIONS, SAMPLE_HASHIDS_PPA,
-    SAMPLE_HASHIDS_KEY)
+    SourceConflictError, INSTALL_PACKAGES, DEFAULT_INSTALL_OPTIONS,
+    SAMPLE_HASHIDS_PPA, SAMPLE_HASHIDS_KEY)
 from lib.tests.stubs import FetchStub, SubprocessStub
 from lib.tests.helpers import HookenvTest
 from lib.tests.rootdir import RootDir
@@ -16,7 +16,9 @@
 
     def setUp(self):
         super(AptTest, self).setUp()
-        self.fetch = FetchStub()
+        # charmhelpers.fetch doesn't support dependency injection, so
+        # we pass in our config to be able to inspect what was used
+        self.fetch = FetchStub(self.hookenv.config)
         self.subprocess = SubprocessStub()
         self.root_dir = self.useFixture(RootDir())
         self.paths = self.root_dir.paths
@@ -64,6 +66,8 @@
         self.apt.set_sources()
         self.assertEqual([("ppa:landscape/14.10", None)], self.fetch.sources)
         self.assertEqual([True], self.fetch.updates)
+        # With source set, fetch.configure_sources is not called
+        self.assertEqual(self.fetch.configured_sources, [])
 
     def test_set_shorthand_sources(self):
         """
@@ -154,9 +158,33 @@
         self.hookenv.config()["key"] = "xyz"
         with self.assertRaises(AptSourceAndKeyDontMatchError) as error:
             self.apt.set_sources()
-            self.assertEqual(
-                "The 'source' and 'key' lists have different lengths",
-                str(error))
+        self.assertEqual(
+            "The 'source' and 'key' lists have different lengths",
+            str(error.exception))
+
+    def test_set_sources_and_install_sources(self):
+        """
+        If sources and install_sources are set set_sources complains.
+        """
+        self.hookenv.config()["source"] = "ppa:landscape/14.10"
+        self.hookenv.config()["install_sources"] = "anything"
+        with self.assertRaises(SourceConflictError) as error:
+            self.apt.set_sources()
+        self.assertEqual(
+            "install_sources: 'anything' and source: 'ppa:landscape/14.10' "
+            "are mutually exclusive.",
+            str(error.exception))
+
+    def test_install_sources(self):
+        """
+        If install_sources is set, it's passed to fetch.configure_sources.
+        """
+        self.hookenv.config()["install_sources"] = "ppa:landscape/19.01"
+        self.hookenv.config()["install_keys"] = "DEADBEEF"
+        self.apt.set_sources()
+        self.assertEqual(
+            self.fetch.configured_sources,
+            [('ppa:landscape/19.01', 'DEADBEEF')])
 
     def test_local_tarball(self):
         """

=== modified file 'lib/tests/test_install.py'
--- lib/tests/test_install.py	2019-01-17 14:18:28 +0000
+++ lib/tests/test_install.py	2019-01-17 14:24:17 +0000
@@ -9,7 +9,7 @@
 
     def setUp(self):
         super(InstallHookTest, self).setUp()
-        self.fetch = FetchStub()
+        self.fetch = FetchStub(self.hookenv.config)
         self.subprocess = SubprocessStub()
         self.subprocess.add_fake_executable("apt-mark")
         self.hook = InstallHook(

=== modified file 'lib/tests/test_services.py'
--- lib/tests/test_services.py	2019-01-17 14:18:28 +0000
+++ lib/tests/test_services.py	2019-01-17 14:24:17 +0000
@@ -31,7 +31,7 @@
         self.root_dir = self.useFixture(RootDir())
         self.paths = self.root_dir.paths
         self.root_dir = self.useFixture(RootDir())
-        self.fetch = FetchStub()
+        self.fetch = FetchStub(self.hookenv.config)
         self.psutil = PsutilStub(num_cpus=2, physical_memory=1*1024**3)
         self.hook = ServicesHook(
             hookenv=self.hookenv, host=self.host, subprocess=self.subprocess,

=== modified file 'lib/tests/test_upgrade.py'
--- lib/tests/test_upgrade.py	2019-01-17 14:18:28 +0000
+++ lib/tests/test_upgrade.py	2019-01-17 14:24:17 +0000
@@ -11,7 +11,7 @@
 
     def setUp(self):
         super(UpgradeActionTest, self).setUp()
-        self.fetch = FetchStub()
+        self.fetch = FetchStub(self.hookenv.config)
         self.subprocess = SubprocessStub()
         self.subprocess.add_fake_executable("apt-mark")
         self.root_dir = self.useFixture(RootDir())

=== modified file 'tests/helpers.py'
--- tests/helpers.py	2019-01-17 14:18:28 +0000
+++ tests/helpers.py	2019-01-17 14:24:17 +0000
@@ -501,8 +501,8 @@
                     extra_config = yaml.safe_load(fd.read())
             else:
                 extra_config = {
-                    "source": source,
-                    "key": os.environ.get("LS_CHARM_KEY", "4652B4E6")
+                    "install_sources": source,
+                    "install_keys": os.environ.get("LS_CHARM_KEY", "4652B4E6")
                 }
             context["landscape"].update(extra_config)