Merge remote-tracking branch 'moz/master' into syncstorage-sqlite

Eragonfr · Eragonfr · commit 1c08e8602dd0 · 2024-09-23T01:08:50.000+02:00
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Dockerfile b/Dockerfile
@@ -1,5 +1,5 @@
-# NOTE: Ensure builder's Rust version matches CI's in .circleci/config.yml
-FROM docker.io/lukemathwalker/cargo-chef:0.1.67-rust-1.78-bullseye as chef
+# NOTE: Ensure builder's Rust version matches CI's in .circleci/config.yml # RUST_VER
+FROM docker.io/lukemathwalker/cargo-chef:0.1.67-rust-1.78-bookworm as chef
 WORKDIR /app
 
 FROM chef AS planner
@@ -14,7 +14,7 @@ RUN \
     # Fetch and load the MySQL public key. We need to install libmysqlclient-dev to build syncstorage-rs
     # which wants the mariadb
     wget -qO- https://repo.mysql.com/RPM-GPG-KEY-mysql-2023 > /etc/apt/trusted.gpg.d/mysql.asc && \
-    echo "deb https://repo.mysql.com/apt/debian/ bullseye mysql-8.0" >> /etc/apt/sources.list && \
+    echo "deb https://repo.mysql.com/apt/debian/ bookworm mysql-8.0" >> /etc/apt/sources.list && \
     apt-get -q update && \
     apt-get -q install -y --no-install-recommends libmysqlclient-dev cmake
 
@@ -31,14 +31,14 @@ COPY --from=cacher $CARGO_HOME /app/$CARGO_HOME
 RUN \
     # Fetch and load the MySQL public key
     wget -qO- https://repo.mysql.com/RPM-GPG-KEY-mysql-2023 > /etc/apt/trusted.gpg.d/mysql.asc && \
-    echo "deb https://repo.mysql.com/apt/debian/ bullseye mysql-8.0" >> /etc/apt/sources.list && \
+    echo "deb https://repo.mysql.com/apt/debian/ bookworm mysql-8.0" >> /etc/apt/sources.list && \
     # mysql_pubkey.asc from:
     # https://dev.mysql.com/doc/refman/8.0/en/checking-gpg-signature.html
     # related:
     # https://dev.mysql.com/doc/mysql-apt-repo-quick-guide/en/#repo-qg-apt-repo-manual-setup
     apt-get -q update && \
