feat: add vulnerabilities by package

[carlosthe19916]

Fix trustification/trustify#461 and trustification/trustify#412 partially

• The current model is returning multiple duplicate data reported at Surface package status for packages in UI. trustify#461
• For each vulnerability we still don't know its severity

[gildub]

LGTM
Meanwhile I need a specific data set to be able to fully test it.
Could you please provide a CVE which will provide relevant
package details ?

[carlosthe19916]

@gildub I don't have a specific single CVE I can provide to you. Could you please try starting the "CSAF Importer" and let it run for some minutes, it should allow you to see data there

[gildub]

I found a use case meanwhile I get the following error:

Which matches the error returned from the backend in debug mode:

[carlosthe19916]

@gildub Thanks for testing it. I have reported the issue you found at trustification/trustify#466

[gildub]

I believe this is a great opportunity to narrow down unexpected error cases.

Also if that's blocking you we could it in a follow-up PR, meanwhile I'll be tempted not to procrastinate and deal with it right now, but that's up to you and your priorities.

I think we can display a more detailed error message with the content of the error message which is returned by the query. Actually thinking out loud it raise the question maybe using notifications for that (in which case it'll be in separated PRs...).

What do you think ?

[carlosthe19916]

@gildub on one hand I do not like the current error message, it does not follow the standard error template. So let me change the following error with a different look and feel. But give me some time please, I am busy with other tasks.

On the other hand I do not think printing the error from the backend is a good idea. The backend error is:

{
    "error": "Database error",
    "message": "Query Error: error returned from database: invalid input syntax for type numeric: \"sha256:f579ee538e6f4bfa4d9c20bf2680d7fc3ec7a757a40637a743b7962187b4d6a1\""
}

• As I user what is the benefit of showing such technical text? IMHO it does not help.
• I don't remember having seen any other UI in RH that renders the errors in a way that we expose the internals of the server to the user. What we usually do is just render generic messages that a user/human can understand.
• If you could point me to any other RH UI project (using patternfly) where we render the backend text it would be helpful. I would rather avoid experimenting but doing something that was done by other products/teams

[gildub]

@carlosthe19916, a non admin user is more likely to be satisfied with the error type which is "500 - Internal Error"
On the other hand, sysadmin/devops/developers type of users should be ok with the details provided in the console.

As we already have notification framework in place at the application level, shouldn't wee should rely on it to display an empty component and add an error notification ?

[helio-frota]

On the other hand, sysadmin/devops/developers type of users should be ok with the details provided in the console.

👍

an empty component and add an error notification ?

👍

[helio-frota]

logs ?

[carlosthe19916]

@gildub I have updated the code so we reuse the current error component. This is how it should look like now:

a non admin user is more likely to be satisfied with the error type which is "500 - Internal Error"
On the other hand, sysadmin/devops/developers type of users should be ok with the details provided in the console.

Going back to my original question: could you point me to a real RH UI app where the UI takes "any" error response from the backend and just renders it in the UI? A screenshot of that RH UI/Patternfly product would be helpful. Otherwise we are asking something abstract and I will need to ask you to create a mockup for me. I am asking so we don't make assumptions about how to please both "common users" + "sysadmin/devops/developers"

As we already have notification framework in place at the application level, shouldn't wee should rely on it to display an empty component and add an error notification?

If you see the screenshot I pasted above you will see that an error is rendered already in the main central part of the page, I don't see why we would need to render an error in the central part and also as a notification (that would be double error text)

[gildub]

@carlosthe19916,

There are plenty of special cases of back-end error that are "trapped" and exposed to the end user whether using the original error message of the back-end or using a more user friendly version of it.

For me an error message is an opportunity to handle edge cases.
Effectively, in normal operation mode, we don't see such errors.
How the application behaves outside those normal cases is the interesting part.
You're right, we don't want to, and can't really anyway, "trap" all those edge cases.
Although there are, from experience, some edge cases clearly identified, mostly when we get specific errors from API.
But here we're talking about the unknown error.
In such cases, we just need to be as transparent as possible.
And I don't see why we cannot indicate to the user the error we simply received.
In this case a generic "500 - Internal server error" should be simply reported.
Replacing that by a "Cannot connect" is, I believe, misleading the user.

If you see the screenshot I pasted above you will see that an error is rendered already in the main central part of the page, I don't see why we would need to render an error in the central part and also as a notification (that would be double error text)

Let's take that discussion separately outside of this PR.
I believe we need to have a clear strategy of displaying errors and how notifications fit such strategy.

[carlosthe19916]

I agree with you when it comes to "having a concrete strategy for error rendering". I have just created this issue to track it #85

[carlosthe19916]

I consider important to share that "rendering backend errors in the UI" were already proven to be a bad idea in Trustification (So I am talking about a real failure in that attempt)

The PR trustification/trustification#1221 and Ticket https://issues.redhat.com/browse/TC-1091 were created to remove the backend error messages from the UI as they rather than helping just confuse users (we have to remember the app is not made for us/engineers but for normal people like PM and others)

The image below is an example of errors of the backend being rendered in the UI. The text contains explicit mention to internal components and even queries. PM explicitly requested to replace it with generic error messages as that level of error message does not benefit users and it has a negative impact

[gildub]

@carlosthe19916,
I don't want to block this PR more than necessary knowing this edge case error is to be resolved by the backend.

Thanks for creating #85 to follow-up.

[helio-frota]

Thanks for creating #85 to follow-up.

nice move! @carlosthe19916