On 5/1/24 14:33, Eelco Chaudron wrote:
> 
> 
> On 29 Apr 2024, at 14:59, Ilya Maximets wrote:
> 
>> On 4/16/24 09:44, Eelco Chaudron wrote:
>>> This patch adds a daily Coverity run for the OVS main branch
>>> to the GitHub actions. The result of the runs can be found here:
>>>
>>>  https://scan.coverity.com/projects/openvswitch
>>>
>>> Before applying, we need to add the following two actions secrets
>>> to the GitHub openvswitch project:
>>>
>>> - COVERITY_SCAN_TOKEN; The secret token from the project page
>>> - COVERITY_SCAN_EMAIL; The maintainer's email alias
>>>
>>> Signed-off-by: Eelco Chaudron <echau...@redhat.com>
>>> ---
>>>  .github/workflows/coverity.yml | 131 +++++++++++++++++++++++++++++++++
>>>  Makefile.am                    |   1 +
>>>  README.rst                     |   2 +
>>>  3 files changed, 134 insertions(+)
>>>  create mode 100644 .github/workflows/coverity.yml
>>
>> Thanks, Eelco for bringing this up.  Though, continuing the
>> offline discussion, I'm not sure having coverity scans as
>> a GitHub action has any benefits for OVS community.
>>
>> A few issues:
>>
>> 1. Reports are not available to people outside of the OVS
>>    Coverity project.  Adding everyone there doesn't seem
>>    reasonable.  Also a potential security concern.
> 
> I do not see a security concern, as anyone can set up the coverity scan
> on his fork of the project and has all the insights. I do agree that we
> should not try to add everybody to the Coverity project, and this might
> be more for maintainers (who can opt-in to get emails on new issues).

Fair, not a security concern indeed.  But it's still not nice that
reports are not publicly available.

> 
>> 2. Workflow cannot be used in forks due to organization secrets.
>>    (IIUC, current implementation will just fail once a day
>>    annoying people who didn't disable that workflow.)
> 
> I guess this can be fixed, we could check for the variables. This way if
> someone wants to add Coverity scans to his fork, all they need to do is
> add the secrets.
> 
>> 3. Necessity to have secrets in the organization.  We currently
>>    don't have any, and from a security standpoint not having
>>    any secrets is always better.
> 
> Is having the secrets in a 3rd party located robot more secure?

It's not more secure per se, but it makes security concerns someone
else's problem. :)  GitHub been leaking secrets multiple times in the
past few years.  And robot likely has lower attack surface.  At least
we don't need to care about executing binary blobs downloaded from the
internet in the context of the main OVS organization - robot can be
better isolated.  Also, using non-official actions from some random
person is a bit sketchy.

> 
>> 4. A decent amount of code duplication for this workflow.
>>    (Reusable workflows maybe?)
> 
> Yes, I can take a look at composition in GitHub actions, as there is
> no real ‘include’ like feature.

There are 'reusable workflows', but they are a not very intuitive.

> 
>> In its current form, I think, the same functionality can be
>> provided by the 0-day robot that could submit sources once
>> a day for the scan.  And it could also send some emails with
>> build details if necessary (again, there may be some security
>> concerns in case coverity discovers a genuine security issue).
> 
> Guess results are only available in the GUI and through an email,
> don’t think you can query new items through an API.
> 
> @Aaron do you know how this is handled for DPDK?
> 
>> With the total amount of code in the project, IIUC, we could
>> submit up to 21 builds a week with no more than 3 per day
>> (Coverity claims that it checked just under 300K lines in OVS).
>> So, technically, we could create a bit more complex logic
>> for 0-day bot to run a check per patch set (probably, not per
>> patch though).  In most cases we would fit within the limit
>> with the current patch rate.  Maybe reserving one run a week
>> for the main branch.  Such logic would be harder to implement
>> with GHA.
>>
>> All in all, using GHA just for scheduling of a job that wider
>> community can't take advantage of seems a little unjustified.
>>
>> Thoughts?
> 
> I think it would benefit the maintainers to start with, and if implemented
> right people who fork should not see any side effects.

There will be no difference in this aspect if implemented as part
of the 0day robot. 

> In addition, it also
> enables them to integrate Coverity for their own fork if they want to.
> 
> Others?
> 
> //Eelco
> 

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to