From 13a3507066ddac4ce82ce7f89cd4b9522ed721ce Mon Sep 17 00:00:00 2001 From: ifeanyi Date: Sat, 19 Sep 2020 13:38:46 +0200 Subject: [PATCH] Use builder to create Config So that whenever Config's fields are updated, we don't need to go through the codebase to updating every usage. --- src/config/builder.rs | 44 +++++++++++++ src/config/error.rs | 20 ++++++ src/{config.rs => config/mod.rs} | 109 +++++++++++++------------------ src/extensions/filter_chain.rs | 33 +++++----- src/proxy/server.rs | 85 ++++++++---------------- tests/filters.rs | 58 ++++++++-------- tests/local_rate_limit.rs | 16 ++--- tests/metrics.rs | 25 ++++--- tests/no_filter.rs | 24 ++++--- 9 files changed, 214 insertions(+), 200 deletions(-) create mode 100644 src/config/builder.rs create mode 100644 src/config/error.rs rename src/{config.rs => config/mod.rs} (86%) diff --git a/src/config/builder.rs b/src/config/builder.rs new file mode 100644 index 0000000000..8a1cf8b094 --- /dev/null +++ b/src/config/builder.rs @@ -0,0 +1,44 @@ +use super::{Config, ConnectionConfig, Filter, Local}; + +/// Builder for a [`Config`] +#[derive(Debug)] +pub struct Builder { + pub local: Local, + pub filters: Vec, + pub connections: ConnectionConfig, +} + +impl Builder { + /// Returns a [`Builder`] with empty values. + pub fn empty() -> Self { + Builder { + local: Local { port: 0 }, + filters: vec![], + connections: ConnectionConfig::Server { endpoints: vec![] }, + } + } + + pub fn with_local(self, local: Local) -> Self { + Builder { local, ..self } + } + + pub fn with_filters(self, filters: Vec) -> Self { + Builder { filters, ..self } + } + + pub fn with_connections(self, connections: ConnectionConfig) -> Self { + Builder { + connections, + ..self + } + } + + pub fn build(self) -> Config { + Config { + local: self.local, + filters: self.filters, + connections: self.connections, + phantom: None, + } + } +} diff --git a/src/config/error.rs b/src/config/error.rs new file mode 100644 index 0000000000..3f75b94f93 --- /dev/null +++ b/src/config/error.rs @@ -0,0 +1,20 @@ +use crate::extensions::Error as FilterRegistryError; +use std::fmt::{self, Display, Formatter}; + +/// Validation failure for a Config +#[derive(Debug, PartialEq)] +pub enum ValidationError { + NotUnique(String), + FilterInvalid(FilterRegistryError), +} + +impl Display for ValidationError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + ValidationError::NotUnique(field) => write!(f, "field {} is not unique", field), + ValidationError::FilterInvalid(reason) => { + write!(f, "filter configuration is invalid: {}", reason) + } + } + } +} diff --git a/src/config.rs b/src/config/mod.rs similarity index 86% rename from src/config.rs rename to src/config/mod.rs index 1750968713..a41188a203 100644 --- a/src/config.rs +++ b/src/config/mod.rs @@ -14,35 +14,21 @@ * limitations under the License. */ -use crate::extensions::Error as FilterRegistryError; use std::collections::HashSet; -use std::fmt; use std::io; +use std::marker::PhantomData; use std::net::SocketAddr; use base64_serde::base64_serde_type; -use serde::export::Formatter; use serde::{Deserialize, Serialize}; -base64_serde_type!(Base64Standard, base64::STANDARD); +mod builder; +mod error; -/// Validation failure for a Config -#[derive(Debug, PartialEq)] -pub enum ValidationError { - NotUnique(String), - FilterInvalid(FilterRegistryError), -} +pub use builder::Builder; +pub use error::ValidationError; -impl fmt::Display for ValidationError { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self { - ValidationError::NotUnique(field) => write!(f, "field {} is not unique", field), - ValidationError::FilterInvalid(reason) => { - write!(f, "filter configuration is invalid: {}", reason) - } - } - } -} +base64_serde_type!(Base64Standard, base64::STANDARD); /// Config is the configuration for either a Client or Server proxy #[derive(Debug, Deserialize, Serialize)] @@ -52,6 +38,10 @@ pub struct Config { pub filters: Vec, #[serde(flatten)] pub connections: ConnectionConfig, + + // Limit struct creation to the builder. We use an Optional + // so that we can create instances though deserialization. + pub(super) phantom: Option>, } /// Local is the local host configuration options @@ -180,31 +170,29 @@ mod tests { use serde_yaml::Value; use crate::config::{ - Config, ConnectionConfig, ConnectionId, EndPoint, LoadBalancerPolicy, Local, + Builder, Config, ConnectionConfig, ConnectionId, EndPoint, LoadBalancerPolicy, Local, ValidationError, }; #[test] fn deserialise_client() { - let config = Config { - local: Local { port: 7000 }, - filters: vec![], - connections: ConnectionConfig::Client { + let config = Builder::empty() + .with_local(Local { port: 7000 }) + .with_connections(ConnectionConfig::Client { addresses: vec!["127.0.0.1:25999".parse().unwrap()], connection_id: "1234".into(), lb_policy: Some(LoadBalancerPolicy::RoundRobin), - }, - }; + }) + .build(); let yaml = serde_yaml::to_string(&config).unwrap(); println!("{}", yaml); } #[test] fn deserialise_server() { - let config = Config { - local: Local { port: 7000 }, - filters: vec![], - connections: ConnectionConfig::Server { + let config = Builder::empty() + .with_local(Local { port: 7000 }) + .with_connections(ConnectionConfig::Server { endpoints: vec![ EndPoint { name: String::from("No.1"), @@ -217,8 +205,8 @@ mod tests { connection_ids: vec!["1234".into()], }, ], - }, - }; + }) + .build(); let yaml = serde_yaml::to_string(&config).unwrap(); println!("{}", yaml); } @@ -336,34 +324,32 @@ server: #[test] fn validate() { // client - valid - let config = Config { - local: Local { port: 7000 }, - filters: vec![], - connections: ConnectionConfig::Client { + let config = Builder::empty() + .with_local(Local { port: 7000 }) + .with_connections(ConnectionConfig::Client { addresses: vec![ "127.0.0.1:25999".parse().unwrap(), "127.0.0.1:25998".parse().unwrap(), ], connection_id: "1234".into(), lb_policy: Some(LoadBalancerPolicy::RoundRobin), - }, - }; + }) + .build(); assert!(config.validate().is_ok()); // client - non unique address - let config = Config { - local: Local { port: 7000 }, - filters: vec![], - connections: ConnectionConfig::Client { + let config = Builder::empty() + .with_local(Local { port: 7000 }) + .with_connections(ConnectionConfig::Client { addresses: vec![ "127.0.0.1:25999".parse().unwrap(), "127.0.0.1:25999".parse().unwrap(), ], connection_id: "1234".into(), lb_policy: Some(LoadBalancerPolicy::RoundRobin), - }, - }; + }) + .build(); assert_eq!( ValidationError::NotUnique("connections.addresses".to_string()).to_string(), @@ -371,10 +357,9 @@ server: ); // server - valid - let config = Config { - local: Local { port: 7000 }, - filters: vec![], - connections: ConnectionConfig::Server { + let config = Builder::empty() + .with_local(Local { port: 7000 }) + .with_connections(ConnectionConfig::Server { endpoints: vec![ EndPoint { name: String::from("ONE"), @@ -387,15 +372,14 @@ server: connection_ids: vec!["1234".into()], }, ], - }, - }; + }) + .build(); assert!(config.validate().is_ok()); // server - non unique endpoint names - let config = Config { - local: Local { port: 7000 }, - filters: vec![], - connections: ConnectionConfig::Server { + let config = Builder::empty() + .with_local(Local { port: 7000 }) + .with_connections(ConnectionConfig::Server { endpoints: vec![ EndPoint { name: String::from("SAME"), @@ -408,8 +392,8 @@ server: connection_ids: vec!["1234".into()], }, ], - }, - }; + }) + .build(); assert_eq!( ValidationError::NotUnique("endpoint.name".to_string()).to_string(), @@ -417,10 +401,9 @@ server: ); // server - non unique addresses - let config = Config { - local: Local { port: 7000 }, - filters: vec![], - connections: ConnectionConfig::Server { + let config = Builder::empty() + .with_local(Local { port: 7000 }) + .with_connections(ConnectionConfig::Server { endpoints: vec![ EndPoint { name: String::from("ONE"), @@ -433,8 +416,8 @@ server: connection_ids: vec!["1234".into()], }, ], - }, - }; + }) + .build(); assert_eq!( ValidationError::NotUnique("endpoint.address".to_string()).to_string(), diff --git a/src/extensions/filter_chain.rs b/src/extensions/filter_chain.rs index c5803cdd7f..25a2441107 100644 --- a/src/extensions/filter_chain.rs +++ b/src/extensions/filter_chain.rs @@ -118,7 +118,7 @@ mod tests { use std::str::from_utf8; use crate::config; - use crate::config::{ConnectionConfig, EndPoint, Local}; + use crate::config::{Builder, ConnectionConfig, EndPoint}; use crate::extensions::filters::DebugFilterFactory; use crate::extensions::{default_registry, FilterFactory}; use crate::test_utils::{logger, noop_endpoint, TestFilter}; @@ -131,37 +131,36 @@ mod tests { let provider = DebugFilterFactory::new(&log); // everything is fine - let config = Arc::new(Config { - local: Local { port: 0 }, - filters: vec![config::Filter { + let config = Builder::empty() + .with_filters(vec![config::Filter { name: provider.name(), config: Default::default(), - }], - connections: ConnectionConfig::Client { + }]) + .with_connections(ConnectionConfig::Client { addresses: vec!["127.0.0.1:2456".parse().unwrap()], connection_id: "".into(), lb_policy: None, - }, - }); + }) + .build(); let registry = default_registry(&log); - let chain = FilterChain::try_create(config, ®istry, &Registry::default()).unwrap(); + let chain = + FilterChain::try_create(Arc::new(config), ®istry, &Registry::default()).unwrap(); assert_eq!(1, chain.filters.len()); // uh oh, something went wrong - let config = Arc::new(Config { - local: Local { port: 0 }, - filters: vec![config::Filter { + let config = Builder::empty() + .with_filters(vec![config::Filter { name: "this is so wrong".to_string(), config: Default::default(), - }], - connections: ConnectionConfig::Client { + }]) + .with_connections(ConnectionConfig::Client { addresses: vec!["127.0.0.1:2456".parse().unwrap()], connection_id: "".into(), lb_policy: None, - }, - }); - let result = FilterChain::try_create(config, ®istry, &Registry::default()); + }) + .build(); + let result = FilterChain::try_create(Arc::new(config), ®istry, &Registry::default()); assert!(result.is_err()); } diff --git a/src/proxy/server.rs b/src/proxy/server.rs index d6e1fd616a..2f4c48db04 100644 --- a/src/proxy/server.rs +++ b/src/proxy/server.rs @@ -334,7 +334,7 @@ mod tests { use tokio::time::{Duration, Instant}; use crate::config; - use crate::config::{Config, ConnectionConfig, EndPoint, Local}; + use crate::config::{Builder as ConfigBuilder, ConnectionConfig, EndPoint, Local}; use crate::proxy::sessions::{Packet, SESSION_TIMEOUT_SECONDS}; use crate::test_utils::{SplitSocket, TestFilter, TestFilterFactory, TestHelper}; @@ -350,12 +350,11 @@ mod tests { let endpoint2 = t.open_socket_and_recv_single_packet().await; let local_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 12358); - let config = Config { - local: Local { + let config = ConfigBuilder::empty() + .with_local(Local { port: local_addr.port(), - }, - filters: vec![], - connections: ConnectionConfig::Server { + }) + .with_connections(ConnectionConfig::Server { endpoints: vec![ EndPoint { name: String::from("e1"), @@ -368,8 +367,8 @@ mod tests { connection_ids: vec![], }, ], - }, - }; + }) + .build(); t.run_server(config); let msg = "hello"; @@ -389,17 +388,16 @@ mod tests { let mut endpoint = t.open_socket_and_recv_single_packet().await; let local_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 12357); - let config = Config { - local: Local { + let config = ConfigBuilder::empty() + .with_local(Local { port: local_addr.port(), - }, - filters: vec![], - connections: ConnectionConfig::Client { + }) + .with_connections(ConnectionConfig::Client { addresses: vec![endpoint.addr], connection_id: "".into(), lb_policy: None, - }, - }; + }) + .build(); t.run_server(config); let msg = "hello"; @@ -420,20 +418,20 @@ mod tests { let mut endpoint = t.open_socket_and_recv_single_packet().await; let local_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 12367); - let config = Config { - local: Local { + let config = ConfigBuilder::empty() + .with_local(Local { port: local_addr.port(), - }, - filters: vec![config::Filter { + }) + .with_filters(vec![config::Filter { name: "TestFilter".to_string(), config: None, - }], - connections: ConnectionConfig::Client { + }]) + .with_connections(ConnectionConfig::Client { addresses: vec![endpoint.addr], connection_id: "".into(), lb_policy: None, - }, - }; + }) + .build(); t.run_server_with_filter_registry(config, registry); let msg = "hello"; @@ -458,13 +456,12 @@ mod tests { #[tokio::test] async fn bind() { - let config = Config { - local: Local { port: 12345 }, - filters: vec![], - connections: ConnectionConfig::Server { + let config = ConfigBuilder::empty() + .with_local(Local { port: 12345 }) + .with_connections(ConnectionConfig::Server { endpoints: Vec::new(), - }, - }; + }) + .build(); let socket = Server::bind(&config).await.unwrap(); let addr = socket.local_addr().unwrap(); @@ -592,15 +589,7 @@ mod tests { let sessions: SessionMap = Arc::new(RwLock::new(HashMap::new())); let (send_packets, mut recv_packets) = mpsc::channel::(1); - let config = Arc::new(Config { - local: Local { port: 0 }, - filters: vec![], - connections: ConnectionConfig::Client { - addresses: vec![], - connection_id: "".into(), - lb_policy: None, - }, - }); + let config = Arc::new(ConfigBuilder::empty().build()); let server = Builder::from(config).validate().unwrap().build(); server.run_recv_from( @@ -670,15 +659,7 @@ mod tests { unreachable!("failed to send packet over channel"); } - let config = Arc::new(Config { - local: Local { port: 0 }, - filters: vec![], - connections: ConnectionConfig::Client { - addresses: vec![], - connection_id: "".into(), - lb_policy: None, - }, - }); + let config = Arc::new(ConfigBuilder::empty().build()); let server = Builder::from(config).validate().unwrap().build(); server.run_receive_packet(endpoint.send, recv_packet); assert_eq!(msg, endpoint.packet_rx.await.unwrap()); @@ -757,15 +738,7 @@ mod tests { connection_ids: vec![], }; - let config = Arc::new(Config { - local: Local { port: to.port() }, - filters: vec![], - connections: ConnectionConfig::Client { - addresses: vec![], - connection_id: "".into(), - lb_policy: None, - }, - }); + let config = Arc::new(ConfigBuilder::empty().build()); let server = Builder::from(config).validate().unwrap().build(); server.run_prune_sessions(&sessions); Server::ensure_session( diff --git a/tests/filters.rs b/tests/filters.rs index a8193b574e..4887b9c031 100644 --- a/tests/filters.rs +++ b/tests/filters.rs @@ -23,7 +23,7 @@ mod tests { use serde_yaml::{Mapping, Value}; use slog::info; - use quilkin::config::{Config, ConnectionConfig, EndPoint, Filter, Local}; + use quilkin::config::{Builder as ConfigBuilder, ConnectionConfig, EndPoint, Filter, Local}; use quilkin::extensions::filters::DebugFilterFactory; use quilkin::extensions::{default_registry, FilterFactory}; use quilkin::test_utils::{TestFilterFactory, TestHelper}; @@ -37,20 +37,20 @@ mod tests { // create server configuration let server_port = 12346; - let server_config = Config { - local: Local { port: server_port }, - filters: vec![Filter { + let server_config = ConfigBuilder::empty() + .with_local(Local { port: server_port }) + .with_filters(vec![Filter { name: "TestFilter".to_string(), config: None, - }], - connections: ConnectionConfig::Server { + }]) + .with_connections(ConnectionConfig::Server { endpoints: vec![EndPoint { name: "server".to_string(), address: echo, connection_ids: vec![], }], - }, - }; + }) + .build(); assert_eq!(Ok(()), server_config.validate()); // Run server proxy. @@ -60,21 +60,21 @@ mod tests { // create a local client let client_port = 12347; - let client_config = Config { - local: Local { port: client_port }, - filters: vec![Filter { + let client_config = ConfigBuilder::empty() + .with_local(Local { port: client_port }) + .with_filters(vec![Filter { name: "TestFilter".to_string(), config: None, - }], - connections: ConnectionConfig::Client { + }]) + .with_connections(ConnectionConfig::Client { addresses: vec![SocketAddr::new( IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), server_port, )], connection_id: "".into(), lb_policy: None, - }, - }; + }) + .build(); assert_eq!(Ok(()), client_config.validate()); // Run client proxy. @@ -123,41 +123,41 @@ mod tests { map.insert(Value::from("id"), Value::from("server")); // create server configuration let server_port = 12247; - let server_config = Config { - local: Local { port: server_port }, - filters: vec![Filter { + let server_config = ConfigBuilder::empty() + .with_local(Local { port: server_port }) + .with_filters(vec![Filter { name: factory.name(), config: Some(serde_yaml::Value::Mapping(map)), - }], - connections: ConnectionConfig::Server { + }]) + .with_connections(ConnectionConfig::Server { endpoints: vec![EndPoint { name: "server".to_string(), address: echo, connection_ids: vec![], }], - }, - }; + }) + .build(); t.run_server(server_config); let mut map = Mapping::new(); map.insert(Value::from("id"), Value::from("client")); // create a local client let client_port = 12248; - let client_config = Config { - local: Local { port: client_port }, - filters: vec![Filter { + let client_config = ConfigBuilder::empty() + .with_local(Local { port: client_port }) + .with_filters(vec![Filter { name: factory.name(), config: Some(serde_yaml::Value::Mapping(map)), - }], - connections: ConnectionConfig::Client { + }]) + .with_connections(ConnectionConfig::Client { addresses: vec![SocketAddr::new( IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), server_port, )], connection_id: "".into(), lb_policy: None, - }, - }; + }) + .build(); t.run_server(client_config); // let's send the packet diff --git a/tests/local_rate_limit.rs b/tests/local_rate_limit.rs index 2252f2ae8b..44a96260d6 100644 --- a/tests/local_rate_limit.rs +++ b/tests/local_rate_limit.rs @@ -20,7 +20,7 @@ extern crate quilkin; mod tests { use std::net::{IpAddr, Ipv4Addr, SocketAddr}; - use quilkin::config::{Config, ConnectionConfig, EndPoint, Filter, Local}; + use quilkin::config::{Builder as ConfigBuilder, ConnectionConfig, EndPoint, Filter, Local}; use quilkin::extensions::filters::RateLimitFilterFactory; use quilkin::extensions::FilterFactory; use quilkin::test_utils::TestHelper; @@ -36,20 +36,20 @@ period: 1s let echo = t.run_echo_server().await; let server_port = 12346; - let server_config = Config { - local: Local { port: server_port }, - filters: vec![Filter { + let server_config = ConfigBuilder::empty() + .with_local(Local { port: server_port }) + .with_filters(vec![Filter { name: RateLimitFilterFactory::default().name(), config: serde_yaml::from_str(yaml).unwrap(), - }], - connections: ConnectionConfig::Server { + }]) + .with_connections(ConnectionConfig::Server { endpoints: vec![EndPoint { name: "server".to_string(), address: echo, connection_ids: vec![], }], - }, - }; + }) + .build(); t.run_server(server_config); let (mut recv_chan, mut send) = t.open_socket_and_recv_multiple_packets().await; diff --git a/tests/metrics.rs b/tests/metrics.rs index 8c70f18ce1..ed099fd472 100644 --- a/tests/metrics.rs +++ b/tests/metrics.rs @@ -24,7 +24,7 @@ mod tests { use regex::Regex; use slog::info; - use quilkin::config::{Config, ConnectionConfig, EndPoint, Local}; + use quilkin::config::{Builder as ConfigBuilder, ConnectionConfig, EndPoint, Local}; use quilkin::proxy::Metrics; use quilkin::test_utils::TestHelper; @@ -38,34 +38,31 @@ mod tests { // create server configuration let server_port = 12346; - let server_config = Config { - local: Local { port: server_port }, - filters: vec![], - connections: ConnectionConfig::Server { + let server_config = ConfigBuilder::empty() + .with_local(Local { port: server_port }) + .with_connections(ConnectionConfig::Server { endpoints: vec![EndPoint { name: "server".to_string(), address: echo, connection_ids: vec![], }], - }, - }; - + }) + .build(); t.run_server_with_metrics(server_config, server_metrics); // create a local client let client_port = 12347; - let client_config = Config { - local: Local { port: client_port }, - filters: vec![], - connections: ConnectionConfig::Client { + let client_config = ConfigBuilder::empty() + .with_local(Local { port: client_port }) + .with_connections(ConnectionConfig::Client { addresses: vec![SocketAddr::new( IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), server_port, )], connection_id: "".into(), lb_policy: None, - }, - }; + }) + .build(); t.run_server(client_config); // let's send the packet diff --git a/tests/no_filter.rs b/tests/no_filter.rs index f771946a66..757d486907 100644 --- a/tests/no_filter.rs +++ b/tests/no_filter.rs @@ -23,7 +23,7 @@ mod tests { use tokio::select; use tokio::time::{delay_for, Duration}; - use quilkin::config::{Config, ConnectionConfig, EndPoint, Local}; + use quilkin::config::{Builder as ConfigBuilder, ConnectionConfig, EndPoint, Local}; use quilkin::test_utils::TestHelper; #[tokio::test] @@ -36,10 +36,9 @@ mod tests { // create server configuration let server_port = 12345; - let server_config = Config { - local: Local { port: server_port }, - filters: vec![], - connections: ConnectionConfig::Server { + let server_config = ConfigBuilder::empty() + .with_local(Local { port: server_port }) + .with_connections(ConnectionConfig::Server { endpoints: vec![ EndPoint { name: "server1".to_string(), @@ -52,26 +51,25 @@ mod tests { connection_ids: vec![], }, ], - }, - }; + }) + .build(); assert_eq!(Ok(()), server_config.validate()); t.run_server(server_config); // create a local client let client_port = 12344; - let client_config = Config { - local: Local { port: client_port }, - filters: vec![], - connections: ConnectionConfig::Client { + let client_config = ConfigBuilder::empty() + .with_local(Local { port: client_port }) + .with_connections(ConnectionConfig::Client { addresses: vec![SocketAddr::new( IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), server_port, )], connection_id: "".into(), lb_policy: None, - }, - }; + }) + .build(); assert_eq!(Ok(()), client_config.validate()); t.run_server(client_config);