Skip to content

Commit

Permalink
feat: Backport debug_client opt
Browse files Browse the repository at this point in the history
includes latest rust clippy fixes and a sentry bump related to build issues

Closes #779
  • Loading branch information
pjenvey committed Aug 12, 2020
1 parent c5d5d7d commit d899452
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 35 deletions.
107 changes: 89 additions & 18 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ mozsvc-common = "0.1"
protobuf = "2.12"
rand = "0.7"
regex = "1.3"
sentry = { version = "0.18", features = ["with_curl_transport"] }
sentry = { version = "0.19", features = ["with_curl_transport"] }
serde = "1.0"
serde_derive = "1.0"
serde_json = { version = "1.0", features = ["arbitrary_precision"] }
Expand Down
2 changes: 1 addition & 1 deletion src/db/mysql/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ impl MysqlDb {
.bind::<Integer, _>(TOMBSTONE)
.load::<UserCollectionsResult>(&self.conn)?
.into_iter()
.map(|cr| SyncTimestamp::from_i64(cr.last_modified).and_then(|ts| Ok((cr.collection, ts))))
.map(|cr| SyncTimestamp::from_i64(cr.last_modified).map(|ts| (cr.collection, ts)))
.collect::<Result<HashMap<_, _>>>()?;
self.map_collection_names(modifieds)
}
Expand Down
6 changes: 4 additions & 2 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,10 @@ impl ApiError {
name,
ref _tags,
) => {
if description == "size-limit-exceeded" {
return WeaveError::SizeLimitExceeded;
match description.as_ref() {
"over-quota" => return WeaveError::OverQuota,
"size-limit-exceeded" => return WeaveError::SizeLimitExceeded,
_ => {}
}
let name = name.clone().unwrap_or_else(|| "".to_owned());
if *location == RequestErrorLocation::Body
Expand Down
12 changes: 5 additions & 7 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
extern crate slog_scope;

use std::error::Error;
use std::sync::Arc;

use docopt::Docopt;
use serde_derive::Deserialize;
Expand Down Expand Up @@ -36,17 +37,14 @@ async fn main() -> Result<(), Box<dyn Error>> {
// likely grpcio's boringssl
let curl_transport_factory = |options: &sentry::ClientOptions| {
// Note: set options.debug = true when diagnosing sentry issues.
Box::new(sentry::transports::CurlHttpTransport::new(&options))
as Box<dyn sentry::internals::Transport>
Arc::new(sentry::transports::CurlHttpTransport::new(&options))
as Arc<dyn sentry::internals::Transport>
};
let sentry = sentry::init(sentry::ClientOptions {
transport: Box::new(curl_transport_factory),
let _sentry = sentry::init(sentry::ClientOptions {
transport: Some(Arc::new(curl_transport_factory)),
release: sentry::release_name!(),
..sentry::ClientOptions::default()
});
if sentry.is_enabled() {
sentry::integrations::panic::register_panic_handler();
}

// Setup and run the server
let banner = settings.banner();
Expand Down
2 changes: 1 addition & 1 deletion src/server/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ fn test_endpoint(
}
}

fn test_endpoint_with_response<T>(method: http::Method, path: &str, assertions: &dyn Fn(T) -> ())
fn test_endpoint_with_response<T>(method: http::Method, path: &str, assertions: &dyn Fn(T))
where
T: DeserializeOwned,
{
Expand Down
32 changes: 27 additions & 5 deletions src/settings.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Application settings objects and initialization
use std::cmp::min;
use std::str::FromStr;
use std::{cmp::min, collections::HashSet};

use config::{Config, ConfigError, Environment, File};
use serde::{de::Deserializer, Deserialize, Serialize};
Expand Down Expand Up @@ -106,18 +107,33 @@ impl Settings {
}

// Merge the environment overrides
s.merge(Environment::with_prefix(PREFIX))?;
// While the prefix is currently case insensitive, it's traditional that
// environment vars be UPPERCASE, this ensures that will continue should
// Environment ever change their policy about case insensitivity.
// This will accept environment variables specified as
// `SYNC_FOO__BAR_VALUE="gorp"` as `foo.bar_value = "gorp"`
s.merge(Environment::with_prefix(&PREFIX.to_uppercase()).separator("__"))?;

Ok(match s.try_into::<Self>() {
Ok(s) => {
let mut ms = s;
// Adjust the max values if required.
if s.uses_spanner() {
let mut ms = s;
if let Some(uid_list) = &ms.limits.debug_client {
let mut clients: HashSet<u64> = HashSet::new();

for uid in uid_list.split(',') {
u64::from_str(uid.trim())
.map(|v| clients.insert(v))
.expect("Invalid UID specified for debug_client");
}
ms.limits.debug_clients = Some(clients);
}
if ms.uses_spanner() {
ms.limits.max_total_bytes =
min(ms.limits.max_total_bytes, MAX_SPANNER_LOAD_SIZE as u32);
return Ok(ms);
}
s
ms
}
Err(e) => match e {
// Configuration errors are not very sysop friendly, Try to make them
Expand Down Expand Up @@ -179,6 +195,10 @@ pub struct ServerLimits {

/// Maximum BSO count across a batch upload.
pub max_total_records: u32,

// ### debug_client - for testing client
pub debug_client: Option<String>,
pub debug_clients: Option<HashSet<u64>>,
}

impl Default for ServerLimits {
Expand All @@ -191,6 +211,8 @@ impl Default for ServerLimits {
max_request_bytes: DEFAULT_MAX_REQUEST_BYTES,
max_total_bytes: DEFAULT_MAX_TOTAL_BYTES,
max_total_records: DEFAULT_MAX_TOTAL_RECORDS,
debug_client: None,
debug_clients: None,
}
}
}
Expand Down
41 changes: 41 additions & 0 deletions src/web/extractors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,26 @@ impl FromRequest for BsoBodies {
));
}
};

// ### debug_client
if let Some(uids) = &state.limits.debug_clients {
let tested =
HawkIdentifier::uid_from_path(req.uri(), Some(Tags::from_request_head(req.head())))
.unwrap_or(0);
if uids.contains(&tested) {
error!("Returning over quota for {:?}", tested);
return Box::pin(future::err(
ValidationErrorKind::FromDetails(
"over-quota".to_owned(),
RequestErrorLocation::Unknown,
Some("over-quota".to_owned()),
None,
)
.into(),
));
}
}

let max_payload_size = state.limits.max_record_payload_bytes as usize;
let max_post_bytes = state.limits.max_post_bytes as usize;

Expand Down Expand Up @@ -389,6 +409,25 @@ impl FromRequest for BsoBody {
}
};

// ### debug_client
if let Some(uids) = &state.limits.debug_clients {
let tested =
HawkIdentifier::uid_from_path(req.uri(), Some(Tags::from_request_head(req.head())))
.unwrap_or(0);
if uids.contains(&tested) {
error!("Returning over quota for {:?}", tested);
return Box::pin(future::err(
ValidationErrorKind::FromDetails(
"over-quota".to_owned(),
RequestErrorLocation::Unknown,
Some("over-quota".to_owned()),
None,
)
.into(),
));
}
}

let max_payload_size = state.limits.max_record_payload_bytes as usize;

let fut = <Json<BsoBody>>::from_request(&req, payload)
Expand Down Expand Up @@ -957,6 +996,8 @@ impl FromRequest for ConfigRequest {
max_request_bytes: data.max_request_bytes,
max_total_bytes: data.max_total_bytes,
max_total_records: data.max_total_records,
debug_client: data.debug_client.to_owned(),
debug_clients: data.debug_clients.to_owned(),
},
}))
}
Expand Down

0 comments on commit d899452

Please sign in to comment.