Merge ~cjwatson/launchpad:code-import-scheduler-blacklisted-hostnames into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: da3848f9dfc5c30fc4c1c6514daba744df748dfb
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:code-import-scheduler-blacklisted-hostnames
Merge into: launchpad:master
Diff against target: 92 lines (+37/-5)
2 files modified
lib/lp/code/model/codeimportjob.py (+11/-1)
lib/lp/code/model/tests/test_codeimportjob.py (+26/-4)
Reviewer Review Type Date Requested Status
Thiago F. Pappacena (community) Approve
Review via email: mp+393732@code.launchpad.net

Commit message

Pass --exclude-host options to code import workers

Description of the change

This is equivalent to the policy currently implemented in CodeImportBranchOpenPolicy.check_one_url, but requires less tight coupling between the code import workers and the rest of Launchpad.

To post a comment you must log in.
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/model/codeimportjob.py b/lib/lp/code/model/codeimportjob.py
2index fa81587..3afe6b4 100644
3--- a/lib/lp/code/model/codeimportjob.py
4+++ b/lib/lp/code/model/codeimportjob.py
5@@ -36,7 +36,10 @@ from lp.code.enums import (
6 GitRepositoryType,
7 RevisionControlSystems,
8 )
9-from lp.code.interfaces.branch import IBranch
10+from lp.code.interfaces.branch import (
11+ get_blacklisted_hostnames,
12+ IBranch,
13+ )
14 from lp.code.interfaces.codehosting import (
15 branch_id_alias,
16 compose_public_url,
17@@ -165,6 +168,13 @@ class CodeImportJob(StormBase):
18 # XXX cjwatson 2016-10-12: Consider arranging for this to be
19 # passed to worker processes in the environment instead.
20 result.extend(['--macaroon', macaroon.serialize()])
21+ # Refuse pointless self-mirroring.
22+ if rcs_type == target_rcs_type:
23+ result.extend(['--exclude-host', config.vhost.mainsite.hostname])
24+ # Refuse to import from configured hostnames, typically localhost
25+ # and similar.
26+ for hostname in get_blacklisted_hostnames():
27+ result.extend(['--exclude-host', hostname])
28 return result
29
30 def destroySelf(self):
31diff --git a/lib/lp/code/model/tests/test_codeimportjob.py b/lib/lp/code/model/tests/test_codeimportjob.py
32index 14ea8f2..0bd2ebc 100644
33--- a/lib/lp/code/model/tests/test_codeimportjob.py
34+++ b/lib/lp/code/model/tests/test_codeimportjob.py
35@@ -129,7 +129,9 @@ class TestCodeImportJob(TestCaseWithFactory):
36 self.assertArgumentsMatch(
37 code_import, Equals([
38 str(code_import.branch.id), 'bzr', 'bzr',
39- 'http://example.com/foo']))
40+ 'http://example.com/foo',
41+ '--exclude-host', 'launchpad.test',
42+ ]))
43
44 def test_git_arguments(self):
45 code_import = self.factory.makeCodeImport(
46@@ -152,7 +154,9 @@ class TestCodeImportJob(TestCaseWithFactory):
47 Equals('git'), Equals('git'),
48 Equals('git://git.example.com/project.git'),
49 Equals('--macaroon'),
50- CodeImportJobMacaroonVerifies(code_import)]),
51+ CodeImportJobMacaroonVerifies(code_import),
52+ Equals('--exclude-host'), Equals('launchpad.test'),
53+ ]),
54 # Start the job so that the macaroon can be verified.
55 start_job=True)
56
57@@ -183,7 +187,9 @@ class TestCodeImportJob(TestCaseWithFactory):
58 str(code_import.branch.id), 'bzr', 'bzr',
59 'bzr://bzr.example.com/foo',
60 '--stacked-on',
61- compose_public_url('http', branch_id_alias(devfocus))]))
62+ compose_public_url('http', branch_id_alias(devfocus)),
63+ '--exclude-host', 'launchpad.test',
64+ ]))
65
66 def test_bzr_stacked_private(self):
67 # Code imports can't be stacked on private branches.
68@@ -196,7 +202,23 @@ class TestCodeImportJob(TestCaseWithFactory):
69 branch.stacked_on = devfocus
70 self.assertArgumentsMatch(
71 code_import, Equals([
72- str(branch.id), 'bzr', 'bzr', 'bzr://bzr.example.com/foo']))
73+ str(branch.id), 'bzr', 'bzr', 'bzr://bzr.example.com/foo',
74+ '--exclude-host', 'launchpad.test',
75+ ]))
76+
77+ def test_blacklisted_hostnames(self):
78+ # Additional blacklisted hostnames are passed as --exclude-host
79+ # options.
80+ self.pushConfig(
81+ 'codehosting', blacklisted_hostnames='localhost,127.0.0.1')
82+ code_import = self.factory.makeCodeImport(
83+ git_repo_url="git://git.example.com/project.git")
84+ self.assertArgumentsMatch(
85+ code_import, Equals([
86+ str(code_import.branch.id), 'git', 'bzr',
87+ 'git://git.example.com/project.git',
88+ '--exclude-host', 'localhost', '--exclude-host', '127.0.0.1',
89+ ]))
90
91
92 class TestCodeImportJobSet(TestCaseWithFactory):

Subscribers

People subscribed via source and target branches

to status/vote changes: