1210038 - Autolanded pushes should retain requester in the pushlog

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect, P1)

Description

[Gregory Szorc [:gps]]

Currently, if you Autoland something from MozReview, the pushlog contains the [e-mail] user because this is the actual user pushing to the SSH server.

This makes the forensics trail difficult to follow and sacrifices some auditability.

We should find a way to include the user who initiated the autoland into the pushlog.

This is doable with either Mercurial or SSH server magic from Autoland. Easiest solution is likely to allow a custom environment variable from SSH clients and to hack up PASH (v-c-t:hgserver/pash) to allow setting the pushlog user from this variable if the authenticated SSH user is autoland.

It would be nice to retain both SSH user and Autoland requester. Although doing that without changing the pushlog db schema could be "fun." It's certainly a lot more work if we wish to retain both users.

Comment 1

[Mark Côté [:mcote]]

I'm not sure if we strictly need this for launch--many pushes are done by a small number of addresses anyway (sheriffs)--but it's definitely nice to have.

Comment 2

[Dan Minor [:dminor]]

Attachment: MozReview Request: pushlog: use AUTOLAND_REQUEST_USER when autolanding (bug 1210038) r=gps

pushlog: use AUTOLAND_REQUEST_USER when autolanding (bug 1210038) r=gps

If the push is initiated by the autoland user, the value of the
AUTOLAND_REQUEST_USER environment variable is used as the user.

Comment 3

[Dan Minor [:dminor]]

Attachment: MozReview Request: hgserver: allow AUTOLAND_REQUEST_USER to be set in ssh connection (bug 1210038) r=gps

hgserver: allow AUTOLAND_REQUEST_USER to be set in ssh connection (bug 1210038) r=gps

This changes the configuration of the hg server to accept the
AUTOLAND_REQUEST_USER over ssh.

Comment 4

[Dan Minor [:dminor]]

Attachment: MozReview Request: mozreview: send autoland request user to autoland in requests (bug 1210038) r=gps

mozreview: send autoland request user to autoland in requests (bug 1210038) r=gps

Comment 5

[Dan Minor [:dminor]]

Attachment: MozReview Request: autoland: use autoland request user when pushing (bug 1210038) r=gps

autoland: use autoland request user when pushing (bug 1210038) r=gps

This sets the AUTOLAND_REQUEST_USER environment variable to be the
requester received from MozReview when pushing.

Comment 6

[Dan Minor [:dminor]]

I experimented with retaining the fact that the push was autolanded by prepending "autoland: " to the user in the pushlog and then stripping it out when reading from the database. That seemed to be a can of worms because it appears pushlogs can be exchanged. Another alternative would be to selectively strip "autoland: " out using the template functions but that risks breaking consumers we're not aware of. It might be easier just to change the schema.

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8695282 [details]
MozReview Request: pushlog: use AUTOLAND_REQUEST_USER when autolanding (bug 1210038) r=gps

https://reviewboard.mozilla.org/r/27035/#review24493

I know I've mentioned it before, but for the record it would be nice if the pushlog database recorded both users. But this is a massive scope bloat. This solution is easy and good enough IMO.

::: hgext/pushlog/__init__.py:46
(Diff revision 1)
> +AUTOLAND_USER = '[e-mail]'

Ideally this would come from the hgrc config file, as pushlog is technically a generic extension. But I don't think anyone uses it outside Moz. So, meh.

::: hgext/pushlog/__init__.py:508
(Diff revision 1)
> +    if user == AUTOLAND_USER:

Please add a comment.

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8695283 [details]
MozReview Request: hgserver: allow AUTOLAND_REQUEST_USER to be set in ssh connection (bug 1210038) r=gps

https://reviewboard.mozilla.org/r/27037/#review24495

::: hgserver/tests/test-push-autoland.t:17
(Diff revision 1)
> +TODO we should fix this in hgweb_dir or a hack of hgweb_dir

Support for this was added in Mercurial 3.6, which we should deploy in a few weeks. Please add "3.6" somewhere so this shows up when we do the conversion.

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8695284 [details]
MozReview Request: mozreview: send autoland request user to autoland in requests (bug 1210038) r=gps

https://reviewboard.mozilla.org/r/27039/#review24497

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8695285 [details]
MozReview Request: autoland: use autoland request user when pushing (bug 1210038) r=gps

https://reviewboard.mozilla.org/r/27041/#review24499

::: autoland/autoland/transplant.py:30
(Diff revision 1)
> +    os.environ['AUTOLAND_REQUEST_USER'] = requester

I'm not sure how long this process runs for, but it's probably a good idea to copy os.environ and restore it after this function so the modification doesn't linger.

Comment 11

[Dan Minor [:dminor]]

Comment on attachment 8695282 [details]
MozReview Request: pushlog: use AUTOLAND_REQUEST_USER when autolanding (bug 1210038) r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27035/diff/1-2/

Comment 12

[Dan Minor [:dminor]]

Comment on attachment 8695283 [details]
MozReview Request: hgserver: allow AUTOLAND_REQUEST_USER to be set in ssh connection (bug 1210038) r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27037/diff/1-2/

Comment 13

[Dan Minor [:dminor]]

Comment on attachment 8695284 [details]
MozReview Request: mozreview: send autoland request user to autoland in requests (bug 1210038) r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27039/diff/1-2/

Comment 14

[Dan Minor [:dminor]]

Comment on attachment 8695285 [details]
MozReview Request: autoland: use autoland request user when pushing (bug 1210038) r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27041/diff/1-2/