feat: add import warnings to the report

trustification · Jul 9, 2024 · b59a19b · b59a19b
1 parent e5e9c42
commit b59a19b
Show file tree

Hide file tree

Showing 18 changed files with 155 additions and 61 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -86,7 +86,7 @@ reqwest = "0.12"
 ring = "0.17.8"
 rstest = "0.21"
 rust-lzma = "0.6.0"
-sbom-walker = { version = "0.8.0", default-features = false }
+sbom-walker = { version = "0.8.6", default-features = false }
 schemars = "0.8"
 sea-orm = "0.12"
 sea-orm-migration = "0.12.2"

diff --git a/integration-tests/src/sbom/test/mod.rs b/integration-tests/src/sbom/test/mod.rs
@@ -23,6 +23,7 @@ use trustify_module_ingestor::graph::{
     sbom::{self, spdx::parse_spdx, SbomContext, SbomInformation},
     Graph,
 };
+use trustify_module_ingestor::service::Discard;
 use trustify_test_context::TrustifyContext;
 
 #[instrument]
@@ -82,10 +83,15 @@ where
         ctx,
         sbom,
         |data| {
-            let (sbom, _) = parse_spdx(&*data)?;
+            let (sbom, _) = parse_spdx(&Discard, &*data)?;
             Ok(fix_spdx_rels(sbom))
         },
-        |ctx, sbom, tx| Box::pin(async move { Ok(ctx.ingest_spdx(sbom.clone(), &tx).await?) }),
+        |ctx, sbom, tx| {
+            Box::pin(async move {
+                ctx.ingest_spdx(sbom.clone(), &Discard, &tx).await?;
+                Ok(())
+            })
+        },
         |sbom| sbom::spdx::Information(sbom).into(),
         f,
     )

