launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/import-from-lp-git into lp:launchpad.

Commit message:
Allow code imports from Launchpad Git to Launchpad Bazaar.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/import-from-lp-git/+merge/261856

Allow code imports from Launchpad Git to Launchpad Bazaar.

We'll probably need some firewall tweaks as well, but I believe this handles the Launchpad side of things.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/import-from-lp-git into lp:launchpad.

=== modified file 'lib/lp/code/browser/codeimport.py'
--- lib/lp/code/browser/codeimport.py	2013-04-11 04:54:04 +0000
+++ lib/lp/code/browser/codeimport.py	2015-06-12 14:36:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Browser views for CodeImports."""
@@ -194,14 +194,15 @@
                     canonical_url(code_import.branch),
                     code_import.branch.unique_name))
 
-    def _validateURL(self, url, existing_import=None, field_name='url'):
+    def _validateURL(self, url, rcs_type, existing_import=None,
+                     field_name='url'):
         """If the user has specified a url, we need to make sure that there
         isn't already an import with that url."""
         if url is None:
             self.setSecondaryFieldError(
                 field_name, 'Enter the URL of a foreign VCS branch.')
         else:
-            reason = validate_import_url(url, existing_import)
+            reason = validate_import_url(url, rcs_type, existing_import)
             if reason:
                 self.setFieldError(field_name, reason)
 
@@ -435,13 +436,15 @@
             self._validateCVS(data.get('cvs_root'), data.get('cvs_module'))
         elif rcs_type == RevisionControlSystems.BZR_SVN:
             self._validateURL(
-                data.get('svn_branch_url'), field_name='svn_branch_url')
+                data.get('svn_branch_url'), rcs_type,
+                field_name='svn_branch_url')
         elif rcs_type == RevisionControlSystems.GIT:
             self._validateURL(
-                data.get('git_repo_url'), field_name='git_repo_url')
+                data.get('git_repo_url'), rcs_type, field_name='git_repo_url')
         elif rcs_type == RevisionControlSystems.BZR:
             self._validateURL(
-                data.get('bzr_branch_url'), field_name='bzr_branch_url')
+                data.get('bzr_branch_url'), rcs_type,
+                field_name='bzr_branch_url')
         else:
             raise AssertionError(
                 'Unexpected revision control type %r.' % rcs_type)
@@ -565,7 +568,8 @@
                 data.get('cvs_root'), data.get('cvs_module'),
                 self.code_import)
         elif self.code_import.rcs_type in NON_CVS_RCS_TYPES:
-            self._validateURL(data.get('url'), self.code_import)
+            self._validateURL(
+                data.get('url'), self.code_import.rcs_type, self.code_import)
         else:
             raise AssertionError('Unknown rcs_type for code import.')
 
@@ -581,11 +585,14 @@
         return getUtility(ICodeImportMachineSet).getAll()
 
 
-def validate_import_url(url, existing_import=None):
+def validate_import_url(url, rcs_type, existing_import=None):
     """Validate the given import URL."""
-    if urlparse(url).netloc.endswith('launchpad.net'):
+    # XXX cjwatson 2015-06-12: Once we have imports into Git, this should be
+    # extended to prevent Git-to-Git self-imports as well.
+    if (rcs_type == RevisionControlSystems.BZR and
+            urlparse(url).netloc.endswith('launchpad.net')):
         return (
-            "You can not create imports for branches that are hosted by "
+            "You cannot create imports for Bazaar branches that are hosted by "
             "Launchpad.")
     code_import = getUtility(ICodeImportSet).getByURL(url)
     if code_import is not None:

=== modified file 'lib/lp/code/stories/codeimport/xx-create-codeimport.txt'
--- lib/lp/code/stories/codeimport/xx-create-codeimport.txt	2013-09-27 04:13:23 +0000
+++ lib/lp/code/stories/codeimport/xx-create-codeimport.txt	2015-06-12 14:36:19 +0000
@@ -97,8 +97,8 @@
     at http://user:password@xxxxxxxxxxxxxxx/firefox/trunk.
     The next import is scheduled to run
     as soon as possible.
-    
-Specifing a Launchpad URL results in an error.
+
+Specifying a Launchpad URL results in an error.
 
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
     >>> browser.getControl('Branch Name').value = "invalid"
@@ -108,7 +108,23 @@
     >>> browser.getControl('Request Import').click()
     >>> print_feedback_messages(browser.contents)
     There is 1 error.
-    You can not create imports for branches that are hosted by Launchpad.
+    You cannot create imports for Bazaar branches that are hosted by
+    Launchpad.
+
+But a Launchpad Git URL is OK.
+
+    >>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
+    >>> browser.getControl('Project').value = "firefox"
+    >>> browser.getControl('Branch Name').value = "lp-git-import"
+    >>> browser.getControl('Git').click()
+    >>> browser.getControl('Repo URL', index=0).value = (
+    ...     "git://git.launchpad.net/firefox.git")
+    >>> browser.getControl('Request Import').click()
+    >>> print extract_text(find_tag_by_id(browser.contents, "import-details"))
+    Import Status: Reviewed
+    This branch is an import of the HEAD branch of the Git repository at
+    git://git.launchpad.net/firefox.git.
+    The next import is scheduled to run as soon as possible.
 
 Requesting a Subversion import
 ==============================

=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py	2014-02-24 07:19:52 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py	2015-06-12 14:36:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the code import worker."""
@@ -1046,7 +1046,7 @@
         worker = self.makeImportWorker(
             self.factory.makeCodeImportSourceDetails(
                 rcstype=self.rcstype, url="file:///local/path"),
-            opener_policy=CodeImportBranchOpenPolicy())
+            opener_policy=CodeImportBranchOpenPolicy("bzr"))
         self.assertEqual(
             CodeImportWorkerExitCode.FAILURE_FORBIDDEN, worker.run())
 
@@ -1282,7 +1282,7 @@
 
     def setUp(self):
         super(CodeImportBranchOpenPolicyTests, self).setUp()
-        self.policy = CodeImportBranchOpenPolicy()
+        self.policy = CodeImportBranchOpenPolicy("bzr")
 
     def test_follows_references(self):
         self.assertEquals(True, self.policy.shouldFollowReferences())
@@ -1303,8 +1303,16 @@
         self.assertGoodUrl("http://svn.example/branches/trunk";)
         self.assertGoodUrl("http://user:password@svn.example/branches/trunk";)
         self.assertBadUrl("svn+ssh://svn.example.com/bla")
+        self.assertGoodUrl("bzr://bzr.example.com/somebzrurl/")
+        self.assertBadUrl("bzr://bazaar.launchpad.dev/example")
+
+    def test_checkOneURL_git(self):
+        self.policy = CodeImportBranchOpenPolicy("git")
+        self.assertBadUrl("/etc/passwd")
+        self.assertBadUrl("file:///etc/passwd")
+        self.assertBadUrl("unknown-scheme://devpad/")
         self.assertGoodUrl("git://git.example.com/repo")
-        self.assertGoodUrl("bzr://bzr.example.com/somebzrurl/")
+        self.assertGoodUrl("git://git.launchpad.dev/example")
 
 
 class RedirectTests(http_utils.TestCaseWithRedirectedWebserver, TestCase):

=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py	2014-02-24 07:19:52 +0000
+++ lib/lp/codehosting/codeimport/worker.py	2015-06-12 14:36:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """The code import worker. This imports code from foreign repositories."""
@@ -87,12 +87,15 @@
 
     In summary:
      - follow references,
-     - only open non-Launchpad URLs
+     - only open non-Launchpad URLs for imports from Bazaar
      - only open the allowed schemes
     """
 
     allowed_schemes = ['http', 'https', 'svn', 'git', 'ftp', 'bzr']
 
+    def __init__(self, rcstype):
+        self.rcstype = rcstype
+
     def shouldFollowReferences(self):
         """See `BranchOpenPolicy.shouldFollowReferences`.
 
@@ -113,15 +116,19 @@
     def checkOneURL(self, url):
         """See `BranchOpenPolicy.checkOneURL`.
 
