Add initial support for managing vulnerability resolutions via the API#4573
Add initial support for managing vulnerability resolutions via the API#4573mnonnenmacher wants to merge 9 commits intomainfrom
Conversation
|
There's a PR in for resolving #4471 now. |
mnonnenmacher
force-pushed
the
resolution-component
branch
3 times, most recently
from
5feef37 to
43d43a1
Compare
mnonnenmacher
requested review from
MarcelBochtler,
bs-ondem,
nnobelis,
oheger-bosch,
sschuberth and
wkl3nk
as code owners
components/resolutions/backend/src/main/kotlin/vulnerabilities/VulnerabilityResolutionEvent.kt
Show resolved
Hide resolved
| import org.eclipse.apoapsis.ortserver.model.RepositoryId | ||
| import org.eclipse.apoapsis.ortserver.model.runs.repository.VulnerabilityResolutionReason | ||
|
|
||
| internal data class VulnerabilityResolutionEvent( |
There was a problem hiding this comment.
Did we ever agree to use event sourcing as a general approach? As far as I recall, it was agreed that
- you do an experimental implementation on one or two components
- you present your implementation
- the team decides about the approach
The steps 2 and 3 never happened as far as I am aware of.
I am rather concerned about the current state of the implementation. It adds significant complexity, while I am pretty sure that many corner cases are not handled.
There was a problem hiding this comment.
you do an experimental implementation on one or two components
Yes, this would be the second component. The problem with the plugin manager is that work on it basically stalled since summer last year so unfortunately it was not possible to learn from it. I expect more activity in the new resolutions component in the near future which will help to see how well the approach works when model changes are required.
you present your implementation
I thought I would have shown the implementation in an internal tech talk at Bosch last year.
There was a problem hiding this comment.
I thought I would have shown the implementation in an internal tech talk at Bosch last year.
There was only a presentation of the plugin manager UI IIRC, no discussion of the event sourcing implementation.
Yes, this would be the second component. The problem with the plugin manager is that work on it basically stalled since summer last year so unfortunately it was not possible to learn from it. I expect more activity in the new resolutions component in the near future which will help to see how well the approach works when model changes are required.
What does this mean in practice? Should the whole component then be treated as experimental? It is theoretically possible that the implementation causes problems that impact the stability of the whole system. So, would it be possible, maybe by using a feature toggle or whatever, to not expose it to end users, so that it can be switched off in production systems?
There was a problem hiding this comment.
No, I think the test coverage is not worse than for other code and if issues are found they will be addressed. So from the technical side I don't see a need for a feature toggle, only if there is use case that someone does not want their users to manage resolutions via the UI.
There was a problem hiding this comment.
I would like to discuss this in the community meeting. I have added an item to the agenda.
mnonnenmacher
force-pushed
the
resolution-component
branch
from
43d43a1 to
bbb0a6e
Compare
...onents/resolutions/backend/src/main/kotlin/vulnerabilities/VulnerabilityResolutionService.kt
Outdated
Show resolved
Hide resolved
The component will be used to manage server-side resolutions for issues, vulnerabilities, and rule violations. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@doubleopen.org>
Define the events required for managing vulnerability resolutions. At least in the initial implementation, vulnerability resolutions will only be possible on the level of repositories. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@doubleopen.org>
Add a class to store vulnerability resolution events in the database and to get the current state of a specific resolution. At least for now, vulnerabilitiy resolutions can only be managed on the repository level, not on the product or organizations levels. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@doubleopen.org>
Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@doubleopen.org>
Add a service class to manage vulnerability resolutions. For the start, all functions are internal and will only be made public later if required externally. Like the `PluginTemplateService`, the `VulnerabilityResolutionServices` uses the `kotlin-result` [1] library to handle errors as values. In contrast to the plugin manager, it uses the binding API [2] to allow programming in an imperative style. This mimics, for example, the `?` operator in Rust. This allows to compare both approaches and make a decision which is preferrable later on. [1]: https://github.com/michaelbull/kotlin-result [2]: https://github.com/michaelbull/kotlin-result?tab=readme-ov-file#advanced-usage---binding-monad-comprehension Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@doubleopen.org>
This makes the classes available to components. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@doubleopen.org>
Add a new role to control who can manage resolutions via the API and assign it to the `WRITER` role. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@doubleopen.org>
Add the routes required to manage vulnerability resolutions via the API. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@doubleopen.org>
Extend the `createResolutionProvider` helper function to take resolutions from the `VulnerabilityResolutionService` into account. As only the advisor workers resolved vulnerabilities, the service is only passed to the function in that worker. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@doubleopen.org>
mnonnenmacher
force-pushed
the
resolution-component
branch
from
bbb0a6e to
2352751
Compare
5 participants
Add a new
resolutionscomponent which will be used for managing resolutions on the server. For the start, support for vulnerability resolutions is implemented.This PR replaces #3846 and takes several of the review comments into account. It does not yet add information about new resolutions to the
/runs/{runId}/vulnerabilitiesendpoint as #3846 did, because that feature requires more design work for proper UI support and is probably easier to implement once #4471 is done. The reason is that the UI needs to know for each vulnerability resolution if it is managed by the server or not, and because currently resolutions are taken from theOrtResultclass that information is not available.Please see the commit messages for details.