Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(connlib): report resource status to client #4931

Merged
merged 19 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions rust/connlib/clients/android/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
// ecosystem, so it's used here for consistency.

use connlib_client_shared::{
file_logger, keypair, Callbacks, Cidrv4, Cidrv6, Error, LoginUrl, LoginUrlError,
ResourceDescription, Session, Sockets,
callbacks::ResourceDescription, file_logger, keypair, Callbacks, Cidrv4, Cidrv6, Error,
LoginUrl, LoginUrlError, Session, Sockets,
};
use jni::{
objects::{GlobalRef, JClass, JObject, JString, JValue},
Expand Down
4 changes: 2 additions & 2 deletions rust/connlib/clients/apple/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
#![allow(clippy::unnecessary_cast, improper_ctypes, non_camel_case_types)]

use connlib_client_shared::{
file_logger, keypair, Callbacks, Cidrv4, Cidrv6, Error, LoginUrl, ResourceDescription, Session,
Sockets,
callbacks::ResourceDescription, file_logger, keypair, Callbacks, Cidrv4, Cidrv6, Error,
LoginUrl, Session, Sockets,
};
use secrecy::SecretString;
use std::{
Expand Down
13 changes: 8 additions & 5 deletions rust/connlib/clients/shared/src/eventloop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ where
gateway_id,
resource_id,
relays,
site_id,
..
}) => {
let should_accept = self
Expand All @@ -280,10 +281,12 @@ where
return;
}

