Differential D249

HTTP 500 errors on incorrect bugzilla credential entry
ClosedPublic
Actions

Authored by mkrizek on Oct 15 2014, 10:57 AM.

Details

Reviewers
tflink
Maniphest Tasks
T358: HTTP 500 errors on incorrect bugzilla credential entry
Commits
rBLKR832d49871ddf: HTTP 500 errors on incorrect bugzilla credential entry
Summary

Caused by a change in the interface in python-bugzilla:
https://git.fedorahosted.org/cgit/python-bugzilla.git/commit/?id=2e36c4092bf644d41bb5c80322a10512458c0bdf

Also renames BugzillaError exception since its names collides with
python-bugzilla's exception of the same name and creates confusion.

Fixes T358

Test Plan

Tests amended. Tested on dev instance. Works for me.

Diff Detail

Repository
rBLKR blockerbugs
Lint
Lint Skipped
Unit
Unit Tests Skipped
mkrizek retitled this revision from to HTTP 500 errors on incorrect bugzilla credential entry.Oct 15 2014, 10:57 AM
mkrizek updated this object.
mkrizek edited the test plan for this revision. (Show Details)
mkrizek added a reviewer: tflink.
mkrizek added a task: T358: HTTP 500 errors on incorrect bugzilla credential entry.
tflink accepted this revision.Oct 15 2014, 6:57 PM

LGTM, also tested locally

This revision is now accepted and ready to land.Oct 15 2014, 6:57 PM
mkrizek closed this revision.Oct 15 2014, 7:13 PM
mkrizek updated this revision to Diff 706.

Closed by commit rBLKR832d49871ddf (authored by @mkrizek).

Revision Contents

Diff 706

View Options

blockerbugs/controllers/main.py

