chore(rust): ban keys and values from HashMap (#10569)

In addition to the `iter` functions, `keys` and `values` also iterate
over the contents of a `HashMap` and are thus non-deterministic. This
can create problems where our test-suite is non-deterministic.

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." },

rust/connlib/dns-over-tcp/src/client.rs

@@ -29,7 +29,7 @@ pub struct Client<const MIN_PORT: u16 = 49152, const MAX_PORT: u16 = 65535> {
 
     sockets: SocketSet<'static>,
     sockets_by_remote: BTreeMap<SocketAddr, l3_tcp::SocketHandle>,
-    local_ports_by_socket: HashMap<l3_tcp::SocketHandle, u16>,
+    local_ports_by_socket: BTreeMap<l3_tcp::SocketHandle, u16>,
     /// Queries we should send to a DNS resolver.
     pending_queries_by_remote: HashMap<SocketAddr, VecDeque<dns_types::Query>>,
     /// Queries we have sent to a DNS resolver and are waiting for a reply.

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<SocketHandle, SocketAddr>,
+    listen_endpoints: BTreeMap<SocketHandle, SocketAddr>,
 
     /// 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 {

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<ResourceId, ResourceOnGateway>,
+    resources: BTreeMap<ResourceId, ResourceOnGateway>,
     /// Caches the existence of internet resource
     internet_resource_enabled: bool,
     filters: IpNetworkTable<FilterEngine>,
@@ -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<ResourceId>) {
-        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<DomainName, BTreeSet<IpAddr>>,
+        domains: BTreeMap<DomainName, BTreeSet<IpAddr>>,
         filters: Filters,
         expires_at: Option<DateTime<Utc>>,
     },
@@ -601,7 +611,7 @@ impl ResourceOnGateway {
     fn new(resource: ResourceDescription, expires_at: Option<DateTime<Utc>>) -> Self {
         match resource {
             ResourceDescription::Dns(r) => ResourceOnGateway::Dns {
-                domains: HashMap::default(),
+                domains: BTreeMap::default(),
                 filters: r.filters,
                 address: r.address,
                 expires_at,