Skip to content

Commit 33513c4

Browse files
committed
Refactor backend logic with an Origin struct
I feel like it's going nowhere... Backend logic is messy and unclear, buffers are needlessly kept alive, borrowing subsets of the HTTP session is harder and harder, reseting default answers request is way too hard for what it's worth (it's probably broken right now), gauges are desynched and we are cloning again and again String ids... Signed-off-by: Eloi DEMOLIS <[email protected]>
1 parent 6110de3 commit 33513c4

File tree

14 files changed

+360
-377
lines changed

14 files changed

+360
-377
lines changed

command/assets/custom_200.html

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
HTTP/1.1 200 OK
2-
%Content-Length: %CONTENT_LENGTH
32
Sozu-Id: %REQUEST_ID
43

54
<h1>%CLUSTER_ID Custom 200</h1>

command/assets/custom_404.html

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
HTTP/1.1 404 Not Found
22
Cache-Control: no-cache
3-
Connection: close
43
Sozu-Id: %REQUEST_ID
54

65
<h1>My own 404 error page</h1>

command/assets/custom_503.html

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
HTTP/1.1 503 Service Unavailable
22
Cache-Control: no-cache
33
Connection: close
4-
%Content-Length: %CONTENT_LENGTH
54
Sozu-Id: %REQUEST_ID
65

76
<h1>MyCluster: 503 Service Unavailable</h1>

command/src/state.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1724,6 +1724,7 @@ mod tests {
17241724
hostname: String::from("test.local"),
17251725
path: PathRule::prefix(String::from("/abc")),
17261726
address: SocketAddress::new_v4(0, 0, 0, 0, 8080),
1727+
required_auth: Some(false),
17271728
redirect: Some(RedirectPolicy::Forward.into()),
17281729
redirect_scheme: Some(RedirectScheme::UseSame.into()),
17291730
..Default::default()

e2e/src/http_utils/mod.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,17 @@ pub fn http_request<S1: Into<String>, S2: Into<String>, S3: Into<String>, S4: In
2424
)
2525
}
2626

