Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify and speed up the clamav build. #1416

Merged
merged 3 commits into from
Jun 24, 2024
Merged

Simplify and speed up the clamav build. #1416

merged 3 commits into from
Jun 24, 2024

Conversation

sengi
Copy link
Contributor

@sengi sengi commented Jun 21, 2024

  • Use govuk-ruby-builder for clamav. Simplifies the Dockerfile by eliminating boilerplate and speeds up the build by not having to install a bunch of stuff that already ships with govuk-ruby-builder.
  • Don't run the clamav Python testsuite; it's super slow and we're building from an official source tarball with a very common config, so it's not worth blowing up asset-manager's build time.
  • Add a crude but fast and somewhat effective smoke test for the two clam binaries that asset-manager needs.
  • Use the system Rust instead of faffing about installing it ourselves, since we don't anticipate any need to run on PowerPC any time soon.
  • Remove a bunch of unneeded Apt packages.
  • Remove some unneeded workarounds, e.g. install the clam binaries in the desired directory (which happens to be the default one from the CMakelists.txt anyway) rather than symlinking them afterwards.
  • Remove some cough GPLed code cough that was mistakenly copy-pasted into the Dockerfile of our MIT-licensed codebase here.
  • Use a valid command for clamdscan so that it works by default rather than requiring brittle configuration on deployment.

Tested: built and ran locally with Docker Desktop, copied configs into /usr/local/etc/*clam*.conf, successfully ran freshclam and scanned some files with clamdscan connecting to clamd via TCP on 127.0.0.1 (which is how we do it in prod).

Rollout: needs manual e2e testing in integration or staging before pushing to prod. (Done — successfully created a document with at attachment and an image in integration via whitehall-admin).

- Use govuk-ruby-builder for clamav. Simplifies the Dockerfile by
  eliminating boilerplate and speeds up the build by not having to
  install a bunch of stuff that already ships with govuk-ruby-builder.
- Don't run the clamav Python testsuite; it's super slow and we're
  building from an official source tarball with a very common config,
  so it's not worth blowing up asset-manager's build time.
- Add a crude but fast and farly effective smoke test for the two clam
  binaries that asset-manager needs.
- Use the system Rust instead of faffing about installing it ourselves,
  since we don't antipcate any need to run on PowerPC any time soon.
- Remove a bunch of unneeded Apt packages.
- Remove some unneeded workarounds, e.g. install the clam binaries in
  the desired directory (which happens to be the default one from the
  CMakelists.txt anyway) rather than symlinking them afterwards.
- Remove some (cough) [GPLed code] (cough) that was mistakenly
  copy-pasted into the Dockerfile of our MIT-licensed codebase here.

[GPLed code]: https://www.github.com/Cisco-Talos/clamav-docker/blob/bf4c0c2/clamav/1.3/debian/Dockerfile

Tested: built locally with Docker Desktop, copied some lightly-modified default configs into /usr/local/etc/*clam*.conf, successfully ran freshclam and scanned some files with clamdscan connecting to clamd via TCP on 127.0.0.1 (which is how we do it in prod).
This eliminates some unnecessary config from govuk-helm-charts and makes
asset-manager more robust against the clam binaries moving to a slightly
different directory in PATH (which was the cause of a recent production
outage).

Remove a unhelpful change-detector test for the default value.
It passes now, but it's still ugly.
@sengi sengi merged commit 65aaa64 into main Jun 24, 2024
9 checks passed
@sengi sengi deleted the clam branch June 24, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants