From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 2926073311; Thu, 17 Jun 2021 16:16:58 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 268611D53F; Thu, 17 Jun 2021 16:16:58 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (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 953CC1D533; Thu, 17 Jun 2021 16:16:56 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 6F30F441FE; Thu, 17 Jun 2021 16:16:56 +0200 (CEST) Date: Thu, 17 Jun 2021 16:16:47 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: pbs-devel@lists.proxmox.com, Proxmox VE development discussion References: <20210611114418.28772-1-f.ebner@proxmox.com> <20210611114418.28772-5-f.ebner@proxmox.com> In-Reply-To: <20210611114418.28772-5-f.ebner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1623937751.bt9u5op5bu.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.794 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH v6 proxmox-apt 04/11] add check_repositories function X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Jun 2021 14:16:58 -0000 On June 11, 2021 1:43 pm, Fabian Ebner wrote: > which checks for bad suites and official URIs. >=20 > Signed-off-by: Fabian Ebner > --- >=20 > Changes from v5: > * split out host_from_uri helper and also handle userinfo and port > * test an offical URI with port > * match all *.debian.org and *.proxmox.com as official to avoid (futu= re) > false negatives. > * add bookworm and trixie codenames to the list of new_suites >=20 > src/repositories/check.rs | 174 +++++++++++++++++++++- > src/repositories/mod.rs | 19 ++- > src/types.rs | 19 +++ > tests/repositories.rs | 97 +++++++++++- > tests/sources.list.d.expected/bad.sources | 30 ++++ > tests/sources.list.d/bad.sources | 29 ++++ > 6 files changed, 364 insertions(+), 4 deletions(-) > create mode 100644 tests/sources.list.d.expected/bad.sources > create mode 100644 tests/sources.list.d/bad.sources >=20 > diff --git a/src/repositories/check.rs b/src/repositories/check.rs > index a682b69..585c28d 100644 > --- a/src/repositories/check.rs > +++ b/src/repositories/check.rs > @@ -1,6 +1,45 @@ > use anyhow::{bail, Error}; > =20 > -use crate::types::{APTRepository, APTRepositoryFileType, APTRepositoryPa= ckageType}; > +use crate::types::{ > + APTRepository, APTRepositoryFile, APTRepositoryFileType, APTReposito= ryInfo, > + APTRepositoryPackageType, > +}; > + > +/// Splits the suite into its base part and variant. > +fn suite_variant(suite: &str) -> (&str, &str) { > + let variants =3D ["-backports-sloppy", "-backports", "-updates", "/u= pdates"]; > + > + for variant in variants.iter() { > + if let Some(base) =3D suite.strip_suffix(variant) { > + return (base, variant); > + } > + } > + > + (suite, "") > +} > + > +/// Get the host part from a given URI. > +fn host_from_uri(uri: &str) -> Option<&str> { this has false-positives for file:// URIs. the way it is currently used,=20 it might make sense to limit it to http(s)? > + if let Some(begin) =3D uri.find("://") { > + let mut host =3D uri.split_at(begin + 3).1; > + > + if let Some(end) =3D host.find('/') { > + host =3D host.split_at(end).0; > + } > + > + if let Some(begin) =3D host.find('@') { > + host =3D host.split_at(begin + 1).1; > + } > + > + if let Some(end) =3D host.find(':') { > + host =3D host.split_at(end).0; > + } > + > + return Some(host); > + } > + > + None > +} > =20 > impl APTRepository { > /// Makes sure that all basic properties of a repository are present= and > @@ -102,4 +141,137 @@ impl APTRepository { > false > } > } > + > + /// Checks if old or unstable suites are configured and also that th= e > + /// `stable` keyword is not used. > + fn check_suites(&self, add_info: &mut dyn FnMut(String, String)) { > + let old_suites =3D [ > + "lenny", > + "squeeze", > + "wheezy", > + "jessie", > + "stretch", > + "oldoldstable", > + "oldstable", > + ]; > + > + let next_suite =3D "bullseye"; > + > + let new_suites =3D [ > + "bookworm", > + "trixie", > + "testing", > + "unstable", > + "sid", > + "experimental", > + ]; > + > + if self > + .types > + .iter() > + .any(|package_type| *package_type =3D=3D APTRepositoryPackag= eType::Deb) > + { > + for suite in self.suites.iter() { > + if old_suites > + .iter() > + .any(|base_suite| suite_variant(suite).0 =3D=3D *bas= e_suite) > + { > + add_info( > + "warning".to_string(), > + format!("old suite '{}' configured!", suite), > + ); > + } > + > + if suite_variant(suite).0 =3D=3D next_suite { > + add_info( > + "ignore-pre-upgrade-warning".to_string(), > + format!("suite '{}' should not be used in produc= tion!", suite), > + ); > + } > + > + if new_suites > + .iter() > + .any(|base_suite| suite_variant(suite).0 =3D=3D *bas= e_suite) > + { > + add_info( > + "warning".to_string(), > + format!("suite '{}' should not be used in produc= tion!", suite), > + ); > + } > + > + if suite_variant(suite).0 =3D=3D "stable" { > + add_info( > + "warning".to_string(), > + "use the name of the stable distribution instead= of 'stable'!".to_string(), > + ); > + } > + } > + } > + } > + > + /// Checks if an official host is configured in the repository. > + fn check_uris(&self) -> Option<(String, String)> { > + let official_host =3D |domains: &Vec<&str>| match domains.split_= last() { > + Some((last, rest)) =3D> match rest.split_last() { > + Some((second_to_last, _rest)) =3D> { > + (*last =3D=3D "org" && *second_to_last =3D=3D "debia= n") > + || (*last =3D=3D "com" && *second_to_last =3D=3D= "proxmox") > + } > + None =3D> false, > + }, > + None =3D> false, > + }; > + > + for uri in self.uris.iter() { > + if let Some(host) =3D host_from_uri(uri) { > + let domains =3D host.split('.').collect(); > + > + if official_host(&domains) { > + return Some(("badge".to_string(), "official host nam= e".to_string())); > + } > + } > + } > + > + None > + } > +} > + > +impl APTRepositoryFile { > + /// Checks if old or unstable suites are configured and also that th= e > + /// `stable` keyword is not used. > + pub fn check_suites(&self) -> Vec { > + let mut infos =3D vec![]; > + > + for (n, repo) in self.repositories.iter().enumerate() { > + let mut add_info =3D |kind, message| { > + infos.push(APTRepositoryInfo { > + path: self.path.clone(), > + number: n + 1, > + kind, > + message, > + }) > + }; > + repo.check_suites(&mut add_info); > + } > + > + infos > + } > + > + /// Checks for official URIs. > + pub fn check_uris(&self) -> Vec { > + let mut infos =3D vec![]; > + > + for (n, repo) in self.repositories.iter().enumerate() { > + if let Some((kind, message)) =3D repo.check_uris() { > + infos.push(APTRepositoryInfo { > + path: self.path.clone(), > + number: n + 1, > + kind, > + message, > + }); > + } > + } > + > + infos > + } > } > diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs > index b7919a9..c2bbc06 100644 > --- a/src/repositories/mod.rs > +++ b/src/repositories/mod.rs > @@ -4,7 +4,7 @@ use anyhow::{bail, format_err, Error}; > =20 > use crate::types::{ > APTRepository, APTRepositoryFile, APTRepositoryFileError, APTReposit= oryFileType, > - APTRepositoryOption, > + APTRepositoryInfo, APTRepositoryOption, > }; > =20 > mod list_parser; > @@ -148,6 +148,23 @@ impl APTRepositoryFile { > } > } > =20 > +/// 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 configure= d. > +/// `badge` for official URIs. > +pub fn check_repositories(files: &[APTRepositoryFile]) -> Vec { > + let mut infos =3D vec![]; > + > + for file in files.iter() { > + infos.append(&mut file.check_suites()); > + infos.append(&mut file.check_uris()); > + } > + > + infos > +} > + > /// Checks if the enterprise repository for the specified Proxmox produc= t is > /// configured and enabled. > pub fn enterprise_repository_enabled(files: &[APTRepositoryFile], produc= t: &str) -> bool { > diff --git a/src/types.rs b/src/types.rs > index bbd8e7e..057fffa 100644 > --- a/src/types.rs > +++ b/src/types.rs > @@ -244,3 +244,22 @@ impl std::error::Error for APTRepositoryFileError { > None > } > } > + > +#[api] > +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deseri= alize)] > +#[serde(rename_all =3D "lowercase")] > +/// Additional information for a repository. > +pub struct APTRepositoryInfo { > + /// Path to the defining file. > + #[serde(skip_serializing_if =3D "String::is_empty")] > + pub path: String, > + > + /// Number of the associated respository within the file. > + pub number: usize, > + > + /// Info kind (e.g. "warning") > + pub kind: String, > + > + /// Info message > + pub message: String, > +} > diff --git a/tests/repositories.rs b/tests/repositories.rs > index ffb1888..9b0cd56 100644 > --- a/tests/repositories.rs > +++ b/tests/repositories.rs > @@ -3,9 +3,10 @@ use std::path::PathBuf; > use anyhow::{bail, format_err, Error}; > =20 > use proxmox_apt::repositories::{ > - enterprise_repository_enabled, no_subscription_repository_enabled, w= rite_repositories, > + check_repositories, enterprise_repository_enabled, no_subscription_r= epository_enabled, > + write_repositories, > }; > -use proxmox_apt::types::APTRepositoryFile; > +use proxmox_apt::types::{APTRepositoryFile, APTRepositoryInfo}; > =20 > #[test] > fn test_parse_write() -> Result<(), Error> { > @@ -159,3 +160,95 @@ fn test_proxmox_repositories() -> Result<(), Error> = { > =20 > Ok(()) > } > + > +#[test] > +fn test_check_repositories() -> Result<(), Error> { > + let test_dir =3D std::env::current_dir()?.join("tests"); > + let read_dir =3D test_dir.join("sources.list.d"); > + > + let absolute_suite_list =3D read_dir.join("absolute_suite.list"); > + let mut file =3D APTRepositoryFile::new(&absolute_suite_list)?.unwra= p(); > + file.parse()?; > + > + let infos =3D check_repositories(&vec![file]); > + > + assert_eq!(infos.is_empty(), true); > + let pve_list =3D read_dir.join("pve.list"); > + let mut file =3D APTRepositoryFile::new(&pve_list)?.unwrap(); > + file.parse()?; > + > + let path_string =3D pve_list.into_os_string().into_string().unwrap()= ; > + > + let mut expected_infos =3D vec![]; > + for n in 1..=3D5 { > + expected_infos.push(APTRepositoryInfo { > + path: path_string.clone(), > + number: n, > + kind: "badge".to_string(), > + message: "official host name".to_string(), > + }); > + } > + > + let mut infos =3D check_repositories(&vec![file]); > + > + assert_eq!(infos.sort(), expected_infos.sort()); > + > + let bad_sources =3D read_dir.join("bad.sources"); > + let mut file =3D APTRepositoryFile::new(&bad_sources)?.unwrap(); > + file.parse()?; > + > + let path_string =3D bad_sources.into_os_string().into_string().unwra= p(); > + > + let mut expected_infos =3D vec![ > + APTRepositoryInfo { > + path: path_string.clone(), > + number: 1, > + kind: "warning".to_string(), > + message: "suite 'sid' should not be used in production!".to_= string(), > + }, > + APTRepositoryInfo { > + path: path_string.clone(), > + number: 2, > + kind: "warning".to_string(), > + message: "old suite 'lenny-backports' configured!".to_string= (), > + }, > + APTRepositoryInfo { > + path: path_string.clone(), > + number: 3, > + kind: "warning".to_string(), > + message: "old suite 'stretch/updates' configured!".to_string= (), > + }, > + APTRepositoryInfo { > + path: path_string.clone(), > + number: 4, > + kind: "warning".to_string(), > + message: "use the name of the stable distribution instead of= 'stable'!".to_string(), > + }, > + APTRepositoryInfo { > + path: path_string.clone(), > + number: 5, > + kind: "ignore-pre-upgrade-warning".to_string(), > + message: "suite 'bullseye' should not be used in production!= ".to_string(), > + }, > + APTRepositoryInfo { > + path: path_string.clone(), > + number: 6, > + kind: "warning".to_string(), > + message: "suite 'testing' should not be used in production!"= .to_string(), > + }, > + ]; > + for n in 1..=3D6 { > + expected_infos.push(APTRepositoryInfo { > + path: path_string.clone(), > + number: n, > + kind: "badge".to_string(), > + message: "official URI".to_string(), > + }); > + } > + > + let mut infos =3D check_repositories(&vec![file]); > + > + assert_eq!(infos.sort(), expected_infos.sort()); > + > + Ok(()) > +} > diff --git a/tests/sources.list.d.expected/bad.sources b/tests/sources.li= st.d.expected/bad.sources > new file mode 100644 > index 0000000..b630c89 > --- /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: bullseye > +Components: main > + > +Types: deb > +URIs: http://ftp.at.debian.org/debian > +Suites: testing > +Components: main > + > diff --git a/tests/sources.list.d/bad.sources b/tests/sources.list.d/bad.= sources > new file mode 100644 > index 0000000..1aab2ea > --- /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: bullseye > +URIs: http://ftp.at.debian.org/debian > +Components: main > +Types: deb > + > +Suites: testing > +URIs: http://ftp.at.debian.org/debian > +Components: main > +Types: deb > --=20 > 2.20.1 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20