1426201 - Restrict [e-mail] to only commit to subtree on m-c

Status: RESOLVED FIXED

(Developer Services :: General, task)

Description

[hwine]

Goal: support WPT auto merging of tests into mozilla-central. See RRA (bug 1419058) & Credential Request (bug 1418492). Being able to ensure commits can only be made into the testing subtree is a strong recommendation from the RRA

Ideally something would be ready for production use by the end of January.

I know of at least one similar request in the past (bug 991749) so a more general solution may be warranted.

Comment 1

[hwine]

:jgraham - can you provide the following additional information, please?

1. What is the subtree to which WPT needs to commit?

2. Which branches will WPT eventually need to commit to?

Comment 2

[James Graham [:jgraham]]

(In reply to Hal Wine [:hwine] (use NI) from comment #1)
> :jgraham - can you provide the following additional information, please?
> 
> 1. What is the subtree to which WPT needs to commit?

Everything is under testing/web-platform. I think it's testing/web-platform/moz.build, testing/web-platform/meta/** and testing/web-platform/tests/**
 
> 2. Which branches will WPT eventually need to commit to?

Currently just mozilla-inbound, although in a Lando world that may change of course.

Comment 3

[James Graham [:jgraham]]

What's the status of this work? Is there anything we can do to help the process along?

Comment 4

[James Graham [:jgraham]]

Attachment: Possible hook

I have literally never done this before, but it seems like a hg hook as in the attachment would do the right thing? If this is the right approach, I can figure out how to write tests &c.

Comment 5

[hwine]

:jgraham - :gps is the technical guru & hg architect, adding :coop for scheduling prioritization

Comment 6

[Gregory Szorc [:gps]]

sheehan is working on a unified commit access policy hook.

Comment 7

[James Graham [:jgraham]]

sheehan: Do you have an estimate on when this will be ready? We have a service that per our RRA should have access to restricted paths that we would like to launch in the next week or two.

Comment 8

[Connor Sheehan [:sheehan]]

Sorry for the delayed response, James! I will be working on the new commit access policy over the next two weeks but I doubt it will be done in time for the release of your service. That work is being tracked in bug 1425872.

We can use the other change-preventing hooks as a blueprint and roll something out in the meantime. Those are 

* prevent_vendored_changes.py hook
* prevent_webidl_changes.py hook
* whitelist_qa.py hook
* whitelist_releng.py hook

It looks to me like the hook in attachment 8942883 [details] is already on the right track.

Comment 9

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Attachment: hghooks: add hook to limit wptsync user to testing/web-platform (bug 1426201)

Review commit: https://reviewboard.mozilla.org/r/215056/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/215056/

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8944912 [details]
hghooks: add hook to limit wptsync user to testing/web-platform (bug 1426201)

https://reviewboard.mozilla.org/r/215056/#review221036

The logic in the hook is fine.

However, we'll need to manually install this hook on repos in order to enable it. If the hook were refactored to use our modern API for writing hooks, it would "just work." That wouldn't be that much work. You just need to refactor the hook to use a base class and implement some methods. See the various files in hghooks/mozhghooks/check. The `relevant()` method should `return self.repo_metadata['firefox_releasing']` to apply to all "production" Firefox repos (that aren't Try or user repos).

Ping me on IRC if you want to discuss how to proceed. I'm a bit busy starting in ~1 hour though.

Comment 11

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8944912 [details]
hghooks: add hook to limit wptsync user to testing/web-platform (bug 1426201)

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

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8944912 [details]
hghooks: add hook to limit wptsync user to testing/web-platform (bug 1426201)

https://reviewboard.mozilla.org/r/215056/#review221440

Very nice! Now all I have to do is deploy this and it should start working!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/b82f4f34d8d7
hghooks: add hook to limit wptsync user to testing/web-platform r=gps

Comment 14

[hwine]

gps: Can you clarify the following points about this hook, please?

1. Is L3 access still required for the [e-mail] user?

2. How do we protect against hook changes slipping into production?

3. IIRC, the original request was for access to mozilla-inbound only -- is there any easy way to keep that restriction in this framework?

Thanks.

Comment 15

[Gregory Szorc [:gps]]

(In reply to Hal Wine [:hwine] (use NI) from comment #14)
> gps: Can you clarify the following points about this hook, please?
> 
> 1. Is L3 access still required for the [e-mail] user?

Yes. Repository write access requires filesystem privileges that are determined by group membership. inbound, central, release repos are group owned by scm_level_3. Autoland is scm_autoland.

> 2. How do we protect against hook changes slipping into production?

Same as any other hook. We rely on code review policies being followed and for the person doing the deploy to spot check deployed changesets for badness.

> 
> 3. IIRC, the original request was for access to mozilla-inbound only -- is
> there any easy way to keep that restriction in this framework?

Yes. We can change the hook to deny access for all repositories except integration/mozilla-inbound.

Comment 16

[hwine]

gps: thanks

:maja_zf - if I remembered correctly about only needing access to mozilla-inbound, can you make that change please?
Or disallow commits to any release branch? (m-c, m-beta, m-release, and the ESR branches) "Least privilege" is the goal here.

Comment 17

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Follow-up patch is at Bug 1433595. (MozReview wouldn't let me associate the additional patch with this bug.)