-    apt-get -q install -y --no-install-recommends libmysqlclient-dev cmake golang-go python3-dev python3-pip python3-setuptools python3-wheel && \
-    pip3 install -r requirements.txt && \
+    apt-get -q install -y --no-install-recommends libmysqlclient-dev cmake golang-go pkg-config python3-dev python3-pip python3-setuptools python3-wheel && \
+    pip3 install --break-system-packages -r /app/requirements.txt && \
     rm -rf /var/lib/apt/lists/*
 
 ENV PATH=$PATH:/root/.cargo/bin
@@ -49,7 +49,7 @@ RUN \
     cargo install --path ./syncserver --no-default-features --features=$DATABASE_BACKEND,py_verifier --locked --root /app && \
     if [ "$DATABASE_BACKEND" = "spanner" ] ; then cargo install --path ./syncstorage-spanner --locked --root /app --bin purge_ttl ; fi
 
-FROM docker.io/library/debian:bullseye-slim
+FROM docker.io/library/debian:bookworm-slim
 WORKDIR /app
 COPY --from=builder /app/requirements.txt /app
 # Due to a build error that occurs with the Python cryptography package, we
@@ -69,17 +69,17 @@ RUN \
     apt-get -q update && \
     # and ca-certificates needed for https://repo.mysql.com
     apt-get install -y gnupg ca-certificates wget && \
+    echo "deb https://repo.mysql.com/apt/debian/ bookworm mysql-8.0" >> /etc/apt/sources.list && \
     # Fetch and load the MySQL public key
-    echo "deb https://repo.mysql.com/apt/debian/ bullseye mysql-8.0" >> /etc/apt/sources.list && \
     wget -qO- https://repo.mysql.com/RPM-GPG-KEY-mysql-2023 > /etc/apt/trusted.gpg.d/mysql.asc && \
     # update again now that we trust repo.mysql.com
     apt-get -q update && \
-    apt-get -q install -y build-essential libmysqlclient-dev libssl-dev libffi-dev libcurl4 python3-dev python3-pip python3-setuptools python3-wheel cargo curl jq && \
+    apt-get -q install -y build-essential libmysqlclient-dev libssl-dev libffi-dev libcurl4 pkg-config python3-dev python3-pip python3-setuptools python3-wheel cargo curl jq && \
     # The python3-cryptography debian package installs version 2.6.1, but we
     # we want to use the version specified in requirements.txt. To do this,
     # we have to remove the python3-cryptography package here.
     apt-get -q remove -y python3-cryptography && \
-    pip3 install -r /app/requirements.txt && \
+    pip3 install --break-system-packages -r /app/requirements.txt && \
     rm -rf /var/lib/apt/lists/*
 
 COPY --from=builder /app/bin /app/bin
@@ -92,8 +92,10 @@ COPY --from=builder /app/scripts/start_mock_fxa_server.sh /app/scripts/start_moc
 COPY --from=builder /app/syncstorage-spanner/src/schema.ddl /app/schema.ddl
 
 RUN chmod +x /app/scripts/prepare-spanner.sh
-RUN pip3 install -r /app/tools/integration_tests/requirements.txt
-RUN pip3 install -r /app/tools/tokenserver/requirements.txt
+RUN \
+    pip3 install --break-system-packages -r /app/tools/integration_tests/requirements.txt
+RUN \
+    pip3 install --break-system-packages -r /app/tools/tokenserver/requirements.txt
 
 USER app:app
 
diff --git a/syncserver-common/Cargo.toml b/syncserver-common/Cargo.toml
@@ -6,13 +6,16 @@ authors.workspace = true
 edition.workspace = true
 
 [dependencies]
+actix-web.workspace = true
 backtrace.workspace = true
 cadence.workspace = true
 futures.workspace = true
+futures-util.workspace = true
 sha2.workspace = true
+sentry.workspace = true
+sentry-backtrace.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 slog.workspace = true
 slog-scope.workspace = true
-actix-web.workspace = true
 hkdf.workspace = true
diff --git a/syncserver-common/src/lib.rs b/syncserver-common/src/lib.rs
@@ -2,6 +2,8 @@
 extern crate slog_scope;
 
 mod metrics;
+pub mod middleware;
+mod tags;
 
 use std::{
     fmt,
@@ -15,6 +17,7 @@ use serde_json::Value;
 use sha2::Sha256;
 
 pub use metrics::{metrics_from_opts, MetricError, Metrics};
+pub use tags::Taggable;
 
 // header statics must be lower case, numbers and symbols per the RFC spec. This reduces chance of error.
 pub static X_LAST_MODIFIED: &str = "x-last-modified";
@@ -60,7 +63,7 @@ macro_rules! impl_fmt_display {
     };
 }
 
-pub trait ReportableError: std::fmt::Display {
+pub trait ReportableError: std::fmt::Display + std::fmt::Debug {
     /// Like [Error::source] but returns the source (if any) of this error as a
     /// [ReportableError] if it implements the trait. Otherwise callers of this
     /// method will likely subsequently call [Error::source] to return the
diff --git a/syncserver-common/src/middleware/mod.rs b/syncserver-common/src/middleware/mod.rs
@@ -0,0 +1 @@
+pub mod sentry;
diff --git a/syncserver-common/src/middleware/sentry.rs b/syncserver-common/src/middleware/sentry.rs
@@ -9,8 +9,7 @@ use futures::{future::LocalBoxFuture, FutureExt};
 use futures_util::future::{ok, Ready};
 use sentry::{protocol::Event, Hub};
 
-use crate::server::tags::Taggable;
-use syncserver_common::ReportableError;
+use crate::{ReportableError, Taggable};
 
 #[derive(Clone)]
 pub struct SentryWrapper<E> {
@@ -249,11 +248,11 @@ pub fn event_from_error(
 ///
 /// Based moreso on sentry_failure's `exception_from_single_fail`. Includes a
 /// stacktrace if available.
-fn exception_from_reportable_error(err: &dyn ReportableError) -> sentry::protocol::Exception {
-    let dbg = format!("{:?}", &err.to_string());
+pub fn exception_from_reportable_error(err: &dyn ReportableError) -> sentry::protocol::Exception {
+    let dbg = format!("{:?}", &err);
     sentry::protocol::Exception {
         ty: sentry::parse_type_from_debug(&dbg).to_owned(),
-        value: Some(dbg),
+        value: Some(err.to_string()),
         stacktrace: err
             .backtrace()
             .map(sentry_backtrace::backtrace_to_stacktrace)
diff --git a/syncserver-common/src/tags.rs b/syncserver-common/src/tags.rs
diff --git a/syncserver/Cargo.toml b/syncserver/Cargo.toml
@@ -14,15 +14,13 @@ cadence.workspace = true
 chrono.workspace = true
 docopt.workspace = true
 futures.workspace = true
-futures-util.workspace = true
 hex.workspace = true
 hostname.workspace = true
 http.workspace = true
 lazy_static.workspace = true
 rand.workspace = true
 regex.workspace = true
 sentry.workspace = true
-sentry-backtrace.workspace = true
 serde.workspace = true
 serde_derive.workspace = true
 serde_json.workspace = true
diff --git a/syncserver/src/server/mod.rs b/syncserver/src/server/mod.rs
@@ -13,17 +13,15 @@ use actix_web::{
 };
 use cadence::{Gauged, StatsdClient};
 use futures::future::{self, Ready};
-use syncserver_common::{BlockingThreadpool, Metrics};
+use syncserver_common::{middleware::sentry::SentryWrapper, BlockingThreadpool, Metrics, Taggable};
 use syncserver_db_common::{GetPoolState, PoolState};
 use syncserver_settings::Settings;
 use syncstorage_db::{DbError, DbPool, DbPoolImpl};
 use syncstorage_settings::{Deadman, ServerLimits};
 use tokio::{sync::RwLock, time};
 
 use crate::error::ApiError;
-use crate::server::tags::Taggable;
 use crate::tokenserver;
-use crate::web::middleware::sentry::SentryWrapper;
 use crate::web::{handlers, middleware};
 
 pub const BSO_ID_REGEX: &str = r"[ -~]{1,64}";
@@ -33,7 +31,6 @@ pub const SYNC_DOCS_URL: &str =
 const MYSQL_UID_REGEX: &str = r"[0-9]{1,10}";
 const SYNC_VERSION_PATH: &str = "1.5";
 
-pub mod tags;
 #[cfg(test)]
 mod test;
 pub mod user_agent;
@@ -197,7 +194,7 @@ macro_rules! build_app_without_syncstorage {
             // Middleware is applied LIFO
             // These will wrap all outbound responses with matching status codes.
             .wrap(ErrorHandlers::new().handler(StatusCode::NOT_FOUND, ApiError::render_404))
-            .wrap(SentryWrapper::<ApiError>::new(
+            .wrap(SentryWrapper::<tokenserver_common::TokenserverError>::new(
                 $metrics.clone(),
                 "api_error".to_owned(),
             ))
diff --git a/syncserver/src/tokenserver/extractors.rs b/syncserver/src/tokenserver/extractors.rs
@@ -21,12 +21,13 @@ use lazy_static::lazy_static;
 use regex::Regex;
 use serde::Deserialize;
 use sha2::Sha256;
+use syncserver_common::Taggable;
 use syncserver_settings::Secrets;
 use tokenserver_common::{ErrorLocation, NodeType, TokenserverError};
 use tokenserver_db::{params, results, Db, DbPool};
 
 use super::{LogItemsMutator, ServerState, TokenserverMetrics};
-use crate::server::{tags::Taggable, MetricsWrapper};
+use crate::server::MetricsWrapper;
 
 lazy_static! {
     static ref CLIENT_STATE_REGEX: Regex = Regex::new("^[a-zA-Z0-9._-]{1,32}$").unwrap();
diff --git a/syncserver/src/web/extractors.rs b/syncserver/src/web/extractors.rs
@@ -26,7 +26,7 @@ use serde::{
     Deserialize, Serialize,
 };
 use serde_json::Value;
-use syncserver_common::{Metrics, X_WEAVE_RECORDS};
+use syncserver_common::{Metrics, Taggable, X_WEAVE_RECORDS};
 use syncstorage_db::{
     params::{self, PostCollectionBso},
     DbError, DbPool, Sorting, SyncTimestamp, UserIdentifier,
@@ -36,9 +36,7 @@ use validator::{Validate, ValidationError};
 
 use crate::error::{ApiError, ApiErrorKind};
 use crate::label;
-use crate::server::{
-    tags::Taggable, MetricsWrapper, ServerState, BSO_ID_REGEX, COLLECTION_ID_REGEX,
-};
+use crate::server::{MetricsWrapper, ServerState, BSO_ID_REGEX, COLLECTION_ID_REGEX};
 use crate::web::{
     auth::HawkPayload,
     error::{HawkErrorKind, ValidationErrorKind},
diff --git a/syncserver/src/web/middleware/mod.rs b/syncserver/src/web/middleware/mod.rs
@@ -1,5 +1,4 @@
 pub mod rejectua;
-pub mod sentry;
 pub mod weave;
 
 // # Web Middleware
diff --git a/syncserver/src/web/transaction.rs b/syncserver/src/web/transaction.rs
@@ -7,11 +7,10 @@ use actix_web::web::Data;
 use actix_web::{FromRequest, HttpRequest, HttpResponse};
 use futures::future::LocalBoxFuture;
 use futures::FutureExt;
-use syncserver_common::X_LAST_MODIFIED;
+use syncserver_common::{Taggable, X_LAST_MODIFIED};
 use syncstorage_db::{params, results::ConnectionInfo, Db, DbError, DbPool, UserIdentifier};
 
 use crate::error::{ApiError, ApiErrorKind};
-use crate::server::tags::Taggable;
 use crate::server::{MetricsWrapper, ServerState};
 use crate::web::extractors::{
     BsoParam, CollectionParam, HawkIdentifier, PreConditionHeader, PreConditionHeaderOpt,
diff --git a/syncstorage-spanner/src/manager/session.rs b/syncstorage-spanner/src/manager/session.rs
@@ -213,7 +213,6 @@ pub async fn recycle_spanner_session(
                 let age = now - conn.create_time;
                 if age > max_life as i64 {
                     metrics.incr("db.connection.max_life");
-                    dbg!("### aging out", this_session.get_name());
                     return Err(DbError::expired());
                 }
             }
@@ -230,7 +229,6 @@ pub async fn recycle_spanner_session(
                     .unwrap_or_default();
                 if idle > max_idle as i64 {
                     metrics.incr("db.connection.max_idle");
-                    dbg!("### idling out", this_session.get_name());
                     return Err(DbError::expired());
                 }
                 // and update the connection's reference session info
diff --git a/tokenserver-auth/src/oauth/verify.py b/tokenserver-auth/src/oauth/verify.py
@@ -16,6 +16,4 @@ def verify_token(self, token):
             # Serialize the data to make it easier to parse in Rust
             return json.dumps(token_data)
         except (ClientError, TrustError):
-            # XXX: debugging
-            #return None
-            raise
+            return None
diff --git a/tokenserver-common/src/error.rs b/tokenserver-common/src/error.rs
@@ -313,3 +313,20 @@ impl InternalError for TokenserverError {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::TokenserverError;
+    use syncserver_common::middleware::sentry::exception_from_reportable_error;
+
+    #[test]
+    fn sentry_event() {
+        let err = TokenserverError {
+            context: "OAuth verification timeout".to_owned(),
+            ..TokenserverError::resource_unavailable()
+        };
+        let exc = exception_from_reportable_error(&err);
+        assert_eq!(exc.ty, "TokenserverError");
+        assert_eq!(exc.value, Some(err.context));
+    }
+}
diff --git a/tools/integration_tests/requirements.txt b/tools/integration_tests/requirements.txt
@@ -1,4 +1,4 @@
-cryptography==42.0.8
+cryptography==43.0.1
 hawkauthlib
 konfig
 mysqlclient
diff --git a/tools/tokenserver/process_account_events.py b/tools/tokenserver/process_account_events.py
@@ -115,22 +115,24 @@ def process_account_event(database, body, metrics=None):
     else:
         if email is not None:
             record_metric = True
-            if event_type == "delete":
-                # Mark the user as retired.
-                # Actual cleanup is done by a separate process.
-                logger.info("Processing account delete for %r", email)
-                database.retire_user(email)
-            elif event_type == "reset":
-                logger.info("Processing account reset for %r", email)
-                update_generation_number(
-                    database, email, generation, metrics=metrics)
-            elif event_type == "passwordChange":
-                logger.info("Processing password change for %r", email)
-                update_generation_number(
-                    database, email, generation, metrics=metrics)
-            else:
-                record_metric = False
-                logger.warning("Dropping unknown event type %r", event_type)
+            match event_type:
+                case "delete":
+                    # Mark the user as retired.
+                    # Actual cleanup is done by a separate process.
+                    logger.info("Processing account delete for %r", email)
+                    database.retire_user(email)
+                case "reset":
+                    logger.info("Processing account reset for %r", email)
+                    update_generation_number(
+                        database, email, generation, metrics=metrics)
+                case "passwordChange":
+                    logger.info("Processing password change for %r", email)
+                    update_generation_number(
+                        database, email, generation, metrics=metrics)
+                case _:
+                    record_metric = False
+                    logger.warning("Dropping unknown event type %r",
+                                   event_type)
             if record_metric and metrics:
                 metrics.incr(event_type)
 
diff --git a/tools/tokenserver/util.py b/tools/tokenserver/util.py
@@ -105,9 +105,9 @@ def __init__(self, opts, namespace=""):
             namespace=namespace,
             statsd_namespace=namespace,
             statsd_host=getattr(
-                opts, "metric_host", os.environ.get("METRIC_HOST")),
+                opts, "metric_host", os.environ.get("SYNC_STATSD_HOST")),
             statsd_port=getattr(
-                opts, "metric_port", os.environ.get("METRIC_PORT")),
+                opts, "metric_port", os.environ.get("SYNC_STATSD_PORT")),
         )
         self.prefix = options.get("namespace")
         initialize(**options)
