Skip to content

ROX-27772: SBOM fix scan time update edge case#14089

Merged
dcaravel merged 2 commits intomasterfrom
dc/sbom-scan-time
Feb 5, 2025
Merged

ROX-27772: SBOM fix scan time update edge case#14089
dcaravel merged 2 commits intomasterfrom
dc/sbom-scan-time

Conversation

@dcaravel
Copy link
Copy Markdown
Contributor

@dcaravel dcaravel commented Feb 3, 2025 •
edited
Loading

Description

If SBOM generation results in a successful image scan ensures that the image scan is saved to Central DB.

Also ensures that the HTTP error code when generating an SBOM is derived from the error and not always 500 (for example, if the image name is empty the error should be 400 due to errox.InvalidArgs being wrapped here)

User-facing documentation

  • CHANGELOG is updated OR update is not needed
  • documentation PR is created and is linked above OR is not needed

Testing and quality

  • the change is production ready: the change is GA or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • modified existing tests

How I validated my change

Unit tests + manual test.

Image Name missing 400

Before Fix:

Notice HTTP 500 is returned

$ cat payload.json 
{
    "imageName": "",
    "force": false,
    "cluster": ""
}

$ curl -ksS -H "Authorization: Bearer $ROX_API_TOKEN" -X POST "https://$ROX_ENDPOINT/api/v1/images/sbom" -i -d @payload.json
HTTP/2 500 
vary: Accept-Encoding
content-type: text/plain; charset=utf-8
content-length: 88
date: Mon, 03 Feb 2025 23:47:02 GMT

{"code":13,"message":"generating SBOM: image name must be specified: invalid arguments"}

---

$ cat payload.json 
{
    "imageName": "  ",
    "force": false,
    "cluster": ""
}

$ curl -ksS -H "Authorization: Bearer $ROX_API_TOKEN" -X POST "https://$ROX_ENDPOINT/api/v1/images/sbom" -i -d @payload.json
HTTP/2 500 
vary: Accept-Encoding
content-type: text/plain; charset=utf-8
content-length: 116
date: Mon, 03 Feb 2025 23:48:06 GMT

{"code":13,"message":"generating SBOM: error parsing image name '   ': invalid reference format: invalid arguments"}

After Fix:

$ cat payload.json 
{
    "imageName": "",
    "force": false,
    "cluster": ""
}

$ curl -ksS -H "Authorization: Bearer $ROX_API_TOKEN" -X POST "https://$ROX_ENDPOINT/api/v1/images/sbom" -i -d @payload.json
HTTP/2 400 
vary: Accept-Encoding
content-type: text/plain; charset=utf-8
content-length: 87
date: Mon, 03 Feb 2025 23:31:51 GMT

{"code":3,"message":"generating SBOM: image name must be specified: invalid arguments"}

---

$ cat payload.json 
{
    "imageName": "  ",
    "force": false,
    "cluster": ""
}

$ curl -ksS -H "Authorization: Bearer $ROX_API_TOKEN" -X POST "https://$ROX_ENDPOINT/api/v1/images/sbom" -i -d @payload.json
HTTP/2 400 
vary: Accept-Encoding
content-type: text/plain; charset=utf-8
content-length: 87
date: Mon, 03 Feb 2025 23:30:59 GMT

{"code":3,"message":"generating SBOM: image name must be specified: invalid arguments"}
Scan Time Update

In between each of the tests, ran the following to remove the index report from scanner v4 db (the digest is that of nginx:1.27 at the time of writing this). This is necessary to setup the test condition where Central DB has a Scanner V4 scan but Scanner V4 has no index report.

roxdb scannerv4

DELETE 
  FROM manifest 
 WHERE hash in (
       SELECT concat('sha512:', 
           encode(sha512('/v4/containerimage/sha256:2426c815287ed75a3a33dd28512eba4f0f783946844209ccf3fa8990817a4eb9'::bytea), 'hex')
       ));

Before Fix:

Scan time did not change unless using the -f flag.

After Fix:

Start

image

Sanity check, generate SBOM and observe no new scan executed:

rctl image sbom --image=nginx:1.27

image

Now delete index report using query above, and re-try:

rctl image sbom --image=nginx:1.27

image

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Feb 3, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@rhacs-bot
Copy link
Copy Markdown
Contributor

rhacs-bot commented Feb 3, 2025 •
edited
Loading

Images are ready for the commit at a3eb2ba.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.7.x-601-ga3eb2ba972.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 3, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 70.83333% with 7 lines in your changes missing coverage. Please review.

Project coverage is 48.92%. Comparing base (08a5f73) to head (85c0015).
Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
central/image/service/http_handler.go 70.83% 5 Missing and 2 partials ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #14089   +/-   ##
=======================================
  Coverage   48.92%   48.92%           
=======================================
  Files        2500     2500           
  Lines      180852   180880   +28     
=======================================
+ Hits        88474    88496   +22     
- Misses      85357    85361    +4     
- Partials     7021     7023    +2
Flag Coverage Δ
go-unit-tests 48.92% <70.83%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dcaravel dcaravel marked this pull request as ready for review February 3, 2025 23:51
@dcaravel dcaravel requested a review from a team as a code owner February 3, 2025 23:51
RTann
RTann reviewed Feb 4, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@RTann RTann left a comment

Choose a reason for hiding this comment

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

minor nits and a question. Should be good to go after another round

@dcaravel dcaravel requested a review from a team as a code owner February 4, 2025 01:36
@dcaravel dcaravel requested a review from RTann February 4, 2025 01:37
@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Feb 4, 2025

@dcaravel: 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/ocp-4-17-scanner-v4-install-tests a3eb2ba link false /test ocp-4-17-scanner-v4-install-tests

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

BradLugo
BradLugo approved these changes Feb 5, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@BradLugo BradLugo left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for adding that 400 status code.

@dcaravel dcaravel merged commit d224926 into master Feb 5, 2025
@dcaravel dcaravel deleted the dc/sbom-scan-time branch February 5, 2025 21:02
shireenf-ibm pushed a commit to shireenf-ibm/stackrox that referenced this pull request Feb 10, 2025
@dcaravel @shireenf-ibm
ROX-27772: SBOM fix scan time update edge case (stackrox#14089)
5f5650f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@janisz janisz janisz approved these changes

@BradLugo BradLugo BradLugo approved these changes

@ksurabhi91 ksurabhi91 Awaiting requested review from ksurabhi91

+1 more reviewer

@RTann RTann RTann approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/central area/roxctl

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dcaravel @rhacs-bot @janisz @BradLugo @RTann