Refactor: split greenbone-scanner-framework and move into openvasd - #2306
Refactor: split greenbone-scanner-framework and move into openvasd#2306tsurai wants to merge 1 commit into
Conversation
Dependency ReviewThe following issues were found:
Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. License Issuesrust/Cargo.toml
OpenSSF Scorecard
Scanned Files
|
316d061 to
fd17bc6
Compare
There was a problem hiding this comment.
I think a cleaner approach is to move models into scannerlib entirely and to expose it to openvasd.
The reason why greenbone-scanner-framework was a separate crate to begin with is that once upon a time we thought about using it in entirely different projects. This is not necessary anymore. The remaining advantage of the crate-split is incremental compile times, but we have decided some time ago that we would rather have a monolithic lib crate for the simplicity that it brings and sacrifice a bit of compile time in the process. I'd prefer if we could stick with this approach here too.
(Side note for the openvasd historians among us: This is the eternal cycle of life: models used to be a workspace crate, was turned into a scannerlib module, then moved into gsf and has now - although only temporarily - become a workspace crate again before (in my opinion hopefully) reaching its true form as a scannerlib module again)
|
On another note: I saw that |
fd17bc6 to
b734746
Compare
|
I've pulled
Yea, it was my intention to rename it to just |
There was a problem hiding this comment.
Thank you - Looks good for the most part, but please check what happened to the snapshots.
If I run cargo insta --unreferenced=delete on this PR, I find three unused snapshots. One of them was created, not moved, but I haven't looked into what happened with the other two. I checked and found some missing tests, on main I get 689 tests in total, on this branch I get 686. This fits with the three unused snapshots.
Also linting still fails due to unused stuff.
| // SPDX-FileCopyrightText: 2023 Greenbone AG | ||
| // | ||
| // SPDX-License-Identifier: GPL-2.0-or-later WITH x11vnc-openssl-exception | ||
|
|
||
| //! Defines time implementation for storage usages | ||
|
|
||
| /// Is an extension to add as_timestamp method for various types | ||
| pub trait AsUnixTimeStamp { | ||
| /// Returns a i64 unix time stamp when parsable otherwise None | ||
| fn as_timestamp(&self) -> Option<i64>; | ||
| } | ||
|
|
||
| use time::{OffsetDateTime, format_description}; | ||
|
|
||
| // the function panics because the support formats are hardcoded and therefore the user cannot change anything | ||
| fn parse_or_panic(input: &str) -> Vec<time::format_description::FormatItem<'_>> { | ||
| match format_description::parse_borrowed::<1>(input) { | ||
| Ok(x) => x, | ||
| Err(e) => panic!("expected {input} to be parsable: {e:?}"), | ||
| } | ||
| } | ||
|
|
||
| // for more information see: | ||
| // https://time-rs.github.io/book/api/format-description.html | ||
| const SUPPORTED_FORMATS: &[&str] = &[ | ||
| "[year]-[month]-[day] [hour]:[minute]:[second] [offset_hour][offset_minute]", | ||
| "[weekday repr:short] [month repr:short] [day] [hour]:[minute]:[second] [year] [offset_hour][offset_minute]", | ||
| "[weekday repr:short], [day] [month repr:short] [year] [hour]:[minute]:[second] [offset_hour][offset_minute]", | ||
| ]; | ||
|
|
||
| impl AsUnixTimeStamp for String { | ||
| fn as_timestamp(&self) -> Option<i64> { | ||
| (self as &str).as_timestamp() | ||
| } | ||
| } | ||
|
|
||
| impl AsUnixTimeStamp for &str { | ||
| fn as_timestamp(&self) -> Option<i64> { | ||
| let to_parse = { | ||
| // transforms `wanted (....)` to wanted | ||
| self.splitn(2, " (") | ||
| .find(|x| !x.is_empty()) | ||
| .unwrap_or_default() | ||
| }; | ||
|
|
||
| SUPPORTED_FORMATS | ||
| .iter() | ||
| .map(|x| parse_or_panic(x)) | ||
| .filter_map(|x| OffsetDateTime::parse(to_parse, &x).ok()) | ||
| .map(|x| x.unix_timestamp()) | ||
| .next() | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::AsUnixTimeStamp; | ||
|
|
||
| #[test] | ||
| fn date_string() { | ||
| let example = "2018-09-07 11:08:31 +0200 (Fri, 07 Sep 2018)"; | ||
| assert_eq!(example.as_timestamp(), Some(1536311311)); | ||
| } | ||
|
|
||
| #[test] | ||
| fn iso_orientated() { | ||
| let example = "2012-09-23 02:15:34 -0400"; | ||
| assert_eq!(example.as_timestamp(), Some(1348380934)); | ||
| let example = "2012-09-23 02:15:34 +0400"; | ||
| assert_eq!(example.as_timestamp(), Some(1348352134)); | ||
| } | ||
|
|
||
| #[test] | ||
| fn something_else() { | ||
| let example = "Fri Feb 10 16:09:30 2023 +0100"; | ||
| assert_eq!(example.as_timestamp(), Some(1676041770)); | ||
| let example = "Fri, 10 Feb 2023 16:09:30 +0100"; | ||
| assert_eq!(example.as_timestamp(), Some(1676041770)); | ||
| } | ||
| } |
| --- | ||
| source: crates/greenbone-scanner-framework/src/get_scans.rs | ||
| expression: resp | ||
| --- | ||
| [""] |
There was a problem hiding this comment.
I get a test error when I remove this snap file. Do you mean greenbone_scanner_framework__get_scans__tests__get_scans.snap because that doesn't seem to be used?
There was a problem hiding this comment.
Ah yes, my bad. Didn't mentally parse the diff between the two file names :D
One of those was a copy-paste fail the other two are also unreferenced on |
Haven't looked in detail but I dont get any unused snapshots running
But the time module still exists? It just temporarily existed twice, unless I'm missing something |
For whatever reason running So when run from |
The greenbone-scanner-framework crate is being split into two parts. A models crate containing shared data used among the workspace and the API related code only used by openvasd. The later is moved directly into openvasd as a module. This is an intermediary step in preperation of a major API rework towards axum.
97e3052 to
57bd05d
Compare
2 participants
The
greenbone-scanner-frameworkcrate is being split into two parts. Amodelscrate containing shared data used among the workspace and the API related code only used byopenvasd. The later is moved directly intoopenvasdas a module.This is an intermediate step in preperation of a major API rework towards axum.