diff --git a/rust/client-shared/src/eventloop.rs b/rust/client-shared/src/eventloop.rs index 7ab107866..ae753d301 100644 --- a/rust/client-shared/src/eventloop.rs +++ b/rust/client-shared/src/eventloop.rs @@ -201,15 +201,15 @@ impl Eventloop { fn handle_tunnel_event(&mut self, event: firezone_tunnel::ClientEvent) -> Option { match event { firezone_tunnel::ClientEvent::AddedIceCandidates { - conn_id: gateway, + conn_id: gid, candidates, } => { - tracing::debug!(%gateway, ?candidates, "Sending new ICE candidates to gateway"); + tracing::debug!(%gid, ?candidates, "Sending new ICE candidates to gateway"); self.portal.send( PHOENIX_TOPIC, EgressMessages::BroadcastIceCandidates(GatewaysIceCandidates { - gateway_ids: vec![gateway], + gateway_ids: vec![gid], candidates, }), ); @@ -217,15 +217,15 @@ impl Eventloop { None } firezone_tunnel::ClientEvent::RemovedIceCandidates { - conn_id: gateway, + conn_id: gid, candidates, } => { - tracing::debug!(%gateway, ?candidates, "Sending invalidated ICE candidates to gateway"); + tracing::debug!(%gid, ?candidates, "Sending invalidated ICE candidates to gateway"); self.portal.send( PHOENIX_TOPIC, EgressMessages::BroadcastInvalidatedIceCandidates(GatewaysIceCandidates { - gateway_ids: vec![gateway], + gateway_ids: vec![gid], candidates, }), ); diff --git a/rust/connlib/tunnel/src/client.rs b/rust/connlib/tunnel/src/client.rs index e2f59d431..05ada3d8c 100644 --- a/rust/connlib/tunnel/src/client.rs +++ b/rust/connlib/tunnel/src/client.rs @@ -589,12 +589,12 @@ impl ClientState { self.drain_node_events(); } - #[tracing::instrument(level = "debug", skip_all, fields(%resource_id))] + #[tracing::instrument(level = "debug", skip_all, fields(%rid))] #[expect(clippy::too_many_arguments)] pub fn handle_flow_created( &mut self, - resource_id: ResourceId, - gateway_id: GatewayId, + rid: ResourceId, + gid: GatewayId, gateway_key: PublicKey, gateway_tun: IpConfig, site_id: SiteId, @@ -603,30 +603,27 @@ impl ClientState { gateway_ice: IceCredentials, now: Instant, ) -> anyhow::Result> { - tracing::debug!(%gateway_id, "New flow authorized for resource"); + tracing::debug!(%gid, "New flow authorized for resource"); - let resource = self - .resources_by_id - .get(&resource_id) - .context("Unknown resource")?; + let resource = self.resources_by_id.get(&rid).context("Unknown resource")?; - let Some(pending_flow) = self.pending_flows.remove(&resource_id) else { + let Some(pending_flow) = self.pending_flows.remove(&rid) else { tracing::debug!("No pending flow"); return Ok(Ok(())); }; - if let Some(old_gateway_id) = self.resources_gateways.insert(resource_id, gateway_id) + if let Some(old_gateway_id) = self.resources_gateways.insert(rid, gid) && self.peers.get(&old_gateway_id).is_some() { assert_eq!( - old_gateway_id, gateway_id, - "Resources are not expected to change gateways without a previous message, resource_id = {resource_id}" + old_gateway_id, gid, + "Resources are not expected to change gateways without a previous message, resource_id = {rid}" ) } match self.node.upsert_connection( - gateway_id, + gid, gateway_key, Secret::new(preshared_key.expose_secret().0), snownet::Credentials { @@ -642,19 +639,19 @@ impl ClientState { Ok(()) => {} Err(e) => return Ok(Err(e)), }; - self.resources_gateways.insert(resource_id, gateway_id); - self.gateways_site.insert(gateway_id, site_id); - self.recently_connected_gateways.put(gateway_id, ()); + self.resources_gateways.insert(rid, gid); + self.gateways_site.insert(gid, site_id); + self.recently_connected_gateways.put(gid, ()); - if self.peers.get(&gateway_id).is_none() { + if self.peers.get(&gid).is_none() { self.peers - .insert(GatewayOnClient::new(gateway_id, gateway_tun), &[]); + .insert(GatewayOnClient::new(gid, gateway_tun), &[]); }; // Allow looking up the Gateway via its TUN IP. // Resources are not allowed to be in our CG-NAT range, therefore in practise this cannot overlap with resources. - self.peers.add_ip(&gateway_id, &gateway_tun.v4.into()); - self.peers.add_ip(&gateway_id, &gateway_tun.v6.into()); + self.peers.add_ip(&gid, &gateway_tun.v4.into()); + self.peers.add_ip(&gid, &gateway_tun.v6.into()); // Deal with buffered packets @@ -663,17 +660,14 @@ impl ClientState { match resource { Resource::Cidr(_) | Resource::Internet(_) => { - self.peers.add_ips_with_resource( - &gateway_id, - resource.addresses().into_iter(), - &resource_id, - ); + self.peers + .add_ips_with_resource(&gid, resource.addresses().into_iter(), &rid); // For CIDR and Internet resources, we can directly queue the buffered packets. for packet in buffered_resource_packets { encapsulate_and_buffer( packet, - gateway_id, + gid, now, &mut self.node, &mut self.buffered_transmits, @@ -687,7 +681,7 @@ impl ClientState { // 2. Buffered UDP DNS queries for the Gateway for packet in pending_flow.udp_dns_queries { - let gateway = self.peers.get(&gateway_id).context("Unknown peer")?; // If this error happens we have a bug: We just inserted it above. + let gateway = self.peers.get(&gid).context("Unknown peer")?; // If this error happens we have a bug: We just inserted it above. let upstream = gateway.tun_dns_server_endpoint(packet.destination()); let packet = @@ -695,7 +689,7 @@ impl ClientState { encapsulate_and_buffer( packet, - gateway_id, + gid, now, &mut self.node, &mut self.buffered_transmits, @@ -768,10 +762,10 @@ impl ClientState { self.cleanup_connected_gateway(&disconnected_gateway); } - #[tracing::instrument(level = "debug", skip_all, fields(%resource))] + #[tracing::instrument(level = "debug", skip_all, fields(%rid))] fn on_not_connected_resource( &mut self, - resource: ResourceId, + rid: ResourceId, trigger: impl Into, now: Instant, ) { @@ -779,9 +773,9 @@ impl ClientState { let trigger = trigger.into(); - debug_assert!(self.resources_by_id.contains_key(&resource)); + debug_assert!(self.resources_by_id.contains_key(&rid)); - match self.pending_flows.entry(resource) { + match self.pending_flows.entry(rid) { Entry::Vacant(v) => { v.insert(PendingFlow::new(now, trigger)); } @@ -804,7 +798,7 @@ impl ClientState { self.buffered_events .push_back(ClientEvent::ConnectionIntent { - resource, + resource: rid, connected_gateway_ids: self.connected_gateway_ids(), }) } @@ -915,7 +909,7 @@ impl ClientState { .map(|(_, r)| *r) .filter(|resource| self.is_resource_enabled(resource)) .inspect( - |resource| tracing::trace!(target: "tunnel_test_coverage", %destination, %resource, "Packet for DNS resource"), + |rid| tracing::trace!(target: "tunnel_test_coverage", %destination, %rid, "Packet for DNS resource"), ); // We don't need to filter from here because resources are removed from the active_cidr_resources as soon as they are disabled. @@ -924,7 +918,7 @@ impl ClientState { .longest_match(destination) .map(|(_, res)| res.id) .inspect( - |resource| tracing::trace!(target: "tunnel_test_coverage", %destination, %resource, "Packet for CIDR resource"), + |rid| tracing::trace!(target: "tunnel_test_coverage", %destination, %rid, "Packet for CIDR resource"), ); maybe_dns_resource_id diff --git a/rust/connlib/tunnel/src/dns.rs b/rust/connlib/tunnel/src/dns.rs index 195931684..851ceefef 100644 --- a/rust/connlib/tunnel/src/dns.rs +++ b/rust/connlib/tunnel/src/dns.rs @@ -283,7 +283,7 @@ impl StubResolver { self.get_or_assign_aaaa_records(domain.clone(), resource) } (RecordType::SRV | RecordType::TXT, Some(resource)) => { - tracing::debug!(%qtype, resource = %resource.id, "Forwarding query for DNS resource to corresponding site"); + tracing::debug!(%qtype, rid = %resource.id, "Forwarding query for DNS resource to corresponding site"); return ResolveStrategy::RecurseSite(resource.id); } @@ -323,11 +323,11 @@ impl StubResolver { } } -pub fn is_subdomain(name: &dns_types::DomainName, resource: &str) -> bool { - let pattern = match Pattern::new(resource) { +pub fn is_subdomain(name: &dns_types::DomainName, pattern: &str) -> bool { + let pattern = match Pattern::new(pattern) { Ok(p) => p, Err(e) => { - tracing::warn!(%resource, "Unable to parse pattern: {}", err_with_src(&e)); + tracing::warn!(%pattern, "Unable to parse pattern: {}", err_with_src(&e)); return false; } }; diff --git a/rust/connlib/tunnel/src/gateway.rs b/rust/connlib/tunnel/src/gateway.rs index b729e09e0..e7d14c6ee 100644 --- a/rust/connlib/tunnel/src/gateway.rs +++ b/rust/connlib/tunnel/src/gateway.rs @@ -193,15 +193,15 @@ impl GatewayState { self.drain_node_events(); } - #[tracing::instrument(level = "debug", skip_all, fields(%resource, %client))] - pub fn remove_access(&mut self, client: &ClientId, resource: &ResourceId) { - let Some(peer) = self.peers.get_mut(client) else { + #[tracing::instrument(level = "debug", skip_all, fields(%rid, %cid))] + pub fn remove_access(&mut self, cid: &ClientId, rid: &ResourceId) { + let Some(peer) = self.peers.get_mut(cid) else { return; }; - peer.remove_resource(resource); + peer.remove_resource(rid); if peer.is_emptied() { - self.peers.remove(client); + self.peers.remove(cid); } tracing::debug!("Access removed"); @@ -230,11 +230,11 @@ impl GatewayState { }) } - #[tracing::instrument(level = "debug", skip_all, fields(%client_id))] + #[tracing::instrument(level = "debug", skip_all, fields(%cid))] #[expect(clippy::too_many_arguments)] pub fn authorize_flow( &mut self, - client_id: ClientId, + cid: ClientId, client_key: PublicKey, preshared_key: SecretKey, client_ice: IceCredentials, @@ -245,7 +245,7 @@ impl GatewayState { now: Instant, ) -> Result<(), NoTurnServers> { self.node.upsert_connection( - client_id, + cid, client_key, Secret::new(preshared_key.expose_secret().0), Credentials { @@ -259,7 +259,7 @@ impl GatewayState { now, )?; - let result = self.allow_access(client_id, client_tun, expires_at, resource, None); + let result = self.allow_access(cid, client_tun, expires_at, resource, None); debug_assert!( result.is_ok(), "`allow_access` should never fail without a `DnsResourceEntry`" @@ -495,7 +495,7 @@ fn handle_p2p_control_packet( }; if !peer.is_allowed(req.resource) { - tracing::warn!(cid = %peer.id(), resource = %req.resource, "Received `AssignedIpsEvent` for resource that is not allowed"); + tracing::warn!(cid = %peer.id(), rid = %req.resource, "Received `AssignedIpsEvent` for resource that is not allowed"); let packet = dns_resource_nat::domain_status( req.resource, diff --git a/rust/connlib/tunnel/src/peer.rs b/rust/connlib/tunnel/src/peer.rs index 4aaf133b8..d38e0bf1a 100644 --- a/rust/connlib/tunnel/src/peer.rs +++ b/rust/connlib/tunnel/src/peer.rs @@ -209,7 +209,7 @@ impl ClientOnGateway { resource: crate::messages::gateway::ResourceDescription, expires_at: Option>, ) { - tracing::info!(client = %self.id, resource = %resource.id(), expires = ?expires_at.map(|e| e.to_rfc3339()), "Allowing access to resource"); + 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) => { @@ -233,13 +233,9 @@ impl ClientOnGateway { self.recalculate_filters(); } - pub(crate) fn update_resource_expiry( - &mut self, - resource: ResourceId, - new_expiry: DateTime, - ) { - let Some(resource) = self.resources.get_mut(&resource) else { - tracing::debug!(%resource, "Unknown resource"); + pub(crate) fn update_resource_expiry(&mut self, rid: ResourceId, new_expiry: DateTime) { + let Some(resource) = self.resources.get_mut(&rid) else { + tracing::debug!(%rid, "Unknown resource"); return; }; @@ -256,11 +252,11 @@ impl ClientOnGateway { } pub(crate) fn retain_authorizations(&mut self, authorization: BTreeSet) { - for (resource, _) in self + for (rid, _) in self .resources .extract_if(|resource, _| !authorization.contains(resource)) { - tracing::info!(%resource, "Revoking resource authorization"); + tracing::info!(%rid, "Revoking resource authorization"); } self.recalculate_filters(); diff --git a/rust/connlib/tunnel/src/tests/sim_client.rs b/rust/connlib/tunnel/src/tests/sim_client.rs index 66ab50e09..a302bc134 100644 --- a/rust/connlib/tunnel/src/tests/sim_client.rs +++ b/rust/connlib/tunnel/src/tests/sim_client.rs @@ -525,12 +525,12 @@ impl RefClient { if let Some(overlapping_resource) = table.exact_match(resource.address) && self.is_connected_to_internet_or_cidr(*overlapping_resource) { - tracing::debug!(%overlapping_resource, resource = %resource.id, address = %resource.address, "Already connected to resource with this exact address, retaining existing route"); + tracing::debug!(%overlapping_resource, rid = %resource.id, address = %resource.address, "Already connected to resource with this exact address, retaining existing route"); continue; } - tracing::debug!(resource = %resource.id, address = %resource.address, "Adding CIDR route"); + tracing::debug!(rid = %resource.id, address = %resource.address, "Adding CIDR route"); table.insert(resource.address, resource.id); } @@ -750,16 +750,16 @@ impl RefClient { } } - fn set_resource_online(&mut self, resource: ResourceId) { - let Some(site) = self.site_for_resource(resource) else { - tracing::error!(%resource, "Unknown resource or multi-site resource"); + fn set_resource_online(&mut self, rid: ResourceId) { + let Some(site) = self.site_for_resource(rid) else { + tracing::error!(%rid, "Unknown resource or multi-site resource"); return; }; let previous = self.site_status.insert(site.id, ResourceStatus::Online); if previous.is_none_or(|s| s != ResourceStatus::Online) { - tracing::debug!(%resource, "Resource is now online"); + tracing::debug!(%rid, "Resource is now online"); } } @@ -767,17 +767,17 @@ impl RefClient { self.is_connected_to_cidr(resource) || self.is_connected_to_internet(resource) } - fn connect_to_internet_or_cidr_resource(&mut self, resource: ResourceId) { - if self.internet_resource.is_some_and(|r| r == resource) { + fn connect_to_internet_or_cidr_resource(&mut self, rid: ResourceId) { + if self.internet_resource.is_some_and(|r| r == rid) { self.connected_internet_resource = true; return; } - if self.cidr_resources.iter().any(|(_, r)| *r == resource) { - let is_new = self.connected_cidr_resources.insert(resource); + if self.cidr_resources.iter().any(|(_, r)| *r == rid) { + let is_new = self.connected_cidr_resources.insert(rid); if is_new { - tracing::debug!(%resource, "Now connected to CIDR resource"); + tracing::debug!(%rid, "Now connected to CIDR resource"); } } } diff --git a/rust/gateway/src/eventloop.rs b/rust/gateway/src/eventloop.rs index 88fcda4b3..0718f4f81 100644 --- a/rust/gateway/src/eventloop.rs +++ b/rust/gateway/src/eventloop.rs @@ -423,17 +423,17 @@ impl Eventloop { }, )); for Authorization { - client_id, - resource_id, + client_id: cid, + resource_id: rid, expires_at, } in authorizations { - if let Err(e) = self.tunnel.state_mut().update_access_authorization_expiry( - client_id, - resource_id, - expires_at, - ) { - tracing::debug!(%client_id, %resource_id, "Failed to update access authorization: {e:#}"); + if let Err(e) = self + .tunnel + .state_mut() + .update_access_authorization_expiry(cid, rid, expires_at) + { + tracing::debug!(%cid, %rid, "Failed to update access authorization: {e:#}"); } } @@ -474,19 +474,19 @@ impl Eventloop { msg: IngressMessages::AccessAuthorizationExpiryUpdated( AccessAuthorizationExpiryUpdated { - client_id, - resource_id, + client_id: cid, + resource_id: rid, expires_at, }, ), .. } => { - if let Err(e) = self.tunnel.state_mut().update_access_authorization_expiry( - client_id, - resource_id, - expires_at, - ) { - tracing::warn!(%client_id, %resource_id, "Failed to update expiry of access authorization: {e:#}") + if let Err(e) = self + .tunnel + .state_mut() + .update_access_authorization_expiry(cid, rid, expires_at) + { + tracing::warn!(%cid, %rid, "Failed to update expiry of access authorization: {e:#}") }; } phoenix_channel::Event::ErrorResponse { topic, req_id, res } => { @@ -518,7 +518,7 @@ impl Eventloop { let addresses = match result { Ok(addresses) => addresses, Err(e) => { - tracing::debug!(client = %req.client.id, reference = %req.reference, "DNS resolution failed as part of connection request: {e:#}"); + tracing::debug!(cid = %req.client.id, reference = %req.reference, "DNS resolution failed as part of connection request: {e:#}"); return; // Fail the connection so the client runs into a timeout. } @@ -558,10 +558,10 @@ impl Eventloop { .domain .map(|r| DnsResourceNatEntry::new(r, addresses)), ) { - let client = req.client.id; + let cid = req.client.id; - self.tunnel.state_mut().cleanup_connection(&client); - tracing::debug!(%client, "Connection request failed: {e:#}"); + self.tunnel.state_mut().cleanup_connection(&cid); + tracing::debug!(%cid, "Connection request failed: {e:#}"); return; } @@ -586,7 +586,7 @@ impl Eventloop { let addresses = match result { Ok(addresses) => addresses, Err(e) => { - tracing::debug!(client = %req.client_id, reference = %req.reference, "DNS resolution failed as part of allow access request: {e:#}"); + tracing::debug!(cid = %req.client_id, reference = %req.reference, "DNS resolution failed as part of allow access request: {e:#}"); vec![] } @@ -602,7 +602,7 @@ impl Eventloop { req.resource, req.payload.map(|r| DnsResourceNatEntry::new(r, addresses)), ) { - tracing::warn!(client = %req.client_id, "Allow access request failed: {e:#}"); + tracing::warn!(cid = %req.client_id, "Allow access request failed: {e:#}"); }; }