launchpad-reviewers team mailing list archive

[Maximiliano Bertacchini <maximiliano.bertacchini@xxxxxxxxxxxxx>]

Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/snap-name-extraction into lp:launchpad with lp:~cjwatson/launchpad/snap-store-add-edit-views as a prerequisite.

Commit message:
Extract Snap.store_name from snapcraft.yaml (Git only).

Requested reviews:
  Colin Watson (cjwatson)

For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/snap-name-extraction/+merge/295114

Extract Snap.store_name from snapcraft.yaml (Git only).
-- 
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py	2016-05-18 18:02:43 +0000
+++ lib/lp/snappy/browser/snap.py	2016-05-18 18:02:43 +0000
@@ -26,6 +26,7 @@
     )
 from pymacaroons import Macaroon
 import transaction
+import yaml
 from zope.component import getUtility
 from zope.interface import Interface
 from zope.schema import (
@@ -53,12 +54,14 @@
     LaunchpadRadioWidget,
     )
 from lp.code.browser.widgets.gitref import GitRefWidget
+from lp.code.errors import GitRepositoryScanFault
 from lp.code.interfaces.gitref import IGitRef
 from lp.registry.enums import VCSType
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.features import getFeatureFlag
 from lp.services.helpers import english_list
 from lp.services.openid.adapters.openid import CurrentOpenIDEndPoint
+from lp.services.scripts import log
 from lp.services.webapp import (
     canonical_url,
     ContextMenu,
@@ -357,12 +360,28 @@
 
     @property
     def initial_values(self):
+        store_name = None
+        if self.has_snappy_series and IGitRef.providedBy(self.context):
+            # Try to extract Snap store name from snapcraft.yaml file.
+            try:
+                blob = self.context.repository.getBlob(
+                    'snapcraft.yaml', self.context.name)
+                # Beware of unsafe yaml.load()!
+                store_name = yaml.safe_load(blob).get('name')
+            except GitRepositoryScanFault:
+                log.info("Failed to get Snap manifest from Git %s",
+                          self.context.unique_name)
+            except Exception:
+                log.debug("Failed to parse Snap manifest at %s",
+                          self.context.unique_name)
+
         # XXX cjwatson 2015-09-18: Hack to ensure that we don't end up
         # accidentally selecting ubuntu-rtm/14.09 or similar.
         # ubuntu.currentseries will always be in BuildableDistroSeries.
         series = getUtility(ILaunchpadCelebrities).ubuntu.currentseries
         sds_set = getUtility(ISnappyDistroSeriesSet)
         return {
+            'store_name': store_name,
             'owner': self.user,
             'store_distro_series': sds_set.getByDistroSeries(series).first(),
             }

=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py	2016-05-18 18:02:43 +0000
+++ lib/lp/snappy/browser/tests/test_snap.py	2016-05-18 18:02:43 +0000
@@ -21,6 +21,7 @@
     HTTMock,
     )
 from mechanize import LinkNotFoundError
+import mock
 from pymacaroons import Macaroon
 import pytz
 import soupmatchers
@@ -29,6 +30,7 @@
     MatchesStructure,
     )
 import transaction
+import yaml
 from zope.component import getUtility
 from zope.publisher.interfaces import NotFound
 from zope.security.interfaces import Unauthorized
@@ -37,6 +39,7 @@
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.processor import IProcessorSet
+from lp.code.errors import GitRepositoryScanFault
 from lp.registry.enums import PersonVisibility
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
@@ -70,6 +73,7 @@
     TestCaseWithFactory,
     time_counter,
     )
+from lp.testing.fakemethod import FakeMethod
 from lp.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
@@ -81,8 +85,8 @@
 from lp.testing.pages import (
     extract_text,
     find_main_content,
+    find_tag_by_id,
     find_tags_by_class,
-    find_tag_by_id,
     )
 from lp.testing.publication import test_traverse
 from lp.testing.views import (
@@ -350,6 +354,41 @@
             "discharge_macaroon_field=field.discharge_macaroon",
             redirection.hdrs["Location"])
 
+    def test_initial_name_extraction_git(self):
+        [git_ref] = self.factory.makeGitRefs()
+        git_ref.repository.getBlob = FakeMethod(result='name: test-snap')
+        view = create_initialized_view(git_ref, "+new-snap")
+        initial_values = view.initial_values
+        self.assertIn('store_name', initial_values)
+        self.assertEqual('test-snap', initial_values['store_name'])
+
+    def test_initial_name_extraction_git_repo_error(self):
+        [git_ref] = self.factory.makeGitRefs()
+        git_ref.repository.getBlob = FakeMethod(failure=GitRepositoryScanFault)
+        view = create_initialized_view(git_ref, "+new-snap")
+        initial_values = view.initial_values
+        self.assertIn('store_name', initial_values)
+        self.assertIsNone(initial_values['store_name'])
+
+    def test_initial_name_extraction_git_invalid_data(self):
+        for invalid_result in (None, 123, '', '[][]', '#name:test', ']'):
+            [git_ref] = self.factory.makeGitRefs()
+            git_ref.repository.getBlob = FakeMethod(result=invalid_result)
+            view = create_initialized_view(git_ref, "+new-snap")
+            initial_values = view.initial_values
+            self.assertIn('store_name', initial_values)
+            self.assertIsNone(initial_values['store_name'])
+
+    def test_initial_name_extraction_git_safe_yaml(self):
+        [git_ref] = self.factory.makeGitRefs()
+        git_ref.repository.getBlob = FakeMethod(result='Malicious YAML!')
+        view = create_initialized_view(git_ref, "+new-snap")
+        with mock.patch('yaml.load') as unsafe_load:
+            with mock.patch('yaml.safe_load') as safe_load:
+                initial_values = view.initial_values
+        self.assertEqual(0, unsafe_load.call_count)
+        self.assertEqual(1, safe_load.call_count)
+
 
 class TestSnapAdminView(BrowserTestCase):
 

=== modified file 'setup.py'
--- setup.py	2016-05-18 18:02:43 +0000
+++ setup.py	2016-05-18 18:02:43 +0000
@@ -83,6 +83,7 @@
         'python-memcached',
         'python-openid',
         'pytz',
+        'PyYAML',
         'rabbitfixture',
         'requests',
         'requests-toolbelt',