fallback list: Introduce and use FallbackListBuilder

Now the network fallbacks configuration wants to Deserialize
a Vec<FallbackDirBuilder>, rather than validated Vec<FallbackDir>.

Methods on FallbackListBuilder are as per
  https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/462#note_2797697
mutatis mutandi for the fact that this struct has only fallbacks in it.

crates/arti-client/src/config.rs

@@ -526,13 +526,13 @@ mod test {
             .rsa_identity([23; 20].into())
             .ed_identity([99; 32].into())
             .orports(vec!["127.0.0.7:7".parse().unwrap()])
-            .build()
-            .unwrap();
+            .clone();
 
         let mut bld = TorClientConfig::builder();
         bld.tor_network()
             .authorities(vec![auth])
-            .fallback_caches(vec![fallback]);
+            .fallback_caches()
+            .set(vec![fallback]);
         bld.storage()
             .cache_dir(CfgPath::new("/var/tmp/foo".to_owned()))
             .state_dir(CfgPath::new("/var/tmp/bar".to_owned()));

crates/arti/src/cfg.rs

@@ -309,8 +309,7 @@ mod test {
             .rsa_identity([23; 20].into())
             .ed_identity([99; 32].into())
             .orports(vec!["127.0.0.7:7".parse().unwrap()])
-            .build()
-            .unwrap();
+            .clone();
 
         let mut bld = ArtiConfig::builder();
         bld.proxy().socks_port(Some(9999));
@@ -318,7 +317,8 @@ mod test {
         bld.tor()
             .tor_network()
             .authorities(vec![auth])
-            .fallback_caches(vec![fallback]);
+            .fallback_caches()
+            .set(vec![fallback]);
         bld.tor()
             .storage()
             .cache_dir(CfgPath::new("/var/tmp/foo".to_owned()))

crates/tor-dirmgr/src/config.rs

@@ -12,6 +12,7 @@ use crate::retry::DownloadSchedule;
 use crate::storage::DynStore;
 use crate::{Authority, Result};
 use tor_config::ConfigBuildError;
+use tor_guardmgr::fallback::FallbackListBuilder;
 use tor_netdoc::doc::netstatus;
 
 use derive_builder::Builder;
@@ -39,11 +40,11 @@ pub struct NetworkConfig {
     ///
     /// This section can be changed in a running Arti client.  Doing so will
     /// affect future download attempts only.
-    #[serde(default = "fallbacks::default_fallbacks")]
-    #[builder(default = "fallbacks::default_fallbacks()")]
+    #[builder(sub_builder)]
+    #[serde(default)]
     #[serde(rename = "fallback_caches")]
     #[builder_field_attr(serde(rename = "fallback_caches"))]
-    #[builder(setter(into, name = "fallback_caches"))]
+    #[builder(setter(name = "fallback_caches"))]
     pub(crate) fallbacks: tor_guardmgr::fallback::FallbackList,
 
     /// List of directory authorities which we expect to sign consensus
@@ -61,7 +62,9 @@ pub struct NetworkConfig {
 impl Default for NetworkConfig {
     fn default() -> Self {
         NetworkConfig {
-            fallbacks: fallbacks::default_fallbacks(),
+            fallbacks: FallbackListBuilder::default()
+                .build()
+                .expect("build default fallbacks"),
             authorities: crate::authority::default_authorities(),
         }
     }
@@ -82,7 +85,7 @@ impl NetworkConfig {
 impl NetworkConfigBuilder {
     /// Check that this builder will give a reasonable network.
     fn validate(&self) -> std::result::Result<(), ConfigBuildError> {
-        if self.authorities.is_some() && self.fallbacks.is_none() {
+        if self.authorities.is_some() && self.fallbacks.is_unmodified_default() {
             return Err(ConfigBuildError::Inconsistent {
                 fields: vec!["authorities".to_owned(), "fallbacks".to_owned()],
                 problem: "Non-default authorities are use, but the fallback list is not overridden"
@@ -310,11 +313,6 @@ impl DownloadScheduleConfig {
     }
 }
 
-/// Compatibility alias, will go away in a moment
-mod fallbacks {
-    pub(crate) use tor_guardmgr::fallback::default_fallbacks;
-}
-
 #[cfg(test)]
 mod test {
     #![allow(clippy::unwrap_used)]
@@ -367,13 +365,12 @@ mod test {
         ]);
         assert!(bld.build().is_err());
 
-        bld.fallback_caches(vec![FallbackDir::builder()
+        bld.fallback_caches().set(vec![FallbackDir::builder()
             .rsa_identity([b'x'; 20].into())
             .ed_identity([b'y'; 32].into())
             .orport("127.0.0.1:99".parse().unwrap())
             .orport("[::]:99".parse().unwrap())
-            .build()
-            .unwrap()]);
+            .clone()]);
         let cfg = bld.build().unwrap();
         assert_eq!(cfg.authorities.len(), 2);
         assert_eq!(cfg.fallbacks.len(), 1);

crates/tor-dirmgr/src/state.rs

@@ -1012,7 +1012,7 @@ mod test {
     impl DirRcv {
         fn new(now: SystemTime, authorities: Option<Vec<Authority>>) -> Self {
             let mut netcfg = crate::NetworkConfig::builder();
-            netcfg.fallback_caches(vec![]);
+            netcfg.fallback_caches().set(vec![]);
             if let Some(a) = authorities {
                 netcfg.authorities(a);
             }

crates/tor-guardmgr/src/fallback.rs

@@ -22,8 +22,8 @@ use serde::Deserialize;
 use std::net::SocketAddr;
 
 use crate::dirstatus::DirStatus;
-pub use set::FallbackList;
 pub(crate) use set::FallbackState;
+pub use set::{FallbackList, FallbackListBuilder};
 
 /// A directory whose location ships with Tor (or arti), and which we
 /// can use for bootstrapping when we don't know anything else about
@@ -90,11 +90,9 @@ impl FallbackDirBuilder {
 
 /// Return a list of the default fallback directories shipped with
 /// arti.
-//
-// TODO: this shouldn't be exposed in the public API; we will deal with that in a moment
-pub fn default_fallbacks() -> FallbackList {
+pub(crate) fn default_fallbacks() -> Vec<FallbackDirBuilder> {
     /// Build a fallback directory; panic if input is bad.
-    fn fallback(rsa: &str, ed: &str, ports: &[&str]) -> FallbackDir {
+    fn fallback(rsa: &str, ed: &str, ports: &[&str]) -> FallbackDirBuilder {
         let rsa = RsaIdentity::from_hex(rsa).expect("Bad hex in built-in fallback list");
         let ed = base64::decode_config(ed, base64::STANDARD_NO_PAD)
             .expect("Bad hex in built-in fallback list");
@@ -109,10 +107,9 @@ pub fn default_fallbacks() -> FallbackList {
                 bld.orport(p);
             });
 
-        bld.build()
-            .expect("Unable to build default fallback directory!?")
+        bld
     }
-    include!("fallback_dirs.inc").into()
+    include!("fallback_dirs.inc")
 }
 
 impl tor_linkspec::ChanTarget for FallbackDir {

crates/tor-guardmgr/src/fallback/set.rs

@@ -4,9 +4,11 @@ use crate::skew::SkewObservation;
 use rand::seq::IteratorRandom;
 use std::time::{Duration, Instant};
 
-use super::{DirStatus, FallbackDir};
+use super::{DirStatus, FallbackDir, FallbackDirBuilder};
+use crate::fallback::default_fallbacks;
 use crate::{ids::FallbackId, PickGuardError};
 use serde::Deserialize;
+use tor_config::ConfigBuildError;
 
 /// A list of fallback directories.
 ///
@@ -28,6 +30,56 @@ impl<T: IntoIterator<Item = FallbackDir>> From<T> for FallbackList {
     }
 }
 
+/// List of fallback directories, being built as part of the configuration
+///
+/// Fallback directories (represented by [`FallbackDir`]) are used by Tor
+/// clients when they don't already have enough other directory information to
+/// contact the network.
+///
+/// This is a builder pattern struct which will be resolved into a `FallbackList`
+/// during configuration resolution, via the [`build`](FallbackListBuilder::build) method.
+#[derive(Default, Clone, Deserialize)]
+#[serde(transparent)]
+pub struct FallbackListBuilder {
+    /// The fallbacks, as overridden
+    pub(crate) fallbacks: Option<Vec<FallbackDirBuilder>>,
+}
+
+impl FallbackListBuilder {
+    /// Append `fallback` to the list
+    pub fn append(&mut self, fallback: FallbackDirBuilder) {
+        self.fallbacks
+            .get_or_insert_with(default_fallbacks)
+            .push(fallback);
+    }
+
+    /// Replace the list with the supplied one
+    pub fn set(&mut self, fallbacks: impl IntoIterator<Item = FallbackDirBuilder>) {
+        self.fallbacks = Some(fallbacks.into_iter().collect());
+    }
+
+    /// Checks whether any calls have been made to set or adjust the set
+    pub fn is_unmodified_default(&self) -> bool {
+        self.fallbacks.is_none()
+    }
+
+    /// Resolve into a `FallbackList`
+    pub fn build(&self) -> Result<FallbackList, ConfigBuildError> {
+        let default_buffer;
+        let fallbacks = if let Some(f) = &self.fallbacks {
+            f
+        } else {
+            default_buffer = default_fallbacks();
+            &default_buffer
+        };
+        let fallbacks = fallbacks
+            .iter()
+            .map(|item| item.build())
+            .collect::<Result<_, _>>()?;
+        Ok(FallbackList { fallbacks })
+    }
+}
+
 impl FallbackList {
     /// Return the number of fallbacks in this list.
     pub fn len(&self) -> usize {

doc/semver_status.md

@@ -18,6 +18,10 @@ We can delete older sections here after we bump the releases.
 
 ## Since Arti 0.2.0
 
+### configuration (affecting arti, arti-client, tor-dirmgr, tor-guardmgr
+
+BREAKING: Configuration of fallback directories overhauled; now uses FalllbadkDirBuilder more.
+
 ### tor-basic-util
 
 MODIFIED: Added `reset()` method to RetrySchedule.