Skip to content

Conversation

mboersma
Copy link
Contributor

What this PR does / why we need it:

Adds the make verify-security target and runs it on a weekly basis. This is ported from similar code in CAPI and CAPZ.

Which issue(s) this PR fixes:

Fixes #

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 27, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mboersma

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:

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

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 27, 2025
@mboersma mboersma force-pushed the verify-security branch 2 times, most recently from a76f7e6 to 442df18 Compare August 28, 2025 18:00
RELEASE_NOTES_BIN := release-notes
RELEASE_NOTES := $(TOOLS_BIN_DIR)/$(RELEASE_NOTES_BIN)-$(RELEASE_NOTES_VER)

TRIVY_VER := 0.64.0
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't it be nice if there were a way to integrate general purpose tool binary installs into dependabot? seems that's an unsolved problem https://docs.github.com/en/code-security/dependabot/ecosystems-supported-by-dependabot/supported-ecosystems-and-repositories#supported-ecosystems-and-repositories

Copy link
Contributor

Choose a reason for hiding this comment

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

actually for trivy maybe we can use docker run?

(not blocking this PR, just thinking out loud)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for trivy maybe we can use docker run?

Sure, that would probably be cleaner. I was just trying to minimize the delta between these same scripts here and in CAPI and CAPZ.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 8, 2025
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2025
@mboersma mboersma requested a review from Copilot September 30, 2025 19:13
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a weekly security scanning workflow for the Kubernetes cluster-api-helm project. It adds comprehensive security verification including both container image scanning and Go code vulnerability checks.

  • Introduces a new verify-security Makefile target that runs both container image and Go vulnerability scans
  • Refactors Trivy installation to use a reusable script with version-specific caching
  • Replaces the existing weekly scan workflow with a more comprehensive security scanning approach

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
hack/verify-container-images.sh Simplified by delegating Trivy installation to a separate script and fixing variable references
hack/ensure-trivy.sh New script that handles Trivy installation with version-specific caching
Makefile Adds security verification targets and govulncheck tool configuration
.github/workflows/security-scan.yml New comprehensive weekly security scan workflow
.github/workflows/scan.yml Removed old scan workflow (replaced by security-scan.yml)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +45 to +46
TOOL_BIN=hack/tools/bin
mkdir -p ${TOOL_BIN}
Copy link
Preview

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

Variables should be quoted to handle paths with spaces. Change to TOOL_BIN="hack/tools/bin" and mkdir -p "${TOOL_BIN}".

Suggested change
TOOL_BIN=hack/tools/bin
mkdir -p ${TOOL_BIN}
TOOL_BIN="hack/tools/bin"
mkdir -p "${TOOL_BIN}"

Copilot uses AI. Check for mistakes.

@mboersma
Copy link
Contributor Author

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants