Skip to content

Conversation

lentzi90
Copy link
Contributor

@lentzi90 lentzi90 commented May 9, 2025

This adds a new make target "verify-security" that runs govulncheck agains the code base and also builds the contaier image and scans it with trivy. I tried keeping things similar to how they are done elsewhere in the code. This is all based on how they do it in Cluster API (hence the copyright text in the scripts).

Probably the most controversial thing here is that I added the GO_VERSION variable. The idea is to make sure we know what exact version is used.

@github-actions github-actions bot added the semver:patch No API change label May 9, 2025
@lentzi90
Copy link
Contributor Author

lentzi90 commented May 9, 2025

Oh I need to include the go version in the e2e image build also

@lentzi90 lentzi90 force-pushed the lentzi90/security-scan branch from 15b9513 to 9c6a34c Compare May 9, 2025 15:48
Copy link
Collaborator

@mandre mandre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

This is in line with kubernetes-sigs/cluster-api-provider-openstack#2536, which provides a bit more context.

Just ran this locally, and noticed it produces a warning about possible invalid image when GO_VERSION is unset. Perhaps that's something we would like to fix.

Dockerfile Outdated
@@ -1,5 +1,6 @@
# Build the manager binary
FROM golang:1.23 AS builder
ARG GO_VERSION
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to set a default value there to prevent the warning?
https://docs.docker.com/reference/dockerfile/#default-values

 1 warning found (use docker --debug to expand):
 - InvalidDefaultArgInFrom: Default value for ARG golang:${GO_VERSION} results in empty or invalid base image name (line 3)

Even if we expect to always build the image via make, which sets a default value, it probably wouldn't hurt to set a default here too, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense. I'll update and probably push a PR to CAPI/CAPO also since they both get the same warning

This adds a new make target "verify-security" that runs govulncheck agains the code base
and also builds the contaier image and scans it with trivy.
I tried keeping things similar to how they are done elsewhere in the code.
This is all based on how they do it in Cluster API (hence the copyright text in the scripts).

Probably the most controversial thing here is that I added the GO_VERSION variable. The idea is to make sure we know what exact version is used.

Signed-off-by: Lennart Jern <[email protected]>
@lentzi90 lentzi90 force-pushed the lentzi90/security-scan branch from 9c6a34c to 810a068 Compare May 12, 2025 12:30
@mandre mandre merged commit 89e0e1e into k-orc:main May 12, 2025
8 checks passed
@lentzi90 lentzi90 deleted the lentzi90/security-scan branch May 12, 2025 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch No API change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants