KEP-2290: New label for trusted PR identification

Implementation History
STABLE Implemented
Created 2018-06-25
Updated 2021-08-16
Latest v1.13
Milestones
Alpha v1.13
Beta v1.13
Stable v1.14
Ownership
Owning SIG
SIG Testing
Participating SIGs
SIG Contributor Experience
Primary Authors
matthyx

New label for trusted PR identification

Table of Contents

Summary

This document describes a major change to the way the trigger plugin determines if test jobs should be started on a pull request (PR).

We propose introducing a new label named ok-to-test that will be applied on non-member PRs once they have been /ok-to-test by a legitimate reviewer.

Motivation

PR test jobs are started by the trigger plugin on trusted PR events, or when a untrusted PR becomes trusted.

A PR is considered trusted if the author is a member of the trusted organization for the repository or if such a member has left an /ok-to-test command on the PR.

It is easy spot an untrusted PR opened by a non-member of the organization by its needs-ok-to-test label. However the contrary is difficult and involves scanning every comment for a /ok-to-test, which increases code complexity and API token consumption.

Goals

This KEP will only target PRs authored from non-members of the organization:

  • introduce a new ok-to-test label
  • modify /ok-to-test command to apply ok-to-test on success
  • modify trigger plugin and other tools to use ok-to-test for PR trust
  • support /ok-to-test calls inside review comments

Non-Goals

This KEP will not change the current process for members of the organization.

Proposal

We suggest introducing a new label named ok-to-test that would be required on any non-member PR before automatic test jobs can be started by the trigger plugin.

This label will be added by members of the trusted organization for the repository using the /ok-to-test command, detected with a single GenericCommentEvent handler on corresponding events (issue_comment, pull_request_review, and pull_request_review_comment).

Implementation Details/Notes/Constraints

  1. PR: declare ok-to-test
    • add ok-to-test to label_sync
  2. (custom tool needed) batch add ok-to-test label to non-members trusted PRs
    • for all PR without ok-to-test or needs-ok-to-test
    • if author is not a member of trusted org
    • add ok-to-test
  3. documentation updates
    • edit all documentation references to needs-ok-to-test
  4. other cookie crumbs updates
  5. PR: switch to ok-to-test
    • remove needs-ok-to-test from missingLabels in prow/config.yaml
    • edit prow/config/jobs_test.go
    • edit prow/cmd/deck/static/style.css
    • edit prow/cmd/tide/README.md
    • code changes in trigger:
      • /ok-to-test adds ok-to-test
      • PR trust relies on ok-to-test
      • if PR has both labels, drop needs-ok-to-test
      • edit all references to needs-ok-to-test
  6. run batch job again, to catch new PRs that arrived between first run and merge/deploy
  7. (to be discussed) periodically check for and report PRs with both ok-to-test and needs-ok-to-test labels

Benefits

  • Trusted PRs are easily identified by either being authored by org members, or by having the ok-to-test label.
  • Race conditions can no longer happen when checking if a PR is trusted.
  • API tokens are saved by avoiding listing the comments, reviews, and review comments every time we need to check if a PR is trusted.

Risks and Mitigations

N/A

Graduation Criteria

Alpha:

  • /ok-to-test workflow live

Beta:

  • /ok-to-test workflow fully documented
  • Comment parsing for /ok-to-test removed in favor of ok-to-test label presence

Stable:

  • /ok-to-test workflow used without incident for more than 6 months

Future evolutions

In the future, we might decide to require the new label for all PRs, which means that organization members will also need the ok-to-test label applied to their PRs before automatic testing can be triggered.

Trusted and untrusted PRs will be even easier to tell apart.

This would require adding automatically the ok-to-test label to member authored PRs to keep the current functionality.

References

Implementation History

  • 2018-06-25: creation of the KEP
  • 2018-07-09: KEP content LGTM during sig-testing presentation
  • 2018-07-24: KEP updated to keep needs-ok-to-test for better UX
  • 2018-09-03: KEP rewritten with template
  • 2018-10-04: KEP merged into master
  • 2018-10-08: start of implementation
  • 2018-10-10: ok-to-test label added
  • 2019-01-29: remove comment parsing code for PR trust
  • 2021-08-16: Retroactive stable declaration