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

Improve the trivy CVE scanning output #2764

Conversation

hansinikarunarathne
Copy link
Member

Pull Request Template for Kubeflow manifests Issues

  • Please include a summary of changes and the related issue.
  • List any dependencies that are required for this change.
  • Please delete the options that are not relevant.
  • The following checklist will help you to satisfy the requirements.

✏️ A brief description of the changes

Fixed the requested chnages

✅ Contributor checklist


You can join the CNCF Slack and access our meetings at the Kubeflow Community website. Our channel on the CNCF Slack is here #kubeflow-platform.

Signed-off-by: hansinikarunarathne <107214435+hansinikarunarathne@users.noreply.github.com>
@juliusvonkohout
Copy link
Member

juliusvonkohout commented Jun 25, 2024

Please also get rid of the /documents AND /image_lists folderS in this PR and address all comments from the previous PR.

Signed-off-by: hansinikarunarathne <107214435+hansinikarunarathne@users.noreply.github.com>
@juliusvonkohout
Copy link
Member

juliusvonkohout commented Jun 25, 2024

I see for example

File ../image_lists/kf_latest_model-registry_images.txt successfully created
File ../image_lists/kf_latest_all_images.txt successfully created
Started scanning images
Scanning images in ../image_lists/kf_latest_workbenches_images.txt
Scanning centraldashboard:v1.9.0-rc.0

Scanning jupyter-web-app:v1.9.0-rc.0

...

can we have something like

Scanning images in ../image_lists/kf_latest_workbenches_images.txt
Scanning centraldashboard:v1.9.0-rc.0

Critical High Medium Low
1 2 3 4

Scanning jupyter-web-app:v1.9.0-rc.0

Critical High Medium Low
5 6 7 8

...

? So a mini table per image in the logs. This makes it easier to verify the numbers for third parties.

@juliusvonkohout juliusvonkohout changed the title made fix to maintain the correct order of logs Imrpove the trivy CVE sanning output Jun 25, 2024
@juliusvonkohout juliusvonkohout changed the title Imrpove the trivy CVE sanning output Improve the trivy CVE scanning output Jun 25, 2024
Signed-off-by: hansinikarunarathne <107214435+hansinikarunarathne@users.noreply.github.com>
Signed-off-by: hansinikarunarathne <107214435+hansinikarunarathne@users.noreply.github.com>
@hansinikarunarathne
Copy link
Member Author

Please also get rid of the /documents AND /image_lists folderS in this PR and address all comments from the previous PR.

I fixed it. Can you check it ?

@hansinikarunarathne
Copy link
Member Author

hansinikarunarathne commented Jun 25, 2024

I see for example

File ../image_lists/kf_latest_model-registry_images.txt successfully created File ../image_lists/kf_latest_all_images.txt successfully created Started scanning images Scanning images in ../image_lists/kf_latest_workbenches_images.txt Scanning centraldashboard:v1.9.0-rc.0

Scanning jupyter-web-app:v1.9.0-rc.0

...

can we have something like

Scanning images in ../image_lists/kf_latest_workbenches_images.txt Scanning centraldashboard:v1.9.0-rc.0

Critical High Medium Low
1 2 3 4
Scanning jupyter-web-app:v1.9.0-rc.0

Critical High Medium Low
5 6 7 8
...

? So a mini table per image in the logs. This makes it easier to verify the numbers for third parties.

Added the table for each log of scanning

hack/trivy_scan.py Outdated Show resolved Hide resolved
hack/trivy_scan.py Outdated Show resolved Hide resolved
hack/trivy_scan.py Outdated Show resolved Hide resolved
hack/trivy_scan.py Outdated Show resolved Hide resolved
.github/workflows/trivy.yaml Outdated Show resolved Hide resolved
hack/trivy_scan.py Outdated Show resolved Hide resolved
hack/trivy_scan.py Outdated Show resolved Hide resolved
@juliusvonkohout
Copy link
Member

juliusvonkohout commented Jun 26, 2024

ah and it should not run on pull requests by default. It should run only if a PR has been merged into the master branch. This blocks other PRs such as #2769 because it takes 40 minutes to run.

Add i think the formatting is a bit off
It should look like

Started scanning images
Scanning images in ../image_lists/kf_latest_workbenches_images.txt
Scanning  docker.io/kubeflownotebookswg/centraldashboard:v1.9.0-rc.0
+----------+------+--------+-----+
| Critical | High | Medium | Low |
+----------+------+--------+-----+
|    1     |  11  |   37   |  4  |
+----------+------+--------+-----+

