Launchpad itself

Merge ~ilasc/launchpad:change-permissions-builder-reset into launchpad:master

  1. Git
  2. lp:~ilasc/launchpad
  3. change-permissions-builder-reset
  4. Merge into master
Proposed by Ioana Lasc
Status: Merged
Approved by: Ioana Lasc
Approved revision: 583fff76fb66f86a2d90d9b75b9a638fdbeaad9b
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~ilasc/launchpad:change-permissions-builder-reset
Merge into: launchpad:master
Diff against target: 181 lines (+69/-19)
4 files modified
lib/lp/buildmaster/configure.zcml (+5/-1)
lib/lp/buildmaster/interfaces/builder.py (+18/-14)
lib/lp/buildmaster/tests/test_webservice.py (+37/-4)
lib/lp/security.py (+9/-0)
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+393716@code.launchpad.net

Commit message

Change permissions required to reset builders

Description of the change

This branch changes permissions required to reset builders to:

     ~admins and ~launchpad-buildd-admins can edit the entire set of attributes on IBuilderView
     ~registry team members (registry experts) can edit only 3 attributes on builder: builderok, manual and failnotes

These will be exposed in browser via the "Change details" link on the Edit Builder view.

The branch also introduces unit test for the builder view (they're currently doctests).

To post a comment you must log in.
Revision history for this message
Ioana Lasc (ilasc) wrote :

Hit enter too fast on above comment.

The branch is work in progress at this point, open for visibility and direction review only.

I'm working backwards: first granting access to the set of 3 attributes on the Edit view to the 3 teams (~admins, ~launchpad-buildd-aminds, ~registry).

The next step would be to expose the full set of attributes but only for 2 teams: ~admins and ~launchpad-buildd-admins.

Revision history for this message
Colin Watson (cjwatson) :
~ilasc/launchpad:change-permissions-builder-reset updated
aa19116... by Ioana Lasc

Address code review comments

73a7137... by Ioana Lasc

Add test for manual and failnotes fields

Revision history for this message
Ioana Lasc (ilasc) wrote :

Thanks Colin!
Code review comments addressed and MP ready for another review.

Revision history for this message
Colin Watson (cjwatson) :
review: Approve
~ilasc/launchpad:change-permissions-builder-reset updated
583fff7... by Ioana Lasc

Swap IBuilderModerateAttributes for IBuilder in security.py

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/buildmaster/configure.zcml b/lib/lp/buildmaster/configure.zcml
2index 3d34cb1..4b35965 100644
3--- a/lib/lp/buildmaster/configure.zcml
4+++ b/lib/lp/buildmaster/configure.zcml
5@@ -35,11 +35,15 @@
6 <class
7 class="lp.buildmaster.model.builder.Builder">
8 <allow
9- interface="lp.buildmaster.interfaces.builder.IBuilderView"/>
10+ interface="lp.buildmaster.interfaces.builder.IBuilderView
11+ lp.buildmaster.interfaces.builder.IBuilderModerateAttributes"/>
12 <require
13 permission="launchpad.Edit"
14 interface="lp.buildmaster.interfaces.builder.IBuilderEdit"
15 set_schema="lp.buildmaster.interfaces.builder.IBuilderView"/>
16+ <require
17+ permission="launchpad.Moderate"
18+ set_schema="lp.buildmaster.interfaces.builder.IBuilderModerateAttributes"/>
19 </class>
20
21
22diff --git a/lib/lp/buildmaster/interfaces/builder.py b/lib/lp/buildmaster/interfaces/builder.py
23index 6be226f..a827192 100644
24--- a/lib/lp/buildmaster/interfaces/builder.py
25+++ b/lib/lp/buildmaster/interfaces/builder.py
26@@ -13,6 +13,7 @@ __all__ = [
27 'CannotFetchFile',
28 'CannotResumeHost',
29 'IBuilder',
30+ 'IBuilderModerateAttributes',
31 'IBuilderSet',
32 ]
33
34@@ -98,6 +99,22 @@ class BuildSlaveFailure(BuildDaemonError):
35 """The build slave has suffered an error and cannot be used."""
36
37
38+class IBuilderModerateAttributes(Interface):
39+ manual = exported(Bool(
40+ title=_('Manual Mode'), required=False, default=False,
41+ description=_('The auto-build system does not dispatch '
42+ 'jobs automatically for slaves in manual mode.')))
43+
44+ builderok = exported(Bool(
45+ title=_('Builder State OK'), required=True, default=True,
46+ description=_('Whether or not the builder is ok')),
47+ as_of='devel')
48+
49+ failnotes = exported(Text(
50+ title=_('Failure Notes'), required=False,
51+ description=_('The reason for a builder not being ok')))
52+
53+
54 class IBuilderView(IHasBuildRecords, IHasOwner):
55
56 id = Attribute("Builder identifier")
57@@ -146,19 +163,6 @@ class IBuilderView(IHasBuildRecords, IHasOwner):
58 description=_('Whether or not the builder is a virtual Xen '
59 'instance.')))
60
61- manual = exported(Bool(
62- title=_('Manual Mode'), required=False, default=False,
63- description=_('The auto-build system does not dispatch '
64- 'jobs automatically for slaves in manual mode.')))
65-
66- builderok = exported(Bool(
67- title=_('Builder State OK'), required=True, default=True,
68- description=_('Whether or not the builder is ok')))
69-
70- failnotes = exported(Text(
71- title=_('Failure Notes'), required=False,
72- description=_('The reason for a builder not being ok')))
73-
74 vm_host = exported(TextLine(
75 title=_('VM host'), required=False,
76 description=_('The machine hostname hosting the virtual '
77@@ -220,7 +224,7 @@ class IBuilderEdit(Interface):
78
79
80 @exported_as_webservice_entry()
81-class IBuilder(IBuilderEdit, IBuilderView):
82+class IBuilder(IBuilderEdit, IBuilderView, IBuilderModerateAttributes):
83 """Build-slave information and state.
84
85 Builder instance represents a single builder slave machine within the
86diff --git a/lib/lp/buildmaster/tests/test_webservice.py b/lib/lp/buildmaster/tests/test_webservice.py
87index e51ca79..5534c09 100644
88--- a/lib/lp/buildmaster/tests/test_webservice.py
89+++ b/lib/lp/buildmaster/tests/test_webservice.py
90@@ -12,6 +12,7 @@ from json import dumps
91 from testtools.matchers import Equals
92 from zope.component import getUtility
93
94+from lp.buildmaster.interfaces.builder import IBuilderSet
95 from lp.registry.interfaces.person import IPersonSet
96 from lp.services.webapp import canonical_url
97 from lp.services.webapp.interfaces import OAuthPermission
98@@ -135,17 +136,21 @@ class TestBuilderEntry(TestCaseWithFactory):
99 self.webservice = LaunchpadWebServiceCaller()
100
101 def test_security(self):
102- # Attributes can only be set by buildd admins.
103+ # Most builder attributes can only be set by buildd admins.
104+ # We've introduced registry_experts privileges on 3 attributes
105+ # for builder reset, tested in next method.
106+
107 builder = self.factory.makeBuilder()
108 user = self.factory.makePerson()
109 user_webservice = webservice_for_person(
110 user, permission=OAuthPermission.WRITE_PUBLIC)
111- patch = dumps({'clean_status': 'Cleaning'})
112+ clean_status_patch = dumps({'clean_status': 'Cleaning'})
113 logout()
114
115 # A normal user is unauthorized.
116 response = user_webservice.patch(
117- api_url(builder), 'application/json', patch, api_version='devel')
118+ api_url(builder), 'application/json',
119+ clean_status_patch, api_version='devel')
120 self.assertEqual(401, response.status)
121
122 # But a buildd admin can set the attribute.
123@@ -154,10 +159,38 @@ class TestBuilderEntry(TestCaseWithFactory):
124 'launchpad-buildd-admins')
125 buildd_admins.addMember(user, buildd_admins.teamowner)
126 response = user_webservice.patch(
127- api_url(builder), 'application/json', patch, api_version='devel')
128+ api_url(builder), 'application/json',
129+ clean_status_patch, api_version='devel')
130 self.assertEqual(209, response.status)
131 self.assertEqual('Cleaning', response.jsonBody()['clean_status'])
132
133+ def test_security_builder_reset(self):
134+ builder = getUtility(IBuilderSet)['bob']
135+ person = self.factory.makePerson()
136+ user_webservice = webservice_for_person(
137+ person, permission=OAuthPermission.WRITE_PUBLIC)
138+ change_patch = dumps({'builderok': False, 'manual': False,
139+ 'failnotes': 'test notes'})
140+ logout()
141+
142+ # A normal user is unauthorized.
143+ response = user_webservice.patch(
144+ api_url(builder), 'application/json', change_patch,
145+ api_version='devel')
146+ self.assertEqual(401, response.status)
147+
148+ # But a registry expert can set the attributes.
149+ with admin_logged_in():
150+ reg_expert = getUtility(IPersonSet).getByName('registry')
151+ reg_expert.addMember(person, reg_expert)
152+ response = user_webservice.patch(
153+ api_url(builder), 'application/json', change_patch,
154+ api_version='devel')
155+ self.assertEqual(209, response.status)
156+ self.assertEqual(False, response.jsonBody()['builderok'])
157+ self.assertEqual(False, response.jsonBody()['manual'])
158+ self.assertEqual('test notes', response.jsonBody()['failnotes'])
159+
160 def test_exports_processor(self):
161 processor = self.factory.makeProcessor('s1')
162 builder = self.factory.makeBuilder(processors=[processor])
163diff --git a/lib/lp/security.py b/lib/lp/security.py
164index 243c1a8..459ab8c 100644
165--- a/lib/lp/security.py
166+++ b/lib/lp/security.py
167@@ -1975,6 +1975,15 @@ class EditBuilder(AdminByBuilddAdmin):
168 usedfor = IBuilder
169
170
171+class ModerateBuilder(EditBuilder):
172+ permission = 'launchpad.Moderate'
173+ usedfor = IBuilder
174+
175+ def checkAuthenticated(self, user):
176+ return (user.in_registry_experts or
177+ super(ModerateBuilder, self).checkAuthenticated(user))
178+
179+
180 class AdminBuildRecord(AdminByBuilddAdmin):
181 usedfor = IBuildFarmJob
182

Subscribers

People subscribed via source and target branches

to status/vote changes: