refactor(settings): drop NODE_ENV and add LOG_LEVEL (#141) r=@vladikoff

Fixes #138
Connects to #139

Dockerfile

@@ -37,6 +37,7 @@ COPY --from=builder /app/bin /app/bin
 WORKDIR /app/bin
 USER app
 
+ENV FXA_EMAIL_ENV production
 ENV ROCKET_ENV production
 
 CMD ["/app/bin/fxa_email_send"]

config/default.json

@@ -21,7 +21,10 @@
   },
   "hmackey": "YOU MUST CHANGE ME",
   "host": "127.0.0.1",
-  "logging": "mozlog",
+  "log": {
+    "level": "off",
+    "format": "mozlog"
+  },
   "port": 8001,
   "provider": "ses",
   "redis": {

config/dev.json

@@ -1,5 +1,8 @@
 {
-  "logging": "pretty",
+  "log": {
+    "level": "normal",
+    "format": "pretty"
+  },
   "provider": "smtp",
   "smtp": {
     "port": 9999

config/test.json

@@ -1,3 +0,0 @@
-{
-    "logging": "null"
-}

scripts/test_standalone.sh

@@ -2,8 +2,8 @@
 
 export RUST_BACKTRACE=1
 
-if [ -z "$NODE_ENV" ]; then
+if [ -z "$FXA_EMAIL_LOG_FORMAT" ]; then
-  export NODE_ENV=test
+  export FXA_EMAIL_LOG_FORMAT=null
 fi
 
 cargo test -- --test-threads=1

scripts/test_with_authdb.sh

@@ -17,8 +17,8 @@ fi
 
 sleep 2
 
-if [ -z "$NODE_ENV" ]; then
+if [ -z "$FXA_EMAIL_LOG_FORMAT" ]; then
-  export NODE_ENV=test
+  export FXA_EMAIL_LOG_FORMAT=null
 fi
 
 export RUST_BACKTRACE=1

src/logging/mod.rs

@@ -73,7 +73,7 @@ pub struct MozlogLogger(slog::Logger);
 impl MozlogLogger {
     /// Construct a logger.
     pub fn new(settings: &Settings) -> Result<MozlogLogger, Error> {
-        let logger = match &*settings.logging.0.as_ref() {
+        let logger = match &*settings.log.format.0.as_ref() {
             "mozlog" => {
                 let drain = MozLogJson::new(io::stdout())
                     .logger_name(LOGGER_NAME.to_owned())

src/settings/mod.rs

@@ -12,7 +12,7 @@ use std::{
 use config::{Config, ConfigError, Environment, File};
 use rocket::config::{
     Config as RocketConfig, ConfigError as RocketConfigError, Environment as RocketEnvironment,
-    LoggingLevel,
+    LoggingLevel as RocketLoggingLevel,
 };
 use serde::de::{Deserialize, Deserializer, Error, Unexpected};
 
@@ -63,10 +63,14 @@ deserialize_and_validate! {
     (AwsSecret, aws_secret, "AWS secret key"),
     /// Base URI type.
     (BaseUri, base_uri, "base URI"),
+    /// Env type.
+    (Env, env, "'dev', 'staging', 'production' or 'test'"),
     /// Host name or IP address type.
     (Host, host, "host name or IP address"),
+    /// Logging level type.
+    (LoggingLevel, logging_level, "'normal', 'debug', 'critical' or 'off'"),
     /// Logging format type.
-    (Logging, logging, "'mozlog', 'pretty' or 'null'"),
+    (LoggingFormat, logging_format, "'mozlog', 'pretty' or 'null'"),
     /// Email provider type.
     (Provider, provider, "'ses', 'sendgrid' or 'smtp'"),
     /// Sender name type.
@@ -144,6 +148,16 @@ pub struct BounceLimits {
     pub soft: Vec<BounceLimit>,
 }
 
+/// Settings for logging.
+#[derive(Debug, Default, Deserialize, Serialize)]
+pub struct Log {
+    /// The logging level.
+    pub level: LoggingLevel,
+
+    /// The logging format.
+    pub format: LoggingFormat,
+}
+
 /// Settings for Redis.
 #[derive(Debug, Default, Deserialize, Serialize)]
 pub struct Redis {
@@ -266,6 +280,10 @@ pub struct Settings {
     /// will fail with a `429` error.
     pub bouncelimits: BounceLimits,
 
+    /// The env sets which environment we are in.
+    /// It defaults to `dev` if not set.
+    pub env: Env,
+
     /// The HMAC key to use internally
     /// for hashing message ids.
     /// This is sensitive data
@@ -274,9 +292,9 @@ pub struct Settings {
 
     pub host: Host,
 
-    /// The logging format to use,
+    /// The logging settings,
-    /// can be `"mozlog"`, `"pretty"` or `"null"`.
+    /// about level and formatting.
-    pub logging: Logging,
+    pub log: Log,
 
     /// The port this application is listening to.
     pub port: u16,
@@ -317,13 +335,9 @@ impl Settings {
     ///
     ///   1. Environment variables: `$FXA_EMAIL_<UPPERCASE_KEY_NAME>`
     ///   2. File: `config/local.json`
-    ///   3. File: `config/<$NODE_ENV>.json`
+    ///   3. File: `config/<$FXA_EMAIL_ENV>.json`
     ///   4. File: `config/default.json`
     ///
-    /// `$NODE_ENV` is used so that this service automatically picks up the
-    /// appropriate state from our existing node.js ecosystem, without needing
-    /// to manage an extra environment variable.
-    ///
     /// Immediately before returning, the parsed config object will be logged to
     /// the console.
     pub fn new() -> Result<Self, ConfigError> {
@@ -331,9 +345,12 @@ impl Settings {
 
         config.merge(File::with_name("config/default"))?;
 
-        if let Ok(node_env) = env::var("NODE_ENV") {
+        let current_env = match env::var("FXA_EMAIL_ENV") {
-            config.merge(File::with_name(&format!("config/{}", node_env)).required(false))?;
+            Ok(env) => env,
-        }
+            _ => String::from("dev"),
+        };
+        config.merge(File::with_name(&format!("config/{}", current_env)).required(false))?;
+        config.set_default("env", "dev")?;
 
         config.merge(File::with_name("config/local").required(false))?;
         let env = Environment::with_prefix("fxa_email");
@@ -341,10 +358,8 @@ impl Settings {
 
         match config.try_into::<Settings>() {
             Ok(settings) => {
-                if let Ok(node_env) = env::var("NODE_ENV") {
+                if current_env == "production" && &settings.hmackey == "YOU MUST CHANGE ME" {
-                    if node_env == "production" && &settings.hmackey == "YOU MUST CHANGE ME" {
+                    panic!("Please set a valid HMAC key.")
-                        panic!("Please set a valid HMAC key.")
-                    }
                 }
 
                 let logger =
@@ -356,25 +371,24 @@ impl Settings {
         }
     }
 
-    /// Create rocket configuration based on the `NODE_ENV` environment
+    /// Create rocket configuration based on the environment
-    /// variable. Defaults to `dev` mode if `NODE_ENV` is not set.
+    /// variable. Defaults to `dev` mode if `FXA_EMAIL_ENV` is not set.
     pub fn build_rocket_config(&self) -> Result<RocketConfig, RocketConfigError> {
-        match env::var("NODE_ENV").as_ref().map(String::as_ref) {
+        let log_level = match self.log.level.0.as_str() {
-            Ok("production") => RocketConfig::build(RocketEnvironment::Production)
+            "debug" => RocketLoggingLevel::Debug,
-                .address(self.host.0.clone())
+            "critical" => RocketLoggingLevel::Critical,
-                .port(self.port.clone())
+            "off" => RocketLoggingLevel::Off,
-                .log_level(LoggingLevel::Off)
+            _ => RocketLoggingLevel::Normal,
-                .finalize(),
+        };
-            Ok("staging") => RocketConfig::build(RocketEnvironment::Staging)
+        let rocket_config = match self.env.0.as_str() {
-                .address(self.host.0.clone())
+            "production" => RocketEnvironment::Production,
-                .port(self.port.clone())
+            "staging" => RocketEnvironment::Staging,
-                .log_level(LoggingLevel::Critical)
+            _ => RocketEnvironment::Development,
-                .finalize(),
+        };
-            _ => RocketConfig::build(RocketEnvironment::Development)
+        RocketConfig::build(rocket_config)
-                .address(self.host.0.clone())
+            .address(self.host.0.clone())
-                .port(self.port.clone())
+            .port(self.port.clone())
-                .log_level(LoggingLevel::Normal)
+            .log_level(log_level)
-                .finalize(),
+            .finalize()
-        }
     }
 }

src/settings/test.rs

@@ -65,6 +65,9 @@ fn env_vars_take_precedence() {
         "FXA_EMAIL_AWS_SQSURLS_DELIVERY",
         "FXA_EMAIL_AWS_SQSURLS_NOTIFICATION",
         "FXA_EMAIL_BOUNCELIMITS_ENABLED",
+        "FXA_EMAIL_ENV",
+        "FXA_EMAIL_LOG_LEVEL",
+        "FXA_EMAIL_LOG_FORMAT",
         "FXA_EMAIL_MESSAGEDATA_HMACKEY",
         "FXA_EMAIL_PROVIDER",
         "FXA_EMAIL_REDIS_HOST",
@@ -123,6 +126,7 @@ fn env_vars_take_precedence() {
                 }
             };
             let bounce_limits_enabled = !settings.bouncelimits.enabled;
+            let current_env = Env(String::from("test"));
             let hmac_key = String::from("something else");
             let provider = if settings.provider == Provider("ses".to_string()) {
                 "sendgrid"
@@ -149,6 +153,7 @@ fn env_vars_take_precedence() {
                     password: String::from("5"),
                 }
             };
+
             let socketlabs = if let Some(ref socketlabs) = settings.socketlabs {
                 SocketLabs {
                     serverid: socketlabs.serverid + 1,
@@ -161,6 +166,19 @@ fn env_vars_take_precedence() {
                 }
             };
 
+            let log = Log {
+                level: if settings.log.level == LoggingLevel("debug".to_string()) {
+                    LoggingLevel("off".to_string())
+                } else {
+                    LoggingLevel("debug".to_string())
+                },
+                format: if settings.log.format == LoggingFormat("null".to_string()) {
+                    LoggingFormat("pretty".to_string())
+                } else {
+                    LoggingFormat("null".to_string())
+                },
+            };
+
             env::set_var("FXA_EMAIL_AUTHDB_BASEURI", &auth_db_base_uri);
             env::set_var("FXA_EMAIL_AWS_REGION", &aws_region);
             env::set_var("FXA_EMAIL_AWS_KEYS_ACCESS", &aws_keys.access.0);
@@ -177,6 +195,9 @@ fn env_vars_take_precedence() {
                 &bounce_limits_enabled.to_string(),
             );
             env::set_var("FXA_EMAIL_HMACKEY", &hmac_key.to_string());
+            env::set_var("FXA_EMAIL_ENV", &current_env.0);
+            env::set_var("FXA_EMAIL_LOG_LEVEL", &log.level.0);
+            env::set_var("FXA_EMAIL_LOG_FORMAT", &log.format.0);
             env::set_var("FXA_EMAIL_PROVIDER", &provider);
             env::set_var("FXA_EMAIL_REDIS_HOST", &redis_host);
             env::set_var("FXA_EMAIL_REDIS_PORT", &redis_port.to_string());
@@ -201,7 +222,10 @@ fn env_vars_take_precedence() {
                     assert_eq!(env_settings.authdb.baseuri, BaseUri(auth_db_base_uri));
                     assert_eq!(env_settings.aws.region, AwsRegion(aws_region.to_string()));
                     assert_eq!(env_settings.bouncelimits.enabled, bounce_limits_enabled);
+                    assert_eq!(env_settings.env, current_env);
                     assert_eq!(env_settings.hmackey, hmac_key);
+                    assert_eq!(env_settings.log.level, log.level);
+                    assert_eq!(env_settings.log.format, log.format);
                     assert_eq!(env_settings.provider, Provider(provider.to_string()));
                     assert_eq!(env_settings.redis.host, Host(redis_host));
                     assert_eq!(env_settings.redis.port, redis_port);
@@ -259,6 +283,16 @@ fn env_vars_take_precedence() {
     }
 }
 
+#[test]
+fn default_env() {
+    let _clean_env = CleanEnvironment::new(vec!["FXA_EMAIL_ENV"]);
+
+    match Settings::new() {
+        Ok(settings) => assert_eq!(settings.env, Env("dev".to_string())),
+        Err(_error) => assert!(false, "Settings::new shouldn't have failed"),
+    }
+}
+
 #[test]
 fn hidden_sensitive_data() {
     let _clean_env = CleanEnvironment::new(vec![

src/validate/mod.rs

@@ -28,7 +28,9 @@ lazy_static! {
     static ref EMAIL_ADDRESS_FORMAT: Regex = Regex::new(
         "^[a-zA-Z0-9.!#$%&’*+/=?^_`{|}~-]{1,64}@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,253}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,253}[a-zA-Z0-9])?)+$"
     ).unwrap();
+    static ref ENV: Regex = Regex::new("^(?:dev|staging|production|test)$").unwrap();
     static ref HOST_FORMAT: Regex = Regex::new("^[A-Za-z0-9-]+(?:\\.[A-Za-z0-9-]+)*$").unwrap();
+    static ref LOGGING_LEVEL: Regex = Regex::new("^(?:normal|debug|critical|off)$").unwrap();
     static ref LOGGING_FORMAT: Regex = Regex::new("^(?:mozlog|pretty|null)$").unwrap();
     static ref PROVIDER_FORMAT: Regex = Regex::new("^(?:mock|sendgrid|ses|smtp|socketlabs)$").unwrap();
     static ref SENDER_NAME_FORMAT: Regex =
@@ -63,13 +65,23 @@ pub fn email_address(value: &str) -> bool {
     value.len() < 254 && EMAIL_ADDRESS_FORMAT.is_match(value)
 }
 
+/// Validate env.
+pub fn env(value: &str) -> bool {
+    ENV.is_match(value)
+}
+
 /// Validate a host name or IP address.
 pub fn host(value: &str) -> bool {
     HOST_FORMAT.is_match(value)
 }
 
 /// Validate logging level.
-pub fn logging(value: &str) -> bool {
+pub fn logging_level(value: &str) -> bool {
+    LOGGING_LEVEL.is_match(value)
+}
+
+/// Validate logging format.
+pub fn logging_format(value: &str) -> bool {
     LOGGING_FORMAT.is_match(value)
 }
 

src/validate/test.rs

@@ -125,6 +125,15 @@ fn invalid_email_address() {
     )));
 }
 
+#[test]
+fn env() {
+    assert!(validate::env("test"));
+    assert!(validate::env("dev"));
+    assert!(validate::env("staging"));
+    assert!(validate::env("production"));
+    assert_eq!(false, validate::env("something else"));
+}
+
 #[test]
 fn host() {
     assert!(validate::host("foo"));
@@ -132,6 +141,23 @@ fn host() {
     assert!(validate::host("127.0.0.1"));
 }
 
+#[test]
+fn logging_level() {
+    assert!(validate::logging_level("normal"));
+    assert!(validate::logging_level("debug"));
+    assert!(validate::logging_level("critical"));
+    assert!(validate::logging_level("off"));
+    assert_eq!(false, validate::logging_level("something else"));
+}
+
+#[test]
+fn logging_format() {
+    assert!(validate::logging_format("mozlog"));
+    assert!(validate::logging_format("pretty"));
+    assert!(validate::logging_format("null"));
+    assert_eq!(false, validate::logging_format("something else"));
+}
+
 #[test]
 fn invalid_host() {
     assert_eq!(validate::host("foo/bar"), false);