match self
.tunnel
.create_or_reuse_connection(resource_id, gateway_id, relays)
{
match self.tunnel.create_or_reuse_connection(
resource_id,
gateway_id,
relays,
site_id,
) {
Ok(firezone_tunnel::Request::NewConnection(connection_request)) => {
// TODO: keep track for the response
let _id = self.portal.send(
Expand Down Expand Up @@ -321,7 +324,7 @@ where

tracing::debug!(resource_id = %offline_resource, "Resource is offline");

self.tunnel.cleanup_connection(offline_resource);
self.tunnel.set_resource_offline(offline_resource);
}

ErrorReply::Disabled => {
Expand Down
2 changes: 1 addition & 1 deletion rust/connlib/clients/shared/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Main connlib library for clients.
pub use connlib_shared::messages::client::ResourceDescription;
pub use connlib_shared::{
keypair, Callbacks, Cidrv4, Cidrv6, Error, LoginUrl, LoginUrlError, StaticSecret,
callbacks, keypair, Callbacks, Cidrv4, Cidrv6, Error, LoginUrl, LoginUrlError, StaticSecret,
};
pub use firezone_tunnel::Sockets;
pub use tracing_appender::non_blocking::WorkerGuard;
Expand Down
20 changes: 12 additions & 8 deletions rust/connlib/clients/shared/src/messages.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use connlib_shared::messages::{
client::ResourceDescription, GatewayId, GatewayResponse, Interface, Key, Relay, RelaysPresence,
RequestConnection, ResourceId, ReuseConnection,
client::{ResourceDescription, SiteId},
GatewayId, GatewayResponse, Interface, Key, Relay, RelaysPresence, RequestConnection,
ResourceId, ReuseConnection,
};
use serde::{Deserialize, Serialize};
use std::{collections::HashSet, net::IpAddr};
Expand All @@ -25,6 +26,8 @@ pub struct ConnectionDetails {
pub resource_id: ResourceId,
pub gateway_id: GatewayId,
pub gateway_remote_ip: IpAddr,
#[serde(rename = "gateway_group_id")]
pub site_id: SiteId,
}

#[derive(Debug, Deserialize, Clone, PartialEq)]
Expand Down Expand Up @@ -101,8 +104,7 @@ mod test {
use super::*;
use chrono::DateTime;
use connlib_shared::messages::{
client::ResourceDescriptionCidr,
client::{GatewayGroup, ResourceDescriptionDns},
client::{ResourceDescriptionCidr, ResourceDescriptionDns, Site},
DnsServer, IpDnsServer, Stun, Turn,
};
use phoenix_channel::{OutboundRequestId, PhoenixMessage};
Expand Down Expand Up @@ -234,7 +236,7 @@ mod test {
address: "172.172.0.0/16".parse().unwrap(),
name: "172.172.0.0/16".to_string(),
address_description: "cidr resource".to_string(),
gateway_groups: vec![GatewayGroup {
sites: vec![Site {
name: "test".to_string(),
id: "bf56f32d-7b2c-4f5d-a784-788977d014a4".parse().unwrap(),
}],
Expand All @@ -244,7 +246,7 @@ mod test {
address: "gitlab.mycorp.com".to_string(),
name: "gitlab.mycorp.com".to_string(),
address_description: "dns resource".to_string(),
gateway_groups: vec![GatewayGroup {
sites: vec![Site {
name: "test".to_string(),
id: "bf56f32d-7b2c-4f5d-a784-788977d014a4".parse().unwrap(),
}],
Expand Down Expand Up @@ -307,7 +309,7 @@ mod test {
address: "172.172.0.0/16".parse().unwrap(),
name: "172.172.0.0/16".to_string(),
address_description: "cidr resource".to_string(),
gateway_groups: vec![GatewayGroup {
sites: vec![Site {
name: "test".to_string(),
id: "bf56f32d-7b2c-4f5d-a784-788977d014a4".parse().unwrap(),
}],
Expand All @@ -317,7 +319,7 @@ mod test {
address: "gitlab.mycorp.com".to_string(),
name: "gitlab.mycorp.com".to_string(),
address_description: "dns resource".to_string(),
gateway_groups: vec![GatewayGroup {
sites: vec![Site {
name: "test".to_string(),
id: "bf56f32d-7b2c-4f5d-a784-788977d014a4".parse().unwrap(),
}],
Expand Down Expand Up @@ -536,6 +538,7 @@ mod test {
gateway_id: "73037362-715d-4a83-a749-f18eadd970e6".parse().unwrap(),
gateway_remote_ip: "172.28.0.1".parse().unwrap(),
resource_id: "f16ecfa0-a94f-4bfd-a2ef-1cc1f2ef3da3".parse().unwrap(),
site_id: "bf56f32d-7b2c-4f5d-a784-788977d014a4".parse().unwrap(),
relays: vec![
Relay::Stun(Stun {
id: "c9cb8892-e355-41e6-a882-b6d6c38beb66".parse().unwrap(),
Expand Down Expand Up @@ -573,6 +576,7 @@ mod test {
"resource_id": "f16ecfa0-a94f-4bfd-a2ef-1cc1f2ef3da3",
"gateway_id": "73037362-715d-4a83-a749-f18eadd970e6",
"gateway_remote_ip": "172.28.0.1",
"gateway_group_id": "bf56f32d-7b2c-4f5d-a784-788977d014a4",
"relays": [
{
"id": "c9cb8892-e355-41e6-a882-b6d6c38beb66",
Expand Down
4 changes: 3 additions & 1 deletion rust/connlib/shared/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ edition = "2021"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[features]
mock = []
proptest = ["dep:proptest", "dep:itertools"]

[dependencies]
anyhow = "1.0.82"
Expand Down Expand Up @@ -34,13 +35,14 @@ libc = "0.2"
snownet = { workspace = true }
phoenix-channel = { workspace = true }
proptest = { version = "1.4.0", optional = true }
itertools = { version = "0.12", optional = true }

# Needed for Android logging until tracing is working
log = "0.4"

[dev-dependencies]
itertools = "0.12"
tempfile = "3.10.1"
itertools = "0.12"
mutants = "0.0.3" # Needed to mark functions as exempt from `cargo-mutants` testing
tokio = { version = "1.36", features = ["macros", "rt"] }

Expand Down
126 changes: 123 additions & 3 deletions rust/connlib/shared/src/callbacks.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use crate::messages::client::ResourceDescription;
use ip_network::{Ipv4Network, Ipv6Network};
use serde::Serialize;
use ip_network::{IpNetwork, Ipv4Network, Ipv6Network};
use serde::{Deserialize, Serialize};
use std::borrow::Cow;
use std::fmt::Debug;
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};

use crate::messages::client::Site;
use crate::messages::ResourceId;

// Avoids having to map types for Windows
type RawFd = i32;

Expand Down Expand Up @@ -39,6 +42,123 @@ impl From<Ipv6Network> for Cidrv6 {
}
}

#[derive(Debug, Serialize, Deserialize, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub enum Status {
Unknown,
Online,
Offline,
}

#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum ResourceDescription {
ReactorScram marked this conversation as resolved.
Show resolved Hide resolved
Dns(ResourceDescriptionDns),
Cidr(ResourceDescriptionCidr),
}

impl ResourceDescription {
pub fn name(&self) -> &str {
match self {
ResourceDescription::Dns(r) => &r.name,
ResourceDescription::Cidr(r) => &r.name,
}
}

pub fn status(&self) -> Status {
match self {
ResourceDescription::Dns(r) => r.status,
ResourceDescription::Cidr(r) => r.status,
}
}

pub fn id(&self) -> ResourceId {
match self {
ResourceDescription::Dns(r) => r.id,
ResourceDescription::Cidr(r) => r.id,
}
}

Comment on lines +60 to +79
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm we can't flip ResourceDescription into a struct that shares these fields, because it would break compatibility with older code, right? Or if this type of ResourceDescription is only used within the Client, between connlib and the GUI, it could be done?

e.g.

struct ResourceDescription {
    id: ResourceId,
    name: String,
    status: Status,
    other: ResourceType,
}

enum ResourceType {
    Cidr,
    Dns,
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

address is also part of a ResourceDescription and it has a different type depending on whether it's DNS or CIDR

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the other ones look like they have the same type, since there's no into

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, but some clients uses the address, maybe we can convert it to a String but I'm not sure having a single type will simplify things, specially since these are used only for serialization and we don't need to access the fields.

/// What the GUI clients should paste to the clipboard, e.g. `https://github.com/firezone`
pub fn pastable(&self) -> Cow<'_, str> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick note -- this will be updated to use the address_description in #3514

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 this is just copy-pasted though

match self {
ResourceDescription::Dns(r) => Cow::from(&r.address),
ResourceDescription::Cidr(r) => Cow::from(r.address.to_string()),
}
}
}

impl From<ResourceDescription> for crate::messages::client::ResourceDescription {
fn from(value: ResourceDescription) -> Self {
match value {
ResourceDescription::Dns(r) => {
crate::messages::client::ResourceDescription::Dns(r.into())
}
ResourceDescription::Cidr(r) => {
crate::messages::client::ResourceDescription::Cidr(r.into())
}
}
}
}

#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash)]
pub struct ResourceDescriptionDns {
/// Resource's id.
pub id: ResourceId,
/// Internal resource's domain name.
pub address: String,
/// Name of the resource.
///
/// Used only for display.
pub name: String,

pub address_description: String,
pub sites: Vec<Site>,

pub status: Status,
}

impl From<ResourceDescriptionDns> for crate::messages::client::ResourceDescriptionDns {
fn from(r: ResourceDescriptionDns) -> Self {
crate::messages::client::ResourceDescriptionDns {
id: r.id,
address: r.address,
address_description: r.address_description,
name: r.name,
sites: r.sites,
}
}
}

/// Description of a resource that maps to a CIDR.
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct ResourceDescriptionCidr {
/// Resource's id.
pub id: ResourceId,
/// CIDR that this resource points to.
pub address: IpNetwork,
/// Name of the resource.
///
/// Used only for display.
pub name: String,

pub address_description: String,
pub sites: Vec<Site>,

pub status: Status,
}

impl From<ResourceDescriptionCidr> for crate::messages::client::ResourceDescriptionCidr {
fn from(r: ResourceDescriptionCidr) -> Self {
crate::messages::client::ResourceDescriptionCidr {
id: r.id,
address: r.address,
address_description: r.address_description,
name: r.name,
sites: r.sites,
}
}
}

/// Traits that will be used by connlib to callback the client upper layers.
pub trait Callbacks: Clone + Send + Sync {
/// Called when the tunnel address is set.
Expand Down
2 changes: 1 addition & 1 deletion rust/connlib/shared/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! This includes types provided by external crates, i.e. [boringtun] to make sure that
//! we are using the same version across our own crates.

mod callbacks;
pub mod callbacks;
pub mod error;
pub mod messages;

Expand Down
11 changes: 9 additions & 2 deletions rust/connlib/shared/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ impl ClientId {
}
}

impl GatewayId {
#[cfg(feature = "proptest")]
pub(crate) fn from_u128(v: u128) -> Self {
Self(Uuid::from_u128(v))
}
}

#[derive(Hash, Debug, Deserialize, Serialize, Clone, Copy, PartialEq, Eq)]
pub struct ClientId(Uuid);

Expand Down Expand Up @@ -315,7 +322,7 @@ mod tests {

use super::{
client::ResourceDescription,
client::{GatewayGroup, ResourceDescriptionDns},
client::{ResourceDescriptionDns, Site},
ResourceId,
};

Expand All @@ -325,7 +332,7 @@ mod tests {
name: name.to_string(),
address: "unused.example.com".to_string(),
address_description: "test description".to_string(),
gateway_groups: vec![GatewayGroup {
sites: vec![Site {
name: "test".to_string(),
id: "99ba0c1e-5189-4cfc-a4db-fd6cb1c937fd".parse().unwrap(),
}],
Expand Down