feat(central): defs: Switch to multi bundle definitions

[jvdm]

Description

This will modify Central to serve multi-bundle vulnerabilities upon the client passing X-Scanner-V4-Accept headers with a specific application-specific content type. It will modify Scanner V4 to pass such header if the multi-bundle feature is set to "true", which is now the default. A test was added for the multi-bundle updater.

Checklist

• Investigated and inspected CI test results
• Unit test and regression tests added
• Evaluated and added CHANGELOG entry if required
• Determined and documented upgrade steps
• Documented user facing changes (create PR based on openshift/openshift-docs and merge into rhacs-docs)

If any of these don't apply, please comment below.

Testing Performed

Here I tell how I validated my change

Deploy the image and observe Scanner V4 requesting multi-bundle vulnerabilities, and Central granting them.

This can be confirmed by observing the two Matchers running updates concurrently.

Reminder for reviewers

In addition to reviewing code here, reviewers must also review testing and request further testing in case the
performed one does not seem sufficient. As a reviewer, you must not approve the change until you understand the
performed testing and you are satisfied with it.

[rhacs-bot]

Images are ready for the commit at 0dfeb7d.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.4.x-744-g0dfeb7d83c.

[dcaravel]

Changes look OK to me, could you fill in the description and how it was tested?

Also, what drove the decision to use a header vs. a query string to define the response 'format'? I've seen APIs use the query string to define format, ie ?format=json as well as headers Accept: application/json, since this is not a standard mime type, perhaps a query string would be more conventional? ?multi=true or ?format=multi-bundle, etc.

[jvdm]

I've seen APIs use the query string to define format, ie ?format=json as well as headers Accept: application/json, since this is not a standard mime type, perhaps a query string would be more conventional?

Absolutely, we could use Accept. The MIME type could be vendor-specific, like application/vnd.stackrox.scanner-v4.multi-bundle+zip. Then, I think the server should return' 406 Not Acceptable` when multi-bundle is disabled.

My original approach was to make things non-standard but simple. Using request headers is generally a standard approach for specifying the desired format. Headers are designed for such metadata about the request. But, using a well-known header could make some endpoints not accept the request because the MIME type is vendor-specific is not supported. Using a non-standard header gives us more flexibility because they are supposed to be ignored.

Also, just so you know, my goal is to decommission the single bundle eventually.

[codecov]

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 47.91%. Comparing base (dbce8f3) to head (0dfeb7d).

Files	Patch %	Lines
central/scannerdefinitions/handler/handler.go	50.00%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11167      +/-   ##
==========================================
+ Coverage   47.90%   47.91%   +0.01%     
==========================================
  Files        2330     2330              
  Lines      166491   166500       +9     
==========================================
+ Hits        79752    79780      +28     
+ Misses      80411    80383      -28     
- Partials     6328     6337       +9     

Flag	Coverage Δ
go-unit-tests	47.91% <66.66%> (+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.

[openshift-ci]

@jvdm: The following tests 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-12-scanner-v4-tests	0d0b669	link	false	/test ocp-4-12-scanner-v4-tests
ci/prow/gke-qa-e2e-tests	9ba7fd3	link	false	/test gke-qa-e2e-tests
ci/prow/gke-scanner-v4-tests	9ba7fd3	link	false	/test gke-scanner-v4-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-sigs/prow repository. I understand the commands that are listed here.

[RTann]

Only found out about this due to another PR, but it looks like the X- prefix is not recommended for private purposes. Seems like X. is preferred https://datatracker.ietf.org/doc/html/rfc6838#section-3.4

[RTann]

apologies that RFC is for the media type. This one deprecates the the X- convention: https://datatracker.ietf.org/doc/html/rfc6648

That being said, I'm sure this is fine as-is, but I think the community (ie whoever wrote this RFC) may prefer something like StackRox-Scanner-V4-Accept (makes it clear this is for StackRox purposes and can't be confused for some other Scanner V4. https://datatracker.ietf.org/doc/html/rfc6648#section-3)