27-
pub fn immutable_answer(status: u16) -> String {
27+
pub fn immutable_answer(status: u16, content_length: bool) -> String {
28+
let content_length = if content_length {
29+
"\r\nContent-Length: 0"
30+
} else {
31+
""
32+
};
2833
match status {
29-
400 => String::from("HTTP/1.1 400 Bad Request\r\nCache-Control: no-cache\r\nConnection: close\r\n\r\n"),
30-
404 => String::from("HTTP/1.1 404 Not Found\r\nCache-Control: no-cache\r\nConnection: close\r\n\r\n"),
31-
502 => String::from("HTTP/1.1 502 Bad Gateway\r\nCache-Control: no-cache\r\nConnection: close\r\n\r\n"),
32-
503 => String::from("HTTP/1.1 503 Service Unavailable\r\nCache-Control: no-cache\r\nConnection: close\r\n\r\n"),
34+
400 => format!("HTTP/1.1 400 Bad Request\r\nCache-Control: no-cache\r\nConnection: close{content_length}\r\n\r\n"),
35+
404 => format!("HTTP/1.1 404 Not Found\r\nCache-Control: no-cache\r\nConnection: close{content_length}\r\n\r\n"),
36+
502 => format!("HTTP/1.1 502 Bad Gateway\r\nCache-Control: no-cache\r\nConnection: close{content_length}\r\n\r\n"),
37+
503 => format!("HTTP/1.1 503 Service Unavailable\r\nCache-Control: no-cache\r\nConnection: close{content_length}\r\n\r\n"),
3338
_ => unimplemented!()
3439
}
3540
}

e2e/src/tests/tests.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -645,10 +645,10 @@ fn try_http_behaviors() -> State {
645645
.to_http(None)
646646
.unwrap();
647647
http_config.answers = BTreeMap::from([
648-
("400".to_string(), immutable_answer(400)),
649-
("404".to_string(), immutable_answer(404)),
650-
("502".to_string(), immutable_answer(502)),
651-
("503".to_string(), immutable_answer(503)),
648+
("400".to_string(), immutable_answer(400, false)),
649+
("404".to_string(), immutable_answer(404, false)),
650+
("502".to_string(), immutable_answer(502, false)),
651+
("503".to_string(), immutable_answer(503, false)),
652652
]);
653653

654654
worker.send_proxy_request_type(RequestType::AddHttpListener(http_config));
@@ -671,7 +671,7 @@ fn try_http_behaviors() -> State {
671671

672672
let response = client.receive();
673673
println!("response: {response:?}");
674-
assert_eq!(response, Some(immutable_answer(404)));
674+
assert_eq!(response, Some(immutable_answer(404, true)));
675675
assert_eq!(client.receive(), None);
676676

677677
worker.send_proxy_request_type(RequestType::AddHttpFrontend(RequestHttpFrontend {
@@ -686,7 +686,7 @@ fn try_http_behaviors() -> State {
686686

687687
let response = client.receive();
688688
println!("response: {response:?}");
689-
assert_eq!(response, Some(immutable_answer(503)));
689+
assert_eq!(response, Some(immutable_answer(503, true)));
690690
assert_eq!(client.receive(), None);
691691

692692
let back_address = create_local_address();
@@ -706,7 +706,7 @@ fn try_http_behaviors() -> State {
706706

707707
let response = client.receive();
708708
println!("response: {response:?}");
709-
assert_eq!(response, Some(immutable_answer(400)));
709+
assert_eq!(response, Some(immutable_answer(400, true)));
710710
assert_eq!(client.receive(), None);
711711

712712
let mut backend = SyncBackend::new("backend", back_address, "TEST\r\n\r\n");
@@ -723,7 +723,7 @@ fn try_http_behaviors() -> State {
723723
let response = client.receive();
724724
println!("request: {request:?}");
725725
println!("response: {response:?}");
726-
assert_eq!(response, Some(immutable_answer(502)));
726+
assert_eq!(response, Some(immutable_answer(502, true)));
727727
assert_eq!(client.receive(), None);
728728

729729
info!("expecting 200");
@@ -786,7 +786,7 @@ fn try_http_behaviors() -> State {
786786
let response = client.receive();
787787
println!("request: {request:?}");
788788
println!("response: {response:?}");
789-
assert_eq!(response, Some(immutable_answer(503)));
789+
assert_eq!(response, Some(immutable_answer(503, true)));
790790
assert_eq!(client.receive(), None);
791791

792792
worker.send_proxy_request_type(RequestType::RemoveBackend(RemoveBackend {
@@ -984,7 +984,7 @@ fn try_https_redirect() -> State {
984984
client.connect();
985985
client.send();
986986
let answer = client.receive();
987-
let expected_answer = format!("{answer_301_prefix}https://example.com/redirected?true\r\n\r\n");
987+
let expected_answer = format!("{answer_301_prefix}https://example.com/redirected?true\r\nContent-Length: 0\r\n\r\n");
988988
assert_eq!(answer, Some(expected_answer));
989989

990990
State::Success

lib/src/http.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -209,11 +209,14 @@ impl HttpSession {
209209
}
210210
}
211211

212-
fn upgrade_http(&mut self, http: Http<TcpStream, HttpListener>) -> Option<HttpStateMachine> {
212+
fn upgrade_http(
213+
&mut self,
214+
mut http: Http<TcpStream, HttpListener>,
215+
) -> Option<HttpStateMachine> {
213216
debug!("http switching to ws");
214-
let front_token = self.frontend_token;
215-
let back_token = match http.backend_token {
216-
Some(back_token) => back_token,
217+
let frontend_token = self.frontend_token;
218+
let origin = match http.origin.take() {
219+
Some(origin) => origin,
217220
None => {
218221
warn!(
219222
"Could not upgrade http request on cluster '{:?}' ({:?}) using backend '{:?}' into websocket for request '{}'",
@@ -223,7 +226,7 @@ impl HttpSession {
223226
}
224227
};
225228

226-
let ws_context = http.websocket_context();
229+
let websocket_context = http.websocket_context();
227230
let mut container_frontend_timeout = http.container_frontend_timeout;
228231
let mut container_backend_timeout = http.container_backend_timeout;
229232
container_frontend_timeout.reset();
@@ -237,25 +240,25 @@ impl HttpSession {
237240

238241
let mut pipe = Pipe::new(
239242
backend_buffer,
240-
http.context.backend_id,
241-
http.backend_socket,
242-
http.backend,
243+
Some(origin.backend_id),
244+
Some(origin.socket),
245+
Some(origin.backend),
243246
Some(container_backend_timeout),
244247
Some(container_frontend_timeout),
245248
http.context.cluster_id,
246249
http.request_stream.storage.buffer,
247-
front_token,
250+
frontend_token,
248251
http.frontend_socket,
249252
self.listener.clone(),
250253
Protocol::HTTP,
251254
http.context.id,
252255
http.context.session_address,
253-
ws_context,
256+
websocket_context,
254257
);
255258

256259
pipe.frontend_readiness.event = http.frontend_readiness.event;
257260
pipe.backend_readiness.event = http.backend_readiness.event;
258-
pipe.set_back_token(back_token);
261+
pipe.set_back_token(origin.token);
259262

260263
gauge_add!("protocol.http", -1);
261264
gauge_add!("protocol.ws", 1);

lib/src/https.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -329,11 +329,14 @@ impl HttpsSession {
329329
}
330330
}
331331

332-
fn upgrade_http(&self, http: Http<FrontRustls, HttpsListener>) -> Option<HttpsStateMachine> {
332+
fn upgrade_http(
333+
&self,
334+
mut http: Http<FrontRustls, HttpsListener>,
335+
) -> Option<HttpsStateMachine> {
333336
debug!("https switching to wss");
334337
let front_token = self.frontend_token;
335-
let back_token = match http.backend_token {
336-
Some(back_token) => back_token,
338+
let origin = match http.origin.take() {
339+
Some(origin) => origin,
337340
None => {
338341
warn!(
339342
"Could not upgrade https request on cluster '{:?}' ({:?}) using backend '{:?}' into secure websocket for request '{}'",
@@ -343,7 +346,7 @@ impl HttpsSession {
343346
}
344347
};
345348

346-
let ws_context = http.websocket_context();
349+
let websocket_context = http.websocket_context();
347350
let mut container_frontend_timeout = http.container_frontend_timeout;
348351
let mut container_backend_timeout = http.container_backend_timeout;
349352
container_frontend_timeout.reset();
@@ -357,25 +360,25 @@ impl HttpsSession {
357360

358361
let mut pipe = Pipe::new(
359362
backend_buffer,
360-
http.context.backend_id,
361-
http.backend_socket,
362-
http.backend,
363+
Some(origin.backend_id),
364+
Some(origin.socket),
365+
Some(origin.backend),
363366
Some(container_backend_timeout),
364367
Some(container_frontend_timeout),
365368
http.context.cluster_id,
366369
http.request_stream.storage.buffer,
367370
front_token,
368371
http.frontend_socket,
369372
self.listener.clone(),
370-
Protocol::HTTP,
373+
Protocol::HTTPS,
371374
http.context.id,
372375
http.context.session_address,
373-
ws_context,
376+
websocket_context,
374377
);
375378

376379
pipe.frontend_readiness.event = http.frontend_readiness.event;
377380
pipe.backend_readiness.event = http.backend_readiness.event;
378-
pipe.set_back_token(back_token);
381+
pipe.set_back_token(origin.token);
379382

380383
gauge_add!("protocol.https", -1);
381384
gauge_add!("protocol.wss", 1);

lib/src/lib.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -953,7 +953,6 @@ pub struct SessionMetrics {
953953
pub service_start: Option<Instant>,
954954
pub wait_start: Instant,
955955

956-
pub backend_id: Option<String>,
957956
pub backend_start: Option<Instant>,
958957
pub backend_connected: Option<Instant>,
959958
pub backend_stop: Option<Instant>,
@@ -971,7 +970,6 @@ impl SessionMetrics {
971970
bout: 0,
972971
service_start: None,
973972
wait_start: Instant::now(),
974-
backend_id: None,
975973
backend_start: None,
976974
backend_connected: None,
977975
backend_stop: None,
@@ -1072,7 +1070,7 @@ impl SessionMetrics {
10721070
time!("request_time", request_time.as_millis());
10731071
time!("service_time", service_time.as_millis());
10741072

1075-
if let Some(backend_id) = self.backend_id.as_ref() {
1073+
if let Some(backend_id) = context.backend_id {
10761074
if let Some(backend_response_time) = self.backend_response_time() {
10771075
record_backend_metrics!(
10781076
context.cluster_id.as_str_or("-"),

0 commit comments

Comments
 (0)