From ee2430f9c85ee846314ebb09b8cc981267ba427b Mon Sep 17 00:00:00 2001 From: Emmanuel Bosquet Date: Thu, 4 Apr 2024 13:44:03 +0200 Subject: [PATCH] make Http[s]ListenerConfig::http_answers optional --- Cargo.lock | 76 +++++++++++++---------------- command/src/command.proto | 4 +- command/src/config.rs | 4 +- command/src/logging/logs.rs | 3 +- command/src/proto/display.rs | 60 ++++++++++++----------- command/src/state.rs | 14 ++---- e2e/src/tests/tests.rs | 24 ++++++--- lib/src/backends.rs | 4 +- lib/src/http.rs | 2 +- lib/src/https.rs | 2 +- lib/src/metrics/local_drain.rs | 10 +--- lib/src/protocol/kawa_h1/answers.rs | 72 ++++++++++++++++++++++----- 12 files changed, 158 insertions(+), 117 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2d7904e84..d0018a648 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -110,7 +110,7 @@ checksum = "7378575ff571966e99a744addeff0bff98b8ada0dedf1956d59e634db95eaac1" dependencies = [ "proc-macro2", "quote", - "syn 2.0.53", + "syn 2.0.52", "synstructure", ] @@ -122,7 +122,7 @@ checksum = "7b18050c2cd6fe86c3a76584ef5e0baf286d038cda203eb6223df2cc413565f7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.53", + "syn 2.0.52", ] [[package]] @@ -184,7 +184,7 @@ version = "0.69.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a00dc851838a2120612785d195287475a3ac45514741da670b735818822129a0" dependencies = [ - "bitflags 2.5.0", + "bitflags 2.4.2", "cexpr", "clang-sys", "itertools 0.12.1", @@ -197,7 +197,7 @@ dependencies = [ "regex", "rustc-hash", "shlex", - "syn 2.0.53", + "syn 2.0.52", "which", ] @@ -209,9 +209,9 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bitflags" -version = "2.5.0" +version = "2.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf4b9d6a944f767f8e5e0db018570623c85f3d925ac718db4e06d0187adb21c1" +checksum = "ed570934406eb16438a4e976b1b4500774099c13b8cb96eec99f620f05090ddf" [[package]] name = "block-buffer" @@ -280,9 +280,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.3" +version = "4.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "949626d00e063efc93b6dca932419ceb5432f99769911c0b995f7e884c778813" +checksum = "b230ab84b0ffdf890d5a10abdbc8b83ae1c4918275daea1ab8801f71536b2651" dependencies = [ "clap_builder", "clap_derive", @@ -302,14 +302,14 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.5.3" +version = "4.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "90239a040c80f5e14809ca132ddc4176ab33d5e17e49691793296e3fcb34d72f" +checksum = "307bc0538d5f0f83b8248db3087aa92fe504e4691294d0c96c0eabc33f47ba47" dependencies = [ - "heck 0.5.0", + "heck", "proc-macro2", "quote", - "syn 2.0.53", + "syn 2.0.52", ] [[package]] @@ -463,7 +463,7 @@ checksum = "487585f4d0c6655fe74905e2504d8ad6908e4db67f744eb140876906c2f3175d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.53", + "syn 2.0.52", ] [[package]] @@ -600,7 +600,7 @@ checksum = "87750cf4b7a4c0625b1529e4c543c2182106e4dedc60a2a6455e00d212c489ac" dependencies = [ "proc-macro2", "quote", - "syn 2.0.53", + "syn 2.0.52", ] [[package]] @@ -692,12 +692,6 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" -[[package]] -name = "heck" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" - [[package]] name = "hermit-abi" version = "0.3.9" @@ -919,7 +913,7 @@ version = "0.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "85c833ca1e66078851dba29046874e38f08b2c883700aa29a03ddd3b23814ee8" dependencies = [ - "bitflags 2.5.0", + "bitflags 2.4.2", "libc", "redox_syscall", ] @@ -930,7 +924,7 @@ version = "0.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3af92c55d7d839293953fcd0fda5ecfe93297cfde6ffbdec13b41d99c0ba6607" dependencies = [ - "bitflags 2.5.0", + "bitflags 2.4.2", "libc", "redox_syscall", ] @@ -1026,7 +1020,7 @@ version = "0.28.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab2156c4fce2f8df6c499cc1c763e4394b7482525bf2a9701c9d79d215f519e4" dependencies = [ - "bitflags 2.5.0", + "bitflags 2.4.2", "cfg-if", "cfg_aliases", "libc", @@ -1230,7 +1224,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a41cf62165e97c7f814d2221421dbb9afcbcdb0a88068e5ea206e19951c2cbb5" dependencies = [ "proc-macro2", - "syn 2.0.53", + "syn 2.0.52", ] [[package]] @@ -1248,9 +1242,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.79" +version = "1.0.78" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e835ff2298f5721608eb1a980ecaee1aef2c132bf95ecc026a11b7bf3c01c02e" +checksum = "e2422ad645d89c99f8f3e6b88a9fdeca7fabeac836b1002371c4367c8f984aae" dependencies = [ "unicode-ident", ] @@ -1272,7 +1266,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c55e02e35260070b6f716a2423c2ff1c3bb1642ddca6f99e1f26d06268a0e2d2" dependencies = [ "bytes", - "heck 0.4.1", + "heck", "itertools 0.11.0", "log 0.4.21", "multimap", @@ -1282,7 +1276,7 @@ dependencies = [ "prost", "prost-types", "regex", - "syn 2.0.53", + "syn 2.0.52", "tempfile", "which", ] @@ -1297,7 +1291,7 @@ dependencies = [ "itertools 0.11.0", "proc-macro2", "quote", - "syn 2.0.53", + "syn 2.0.52", ] [[package]] @@ -1456,7 +1450,7 @@ version = "0.38.31" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ea3e1a662af26cd7a3ba09c0297a31af215563ecf42817c98df621387f4e949" dependencies = [ - "bitflags 2.5.0", + "bitflags 2.4.2", "errno", "libc", "linux-raw-sys", @@ -1477,9 +1471,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.2" +version = "0.23.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5dfbdb5ddfafe3040e01fe9dced711e27b5336ac97d4a9b2089f0066a04b5846" +checksum = "8c4d6d8ad9f2492485e13453acbb291dd08f64441b6609c491f1c2cd2c6b4fe1" dependencies = [ "aws-lc-rs", "log 0.4.21", @@ -1585,7 +1579,7 @@ checksum = "7eb0b34b42edc17f6b7cac84a52a1c5f0e1bb2227e997ca9011ea3dd34e8610b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.53", + "syn 2.0.52", ] [[package]] @@ -1630,7 +1624,7 @@ checksum = "b93fb4adc70021ac1b47f7d45e8cc4169baaa7ea58483bc5b721d19a26202212" dependencies = [ "proc-macro2", "quote", - "syn 2.0.53", + "syn 2.0.52", ] [[package]] @@ -1763,7 +1757,7 @@ dependencies = [ "quickcheck", "rand", "regex", - "rustls 0.23.2", + "rustls 0.23.4", "rustls-pemfile", "rusty_ulid", "serial_test", @@ -1807,9 +1801,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.53" +version = "2.0.52" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7383cd0e49fff4b6b90ca5670bfd3e9d6a733b3f90c686605aa7eec8c4996032" +checksum = "b699d15b36d1f02c3e7c69f8ffef53de37aefae075d8488d4ba1a7788d574a07" dependencies = [ "proc-macro2", "quote", @@ -1824,7 +1818,7 @@ checksum = "c8af7666ab7b6390ab78131fb5b0fce11d6b7a6951602017c35fa82800708971" dependencies = [ "proc-macro2", "quote", - "syn 2.0.53", + "syn 2.0.52", ] [[package]] @@ -1879,7 +1873,7 @@ checksum = "c61f3ba182994efc43764a46c018c347bc492c79f024e705f46567b418f6d4f7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.53", + "syn 2.0.52", ] [[package]] @@ -1988,9 +1982,9 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.22.8" +version = "0.22.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c12219811e0c1ba077867254e5ad62ee2c9c190b0d957110750ac0cda1ae96cd" +checksum = "8e40bb779c5187258fd7aad0eb68cb8706a0a81fa712fbea808ab43c4b8374c4" dependencies = [ "indexmap", "serde", diff --git a/command/src/command.proto b/command/src/command.proto index d57bd6092..2487e5ef4 100644 --- a/command/src/command.proto +++ b/command/src/command.proto @@ -129,7 +129,7 @@ message HttpListenerConfig { required uint32 request_timeout = 10 [default = 10]; // wether the listener is actively listening on its socket required bool active = 11 [default = false]; - required CustomHttpAnswers http_answers = 12; + optional CustomHttpAnswers http_answers = 12; } // details of an HTTPS listener @@ -161,7 +161,7 @@ message HttpsListenerConfig { // The tickets allow the client to resume a session. This protects the client // agains session tracking. Defaults to 4. required uint64 send_tls13_tickets = 20; - required CustomHttpAnswers http_answers = 21; + optional CustomHttpAnswers http_answers = 21; } // details of an TCP listener diff --git a/command/src/config.rs b/command/src/config.rs index 70016e54c..346f418b8 100644 --- a/command/src/config.rs +++ b/command/src/config.rs @@ -427,7 +427,7 @@ impl ListenerBuilder { } /// Get the custom HTTP answers from the file system using the provided paths - fn get_http_answers(&self) -> Result { + fn get_http_answers(&self) -> Result, ConfigError> { let http_answers = CustomHttpAnswers { answer_301: read_http_answer_file(&self.answer_301)?, answer_400: read_http_answer_file(&self.answer_400)?, @@ -440,7 +440,7 @@ impl ListenerBuilder { answer_504: read_http_answer_file(&self.answer_504)?, answer_507: read_http_answer_file(&self.answer_507)?, }; - Ok(http_answers) + Ok(Some(http_answers)) } /// Assign the timeouts of the config to this listener, only if timeouts did not exist diff --git a/command/src/logging/logs.rs b/command/src/logging/logs.rs index 074166206..a63fcaeb6 100644 --- a/command/src/logging/logs.rs +++ b/command/src/logging/logs.rs @@ -1,7 +1,6 @@ use std::{ cell::RefCell, - cmp, - env, + cmp, env, fmt::Arguments, fs::{File, OpenOptions}, io::{stdout, Error as IoError, ErrorKind as IoErrorKind, Stdout, Write}, diff --git a/command/src/proto/display.rs b/command/src/proto/display.rs index b5ffc3f1f..4d91d7777 100644 --- a/command/src/proto/display.rs +++ b/command/src/proto/display.rs @@ -923,7 +923,7 @@ impl Display for HttpListenerConfig { table.set_format(*prettytable::format::consts::FORMAT_BOX_CHARS); table.add_row(row!["socket address", format!("{:?}", self.address)]); table.add_row(row!["public address", format!("{:?}", self.public_address),]); - for http_answer_row in self.http_answers.to_rows() { + for http_answer_row in CustomHttpAnswers::to_rows(&self.http_answers) { table.add_row(http_answer_row); } table.add_row(row!["expect proxy", self.expect_proxy]); @@ -948,7 +948,7 @@ impl Display for HttpsListenerConfig { table.add_row(row!["socket address", format!("{:?}", self.address)]); table.add_row(row!["public address", format!("{:?}", self.public_address)]); - for http_answer_row in self.http_answers.to_rows() { + for http_answer_row in CustomHttpAnswers::to_rows(&self.http_answers) { table.add_row(http_answer_row); } table.add_row(row!["versions", tls_versions]); @@ -972,34 +972,36 @@ impl Display for HttpsListenerConfig { } impl CustomHttpAnswers { - fn to_rows(&self) -> Vec { + fn to_rows(option: &Option) -> Vec { let mut rows = Vec::new(); - if let Some(a) = &self.answer_301 { - rows.push(row!("301", a)); - } - if let Some(a) = &self.answer_400 { - rows.push(row!("400", a)); - } - if let Some(a) = &self.answer_404 { - rows.push(row!("404", a)); - } - if let Some(a) = &self.answer_408 { - rows.push(row!("408", a)); - } - if let Some(a) = &self.answer_413 { - rows.push(row!("413", a)); - } - if let Some(a) = &self.answer_502 { - rows.push(row!("502", a)); - } - if let Some(a) = &self.answer_503 { - rows.push(row!("503", a)); - } - if let Some(a) = &self.answer_504 { - rows.push(row!("504", a)); - } - if let Some(a) = &self.answer_507 { - rows.push(row!("507", a)); + if let Some(answers) = option { + if let Some(a) = &answers.answer_301 { + rows.push(row!("301", a)); + } + if let Some(a) = &answers.answer_400 { + rows.push(row!("400", a)); + } + if let Some(a) = &answers.answer_404 { + rows.push(row!("404", a)); + } + if let Some(a) = &answers.answer_408 { + rows.push(row!("408", a)); + } + if let Some(a) = &answers.answer_413 { + rows.push(row!("413", a)); + } + if let Some(a) = &answers.answer_502 { + rows.push(row!("502", a)); + } + if let Some(a) = &answers.answer_503 { + rows.push(row!("503", a)); + } + if let Some(a) = &answers.answer_504 { + rows.push(row!("504", a)); + } + if let Some(a) = &answers.answer_507 { + rows.push(row!("507", a)); + } } rows } diff --git a/command/src/state.rs b/command/src/state.rs index 623fa0078..e77176f66 100644 --- a/command/src/state.rs +++ b/command/src/state.rs @@ -461,10 +461,7 @@ impl ConfigState { } fn add_tcp_frontend(&mut self, front: &RequestTcpFrontend) -> Result<(), StateError> { - let tcp_frontends = self - .tcp_fronts - .entry(front.cluster_id.clone()) - .or_default(); + let tcp_frontends = self.tcp_fronts.entry(front.cluster_id.clone()).or_default(); let tcp_frontend = TcpFrontend { cluster_id: front.cluster_id.clone(), @@ -511,10 +508,7 @@ impl ConfigState { load_balancing_parameters: add_backend.load_balancing_parameters.clone(), backup: add_backend.backup, }; - let backends = self - .backends - .entry(backend.cluster_id.clone()) - .or_default(); + let backends = self.backends.entry(backend.cluster_id.clone()).or_default(); // we might be modifying the sticky id or load balancing parameters backends.retain(|b| b.backend_id != backend.backend_id || b.address != backend.address); @@ -2000,10 +1994,10 @@ mod tests { #[test] fn listener_diff() { let mut state: ConfigState = Default::default(); - let custom_http_answers = CustomHttpAnswers { + let custom_http_answers = Some(CustomHttpAnswers { answer_404: Some("test".to_string()), ..Default::default() - }; + }); state .dispatch( &RequestType::AddTcpListener(TcpListenerConfig { diff --git a/e2e/src/tests/tests.rs b/e2e/src/tests/tests.rs index db86950c2..26ef0818c 100644 --- a/e2e/src/tests/tests.rs +++ b/e2e/src/tests/tests.rs @@ -10,7 +10,7 @@ use sozu_command_lib::{ logging::setup_default_logging, proto::command::{ request::RequestType, ActivateListener, AddCertificate, CertificateAndKey, Cluster, - ListenerType, RemoveBackend, RequestHttpFrontend, SocketAddress, + CustomHttpAnswers, ListenerType, RemoveBackend, RequestHttpFrontend, SocketAddress, }, scm_socket::Listeners, state::ConfigState, @@ -641,10 +641,15 @@ fn try_http_behaviors() -> State { let mut http_config = ListenerBuilder::new_http(front_address.into()) .to_http(None) .unwrap(); - http_config.http_answers.answer_400 = Some(immutable_answer(400)); - http_config.http_answers.answer_404 = Some(immutable_answer(404)); - http_config.http_answers.answer_502 = Some(immutable_answer(502)); - http_config.http_answers.answer_503 = Some(immutable_answer(503)); + let http_answers = CustomHttpAnswers { + answer_400: Some(immutable_answer(400)), + answer_404: Some(immutable_answer(404)), + answer_502: Some(immutable_answer(502)), + answer_503: Some(immutable_answer(503)), + ..Default::default() + }; + http_config.http_answers = Some(http_answers); + worker.send_proxy_request_type(RequestType::AddHttpListener(http_config)); worker.send_proxy_request_type(RequestType::ActivateListener(ActivateListener { address: front_address.into(), @@ -944,8 +949,13 @@ fn try_https_redirect() -> State { .to_http(None) .unwrap(); let answer_301_prefix = "HTTP/1.1 301 Moved Permanently\r\nLocation: "; - http_config.http_answers.answer_301 = - Some(format!("{answer_301_prefix}%REDIRECT_LOCATION\r\n\r\n")); + + let http_answers = CustomHttpAnswers { + answer_301: Some(format!("{answer_301_prefix}%REDIRECT_LOCATION\r\n\r\n")), + ..Default::default() + }; + http_config.http_answers = Some(http_answers); + worker.send_proxy_request_type(RequestType::AddHttpListener(http_config)); worker.send_proxy_request_type(RequestType::ActivateListener(ActivateListener { address: front_address.into(), diff --git a/lib/src/backends.rs b/lib/src/backends.rs index dcc0678cb..1ee7f04a1 100644 --- a/lib/src/backends.rs +++ b/lib/src/backends.rs @@ -349,9 +349,7 @@ impl BackendMap { } pub fn get_or_create_backend_list_for_cluster(&mut self, cluster_id: &str) -> &mut BackendList { - self.backends - .entry(cluster_id.to_string()) - .or_default() + self.backends.entry(cluster_id.to_string()).or_default() } } diff --git a/lib/src/http.rs b/lib/src/http.rs index 566cdf89a..671a2ecb5 100644 --- a/lib/src/http.rs +++ b/lib/src/http.rs @@ -1377,7 +1377,7 @@ mod tests { address: address.into(), fronts, answers: Rc::new(RefCell::new( - HttpAnswers::new(&CustomHttpAnswers::default()).unwrap(), + HttpAnswers::new(&Some(CustomHttpAnswers::default())).unwrap(), )), config: default_config, token: Token(0), diff --git a/lib/src/https.rs b/lib/src/https.rs index 6db23fb06..dfe20d17f 100644 --- a/lib/src/https.rs +++ b/lib/src/https.rs @@ -1599,7 +1599,7 @@ mod tests { rustls_details, resolver, answers: Rc::new(RefCell::new( - HttpAnswers::new(&CustomHttpAnswers::default()).unwrap(), + HttpAnswers::new(&Some(CustomHttpAnswers::default())).unwrap(), )), config: default_config, token: Token(0), diff --git a/lib/src/metrics/local_drain.rs b/lib/src/metrics/local_drain.rs index 061bd8608..2386c6867 100644 --- a/lib/src/metrics/local_drain.rs +++ b/lib/src/metrics/local_drain.rs @@ -125,10 +125,7 @@ pub struct LocalClusterMetrics { } impl LocalClusterMetrics { - fn to_filtered_metrics( - &self, - metric_names: &[String], - ) -> Result { + fn to_filtered_metrics(&self, metric_names: &[String]) -> Result { let cluster = self .cluster .iter() @@ -179,10 +176,7 @@ pub struct LocalBackendMetrics { } impl LocalBackendMetrics { - fn to_filtered_metrics( - &self, - metric_names: &[String], - ) -> Result { + fn to_filtered_metrics(&self, metric_names: &[String]) -> Result { let filtered_backend_metrics = self .metrics .iter() diff --git a/lib/src/protocol/kawa_h1/answers.rs b/lib/src/protocol/kawa_h1/answers.rs index 297d911ee..6c4ab2056 100644 --- a/lib/src/protocol/kawa_h1/answers.rs +++ b/lib/src/protocol/kawa_h1/answers.rs @@ -704,19 +704,69 @@ impl HttpAnswers { .map_err(|e| (status, e)) } - pub fn new(conf: &CustomHttpAnswers) -> Result { + pub fn new(conf: &Option) -> Result { Ok(HttpAnswers { listener_answers: ListenerAnswers { - answer_301: Self::template(301, conf.answer_301.clone().unwrap_or(default_301()))?, - answer_400: Self::template(400, conf.answer_400.clone().unwrap_or(default_400()))?, - answer_401: Self::template(401, conf.answer_401.clone().unwrap_or(default_401()))?, - answer_404: Self::template(404, conf.answer_404.clone().unwrap_or(default_404()))?, - answer_408: Self::template(408, conf.answer_408.clone().unwrap_or(default_408()))?, - answer_413: Self::template(413, conf.answer_413.clone().unwrap_or(default_413()))?, - answer_502: Self::template(502, conf.answer_502.clone().unwrap_or(default_502()))?, - answer_503: Self::template(503, conf.answer_503.clone().unwrap_or(default_503()))?, - answer_504: Self::template(504, conf.answer_504.clone().unwrap_or(default_504()))?, - answer_507: Self::template(507, conf.answer_507.clone().unwrap_or(default_507()))?, + answer_301: Self::template( + 301, + conf.as_ref() + .and_then(|c| c.answer_301.clone()) + .unwrap_or(default_301()), + )?, + answer_400: Self::template( + 400, + conf.as_ref() + .and_then(|c| c.answer_400.clone()) + .unwrap_or(default_400()), + )?, + answer_401: Self::template( + 401, + conf.as_ref() + .and_then(|c| c.answer_401.clone()) + .unwrap_or(default_401()), + )?, + answer_404: Self::template( + 404, + conf.as_ref() + .and_then(|c| c.answer_404.clone()) + .unwrap_or(default_404()), + )?, + answer_408: Self::template( + 408, + conf.as_ref() + .and_then(|c| c.answer_408.clone()) + .unwrap_or(default_408()), + )?, + answer_413: Self::template( + 413, + conf.as_ref() + .and_then(|c| c.answer_413.clone()) + .unwrap_or(default_413()), + )?, + answer_502: Self::template( + 502, + conf.as_ref() + .and_then(|c| c.answer_502.clone()) + .unwrap_or(default_502()), + )?, + answer_503: Self::template( + 503, + conf.as_ref() + .and_then(|c| c.answer_503.clone()) + .unwrap_or(default_503()), + )?, + answer_504: Self::template( + 504, + conf.as_ref() + .and_then(|c| c.answer_504.clone()) + .unwrap_or(default_504()), + )?, + answer_507: Self::template( + 507, + conf.as_ref() + .and_then(|c| c.answer_507.clone()) + .unwrap_or(default_507()), + )?, }, cluster_custom_answers: HashMap::new(), })