Skip to content

Commit

Permalink
Allow disabling tap by setting an env var (linkerd#252)
Browse files Browse the repository at this point in the history 
This PR fixes linkerd/linkerd2#2811. Now if
`LINKERD2_PROXY_TAP_DISABLED` is set, the tap is not served at all. The
approach taken is that  the `ProxyParts` is changed so the
`control_listener` is now an `Option` that will be None if tap is
disabled as this control_listener seems to be exclusively used to serve
the tap. Feel free to suggest a better approach. 

Signed-off-by: Zahari Dichev <zaharidichev@gmail.com>
  • Loading branch information
@zaharidichev @hawkw
zaharidichev authored and hawkw committed May 15, 2019
1 parent b70c68d commit 6525b06
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 17 deletions.
26 changes: 20 additions & 6 deletions src/app/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub struct Config {
pub inbound_listener: Listener,

/// Where to listen for connections initiated by the control plane.
pub control_listener: Listener,
pub control_listener: Option<Listener>,

/// Where to serve admin HTTP.
pub admin_listener: Listener,
Expand Down Expand Up @@ -278,6 +278,7 @@ pub const ENV_DESTINATION_CONTEXT: &str = "LINKERD2_PROXY_DESTINATION_CONTEXT";
pub const ENV_CONTROL_EXP_BACKOFF_MIN: &str = "LINKERD2_PROXY_CONTROL_EXP_BACKOFF_MIN";
pub const ENV_CONTROL_EXP_BACKOFF_MAX: &str = "LINKERD2_PROXY_CONTROL_EXP_BACKOFF_MAX";
pub const ENV_CONTROL_EXP_BACKOFF_JITTER: &str = "LINKERD2_PROXY_CONTROL_EXP_BACKOFF_JITTER";
pub const ENV_TAP_DISABLED: &str = "LINKERD2_PROXY_TAP_DISABLED";
const ENV_CONTROL_CONNECT_TIMEOUT: &str = "LINKERD2_PROXY_CONTROL_CONNECT_TIMEOUT";
const ENV_CONTROL_DISPATCH_TIMEOUT: &str = "LINKERD2_PROXY_CONTROL_DISPATCH_TIMEOUT";
const ENV_RESOLV_CONF: &str = "LINKERD2_PROXY_RESOLV_CONF";
Expand Down Expand Up @@ -382,7 +383,6 @@ impl Config {
// defer returning any errors until all of them have been parsed.
let outbound_listener_addr = parse(strings, ENV_OUTBOUND_LISTEN_ADDR, parse_socket_addr);
let inbound_listener_addr = parse(strings, ENV_INBOUND_LISTEN_ADDR, parse_socket_addr);
let control_listener_addr = parse(strings, ENV_CONTROL_LISTEN_ADDR, parse_socket_addr);
let admin_listener_addr = parse(strings, ENV_ADMIN_LISTEN_ADDR, parse_socket_addr);
let inbound_forward = parse(strings, ENV_INBOUND_FORWARD, parse_socket_addr);

Expand Down Expand Up @@ -464,6 +464,8 @@ impl Config {
let initial_connection_window_size =
parse(strings, ENV_INITIAL_CONNECTION_WINDOW_SIZE, parse_number);

let control_listener = parse_control_listener(strings);

Ok(Config {
outbound_listener: Listener {
addr: outbound_listener_addr?
Expand All @@ -473,10 +475,7 @@ impl Config {
addr: inbound_listener_addr?
.unwrap_or_else(|| parse_socket_addr(DEFAULT_INBOUND_LISTEN_ADDR).unwrap()),
},
control_listener: Listener {
addr: control_listener_addr?
.unwrap_or_else(|| parse_socket_addr(DEFAULT_CONTROL_LISTEN_ADDR).unwrap()),
},
control_listener: control_listener?,
admin_listener: Listener {
addr: admin_listener_addr?
.unwrap_or_else(|| parse_socket_addr(DEFAULT_ADMIN_LISTEN_ADDR).unwrap()),
Expand Down Expand Up @@ -614,6 +613,21 @@ impl Strings for TestEnv {

// ===== Parsing =====

fn parse_control_listener(strings: &Strings) -> Result<Option<Listener>, Error> {
let tap_disabled = strings
.get(ENV_TAP_DISABLED)?
.map(|d| !d.is_empty())
.unwrap_or(false);

if tap_disabled {
Ok(None)
} else {
let addr = parse(strings, ENV_CONTROL_LISTEN_ADDR, parse_socket_addr)?
.unwrap_or_else(|| parse_socket_addr(DEFAULT_CONTROL_LISTEN_ADDR).unwrap());
Ok(Some(Listener { addr }))
}
}

fn parse_number<T>(s: &str) -> Result<T, ParseError>
where
T: FromStr,
Expand Down
21 changes: 14 additions & 7 deletions src/app/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ struct ProxyParts<G> {
start_time: SystemTime,

admin_listener: Listen<identity::Local, ()>,
control_listener: Listen<identity::Local, ()>,
control_listener: Option<Listen<identity::Local, ()>>,

inbound_listener: Listen<identity::Local, G>,
outbound_listener: Listen<identity::Local, G>,
Expand Down Expand Up @@ -96,8 +96,10 @@ where
let identity = config.identity_config.as_ref().map(identity::Local::new);
let local_identity = identity.as_ref().map(|(l, _)| l.clone());

let control_listener = Listen::bind(config.control_listener.addr, local_identity.clone())
.expect("dst_svc listener bind");
let control_listener = config
.control_listener
.as_ref()
.map(|l| Listen::bind(l.addr, local_identity.clone()).expect("dst_svc listener bind"));

let admin_listener = Listen::bind(config.admin_listener.addr, local_identity.clone())
.expect("metrics listener bind");
Expand Down Expand Up @@ -135,8 +137,11 @@ where
}
}

pub fn control_addr(&self) -> SocketAddr {
self.proxy_parts.control_listener.local_addr()
pub fn control_addr(&self) -> Option<SocketAddr> {
self.proxy_parts
.control_listener
.as_ref()
.map(|l| l.local_addr().clone())
}

pub fn inbound_addr(&self) -> SocketAddr {
Expand Down Expand Up @@ -378,8 +383,10 @@ where
Admin::new(report, readiness),
));

rt.spawn(tap_daemon.map_err(|_| ()));
rt.spawn(serve_tap(control_listener, TapServer::new(tap_grpc)));
if let Some(listener) = control_listener {
rt.spawn(tap_daemon.map_err(|_| ()));
rt.spawn(serve_tap(listener, TapServer::new(tap_grpc)));
}

rt.spawn(::logging::admin().bg("dns-resolver").future(dns_bg));

Expand Down
7 changes: 5 additions & 2 deletions tests/support/proxy.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub struct Proxy {
}

pub struct Listening {
pub control: SocketAddr,
pub control: Option<SocketAddr>,
pub inbound: SocketAddr,
pub outbound: SocketAddr,
pub metrics: SocketAddr,
Expand Down Expand Up @@ -273,7 +273,10 @@ fn run(proxy: Proxy, mut env: app::config::TestEnv) -> Listening {
// printlns will show if the test fails...
println!(
"proxy running; destination={}, identity={:?}, inbound={}{}, outbound={}{}, metrics={}",
control_addr,
control_addr
.as_ref()
.map(SocketAddr::to_string)
.unwrap_or_else(String::new),
identity_addr,
inbound_addr,
inbound
Expand Down
23 changes: 21 additions & 2 deletions tests/tap.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fn inbound_http1() {

let proxy = proxy::new().inbound(srv).run();

let mut tap = tap::client(proxy.control);
let mut tap = tap::client(proxy.control.unwrap());
let events = tap.observe(tap::observe_request());

let authority = "tap.test.svc.cluster.local";
Expand Down Expand Up @@ -54,7 +54,7 @@ fn grpc_headers_end() {

let proxy = proxy::new().inbound(srv).run();

let mut tap = tap::client(proxy.control);
let mut tap = tap::client(proxy.control.unwrap());
let events = tap.observe(tap::observe_request());

let authority = "tap.test.svc.cluster.local";
Expand All @@ -73,3 +73,22 @@ fn grpc_headers_end() {

assert_eq!(ev.response_end_eos_grpc(), 1);
}

#[test]
fn tap_enabled_by_default() {
let _ = env_logger_init();
let proxy = proxy::new().run();

assert!(proxy.control.is_some())
}

#[test]
fn can_disable_tap() {
let _ = env_logger_init();
let mut env = app::config::TestEnv::new();
env.put(app::config::ENV_TAP_DISABLED, "true".to_owned());

let proxy = proxy::new().run_with_test_env(env);

assert!(proxy.control.is_none())
}

0 comments on commit 6525b06

Please sign in to comment.