[pve-devel] [PATCH v7 proxmox-apt 3/5] add more functions to check repositories

Fabian Ebner f.ebner at proxmox.com
Wed Jun 23 15:38:56 CEST 2021


Currently includes check for suites and check for official URIs

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

Changes from v6:
    * move impl blocks and APTRepositoryInfo type  and helpers into
      repository.rs and mod.rs
    * limit host_from_uri to http(s)
    * fix test by not using assert_eq!(a.sort(), b.sort()) (sort() returns (),
      so this was always true...)
    * simplify host matching in check_uris()
    * make check_suites dependent on the current release from /etc/os-release
    * add 'property' to APTRepositoryInfo to know where it originates from
    * use index (starting from 0) instead of number (starting from 1) in
      APTRepositoryInfo. Originally it came from line number, but each
      APTRepositoryFile contains an array of repos and thus index is the better
      fit.

 src/repositories/file.rs                  | 119 +++++++++++++++++++++-
 src/repositories/mod.rs                   |  21 +++-
 src/repositories/release.rs               |  42 ++++++++
 src/repositories/repository.rs            |  58 +++++++++++
 tests/repositories.rs                     | 106 ++++++++++++++++++-
 tests/sources.list.d.expected/bad.sources |  30 ++++++
 tests/sources.list.d.expected/pve.list    |   2 +
 tests/sources.list.d/bad.sources          |  29 ++++++
 tests/sources.list.d/pve.list             |   2 +
 9 files changed, 405 insertions(+), 4 deletions(-)
 create mode 100644 src/repositories/release.rs
 create mode 100644 tests/sources.list.d.expected/bad.sources
 create mode 100644 tests/sources.list.d/bad.sources

diff --git a/src/repositories/file.rs b/src/repositories/file.rs
index bc48bf2..6225f1c 100644
--- a/src/repositories/file.rs
+++ b/src/repositories/file.rs
@@ -2,10 +2,13 @@ use std::convert::TryFrom;
 use std::fmt::Display;
 use std::path::{Path, PathBuf};
 
-use anyhow::{format_err, Error};
+use anyhow::{bail, format_err, Error};
 use serde::{Deserialize, Serialize};
 
-use crate::repositories::repository::{APTRepository, APTRepositoryFileType};
+use crate::repositories::release::{get_current_release_codename, DEBIAN_SUITES};
+use crate::repositories::repository::{
+    APTRepository, APTRepositoryFileType, APTRepositoryPackageType,
+};
 
 use proxmox::api::api;
 
@@ -85,6 +88,29 @@ impl std::error::Error for APTRepositoryFileError {
     }
 }
 
+#[api]
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Additional information for a repository.
+pub struct APTRepositoryInfo {
+    /// Path to the defining file.
+    #[serde(skip_serializing_if = "String::is_empty")]
+    pub path: String,
+
+    /// Index of the associated respository within the file (starting from 0).
+    pub index: usize,
+
+    /// The property from which the info originates (e.g. "Suites")
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub property: Option<String>,
+
+    /// Info kind (e.g. "warning")
+    pub kind: String,
+
+    /// Info message
+    pub message: String,
+}
+
 impl APTRepositoryFile {
     /// Creates a new `APTRepositoryFile` without parsing.
     ///
@@ -271,4 +297,93 @@ impl APTRepositoryFile {
 
         Ok(())
     }
+
+    /// Checks if old or unstable suites are configured and also that the
+    /// `stable` keyword is not used.
+    pub fn check_suites(&self) -> Result<Vec<APTRepositoryInfo>, Error> {
+        let mut infos = vec![];
+
+        for (n, repo) in self.repositories.iter().enumerate() {
+            if !repo
+                .types
+                .iter()
+                .any(|package_type| *package_type == APTRepositoryPackageType::Deb)
+            {
+                continue;
+            }
+
+            let mut add_info = |kind, message| {
+                infos.push(APTRepositoryInfo {
+                    path: self.path.clone(),
+                    index: n,
+                    property: Some("Suites".to_string()),
+                    kind,
+                    message,
+                })
+            };
+
+            let current_suite = get_current_release_codename()?;
+
+            let current_index = match DEBIAN_SUITES
+                .iter()
+                .position(|&suite| suite == current_suite)
+            {
+                Some(index) => index,
+                None => bail!("unknown release {}", current_suite),
+            };
+
+            for (n, suite) in DEBIAN_SUITES.iter().enumerate() {
+                if repo.has_suite_variant(suite) {
+                    if n < current_index {
+                        add_info(
+                            "warning".to_string(),
+                            format!("old suite '{}' configured!", suite),
+                        );
+                    }
+
+                    if n == current_index + 1 {
+                        add_info(
+                            "ignore-pre-upgrade-warning".to_string(),
+                            format!("suite '{}' should not be used in production!", suite),
+                        );
+                    }
+
+                    if n > current_index + 1 {
+                        add_info(
+                            "warning".to_string(),
+                            format!("suite '{}' should not be used in production!", suite),
+                        );
+                    }
+                }
+            }
+
+            if repo.has_suite_variant("stable") {
+                add_info(
+                    "warning".to_string(),
+                    "use the name of the stable distribution instead of 'stable'!".to_string(),
+                );
+            }
+        }
+
+        Ok(infos)
+    }
+
+    /// Checks for official URIs.
+    pub fn check_uris(&self) -> Vec<APTRepositoryInfo> {
+        let mut infos = vec![];
+
+        for (n, repo) in self.repositories.iter().enumerate() {
+            if repo.has_official_uri() {
+                infos.push(APTRepositoryInfo {
+                    path: self.path.clone(),
+                    index: n,
+                    kind: "badge".to_string(),
+                    property: Some("URIs".to_string()),
+                    message: "official host name".to_string(),
+                });
+            }
+        }
+
+        infos
+    }
 }
diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
index d540bcb..fc54857 100644
--- a/src/repositories/mod.rs
+++ b/src/repositories/mod.rs
@@ -9,7 +9,9 @@ pub use repository::{
 };
 
 mod file;
-pub use file::{APTRepositoryFile, APTRepositoryFileError};
+pub use file::{APTRepositoryFile, APTRepositoryFileError, APTRepositoryInfo};
+
+mod release;
 
 const APT_SOURCES_LIST_FILENAME: &str = "/etc/apt/sources.list";
 const APT_SOURCES_LIST_DIRECTORY: &str = "/etc/apt/sources.list.d/";
@@ -37,6 +39,23 @@ fn common_digest(files: &[APTRepositoryFile]) -> [u8; 32] {
     openssl::sha::sha256(&common_raw[..])
 }
 
+/// Provides additional information about the repositories.
+///
+/// The kind of information can be:
+/// `warnings` for bad suites.
+/// `ignore-pre-upgrade-warning` when the next stable suite is configured.
+/// `badge` for official URIs.
+pub fn check_repositories(files: &[APTRepositoryFile]) -> Result<Vec<APTRepositoryInfo>, Error> {
+    let mut infos = vec![];
+
+    for file in files.iter() {
+        infos.append(&mut file.check_suites()?);
+        infos.append(&mut file.check_uris());
+    }
+
+    Ok(infos)
+}
+
 /// Returns all APT repositories configured in `/etc/apt/sources.list` and
 /// in `/etc/apt/sources.list.d` including disabled repositories.
 ///
diff --git a/src/repositories/release.rs b/src/repositories/release.rs
new file mode 100644
index 0000000..688f038
--- /dev/null
+++ b/src/repositories/release.rs
@@ -0,0 +1,42 @@
+use anyhow::{bail, format_err, Error};
+
+use std::io::{BufRead, BufReader};
+
+/// The suites of Debian releases, ordered chronologically, with variable releases
+/// like 'oldstable' and 'testing' ordered at the extremes. Does not include 'stable'.
+pub const DEBIAN_SUITES: [&str; 15] = [
+    "oldoldstable",
+    "oldstable",
+    "lenny",
+    "squeeze",
+    "wheezy",
+    "jessie",
+    "stretch",
+    "buster",
+    "bullseye",
+    "bookworm",
+    "trixie",
+    "sid",
+    "testing",
+    "unstable",
+    "experimental",
+];
+
+/// Read the `VERSION_CODENAME` from `/etc/os-release`.
+pub fn get_current_release_codename() -> Result<String, Error> {
+    let raw = std::fs::read("/etc/os-release")
+        .map_err(|err| format_err!("unable to read '/etc/os-release' - {}", err))?;
+
+    let reader = BufReader::new(&*raw);
+
+    for line in reader.lines() {
+        let line = line.map_err(|err| format_err!("unable to read '/etc/os-release' - {}", err))?;
+
+        if let Some(codename) = line.strip_prefix("VERSION_CODENAME=") {
+            let codename = codename.trim_matches(&['"', '\''][..]);
+            return Ok(codename.to_string());
+        }
+    }
+
+    bail!("unable to parse codename from '/etc/os-release'");
+}
diff --git a/src/repositories/repository.rs b/src/repositories/repository.rs
index 9ee7df9..875e4ee 100644
--- a/src/repositories/repository.rs
+++ b/src/repositories/repository.rs
@@ -266,6 +266,30 @@ impl APTRepository {
         Ok(())
     }
 
