Merge lp:~sil2100/cupstream2distro/fix_ppa_publish into lp:cupstream2distro

[Łukasz Zemczak]

When checking user upload permissions, check all of it regarding the main_archive - also get the component name from the archive itself. This has some implications, since we don't check upload permissions to the dest archive but take everything through the eyes of the archive.

When checking user upload permissions, check all of it regarding the main_archive - also get the component name from the archive itself. This has some implications, since we don't check upload permissions to the dest archive but take everything through the eyes of the archive.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1099
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/788/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/788/rebuild

review: Approve (continuous-integration)

[lp:~sil2100/cupstream2distro/fix_ppa_publish]

1100. By Łukasz Zemczak on 2015-09-16

Change of approach. As per Colin's recommendations, we need to check all permissions as per the main archive.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1100
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/789/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/789/rebuild

review: Approve (continuous-integration)

[Łukasz Zemczak (sil2100)]

Will test that in staging tomorrow.

[Łukasz Zemczak (sil2100)]

Looks correct on staging, merging ;)

=== modified file 'citrain/recipes/base.py'
--- citrain/recipes/base.py	2015-09-14 06:52:53 +0000
+++ citrain/recipes/base.py	2015-09-16 15:37:22 +0000
@@ -296,7 +296,7 @@
 
     def checkupload_phase(self):
         """Confirm that the silo publisher has upload rights."""
-        sourcepub = self.dest.getPublishedSources(
+        sourcepub = self.main_archive.getPublishedSources(
             distro_series=self.series,
             source_name=self.name,
             status='Published',
@@ -315,7 +315,7 @@
             kwargs.update(
                 component=sourcepub[0].component_name,
                 sourcepackagename=self.name)
-        log_value_of.kwargs('self.dest.checkUpload')
+        log_value_of.kwargs('self.main_archive.checkUpload')
         try:
             self.main_archive.checkUpload(**kwargs)
             self.authorized = True

=== modified file 'tests/unit/test_recipe_base.py'
--- tests/unit/test_recipe_base.py	2015-09-14 06:52:53 +0000
+++ tests/unit/test_recipe_base.py	2015-09-16 15:37:22 +0000
@@ -428,9 +428,9 @@
         lp_mock.people = dict(robru=robru)
         build = BuildBase('qt', self.series, self.dest, self.ppa)
         build.blame = robru
-        build.dest.getPublishedSources.return_value = [src]
+        build.main_archive.getPublishedSources.return_value = [src]
         build.checkupload_phase()
-        build.dest.getPublishedSources.assert_called_once_with(
+        build.main_archive.getPublishedSources.assert_called_once_with(
             distro_series=self.series,
             status='Published',
             pocket='Release',
@@ -452,9 +452,9 @@
         lp_mock.people = dict(robru=robru)
         build = BuildBase('qt', self.series, self.dest, self.ppa)
         build.blame = robru
-        build.dest.getPublishedSources.return_value = None
+        build.main_archive.getPublishedSources.return_value = None
         build.checkupload_phase()
-        build.dest.getPublishedSources.assert_called_once_with(
+        build.main_archive.getPublishedSources.assert_called_once_with(
             distro_series=self.series,
             status='Published',
             pocket='Release',
@@ -477,11 +477,25 @@
         lp_mock.people = dict(robru=robru)
         build = BuildBase('qt', self.series, self.dest, self.ppa)
         build.blame = Mock()
-        build.dest.getPublishedSources.return_value = [src]
+        build.main_archive.getPublishedSources.return_value = [src]
         build.main_archive.checkUpload.side_effect = ClientError(None, None)
         build.checkupload_phase()
         self.assertFalse(build.authorized)
 
+    @patch('citrain.recipes.base.lp')
+    def test_buildbase_checkupload_phase_non_archive(self, lp_mock):
+        """Ensure that we check archive permissions, even for PPAs."""
+        src = Mock()
+        robru = Mock()
+        env.BUILD_USER_ID = 'robru'
+        lp_mock.people = dict(robru=robru)
+        build = BuildBase('qt', self.series, self.dest, self.ppa)
+        build.blame = Mock()
+        build.main_archive.getPublishedSources.return_value = [src]
+        build.dest.checkUpload.side_effect = ClientError(None, None)
+        build.checkupload_phase()
+        self.assertTrue(build.authorized)
+
     def test_buildbase_ackaging_phase_authorized_success(self):
         """Ensure that authorized people can ACK."""
         env.ACK_PACKAGING = 'true'