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
diff --git a/lib/lp/code/model/codeimportjob.py b/lib/lp/code/model/codeimportjob.py
index fa81587..3afe6b4 100644
--- a/lib/lp/code/model/codeimportjob.py
+++ b/lib/lp/code/model/codeimportjob.py
@@ -36,7 +36,10 @@ from lp.code.enums import (
36 GitRepositoryType,36 GitRepositoryType,
37 RevisionControlSystems,37 RevisionControlSystems,
38 )38 )
39from lp.code.interfaces.branch import IBranch39from lp.code.interfaces.branch import (
40 get_blacklisted_hostnames,
41 IBranch,
42 )
40from lp.code.interfaces.codehosting import (43from lp.code.interfaces.codehosting import (
41 branch_id_alias,44 branch_id_alias,
42 compose_public_url,45 compose_public_url,
@@ -165,6 +168,13 @@ class CodeImportJob(StormBase):
165 # XXX cjwatson 2016-10-12: Consider arranging for this to be168 # XXX cjwatson 2016-10-12: Consider arranging for this to be
166 # passed to worker processes in the environment instead.169 # passed to worker processes in the environment instead.
167 result.extend(['--macaroon', macaroon.serialize()])170 result.extend(['--macaroon', macaroon.serialize()])
171 # Refuse pointless self-mirroring.
172 if rcs_type == target_rcs_type:
173 result.extend(['--exclude-host', config.vhost.mainsite.hostname])
174 # Refuse to import from configured hostnames, typically localhost
175 # and similar.
176 for hostname in get_blacklisted_hostnames():
177 result.extend(['--exclude-host', hostname])
168 return result178 return result
169179
170 def destroySelf(self):180 def destroySelf(self):
diff --git a/lib/lp/code/model/tests/test_codeimportjob.py b/lib/lp/code/model/tests/test_codeimportjob.py
index 14ea8f2..0bd2ebc 100644
--- a/lib/lp/code/model/tests/test_codeimportjob.py
+++ b/lib/lp/code/model/tests/test_codeimportjob.py
@@ -129,7 +129,9 @@ class TestCodeImportJob(TestCaseWithFactory):
129 self.assertArgumentsMatch(129 self.assertArgumentsMatch(
130 code_import, Equals([130 code_import, Equals([
131 str(code_import.branch.id), 'bzr', 'bzr',131 str(code_import.branch.id), 'bzr', 'bzr',
132 'http://example.com/foo']))132 'http://example.com/foo',
133 '--exclude-host', 'launchpad.test',
134 ]))
133135
134 def test_git_arguments(self):136 def test_git_arguments(self):
135 code_import = self.factory.makeCodeImport(137 code_import = self.factory.makeCodeImport(
@@ -152,7 +154,9 @@ class TestCodeImportJob(TestCaseWithFactory):
152 Equals('git'), Equals('git'),154 Equals('git'), Equals('git'),
153 Equals('git://git.example.com/project.git'),155 Equals('git://git.example.com/project.git'),
154 Equals('--macaroon'),156 Equals('--macaroon'),
155 CodeImportJobMacaroonVerifies(code_import)]),157 CodeImportJobMacaroonVerifies(code_import),
158 Equals('--exclude-host'), Equals('launchpad.test'),
159 ]),
156 # Start the job so that the macaroon can be verified.160 # Start the job so that the macaroon can be verified.
157 start_job=True)161 start_job=True)
158162
@@ -183,7 +187,9 @@ class TestCodeImportJob(TestCaseWithFactory):
183 str(code_import.branch.id), 'bzr', 'bzr',187 str(code_import.branch.id), 'bzr', 'bzr',
184 'bzr://bzr.example.com/foo',188 'bzr://bzr.example.com/foo',
185 '--stacked-on',189 '--stacked-on',
186 compose_public_url('http', branch_id_alias(devfocus))]))190 compose_public_url('http', branch_id_alias(devfocus)),
191 '--exclude-host', 'launchpad.test',
192 ]))
187193
188 def test_bzr_stacked_private(self):194 def test_bzr_stacked_private(self):
189 # Code imports can't be stacked on private branches.195 # Code imports can't be stacked on private branches.
@@ -196,7 +202,23 @@ class TestCodeImportJob(TestCaseWithFactory):
196 branch.stacked_on = devfocus202 branch.stacked_on = devfocus
197 self.assertArgumentsMatch(203 self.assertArgumentsMatch(
198 code_import, Equals([204 code_import, Equals([
199 str(branch.id), 'bzr', 'bzr', 'bzr://bzr.example.com/foo']))205 str(branch.id), 'bzr', 'bzr', 'bzr://bzr.example.com/foo',
206 '--exclude-host', 'launchpad.test',
207 ]))
208
209 def test_blacklisted_hostnames(self):
210 # Additional blacklisted hostnames are passed as --exclude-host
211 # options.
212 self.pushConfig(
213 'codehosting', blacklisted_hostnames='localhost,127.0.0.1')
214 code_import = self.factory.makeCodeImport(
215 git_repo_url="git://git.example.com/project.git")
216 self.assertArgumentsMatch(
217 code_import, Equals([
218 str(code_import.branch.id), 'git', 'bzr',
219 'git://git.example.com/project.git',
220 '--exclude-host', 'localhost', '--exclude-host', '127.0.0.1',
221 ]))
200222
201223
202class TestCodeImportJobSet(TestCaseWithFactory):224class TestCodeImportJobSet(TestCaseWithFactory):

Subscribers

People subscribed via source and target branches

to status/vote changes: