proxy: Parse units with duration configurations (#909)

Configuration values that take durations are currently specified as
time values with no units.  So `600` may mean 600ms in some contexts and
10 minutes in others.

In order to avoid this problem, this change now requires that
configurations provide explicit units for time values such as '600ms' or
10 minutes'.

Fixes #27.
This commit is contained in:
Oliver Gould 2018-05-08 13:54:12 -07:00 committed by GitHub
parent 416381cdfd
commit 63fbbd6931
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 118 additions and 22 deletions

12
Cargo.lock generated
View File

@ -142,7 +142,7 @@ dependencies = [
"prost-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
"quickcheck 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
"rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
"regex 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)",
"regex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
"tokio-connect 0.1.0 (git+https://github.com/carllerche/tokio-connect)",
"tokio-core 0.1.12 (registry+https://github.com/rust-lang/crates.io-index)",
"tokio-io 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
@ -767,19 +767,19 @@ dependencies = [
[[package]]
name = "regex"
version = "0.2.10"
version = "1.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"aho-corasick 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)",
"memchr 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"regex-syntax 0.5.5 (registry+https://github.com/rust-lang/crates.io-index)",
"regex-syntax 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
"thread_local 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)",
"utf8-ranges 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
]
[[package]]
name = "regex-syntax"
version = "0.5.5"
version = "0.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"ucd-util 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
@ -1317,8 +1317,8 @@ dependencies = [
"checksum rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "eba5f8cb59cc50ed56be8880a5c7b496bfd9bd26394e176bc67884094145c2c5"
"checksum redox_syscall 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)" = "0d92eecebad22b767915e4d529f89f28ee96dbbf5a4810d2b844373f136417fd"
"checksum redox_termios 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7e891cfe48e9100a70a3b6eb652fef28920c117d366339687bd5576160db0f76"
"checksum regex 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)" = "aec3f58d903a7d2a9dc2bf0e41a746f4530e0cab6b615494e058f67a3ef947fb"
"checksum regex-syntax 0.5.5 (registry+https://github.com/rust-lang/crates.io-index)" = "bd90079345f4a4c3409214734ae220fd773c6f2e8a543d07370c6c1c369cfbfb"
"checksum regex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "75ecf88252dce580404a22444fc7d626c01815debba56a7f4f536772a5ff19d3"
"checksum regex-syntax 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8f1ac0f60d675cc6cf13a20ec076568254472551051ad5dd050364d70671bf6b"
"checksum relay 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f301bafeb60867c85170031bdb2fcf24c8041f33aee09e7b116a58d4e9f781c5"
"checksum remove_dir_all 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "dfc5b3ce5d5ea144bb04ebd093a9e14e9765bcfec866aecda9b6dec43b3d1e24"
"checksum resolv-conf 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8e1b086bb6a2659d6ba66e4aa21bde8a53ec03587cd5c80b83bdc3a330f35cab"

View File

@ -29,6 +29,9 @@ log = "0.4.1"
indexmap = "1.0.0"
rand = "0.4"
# for config parsing
regex = "1.0.0"
tokio-core = "0.1"
tokio-io = "0.1"
tokio-signal = "0.1"
@ -57,5 +60,4 @@ libc = "0.2"
[dev-dependencies]
quickcheck = { version = "0.6", default-features = false }
conduit-proxy-controller-grpc = { path = "./controller-grpc" , features = ["arbitrary"] }
regex = "0.2"
flate2 = { version = "1.0.1", default-features = false, features = ["rust_backend"] }

View File

@ -86,16 +86,17 @@ pub enum Error {
InvalidEnvVar
}
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq)]
pub enum ParseError {
EnvironmentUnsupported,
NotADuration,
NotANumber,
HostIsNotAnIpAddress,
NotUnicode,
UrlError(UrlError),
}
#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, PartialEq)]
pub enum UrlError {
/// The URl is syntactically invalid according to general URL parsing rules.
SyntaxError,
@ -195,16 +196,16 @@ impl<'a> TryFrom<&'a Strings> for Config {
let control_listener_addr = parse(strings, ENV_CONTROL_LISTENER, str::parse);
let metrics_listener_addr = parse(strings, ENV_METRICS_LISTENER, str::parse);
let private_forward = parse(strings, ENV_PRIVATE_FORWARD, str::parse);
let public_connect_timeout = parse(strings, ENV_PUBLIC_CONNECT_TIMEOUT, parse_number);
let private_connect_timeout = parse(strings, ENV_PRIVATE_CONNECT_TIMEOUT, parse_number);
let public_connect_timeout = parse(strings, ENV_PUBLIC_CONNECT_TIMEOUT, parse_duration);
let private_connect_timeout = parse(strings, ENV_PRIVATE_CONNECT_TIMEOUT, parse_duration);
let inbound_disable_ports = parse(strings, ENV_INBOUND_PORTS_DISABLE_PROTOCOL_DETECTION, parse_port_set);
let outbound_disable_ports = parse(strings, ENV_OUTBOUND_PORTS_DISABLE_PROTOCOL_DETECTION, parse_port_set);
let inbound_router_capacity = parse(strings, ENV_INBOUND_ROUTER_CAPACITY, parse_number);
let outbound_router_capacity = parse(strings, ENV_OUTBOUND_ROUTER_CAPACITY, parse_number);
let bind_timeout = parse(strings, ENV_BIND_TIMEOUT, parse_number);
let bind_timeout = parse(strings, ENV_BIND_TIMEOUT, parse_duration);
let resolv_conf_path = strings.get(ENV_RESOLV_CONF);
let event_buffer_capacity = parse(strings, ENV_EVENT_BUFFER_CAPACITY, parse_number);
let metrics_retain_idle = parse(strings, ENV_METRICS_RETAIN_IDLE, parse_number);
let metrics_retain_idle = parse(strings, ENV_METRICS_RETAIN_IDLE, parse_duration);
let pod_namespace = strings.get(ENV_POD_NAMESPACE).and_then(|maybe_value| {
// There cannot be a default pod namespace, and the pod namespace is required.
maybe_value.ok_or_else(|| {
@ -243,9 +244,9 @@ impl<'a> TryFrom<&'a Strings> for Config {
},
private_forward: private_forward?,
public_connect_timeout: public_connect_timeout?.map(Duration::from_millis)
public_connect_timeout: public_connect_timeout?
.unwrap_or(DEFAULT_PUBLIC_CONNECT_TIMEOUT),
private_connect_timeout: private_connect_timeout?.map(Duration::from_millis)
private_connect_timeout: private_connect_timeout?
.unwrap_or(DEFAULT_PRIVATE_CONNECT_TIMEOUT),
inbound_ports_disable_protocol_detection: inbound_disable_ports?
@ -264,11 +265,9 @@ impl<'a> TryFrom<&'a Strings> for Config {
control_host_and_port: control_host_and_port?,
event_buffer_capacity: event_buffer_capacity?.unwrap_or(DEFAULT_EVENT_BUFFER_CAPACITY),
metrics_retain_idle: metrics_retain_idle?.map(Duration::from_secs)
.unwrap_or(DEFAULT_METRICS_RETAIN_IDLE),
metrics_retain_idle: metrics_retain_idle?.unwrap_or(DEFAULT_METRICS_RETAIN_IDLE),
bind_timeout: bind_timeout?.map(Duration::from_millis)
.unwrap_or(DEFAULT_BIND_TIMEOUT),
bind_timeout: bind_timeout?.unwrap_or(DEFAULT_BIND_TIMEOUT),
pod_namespace: pod_namespace?,
})
@ -340,6 +339,27 @@ fn parse_number<T>(s: &str) -> Result<T, ParseError> where T: FromStr {
s.parse().map_err(|_| ParseError::NotANumber)
}
fn parse_duration(s: &str) -> Result<Duration, ParseError> {
use regex::Regex;
let re = Regex::new(r"^\s*(\d+)(ms|s|m|h|d)?\s*$")
.expect("duration regex");
let cap = re.captures(s)
.ok_or(ParseError::NotADuration)?;
let magnitude = parse_number(&cap[1])?;
match cap.get(2).map(|m| m.as_str()) {
None if magnitude == 0 => Ok(Duration::from_secs(0)),
Some("ms") => Ok(Duration::from_millis(magnitude)),
Some("s") => Ok(Duration::from_secs(magnitude)),
Some("m") => Ok(Duration::from_secs(magnitude * 60)),
Some("h") => Ok(Duration::from_secs(magnitude * 60 * 60)),
Some("d") => Ok(Duration::from_secs(magnitude * 60 * 60 * 24)),
_ => Err(ParseError::NotADuration),
}
}
fn parse_url(s: &str) -> Result<HostAndPort, ParseError> {
let url = s.parse::<http::Uri>().map_err(|_| ParseError::UrlError(UrlError::SyntaxError))?;
if url.scheme_part().map(|s| s.as_str()) != Some("tcp") {
@ -380,3 +400,76 @@ fn parse<T, Parse>(strings: &Strings, name: &str, parse: Parse) -> Result<Option
None => Ok(None),
}
}
#[cfg(test)]
mod tests {
use super::*;
fn test_unit<F: Fn(u64) -> Duration>(unit: &str, to_duration: F) {
for v in &[0, 1, 23, 456_789] {
let d = to_duration(*v);
let text = format!("{}{}", v, unit);
assert_eq!(parse_duration(&text), Ok(d), "text=\"{}\"", text);
let text = format!(" {}{}\t", v, unit);
assert_eq!(parse_duration(&text), Ok(d), "text=\"{}\"", text);
}
}
#[test]
fn parse_duration_unit_ms() {
test_unit("ms", |v| Duration::from_millis(v));
}
#[test]
fn parse_duration_unit_s() {
test_unit("s", |v| Duration::from_secs(v));
}
#[test]
fn parse_duration_unit_m() {
test_unit("m", |v| Duration::from_secs(v * 60));
}
#[test]
fn parse_duration_unit_h() {
test_unit("h", |v| Duration::from_secs(v * 60 * 60));
}
#[test]
fn parse_duration_unit_d() {
test_unit("d", |v| Duration::from_secs(v * 60 * 60 * 24));
}
#[test]
fn parse_duration_floats_invalid() {
assert_eq!(parse_duration(".123h"), Err(ParseError::NotADuration));
assert_eq!(parse_duration("1.23h"), Err(ParseError::NotADuration));
}
#[test]
fn parse_duration_space_before_unit_invalid() {
assert_eq!(parse_duration("1 ms"), Err(ParseError::NotADuration));
}
#[test]
fn parse_duration_overflows_invalid() {
assert_eq!(parse_duration("123456789012345678901234567890ms"), Err(ParseError::NotANumber));
}
#[test]
fn parse_duration_invalid_unit() {
assert_eq!(parse_duration("12moons"), Err(ParseError::NotADuration));
assert_eq!(parse_duration("12y"), Err(ParseError::NotADuration));
}
#[test]
fn parse_duration_zero_without_unit() {
assert_eq!(parse_duration("0"), Ok(Duration::from_secs(0)));
}
#[test]
fn parse_duration_number_without_unit_is_invalid() {
assert_eq!(parse_duration("1"), Err(ParseError::NotADuration));
}
}

View File

@ -28,6 +28,7 @@ extern crate prost_types;
#[macro_use]
extern crate quickcheck;
extern crate rand;
extern crate regex;
extern crate tokio_connect;
extern crate tokio_core;
extern crate tokio_io;

View File

@ -36,7 +36,7 @@ pub enum Host {
Ip(IpAddr),
}
#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, PartialEq)]
pub enum HostAndPortError {
/// The host is not a valid DNS name or IP address.
InvalidHost,

View File

@ -69,7 +69,7 @@ macro_rules! generate_tests {
let mut env = config::TestEnv::new();
// set the bind timeout to 100 ms.
env.put(config::ENV_BIND_TIMEOUT, "100".to_owned());
env.put(config::ENV_BIND_TIMEOUT, "100ms".to_owned());
let srv = $make_server().route("/", "hello").run();
let ctrl = controller::new();
@ -111,7 +111,7 @@ macro_rules! generate_tests {
let mut env = config::TestEnv::new();
// set the bind timeout to 100 ms.
env.put(config::ENV_BIND_TIMEOUT, "100".to_owned());
env.put(config::ENV_BIND_TIMEOUT, "100ms".to_owned());
let srv = $make_server().route("/hi", "hello").run();
let ctrl = controller::new();