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

chore: enhance vuln monitor #8570

Merged

Conversation

ericzzzzzzz
Copy link
Contributor

@ericzzzzzzz ericzzzzzzz commented Mar 22, 2023

Related: #8503

Description

  • update monitor to scan skaffold go binary and skaffold base image vulnerabilities and report to github
  • The monitor scans images built from the HAED of main branch as well as latest LTS version binary, such v2.0.6, v1.39.6 at the moment.
  • The monitor will create/close issues based on the scanning result
    • For edge binary(built from the HEAD of main)
      • If there is an open issue for edge vulns, and the scanning result shows vulns, then there is no action.
      • If there is an open issue for edge vulns, and the scanning result shows no vulns, the script will close the issue.
      • If there is no open issue for edge vulns, and the scanning result shows vulns, then script create an issue to track the problem.
  • For LTS, the scanner always scans the up-to-date LTS although there might be an open issue about vulns found in previous version. For example, there is an open issue for v2.0.x, and we release a new version v2.0.x+1 , the scanner will target for v2.0.x+1, if the new release fixes the issue, the script will just close the original one(v2.0.x), otherwise other than closing the old one, it will also create a new issue to track skaffold vuls issues. If we don't have a new release, then the behavior should be the same as for edge version. Note that, we uses the release time of x.y.1 as the LTS start time, we only scan the latest LTS release if its starting point is within one year.
  • The monitor will ping skaffold-team for every issue it creates to increase awareness.
  • There are currently two triggers : test-os-scanning, test-bin-scanning with linked to my remote branch, after the pr gets merged. we can rename those triggers and set up schedule jobs and switch target branches to main

Test Plan

  • manually run test-bin-scanning build trigger in skaffold project, I tested this morning and related vuls were already created by my run, you can close some of those issues and trigger the build to see the result, this should work as the description.

Monitor Frenquency

  • set up on demand scanning with pub/sub with target images creation events, the images should only be pushed by /deploy/cloudbuild and deploy/cloudbuild-release-lts.
  • set up daily scanning job

@ericzzzzzzz ericzzzzzzz marked this pull request as ready for review March 22, 2023 15:55
@codecov
Copy link

codecov bot commented Mar 22, 2023

Codecov Report

Merging #8570 (ad38d80) into main (290280e) will decrease coverage by 6.16%.
The diff coverage is 54.44%.

❗ Current head ad38d80 differs from pull request most recent head 59c2c13. Consider uploading reports for the commit 59c2c13 to get more accurate results

@@            Coverage Diff             @@
##             main    #8570      +/-   ##
==========================================
- Coverage   70.48%   64.32%   -6.16%     
==========================================
  Files         515      607      +92     
  Lines       23150    30421    +7271     
==========================================
+ Hits        16317    19568    +3251     
- Misses       5776     9387    +3611     
- Partials     1057     1466     +409     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/completion.go 13.04% <0.00%> (-1.25%) ⬇️
cmd/skaffold/app/cmd/config/list.go 65.21% <ø> (ø)
cmd/skaffold/app/cmd/config/set.go 88.72% <ø> (ø)
cmd/skaffold/app/cmd/config/util.go 54.28% <ø> (ø)
cmd/skaffold/app/cmd/credits.go 100.00% <ø> (ø)
cmd/skaffold/app/cmd/credits/export.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go 40.90% <0.00%> (-12.94%) ⬇️
cmd/skaffold/app/cmd/diagnose.go 62.22% <0.00%> (-2.65%) ⬇️
cmd/skaffold/app/cmd/generate_pipeline.go 60.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_modules.go 65.00% <ø> (ø)
... and 38 more

... and 385 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more


set -xeo pipefail
# Variables that will be substituted by trigger configuration or valued provided through command line with --substitutions flag.
if [ -z "$_TAG_FILTER" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the _TAG_FILTER variable used? Might make sense to delete it if not

# We should only scan lts images within 1 year window from the first patch of the release.
targeted_base_tags="$(gcloud container images list-tags "$base_image" --filter="timestamp.datetime > -P1Y AND tags~v.*\.1-lts" --format='value(tags)')"
for line in $targeted_base_tags; do
IFS=',' read -ra t <<< "${line}"
Copy link
Contributor

@aaron-prindle aaron-prindle Mar 24, 2023

Choose a reason for hiding this comment

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

nit: the other IFS call uses double quotes (","), might make sense to use consistent style

Copy link
Contributor

@aaron-prindle aaron-prindle left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants