Proxy: Parse environment variables in one place (linkerd#26)

Previously `Process` did its own environment variable parsing and did
not benefit from the improved error handling that `config` now has.
Additionally, future changes will need access to these same environment
variables in other parts of the proxy.

Move `Process`'s environment variable parsing to `config` to address
both of these issues. Now there are no uses of `env::var` outside of
`config` except for logging, which is the final desired state.

I validated this manually.

proxy/src/config.rs

@@ -48,6 +48,10 @@ pub struct Config {
 
     /// Timeout after which to cancel telemetry reports.
     pub report_timeout: Duration,
+
+    pub pod_name: Option<String>,
+    pub pod_namespace: Option<String>,
+    pub node_name: Option<String>,
 }
 
 /// Configuration settings for binding a listener.
@@ -125,10 +129,9 @@ pub const ENV_CONTROL_LISTENER: &str = "CONDUIT_PROXY_CONTROL_LISTENER";
 const ENV_PRIVATE_CONNECT_TIMEOUT: &str = "CONDUIT_PROXY_PRIVATE_CONNECT_TIMEOUT";
 const ENV_PUBLIC_CONNECT_TIMEOUT: &str = "CONDUIT_PROXY_PUBLIC_CONNECT_TIMEOUT";
 
-// the following are `pub` because they're used in the `ctx` module for populating `Process`.
-pub const ENV_NODE_NAME: &str = "CONDUIT_PROXY_NODE_NAME";
-pub const ENV_POD_NAME: &str = "CONDUIT_PROXY_POD_NAME";
-pub const ENV_POD_NAMESPACE: &str = "CONDUIT_PROXY_POD_NAMESPACE";
+const ENV_NODE_NAME: &str = "CONDUIT_PROXY_NODE_NAME";
+const ENV_POD_NAME: &str = "CONDUIT_PROXY_POD_NAME";
+const ENV_POD_NAMESPACE: &str = "CONDUIT_PROXY_POD_NAMESPACE";
 
 pub const ENV_CONTROL_URL: &str = "CONDUIT_PROXY_CONTROL_URL";
 const ENV_RESOLV_CONF: &str = "CONDUIT_RESOLV_CONF";
@@ -164,6 +167,9 @@ impl<'a> TryFrom<&'a Strings> for Config {
         let metrics_flush_interval_secs =
             parse(strings, ENV_METRICS_FLUSH_INTERVAL_SECS, parse_number);
         let report_timeout = parse(strings, ENV_REPORT_TIMEOUT_SECS, parse_number);
+        let pod_name = strings.get(ENV_POD_NAME);
+        let pod_namespace = strings.get(ENV_POD_NAMESPACE);
+        let node_name = strings.get(ENV_NODE_NAME);
 
         Ok(Config {
             private_listener: Listener {
@@ -193,6 +199,9 @@ impl<'a> TryFrom<&'a Strings> for Config {
                                         .unwrap_or(DEFAULT_METRICS_FLUSH_INTERVAL_SECS)),
             report_timeout:
                 Duration::from_secs(report_timeout?.unwrap_or(DEFAULT_REPORT_TIMEOUT_SECS)),
+            pod_name: pod_name?,
+            pod_namespace: pod_namespace?,
+            node_name: node_name?,
         })
     }
 }

proxy/src/ctx/mod.rs

@@ -7,8 +7,8 @@
 //! As a rule, context types should implement `Clone + Send + Sync`. This allows them to
 //! be stored in `http::Extensions`, for instance. Furthermore, because these contexts
 //! will be sent to a telemetry processing thread, we want to avoid excessive cloning.
+use config;
 use control::pb::proxy::telemetry as proto;
-use std::env;
 use std::sync::Arc;
 pub mod http;
 pub mod transport;
@@ -47,7 +47,8 @@ pub enum Proxy {
 }
 
 impl Process {
-    pub fn new(node: &str, instance: &str, ns: &str) -> Arc<Self> {
+    #[cfg(test)]
+    pub fn test(node: &str, instance: &str, ns: &str) -> Arc<Self> {
         Arc::new(Self {
             node: node.into(),
             scheduled_instance: instance.into(),
@@ -56,25 +57,18 @@ impl Process {
     }
 
     /// Construct a new `Process` from environment variables.
-    pub fn from_env() -> Arc<Self> {
-        fn get_var(key: &str) -> String {
-            env::var(key).unwrap_or_else(|why| {
-                warn!(
-                    "Process::from_env(): Failed to get value of {} environment variable: {:?}",
-                    key,
-                    why
-                );
-                String::from("")
-            })
+    pub fn new(config: &config::Config) -> Arc<Self> {
+        fn empty_if_missing(s: &Option<String>) -> String {
+            match *s {
+                Some(ref s) => s.clone(),
+                None => "".to_owned(),
+            }
         }
 
-        let node = get_var(::config::ENV_NODE_NAME);
-        let scheduled_instance = get_var(::config::ENV_POD_NAME);
-        let scheduled_namespace = get_var(::config::ENV_POD_NAMESPACE);
         Arc::new(Self {
-            node,
-            scheduled_instance,
-            scheduled_namespace,
+            node: empty_if_missing(&config.node_name),
+            scheduled_instance: empty_if_missing(&config.pod_name),
+            scheduled_namespace: empty_if_missing(&config.pod_namespace),
         })
     }
 }

proxy/src/inbound.rs

@@ -176,7 +176,7 @@ mod tests {
             local: net::SocketAddr,
             remote: net::SocketAddr
         ) -> bool {
-            let ctx = ctx::Proxy::inbound(&ctx::Process::new("test", "test", "test"));
+            let ctx = ctx::Proxy::inbound(&ctx::Process::test("test", "test", "test"));
 
             let inbound = new_inbound(None, &ctx);
 
@@ -198,7 +198,7 @@ mod tests {
             local: net::SocketAddr,
             remote: net::SocketAddr
         ) -> bool {
-            let ctx = ctx::Proxy::inbound(&ctx::Process::new("test", "test", "test"));
+            let ctx = ctx::Proxy::inbound(&ctx::Process::test("test", "test", "test"));
 
             let inbound = new_inbound(default, &ctx);
 
@@ -210,7 +210,7 @@ mod tests {
         }
 
         fn recognize_default_no_ctx(default: Option<net::SocketAddr>) -> bool {
-            let ctx = ctx::Proxy::inbound(&ctx::Process::new("test", "test", "test"));
+            let ctx = ctx::Proxy::inbound(&ctx::Process::test("test", "test", "test"));
 
             let inbound = new_inbound(default, &ctx);
 
@@ -224,7 +224,7 @@ mod tests {
             local: net::SocketAddr,
             remote: net::SocketAddr
         ) -> bool {
-            let ctx = ctx::Proxy::inbound(&ctx::Process::new("test", "test", "test"));
+            let ctx = ctx::Proxy::inbound(&ctx::Process::test("test", "test", "test"));
 
             let inbound = new_inbound(default, &ctx);
 

proxy/src/lib.rs

@@ -137,6 +137,8 @@ impl Main {
     where
         F: Future<Item = (), Error = ()>,
     {
+        let process_ctx = ctx::Process::new(&self.config);
+
         let Main {
             config,
             control_listener,
@@ -154,7 +156,6 @@ impl Main {
             config.private_forward
         );
 
-        let process_ctx = ctx::Process::from_env();
         let (sensors, telemetry) = telemetry::new(
             &process_ctx,
             config.event_buffer_capacity,