Show All 21 Lines
22 22​
23from flask import Blueprint, render_template, redirect, url_for, abort, g, flash, make_response 23​from flask import Blueprint, render_template, redirect, url_for, abort, g, flash, make_response
24import datetime 24​import datetime
25from sqlalchemy import func, desc, or_ 25​from sqlalchemy import func, desc, or_
26import bugzilla 26​import bugzilla
27from flask_fas_openid import fas_login_required 27​from flask_fas_openid import fas_login_required
28 28​
29from blockerbugs import app, db 29​from blockerbugs import app, db
30from blockerbugs.util.bz_interface import BlockerProposal, BugzillaError 30​from blockerbugs.util.bz_interface import BlockerProposal, BZInterfaceError
31from blockerbugs.models.bug import Bug 31​from blockerbugs.models.bug import Bug
32from blockerbugs.models.milestone import Milestone 32​from blockerbugs.models.milestone import Milestone
33from blockerbugs.models.update import Update 33​from blockerbugs.models.update import Update
34from blockerbugs.models.release import Release 34​from blockerbugs.models.release import Release
35from blockerbugs.models.userinfo import UserInfo 35​from blockerbugs.models.userinfo import UserInfo
36from blockerbugs.models.spin import Spin, SpinState 36​from blockerbugs.models.spin import Spin, SpinState
37from blockerbugs.controllers.forms import BugProposeForm, FasBugzillaForm 37​from blockerbugs.controllers.forms import BugProposeForm, FasBugzillaForm
38 38​
▲ Show 20 Lines • Show All 276 Lines • ▼ Show 20 Line(s)305 if not app.testing:
315 else: 315​ else:
316 app.logger.warning('No FAS_USER configured. bugzilla modifications will NOT work!') 316​ app.logger.warning('No FAS_USER configured. bugzilla modifications will NOT work!')
317 return bugzilla.RHBugzilla4(url=app.config['BUGZILLA_XMLRPC'], cookiefile=None, tokenfile=None) 317​ return bugzilla.RHBugzilla4(url=app.config['BUGZILLA_XMLRPC'], cookiefile=None, tokenfile=None)
318 else: 318​ else:
319 app.logger.error('Bugzilla connections with test configuration is not supported!') 319​ app.logger.error('Bugzilla connections with test configuration is not supported!')
320 return None 320​ return None
321 321​
322 322​
323def check_bz_user(user, password): ​
324 bz = bugzilla.RHBugzilla4(url=app.config['BUGZILLA_XMLRPC'], cookiefile=None, tokenfile=None) ​
325 if not bz or not bz.login(user, password): ​
326 return False ​
327 bz.logout() ​
328 return True ​
329 ​
330 ​
331@main.route('/fas_bugzilla', methods=['GET', 'POST']) 323​@main.route('/fas_bugzilla', methods=['GET', 'POST'])
332@fas_login_required 324​@fas_login_required
333def fas_bugzilla(): 325​def fas_bugzilla():
334 fas_bz_form = FasBugzillaForm() 326​ fas_bz_form = FasBugzillaForm()
335 327​
336 if fas_bz_form.validate_on_submit(): 328​ if fas_bz_form.validate_on_submit():
337 if check_bz_user(fas_bz_form.bz_user.data, fas_bz_form.bz_pass.data): 329​ try:
330​ # try logging into bz if user/pass are valid
331​ bz = bugzilla.RHBugzilla4(url=app.config['BUGZILLA_XMLRPC'],
332​ user=fas_bz_form.bz_user.data,
333​ password=fas_bz_form.bz_pass.data,
334​ cookiefile=None, tokenfile=None)
335​ bz.disconnect()
336​
338 #create new user_info if user with given FAS-login does not exist 337​ #create new user_info if user with given FAS-login does not exist
339 #if user_with given FAS-login exists just update user_info.bz_user 338​ #if user_with given FAS-login exists just update user_info.bz_user
340 user_info = UserInfo.query.filter_by(fas_login=g.fas_user.username).first() 339​ user_info = UserInfo.query.filter_by(fas_login=g.fas_user.username).first()
341 if user_info: 340​ if user_info:
342 user_info.bz_user = fas_bz_form.bz_user.data 341​ user_info.bz_user = fas_bz_form.bz_user.data
343 else: 342​ else:
344 user_info = UserInfo(g.fas_user.username, fas_bz_form.bz_user.data) 343​ user_info = UserInfo(g.fas_user.username, fas_bz_form.bz_user.data)
345 db.session.add(user_info) 344​ db.session.add(user_info)
346 db.session.commit() 345​ db.session.commit()
347 return redirect(url_for('.propose_bug')) 346​ return redirect(url_for('.propose_bug'))
348 else: 347​ except bugzilla.BugzillaError, e:
349 app.logger.info('FAILED Bugzilla Authentication') 348​ app.logger.info(e)
350 flash('Bugzilla Authentication Failed! Try Again!') 349​ flash(e)
351 350​
352 return render_template('fas_bugzilla.html', title='Link your FAS and Red Hat Bugzilla Accounts', 351​ return render_template('fas_bugzilla.html', title='Link your FAS and Red Hat Bugzilla Accounts',
353 fas_bz_form=fas_bz_form) 352​ fas_bz_form=fas_bz_form)
354 353​
355 354​
356@main.route('/propose_bug', methods=['GET', 'POST']) 355​@main.route('/propose_bug', methods=['GET', 'POST'])
357@fas_login_required 356​@fas_login_required
358def propose_bug(): 357​def propose_bug():
Show All 25 Lines366 if bugform.validate_on_submit():
384 # data 383​ # data
385 bz = get_bugzilla() 384​ bz = get_bugzilla()
386 proposer = BlockerProposal(bz, bugid, trackers, bugform.blocker.data, 385​ proposer = BlockerProposal(bz, bugid, trackers, bugform.blocker.data,
387 bugform.freeze_exception.data, user_info.bz_user) 386​ bugform.freeze_exception.data, user_info.bz_user)
388 387​
389 # check bug to make sure it exists and that it isn't closed 388​ # check bug to make sure it exists and that it isn't closed
390 try: 389​ try:
391 proposer.check_proposed_bug() 390​ proposer.check_proposed_bug()
392 except BugzillaError, e: 391​ except BZInterfaceError, e:
393 bugform.bugid.errors = [e.msg] 392​ bugform.bugid.errors = [e.msg]
394 393​
395 # check to make sure that any proposals aren't being re-done 394​ # check to make sure that any proposals aren't being re-done
396 if bugform.blocker.data: 395​ if bugform.blocker.data:
397 app.logger.debug('checking for valid blocker proposal') 396​ app.logger.debug('checking for valid blocker proposal')
398 if not proposer.check_blocker_proposal(): 397​ if not proposer.check_blocker_proposal():
399 bugform.blocker.errors = ['Bug %i is already proposed as a blocker' % bugid] 398​ bugform.blocker.errors = ['Bug %i is already proposed as a blocker' % bugid]
400 if bugform.freeze_exception.data: 399​ if bugform.freeze_exception.data:
401 app.logger.debug('checking for valid freeze exception proposal') 400​ app.logger.debug('checking for valid freeze exception proposal')
402 if not proposer.check_fe_proposal(): 401​ if not proposer.check_fe_proposal():
403 bugform.freeze_exception.errors = ['Bug %i is already proposed as a freeze exception' % bugid] 402​ bugform.freeze_exception.errors = ['Bug %i is already proposed as a freeze exception' % bugid]
404 403​
405 if len(bugform.errors) == 0: 404​ if len(bugform.errors) == 0:
406 user = 'Fedora user %s' % g.fas_user.username 405​ user = 'Fedora user %s' % g.fas_user.username
407 app.logger.info('%s proposing %i as %s for %s' % ( 406​ app.logger.info('%s proposing %i as %s for %s' % (
408 user, bugid, proposer.get_tracker_type(), selected_milestone.name)) 407​ user, bugid, proposer.get_tracker_type(), selected_milestone.name))
409 try: 408​ try:
410 proposer.propose_bugs(user, selected_milestone.name, bugform.justification.data) 409​ proposer.propose_bugs(user, selected_milestone.name, bugform.justification.data)
411 return render_template('thanks.html', bugid=bugid, 410​ return render_template('thanks.html', bugid=bugid,
412 isblocker=bugform.blocker.data, 411​ isblocker=bugform.blocker.data,
413 isfe=bugform.freeze_exception.data) 412​ isfe=bugform.freeze_exception.data)
414 except BugzillaError, e: 413​ except BZInterfaceError, e:
415 bugform.bugid.errors = [e.msg] 414​ bugform.bugid.errors = [e.msg]
416 app.logger.info('bug proposal form errors: %s' % e.msg) 415​ app.logger.info('bug proposal form errors: %s' % e.msg)
417 else: 416​ else:
418 app.logger.info('bug proposal form errors: %s' % str(bugform.errors)) 417​ app.logger.info('bug proposal form errors: %s' % str(bugform.errors))
419 418​
420 # this assumes that the current milestone is the release against which the 419​ # this assumes that the current milestone is the release against which the
421 # blocker is being proposed. It isn't an ideal assumption but it should hold 420​ # blocker is being proposed. It isn't an ideal assumption but it should hold
422 # for most use cases. 421​ # for most use cases.
423 422​
424 current_release = get_current_milestone().release.number 423​ current_release = get_current_milestone().release.number
425 return render_template('propose_bug.html', title='Propose a Blocker or Freeze Exception', 424​ return render_template('propose_bug.html', title='Propose a Blocker or Freeze Exception',
426 bugform=bugform, release=current_release) 425​ bugform=bugform, release=current_release)
View Options

blockerbugs/util/bz_interface.py

Show All 25 Lines
26import logging 26​import logging
27import datetime 27​import datetime
28from xmlrpclib import Fault 28​from xmlrpclib import Fault
29 29​
30base_query = {'o1': 'anywords', 30​base_query = {'o1': 'anywords',
31 'f1': 'blocked', 31​ 'f1': 'blocked',
32 'query_format':'advanced'} 32​ 'query_format':'advanced'}
33 33​
34class BugzillaError(Exception): 34​class BZInterfaceError(Exception):
35 def __init__(self, msg): 35​ def __init__(self, msg):
36 self.msg = msg 36​ self.msg = msg
37 37​
38 def __str__(self): 38​ def __str__(self):
39 return repr(self.msg) 39​ return repr(self.msg)
40 40​
41 41​
42class BlockerBugs(): 42​class BlockerBugs():
▲ Show 20 Lines • Show All 116 Lines • ▼ Show 20 Line(s)147 def __init__(self, bz, bugid, trackers, is_blocker=False, is_fe=False, bz_user=''):
159 self.log = logging.getLogger('bz_interface.bugproposal') 159​ self.log = logging.getLogger('bz_interface.bugproposal')
160 160​
161 def get_bugdata(self): 161​ def get_bugdata(self):
162 # get bug data, will raise XMLRPC fault if the bug does not exist 162​ # get bug data, will raise XMLRPC fault if the bug does not exist
163 try: 163​ try:
164 self.bugdata = self.bz.getbug(self.bugid) 164​ self.bugdata = self.bz.getbug(self.bugid)
165 except Fault, e: 165​ except Fault, e:
166 if e.faultCode == 101: 166​ if e.faultCode == 101:
167 raise BugzillaError(e.faultString) 167​ raise BZInterfaceError(e.faultString)
168 else: 168​ else:
169 raise 169​ raise
170 170​
171 def get_tracker_type(self): 171​ def get_tracker_type(self):
172 if self.is_blocker and self.is_fe: 172​ if self.is_blocker and self.is_fe:
173 return 'Blocker and Freeze Exception' 173​ return 'Blocker and Freeze Exception'
174 if self.is_blocker: 174​ if self.is_blocker:
175 return 'Blocker' 175​ return 'Blocker'
Show All 14 Lines189 try:
190 self._do_proposal(tracker_bugs, self.bugid, ' '.join(comment), self.bz_user) 190​ self._do_proposal(tracker_bugs, self.bugid, ' '.join(comment), self.bz_user)
191 except Fault, e: 191​ except Fault, e:
192 if e.faultCode == 51: 192​ if e.faultCode == 51:
193 # bugzilla account does not exist, this should happen very rarely, if ever 193​ # bugzilla account does not exist, this should happen very rarely, if ever
194 # so just redo the call with nothing to add to cc 194​ # so just redo the call with nothing to add to cc
195 # TODO - it might be useful to ask the user to re-associate accounts again here 195​ # TODO - it might be useful to ask the user to re-associate accounts again here
196 self._do_proposal(tracker_bugs, self.bugid, ' '.join(comment), '') 196​ self._do_proposal(tracker_bugs, self.bugid, ' '.join(comment), '')
197 else: 197​ else:
198 raise BugzillaError(e.faultString) 198​ raise BZInterfaceError(e.faultString)
199 199​
200 def _do_proposal(self, tracker, proposed_bugid, comment, bz_user): 200​ def _do_proposal(self, tracker, proposed_bugid, comment, bz_user):
201 bug_update = self.bz.build_update(blocks_add=tracker, 201​ bug_update = self.bz.build_update(blocks_add=tracker,
202 cc_add=[bz_user], 202​ cc_add=[bz_user],
203 comment=comment) 203​ comment=comment)
204 self.bz.update_bugs(proposed_bugid, bug_update) 204​ self.bz.update_bugs(proposed_bugid, bug_update)
205 205​
206 def check_proposed_bug(self): 206​ def check_proposed_bug(self):
207 if not self.bugdata: 207​ if not self.bugdata:
208 self.get_bugdata() 208​ self.get_bugdata()
209 209​
210 # check to make sure that bug is not already CLOSED 210​ # check to make sure that bug is not already CLOSED
211 bug_status = self.bugdata.bug_status 211​ bug_status = self.bugdata.bug_status
212 if 'CLOSED' in bug_status.split(): 212​ if 'CLOSED' in bug_status.split():
213 raise BugzillaError('Bug %i is CLOSED: %s' % (self.bugid, bug_status)) 213​ raise BZInterfaceError('Bug %i is CLOSED: %s' % (self.bugid, bug_status))
214 214​
215 def get_blocks(self): 215​ def get_blocks(self):
216 if not self.bugdata: 216​ if not self.bugdata:
217 self.get_bugdata() 217​ self.get_bugdata()
218 self.blocks = self.bugdata.blocked 218​ self.blocks = self.bugdata.blocked
219 219​
220 def check_blocker_proposal(self): 220​ def check_blocker_proposal(self):
221 if not self.blocks: 221​ if not self.blocks:
Show All 14 Lines
View Options

