Skip to content

Commit

Permalink
fix deadlock and add retry to LE thread rather than just breaking the…
Browse files Browse the repository at this point in the history 
… loop
  • Loading branch information
@OlofBlomqvist
OlofBlomqvist committed Nov 20, 2024
1 parent f7f6916 commit 65ab545
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 95 deletions.
27 changes: 17 additions & 10 deletions src/api/controllers/settings.rs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use std::time::Duration;

use crate::configuration::OddBoxConfiguration;

use super::*;

use utoipa::ToSchema;
use crate::configuration::OddBoxConfiguration;


#[derive(Debug,Serialize,Deserialize,Clone,ToSchema)]
Expand Down Expand Up @@ -157,6 +158,7 @@ pub async fn get_settings_handler(
(status = 500, description = "When something goes wrong", body = String),
)
)]

pub async fn set_settings_handler(
axum::extract::State(global_state): axum::extract::State<Arc<GlobalState>>,
Json(new_settings): Json<SaveGlobalConfig>
Expand All @@ -166,6 +168,8 @@ pub async fn set_settings_handler(
// then we write it to disk. the disk update will be picked up by the reload function
// which will then apply the new settings.
let mut config = { global_state.config.read().await.clone() };
let old_config = config.clone();

let nlea = if new_settings.lets_encrypt_account_email.len() > 0 {
Some(new_settings.lets_encrypt_account_email.clone())
} else {
Expand Down Expand Up @@ -207,22 +211,25 @@ pub async fn set_settings_handler(
} else {
config.root_dir = Some(new_settings.root_dir.clone());
}

if config.eq(&old_config) {
tracing::warn!("Request to update settings thru api changed nothing.");
return Ok::<(),(StatusCode,String)>(())
}

config.write_to_disk().map_err(|e|(StatusCode::BAD_REQUEST,format!("{}",e.to_string())))?;



let mut i = 0;
tracing::info!("Configuration updated and written to disk. Waiting for reload to complete.");

loop {
if i > 1000 { // 10 seconds
return Err((StatusCode::INTERNAL_SERVER_ERROR,"Failed to update global settings".to_string()));
}
{
let active_config = global_state.config.read().await;
if active_config.internal_version > config.internal_version {
break;
}
while i < 1000 {
let active_config = global_state.config.read().await;
if active_config.internal_version > old_config.internal_version {
break;
}
drop(active_config);
i+=1;
tokio::time::sleep(Duration::from_millis(10)).await;
}
Expand Down
9 changes: 5 additions & 4 deletions src/api/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,8 @@ impl OddBoxAPI {
next: axum::middleware::Next,
state: Arc<crate::global_state::GlobalState>
) -> Result<Response, axum::http::StatusCode> {
let guard = state.config.read().await;

let path = req.uri().path();

let path = req.uri().path();
if req.method() == hyper::Method::GET {
if path.starts_with("/api-docs/") {
return Ok(next.run(req).await);
Expand All @@ -301,7 +299,10 @@ impl OddBoxAPI {
return Ok(next.run(req).await);
}
}
if let Some(pwd) = &guard.odd_box_password {

let possibly_password = { state.config.read().await.odd_box_password.clone() };

if let Some(pwd) = &possibly_password {
match req.headers().get("Authorization") {
Some(value) if value == pwd => {
Ok(next.run(req).await)
Expand Down
6 changes: 3 additions & 3 deletions src/http_proxy/websockets.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@ pub async fn handle_ws(req:Request<IncomingBody>,service:ReverseProxyService,ws:

tracing::trace!("Handling websocket request: {req_host_name:?} --> {req_path}");

let read_guard = service.state.config.read().await;
let cfg = { service.state.config.read().await.clone() };


let target = {

if let Some(proc) = read_guard.hosted_process.iter().flatten().find(|p| {
if let Some(proc) = cfg.hosted_process.iter().flatten().find(|p| {
req_host_name == p.host_name
|| p.capture_subdomains.unwrap_or_default() && req_host_name.ends_with(&format!(".{}",p.host_name))
}) {
crate::http_proxy::utils::Target::Proc(proc.clone())
} else if let Some(remsite) = read_guard.remote_target.iter().flatten().find(|x| {
} else if let Some(remsite) = cfg.remote_target.iter().flatten().find(|x| {
req_host_name == x.host_name
|| x.capture_subdomains.unwrap_or_default() && req_host_name.ends_with(&format!(".{}",x.host_name))
}) {
Expand Down
155 changes: 82 additions & 73 deletions src/letsencrypt.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -707,97 +707,106 @@ pub async fn bg_worker_for_lets_encrypt_certs(state: Arc<GlobalState>) {
// NOTE 2: We generate these certificates in a loop and not OTF. This is to avoid concurrent requests to lets-encrypt.
loop {

tokio::time::sleep(Duration::from_secs(5)).await;
{
let mail = {
if let Some(e) = state.config.read().await.lets_encrypt_account_email.clone() {
e
} else {

crate::BG_WORKER_THREAD_MAP.insert("Lets Encrypt".into(), BgTaskInfo {
liveness_ptr: Arc::downgrade(&liveness_token),
status: format!("Disabled. lets_encrypt_account_email not set.")
});
continue;

}

let state_guard = state.config.read().await;
if state_guard.lets_encrypt_account_email.is_none() {
crate::BG_WORKER_THREAD_MAP.insert("Lets Encrypt".into(), BgTaskInfo {
liveness_ptr: Arc::downgrade(&liveness_token),
status: format!("Disabled. lets_encrypt_account_email not set.")
});
drop(state_guard);
tokio::time::sleep(Duration::from_secs(10)).await;
continue;
}

let mut lem_guard = state.cert_resolver.lets_encrypt_manager.write().await;
if lem_guard.is_none() {
let state_guard = state.config.read().await;
lem_guard.replace(
LECertManager::new(state_guard.lets_encrypt_account_email.as_ref().unwrap()).await.unwrap()
);
}
drop(lem_guard);
};

let mut lem_guard = state.cert_resolver.lets_encrypt_manager.write().await;
if lem_guard.is_none() {
match LECertManager::new(&mail).await {
Ok(v) => lem_guard.replace(v) ,
Err(e) => {
tracing::warn!("Failed to create lets-encrypt manager: {e:?}");
continue
}
};
}
drop(lem_guard);

let active_challenges_count = crate::letsencrypt::DOMAIN_TO_CHALLENGE_TOKEN_MAP.len();
let active_challenges_count = crate::letsencrypt::DOMAIN_TO_CHALLENGE_TOKEN_MAP.len();



// TODO: should filter out local sites so we do not try to create certs for things like test.localhost or test.localtest.me etc.
// but instead write a warning about it.
let mut all_sites_with_lets_encrypt_enabled =
state_guard.remote_target
.iter()
.flatten()
.filter(|x|x.enable_lets_encrypt.unwrap_or(false)).map(|x|x.host_name.clone())
.chain(
state_guard.hosted_process
.iter()
.flatten()
.filter(|x|x.enable_lets_encrypt.unwrap_or(false)).map(|x|x.host_name.clone())
).chain(
state_guard.dir_server


// TODO: should filter out local sites so we do not try to create certs for things like test.localhost or test.localtest.me etc.
// but instead write a warning about it.
let state_guard = state.config.read().await;
let mut all_sites_with_lets_encrypt_enabled =
state_guard.remote_target
.iter()
.flatten()
.filter(|x|x.enable_lets_encrypt.unwrap_or(false)).map(|x|x.host_name.clone())
).collect::<Vec<String>>();

if let Some(ourl) = state_guard.odd_box_url.as_ref() {
all_sites_with_lets_encrypt_enabled.push(ourl.clone());
}

drop(state_guard);
.chain(
state_guard.hosted_process
.iter()
.flatten()
.filter(|x|x.enable_lets_encrypt.unwrap_or(false)).map(|x|x.host_name.clone())
).chain(
state_guard.dir_server
.iter()
.flatten()
.filter(|x|x.enable_lets_encrypt.unwrap_or(false)).map(|x|x.host_name.clone())
).collect::<Vec<String>>();

if let Some(ourl) = state_guard.odd_box_url.as_ref() {
all_sites_with_lets_encrypt_enabled.push(ourl.clone());
}


drop(state_guard);

all_sites_with_lets_encrypt_enabled.sort();
all_sites_with_lets_encrypt_enabled.dedup();


let guard = state.cert_resolver.lets_encrypt_manager.read().await;
all_sites_with_lets_encrypt_enabled.sort();
all_sites_with_lets_encrypt_enabled.dedup();

if let Some(mgr) = guard.as_ref() {
for domain_name in all_sites_with_lets_encrypt_enabled {
let guard = state.cert_resolver.lets_encrypt_manager.read().await;

if let Some(_) = state.cert_resolver.get_lets_encrypt_signed_cert_from_mem_cache(&domain_name) {
tracing::info!("LE CERT IS OK FOR: {}", domain_name);
continue;
}
if let Some(mgr) = guard.as_ref() {
for domain_name in all_sites_with_lets_encrypt_enabled {

match mgr.get_or_create_cert(&domain_name).await.context(format!("generating lets-encrypt cert for site {}",domain_name)) {
Ok(v) => {
state.cert_resolver.add_lets_encrypt_signed_cert_to_mem_cache(&domain_name, v);
generated_count += 1;
if let Some(_) = state.cert_resolver.get_lets_encrypt_signed_cert_from_mem_cache(&domain_name) {
tracing::info!("LE CERT IS OK FOR: {}", domain_name);
continue;
}
Err(e) => {
tracing::error!("Failed to generate certificate for domain: {}. {e:?}", domain_name);
}
}


match mgr.get_or_create_cert(&domain_name).await.context(format!("generating lets-encrypt cert for site {}",domain_name)) {
Ok(v) => {
state.cert_resolver.add_lets_encrypt_signed_cert_to_mem_cache(&domain_name, v);
generated_count += 1;
}
Err(e) => {
tracing::error!("Failed to generate certificate for domain: {}. {e:?}", domain_name);
}
}


}
} else {
tracing::error!("LE Manager not available.. will retry in 10 seconds.");
}
} else {
tracing::error!("LE Manager not available.. will retry in 10 seconds.");
}




crate::BG_WORKER_THREAD_MAP.insert("Lets Encrypt".into(), BgTaskInfo {
liveness_ptr: Arc::downgrade(&liveness_token),
status: format!("Generated: {generated_count} - Pending: {active_challenges_count}.")
}); // we dont need to clean this up if we exit, there is a cleanup task that will do it.


tokio::time::sleep(Duration::from_secs(320)).await;
crate::BG_WORKER_THREAD_MAP.insert("Lets Encrypt".into(), BgTaskInfo {
liveness_ptr: Arc::downgrade(&liveness_token),
status: format!("Generated: {generated_count} - Pending: {active_challenges_count}.")
}); // we dont need to clean this up if we exit, there is a cleanup task that will do it.

drop(guard);
}
tokio::time::sleep(Duration::from_secs(315)).await;
}

}
5 changes: 0 additions & 5 deletions src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -675,18 +675,13 @@ async fn main() -> anyhow::Result<()> {
match self_update::current_is_latest().await {
Err(e) => {
tracing::warn!("It was not possible to retrieve information regarding the latest available version of odd-box: {e:?}");
std::thread::sleep(Duration::from_secs(5));
},
Ok(Some(v)) => {
tracing::info!("There is a newer version of odd-box available - please consider upgrading to {v:?}. For unmanaged installations you can run 'odd-box --update' otherwise see your package manager for upgrade instructions.");
std::thread::sleep(Duration::from_secs(3));
},
Ok(None) => {
tracing::info!("You are running the latest version of odd-box :D");
std::thread::sleep(Duration::from_secs(1));
}


}


Expand Down

0 comments on commit 65ab545

Please sign in to comment.