On 22 Feb 2017, at 10:45, Martin Sivak <msivak(a)redhat.com>
wrote:
> Just think about the goals we have
> (like getting a real developer community) while fixing it.
Btw, I just found a project that uses whitelist - kubernetes. But they
make it very easy for maintainers to accept patches. Check for example
this pull request:
https://github.com/kubernetes/kubernetes/pull/41119
The idea I like is that the CI will immediately tell me what needs to
be done in the comments. No need to search through documentation for
the right keyword.
- The bot told the contributor who the relevant maintainers are,
computed the list of people with enough permissions and notified them
too. And just a comment by maintainer was needed to enable CI for the
pull request.
- The same for failing tests - they provide the command to retrigger
it directly in the message
It all basically boils down to "Do not make me think" approach.
I agree with everything you said (in this thread, that is;-), thanks.
It’s more like “Do not force me to go through email archives and waste more time than it
took me to fix the damn code” approach
Everything that needs to be done is immediately obvious without
searching. And that I could accept even with the whitelist enabled.
Martin
On Tue, Feb 21, 2017 at 9:45 PM, Martin Sivak <msivak(a)redhat.com> wrote:
> Hi,
>
> now the longer version..
>
>> We (the infra team) know that the current mechanism can sound
>> draconian and be inconvenient. But we had to put something in place
>> quickly. Please join the discussion at [1] to improve it.
>>
>> [1]:
https://ovirt-jira.atlassian.net/browse/OVIRT-1154
>
> The JIRA issue deals with automating the whitelist. But the whitelist
> by itself is a wrong solution. Let me explain why I think we should do
> stuff (very) differently:
>
> 1) automation directory is part of sources
>
> This allows any pull request to configure the CI scripts for itself,
> that is indeed dangerous and is one of the reasons why I do not like
> it (Travis has the same issue). The automation (and build matrix)
> configuration should be limited to maintainers which means we need a
> separate repository with project configuration for it (I keep telling
> you that something distgit-like is what we need)
>
> 2) no patch can be merged without CI approval and maintainers are
> commonly not even looking at patches without CI+ flag
>
> The whitelist will create another barrier to entry for anybody wanting
> to contribute just a random patch or two. Most of my contributions to
> other project fall exactly in that category - fixing the one thing
> that bothers me and moving on. Our code presents a barrier that is
> already high enough for that and making it even higher won't win us
> any more contributors.
>
> 3) most projects I know about limit what you can do during build or
> use monitoring and blacklists
>
> Fedora does not allow you to access network during builds at all, copr
> does, but you can be banned. The same Git Hub / Travis where you are
> allowed to do anything, but you can be banned for abusing the system.
> You do not have to be whitelisted first to configure Travis CI or to
> submit a copr job. Fedora used to require a sponsor and a GPG signed
> CLA, but the requirement was removed. We are again moving against the
> crowd here.
>
>
>
> Unfortunately I really feel we are again hacking around something and
> repeating other's mistakes instead of designing a proper solution
> while learning from others. But the current CI was indeed a security
> hole, there is no questioning that. Just think about the goals we have
> (like getting a real developer community) while fixing it.
>
> Regards
>
> Martin Sivak
> oVirt / SLA
>
> On Tue, Feb 21, 2017 at 6:53 PM, Barak Korren <bkorren(a)redhat.com> wrote:
>> TL;DR: If you are a patch author, please ask your project maintainer
>> to have you added to the CI whitelist.
>>
>> The oVirt CI system tries to be a useful and powerful tool for patch
>> authors and project maintainers. With the current CI-standards, power
>> is placed in the hands of patch authors to make the system do almost
>> anything.
>>
>> We try to be an "open" Open-Source project, so permission is given to
>> anyone to open a Gerrit account and submit patches.
>>
>> The above opens the door to the CI system being maliciously exploited,
>> so some countermeasure was needed. The builders of the CI system
>> foresaw this and have put in place a white-list mechanism that makes
>> the CI system only run jobs for patches that come from listed authors.
>>
>> In recent years the CI system had been re-engineered with the push
>> towards the CI standards, and the whitelist mechanism was rendered
>> non-active.
>>
>> Finally realizing the potential for harm, we acted quickly to
>> re-enable the whitelist mechanism. We made an effort to include
>> current authors and maintainers, but we do not know everyone. People
>> not included in the whitelist will see the following message when
>> submitting patches:
>>
>> To avoid overloading the infrastructure, a whitelist for
>> running gerrit triggered jobs has been set in place, if
>> you feel like you should be in it, please contact infra at
>> ovirt dot org
>>
>> If you come across this message, please ask the project maintainer to
>> send a message to the infra team asking for you to added to the CI
>> whitelist. We'd rather not receive direct messages from individual
>> contributors, because we do not know everyone and cannot verify
>> sources.
>>
>> We (the infra team) know that the current mechanism can sound
>> draconian and be inconvenient. But we had to put something in place
>> quickly. Please join the discussion at [1] to improve it.
>>
>> [1]:
https://ovirt-jira.atlassian.net/browse/OVIRT-1154
>>
>> --
>> Barak Korren
>> bkorren(a)redhat.com
>> RHCE, RHCi, RHV-DevOps Team
>>
https://ifireball.wordpress.com/
>> _______________________________________________
>> Devel mailing list
>> Devel(a)ovirt.org
>>
http://lists.ovirt.org/mailman/listinfo/devel
_______________________________________________
Devel mailing list
Devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel