Modular Diplonat (with the option to disable useless modules) #8

Closed
adrien wants to merge 12 commits from adrien/diplonat:feature/modular-config into main
11 changed files with 396 additions and 285 deletions

View file

@ -40,10 +40,13 @@ cargo build
consul agent -dev # in a separate terminal
# adapt following values to your configuration
export DIPLONAT_PRIVATE_IP="192.168.0.18"
export DIPLONAT_REFRESH_TIME="60"
export DIPLONAT_EXPIRATION_TIME="300"
export DIPLONAT_CONSUL_NODE_NAME="lheureduthe"
export DIPLONAT_FIREWALL_ENABLE="true"
export DIPLONAT_FIREWALL_REFRESH_TIME="300"
export DIPLONAT_IGD_ENABLE="true"
export DIPLONAT_IGD_PRIVATE_IP="192.168.0.18"
export DIPLONAT_IGD_REFRESH_TIME="60"
export DIPLONAT_IGD_EXPIRATION_TIME="300"
export RUST_LOG=debug
cargo run
```

View file

@ -5,10 +5,13 @@ services:
image: darkgallium/amd64_diplonat:v2
network_mode: host # required by UPNP/IGD
environment:
DIPLONAT_PRIVATE_IP: 192.168.0.18
DIPLONAT_REFRESH_TIME: 60
DIPLONAT_EXPIRATION_TIME: 300
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

153
src/config/config_test.rs Normal file
View file

@ -0,0 +1,153 @@
use std::{collections::HashMap, time::Duration};
use crate::config::*;
// Environment variables are set for the entire process and
// tests are run whithin the same process.
// => We cannot test ConfigOpts::from_env(),
// because tests modify each other's environment.
// This is why we only test ConfigOpts::from_iter(iter).
fn minimal_valid_options() -> HashMap<String, String> {
let mut opts = HashMap::new();
opts.insert(
"DIPLONAT_CONSUL_NODE_NAME".to_string(),
"consul_node".to_string(),
);
opts
}
fn all_valid_options() -> HashMap<String, String> {
let mut opts = minimal_valid_options();
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_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
}
#[test]
#[should_panic]
fn err_empty_env() {
std::env::remove_var("DIPLONAT_CONSUL_NODE_NAME");
let opts: HashMap<String, String> = HashMap::new();
ConfigOpts::from_iter(opts).unwrap();
}
#[test]
fn ok_minimal_valid_options() {
let opts = minimal_valid_options();
let rt_config = ConfigOpts::from_iter(opts.clone()).unwrap();
assert_eq!(
&rt_config.consul.node_name,
opts.get(&"DIPLONAT_CONSUL_NODE_NAME".to_string()).unwrap()
);
assert_eq!(rt_config.consul.url, CONSUL_URL.to_string());
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,
// Duration::from_secs(REFRESH_TIME.into())
// );
// assert_eq!(
// &rt_config.igd.private_ip,
// opts.get(&"DIPLONAT_PRIVATE_IP".to_string()).unwrap()
// );
// assert_eq!(
// rt_config.igd.expiration_time,
// Duration::from_secs(EXPIRATION_TIME.into())
// );
// assert_eq!(
// rt_config.igd.refresh_time,
// Duration::from_secs(REFRESH_TIME.into())
// );
}
#[test]
#[should_panic]
fn err_invalid_igd_options() {
let mut opts = minimal_valid_options();
opts.insert("DIPLONAT_IGD_ENABLE".to_string(), "true".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();
}
#[test]
fn ok_all_valid_options() {
let opts = all_valid_options();
let rt_config = ConfigOpts::from_iter(opts.clone()).unwrap();
let firewall_refresh_time = Duration::from_secs(
opts
.get(&"DIPLONAT_FIREWALL_REFRESH_TIME".to_string())
.unwrap()
.parse::<u64>()
.unwrap()
.into(),
);
let igd_expiration_time = Duration::from_secs(
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()
.into(),
);
assert_eq!(
&rt_config.consul.node_name,
opts.get(&"DIPLONAT_CONSUL_NODE_NAME".to_string()).unwrap()
);
assert_eq!(
&rt_config.consul.url,
opts.get(&"DIPLONAT_CONSUL_URL".to_string()).unwrap()
);
assert!(rt_config.acme.is_some());
let acme = rt_config.acme.unwrap();
assert_eq!(
&acme.email,
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!(igd.expiration_time, igd_expiration_time);
assert_eq!(igd.refresh_time, igd_refresh_time);
}

View file

@ -1,9 +1,11 @@
mod options;
#[cfg(test)]
mod options_test;
mod config_test;
mod options;
mod runtime;
pub use options::{ConfigOpts, ConfigOptsAcme, ConfigOptsBase, ConfigOptsConsul};
pub use options::{
ConfigOpts, ConfigOptsAcme, ConfigOptsConsul, ConfigOptsFirewall, ConfigOptsIgd,
};
pub use runtime::{
RuntimeConfig, RuntimeConfigAcme, RuntimeConfigConsul, RuntimeConfigFirewall, RuntimeConfigIgd,
};

View file

@ -8,27 +8,12 @@ use crate::config::RuntimeConfig;
// This file parses the options that can be declared in the environment.
// runtime.rs applies business logic and builds RuntimeConfig structs.
/// Base configuration options
#[derive(Clone, Default, Deserialize)]
pub struct ConfigOptsBase {
/// 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 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>,
}
// Note for the future
// There is no *need* to have a 'DIPLONAT_XXX_*' prefix for all config options.
// If some config options are shared by several modules, a ConfigOptsBase could
// contain them, and parse the 'DIPLONAT_*' prefix directly.
// Only in runtime.rs would these options find their proper location in each
// module's struct.
/// Consul configuration options
#[derive(Clone, Default, Deserialize)]
@ -39,23 +24,63 @@ pub struct ConfigOptsConsul {
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
pub struct ConfigOpts {
pub base: ConfigOptsBase,
pub acme: ConfigOptsAcme,
pub consul: ConfigOptsConsul,
pub acme: ConfigOptsAcme,
pub firewall: ConfigOptsFirewall,
pub igd: ConfigOptsIgd,
}
impl ConfigOpts {
pub fn from_env() -> Result<RuntimeConfig> {
let base: ConfigOptsBase = envy::prefixed("DIPLONAT_").from_env()?;
let consul: ConfigOptsConsul = envy::prefixed("DIPLONAT_CONSUL_").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 {
base: base,
consul: consul,
acme: acme,
consul,
acme,
firewall,
igd,
})
}
@ -63,14 +88,17 @@ impl ConfigOpts {
#[allow(dead_code)]
pub fn from_iter<Iter: Clone>(iter: Iter) -> Result<RuntimeConfig>
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 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 {
base: base,
consul: consul,
acme: acme,
consul,
acme,
firewall,
igd,
})
}
}

View file

@ -1,128 +0,0 @@
use std::{collections::HashMap, time::Duration};
use crate::config::*;
// Environment variables are set for the entire process and
// tests are run whithin the same process.
// => We cannot test ConfigOpts::from_env(),
// because tests modify each other's environment.
// This is why we only test ConfigOpts::from_iter(iter).
fn minimal_valid_options() -> HashMap<String, String> {
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
}
fn all_valid_options() -> HashMap<String, String> {
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_ACME_ENABLE".to_string(), "true".to_string());
opts.insert(
"DIPLONAT_ACME_EMAIL".to_string(),
"bozo@bozo.net".to_string(),
);
opts
}
#[test]
#[should_panic]
fn err_empty_env() {
std::env::remove_var("DIPLONAT_PRIVATE_IP");
std::env::remove_var("DIPLONAT_CONSUL_NODE_NAME");
ConfigOpts::from_env().unwrap();
}
#[test]
fn ok_from_iter_minimal_valid_options() {
let opts = minimal_valid_options();
let rt_config = ConfigOpts::from_iter(opts.clone()).unwrap();
assert!(rt_config.acme.is_none());
assert_eq!(
&rt_config.consul.node_name,
opts.get(&"DIPLONAT_CONSUL_NODE_NAME".to_string()).unwrap()
);
assert_eq!(rt_config.consul.url, CONSUL_URL.to_string());
assert_eq!(
rt_config.firewall.refresh_time,
Duration::from_secs(REFRESH_TIME.into())
);
assert_eq!(
&rt_config.igd.private_ip,
opts.get(&"DIPLONAT_PRIVATE_IP".to_string()).unwrap()
);
assert_eq!(
rt_config.igd.expiration_time,
Duration::from_secs(EXPIRATION_TIME.into())
);
assert_eq!(
rt_config.igd.refresh_time,
Duration::from_secs(REFRESH_TIME.into())
);
}
#[test]
#[should_panic]
fn err_from_iter_invalid_refresh_time() {
let mut opts = minimal_valid_options();
opts.insert("DIPLONAT_EXPIRATION_TIME".to_string(), "60".to_string());
opts.insert("DIPLONAT_REFRESH_TIME".to_string(), "60".to_string());
ConfigOpts::from_iter(opts).unwrap();
}
#[test]
fn ok_from_iter_all_valid_options() {
let opts = all_valid_options();
let rt_config = ConfigOpts::from_iter(opts.clone()).unwrap();
let expiration_time = Duration::from_secs(
opts
.get(&"DIPLONAT_EXPIRATION_TIME".to_string())
.unwrap()
.parse::<u64>()
.unwrap()
.into(),
);
let refresh_time = Duration::from_secs(
opts
.get(&"DIPLONAT_REFRESH_TIME".to_string())
.unwrap()
.parse::<u64>()
.unwrap()
.into(),
);
assert!(rt_config.acme.is_some());
assert_eq!(
&rt_config.acme.unwrap().email,
opts.get(&"DIPLONAT_ACME_EMAIL".to_string()).unwrap()
);
assert_eq!(
&rt_config.consul.node_name,
opts.get(&"DIPLONAT_CONSUL_NODE_NAME".to_string()).unwrap()
);
assert_eq!(
&rt_config.consul.url,
opts.get(&"DIPLONAT_CONSUL_URL".to_string()).unwrap()
);
assert_eq!(rt_config.firewall.refresh_time, refresh_time);
assert_eq!(
&rt_config.igd.private_ip,
opts.get(&"DIPLONAT_PRIVATE_IP".to_string()).unwrap()
);
assert_eq!(rt_config.igd.expiration_time, expiration_time);
assert_eq!(rt_config.igd.refresh_time, refresh_time);
}

View file

@ -2,17 +2,16 @@ use std::time::Duration;
use anyhow::{anyhow, Result};
use crate::config::{ConfigOpts, ConfigOptsAcme, ConfigOptsBase, ConfigOptsConsul};
use crate::config::{
ConfigOpts, ConfigOptsAcme, ConfigOptsConsul, ConfigOptsFirewall, ConfigOptsIgd,
};
// 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. We apply default values and business logic.
#[derive(Debug)]
pub struct RuntimeConfigAcme {
pub email: String,
}
// Consul config is mandatory, all the others are optional.
#[derive(Debug)]
pub struct RuntimeConfigConsul {
@ -20,6 +19,11 @@ pub struct RuntimeConfigConsul {
pub url: String,
}
#[derive(Debug)]
pub struct RuntimeConfigAcme {
pub email: String,
}
#[derive(Debug)]
pub struct RuntimeConfigFirewall {
pub refresh_time: Duration,
@ -34,18 +38,18 @@ pub struct RuntimeConfigIgd {
#[derive(Debug)]
pub struct RuntimeConfig {
pub acme: Option<RuntimeConfigAcme>,
pub consul: RuntimeConfigConsul,
pub firewall: RuntimeConfigFirewall,
pub igd: RuntimeConfigIgd,
pub acme: Option<RuntimeConfigAcme>,
pub firewall: Option<RuntimeConfigFirewall>,
pub igd: Option<RuntimeConfigIgd>,
}
impl RuntimeConfig {
pub fn new(opts: ConfigOpts) -> Result<Self> {
let acme = RuntimeConfigAcme::new(opts.acme.clone())?;
let consul = RuntimeConfigConsul::new(opts.consul.clone())?;
let firewall = RuntimeConfigFirewall::new(opts.base.clone())?;
let igd = RuntimeConfigIgd::new(opts.base.clone())?;
let consul = RuntimeConfigConsul::new(opts.consul)?;
let acme = RuntimeConfigAcme::new(opts.acme)?;
let firewall = RuntimeConfigFirewall::new(opts.firewall)?;
let igd = RuntimeConfigIgd::new(opts.igd)?;
Ok(Self {
acme,
@ -56,51 +60,87 @@ impl RuntimeConfig {
}
}
impl RuntimeConfigAcme {
pub fn new(opts: ConfigOptsAcme) -> Result<Option<Self>> {
if !opts.enable {
return Ok(None)
}
let email = opts.email.expect(
"'DIPLONAT_ACME_EMAIL' environment variable is required if 'DIPLONAT_ACME_ENABLE' == 'true'",
);
Ok(Some(Self { email }))
}
}
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());
let node_name = match opts.node_name {
Some(n) => n,
_ => return Err(anyhow!("'DIPLONAT_CONSUL_NODE_NAME' is required")),
};
let url = match opts.url {
Some(url) => url,
_ => super::CONSUL_URL.to_string(),
};
Ok(Self { node_name, url })
}
}
impl RuntimeConfigFirewall {
pub(super) fn new(opts: ConfigOptsBase) -> Result<Self> {
let refresh_time = Duration::from_secs(opts.refresh_time.unwrap_or(super::REFRESH_TIME).into());
impl RuntimeConfigAcme {
pub fn new(opts: ConfigOptsAcme) -> Result<Option<Self>> {
if !opts.enable {
return Ok(None)
};
Ok(Self { refresh_time })
let email = match opts.email {
Some(email) => email,
_ => {
return Err(anyhow!(
"'DIPLONAT_ACME_EMAIL' is required if ACME is enabled"
))
}
};
Ok(Some(Self { email }))
}
}
impl RuntimeConfigFirewall {
pub(super) fn new(opts: ConfigOptsFirewall) -> Result<Option<Self>> {
if !opts.enable {
return Ok(None)
}
let refresh_time = Duration::from_secs(
match opts.refresh_time {
Some(t) => t,
_ => super::REFRESH_TIME,
}
.into(),
);
Ok(Some(Self { refresh_time }))
}
}
impl RuntimeConfigIgd {
pub(super) fn new(opts: ConfigOptsBase) -> Result<Self> {
let private_ip = opts
.private_ip
.expect("'DIPLONAT_PRIVATE_IP' environment variable is required");
pub(super) fn new(opts: ConfigOptsIgd) -> Result<Option<Self>> {
if !opts.enable {
return Ok(None)
}
let private_ip = match opts.private_ip {
Some(ip) => ip,
_ => {
return Err(anyhow!(
"'DIPLONAT_IGD_PRIVATE_IP' is required if IGD is enabled"
))
}
};
let expiration_time = Duration::from_secs(
opts
.expiration_time
.unwrap_or(super::EXPIRATION_TIME)
.into(),
match opts.expiration_time {
Some(t) => t.into(),
_ => super::EXPIRATION_TIME,
}
.into(),
);
let refresh_time = Duration::from_secs(
match opts.refresh_time {
Some(t) => t,
_ => super::REFRESH_TIME,
}
.into(),
);
let refresh_time = Duration::from_secs(opts.refresh_time.unwrap_or(super::REFRESH_TIME).into());
if refresh_time.as_secs() * 2 > expiration_time.as_secs() {
return Err(anyhow!(
@ -111,10 +151,10 @@ impl RuntimeConfigIgd {
))
}
Ok(Self {
Ok(Some(Self {
private_ip,
expiration_time,
refresh_time,
})
}))
}
}

View file

@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize};
use serde_lexpr::{error, from_str};
use tokio::{sync::watch, time::delay_for};
use crate::{consul, messages};
use crate::{config::RuntimeConfigConsul, consul, messages};
#[derive(Serialize, Deserialize, Debug)]
pub enum DiplonatParameter {
@ -25,6 +25,7 @@ pub struct ConsulActor {
consul: consul::Consul,
node: String,
retries: u32,
tx_open_ports: watch::Sender<messages::PublicExposedPorts>,
}
@ -74,18 +75,18 @@ fn to_open_ports(params: &Vec<DiplonatConsul>) -> messages::PublicExposedPorts {
}
impl ConsulActor {
pub fn new(url: &str, node: &str) -> Self {
pub fn new(config: RuntimeConfigConsul) -> Self {
let (tx, rx) = watch::channel(messages::PublicExposedPorts {
tcp_ports: HashSet::new(),
udp_ports: HashSet::new(),
});
return Self {
consul: consul::Consul::new(url),
consul: consul::Consul::new(&config.url),
node: config.node_name,
retries: 0,
rx_open_ports: rx,
tx_open_ports: tx,
node: node.to_string(),
retries: 0,
}
}

View file

@ -1,49 +1,50 @@
use anyhow::Result;
use tokio::try_join;
use std::pin::Pin;
use anyhow::{anyhow, Result};
use futures::future::{try_join_all, Future};
// use tokio::try_join;
type ActorTask = Pin<Box<dyn Future<Output = Result<()>>>>;
use crate::{
config::ConfigOpts, consul_actor::ConsulActor, fw_actor::FirewallActor, igd_actor::IgdActor,
};
pub struct Diplonat {
consul: ConsulActor,
firewall: FirewallActor,
igd: IgdActor,
actors: Vec<ActorTask>,
}
impl Diplonat {
pub async fn new() -> Result<Self> {
let rt_cfg = ConfigOpts::from_env()?;
println!("{:#?}", rt_cfg);
let config = ConfigOpts::from_env()?;
println!("{:#?}", config);
Review

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

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 mut consul_actor = ConsulActor::new(config.consul);
let consul_rx = consul_actor.rx_open_ports.clone();
let actor_task: ActorTask = Box::pin(consul_actor.listen());
let fw = FirewallActor::new(rt_cfg.firewall.refresh_time, &ca.rx_open_ports).await?;
let mut actors = vec![actor_task];
let ia = IgdActor::new(
&rt_cfg.igd.private_ip,
rt_cfg.igd.refresh_time,
rt_cfg.igd.expiration_time,
&ca.rx_open_ports,
)
.await?;
if let Some(mut actor) = FirewallActor::new(config.firewall, &consul_rx)? {
let actor_task: ActorTask = Box::pin(actor.listen());
actors.push(actor_task);
}
if let Some(mut actor) = IgdActor::new(config.igd, &consul_rx).await? {
let actor_task: ActorTask = Box::pin(actor.listen());
actors.push(actor_task);
}
let ctx = Self {
consul: ca,
igd: ia,
firewall: fw,
};
if actors.len() == 1 {
return Err(anyhow!(
"At least enable *one* module, otherwise it's boring!"
))
}
return Ok(ctx)
Ok(Self { actors })
}
pub async fn listen(&mut self) -> Result<()> {
try_join!(
self.consul.listen(),
self.igd.listen(),
self.firewall.listen()
)?;
return Ok(())
pub async fn listen(&self) -> Result<()> {
try_join_all(self.actors);
Ok(())
}
}

View file

@ -9,30 +9,36 @@ use tokio::{
time::{self, Duration},
};
use crate::{fw, messages};
use crate::{config::RuntimeConfigFirewall, fw, messages};
pub struct FirewallActor {
pub ipt: iptables::IPTables,
rx_ports: watch::Receiver<messages::PublicExposedPorts>,
last_ports: messages::PublicExposedPorts,
refresh: Duration,
rx_ports: watch::Receiver<messages::PublicExposedPorts>,
}
impl FirewallActor {
pub async fn new(
_refresh: Duration,
pub fn new(
config: Option<RuntimeConfigFirewall>,
rxp: &watch::Receiver<messages::PublicExposedPorts>,
) -> Result<Self> {
let ctx = Self {
ipt: iptables::new(false)?,
rx_ports: rxp.clone(),
last_ports: messages::PublicExposedPorts::new(),
refresh: _refresh,
};
) -> Result<Option<Self>> {
match config {
None => Ok(None),
Some(c) => {
let ctx = Self {
ipt: iptables::new(false)?,
last_ports: messages::PublicExposedPorts::new(),
refresh: c.refresh_time,
rx_ports: rxp.clone(),
};
fw::setup(&ctx.ipt)?;
fw::setup(&ctx.ipt)?;
return Ok(ctx)
Ok(Some(ctx))
}
}
}
pub async fn listen(&mut self) -> Result<()> {

View file

@ -9,39 +9,41 @@ use tokio::{
time::{self, Duration},
};
use crate::messages;
use crate::{config::RuntimeConfigIgd, messages};
pub struct IgdActor {
last_ports: messages::PublicExposedPorts,
rx_ports: watch::Receiver<messages::PublicExposedPorts>,
gateway: Gateway,
refresh: Duration,
expire: Duration,
gateway: Gateway,
last_ports: messages::PublicExposedPorts,
private_ip: String,
refresh: Duration,
rx_ports: watch::Receiver<messages::PublicExposedPorts>,
}
impl IgdActor {
pub async fn new(
priv_ip: &str,
refresh: Duration,
expire: Duration,
config: Option<RuntimeConfigIgd>,
rxp: &watch::Receiver<messages::PublicExposedPorts>,
) -> Result<Self> {
let gw = search_gateway(Default::default())
.await
.context("Failed to find IGD gateway")?;
info!("IGD gateway: {}", gw);
) -> Result<Option<Self>> {
match config {
None => Ok(None),
Some(c) => {
let gw = search_gateway(Default::default())
.await
.context("Failed to find IGD gateway")?;
info!("IGD gateway: {}", gw);
let ctx = Self {
gateway: gw,
rx_ports: rxp.clone(),
private_ip: priv_ip.to_string(),
refresh: refresh,
expire: expire,
last_ports: messages::PublicExposedPorts::new(),
};
return Ok(ctx)
Ok(Some(Self {
expire: c.expiration_time,
gateway: gw,
last_ports: messages::PublicExposedPorts::new(),
private_ip: c.private_ip,
refresh: c.refresh_time,
rx_ports: rxp.clone(),
}))
}
}
}
pub async fn listen(&mut self) -> Result<()> {