From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <f.ebner@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 1F73F6937F
 for <pbs-devel@lists.proxmox.com>; Mon, 22 Mar 2021 13:00:31 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id C7740232C5
 for <pbs-devel@lists.proxmox.com>; Mon, 22 Mar 2021 13:00:00 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [212.186.127.180])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id 3124823224
 for <pbs-devel@lists.proxmox.com>; Mon, 22 Mar 2021 12:59:56 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id EDD0F4635B
 for <pbs-devel@lists.proxmox.com>; Mon, 22 Mar 2021 12:59:55 +0100 (CET)
From: Fabian Ebner <f.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Mon, 22 Mar 2021 12:59:39 +0100
Message-Id: <20210322115945.1362-5-f.ebner@proxmox.com>
X-Mailer: git-send-email 2.20.1
In-Reply-To: <20210322115945.1362-1-f.ebner@proxmox.com>
References: <20210322115945.1362-1-f.ebner@proxmox.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.046 Adjusted score from AWL reputation of From: address
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 PROLO_LEO1                0.1 Meta Catches all Leo drug variations so far
 RCVD_IN_DNSWL_MED        -2.3 Sender listed at https://www.dnswl.org/,
 medium trust
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: [pbs-devel] [PATCH v3 proxmox-apt 04/10] add check_repositories
 function
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Mon, 22 Mar 2021 12:00:31 -0000

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