Modular Diplonat (with the option to disable useless modules) #8
|
@ -5,10 +5,13 @@ services:
|
||||||
image: darkgallium/amd64_diplonat:v2
|
image: darkgallium/amd64_diplonat:v2
|
||||||
network_mode: host # required by UPNP/IGD
|
network_mode: host # required by UPNP/IGD
|
||||||
environment:
|
environment:
|
||||||
DIPLONAT_PRIVATE_IP: 192.168.0.18
|
|
||||||
DIPLONAT_REFRESH_TIME: 60
|
|
||||||
DIPLONAT_EXPIRATION_TIME: 300
|
|
||||||
DIPLONAT_CONSUL_NODE_NAME: lheureduthe
|
DIPLONAT_CONSUL_NODE_NAME: lheureduthe
|
||||||
|
DIPLONAT_FIREWALL_ENABLE: true
|
||||||
|
DIPLONAT_FIREWALL_REFRESH_TIME: 60
|
||||||
|
DIPLONAT_IGD_ENABLE: true
|
||||||
|
DIPLONAT_IGD_PRIVATE_IP: 192.168.0.18
|
||||||
|
DIPLONAT_IGD_EXPIRATION_TIME: 300
|
||||||
|
DIPLONAT_IGD_REFRESH_TIME: 60
|
||||||
RUST_LOG: debug
|
RUST_LOG: debug
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -3,7 +3,7 @@ mod options;
|
||||||
mod options_test;
|
mod options_test;
|
||||||
mod runtime;
|
mod runtime;
|
||||||
|
|
||||||
pub use options::{ConfigOpts, ConfigOptsAcme, ConfigOptsBase, ConfigOptsConsul};
|
pub use options::{ConfigOpts, ConfigOptsConsul, ConfigOptsAcme, ConfigOptsFirewall, ConfigOptsIgd};
|
||||||
pub use runtime::{RuntimeConfig, RuntimeConfigAcme, RuntimeConfigConsul, RuntimeConfigFirewall, RuntimeConfigIgd};
|
pub use runtime::{RuntimeConfig, RuntimeConfigAcme, RuntimeConfigConsul, RuntimeConfigFirewall, RuntimeConfigIgd};
|
||||||
|
|
||||||
pub const EXPIRATION_TIME: u16 = 300;
|
pub const EXPIRATION_TIME: u16 = 300;
|
||||||
|
|
|
@ -8,27 +8,13 @@ use crate::config::RuntimeConfig;
|
||||||
// This file parses the options that can be declared in the environment.
|
// This file parses the options that can be declared in the environment.
|
||||||
// runtime.rs applies business logic and builds RuntimeConfig structs.
|
// runtime.rs applies business logic and builds RuntimeConfig structs.
|
||||||
|
|
||||||
/// Base configuration options
|
// - Note for the future -
|
||||||
#[derive(Clone, Default, Deserialize)]
|
// There is no *need* to have a 'DIPLONAT_XXX_*' prefix for all config options.
|
||||||
pub struct ConfigOptsBase {
|
// If some config options are shared by several modules, a ConfigOptsBase could
|
||||||
/// This node's private IP address [default: None]
|
// contain them, and parse the 'DIPLONAT_*' prefix directly.
|
||||||
pub private_ip: Option<String>,
|
// Only in runtime.rs would these options find their proper location in each
|
||||||
/// Expiration time for IGD rules [default: 60]
|
// module's struct.
|
||||||
pub expiration_time: Option<u16>,
|
|
||||||
/// Refresh time for IGD and Firewall rules [default: 300]
|
|
||||||
pub refresh_time: Option<u16>,
|
|
||||||
}
|
|
||||||
|
|
||||||
/// ACME configuration options
|
|
||||||
#[derive(Clone, Default, Deserialize)]
|
|
||||||
pub struct ConfigOptsAcme {
|
|
||||||
/// Whether ACME is enabled [default: false]
|
|
||||||
#[serde(default)]
|
|
||||||
pub enable: bool,
|
|
||||||
|
|
||||||
/// The default domain holder's e-mail [default: None]
|
|
||||||
pub email: Option<String>,
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Consul configuration options
|
/// Consul configuration options
|
||||||
#[derive(Clone, Default, Deserialize)]
|
#[derive(Clone, Default, Deserialize)]
|
||||||
|
@ -39,23 +25,63 @@ pub struct ConfigOptsConsul {
|
||||||
pub url: Option<String>,
|
pub url: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// ACME configuration options
|
||||||
|
#[derive(Clone, Default, Deserialize)]
|
||||||
|
pub struct ConfigOptsAcme {
|
||||||
|
/// Whether the ACME module is enabled [default: false]
|
||||||
|
#[serde(default)]
|
||||||
|
pub enable: bool,
|
||||||
|
|
||||||
|
/// The default domain holder's e-mail [default: None]
|
||||||
|
pub email: Option<String>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Firewall configuration options
|
||||||
|
#[derive(Clone, Default, Deserialize)]
|
||||||
|
pub struct ConfigOptsFirewall {
|
||||||
|
/// Whether the firewall module is enabled [default: false]
|
||||||
|
#[serde(default)]
|
||||||
|
pub enable: bool,
|
||||||
|
|
||||||
|
/// Refresh time for firewall rules [default: 300]
|
||||||
|
pub refresh_time: Option<u16>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// IGD configuration options
|
||||||
|
#[derive(Clone, Default, Deserialize)]
|
||||||
|
pub struct ConfigOptsIgd {
|
||||||
|
/// Whether the IGD module is enabled [default: false]
|
||||||
|
#[serde(default)]
|
||||||
|
pub enable: bool,
|
||||||
|
|
||||||
|
/// This node's private IP address [default: None]
|
||||||
|
pub private_ip: Option<String>,
|
||||||
|
/// Expiration time for IGD rules [default: 60]
|
||||||
|
pub expiration_time: Option<u16>,
|
||||||
|
/// Refresh time for IGD rules [default: 300]
|
||||||
|
pub refresh_time: Option<u16>,
|
||||||
|
}
|
||||||
|
|
||||||
/// Model of all potential configuration options
|
/// Model of all potential configuration options
|
||||||
pub struct ConfigOpts {
|
pub struct ConfigOpts {
|
||||||
pub base: ConfigOptsBase,
|
|
||||||
pub acme: ConfigOptsAcme,
|
|
||||||
pub consul: ConfigOptsConsul,
|
pub consul: ConfigOptsConsul,
|
||||||
|
pub acme: ConfigOptsAcme,
|
||||||
|
pub firewall: ConfigOptsFirewall,
|
||||||
|
pub igd: ConfigOptsIgd,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ConfigOpts {
|
impl ConfigOpts {
|
||||||
pub fn from_env() -> Result<RuntimeConfig> {
|
pub fn from_env() -> Result<RuntimeConfig> {
|
||||||
let base: ConfigOptsBase = envy::prefixed("DIPLONAT_").from_env()?;
|
|
||||||
let consul: ConfigOptsConsul = envy::prefixed("DIPLONAT_CONSUL_").from_env()?;
|
let consul: ConfigOptsConsul = envy::prefixed("DIPLONAT_CONSUL_").from_env()?;
|
||||||
let acme: ConfigOptsAcme = envy::prefixed("DIPLONAT_ACME_").from_env()?;
|
let acme: ConfigOptsAcme = envy::prefixed("DIPLONAT_ACME_").from_env()?;
|
||||||
|
let firewall: ConfigOptsFirewall = envy::prefixed("DIPLONAT_FIREWALL_").from_env()?;
|
||||||
|
let igd: ConfigOptsIgd = envy::prefixed("DIPLONAT_IGD_").from_env()?;
|
||||||
|
|
||||||
RuntimeConfig::new(Self {
|
RuntimeConfig::new(Self {
|
||||||
base: base,
|
consul,
|
||||||
consul: consul,
|
acme,
|
||||||
acme: acme,
|
firewall,
|
||||||
|
igd,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -63,14 +89,16 @@ impl ConfigOpts {
|
||||||
#[allow(dead_code)]
|
#[allow(dead_code)]
|
||||||
pub fn from_iter<Iter: Clone>(iter: Iter) -> Result<RuntimeConfig>
|
pub fn from_iter<Iter: Clone>(iter: Iter) -> Result<RuntimeConfig>
|
||||||
where Iter: IntoIterator<Item = (String, String)> {
|
where Iter: IntoIterator<Item = (String, String)> {
|
||||||
let base: ConfigOptsBase = envy::prefixed("DIPLONAT_").from_iter(iter.clone())?;
|
|
||||||
let consul: ConfigOptsConsul = envy::prefixed("DIPLONAT_CONSUL_").from_iter(iter.clone())?;
|
let consul: ConfigOptsConsul = envy::prefixed("DIPLONAT_CONSUL_").from_iter(iter.clone())?;
|
||||||
let acme: ConfigOptsAcme = envy::prefixed("DIPLONAT_ACME_").from_iter(iter.clone())?;
|
let acme: ConfigOptsAcme = envy::prefixed("DIPLONAT_ACME_").from_iter(iter.clone())?;
|
||||||
|
let firewall: ConfigOptsFirewall = envy::prefixed("DIPLONAT_FIREWALL_").from_iter(iter.clone())?;
|
||||||
|
let igd: ConfigOptsIgd = envy::prefixed("DIPLONAT_IGD_").from_iter(iter.clone())?;
|
||||||
|
|
||||||
RuntimeConfig::new(Self {
|
RuntimeConfig::new(Self {
|
||||||
base: base,
|
consul,
|
||||||
consul: consul,
|
acme,
|
||||||
acme: acme,
|
firewall,
|
||||||
|
igd,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
|
@ -11,35 +11,44 @@ use crate::config::*;
|
||||||
|
|
||||||
fn minimal_valid_options() -> HashMap<String, String> {
|
fn minimal_valid_options() -> HashMap<String, String> {
|
||||||
let mut opts = HashMap::new();
|
let mut opts = HashMap::new();
|
||||||
opts.insert("DIPLONAT_PRIVATE_IP".to_string(), "172.123.43.555".to_string());
|
|
||||||
opts.insert("DIPLONAT_CONSUL_NODE_NAME".to_string(), "consul_node".to_string());
|
opts.insert("DIPLONAT_CONSUL_NODE_NAME".to_string(), "consul_node".to_string());
|
||||||
opts
|
opts
|
||||||
}
|
}
|
||||||
|
|
||||||
fn all_valid_options() -> HashMap<String, String> {
|
fn all_valid_options() -> HashMap<String, String> {
|
||||||
let mut opts = minimal_valid_options();
|
let mut opts = minimal_valid_options();
|
||||||
opts.insert("DIPLONAT_EXPIRATION_TIME".to_string(), "30".to_string());
|
|
||||||
opts.insert("DIPLONAT_REFRESH_TIME".to_string(), "10".to_string());
|
|
||||||
opts.insert("DIPLONAT_CONSUL_URL".to_string(), "http://127.0.0.1:9999".to_string());
|
opts.insert("DIPLONAT_CONSUL_URL".to_string(), "http://127.0.0.1:9999".to_string());
|
||||||
opts.insert("DIPLONAT_ACME_ENABLE".to_string(), "true".to_string());
|
opts.insert("DIPLONAT_ACME_ENABLE".to_string(), "true".to_string());
|
||||||
opts.insert("DIPLONAT_ACME_EMAIL".to_string(), "bozo@bozo.net".to_string());
|
opts.insert("DIPLONAT_ACME_EMAIL".to_string(), "bozo@bozo.net".to_string());
|
||||||
|
opts.insert("DIPLONAT_FIREWALL_ENABLE".to_string(), "true".to_string());
|
||||||
|
opts.insert("DIPLONAT_FIREWALL_REFRESH_TIME".to_string(), "20".to_string());
|
||||||
|
opts.insert("DIPLONAT_IGD_ENABLE".to_string(), "true".to_string());
|
||||||
|
opts.insert("DIPLONAT_IGD_PRIVATE_IP".to_string(), "172.123.43.555".to_string());
|
||||||
|
opts.insert("DIPLONAT_IGD_EXPIRATION_TIME".to_string(), "60".to_string());
|
||||||
|
opts.insert("DIPLONAT_IGD_REFRESH_TIME".to_string(), "10".to_string());
|
||||||
opts
|
opts
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// #[test]
|
||||||
|
// #[should_panic]
|
||||||
|
// fn err_empty_env() {
|
||||||
|
// std::env::remove_var("DIPLONAT_CONSUL_NODE_NAME");
|
||||||
|
// ConfigOpts::from_env().unwrap();
|
||||||
|
// }
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
#[should_panic]
|
#[should_panic]
|
||||||
fn err_empty_env() {
|
fn err_empty_env() {
|
||||||
std::env::remove_var("DIPLONAT_PRIVATE_IP");
|
|
||||||
std::env::remove_var("DIPLONAT_CONSUL_NODE_NAME");
|
std::env::remove_var("DIPLONAT_CONSUL_NODE_NAME");
|
||||||
ConfigOpts::from_env().unwrap();
|
let opts: HashMap<String, String> = HashMap::new();
|
||||||
|
ConfigOpts::from_iter(opts).unwrap();
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn ok_from_iter_minimal_valid_options() {
|
fn ok_minimal_valid_options() {
|
||||||
let opts = minimal_valid_options();
|
let opts = minimal_valid_options();
|
||||||
let rt_config = ConfigOpts::from_iter(opts.clone()).unwrap();
|
let rt_config = ConfigOpts::from_iter(opts.clone()).unwrap();
|
||||||
|
|
||||||
assert!(rt_config.acme.is_none());
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
&rt_config.consul.node_name,
|
&rt_config.consul.node_name,
|
||||||
opts.get(&"DIPLONAT_CONSUL_NODE_NAME".to_string()).unwrap()
|
opts.get(&"DIPLONAT_CONSUL_NODE_NAME".to_string()).unwrap()
|
||||||
|
@ -48,7 +57,10 @@ fn ok_from_iter_minimal_valid_options() {
|
||||||
rt_config.consul.url,
|
rt_config.consul.url,
|
||||||
CONSUL_URL.to_string()
|
CONSUL_URL.to_string()
|
||||||
);
|
);
|
||||||
assert_eq!(
|
assert!(rt_config.acme.is_none());
|
||||||
|
assert!(rt_config.firewall.is_none());
|
||||||
|
assert!(rt_config.igd.is_none());
|
||||||
|
/*assert_eq!(
|
||||||
rt_config.firewall.refresh_time,
|
rt_config.firewall.refresh_time,
|
||||||
Duration::from_secs(REFRESH_TIME.into())
|
Duration::from_secs(REFRESH_TIME.into())
|
||||||
);
|
);
|
||||||
|
@ -63,36 +75,37 @@ fn ok_from_iter_minimal_valid_options() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
rt_config.igd.refresh_time,
|
rt_config.igd.refresh_time,
|
||||||
Duration::from_secs(REFRESH_TIME.into())
|
Duration::from_secs(REFRESH_TIME.into())
|
||||||
);
|
);*/
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
#[should_panic]
|
#[should_panic]
|
||||||
fn err_from_iter_invalid_refresh_time() {
|
fn err_invalid_igd_options() {
|
||||||
let mut opts = minimal_valid_options();
|
let mut opts = minimal_valid_options();
|
||||||
opts.insert("DIPLONAT_EXPIRATION_TIME".to_string(), "60".to_string());
|
opts.insert("DIPLONAT_IGD_ENABLE".to_string(), "true".to_string());
|
||||||
opts.insert("DIPLONAT_REFRESH_TIME".to_string(), "60".to_string());
|
opts.insert("DIPLONAT_IGD_EXPIRATION_TIME".to_string(), "60".to_string());
|
||||||
|
opts.insert("DIPLONAT_IGD_REFRESH_TIME".to_string(), "60".to_string());
|
||||||
ConfigOpts::from_iter(opts).unwrap();
|
ConfigOpts::from_iter(opts).unwrap();
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn ok_from_iter_all_valid_options() {
|
fn ok_all_valid_options() {
|
||||||
let opts = all_valid_options();
|
let opts = all_valid_options();
|
||||||
let rt_config = ConfigOpts::from_iter(opts.clone()).unwrap();
|
let rt_config = ConfigOpts::from_iter(opts.clone()).unwrap();
|
||||||
|
|
||||||
let expiration_time = Duration::from_secs(
|
let firewall_refresh_time = Duration::from_secs(
|
||||||
opts.get(&"DIPLONAT_EXPIRATION_TIME".to_string()).unwrap()
|
opts.get(&"DIPLONAT_FIREWALL_REFRESH_TIME".to_string()).unwrap()
|
||||||
.parse::<u64>().unwrap()
|
.parse::<u64>().unwrap()
|
||||||
.into());
|
.into());
|
||||||
let refresh_time = Duration::from_secs(
|
let igd_expiration_time = Duration::from_secs(
|
||||||
opts.get(&"DIPLONAT_REFRESH_TIME".to_string()).unwrap()
|
opts.get(&"DIPLONAT_IGD_EXPIRATION_TIME".to_string()).unwrap()
|
||||||
|
.parse::<u64>().unwrap()
|
||||||
|
.into());
|
||||||
|
let igd_refresh_time = Duration::from_secs(
|
||||||
|
opts.get(&"DIPLONAT_IGD_REFRESH_TIME".to_string()).unwrap()
|
||||||
.parse::<u64>().unwrap()
|
.parse::<u64>().unwrap()
|
||||||
.into());
|
.into());
|
||||||
|
|
||||||
assert!(rt_config.acme.is_some());
|
|
||||||
assert_eq!(
|
|
||||||
&rt_config.acme.unwrap().email,
|
|
||||||
opts.get(&"DIPLONAT_ACME_EMAIL".to_string()).unwrap());
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
&rt_config.consul.node_name,
|
&rt_config.consul.node_name,
|
||||||
opts.get(&"DIPLONAT_CONSUL_NODE_NAME".to_string()).unwrap()
|
opts.get(&"DIPLONAT_CONSUL_NODE_NAME".to_string()).unwrap()
|
||||||
|
@ -101,20 +114,32 @@ fn ok_from_iter_all_valid_options() {
|
||||||
&rt_config.consul.url,
|
&rt_config.consul.url,
|
||||||
opts.get(&"DIPLONAT_CONSUL_URL".to_string()).unwrap()
|
opts.get(&"DIPLONAT_CONSUL_URL".to_string()).unwrap()
|
||||||
);
|
);
|
||||||
|
|
||||||
|
assert!(rt_config.acme.is_some());
|
||||||
|
let acme = rt_config.acme.unwrap();
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
rt_config.firewall.refresh_time,
|
&acme.email,
|
||||||
refresh_time
|
opts.get(&"DIPLONAT_ACME_EMAIL".to_string()).unwrap());
|
||||||
|
|
||||||
|
assert!(rt_config.firewall.is_some());
|
||||||
|
let firewall = rt_config.firewall.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
firewall.refresh_time,
|
||||||
|
firewall_refresh_time
|
||||||
|
);
|
||||||
|
|
||||||
|
assert!(rt_config.igd.is_some());
|
||||||
|
let igd = rt_config.igd.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
&igd.private_ip,
|
||||||
|
opts.get(&"DIPLONAT_IGD_PRIVATE_IP".to_string()).unwrap()
|
||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
&rt_config.igd.private_ip,
|
igd.expiration_time,
|
||||||
opts.get(&"DIPLONAT_PRIVATE_IP".to_string()).unwrap()
|
igd_expiration_time
|
||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
rt_config.igd.expiration_time,
|
igd.refresh_time,
|
||||||
expiration_time
|
igd_refresh_time
|
||||||
);
|
|
||||||
assert_eq!(
|
|
||||||
rt_config.igd.refresh_time,
|
|
||||||
refresh_time
|
|
||||||
);
|
);
|
||||||
}
|
}
|
|
@ -2,17 +2,14 @@ use std::time::Duration;
|
||||||
|
|
||||||
use anyhow::{Result, anyhow};
|
use anyhow::{Result, anyhow};
|
||||||
|
|
||||||
use crate::config::{ConfigOpts, ConfigOptsAcme, ConfigOptsBase, ConfigOptsConsul};
|
use crate::config::{ConfigOpts, ConfigOptsConsul, ConfigOptsAcme, ConfigOptsFirewall, ConfigOptsIgd};
|
||||||
|
|
||||||
// This code is inspired by the Trunk crate (https://github.com/thedodd/trunk)
|
// This code is inspired by the Trunk crate (https://github.com/thedodd/trunk)
|
||||||
|
|
||||||
// In this file, we take ConfigOpts and transform them into ready-to-use RuntimeConfig.
|
// In this file, we take ConfigOpts and transform them into ready-to-use RuntimeConfig.
|
||||||
// We apply default values and business logic.
|
// We apply default values and business logic.
|
||||||
|
|
||||||
#[derive(Debug)]
|
// Consul config is mandatory, all the others are optional.
|
||||||
pub struct RuntimeConfigAcme {
|
|
||||||
pub email: String,
|
|
||||||
}
|
|
||||||
|
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
pub struct RuntimeConfigConsul {
|
pub struct RuntimeConfigConsul {
|
||||||
|
@ -20,6 +17,11 @@ pub struct RuntimeConfigConsul {
|
||||||
pub url: String,
|
pub url: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Debug)]
|
||||||
|
pub struct RuntimeConfigAcme {
|
||||||
|
pub email: String,
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
pub struct RuntimeConfigFirewall {
|
pub struct RuntimeConfigFirewall {
|
||||||
pub refresh_time: Duration,
|
pub refresh_time: Duration,
|
||||||
|
@ -34,18 +36,18 @@ pub struct RuntimeConfigIgd {
|
||||||
|
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
pub struct RuntimeConfig {
|
pub struct RuntimeConfig {
|
||||||
pub acme: Option<RuntimeConfigAcme>,
|
|
||||||
pub consul: RuntimeConfigConsul,
|
pub consul: RuntimeConfigConsul,
|
||||||
pub firewall: RuntimeConfigFirewall,
|
pub acme: Option<RuntimeConfigAcme>,
|
||||||
pub igd: RuntimeConfigIgd,
|
pub firewall: Option<RuntimeConfigFirewall>,
|
||||||
|
pub igd: Option<RuntimeConfigIgd>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl RuntimeConfig {
|
impl RuntimeConfig {
|
||||||
pub fn new(opts: ConfigOpts) -> Result<Self> {
|
pub fn new(opts: ConfigOpts) -> Result<Self> {
|
||||||
let acme = RuntimeConfigAcme::new(opts.acme.clone())?;
|
|
||||||
let consul = RuntimeConfigConsul::new(opts.consul.clone())?;
|
let consul = RuntimeConfigConsul::new(opts.consul.clone())?;
|
||||||
let firewall = RuntimeConfigFirewall::new(opts.base.clone())?;
|
let acme = RuntimeConfigAcme::new(opts.acme.clone())?;
|
||||||
let igd = RuntimeConfigIgd::new(opts.base.clone())?;
|
let firewall = RuntimeConfigFirewall::new(opts.firewall.clone())?;
|
||||||
|
let igd = RuntimeConfigIgd::new(opts.igd.clone())?;
|
||||||
|
|||||||
|
|
||||||
Ok(Self {
|
Ok(Self {
|
||||||
acme,
|
acme,
|
||||||
|
@ -56,6 +58,19 @@ impl RuntimeConfig {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl RuntimeConfigConsul {
|
||||||
|
pub(super) fn new(opts: ConfigOptsConsul) -> Result<Self> {
|
||||||
|
let node_name = opts.node_name.expect(
|
||||||
|
"'DIPLONAT_CONSUL_NODE_NAME' is required");
|
||||||
|
let url = opts.url.unwrap_or(super::CONSUL_URL.to_string());
|
||||||
|
|
||||||
|
Ok(Self {
|
||||||
quentin
commented
cf my following comment on cf my following comment on `DIPLONAT_ACME_EMAIL`
|
|||||||
|
node_name,
|
||||||
|
url,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
impl RuntimeConfigAcme {
|
impl RuntimeConfigAcme {
|
||||||
pub fn new(opts: ConfigOptsAcme) -> Result<Option<Self>> {
|
pub fn new(opts: ConfigOptsAcme) -> Result<Option<Self>> {
|
||||||
if !opts.enable {
|
if !opts.enable {
|
||||||
|
@ -63,8 +78,7 @@ impl RuntimeConfigAcme {
|
||||||
}
|
}
|
||||||
|
|
||||||
let email = opts.email.expect(
|
let email = opts.email.expect(
|
||||||
"'DIPLONAT_ACME_EMAIL' environment variable is required \
|
"'DIPLONAT_ACME_EMAIL' is required if ACME is enabled");
|
||||||
if 'DIPLONAT_ACME_ENABLE' == 'true'");
|
|
||||||
|
|
||||||
quentin
commented
I know this is not linked to this specific pull request but when your function returns a Result, there is no reason to panic with expect. Instead, replace it with:
Please do it for the whole file :) I know this is not linked to this specific pull request but when your function returns a Result, there is no reason to panic with expect.
Instead, replace it with:
```rust
let email = opts
.email
.context("'DIPLONAT_ACME_EMAIL' is required if ACME is enabled")?;
```
Please do it for the whole file :)
adrien
commented
There's sense in doing so: when a parameter is invalid (like here), the method should panic or return an Err(..). I'm still modifying the calls to return errors instead of panicking straight away. Using `.context(...)` is not a method of `Option`.
There's sense in doing so: when a parameter is invalid (like here), the method should panic or return an Err(..).
I'm still modifying the calls to return errors instead of panicking straight away. Using `match` for better clarity ;)
quentin
commented
You can convert an Option to an Error with But we need an Error to map to the None. It is a bit challenging as it will be our first error in Diplonat, previously we were relaying only on errors generated by our dependencies. For now, let's keep it simple, our error library provides us a simple macro to create an error from a string. Just a simple example:
So you can write:
For your information, You made the following point:
First, you're right, when a parameter is invalid, we must handle this as an error. It was what I was suggesting to you in my first comment but I thought the return type of Second, we are writing Rust and we are in a library: in this situation, it is not acceptable to panic. As there is no clean way to recover from a panic, we terminate the program from a library without giving the program a chance to recover from the error. Even if we are not a "standalone library" and are writing the main program, we might change our mind the future. We should Third, I am not sure we discussed it but
So in fact, what I suggest is that we return from the function with an Error if this parameter has not be provided. You can convert an Option to an Error with `ok_or`: https://doc.rust-lang.org/std/option/enum.Option.html#method.ok_or
But we need an Error to map to the None. It is a bit challenging as it will be our first error in Diplonat, previously we were relaying only on errors generated by our dependencies.
For now, let's keep it simple, our error library provides us a simple macro to create an error from a string. Just a simple example:
```rust
return Err(anyhow!("Missing attribute: {}", missing))
```
So you can write:
```rust
let email = opts
.email
.ok_or(anyhow!("'DIPLONAT_ACME_EMAIL' is required if ACME is enabled"))?;
```
For your information, `context` allows to enrich an existing error with some context. Indeed, here it is not appropriate as we have no error at all. But in the case where your library throw a cryptic error, you can enrich this error with some context in your program and your users will thank you many times later.
You made the following point:
> There's sense in doing so: when a parameter is invalid (like here), the method should panic or return an Err(..).
First, you're right, when a parameter is invalid, we must handle this as an error. It was what I was suggesting to you in my first comment but I thought the return type of `opts.email` would be a `Result` and not an `Option`.
Second, we are writing Rust and we are in a library: in this situation, it is not acceptable to panic. As there is no clean way to recover from a panic, we terminate the program from a library without giving the program a chance to recover from the error. Even if we are not a "standalone library" and are writing the main program, we might change our mind the future. We should `panic!` only when there is no option. To illustrate my point, we could think that later, we would allow our main program to reload its configuration and retry the initialization if one actor fails.
Third, I am not sure we discussed it but `?` is syntaxic sugar for:
```rust
let err = anyhow!("'DIPLONAT_ACME_EMAIL' is required if ACME is enabled")
match opts.email.ok_or(err) {
Ok(email) => {
// our logic
}
Err(e) => return Err(e)
}
```
So in fact, what I suggest is that we return from the function with an Error if this parameter has not be provided.
adrien
commented
Here's my new RuntimeConfigAcme implementation in full (in commit
We do return an error instead of panicking. But I may have overdone it with using match, but I didn't find any nice shorthand in PS: Quite puzzled by the fact that Rust allows putting a Here's my new RuntimeConfigAcme implementation in full (in commit `f5ac36e`). Do you find it better?
```rust
impl RuntimeConfigAcme {
pub fn new(opts: ConfigOptsAcme) -> Result<Option<Self>> {
if !opts.enable {
return Ok(None)
};
let email = match opts.email {
Some(email) => email,
_ => {
return Err(anyhow!(
"'DIPLONAT_ACME_EMAIL' is required if ACME is enabled"
))
}
};
Ok(Some(Self { email }))
}
}
```
We do return an error instead of panicking. But I may have overdone it with using match, but I didn't find any nice shorthand in `Option` methods (I checked `ok_or`). At least the intent is clear!
PS: Quite puzzled by the fact that Rust allows putting a `return` inside a `match` arm, which should be an expression returning a String (in this case). Welp, gotta get used to it!
quentin
commented
Sorry, I was not clear enough >< About handling error, I want to avoid But So, I don't think you should replace your I have written a small example on how to use See the code without leaving gitea
So, you may ask why I was advocating for To sum up, I think we can define the following rules:
I hope that this time I will be more clear. Sorry, I was not clear enough ><
About handling error, I want to avoid `expect` and `unwrap` because they make our program panic, ie. the program is killed.
But `unwrap_or` is totally fine as it does not make the program panic.
And when the case is simple enough we can use one of these keywords: `unwrap_or` on `Result` or `ok_or` on Options instead of a `match`, it is totally ok.
So, I don't think you should replace your `unwrap_or` with a `match` in `f5ac36e`.
I have written a small example on how to use `ok_or` to convert an `Option` into a `Result` and checked that it compiles here: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=5bc6046485bab0cd4ae694b94d2ba1db
<details>
<summary>See the code without leaving gitea</summary>
```rust
use std::{
fmt,
error::Error
};
#[derive(Debug)]
struct RequiredParameterError;
impl Error for RequiredParameterError {}
impl fmt::Display for RequiredParameterError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "A required parameter is not defined")
}
}
fn foo(param: Option<String>) -> Result<String, RequiredParameterError> {
let email = param.ok_or(RequiredParameterError{})?;
return Ok("maito:".to_string() + &email)
}
fn main() {
let user_param1 = Some("hello@example.org".to_string());
let user_param2 = None;
assert!(foo(user_param1).is_ok());
assert!(foo(user_param2).is_err());
}
```
</details>
<br/>
So, you may ask why I was advocating for `match`?
Because in `fw_actors.rs` you were using again `unwrap()` that can cause a panic in the program and we can't replace it by these simple keywords that are `ok_or`, `unwrap_or`, etc.
To sum up, I think we can define the following rules:
1. Never write `unwrap()` or `expect()` (yes, never is too brutal, but it should be an exception)
2. Replace them, when possible, by an appropriate small function if it exists
3. Otherwise, if your case is too specific, use a match
I hope that this time I will be more clear.
I know that when we start coding together, it takes some times to align our minds but once it will be done, it will be easier :)
|
|||||||
Ok(Some(Self {
|
Ok(Some(Self {
|
||||||
email,
|
email,
|
||||||
|
@ -72,34 +86,29 @@ impl RuntimeConfigAcme {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl RuntimeConfigConsul {
|
|
||||||
pub(super) fn new(opts: ConfigOptsConsul) -> Result<Self> {
|
|
||||||
let node_name = opts.node_name.expect(
|
|
||||||
"'DIPLONAT_CONSUL_NODE_NAME' environment variable is required");
|
|
||||||
let url = opts.url.unwrap_or(super::CONSUL_URL.to_string());
|
|
||||||
|
|
||||||
Ok(Self {
|
|
||||||
node_name,
|
|
||||||
url,
|
|
||||||
})
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
impl RuntimeConfigFirewall {
|
impl RuntimeConfigFirewall {
|
||||||
pub(super) fn new(opts: ConfigOptsBase) -> Result<Self> {
|
pub(super) fn new(opts: ConfigOptsFirewall) -> Result<Option<Self>> {
|
||||||
|
if !opts.enable {
|
||||||
|
return Ok(None);
|
||||||
|
}
|
||||||
|
|
||||||
let refresh_time = Duration::from_secs(
|
let refresh_time = Duration::from_secs(
|
||||||
opts.refresh_time.unwrap_or(super::REFRESH_TIME).into());
|
opts.refresh_time.unwrap_or(super::REFRESH_TIME).into());
|
||||||
|
|
||||||
Ok(Self {
|
Ok(Some(Self {
|
||||||
refresh_time,
|
refresh_time,
|
||||||
})
|
}))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl RuntimeConfigIgd {
|
impl RuntimeConfigIgd {
|
||||||
pub(super) fn new(opts: ConfigOptsBase) -> Result<Self> {
|
pub(super) fn new(opts: ConfigOptsIgd) -> Result<Option<Self>> {
|
||||||
|
if !opts.enable {
|
||||||
|
return Ok(None);
|
||||||
|
}
|
||||||
|
|
||||||
let private_ip = opts.private_ip.expect(
|
let private_ip = opts.private_ip.expect(
|
||||||
"'DIPLONAT_PRIVATE_IP' environment variable is required");
|
"'DIPLONAT_IGD_PRIVATE_IP' is required if IGD is enabled");
|
||||||
let expiration_time = Duration::from_secs(
|
let expiration_time = Duration::from_secs(
|
||||||
opts.expiration_time.unwrap_or(super::EXPIRATION_TIME).into());
|
opts.expiration_time.unwrap_or(super::EXPIRATION_TIME).into());
|
||||||
let refresh_time = Duration::from_secs(
|
let refresh_time = Duration::from_secs(
|
||||||
|
@ -112,10 +121,10 @@ impl RuntimeConfigIgd {
|
||||||
refresh_time.as_secs()));
|
refresh_time.as_secs()));
|
||||||
}
|
}
|
||||||
|
|
||||||
Ok(Self {
|
Ok(Some(Self {
|
||||||
private_ip,
|
private_ip,
|
||||||
expiration_time,
|
expiration_time,
|
||||||
refresh_time,
|
refresh_time,
|
||||||
})
|
}))
|
||||||
}
|
}
|
||||||
}
|
}
|
|
@ -1,6 +1,7 @@
|
||||||
use serde::{Serialize, Deserialize};
|
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
|
|
||||||
use anyhow::{Result, anyhow};
|
use anyhow::{Result, anyhow};
|
||||||
|
use serde::{Serialize, Deserialize};
|
||||||
|
|
||||||
#[derive(Serialize, Deserialize, Debug)]
|
#[derive(Serialize, Deserialize, Debug)]
|
||||||
pub struct ServiceEntry {
|
pub struct ServiceEntry {
|
||||||
|
|
|
@ -1,14 +1,17 @@
|
||||||
use std::cmp;
|
use std::cmp;
|
||||||
|
use std::collections::HashSet;
|
||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
use log::*;
|
|
||||||
use tokio::sync::watch;
|
|
||||||
use tokio::time::delay_for;
|
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
|
use log::*;
|
||||||
use serde::{Serialize, Deserialize};
|
use serde::{Serialize, Deserialize};
|
||||||
use serde_lexpr::{from_str,error};
|
use serde_lexpr::{from_str,error};
|
||||||
use crate::messages;
|
use tokio::sync::watch;
|
||||||
|
use tokio::time::delay_for;
|
||||||
|
|
||||||
|
use crate::config::RuntimeConfigConsul;
|
||||||
use crate::consul;
|
use crate::consul;
|
||||||
use std::collections::HashSet;
|
use crate::messages;
|
||||||
|
|
||||||
#[derive(Serialize, Deserialize, Debug)]
|
#[derive(Serialize, Deserialize, Debug)]
|
||||||
pub enum DiplonatParameter {
|
pub enum DiplonatParameter {
|
||||||
|
@ -27,6 +30,7 @@ pub struct ConsulActor {
|
||||||
consul: consul::Consul,
|
consul: consul::Consul,
|
||||||
node: String,
|
node: String,
|
||||||
retries: u32,
|
retries: u32,
|
||||||
|
|
||||||
tx_open_ports: watch::Sender<messages::PublicExposedPorts>
|
tx_open_ports: watch::Sender<messages::PublicExposedPorts>
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -72,18 +76,18 @@ fn to_open_ports(params: &Vec<DiplonatConsul>) -> messages::PublicExposedPorts {
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ConsulActor {
|
impl ConsulActor {
|
||||||
pub fn new(url: &str, node: &str) -> Self {
|
pub fn new(config: RuntimeConfigConsul) -> Self {
|
||||||
let (tx, rx) = watch::channel(messages::PublicExposedPorts{
|
let (tx, rx) = watch::channel(messages::PublicExposedPorts{
|
||||||
tcp_ports: HashSet::new(),
|
tcp_ports: HashSet::new(),
|
||||||
udp_ports: HashSet::new()
|
udp_ports: HashSet::new()
|
||||||
});
|
});
|
||||||
|
|
||||||
return Self {
|
return Self {
|
||||||
consul: consul::Consul::new(url),
|
consul: consul::Consul::new(&config.url),
|
||||||
|
node: config.node_name,
|
||||||
|
retries: 0,
|
||||||
rx_open_ports: rx,
|
rx_open_ports: rx,
|
||||||
tx_open_ports: tx,
|
tx_open_ports: tx,
|
||||||
node: node.to_string(),
|
|
||||||
retries: 0,
|
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -1,4 +1,4 @@
|
||||||
use anyhow::Result;
|
use anyhow::{Result, anyhow};
|
||||||
use tokio::try_join;
|
use tokio::try_join;
|
||||||
|
|
||||||
use crate::config::ConfigOpts;
|
use crate::config::ConfigOpts;
|
||||||
|
@ -8,43 +8,60 @@ use crate::igd_actor::IgdActor;
|
||||||
|
|
||||||
pub struct Diplonat {
|
pub struct Diplonat {
|
||||||
consul: ConsulActor,
|
consul: ConsulActor,
|
||||||
firewall: FirewallActor,
|
|
||||||
igd: IgdActor,
|
firewall: Option<FirewallActor>,
|
||||||
|
igd: Option<IgdActor>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Diplonat {
|
impl Diplonat {
|
||||||
pub async fn new() -> Result<Self> {
|
pub async fn new() -> Result<Self> {
|
||||||
let rt_cfg = ConfigOpts::from_env()?;
|
let config = ConfigOpts::from_env()?;
|
||||||
println!("{:#?}", rt_cfg);
|
println!("{:#?}", config);
|
||||||
|
|
||||||
quentin
commented
Now that our actors are modular, I think that this function fails to convey the real meaning of what we want to do. IMHO, I think we want to:
With the current code, I see many problems. ((1)) I have a feeling that we are duplicating logic between the "runtime config" and our actor initialization. Currently, the constructor of the actor is only:
To fix this problem, I see 2 options:
((2)) The current Tip: start by solving problem number 2, it will help you solve problem number 1 Now that our actors are modular, I think that this function fails to convey the real meaning of what we want to do.
IMHO, I think we want to:
1. Instantiate actors chosen by the user
2. Run them
---
With the current code, I see many problems.
((1)) I have a feeling that we are duplicating logic between the "runtime config" and our actor initialization. Currently, the constructor of the actor is only:
- some boilerplate
- a duplicated check found in runtime
To fix this problem, I see 2 options:
- Delete your runtime configuration file and let actors initialize themselves from their constructor
- Remove actors' constructor and instantiate them directly from this function only if they are required
((2)) The current `try_join!` fails to convey the information that we run only the initialized actors. I think that, independently of the choice you made for 1, you should create a vector of initialized actors that you will populate and then run. You will replace `try_join!` by [`try_join_all`](https://docs.rs/futures/0.3.17/futures/future/fn.try_join_all.html)
*Tip: start by solving problem number 2, it will help you solve problem number 1*
|
|||||||
let ca = ConsulActor::new(&rt_cfg.consul.url, &rt_cfg.consul.node_name);
|
let consul_actor = ConsulActor::new(config.consul);
|
||||||
|
|
||||||
let fw = FirewallActor::new(
|
let firewall_actor = FirewallActor::new(
|
||||||
rt_cfg.firewall.refresh_time,
|
config.firewall,
|
||||||
&ca.rx_open_ports
|
&consul_actor.rx_open_ports
|
||||||
).await?;
|
).await?;
|
||||||
|
|
||||||
let ia = IgdActor::new(
|
let igd_actor = IgdActor::new(
|
||||||
&rt_cfg.igd.private_ip,
|
config.igd,
|
||||||
rt_cfg.igd.refresh_time,
|
&consul_actor.rx_open_ports
|
||||||
rt_cfg.igd.expiration_time,
|
|
||||||
&ca.rx_open_ports
|
|
||||||
).await?;
|
).await?;
|
||||||
|
|
||||||
|
if firewall_actor.is_none() && igd_actor.is_none() {
|
||||||
|
return Err(anyhow!(
|
||||||
|
"At least enable *one* module, otherwise it's boring!"));
|
||||||
|
}
|
||||||
|
|
||||||
let ctx = Self {
|
let ctx = Self {
|
||||||
consul: ca,
|
consul: consul_actor,
|
||||||
igd: ia,
|
firewall: firewall_actor,
|
||||||
firewall: fw
|
igd: igd_actor,
|
||||||
};
|
};
|
||||||
|
|
||||||
return Ok(ctx);
|
return Ok(ctx);
|
||||||
}
|
}
|
||||||
|
|
||||||
pub async fn listen(&mut self) -> Result<()> {
|
pub async fn listen(&mut self) -> Result<()> {
|
||||||
quentin
commented
This is not very idiomatic, please refer to my previous comment to know how to improve it. This is not very idiomatic, please refer to my previous comment to know how to improve it.
adrien
commented
I died. I won't touch that, I don't have a good enough understanding of Rust and this goes deep into ownership, lifetimes and (async) traits. I think we won't go far on our actor model without writing some minimal actor library, or using Actix. I did try to write a simple generic Actor trait, but did not succeed once. Send help :O I died. I won't touch that, I don't have a good enough understanding of Rust and this goes deep into ownership, lifetimes and (async) traits.
I think we won't go far on our actor model without writing some minimal actor library, or using [Actix](https://docs.rs/actix/0.12.0/actix/index.html). I did try to write a simple generic Actor trait, but did not succeed once.
Send help :O
|
|||||||
|
let firewall = &mut self.firewall;
|
||||||
|
let igd = &mut self.igd;
|
||||||
|
|
||||||
try_join!(
|
try_join!(
|
||||||
self.consul.listen(),
|
self.consul.listen(),
|
||||||
self.igd.listen(),
|
async {
|
||||||
self.firewall.listen()
|
match firewall {
|
||||||
|
Some(x) => x.listen().await,
|
||||||
|
None => Ok(())
|
||||||
|
}
|
||||||
|
},
|
||||||
|
async {
|
||||||
|
match igd {
|
||||||
|
Some(x) => x.listen().await,
|
||||||
|
None => Ok(())
|
||||||
|
}
|
||||||
|
},
|
||||||
)?;
|
)?;
|
||||||
|
|
||||||
return Ok(());
|
return Ok(());
|
||||||
|
|
|
@ -1,9 +1,11 @@
|
||||||
use iptables;
|
|
||||||
use regex::Regex;
|
|
||||||
use std::collections::HashSet;
|
use std::collections::HashSet;
|
||||||
use crate::messages;
|
|
||||||
use anyhow::{Result,Context};
|
use anyhow::{Result,Context};
|
||||||
|
use iptables;
|
||||||
use log::*;
|
use log::*;
|
||||||
|
use regex::Regex;
|
||||||
|
|
||||||
|
use crate::messages;
|
||||||
|
|
||||||
pub fn setup(ipt: &iptables::IPTables) -> Result<()> {
|
pub fn setup(ipt: &iptables::IPTables) -> Result<()> {
|
||||||
|
|
||||||
|
|
|
@ -1,37 +1,47 @@
|
||||||
|
use std::collections::HashSet;
|
||||||
|
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
|
use iptables;
|
||||||
|
use log::*;
|
||||||
use tokio::{
|
use tokio::{
|
||||||
select,
|
select,
|
||||||
sync::watch,
|
sync::watch,
|
||||||
time::{
|
time::{
|
||||||
|
Duration,
|
||||||
self,
|
self,
|
||||||
Duration
|
|
||||||
}};
|
}};
|
||||||
use log::*;
|
|
||||||
|
|
||||||
use iptables;
|
use crate::config::RuntimeConfigFirewall;
|
||||||
use crate::messages;
|
|
||||||
use crate::fw;
|
use crate::fw;
|
||||||
use std::collections::HashSet;
|
use crate::messages;
|
||||||
|
|
||||||
|
|
||||||
pub struct FirewallActor {
|
pub struct FirewallActor {
|
||||||
pub ipt: iptables::IPTables,
|
pub ipt: iptables::IPTables,
|
||||||
rx_ports: watch::Receiver<messages::PublicExposedPorts>,
|
|
||||||
last_ports: messages::PublicExposedPorts,
|
last_ports: messages::PublicExposedPorts,
|
||||||
refresh: Duration
|
refresh: Duration,
|
||||||
|
|
||||||
|
rx_ports: watch::Receiver<messages::PublicExposedPorts>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl FirewallActor {
|
impl FirewallActor {
|
||||||
pub async fn new(_refresh: Duration, rxp: &watch::Receiver<messages::PublicExposedPorts>) -> Result<Self> {
|
pub async fn new(config: Option<RuntimeConfigFirewall>, rxp: &watch::Receiver<messages::PublicExposedPorts>) -> Result<Option<Self>> {
|
||||||
|
if config.is_none() {
|
||||||
|
return Ok(None);
|
||||||
quentin
commented
No. Never use unwrap. Your function should be written with a match:
As a general rule, in many places in your code, you could better communicate your intent with a No. Never use unwrap.
Your function should be written with a match:
```rust
impl FirewallActor {
pub async fn new(
_refresh: Duration,
config: Option<RuntimeConfigFirewall>,
rxp: &watch::Receiver<messages::PublicExposedPorts>,
) -> Result<Option<Self>> {
match config {
None => Ok(None)
Some(c) => {
let ctx = Self { ... }
fw::setup(&ctx.ipt)?;
return Ok(Some(ctx))
}
}
}
```
As a general rule, in many places in your code, you could better communicate your intent with a `match`
|
|||||||
|
}
|
||||||
|
let config = config.unwrap();
|
||||||
|
|
||||||
let ctx = Self {
|
let ctx = Self {
|
||||||
ipt: iptables::new(false)?,
|
ipt: iptables::new(false)?,
|
||||||
rx_ports: rxp.clone(),
|
|
||||||
last_ports: messages::PublicExposedPorts::new(),
|
last_ports: messages::PublicExposedPorts::new(),
|
||||||
refresh: _refresh,
|
refresh: config.refresh_time,
|
||||||
|
rx_ports: rxp.clone(),
|
||||||
};
|
};
|
||||||
|
|
||||||
fw::setup(&ctx.ipt)?;
|
fw::setup(&ctx.ipt)?;
|
||||||
|
|
||||||
return Ok(ctx);
|
return Ok(Some(ctx));
|
||||||
}
|
}
|
||||||
|
|
||||||
pub async fn listen(&mut self) -> Result<()> {
|
pub async fn listen(&mut self) -> Result<()> {
|
||||||
|
|
|
@ -1,43 +1,52 @@
|
||||||
|
use std::net::SocketAddrV4;
|
||||||
|
|
||||||
|
use anyhow::{Result, Context};
|
||||||
use igd::aio::*;
|
use igd::aio::*;
|
||||||
use igd::PortMappingProtocol;
|
use igd::PortMappingProtocol;
|
||||||
use std::net::SocketAddrV4;
|
|
||||||
use log::*;
|
use log::*;
|
||||||
use anyhow::{Result, Context};
|
|
||||||
use tokio::{
|
use tokio::{
|
||||||
select,
|
select,
|
||||||
sync::watch,
|
sync::watch,
|
||||||
time::{
|
time::{
|
||||||
|
Duration,
|
||||||
self,
|
self,
|
||||||
Duration
|
|
||||||
}};
|
}};
|
||||||
|
|
||||||
|
use crate::config::RuntimeConfigIgd;
|
||||||
use crate::messages;
|
use crate::messages;
|
||||||
|
|
||||||
pub struct IgdActor {
|
pub struct IgdActor {
|
||||||
last_ports: messages::PublicExposedPorts,
|
|
||||||
rx_ports: watch::Receiver<messages::PublicExposedPorts>,
|
|
||||||
gateway: Gateway,
|
|
||||||
refresh: Duration,
|
|
||||||
expire: Duration,
|
expire: Duration,
|
||||||
private_ip: String
|
gateway: Gateway,
|
||||||
|
last_ports: messages::PublicExposedPorts,
|
||||||
|
private_ip: String,
|
||||||
|
refresh: Duration,
|
||||||
|
|
||||||
|
rx_ports: watch::Receiver<messages::PublicExposedPorts>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl IgdActor {
|
impl IgdActor {
|
||||||
pub async fn new(priv_ip: &str, refresh: Duration, expire: Duration, rxp: &watch::Receiver<messages::PublicExposedPorts>) -> Result<Self> {
|
pub async fn new(config: Option<RuntimeConfigIgd>, rxp: &watch::Receiver<messages::PublicExposedPorts>) -> Result<Option<Self>> {
|
||||||
quentin
commented
Same as my previous comment Same as my previous comment
|
|||||||
|
if config.is_none() {
|
||||||
|
return Ok(None);
|
||||||
|
}
|
||||||
|
let config = config.unwrap();
|
||||||
|
|
||||||
let gw = search_gateway(Default::default())
|
let gw = search_gateway(Default::default())
|
||||||
.await
|
.await
|
||||||
.context("Failed to find IGD gateway")?;
|
.context("Failed to find IGD gateway")?;
|
||||||
info!("IGD gateway: {}", gw);
|
info!("IGD gateway: {}", gw);
|
||||||
|
|
||||||
let ctx = Self {
|
let ctx = Self {
|
||||||
|
expire: config.expiration_time,
|
||||||
gateway: gw,
|
gateway: gw,
|
||||||
|
last_ports: messages::PublicExposedPorts::new(),
|
||||||
|
private_ip: config.private_ip,
|
||||||
|
refresh: config.refresh_time,
|
||||||
rx_ports: rxp.clone(),
|
rx_ports: rxp.clone(),
|
||||||
private_ip: priv_ip.to_string(),
|
|
||||||
refresh: refresh,
|
|
||||||
expire: expire,
|
|
||||||
last_ports: messages::PublicExposedPorts::new()
|
|
||||||
};
|
};
|
||||||
|
|
||||||
return Ok(ctx);
|
return Ok(Some(ctx));
|
||||||
}
|
}
|
||||||
|
|
||||||
pub async fn listen(&mut self) -> Result<()> {
|
pub async fn listen(&mut self) -> Result<()> {
|
||||||
|
|
Not related to this PR, but do we really need to clone the configuration here, it is strange because I do not think we modify it.