[pbs-devel] [PATCH v2 proxmox-apt 04/10] add check_repositories function

[Fabian Ebner <f.ebner@proxmox.com>]

which, for now, checks the suites.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * split this and information about Proxmox repositories in two
    * add tests

 src/repositories/check.rs                 | 78 ++++++++++++++++++++++-
 src/repositories/mod.rs                   | 16 ++++-
 src/types.rs                              | 30 +++++++++
 tests/repositories.rs                     | 60 ++++++++++++++++-
 tests/sources.list.d.expected/bad.sources | 20 ++++++
 tests/sources.list.d/bad.sources          | 19 ++++++
 6 files changed, 219 insertions(+), 4 deletions(-)
 create mode 100644 tests/sources.list.d.expected/bad.sources
 create mode 100644 tests/sources.list.d/bad.sources

diff --git a/src/repositories/check.rs b/src/repositories/check.rs
index d0656cd..7726ff3 100644
--- a/src/repositories/check.rs
+++ b/src/repositories/check.rs
@@ -1,6 +1,23 @@
 use anyhow::{bail, Error};
 
-use crate::types::{APTRepository, APTRepositoryFileType};
+use crate::types::{
+    APTRepository, APTRepositoryFileType, APTRepositoryPackageType, APTRepositoryWarning,
+};
+
+/// Checks if `suite` is some variant of `base_suite`, e.g. `buster-backports`
+/// is a variant of `buster`.
+#[allow(clippy::useless_format)]
+fn suite_is_variant(suite: &str, base_suite: &str) -> bool {
+    let variants = vec![
+        format!("{}", base_suite),
+        format!("{}-backports", base_suite),
+        format!("{}-backports-sloppy", base_suite),
+        format!("{}-updates", base_suite),
+        format!("{}/updates", base_suite),
+    ];
+
+    variants.iter().any(|variant| *variant == suite)
+}
 
 impl APTRepository {
     /// Makes sure that all basic properties of a repository are present and
@@ -45,6 +62,65 @@ impl APTRepository {
         Ok(())
     }
 
+    /// Checks if old or unstable suites are configured and also that the `stable`
+    /// keyword is not used.
+    pub fn check_suites(&self) -> Vec<APTRepositoryWarning> {
+        let old_suites = vec![
+            "lenny",
+            "squeeze",
+            "wheezy",
+            "jessie",
+            "stretch",
+            "oldoldstable",
+            "oldstable",
+        ];
+
+        let new_suites = vec!["unstable", "sid", "experimental"];
+
+        let mut warnings = vec![];
+
+        let mut add_warning = |warning| {
+            warnings.push(APTRepositoryWarning {
+                path: self.path.clone(),
+                number: self.number,
+                warning,
+            });
+        };
+
+        if self
+            .types
+            .iter()
+            .any(|package_type| *package_type == APTRepositoryPackageType::Deb)
+        {
+            for suite in self.suites.iter() {
+                if old_suites
+                    .iter()
+                    .any(|base_suite| suite_is_variant(suite, base_suite))
+                {
+                    add_warning(format!("old suite '{}' configured!", suite));
+                }
+
+                if new_suites
+                    .iter()
+                    .any(|base_suite| suite_is_variant(suite, base_suite))
+                {
+                    add_warning(format!(
+                        "suite '{}' should not be used in production!",
+                        suite,
+                    ));
+                }
+
+                if suite_is_variant(suite, "stable") {
+                    add_warning(
+                        "use the name of the stable distribution instead of 'stable'!".to_string(),
+                    );
+                }
+            }
+        }
+
+        warnings
+    }
+
     /// Checks if the repository is the no-subscription repository of the specified
     /// Proxmox product.
     pub fn is_no_subscription(&self, product: &str) -> bool {
diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
index 4951cb1..38933ef 100644
--- a/src/repositories/mod.rs
+++ b/src/repositories/mod.rs
@@ -5,7 +5,9 @@ use std::path::{Path, PathBuf};
 
 use anyhow::{bail, format_err, Error};
 
-use crate::types::{APTRepository, APTRepositoryFileType, APTRepositoryOption};
+use crate::types::{
+    APTRepository, APTRepositoryFileType, APTRepositoryOption, APTRepositoryWarning,
+};
 
 mod list_parser;
 use list_parser::APTListFileParser;
@@ -105,6 +107,18 @@ fn check_filename<P: AsRef<Path>>(
     Ok(Some((file_type, path_string)))
 }
 
+/// Produces warnings if there are problems withe the repositories.
+/// Currently only checks the suites.
+pub fn check_repositories<A: AsRef<APTRepository>>(repos: &[A]) -> Vec<APTRepositoryWarning> {
+    let mut warnings = vec![];
+
+    for repo in repos.iter() {
+        warnings.append(&mut repo.as_ref().check_suites());
+    }
+
+    warnings
+}
+
 /// Checks if the enterprise repository for the specified Proxmox product is
 /// configured and enabled.
 pub fn enterprise_repository_enabled<A: AsRef<APTRepository>>(repos: &[A], product: &str) -> bool {
diff --git a/src/types.rs b/src/types.rs
index 7a95daf..1a4a50e 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -195,3 +195,33 @@ impl AsRef<APTRepository> for APTRepository {
         &self
     }
 }
+
+#[api(
+    properties: {
+        Path: {
+            description: "Path to the defining file.",
+            type: String,
+        },
+        Number: {
+            description: "Line or stanza number.",
+            type: Integer,
+        },
+        Warning: {
+            description: "Warning message.",
+            type: String,
+        },
+    },
+)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "PascalCase")]
+/// Warning for an APT repository.
+pub struct APTRepositoryWarning {
+    /// Path to the defining file.
+    #[serde(skip_serializing_if = "String::is_empty")]
+    pub path: String,
+    /// Line or stanza number.
+    pub number: usize,
+    /// Warning
+    #[serde(skip_serializing_if = "String::is_empty")]
+    pub warning: String,
+}
diff --git a/tests/repositories.rs b/tests/repositories.rs
index deb2f6a..1b548bd 100644
--- a/tests/repositories.rs
+++ b/tests/repositories.rs
@@ -4,8 +4,8 @@ use std::path::PathBuf;
 use anyhow::{bail, format_err, Error};
 
 use proxmox_apt::repositories::{
-    enterprise_repository_enabled, no_subscription_repository_enabled, repositories_from_files,
-    write_repositories,
+    check_repositories, enterprise_repository_enabled, no_subscription_repository_enabled,
+    repositories_from_files, write_repositories,
 };
 use proxmox_apt::types::APTRepository;
 
@@ -98,3 +98,59 @@ fn test_proxmox_repositories() -> Result<(), Error> {
 
     Ok(())
 }
+
+#[test]
+fn test_check_repositories() -> Result<(), Error> {
+    let test_dir = std::env::current_dir()?.join("tests");
+    let read_dir = test_dir.join("sources.list.d");
+
+    let pve_list = read_dir.join("pve.list");
+    let repos = repositories_from_files(&vec![pve_list])?;
+
+    let warnings = check_repositories(&repos);
+
+    assert_eq!(warnings.is_empty(), true);
+
+    let bad_sources = read_dir.join("bad.sources");
+    let repos = repositories_from_files(&vec![bad_sources])?;
+
+    let mut expected_warnings = HashMap::<String, String>::new();
+    expected_warnings.insert(
+        "bad.sources:1".to_string(),
+        "suite 'sid' should not be used in production!".to_string(),
+    );
+    expected_warnings.insert(
+        "bad.sources:2".to_string(),
+        "old suite 'lenny-backports' configured!".to_string(),
+    );
+    expected_warnings.insert(
+        "bad.sources:3".to_string(),
+        "old suite 'stretch/updates' configured!".to_string(),
+    );
+    expected_warnings.insert(
+        "bad.sources:4".to_string(),
+        "use the name of the stable distribution instead of 'stable'!".to_string(),
+    );
+
+    let warnings = check_repositories(&repos);
+
+    assert_eq!(warnings.len(), expected_warnings.len());
+
+    for warning in warnings.iter() {
+        let path = PathBuf::from(warning.path.clone());
+        let file_name = path
+            .file_name()
+            .unwrap()
+            .to_os_string()
+            .into_string()
+            .unwrap();
+
+        let key = format!("{}:{}", file_name, warning.number);
+        let message = expected_warnings.get(&key);
+
+        assert_eq!(message.is_some(), true);
+        assert_eq!(*message.unwrap(), warning.warning);
+    }
+
+    Ok(())
+}
diff --git a/tests/sources.list.d.expected/bad.sources b/tests/sources.list.d.expected/bad.sources
new file mode 100644
index 0000000..7601263
--- /dev/null
+++ b/tests/sources.list.d.expected/bad.sources
@@ -0,0 +1,20 @@
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: sid
+Components: main contrib
+
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: lenny-backports
+Components: contrib
+
+Types: deb
+URIs: http://security.debian.org
+Suites: stretch/updates
+Components: main contrib
+
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: stable
+Components: main
+
diff --git a/tests/sources.list.d/bad.sources b/tests/sources.list.d/bad.sources
new file mode 100644
index 0000000..963f527
--- /dev/null
+++ b/tests/sources.list.d/bad.sources
@@ -0,0 +1,19 @@
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: sid
+Components: main contrib
+
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: lenny-backports
+Components: contrib
+
+Types: deb
+URIs: http://security.debian.org
+Suites: stretch/updates
+Components: main contrib
+
+Suites: stable
+URIs: http://ftp.at.debian.org/debian
+Components: main
+Types: deb
-- 
2.20.1