From ff07bb9cc6e1375917633ad4d531b089fb263ece Mon Sep 17 00:00:00 2001 From: Beatriz Rizental Date: Wed, 25 Jul 2018 17:50:32 -0700 Subject: [PATCH] refactor(settings): drop NODE_ENV and add LOG_LEVEL (#141) r=@vladikoff Fixes #138 Connects to #139 --- Dockerfile | 1 + config/default.json | 5 ++- config/dev.json | 5 ++- config/test.json | 3 -- scripts/test_standalone.sh | 4 +- scripts/test_with_authdb.sh | 4 +- src/logging/mod.rs | 2 +- src/settings/mod.rs | 86 +++++++++++++++++++++---------------- src/settings/test.rs | 34 +++++++++++++++ src/validate/mod.rs | 14 +++++- src/validate/test.rs | 26 +++++++++++ 11 files changed, 137 insertions(+), 47 deletions(-) delete mode 100644 config/test.json diff --git a/Dockerfile b/Dockerfile index d04a986..1ab1f8c 100644 --- a/Dockerfile +++ b/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"] diff --git a/config/default.json b/config/default.json index a40b71f..1c0ad65 100644 --- a/config/default.json +++ b/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": { diff --git a/config/dev.json b/config/dev.json index 360cfe6..69b6c11 100644 --- a/config/dev.json +++ b/config/dev.json @@ -1,5 +1,8 @@ { - "logging": "pretty", + "log": { + "level": "normal", + "format": "pretty" + }, "provider": "smtp", "smtp": { "port": 9999 diff --git a/config/test.json b/config/test.json deleted file mode 100644 index f4ad41a..0000000 --- a/config/test.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "logging": "null" -} diff --git a/scripts/test_standalone.sh b/scripts/test_standalone.sh index 4c7bb9b..cf9f8b8 100755 --- a/scripts/test_standalone.sh +++ b/scripts/test_standalone.sh @@ -2,8 +2,8 @@ export RUST_BACKTRACE=1 -if [ -z "$NODE_ENV" ]; then - export NODE_ENV=test +if [ -z "$FXA_EMAIL_LOG_FORMAT" ]; then + export FXA_EMAIL_LOG_FORMAT=null fi cargo test -- --test-threads=1 diff --git a/scripts/test_with_authdb.sh b/scripts/test_with_authdb.sh index 00eac79..4d38561 100755 --- a/scripts/test_with_authdb.sh +++ b/scripts/test_with_authdb.sh @@ -17,8 +17,8 @@ fi sleep 2 -if [ -z "$NODE_ENV" ]; then - export NODE_ENV=test +if [ -z "$FXA_EMAIL_LOG_FORMAT" ]; then + export FXA_EMAIL_LOG_FORMAT=null fi export RUST_BACKTRACE=1 diff --git a/src/logging/mod.rs b/src/logging/mod.rs index 30000fc..a028c82 100644 --- a/src/logging/mod.rs +++ b/src/logging/mod.rs @@ -73,7 +73,7 @@ pub struct MozlogLogger(slog::Logger); impl MozlogLogger { /// Construct a logger. pub fn new(settings: &Settings) -> Result { - 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()) diff --git a/src/settings/mod.rs b/src/settings/mod.rs index b7c0482..ea795e2 100644 --- a/src/settings/mod.rs +++ b/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, } +/// 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, - /// can be `"mozlog"`, `"pretty"` or `"null"`. - pub logging: Logging, + /// The logging settings, + /// about level and formatting. + pub log: Log, /// The port this application is listening to. pub port: u16, @@ -317,13 +335,9 @@ impl Settings { /// /// 1. Environment variables: `$FXA_EMAIL_` /// 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 { @@ -331,9 +345,12 @@ impl Settings { config.merge(File::with_name("config/default"))?; - if let Ok(node_env) = env::var("NODE_ENV") { - config.merge(File::with_name(&format!("config/{}", node_env)).required(false))?; - } + let current_env = match env::var("FXA_EMAIL_ENV") { + 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::() { Ok(settings) => { - if let Ok(node_env) = env::var("NODE_ENV") { - if node_env == "production" && &settings.hmackey == "YOU MUST CHANGE ME" { - panic!("Please set a valid HMAC key.") - } + if current_env == "production" && &settings.hmackey == "YOU MUST CHANGE ME" { + panic!("Please set a valid HMAC key.") } let logger = @@ -356,25 +371,24 @@ impl Settings { } } - /// Create rocket configuration based on the `NODE_ENV` environment - /// variable. Defaults to `dev` mode if `NODE_ENV` is not set. + /// Create rocket configuration based on the environment + /// variable. Defaults to `dev` mode if `FXA_EMAIL_ENV` is not set. pub fn build_rocket_config(&self) -> Result { - match env::var("NODE_ENV").as_ref().map(String::as_ref) { - Ok("production") => RocketConfig::build(RocketEnvironment::Production) - .address(self.host.0.clone()) - .port(self.port.clone()) - .log_level(LoggingLevel::Off) - .finalize(), - Ok("staging") => RocketConfig::build(RocketEnvironment::Staging) - .address(self.host.0.clone()) - .port(self.port.clone()) - .log_level(LoggingLevel::Critical) - .finalize(), - _ => RocketConfig::build(RocketEnvironment::Development) - .address(self.host.0.clone()) - .port(self.port.clone()) - .log_level(LoggingLevel::Normal) - .finalize(), - } + let log_level = match self.log.level.0.as_str() { + "debug" => RocketLoggingLevel::Debug, + "critical" => RocketLoggingLevel::Critical, + "off" => RocketLoggingLevel::Off, + _ => RocketLoggingLevel::Normal, + }; + let rocket_config = match self.env.0.as_str() { + "production" => RocketEnvironment::Production, + "staging" => RocketEnvironment::Staging, + _ => RocketEnvironment::Development, + }; + RocketConfig::build(rocket_config) + .address(self.host.0.clone()) + .port(self.port.clone()) + .log_level(log_level) + .finalize() } } diff --git a/src/settings/test.rs b/src/settings/test.rs index ad53f5d..3ec4763 100644 --- a/src/settings/test.rs +++ b/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", ¤t_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![ diff --git a/src/validate/mod.rs b/src/validate/mod.rs index ea782bd..b711177 100644 --- a/src/validate/mod.rs +++ b/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) } diff --git a/src/validate/test.rs b/src/validate/test.rs index 396c891..9803f01 100644 --- a/src/validate/test.rs +++ b/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);