diff --git a/CHANGELOG.md b/CHANGELOG.md index 725d3be11a..eff8f59218 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,10 @@ - Report proper status codes and error messages when sending invalid JSON payloads to an endpoint with a `X-Sentry-Relay-Signature` header. ([#1090](https://github.com/getsentry/relay/pull/1090)) **Internal**: + - Add the exclusive time of the transaction's root span. ([#1083](https://github.com/getsentry/relay/pull/1083)) - Add session.status tag to extracted session.duration metric. ([#1087](https://github.com/getsentry/relay/pull/1087)) +- Serve project configs for batched requests where one of the project keys cannot be parsed. ([#1093](https://github.com/getsentry/relay/pull/1093)) ## 21.9.0 diff --git a/relay-server/src/actors/project_upstream.rs b/relay-server/src/actors/project_upstream.rs index 731299a0dd..258d867aff 100644 --- a/relay-server/src/actors/project_upstream.rs +++ b/relay-server/src/actors/project_upstream.rs @@ -35,13 +35,15 @@ mod _macro { } } -#[derive(Debug, Deserialize, Serialize)] +/// A query to retrieve a batch of project states from upstream. +/// +/// This query does not implement `Deserialize`. To parse the query, use a wrapper that skips +/// invalid project keys instead of failing the entire batch. +#[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] pub struct GetProjectStates { pub public_keys: Vec, - #[serde(default)] pub full_config: bool, - #[serde(default)] pub no_cache: bool, } diff --git a/relay-server/src/endpoints/project_configs.rs b/relay-server/src/endpoints/project_configs.rs index cd3b044131..4471286e7f 100644 --- a/relay-server/src/endpoints/project_configs.rs +++ b/relay-server/src/endpoints/project_configs.rs @@ -9,9 +9,9 @@ use relay_common::ProjectKey; use crate::actors::project::{LimitedProjectState, ProjectState}; use crate::actors::project_cache::{GetProjectState, ProjectCache}; -use crate::actors::project_upstream::GetProjectStates; use crate::extractors::SignedJson; use crate::service::ServiceApp; +use crate::utils::ErrorBoundary; /// Helper to deserialize the `version` query parameter. #[derive(Debug, Deserialize)] @@ -58,14 +58,31 @@ struct GetProjectStatesResponseWrapper { configs: HashMap>, } +/// Request payload of the project config endpoint. +/// +/// This is a replica of [`GetProjectStates`](crate::actors::project_upstream::GetProjectStates) +/// which allows skipping invalid project keys. +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct GetProjectStatesRequest { + public_keys: Vec>, + #[serde(default)] + full_config: bool, + #[serde(default)] + no_cache: bool, +} + fn get_project_configs( - body: SignedJson, + body: SignedJson, ) -> ResponseFuture, Error> { let relay = body.relay; let full = relay.internal && body.inner.full_config; let no_cache = body.inner.no_cache; - let futures = body.inner.public_keys.into_iter().map(move |project_key| { + // Skip unparsable public keys. The downstream Relay will consider them `ProjectState::missing`. + let valid_keys = body.inner.public_keys.into_iter().filter_map(|e| e.ok()); + + let futures = valid_keys.map(move |project_key| { let relay = relay.clone(); ProjectCache::from_registry() .send(GetProjectState::new(project_key).no_cache(no_cache)) diff --git a/relay-server/src/utils/error_boundary.rs b/relay-server/src/utils/error_boundary.rs index d874fb428f..9be97687a2 100644 --- a/relay-server/src/utils/error_boundary.rs +++ b/relay-server/src/utils/error_boundary.rs @@ -59,6 +59,14 @@ impl ErrorBoundary { !self.is_ok() } + #[inline] + pub fn ok(self) -> Option { + match self { + ErrorBoundary::Err(_) => None, + ErrorBoundary::Ok(value) => Some(value), + } + } + #[inline] pub fn unwrap_or_else(self, op: F) -> T where diff --git a/tests/integration/test_projectconfigs.py b/tests/integration/test_projectconfigs.py index 4d846cc354..c860f5da70 100644 --- a/tests/integration/test_projectconfigs.py +++ b/tests/integration/test_projectconfigs.py @@ -2,6 +2,7 @@ Tests the project_configs endpoint (/api/0/relays/projectconfigs/) """ +import json import uuid import pytest from collections import namedtuple @@ -98,7 +99,7 @@ def test_dynamic_relays(mini_sentry, relay, caller, projects): def test_invalid_json(mini_sentry, relay): relay = relay(mini_sentry, wait_healthcheck=True) - body = "{}" # missing the required `public_keys` field + body = {} # missing the required `publicKeys` field packed, signature = SecretKey.parse(relay.secret_key).pack(body) response = relay.post( @@ -119,7 +120,7 @@ def test_invalid_signature(mini_sentry, relay): response = relay.post( "/api/0/relays/projectconfigs/?version=2", - data='{"public_keys":[]}', + data='{"publicKeys":[]}', headers={ "X-Sentry-Relay-Id": relay.relay_id, "X-Sentry-Relay-Signature": "broken", @@ -128,3 +129,31 @@ def test_invalid_signature(mini_sentry, relay): assert response.status_code == 401 # Unauthorized assert "signature" in response.text + + +def test_broken_projectkey(mini_sentry, relay): + relay = relay(mini_sentry, wait_healthcheck=True) + mini_sentry.add_basic_project_config(42) + public_key = mini_sentry.get_dsn_public_key(42) + + body = { + "publicKeys": [ + public_key, # valid + "deadbeef", # wrong length + 42, # wrong type + "/?$äß000000000000000000000000000", # invalid characters + ] + } + packed, signature = SecretKey.parse(relay.secret_key).pack(body) + + response = relay.post( + "/api/0/relays/projectconfigs/?version=2", + data=packed, + headers={ + "X-Sentry-Relay-Id": relay.relay_id, + "X-Sentry-Relay-Signature": signature, + }, + ) + + assert response.ok + assert public_key in response.json()["configs"]