all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fabian Ebner <f.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH v3 proxmox-apt 04/10] add check_repositories function
Date: Mon, 22 Mar 2021 12:59:39 +0100	[thread overview]
Message-ID: <20210322115945.1362-5-f.ebner@proxmox.com> (raw)
In-Reply-To: <20210322115945.1362-1-f.ebner@proxmox.com>

which, for now, checks the suites.

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

Changes from v2:
    * incorporate Wolfgang's feedback:
        * improve suite_is_variant by using strip_prefix and matches!(), which
          as a bonus allows to gets rid of the one clippy exception I had added
          for readability reasons

 src/repositories/check.rs                 | 76 ++++++++++++++++++++++-
 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, 217 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..d63eb3c 100644
--- a/src/repositories/check.rs
+++ b/src/repositories/check.rs
@@ -1,6 +1,21 @@
 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`.
+fn suite_is_variant(suite: &str, base_suite: &str) -> bool {
+    matches!(
+        suite.strip_prefix(base_suite),
+        Some("")
+            | Some("-backports")
+            | Some("-backports-sloppy")
+            | Some("-updates")
+            | Some("/updates")
+    )
+}
 
 impl APTRepository {
     /// Makes sure that all basic properties of a repository are present and
@@ -45,6 +60,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 bd0af1d..cc7c666 100644
--- a/src/repositories/mod.rs
+++ b/src/repositories/mod.rs
@@ -6,7 +6,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;
@@ -107,6 +109,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 be69652..8bfafd2 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -209,3 +209,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





  parent reply	other threads:[~2021-03-22 12:00 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-22 11:59 [pbs-devel] [PATCH-SERIES v3] APT repositories API/UI Fabian Ebner
2021-03-22 11:59 ` [pbs-devel] [PATCH v3 proxmox-apt 01/10] initial commit Fabian Ebner
2021-03-22 11:59 ` [pbs-devel] [PATCH v3 proxmox-apt 02/10] add files for Debian packaging Fabian Ebner
2021-03-22 11:59 ` [pbs-devel] [PATCH v3 proxmox-apt 03/10] add functions to check for Proxmox repositories Fabian Ebner
2021-03-22 11:59 ` Fabian Ebner [this message]
2021-03-22 11:59 ` [pbs-devel] [PATCH v3 proxmox-backup 05/10] depend on new proxmox-apt crate Fabian Ebner
2021-03-22 11:59 ` [pbs-devel] [PATCH v3 proxmox-backup 06/10] api: apt: add repositories call Fabian Ebner
2021-03-22 11:59 ` [pbs-devel] [PATCH v3 proxmox-backup 07/10] ui: add repositories Fabian Ebner
2021-03-22 11:59 ` [pbs-devel] [PATCH v3 proxmox-backup 08/10] add check_repositories_call Fabian Ebner
2021-03-22 11:59 ` [pbs-devel] [PATCH v3 widget-toolkit 09/10] add UI for APT repositories Fabian Ebner
2021-03-22 11:59 ` [pbs-devel] [PATCH v3 widget-toolkit 10/10] APT repositories: add warnings Fabian Ebner
2021-03-23 10:29 ` [pbs-devel] [PATCH-SERIES v3] APT repositories API/UI Fabian Grünbichler
2021-03-24  9:40   ` Fabian Ebner
2021-03-24 10:06     ` Fabian Grünbichler
2021-03-24 12:08       ` Fabian Ebner
2021-03-24 12:39         ` Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210322115945.1362-5-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal