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

ROX-20753: Add scanner RHTAP build pipeline #1334

Merged
merged 39 commits into from
Feb 6, 2024

Conversation

kylape
Copy link
Contributor

@kylape kylape commented Nov 28, 2023

A few differences from the standard RHTAP build pipeline:

  • Use of 6GB buildah image to avoid OOMs during container build task
  • There is an extra step to fetch the vuln feed data. This is done outside of the build step to make hermetic builds easier.

The vuln feed data script pulls the definitions from the Google storage location and writes them to the "source" folder of the "source" workspace which is shared by the build-container task. This task will build using image/scanner/rhtap/Dockerfile, which expects the vuln files to be in the buildah working directory.

I decided to create a separate dockerfile for RHTAP to avoid any regressions with modifying the existing ones. The dockerfile was created using a combination of the upstream and downstream dockerfiles.

Current RHTAP build trigger config:

  • PRs will only build in RHTAP when "rhtap" is in the branch name
  • Pushes to master will trigger an RHTAP build

@ghost
Copy link

ghost commented Nov 28, 2023

Images are ready for the commit at 6360f8f.

To use the images, use the tag 2.31.x-91-g6360f8f480.

.containerignore Outdated Show resolved Hide resolved
@BradLugo BradLugo requested a review from a team December 5, 2023 22:43
.tekton/scanner-push.yaml Outdated Show resolved Hide resolved
image/scanner/rhtap/Dockerfile Outdated Show resolved Hide resolved
.github/workflows/style.yaml Outdated Show resolved Hide resolved
.github/workflows/style.yaml Outdated Show resolved Hide resolved
.tekton/scanner-pull-request.yaml Outdated Show resolved Hide resolved
.tekton/scanner-pull-request.yaml Outdated Show resolved Hide resolved
.tekton/scanner-pull-request.yaml Show resolved Hide resolved
.tekton/scanner-push.yaml Outdated Show resolved Hide resolved
@kylape kylape force-pushed the klape/rhtap-scanner-onboarding branch from 71899f4 to 52eef4b Compare January 3, 2024 22:29
@kylape
Copy link
Contributor Author

kylape commented Jan 3, 2024

I removed all the commits from this PR branch committed since I had switched from downloading the vuln dump to generating it. In other words, I just switched back to downloading the vuln dump from GCS.

@kylape kylape force-pushed the klape/rhtap-scanner-onboarding branch 2 times, most recently from e921ec1 to 91e999b Compare January 4, 2024 18:08
@kylape
Copy link
Contributor Author

kylape commented Jan 5, 2024

/retest

@kylape kylape force-pushed the klape/rhtap-scanner-onboarding branch from b4f63b9 to 6360f8f Compare January 10, 2024 18:22
@kylape kylape requested a review from tommartensen January 10, 2024 20:51
.containerignore Outdated Show resolved Hide resolved
Copy link
Contributor

@tommartensen tommartensen left a comment

Choose a reason for hiding this comment

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

all makes sense, please update the comment in the .containerignore.
Formatting of the Tekton Pipeline files is different from the ones we already have, but we have a reconcile planned already.

A few differences from the standard RHTAP build pipeline:

* Use of 6GB buildah image to avoid OOMs during container build task
* There is an extra step to fetch the vuln feed data.  This is done
  outside of the build step to make hermetic builds easier.

The vuln feed data script pulls the definitions from the Google storage
location and writes them to the "source" folder of the "source"
workspace which is shared by the build-container task.  This task will
build using `image/scanner/rhtap/Dockerfile`, which expects the vuln
files to be in the buildah working directory.

I decided to create a separate dockerfile for RHTAP to avoid any
regressions with modifying the existing ones.  The dockerfile was
created using a combination of the upstream and downstream dockerfiles.

Current RHTAP build trigger config:

* PRs will only build in RHTAP when "rhtap" is in the branch name
* Pushes to master will trigger an RHTAP build
This will hopefully fix an intermittent issue where the vuln feed zip
files disappear from the workspace before they make it to the build
step.
scripts/konflux/version.sh Show resolved Hide resolved
scripts/konflux/version.sh Outdated Show resolved Hide resolved
image/scanner/konflux/Dockerfile Outdated Show resolved Hide resolved
image/scanner/konflux/Dockerfile Outdated Show resolved Hide resolved
.tekton/scanner-pull-request.yaml Outdated Show resolved Hide resolved
.tekton/scanner-pull-request.yaml Outdated Show resolved Hide resolved
.tekton/scanner-pull-request.yaml Outdated Show resolved Hide resolved
scripts/konflux/fetch-vuln-feed-data.sh Outdated Show resolved Hide resolved
scripts/konflux/fetch-vuln-feed-data.sh Show resolved Hide resolved
.tekton/scanner-pull-request.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@msugakov msugakov left a comment

Choose a reason for hiding this comment

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

Everything from this round is minor.
ClamAV is something that can block the merge (semantically, not technically) if it fails EC (I left a separate comment about it).

.tekton/scanner-pull-request.yaml Show resolved Hide resolved
.tekton/scanner-pull-request.yaml Outdated Show resolved Hide resolved
msugakov added a commit that referenced this pull request Feb 1, 2024
It's actually stolen from
#1334
with small modifications.
msugakov added a commit that referenced this pull request Feb 1, 2024
It's actually stolen from
#1334
with small modifications.
@msugakov msugakov changed the title Add RHTAP build pipeline ROX-20753: Add scanner RHTAP build pipeline Feb 2, 2024
.tekton/scanner-pull-request.yaml Outdated Show resolved Hide resolved
.tekton/scanner-push.yaml Outdated Show resolved Hide resolved
.tekton/scanner-pull-request.yaml Outdated Show resolved Hide resolved
.tekton/scanner-push.yaml Outdated Show resolved Hide resolved
@msugakov
Copy link
Contributor

msugakov commented Feb 6, 2024

@kylape Here's what I meant by unifying the script - #1395. Note that I had to rebase on master therefore all commits show up as new. Only the last two commits are significant.

I suggest you merge this #1334 as-is, and then either of us can refresh #1395 and publish for review. This way we don't hold #1334 from merging.

@kylape kylape requested a review from a team as a code owner February 6, 2024 14:59
Copy link

openshift-ci bot commented Feb 6, 2024

@kylape: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-tests e36ec93 link false /test e2e-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Co-authored-by: red-hat-konflux <123456+red-hat-konflux[bot]@users.noreply.github.com>
@kylape kylape merged commit 76f3315 into master Feb 6, 2024
16 checks passed
@kylape kylape deleted the klape/rhtap-scanner-onboarding branch February 6, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants