Merge lp:~blake-rouse/maas/import-with-gpg-home into lp:~maas-committers/maas/trunk

[Blake Rouse]

On the region import boot resource with gpg home directory set to the maas users. This was done on the cluster side, but was not on the region side, causing verification of simplestreams data to fail.

[Jeroen T. Vermeulen (jtv)]

test__import_resources_has_env_GNUPGHOME_set has this assertion:

        self.assertTrue(
            bootresources.MAAS_USER_GPGHOME,
            fake_download.env['GNUPGHOME'])

I think that assertTrue should have been assertEqual here.

One big question I'm left with is: who owns that home directory when you create it? It's still the maas user, right? It's important to get that right, but not the sort of thing you can really test.

review: Approve

[Blake Rouse (blake-rouse)]

Yes it is owned by maas:maas. It uses the same code, that is used by pserv.

=== modified file 'src/maasserver/bootresources.py'
--- src/maasserver/bootresources.py	2014-08-26 17:32:40 +0000
+++ src/maasserver/bootresources.py	2014-08-27 12:28:22 +0000
@@ -48,6 +48,7 @@
 from maasserver.utils import absolute_reverse
 from maasserver.utils.async import transactional
 from maasserver.utils.orm import get_one
+from provisioningserver.auth import MAAS_USER_GPGHOME
 from provisioningserver.import_images.download_descriptions import (
     download_all_image_descriptions,
     )
@@ -58,6 +59,7 @@
 from provisioningserver.import_images.keyrings import write_all_keyrings
 from provisioningserver.import_images.product_mapping import map_products
 from provisioningserver.logger import get_maas_logger
+from provisioningserver.utils.env import environment_variables
 from provisioningserver.utils.twisted import synchronous
 from simplestreams import util as sutil
 from simplestreams.mirrors import (
@@ -851,20 +853,24 @@
         return
 
     try:
-        maaslog.info("Started importing of boot resources.")
-        sources = [source.to_dict() for source in BootSource.objects.all()]
-        sources = write_all_keyrings(sources)
-
-        image_descriptions = download_all_image_descriptions(sources)
-        if image_descriptions.is_empty():
-            maaslog.warn(
-                "Unable to import boot resources, no image "
-                "descriptions avaliable.")
-            return
-        product_mapping = map_products(image_descriptions)
-
-        download_all_boot_resources(sources, product_mapping)
-        maaslog.info("Finished importing of boot resources.")
+        variables = {
+            'GNUPGHOME': MAAS_USER_GPGHOME,
+            }
+        with environment_variables(variables):
+            maaslog.info("Started importing of boot resources.")
+            sources = [source.to_dict() for source in BootSource.objects.all()]
+            sources = write_all_keyrings(sources)
+
+            image_descriptions = download_all_image_descriptions(sources)
+            if image_descriptions.is_empty():
+                maaslog.warn(
+                    "Unable to import boot resources, no image "
+                    "descriptions avaliable.")
+                return
+            product_mapping = map_products(image_descriptions)
+
+            download_all_boot_resources(sources, product_mapping)
+            maaslog.info("Finished importing of boot resources.")
     finally:
         kill_event.set()
         lock_thread.join()

=== modified file 'src/maasserver/start_up.py'
--- src/maasserver/start_up.py	2014-08-15 09:44:42 +0000
+++ src/maasserver/start_up.py	2014-08-27 12:28:22 +0000
@@ -38,6 +38,7 @@
     BootImage,
     NodeGroup,
     )
+from provisioningserver.upgrade_cluster import create_gnupg_home
 
 
 def start_up():
@@ -88,6 +89,11 @@
     # This must be serialized or we may initialize the master more than once.
     NodeGroup.objects.ensure_master()
 
+    # Make sure that maas user's GNUPG home directory exists. This is needed
+    # for importing of boot resources, that now occurs on the region as well
+    # as the clusters.
+    create_gnupg_home()
+
     # If no boot-source definitions yet, create the default definition.
     ensure_boot_source_definition()
 

=== modified file 'src/maasserver/tests/test_bootresources.py'
--- src/maasserver/tests/test_bootresources.py	2014-08-25 14:19:51 +0000
+++ src/maasserver/tests/test_bootresources.py	2014-08-27 12:28:22 +0000
@@ -16,6 +16,7 @@
 
 import httplib
 import json
+from os import environ
 from random import randint
 from StringIO import StringIO
 
@@ -1010,3 +1011,26 @@
         self.assertThat(
             fake_download_all_boot_resources,
             MockCalledOnceWith(sentinel.sources, sentinel.mapping))
+
+    def test__import_resources_has_env_GNUPGHOME_set(self):
+        fake_image_descriptions = self.patch(
+            bootresources, 'download_all_image_descriptions')
+        descriptions = Mock()
+        descriptions.is_empty.return_value = False
+        fake_image_descriptions.return_value = descriptions
+        self.patch(bootresources, 'map_products')
+
+        class CaptureEnv:
+            """Fake function; records a copy of the environment."""
+
+            def __call__(self, *args, **kwargs):
+                self.args = args
+                self.env = environ.copy()
+
+        fake_download = self.patch(
+            bootresources, 'download_all_boot_resources', CaptureEnv())
+
+        bootresources._import_resources(force=True)
+        self.assertEqual(
+            bootresources.MAAS_USER_GPGHOME,
+            fake_download.env['GNUPGHOME'])

=== modified file 'src/maasserver/tests/test_start_up.py'
--- src/maasserver/tests/test_start_up.py	2014-08-15 03:33:51 +0000
+++ src/maasserver/tests/test_start_up.py	2014-08-27 12:28:22 +0000
@@ -66,6 +66,7 @@
     def setUp(self):
         super(TestStartUp, self).setUp()
         self.useFixture(RegionEventLoopFixture())
+        self.patch(start_up, 'create_gnupg_home')
 
     def tearDown(self):
         super(TestStartUp, self).tearDown()
@@ -93,6 +94,11 @@
         ('celery', FixtureResource(CeleryFixture())),
         )
 
+    def setUp(self):
+        super(TestInnerStartUp, self).setUp()
+        self.mock_create_gnupg_home = self.patch(
+            start_up, 'create_gnupg_home')
+
     def test__calls_write_full_dns_config(self):
         recorder = FakeMethod()
         self.patch(start_up, 'write_full_dns_config', recorder)
@@ -107,6 +113,10 @@
         self.assertThat(clusters, HasLength(1))
         self.assertItemsEqual([NodeGroup.objects.ensure_master()], clusters)
 
+    def test__calls_create_gnupg_home(self):
+        start_up.inner_start_up()
+        self.assertThat(self.mock_create_gnupg_home, MockCalledOnceWith())
+
     def test__initialises_boot_source_config(self):
         self.assertItemsEqual([], BootSource.objects.all())
         start_up.inner_start_up()