From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 19AFA1FF2CF for ; Tue, 9 Jul 2024 08:18:48 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E4E5418713; Tue, 9 Jul 2024 08:19:09 +0200 (CEST) From: Wolfgang Bumiller To: pbs-devel@lists.proxmox.com Date: Tue, 9 Jul 2024 08:18:29 +0200 Message-Id: <20240709061832.52121-3-w.bumiller@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240709061832.52121-1-w.bumiller@proxmox.com> References: <20240709061832.52121-1-w.bumiller@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.087 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy 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: [pbs-devel] [PATCH proxmox 3/6] apt: avoid direct impl on api types (use traits instead) X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" From: Dietmar Maurer So that we can use api types from expternal crate proxmox-apt-api-types. Signed-off-by: Dietmar Maurer --- proxmox-apt/src/repositories/file.rs | 68 ++++++++++++------- .../src/repositories/file/list_parser.rs | 4 +- .../src/repositories/file/sources_parser.rs | 1 + proxmox-apt/src/repositories/mod.rs | 3 + proxmox-apt/src/repositories/repository.rs | 60 ++++++++++------ proxmox-apt/src/repositories/standard.rs | 38 +++++++---- proxmox-apt/tests/repositories.rs | 3 + 7 files changed, 118 insertions(+), 59 deletions(-) diff --git a/proxmox-apt/src/repositories/file.rs b/proxmox-apt/src/repositories/file.rs index 0d21ce3c..086abf49 100644 --- a/proxmox-apt/src/repositories/file.rs +++ b/proxmox-apt/src/repositories/file.rs @@ -9,6 +9,8 @@ use crate::repositories::repository::{ APTRepository, APTRepositoryFileType, APTRepositoryPackageType, }; +use crate::repositories::repository::APTRepositoryImpl; + use proxmox_schema::api; mod list_parser; @@ -116,13 +118,46 @@ pub struct APTRepositoryInfo { pub message: String, } -impl APTRepositoryFile { +pub trait APTRepositoryFileImpl { /// Creates a new `APTRepositoryFile` without parsing. /// /// If the file is hidden, the path points to a directory, or the extension /// is usually ignored by APT (e.g. `.orig`), `Ok(None)` is returned, while /// invalid file names yield an error. - pub fn new>(path: P) -> Result, APTRepositoryFileError> { + fn new>(path: P) -> Result, APTRepositoryFileError>; + + fn with_content(content: String, content_type: APTRepositoryFileType) -> Self; + + /// Check if the file exists. + fn exists(&self) -> bool; + + fn read_with_digest(&self) -> Result<(Vec, [u8; 32]), APTRepositoryFileError>; + + /// Create an `APTRepositoryFileError`. + fn err(&self, error: Error) -> APTRepositoryFileError; + + /// Parses the APT repositories configured in the file on disk, including + /// disabled ones. + /// + /// Resets the current repositories and digest, even on failure. + fn parse(&mut self) -> Result<(), APTRepositoryFileError>; + + /// Writes the repositories to the file on disk. + /// + /// If a digest is provided, checks that the current content of the file still + /// produces the same one. + fn write(&self) -> Result<(), APTRepositoryFileError>; + + /// Checks if old or unstable suites are configured and that the Debian security repository + /// has the correct suite. Also checks that the `stable` keyword is not used. + fn check_suites(&self, current_codename: DebianCodename) -> Vec; + + /// Checks for official URIs. + fn check_uris(&self) -> Vec; +} + +impl APTRepositoryFileImpl for APTRepositoryFile { + fn new>(path: P) -> Result, APTRepositoryFileError> { let path: PathBuf = path.as_ref().to_path_buf(); let new_err = |path_string: String, err: &str| APTRepositoryFileError { @@ -197,7 +232,7 @@ impl APTRepositoryFile { })) } - pub fn with_content(content: String, content_type: APTRepositoryFileType) -> Self { + fn with_content(content: String, content_type: APTRepositoryFileType) -> Self { Self { file_type: content_type, content: Some(content), @@ -207,8 +242,7 @@ impl APTRepositoryFile { } } - /// Check if the file exists. - pub fn exists(&self) -> bool { + fn exists(&self) -> bool { if let Some(path) = &self.path { PathBuf::from(path).exists() } else { @@ -216,7 +250,7 @@ impl APTRepositoryFile { } } - pub fn read_with_digest(&self) -> Result<(Vec, [u8; 32]), APTRepositoryFileError> { + fn read_with_digest(&self) -> Result<(Vec, [u8; 32]), APTRepositoryFileError> { if let Some(path) = &self.path { let content = std::fs::read(path).map_err(|err| self.err(format_err!("{}", err)))?; let digest = openssl::sha::sha256(&content); @@ -233,19 +267,14 @@ impl APTRepositoryFile { } } - /// Create an `APTRepositoryFileError`. - pub fn err(&self, error: Error) -> APTRepositoryFileError { + fn err(&self, error: Error) -> APTRepositoryFileError { APTRepositoryFileError { path: self.path.clone().unwrap_or_default(), error: error.to_string(), } } - /// Parses the APT repositories configured in the file on disk, including - /// disabled ones. - /// - /// Resets the current repositories and digest, even on failure. - pub fn parse(&mut self) -> Result<(), APTRepositoryFileError> { + fn parse(&mut self) -> Result<(), APTRepositoryFileError> { self.repositories.clear(); self.digest = None; @@ -269,11 +298,7 @@ impl APTRepositoryFile { Ok(()) } - /// Writes the repositories to the file on disk. - /// - /// If a digest is provided, checks that the current content of the file still - /// produces the same one. - pub fn write(&self) -> Result<(), APTRepositoryFileError> { + fn write(&self) -> Result<(), APTRepositoryFileError> { let path = match &self.path { Some(path) => path, None => { @@ -336,9 +361,7 @@ impl APTRepositoryFile { Ok(()) } - /// Checks if old or unstable suites are configured and that the Debian security repository - /// has the correct suite. Also checks that the `stable` keyword is not used. - pub fn check_suites(&self, current_codename: DebianCodename) -> Vec { + fn check_suites(&self, current_codename: DebianCodename) -> Vec { let mut infos = vec![]; let path = match &self.path { @@ -427,8 +450,7 @@ impl APTRepositoryFile { infos } - /// Checks for official URIs. - pub fn check_uris(&self) -> Vec { + fn check_uris(&self) -> Vec { let mut infos = vec![]; let path = match &self.path { diff --git a/proxmox-apt/src/repositories/file/list_parser.rs b/proxmox-apt/src/repositories/file/list_parser.rs index 0edeea7f..93bbcc12 100644 --- a/proxmox-apt/src/repositories/file/list_parser.rs +++ b/proxmox-apt/src/repositories/file/list_parser.rs @@ -3,9 +3,9 @@ use std::iter::Iterator; use anyhow::{bail, format_err, Error}; -use crate::repositories::{APTRepository, APTRepositoryFileType, APTRepositoryOption}; - use super::APTRepositoryParser; +use crate::repositories::APTRepositoryImpl; +use crate::repositories::{APTRepository, APTRepositoryFileType, APTRepositoryOption}; // TODO convert %-escape characters. Also adapt printing back accordingly, // because at least '%' needs to be re-escaped when printing. diff --git a/proxmox-apt/src/repositories/file/sources_parser.rs b/proxmox-apt/src/repositories/file/sources_parser.rs index 9424bbe2..213db9d6 100644 --- a/proxmox-apt/src/repositories/file/sources_parser.rs +++ b/proxmox-apt/src/repositories/file/sources_parser.rs @@ -3,6 +3,7 @@ use std::iter::Iterator; use anyhow::{bail, Error}; +use crate::repositories::APTRepositoryImpl; use crate::repositories::{ APTRepository, APTRepositoryFileType, APTRepositoryOption, APTRepositoryPackageType, }; diff --git a/proxmox-apt/src/repositories/mod.rs b/proxmox-apt/src/repositories/mod.rs index 26e4192c..014f1820 100644 --- a/proxmox-apt/src/repositories/mod.rs +++ b/proxmox-apt/src/repositories/mod.rs @@ -4,17 +4,20 @@ use std::path::PathBuf; use anyhow::{bail, Error}; mod repository; +pub use repository::APTRepositoryImpl; pub use repository::{ APTRepository, APTRepositoryFileType, APTRepositoryOption, APTRepositoryPackageType, }; mod file; +pub use file::APTRepositoryFileImpl; pub use file::{APTRepositoryFile, APTRepositoryFileError, APTRepositoryInfo}; mod release; pub use release::{get_current_release_codename, DebianCodename}; mod standard; +pub use standard::APTRepositoryHandleImpl; pub use standard::{APTRepositoryHandle, APTStandardRepository}; const APT_SOURCES_LIST_FILENAME: &str = "/etc/apt/sources.list"; diff --git a/proxmox-apt/src/repositories/repository.rs b/proxmox-apt/src/repositories/repository.rs index 91bd64f2..a07db7cb 100644 --- a/proxmox-apt/src/repositories/repository.rs +++ b/proxmox-apt/src/repositories/repository.rs @@ -8,6 +8,7 @@ use serde::{Deserialize, Serialize}; use proxmox_schema::api; use crate::repositories::standard::APTRepositoryHandle; +use crate::repositories::standard::APTRepositoryHandleImpl; #[api] #[derive(Debug, Copy, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -188,9 +189,41 @@ pub struct APTRepository { pub enabled: bool, } -impl APTRepository { +pub trait APTRepositoryImpl { /// Crates an empty repository. - pub fn new(file_type: APTRepositoryFileType) -> Self { + fn new(file_type: APTRepositoryFileType) -> Self; + + /// Changes the `enabled` flag and makes sure the `Enabled` option for + /// `APTRepositoryPackageType::Sources` repositories is updated too. + fn set_enabled(&mut self, enabled: bool); + + /// Makes sure that all basic properties of a repository are present and not obviously invalid. + fn basic_check(&self) -> Result<(), Error>; + + /// Checks if the repository is the one referenced by the handle. + fn is_referenced_repository( + &self, + handle: APTRepositoryHandle, + product: &str, + suite: &str, + ) -> bool; + + /// Guess the origin from the repository's URIs. + /// + /// Intended to be used as a fallback for get_cached_origin. + fn origin_from_uris(&self) -> Option; + + /// Get the `Origin:` value from a cached InRelease file. + fn get_cached_origin(&self) -> Result, Error>; + + /// Writes a repository in the corresponding format followed by a blank. + /// + /// Expects that `basic_check()` for the repository was successful. + fn write(&self, w: &mut dyn Write) -> Result<(), Error>; +} + +impl APTRepositoryImpl for APTRepository { + fn new(file_type: APTRepositoryFileType) -> Self { Self { types: vec![], uris: vec![], @@ -203,9 +236,7 @@ impl APTRepository { } } - /// Changes the `enabled` flag and makes sure the `Enabled` option for - /// `APTRepositoryPackageType::Sources` repositories is updated too. - pub fn set_enabled(&mut self, enabled: bool) { + fn set_enabled(&mut self, enabled: bool) { self.enabled = enabled; if self.file_type == APTRepositoryFileType::Sources { @@ -226,8 +257,7 @@ impl APTRepository { } } - /// Makes sure that all basic properties of a repository are present and not obviously invalid. - pub fn basic_check(&self) -> Result<(), Error> { + fn basic_check(&self) -> Result<(), Error> { if self.types.is_empty() { bail!("missing package type(s)"); } @@ -267,8 +297,7 @@ impl APTRepository { Ok(()) } - /// Checks if the repository is the one referenced by the handle. - pub fn is_referenced_repository( + fn is_referenced_repository( &self, handle: APTRepositoryHandle, product: &str, @@ -299,10 +328,7 @@ impl APTRepository { && found_component } - /// Guess the origin from the repository's URIs. - /// - /// Intended to be used as a fallback for get_cached_origin. - pub fn origin_from_uris(&self) -> Option { + fn origin_from_uris(&self) -> Option { for uri in self.uris.iter() { if let Some(host) = host_from_uri(uri) { if host == "proxmox.com" || host.ends_with(".proxmox.com") { @@ -318,8 +344,7 @@ impl APTRepository { None } - /// Get the `Origin:` value from a cached InRelease file. - pub fn get_cached_origin(&self) -> Result, Error> { + fn get_cached_origin(&self) -> Result, Error> { for uri in self.uris.iter() { for suite in self.suites.iter() { let mut file = release_filename(uri, suite, false); @@ -353,10 +378,7 @@ impl APTRepository { Ok(None) } - /// Writes a repository in the corresponding format followed by a blank. - /// - /// Expects that `basic_check()` for the repository was successful. - pub fn write(&self, w: &mut dyn Write) -> Result<(), Error> { + fn write(&self, w: &mut dyn Write) -> Result<(), Error> { match self.file_type { APTRepositoryFileType::List => write_one_line(self, w), APTRepositoryFileType::Sources => write_stanza(self, w), diff --git a/proxmox-apt/src/repositories/standard.rs b/proxmox-apt/src/repositories/standard.rs index 29cc7885..7858fac4 100644 --- a/proxmox-apt/src/repositories/standard.rs +++ b/proxmox-apt/src/repositories/standard.rs @@ -111,9 +111,26 @@ impl Display for APTRepositoryHandle { } } -impl APTRepositoryHandle { +pub trait APTRepositoryHandleImpl { /// Get the description for the repository. - pub fn description(self) -> String { + fn description(self) -> String; + /// Get the display name of the repository. + fn name(self) -> String; + /// Get the standard file path for the repository referenced by the handle. + fn path(self, product: &str) -> String; + /// Get package type, possible URIs and the component associated with the handle. + /// + /// The first URI is the preferred one. + fn info(self, product: &str) -> (APTRepositoryPackageType, Vec, String); + /// Get the standard repository referenced by the handle. + /// + /// An URI in the result is not '/'-terminated (under the assumption that no valid + /// product name is). + fn to_repository(self, product: &str, suite: &str) -> APTRepository; +} + +impl APTRepositoryHandleImpl for APTRepositoryHandle { + fn description(self) -> String { match self { APTRepositoryHandle::Enterprise => { "This is the default, stable, and recommended repository, available for all \ @@ -155,8 +172,7 @@ impl APTRepositoryHandle { .to_string() } - /// Get the display name of the repository. - pub fn name(self) -> String { + fn name(self) -> String { match self { APTRepositoryHandle::Enterprise => "Enterprise", APTRepositoryHandle::NoSubscription => "No-Subscription", @@ -171,8 +187,7 @@ impl APTRepositoryHandle { .to_string() } - /// Get the standard file path for the repository referenced by the handle. - pub fn path(self, product: &str) -> String { + fn path(self, product: &str) -> String { match self { APTRepositoryHandle::Enterprise => { format!("/etc/apt/sources.list.d/{}-enterprise.list", product) @@ -188,10 +203,7 @@ impl APTRepositoryHandle { } } - /// Get package type, possible URIs and the component associated with the handle. - /// - /// The first URI is the preferred one. - pub fn info(self, product: &str) -> (APTRepositoryPackageType, Vec, String) { + fn info(self, product: &str) -> (APTRepositoryPackageType, Vec, String) { match self { APTRepositoryHandle::Enterprise => ( APTRepositoryPackageType::Deb, @@ -259,11 +271,7 @@ impl APTRepositoryHandle { } } - /// Get the standard repository referenced by the handle. - /// - /// An URI in the result is not '/'-terminated (under the assumption that no valid - /// product name is). - pub fn to_repository(self, product: &str, suite: &str) -> APTRepository { + fn to_repository(self, product: &str, suite: &str) -> APTRepository { let (package_type, uris, component) = self.info(product); APTRepository { diff --git a/proxmox-apt/tests/repositories.rs b/proxmox-apt/tests/repositories.rs index ae92e51c..37d665bf 100644 --- a/proxmox-apt/tests/repositories.rs +++ b/proxmox-apt/tests/repositories.rs @@ -8,6 +8,9 @@ use proxmox_apt::repositories::{ check_repositories, get_current_release_codename, standard_repositories, APTRepositoryFile, APTRepositoryHandle, APTRepositoryInfo, APTStandardRepository, DebianCodename, }; +use proxmox_apt::repositories::{ + APTRepositoryFileImpl, APTRepositoryHandleImpl, APTRepositoryImpl, +}; fn create_clean_directory(path: &PathBuf) -> Result<(), Error> { match std::fs::remove_dir_all(path) { -- 2.39.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel