qyearsley@, this sounds more like a P2 than a P3, would you agree?

robertma@, assuming this isn't much work, could you take a look?

Sounds great!

I didn't know an account could have the permission to commit on master without being a full committer. It's the right thing to do for sure.

I'll review the doc/slides to see if there's more we can do as well.

The bot account has been added to a Gerrit group which has submission permission on master, so I suppose we can move on to the next step, removing the bot from full committers.

Quinten, how did you add the bot to committers? And how to revert?

I believe it was added to the mailing list group committers@chromium.org (see  bug 634200 ). List of committers may be seen at: https://chromium-committers.googleplex.com/lists/committers@chromium.org

In that bug, Paweł had helped us add the bot to the committers list; Paweł: Could you help us remove it now? Also, is there a process for requests to remove accounts from committers?

Per  bug 783662 , I don't think he's around. Perhaps dpranke can help?

Hi Dirk, do you know who admins the committers@ list and how I should request an account removed?

benhenry@, efoo@, and I can all remove it from the group.

Dirk has now removed the bot from the committers list - possible actions left before considering this done:
 - Add Gerrit submit rules in rules.pl
 - Add rules for the cr-audit-commits app

It may also be OK if one of these is considered redundant or unnecessary; this bug is about safety and preventing the bot from accidentally doing something unintentional.

Thanks! The remaining two steps, IMHO, have lower priority. Downgrading to P3. Will come back when more urgent stuff is taken care of.