diff --git a/modules/importer/src/server/csaf/report.rs b/modules/importer/src/server/csaf/report.rs
@@ -1,7 +1,7 @@
 use crate::server::{
     common::storage::StorageError,
     csaf::storage::StorageVisitor,
-    report::{Phase, ReportVisitor, Severity},
+    report::{Phase, ReportVisitor},
 };
 use csaf_walker::{
     retrieve::RetrievalError,
@@ -42,7 +42,6 @@ impl ValidatedVisitor for CsafReportVisitor {
                     self.0.report.lock().add_error(
                         Phase::Retrieval,
                         file,
-                        Severity::Error,
                         format!("retrieval of document failed: {code}"),
                     );
 
@@ -60,7 +59,6 @@ impl ValidatedVisitor for CsafReportVisitor {
                     self.0.report.lock().add_error(
                         Phase::Validation,
                         file,
-                        Severity::Error,
                         format!("digest mismatch - expected: {expected}, actual: {actual}"),
                     );
 
@@ -72,7 +70,6 @@ impl ValidatedVisitor for CsafReportVisitor {
                     self.0.report.lock().add_error(
                         Phase::Validation,
                         file,
-                        Severity::Error,
                         format!("unable to verify signature: {error}"),
                     );
 
@@ -84,7 +81,6 @@ impl ValidatedVisitor for CsafReportVisitor {
                     self.0.report.lock().add_error(
                         Phase::Upload,
                         file,
-                        Severity::Error,
                         format!("processing failed: {err}"),
                     );
 
@@ -96,7 +92,6 @@ impl ValidatedVisitor for CsafReportVisitor {
                     self.0.report.lock().add_error(
                         Phase::Upload,
                         file,
-                        Severity::Error,
                         format!("upload failed: {err}"),
                     );
 

diff --git a/modules/importer/src/server/cve/mod.rs b/modules/importer/src/server/cve/mod.rs
@@ -6,7 +6,7 @@ use crate::{
         common::walker::{CallbackError, Callbacks},
         context::RunContext,
         cve::walker::CveWalker,
-        report::{Phase, ReportBuilder, ScannerError, Severity},
+        report::{Phase, ReportBuilder, ScannerError},
         RunOutput,
     },
 };
@@ -57,22 +57,16 @@ impl Context {
 
 impl Callbacks<Cve> for Context {
     fn loading_error(&mut self, path: PathBuf, message: String) {
-        self.report.lock().add_error(
-            Phase::Validation,
-            path.to_string_lossy(),
-            Severity::Error,
-            message,
-        );
+        self.report
+            .lock()
+            .add_error(Phase::Validation, path.to_string_lossy(), message);
     }
 
     fn process(&mut self, path: &Path, cve: Cve) -> Result<(), CallbackError> {
         if let Err(err) = self.store(path, cve) {
-            self.report.lock().add_error(
-                Phase::Upload,
-                path.to_string_lossy(),
-                Severity::Error,
-                err.to_string(),
-            );
+            self.report
+                .lock()
+                .add_error(Phase::Upload, path.to_string_lossy(), err.to_string());
         }
 
         self.context.check_canceled_sync(|| CallbackError::Canceled)

diff --git a/modules/importer/src/server/osv/mod.rs b/modules/importer/src/server/osv/mod.rs
@@ -6,7 +6,7 @@ use crate::{
         common::walker::{CallbackError, Callbacks},
         context::RunContext,
         osv::walker::OsvWalker,
-        report::{Phase, ReportBuilder, ScannerError, Severity},
+        report::{Phase, ReportBuilder, ScannerError},
         RunOutput,
     },
 };
@@ -57,22 +57,16 @@ impl Context {
 
 impl Callbacks<Vulnerability> for Context {
     fn loading_error(&mut self, path: PathBuf, message: String) {
-        self.report.lock().add_error(
-            Phase::Validation,
-            path.to_string_lossy(),
-            Severity::Error,
-            message,
-        );
+        self.report
+            .lock()
+            .add_error(Phase::Validation, path.to_string_lossy(), message);
     }
 
     fn process(&mut self, path: &Path, osv: Vulnerability) -> Result<(), CallbackError> {
         if let Err(err) = self.store(path, osv) {
-            self.report.lock().add_error(
-                Phase::Upload,
-                path.to_string_lossy(),
-                Severity::Error,
-                err.to_string(),
-            );
+            self.report
+                .lock()
+                .add_error(Phase::Upload, path.to_string_lossy(), err.to_string());
         }
 
         self.context.check_canceled_sync(|| CallbackError::Canceled)

diff --git a/modules/importer/src/server/report.rs b/modules/importer/src/server/report.rs
@@ -1,7 +1,6 @@
 use crate::server::RunOutput;
 use parking_lot::Mutex;
-use std::collections::BTreeMap;
-use std::sync::Arc;
+use std::{collections::BTreeMap, iter, sync::Arc};
 use time::OffsetDateTime;
 
 #[derive(
@@ -46,6 +45,22 @@ pub struct Message {
     pub message: String,
 }
 
+impl Message {
+    pub fn error(message: impl Into<String>) -> Self {
+        Self {
+            severity: Severity::Error,
+            message: message.into(),
+        }
+    }
+
+    pub fn warning(message: impl Into<String>) -> Self {
+        Self {
+            severity: Severity::Warning,
+            message: message.into(),
+        }
+    }
+}
+
 #[derive(Clone, Debug)]
 pub struct ReportBuilder {
     report: Report,
@@ -67,23 +82,55 @@ impl ReportBuilder {
         self.report.number_of_items += 1;
     }
 
-    pub fn add_error(
+    /// Add a single message
+    pub fn add_message(
         &mut self,
         phase: Phase,
         file: impl Into<String>,
         severity: Severity,
         message: impl Into<String>,
+    ) {
+        self.extend_messages(
+            phase,
+            file,
+            [Message {
+                severity,
+                message: message.into(),
+            }],
+        )
+    }
+
+    /// Add a single error
+    pub fn add_error(&mut self, phase: Phase, file: impl Into<String>, message: impl Into<String>) {
+        self.add_message(phase, file, Severity::Error, message)
+    }
+
+    pub fn extend_messages(
+        &mut self,
+        phase: Phase,
+        file: impl Into<String>,
+        messages: impl IntoIterator<Item = Message>,
     ) {
         let file = file.into();
-        let message = message.into();
+        let mut messages = messages.into_iter();
+
+        // check if we have at least one item
+
+        let first = messages.next();
+        let Some(first) = first else {
+            // if not, return without creating any phase or file
+            return;
+        };
+
+        // now add the first, and all remaining messages
 
         self.report
             .messages
             .entry(phase)
             .or_default()
             .entry(file)
             .or_default()
-            .push(Message { severity, message });
+            .extend(iter::once(first).chain(messages));
     }
 
     pub fn build(mut self) -> Report {

diff --git a/modules/importer/src/server/sbom/report.rs b/modules/importer/src/server/sbom/report.rs
@@ -1,6 +1,6 @@
-use crate::server::common::storage::StorageError;
 use crate::server::{
-    report::{Phase, ReportVisitor, Severity},
+    common::storage::StorageError,
+    report::{Phase, ReportVisitor},
     sbom::storage::StorageVisitor,
 };
 use sbom_walker::{
@@ -42,7 +42,6 @@ impl ValidatedVisitor for SbomReportVisitor {
                     self.0.report.lock().add_error(
                         Phase::Retrieval,
                         file,
-                        Severity::Error,
                         format!("retrieval of document failed: {code}"),
                     );
 
@@ -60,7 +59,6 @@ impl ValidatedVisitor for SbomReportVisitor {
                     self.0.report.lock().add_error(
                         Phase::Validation,
                         file,
-                        Severity::Error,
                         format!("digest mismatch - expected: {expected}, actual: {actual}"),
                     );
 
@@ -72,7 +70,6 @@ impl ValidatedVisitor for SbomReportVisitor {
                     self.0.report.lock().add_error(
                         Phase::Validation,
                         file,
-                        Severity::Error,
                         format!("unable to verify signature: {error}"),
                     );
 
@@ -84,7 +81,6 @@ impl ValidatedVisitor for SbomReportVisitor {
                     self.0.report.lock().add_error(
                         Phase::Upload,
                         file,
-                        Severity::Error,
                         format!("processing failed: {err}"),
                     );
 
@@ -96,7 +92,6 @@ impl ValidatedVisitor for SbomReportVisitor {
                     self.0.report.lock().add_error(
                         Phase::Upload,
                         file,
-                        Severity::Error,
                         format!("upload failed: {err}"),
                     );
 

diff --git a/modules/importer/src/server/sbom/storage.rs b/modules/importer/src/server/sbom/storage.rs
@@ -1,4 +1,5 @@
 use crate::server::common::storage::StorageError;
+use crate::server::report::{Message, Phase};
 use crate::server::{context::RunContext, report::ReportBuilder};
 use parking_lot::Mutex;
 use sbom_walker::validation::{
@@ -49,12 +50,13 @@ impl ValidatedVisitor for StorageVisitor {
 
         let fmt = Format::sbom_from_bytes(&data).map_err(|e| StorageError::Processing(e.into()))?;
 
-        self.ingestor
+        let result = self
+            .ingestor
             .ingest(
                 Labels::new()
                     .add("source", &self.source)
                     .add("importer", self.context.name())
-                    .add("file", file)
+                    .add("file", &file)
                     .extend(&self.labels.0),
                 None,
                 fmt,
@@ -63,6 +65,12 @@ impl ValidatedVisitor for StorageVisitor {
             .await
             .map_err(StorageError::Storage)?;
 
+        self.report.lock().extend_messages(
+            Phase::Upload,
+            file,
+            result.warnings.into_iter().map(Message::warning),
+        );
+
         self.context.check_canceled(|| StorageError::Canceled).await
     }
 }
diff --git a/modules/ingestor/Cargo.toml b/modules/ingestor/Cargo.toml
@@ -26,7 +26,9 @@ lenient_semver = { workspace = true }
 log = { workspace = true }
 osv = { workspace = true, features = ["schema"] }
 packageurl = { workspace = true }
+parking_lot = { workspace = true }
 ring = { workspace = true }
+sbom-walker = { workspace = true }
 sea-orm = { workspace = true }
 sea-query = { workspace = true }
 serde = { workspace = true, features = ["derive"] }