diff --git a/rust/clippy.toml b/rust/clippy.toml index ba5dd243e..9276330f5 100644 --- a/rust/clippy.toml +++ b/rust/clippy.toml @@ -1,6 +1,8 @@ avoid-breaking-exported-api = false # We don't publish anything to crates.io, hence we don't need to worry about breaking Rust API changes. disallowed-methods = [ { path = "std::collections::HashMap::iter", reason = "HashMap has non-deterministic iteration order, use BTreeMap instead" }, + { path = "std::collections::HashMap::keys", reason = "HashMap has non-deterministic iteration order, use BTreeMap instead" }, + { path = "std::collections::HashMap::values", reason = "HashMap has non-deterministic iteration order, use BTreeMap instead" }, { path = "std::collections::HashSet::iter", reason = "HashSet has non-deterministic iteration order, use BTreeSet instead" }, { path = "std::collections::HashMap::iter_mut", reason = "HashMap has non-deterministic iteration order, use BTreeMap instead" }, { path = "tracing::subscriber::set_global_default", reason = "Does not init `LogTracer`, use `firezone_logging::init` instead." }, diff --git a/rust/connlib/dns-over-tcp/src/client.rs b/rust/connlib/dns-over-tcp/src/client.rs index ecba7b7fe..beafffc94 100644 --- a/rust/connlib/dns-over-tcp/src/client.rs +++ b/rust/connlib/dns-over-tcp/src/client.rs @@ -29,7 +29,7 @@ pub struct Client { sockets: SocketSet<'static>, sockets_by_remote: BTreeMap, - local_ports_by_socket: HashMap, + local_ports_by_socket: BTreeMap, /// Queries we should send to a DNS resolver. pending_queries_by_remote: HashMap>, /// Queries we have sent to a DNS resolver and are waiting for a reply. diff --git a/rust/connlib/dns-over-tcp/src/server.rs b/rust/connlib/dns-over-tcp/src/server.rs index 6be533a37..9c06d88cc 100644 --- a/rust/connlib/dns-over-tcp/src/server.rs +++ b/rust/connlib/dns-over-tcp/src/server.rs @@ -1,5 +1,5 @@ use std::{ - collections::{BTreeSet, HashMap, VecDeque}, + collections::{BTreeMap, BTreeSet, HashMap, VecDeque}, net::SocketAddr, time::{Duration, Instant}, }; @@ -20,7 +20,7 @@ pub struct Server { interface: Interface, sockets: SocketSet<'static>, - listen_endpoints: HashMap, + listen_endpoints: BTreeMap, /// Tracks the [`SocketHandle`] on which we need to send a reply for a given query by the local socket address, remote socket address and query ID. pending_sockets_by_local_remote_and_query_id: @@ -85,7 +85,7 @@ impl Server { let mut sockets = SocketSet::new(Vec::with_capacity(addresses.len() * NUM_CONCURRENT_CLIENTS)); - let mut listen_endpoints = HashMap::with_capacity(addresses.len()); + let mut listen_endpoints = BTreeMap::new(); for listen_endpoint in addresses { for _ in 0..NUM_CONCURRENT_CLIENTS { diff --git a/rust/connlib/tunnel/src/peer.rs b/rust/connlib/tunnel/src/peer.rs index d23576f1e..6b0aa1fba 100644 --- a/rust/connlib/tunnel/src/peer.rs +++ b/rust/connlib/tunnel/src/peer.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque, hash_map}; +use std::collections::{BTreeMap, BTreeSet, HashSet, VecDeque, btree_map}; use std::iter; use std::net::{IpAddr, SocketAddr}; use std::time::Instant; @@ -68,7 +68,7 @@ pub struct ClientOnGateway { client_tun: IpConfig, gateway_tun: IpConfig, - resources: HashMap, + resources: BTreeMap, /// Caches the existence of internet resource internet_resource_enabled: bool, filters: IpNetworkTable, @@ -89,7 +89,7 @@ impl ClientOnGateway { id, client_tun, gateway_tun, - resources: HashMap::new(), + resources: BTreeMap::new(), filters: IpNetworkTable::new(), permanent_translations: Default::default(), nat_table: Default::default(), @@ -187,10 +187,17 @@ impl ClientOnGateway { let cid = self.id; let mut any_expired = false; - for (rid, _) in self.resources.extract_if(|_, r| !r.is_allowed(&now)) { - any_expired = true; + // TODO: Replace with `extract_if` once we are on Rust 1.91 + self.resources.retain(|rid, r| { + if r.is_allowed(&now) { + return true; + } + tracing::info!(%cid, %rid, "Access to resource expired"); - } + + any_expired = true; + false + }); if any_expired { self.recalculate_filters(); @@ -218,10 +225,10 @@ impl ClientOnGateway { tracing::info!(cid = %self.id, rid = %resource.id(), expires = ?expires_at.map(|e| e.to_rfc3339()), "Allowing access to resource"); match self.resources.entry(resource.id()) { - hash_map::Entry::Vacant(v) => { + btree_map::Entry::Vacant(v) => { v.insert(ResourceOnGateway::new(resource, expires_at)); } - hash_map::Entry::Occupied(mut o) => o.get_mut().update(&resource), + btree_map::Entry::Occupied(mut o) => o.get_mut().update(&resource), } self.recalculate_filters(); @@ -258,12 +265,15 @@ impl ClientOnGateway { } pub(crate) fn retain_authorizations(&mut self, authorization: BTreeSet) { - for (rid, _) in self - .resources - .extract_if(|resource, _| !authorization.contains(resource)) - { + // TODO: Replace with `extract_if` once we are on Rust 1.91 + self.resources.retain(|rid, _| { + if authorization.contains(rid) { + return true; + } + tracing::info!(%rid, "Revoking resource authorization"); - } + false + }); self.recalculate_filters(); } @@ -588,7 +598,7 @@ enum ResourceOnGateway { }, Dns { address: String, - domains: HashMap>, + domains: BTreeMap>, filters: Filters, expires_at: Option>, }, @@ -601,7 +611,7 @@ impl ResourceOnGateway { fn new(resource: ResourceDescription, expires_at: Option>) -> Self { match resource { ResourceDescription::Dns(r) => ResourceOnGateway::Dns { - domains: HashMap::default(), + domains: BTreeMap::default(), filters: r.filters, address: r.address, expires_at,