Remove https requirement to use external_domains

Each new task corresponds to syncing a particular kind of thing for each
domain, e.g. origins, https certs, etc...

config.yml

@@ -82,8 +82,6 @@ domain:
   # External domains will have a TLS key/cert generated and signed for them, but
   # which will not be served by domani itself. The key/cert files will be placed
   # in the configured paths.
-  #
-  # HTTPS must be enabled for external_domains to be used.
   #external_domains:
     #example.com
     #  tls_key_path: /dir/path/key.pem

src/domain/manager.rs

@@ -3,7 +3,6 @@ use crate::error::unexpected::{self, Mappable};
 use crate::{origin, task_stack, util};
 
 use std::sync;
-use tokio_util::sync::CancellationToken;
 
 pub enum GetSettingsResult {
     Stored(domain::Settings),
@@ -183,13 +182,57 @@ impl ManagerImpl {
             config,
         });
 
-        task_stack.push_spawn(|canceller| {
-            let manager = manager.clone();
-            async move {
-                manager.sync_all_domains_job(canceller).await;
-                Ok(())
+        const SYNC_PERIOD_SECS: u64 = 20 * 60; // 20 minutes
+
+        task_stack.push_spawn_periodically(
+            manager.clone(),
+            SYNC_PERIOD_SECS,
+            |_canceller, manager| async move {
+                manager
+                    .sync_origins()
+                    .await
+                    .or_unexpected_while("syncing origins")
+            },
+        );
+
+        if manager.can_sync_gemini_cert() {
+            task_stack.push_spawn_periodically(
+                manager.clone(),
+                SYNC_PERIOD_SECS,
+                |_canceller, manager| async move {
+                    manager
+                        .sync_gemini_certs()
+                        .await
+                        .or_unexpected_while("syncing gemini certs")
+                },
+            );
+        }
+
+        if manager.can_sync_https_cert() {
+            task_stack.push_spawn_periodically(
+                manager.clone(),
+                SYNC_PERIOD_SECS,
+                |_canceller, manager| async move {
+                    manager
+                        .sync_https_certs()
+                        .await
+                        .or_unexpected_while("syncing https certs")
+                },
+            );
+        }
+
+        if manager.can_sync_external_cert() {
+            task_stack.push_spawn_periodically(
+                manager.clone(),
+                SYNC_PERIOD_SECS,
+                |_canceller, manager| async move {
+                    manager
+                        .sync_external_certs()
+                        .await
+                        .or_unexpected_while("syncing external certs")
+                },
+            );
         }
-        });
 
         manager
     }
@@ -203,12 +246,45 @@ impl ManagerImpl {
         self.origin_store.sync(origin_descr)
     }
 
