CI: should not keep the PR 'ok-to-test' labeled

[wainersm]

With the recent changes in our e2e jobs, a committer/maintainer (write access) should label the pull request with 'ok-to-test' otherwise the jobs aren't triggered. This is required to avoid running pull requests when they are opened and so preventing a malicious actor from leaking secrets and/or exploiting the runners.

There is a security flaw with the label approach which is the pull request is left labeled after the first execution; so a malicious update to the same pull request will be able to trigger the workflows again.

@portersrc and @ldoktor proposed slightly different solution for this problem in our community meeting. Mind to share it here please?

Cc @fidencio @sprt as likely a similar fix should be done on kata CI
Cc @stevenhorsman to remember me that we should fix it on cloud-api-adaptor too :)
Cc @mkulke for awareness and in case you want to propose something different. Perhaps on long term we could adopt a bot?

[sprt]

I agree automatically removing the label on push would be more secure but be mindful of the usability trade-off: this will add friction for newcomers and increase maintainers' workload. Still a +1 from me, but because of this I would also have a process where we can easily give labeling permissions to trusted newcomers - is this possible through GH team permissions? ok-to-test was especially painful for me when I didn't have contributor permissions yet and was already working on CI: every little change took forever to get validated, and sometimes it's not possible to test locally.

[ldoktor]

Well I can see several variants that can combine:

1. define tiers (GH runners would run always, behind-the-door ones would require ack)
2. define label-groups (first-time contributors would have to wait for acks)
3. come-up with a logic to detect changes that would be auto-acked (not the same level of security but some changes should be fairly safe especially if we only allow that for regular contributors - step no. 2)

As for me I'd suggest going with required ok-to-test, maybe with a few tests that would be always allowed or would be allowed to be triggered by anyone.

Note if we go with multiple tears then it'd be nice to add a chatbot that would explicitly mention that on a PR (similarly to https://github.com/openshift/release repo where one knows exactly what to comment in order to get to the desired state).

[portersrc]

I'm favoring option (1) (tiers with GH runners always running and self-hosted runners requiring an ACK). For the ACK, I prefer the trigger to be a comment (rather than constantly adding and auto-removing labels).

The work required here is to enforce these tiers in the workflows. I'm also wondering where self-hosted runners are actually needed in the next CI (vs. where it's overkill).