testing/test_bugchange.py

1from dingus import Dingus, exception_raiser 1​from dingus import Dingus, exception_raiser
2from datetime import datetime 2​from datetime import datetime
3from xmlrpclib import Fault 3​from xmlrpclib import Fault
4import pytest 4​import pytest
5 5​
6from blockerbugs.util.bz_interface import BlockerProposal, BugzillaError 6​from blockerbugs.util.bz_interface import BlockerProposal, BZInterfaceError
7 7​
8bz_success = {'bugs': [{'alias': [], 8​bz_success = {'bugs': [{'alias': [],
9 'changes': {}, 9​ 'changes': {},
10 'id': 806497, 10​ 'id': 806497,
11 'last_change_time': datetime.utcnow()}]} 11​ 'last_change_time': datetime.utcnow()}]}
12 12​
13 13​
14class TestProposeBlocker: 14​class TestProposeBlocker:
▲ Show 20 Lines • Show All 74 Lines • ▼ Show 20 Line(s)
89 89​
90 def test_proposed_cc_invalid(self): 90​ def test_proposed_cc_invalid(self):
91 stubbz = Dingus(_update_bug__returns=True, build_update__returns=bz_success) 91​ stubbz = Dingus(_update_bug__returns=True, build_update__returns=bz_success)
92 stubbz.update_bugs = exception_raiser(Fault(50, 'not 51 exception')) 92​ stubbz.update_bugs = exception_raiser(Fault(50, 'not 51 exception'))
93 93​
94 test_bz = BlockerProposal(stubbz, self.ref_proposed, self.ref_trackers, 94​ test_bz = BlockerProposal(stubbz, self.ref_proposed, self.ref_trackers,
95 is_blocker = True, bz_user = self.ref_cc) 95​ is_blocker = True, bz_user = self.ref_cc)
96 96​
97 with pytest.raises(BugzillaError) as excinfo: 97​ with pytest.raises(BZInterfaceError) as excinfo:
98 test_bz.propose_bugs(self.ref_user, self.ref_milestone, 98​ test_bz.propose_bugs(self.ref_user, self.ref_milestone,
99 self.ref_justification) 99​ self.ref_justification)
100 100​
101 assert excinfo.value.msg == 'not 51 exception' 101​ assert excinfo.value.msg == 'not 51 exception'
102 102​
103 103​
104class TestBugProposalTrackerCheck: 104​class TestBugProposalTrackerCheck:
105 def setup_method(self, method): 105​ def setup_method(self, method):
▲ Show 20 Lines • Show All 79 Lines • ▼ Show 20 Line(s)177 def test_check_bothpropose_fe_already_blocker_ok(self):
185 assert test_result == True 185​ assert test_result == True
186 186​
187 def test_proposed_bug_notexist(self): 187​ def test_proposed_bug_notexist(self):
188 stubbz = Dingus() 188​ stubbz = Dingus()
189 stubbz.getbug = exception_raiser(Fault(101, 'bug does not exist')) 189​ stubbz.getbug = exception_raiser(Fault(101, 'bug does not exist'))
190 190​
191 test_bz = BlockerProposal(stubbz, self.ref_proposed, self.ref_trackers, 191​ test_bz = BlockerProposal(stubbz, self.ref_proposed, self.ref_trackers,
192 is_blocker=True) 192​ is_blocker=True)
193 with pytest.raises(BugzillaError): 193​ with pytest.raises(BZInterfaceError):
194 test_bz.check_blocker_proposal() 194​ test_bz.check_blocker_proposal()
195 195​
196 196​
197class TestProposedBug(): 197​class TestProposedBug():
198 def setup_method(self, method): 198​ def setup_method(self, method):
199 self.blocker_tracker = 123456 199​ self.blocker_tracker = 123456
200 self.fe_tracker = 234567 200​ self.fe_tracker = 234567
201 self.ref_justification = 'because I said so' 201​ self.ref_justification = 'because I said so'
202 self.ref_user = 'joe@allthebugs.com' 202​ self.ref_user = 'joe@allthebugs.com'
203 self.ref_tracker_type = 'Blocker' 203​ self.ref_tracker_type = 'Blocker'
204 self.ref_milestone = 'F19-alpha' 204​ self.ref_milestone = 'F19-alpha'
205 self.ref_proposed = 345678 205​ self.ref_proposed = 345678
206 206​
207 def test_proposed_bug_closed(self): 207​ def test_proposed_bug_closed(self):
208 ref_status = 'CLOSED NOTABUG' 208​ ref_status = 'CLOSED NOTABUG'
209 refbug = Dingus(blocked=[self.blocker_tracker], bug_status=ref_status) 209​ refbug = Dingus(blocked=[self.blocker_tracker], bug_status=ref_status)
210 stubbz = Dingus(getbug__returns=refbug) 210​ stubbz = Dingus(getbug__returns=refbug)
211 211​
212 test_bz = BlockerProposal(stubbz, self.ref_proposed, 212​ test_bz = BlockerProposal(stubbz, self.ref_proposed,
213 [self.blocker_tracker], 213​ [self.blocker_tracker],
214 self.ref_tracker_type) 214​ self.ref_tracker_type)
215 with pytest.raises(BugzillaError) as excinfo: 215​ with pytest.raises(BZInterfaceError) as excinfo:
216 test_bz.check_proposed_bug() 216​ test_bz.check_proposed_bug()
217 217​
218 assert excinfo.value.msg == 'Bug %i is CLOSED: %s' % ( 218​ assert excinfo.value.msg == 'Bug %i is CLOSED: %s' % (
219 self.ref_proposed, ref_status) 219​ self.ref_proposed, ref_status)
220 220​
221 def test_proposed_bug_invalid(self): 221​ def test_proposed_bug_invalid(self):
222 stubbz = Dingus() 222​ stubbz = Dingus()
223 stubbz.getbug = exception_raiser(Fault(101, 'bug does not exist')) 223​ stubbz.getbug = exception_raiser(Fault(101, 'bug does not exist'))
224 224​
225 test_bz = BlockerProposal(stubbz, self.ref_proposed, 225​ test_bz = BlockerProposal(stubbz, self.ref_proposed,
226 [self.blocker_tracker], 226​ [self.blocker_tracker],
227 self.ref_tracker_type) 227​ self.ref_tracker_type)
228 with pytest.raises(BugzillaError) as excinfo: 228​ with pytest.raises(BZInterfaceError) as excinfo:
229 test_bz.check_proposed_bug() 229​ test_bz.check_proposed_bug()
230 230​
231 assert excinfo.value.msg == 'bug does not exist' 231​ assert excinfo.value.msg == 'bug does not exist'
232 232​