Skip to content

Commit

Permalink
remove stuff related to error synthesis
Browse files Browse the repository at this point in the history
  • Loading branch information
hawkw committed Aug 30, 2023
1 parent b90c5a9 commit aa79e92
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 67 deletions.
2 changes: 1 addition & 1 deletion linkerd/app/core/src/errors/respond.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub struct SyntheticHttpResponse {
location: Option<HeaderValue>,
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
#[derive(Copy, Clone, Debug)]
pub struct EmitHeaders(pub bool);

#[derive(Clone, Debug)]
Expand Down
13 changes: 1 addition & 12 deletions linkerd/app/outbound/src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ pub use linkerd_app_core::proxy::http::{self as http, *};
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct Http<T> {
target: T,
error_headers: errors::respond::EmitHeaders,
}

pub fn spawn_routes<T>(
Expand Down Expand Up @@ -118,12 +117,8 @@ impl<N> Outbound<N> {
.push_http_concrete(resolve)
.push_http_logical()
.map_stack(move |config, _, stk| {
let error_headers = config.emit_headers;
stk.push_new_idle_cached(config.discovery_idle_timeout)
.push_map_target(move |target| Http {
target,
error_headers: errors::respond::EmitHeaders(error_headers),
})
.push_map_target(|target| Http { target })
.push(svc::ArcNewService::layer())
})
}
Expand All @@ -148,9 +143,3 @@ where
self.target.param()
}
}

impl<T> svc::Param<errors::respond::EmitHeaders> for Http<T> {
fn param(&self) -> errors::respond::EmitHeaders {
self.error_headers
}
}
12 changes: 0 additions & 12 deletions linkerd/app/outbound/src/http/logical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use super::concrete;
use crate::{BackendRef, EndpointRef, Outbound, OutboundMetrics, ParentRef};
use linkerd_app_core::{
errors,
proxy::{api_resolve::Metadata, http},
svc,
transport::addrs::*,
Expand Down Expand Up @@ -98,7 +97,6 @@ impl<N> Outbound<N> {
where
// Logical target.
T: svc::Param<watch::Receiver<Routes>>,
T: svc::Param<errors::respond::EmitHeaders>,
T: Eq + Hash + Clone + Debug + Send + Sync + 'static,
// Concrete stack.
N: svc::NewService<Concrete<T>, Service = NSvc> + Clone + Send + Sync + 'static,
Expand Down Expand Up @@ -132,7 +130,6 @@ impl<N> Outbound<N> {
impl<T> RouterParams<T>
where
T: Clone + Debug + Eq + Hash + Send + Sync + 'static,
T: svc::Param<errors::respond::EmitHeaders>,
{
fn layer<N, S>(
metrics: OutboundMetrics,
Expand Down Expand Up @@ -283,15 +280,6 @@ impl<T> svc::Param<policy::FailureAccrual> for Concrete<T> {
}
}

impl<T> svc::Param<errors::respond::EmitHeaders> for Concrete<T>
where
T: svc::Param<errors::respond::EmitHeaders>,
{
fn param(&self) -> errors::respond::EmitHeaders {
self.parent.param()
}
}

// === impl CanonicalDstHeader ===

impl From<CanonicalDstHeader> for http::HeaderPair {
Expand Down
1 change: 0 additions & 1 deletion linkerd/app/outbound/src/http/logical/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ where
// Parent target type.
T: Debug + Eq + Hash,
T: Clone + Send + Sync + 'static,
T: svc::Param<linkerd_app_core::errors::respond::EmitHeaders>,
{
/// Builds a stack that dynamically updates and applies HTTP or gRPC policy
/// routing configurations to route requests over cached inner backend
Expand Down
3 changes: 1 addition & 2 deletions linkerd/app/outbound/src/http/logical/policy/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use super::{
};
use crate::{http::retry, BackendRef, EndpointRef, ParentRef, RouteRef};
use linkerd_app_core::{
classify, errors, proxy::http, svc, transport::addrs::*, Addr, Error, NameAddr, Result,
classify, proxy::http, svc, transport::addrs::*, Addr, Error, NameAddr, Result,
};
use linkerd_distribute as distribute;
use linkerd_http_route as http_route;
Expand Down Expand Up @@ -47,7 +47,6 @@ impl<T, M, F, E> Router<T, M, F, E>
where
// Parent target type.
T: Clone + Debug + Eq + Hash + Send + Sync + 'static,
T: svc::Param<errors::respond::EmitHeaders>,
// Request matcher.
M: http_route::Match,
M: Clone + Send + Sync + 'static,
Expand Down
35 changes: 7 additions & 28 deletions linkerd/app/outbound/src/http/logical/policy/tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use super::{super::concrete, *};
use crate::ParentRef;
use linkerd_app_core::{
errors,
svc::NewService,
svc::{Layer, ServiceExt},
trace,
Expand Down Expand Up @@ -61,7 +60,7 @@ async fn header_based_route() {
// Stack that produces mock services.
let (inner_default, mut default) = tower_test::mock::pair();
let (inner_special, mut special) = tower_test::mock::pair();
let inner = move |concrete: Concrete<Target>| {
let inner = move |concrete: Concrete<()>| {
if let concrete::Dispatch::Balance(ref addr, ..) = concrete.target {
if addr
.name()
Expand Down Expand Up @@ -121,17 +120,13 @@ async fn header_based_route() {
let metrics = RouteBackendMetrics::default();
let router = Policy::layer(metrics.clone())
.layer(inner)
.new_service(Policy::from((routes, Target)));
.new_service(Policy::from((routes, ())));

default.allow(1);
special.allow(1);

let mut req = http::Request::builder()
let req = http::Request::builder()
.body(http::BoxBody::default())
.unwrap();
let (client_handle, _close) = http::ClientHandle::new(([127, 0, 0, 1], 9999).into());
req.extensions_mut().insert(client_handle);

let _ = tokio::select! {
biased;
_ = router.clone().oneshot(req) => panic!("unexpected response"),
Expand All @@ -142,13 +137,10 @@ async fn header_based_route() {

default.allow(1);
special.allow(1);
let mut req = http::Request::builder()
let req = http::Request::builder()
.header("x-special", "true")
.body(http::BoxBody::default())
.unwrap();
let (client_handle, _close) = http::ClientHandle::new(([127, 0, 0, 1], 9999).into());
req.extensions_mut().insert(client_handle);

let _ = tokio::select! {
biased;
_ = router.clone().oneshot(req) => panic!("unexpected response"),
Expand Down Expand Up @@ -191,7 +183,7 @@ async fn http_filter_request_headers() {

// Stack that produces mock services.
let (inner, mut handle) = tower_test::mock::pair();
let inner = move |_: Concrete<Target>| inner.clone();
let inner = move |_: Concrete<()>| inner.clone();

// Routes that configure a special header-based route and a default route.
static PIZZA: http::HeaderName = http::HeaderName::from_static("pizza");
Expand Down Expand Up @@ -240,17 +232,13 @@ async fn http_filter_request_headers() {

let router = Policy::layer(Default::default())
.layer(inner)
.new_service(Policy::from((routes, Target)));
.new_service(Policy::from((routes, ())));

handle.allow(1);

let mut req = http::Request::builder()
let req = http::Request::builder()
.header(&PIZZA, &PARTY)
.body(http::BoxBody::default())
.unwrap();
let (client_handle, _close) = http::ClientHandle::new(([127, 0, 0, 1], 9999).into());
req.extensions_mut().insert(client_handle);

let (req, _rsp) = tokio::select! {
biased;
_ = router.clone().oneshot(req) => panic!("unexpected response"),
Expand All @@ -266,12 +254,3 @@ async fn http_filter_request_headers() {
// Hold the router to prevent inner services from being dropped.
drop(router);
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
struct Target;

impl svc::Param<errors::respond::EmitHeaders> for Target {
fn param(&self) -> errors::respond::EmitHeaders {
errors::respond::EmitHeaders(true)
}
}
19 changes: 8 additions & 11 deletions linkerd/app/outbound/src/http/logical/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,23 +416,20 @@ async fn backend_request_timeout() {
// Still send a response, so that if we didn't hit the backend timeout
// timeout, we don't hit the route timeout and succeed incorrectly.
send_rsp.send_response(mk_rsp(StatusCode::OK, "good"));
// This timeout returns a 504 response, rather than an error, because they
// must be mapped to synthesized HTTP responses in the backend stack for
// retries.
assert_eq!(
rsp.await.expect("request must succeed with a 504").status(),
http::StatusCode::GATEWAY_TIMEOUT
);
let error = rsp.await.expect_err("request must fail with a timeout");
assert!(errors::is_caused_by::<http::timeout::ResponseTimeoutError>(
error.as_ref()
));

// The route request timeout should still apply to time spent before
// the backend is acquired.
let rsp = send_req(svc.clone(), http::Request::get("/"));
tokio::time::sleep(ROUTE_REQUEST_TIMEOUT + Duration::from_millis(1)).await;
handle.allow(1);
assert_eq!(
rsp.await.expect("request must succeed with a 504").status(),
http::StatusCode::GATEWAY_TIMEOUT
);
let error = rsp.await.expect_err("request must fail with a timeout");
assert!(errors::is_caused_by::<http::timeout::ResponseTimeoutError>(
error.as_ref()
));
}

#[derive(Clone, Debug)]
Expand Down

0 comments on commit aa79e92

Please sign in to comment.