New issue
Advanced search Search tips

Issue 863631 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 15
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-08-15
OS: ----
Pri: 2
Type: Feature

Blocked on:
issue 872500



Sign in to add a comment

Chromium CQ should not submit private changes

Project Member Reported by aga...@chromium.org, Jul 13

Issue description

We know that the CQ needs to be able to *try* private changes -- so that someone doing security work can do dry runs, and so that we don't pre-emptively open up a private change before a failed CQ run which then sits there all weekend.

But we shouldn't (I believe, and CrOS just made this policy change as well) have any merged-and-yet-private-anyway changes.

Since I don't believe it's a good idea for the CQ to get the admin privileges necessary to remove the Private bit from changes, it should instead refuse to make the Submit API call for changes which are Private, and instead post an explanatory message to that change.


Note that this would likely break the v8 autoroller, which just *loves* to submit private changes for some reason.

https://chromium-review.googlesource.com/q/is:private+status:merged
 
Labels: -Type-Bug Type-Feature
Owner: tandrii@chromium.org
Status: Assigned (was: Untriaged)
agree. will need psa if intent, then waiting period while CQ posts a warning but still submits private cls, and a final psa+deployment.
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 8

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/be7998de32beedd5d7e97faa5892c625603c57b1

commit be7998de32beedd5d7e97faa5892c625603c57b1
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Wed Aug 08 21:57:51 2018

Labels: -OS-Chrome
Status: Started (was: Assigned)
PSAs sent: 

public: https://groups.google.com/a/chromium.org/forum/#!topic/infra-dev/x8Q1Qs_oGKk
(google internal http://shortn/_gLh1pinR3W)
>> Note that this would likely break the v8 autoroller, which just *loves* to submit private changes for some reason.

git-blame is your friend :-). Found this conversation between machenbach and agable: https://chromium-review.googlesource.com/c/chromium/tools/build/+/579989/4/scripts/slave/recipes/v8/auto_tag.py#125. But then, we land such CLs with git-cl-land bypassing CQ. I assume this won't break, right?
>  But then, we land such CLs with git-cl-land bypassing CQ. I assume this won't break, right?

Yep.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 8

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/df92af40eea5f16d3b8c7cc01bbc899eeece4d50

commit df92af40eea5f16d3b8c7cc01bbc899eeece4d50
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Wed Aug 08 22:36:24 2018

Blockedon: 872500
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 9

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/523dcd0c3ebd29bc23156124b0b73b4a069df3bd

commit 523dcd0c3ebd29bc23156124b0b73b4a069df3bd
Author: Andrii Shyshkalov <tandrii@google.com>
Date: Thu Aug 09 19:56:26 2018

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 9

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/5cdcdc42f80b2556d8273c7f8b485575359e36fc

commit 5cdcdc42f80b2556d8273c7f8b485575359e36fc
Author: Andrii Shyshkalov <tandrii@google.com>
Date: Thu Aug 09 19:56:46 2018

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/df50c3c812bb909333e48ac1767168a54341510f

commit df50c3c812bb909333e48ac1767168a54341510f
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Fri Aug 10 00:41:04 2018

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/puppet/+/18accdc24366451d64c1b0a76cd0b981131089c5

commit 18accdc24366451d64c1b0a76cd0b981131089c5
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Fri Aug 10 01:04:35 2018

Finally released CQ which sends warnings. Was delayed by  issue 872500 
NextAction: 2018-08-15
Next week, I'll finish this.
The NextAction date has arrived: 2018-08-15
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 15

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/33d8ac03858557d39f92f938d8c1effa62781a16

commit 33d8ac03858557d39f92f938d8c1effa62781a16
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Wed Aug 15 18:35:08 2018

PSAs updated. Canary deployed. Final test before prod release: 
https://chrome-internal-review.googlesource.com/c/infra/infra_internal/+/663036
Project Member

Comment 18 by bugdroid1@chromium.org, Aug 15

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/puppet/+/dae9a49c22251649ffc09927f8d8ccde8c8e6662

commit dae9a49c22251649ffc09927f8d8ccde8c8e6662
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Wed Aug 15 18:49:58 2018

Status: Fixed (was: Started)
Release finished.

Sign in to add a comment