+    /// Check if a variant of the given suite is configured in this repository
+    pub fn has_suite_variant(&self, base_suite: &str) -> bool {
+        self.suites
+            .iter()
+            .any(|suite| suite_variant(suite).0 == base_suite)
+    }
+
+    /// Checks if an official host is configured in the repository.
+    pub fn has_official_uri(&self) -> bool {
+        for uri in self.uris.iter() {
+            if let Some(host) = host_from_uri(uri) {
+                if host == "proxmox.com"
+                    || host.ends_with(".proxmox.com")
+                    || host == "debian.org"
+                    || host.ends_with(".debian.org")
+                {
+                    return true;
+                }
+            }
+        }
+
+        false
+    }
+
     /// Writes a repository in the corresponding format followed by a blank.
     ///
     /// Expects that `basic_check()` for the repository was successful.
@@ -277,6 +301,40 @@ impl APTRepository {
     }
 }
 
+/// Get the host part from a given URI.
+fn host_from_uri(uri: &str) -> Option<&str> {
+    let host = uri.strip_prefix("http")?;
+    let host = host.strip_prefix("s").unwrap_or(host);
+    let mut host = host.strip_prefix("://")?;
+
+    if let Some(end) = host.find('/') {
+        host = &host[..end];
+    }
+
+    if let Some(begin) = host.find('@') {
+        host = &host[(begin + 1)..];
+    }
+
+    if let Some(end) = host.find(':') {
+        host = &host[..end];
+    }
+
+    Some(host)
+}
+
+/// Splits the suite into its base part and variant.
+fn suite_variant(suite: &str) -> (&str, &str) {
+    let variants = ["-backports-sloppy", "-backports", "-updates", "/updates"];
+
+    for variant in variants.iter() {
+        if let Some(base) = suite.strip_suffix(variant) {
+            return (base, variant);
+        }
+    }
+
+    (suite, "")
+}
+
 /// Writes a repository in one-line format followed by a blank line.
 ///
 /// Expects that `repo.file_type == APTRepositoryFileType::List`.
diff --git a/tests/repositories.rs b/tests/repositories.rs
index 477d718..58f1322 100644
--- a/tests/repositories.rs
+++ b/tests/repositories.rs
@@ -2,7 +2,7 @@ use std::path::PathBuf;
 
 use anyhow::{bail, format_err, Error};
 
