Commit a566f82d authored by eta's avatar eta
Browse files

Merge branch 'config-sub' into 'main'

Replace much handwritten config code with use of derive_builder

See merge request !462
parents 225accd6 89824fc7
Loading
Loading
Loading
Loading
+19 −21
Original line number Diff line number Diff line
@@ -828,9 +828,9 @@ dependencies = [

[[package]]
name = "darling"
version = "0.12.4"
version = "0.14.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5f2c43f534ea4b0b049015d00269734195e6d3f0f6635cb692251aca6f9f8b3c"
checksum = "f1a5d2e8b5a94b2261efb20e99a01255b9c5293797d69bbf04600567b2f9b8d7"
dependencies = [
 "darling_core",
 "darling_macro",
@@ -838,9 +838,9 @@ dependencies = [

[[package]]
name = "darling_core"
version = "0.12.4"
version = "0.14.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8e91455b86830a1c21799d94524df0845183fa55bafd9aa137b01c7d1065fa36"
checksum = "8f1c7d56716be82d9c6adb967cfe700955179ea88806e898483dad6987330a54"
dependencies = [
 "fnv",
 "ident_case",
@@ -852,9 +852,9 @@ dependencies = [

[[package]]
name = "darling_macro"
version = "0.12.4"
version = "0.14.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "29b5acf0dea37a7f66f7b25d2c5e93fd46f8f6968b1a5d7a3e02e97768afc95a"
checksum = "64dd7e5a75a00cb6799ae9fbbfc3bba0134def6579a9e27564e72c839c837bed"
dependencies = [
 "darling_core",
 "quote",
@@ -879,18 +879,16 @@ dependencies = [

[[package]]
name = "derive_builder"
version = "0.11.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "11d918e7dabe374a51dae0f29d818fece3b218b8b4eabec3bc4d42c537e7ed8f"
version = "0.11.2"
source = "git+https://github.com/ijackson/rust-derive-builder?rev=4516209ce98f6506bf729aa7cdbced97747f7dd1#4516209ce98f6506bf729aa7cdbced97747f7dd1"
dependencies = [
 "derive_builder_macro",
]

[[package]]
name = "derive_builder_core"
version = "0.11.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f712c2d4e52d5fcae53584e461dcb92fb2202e144ebf83ab0ba4360d18b767c7"
version = "0.11.2"
source = "git+https://github.com/ijackson/rust-derive-builder?rev=4516209ce98f6506bf729aa7cdbced97747f7dd1#4516209ce98f6506bf729aa7cdbced97747f7dd1"
dependencies = [
 "darling",
 "proc-macro2",
@@ -900,9 +898,8 @@ dependencies = [

[[package]]
name = "derive_builder_macro"
version = "0.11.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a8a2ac71b4a9a590dde6cee3ca4687aca5e7ce06f4ee297c5a959de5f1e42b2e"
version = "0.11.2"
source = "git+https://github.com/ijackson/rust-derive-builder?rev=4516209ce98f6506bf729aa7cdbced97747f7dd1#4516209ce98f6506bf729aa7cdbced97747f7dd1"
dependencies = [
 "derive_builder_core",
 "syn",
@@ -2342,9 +2339,9 @@ checksum = "dbf0c48bc1d91375ae5c3cd81e3722dff1abcf81a30960240640d223f59fe0e5"

[[package]]
name = "proc-macro2"
version = "1.0.36"
version = "1.0.37"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c7342d5883fbccae1cc37a2353b09c87c9b0f3afd73f5fb9bba687a1f733b029"
checksum = "ec757218438d5fda206afc041538b2f6d889286160d649a86a24d37e1235afd1"
dependencies = [
 "unicode-xid",
]
@@ -2366,9 +2363,9 @@ dependencies = [

[[package]]
name = "quote"
version = "1.0.17"
version = "1.0.18"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "632d02bff7f874a36f33ea8bb416cd484b90cc66c1194b1a1110d067a7013f58"
checksum = "a1feb54ed693b93a84e14094943b84b7c4eae204c512b7ccb95ab0c66d278ad1"
dependencies = [
 "proc-macro2",
]
@@ -2954,9 +2951,9 @@ checksum = "6bdef32e8150c2a081110b42772ffe7d7c9032b606bc226c8260fd97e0976601"

[[package]]
name = "syn"
version = "1.0.90"
version = "1.0.91"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "704df27628939572cd88d33f171cd6f896f4eaca85252c6e0a72d8d8287ee86f"
checksum = "b683b2b825c8eef438b77c36a06dc262294da3d5a5813fac20da149241dcd44d"
dependencies = [
 "proc-macro2",
 "quote",
@@ -3459,6 +3456,7 @@ dependencies = [
name = "tor-guardmgr"
version = "0.2.0"
dependencies = [
 "base64",
 "derive_builder",
 "derive_more",
 "educe",
+7 −3
Original line number Diff line number Diff line
@@ -62,6 +62,10 @@ codegen-units = 1
# 1.56.  It saves about 11% download size over the default value of '3'.]
opt-level = 's'

#[patch.crates-io.derive_builder_core]
#git = "https://github.com/ijackson/rust-derive-builder"
#rev = "dccbbb8ad75717c8bc0070b6f0364b2c3a54abb7"
[patch.crates-io.derive_builder]
git = "https://github.com/ijackson/rust-derive-builder"
rev = "4516209ce98f6506bf729aa7cdbced97747f7dd1"

[patch.crates-io.derive_builder_core]
git = "https://github.com/ijackson/rust-derive-builder"
rev = "4516209ce98f6506bf729aa7cdbced97747f7dd1"
+1 −1
Original line number Diff line number Diff line
@@ -43,7 +43,7 @@ tor-proto = { path = "../tor-proto", version = "0.2.0"}
tor-rtcompat = { path = "../tor-rtcompat", version = "0.2.0"}

humantime-serde = "1.1.1"
derive_builder = "0.11"
derive_builder = "0.11.2"
derive_more = "0.99"
directories = "4"
educe = "0.4.6"
+44 −174
Original line number Diff line number Diff line
@@ -239,40 +239,80 @@ impl StorageConfig {
/// to change. For more information see ticket [#285].
///
/// [#285]: https://gitlab.torproject.org/tpo/core/arti/-/issues/285
#[derive(Clone, Debug, Eq, PartialEq, AsRef)]
#[derive(Clone, Builder, Debug, Eq, PartialEq, AsRef)]
#[builder(build_fn(error = "ConfigBuildError"))]
#[builder(derive(Deserialize))]
pub struct TorClientConfig {
    /// Information about the Tor network we want to connect to.
    #[builder(sub_builder)]
    #[builder_field_attr(serde(default))]
    tor_network: dir::NetworkConfig,

    /// Directories for storing information on disk
    #[builder(sub_builder)]
    #[builder_field_attr(serde(default))]
    pub(crate) storage: StorageConfig,

    /// Information about when and how often to download directory information
    #[builder(sub_builder)]
    #[builder_field_attr(serde(default))]
    download_schedule: dir::DownloadScheduleConfig,

    /// Facility to override network parameters from the values set in the
    /// consensus.
    //
    // TODO: This field seems anomalous and should perhaps be changed somehow.
    // Maybe NetParams<i32> ought to derive Builder.
    #[builder(
        sub_builder,
        field(
            type = "HashMap<String, i32>",
            build = "convert_override_net_params(&self.override_net_params)"
        )
    )]
    #[builder_field_attr(serde(default))]
    override_net_params: tor_netdoc::doc::netstatus::NetParams<i32>,

    /// Information about how to build paths through the network.
    #[as_ref]
    #[builder(sub_builder)]
    #[builder_field_attr(serde(default))]
    path_rules: circ::PathConfig,

    /// Information about preemptive circuits.
    #[as_ref]
    #[builder(sub_builder)]
    #[builder_field_attr(serde(default))]
    preemptive_circuits: circ::PreemptiveCircuitConfig,

    /// Information about how to retry and expire circuits and request for circuits.
    #[as_ref]
    #[builder(sub_builder)]
    #[builder_field_attr(serde(default))]
    circuit_timing: circ::CircuitTiming,

    /// Rules about which addresses the client is willing to connect to.
    #[builder(sub_builder)]
    #[builder_field_attr(serde(default))]
    pub(crate) address_filter: ClientAddrConfig,

    /// Information about timing out client requests.
    #[builder(sub_builder)]
    #[builder_field_attr(serde(default))]
    pub(crate) stream_timeouts: StreamTimeoutConfig,
}

/// Helper to convert convert_override_net_params
fn convert_override_net_params(
    builder: &HashMap<String, i32>,
) -> tor_netdoc::doc::netstatus::NetParams<i32> {
    let mut override_net_params = tor_netdoc::doc::netstatus::NetParams::new();
    for (k, v) in builder {
        override_net_params.set(k.clone(), *v);
    }
    override_net_params
}

impl tor_circmgr::CircMgrConfig for TorClientConfig {}

impl AsRef<tor_guardmgr::fallback::FallbackList> for TorClientConfig {
@@ -311,92 +351,7 @@ impl TryInto<dir::DirMgrConfig> for &TorClientConfig {
    }
}

/// Builder object used to construct a [`TorClientConfig`].
///
/// Unlike other builder types in Arti, this builder works by exposing an
/// inner builder for each section in the [`TorClientConfig`].
#[derive(Clone, Default, Deserialize)]
pub struct TorClientConfigBuilder {
    /// Inner builder for the `tor_network` section.
    #[serde(default)]
    tor_network: dir::NetworkConfigBuilder,
    /// Inner builder for the `storage` section.
    #[serde(default)]
    storage: StorageConfigBuilder,
    /// Inner builder for the `download_schedule` section.
    #[serde(default)]
    download_schedule: dir::DownloadScheduleConfigBuilder,
    /// Inner builder for the `override_net_params` section.
    #[serde(default)]
    override_net_params: HashMap<String, i32>,
    /// Inner builder for the `path_rules` section.
    #[serde(default)]
    path_rules: circ::PathConfigBuilder,
    /// Inner builder for the `circuit_timing` section.
    #[serde(default)]
    preemptive_circuits: circ::PreemptiveCircuitConfigBuilder,
    /// Inner builder for the `circuit_timing` section.
    #[serde(default)]
    circuit_timing: circ::CircuitTimingBuilder,
    /// Inner builder for the `address_filter` section.
    #[serde(default)]
    address_filter: ClientAddrConfigBuilder,
    /// Inner builder for the `stream_timeouts` section.
    #[serde(default)]
    stream_timeouts: StreamTimeoutConfigBuilder,
}

impl TorClientConfigBuilder {
    /// Construct a [`TorClientConfig`] from this builder.
    pub fn build(&self) -> Result<TorClientConfig, ConfigBuildError> {
        let tor_network = self
            .tor_network
            .build()
            .map_err(|e| e.within("tor_network"))?;
        let storage = self.storage.build().map_err(|e| e.within("storage"))?;
        let download_schedule = self
            .download_schedule
            .build()
            .map_err(|e| e.within("download_schedule"))?;

        let mut override_net_params = tor_netdoc::doc::netstatus::NetParams::new();
        for (k, v) in &self.override_net_params {
            override_net_params.set(k.clone(), *v);
        }
        let path_rules = self
            .path_rules
            .build()
            .map_err(|e| e.within("path_rules"))?;
        let preemptive_circuits = self
            .preemptive_circuits
            .build()
            .map_err(|e| e.within("preemptive_circuits"))?;
        let circuit_timing = self
            .circuit_timing
            .build()
            .map_err(|e| e.within("circuit_timing"))?;
        let address_filter = self
            .address_filter
            .build()
            .map_err(|e| e.within("address_filter"))?;
        let stream_timeouts = self
            .stream_timeouts
            .build()
            .map_err(|e| e.within("stream_timeouts"))?;

        Ok(TorClientConfig {
            tor_network,
            storage,
            download_schedule,
            override_net_params,
            path_rules,
            preemptive_circuits,
            circuit_timing,
            address_filter,
            stream_timeouts,
        })
    }

    /// Returns a `TorClientConfigBuilder` using the specified state and cache directories.
    ///
    /// All other configuration options are set to their defaults.
@@ -412,91 +367,6 @@ impl TorClientConfigBuilder {
            .state_dir(CfgPath::from_path(state_dir));
        builder
    }

    /// Return a mutable reference to a
    /// [`NetworkConfigBuilder`](dir::NetworkConfigBuilder)
    /// to use in configuring the underlying Tor network.
    ///
    /// Most programs shouldn't need to alter this configuration: it's only for
    /// cases when you need to use a nonstandard set of Tor directory authorities
    /// and fallback caches.
    pub fn tor_network(&mut self) -> &mut dir::NetworkConfigBuilder {
        &mut self.tor_network
    }

    /// Return a mutable reference to a [`StorageConfigBuilder`].
    ///
    /// This section is used to configure the locations where Arti should
    /// store files on disk.
    pub fn storage(&mut self) -> &mut StorageConfigBuilder {
        &mut self.storage
    }

    /// Return a mutable reference to a
    /// [`DownloadScheduleConfigBuilder`](dir::DownloadScheduleConfigBuilder).
    ///
    /// This section is used to override Arti's schedule when attempting and
    /// retrying to download directory objects.
    pub fn download_schedule(&mut self) -> &mut dir::DownloadScheduleConfigBuilder {
        &mut self.download_schedule
    }

    /// Return a mutable reference to a [`HashMap`] of network parameters
    /// that should be used to override those specified in the consensus
    /// directory.
    ///
    /// This section should not usually be used for anything but testing:
    /// if you find yourself needing to configure an override here for
    /// production use, please consider opening a feature request for it
    /// instead.
    ///
    /// For a complete list of Tor's defined network parameters (not all of
    /// which are yet supported by Arti), see
    /// [`path-spec.txt`](https://gitlab.torproject.org/tpo/core/torspec/-/blob/main/param-spec.txt).
    pub fn override_net_params(&mut self) -> &mut HashMap<String, i32> {
        &mut self.override_net_params
    }

    /// Return a mutable reference to a [`PathConfigBuilder`](circ::PathConfigBuilder).
    ///
    /// This section is used to override Arti's rules for selecting which
    /// relays should be used in a given circuit.
    pub fn path_rules(&mut self) -> &mut circ::PathConfigBuilder {
        &mut self.path_rules
    }

    /// Return a mutable reference to a [`PreemptiveCircuitConfigBuilder`](circ::PreemptiveCircuitConfigBuilder).
    ///
    /// This section overrides Arti's rules for preemptive circuits.
    pub fn preemptive_circuits(&mut self) -> &mut circ::PreemptiveCircuitConfigBuilder {
        &mut self.preemptive_circuits
    }

    /// Return a mutable reference to a [`CircuitTimingBuilder`](circ::CircuitTimingBuilder).
    ///
    /// This section overrides Arti's rules for deciding how long to use
    /// circuits, and when to give up on attempts to launch them.
    pub fn circuit_timing(&mut self) -> &mut circ::CircuitTimingBuilder {
        &mut self.circuit_timing
    }

    /// Return a mutable reference to a [`StreamTimeoutConfigBuilder`].
    ///
    /// This section overrides Arti's rules for deciding how long a stream
    /// request (that is, an attempt to connect or resolve) should wait
    /// for a response before deciding that the stream has timed out.
    pub fn stream_timeouts(&mut self) -> &mut StreamTimeoutConfigBuilder {
        &mut self.stream_timeouts
    }

    /// Return a mutable reference to a [`ClientAddrConfigBuilder`].
    ///
    /// This section controls which addresses Arti is willing to launch connections
    /// to over the Tor network.  Any addresses rejected by this section cause
    /// stream attempts to fail before any traffic is sent over the network.
    pub fn address_filter(&mut self) -> &mut ClientAddrConfigBuilder {
        &mut self.address_filter
    }
}

#[cfg(test)]
@@ -526,13 +396,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()));
+1 −1
Original line number Diff line number Diff line
@@ -20,7 +20,7 @@ serde = { version = "1.0.103", features = ["derive"] }
toml = "0.5"
regex = { version = "1", default-features = false, features = ["std"] }
thiserror = "1"
derive_builder = "0.11"
derive_builder = "0.11.2"

[dev-dependencies]
tempfile = "3"
Loading