1833442 - When a revision is abandoned for an uplift repo in Phabricator, Phabbugz is trying to set the approval-mozilla-beta flag to '-' and failing

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Phabricator Integration, defect)

Description

[David Lawrence [:dkl]]

Only users in 'mozilla-next-drivers' can set the approval flags to either '+' or '-'. Also onlyh users in 'mozilla-next-drivers' can set a flag back to ? or clear it if it is already granted or denied.

This occurred for https://phabricator.services.mozilla.com/D178184 on bug 1831833.

{"Type":"Bugzilla.Extension.PhabBugz.Feed","EnvVersion":2.0,"Pid":7,"Logger":"STDERR","Severity":6,"Timestamp":1684254309000000000,"Fields":{"type":"FEED","msg":"STORY TEXT: issammani abandoned D178184: Bug 183183
3 - Import modules lazily where possible.."},"Hostname":"gha-bugzilla-phabbugz-7dc75d7499-skdnc"}                                                                                                                    
{"Logger":"STDERR","Pid":7,"EnvVersion":2.0,"Type":"Bugzilla.Extension.PhabBugz.Feed","Timestamp":1684254309000000000,"Fields":{"type":"FEED","msg":"REVISION CHANGE FOUND: D178184: Bug 1831833 - Import modules laz
ily where possible. | bug: 1831833 | issammani | issammani abandoned D178184: Bug 1831833 - Import modules lazily where possible.."},"Severity":6,"Hostname":"gha-bugzilla-phabbugz-7dc75d7499-skdnc"}               
{"Type":"Bugzilla.Extension.PhabBugz.Feed","EnvVersion":2.0,"Pid":7,"Logger":"STDERR","Severity":6,"Timestamp":1684254310000000000,"Fields":{"msg":"Bug is public so setting view/edit public","type":"FEED"},"Hostna
me":"gha-bugzilla-phabbugz-7dc75d7499-skdnc"}                                                                                                                                                                        
{"Hostname":"gha-bugzilla-phabbugz-7dc75d7499-skdnc","Logger":"STDERR","EnvVersion":2.0,"Type":"Bugzilla.Extension.PhabBugz.Feed","Pid":7,"Fields":{"type":"FEED","msg":"Checking for revision attachment"},"Timestam
p":1684254310000000000,"Severity":6}                                                                                                                                                                                 
{"Hostname":"gha-bugzilla-phabbugz-7dc75d7499-skdnc","EnvVersion":2.0,"Type":"Bugzilla.Extension.PhabBugz.Feed","Pid":7,"Logger":"STDERR","Severity":6,"Fields":{"msg":"Attachment 9333939 created or already exists.
","type":"FEED"},"Timestamp":1684254310000000000}                                                                                                                                                                    
{"Hostname":"gha-bugzilla-phabbugz-7dc75d7499-skdnc","Logger":"STDERR","Pid":7,"Type":"Bugzilla.Extension.PhabBugz.Feed","EnvVersion":2.0,"Fields":{"msg":"Updating obsolete status on attachmment 9333939","type":"F
EED"},"Timestamp":1684254310000000000,"Severity":6}                                                                                                                                                                  
{"Hostname":"gha-bugzilla-phabbugz-7dc75d7499-skdnc","Timestamp":1684254310000000000,"Fields":{"type":"FEED","msg":"Setting approval-mozilla-beta flag on revision attachment."},"Severity":6,"Logger":"STDERR","Type
":"Bugzilla.Extension.PhabBugz.Util","EnvVersion":2.0,"Pid":7}                                                                                                                                                       
{"Hostname":"gha-bugzilla-phabbugz-7dc75d7499-skdnc","Severity":0,"Timestamp":1684254310000000000,"Fields":{"vars":"{\"name\":\"approval-mozilla-beta\",\"old_status\":\"\",\"status\":\"-\"}","msg":"You tried to de
ny approval-mozilla-beta. Only a user with the required\npermissions may make this change.\n"},"Type":"Bugzilla.Extension.PhabBugz.Feed","EnvVersion":2.0,"Pid":7,"Logger":"STDERR"}

Comment 1

[David Lawrence [:dkl]]

Got another slighty related error later today:

{"Fields":{"type":"FEED","msg":"STORY TEXT: glandium changed the repository for D178247: Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+. from rMOZILLACENTRAL mozilla-central to rESRONEOHTWO mozilla-esr102."},"Pid":7,"Hostname":"gha-bugzilla-phabbugz-65ccc77554-56qrn","EnvVersion":2.0,"Severity":6,"Logger":"STDERR","Type":"Bugzilla.Extension.PhabBugz.Feed","Timestamp":1684281327000000000}
{"Hostname":"gha-bugzilla-phabbugz-65ccc77554-56qrn","Severity":6,"EnvVersion":2.0,"Fields":{"msg":"REVISION CHANGE FOUND: D178247: Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+. | bug: 1831242 | glandium | glandium changed the repository for D178247: Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+. from rMOZILLACENTRAL mozilla-central to rESRONEOHTWO mozilla-esr102.","type":"FEED"},"Pid":7,"Timestamp":1684281327000000000,"Type":"Bugzilla.Extension.PhabBugz.Feed","Logger":"STDERR"}
{"Timestamp":1684281327000000000,"Type":"Bugzilla.Extension.PhabBugz.Feed","Logger":"STDERR","Severity":6,"EnvVersion":2.0,"Hostname":"gha-bugzilla-phabbugz-65ccc77554-56qrn","Fields":{"msg":"Bug is public so setting view/edit public","type":"FEED"},"Pid":7}
{"Logger":"STDERR","Timestamp":1684281327000000000,"Type":"Bugzilla.Extension.PhabBugz.Feed","Fields":{"msg":"Checking for revision attachment","type":"FEED"},"Pid":7,"Severity":6,"EnvVersion":2.0,"Hostname":"gha-bugzilla-phabbugz-65ccc77554-56qrn"}
{"Type":"Bugzilla.Extension.PhabBugz.Feed","Timestamp":1684281327000000000,"Logger":"STDERR","EnvVersion":2.0,"Severity":6,"Hostname":"gha-bugzilla-phabbugz-65ccc77554-56qrn","Pid":7,"Fields":{"type":"FEED","msg":"Attachment 9334218 created or already exists."}}
{"Fields":{"type":"FEED","msg":"Updating obsolete status on attachmment 9334218"},"Pid":7,"Hostname":"gha-bugzilla-phabbugz-65ccc77554-56qrn","EnvVersion":2.0,"Severity":6,"Logger":"STDERR","Type":"Bugzilla.Extension.PhabBugz.Feed","Timestamp":1684281327000000000}
{"Timestamp":1684281327000000000,"Type":"Bugzilla.Extension.PhabBugz.Util","Logger":"STDERR","Severity":6,"EnvVersion":2.0,"Hostname":"gha-bugzilla-phabbugz-65ccc77554-56qrn","Fields":{"type":"FEED","msg":"Setting approval-mozilla-esr102 flag on revision attachment."},"Pid":7}
{"Pid":7,"Fields":{"vars":"{\"type\":\"approval-mozilla-esr102\"}","msg":"The flag type approval-mozilla-esr102 is inactive and cannot be used to\ncreate new flags.\n"},"Hostname":"gha-bugzilla-phabbugz-65ccc77554-56qrn","EnvVersion":2.0,"Severity":0,"Logger":"STDERR","Type":"Bugzilla.Extension.PhabBugz.Feed","Timestamp":1684281327000000000}

somehow new approval-mozilla-esr102 attachment flags exist. One was disabled and one was enabled. Someone must have created a new one thinking the old one was bad and deactivated the old one. The attachment for the revision, phabbugz was trying to set the flag that was disabled instead of the enabled one and the code was crashing with the error.

Comment 2

[David Lawrence [:dkl]]

So I see we have two options to fix this:

1. Set the flag setter up with all groups which would allow the revision author to set approval grant and deny flags as well as clear old flags. This is what we do when creating a new attachment currently.

2. Only allow flag changes that the user can normally do such as set ? and clear the ? is the flag is abandoned. If the approval flag is set to + or - (grant or deny) then we just leave it alone since the normal user cannot clear those.

Obviously #2 is the safest from a security standpoint but not sure if it breaks the uplift workflow. For example, what if a revision attachment has approval-mozilla-beta set to + (grant) and then the revision is abandoned. If we leave it alone, then will that flag being set cause issues in other places?

Once we are happy I can fix this pretty quickly and get it out. This is another reason I need to work on bug 1832654 to fix the bigger picture issues. Phabbugz has become more fragile the more things we add to it.

Comment 3

[BMO Github Automation]

Attachment: [mozilla-bteam/bmo] Bug 1833442 - When a revision is abandoned for an uplift repo in Phabricator, Phabbugz is trying to set the approval-mozilla-beta flag to '-' and failing (#2071)

Comment 4

[David Lawrence [:dkl]]

Merged
https://github.com/mozilla-bteam/bmo/commit/b33c96da4bba68b81184892f2f3976670adf11a1