New issue
Advanced search Search tips

Issue 908647 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jan 3
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Tricium whitelist doesn't behave as expected for playground repo

Project Member Reported by qyears...@chromium.org, Nov 26

Issue description

The project config for the playground repo (https://chromium.googlesource.com/playground/gerrit-tricium/+/infra/config/tricium-dev.cfg) now contains:

repos {
  gerrit_project {
    host: "chromium-review.googlesource.com"
    project: "playground/gerrit-tricium"
    git_url: "https://chromium.googlesource.com/playground/gerrit-tricium"
  }
  whitelisted_group: "project-infra-committers"
}

And according to chrome-infra-auth.appspot.com, qyearsley@chromium.org is a member of "project-infra-committers".

But, the logs for a poll for any of my test changes now includes:

  Owner "qyearsley@chromium.org" is not whitelisted, skipping Analyze.

I ran into this just now because I'm trying to test the new eslint analyzer, and this is the first test change that I tried since the change in October which added a whitelist to this repo (https://chromium.googlesource.com/playground/gerrit-tricium/+/65a3ab582a4b3178ac97ca5bfeab94f95717b4af).

 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 28

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

commit d4f0ad943fea2bf74bdb4b807ab3fa98efb5087c
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Wed Nov 28 22:56:37 2018

[tricium] Clarify gerrit/poll.go

When trying to test eslint, I found that my changes to the playground
repo are no longer analyzed after I added a whitelist to that repo,
which means that I either specified the whitelist wrong or there's a
bug in handling of whitelists.

At first I didn't realize that this was the issue so I tried to debug
by reading through poll.go again, and made several changes to try to
clarify what was happening during the poll.

In this change:
 - Update logging to be less verbose and more useful.
 - Extract helper function isOpen, and note that
   we only ever see NEW, and not DRAFT. This is a minor
   change that should not affect actual behavior --
   but it could be separated from this CL.
 - Update comments.

Bug:  908647 
Change-Id: Id7572d909900b46c4da96c07bd9bd0b1acb8d4ba
Reviewed-on: https://chromium-review.googlesource.com/c/1351832
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#19211}
[modify] https://crrev.com/d4f0ad943fea2bf74bdb4b807ab3fa98efb5087c/go/src/infra/tricium/appengine/gerrit/poll.go
[modify] https://crrev.com/d4f0ad943fea2bf74bdb4b807ab3fa98efb5087c/go/src/infra/tricium/appengine/gerrit/poll_test.go
[modify] https://crrev.com/d4f0ad943fea2bf74bdb4b807ab3fa98efb5087c/go/src/infra/tricium/appengine/gerrit/gerrit.go

Labels: -Pri-3 Pri-2
Status: Started (was: Assigned)
Note: Now that polling is working again for chromium/src, all CLs in chromium/src are being skipped with a message like:

  Preparing to enqueue Analyze requests for 1 changes for project "chromium".
  Owner not whitelisted; skipping Analyze. :: {"email":"kbr@chromium.org", "groups":[]string{"project-chrome-tryjob-access"}}
  ...

where clearly kbr et al are long-time members of Chromium and should be whitelisted. Upon further inspection, it appears that the project-chrome just has google accounts, not chromium ones. So that should be fixed.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d52704a91bf87659854bb7707ece541d486b00d6

commit d52704a91bf87659854bb7707ece541d486b00d6
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Tue Dec 18 17:57:26 2018

Change whitelist for chromium/src to chromium group

project-chrome-tryjob-access is for internal stuff,
and contains google accounts but not chromium accounts.
project-chromium-tryjob-access is what I should have
added; after committing this we'll see whether there's
another problem with whitelist checking that affects
chromium/src.

Bug:  908647 
Change-Id: I37e81c07e75e6b8526c1718cf538fe1e01f53230
Reviewed-on: https://chromium-review.googlesource.com/c/1381451
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617556}
[modify] https://crrev.com/d52704a91bf87659854bb7707ece541d486b00d6/infra/config/global/tricium-prod.cfg

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 19

Labels: merge-merged-config
The following revision refers to this bug:
  https://chromium.googlesource.com/playground/gerrit-tricium/+/e8b61bb2240d5e304118fb74bb9accbb7065f2d0

commit e8b61bb2240d5e304118fb74bb9accbb7065f2d0
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Wed Dec 19 22:43:25 2018

Add whitelist "all" as experiment in playground repo

For some reason "project-infra-committers" didn't work, if even
"all" doesn't work (denies running jobs) then something is wrong
in general, if it works (keeps running jobs) then maybe the problem
was something group-specific.

TBR=maruel

Bug:  908647 
Change-Id: I8736146b014a72b53fc3fa7db00a9bfb1a1bf9ea

[modify] https://crrev.com/e8b61bb2240d5e304118fb74bb9accbb7065f2d0/tricium-dev.cfg

