Add govulncheck to CI

[pgporada]

Fixes #6354

Installs govulncheck into a one-shot container so that at PR creation, updates to a PR, and merges to main can contact the govuln API and check for known vulnerabilities.

Lastly, upgrades the version of golangci-lint to the latest available (v1.53.3).

[pgporada]

I need to build new containers before govulncheck actually runs here, but I wanted to discuss the PR first before doing that work. Do we want to run it in offline mode? Should I have a sidecar container that comes online with network access?

$ govulncheck ./...
govulncheck is an experimental tool. Share feedback at https://go.dev/s/govulncheck-feedback.

Using go1.20.5 and govulncheck@v0.0.0 with
vulnerability data from https://vuln.go.dev (last modified 2023-06-26 16:56:57 +0000 UTC).

Scanning your code and 507 packages across 79 dependent modules for known vulnerabilities...
No vulnerabilities found.

[aarongable]

On the whole, I think this approach is fine. It's unfortunate that it only updates the database when we re-run tag_and_upload, but that's still a step up from where we are today. I've left some comments assuming we continue down this path.

That said, there is another possible approach: add a whole new github workflow which just uses the official govulncheck action. It means we can't run it locally as part of t.sh -l, but it also means that we write approximately zero code and it runs against the live database.

[pgporada]

I decided to go the low code route and only run on Github CI. This has the benefit that govulncheck is run against the live vuln database at PR creation, each commit that run tests, and at merge time.

I've changed the way this PR works.

[aarongable]

It doesn't look like govulncheck is running on this PR yet. I thought you were moving the job from govulncheck.yml into boulder-ci.yml; is that commit just missing from the PR?

[pgporada]

Pushed commits are out of sync with what is being displayed in this PR. I pushed this an hour ago, but it's still not showing up here. 6d76aef

[aarongable]

Looks like that github outage left this PR permanently messed up. I'm trying to re-push your same commits to see if we can kick it into working.

[aarongable]

Should the boulder_ci_test_matrix_status job be updated to check the status of the govulncheck jobs as well? Having one of these jobs fail isn't quite the same as having a test fail, but I don't think we want to accidentally submit code which failed these jobs either.

[pgporada]

Yup, that's a good point and I completely failed to see the utility build job in the workflow.

[aarongable]

LGTM, I'm glad we'll be running this now!

Now you have me curious whether its faster to pull the boulder/netaccess docker image (as this workflow does) or to install Go from scratch (as the govulncheck action does). It's a handful of seconds either way, so it doesn't matter compared to our integration tests.

[pgporada]

The govulncheck action runs a setup-go action which cannot retrieve go1.21 release candidates. I wanted to be able to run vulnchecks against the exact versions of go we're using hence the container shenanigans.

[aarongable]

Oh really? I thought it would work when specified like 1.21.0-rc.1, like in the example it gives. Weird that it doesn't. Alrighty then!

[pgporada]

I was wrong, I figured out how to get it to retrieve release candidates. However, something I learned about the govulncheck action proper is that it runs setup-go which doesn't yet have the ability to use an already installed golang and will always install a fresh copy which is wasteful. The implementation in this PR avoids that.

[jsha]

Why combine the rsyslog editing steps with the install-go step? Though I do like combining the different sed invocations into a single invocation.

[pgporada]

RUN, COPY, and ADD create new layers which use some amount of disk space. Having less layers is more efficient disk space-wise. There's further optimizations that can be done with a multi-stage build, but that's outside the scope of this PR. Hell, even this change is now outside the scope of this PR because I changed strategies mid-way through.

[pgporada]

Layers prior to this change

Layers after this change

[jsha]

Good point, and that's why for instance we put a bunch of stuff inside build.sh rather than in RUN commands directly. Perhaps the sed commands should go inside build.sh? That makes more sense to me logically than clustering them with install-go.sh. It would probably be interesting to look at the git history and see if there's a specific reason they're in the Dockerfile rather than build.sh.