Skip to content

Commit 1410ed7

Browse files
committed
Auto merge of #3892 - pietroalbini:pa-block-endpoints, r=Turbo87
Add `BLOCKED_ROUTES` environment variable This PR adds the `BLOCKED_ROUTES` environment variable, which allows to block individual routes through the environment. This is going to be useful during incident response if we need to block an individual route (for example if the route is degrading the performance of the database), as it would remove the need to deploy an hotfix to disable that route. r? `@jtgeibel`
2 parents b93b03b + bdd4402 commit 1410ed7

File tree

7 files changed

+83
-3
lines changed

7 files changed

+83
-3
lines changed

src/config.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ mod database_pools;
66

77
pub use self::base::Base;
88
pub use self::database_pools::DatabasePools;
9+
use std::collections::HashSet;
910

1011
pub struct Server {
1112
pub base: Base,
@@ -28,6 +29,7 @@ pub struct Server {
2829
pub use_test_database_pool: bool,
2930
pub instance_metrics_log_every_seconds: Option<u64>,
3031
pub force_unconditional_redirects: bool,
32+
pub blocked_routes: HashSet<String>,
3133
}
3234

3335
impl Default for Server {
@@ -58,6 +60,8 @@ impl Default for Server {
5860
/// If the environment variable is not present instance metrics are not logged.
5961
/// - `FORCE_UNCONDITIONAL_REDIRECTS`: Whether to force unconditional redirects in the download
6062
/// endpoint even with a healthy database pool.
63+
/// - `BLOCKED_ROUTES`: A comma separated list of HTTP route patterns that are manually blocked
64+
/// by an operator (e.g. `/crates/:crate_id/:version/download`).
6165
///
6266
/// # Panics
6367
///
@@ -100,6 +104,9 @@ impl Default for Server {
100104
use_test_database_pool: false,
101105
instance_metrics_log_every_seconds: env_optional("INSTANCE_METRICS_LOG_EVERY_SECONDS"),
102106
force_unconditional_redirects: dotenv::var("FORCE_UNCONDITIONAL_REDIRECTS").is_ok(),
107+
blocked_routes: env_optional("BLOCKED_ROUTES")
108+
.map(|routes: String| routes.split(',').map(|s| s.into()).collect())
109+
.unwrap_or_else(HashSet::new),
103110
}
104111
}
105112
}

src/router.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
use std::sync::Arc;
22

33
use conduit::{Handler, HandlerResult, RequestExt};
4-
use conduit_router::{RequestParams, RouteBuilder};
4+
use conduit_router::{RequestParams, RouteBuilder, RoutePattern};
55

66
use crate::controllers::*;
7-
use crate::util::errors::{std_error, AppError};
7+
use crate::middleware::app::RequestApp;
8+
use crate::util::errors::{std_error, AppError, RouteBlocked};
89
use crate::util::EndpointResult;
910
use crate::{App, Env};
1011

@@ -150,6 +151,15 @@ struct C(pub fn(&mut dyn RequestExt) -> EndpointResult);
150151

151152
impl Handler for C {
152153
fn call(&self, req: &mut dyn RequestExt) -> HandlerResult {
154+
// Allow blocking individual routes by their pattern through the `BLOCKED_ROUTES`
155+
// environment variable. This is not in a middleware because we need access to
156+
// `RoutePattern` before executing the response handler.
157+
if let Some(pattern) = req.extensions().find::<RoutePattern>() {
158+
if req.app().config.blocked_routes.contains(pattern.pattern()) {
159+
return Ok(RouteBlocked.response().unwrap());
160+
}
161+
}
162+
153163
let C(f) = *self;
154164
match f(req) {
155165
Ok(resp) => Ok(resp),

src/tests/all.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ use diesel::prelude::*;
3232
mod account_lock;
3333
mod authentication;
3434
mod badge;
35+
mod blocked_routes;
3536
mod builders;
3637
mod categories;
3738
mod category;

src/tests/blocked_routes.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
use crate::builders::{CrateBuilder, VersionBuilder};
2+
use crate::util::{RequestHelper, TestApp};
3+
use conduit::StatusCode;
4+
5+
#[test]
6+
fn test_non_blocked_download_route() {
7+
let (app, anon, user) = TestApp::init()
8+
.with_config(|config| {
9+
config.blocked_routes.clear();
10+
})
11+
.with_user();
12+
13+
app.db(|conn| {
14+
CrateBuilder::new("foo", user.as_model().id)
15+
.version(VersionBuilder::new("1.0.0"))
16+
.expect_build(conn);
17+
});
18+
19+
let status = anon.get::<()>("/api/v1/crates/foo/1.0.0/download").status();
20+
assert_eq!(StatusCode::FOUND, status);
21+
}
22+
23+
#[test]
24+
fn test_blocked_download_route() {
25+
let (app, anon, user) = TestApp::init()
26+
.with_config(|config| {
27+
config.blocked_routes.clear();
28+
config
29+
.blocked_routes
30+
.insert("/crates/:crate_id/:version/download".into());
31+
})
32+
.with_user();
33+
34+
app.db(|conn| {
35+
CrateBuilder::new("foo", user.as_model().id)
36+
.version(VersionBuilder::new("1.0.0"))
37+
.expect_build(conn);
38+
});
39+
40+
let status = anon.get::<()>("/api/v1/crates/foo/1.0.0/download").status();
41+
assert_eq!(StatusCode::SERVICE_UNAVAILABLE, status);
42+
}

src/tests/util/test_app.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use cargo_registry::git::Repository as WorkerRepository;
1414
use diesel::PgConnection;
1515
use git2::Repository as UpstreamRepository;
1616
use reqwest::{blocking::Client, Proxy};
17+
use std::collections::HashSet;
1718
use swirl::Runner;
1819
use url::Url;
1920

@@ -335,6 +336,7 @@ fn simple_config() -> config::Server {
335336
use_test_database_pool: true,
336337
instance_metrics_log_every_seconds: None,
337338
force_unconditional_redirects: false,
339+
blocked_routes: HashSet::new(),
338340
}
339341
}
340342

src/util/errors.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ mod json;
2828
pub use json::TOKEN_FORMAT_ERROR;
2929
pub(crate) use json::{
3030
InsecurelyGeneratedTokenRevoked, MetricsDisabled, NotFound, OwnershipInvitationExpired,
31-
ReadOnlyMode, TooManyRequests,
31+
ReadOnlyMode, RouteBlocked, TooManyRequests,
3232
};
3333

3434
/// Returns an error with status 200 and the provided description as JSON

src/util/errors/json.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,3 +271,21 @@ impl fmt::Display for MetricsDisabled {
271271
f.write_str("Metrics are disabled on this crates.io instance")
272272
}
273273
}
274+
275+
#[derive(Debug)]
276+
pub(crate) struct RouteBlocked;
277+
278+
impl AppError for RouteBlocked {
279+
fn response(&self) -> Option<AppResponse> {
280+
Some(json_error(
281+
&self.to_string(),
282+
StatusCode::SERVICE_UNAVAILABLE,
283+
))
284+
}
285+
}
286+
287+
impl fmt::Display for RouteBlocked {
288+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
289+
f.write_str("This route is temporarily blocked. See https://status.crates.io.")
290+
}
291+
}

0 commit comments

Comments
 (0)