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

BOM validation soft failing #3795

Open
2 tasks done
rkg-mm opened this issue Jun 1, 2024 · 3 comments
Open
2 tasks done

BOM validation soft failing #3795

rkg-mm opened this issue Jun 1, 2024 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers p3 Nice-to-have features size/S Small effort

Comments

@rkg-mm
Copy link
Contributor

rkg-mm commented Jun 1, 2024

Current Behavior

Currently, you either disable BOM validation, or you enable it. Which either results in no validation or possibly many failures right now.

Proposed Behavior

Add another state "soft validation" which

  1. Does enable validation
  2. On failure:
    2a) Logs validation failure with details
    2b) sends out notification of validation failed (see Notification Event: Bom Validation Failed #3778)
    2c) Possibly gives feedback in return of API call
  3. But: DOES NOT FAIL the import, and still returns API status 200 OK.

This would allow the monitoring of imports for a while, to ensure all used tools behave properly. Due to usage of many tools in different projects and different circumstances in different projects it would be beneficial to first observe the behaviour for some weeks before letting uploads faile. Furthermore it might give a hint in the logs if a validation failed AND later the processing failed. This might help identifying processing issues.

Checklist

@rkg-mm rkg-mm added the enhancement New feature or request label Jun 1, 2024
@nscuro nscuro added p3 Nice-to-have features good first issue Good for newcomers size/S Small effort labels Jun 1, 2024
@nscuro
Copy link
Member

nscuro commented Jun 1, 2024

Add another state "soft validation"

Instead of the soft validation wording, I'd prefer the current boolean option Validation enabled to become a BomValidation enum with possible values being along the lines of:

  • DISABLED - Do nothing
  • MONITOR - Log and alert
  • ENFORCE - Reject invalid BOMs (still log and alert?)

2c) Possibly gives feedback in return of API call

This could lead to some confusion / inconsistencies WRT API design. Getting a successful (HTTP 200) response that indicates errors in its body is an anti-pattern. Not sure how to implement this suggestion in a sensible way.

@aravindparappil46
Copy link
Contributor

aravindparappil46 commented Jun 2, 2024

I like the idea of this "continue-on-error" kind of validation.

We had to turn off BOM validation due to external references URL being invalid from cdxgen (CycloneDX/cdxgen#1107), so definitely see the value here.

The enum approach sounds neat. Maybe could call it Validation Mode? (STRICT, LENIENT, DISABLED)

@rkg-mm
Copy link
Contributor Author

rkg-mm commented Jun 3, 2024

2c) Possibly gives feedback in return of API call

This could lead to some confusion / inconsistencies WRT API design. Getting a successful (HTTP 200) response that indicates errors in its body is an anti-pattern. Not sure how to implement this suggestion in a sensible way.

Not a hard requirement from me, just thought it could come handy for some people. However, I think if its a "Warning" not an Error, it could be fine to return.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers p3 Nice-to-have features size/S Small effort
Projects
None yet
Development

No branches or pull requests

3 participants