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

feat: monitor OS vulnerability in Skaffold LTS images. #6964

Merged
merged 4 commits into from
Jan 5, 2022

Conversation

ChrisGe4
Copy link
Collaborator

Add a Cloud Build job to monitor the vulnerabilities that are found in the LTS images.

Reference: go/cd-lts-image-security.

@codecov
Copy link

codecov bot commented Dec 13, 2021

Codecov Report

Merging #6964 (82424d1) into main (290280e) will decrease coverage by 1.71%.
The diff coverage is 56.70%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6964      +/-   ##
==========================================
- Coverage   70.48%   68.77%   -1.72%     
==========================================
  Files         515      551      +36     
  Lines       23150    25370    +2220     
==========================================
+ Hits        16317    17447    +1130     
- Misses       5776     6740     +964     
- Partials     1057     1183     +126     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/deploy.go 52.00% <ø> (-1.85%) ⬇️
cmd/skaffold/app/cmd/dev.go 84.61% <0.00%> (ø)
cmd/skaffold/app/cmd/render.go 36.66% <0.00%> (-4.72%) ⬇️
cmd/skaffold/skaffold.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_tests.go 62.50% <14.28%> (-1.14%) ⬇️
cmd/skaffold/app/cmd/lsp.go 28.12% <28.12%> (ø)
cmd/skaffold/app/cmd/fix.go 68.85% <40.00%> (-7.62%) ⬇️
cmd/skaffold/app/cmd/lint.go 42.85% <42.85%> (ø)
cmd/skaffold/app/cmd/find_configs.go 48.88% <50.00%> (+0.24%) ⬆️
cmd/skaffold/app/skaffold.go 76.19% <70.00%> (-8.43%) ⬇️
... and 175 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 248b74e...82424d1. Read the comment docs.

@ChrisGe4 ChrisGe4 changed the title Monitor OS vulnerability in Skaffold LTS images. feat: monitor OS vulnerability in Skaffold LTS images. Dec 13, 2021
steps:
- id: Get github token.
name: gcr.io/cloud-builders/gcloud
entrypoint: 'bash'
Copy link
Member

Choose a reason for hiding this comment

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

Should use /bin/bash to be consistent with line 8

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

check_existing_issue() {
label=$1
if [ $(gh issue list --label="$label" --repo="$_REPO" | wc -c) -ne 0 ]; then
echo "There is already an issue opened for the vulnerabilities that are found in the LTS images." && exit 0
Copy link
Member

Choose a reason for hiding this comment

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

What if there are new, more severe, vulnerabilities reported?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The github issue only serves as a reminder for skaffold team to look up the vulnerabilities in the report that is created by container analysis.

Copy link
Member

Choose a reason for hiding this comment

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

Except that you're listing the images with vulnerabilities, and a new vulnerability could have been discovered in a different image.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added the code to update the existing issue.

deploy/lts-vuln-monitor/report.sh Outdated Show resolved Hide resolved
deploy/lts-vuln-monitor/scan.sh Outdated Show resolved Hide resolved
deploy/lts-vuln-monitor/scan.sh Outdated Show resolved Hide resolved
# If changed, also change the same variable in report.sh.
OS_VULN_FILE=/workspace/os_vuln.txt
GREP_TEMPLATE="-e "
append() {
Copy link
Member

Choose a reason for hiding this comment

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

Feels like there should be a blank line to separate the unrelated variables from this function

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

deploy/lts-vuln-monitor/scan.sh Outdated Show resolved Hide resolved
deploy/lts-vuln-monitor/scan.sh Outdated Show resolved Hide resolved
deploy/lts-vuln-monitor/cloudbuild.yaml Outdated Show resolved Hide resolved

set -x

# Variables that will be substituted in cloudbuild.yaml.
Copy link
Member

Choose a reason for hiding this comment

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

Note that these substitutions must be explicitly propagated into the environment with an env: section on a step in the cloudbuild.yaml

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah, I only tested with default values before. Added the substitutions section.

@tejal29
Copy link
Member

tejal29 commented Dec 15, 2021

Thanks. This is looking pretty solid.

  1. Can you fix the CLA bot issue. https://github.com/GoogleContainerTools/skaffold/pull/6964/checks?check_run_id=4525719034

  2. Can you link a sample build job?

@ChrisGe4
Copy link
Collaborator Author

ChrisGe4 commented Dec 15, 2021

Thanks. This is looking pretty solid.

  1. Can you fix the CLA bot issue. https://github.com/GoogleContainerTools/skaffold/pull/6964/checks?check_run_id=4525719034
  2. Can you link a sample build job?
  1. Resolved.
  2. A job triggered from my test project with the code in this repo:https://pantheon.corp.google.com/cloud-build/builds;region=global/fe415473-1bb1-4b01-bf83-f0b317ef6262?project=cd-demo-01 Log: https://paste.googleplex.com/6644379658747904

@tejal29
Copy link
Member

tejal29 commented Dec 16, 2021

  1. A job triggered from my test project with the code in this repo:https://pantheon.corp.google.com/cloud-build/builds;region=global/fe415473-1bb1-4b01-bf83-f0b317ef6262?project=cd-demo-01

Can you copy the logs and share a gpaste/ link?
I don't have access to see the build log.

@tejal29 tejal29 added the kokoro:force-run forces a kokoro re-run on a PR label Dec 16, 2021
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Dec 16, 2021
@ChrisGe4
Copy link
Collaborator Author

  1. A job triggered from my test project with the code in this repo:https://pantheon.corp.google.com/cloud-build/builds;region=global/fe415473-1bb1-4b01-bf83-f0b317ef6262?project=cd-demo-01

Can you copy the logs and share a gpaste/ link? I don't have access to see the build log.

I have added the gpaste link.

@tejal29
Copy link
Member

tejal29 commented Jan 5, 2022

Nice! looks like issues are getting created here https://github.com/ChrisGe4/cd-lts-test/issues

@tejal29
Copy link
Member

tejal29 commented Jan 5, 2022

Address google cloud API warning and then LGTM!.

Thank you for your patience.

Step #1 - "Check vulnerability report.": WARNING: --filter : operator evaluation is changing for consistency across Google APIs.  tags:v*lts currently matches but will not match in the near future.  Run `gcloud topic filters` for details.

@tejal29 tejal29 enabled auto-merge (squash) January 5, 2022 17:26
@briandealwis briandealwis added the kokoro:run runs the kokoro jobs on a PR label Jan 5, 2022
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jan 5, 2022
@tejal29 tejal29 merged commit e15fca6 into GoogleContainerTools:main Jan 5, 2022
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

4 participants