-use proxmox_apt::repositories::APTRepositoryFile;
+use proxmox_apt::repositories::{check_repositories, APTRepositoryFile, APTRepositoryInfo};
 
 #[test]
 fn test_parse_write() -> Result<(), Error> {
@@ -160,3 +160,107 @@ fn test_empty_write() -> 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 absolute_suite_list = read_dir.join("absolute_suite.list");
+    let mut file = APTRepositoryFile::new(&absolute_suite_list)?.unwrap();
+    file.parse()?;
+
+    let infos = check_repositories(&vec![file])?;
+
+    assert_eq!(infos.is_empty(), true);
+    let pve_list = read_dir.join("pve.list");
+    let mut file = APTRepositoryFile::new(&pve_list)?.unwrap();
+    file.parse()?;
+
+    let path_string = pve_list.into_os_string().into_string().unwrap();
+
+    let mut expected_infos = vec![];
+    for n in 0..=5 {
+        expected_infos.push(APTRepositoryInfo {
+            path: path_string.clone(),
+            index: n,
+            property: Some("URIs".to_string()),
+            kind: "badge".to_string(),
+            message: "official host name".to_string(),
+        });
+    }
+    expected_infos.sort();
+
+    let mut infos = check_repositories(&vec![file])?;
+    infos.sort();
+
+    assert_eq!(infos, expected_infos);
+
+    let bad_sources = read_dir.join("bad.sources");
+    let mut file = APTRepositoryFile::new(&bad_sources)?.unwrap();
+    file.parse()?;
+
+    let path_string = bad_sources.into_os_string().into_string().unwrap();
+
+    let mut expected_infos = vec![
+        APTRepositoryInfo {
+            path: path_string.clone(),
+            index: 0,
+            property: Some("Suites".to_string()),
+            kind: "warning".to_string(),
+            message: "suite 'sid' should not be used in production!".to_string(),
+        },
+        APTRepositoryInfo {
+            path: path_string.clone(),
+            index: 1,
+            property: Some("Suites".to_string()),
+            kind: "warning".to_string(),
+            message: "old suite 'lenny' configured!".to_string(),
+        },
+        APTRepositoryInfo {
+            path: path_string.clone(),
+            index: 2,
+            property: Some("Suites".to_string()),
+            kind: "warning".to_string(),
+            message: "old suite 'stretch' configured!".to_string(),
+        },
+        APTRepositoryInfo {
+            path: path_string.clone(),
+            index: 3,
+            property: Some("Suites".to_string()),
+            kind: "warning".to_string(),
+            message: "use the name of the stable distribution instead of 'stable'!".to_string(),
+        },
+        APTRepositoryInfo {
+            path: path_string.clone(),
+            index: 4,
+            property: Some("Suites".to_string()),
+            kind: "ignore-pre-upgrade-warning".to_string(),
+            message: "suite 'bookworm' should not be used in production!".to_string(),
+        },
+        APTRepositoryInfo {
+            path: path_string.clone(),
+            index: 5,
+            property: Some("Suites".to_string()),
+            kind: "warning".to_string(),
+            message: "suite 'testing' should not be used in production!".to_string(),
+        },
+    ];
+    for n in 0..=5 {
+        expected_infos.push(APTRepositoryInfo {
+            path: path_string.clone(),
+            index: n,
+            property: Some("URIs".to_string()),
+            kind: "badge".to_string(),
+            message: "official host name".to_string(),
+        });
+    }
+    expected_infos.sort();
+
+    let mut infos = check_repositories(&vec![file])?;
+    infos.sort();
+
+    assert_eq!(infos, expected_infos);
+
+    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..7eff6a5
--- /dev/null
+++ b/tests/sources.list.d.expected/bad.sources
@@ -0,0 +1,30 @@
+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:80
+Suites: stretch/updates
+Components: main contrib
+
+Types: deb
+URIs: http://ftp.at.debian.org:80/debian
+Suites: stable
+Components: main
+
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: bookworm
+Components: main
+
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: testing
+Components: main
+
diff --git a/tests/sources.list.d.expected/pve.list b/tests/sources.list.d.expected/pve.list
index 95725ab..c801261 100644
--- a/tests/sources.list.d.expected/pve.list
+++ b/tests/sources.list.d.expected/pve.list
@@ -8,6 +8,8 @@ deb http://download.proxmox.com/debian/pve bullseye pve-no-subscription
 
 # deb https://enterprise.proxmox.com/debian/pve bullseye pve-enterprise
 
+deb-src https://enterprise.proxmox.com/debian/pve buster pve-enterprise
+
 # security updates
 deb http://security.debian.org/debian-security bullseye-security main contrib
 
diff --git a/tests/sources.list.d/bad.sources b/tests/sources.list.d/bad.sources
new file mode 100644
index 0000000..46eb82a
--- /dev/null
+++ b/tests/sources.list.d/bad.sources
@@ -0,0 +1,29 @@
+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:80
+Suites: stretch/updates
+Components: main contrib
+
+Suites: stable
+URIs: http://ftp.at.debian.org:80/debian
+Components: main
+Types: deb
+
+Suites: bookworm
+URIs: http://ftp.at.debian.org/debian
+Components: main
+Types: deb
+
+Suites: testing
+URIs: http://ftp.at.debian.org/debian
+Components: main
+Types: deb
diff --git a/tests/sources.list.d/pve.list b/tests/sources.list.d/pve.list
index c6d451a..4d36d3d 100644
--- a/tests/sources.list.d/pve.list
+++ b/tests/sources.list.d/pve.list
@@ -6,5 +6,7 @@ deb http://ftp.debian.org/debian bullseye-updates main contrib
 deb http://download.proxmox.com/debian/pve bullseye pve-no-subscription
 # deb https://enterprise.proxmox.com/debian/pve bullseye pve-enterprise
 
+deb-src https://enterprise.proxmox.com/debian/pve buster pve-enterprise
+
 # security updates
 deb http://security.debian.org/debian-security bullseye-security main contrib
-- 
2.30.2






More information about the pve-devel mailing list