-        We refuse to mirror from Launchpad or a ssh-like or file URL.
+        We refuse to mirror Bazaar branches from Launchpad, or any branches
+        from a ssh-like or file URL.
         """
         try:
             uri = URI(url)
         except InvalidURIError:
             raise BadUrl(url)
-        launchpad_domain = config.vhost.mainsite.hostname
-        if uri.underDomain(launchpad_domain):
-            raise BadUrl(url)
+        # XXX cjwatson 2015-06-12: Once we have imports into Git, this
+        # should be extended to prevent Git-to-Git self-imports as well.
+        if self.rcstype == "bzr":
+            launchpad_domain = config.vhost.mainsite.hostname
+            if uri.underDomain(launchpad_domain):
+                raise BadUrl(url)
         for hostname in get_blacklisted_hostnames():
             if uri.underDomain(hostname):
                 raise BadUrl(url)

=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py	2014-12-06 00:16:55 +0000
+++ lib/lp/registry/browser/productseries.py	2015-06-12 14:36:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """View classes for `IProductSeries`."""
@@ -885,7 +885,7 @@
             self.setFieldError(
                 'repo_url', 'You must set the external repository URL.')
         else:
-            reason = validate_import_url(repo_url)
+            reason = validate_import_url(repo_url, rcs_type)
             if reason:
                 self.setFieldError('repo_url', reason)
 

=== modified file 'lib/lp/registry/browser/tests/productseries-setbranch-view.txt'
--- lib/lp/registry/browser/tests/productseries-setbranch-view.txt	2013-04-11 01:33:23 +0000
+++ lib/lp/registry/browser/tests/productseries-setbranch-view.txt	2015-06-12 14:36:19 +0000
@@ -161,8 +161,8 @@
     blazer-branch
     >>> series.branch.registrant.name == driver.name
     True
-    
-External imports can not use an Launchpad URL.
+
+External Bazaar imports can not use an Launchpad URL.
 
     >>> form['field.repo_url'] = 'http://bazaar.launchpad.net/firefox/foo'
     >>> form['field.branch_name'] = 'chevette-branch'
@@ -171,9 +171,26 @@
     >>> transaction.commit()
     >>> for error in view.errors:
     ...     print error
-    You can not create imports for branches that are hosted by Launchpad.
-
-Git branches cannnot use svn.
+    You cannot create imports for Bazaar branches that are hosted by
+    Launchpad.
+
+Git imports can use a Launchpad URL.
+
+    >>> form['field.repo_url'] = 'https://git.launchpad.net/blazer'
+    >>> form['field.rcs_type'] = 'GIT'
+    >>> form['field.branch_name'] = 'blazer-git-branch'
+    >>> view = create_initialized_view(
+    ...     series, name='+setbranch', principal=driver, form=form)
+    >>> transaction.commit()
+    >>> for error in view.errors:
+    ...     print error
+    >>> for notification in view.request.response.notifications:
+    ...     print notification.message
+    Code import created and branch linked to the series.
+    >>> print series.branch.name
+    blazer-git-branch
+
+Git branches cannot use svn.
 
     >>> form = {
     ...     'field.branch_type': 'import-external',

=== modified file 'scripts/code-import-worker.py'
--- scripts/code-import-worker.py	2014-02-24 07:19:52 +0000
+++ scripts/code-import-worker.py	2015-06-12 14:36:19 +0000
@@ -1,6 +1,6 @@
 #!/usr/bin/python -S
 #
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Process a code import described by the command line arguments.
@@ -37,8 +37,8 @@
 
 
 opener_policies = {
-    "anything": AcceptAnythingPolicy(),
-    "default": CodeImportBranchOpenPolicy()
+    "anything": lambda rcstype: AcceptAnythingPolicy(),
+    "default": CodeImportBranchOpenPolicy,
     }
 
 
@@ -82,11 +82,12 @@
         else:
             raise AssertionError(
                 'unknown rcstype %r' % source_details.rcstype)
+        opener_policy = opener_policies[self.options.access_policy](
+            source_details.rcstype)
         import_worker = import_worker_cls(
             source_details,
             get_transport(config.codeimport.foreign_tree_store),
-            get_default_bazaar_branch_store(), self.logger,
-            opener_policies[self.options.access_policy])
+            get_default_bazaar_branch_store(), self.logger, opener_policy)
         return import_worker.run()