(+leon.han who's on duty today)

It looks like something went wrong in the steps above, and wpt-importer is now failing to CQ+1 its CLs. From https://ci.chromium.org/buildbot/chromium.infra.cron/wpt-importer/11005:

2018-01-26 01:31:51,490 - Issue: Issue number: 888718 (https://chromium-review.googlesource.com/888718)
2018-01-26 01:31:51,490 - Triggering try jobs for updating expectations.
Traceback (most recent call last):
  File "/mnt/data/b/rr/tmp9azemI/w/src/third_party/WebKit/Tools/Scripts/wpt-import", line 24, in <module>
    main()
  File "/mnt/data/b/rr/tmp9azemI/w/src/third_party/WebKit/Tools/Scripts/wpt-import", line 17, in main
    host.exit(importer.main())
  File "/mnt/data/b/rr/tmp9azemI/w/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py", line 163, in main
    if not self.update_expectations_for_cl():
  File "/mnt/data/b/rr/tmp9azemI/w/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py", line 186, in update_expectations_for_cl
    self.git_cl.trigger_try_jobs(self.blink_try_bots())
  File "/mnt/data/b/rr/tmp9azemI/w/src/third_party/WebKit/Tools/Scripts/webkitpy/common/net/git_cl.py", line 69, in trigger_try_jobs
    master='tryserver.chromium.android')
  File "/mnt/data/b/rr/tmp9azemI/w/src/third_party/WebKit/Tools/Scripts/webkitpy/common/net/git_cl.py", line 77, in trigger_try_jobs
    self.run(command)
  File "/mnt/data/b/rr/tmp9azemI/w/src/third_party/WebKit/Tools/Scripts/webkitpy/common/net/git_cl.py", line 60, in run
    return self._host.executive.run_command(command, cwd=self._cwd)
  File "/mnt/data/b/rr/tmp9azemI/w/src/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 337, in run_command
    (error_handler or self.default_error_handler)(script_error)
  File "/mnt/data/b/rr/tmp9azemI/w/src/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 244, in default_error_handler
    raise error
webkitpy.common.system.executive.ScriptError: Failed to run "['git', 'cl', 'try', '-m', 'tryserver.chromium.android', '-b', 'android_blink_rel', '--auth-refresh-token-json', '/creds/refresh_tokens/blink-w3c-test-autoroller']" exit_code: 1
output: ERROR: Access denied: User user:blink-w3c-test-autoroller@chromium.org cannot add builds to bucket master.tryserver.chromium.android
step returned non-zero exit code: 1

Yikes, upgrading to P1, apparently just removing the privileges was not enough.

I can't see that any config was actually changed remove removing blink-w3c-test-autoroller from the committers list. I think I may have seen something in an email, but can't find it.

https://chromium-review.googlesource.com/q/project:chromium%252Fsrc+branch:refs%252Fmeta%252Fconfig+status:merged is where I think such reviews would have shown up.

Ah, found it via #4:
https://chromium.googlesource.com/chromium/src/+/0dae0073ae1b73694a641ce4fc6c7808fc5f2bdf%5E%21/#F1

agable@, I can't see anything listed in https://canary-chromium-review.googlesource.com/admin/groups/5753,members, is blink-w3c-test-autoroller@chromium.org really in it?

And does it have enough permissions? I can't find anything about try jobs in project.config, and I suspect that isn't governed by Gerrit at all.

Just in case it's the right fix, I'll now request tryjob access for the account.

I've send "[a little urgent] tryjob access for blink-w3c-test-autoroller@chromium.org" and am leaving this alone now.

Requesting help from troopers

I've readded it to the committer list for now, while we sort this out.

@16: you can't see anything in the chromium-robot-submitters gerrit group because you're not in that group. I'm an admin of it, and can confirm that blink-w3c-test-autoroller is in fact a member of it.

That group is granted submit permission. That group is *not* granted tryjob access; you're right that tryjob access is mediated via a wholly different mechanism.

Tryjob access is controlled via cq.cfg: https://chromium.googlesource.com/chromium/src/+/01c8891fe0ae0c258b1cbd60924aa6eb912a6771/infra/config/cq.cfg#16

In issue 806285 I have added blink-w3c-test-autoroller to project-chromium-tryjob-access.

Once everything propagates (a matter of minutes) it should be safe to remove it from committers@ again (which will also then take a while to propagate).

robertma@, do you think this should still be a P1?

Not as urgent as some other tasks I currently have in my hands. Downgrading to P2 and I'll come back to this early Q2.

The bot now has submit permission on the master branch (via a Gerrit group), and trybot access (via cria). Supposedly, it's now safe to remove the bot account from committers, but we might still miss something else.

I think the remaining task is:

1. Remove wpt-bot from committers
(I'd like to find a peaceful day and kindly ask ChOps' assistance in advance, due to the incident above: remove the bot from committers; if anything goes wrong, add it back and investigate.)
2. Add Gerrit submit rules
3. Add rules for the audit app

robertma@, is step 1 ready for another try? (doing the rotation)

Ping robertma@?

Included in Q4 planning:
https://docs.google.com/document/d/1uh7A24pT6P70Lb9CeFCKga3RrrdzLK7tSAR7o8C7No0/edit?usp=sharing

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/1de73cce1e2f681d2f2de58d703b2909ec0c4e8e

commit 1de73cce1e2f681d2f2de58d703b2909ec0c4e8e
Author: Robert Ma <robertma@chromium.org>
Date: Fri Nov 09 19:43:22 2018

[cr-audit-commits] Add an audit rule for WPT autoroller

WPT autoroller should only modify the LayoutTests directory.

This is the new service account used by wpt-importer after LUCI
migration and will be used in the foreseeable future.

Bug:  801003 
Change-Id: I12d9ac65ac80009df00824f77adde6121c3cb723
Reviewed-on: https://chromium-review.googlesource.com/c/1327287
Reviewed-by: Shuotao Gao <stgao@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#18897}
[modify] https://crrev.com/1de73cce1e2f681d2f2de58d703b2909ec0c4e8e/go/src/infra/appengine/cr-audit-commits/app/rules_config.go
[modify] https://crrev.com/1de73cce1e2f681d2f2de58d703b2909ec0c4e8e/go/src/infra/appengine/cr-audit-commits/app/autoroll_rules.go

Update (should've been posted before comment #26):

Now that the bots have been migrated to LUCI ( issue 803111 ) with a new dedicated service account that has the necessary and minimal permission permissions, we can simply revoke all the special privileges (commit, trybot access, etc.) for blink-w3c-test-autoroller@chromium.org. I've sent out an email to committers@ requesting so.

Besides, the CL in comment #26 added the cr-audit rules for the new service account.

Permissions for the old account have been revoked in  issue 906321 .