+    async fn sync_origins(&self) -> unexpected::Result<()> {
+        let domains = self
+            .all_domains()
+            .or_unexpected_while("fetching all domains")?
+            .into_iter();
+
+        for ManagedDomain { domain, .. } in domains {
+            let settings = match self
+                .get_settings(&domain)
+                .map_unexpected_while(|| format!("fetching settings for {domain}"))?
+            {
+                GetSettingsResult::Stored(settings) => settings,
+                GetSettingsResult::Builtin(config) => config.settings,
+                _ => continue,
+            };
+
+            self.sync_domain_origin(&domain, &settings.origin_descr)
+                .map_unexpected_while(|| {
+                    format!(
+                        "syncing origin {:?} for domain {domain}",
+                        &settings.origin_descr,
+                    )
+                })?;
+        }
+
+        Ok(())
+    }
+
+    fn can_sync_gemini_cert(&self) -> bool {
+        self.gemini_store.is_some()
+    }
+
     fn sync_domain_gemini_cert(&self, domain: &domain::Name) -> unexpected::Result<()> {
-        if let Some(ref gemini_store) = self.gemini_store {
+        let gemini_store = self.gemini_store.as_ref().unwrap();
+
         log::info!("Syncing gemini certificate for domain {domain}");
         if gemini_store
             .get_certificate(domain)
-                .or_unexpected()?
+            .or_unexpected_while("checking if cert is already stored")?
             .is_some()
         {
             return Ok(());
@@ -219,100 +295,111 @@ impl ManagerImpl {
         let cert = tls::Certificate::new_self_signed(&pkey, domain)
             .or_unexpected_while("creating self-signed cert")?;
 
-            gemini_store.set_certificate(domain, pkey, cert)?;
+        gemini_store.set_certificate(domain, pkey, cert)
     }
+
+    async fn sync_gemini_certs(&self) -> unexpected::Result<()> {
+        let domains = self
+            .all_domains()
+            .or_unexpected_while("fetching all domains")?
+            .into_iter();
+
+        for ManagedDomain { domain, .. } in domains {
+            match self
+                .get_settings(&domain)
+                .map_unexpected_while(|| format!("fetching settings for {domain}"))?
+            {
+                GetSettingsResult::Stored(_) => (),
+                GetSettingsResult::Builtin(_) => (),
+                _ => continue,
+            };
+
+            self.sync_domain_gemini_cert(&domain)
+                .map_unexpected_while(|| format!("syncing domain {domain}"))?;
+        }
+
         Ok(())
     }
 
+    fn can_sync_https_cert(&self) -> bool {
+        self.acme_manager.is_some()
+    }
+
     async fn sync_domain_https_cert(&self, domain: &domain::Name) -> unexpected::Result<()> {
-        if let Some(ref acme_manager) = self.acme_manager {
         log::info!("Syncing HTTPS certificate for domain {domain}");
-            acme_manager.sync_domain(domain.clone(), None).await?;
+        self.acme_manager
+            .as_ref()
+            .unwrap()
+            .sync_domain(domain.clone(), None)
+            .await
+    }
+
+    async fn sync_https_certs(&self) -> unexpected::Result<()> {
+        let domains = self
+            .all_domains()
+            .or_unexpected_while("fetching all domains")?
+            .into_iter();
+
+        for ManagedDomain { domain, .. } in domains {
+            match self
+                .get_settings(&domain)
+                .map_unexpected_while(|| format!("fetching settings for {domain}"))?
+            {
+                GetSettingsResult::Stored(_) => (),
+                GetSettingsResult::Builtin(_) => (),
+                GetSettingsResult::Proxied(config) => {
+                    if config.https_disabled {
+                        continue;
+                    }
+                }
+                GetSettingsResult::Interface => (),
+                _ => continue,
+            };
+
+            self.sync_domain_https_cert(&domain)
+                .await
+                .map_unexpected_while(|| format!("syncing domain {domain}",))?;
         }
 
         Ok(())
     }
 
+    fn can_sync_external_cert(&self) -> bool {
+        self.acme_manager.is_some()
+    }
+
     async fn sync_external_domain(
         &self,
         domain: &domain::Name,
         config: &domain::ConfigExternalDomain,
     ) -> unexpected::Result<()> {
-        if let Some(ref acme_manager) = self.acme_manager {
         log::info!("Syncing HTTPS certificate for external domain {domain}");
-            acme_manager
+        self.acme_manager
+            .as_ref()
+            .unwrap()
             .sync_domain(domain.clone(), Some(config.clone()))
-                .await?;
+            .await
     }
 
-        Ok(())
-    }
-
-    async fn sync_all_domains(&self) -> unexpected::Result<()> {
+    async fn sync_external_certs(&self) -> unexpected::Result<()> {
         let domains = self
             .all_domains()
             .or_unexpected_while("fetching all domains")?
             .into_iter();
 
         for ManagedDomain { domain, .. } in domains {
-            let (settings, https_cert, gemini_cert) = match self
+            if let GetSettingsResult::External(config) = self
                 .get_settings(&domain)
                 .map_unexpected_while(|| format!("fetching settings for {domain}"))?
             {
-                GetSettingsResult::Stored(settings) => (Some(settings), true, true),
-                GetSettingsResult::Builtin(config) => (Some(config.settings), true, true),
-
-                // A proxied domain never needs gemini certs, since gemini requests will be
-                // transparently proxied to the backing server anyway.
-                GetSettingsResult::Proxied(config) => (None, !config.https_disabled, false),
-
-                GetSettingsResult::Interface => (None, true, false),
-
-                // External domains do their own thing, separate from the rest of this flow.
-                GetSettingsResult::External(config) => {
                 self.sync_external_domain(&domain, &config)
                     .await
-                        .map_unexpected_while(|| format!("syncing external domain {domain}"))?;
-                    continue;
-                }
-            };
-
-            if let Some(settings) = settings {
-                self.sync_domain_origin(&domain, &settings.origin_descr)
-                    .map_unexpected_while(|| {
-                        format!(
-                            "syncing origin {:?} for domain {domain}",
-                            &settings.origin_descr,
-                        )
-                    })?;
-            }
-
-            if gemini_cert {
-                self.sync_domain_gemini_cert(&domain)
-                    .map_unexpected_while(|| format!("syncing gemini cert for domain {domain}"))?;
-            }
-
-            if https_cert {
-                self.sync_domain_https_cert(&domain)
-                    .await
-                    .map_unexpected_while(|| format!("syncing https cert for domain {domain}",))?;
+                    .map_unexpected_while(|| format!("syncing external {domain}"))?;
             }
         }
 
         Ok(())
     }
-
-    async fn sync_all_domains_job(&self, canceller: CancellationToken) {
-        let mut interval = tokio::time::interval(tokio::time::Duration::from_secs(20 * 60));
-        loop {
-            tokio::select! {
-                _ = canceller.cancelled() => return,
-                _ = interval.tick() => if let Err(err) = self.sync_all_domains().await {
-                    log::error!("Failed to sync all domains: {err}")
-                },
-            }
-        }
-    }
 }
 
 impl Manager for ManagerImpl {
@@ -391,12 +478,16 @@ impl Manager for ManagerImpl {
 
             self.sync_domain_origin(&domain, &settings.origin_descr)?;
 
+            if self.can_sync_gemini_cert() {
                 self.sync_domain_gemini_cert(&domain)
                     .or_unexpected_while("syncing domain gemini cert")?;
+            }
 
+            if self.can_sync_https_cert() {
                 self.sync_domain_https_cert(&domain)
                     .await
                     .or_unexpected_while("syncing domain https cert")?;
+            }
 
             self.domain_store.set(&domain, &settings)?;
 

src/main.rs

@@ -74,10 +74,6 @@ async fn main() {
             }
         }
 
-        if !config.domain.external_domains.is_empty() && config.service.http.https_addr.is_none() {
-            panic!("https must be enabled to use external_domains")
-        }
-
         config
     };
 
@@ -87,7 +83,9 @@ async fn main() {
         return;
     };
 
+    let https_enabled = config.service.http.https_addr.is_some();
     let gemini_enabled = config.service.gemini.gemini_addr.is_some();
+    let external_domains_enabled = !config.domain.external_domains.is_empty();
 
     let origin_store = domani::origin::git::FSStore::new(&config.origin)
         .expect("git origin store initialization failed");
@@ -103,7 +101,7 @@ async fn main() {
         domani::domain::store::FSStore::new(&config.domain.store_dir_path.join("domains"))
             .expect("domain config store initialization failed");
 
-    let domain_acme_manager = if config.service.http.https_addr.is_some() {
+    let domain_acme_manager = if https_enabled || external_domains_enabled {
         let acme_config = config
             .domain
             .acme

src/service/http.rs

@@ -66,8 +66,6 @@ impl Service {
     where
         CertResolver: rustls::server::ResolvesServerCert + 'static,
     {
-        let https_enabled = config.http.https_addr.is_some();
-
         let service = sync::Arc::new(Service {
             domain_manager: domain_manager.clone(),
             cert_resolver: sync::Arc::from(cert_resolver),
@@ -79,13 +77,17 @@ impl Service {
 
         task_stack.push_spawn(|canceller| tasks::listen_http(service.clone(), canceller));
 
-        if https_enabled {
+        if service.https_enabled() {
             task_stack.push_spawn(|canceller| tasks::listen_https(service.clone(), canceller));
         }
 
         service
     }
 
+    fn https_enabled(&self) -> bool {
+        self.config.http.https_addr.is_some()
+    }
+
     fn serve(&self, status_code: u16, path: &str, body: Body) -> Response<Body> {
         match Response::builder()
             .status(status_code)
@@ -125,7 +127,7 @@ impl Service {
     }
 
     fn presenter_http_scheme(&self) -> &str {
-        if self.config.http.https_addr.is_some() {
+        if self.https_enabled() {
             return "https";
         }
         "http"
@@ -542,7 +544,7 @@ impl Service {
         // - /.well-known urls
         // - proxied domains with https_disabled set on them
         // everything else must use https if possible.
-        let https_upgradable = self.config.http.https_addr.is_some() && !req_is_https;
+        let https_upgradable = self.https_enabled() && !req_is_https;
 
         if let Some(config) = self.proxied_domains.get(&domain) {
             if config.http_url.is_none() {

src/task_stack.rs

@@ -42,6 +42,34 @@ where
         });
     }
 
+    pub fn push_spawn_periodically<State, F, Fut>(&mut self, state: State, period_secs: u64, f: F)
+    where
+        State: Clone + Send + Sync + 'static,
+        Fut: futures::Future<Output = Result<(), E>> + Send + 'static,
+        F: Fn(CancellationToken, State) -> Fut + Send + Sync + 'static,
+    {
+        let canceller = CancellationToken::new();
+        let handle = {
+            let canceller = canceller.clone();
+            tokio::spawn(async move {
+                let mut interval =
+                    tokio::time::interval(tokio::time::Duration::from_secs(period_secs));
+                loop {
+                    tokio::select! {
+                        _ = canceller.cancelled() => return Ok(()),
+                        _ = interval.tick() => if let Err(err) = f(canceller.clone(), state.clone()).await {
+                            log::error!("Failed to sync all domains: {err}")
+                        },
+                    }
+                }
+            })
+        };
+        self.push(async move {
+            canceller.cancel();
+            handle.await.expect("failed to join task")
+        });
+    }
+
     /// stop will process all operations which have been pushed onto the stack in the reverse order
     /// they were pushed.
     pub async fn stop(mut self) -> Result<(), E> {