Result of above experiment: after that change, tricium runs jobs on my CL (https://chromium-review.googlesource.com/c/playground/gerrit-tricium/+/1351436), so it's possible that the issue before was group-specific.
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 19

The following revision refers to this bug:
  https://chromium.googlesource.com/playground/gerrit-tricium/+/cd65a41badbfdac42863989b0c1bffd07c0f01b5

commit cd65a41badbfdac42863989b0c1bffd07c0f01b5
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Wed Dec 19 23:29:26 2018

Use whitelisted group project-infra-tryjob-access

This is meant to test whether the whitelist works here.
Using the tryjob-access group makes sense anyway since
this is similar to what's done for chromium/src.

TBR=maruel

Bug:  908647 
Change-Id: Ibfab307541ef3ddf67a94446b42f70404506f45e

[modify] https://crrev.com/cd65a41badbfdac42863989b0c1bffd07c0f01b5/tricium-dev.cfg

After trying to use project-infra-tryjob-access, Tricium doesn't analyze changes from qyearsley@chromium.org, claiming that the account is not in the group, although https://chrome-infra-auth.appspot.com/auth/lookup?p=qyearsley%40chromium.org indicates otherwise.

Definitely something wrong with the way that the lookup is done, although it appears to work for chromium/src, with the group project-chromium-tryjob-access.
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 3

The following revision refers to this bug:
  https://chromium.googlesource.com/playground/gerrit-tricium/+/e06dbd70d7daa28d52b42cd4df9f9d15c215a7ef

commit e06dbd70d7daa28d52b42cd4df9f9d15c215a7ef
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Thu Jan 03 22:41:55 2019

[tricium] Add a variety of groups to playground repo config

This change goes together with https://crrev.com/c/1391369,
and is also meant as a temporary debugging change.

It includes some groups that qyearsley@chromium.org belongs
to directly, belongs to indirectly, doesn't belong to,
and non-existent groups, and groups of different types.

TBR=maruel
TBR_REASON=playground repo

Bug:  908647 
Change-Id: Ia0a572d38949a0154e41acf821e4b277781fb668

[modify] https://crrev.com/e06dbd70d7daa28d52b42cd4df9f9d15c215a7ef/tricium-dev.cfg

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 3

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

commit 4b43f082887590d6bf54685ef1c1c62291e113fc
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Thu Jan 03 22:48:38 2019

[tricium] Temp debugging change: list membership groups

This should have no effect on behavior.

From previous experiments, I saw that checking the groups
 - project-fuchsia-committers,
 - project-chromium-tryjob-access, and
 - all
all seem to work as expected, whereas the groups
 - project-infra-committers, and
 - project-infra-tryjob-access
both seem to never contain qyearsley@chromium.org.

To continue debugging this, as a next step, I plan
to add more whitelisted groups to the playground config
and see which ones seem to work as expected. Maybe
there will be some kind of pattern.

Meanwhile, this change adds extra logging to regular
requests, so it should be reverted when the mystery
is solved.

TBR=maruel
TBR_REASON=temporary change that should not affect behavior

Bug:  908647 
Change-Id: I513e9c6f724d17cf2acfed0e672b0319de60423a
Reviewed-on: https://chromium-review.googlesource.com/c/1391369
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#19766}
[modify] https://crrev.com/4b43f082887590d6bf54685ef1c1c62291e113fc/go/src/infra/tricium/appengine/gerrit/poll.go

Status: WontFix (was: Started)
Vadim just explained what's going on:

> tricium-dev.appspot.com uses chrome-infra-auth-dev for groups (see https://tricium-dev.appspot.com/admin/portal/auth_service)
> 
> chrome-infra-auth-dev doesn't have project-infra-committers or -tryjob-acccess groups. It is completely separate group namespaces.
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 3

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

commit f9f39c382dd6dc302e765b9f9fa89fad7e27d14a
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Thu Jan 03 23:31:04 2019

Revert "[tricium] Temp debugging change: list membership groups"

This reverts commit 4b43f082887590d6bf54685ef1c1c62291e113fc.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> [tricium] Temp debugging change: list membership groups
> 
> This should have no effect on behavior.
> 
> From previous experiments, I saw that checking the groups
>  - project-fuchsia-committers,
>  - project-chromium-tryjob-access, and
>  - all
> all seem to work as expected, whereas the groups
>  - project-infra-committers, and
>  - project-infra-tryjob-access
> both seem to never contain qyearsley@chromium.org.
> 
> To continue debugging this, as a next step, I plan
> to add more whitelisted groups to the playground config
> and see which ones seem to work as expected. Maybe
> there will be some kind of pattern.
> 
> Meanwhile, this change adds extra logging to regular
> requests, so it should be reverted when the mystery
> is solved.
> 
> TBR=maruel
> TBR_REASON=temporary change that should not affect behavior
> 
> Bug:  908647 
> Change-Id: I513e9c6f724d17cf2acfed0e672b0319de60423a
> Reviewed-on: https://chromium-review.googlesource.com/c/1391369
> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
> Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#19766}

TBR=maruel@chromium.org,qyearsley@chromium.org

Change-Id: I549fdc625103d2a47efb32876374033d67e29232
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  908647 
Reviewed-on: https://chromium-review.googlesource.com/c/1395257
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#19768}
[modify] https://crrev.com/f9f39c382dd6dc302e765b9f9fa89fad7e27d14a/go/src/infra/tricium/appengine/gerrit/poll.go

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/playground/gerrit-tricium/+/260d011e463c20849edeb00914942d349c53ccb0

commit 260d011e463c20849edeb00914942d349c53ccb0
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Thu Jan 03 23:53:52 2019

[tricium] Update playground config whitelist, add comment

Bug:  908647 
Change-Id: Id9cf14c1374b7fbcd32889cc50faf6785d0879ae
[modify] https://crrev.com/260d011e463c20849edeb00914942d349c53ccb0/tricium-dev.cfg

Sign in to add a comment