From e67cd6725a09d9b08675ed5056ae828c9b92beab Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Thu, 3 Aug 2023 10:29:57 +0200 Subject: [PATCH] Return all domains from all_domains This fixes an issue where the domain manager sync job wouldn't have synced private builtin domains ever. --- src/domain/manager.rs | 12 ++++---- src/domain/store.rs | 69 ++++++++++++++++++++++++++++++------------- src/service/http.rs | 53 +++++++++++++++++++++++---------- 3 files changed, 93 insertions(+), 41 deletions(-) diff --git a/src/domain/manager.rs b/src/domain/manager.rs index 9b2fa0c..9bc8365 100644 --- a/src/domain/manager.rs +++ b/src/domain/manager.rs @@ -139,7 +139,9 @@ impl From for SyncWithSettingsError { pub type GetAcmeHttp01ChallengeKeyError = acme::manager::GetHttp01ChallengeKeyError; -//#[mockall::automock] +pub type StoredDomain = domain::store::StoredDomain; + +#[mockall::automock] pub trait Manager: Sync + Send { fn get_settings(&self, domain: &domain::Name) -> Result; @@ -165,7 +167,7 @@ pub trait Manager: Sync + Send { domain: &domain::Name, ) -> unexpected::Result>; - fn all_domains(&self) -> Result, unexpected::Error>; + fn all_domains(&self) -> Result, unexpected::Error>; } pub struct ManagerImpl { @@ -247,13 +249,13 @@ impl ManagerImpl { } .into_iter(); - for domain in domains { + for StoredDomain { domain, .. } in domains { log::info!("Syncing domain {}", &domain); let get_res = match self.domain_store.get(&domain) { Ok(get_res) => get_res, Err(err) => { - log::error!("Error syncing {domain}: {err}"); + log::error!("Failed to fetch settings for domain {domain}: {err}"); return; } }; @@ -334,7 +336,7 @@ impl Manager for ManagerImpl { self.domain_checker.get_challenge_token(domain) } - fn all_domains(&self) -> Result, unexpected::Error> { + fn all_domains(&self) -> Result, unexpected::Error> { self.domain_store.all_domains() } } diff --git a/src/domain/store.rs b/src/domain/store.rs index 016f87f..3bd8bfa 100644 --- a/src/domain/store.rs +++ b/src/domain/store.rs @@ -1,15 +1,21 @@ -use std::{collections, fs, io, path, str::FromStr}; +use std::{collections, fs, io, path}; use crate::domain; use crate::error::unexpected::{self, Intoable, Mappable}; #[derive(Debug, PartialEq)] -pub struct GetResult { - pub settings: domain::Settings, +/// Extra information about a domain which is related to how its stored. +pub struct Metadata { pub builtin: bool, pub public: bool, } +#[derive(Debug, PartialEq)] +pub struct GetResult { + pub settings: domain::Settings, + pub metadata: Metadata, +} + #[derive(thiserror::Error, Debug)] pub enum GetError { #[error("not found")] @@ -28,11 +34,16 @@ pub enum SetError { Unexpected(#[from] unexpected::Error), } +pub struct StoredDomain { + pub domain: domain::Name, + pub metadata: Metadata, +} + #[mockall::automock] pub trait Store { fn get(&self, domain: &domain::Name) -> Result; fn set(&self, domain: &domain::Name, settings: &domain::Settings) -> Result<(), SetError>; - fn all_domains(&self) -> Result, unexpected::Error>; + fn all_domains(&self) -> Result, unexpected::Error>; } pub struct FSStore { @@ -71,8 +82,10 @@ impl Store for FSStore { Ok(GetResult { settings, - public: true, - builtin: false, + metadata: Metadata { + public: true, + builtin: false, + }, }) } @@ -91,18 +104,23 @@ impl Store for FSStore { Ok(()) } - fn all_domains(&self) -> Result, unexpected::Error> { + fn all_domains(&self) -> Result, unexpected::Error> { fs::read_dir(&self.dir_path) .or_unexpected()? .map( - |dir_entry_res: io::Result| -> Result { + |dir_entry_res: io::Result| -> Result { let domain = dir_entry_res.or_unexpected()?.file_name(); let domain = domain.to_str().ok_or(unexpected::Error::from( "couldn't convert os string to &str", ))?; - domain::Name::from_str(domain) - .map_unexpected_while(|| format!("parsing {domain} as domain name")) + Ok(StoredDomain{ + domain: domain.parse().map_unexpected_while(|| format!("parsing {domain} as domain name"))?, + metadata: Metadata{ + public: true, + builtin: false, + }, + }) }, ) .try_collect() @@ -131,8 +149,10 @@ impl Store for StoreWithBuiltin { if let Some(domain) = self.domains.get(domain) { return Ok(GetResult { settings: domain.settings.clone(), - public: domain.public, - builtin: true, + metadata: Metadata { + public: domain.public, + builtin: true, + }, }); } self.inner.get(domain) @@ -145,13 +165,18 @@ impl Store for StoreWithBuiltin { self.inner.set(domain, settings) } - fn all_domains(&self) -> Result, unexpected::Error> { + fn all_domains(&self) -> Result, unexpected::Error> { let inner_domains = self.inner.all_domains()?; - let mut domains: Vec = self + let mut domains: Vec = self .domains .iter() - .filter(|(_, v)| v.public) - .map(|(k, _)| k.clone()) + .map(|(domain, v)| StoredDomain { + domain: domain.clone(), + metadata: Metadata { + public: v.public, + builtin: true, + }, + }) .collect(); domains.extend(inner_domains); Ok(domains) @@ -193,8 +218,10 @@ mod tests { assert_eq!( GetResult { settings, - public: true, - builtin: false, + metadata: Metadata { + public: true, + builtin: false, + }, }, store.get(&domain).expect("settings retrieved") ); @@ -211,8 +238,10 @@ mod tests { assert_eq!( GetResult { settings: new_settings, - public: true, - builtin: false, + metadata: Metadata { + public: true, + builtin: false, + }, }, store.get(&domain).expect("settings retrieved") ); diff --git a/src/service/http.rs b/src/service/http.rs index 25bd6a5..b61f774 100644 --- a/src/service/http.rs +++ b/src/service/http.rs @@ -210,6 +210,8 @@ impl Service { } fn domain(&self, args: DomainArgs) -> Response { + use domain::store::Metadata; + #[derive(Serialize)] struct Data { domain: domain::Name, @@ -217,24 +219,42 @@ impl Service { } let settings = match self.domain_manager.get_settings(&args.domain) { + Ok(domain::manager::GetSettingsResult { + metadata: + Metadata { + public: false, + builtin: _, + }, + .. + }) => None, + Ok(domain::manager::GetSettingsResult { settings, - public, - builtin, - }) => match (public, builtin) { - (false, _) => None, - (true, false) => Some(settings), - (true, true) => { - return self.render_error_page( - 403, - format!( - "Settings for domain {} cannot be viewed or modified", - args.domain - ) - .as_str(), + metadata: + Metadata { + public: true, + builtin: false, + }, + }) => Some(settings), + + Ok(domain::manager::GetSettingsResult { + metadata: + Metadata { + public: true, + builtin: true, + }, + .. + }) => { + return self.render_error_page( + 403, + format!( + "Settings for domain {} cannot be viewed or modified", + args.domain ) - } - }, + .as_str(), + ) + } + Err(domain::manager::GetSettingsError::NotFound) => None, Err(domain::manager::GetSettingsError::Unexpected(e)) => { return self.internal_error( @@ -367,7 +387,8 @@ impl Service { let mut domains: Vec = domains .into_iter() - .map(|domain| domain.as_str().to_string()) + .filter(|d| d.metadata.public) + .map(|d| d.domain.as_str().to_string()) .collect(); domains.sort();