Make govulncheck only run on release branches

[AndrewSirenko]

Is this a bug fix or adding new feature?

N/A

What is this PR about? / Why do we need it?

This makes the govulncheck github action only run on PRs merging into release branches:

• New vulnerability blocks the merge of a new contributor, requiring a rebase. This is not a great contributor experience and has led to delays due to PR churn.

The risk here is that we suddenly find out about a vulnerability for our release PR, instead of in any pr, which may delay our release by 1-3 hours (due to waiting for CI to re-run). But that should be rare since we are bumping dependencies right before a release anyway.

What testing is done?

CI

[ConnorJC3]

From what I can tell reading the GitHub docs, this will make it impossible for the job to fail at all, which means that we will not get the feedback unless someone goes out of their way to read the job results.

It's possible that I am wrong, but please test this in a personal fork and link example PRs here so that we can see what the result looks like before merging.

[AndrewSirenko]

Thank you for your concern @ConnorJC3, but please hold off on review until it is no longer marked WIP because I have yet to do the testing that I described in PR comment.

What I saw from the docs and relevant stack overflow posts was that the job will fail with a red X, the failure will not block other CI actions for the PR. So there will be a failure shown without blocking merge, but this must be validated.

If a particular step was marked as optional, what you said would be correct.

[AndrewSirenko]

Looks like from GitHub's point of view, maintainers can merge if govulncheck action fails because branch protection rules do not include it as a required check.

However from PROW's POV, tide will block merges because it implicitly sets all actions as required for merge (See #2139).

We may be able to exclude this action in the prow config, which would mean we get best of both worlds (clear red x to signify govulncheck failed, but our ci wouldn't be blocked).

If changing prow config is not desired, we can run this job only on release-branches as to not block new maintainers.

---

Either way, we can consider updating our default branch protection rules to include most of the other status checks, including tide, if we want to ensure maintainers cannot merge without PROW.

[ConnorJC3]

My suggestion would be to run it only on the release branch. If we make it optional on Prow, we are going to eventually accidentally merge a release with a known CVE.

There's no circumstance where we would want to intentionally merge a release PR containing known vulnerabilities, so the only potential downside of this solution is false positives. False positives are very rare (I think we've had one over the past year?) and the only reason we're talking about them is recency bias. Additionally, it's trivial to disable the job if that ever happens during a release.

[AndrewSirenko]

/unhold

[ConnorJC3]

/lgtm

[torredil]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: torredil

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [torredil]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment