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

[TEP-0079] Image Scanning for CVEs #871

Merged

Conversation

QuanZhang-William
Copy link
Member

This commit updates the "Image Scanning for Common Vulnerabilities and Exposures" section of TEP-0079, which proposes to use Trivy as the CVEs scanning tool. It also proposes to integrate Tekton Catalogs to the Artifact Hub Scanner service to run the reports periodically and show the reports in the Artifact Hub UI.

Signed-off-by: Quan Zhang zhangquan@google.com

@tekton-robot tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 7, 2022
@QuanZhang-William
Copy link
Member Author

@jerop
Copy link
Member

jerop commented Nov 7, 2022

/kind tep

@tekton-robot tekton-robot added the kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). label Nov 7, 2022
@dibyom
Copy link
Member

dibyom commented Nov 7, 2022

/assign @jerop
/assign @afrittoli

@QuanZhang-William
Copy link
Member Author

Here is a demo video I did in last week's Catalog WG: https://drive.google.com/file/d/1AKH2tPTp_LIeTT49BA4bWkIHgQKqqUzy/view

```
the container images used in the `pipelineTask` (i.e. `build-and-push`) will **not** be included in the security report.

If the `image` value (or part of the value) is specified by `params`, the default value of the `params` (if provided) are collected and used to run the security report. For example, `gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/git-init:v0.40.2` is extracted from the following `task`:
Copy link
Member

@jerop jerop Nov 7, 2022

Choose a reason for hiding this comment

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

how will you know that a given param is an image?

Copy link
Member Author

Choose a reason for hiding this comment

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

We actually cannot really know if a given param is an image or not. The assumptions we are making are that:

  1. If a step.image field exist and does not use a param, we use the value of the step.image directly
  2. If a step.image references to a param, we assume the default value of the referenced param is a container image

If wrong values are specified in the step.image (either a wrong param pointer or invalid container value), the resource itself is invalid and therefore no vulnerability report will be generated for this step.

Copy link
Member

Choose a reason for hiding this comment

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

makes sense, could you please include this detail to the TEP?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added in the latest commit, PTAL 😁


#### Design Evaluation

This design builds on the existing infrastructure and the security report UI design of the Artifact Hub. A significant amount of engineering effort can be saved by leveraging the out-of-box features provided by the Artifact Hub.
Copy link
Member

Choose a reason for hiding this comment

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

🎉

Comment on lines +553 to +601
```yaml
apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
name: demo-pipeline
...
Copy link
Member

Choose a reason for hiding this comment

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

should we use Pipeline as an example here? Asking as we haven't laid out guidelines as how pipelines should be organised within the catalog. This might create confusion. @jerop @vdemeester please let me know your thoughts.

Copy link
Member

Choose a reason for hiding this comment

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

I tend to agree with @vinamra28 on this. I feel we should give the example on the Task to start with 👼🏼

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the feedback @vinamra28 and @vdemeester! I will use Task as the first example

I agree that pipeline is not the main focus of Catalog, but it is already surfaced as a "kind" in Artifact Hub, so I think it still worth adding a pipeline example and callout what tasks in the pipeline will/ won't be scanned?

Comment on lines +575 to +626
If a `pipeline` resource contains `pipelineTask` specified by `taskRef` for example:

```yaml
tasks:
- name: build-image
taskRef:
name: build-and-push
```
the container images used in the `pipelineTask` (i.e. `build-and-push`) will **not** be included in the security report.
Copy link
Member

Choose a reason for hiding this comment

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

same as above. I guess we should avoid referring Pipeline here 🤔

Comment on lines +553 to +601
```yaml
apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
name: demo-pipeline
...
Copy link
Member

Choose a reason for hiding this comment

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

I tend to agree with @vinamra28 on this. I feel we should give the example on the Task to start with 👼🏼

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 9, 2022
Copy link
Member

@jerop jerop left a comment

Choose a reason for hiding this comment

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

Cool demo 👏🏾

Happy to see the benefits of moving to Artifact Hub which has most of the functionality we need!

Thank you @QuanZhang-William!


#### Extract Container Images from Catalogs

We propose to extract and collect all the container images used in the Tekton resource (`task` or `pipeline`) by iterating through all the values in the `tasks.steps.image` fields of the resource, for example, `bash:latest` and `alpine` are extracted in the following `task`:
Copy link
Member

Choose a reason for hiding this comment

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

Where are all the values going stored? How does this integrate with ArtifactHub?
Is the collection done per tag / release, on main?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @afrittoli, the container image strings will be stored in the Artifact Hub DB. There is a Tracker component in the Artifact Hub that will periodically pull the yaml files from the Github repo, do the above mentioned processing, and store the images in the DB.

And another component Scanner will also run periodically to generate the vulnerability report based on the container images prepared by Tracker

And yes, the collection is done per tag/release 😄

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good, thank you. Mind adding this info to the TEP? Thank you!!

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @afrittoli, I did some minor update to explain the process better. PTAL

This commit updates the "Image Scanning for Common Vulnerabilities and Exposures" section of TEP-0079, which proposes to use Trivy as the CVEs scanning tool. It also proposes to integrate Tekton Catalogs to the Artifact Hub Scanner service to run the reports periodically and show the reports in the Artifact Hub UI.

Signed-off-by: Quan Zhang <zhangquan@google.com>
@QuanZhang-William
Copy link
Member Author

Hi @afrittoli, I added some clarification based on your questions, can you please take another look 😁 ?

Copy link
Member

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

Thank you for the updates.
/approve


#### Extract Container Images from Catalogs

We propose to extract and collect all the container images used in the Tekton resource (`task` or `pipeline`) by iterating through all the values in the `tasks.steps.image` fields of the resource, for example, `bash:latest` and `alpine` are extracted in the following `task`:
Copy link
Member

Choose a reason for hiding this comment

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

Sounds good, thank you. Mind adding this info to the TEP? Thank you!!

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli, jerop, vdemeester

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

@pritidesai
Copy link
Member

API WG - ready to merge ...

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 21, 2022
@tekton-robot tekton-robot merged commit e498ba3 into tektoncd:main Nov 21, 2022
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. kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Status: UnAssigned
Development

Successfully merging this pull request may close these issues.

None yet

8 participants