Scanning  docker.io/kubeflownotebookswg/jupyter-web-app:v1.9.0-rc.0
+----------+------+--------+-----+
| Critical | High | Medium | Low |
+----------+------+--------+-----+
|    1     |  11  |   39   |  67 |
+----------+------+--------+-----+

instead of

Started scanning images
Scanning images in ../image_lists/kf_latest_workbenches_images.txt
Scanning  docker.io/kubeflownotebookswg/centraldashboard:v1.9.0-rc.0

+----------+------+--------+-----+
| Critical | High | Medium | Low |
+----------+------+--------+-----+
|    1     |  11  |   37   |  4  |
+----------+------+--------+-----+
Scanning  docker.io/kubeflownotebookswg/jupyter-web-app:v1.9.0-rc.0

+----------+------+--------+-----+
| Critical | High | Medium | Low |
+----------+------+--------+-----+
|    1     |  11  |   39   |  67 |
+----------+------+--------+-----+

@juliusvonkohout
Copy link
Member

@AndersBennedsgaard I hope you can verify the CVE numbers per image and WG independently.

@AndersBennedsgaard
Copy link
Contributor

@AndersBennedsgaard I hope you can verify the CVE numbers per image and WG independently.

@juliusvonkohout did you mean to tag @hansinikarunarathne instead ?

@juliusvonkohout
Copy link
Member

@AndersBennedsgaard I hope you can verify the CVE numbers per image and WG independently.

@juliusvonkohout did you mean to tag @hansinikarunarathne instead ?

No, i would like others to validate as well.

@AndersBennedsgaard
Copy link
Contributor

@juliusvonkohout I'm not entirely sure what you mean. Do you want me to check if all Kubeflow images are caught by the script, or do you want me to run Trivy on the same images? Running Trivy to verify the numbers are the same when running locally version in GH actions seems redundant - especially due to the time it takes to scan all the images.

However, speaking of the time to scan: you could probably introduce threading/multi-processing when scanning different images to speed up the script by a lot, since it should be possible to run the trivy command concurrently.

@juliusvonkohout
Copy link
Member

Do you want me to check if all Kubeflow images are caught by the script

Yes, if you think that the CVE numbers are not worth checking.

Signed-off-by: hansinikarunarathne <107214435+hansinikarunarathne@users.noreply.github.com>
Signed-off-by: hansinikarunarathne <107214435+hansinikarunarathne@users.noreply.github.com>
…nages

Signed-off-by: hansinikarunarathne <107214435+hansinikarunarathne@users.noreply.github.com>
@hansinikarunarathne
Copy link
Member Author

ah and it should not run on pull requests by default. It should run only if a PR has been merged into the master branch. This blocks other PRs such as #2769 because it takes 40 minutes to run.

Add i think the formatting is a bit off It should look like

Started scanning images
Scanning images in ../image_lists/kf_latest_workbenches_images.txt
Scanning  docker.io/kubeflownotebookswg/centraldashboard:v1.9.0-rc.0
+----------+------+--------+-----+
| Critical | High | Medium | Low |
+----------+------+--------+-----+
|    1     |  11  |   37   |  4  |
+----------+------+--------+-----+

Scanning  docker.io/kubeflownotebookswg/jupyter-web-app:v1.9.0-rc.0
+----------+------+--------+-----+
| Critical | High | Medium | Low |
+----------+------+--------+-----+
|    1     |  11  |   39   |  67 |
+----------+------+--------+-----+

instead of

Started scanning images
Scanning images in ../image_lists/kf_latest_workbenches_images.txt
Scanning  docker.io/kubeflownotebookswg/centraldashboard:v1.9.0-rc.0

+----------+------+--------+-----+
| Critical | High | Medium | Low |
+----------+------+--------+-----+
|    1     |  11  |   37   |  4  |
+----------+------+--------+-----+
Scanning  docker.io/kubeflownotebookswg/jupyter-web-app:v1.9.0-rc.0

+----------+------+--------+-----+
| Critical | High | Medium | Low |
+----------+------+--------+-----+
|    1     |  11  |   39   |  67 |
+----------+------+--------+-----+

I made the adjustment according to this

Copy link
Contributor

@AndersBennedsgaard AndersBennedsgaard left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: hansinikarunarathne <107214435+hansinikarunarathne@users.noreply.github.com>
hack/trivy_scan.py Outdated Show resolved Hide resolved
Signed-off-by: hansinikarunarathne <107214435+hansinikarunarathne@users.noreply.github.com>
@juliusvonkohout
Copy link
Member

/lgtm
/approve

Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AndersBennedsgaard, juliusvonkohout

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

@google-oss-prow google-oss-prow bot merged commit 60348d7 into kubeflow:master Jun 30, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants