From 1c01680d3df563589ba8e51aa90e814917f15522 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 4 Sep 2018 13:36:39 +0200 Subject: [PATCH 1/8] feat: Override for project configs --- common/src/config.rs | 45 +++++++++- server/src/actors/events.rs | 2 +- server/src/actors/project.rs | 106 +++++++++++++++--------- server/src/endpoints/project_configs.rs | 2 +- tests/integration/test_basic.py | 25 ++++++ 5 files changed, 136 insertions(+), 44 deletions(-) diff --git a/common/src/config.rs b/common/src/config.rs index 0bdcba1bb8a..fbbd3fbc946 100644 --- a/common/src/config.rs +++ b/common/src/config.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::env; use std::fmt; use std::fs; @@ -9,8 +10,9 @@ use std::time::Duration; use failure::{Backtrace, Context, Fail}; use log; +use marshal::processor::PiiConfig; // Dsn must be imported from sentry and not sentry-types for compatibility with sentry::init! -use sentry_types::Dsn; +use sentry_types::{Dsn, ProjectId}; use serde::de::DeserializeOwned; use serde::ser::Serialize; use serde_json; @@ -291,6 +293,40 @@ impl Default for Sentry { } } +/// These are config values that the user can modify in the UI. +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +#[serde(rename_all = "camelCase")] +pub struct ProjectConfig { + /// URLs that are permitted for cross original JavaScript requests. + pub allowed_domains: Vec, + /// List of relay public keys that are permitted to access this project. + pub trusted_relays: Vec, + /// Configuration for PII stripping. + pub pii_config: Option, +} + +/// Project state that was hardcoded by the user in the local config +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct StaticProjectState { + /// Indicates that the project is disabled. + pub disabled: bool, + /// A container of known public keys in the project. + pub public_keys: HashMap, + /// The project's slug if configured. + pub slug: String, + /// The project's current config. + pub config: ProjectConfig, +} + +/// Local overrides for project state +#[derive(Serialize, Deserialize, Debug, Default)] +#[serde(default)] +pub struct Projects { + /// The overridden projects + pub configs: HashMap, +} + /// Controls interal reporting to Sentry. #[derive(Serialize, Deserialize, Debug, Default)] #[serde(default)] @@ -348,6 +384,8 @@ struct ConfigValues { metrics: Metrics, #[serde(default)] sentry: Sentry, + #[serde(default)] + projects: Projects, } impl ConfigObject for ConfigValues { @@ -621,6 +659,11 @@ impl Config { None } } + + /// Return local project state overrides + pub fn projects(&self) -> &Projects { + &self.values.projects + } } enum ConfigFormat { diff --git a/server/src/actors/events.rs b/server/src/actors/events.rs index 1590e94480e..29a2bb0ee15 100644 --- a/server/src/actors/events.rs +++ b/server/src/actors/events.rs @@ -137,7 +137,7 @@ impl ProcessEvent { event.id.set_value(Some(Some(self.event_id))) } - let processed_event = match self.project_state.config.pii_config { + let processed_event = match self.project_state.config().pii_config { Some(ref pii_config) => pii_config.processor().process_root_value(event), None => event, }; diff --git a/server/src/actors/project.rs b/server/src/actors/project.rs index cc574eaeb68..699d0306ad2 100644 --- a/server/src/actors/project.rs +++ b/server/src/actors/project.rs @@ -12,7 +12,7 @@ use futures::{future::Shared, sync::oneshot, Future}; use url::Url; use uuid::Uuid; -use semaphore_common::{processor::PiiConfig, Config, ProjectId, PublicKey, RetryBackoff}; +use semaphore_common::{Config, ProjectConfig, ProjectId, RetryBackoff, StaticProjectState}; use actors::controller::{Controller, Shutdown, Subscribe, TimeoutError}; use actors::upstream::{SendQuery, UpstreamQuery, UpstreamRelay}; @@ -63,8 +63,14 @@ impl Project { &mut self, context: &mut Context, ) -> Response, ProjectError> { + if let Some(state) = self.config.projects().configs.get(&self.id) { + self.state = Some(Arc::new(ProjectState::FromLocalConfig(state.clone()))); + } + if let Some(ref state) = self.state { - return Response::ok(state.clone()); + if !state.outdated(&self.config) { + return Response::ok(state.clone()); + } } debug!("project {} state requested", self.id); @@ -158,58 +164,60 @@ pub enum PublicKeyStatus { Enabled, } -/// These are config values that the user can modify in the UI. -#[derive(Debug, Clone, Serialize, Deserialize, Default)] -#[serde(rename_all = "camelCase")] -pub struct ProjectConfig { - /// URLs that are permitted for cross original JavaScript requests. - pub allowed_domains: Vec, - /// List of relay public keys that are permitted to access this project. - pub trusted_relays: Vec, - /// Configuration for PII stripping. - pub pii_config: Option, -} - -/// The project state is a cached server state of a project. +/// Project state that came from upstream #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -pub struct ProjectState { +pub struct FetchedProjectState { /// The timestamp of when the state was received. pub last_fetch: DateTime, /// The timestamp of when the state was last changed. - /// - /// This might be `None` in some rare cases like where states - /// are faked locally. - pub last_change: Option>, + pub last_change: DateTime, /// Indicates that the project is disabled. pub disabled: bool, /// A container of known public keys in the project. pub public_keys: HashMap, - /// The project's slug if available. - pub slug: Option, - /// The project's current config + /// The project's slug. + pub slug: String, + /// The project's current config. pub config: ProjectConfig, /// The project state's revision id. pub rev: Option, } +/// The project state is a cached server state of a project. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(untagged)] +#[cfg_attr(feature = "cargo-clippy", allow(large_enum_variant))] +pub enum ProjectState { + /// Project state that came from upstream + FromServer(FetchedProjectState), + /// Project state that was set through the local config + FromLocalConfig(StaticProjectState), + /// Missing project state, used when project state was not fetched yet. + #[serde(rename_all = "camelCase")] + Missing { + /// The timestamp of when the state was checked. + last_fetch: DateTime, + }, +} + impl ProjectState { /// Project state for a missing project. pub fn missing() -> Self { - ProjectState { + ProjectState::Missing { last_fetch: Utc::now(), - last_change: None, - disabled: true, - public_keys: HashMap::new(), - slug: None, - config: Default::default(), - rev: None, } } /// Returns the current status of a key. pub fn get_public_key_status(&self, public_key: &str) -> PublicKeyStatus { - match self.public_keys.get(public_key) { + let public_keys = match *self { + ProjectState::FromServer(ref x) => &x.public_keys, + ProjectState::FromLocalConfig(ref x) => &x.public_keys, + ProjectState::Missing { .. } => return PublicKeyStatus::Unknown, + }; + + match public_keys.get(public_key) { Some(&true) => PublicKeyStatus::Enabled, Some(&false) => PublicKeyStatus::Disabled, None => PublicKeyStatus::Unknown, @@ -219,27 +227,43 @@ impl ProjectState { /// Returns `true` if the entire project should be considered /// disabled (blackholed, deleted etc.). pub fn disabled(&self) -> bool { - self.disabled + match *self { + ProjectState::Missing { .. } => true, + ProjectState::FromServer(ref x) => x.disabled, + ProjectState::FromLocalConfig(ref x) => x.disabled, + } } /// Returns whether this state is outdated and needs to be refetched. pub fn outdated(&self, config: &Config) -> bool { - SystemTime::from(self.last_fetch) - .elapsed() - .map(|e| match self.slug { - Some(_) => e > config.project_cache_expiry(), - None => e > config.cache_miss_expiry(), - }) - .unwrap_or(false) + let elapsed = |dt| SystemTime::from(dt).elapsed().ok(); + + match *self { + ProjectState::FromLocalConfig(_) => false, + ProjectState::FromServer(ref x) => elapsed(x.last_fetch) + .map(|lf| lf > config.project_cache_expiry()) + .unwrap_or(false), + ProjectState::Missing { ref last_fetch } => elapsed(*last_fetch) + .map(|lf| lf > config.cache_miss_expiry()) + .unwrap_or(false), + } } /// Returns the project config. pub fn config(&self) -> &ProjectConfig { - &self.config + lazy_static! { + static ref DUMMY_CONFIG: ProjectConfig = ProjectConfig::default(); + } + + match *self { + ProjectState::FromServer(ref x) => &x.config, + ProjectState::FromLocalConfig(ref x) => &x.config, + ProjectState::Missing { .. } => &*DUMMY_CONFIG, + } } /// Checks if this origin is allowed for this project. - fn is_valid_origin(&self, origin: Option<&Url>) -> bool { + pub fn is_valid_origin(&self, origin: Option<&Url>) -> bool { // Generally accept any event without an origin. let origin = match origin { Some(origin) => origin, diff --git a/server/src/endpoints/project_configs.rs b/server/src/endpoints/project_configs.rs index 3c9366535de..d321c5a297f 100644 --- a/server/src/endpoints/project_configs.rs +++ b/server/src/endpoints/project_configs.rs @@ -23,7 +23,7 @@ fn get_project_configs( let project_state = project_state.ok()?; // If public key is known (even if rate-limited, which is Some(false)), it has // access to the project config - if project_state.config.trusted_relays.contains(&public_key) { + if project_state.config().trusted_relays.contains(&public_key) { Some((*project_state).clone()) } else { None diff --git a/tests/integration/test_basic.py b/tests/integration/test_basic.py index 495fdf6eef9..7ac59e58384 100644 --- a/tests/integration/test_basic.py +++ b/tests/integration/test_basic.py @@ -211,3 +211,28 @@ def get_project_config(): (_, error), = mini_sentry.test_failures assert isinstance(error, socket.error) mini_sentry.test_failures.clear() + + +def test_local_project_config(mini_sentry, relay): + config = mini_sentry.basic_project_config() + del config['lastChange'] + del config['lastFetch'] + del config['rev'] + + relay = relay(mini_sentry, { + 'projects': { + 'configs': { + '42': config + } + } + }) + + relay.wait_relay_healthcheck() + + client = sentry_sdk.Client(relay.dsn, default_integrations=False) + hub = sentry_sdk.Hub(client) + hub.capture_message("hü") + client.drain_events() + + event = mini_sentry.captured_events.get(timeout=4) + assert event["message"] == "hü" From c366a5fe76c7c35adac89bba759597c5fec2eaf5 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 4 Sep 2018 17:19:58 +0200 Subject: [PATCH 2/8] fix: Fix nameerror in tests --- tests/integration/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 54e51dfacbb..b63760e5191 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -104,7 +104,7 @@ def fail(e): @request.addfinalizer def reraise_test_failures(): if sentry.test_failures: - raise AssertionError(f"Exceptions happened in mini_sentry: {test_failures}") + raise AssertionError(f"Exceptions happened in mini_sentry: {sentry.est_failures}") server = WSGIServer(application=app, threaded=True) server.start() From 04bcfb88c08eefe1fd5fb59da3566a52983b1d21 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 4 Sep 2018 17:27:39 +0200 Subject: [PATCH 3/8] test: Set correct timestamp for project config --- tests/integration/conftest.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index b63760e5191..78d9b9c3ee0 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -1,3 +1,4 @@ +import datetime import os import uuid import pytest @@ -170,8 +171,8 @@ def basic_project_config(self): "publicKeys": {self.dsn_public_key: True}, "rev": "5ceaea8c919811e8ae7daae9fe877901", "disabled": False, - "lastFetch": "2018-08-24T17:29:04.426Z", - "lastChange": "2018-07-27T12:27:01.481Z", + "lastFetch": datetime.datetime.now().isoformat(), + "lastChange": datetime.datetime.now().isoformat(), "config": { "allowedDomains": ["*"], "trustedRelays": list(self.iter_public_keys()), From 3bf2a510a171fef9d12237477db008f5017efea8 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 4 Sep 2018 17:37:36 +0200 Subject: [PATCH 4/8] fix: Typo --- tests/integration/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 78d9b9c3ee0..8e7919fa7c4 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -105,7 +105,7 @@ def fail(e): @request.addfinalizer def reraise_test_failures(): if sentry.test_failures: - raise AssertionError(f"Exceptions happened in mini_sentry: {sentry.est_failures}") + raise AssertionError(f"Exceptions happened in mini_sentry: {sentry.test_failures}") server = WSGIServer(application=app, threaded=True) server.start() From 547d2ac06dfe069174fe1dc4d6cf37e94cc30df3 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 5 Sep 2018 09:47:06 +0200 Subject: [PATCH 5/8] feat: Default attributes for static project config --- common/src/config.rs | 4 +++- tests/integration/test_basic.py | 9 +++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/common/src/config.rs b/common/src/config.rs index fbbd3fbc946..37da3ae4e3d 100644 --- a/common/src/config.rs +++ b/common/src/config.rs @@ -310,11 +310,13 @@ pub struct ProjectConfig { #[serde(rename_all = "camelCase")] pub struct StaticProjectState { /// Indicates that the project is disabled. + #[serde(default)] pub disabled: bool, /// A container of known public keys in the project. pub public_keys: HashMap, /// The project's slug if configured. - pub slug: String, + #[serde(default)] + pub slug: Option, /// The project's current config. pub config: ProjectConfig, } diff --git a/tests/integration/test_basic.py b/tests/integration/test_basic.py index 7ac59e58384..8e95cd66d0c 100644 --- a/tests/integration/test_basic.py +++ b/tests/integration/test_basic.py @@ -215,14 +215,15 @@ def get_project_config(): def test_local_project_config(mini_sentry, relay): config = mini_sentry.basic_project_config() - del config['lastChange'] - del config['lastFetch'] - del config['rev'] relay = relay(mini_sentry, { 'projects': { 'configs': { - '42': config + '42': { + # remove defaults to assert they work + 'publicKeys': config['publicKeys'], + 'config': config['config'], + } } } }) From c3b7952e269bce1f37acf96735ec2827dfe9ed0a Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Thu, 6 Sep 2018 10:32:52 +0200 Subject: [PATCH 6/8] fix: Improve error message when no credentials set --- common/src/config.rs | 12 ++++++------ server/src/actors/keys.rs | 8 ++++++++ server/src/actors/project.rs | 8 +++++++- server/src/actors/upstream.rs | 5 ++++- src/cli.rs | 6 ------ src/setup.rs | 16 ++++++++++++---- 6 files changed, 37 insertions(+), 18 deletions(-) diff --git a/common/src/config.rs b/common/src/config.rs index 37da3ae4e3d..7d448473ef2 100644 --- a/common/src/config.rs +++ b/common/src/config.rs @@ -505,18 +505,18 @@ impl Config { } /// Returns the secret key if set. - pub fn secret_key(&self) -> &SecretKey { - &self.credentials.as_ref().unwrap().secret_key + pub fn secret_key(&self) -> Option<&SecretKey> { + self.credentials.as_ref().map(|x| &x.secret_key) } /// Returns the public key if set. - pub fn public_key(&self) -> &PublicKey { - &self.credentials.as_ref().unwrap().public_key + pub fn public_key(&self) -> Option<&PublicKey> { + self.credentials.as_ref().map(|x| &x.public_key) } /// Returns the relay ID. - pub fn relay_id(&self) -> &RelayId { - &self.credentials.as_ref().unwrap().id + pub fn relay_id(&self) -> Option<&RelayId> { + self.credentials.as_ref().map(|x| &x.id) } /// Returns the upstream target as descriptor. diff --git a/server/src/actors/keys.rs b/server/src/actors/keys.rs index 6dbd310fe49..779b139d690 100644 --- a/server/src/actors/keys.rs +++ b/server/src/actors/keys.rs @@ -187,6 +187,14 @@ impl KeyCache { } } + if self.config.credentials().is_none() { + error!( + "No credentials configured. Relay {} cannot send requests to this relay.", + relay_id + ); + return Response::ok((relay_id, None)); + } + debug!("relay {} public key requested", relay_id); if !self.backoff.started() { self.backoff.reset(); diff --git a/server/src/actors/project.rs b/server/src/actors/project.rs index 699d0306ad2..877871e2c6d 100644 --- a/server/src/actors/project.rs +++ b/server/src/actors/project.rs @@ -64,7 +64,13 @@ impl Project { context: &mut Context, ) -> Response, ProjectError> { if let Some(state) = self.config.projects().configs.get(&self.id) { - self.state = Some(Arc::new(ProjectState::FromLocalConfig(state.clone()))); + return Response::ok(Arc::new(ProjectState::FromLocalConfig(state.clone()))); + } + + if self.config.credentials().is_none() { + error!("No credentials configured. Configure project {} in your config.yml", + self.id); + return Response::ok(Arc::new(ProjectState::missing())); } if let Some(ref state) = self.state { diff --git a/server/src/actors/upstream.rs b/server/src/actors/upstream.rs index ac4691aebf5..293c120132c 100644 --- a/server/src/actors/upstream.rs +++ b/server/src/actors/upstream.rs @@ -167,7 +167,10 @@ impl Handler for UpstreamRelay { let credentials = match self.config.credentials() { Some(x) => x, None => { - warn!("no credentials configured, not authenticating."); + warn!( + "no credentials configured, not authenticating. \ + Other relays will not be able to use this relay." + ); return Box::new(fut::err(())); } }; diff --git a/src/cli.rs b/src/cli.rs index 99472534e34..728050f6230 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -298,12 +298,6 @@ pub fn process_event<'a>(matches: &ArgMatches<'a>) -> Result<(), Error> { } pub fn run<'a>(config: Config, _matches: &ArgMatches<'a>) -> Result<(), Error> { - if !config.has_credentials() { - return Err(err_msg( - "relay has no stored credentials. Generate some \ - with \"semaphore credentials generate\" first.", - )); - } setup::dump_spawn_infos(&config); setup::init_metrics(&config)?; semaphore_server::run(config)?; diff --git a/src/setup.rs b/src/setup.rs index 912c4be1e52..229b66cd279 100644 --- a/src/setup.rs +++ b/src/setup.rs @@ -19,15 +19,23 @@ pub fn dump_spawn_infos(config: &Config) { "launching relay from config folder {}", config.path().display() ); - info!(" relay id: {}", config.relay_id()); - info!(" public key: {}", config.public_key()); + + dump_credentials(config); + info!(" log level: {}", config.log_level_filter()); } /// Dumps out credential info. pub fn dump_credentials(config: &Config) { - println!(" relay id: {}", config.relay_id()); - println!(" public key: {}", config.public_key()); + match config.relay_id() { + Some(id) => info!(" relay id: {}", id), + None => info!(" relay id: -"), + }; + + match config.public_key() { + Some(key) => info!(" public key: {}", key), + None => info!(" public key: -"), + }; } /// Initialize the logging system. From ead794e24abcff1ab66df24b6babdeda15a3ca0e Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Fri, 7 Sep 2018 11:11:43 +0200 Subject: [PATCH 7/8] fix: Restore old behavior for dump_credentials --- server/src/actors/project.rs | 6 ++++-- src/setup.rs | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/server/src/actors/project.rs b/server/src/actors/project.rs index accc903e542..4ca9e31ac3d 100644 --- a/server/src/actors/project.rs +++ b/server/src/actors/project.rs @@ -68,8 +68,10 @@ impl Project { } if self.config.credentials().is_none() { - error!("No credentials configured. Configure project {} in your config.yml", - self.id); + error!( + "No credentials configured. Configure project {} in your config.yml", + self.id + ); return Response::ok(Arc::new(ProjectState::missing())); } diff --git a/src/setup.rs b/src/setup.rs index 5357f81c24f..1ccd803e0a7 100644 --- a/src/setup.rs +++ b/src/setup.rs @@ -20,7 +20,14 @@ pub fn dump_spawn_infos(config: &Config) { config.path().display() ); - dump_credentials(config); + match config.relay_id() { + Some(id) => info!(" relay id: {}", id), + None => info!(" relay id: -"), + }; + match config.public_key() { + Some(key) => info!(" public key: {}", key), + None => info!(" public key: -"), + }; info!(" log level: {}", config.log_level_filter()); } @@ -28,13 +35,13 @@ pub fn dump_spawn_infos(config: &Config) { /// Dumps out credential info. pub fn dump_credentials(config: &Config) { match config.relay_id() { - Some(id) => info!(" relay id: {}", id), - None => info!(" relay id: -"), + Some(id) => println!(" relay id: {}", id), + None => println!(" relay id: -"), }; match config.public_key() { - Some(key) => info!(" public key: {}", key), - None => info!(" public key: -"), + Some(key) => println!(" public key: {}", key), + None => println!(" public key: -"), }; } From 1c78c0c797e28c36be27369da77675e33b6615e4 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Fri, 7 Sep 2018 11:13:31 +0200 Subject: [PATCH 8/8] fix: Change test to work with sentry-sdk 0.2 --- tests/integration/test_basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_basic.py b/tests/integration/test_basic.py index 8a0ea23e70f..f2775157e53 100644 --- a/tests/integration/test_basic.py +++ b/tests/integration/test_basic.py @@ -270,7 +270,7 @@ def test_local_project_config(mini_sentry, relay): client = sentry_sdk.Client(relay.dsn, default_integrations=False) hub = sentry_sdk.Hub(client) hub.capture_message("hü") - client.drain_events() + client.close() event = mini_sentry.captured_events.get(timeout=4) assert event["message"] == "hü"